Merge pull request #575 from wpaulino/syncedto-rescan

wallet/wallet: prevent always rescanning from birthday block

waddrmgr/db.go

@@ -253,10 +253,11 @@ var (
 	watchingOnlyName    = []byte("watchonly")
 
 	// Sync related key names (sync bucket).
-	syncedToName      = []byte("syncedto")
+	syncedToName              = []byte("syncedto")
-	startBlockName    = []byte("startblock")
+	startBlockName            = []byte("startblock")
-	birthdayName      = []byte("birthday")
+	birthdayName              = []byte("birthday")
-	birthdayBlockName = []byte("birthdayblock")
+	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 {

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.
+// been used, for the manager. A boolean is also returned to indicate whether
-func (m *Manager) BirthdayBlock(ns walletdb.ReadBucket) (BlockStamp, error) {
+// the birthday block has been verified as correct.
-	return fetchBirthdayBlock(ns)
+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)
 }

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 (
-	var birthdayBlock waddrmgr.BlockStamp
+		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
+	// At this point, we've found a new, better candidate, so we'll write it
-	// conditions above. If this is our current birthday block, then we can
+	// to disk.
-	// 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.
 	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)

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()
@@ -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,
@@ -662,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
@@ -681,7 +685,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
@@ -2671,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
 		}
@@ -2683,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