From 4f5baed780ba77a6af6914e62b71ad240906ebea Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 15 Nov 2018 13:58:07 -0800 Subject: [PATCH 1/6] wallet/wallet: prevent always rescanning from birthday block In this commit, we address an issue with the wallet where it would always request a rescan from the birthday block. This is very crucial for older wallets, as it'll potentially go through thousands of blocks. To address this, we'll now only request a rescan from our birthday if we're recovering our wallet from our seed, the birthday block was rolled back, or if we're performing our initial sync. Otherwise, we'll request a rescan from tip. --- wallet/wallet.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/wallet/wallet.go b/wallet/wallet.go index 059dd6e..8455975 100644 --- a/wallet/wallet.go +++ b/wallet/wallet.go @@ -653,8 +653,10 @@ func (w *Wallet) syncWithChain(birthdayStamp *waddrmgr.BlockStamp) error { // If a birthday stamp was found during the initial sync and the // rollback causes us to revert it, update the birthday stamp so that it // points at the new tip. + birthdayRollback := false if birthdayStamp != nil && rollbackStamp.Height <= birthdayStamp.Height { birthdayStamp = &rollbackStamp + birthdayRollback = true log.Debugf("Found new birthday block after rollback: "+ "height=%d, hash=%v", birthdayStamp.Height, @@ -681,7 +683,16 @@ func (w *Wallet) syncWithChain(birthdayStamp *waddrmgr.BlockStamp) error { return err } - return w.rescanWithTarget(addrs, unspent, birthdayStamp) + // If this was our initial sync, we're recovering from our seed, or our + // birthday was rolled back due to a chain reorg, we'll dispatch a + // rescan from our birthday block to ensure we detect all relevant + // on-chain events from this point. + if isInitialSync || isRecovery || birthdayRollback { + return w.rescanWithTarget(addrs, unspent, birthdayStamp) + } + + // Otherwise, we'll rescan from tip. + return w.rescanWithTarget(addrs, unspent, nil) } // defaultScopeManagers fetches the ScopedKeyManagers from the wallet using the From 6568c433fe26f7b42522aff6c3a1ce3eff883bad Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 15 Nov 2018 17:58:41 -0800 Subject: [PATCH 2/6] waddrmgr/db: store birthday block verification status In this commit, we add a new key/value pair to the waddrmgr's sync bucket to store the verification status of the birthday block. This verification status determines whether the wallet has verified the correctness of its birthday block through its sanity check on startup. --- waddrmgr/db.go | 50 ++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 46 insertions(+), 4 deletions(-) diff --git a/waddrmgr/db.go b/waddrmgr/db.go index e9dd41d..a7c41a0 100644 --- a/waddrmgr/db.go +++ b/waddrmgr/db.go @@ -253,10 +253,11 @@ var ( watchingOnlyName = []byte("watchonly") // Sync related key names (sync bucket). - syncedToName = []byte("syncedto") - startBlockName = []byte("startblock") - birthdayName = []byte("birthday") - birthdayBlockName = []byte("birthdayblock") + syncedToName = []byte("syncedto") + startBlockName = []byte("startblock") + birthdayName = []byte("birthday") + birthdayBlockName = []byte("birthdayblock") + birthdayBlockVerifiedName = []byte("birthdayblockverified") ) // uint32ToBytes converts a 32 bit unsigned integer into a 4-byte slice in @@ -2012,6 +2013,47 @@ func putBirthdayBlock(ns walletdb.ReadWriteBucket, block BlockStamp) error { return nil } +// fetchBirthdayBlockVerification retrieves the bit that determines whether the +// wallet has verified that its birthday block is correct. +func fetchBirthdayBlockVerification(ns walletdb.ReadBucket) bool { + bucket := ns.NestedReadBucket(syncBucketName) + verifiedValue := bucket.Get(birthdayBlockVerifiedName) + + // If there is no verification status, we can assume it has not been + // verified yet. + if verifiedValue == nil { + return false + } + + // Otherwise, we'll determine if it's verified by the value stored. + verified := binary.BigEndian.Uint16(verifiedValue[:]) + return verified != 0 +} + +// putBirthdayBlockVerification stores a bit that determines whether the +// birthday block has been verified by the wallet to be correct. +func putBirthdayBlockVerification(ns walletdb.ReadWriteBucket, verified bool) error { + // Convert the boolean to an integer in its binary representation as + // there is no way to insert a boolean directly as a value of a + // key/value pair. + verifiedValue := uint16(0) + if verified { + verifiedValue = 1 + } + + var verifiedBytes [2]byte + binary.BigEndian.PutUint16(verifiedBytes[:], verifiedValue) + + bucket := ns.NestedReadWriteBucket(syncBucketName) + err := bucket.Put(birthdayBlockVerifiedName, verifiedBytes[:]) + if err != nil { + str := "failed to store birthday block verification" + return managerError(ErrDatabase, str, err) + } + + return nil +} + // managerExists returns whether or not the manager has already been created // in the given database namespace. func managerExists(ns walletdb.ReadBucket) bool { From 7c377b2906d6d54844db7c17cca2543331ee9bbe Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 15 Nov 2018 17:59:01 -0800 Subject: [PATCH 3/6] waddrmgr/sync: expose verification status in Manager's birthday block methods --- waddrmgr/sync.go | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/waddrmgr/sync.go b/waddrmgr/sync.go index 2c15762..ed4ebe8 100644 --- a/waddrmgr/sync.go +++ b/waddrmgr/sync.go @@ -112,15 +112,26 @@ func (m *Manager) SetBirthday(ns walletdb.ReadWriteBucket, } // BirthdayBlock returns the birthday block, or earliest block a key could have -// been used, for the manager. -func (m *Manager) BirthdayBlock(ns walletdb.ReadBucket) (BlockStamp, error) { - return fetchBirthdayBlock(ns) +// been used, for the manager. A boolean is also returned to indicate whether +// the birthday block has been verified as correct. +func (m *Manager) BirthdayBlock(ns walletdb.ReadBucket) (BlockStamp, bool, error) { + birthdayBlock, err := fetchBirthdayBlock(ns) + if err != nil { + return BlockStamp{}, false, err + } + + return birthdayBlock, fetchBirthdayBlockVerification(ns), nil } // SetBirthdayBlock sets the birthday block, or earliest time a key could have -// been used, for the manager. +// been used, for the manager. The verified boolean can be used to specify +// whether this birthday block should be sanity checked to determine if there +// exists a better candidate to prevent less block fetching. func (m *Manager) SetBirthdayBlock(ns walletdb.ReadWriteBucket, - block BlockStamp) error { + block BlockStamp, verified bool) error { - return putBirthdayBlock(ns, block) + if err := putBirthdayBlock(ns, block); err != nil { + return err + } + return putBirthdayBlockVerification(ns, verified) } From f9df4908b3ee23eafa5fe05a78688eea584ac318 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 15 Nov 2018 17:59:45 -0800 Subject: [PATCH 4/6] wallet/chainntfns: prevent sanity check if correctness of birthday block has been verified In this commit, we prevent any further sanity check attempts by the wallet if its correctness has previously been verified. We do this to ensure we don't unnecessarily attempt to find a new candidate. --- wallet/chainntfns.go | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/wallet/chainntfns.go b/wallet/chainntfns.go index 87715e6..bc6f649 100644 --- a/wallet/chainntfns.go +++ b/wallet/chainntfns.go @@ -360,12 +360,15 @@ func (w *Wallet) birthdaySanityCheck() (*waddrmgr.BlockStamp, error) { } // We'll then fetch our wallet's birthday timestamp and block. - birthdayTimestamp := w.Manager.Birthday() - var birthdayBlock waddrmgr.BlockStamp + var ( + birthdayTimestamp = w.Manager.Birthday() + birthdayBlock waddrmgr.BlockStamp + birthdayBlockVerified bool + ) err = walletdb.View(w.db, func(tx walletdb.ReadTx) error { var err error ns := tx.ReadBucket(waddrmgrNamespaceKey) - birthdayBlock, err = w.Manager.BirthdayBlock(ns) + birthdayBlock, birthdayBlockVerified, err = w.Manager.BirthdayBlock(ns) return err }) @@ -380,6 +383,17 @@ func (w *Wallet) birthdaySanityCheck() (*waddrmgr.BlockStamp, error) { return nil, err } + // If the birthday block has already been verified to be correct, we can + // exit our sanity check to prevent potentially fetching a better + // candidate. + if birthdayBlockVerified { + log.Debugf("Birthday block has already been verified: "+ + "height=%d, hash=%v", birthdayBlock.Height, + birthdayBlock.Hash) + + return &birthdayBlock, nil + } + log.Debugf("Starting sanity check for the wallet's birthday block "+ "from: height=%d, hash=%v", birthdayBlock.Height, birthdayBlock.Hash) @@ -505,21 +519,15 @@ func (w *Wallet) birthdaySanityCheck() (*waddrmgr.BlockStamp, error) { timestampDelta = birthdayTimestamp.Sub(header.Timestamp) } - // At this point, we've found a valid candidate that satisfies our - // conditions above. If this is our current birthday block, then we can - // exit to avoid the additional database transaction. - if candidate.Hash.IsEqual(&birthdayBlock.Hash) { - return &candidate, nil - } - - // Otherwise, we have a new, better candidate, so we'll write it to - // disk. + // At this point, we've found a new, better candidate, so we'll write it + // to disk. log.Debugf("Found a new valid wallet birthday block: height=%d, hash=%v", candidate.Height, candidate.Hash) err = walletdb.Update(w.db, func(tx walletdb.ReadWriteTx) error { ns := tx.ReadWriteBucket(waddrmgrNamespaceKey) - if err := w.Manager.SetBirthdayBlock(ns, candidate); err != nil { + err := w.Manager.SetBirthdayBlock(ns, candidate, true) + if err != nil { return err } return w.Manager.SetSyncedTo(ns, &candidate) From 16ea72e95bb53ec3e6598bb85fbb45eb3c28ac38 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 15 Nov 2018 18:00:19 -0800 Subject: [PATCH 5/6] wallet/wallet: update to latest SetBirthdayBlock changes --- wallet/wallet.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/wallet/wallet.go b/wallet/wallet.go index 8455975..f5abdf5 100644 --- a/wallet/wallet.go +++ b/wallet/wallet.go @@ -498,7 +498,7 @@ func (w *Wallet) syncWithChain(birthdayStamp *waddrmgr.BlockStamp) error { birthdayStamp.Hash) err := w.Manager.SetBirthdayBlock( - ns, *birthdayStamp, + ns, *birthdayStamp, true, ) if err != nil { tx.Rollback() @@ -664,7 +664,9 @@ func (w *Wallet) syncWithChain(birthdayStamp *waddrmgr.BlockStamp) error { err := walletdb.Update(w.db, func(tx walletdb.ReadWriteTx) error { ns := tx.ReadWriteBucket(waddrmgrNamespaceKey) - return w.Manager.SetBirthdayBlock(ns, *birthdayStamp) + return w.Manager.SetBirthdayBlock( + ns, *birthdayStamp, true, + ) }) if err != nil { return nil From bd95bfa6fb67bfa230e0398eafae0516b8712bab Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 15 Nov 2018 18:00:43 -0800 Subject: [PATCH 6/6] wallet/wallet: prompt sanity check upon setting new birthday block within ImportPrivateKey In this commit, we set the verified bit to false upon setting the new birthday block to ensure its correctness as it was provided by the caller. --- wallet/wallet.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/wallet/wallet.go b/wallet/wallet.go index f5abdf5..a50dbcf 100644 --- a/wallet/wallet.go +++ b/wallet/wallet.go @@ -2684,7 +2684,7 @@ func (w *Wallet) ImportPrivateKey(scope waddrmgr.KeyScope, wif *btcutil.WIF, // before our current one. Otherwise, if we do, we can // potentially miss detecting relevant chain events that // occurred between them while rescanning. - birthdayBlock, err := w.Manager.BirthdayBlock(addrmgrNs) + birthdayBlock, _, err := w.Manager.BirthdayBlock(addrmgrNs) if err != nil { return err } @@ -2696,7 +2696,11 @@ func (w *Wallet) ImportPrivateKey(scope waddrmgr.KeyScope, wif *btcutil.WIF, if err != nil { return err } - return w.Manager.SetBirthdayBlock(addrmgrNs, *bs) + + // To ensure this birthday block is correct, we'll mark it as + // unverified to prompt a sanity check at the next restart to + // ensure it is correct as it was provided by the caller. + return w.Manager.SetBirthdayBlock(addrmgrNs, *bs, false) }) if err != nil { return "", err