From dfd9f6d1ca9714c53432efd9fac1de167b6219c4 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Mon, 15 May 2017 17:47:03 -0700 Subject: [PATCH] chainntnfs/btcdnotify: fix historical confirmation dispatch bugs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit fixes to distinct bugs in the way we previously dipatched notifications for transactions which needed a historical dispatch. Previously we would compare transactions when scanning the block using the `tx.Hash` field. This was incorrect has the `Hash` field is actually the wtxid, not the txid which should be the item being compared. We fix this within the second bug fix by actually using the txid to find the proper transaction. The second fix has to due with a slight race condition which led to an off-by-one error when dispatching the historical confirmation. If while we were dispatching the confirmation, a new block was found, then we could calculate the wrong block height (off by one) as we were using the ‘currentHeight’ instead the height of the block which included the transaction. --- chainntnfs/btcdnotify/btcd.go | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/chainntnfs/btcdnotify/btcd.go b/chainntnfs/btcdnotify/btcd.go index 335fb869..03fc8ef6 100644 --- a/chainntnfs/btcdnotify/btcd.go +++ b/chainntnfs/btcdnotify/btcd.go @@ -431,24 +431,18 @@ func (b *BtcdNotifier) attemptHistoricalDispatch(msg *confirmationsNotification, "historical dispatch: %v", tx.BlockHash, err) return false } - block, err := b.chainConn.GetBlock(blockHash) + block, err := b.chainConn.GetBlockVerbose(blockHash) if err != nil { chainntnfs.Log.Errorf("unable to get block hash: %v", err) return false } - txHash, err := chainhash.NewHashFromStr(tx.Hash) - if err != nil { - chainntnfs.Log.Errorf("unable to convert to hash: %v", err) - return false - } - // If the block obtained, locate the transaction's index within the // block so we can give the subscriber full confirmation details. var txIndex uint32 - for i, t := range block.Transactions { - h := t.TxHash() - if txHash.IsEqual(&h) { + targetTxidStr := msg.txid.String() + for i, txHash := range block.Tx { + if txHash == targetTxidStr { txIndex = uint32(i) break } @@ -456,7 +450,7 @@ func (b *BtcdNotifier) attemptHistoricalDispatch(msg *confirmationsNotification, confDetails := &chainntnfs.TxConfirmation{ BlockHash: blockHash, - BlockHeight: uint32(currentHeight) - uint32(tx.Confirmations) + 1, + BlockHeight: uint32(block.Height), TxIndex: txIndex, }