From bf6001074c911650b40a05c98de48319f855bb2f Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Sun, 28 Jan 2018 14:46:54 -0800 Subject: [PATCH] chainntnfs: fix spend notification registration race condition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In this commit, we fix a race condition related to the way we attempt to query to see if an outpoint has already been spent by the time it’s registered within the ChainNotifier. If the transaction creating the outpoint hasn’t made it into the mempool by the time we execute the GetTxOut call, then we’ll attempt to query for the transaction itself. In this case, if we query for the transaction, then the block hash field will be empty as it hasn’t yet made it into a block. Under the previous logic, we’d then attempt to force a rescan. This is an issue as the forced rescan will fail since it’ll try to fetch the block hash of all zeroes. In this commit, we fix this issue by only entering this “fallback to rescan” logic iff, the transaction has actually been mined. --- chainntnfs/bitcoindnotify/bitcoind.go | 7 ++++++- chainntnfs/btcdnotify/btcd.go | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/chainntnfs/bitcoindnotify/bitcoind.go b/chainntnfs/bitcoindnotify/bitcoind.go index 8be208b6..5eb3ff98 100644 --- a/chainntnfs/bitcoindnotify/bitcoind.go +++ b/chainntnfs/bitcoindnotify/bitcoind.go @@ -527,7 +527,12 @@ func (b *BitcoindNotifier) RegisterSpendNtfn(outpoint *wire.OutPoint, } } - if transaction != nil { + // We'll only request a rescan if the transaction has actually + // been included within a block. Otherwise, we'll encounter an + // error when scanning for blocks. This can happens in the case + // of a race condition, wherein the output itself is unspent, + // and only arrives in the mempool after the getxout call. + if transaction != nil && transaction.BlockHash != "" { blockhash, err := chainhash.NewHashFromStr(transaction.BlockHash) if err != nil { return nil, err diff --git a/chainntnfs/btcdnotify/btcd.go b/chainntnfs/btcdnotify/btcd.go index cdc2f816..a5ff7ec5 100644 --- a/chainntnfs/btcdnotify/btcd.go +++ b/chainntnfs/btcdnotify/btcd.go @@ -547,7 +547,12 @@ func (b *BtcdNotifier) RegisterSpendNtfn(outpoint *wire.OutPoint, } } - if transaction != nil { + // We'll only request a rescan if the transaction has actually + // been included within a block. Otherwise, we'll encounter an + // error when scanning for blocks. This can happens in the case + // of a race condition, wherein the output itself is unspent, + // and only arrives in the mempool after the getxout call. + if transaction != nil && transaction.BlockHash != "" { blockhash, err := chainhash.NewHashFromStr(transaction.BlockHash) if err != nil { return nil, err