From c5f404939449eb779c9aa2d5b80624f9692792e8 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Tue, 4 Jul 2017 15:54:35 -0700 Subject: [PATCH] chainntnfs: convert ChainNotifier interface tests to use sub-tests --- chainntnfs/interface_test.go | 90 +++++++++++++++++---------- chainntnfs/neutrinonotify/neutrino.go | 18 ++++++ 2 files changed, 76 insertions(+), 32 deletions(-) diff --git a/chainntnfs/interface_test.go b/chainntnfs/interface_test.go index 156d4d0b6..401995693 100644 --- a/chainntnfs/interface_test.go +++ b/chainntnfs/interface_test.go @@ -2,6 +2,7 @@ package chainntnfs_test import ( "bytes" + "fmt" "io/ioutil" "log" "os" @@ -66,8 +67,6 @@ func getTestTxId(miner *rpctest.Harness) (*chainhash.Hash, error) { func testSingleConfirmationNotification(miner *rpctest.Harness, notifier chainntnfs.ChainNotifier, t *testing.T) { - t.Logf("testing single conf notification") - // We'd like to test the case of being notified once a txid reaches // a *single* confirmation. // @@ -133,8 +132,6 @@ func testSingleConfirmationNotification(miner *rpctest.Harness, func testMultiConfirmationNotification(miner *rpctest.Harness, notifier chainntnfs.ChainNotifier, t *testing.T) { - t.Logf("testing mulit-conf notification") - // We'd like to test the case of being notified once a txid reaches // N confirmations, where N > 1. // @@ -182,8 +179,6 @@ func testMultiConfirmationNotification(miner *rpctest.Harness, func testBatchConfirmationNotification(miner *rpctest.Harness, notifier chainntnfs.ChainNotifier, t *testing.T) { - t.Logf("testing batch mulit-conf notification") - // We'd like to test a case of serving notifiations to multiple // clients, each requesting to be notified once a txid receives // various numbers of confirmations. @@ -308,8 +303,6 @@ func createSpendTx(outpoint *wire.OutPoint, pkScript []byte, func testSpendNotification(miner *rpctest.Harness, notifier chainntnfs.ChainNotifier, t *testing.T) { - t.Logf("testing multi-client spend notification") - // We'd like to test the spend notifications for all ChainNotifier // concrete implementations. // @@ -401,8 +394,6 @@ func testSpendNotification(miner *rpctest.Harness, func testBlockEpochNotification(miner *rpctest.Harness, notifier chainntnfs.ChainNotifier, t *testing.T) { - t.Logf("testing block epoch notification") - // We'd like to test the case of multiple registered clients receiving // block epoch notifications. @@ -451,8 +442,6 @@ func testBlockEpochNotification(miner *rpctest.Harness, func testMultiClientConfirmationNotification(miner *rpctest.Harness, notifier chainntnfs.ChainNotifier, t *testing.T) { - t.Logf("testing multi-client multi-conf notification") - // We'd like to test the case of a multiple clients registered to // receive a confirmation notification for the same transaction. @@ -513,8 +502,6 @@ func testMultiClientConfirmationNotification(miner *rpctest.Harness, func testTxConfirmedBeforeNtfnRegistration(miner *rpctest.Harness, notifier chainntnfs.ChainNotifier, t *testing.T) { - t.Logf("testing transaction confirmed before notification registration") - // 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. @@ -636,8 +623,6 @@ func testTxConfirmedBeforeNtfnRegistration(miner *rpctest.Harness, func testSpendBeforeNtfnRegistration(miner *rpctest.Harness, notifier chainntnfs.ChainNotifier, t *testing.T) { - t.Logf("testing spend broadcast before notification registration") - // We'd like to test the spend notifications for all ChainNotifier // concrete implementations. // @@ -748,8 +733,6 @@ func testSpendBeforeNtfnRegistration(miner *rpctest.Harness, func testCancelSpendNtfn(node *rpctest.Harness, notifier chainntnfs.ChainNotifier, t *testing.T) { - t.Logf("testing cancel spend notification") - // We'd like to test that once a spend notification is registered, it // can be cancelled before the notification is dispatched. @@ -836,8 +819,6 @@ func testCancelSpendNtfn(node *rpctest.Harness, func testCancelEpochNtfn(node *rpctest.Harness, notifier chainntnfs.ChainNotifier, t *testing.T) { - t.Logf("testing cancel epoch ntfn") - // We'd like to ensure that once a client cancels their block epoch // notifications, no further notifications are sent over the channel // if/when new blocks come in. @@ -885,17 +866,53 @@ func testCancelEpochNtfn(node *rpctest.Harness, notifier chainntnfs.ChainNotifie } } -var ntfnTests = []func(node *rpctest.Harness, notifier chainntnfs.ChainNotifier, t *testing.T){ - testSingleConfirmationNotification, - testMultiConfirmationNotification, - testBatchConfirmationNotification, - testMultiClientConfirmationNotification, - testSpendNotification, - testBlockEpochNotification, - testTxConfirmedBeforeNtfnRegistration, - testSpendBeforeNtfnRegistration, - testCancelSpendNtfn, - testCancelEpochNtfn, +type testCase struct { + name string + + test func(node *rpctest.Harness, notifier chainntnfs.ChainNotifier, t *testing.T) +} + +var ntfnTests = []testCase{ + { + name: "single conf ntfn", + test: testSingleConfirmationNotification, + }, + { + name: "multi conf ntfn", + test: testMultiConfirmationNotification, + }, + { + name: "batch conf ntfn", + test: testBatchConfirmationNotification, + }, + { + name: "multi client conf", + test: testMultiClientConfirmationNotification, + }, + { + name: "spend ntfn", + test: testSpendNotification, + }, + { + name: "block epoch", + test: testBlockEpochNotification, + }, + { + name: "historical conf dispatch", + test: testTxConfirmedBeforeNtfnRegistration, + }, + { + name: "historical spend dispatch", + test: testSpendBeforeNtfnRegistration, + }, + { + name: "cancel spend ntfn", + test: testCancelSpendNtfn, + }, + { + name: "cancel epoch ntfn", + test: testCancelEpochNtfn, + }, } // TestInterfaces tests all registered interfaces with a unified set of tests @@ -997,7 +1014,16 @@ func TestInterfaces(t *testing.T) { } for _, ntfnTest := range ntfnTests { - ntfnTest(miner, notifier, t) + testName := fmt.Sprintf("%v: %v", notifierType, + ntfnTest.name) + + success := t.Run(testName, func(t *testing.T) { + ntfnTest.test(miner, notifier, t) + }) + + if !success { + break + } } notifier.Stop() diff --git a/chainntnfs/neutrinonotify/neutrino.go b/chainntnfs/neutrinonotify/neutrino.go index 8f2cd628c..4e9f20347 100644 --- a/chainntnfs/neutrinonotify/neutrino.go +++ b/chainntnfs/neutrinonotify/neutrino.go @@ -5,6 +5,7 @@ import ( "errors" "sync" "sync/atomic" + "time" "github.com/lightninglabs/neutrino" "github.com/lightningnetwork/lnd/chainntnfs" @@ -716,6 +717,22 @@ func (n *NeutrinoNotifier) RegisterSpendNtfn(outpoint *wire.OutPoint, }, } + // Ensure that neutrino is caught up to the height hint before we + // attempt to fetch the utxo fromt the chain. If we're behind, then we + // may miss a notification dispatch. + for { + n.heightMtx.RLock() + currentHeight := n.bestHeight + n.heightMtx.RUnlock() + + if currentHeight < heightHint { + time.Sleep(time.Millisecond * 200) + continue + } + + break + } + // Before sending off the notification request, we'll attempt to see if // this output is still spent or not at this point in the chain. spendReport, err := n.p2pNode.GetUtxo( @@ -758,6 +775,7 @@ func (n *NeutrinoNotifier) RegisterSpendNtfn(outpoint *wire.OutPoint, neutrino.AddOutPoints(*outpoint), neutrino.Rewind(currentHeight), } + if err := n.chainView.Update(rescanUpdate...); err != nil { return nil, err }