From f88f6f68d475cfebd55c178204daa55be2fb316e Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 16 Aug 2018 19:29:37 -0700 Subject: [PATCH 01/19] chainntnfs: extract helper test vars and functions In this commit, we extract some of the helper test variables and functions into their own file and guard them under a build flag. This is needed as some unit tests will be introduced in a future commit where most of the same functions within the interface tests are reused. In order to prevent these variables and functions from being exportable, we guard them by the "debug" build tag. --- chainntnfs/interface_test.go | 387 ++++++----------------------------- chainntnfs/test_utils.go | 268 ++++++++++++++++++++++++ 2 files changed, 329 insertions(+), 326 deletions(-) create mode 100644 chainntnfs/test_utils.go diff --git a/chainntnfs/interface_test.go b/chainntnfs/interface_test.go index 242e9ed9..b4fb5cc9 100644 --- a/chainntnfs/interface_test.go +++ b/chainntnfs/interface_test.go @@ -1,3 +1,5 @@ +// +build debug + package chainntnfs_test import ( @@ -5,25 +7,15 @@ import ( "fmt" "io/ioutil" "log" - "math/rand" - "os" - "os/exec" - "path/filepath" "sync" "testing" "time" - "github.com/btcsuite/btcd/btcec" - "github.com/btcsuite/btcd/btcjson" - "github.com/btcsuite/btcd/chaincfg" "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/btcsuite/btcd/integration/rpctest" "github.com/btcsuite/btcd/rpcclient" - "github.com/btcsuite/btcd/txscript" "github.com/btcsuite/btcd/wire" "github.com/btcsuite/btcutil" - "github.com/btcsuite/btcwallet/chain" - "github.com/btcsuite/btcwallet/walletdb" "github.com/lightninglabs/neutrino" "github.com/lightningnetwork/lnd/chainntnfs" "github.com/lightningnetwork/lnd/channeldb" @@ -41,78 +33,10 @@ import ( "github.com/lightningnetwork/lnd/chainntnfs/neutrinonotify" // Required to register the boltdb walletdb implementation. + "github.com/btcsuite/btcwallet/chain" _ "github.com/btcsuite/btcwallet/walletdb/bdb" ) -var ( - testPrivKey = []byte{ - 0x81, 0xb6, 0x37, 0xd8, 0xfc, 0xd2, 0xc6, 0xda, - 0x63, 0x59, 0xe6, 0x96, 0x31, 0x13, 0xa1, 0x17, - 0xd, 0xe7, 0x95, 0xe4, 0xb7, 0x25, 0xb8, 0x4d, - 0x1e, 0xb, 0x4c, 0xfd, 0x9e, 0xc5, 0x8c, 0xe9, - } - - netParams = &chaincfg.RegressionNetParams - privKey, pubKey = btcec.PrivKeyFromBytes(btcec.S256(), testPrivKey) - addrPk, _ = btcutil.NewAddressPubKey(pubKey.SerializeCompressed(), - netParams) - testAddr = addrPk.AddressPubKeyHash() -) - -func getTestTxIdAndScript(miner *rpctest.Harness) (*chainhash.Hash, []byte, error) { - script, err := txscript.PayToAddrScript(testAddr) - if err != nil { - return nil, nil, err - } - - outputs := []*wire.TxOut{ - { - Value: 2e8, - PkScript: script, - }, - } - - txid, err := miner.SendOutputs(outputs, 10) - if err != nil { - return nil, nil, err - } - - return txid, script, nil -} - -func waitForMempoolTx(r *rpctest.Harness, txid *chainhash.Hash) error { - var found bool - var tx *btcutil.Tx - var err error - timeout := time.After(10 * time.Second) - for !found { - // Do a short wait - select { - case <-timeout: - return fmt.Errorf("timeout after 10s") - default: - } - time.Sleep(100 * time.Millisecond) - - // Check for the harness' knowledge of the txid - tx, err = r.Node.GetRawTransaction(txid) - if err != nil { - switch e := err.(type) { - case *btcjson.RPCError: - if e.Code == btcjson.ErrRPCNoTxInfo { - continue - } - default: - } - return err - } - if tx != nil && tx.MsgTx().TxHash() == *txid { - found = true - } - } - return nil -} - func testSingleConfirmationNotification(miner *rpctest.Harness, notifier chainntnfs.TestChainNotifier, t *testing.T) { @@ -122,14 +46,11 @@ func testSingleConfirmationNotification(miner *rpctest.Harness, // So first, let's send some coins to "ourself", obtaining a txid. // We're spending from a coinbase output here, so we use the dedicated // function. - - txid, pkScript, err := getTestTxIdAndScript(miner) + txid, pkScript, err := chainntnfs.GetTestTxidAndScript(miner) if err != nil { t.Fatalf("unable to create test tx: %v", err) } - - err = waitForMempoolTx(miner, txid) - if err != nil { + if err := chainntnfs.WaitForMempoolTx(miner, txid); err != nil { t.Fatalf("tx not relayed to miner: %v", err) } @@ -192,13 +113,11 @@ func testMultiConfirmationNotification(miner *rpctest.Harness, // // Again, we'll begin by creating a fresh transaction, so we can obtain // a fresh txid. - txid, pkScript, err := getTestTxIdAndScript(miner) + txid, pkScript, err := chainntnfs.GetTestTxidAndScript(miner) if err != nil { t.Fatalf("unable to create test addr: %v", err) } - - err = waitForMempoolTx(miner, txid) - if err != nil { + if err := chainntnfs.WaitForMempoolTx(miner, txid); err != nil { t.Fatalf("tx not relayed to miner: %v", err) } @@ -251,7 +170,7 @@ func testBatchConfirmationNotification(miner *rpctest.Harness, // verify they're each notified at the proper number of confirmations // below. for i, numConfs := range confSpread { - txid, pkScript, err := getTestTxIdAndScript(miner) + txid, pkScript, err := chainntnfs.GetTestTxidAndScript(miner) if err != nil { t.Fatalf("unable to create test addr: %v", err) } @@ -262,8 +181,7 @@ func testBatchConfirmationNotification(miner *rpctest.Harness, t.Fatalf("unable to register ntfn: %v", err) } confIntents[i] = confIntent - err = waitForMempoolTx(miner, txid) - if err != nil { + if err := chainntnfs.WaitForMempoolTx(miner, txid); err != nil { t.Fatalf("tx not relayed to miner: %v", err) } @@ -310,70 +228,6 @@ func testBatchConfirmationNotification(miner *rpctest.Harness, } } -func createSpendableOutput(miner *rpctest.Harness, - t *testing.T) (*wire.OutPoint, []byte) { - - txid, _, err := getTestTxIdAndScript(miner) - if err != nil { - t.Fatalf("unable to create test addr: %v", err) - } - - err = waitForMempoolTx(miner, txid) - if err != nil { - t.Fatalf("tx not relayed to miner: %v", err) - } - - // Mine a single block which should include that txid above. - if _, err := miner.Node.Generate(1); err != nil { - t.Fatalf("unable to generate single block: %v", err) - } - - // Now that we have the txid, fetch the transaction itself. - wrappedTx, err := miner.Node.GetRawTransaction(txid) - if err != nil { - t.Fatalf("unable to get new tx: %v", err) - } - tx := wrappedTx.MsgTx() - - // Locate the output index sent to us. We need this so we can construct - // a spending txn below. - outIndex := -1 - var pkScript []byte - for i, txOut := range tx.TxOut { - if bytes.Contains(txOut.PkScript, testAddr.ScriptAddress()) { - pkScript = txOut.PkScript - outIndex = i - break - } - } - if outIndex == -1 { - t.Fatalf("unable to locate new output") - } - - return wire.NewOutPoint(txid, uint32(outIndex)), pkScript -} - -func createSpendTx(outpoint *wire.OutPoint, pkScript []byte, - t *testing.T) *wire.MsgTx { - - spendingTx := wire.NewMsgTx(1) - spendingTx.AddTxIn(&wire.TxIn{ - PreviousOutPoint: *outpoint, - }) - spendingTx.AddTxOut(&wire.TxOut{ - Value: 1e8, - PkScript: pkScript, - }) - sigScript, err := txscript.SignatureScript(spendingTx, 0, pkScript, - txscript.SigHashAll, privKey, true) - if err != nil { - t.Fatalf("unable to sign tx: %v", err) - } - spendingTx.TxIn[0].SignatureScript = sigScript - - return spendingTx -} - func checkNotificationFields(ntfn *chainntnfs.SpendDetail, outpoint *wire.OutPoint, spenderSha *chainhash.Hash, height int32, t *testing.T) { @@ -407,7 +261,7 @@ func testSpendNotification(miner *rpctest.Harness, // concrete implementations. // // To do so, we first create a new output to our test target address. - outpoint, pkScript := createSpendableOutput(miner, t) + outpoint, pkScript := chainntnfs.CreateSpendableOutput(t, miner) _, currentHeight, err := miner.Node.GetBestBlock() if err != nil { @@ -432,7 +286,7 @@ func testSpendNotification(miner *rpctest.Harness, } // Next, create a new transaction spending that output. - spendingTx := createSpendTx(outpoint, pkScript, t) + spendingTx := chainntnfs.CreateSpendTx(t, outpoint, pkScript) // Broadcast our spending transaction. spenderSha, err := miner.Node.SendRawTransaction(spendingTx, true) @@ -440,8 +294,7 @@ func testSpendNotification(miner *rpctest.Harness, t.Fatalf("unable to broadcast tx: %v", err) } - err = waitForMempoolTx(miner, spenderSha) - if err != nil { + if err := chainntnfs.WaitForMempoolTx(miner, spenderSha); err != nil { t.Fatalf("tx not relayed to miner: %v", err) } @@ -565,14 +418,11 @@ func testMultiClientConfirmationNotification(miner *rpctest.Harness, // We'd like to test the case of a multiple clients registered to // receive a confirmation notification for the same transaction. - - txid, pkScript, err := getTestTxIdAndScript(miner) + txid, pkScript, err := chainntnfs.GetTestTxidAndScript(miner) if err != nil { t.Fatalf("unable to create test tx: %v", err) } - - err = waitForMempoolTx(miner, txid) - if err != nil { + if err := chainntnfs.WaitForMempoolTx(miner, txid); err != nil { t.Fatalf("tx not relayed to miner: %v", err) } @@ -632,14 +482,11 @@ func testTxConfirmedBeforeNtfnRegistration(miner *rpctest.Harness, // First, let's send some coins to "ourself", obtaining a txid. We're // spending from a coinbase output here, so we use the dedicated // function. - - txid3, pkScript3, err := getTestTxIdAndScript(miner) + txid3, pkScript3, err := chainntnfs.GetTestTxidAndScript(miner) if err != nil { t.Fatalf("unable to create test tx: %v", err) } - - err = waitForMempoolTx(miner, txid3) - if err != nil { + if err := chainntnfs.WaitForMempoolTx(miner, txid3); err != nil { t.Fatalf("tx not relayed to miner: %v", err) } @@ -653,23 +500,19 @@ func testTxConfirmedBeforeNtfnRegistration(miner *rpctest.Harness, t.Fatalf("unable to generate block: %v", err) } - txid1, pkScript1, err := getTestTxIdAndScript(miner) + txid1, pkScript1, err := chainntnfs.GetTestTxidAndScript(miner) if err != nil { t.Fatalf("unable to create test tx: %v", err) } - - err = waitForMempoolTx(miner, txid1) - if err != nil { + if err := chainntnfs.WaitForMempoolTx(miner, txid1); err != nil { t.Fatalf("tx not relayed to miner: %v", err) } - txid2, pkScript2, err := getTestTxIdAndScript(miner) + txid2, pkScript2, err := chainntnfs.GetTestTxidAndScript(miner) if err != nil { t.Fatalf("unable to create test tx: %v", err) } - - err = waitForMempoolTx(miner, txid2) - if err != nil { + if err := chainntnfs.WaitForMempoolTx(miner, txid2); err != nil { t.Fatalf("tx not relayed to miner: %v", err) } @@ -791,13 +634,11 @@ func testLazyNtfnConsumer(miner *rpctest.Harness, // Create a transaction to be notified about. We'll register for // notifications on this transaction but won't be prompt in checking them - txid, pkScript, err := getTestTxIdAndScript(miner) + txid, pkScript, err := chainntnfs.GetTestTxidAndScript(miner) if err != nil { t.Fatalf("unable to create test tx: %v", err) } - - err = waitForMempoolTx(miner, txid) - if err != nil { + if err := chainntnfs.WaitForMempoolTx(miner, txid); err != nil { t.Fatalf("tx not relayed to miner: %v", err) } @@ -829,13 +670,11 @@ func testLazyNtfnConsumer(miner *rpctest.Harness, // Now make another transaction, just because we haven't checked to see // if the first transaction has confirmed doesn't mean that we shouldn't // be able to see if this transaction confirms first - txid, pkScript, err = getTestTxIdAndScript(miner) + txid, pkScript, err = chainntnfs.GetTestTxidAndScript(miner) if err != nil { t.Fatalf("unable to create test tx: %v", err) } - - err = waitForMempoolTx(miner, txid) - if err != nil { + if err := chainntnfs.WaitForMempoolTx(miner, txid); err != nil { t.Fatalf("tx not relayed to miner: %v", err) } @@ -884,16 +723,15 @@ func testSpendBeforeNtfnRegistration(miner *rpctest.Harness, // concrete implementations. // // To do so, we first create a new output to our test target address. - outpoint, pkScript := createSpendableOutput(miner, t) + outpoint, pkScript := chainntnfs.CreateSpendableOutput(t, miner) // We'll then spend this output and broadcast the spend transaction. - spendingTx := createSpendTx(outpoint, pkScript, t) + spendingTx := chainntnfs.CreateSpendTx(t, outpoint, pkScript) spenderSha, err := miner.Node.SendRawTransaction(spendingTx, true) if err != nil { t.Fatalf("unable to broadcast tx: %v", err) } - err = waitForMempoolTx(miner, spenderSha) - if err != nil { + if err := chainntnfs.WaitForMempoolTx(miner, spenderSha); err != nil { t.Fatalf("tx not relayed to miner: %v", err) } @@ -989,7 +827,7 @@ func testCancelSpendNtfn(node *rpctest.Harness, // First, we'll start by creating a new output that we can spend // ourselves. - outpoint, pkScript := createSpendableOutput(node, t) + outpoint, pkScript := chainntnfs.CreateSpendableOutput(t, node) _, currentHeight, err := node.Node.GetBestBlock() if err != nil { @@ -1013,7 +851,7 @@ func testCancelSpendNtfn(node *rpctest.Harness, } // Next, create a new transaction spending that output. - spendingTx := createSpendTx(outpoint, pkScript, t) + spendingTx := chainntnfs.CreateSpendTx(t, outpoint, pkScript) // Before we broadcast the spending transaction, we'll cancel the // notification of the first client. @@ -1025,8 +863,7 @@ func testCancelSpendNtfn(node *rpctest.Harness, t.Fatalf("unable to broadcast tx: %v", err) } - err = waitForMempoolTx(node, spenderSha) - if err != nil { + if err := chainntnfs.WaitForMempoolTx(node, spenderSha); err != nil { t.Fatalf("tx not relayed to miner: %v", err) } @@ -1127,7 +964,7 @@ func testReorgConf(miner *rpctest.Harness, notifier chainntnfs.TestChainNotifier t *testing.T) { // Set up a new miner that we can use to cause a reorg. - miner2, err := rpctest.New(netParams, nil, nil) + miner2, err := rpctest.New(chainntnfs.NetParams, nil, nil) if err != nil { t.Fatalf("unable to create mining node: %v", err) } @@ -1169,13 +1006,11 @@ func testReorgConf(miner *rpctest.Harness, notifier chainntnfs.TestChainNotifier t.Fatalf("unable to remove node: %v", err) } - txid, pkScript, err := getTestTxIdAndScript(miner) + txid, pkScript, err := chainntnfs.GetTestTxidAndScript(miner) if err != nil { t.Fatalf("unable to create test tx: %v", err) } - - err = waitForMempoolTx(miner, txid) - if err != nil { + if err := chainntnfs.WaitForMempoolTx(miner, txid); err != nil { t.Fatalf("tx not relayed to miner: %v", err) } @@ -1257,9 +1092,7 @@ func testReorgConf(miner *rpctest.Harness, notifier chainntnfs.TestChainNotifier if err != nil { t.Fatalf("unable to get send tx: %v", err) } - - err = waitForMempoolTx(miner, txid) - if err != nil { + if err := chainntnfs.WaitForMempoolTx(miner, txid); err != nil { t.Fatalf("tx not relayed to miner: %v", err) } @@ -1470,7 +1303,7 @@ func testCatchUpOnMissedBlocksWithReorg(miner1 *rpctest.Harness, var wg sync.WaitGroup // Set up a new miner that we can use to cause a reorg. - miner2, err := rpctest.New(netParams, nil, nil) + miner2, err := rpctest.New(chainntnfs.NetParams, nil, nil) if err != nil { t.Fatalf("unable to create mining node: %v", err) } @@ -1629,13 +1462,12 @@ func testCatchUpOnMissedBlocksWithReorg(miner1 *rpctest.Harness, type testCase struct { name string - - test func(node *rpctest.Harness, notifier chainntnfs.TestChainNotifier, t *testing.T) + test func(node *rpctest.Harness, notifier chainntnfs.TestChainNotifier, + t *testing.T) } type blockCatchupTestCase struct { name string - test func(node *rpctest.Harness, notifier chainntnfs.TestChainNotifier, t *testing.T) } @@ -1722,25 +1554,14 @@ func TestInterfaces(t *testing.T) { // dedicated miner to generate blocks, cause re-orgs, etc. We'll set up // this node with a chain length of 125, so we have plenty of BTC to // play around with. - miner, err := rpctest.New(netParams, nil, nil) - if err != nil { - t.Fatalf("unable to create mining node: %v", err) - } - defer miner.TearDown() - if err := miner.SetUp(true, 25); err != nil { - t.Fatalf("unable to set up mining node: %v", err) - } + miner, tearDown := chainntnfs.NewMiner(t, nil, true, 25) + defer tearDown() rpcConfig := miner.RPCConfig() p2pAddr := miner.P2PAddress() - log.Printf("Running %v ChainNotifier interface tests\n", len(ntfnTests)) - var ( - notifier chainntnfs.TestChainNotifier - cleanUp func() + log.Printf("Running %v ChainNotifier interface tests", len(ntfnTests)) - newNotifier func() (chainntnfs.TestChainNotifier, error) - ) for _, notifierDriver := range chainntnfs.RegisteredNotifiers() { // Initialize a height hint cache for each notifier. tempDir, err := ioutil.TempDir("", "channeldb") @@ -1756,72 +1577,21 @@ func TestInterfaces(t *testing.T) { t.Fatalf("unable to create height hint cache: %v", err) } - notifierType := notifierDriver.NotifierType + var ( + cleanUp func() + newNotifier func() (chainntnfs.TestChainNotifier, error) + notifierType = notifierDriver.NotifierType + ) + switch notifierType { case "bitcoind": - // Start a bitcoind instance. - tempBitcoindDir, err := ioutil.TempDir("", "bitcoind") - if err != nil { - t.Fatalf("Unable to create temp dir: %v", err) - } - zmqBlockHost := "ipc:///" + tempBitcoindDir + "/blocks.socket" - zmqTxHost := "ipc:///" + tempBitcoindDir + "/tx.socket" - cleanUp1 := func() { - os.RemoveAll(tempBitcoindDir) - } - cleanUp = cleanUp1 - rpcPort := rand.Int()%(65536-1024) + 1024 - bitcoind := exec.Command( - "bitcoind", - "-datadir="+tempBitcoindDir, - "-regtest", - "-connect="+p2pAddr, - "-txindex", - "-rpcauth=weks:469e9bb14ab2360f8e226efed5ca6f"+ - "d$507c670e800a95284294edb5773b05544b"+ - "220110063096c221be9933c82d38e1", - fmt.Sprintf("-rpcport=%d", rpcPort), - "-disablewallet", - "-zmqpubrawblock="+zmqBlockHost, - "-zmqpubrawtx="+zmqTxHost, + var bitcoindConn *chain.BitcoindConn + bitcoindConn, cleanUp = chainntnfs.NewBitcoindBackend( + t, p2pAddr, true, ) - err = bitcoind.Start() - if err != nil { - cleanUp1() - t.Fatalf("Couldn't start bitcoind: %v", err) - } - cleanUp2 := func() { - bitcoind.Process.Kill() - bitcoind.Wait() - cleanUp1() - } - cleanUp = cleanUp2 - - // Wait for the bitcoind instance to start up. - time.Sleep(time.Second) - - host := fmt.Sprintf("127.0.0.1:%d", rpcPort) - chainConn, err := chain.NewBitcoindConn( - netParams, host, "weks", "weks", zmqBlockHost, - zmqTxHost, 100*time.Millisecond, - ) - if err != nil { - t.Fatalf("unable to establish connection to "+ - "bitcoind: %v", err) - } - if err := chainConn.Start(); err != nil { - t.Fatalf("unable to establish connection to "+ - "bitcoind: %v", err) - } - cleanUp3 := func() { - chainConn.Stop() - cleanUp2() - } - cleanUp = cleanUp3 - newNotifier = func() (chainntnfs.TestChainNotifier, error) { return bitcoindnotify.New( - chainConn, hintCache, hintCache, + bitcoindConn, hintCache, hintCache, ), nil } @@ -1832,45 +1602,11 @@ func TestInterfaces(t *testing.T) { ) } - cleanUp = func() {} - case "neutrino": - spvDir, err := ioutil.TempDir("", "neutrino") - if err != nil { - t.Fatalf("unable to create temp dir: %v", err) - } - - dbName := filepath.Join(spvDir, "neutrino.db") - spvDatabase, err := walletdb.Create("bdb", dbName) - if err != nil { - t.Fatalf("unable to create walletdb: %v", err) - } - - // Create an instance of neutrino connected to the - // running btcd instance. - spvConfig := neutrino.Config{ - DataDir: spvDir, - Database: spvDatabase, - ChainParams: *netParams, - ConnectPeers: []string{p2pAddr}, - } - spvNode, err := neutrino.NewChainService(spvConfig) - if err != nil { - t.Fatalf("unable to create neutrino: %v", err) - } - spvNode.Start() - - cleanUp = func() { - spvNode.Stop() - spvDatabase.Close() - os.RemoveAll(spvDir) - } - - // We'll also wait for the instance to sync up fully to - // the chain generated by the btcd instance. - for !spvNode.IsCurrent() { - time.Sleep(time.Millisecond * 100) - } + var spvNode *neutrino.ChainService + spvNode, cleanUp = chainntnfs.NewNeutrinoBackend( + t, p2pAddr, + ) newNotifier = func() (chainntnfs.TestChainNotifier, error) { return neutrinonotify.New( spvNode, hintCache, hintCache, @@ -1878,13 +1614,14 @@ func TestInterfaces(t *testing.T) { } } - t.Logf("Running ChainNotifier interface tests for: %v", notifierType) + log.Printf("Running ChainNotifier interface tests for: %v", + notifierType) - notifier, err = newNotifier() + notifier, err := newNotifier() if err != nil { - t.Fatalf("unable to create %v notifier: %v", notifierType, err) + t.Fatalf("unable to create %v notifier: %v", + notifierType, err) } - if err := notifier.Start(); err != nil { t.Fatalf("unable to start notifier %v: %v", notifierType, err) @@ -1897,7 +1634,6 @@ func TestInterfaces(t *testing.T) { success := t.Run(testName, func(t *testing.T) { ntfnTest.test(miner, notifier, t) }) - if !success { break } @@ -1905,8 +1641,8 @@ func TestInterfaces(t *testing.T) { notifier.Stop() - // Run catchup tests separately since they require - // restarting the notifier every time. + // Run catchup tests separately since they require restarting + // the notifier every time. for _, blockCatchupTest := range blockCatchupTests { notifier, err = newNotifier() if err != nil { @@ -1931,6 +1667,5 @@ func TestInterfaces(t *testing.T) { if cleanUp != nil { cleanUp() } - cleanUp = nil } } diff --git a/chainntnfs/test_utils.go b/chainntnfs/test_utils.go new file mode 100644 index 00000000..cfdafd85 --- /dev/null +++ b/chainntnfs/test_utils.go @@ -0,0 +1,268 @@ +// +build debug + +package chainntnfs + +import ( + "errors" + "fmt" + "io/ioutil" + "math/rand" + "os" + "os/exec" + "path/filepath" + "testing" + "time" + + "github.com/btcsuite/btcd/btcec" + "github.com/btcsuite/btcd/btcjson" + "github.com/btcsuite/btcd/chaincfg" + "github.com/btcsuite/btcd/chaincfg/chainhash" + "github.com/btcsuite/btcd/integration/rpctest" + "github.com/btcsuite/btcd/txscript" + "github.com/btcsuite/btcd/wire" + "github.com/btcsuite/btcutil" + "github.com/btcsuite/btcwallet/chain" + "github.com/btcsuite/btcwallet/walletdb" + "github.com/lightninglabs/neutrino" +) + +var ( + NetParams = &chaincfg.RegressionNetParams + + testPrivKey = []byte{ + 0x81, 0xb6, 0x37, 0xd8, 0xfc, 0xd2, 0xc6, 0xda, + 0x63, 0x59, 0xe6, 0x96, 0x31, 0x13, 0xa1, 0x17, + 0xd, 0xe7, 0x95, 0xe4, 0xb7, 0x25, 0xb8, 0x4d, + 0x1e, 0xb, 0x4c, 0xfd, 0x9e, 0xc5, 0x8c, 0xe9, + } + privKey, pubKey = btcec.PrivKeyFromBytes(btcec.S256(), testPrivKey) + addrPk, _ = btcutil.NewAddressPubKey( + pubKey.SerializeCompressed(), NetParams, + ) + testAddr = addrPk.AddressPubKeyHash() +) + +// GetTestTxidAndScript generate a new test transaction and returns its txid and +// the script of the output being generated. +func GetTestTxidAndScript(h *rpctest.Harness) (*chainhash.Hash, []byte, error) { + script, err := txscript.PayToAddrScript(testAddr) + if err != nil { + return nil, nil, err + } + + output := &wire.TxOut{Value: 2e8, PkScript: script} + txid, err := h.SendOutputs([]*wire.TxOut{output}, 10) + if err != nil { + return nil, nil, err + } + + return txid, script, nil +} + +// WaitForMempoolTx waits for the txid to be seen in the miner's mempool. +func WaitForMempoolTx(miner *rpctest.Harness, txid *chainhash.Hash) error { + timeout := time.After(10 * time.Second) + for { + // Check for the harness' knowledge of the txid. + tx, err := miner.Node.GetRawTransaction(txid) + if err != nil { + jsonErr, ok := err.(*btcjson.RPCError) + if ok && jsonErr.Code == btcjson.ErrRPCNoTxInfo { + continue + } + return err + } + + if tx != nil && tx.Hash().IsEqual(txid) { + break + } + + select { + case <-time.After(100 * time.Millisecond): + case <-timeout: + return errors.New("timed out waiting for tx") + } + } + + return nil +} + +// CreateSpendableOutput creates and returns an output that can be spent later +// on. +func CreateSpendableOutput(t *testing.T, miner *rpctest.Harness) (*wire.OutPoint, []byte) { + t.Helper() + + // Create a transaction that only has one output, the one destined for + // the recipient. + script, err := txscript.PayToAddrScript(testAddr) + if err != nil { + t.Fatalf("unable to create p2pkh script: %v", err) + } + output := &wire.TxOut{Value: 2e8, PkScript: script} + txid, err := miner.SendOutputsWithoutChange([]*wire.TxOut{output}, 10) + if err != nil { + t.Fatalf("unable to create tx: %v", err) + } + + // Mine the transaction to mark the output as spendable. + if err := WaitForMempoolTx(miner, txid); err != nil { + t.Fatalf("tx not relayed to miner: %v", err) + } + if _, err := miner.Node.Generate(1); err != nil { + t.Fatalf("unable to generate single block: %v", err) + } + + return wire.NewOutPoint(txid, 0), script +} + +// CreateSpendTx creates a transaction spending the specified output. +func CreateSpendTx(t *testing.T, outpoint *wire.OutPoint, pkScript []byte) *wire.MsgTx { + t.Helper() + + spendingTx := wire.NewMsgTx(1) + spendingTx.AddTxIn(&wire.TxIn{PreviousOutPoint: *outpoint}) + spendingTx.AddTxOut(&wire.TxOut{Value: 1e8, PkScript: pkScript}) + + sigScript, err := txscript.SignatureScript( + spendingTx, 0, pkScript, txscript.SigHashAll, privKey, true, + ) + if err != nil { + t.Fatalf("unable to sign tx: %v", err) + } + spendingTx.TxIn[0].SignatureScript = sigScript + + return spendingTx +} + +// NewMiner spawns testing harness backed by a btcd node that can serve as a +// miner. +func NewMiner(t *testing.T, extraArgs []string, createChain bool, + spendableOutputs uint32) (*rpctest.Harness, func()) { + + t.Helper() + + node, err := rpctest.New(NetParams, nil, extraArgs) + if err != nil { + t.Fatalf("unable to create backend node: %v", err) + } + if err := node.SetUp(createChain, spendableOutputs); err != nil { + node.TearDown() + t.Fatalf("unable to set up backend node: %v", err) + } + + return node, func() { node.TearDown() } +} + +// NewBitcoindBackend spawns a new bitcoind node that connects to a miner at the +// specified address. The txindex boolean can be set to determine whether the +// backend node should maintain a transaction index. A connection to the newly +// spawned bitcoind node is returned. +func NewBitcoindBackend(t *testing.T, minerAddr string, + txindex bool) (*chain.BitcoindConn, func()) { + + t.Helper() + + tempBitcoindDir, err := ioutil.TempDir("", "bitcoind") + if err != nil { + t.Fatalf("unable to create temp dir: %v", err) + } + + rpcPort := rand.Intn(65536-1024) + 1024 + zmqBlockHost := "ipc:///" + tempBitcoindDir + "/blocks.socket" + zmqTxHost := "ipc:///" + tempBitcoindDir + "/tx.socket" + + args := []string{ + "-connect=" + minerAddr, + "-datadir=" + tempBitcoindDir, + "-regtest", + "-rpcauth=weks:469e9bb14ab2360f8e226efed5ca6fd$507c670e800a952" + + "84294edb5773b05544b220110063096c221be9933c82d38e1", + fmt.Sprintf("-rpcport=%d", rpcPort), + "-disablewallet", + "-zmqpubrawblock=" + zmqBlockHost, + "-zmqpubrawtx=" + zmqTxHost, + } + if txindex { + args = append(args, "-txindex") + } + + bitcoind := exec.Command("bitcoind", args...) + if err := bitcoind.Start(); err != nil { + os.RemoveAll(tempBitcoindDir) + t.Fatalf("unable to start bitcoind: %v", err) + } + + // Wait for the bitcoind instance to start up. + time.Sleep(time.Second) + + host := fmt.Sprintf("127.0.0.1:%d", rpcPort) + conn, err := chain.NewBitcoindConn( + NetParams, host, "weks", "weks", zmqBlockHost, zmqTxHost, + 100*time.Millisecond, + ) + if err != nil { + bitcoind.Process.Kill() + bitcoind.Wait() + os.RemoveAll(tempBitcoindDir) + t.Fatalf("unable to establish connection to bitcoind: %v", err) + } + if err := conn.Start(); err != nil { + bitcoind.Process.Kill() + bitcoind.Wait() + os.RemoveAll(tempBitcoindDir) + t.Fatalf("unable to establish connection to bitcoind: %v", err) + } + + return conn, func() { + conn.Stop() + bitcoind.Process.Kill() + bitcoind.Wait() + os.RemoveAll(tempBitcoindDir) + } +} + +// NewNeutrinoBackend spawns a new neutrino node that connects to a miner at +// the specified address. +func NewNeutrinoBackend(t *testing.T, minerAddr string) (*neutrino.ChainService, func()) { + t.Helper() + + spvDir, err := ioutil.TempDir("", "neutrino") + if err != nil { + t.Fatalf("unable to create temp dir: %v", err) + } + + dbName := filepath.Join(spvDir, "neutrino.db") + spvDatabase, err := walletdb.Create("bdb", dbName) + if err != nil { + os.RemoveAll(spvDir) + t.Fatalf("unable to create walletdb: %v", err) + } + + // Create an instance of neutrino connected to the running btcd + // instance. + spvConfig := neutrino.Config{ + DataDir: spvDir, + Database: spvDatabase, + ChainParams: *NetParams, + ConnectPeers: []string{minerAddr}, + } + spvNode, err := neutrino.NewChainService(spvConfig) + if err != nil { + os.RemoveAll(spvDir) + spvDatabase.Close() + t.Fatalf("unable to create neutrino: %v", err) + } + + // We'll also wait for the instance to sync up fully to the chain + // generated by the btcd instance. + spvNode.Start() + for !spvNode.IsCurrent() { + time.Sleep(time.Millisecond * 100) + } + + return spvNode, func() { + spvNode.Stop() + spvDatabase.Close() + os.RemoveAll(spvDir) + } +} From 7895f01e2e4af8a98931e57f14b0895207f4d8c7 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 18 Jul 2018 14:01:28 -0700 Subject: [PATCH 02/19] chainntnfs: ensure proper fallback to scanning tx manually In this commit, we address a bug where it's possible that we still attempt to manually scan for a transaction to determine whether it's been included in the chain even after successfully checking the txindex and not finding it there. Now, we'll short-circuit this process by exiting early if the txindex lookup was successful but the transaction in question was not found. Otherwise, we'll fall back to the manual scan. --- chainntnfs/bitcoindnotify/bitcoind.go | 124 +++++++++---- chainntnfs/bitcoindnotify/bitcoind_test.go | 204 +++++++++++++++++++++ chainntnfs/btcdnotify/btcd.go | 125 +++++++++---- chainntnfs/btcdnotify/btcd_test.go | 180 ++++++++++++++++++ 4 files changed, 554 insertions(+), 79 deletions(-) create mode 100644 chainntnfs/bitcoindnotify/bitcoind_test.go create mode 100644 chainntnfs/btcdnotify/btcd_test.go diff --git a/chainntnfs/bitcoindnotify/bitcoind.go b/chainntnfs/bitcoindnotify/bitcoind.go index 134c591b..16addc42 100644 --- a/chainntnfs/bitcoindnotify/bitcoind.go +++ b/chainntnfs/bitcoindnotify/bitcoind.go @@ -3,6 +3,7 @@ package bitcoindnotify import ( "errors" "fmt" + "strings" "sync" "sync/atomic" "time" @@ -27,6 +28,31 @@ const ( reorgSafetyLimit = 100 ) +// txConfStatus denotes the status of a transaction's lookup. +type txConfStatus uint8 + +const ( + // txFoundMempool denotes that the transaction was found within the + // backend node's mempool. + txFoundMempool txConfStatus = iota + + // txFoundIndex denotes that the transaction was found within the + // backend node's txindex. + txFoundIndex + + // txFoundIndex denotes that the transaction was not found within the + // backend node's txindex. + txNotFoundIndex + + // txFoundManually denotes that the transaction was found within the + // chain by scanning for it manually. + txFoundManually + + // txFoundManually denotes that the transaction was not found within the + // chain by scanning for it manually. + txNotFoundManually +) + var ( // ErrChainNotifierShuttingDown is used when we are trying to // measure a spend notification when notifier is already stopped. @@ -268,7 +294,7 @@ out: go func() { defer b.wg.Done() - confDetails, err := b.historicalConfDetails( + confDetails, _, err := b.historicalConfDetails( msg.TxID, msg.heightHint, currentHeight, ) @@ -447,23 +473,32 @@ func (b *BitcoindNotifier) handleRelevantTx(tx chain.RelevantTx, bestHeight int3 // 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, - heightHint, currentHeight uint32) (*chainntnfs.TxConfirmation, error) { + heightHint, currentHeight uint32) (*chainntnfs.TxConfirmation, + txConfStatus, error) { - // First, we'll attempt to retrieve the transaction details using the - // backend node's transaction index. - txConf, err := b.confDetailsFromTxIndex(txid) - if err != nil { - return nil, err + // We'll first attempt to retrieve the transaction using the node's + // txindex. + txConf, txStatus, err := b.confDetailsFromTxIndex(txid) + + // We'll then check the status of the transaction lookup returned to + // determine whether we should proceed with any fallback methods. + switch { + // The transaction was found within the node's mempool. + case txStatus == txFoundMempool: + + // The transaction was found within the node's txindex. + case txStatus == txFoundIndex: + + // The transaction was not found within the node's mempool or txindex. + case err == nil && txStatus == txNotFoundIndex: + + // We failed to look up the transaction within the node's mempool or + // txindex, so we'll proceed to scan the chain manually. + default: + return b.confDetailsManually(txid, heightHint, currentHeight) } - if txConf != nil { - return txConf, nil - } - - // If the backend node's transaction index is not enabled, then we'll - // fall back to manually scanning the chain's blocks, looking for the - // block where the transaction was included in. - return b.confDetailsManually(txid, heightHint, currentHeight) + return txConf, txStatus, nil } // confDetailsFromTxIndex looks up whether a transaction is already included @@ -471,26 +506,33 @@ func (b *BitcoindNotifier) historicalConfDetails(txid *chainhash.Hash, // If the transaction is found, its confirmation details are returned. // Otherwise, nil is returned. func (b *BitcoindNotifier) confDetailsFromTxIndex(txid *chainhash.Hash, -) (*chainntnfs.TxConfirmation, error) { +) (*chainntnfs.TxConfirmation, txConfStatus, error) { // If the transaction has some or all of its confirmations required, // then we may be able to dispatch it immediately. tx, err := b.chainConn.GetRawTransactionVerbose(txid) if err != nil { - // Avoid returning an error if the transaction index is not - // enabled to proceed with fallback methods. + // If the transaction lookup was succesful, but it wasn't found + // within the index itself, then we can exit early. We'll also + // need to look at the error message returned as the error code + // is used for multiple errors. + txNotFoundErr := "No such mempool or blockchain transaction" jsonErr, ok := err.(*btcjson.RPCError) - if !ok || jsonErr.Code != btcjson.ErrRPCNoTxInfo { - return nil, fmt.Errorf("unable to query for txid "+ - "%v: %v", txid, err) + if ok && jsonErr.Code == btcjson.ErrRPCNoTxInfo && + strings.Contains(jsonErr.Message, txNotFoundErr) { + + return nil, txNotFoundIndex, nil } + + return nil, txNotFoundIndex, fmt.Errorf("unable to query for "+ + "txid %v: %v", txid, err) } // Make sure we actually retrieved a transaction that is included in a // block. Without this, we won't be able to retrieve its confirmation // details. if tx == nil || tx.BlockHash == "" { - return nil, nil + return nil, txFoundMempool, nil } // As we need to fully populate the returned TxConfirmation struct, @@ -498,14 +540,16 @@ func (b *BitcoindNotifier) confDetailsFromTxIndex(txid *chainhash.Hash, // locate its exact index within the block. blockHash, err := chainhash.NewHashFromStr(tx.BlockHash) if err != nil { - return nil, fmt.Errorf("unable to get block hash %v for "+ - "historical dispatch: %v", tx.BlockHash, err) + return nil, txNotFoundIndex, fmt.Errorf("unable to get block "+ + "hash %v for historical dispatch: %v", tx.BlockHash, + err) } block, err := b.chainConn.GetBlockVerbose(blockHash) if err != nil { - return nil, fmt.Errorf("unable to get block with hash %v for "+ - "historical dispatch: %v", blockHash, err) + return nil, txNotFoundIndex, fmt.Errorf("unable to get block "+ + "with hash %v for historical dispatch: %v", blockHash, + err) } // If the block was obtained, locate the transaction's index within the @@ -513,18 +557,19 @@ func (b *BitcoindNotifier) confDetailsFromTxIndex(txid *chainhash.Hash, targetTxidStr := txid.String() for txIndex, txHash := range block.Tx { if txHash == targetTxidStr { - return &chainntnfs.TxConfirmation{ + details := &chainntnfs.TxConfirmation{ BlockHash: blockHash, BlockHeight: uint32(block.Height), TxIndex: uint32(txIndex), - }, nil + } + return details, txFoundIndex, nil } } // We return an error because we should have found the transaction // within the block, but didn't. - return nil, fmt.Errorf("unable to locate tx %v in block %v", txid, - blockHash) + return nil, txNotFoundIndex, fmt.Errorf("unable to locate tx %v in "+ + "block %v", txid, blockHash) } // confDetailsManually looks up whether a transaction is already included in a @@ -533,7 +578,7 @@ func (b *BitcoindNotifier) confDetailsFromTxIndex(txid *chainhash.Hash, // 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, - heightHint, currentHeight uint32) (*chainntnfs.TxConfirmation, error) { + heightHint, currentHeight uint32) (*chainntnfs.TxConfirmation, txConfStatus, error) { targetTxidStr := txid.String() @@ -544,38 +589,39 @@ func (b *BitcoindNotifier) confDetailsManually(txid *chainhash.Hash, // processing the next height. select { case <-b.quit: - return nil, ErrChainNotifierShuttingDown + return nil, txNotFoundManually, ErrChainNotifierShuttingDown default: } blockHash, err := b.chainConn.GetBlockHash(int64(height)) if err != nil { - return nil, fmt.Errorf("unable to get hash from block "+ - "with height %d", height) + return nil, txNotFoundManually, fmt.Errorf("unable to "+ + "get hash from block with height %d", height) } block, err := b.chainConn.GetBlockVerbose(blockHash) if err != nil { - return nil, fmt.Errorf("unable to get block with hash "+ - "%v: %v", blockHash, err) + return nil, 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 { - return &chainntnfs.TxConfirmation{ + details := &chainntnfs.TxConfirmation{ BlockHash: blockHash, BlockHeight: height, TxIndex: uint32(txIndex), - }, nil + } + return details, txFoundManually, nil } } } // If we reach here, then we were not able to find the transaction // within a block, so we avoid returning an error. - return nil, nil + return nil, txNotFoundManually, nil } // handleBlockConnected applies a chain update for a new block. Any watched diff --git a/chainntnfs/bitcoindnotify/bitcoind_test.go b/chainntnfs/bitcoindnotify/bitcoind_test.go new file mode 100644 index 00000000..c3f863ba --- /dev/null +++ b/chainntnfs/bitcoindnotify/bitcoind_test.go @@ -0,0 +1,204 @@ +// +build debug + +package bitcoindnotify + +import ( + "testing" + "time" + + "github.com/btcsuite/btcd/chaincfg/chainhash" + "github.com/btcsuite/btcd/integration/rpctest" + "github.com/btcsuite/btcwallet/chain" + "github.com/lightningnetwork/lnd/chainntnfs" +) + +// setUpNotifier is a helper function to start a new notifier backed by a +// bitcoind driver. +func setUpNotifier(t *testing.T, bitcoindConn *chain.BitcoindConn) *BitcoindNotifier { + t.Helper() + + notifier := New(bitcoindConn) + if err := notifier.Start(); err != nil { + t.Fatalf("unable to start notifier: %v", err) + } + + return notifier +} + +// syncNotifierWithMiner is a helper method that attempts to wait until the +// notifier is synced (in terms of the chain) with the miner. +func syncNotifierWithMiner(t *testing.T, notifier *BitcoindNotifier, + miner *rpctest.Harness) uint32 { + + t.Helper() + + _, minerHeight, err := miner.Node.GetBestBlock() + if err != nil { + t.Fatalf("unable to retrieve miner's current height: %v", err) + } + + timeout := time.After(10 * time.Second) + for { + _, bitcoindHeight, err := notifier.chainConn.GetBestBlock() + if err != nil { + t.Fatalf("unable to retrieve bitcoind's current "+ + "height: %v", err) + } + + if bitcoindHeight == minerHeight { + return uint32(bitcoindHeight) + } + + select { + case <-time.After(100 * time.Millisecond): + case <-timeout: + t.Fatalf("timed out waiting to sync notifier") + } + } +} + +// TestHistoricalConfDetailsTxIndex ensures that we correctly retrieve +// historical confirmation details using the backend node's txindex. +func TestHistoricalConfDetailsTxIndex(t *testing.T) { + miner, tearDown := chainntnfs.NewMiner( + t, []string{"--txindex"}, true, 25, + ) + defer tearDown() + + bitcoindConn, cleanUp := chainntnfs.NewBitcoindBackend( + t, miner.P2PAddress(), true, + ) + defer cleanUp() + + notifier := setUpNotifier(t, bitcoindConn) + defer notifier.Stop() + + syncNotifierWithMiner(t, notifier, miner) + + // 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) + if err != nil { + t.Fatalf("unable to retrieve historical conf details: %v", err) + } + + switch txStatus { + case txNotFoundIndex: + case txNotFoundManually: + t.Fatal("should not have proceeded with fallback method, but did") + default: + t.Fatal("should not have found non-existent transaction, but did") + } + + // Now, we'll create a test transaction, confirm it, and attempt to + // retrieve its confirmation details. + txid, _, 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) + } + if _, err := miner.Node.Generate(1); err != nil { + t.Fatalf("unable to generate block: %v", err) + } + + // Ensure the notifier and miner are synced to the same height to ensure + // the txindex includes the transaction just mined. + syncNotifierWithMiner(t, notifier, miner) + + _, txStatus, err = notifier.historicalConfDetails(txid, 0, 0) + if err != nil { + t.Fatalf("unable to retrieve historical conf details: %v", err) + } + + // Since the backend node's txindex is enabled and the transaction has + // confirmed, we should be able to retrieve it using the txindex. + switch txStatus { + case txFoundIndex: + default: + t.Fatal("should have found the transaction within the " + + "txindex, but did not") + } +} + +// TestHistoricalConfDetailsNoTxIndex ensures that we correctly retrieve +// 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) + defer tearDown() + + bitcoindConn, cleanUp := chainntnfs.NewBitcoindBackend( + t, miner.P2PAddress(), false, + ) + defer cleanUp() + + notifier := setUpNotifier(t, bitcoindConn) + defer notifier.Stop() + + // 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 + broadcastHeight := syncNotifierWithMiner(t, notifier, miner) + _, txStatus, err := notifier.historicalConfDetails( + &zeroHash, uint32(broadcastHeight), uint32(broadcastHeight), + ) + if err != nil { + t.Fatalf("unable to retrieve historical conf details: %v", err) + } + + switch txStatus { + case txNotFoundManually: + case txNotFoundIndex: + t.Fatal("should have proceeded with fallback method, but did not") + default: + t.Fatal("should not have found non-existent transaction, but did") + } + + // Now, we'll create a test transaction and attempt to retrieve its + // confirmation details. In order to fall back to manually scanning the + // chain, the transaction must be in the chain and not contain any + // unspent outputs. To ensure this, we'll create a transaction with only + // one output, which we will manually spend. The backend node's + // transaction index should also be disabled, which we've already + // ensured above. + // + // TODO(wilmer): add mempool case once trickle timeout can be specified + // in btcd. + output, pkScript := chainntnfs.CreateSpendableOutput(t, miner) + spendTx := chainntnfs.CreateSpendTx(t, output, pkScript) + spendTxHash, err := miner.Node.SendRawTransaction(spendTx, true) + if err != nil { + t.Fatalf("unable to broadcast tx: %v", err) + } + if err := chainntnfs.WaitForMempoolTx(miner, spendTxHash); err != nil { + t.Fatalf("tx not relayed to miner: %v", err) + } + if _, err := miner.Node.Generate(1); err != nil { + t.Fatalf("unable to generate block: %v", err) + } + + // Ensure the notifier and miner are synced to the same height to ensure + // we can find the transaction when manually scanning the chain. + currentHeight := syncNotifierWithMiner(t, notifier, miner) + _, txStatus, err = notifier.historicalConfDetails( + &output.Hash, uint32(broadcastHeight), uint32(currentHeight), + ) + if err != nil { + t.Fatalf("unable to retrieve historical conf details: %v", err) + } + + // Since the backend node's txindex is disabled and the transaction has + // confirmed, we should be able to find it by falling back to scanning + // the chain manually. + switch txStatus { + case txFoundManually: + default: + t.Fatal("should have found the transaction by manually " + + "scanning the chain, but did not") + } +} diff --git a/chainntnfs/btcdnotify/btcd.go b/chainntnfs/btcdnotify/btcd.go index 22febdd2..66c5747d 100644 --- a/chainntnfs/btcdnotify/btcd.go +++ b/chainntnfs/btcdnotify/btcd.go @@ -3,6 +3,7 @@ package btcdnotify import ( "errors" "fmt" + "strings" "sync" "sync/atomic" "time" @@ -26,6 +27,31 @@ const ( reorgSafetyLimit = 100 ) +// txConfStatus denotes the status of a transaction's lookup. +type txConfStatus uint8 + +const ( + // txFoundMempool denotes that the transaction was found within the + // backend node's mempool. + txFoundMempool txConfStatus = iota + + // txFoundIndex denotes that the transaction was found within the + // backend node's txindex. + txFoundIndex + + // txFoundIndex denotes that the transaction was not found within the + // backend node's txindex. + txNotFoundIndex + + // txFoundManually denotes that the transaction was found within the + // chain by scanning for it manually. + txFoundManually + + // txFoundManually denotes that the transaction was not found within the + // chain by scanning for it manually. + txNotFoundManually +) + var ( // ErrChainNotifierShuttingDown is used when we are trying to // measure a spend notification when notifier is already stopped. @@ -337,7 +363,7 @@ out: go func() { defer b.wg.Done() - confDetails, err := b.historicalConfDetails( + confDetails, _, err := b.historicalConfDetails( msg.TxID, msg.heightHint, bestHeight, ) @@ -516,23 +542,31 @@ out: // 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 *BtcdNotifier) historicalConfDetails(txid *chainhash.Hash, - heightHint, currentHeight uint32) (*chainntnfs.TxConfirmation, error) { + heightHint, currentHeight uint32) (*chainntnfs.TxConfirmation, txConfStatus, error) { - // First, we'll attempt to retrieve the transaction details using the - // backend node's transaction index. - txConf, err := b.confDetailsFromTxIndex(txid) - if err != nil { - return nil, err + // We'll first attempt to retrieve the transaction using the node's + // txindex. + txConf, txStatus, err := b.confDetailsFromTxIndex(txid) + + // We'll then check the status of the transaction lookup returned to + // determine whether we should proceed with any fallback methods. + switch { + // The transaction was found within the node's mempool. + case txStatus == txFoundMempool: + + // The transaction was found within the node's txindex. + case txStatus == txFoundIndex: + + // The transaction was not found within the node's mempool or txindex. + case txStatus == txNotFoundIndex && err == nil: + + // We failed to look up the transaction within the node's mempool or + // txindex, so we'll proceed to scan the chain manually. + default: + return b.confDetailsManually(txid, heightHint, currentHeight) } - if txConf != nil { - return txConf, nil - } - - // If the backend node's transaction index is not enabled, then we'll - // fall back to manually scanning the chain's blocks, looking for the - // block where the transaction was included in. - return b.confDetailsManually(txid, heightHint, currentHeight) + return txConf, txStatus, nil } // confDetailsFromTxIndex looks up whether a transaction is already included @@ -540,26 +574,33 @@ func (b *BtcdNotifier) historicalConfDetails(txid *chainhash.Hash, // If the transaction is found, its confirmation details are returned. // Otherwise, nil is returned. func (b *BtcdNotifier) confDetailsFromTxIndex(txid *chainhash.Hash, -) (*chainntnfs.TxConfirmation, error) { +) (*chainntnfs.TxConfirmation, txConfStatus, error) { // If the transaction has some or all of its confirmations required, // then we may be able to dispatch it immediately. tx, err := b.chainConn.GetRawTransactionVerbose(txid) if err != nil { - // Avoid returning an error if the transaction index is not - // enabled to proceed with fallback methods. + // If the transaction lookup was succesful, but it wasn't found + // within the index itself, then we can exit early. We'll also + // need to look at the error message returned as the error code + // is used for multiple errors. + txNotFoundErr := "No information available about transaction" jsonErr, ok := err.(*btcjson.RPCError) - if !ok || jsonErr.Code != btcjson.ErrRPCNoTxInfo { - return nil, fmt.Errorf("unable to query for txid "+ - "%v: %v", txid, err) + if ok && jsonErr.Code == btcjson.ErrRPCNoTxInfo && + strings.Contains(jsonErr.Message, txNotFoundErr) { + + return nil, txNotFoundIndex, nil } + + return nil, txNotFoundIndex, fmt.Errorf("unable to query for "+ + "txid %v: %v", txid, err) } // Make sure we actually retrieved a transaction that is included in a // block. Without this, we won't be able to retrieve its confirmation // details. if tx == nil || tx.BlockHash == "" { - return nil, nil + return nil, txFoundMempool, nil } // As we need to fully populate the returned TxConfirmation struct, @@ -567,14 +608,16 @@ func (b *BtcdNotifier) confDetailsFromTxIndex(txid *chainhash.Hash, // locate its exact index within the block. blockHash, err := chainhash.NewHashFromStr(tx.BlockHash) if err != nil { - return nil, fmt.Errorf("unable to get block hash %v for "+ - "historical dispatch: %v", tx.BlockHash, err) + return nil, txNotFoundIndex, fmt.Errorf("unable to get block "+ + "hash %v for historical dispatch: %v", tx.BlockHash, + err) } block, err := b.chainConn.GetBlockVerbose(blockHash) if err != nil { - return nil, fmt.Errorf("unable to get block with hash %v for "+ - "historical dispatch: %v", blockHash, err) + return nil, txNotFoundIndex, fmt.Errorf("unable to get block "+ + "with hash %v for historical dispatch: %v", blockHash, + err) } // If the block was obtained, locate the transaction's index within the @@ -582,18 +625,19 @@ func (b *BtcdNotifier) confDetailsFromTxIndex(txid *chainhash.Hash, targetTxidStr := txid.String() for txIndex, txHash := range block.Tx { if txHash == targetTxidStr { - return &chainntnfs.TxConfirmation{ + details := &chainntnfs.TxConfirmation{ BlockHash: blockHash, BlockHeight: uint32(block.Height), TxIndex: uint32(txIndex), - }, nil + } + return details, txFoundIndex, nil } } // We return an error because we should have found the transaction // within the block, but didn't. - return nil, fmt.Errorf("unable to locate tx %v in block %v", txid, - blockHash) + return nil, txNotFoundIndex, fmt.Errorf("unable to locate tx %v in "+ + "block %v", txid, blockHash) } // confDetailsManually looks up whether a transaction is already included in a @@ -601,8 +645,8 @@ func (b *BtcdNotifier) confDetailsFromTxIndex(txid *chainhash.Hash, // 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 *BtcdNotifier) confDetailsManually(txid *chainhash.Hash, - heightHint, currentHeight uint32) (*chainntnfs.TxConfirmation, error) { +func (b *BtcdNotifier) confDetailsManually(txid *chainhash.Hash, heightHint, + currentHeight uint32) (*chainntnfs.TxConfirmation, txConfStatus, error) { targetTxidStr := txid.String() @@ -613,39 +657,40 @@ func (b *BtcdNotifier) confDetailsManually(txid *chainhash.Hash, // processing the next height. select { case <-b.quit: - return nil, ErrChainNotifierShuttingDown + return nil, txNotFoundManually, ErrChainNotifierShuttingDown default: } blockHash, err := b.chainConn.GetBlockHash(int64(height)) if err != nil { - return nil, fmt.Errorf("unable to get hash from block "+ - "with height %d", height) + return nil, txNotFoundManually, fmt.Errorf("unable to "+ + "get hash from block with height %d", height) } // TODO: fetch the neutrino filters instead. block, err := b.chainConn.GetBlockVerbose(blockHash) if err != nil { - return nil, fmt.Errorf("unable to get block with hash "+ - "%v: %v", blockHash, err) + return nil, 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 { - return &chainntnfs.TxConfirmation{ + details := &chainntnfs.TxConfirmation{ BlockHash: blockHash, BlockHeight: height, TxIndex: uint32(txIndex), - }, nil + } + return details, txFoundManually, nil } } } // If we reach here, then we were not able to find the transaction // within a block, so we avoid returning an error. - return nil, nil + return nil, txNotFoundManually, nil } // handleBlockConnected applies a chain update for a new block. Any watched diff --git a/chainntnfs/btcdnotify/btcd_test.go b/chainntnfs/btcdnotify/btcd_test.go new file mode 100644 index 00000000..64b7fadd --- /dev/null +++ b/chainntnfs/btcdnotify/btcd_test.go @@ -0,0 +1,180 @@ +// +build debug + +package btcdnotify + +import ( + "testing" + + "github.com/btcsuite/btcd/chaincfg/chainhash" + "github.com/btcsuite/btcd/integration/rpctest" + "github.com/lightningnetwork/lnd/chainntnfs" +) + +// setUpNotifier is a helper function to start a new notifier backed by a btcd +// driver. +func setUpNotifier(t *testing.T, h *rpctest.Harness) *BtcdNotifier { + rpcCfg := h.RPCConfig() + notifier, err := New(&rpcCfg) + if err != nil { + t.Fatalf("unable to create notifier: %v", err) + } + if err := notifier.Start(); err != nil { + t.Fatalf("unable to start notifier: %v", err) + } + + return notifier +} + +// TestHistoricalConfDetailsTxIndex ensures that we correctly retrieve +// historical confirmation details using the backend node's txindex. +func TestHistoricalConfDetailsTxIndex(t *testing.T) { + t.Parallel() + + harness, tearDown := chainntnfs.NewMiner( + t, []string{"--txindex"}, true, 25, + ) + defer tearDown() + + notifier := setUpNotifier(t, harness) + defer notifier.Stop() + + // 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) + if err != nil { + t.Fatalf("unable to retrieve historical conf details: %v", err) + } + + switch txStatus { + case txNotFoundIndex: + case txNotFoundManually: + t.Fatal("should not have proceeded with fallback method, but did") + default: + t.Fatal("should not have found non-existent transaction, but did") + } + + // Now, we'll create a test transaction and attempt to retrieve its + // confirmation details. + txid, _, err := chainntnfs.GetTestTxidAndScript(harness) + if err != nil { + t.Fatalf("unable to create tx: %v", err) + } + if err := chainntnfs.WaitForMempoolTx(harness, txid); err != nil { + t.Fatalf("unable to find tx in the mempool: %v", err) + } + + _, txStatus, err = notifier.historicalConfDetails(txid, 0, 0) + if err != nil { + t.Fatalf("unable to retrieve historical conf details: %v", err) + } + + // Since it has yet to be included in a block, it should have been found + // within the mempool. + switch txStatus { + case txFoundMempool: + default: + t.Fatal("should have found the transaction within the " + + "mempool, but did not") + } + + // We'll now confirm this transaction and re-attempt to retrieve its + // confirmation details. + if _, err := harness.Node.Generate(1); err != nil { + t.Fatalf("unable to generate block: %v", err) + } + + _, txStatus, err = notifier.historicalConfDetails(txid, 0, 0) + if err != nil { + t.Fatalf("unable to retrieve historical conf details: %v", err) + } + + // Since the backend node's txindex is enabled and the transaction has + // confirmed, we should be able to retrieve it using the txindex. + switch txStatus { + case txFoundIndex: + default: + t.Fatal("should have found the transaction within the " + + "txindex, but did not") + } +} + +// TestHistoricalConfDetailsNoTxIndex ensures that we correctly retrieve +// historical confirmation details using the set of fallback methods when the +// backend node's txindex is disabled. +func TestHistoricalConfDetailsNoTxIndex(t *testing.T) { + t.Parallel() + + harness, tearDown := chainntnfs.NewMiner(t, nil, true, 25) + defer tearDown() + + notifier := setUpNotifier(t, harness) + defer notifier.Stop() + + // 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 + _, txStatus, err := notifier.historicalConfDetails(&zeroHash, 0, 0) + if err != nil { + t.Fatalf("unable to retrieve historical conf details: %v", err) + } + + switch txStatus { + case txNotFoundManually: + case txNotFoundIndex: + t.Fatal("should have proceeded with fallback method, but did not") + default: + t.Fatal("should not have found non-existent transaction, but did") + } + + // Now, we'll create a test transaction and attempt to retrieve its + // confirmation details. We'll note its broadcast height to use as the + // height hint when manually scanning the chain. + _, currentHeight, err := harness.Node.GetBestBlock() + if err != nil { + t.Fatalf("unable to retrieve current height: %v", err) + } + + txid, _, err := chainntnfs.GetTestTxidAndScript(harness) + if err != nil { + t.Fatalf("unable to create tx: %v", err) + } + if err := chainntnfs.WaitForMempoolTx(harness, txid); err != nil { + t.Fatalf("unable to find tx in the mempool: %v", err) + } + + _, txStatus, err = notifier.historicalConfDetails(txid, 0, 0) + if err != nil { + t.Fatalf("unable to retrieve historical conf details: %v", err) + } + + // Since it has yet to be included in a block, it should have been found + // within the mempool. + if txStatus != txFoundMempool { + t.Fatal("should have found the transaction within the " + + "mempool, but did not") + } + + // We'll now confirm this transaction and re-attempt to retrieve its + // confirmation details. + if _, err := harness.Node.Generate(1); err != nil { + t.Fatalf("unable to generate block: %v", err) + } + + _, txStatus, err = notifier.historicalConfDetails( + txid, uint32(currentHeight), uint32(currentHeight)+1, + ) + if err != nil { + t.Fatalf("unable to retrieve historical conf details: %v", err) + } + + // Since the backend node's txindex is disabled and the transaction has + // confirmed, we should be able to find it by falling back to scanning + // the chain manually. + if txStatus != txFoundManually { + t.Fatal("should have found the transaction by manually " + + "scanning the chain, but did not") + } +} From cb688de7ad8ebbc08e8da11f53b40e8453efae1c Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 16 Aug 2018 19:41:32 -0700 Subject: [PATCH 03/19] chainntnfs: guard test chain notifier under debug flag --- chainntnfs/bitcoindnotify/bitcoind_debug.go | 2 ++ chainntnfs/btcdnotify/btcd_debug.go | 2 ++ chainntnfs/interface_debug.go | 2 ++ chainntnfs/neutrinonotify/neutrino_debug.go | 2 ++ 4 files changed, 8 insertions(+) diff --git a/chainntnfs/bitcoindnotify/bitcoind_debug.go b/chainntnfs/bitcoindnotify/bitcoind_debug.go index 85842ca5..04cd030c 100644 --- a/chainntnfs/bitcoindnotify/bitcoind_debug.go +++ b/chainntnfs/bitcoindnotify/bitcoind_debug.go @@ -1,3 +1,5 @@ +// +build debug + package bitcoindnotify import ( diff --git a/chainntnfs/btcdnotify/btcd_debug.go b/chainntnfs/btcdnotify/btcd_debug.go index 47136f0b..1c91d3f6 100644 --- a/chainntnfs/btcdnotify/btcd_debug.go +++ b/chainntnfs/btcdnotify/btcd_debug.go @@ -1,3 +1,5 @@ +// +build debug + package btcdnotify import ( diff --git a/chainntnfs/interface_debug.go b/chainntnfs/interface_debug.go index f3e6eaf4..aeb3461b 100644 --- a/chainntnfs/interface_debug.go +++ b/chainntnfs/interface_debug.go @@ -1,3 +1,5 @@ +// +build debug + package chainntnfs import "github.com/btcsuite/btcd/chaincfg/chainhash" diff --git a/chainntnfs/neutrinonotify/neutrino_debug.go b/chainntnfs/neutrinonotify/neutrino_debug.go index ece4af27..19085c38 100644 --- a/chainntnfs/neutrinonotify/neutrino_debug.go +++ b/chainntnfs/neutrinonotify/neutrino_debug.go @@ -1,3 +1,5 @@ +// +build debug + package neutrinonotify import ( From 191c4f3e2842b0db5a9b885e10fa5959aaedcdc4 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 16 Aug 2018 19:48:25 -0700 Subject: [PATCH 04/19] multi: enable txindex on miner harness --- chainntnfs/interface_test.go | 4 ++-- lnd_test.go | 4 ++-- lnwallet/interface_test.go | 4 ++-- routing/chainview/interface_test.go | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/chainntnfs/interface_test.go b/chainntnfs/interface_test.go index b4fb5cc9..bdd4dca5 100644 --- a/chainntnfs/interface_test.go +++ b/chainntnfs/interface_test.go @@ -964,7 +964,7 @@ func testReorgConf(miner *rpctest.Harness, notifier chainntnfs.TestChainNotifier t *testing.T) { // Set up a new miner that we can use to cause a reorg. - miner2, err := rpctest.New(chainntnfs.NetParams, nil, nil) + miner2, err := rpctest.New(chainntnfs.NetParams, nil, []string{"--txindex"}) if err != nil { t.Fatalf("unable to create mining node: %v", err) } @@ -1303,7 +1303,7 @@ func testCatchUpOnMissedBlocksWithReorg(miner1 *rpctest.Harness, var wg sync.WaitGroup // Set up a new miner that we can use to cause a reorg. - miner2, err := rpctest.New(chainntnfs.NetParams, nil, nil) + miner2, err := rpctest.New(chainntnfs.NetParams, nil, []string{"--txindex"}) if err != nil { t.Fatalf("unable to create mining node: %v", err) } diff --git a/lnd_test.go b/lnd_test.go index 892acbb0..e8236f24 100644 --- a/lnd_test.go +++ b/lnd_test.go @@ -1376,7 +1376,7 @@ func testOpenChannelAfterReorg(net *lntest.NetworkHarness, t *harnessTest) { ctxb := context.Background() // Set up a new miner that we can use to cause a reorg. - args := []string{"--rejectnonstd"} + args := []string{"--rejectnonstd", "--txindex"} miner, err := rpctest.New(harnessNetParams, &rpcclient.NotificationHandlers{}, args) if err != nil { @@ -12086,7 +12086,7 @@ func TestLightningNetworkDaemon(t *testing.T) { // setting of accepting non-standard transactions on simnet to reject them. // Transactions on the lightning network should always be standard to get // better guarantees of getting included in to blocks. - args := []string{"--rejectnonstd"} + args := []string{"--rejectnonstd", "--txindex"} handlers := &rpcclient.NotificationHandlers{ OnTxAccepted: func(hash *chainhash.Hash, amt btcutil.Amount) { lndHarness.OnTxAccepted(hash) diff --git a/lnwallet/interface_test.go b/lnwallet/interface_test.go index 2ca744c4..6bdffc24 100644 --- a/lnwallet/interface_test.go +++ b/lnwallet/interface_test.go @@ -1778,7 +1778,7 @@ func testReorgWalletBalance(r *rpctest.Harness, w *lnwallet.LightningWallet, // Now we cause a reorganization as follows. // Step 1: create a new miner and start it. - r2, err := rpctest.New(r.ActiveNet, nil, nil) + r2, err := rpctest.New(r.ActiveNet, nil, []string{"--txindex"}) if err != nil { t.Fatalf("unable to create mining node: %v", err) } @@ -2050,7 +2050,7 @@ func TestLightningWallet(t *testing.T) { // dedicated miner to generate blocks, cause re-orgs, etc. We'll set // up this node with a chain length of 125, so we have plenty of BTC // to play around with. - miningNode, err := rpctest.New(netParams, nil, nil) + miningNode, err := rpctest.New(netParams, nil, []string{"--txindex"}) if err != nil { t.Fatalf("unable to create mining node: %v", err) } diff --git a/routing/chainview/interface_test.go b/routing/chainview/interface_test.go index 76a66bde..24789b77 100644 --- a/routing/chainview/interface_test.go +++ b/routing/chainview/interface_test.go @@ -540,7 +540,7 @@ func testFilterBlockDisconnected(node *rpctest.Harness, // Create a node that has a shorter chain than the main chain, so we // can trigger a reorg. - reorgNode, err := rpctest.New(netParams, nil, nil) + reorgNode, err := rpctest.New(netParams, nil, []string{"--txindex"}) if err != nil { t.Fatalf("unable to create mining node: %v", err) } @@ -902,7 +902,7 @@ func TestFilteredChainView(t *testing.T) { // dedicated miner to generate blocks, cause re-orgs, etc. We'll set up // this node with a chain length of 125, so we have plenty of BTC to // play around with. - miner, err := rpctest.New(netParams, nil, nil) + miner, err := rpctest.New(netParams, nil, []string{"--txindex"}) if err != nil { t.Fatalf("unable to create mining node: %v", err) } From e7180164640f5973a0432951835e422f8fdad5ba Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Thu, 23 Aug 2018 20:19:08 -0700 Subject: [PATCH 05/19] build: update btcd+btcwallet+neutrino to latest versions --- Gopkg.lock | 14 ++++++++------ Gopkg.toml | 6 +++--- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index 99cf81d6..3791d9da 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -64,7 +64,7 @@ revision = "e404fcfc888570cadd1610538e2dbc89f66af814" [[projects]] - digest = "1:f944518b25f8733eed1d13855eeaaeda9b1f56c2bc75223a7e3427c024168eba" + digest = "1:e0ab2aba19fe77b2367828b186e46a5efa57d35a94df282ae25adf624136ae5c" name = "github.com/btcsuite/btcd" packages = [ "addrmgr", @@ -82,7 +82,7 @@ "wire", ] pruneopts = "UT" - revision = "f899737d7f2764dc13e4d01ff00108ec58f766a9" + revision = "79e00513b1011888b1e675157ab89f527f901cae" [[projects]] digest = "1:30d4a548e09bca4a0c77317c58e7407e2a65c15325e944f9c08a7b7992f8a59e" @@ -107,7 +107,7 @@ revision = "ab6388e0c60ae4834a1f57511e20c17b5f78be4b" [[projects]] - digest = "1:04bf3f47dafa64588795c5e0329dc662e867c3afa191051821dbacbe09ba2ca8" + digest = "1:b6aad1b935c1e7c6cb6be7ecb65288de11e0df0d6224d757816e40009ec52a2c" name = "github.com/btcsuite/btcwallet" packages = [ "chain", @@ -127,7 +127,7 @@ "wtxmgr", ] pruneopts = "UT" - revision = "5fb94231d0c814f02ffc3110eee588278151b4e1" + revision = "7b84dc25a61634c450d21ec7f47d5e916eb88fdb" [[projects]] branch = "master" @@ -266,16 +266,18 @@ revision = "462a8a75388506b68f76661af8d649f0b88e5301" [[projects]] - digest = "1:11ab77a97c0db5cfe9c82f16cb7c47213612033e8bd711e6ddc9a32615fc747d" + digest = "1:17f1db965adb240de22a1662b4b712858ae767fe7da94ffd4e321b4dcb4bd553" name = "github.com/lightninglabs/neutrino" packages = [ ".", + "cache", + "cache/lru", "filterdb", "headerfs", "headerlist", ] pruneopts = "UT" - revision = "0d0ce901538af81e234c1b2376babf20fe976b09" + revision = "166fe699d5964581d24e6bfff0aa329cfb6a8bc9" [[projects]] digest = "1:58ab6d6525898cbeb86dc29a68f8e9bfe95254b9032134eb9458779574872260" diff --git a/Gopkg.toml b/Gopkg.toml index 03a315d3..2dec59e3 100644 --- a/Gopkg.toml +++ b/Gopkg.toml @@ -40,7 +40,7 @@ [[constraint]] name = "github.com/lightninglabs/neutrino" - revision = "0d0ce901538af81e234c1b2376babf20fe976b09" + revision = "166fe699d5964581d24e6bfff0aa329cfb6a8bc9" [[constraint]] name = "github.com/lightningnetwork/lightning-onion" @@ -64,11 +64,11 @@ [[constraint]] name = "github.com/btcsuite/btcd" - revision = "f899737d7f2764dc13e4d01ff00108ec58f766a9" + revision = "79e00513b1011888b1e675157ab89f527f901cae" [[constraint]] name = "github.com/btcsuite/btcwallet" - revision = "5fb94231d0c814f02ffc3110eee588278151b4e1" + revision = "7b84dc25a61634c450d21ec7f47d5e916eb88fdb" [[constraint]] name = "github.com/tv42/zbase32" From 9b6b78a9328b9139f885e47579615664d8ff6aed Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Thu, 23 Aug 2018 20:19:37 -0700 Subject: [PATCH 06/19] chainntnfs/neutrinonotify: update to latest API changes --- chainntnfs/neutrinonotify/neutrino.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/chainntnfs/neutrinonotify/neutrino.go b/chainntnfs/neutrinonotify/neutrino.go index 5fb4d2a3..a0e999d0 100644 --- a/chainntnfs/neutrinonotify/neutrino.go +++ b/chainntnfs/neutrinonotify/neutrino.go @@ -577,7 +577,7 @@ func (n *NeutrinoNotifier) historicalConfDetails(targetHash *chainhash.Hash, // In the case that we do have a match, we'll fetch the block // from the network so we can find the positional data required // to send the proper response. - block, err := n.p2pNode.GetBlockFromNetwork(blockHash) + block, err := n.p2pNode.GetBlock(blockHash) if err != nil { return nil, fmt.Errorf("unable to get block from network: %v", err) } @@ -686,7 +686,7 @@ func (n *NeutrinoNotifier) handleBlockConnected(newBlock *filteredBlock) error { // getFilteredBlock is a utility to retrieve the full filtered block from a block epoch. func (n *NeutrinoNotifier) getFilteredBlock(epoch chainntnfs.BlockEpoch) (*filteredBlock, error) { - rawBlock, err := n.p2pNode.GetBlockFromNetwork(*epoch.Hash) + rawBlock, err := n.p2pNode.GetBlock(*epoch.Hash) if err != nil { return nil, fmt.Errorf("unable to get block: %v", err) } From 85c0dd3de00bf3a9a840ce8d20f42d26eb6ef9db Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Thu, 23 Aug 2018 20:19:47 -0700 Subject: [PATCH 07/19] routing/chainview: update to latest API changes --- routing/chainview/neutrino.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routing/chainview/neutrino.go b/routing/chainview/neutrino.go index 56d16708..6a22bbb1 100644 --- a/routing/chainview/neutrino.go +++ b/routing/chainview/neutrino.go @@ -273,7 +273,7 @@ func (c *CfFilteredChainView) FilterBlock(blockHash *chainhash.Hash) (*FilteredB // If we reach this point, then there was a match, so we'll need to // fetch the block itself so we can scan it for any actual matches (as // there's a fp rate). - block, err := c.p2pNode.GetBlockFromNetwork(*blockHash) + block, err := c.p2pNode.GetBlock(*blockHash) if err != nil { return nil, err } From 16a321f6c50fc258001896bad891f28e941e283b Mon Sep 17 00:00:00 2001 From: Conner Fromknecht Date: Fri, 24 Aug 2018 04:05:18 -0700 Subject: [PATCH 08/19] chainntnfs/bitcoind: initialize with height hint cache --- chainntnfs/bitcoindnotify/bitcoind_test.go | 36 +++++++++++++++++++--- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/chainntnfs/bitcoindnotify/bitcoind_test.go b/chainntnfs/bitcoindnotify/bitcoind_test.go index c3f863ba..6447630d 100644 --- a/chainntnfs/bitcoindnotify/bitcoind_test.go +++ b/chainntnfs/bitcoindnotify/bitcoind_test.go @@ -3,6 +3,7 @@ package bitcoindnotify import ( + "io/ioutil" "testing" "time" @@ -10,14 +11,37 @@ import ( "github.com/btcsuite/btcd/integration/rpctest" "github.com/btcsuite/btcwallet/chain" "github.com/lightningnetwork/lnd/chainntnfs" + "github.com/lightningnetwork/lnd/channeldb" ) +func initHintCache(t *testing.T) *chainntnfs.HeightHintCache { + t.Helper() + + tempDir, err := ioutil.TempDir("", "kek") + if err != nil { + t.Fatalf("unable to create temp dir: %v", err) + } + db, err := channeldb.Open(tempDir) + if err != nil { + t.Fatalf("unable to create db: %v", err) + } + hintCache, err := chainntnfs.NewHeightHintCache(db) + if err != nil { + t.Fatalf("unable to create hint cache: %v", err) + } + + return hintCache +} + // setUpNotifier is a helper function to start a new notifier backed by a // bitcoind driver. -func setUpNotifier(t *testing.T, bitcoindConn *chain.BitcoindConn) *BitcoindNotifier { +func setUpNotifier(t *testing.T, bitcoindConn *chain.BitcoindConn, + spendHintCache chainntnfs.SpendHintCache, + confirmHintCache chainntnfs.ConfirmHintCache) *BitcoindNotifier { + t.Helper() - notifier := New(bitcoindConn) + notifier := New(bitcoindConn, spendHintCache, confirmHintCache) if err := notifier.Start(); err != nil { t.Fatalf("unable to start notifier: %v", err) } @@ -70,7 +94,9 @@ func TestHistoricalConfDetailsTxIndex(t *testing.T) { ) defer cleanUp() - notifier := setUpNotifier(t, bitcoindConn) + hintCache := initHintCache(t) + + notifier := setUpNotifier(t, bitcoindConn, hintCache, hintCache) defer notifier.Stop() syncNotifierWithMiner(t, notifier, miner) @@ -136,7 +162,9 @@ func TestHistoricalConfDetailsNoTxIndex(t *testing.T) { ) defer cleanUp() - notifier := setUpNotifier(t, bitcoindConn) + hintCache := initHintCache(t) + + notifier := setUpNotifier(t, bitcoindConn, hintCache, hintCache) defer notifier.Stop() // Since the node has its txindex disabled, we fall back to scanning the From ecc91305acf28048466a9741c2b4d4a054343145 Mon Sep 17 00:00:00 2001 From: Conner Fromknecht Date: Fri, 24 Aug 2018 04:05:46 -0700 Subject: [PATCH 09/19] chainntnfs/btcdnotify: initialize with height hint cache --- chainntnfs/btcdnotify/btcd_test.go | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/chainntnfs/btcdnotify/btcd_test.go b/chainntnfs/btcdnotify/btcd_test.go index 64b7fadd..464c928e 100644 --- a/chainntnfs/btcdnotify/btcd_test.go +++ b/chainntnfs/btcdnotify/btcd_test.go @@ -3,18 +3,41 @@ package btcdnotify import ( + "io/ioutil" "testing" "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/btcsuite/btcd/integration/rpctest" "github.com/lightningnetwork/lnd/chainntnfs" + "github.com/lightningnetwork/lnd/channeldb" ) +func initHintCache(t *testing.T) *chainntnfs.HeightHintCache { + t.Helper() + + tempDir, err := ioutil.TempDir("", "kek") + if err != nil { + t.Fatalf("unable to create temp dir: %v", err) + } + db, err := channeldb.Open(tempDir) + if err != nil { + t.Fatalf("unable to create db: %v", err) + } + hintCache, err := chainntnfs.NewHeightHintCache(db) + if err != nil { + t.Fatalf("unable to create hint cache: %v", err) + } + + return hintCache +} + // setUpNotifier is a helper function to start a new notifier backed by a btcd // driver. func setUpNotifier(t *testing.T, h *rpctest.Harness) *BtcdNotifier { + hintCache := initHintCache(t) + rpcCfg := h.RPCConfig() - notifier, err := New(&rpcCfg) + notifier, err := New(&rpcCfg, hintCache, hintCache) if err != nil { t.Fatalf("unable to create notifier: %v", err) } From cb3ec07685df4d36cf5f6855ebd23989827fe2bd Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Fri, 24 Aug 2018 13:50:47 +0200 Subject: [PATCH 10/19] chainntnfs/test_util: set 10ms trickleInterval for miner --- chainntnfs/test_utils.go | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/chainntnfs/test_utils.go b/chainntnfs/test_utils.go index cfdafd85..44c844d0 100644 --- a/chainntnfs/test_utils.go +++ b/chainntnfs/test_utils.go @@ -26,6 +26,13 @@ import ( "github.com/lightninglabs/neutrino" ) +var ( + // trickleInterval is the interval at which the miner should trickle + // transactions to its peers. We'll set it small to ensure the miner + // propagates transactions quickly in the tests. + trickleInterval = 10 * time.Millisecond +) + var ( NetParams = &chaincfg.RegressionNetParams @@ -62,6 +69,7 @@ func GetTestTxidAndScript(h *rpctest.Harness) (*chainhash.Hash, []byte, error) { // WaitForMempoolTx waits for the txid to be seen in the miner's mempool. func WaitForMempoolTx(miner *rpctest.Harness, txid *chainhash.Hash) error { timeout := time.After(10 * time.Second) + trickle := time.After(2 * trickleInterval) for { // Check for the harness' knowledge of the txid. tx, err := miner.Node.GetRawTransaction(txid) @@ -84,6 +92,16 @@ func WaitForMempoolTx(miner *rpctest.Harness, txid *chainhash.Hash) error { } } + // To ensure any transactions propagate from the miner to the peers + // before returning, ensure we have waited for at least + // 2*trickleInterval before returning. + select { + case <-trickle: + case <-timeout: + return errors.New("timeout waiting for trickle interval. " + + "Trickle interval to large?") + } + return nil } @@ -141,6 +159,10 @@ func NewMiner(t *testing.T, extraArgs []string, createChain bool, t.Helper() + // Add the trickle interval argument to the extra args. + trickle := fmt.Sprintf("--trickleinterval=%v", trickleInterval) + extraArgs = append(extraArgs, trickle) + node, err := rpctest.New(NetParams, nil, extraArgs) if err != nil { t.Fatalf("unable to create backend node: %v", err) From a91466e9fab96c9da07736aceeb7207883753be8 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Fri, 24 Aug 2018 13:51:54 +0200 Subject: [PATCH 11/19] chainntnfs/bitcoind tests: check found mempool case --- chainntnfs/bitcoindnotify/bitcoind_test.go | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/chainntnfs/bitcoindnotify/bitcoind_test.go b/chainntnfs/bitcoindnotify/bitcoind_test.go index 6447630d..ad71a4d0 100644 --- a/chainntnfs/bitcoindnotify/bitcoind_test.go +++ b/chainntnfs/bitcoindnotify/bitcoind_test.go @@ -127,6 +127,22 @@ func TestHistoricalConfDetailsTxIndex(t *testing.T) { if err := chainntnfs.WaitForMempoolTx(miner, txid); err != nil { t.Fatal(err) } + + // The transaction should be found in the mempool at this point. + _, txStatus, err = notifier.historicalConfDetails(txid, 0, 0) + if err != nil { + t.Fatalf("unable to retrieve historical conf details: %v", err) + } + + // Since it has yet to be included in a block, it should have been found + // within the mempool. + switch txStatus { + case txFoundMempool: + default: + t.Fatal("should have found the transaction within the "+ + "mempool, but did not: %v", txStatus) + } + if _, err := miner.Node.Generate(1); err != nil { t.Fatalf("unable to generate block: %v", err) } @@ -194,9 +210,6 @@ 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. - // - // TODO(wilmer): add mempool case once trickle timeout can be specified - // in btcd. output, pkScript := chainntnfs.CreateSpendableOutput(t, miner) spendTx := chainntnfs.CreateSpendTx(t, output, pkScript) spendTxHash, err := miner.Node.SendRawTransaction(spendTx, true) From 4a6088797484edf901217795b5f50b7fd42e4996 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Fri, 24 Aug 2018 13:54:36 +0200 Subject: [PATCH 12/19] chainntnfs/btcd: remove unnecessary check for tx==nil --- chainntnfs/btcdnotify/btcd.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/chainntnfs/btcdnotify/btcd.go b/chainntnfs/btcdnotify/btcd.go index 66c5747d..e2f1418d 100644 --- a/chainntnfs/btcdnotify/btcd.go +++ b/chainntnfs/btcdnotify/btcd.go @@ -569,10 +569,12 @@ func (b *BtcdNotifier) historicalConfDetails(txid *chainhash.Hash, return txConf, txStatus, nil } -// confDetailsFromTxIndex looks up whether a transaction is already included -// in a block in the active chain by using the backend node's transaction index. -// If the transaction is found, its confirmation details are returned. -// Otherwise, nil is returned. +// confDetailsFromTxIndex looks up whether a transaction is already included in +// a block in the active chain by using the backend node's transaction index. +// If the transaction is found its TxConfStatus is returned. If it was found in +// the mempool this will be TxFoundMempool, if it is found in a block this will +// be TxFoundIndex. Otherwise TxNotFoundIndex is returned. If the tx is found +// in a block its confirmation details are also returned. func (b *BtcdNotifier) confDetailsFromTxIndex(txid *chainhash.Hash, ) (*chainntnfs.TxConfirmation, txConfStatus, error) { @@ -597,9 +599,9 @@ func (b *BtcdNotifier) confDetailsFromTxIndex(txid *chainhash.Hash, } // Make sure we actually retrieved a transaction that is included in a - // block. Without this, we won't be able to retrieve its confirmation - // details. - if tx == nil || tx.BlockHash == "" { + // block. If not, the transaction must be unconfirmed (in the mempool), + // and we'll return TxFoundMempool together with a nil TxConfirmation. + if tx.BlockHash == "" { return nil, txFoundMempool, nil } From 89c33731fe3aa29345f3fc5a752a7fc9db1532ae Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Fri, 24 Aug 2018 13:54:54 +0200 Subject: [PATCH 13/19] chainntnfs/bitcoind: remove unnecessary check for tx==nil --- chainntnfs/bitcoindnotify/bitcoind.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/chainntnfs/bitcoindnotify/bitcoind.go b/chainntnfs/bitcoindnotify/bitcoind.go index 16addc42..01004393 100644 --- a/chainntnfs/bitcoindnotify/bitcoind.go +++ b/chainntnfs/bitcoindnotify/bitcoind.go @@ -501,10 +501,12 @@ func (b *BitcoindNotifier) historicalConfDetails(txid *chainhash.Hash, return txConf, txStatus, nil } -// confDetailsFromTxIndex looks up whether a transaction is already included -// in a block in the active chain by using the backend node's transaction index. -// If the transaction is found, its confirmation details are returned. -// Otherwise, nil is returned. +// confDetailsFromTxIndex looks up whether a transaction is already included in +// a block in the active chain by using the backend node's transaction index. +// If the transaction is found its TxConfStatus is returned. If it was found in +// the mempool this will be TxFoundMempool, if it is found in a block this will +// be TxFoundIndex. Otherwise TxNotFoundIndex is returned. If the tx is found +// in a block its confirmation details are also returned. func (b *BitcoindNotifier) confDetailsFromTxIndex(txid *chainhash.Hash, ) (*chainntnfs.TxConfirmation, txConfStatus, error) { @@ -529,9 +531,9 @@ func (b *BitcoindNotifier) confDetailsFromTxIndex(txid *chainhash.Hash, } // Make sure we actually retrieved a transaction that is included in a - // block. Without this, we won't be able to retrieve its confirmation - // details. - if tx == nil || tx.BlockHash == "" { + // block. If not, the transaction must be unconfirmed (in the mempool), + // and we'll return TxFoundMempool together with a nil TxConfirmation. + if tx.BlockHash == "" { return nil, txFoundMempool, nil } From b552984fcd77b3f2265c446934e56dc4ffcceaa7 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Fri, 24 Aug 2018 14:05:25 +0200 Subject: [PATCH 14/19] chainntnfs: define common TxConfStatus type --- chainntnfs/interface.go | 48 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/chainntnfs/interface.go b/chainntnfs/interface.go index 7c7a2ec6..99772bc1 100644 --- a/chainntnfs/interface.go +++ b/chainntnfs/interface.go @@ -9,6 +9,54 @@ import ( "github.com/btcsuite/btcd/wire" ) +// TxConfStatus denotes the status of a transaction's lookup. +type TxConfStatus uint8 + +const ( + // TxFoundMempool denotes that the transaction was found within the + // backend node's mempool. + TxFoundMempool TxConfStatus = iota + + // TxFoundIndex denotes that the transaction was found within the + // backend node's txindex. + TxFoundIndex + + // TxFoundIndex denotes that the transaction was not found within the + // backend node's txindex. + TxNotFoundIndex + + // TxFoundManually denotes that the transaction was found within the + // chain by scanning for it manually. + TxFoundManually + + // TxFoundManually denotes that the transaction was not found within the + // chain by scanning for it manually. + TxNotFoundManually +) + +// String returns the string representation of the TxConfStatus. +func (t TxConfStatus) String() string { + switch t { + case TxFoundMempool: + return "TxFoundMempool" + + case TxFoundIndex: + return "TxFoundIndex" + + case TxNotFoundIndex: + return "TxNotFoundIndex" + + case TxFoundManually: + return "TxFoundManually" + + case TxNotFoundManually: + return "TxNotFoundManually" + + default: + return "unknown" + } +} + // ChainNotifier represents a trusted source to receive notifications concerning // targeted events on the Bitcoin blockchain. The interface specification is // intentionally general in order to support a wide array of chain notification From 79b2ee7da5d862b59dc0c4d38c49da1cbc2b2be0 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Fri, 24 Aug 2018 14:13:28 +0200 Subject: [PATCH 15/19] chainntnfs/btcd+bitcoind: use common TxConfStatus --- chainntnfs/bitcoindnotify/bitcoind.go | 82 ++++++++++---------------- chainntnfs/btcdnotify/btcd.go | 83 +++++++++++---------------- 2 files changed, 63 insertions(+), 102 deletions(-) diff --git a/chainntnfs/bitcoindnotify/bitcoind.go b/chainntnfs/bitcoindnotify/bitcoind.go index 01004393..a5d630a2 100644 --- a/chainntnfs/bitcoindnotify/bitcoind.go +++ b/chainntnfs/bitcoindnotify/bitcoind.go @@ -28,31 +28,6 @@ const ( reorgSafetyLimit = 100 ) -// txConfStatus denotes the status of a transaction's lookup. -type txConfStatus uint8 - -const ( - // txFoundMempool denotes that the transaction was found within the - // backend node's mempool. - txFoundMempool txConfStatus = iota - - // txFoundIndex denotes that the transaction was found within the - // backend node's txindex. - txFoundIndex - - // txFoundIndex denotes that the transaction was not found within the - // backend node's txindex. - txNotFoundIndex - - // txFoundManually denotes that the transaction was found within the - // chain by scanning for it manually. - txFoundManually - - // txFoundManually denotes that the transaction was not found within the - // chain by scanning for it manually. - txNotFoundManually -) - var ( // ErrChainNotifierShuttingDown is used when we are trying to // measure a spend notification when notifier is already stopped. @@ -474,7 +449,7 @@ func (b *BitcoindNotifier) handleRelevantTx(tx chain.RelevantTx, bestHeight int3 // block in the active chain and, if so, returns details about the confirmation. func (b *BitcoindNotifier) historicalConfDetails(txid *chainhash.Hash, heightHint, currentHeight uint32) (*chainntnfs.TxConfirmation, - txConfStatus, error) { + chainntnfs.TxConfStatus, error) { // We'll first attempt to retrieve the transaction using the node's // txindex. @@ -484,13 +459,13 @@ func (b *BitcoindNotifier) historicalConfDetails(txid *chainhash.Hash, // determine whether we should proceed with any fallback methods. switch { // The transaction was found within the node's mempool. - case txStatus == txFoundMempool: + case txStatus == chainntnfs.TxFoundMempool: // The transaction was found within the node's txindex. - case txStatus == txFoundIndex: + case txStatus == chainntnfs.TxFoundIndex: // The transaction was not found within the node's mempool or txindex. - case err == nil && txStatus == txNotFoundIndex: + case err == nil && txStatus == chainntnfs.TxNotFoundIndex: // We failed to look up the transaction within the node's mempool or // txindex, so we'll proceed to scan the chain manually. @@ -508,7 +483,7 @@ func (b *BitcoindNotifier) historicalConfDetails(txid *chainhash.Hash, // be TxFoundIndex. Otherwise TxNotFoundIndex is returned. If the tx is found // in a block its confirmation details are also returned. func (b *BitcoindNotifier) confDetailsFromTxIndex(txid *chainhash.Hash, -) (*chainntnfs.TxConfirmation, txConfStatus, error) { +) (*chainntnfs.TxConfirmation, chainntnfs.TxConfStatus, error) { // If the transaction has some or all of its confirmations required, // then we may be able to dispatch it immediately. @@ -523,18 +498,18 @@ func (b *BitcoindNotifier) confDetailsFromTxIndex(txid *chainhash.Hash, if ok && jsonErr.Code == btcjson.ErrRPCNoTxInfo && strings.Contains(jsonErr.Message, txNotFoundErr) { - return nil, txNotFoundIndex, nil + return nil, chainntnfs.TxNotFoundIndex, nil } - return nil, txNotFoundIndex, fmt.Errorf("unable to query for "+ - "txid %v: %v", txid, err) + return nil, chainntnfs.TxNotFoundIndex, + fmt.Errorf("unable to query for txid %v: %v", txid, err) } // Make sure we actually retrieved a transaction that is included in a // block. If not, the transaction must be unconfirmed (in the mempool), // and we'll return TxFoundMempool together with a nil TxConfirmation. if tx.BlockHash == "" { - return nil, txFoundMempool, nil + return nil, chainntnfs.TxFoundMempool, nil } // As we need to fully populate the returned TxConfirmation struct, @@ -542,16 +517,16 @@ func (b *BitcoindNotifier) confDetailsFromTxIndex(txid *chainhash.Hash, // locate its exact index within the block. blockHash, err := chainhash.NewHashFromStr(tx.BlockHash) if err != nil { - return nil, txNotFoundIndex, fmt.Errorf("unable to get block "+ - "hash %v for historical dispatch: %v", tx.BlockHash, - err) + return nil, chainntnfs.TxNotFoundIndex, + fmt.Errorf("unable to get block hash %v for "+ + "historical dispatch: %v", tx.BlockHash, err) } block, err := b.chainConn.GetBlockVerbose(blockHash) if err != nil { - return nil, txNotFoundIndex, fmt.Errorf("unable to get block "+ - "with hash %v for historical dispatch: %v", blockHash, - err) + return nil, chainntnfs.TxNotFoundIndex, + fmt.Errorf("unable to get block with hash %v for "+ + "historical dispatch: %v", blockHash, err) } // If the block was obtained, locate the transaction's index within the @@ -564,14 +539,15 @@ func (b *BitcoindNotifier) confDetailsFromTxIndex(txid *chainhash.Hash, BlockHeight: uint32(block.Height), TxIndex: uint32(txIndex), } - return details, txFoundIndex, nil + return details, chainntnfs.TxFoundIndex, nil } } // We return an error because we should have found the transaction // within the block, but didn't. - return nil, txNotFoundIndex, fmt.Errorf("unable to locate tx %v in "+ - "block %v", txid, blockHash) + return nil, chainntnfs.TxNotFoundIndex, + fmt.Errorf("unable to locate tx %v in block %v", txid, + blockHash) } // confDetailsManually looks up whether a transaction is already included in a @@ -580,7 +556,8 @@ func (b *BitcoindNotifier) confDetailsFromTxIndex(txid *chainhash.Hash, // 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, - heightHint, currentHeight uint32) (*chainntnfs.TxConfirmation, txConfStatus, error) { + heightHint, currentHeight uint32) (*chainntnfs.TxConfirmation, + chainntnfs.TxConfStatus, error) { targetTxidStr := txid.String() @@ -591,20 +568,23 @@ func (b *BitcoindNotifier) confDetailsManually(txid *chainhash.Hash, // processing the next height. select { case <-b.quit: - return nil, txNotFoundManually, ErrChainNotifierShuttingDown + return nil, chainntnfs.TxNotFoundManually, + ErrChainNotifierShuttingDown default: } blockHash, err := b.chainConn.GetBlockHash(int64(height)) if err != nil { - return nil, txNotFoundManually, fmt.Errorf("unable to "+ - "get hash from block with height %d", height) + return nil, chainntnfs.TxNotFoundManually, + fmt.Errorf("unable to get hash from block "+ + "with height %d", height) } block, err := b.chainConn.GetBlockVerbose(blockHash) if err != nil { - return nil, txNotFoundManually, fmt.Errorf("unable to "+ - "get block with hash %v: %v", blockHash, err) + return nil, chainntnfs.TxNotFoundManually, + fmt.Errorf("unable to get block with hash "+ + "%v: %v", blockHash, err) } for txIndex, txHash := range block.Tx { @@ -616,14 +596,14 @@ func (b *BitcoindNotifier) confDetailsManually(txid *chainhash.Hash, BlockHeight: height, TxIndex: uint32(txIndex), } - return details, txFoundManually, nil + return details, chainntnfs.TxFoundManually, nil } } } // If we reach here, then we were not able to find the transaction // within a block, so we avoid returning an error. - return nil, txNotFoundManually, nil + return nil, chainntnfs.TxNotFoundManually, nil } // handleBlockConnected applies a chain update for a new block. Any watched diff --git a/chainntnfs/btcdnotify/btcd.go b/chainntnfs/btcdnotify/btcd.go index e2f1418d..724b5656 100644 --- a/chainntnfs/btcdnotify/btcd.go +++ b/chainntnfs/btcdnotify/btcd.go @@ -27,31 +27,6 @@ const ( reorgSafetyLimit = 100 ) -// txConfStatus denotes the status of a transaction's lookup. -type txConfStatus uint8 - -const ( - // txFoundMempool denotes that the transaction was found within the - // backend node's mempool. - txFoundMempool txConfStatus = iota - - // txFoundIndex denotes that the transaction was found within the - // backend node's txindex. - txFoundIndex - - // txFoundIndex denotes that the transaction was not found within the - // backend node's txindex. - txNotFoundIndex - - // txFoundManually denotes that the transaction was found within the - // chain by scanning for it manually. - txFoundManually - - // txFoundManually denotes that the transaction was not found within the - // chain by scanning for it manually. - txNotFoundManually -) - var ( // ErrChainNotifierShuttingDown is used when we are trying to // measure a spend notification when notifier is already stopped. @@ -542,7 +517,8 @@ out: // 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 *BtcdNotifier) historicalConfDetails(txid *chainhash.Hash, - heightHint, currentHeight uint32) (*chainntnfs.TxConfirmation, txConfStatus, error) { + heightHint, currentHeight uint32) (*chainntnfs.TxConfirmation, + chainntnfs.TxConfStatus, error) { // We'll first attempt to retrieve the transaction using the node's // txindex. @@ -552,13 +528,13 @@ func (b *BtcdNotifier) historicalConfDetails(txid *chainhash.Hash, // determine whether we should proceed with any fallback methods. switch { // The transaction was found within the node's mempool. - case txStatus == txFoundMempool: + case txStatus == chainntnfs.TxFoundMempool: // The transaction was found within the node's txindex. - case txStatus == txFoundIndex: + case txStatus == chainntnfs.TxFoundIndex: // The transaction was not found within the node's mempool or txindex. - case txStatus == txNotFoundIndex && err == nil: + case txStatus == chainntnfs.TxNotFoundIndex && err == nil: // We failed to look up the transaction within the node's mempool or // txindex, so we'll proceed to scan the chain manually. @@ -576,7 +552,7 @@ func (b *BtcdNotifier) historicalConfDetails(txid *chainhash.Hash, // be TxFoundIndex. Otherwise TxNotFoundIndex is returned. If the tx is found // in a block its confirmation details are also returned. func (b *BtcdNotifier) confDetailsFromTxIndex(txid *chainhash.Hash, -) (*chainntnfs.TxConfirmation, txConfStatus, error) { +) (*chainntnfs.TxConfirmation, chainntnfs.TxConfStatus, error) { // If the transaction has some or all of its confirmations required, // then we may be able to dispatch it immediately. @@ -591,18 +567,18 @@ func (b *BtcdNotifier) confDetailsFromTxIndex(txid *chainhash.Hash, if ok && jsonErr.Code == btcjson.ErrRPCNoTxInfo && strings.Contains(jsonErr.Message, txNotFoundErr) { - return nil, txNotFoundIndex, nil + return nil, chainntnfs.TxNotFoundIndex, nil } - return nil, txNotFoundIndex, fmt.Errorf("unable to query for "+ - "txid %v: %v", txid, err) + return nil, chainntnfs.TxNotFoundIndex, + fmt.Errorf("unable to query for txid %v: %v", txid, err) } // Make sure we actually retrieved a transaction that is included in a // block. If not, the transaction must be unconfirmed (in the mempool), // and we'll return TxFoundMempool together with a nil TxConfirmation. if tx.BlockHash == "" { - return nil, txFoundMempool, nil + return nil, chainntnfs.TxFoundMempool, nil } // As we need to fully populate the returned TxConfirmation struct, @@ -610,16 +586,16 @@ func (b *BtcdNotifier) confDetailsFromTxIndex(txid *chainhash.Hash, // locate its exact index within the block. blockHash, err := chainhash.NewHashFromStr(tx.BlockHash) if err != nil { - return nil, txNotFoundIndex, fmt.Errorf("unable to get block "+ - "hash %v for historical dispatch: %v", tx.BlockHash, - err) + return nil, chainntnfs.TxNotFoundIndex, + fmt.Errorf("unable to get block hash %v for "+ + "historical dispatch: %v", tx.BlockHash, err) } block, err := b.chainConn.GetBlockVerbose(blockHash) if err != nil { - return nil, txNotFoundIndex, fmt.Errorf("unable to get block "+ - "with hash %v for historical dispatch: %v", blockHash, - err) + return nil, chainntnfs.TxNotFoundIndex, + fmt.Errorf("unable to get block with hash %v for "+ + "historical dispatch: %v", blockHash, err) } // If the block was obtained, locate the transaction's index within the @@ -632,14 +608,15 @@ func (b *BtcdNotifier) confDetailsFromTxIndex(txid *chainhash.Hash, BlockHeight: uint32(block.Height), TxIndex: uint32(txIndex), } - return details, txFoundIndex, nil + return details, chainntnfs.TxFoundIndex, nil } } // We return an error because we should have found the transaction // within the block, but didn't. - return nil, txNotFoundIndex, fmt.Errorf("unable to locate tx %v in "+ - "block %v", txid, blockHash) + return nil, chainntnfs.TxNotFoundIndex, + fmt.Errorf("unable to locate tx %v in block %v", txid, + blockHash) } // confDetailsManually looks up whether a transaction is already included in a @@ -648,7 +625,8 @@ func (b *BtcdNotifier) confDetailsFromTxIndex(txid *chainhash.Hash, // height in the chain. If the transaction is found, its confirmation details // are returned. Otherwise, nil is returned. func (b *BtcdNotifier) confDetailsManually(txid *chainhash.Hash, heightHint, - currentHeight uint32) (*chainntnfs.TxConfirmation, txConfStatus, error) { + currentHeight uint32) (*chainntnfs.TxConfirmation, + chainntnfs.TxConfStatus, error) { targetTxidStr := txid.String() @@ -659,21 +637,24 @@ func (b *BtcdNotifier) confDetailsManually(txid *chainhash.Hash, heightHint, // processing the next height. select { case <-b.quit: - return nil, txNotFoundManually, ErrChainNotifierShuttingDown + return nil, chainntnfs.TxNotFoundManually, + ErrChainNotifierShuttingDown default: } blockHash, err := b.chainConn.GetBlockHash(int64(height)) if err != nil { - return nil, txNotFoundManually, fmt.Errorf("unable to "+ - "get hash from block with height %d", height) + return nil, chainntnfs.TxNotFoundManually, + fmt.Errorf("unable to get hash from block "+ + "with height %d", height) } // TODO: fetch the neutrino filters instead. block, err := b.chainConn.GetBlockVerbose(blockHash) if err != nil { - return nil, txNotFoundManually, fmt.Errorf("unable to "+ - "get block with hash %v: %v", blockHash, err) + return nil, chainntnfs.TxNotFoundManually, + fmt.Errorf("unable to get block with hash "+ + "%v: %v", blockHash, err) } for txIndex, txHash := range block.Tx { @@ -685,14 +666,14 @@ func (b *BtcdNotifier) confDetailsManually(txid *chainhash.Hash, heightHint, BlockHeight: height, TxIndex: uint32(txIndex), } - return details, txFoundManually, nil + return details, chainntnfs.TxFoundManually, nil } } } // If we reach here, then we were not able to find the transaction // within a block, so we avoid returning an error. - return nil, txNotFoundManually, nil + return nil, chainntnfs.TxNotFoundManually, nil } // handleBlockConnected applies a chain update for a new block. Any watched From 31dc387e0f919d68cfe4e3c3f3c15247a217de21 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Fri, 24 Aug 2018 14:13:52 +0200 Subject: [PATCH 16/19] chainntnfs/btcd+bitcoind tests: use common TxConfStatus --- chainntnfs/bitcoindnotify/bitcoind_test.go | 14 +++++++------- chainntnfs/btcdnotify/btcd_test.go | 21 +++++++++++---------- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/chainntnfs/bitcoindnotify/bitcoind_test.go b/chainntnfs/bitcoindnotify/bitcoind_test.go index ad71a4d0..16bbc2ed 100644 --- a/chainntnfs/bitcoindnotify/bitcoind_test.go +++ b/chainntnfs/bitcoindnotify/bitcoind_test.go @@ -111,8 +111,8 @@ func TestHistoricalConfDetailsTxIndex(t *testing.T) { } switch txStatus { - case txNotFoundIndex: - case txNotFoundManually: + case chainntnfs.TxNotFoundIndex: + case chainntnfs.TxNotFoundManually: t.Fatal("should not have proceeded with fallback method, but did") default: t.Fatal("should not have found non-existent transaction, but did") @@ -137,7 +137,7 @@ func TestHistoricalConfDetailsTxIndex(t *testing.T) { // Since it has yet to be included in a block, it should have been found // within the mempool. switch txStatus { - case txFoundMempool: + case chainntnfs.TxFoundMempool: default: t.Fatal("should have found the transaction within the "+ "mempool, but did not: %v", txStatus) @@ -159,7 +159,7 @@ func TestHistoricalConfDetailsTxIndex(t *testing.T) { // Since the backend node's txindex is enabled and the transaction has // confirmed, we should be able to retrieve it using the txindex. switch txStatus { - case txFoundIndex: + case chainntnfs.TxFoundIndex: default: t.Fatal("should have found the transaction within the " + "txindex, but did not") @@ -196,8 +196,8 @@ func TestHistoricalConfDetailsNoTxIndex(t *testing.T) { } switch txStatus { - case txNotFoundManually: - case txNotFoundIndex: + case chainntnfs.TxNotFoundManually: + case chainntnfs.TxNotFoundIndex: t.Fatal("should have proceeded with fallback method, but did not") default: t.Fatal("should not have found non-existent transaction, but did") @@ -237,7 +237,7 @@ func TestHistoricalConfDetailsNoTxIndex(t *testing.T) { // confirmed, we should be able to find it by falling back to scanning // the chain manually. switch txStatus { - case txFoundManually: + case chainntnfs.TxFoundManually: default: t.Fatal("should have found the transaction by manually " + "scanning the chain, but did not") diff --git a/chainntnfs/btcdnotify/btcd_test.go b/chainntnfs/btcdnotify/btcd_test.go index 464c928e..34f53d5b 100644 --- a/chainntnfs/btcdnotify/btcd_test.go +++ b/chainntnfs/btcdnotify/btcd_test.go @@ -71,8 +71,8 @@ func TestHistoricalConfDetailsTxIndex(t *testing.T) { } switch txStatus { - case txNotFoundIndex: - case txNotFoundManually: + case chainntnfs.TxNotFoundIndex: + case chainntnfs.TxNotFoundManually: t.Fatal("should not have proceeded with fallback method, but did") default: t.Fatal("should not have found non-existent transaction, but did") @@ -88,6 +88,7 @@ func TestHistoricalConfDetailsTxIndex(t *testing.T) { t.Fatalf("unable to find tx in the mempool: %v", err) } + // The transaction should be found in the mempool at this point. _, txStatus, err = notifier.historicalConfDetails(txid, 0, 0) if err != nil { t.Fatalf("unable to retrieve historical conf details: %v", err) @@ -96,10 +97,10 @@ func TestHistoricalConfDetailsTxIndex(t *testing.T) { // Since it has yet to be included in a block, it should have been found // within the mempool. switch txStatus { - case txFoundMempool: + case chainntnfs.TxFoundMempool: default: - t.Fatal("should have found the transaction within the " + - "mempool, but did not") + t.Fatalf("should have found the transaction within the "+ + "mempool, but did not: %v", txStatus) } // We'll now confirm this transaction and re-attempt to retrieve its @@ -116,7 +117,7 @@ func TestHistoricalConfDetailsTxIndex(t *testing.T) { // Since the backend node's txindex is enabled and the transaction has // confirmed, we should be able to retrieve it using the txindex. switch txStatus { - case txFoundIndex: + case chainntnfs.TxFoundIndex: default: t.Fatal("should have found the transaction within the " + "txindex, but did not") @@ -145,8 +146,8 @@ func TestHistoricalConfDetailsNoTxIndex(t *testing.T) { } switch txStatus { - case txNotFoundManually: - case txNotFoundIndex: + case chainntnfs.TxNotFoundManually: + case chainntnfs.TxNotFoundIndex: t.Fatal("should have proceeded with fallback method, but did not") default: t.Fatal("should not have found non-existent transaction, but did") @@ -175,7 +176,7 @@ func TestHistoricalConfDetailsNoTxIndex(t *testing.T) { // Since it has yet to be included in a block, it should have been found // within the mempool. - if txStatus != txFoundMempool { + if txStatus != chainntnfs.TxFoundMempool { t.Fatal("should have found the transaction within the " + "mempool, but did not") } @@ -196,7 +197,7 @@ func TestHistoricalConfDetailsNoTxIndex(t *testing.T) { // Since the backend node's txindex is disabled and the transaction has // confirmed, we should be able to find it by falling back to scanning // the chain manually. - if txStatus != txFoundManually { + if txStatus != chainntnfs.TxFoundManually { t.Fatal("should have found the transaction by manually " + "scanning the chain, but did not") } From 01d8953737441525c0534cd05f49b6811a35253e Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Fri, 24 Aug 2018 14:44:41 +0200 Subject: [PATCH 17/19] chainntnfs/btcd: fallback to scan manually only in case of err != nil --- chainntnfs/btcdnotify/btcd.go | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/chainntnfs/btcdnotify/btcd.go b/chainntnfs/btcdnotify/btcd.go index 724b5656..157c6c73 100644 --- a/chainntnfs/btcdnotify/btcd.go +++ b/chainntnfs/btcdnotify/btcd.go @@ -527,6 +527,14 @@ func (b *BtcdNotifier) historicalConfDetails(txid *chainhash.Hash, // We'll then check the status of the transaction lookup returned to // determine whether we should proceed with any fallback methods. switch { + + // We failed querying the index for the transaction, fall back to + // scanning manually. + case err != nil: + chainntnfs.Log.Debugf("Failed getting conf details from "+ + "index (%v), scanning manually", err) + return b.confDetailsManually(txid, heightHint, currentHeight) + // The transaction was found within the node's mempool. case txStatus == chainntnfs.TxFoundMempool: @@ -534,12 +542,12 @@ func (b *BtcdNotifier) historicalConfDetails(txid *chainhash.Hash, case txStatus == chainntnfs.TxFoundIndex: // The transaction was not found within the node's mempool or txindex. - case txStatus == chainntnfs.TxNotFoundIndex && err == nil: + case txStatus == chainntnfs.TxNotFoundIndex: - // We failed to look up the transaction within the node's mempool or - // txindex, so we'll proceed to scan the chain manually. + // Unexpected txStatus returned. default: - return b.confDetailsManually(txid, heightHint, currentHeight) + return nil, txStatus, + fmt.Errorf("Got unexpected txConfStatus: %v", txStatus) } return txConf, txStatus, nil From f82fd5fe86091e5ee73ba9da4d5508705cf0db42 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Fri, 24 Aug 2018 14:45:12 +0200 Subject: [PATCH 18/19] chainntnfs/bitcoind: fallback to scan manually only in case of err != nil --- chainntnfs/bitcoindnotify/bitcoind.go | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/chainntnfs/bitcoindnotify/bitcoind.go b/chainntnfs/bitcoindnotify/bitcoind.go index a5d630a2..82abfd98 100644 --- a/chainntnfs/bitcoindnotify/bitcoind.go +++ b/chainntnfs/bitcoindnotify/bitcoind.go @@ -458,6 +458,14 @@ func (b *BitcoindNotifier) historicalConfDetails(txid *chainhash.Hash, // We'll then check the status of the transaction lookup returned to // determine whether we should proceed with any fallback methods. switch { + + // We failed querying the index for the transaction, fall back to + // scanning manually. + case err != nil: + chainntnfs.Log.Debugf("Failed getting conf details from "+ + "index (%v), scanning manually", err) + return b.confDetailsManually(txid, heightHint, currentHeight) + // The transaction was found within the node's mempool. case txStatus == chainntnfs.TxFoundMempool: @@ -465,12 +473,12 @@ func (b *BitcoindNotifier) historicalConfDetails(txid *chainhash.Hash, case txStatus == chainntnfs.TxFoundIndex: // The transaction was not found within the node's mempool or txindex. - case err == nil && txStatus == chainntnfs.TxNotFoundIndex: + case txStatus == chainntnfs.TxNotFoundIndex: - // We failed to look up the transaction within the node's mempool or - // txindex, so we'll proceed to scan the chain manually. + // Unexpected txStatus returned. default: - return b.confDetailsManually(txid, heightHint, currentHeight) + return nil, txStatus, + fmt.Errorf("Got unexpected txConfStatus: %v", txStatus) } return txConf, txStatus, nil From 20ba489b4a4f5e446d46f501172382e87844f36b Mon Sep 17 00:00:00 2001 From: Conner Fromknecht Date: Fri, 24 Aug 2018 11:29:09 -0700 Subject: [PATCH 19/19] chainntnfs/interface: fix TxConfStatus godocs for linter --- chainntnfs/interface.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/chainntnfs/interface.go b/chainntnfs/interface.go index 99772bc1..9b878669 100644 --- a/chainntnfs/interface.go +++ b/chainntnfs/interface.go @@ -21,7 +21,7 @@ const ( // backend node's txindex. TxFoundIndex - // TxFoundIndex denotes that the transaction was not found within the + // TxNotFoundIndex denotes that the transaction was not found within the // backend node's txindex. TxNotFoundIndex @@ -29,8 +29,8 @@ const ( // chain by scanning for it manually. TxFoundManually - // TxFoundManually denotes that the transaction was not found within the - // chain by scanning for it manually. + // TxNotFoundManually denotes that the transaction was not found within + // the chain by scanning for it manually. TxNotFoundManually )