From b3509d491aabbeacbd8a572a6f147022d2527618 Mon Sep 17 00:00:00 2001 From: Sam Lewis Date: Thu, 9 Nov 2017 18:10:15 +1000 Subject: [PATCH] chainntnfs: Add chainntfs lazy consumer test This test adds a test for a consumer that registers for a transaction confirmation but takes some time to check if that confirmation has occured. The test reveals a race condition that can cause btcdnotify to add a confirmation entry to its internal heap twice. If the notification consumer is not prompt in reading from the confirmation channel, this can cause the notifier to block indefinitely. --- chainntnfs/interface_test.go | 84 ++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/chainntnfs/interface_test.go b/chainntnfs/interface_test.go index c55f3bb49..31846bc08 100644 --- a/chainntnfs/interface_test.go +++ b/chainntnfs/interface_test.go @@ -604,6 +604,86 @@ func testTxConfirmedBeforeNtfnRegistration(miner *rpctest.Harness, } } +// Test the case of a notification consumer having forget or being delayed in +// checking for a confirmation. This should not cause the notifier to stop +// working +func testLazyNtfnConsumer(miner *rpctest.Harness, + notifier chainntnfs.ChainNotifier, t *testing.T) { + + // Create a transaction to be notified about. We'll register for + // notifications on this transaction but won't be prompt in checking them + txid, err := getTestTxId(miner) + if err != nil { + t.Fatalf("unable to create test tx: %v", err) + } + + _, currentHeight, err := miner.Node.GetBestBlock() + if err != nil { + t.Fatalf("unable to get current height: %v", err) + } + + numConfs := uint32(3) + + // Add a block right before registering, this makes race conditions + // between the historical dispatcher and the normal dispatcher more obvious + if _, err := miner.Node.Generate(1); err != nil { + t.Fatalf("unable to generate blocks: %v", err) + } + + firstConfIntent, err := notifier.RegisterConfirmationsNtfn(txid, numConfs, + uint32(currentHeight)) + if err != nil { + t.Fatalf("unable to register ntfn: %v", err) + } + + // Generate another 2 blocks, this should dispatch the confirm notification + if _, err := miner.Node.Generate(2); err != nil { + t.Fatalf("unable to generate blocks: %v", err) + } + + // 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, err = getTestTxId(miner) + if err != nil { + t.Fatalf("unable to create test tx: %v", err) + } + + _, currentHeight, err = miner.Node.GetBestBlock() + if err != nil { + t.Fatalf("unable to get current height: %v", err) + } + + numConfs = 1 + + secondConfIntent, err := notifier.RegisterConfirmationsNtfn(txid, numConfs, + uint32(currentHeight)) + + if err != nil { + t.Fatalf("unable to register ntfn: %v", err) + } + + if _, err := miner.Node.Generate(1); err != nil { + t.Fatalf("unable to generate blocks: %v", err) + } + + select { + case <-secondConfIntent.Confirmed: + // Successfully receive the second notification + break + case <-time.After(30 * time.Second): + t.Fatalf("Second confirmation notification never received") + } + + // Make sure the first tx confirmed successfully + select { + case <-firstConfIntent.Confirmed: + break + case <-time.After(30 * time.Second): + t.Fatalf("First confirmation notification never received") + } +} + // Tests the case in which a spend notification is requested for a spend that // has already been included in a block. In this case, the spend notification // should be dispatched immediately. @@ -900,6 +980,10 @@ var ntfnTests = []testCase{ name: "cancel epoch ntfn", test: testCancelEpochNtfn, }, + { + name: "lazy ntfn consumer", + test: testLazyNtfnConsumer, + }, } // TestInterfaces tests all registered interfaces with a unified set of tests