Merge pull request #617 from wpaulino/bitcoind-client-misc-fixes

chain: misc BitcoindClient fixes

chain/bitcoind_client.go

@@ -336,7 +336,7 @@ func (c *BitcoindClient) RescanBlocks(
 			continue
 		}
 
-		relevantTxs, err := c.filterBlock(block, header.Height, false)
+		relevantTxs := c.filterBlock(block, header.Height, false)
 		if len(relevantTxs) > 0 {
 			rescannedBlock := btcjson.RescannedBlock{
 				Hash: hash.String(),
@@ -567,14 +567,7 @@ func (c *BitcoindClient) ntfnHandler() {
 			c.bestBlockMtx.Unlock()
 			if newBlock.Header.PrevBlock == bestBlock.Hash {
 				newBlockHeight := bestBlock.Height + 1
-				_, err := c.filterBlock(
+				_ = c.filterBlock(newBlock, newBlockHeight, true)
-					newBlock, newBlockHeight, true,
-				)
-				if err != nil {
-					log.Errorf("Unable to filter block %v: %v",
-						newBlock.BlockHash(), err)
-					continue
-				}
 
 				// With the block succesfully filtered, we'll
 				// make it our new best block.
@@ -745,19 +738,22 @@ func (c *BitcoindClient) onRescanFinished(hash *chainhash.Hash, height int32,
 func (c *BitcoindClient) reorg(currentBlock waddrmgr.BlockStamp,
 	reorgBlock *wire.MsgBlock) error {
 
-	log.Debugf("Possible reorg at block %s", reorgBlock.BlockHash())
-
 	// Retrieve the best known height based on the block which caused the
 	// reorg. This way, we can preserve the chain of blocks we need to
 	// retrieve.
 	bestHash := reorgBlock.BlockHash()
 	bestHeight, err := c.GetBlockHeight(&bestHash)
 	if err != nil {
-		return err
+		return fmt.Errorf("unable to get block height for %v: %v",
+			bestHash, err)
 	}
 
+	log.Debugf("Possible reorg at block: height=%v, hash=%s", bestHash,
+		bestHeight)
+
 	if bestHeight < currentBlock.Height {
-		log.Debug("Detected multiple reorgs")
+		log.Debugf("Detected multiple reorgs: best_height=%v below "+
+			"current_height=%v", bestHeight, currentBlock.Height)
 		return nil
 	}
 
@@ -770,7 +766,8 @@ func (c *BitcoindClient) reorg(currentBlock waddrmgr.BlockStamp,
 	for i := bestHeight - 1; i >= currentBlock.Height; i-- {
 		block, err := c.GetBlock(&previousBlock)
 		if err != nil {
-			return err
+			return fmt.Errorf("unable to get block %v: %v",
+				previousBlock, err)
 		}
 		blocksToNotify.PushFront(block)
 		previousBlock = block.Header.PrevBlock
@@ -783,7 +780,8 @@ func (c *BitcoindClient) reorg(currentBlock waddrmgr.BlockStamp,
 	// We'll start by retrieving the header to the best block known to us.
 	currentHeader, err := c.GetBlockHeader(&currentBlock.Hash)
 	if err != nil {
-		return err
+		return fmt.Errorf("unable to get block header for %v: %v",
+			currentBlock.Hash, err)
 	}
 
 	// Then, we'll walk backwards in the chain until we find our common
@@ -792,8 +790,8 @@ func (c *BitcoindClient) reorg(currentBlock waddrmgr.BlockStamp,
 		// Since the previous hashes don't match, the current block has
 		// been reorged out of the chain, so we should send a
 		// BlockDisconnected notification for it.
-		log.Debugf("Disconnecting block %d (%v)", currentBlock.Height,
+		log.Debugf("Disconnecting block: height=%v, hash=%v",
-			currentBlock.Hash)
+			currentBlock.Height, currentBlock.Hash)
 
 		c.onBlockDisconnected(
 			&currentBlock.Hash, currentBlock.Height,
@@ -804,7 +802,8 @@ func (c *BitcoindClient) reorg(currentBlock waddrmgr.BlockStamp,
 		// continue the common ancestor search.
 		currentHeader, err = c.GetBlockHeader(&currentHeader.PrevBlock)
 		if err != nil {
-			return err
+			return fmt.Errorf("unable to get block header for %v: %v",
+				currentHeader.PrevBlock, err)
 		}
 
 		currentBlock.Height--
@@ -815,7 +814,8 @@ func (c *BitcoindClient) reorg(currentBlock waddrmgr.BlockStamp,
 		// once we've found our common ancestor.
 		block, err := c.GetBlock(&previousBlock)
 		if err != nil {
-			return err
+			return fmt.Errorf("unable to get block %v: %v",
+				previousBlock, err)
 		}
 		blocksToNotify.PushFront(block)
 		previousBlock = block.Header.PrevBlock
@@ -824,8 +824,8 @@ func (c *BitcoindClient) reorg(currentBlock waddrmgr.BlockStamp,
 	// Disconnect the last block from the old chain. Since the previous
 	// block remains the same between the old and new chains, the tip will
 	// now be the last common ancestor.
-	log.Debugf("Disconnecting block %d (%v)", currentBlock.Height,
+	log.Debugf("Disconnecting block: height=%v, hash=%v",
-		currentBlock.Hash)
+		currentBlock.Height, currentBlock.Hash)
 
 	c.onBlockDisconnected(
 		&currentBlock.Hash, currentBlock.Height, currentHeader.Timestamp,
@@ -840,13 +840,11 @@ func (c *BitcoindClient) reorg(currentBlock waddrmgr.BlockStamp,
 		nextHash := nextBlock.BlockHash()
 		nextHeader, err := c.GetBlockHeader(&nextHash)
 		if err != nil {
-			return err
+			return fmt.Errorf("unable to get block header for %v: %v",
+				nextHash, err)
 		}
 
-		_, err = c.filterBlock(nextBlock, nextHeight, true)
+		_ = c.filterBlock(nextBlock, nextHeight, true)
-		if err != nil {
-			return err
-		}
 
 		currentBlock.Height = nextHeight
 		currentBlock.Hash = nextHash
@@ -916,8 +914,6 @@ func (c *BitcoindClient) FilterBlocks(
 // the client in the watch list. This is called only within a queue processing
 // loop.
 func (c *BitcoindClient) rescan(start chainhash.Hash) error {
-	log.Infof("Starting rescan from block %s", start)
-
 	// We start by getting the best already processed block. We only use
 	// the height, as the hash can change during a reorganization, which we
 	// catch by testing connectivity from known blocks to the previous
@@ -949,13 +945,6 @@ func (c *BitcoindClient) rescan(start chainhash.Hash) error {
 	}
 	headers.PushBack(previousHeader)
 
-	// Queue a RescanFinished notification to the caller with the last block
-	// processed throughout the rescan once done.
-	defer c.onRescanFinished(
-		previousHash, previousHeader.Height,
-		time.Unix(previousHeader.Time, 0),
-	)
-
 	// Cycle through all of the blocks known to bitcoind, being mindful of
 	// reorgs.
 	for i := previousHeader.Height + 1; i <= bestBlock.Height; i++ {
@@ -1065,9 +1054,7 @@ func (c *BitcoindClient) rescan(start chainhash.Hash) error {
 		headers.PushBack(previousHeader)
 
 		// Notify the block and any of its relevant transacations.
-		if _, err = c.filterBlock(block, i, true); err != nil {
+		_ = c.filterBlock(block, i, true)
-			return err
-		}
 
 		if i%10000 == 0 {
 			c.onRescanProgress(
@@ -1095,18 +1082,20 @@ func (c *BitcoindClient) rescan(start chainhash.Hash) error {
 		}
 	}
 
+	c.onRescanFinished(bestHash, bestHeight, time.Unix(bestHeader.Time, 0))
+
 	return nil
 }
 
 // filterBlock filters a block for watched outpoints and addresses, and returns
 // any matching transactions, sending notifications along the way.
 func (c *BitcoindClient) filterBlock(block *wire.MsgBlock, height int32,
-	notify bool) ([]*wtxmgr.TxRecord, error) {
+	notify bool) []*wtxmgr.TxRecord {
 
 	// If this block happened before the client's birthday, then we'll skip
 	// it entirely.
 	if block.Header.Timestamp.Before(c.birthday) {
-		return nil, nil
+		return nil
 	}
 
 	if c.shouldNotifyBlocks() {
@@ -1162,7 +1151,7 @@ func (c *BitcoindClient) filterBlock(block *wire.MsgBlock, height int32,
 		c.onBlockConnected(&blockHash, height, block.Header.Timestamp)
 	}
 
-	return relevantTxs, nil
+	return relevantTxs
 }
 
 // filterTx determines whether a transaction is relevant to the client by

chain/block_filterer_test.go

@@ -319,6 +319,6 @@ func assertRelevantTxnsContains(t *testing.T, bf *chain.BlockFilterer, wantTx *w
 		}
 	}
 
-	t.Fatalf("unable to find tx: %x in %d relevant txns", wantTx,
+	t.Fatalf("unable to find tx: %x in %d relevant txns", wantTx.TxHash(),
 		len(bf.RelevantTxns))
 }

go.mod

@@ -1,9 +1,9 @@
 module github.com/btcsuite/btcwallet
 
 require (
-	github.com/btcsuite/btcd v0.0.0-20190213025234-306aecffea32
+	github.com/btcsuite/btcd v0.0.0-20190523000118-16327141da8c
 	github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f
-	github.com/btcsuite/btcutil v0.0.0-20190207003914-4c204d697803
+	github.com/btcsuite/btcutil v0.0.0-20190425235716-9e5f4b9a998d
 	github.com/btcsuite/golangcrypto v0.0.0-20150304025918-53f62d9b43e8
 	github.com/btcsuite/websocket v0.0.0-20150119174127-31079b680792
 	github.com/coreos/bbolt v1.3.2

go.sum

@@ -4,14 +4,17 @@ github.com/aead/siphash v1.0.1/go.mod h1:Nywa3cDsYNNK3gaciGTWPwHt0wlpNV15vwmswBA
 github.com/boltdb/bolt v1.3.1 h1:JQmyP4ZBrce+ZQu0dY660FMfatumYDLun9hBCUVIkF4=
 github.com/boltdb/bolt v1.3.1/go.mod h1:clJnj/oiGkjum5o1McbSZDSLxVThjynRyGBgiAx27Ps=
 github.com/btcsuite/btcd v0.0.0-20180823030728-d81d8877b8f3/go.mod h1:Dmm/EzmjnCiweXmzRIAiUWCInVmPgjkzgv5k4tVyXiQ=
-github.com/btcsuite/btcd v0.0.0-20190213025234-306aecffea32 h1:qkOC5Gd33k54tobS36cXdAzJbeHaduLtnLQQwNoIi78=
 github.com/btcsuite/btcd v0.0.0-20190213025234-306aecffea32/go.mod h1:DrZx5ec/dmnfpw9KyYoQyYo7d0KEvTkk/5M/vbZjAr8=
+github.com/btcsuite/btcd v0.0.0-20190523000118-16327141da8c h1:aEbSeNALREWXk0G7UdNhR3ayBV7tZ4M2PNmnrCAph6Q=
+github.com/btcsuite/btcd v0.0.0-20190523000118-16327141da8c/go.mod h1:3J08xEfcugPacsc34/LKRU2yO7YmuT8yt28J8k2+rrI=
 github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f h1:bAs4lUbRJpnnkd9VhRV3jjAVU7DJVjMaK+IsvSeZvFo=
 github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f/go.mod h1:TdznJufoqS23FtqVCzL0ZqgP5MqXbb4fg/WgDys70nA=
 github.com/btcsuite/btcutil v0.0.0-20180706230648-ab6388e0c60a h1:RQMUrEILyYJEoAT34XS/kLu40vC0+po/UfxrBBA4qZE=
 github.com/btcsuite/btcutil v0.0.0-20180706230648-ab6388e0c60a/go.mod h1:+5NJ2+qvTyV9exUAL/rxXi3DcLg2Ts+ymUAY5y4NvMg=
 github.com/btcsuite/btcutil v0.0.0-20190207003914-4c204d697803 h1:j3AgPKKZtZStM2nyhrDSLSYgT7YHrZKdSkq1OYeLjvM=
 github.com/btcsuite/btcutil v0.0.0-20190207003914-4c204d697803/go.mod h1:+5NJ2+qvTyV9exUAL/rxXi3DcLg2Ts+ymUAY5y4NvMg=
+github.com/btcsuite/btcutil v0.0.0-20190425235716-9e5f4b9a998d h1:yJzD/yFppdVCf6ApMkVy8cUxV0XrxdP9rVf6D87/Mng=
+github.com/btcsuite/btcutil v0.0.0-20190425235716-9e5f4b9a998d/go.mod h1:+5NJ2+qvTyV9exUAL/rxXi3DcLg2Ts+ymUAY5y4NvMg=
 github.com/btcsuite/btcwallet v0.0.0-20180904010540-284e2e0e696e33d5be388f7f3d9a26db703e0c06/go.mod h1:/d7QHZsfUAruXuBhyPITqoYOmJ+nq35qPsJjz/aSpCg=
 github.com/btcsuite/btcwallet v0.0.0-20190313032608-acf3b04b0273/go.mod h1:mkOYY8/psBiL5E+Wb0V7M0o+N7NXi2SZJz6+RKkncIc=
 github.com/btcsuite/go-socks v0.0.0-20170105172521-4720035b7bfd h1:R/opQEbFEy9JGkIguV40SvRY1uliPX8ifOvi6ICsFCw=

wallet/wallet.go

@@ -3414,6 +3414,14 @@ func (w *Wallet) publishTransaction(tx *wire.MsgTx) (*chainhash.Hash, error) {
 	}
 
 	txid, err := chainClient.SendRawTransaction(tx, false)
+
+	// Determine if this was an RPC error thrown due to the transaction
+	// already confirming.
+	var rpcTxConfirmed bool
+	if rpcErr, ok := err.(*btcjson.RPCError); ok {
+		rpcTxConfirmed = rpcErr.Code == btcjson.ErrRPCTxAlreadyInChain
+	}
+
 	switch {
 	case err == nil:
 		return txid, nil
@@ -3425,12 +3433,16 @@ func (w *Wallet) publishTransaction(tx *wire.MsgTx) (*chainhash.Hash, error) {
 	//
 	// This error is returned when broadcasting/sending a transaction to a
 	// btcd node that already has it in their mempool.
-	case strings.Contains(err.Error(), "already have transaction"):
+	case strings.Contains(
+		strings.ToLower(err.Error()), "already have transaction",
+	):
 		fallthrough
 
 	// This error is returned when broadcasting a transaction to a bitcoind
 	// node that already has it in their mempool.
-	case strings.Contains(err.Error(), "txn-already-in-mempool"):
+	case strings.Contains(
+		strings.ToLower(err.Error()), "txn-already-in-mempool",
+	):
 		return txid, nil
 
 	// If the transaction has already confirmed, we can safely remove it
@@ -3438,19 +3450,21 @@ func (w *Wallet) publishTransaction(tx *wire.MsgTx) (*chainhash.Hash, error) {
 	// confirmed store. We'll avoid returning an error as the broadcast was
 	// in a sense successful.
 	//
-	// This error is returned when broadcasting/sending a transaction that
+	// This error is returned when sending a transaction that has already
-	// has already confirmed to a btcd node.
+	// confirmed to a btcd/bitcoind node over RPC.
-	case strings.Contains(err.Error(), "transaction already exists"):
+	case rpcTxConfirmed:
 		fallthrough
 
 	// This error is returned when broadcasting a transaction that has
-	// already confirmed to a bitcoind node.
+	// already confirmed to a btcd node over the P2P network.
-	case strings.Contains(err.Error(), "txn-already-known"):
+	case strings.Contains(
+		strings.ToLower(err.Error()), "transaction already exists",
+	):
 		fallthrough
 
-	// This error is returned when sending a transaction that has already
+	// This error is returned when broadcasting a transaction that has
-	// confirmed to a bitcoind node over RPC.
+	// already confirmed to a bitcoind node over the P2P network.
-	case strings.Contains(err.Error(), "transaction already in block chain"):
+	case strings.Contains(strings.ToLower(err.Error()), "txn-already-known"):
 		dbErr := walletdb.Update(w.db, func(dbTx walletdb.ReadWriteTx) error {
 			txmgrNs := dbTx.ReadWriteBucket(wtxmgrNamespaceKey)
 			txRec, err := wtxmgr.NewTxRecordFromMsgTx(tx, time.Now())