chainntnfs: remove queued confirmation notification in CancelConf

This addresses a panic when a notification is canceled after its been
detected as included in a block and before its confirmation notification
is dispatched.
This commit is contained in:
Wilmer Paulino 2020-06-04 17:19:07 -07:00
parent be36776120
commit 69b8a356b2
No known key found for this signature in database
GPG Key ID: 6DF57B9F9514972F
2 changed files with 39 additions and 6 deletions

@ -752,7 +752,21 @@ func (n *TxNotifier) CancelConf(confRequest ConfRequest, confID uint64) {
close(ntfn.Event.Confirmed)
close(ntfn.Event.Updates)
close(ntfn.Event.NegativeConf)
// Finally, we'll clean up any lingering references to this
// notification.
delete(confSet.ntfns, confID)
// Remove the queued confirmation notification if the transaction has
// already confirmed, but hasn't met its required number of
// confirmations.
if confSet.details != nil {
confHeight := confSet.details.BlockHeight +
ntfn.NumConfirmations - 1
if confHeight <= n.currentHeight {
delete(n.ntfnsByConfirmHeight[confHeight], ntfn)
}
}
}
// UpdateConfDetails attempts to update the confirmation details for an active

@ -1138,7 +1138,7 @@ func TestTxNotifierCancelConf(t *testing.T) {
hintCache := newMockHintCache()
n := chainntnfs.NewTxNotifier(startingHeight, 100, hintCache, hintCache)
// We'll register two notification requests. Only the second one will be
// We'll register three notification requests. The last two will be
// canceled.
tx1 := wire.NewMsgTx(1)
tx1.AddTxOut(&wire.TxOut{PkScript: testRawScript})
@ -1155,8 +1155,15 @@ func TestTxNotifierCancelConf(t *testing.T) {
if err != nil {
t.Fatalf("unable to register spend ntfn: %v", err)
}
ntfn3, err := n.RegisterConf(&tx2Hash, testRawScript, 1, 1)
if err != nil {
t.Fatalf("unable to register spend ntfn: %v", err)
}
cancelConfRequest := ntfn2.HistoricalDispatch.ConfRequest
// Construct a block that will confirm both transactions.
// Extend the chain with a block that will confirm both transactions.
// This will queue confirmation notifications to dispatch once their
// respective heights have been met.
block := btcutil.NewBlock(&wire.MsgBlock{
Transactions: []*wire.MsgTx{tx1, tx2},
})
@ -1167,14 +1174,18 @@ func TestTxNotifierCancelConf(t *testing.T) {
Tx: tx1,
}
// Before extending the notifier's tip with the block above, we'll
// cancel the second request.
n.CancelConf(ntfn2.HistoricalDispatch.ConfRequest, 2)
// Cancel the second notification before connecting the block.
n.CancelConf(cancelConfRequest, 2)
err = n.ConnectTip(block.Hash(), startingHeight+1, block.Transactions())
if err != nil {
t.Fatalf("unable to connect block: %v", err)
}
// Cancel the third notification before notifying to ensure its queued
// confirmation notification gets removed as well.
n.CancelConf(cancelConfRequest, 3)
if err := n.NotifyHeight(startingHeight + 1); err != nil {
t.Fatalf("unable to dispatch notifications: %v", err)
}
@ -1188,7 +1199,7 @@ func TestTxNotifierCancelConf(t *testing.T) {
t.Fatalf("expected to receive confirmation notification")
}
// The second one, however, should not have. The event's Confirmed
// The second and third, however, should not have. The event's Confirmed
// channel must have also been closed to indicate the caller that the
// TxNotifier can no longer fulfill their canceled request.
select {
@ -1199,6 +1210,14 @@ func TestTxNotifierCancelConf(t *testing.T) {
default:
t.Fatal("expected Confirmed channel to be closed")
}
select {
case _, ok := <-ntfn3.Event.Confirmed:
if ok {
t.Fatal("expected Confirmed channel to be closed")
}
default:
t.Fatal("expected Confirmed channel to be closed")
}
}
// TestTxNotifierCancelSpend ensures that a spend notification after a client