chainntnfs: remove block info from conf detail copy

Noticed this while updating to latest master in another project:
If there are two notifications for the same output being registered and
the first one does _NOT_ request the block to be included, then the
second one also will not receive the block, even if it explicitly
requests it.
This is caused by the block being removed from the original confirmation
set instead of a copy (as it is done in NotifyHeight and
UpdateConfDetails).

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{

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
 		}