Skip to content

Commit 4ff1f19

Browse files
committed
chainntnfs: make sure notification is sent to the whole set
This commit fixes a bug where the confirmation details may be missed. When the same tx is subscribed via `RegisterConfirmationsNtfn`, we will put them into the same set and notify the whole set. However, this logic is missing when performing the rescan - once the confirmation detail is found, we only notify the current subscriber. Later we will skip notifying other subscribers in `UpdateConfDetails` due to the `confSet.details != nil` check. We now fix it by immediately notify all the subscribers when the confirmation detail is found during the rescan.
1 parent 1200b75 commit 4ff1f19

File tree

2 files changed

+30
-15
lines changed

2 files changed

+30
-15
lines changed

chainntnfs/interface.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,9 @@ type ConfirmationEvent struct {
258258
// channels.
259259
func NewConfirmationEvent(numConfs uint32, cancel func()) *ConfirmationEvent {
260260
return &ConfirmationEvent{
261+
// We cannot rely on the subscriber to immediately read from
262+
// the channel so we need to create a larger buffer to avoid
263+
// blocking the notifier.
261264
Confirmed: make(chan *TxConfirmation, 1),
262265
Updates: make(chan uint32, numConfs),
263266
NegativeConf: make(chan int32, 1),

chainntnfs/txnotifier.go

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -664,8 +664,8 @@ func (n *TxNotifier) RegisterConf(txid *chainhash.Hash, pkScript []byte,
664664
// already been found, we'll attempt to deliver them immediately
665665
// to this client.
666666
Log.Debugf("Attempting to dispatch confirmation for %v on "+
667-
"registration since rescan has finished",
668-
ntfn.ConfRequest)
667+
"registration since rescan has finished, conf_id=%v",
668+
ntfn.ConfRequest, ntfn.ConfID)
669669

670670
// The default notification we assigned above includes the
671671
// block along with the rest of the details. However not all
@@ -679,9 +679,13 @@ func (n *TxNotifier) RegisterConf(txid *chainhash.Hash, pkScript []byte,
679679
confDetails = &confDetailsCopy
680680
}
681681

682-
err := n.dispatchConfDetails(ntfn, confDetails)
683-
if err != nil {
684-
return nil, err
682+
// Deliver the details to the whole conf set where this ntfn
683+
// lives in.
684+
for _, subscriber := range confSet.ntfns {
685+
err := n.dispatchConfDetails(subscriber, confDetails)
686+
if err != nil {
687+
return nil, err
688+
}
685689
}
686690

687691
return &ConfRegistration{
@@ -912,10 +916,16 @@ func (n *TxNotifier) dispatchConfDetails(
912916
// If there are no conf details to dispatch or if the notification has
913917
// already been dispatched, then we can skip dispatching to this
914918
// client.
915-
if details == nil || ntfn.dispatched {
916-
Log.Debugf("Skipping dispatch of conf details(%v) for "+
917-
"request %v, dispatched=%v", details, ntfn.ConfRequest,
918-
ntfn.dispatched)
919+
if details == nil {
920+
Log.Debugf("Skipped dispatching nil conf details for request "+
921+
"%v, conf_id=%v", ntfn.ConfRequest, ntfn.ConfID)
922+
923+
return nil
924+
}
925+
926+
if ntfn.dispatched {
927+
Log.Debugf("Skipped dispatched conf details for request %v "+
928+
"conf_id=%v", ntfn.ConfRequest, ntfn.ConfID)
919929

920930
return nil
921931
}
@@ -925,8 +935,9 @@ func (n *TxNotifier) dispatchConfDetails(
925935
// we'll dispatch a confirmation notification to the caller.
926936
confHeight := details.BlockHeight + ntfn.NumConfirmations - 1
927937
if confHeight <= n.currentHeight {
928-
Log.Debugf("Dispatching %v confirmation notification for %v",
929-
ntfn.NumConfirmations, ntfn.ConfRequest)
938+
Log.Debugf("Dispatching %v confirmation notification for "+
939+
"conf_id=%v, %v", ntfn.NumConfirmations, ntfn.ConfID,
940+
ntfn.ConfRequest)
930941

931942
// We'll send a 0 value to the Updates channel,
932943
// indicating that the transaction/output script has already
@@ -944,8 +955,8 @@ func (n *TxNotifier) dispatchConfDetails(
944955
return ErrTxNotifierExiting
945956
}
946957
} else {
947-
Log.Debugf("Queueing %v confirmation notification for %v at tip ",
948-
ntfn.NumConfirmations, ntfn.ConfRequest)
958+
Log.Debugf("Queueing %v confirmation notification for %v at "+
959+
"tip", ntfn.NumConfirmations, ntfn.ConfRequest)
949960

950961
// Otherwise, we'll keep track of the notification
951962
// request by the height at which we should dispatch the
@@ -1743,8 +1754,9 @@ func (n *TxNotifier) NotifyHeight(height uint32) error {
17431754
for ntfn := range n.ntfnsByConfirmHeight[height] {
17441755
confSet := n.confNotifications[ntfn.ConfRequest]
17451756

1746-
Log.Debugf("Dispatching %v confirmation notification for %v",
1747-
ntfn.NumConfirmations, ntfn.ConfRequest)
1757+
Log.Debugf("Dispatching %v confirmation notification for "+
1758+
"conf_id=%v, %v", ntfn.NumConfirmations, ntfn.ConfID,
1759+
ntfn.ConfRequest)
17481760

17491761
// The default notification we assigned above includes the
17501762
// block along with the rest of the details. However not all

0 commit comments

Comments
 (0)