diff --git a/chainntnfs/test/test_interface.go b/chainntnfs/test/test_interface.go index 9def356c6..de01cc149 100644 --- a/chainntnfs/test/test_interface.go +++ b/chainntnfs/test/test_interface.go @@ -25,6 +25,7 @@ import ( "github.com/lightningnetwork/lnd/chainntnfs/btcdnotify" "github.com/lightningnetwork/lnd/chainntnfs/neutrinonotify" "github.com/lightningnetwork/lnd/channeldb" + "github.com/lightningnetwork/lnd/lnutils" "github.com/stretchr/testify/require" ) @@ -1709,6 +1710,90 @@ func testCatchUpOnMissedBlocksWithReorg(miner1 *rpctest.Harness, } } +// testIncludeBlockAsymmetry tests that if the same output is registered for a +// notification by two callers, one is able to get a notification with the +// block and the other one without it. +func testIncludeBlockAsymmetry(miner *rpctest.Harness, + notifier chainntnfs.TestChainNotifier, scriptDispatch bool, + t *testing.T) { + + // We'll start by creating a new test transaction, waiting long enough + // for it to get into the mempool. + txid, pkScript, err := chainntnfs.GetTestTxidAndScript(miner) + require.NoError(t, err, "unable to create test tx") + if err := chainntnfs.WaitForMempoolTx(miner, txid); err != nil { + t.Fatalf("tx not relayed to miner: %v", err) + } + + _, currentHeight, err := miner.Client.GetBestBlock() + require.NoError(t, err, "unable to get current height") + + var ( + confIntentNoBlock *chainntnfs.ConfirmationEvent + confIntentBlock *chainntnfs.ConfirmationEvent + + numConfsLong = uint32(6) + numConfsShort = uint32(1) + ) + dispatchClients := func() { + dispatchTxid := txid + if scriptDispatch { + dispatchTxid = nil + } + + confIntentNoBlock, err = notifier.RegisterConfirmationsNtfn( + dispatchTxid, pkScript, numConfsLong, + uint32(currentHeight), + ) + require.NoError(t, err) + + confIntentBlock, err = notifier.RegisterConfirmationsNtfn( + dispatchTxid, pkScript, numConfsShort, + uint32(currentHeight), chainntnfs.WithIncludeBlock(), + ) + require.NoError(t, err) + } + assertNtfns := func() { + // Make sure the long confirmation client receives the + // notification but not the block. + confNtfnNoBlock, err := lnutils.RecvOrTimeout( + confIntentNoBlock.Confirmed, time.Second*5, + ) + require.NoError(t, err) + require.Nil(t, (*confNtfnNoBlock).Block, "block not included") + + // And the short confirmation client receives the notification + // and the block. + confNtfnBlock, err := lnutils.RecvOrTimeout( + confIntentBlock.Confirmed, time.Second*5, + ) + require.NoError(t, err) + require.NotNil(t, (*confNtfnBlock).Block, "block included") + } + + // First, we start off by registering two clients for the same txid and + // pkScript. One of them will require 6 confirmations but not request + // the block, the other will just require a single confirmation and + // request the block. + dispatchClients() + + // Next, we'll generate a few blocks, which should confirm the + // transaction created above and trigger the notifications, as it should + // be confirmed enough for both clients. + _, err = miner.Client.Generate(6) + require.NoError(t, err, "unable to generate single block") + + // Make sure we got the notifications we expected. + assertNtfns() + + // Now dispatch the same clients again, which should hit the same + // conditions as above and use the cached rescan details. + dispatchClients() + + // And again, the notifications should be triggered as expected. + assertNtfns() +} + type txNtfnTestCase struct { name string test func(node *rpctest.Harness, notifier chainntnfs.TestChainNotifier, @@ -1772,6 +1857,10 @@ var txNtfnTests = []txNtfnTestCase{ name: "cancel spend ntfn", test: testCancelSpendNtfn, }, + { + name: "test include block asymmetry", + test: testIncludeBlockAsymmetry, + }, } var blockNtfnTests = []blockNtfnTestCase{ diff --git a/chainntnfs/txnotifier.go b/chainntnfs/txnotifier.go index d8f5b4c7c..977ad3def 100644 --- a/chainntnfs/txnotifier.go +++ b/chainntnfs/txnotifier.go @@ -666,11 +666,15 @@ func (n *TxNotifier) RegisterConf(txid *chainhash.Hash, pkScript []byte, // block along with the rest of the details. However not all // clients want the block, so we make a copy here w/o the block // if needed so we can give clients only what they ask for. - if !ntfn.includeBlock && confSet.details != nil { - confSet.details.Block = nil + confDetails := confSet.details + if !ntfn.includeBlock && confDetails != nil { + confDetailsCopy := *confDetails + confDetailsCopy.Block = nil + + confDetails = &confDetailsCopy } - err := n.dispatchConfDetails(ntfn, confSet.details) + err := n.dispatchConfDetails(ntfn, confDetails) if err != nil { return nil, err }