diff --git a/cmd.go b/cmd.go index 6ee5554..44a97ab 100644 --- a/cmd.go +++ b/cmd.go @@ -652,10 +652,6 @@ func (w *BtcWallet) newBlockTxOutHandler(result interface{}, e *btcjson.Error) b w.TxStore.dirty = true w.TxStore.Unlock() - if err = w.writeDirtyToDisk(); err != nil { - log.Errorf("cannot sync dirty wallet: %v", err) - } - // Add to UtxoStore if unspent. if !spent { // First, iterate through all stored utxos. If an unconfirmed utxo @@ -677,9 +673,6 @@ func (w *BtcWallet) newBlockTxOutHandler(result interface{}, e *btcjson.Error) b w.UtxoStore.dirty = true w.UtxoStore.Unlock() - if err = w.writeDirtyToDisk(); err != nil { - log.Errorf("cannot sync dirty wallet: %v", err) - } return false } } @@ -701,14 +694,10 @@ func (w *BtcWallet) newBlockTxOutHandler(result interface{}, e *btcjson.Error) b w.UtxoStore.s = append(w.UtxoStore.s, u) w.UtxoStore.dirty = true w.UtxoStore.Unlock() - if err = w.writeDirtyToDisk(); err != nil { - log.Errorf("cannot sync dirty wallet: %v", err) - } - confirmed := w.CalculateBalance(1) - unconfirmed := w.CalculateBalance(0) - confirmed - NotifyWalletBalance(frontendNotificationMaster, w.name, confirmed) - NotifyWalletBalanceUnconfirmed(frontendNotificationMaster, w.name, unconfirmed) + // If this notification came from mempool (TODO: currently + // unimplemented) notify the new unconfirmed balance immediately. + // Otherwise, wait until the blockconnection notifiation is processed. } // Never remove this handler. diff --git a/cmdmgr.go b/cmdmgr.go index bce41ed..9225a38 100644 --- a/cmdmgr.go +++ b/cmdmgr.go @@ -521,7 +521,8 @@ func handleSendRawTxReply(frontend chan []byte, icmd btcjson.Cmd, log.Errorf("cannot sync dirty wallet: %v", err) } - // Notify all frontends of new account balances. + // Notify all frontends of account's new unconfirmed and + // confirmed balance. confirmed := w.CalculateBalance(1) unconfirmed := w.CalculateBalance(0) - confirmed NotifyWalletBalance(frontendNotificationMaster, w.name, confirmed) diff --git a/disksync.go b/disksync.go index 6f5639c..85d9c77 100644 --- a/disksync.go +++ b/disksync.go @@ -74,27 +74,32 @@ func (w *BtcWallet) writeDirtyToDisk() error { txfilepath := filepath.Join(wdir, "tx.bin") utxofilepath := filepath.Join(wdir, "utxo.bin") - // Wallet - if w.dirty { - w.mtx.Lock() - defer w.mtx.Unlock() - tmpfilepath := wfilepath + "-" + timeStr + // UTXOs and transactions are synced to disk first. This prevents + // any races from saving a wallet marked to be synced with block N + // and btcwallet closing while the UTXO and Tx files are only synced + // with block N-1. + + // UTXOs + if w.UtxoStore.dirty { + w.UtxoStore.Lock() + defer w.UtxoStore.Unlock() + tmpfilepath := utxofilepath + "-" + timeStr tmpfile, err := os.Create(tmpfilepath) if err != nil { return err } - if _, err = w.WriteTo(tmpfile); err != nil { + if _, err = w.UtxoStore.s.WriteTo(tmpfile); err != nil { return err } tmpfile.Close() // TODO(jrick): this should be atomic on *nix, but is not on // Windows. Use _windows.go to provide atomic renames. - if err = os.Rename(tmpfilepath, wfilepath); err != nil { + if err = os.Rename(tmpfilepath, utxofilepath); err != nil { return err } - w.dirty = false + w.UtxoStore.dirty = false } // Transactions @@ -120,27 +125,27 @@ func (w *BtcWallet) writeDirtyToDisk() error { w.TxStore.dirty = false } - // UTXOs - if w.UtxoStore.dirty { - w.UtxoStore.Lock() - defer w.UtxoStore.Unlock() - tmpfilepath := utxofilepath + "-" + timeStr + // Wallet + if w.dirty { + w.mtx.Lock() + defer w.mtx.Unlock() + tmpfilepath := wfilepath + "-" + timeStr tmpfile, err := os.Create(tmpfilepath) if err != nil { return err } - if _, err = w.UtxoStore.s.WriteTo(tmpfile); err != nil { + if _, err = w.WriteTo(tmpfile); err != nil { return err } tmpfile.Close() // TODO(jrick): this should be atomic on *nix, but is not on // Windows. Use _windows.go to provide atomic renames. - if err = os.Rename(tmpfilepath, utxofilepath); err != nil { + if err = os.Rename(tmpfilepath, wfilepath); err != nil { return err } - w.UtxoStore.dirty = false + w.dirty = false } return nil diff --git a/sockets.go b/sockets.go index 17f1e09..ed50f6f 100644 --- a/sockets.go +++ b/sockets.go @@ -381,10 +381,7 @@ func NtfnBlockConnected(n btcws.Notification) { return } - // btcd notifies btcwallet about transactions first, and then sends - // the block notification. This prevents any races from saving a - // synced-to block before all notifications from the block have been - // processed. + // Update the blockstamp for the newly-connected block. bs := &wallet.BlockStamp{ Height: bcn.Height, Hash: *hash, @@ -392,16 +389,44 @@ func NtfnBlockConnected(n btcws.Notification) { curBlock.Lock() curBlock.BlockStamp = *bs curBlock.Unlock() + + // btcd notifies btcwallet about transactions first, and then sends + // the new block notification. New balance notifications for txs + // in blocks are therefore sent here after all tx notifications + // have arrived. + // + // TODO(jrick): send frontend tx notifications once that's + // implemented. for _, w := range wallets.m { - // We do not write synced info immediatelly out to disk. - // If btcd is performing an IBD, that would result in - // writing out the wallet to disk for each processed block. - // Instead, mark as dirty and let another goroutine process - // the dirty wallet. + // Mark wallet as being synced with the new blockstamp. w.mtx.Lock() w.Wallet.SetSyncedWith(bs) - w.dirty = true w.mtx.Unlock() + + // The UTXO store will be dirty if it was modified + // from a tx notification. + if w.UtxoStore.dirty { + // Notify all frontends of account's new unconfirmed + // and confirmed balance. + confirmed := w.CalculateBalance(1) + unconfirmed := w.CalculateBalance(0) - confirmed + NotifyWalletBalance(frontendNotificationMaster, + w.name, confirmed) + NotifyWalletBalanceUnconfirmed(frontendNotificationMaster, + w.name, unconfirmed) + } + + // The account is intentionaly not immediately synced to disk. + // If btcd is performing an IBD, writing the wallet file for + // each newly-connected block would result in too many + // unnecessary disk writes. The UTXO and transaction stores + // could be written, but in the case of btcwallet closing + // before writing the dirty wallet, both would have to be + // pruned anyways. + // + // Instead, the wallet is queued to be written to disk at the + // next scheduled disk sync. + w.dirty = true dirtyWallets.Lock() dirtyWallets.m[w] = true dirtyWallets.Unlock()