From 1a41e23bf4b387dbe65568ed0116d482de5796e1 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 6 Dec 2018 21:14:19 -0800 Subject: [PATCH] chainntnfs/bitcoindnotify: support registration for script confirmations In this commit, we extend the BitcoindNotifier to support registering scripts for confirmation notifications. Once the script has been detected as confirmed within the chain, a confirmation notification will be dispatched to through the Confirmed channel of the ConfirmationEvent returned upon registration. For scripts that have confirmed in the past, the `historicalConfDetails` method has been modified to skip the txindex and go straight to scanning the chain manually if confirmation request is for a script. When scanning the chain, we'll determine whether the script has been confirmed by locating the script in an output of a confirmed transaction. For scripts that have yet to confirm, they will be properly tracked within the TxNotifier. --- chainntnfs/bitcoindnotify/bitcoind.go | 93 ++++++++++++++-------- chainntnfs/bitcoindnotify/bitcoind_test.go | 55 ++++++++++--- 2 files changed, 103 insertions(+), 45 deletions(-) diff --git a/chainntnfs/bitcoindnotify/bitcoind.go b/chainntnfs/bitcoindnotify/bitcoind.go index 6981ae63..1fea9853 100644 --- a/chainntnfs/bitcoindnotify/bitcoind.go +++ b/chainntnfs/bitcoindnotify/bitcoind.go @@ -1,6 +1,7 @@ package bitcoindnotify import ( + "bytes" "errors" "fmt" "strings" @@ -234,7 +235,8 @@ out: defer b.wg.Done() confDetails, _, err := b.historicalConfDetails( - msg.TxID, msg.StartHeight, msg.EndHeight, + msg.ConfRequest, + msg.StartHeight, msg.EndHeight, ) if err != nil { chainntnfs.Log.Error(err) @@ -249,7 +251,7 @@ out: // cache at tip, since any pending // rescans have now completed. err = b.txNotifier.UpdateConfDetails( - *msg.TxID, confDetails, + msg.ConfRequest, confDetails, ) if err != nil { chainntnfs.Log.Error(err) @@ -395,15 +397,28 @@ out: b.wg.Done() } -// historicalConfDetails looks up whether a transaction is already included in a -// block in the active chain and, if so, returns details about the confirmation. -func (b *BitcoindNotifier) historicalConfDetails(txid *chainhash.Hash, +// historicalConfDetails looks up whether a confirmation request (txid/output +// script) has already been included in a block in the active chain and, if so, +// returns details about said block. +func (b *BitcoindNotifier) historicalConfDetails(confRequest chainntnfs.ConfRequest, startHeight, endHeight uint32) (*chainntnfs.TxConfirmation, chainntnfs.TxConfStatus, error) { + // If a txid was not provided, then we should dispatch upon seeing the + // script on-chain, so we'll short-circuit straight to scanning manually + // as there doesn't exist a script index to query. + if confRequest.TxID == chainntnfs.ZeroHash { + return b.confDetailsManually( + confRequest, startHeight, endHeight, + ) + } + + // Otherwise, we'll dispatch upon seeing a transaction on-chain with the + // given hash. + // // We'll first attempt to retrieve the transaction using the node's // txindex. - txConf, txStatus, err := b.confDetailsFromTxIndex(txid) + txConf, txStatus, err := b.confDetailsFromTxIndex(&confRequest.TxID) // We'll then check the status of the transaction lookup returned to // determine whether we should proceed with any fallback methods. @@ -414,7 +429,7 @@ func (b *BitcoindNotifier) historicalConfDetails(txid *chainhash.Hash, case err != nil: chainntnfs.Log.Debugf("Failed getting conf details from "+ "index (%v), scanning manually", err) - return b.confDetailsManually(txid, startHeight, endHeight) + return b.confDetailsManually(confRequest, startHeight, endHeight) // The transaction was found within the node's mempool. case txStatus == chainntnfs.TxFoundMempool: @@ -508,17 +523,14 @@ func (b *BitcoindNotifier) confDetailsFromTxIndex(txid *chainhash.Hash, blockHash) } -// confDetailsManually looks up whether a transaction is already included in a -// block in the active chain by scanning the chain's blocks, starting from the -// earliest height the transaction could have been included in, to the current -// height in the chain. If the transaction is found, its confirmation details -// are returned. Otherwise, nil is returned. -func (b *BitcoindNotifier) confDetailsManually(txid *chainhash.Hash, +// confDetailsManually looks up whether a transaction/output script has already +// been included in a block in the active chain by scanning the chain's blocks +// within the given range. If the transaction/output script is found, its +// confirmation details are returned. Otherwise, nil is returned. +func (b *BitcoindNotifier) confDetailsManually(confRequest chainntnfs.ConfRequest, heightHint, currentHeight uint32) (*chainntnfs.TxConfirmation, chainntnfs.TxConfStatus, error) { - targetTxidStr := txid.String() - // Begin scanning blocks at every height to determine where the // transaction was included in. for height := currentHeight; height >= heightHint && height > 0; height-- { @@ -538,24 +550,26 @@ func (b *BitcoindNotifier) confDetailsManually(txid *chainhash.Hash, "with height %d", height) } - block, err := b.chainConn.GetBlockVerbose(blockHash) + block, err := b.chainConn.GetBlock(blockHash) if err != nil { return nil, chainntnfs.TxNotFoundManually, fmt.Errorf("unable to get block with hash "+ "%v: %v", blockHash, err) } - for txIndex, txHash := range block.Tx { - // If we're able to find the transaction in this block, - // return its confirmation details. - if txHash == targetTxidStr { - details := &chainntnfs.TxConfirmation{ - BlockHash: blockHash, - BlockHeight: height, - TxIndex: uint32(txIndex), - } - return details, chainntnfs.TxFoundManually, nil + // For every transaction in the block, check which one matches + // our request. If we find one that does, we can dispatch its + // confirmation details. + for txIndex, tx := range block.Transactions { + if !confRequest.MatchesTx(tx) { + continue } + + return &chainntnfs.TxConfirmation{ + BlockHash: blockHash, + BlockHeight: height, + TxIndex: uint32(txIndex), + }, chainntnfs.TxFoundManually, nil } } @@ -801,30 +815,41 @@ func (b *BitcoindNotifier) dispatchSpendDetailsManually( return ErrTransactionNotFound } -// RegisterConfirmationsNtfn registers a notification with BitcoindNotifier -// which will be triggered once the txid reaches numConfs number of -// confirmations. +// RegisterConfirmationsNtfn registers an intent to be notified once the target +// txid/output script has reached numConfs confirmations on-chain. When +// intending to be notified of the confirmation of an output script, a nil txid +// must be used. The heightHint should represent the earliest height at which +// the txid/output script could have been included in the chain. +// +// Progress on the number of confirmations left can be read from the 'Updates' +// channel. Once it has reached all of its confirmations, a notification will be +// sent across the 'Confirmed' channel. func (b *BitcoindNotifier) RegisterConfirmationsNtfn(txid *chainhash.Hash, - _ []byte, numConfs, heightHint uint32) (*chainntnfs.ConfirmationEvent, error) { + pkScript []byte, + numConfs, heightHint uint32) (*chainntnfs.ConfirmationEvent, error) { // Construct a notification request for the transaction and send it to // the main event loop. + confRequest, err := chainntnfs.NewConfRequest(txid, pkScript) + if err != nil { + return nil, err + } ntfn := &chainntnfs.ConfNtfn{ ConfID: atomic.AddUint64(&b.confClientCounter, 1), - TxID: txid, + ConfRequest: confRequest, NumConfirmations: numConfs, Event: chainntnfs.NewConfirmationEvent(numConfs), HeightHint: heightHint, } - chainntnfs.Log.Infof("New confirmation subscription: "+ - "txid=%v, numconfs=%v", txid, numConfs) + chainntnfs.Log.Infof("New confirmation subscription: %v, num_confs=%v", + confRequest, numConfs) // Register the conf notification with the TxNotifier. A non-nil value // for `dispatch` will be returned if we are required to perform a // manual scan for the confirmation. Otherwise the notifier will begin // watching at tip for the transaction to confirm. - dispatch, err := b.txNotifier.RegisterConf(ntfn) + dispatch, _, err := b.txNotifier.RegisterConf(ntfn) if err != nil { return nil, err } diff --git a/chainntnfs/bitcoindnotify/bitcoind_test.go b/chainntnfs/bitcoindnotify/bitcoind_test.go index 27b7dff4..c74bf6a2 100644 --- a/chainntnfs/bitcoindnotify/bitcoind_test.go +++ b/chainntnfs/bitcoindnotify/bitcoind_test.go @@ -3,6 +3,7 @@ package bitcoindnotify import ( + "bytes" "io/ioutil" "testing" "time" @@ -14,6 +15,20 @@ import ( "github.com/lightningnetwork/lnd/channeldb" ) +var ( + testScript = []byte{ + // OP_HASH160 + 0xA9, + // OP_DATA_20 + 0x14, + // <20-byte hash> + 0xec, 0x6f, 0x7a, 0x5a, 0xa8, 0xf2, 0xb1, 0x0c, 0xa5, 0x15, + 0x04, 0x52, 0x3a, 0x60, 0xd4, 0x03, 0x06, 0xf6, 0x96, 0xcd, + // OP_EQUAL + 0x87, + } +) + func initHintCache(t *testing.T) *chainntnfs.HeightHintCache { t.Helper() @@ -107,8 +122,13 @@ func TestHistoricalConfDetailsTxIndex(t *testing.T) { // A transaction unknown to the node should not be found within the // txindex even if it is enabled, so we should not proceed with any // fallback methods. - var zeroHash chainhash.Hash - _, txStatus, err := notifier.historicalConfDetails(&zeroHash, 0, 0) + var unknownHash chainhash.Hash + copy(unknownHash[:], bytes.Repeat([]byte{0x10}, 32)) + unknownConfReq, err := chainntnfs.NewConfRequest(&unknownHash, testScript) + if err != nil { + t.Fatalf("unable to create conf request: %v", err) + } + _, txStatus, err := notifier.historicalConfDetails(unknownConfReq, 0, 0) if err != nil { t.Fatalf("unable to retrieve historical conf details: %v", err) } @@ -123,16 +143,20 @@ func TestHistoricalConfDetailsTxIndex(t *testing.T) { // Now, we'll create a test transaction, confirm it, and attempt to // retrieve its confirmation details. - txid, _, err := chainntnfs.GetTestTxidAndScript(miner) + txid, pkScript, err := chainntnfs.GetTestTxidAndScript(miner) if err != nil { t.Fatalf("unable to create tx: %v", err) } if err := chainntnfs.WaitForMempoolTx(miner, txid); err != nil { t.Fatal(err) } + confReq, err := chainntnfs.NewConfRequest(txid, pkScript) + if err != nil { + t.Fatalf("unable to create conf request: %v", err) + } // The transaction should be found in the mempool at this point. - _, txStatus, err = notifier.historicalConfDetails(txid, 0, 0) + _, txStatus, err = notifier.historicalConfDetails(confReq, 0, 0) if err != nil { t.Fatalf("unable to retrieve historical conf details: %v", err) } @@ -154,7 +178,7 @@ func TestHistoricalConfDetailsTxIndex(t *testing.T) { // the txindex includes the transaction just mined. syncNotifierWithMiner(t, notifier, miner) - _, txStatus, err = notifier.historicalConfDetails(txid, 0, 0) + _, txStatus, err = notifier.historicalConfDetails(confReq, 0, 0) if err != nil { t.Fatalf("unable to retrieve historical conf details: %v", err) } @@ -173,7 +197,7 @@ func TestHistoricalConfDetailsTxIndex(t *testing.T) { // historical confirmation details using the set of fallback methods when the // backend node's txindex is disabled. func TestHistoricalConfDetailsNoTxIndex(t *testing.T) { - miner, tearDown := chainntnfs.NewMiner(t, nil, true, 25) + miner, tearDown := chainntnfs.NewMiner(t, nil, true, 400) defer tearDown() bitcoindConn, cleanUp := chainntnfs.NewBitcoindBackend( @@ -189,10 +213,15 @@ func TestHistoricalConfDetailsNoTxIndex(t *testing.T) { // Since the node has its txindex disabled, we fall back to scanning the // chain manually. A transaction unknown to the network should not be // found. - var zeroHash chainhash.Hash + var unknownHash chainhash.Hash + copy(unknownHash[:], bytes.Repeat([]byte{0x10}, 32)) + unknownConfReq, err := chainntnfs.NewConfRequest(&unknownHash, testScript) + if err != nil { + t.Fatalf("unable to create conf request: %v", err) + } broadcastHeight := syncNotifierWithMiner(t, notifier, miner) _, txStatus, err := notifier.historicalConfDetails( - &zeroHash, uint32(broadcastHeight), uint32(broadcastHeight), + unknownConfReq, uint32(broadcastHeight), uint32(broadcastHeight), ) if err != nil { t.Fatalf("unable to retrieve historical conf details: %v", err) @@ -213,8 +242,8 @@ func TestHistoricalConfDetailsNoTxIndex(t *testing.T) { // one output, which we will manually spend. The backend node's // transaction index should also be disabled, which we've already // ensured above. - output, pkScript := chainntnfs.CreateSpendableOutput(t, miner) - spendTx := chainntnfs.CreateSpendTx(t, output, pkScript) + outpoint, output, privKey := chainntnfs.CreateSpendableOutput(t, miner) + spendTx := chainntnfs.CreateSpendTx(t, outpoint, output, privKey) spendTxHash, err := miner.Node.SendRawTransaction(spendTx, true) if err != nil { t.Fatalf("unable to broadcast tx: %v", err) @@ -228,9 +257,13 @@ func TestHistoricalConfDetailsNoTxIndex(t *testing.T) { // Ensure the notifier and miner are synced to the same height to ensure // we can find the transaction when manually scanning the chain. + confReq, err := chainntnfs.NewConfRequest(&outpoint.Hash, output.PkScript) + if err != nil { + t.Fatalf("unable to create conf request: %v", err) + } currentHeight := syncNotifierWithMiner(t, notifier, miner) _, txStatus, err = notifier.historicalConfDetails( - &output.Hash, uint32(broadcastHeight), uint32(currentHeight), + confReq, uint32(broadcastHeight), uint32(currentHeight), ) if err != nil { t.Fatalf("unable to retrieve historical conf details: %v", err)