From 36c299c1d8b0b8ee36257ce34a7b9f7f04451b71 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Thu, 7 Dec 2017 19:08:40 -0800 Subject: [PATCH] chainntnfs/neutrinonotify: fix early historical confirmation dispatch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In this commit, we fix an existing bug within the logic of the neutrino notifier. Rather than properly dispatching only once a transaction had reached the expected number of confirmations, the historical dispatch logic would trigger as soon as the transaction reached a single confirmation. This was due to the fact that we were using the scanHeight variable which would be set to zero to calculate the number of confirmations. The value would end up being the current height, which is generally always greater than the number of expected confirmations. To remedy this, we’ll now properly use the block height the transaction was originally confirmed in to know when to dispatch. This also applies a fix that was discovered in 93981a85c0b47622a3a5e7089b8bca9b80b834c5. --- chainntnfs/neutrinonotify/neutrino.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/chainntnfs/neutrinonotify/neutrino.go b/chainntnfs/neutrinonotify/neutrino.go index 26dd11fd..87d28679 100644 --- a/chainntnfs/neutrinonotify/neutrino.go +++ b/chainntnfs/neutrinonotify/neutrino.go @@ -419,10 +419,7 @@ func (n *NeutrinoNotifier) attemptHistoricalDispatch(msg *confirmationsNotificat targetHash := msg.txid - var ( - confDetails *chainntnfs.TxConfirmation - scanHeight uint32 - ) + var confDetails *chainntnfs.TxConfirmation chainntnfs.Log.Infof("Attempting to trigger dispatch for %v from "+ "historical chain", msg.txid) @@ -503,20 +500,21 @@ chainScan: // Otherwise, we'll calculate the number of confirmations that the // transaction has so we can decide if it has reached the desired // number of confirmations or not. - txConfs := currentHeight - scanHeight + txConfs := currentHeight - confDetails.BlockHeight + 1 // If the transaction has more that enough confirmations, then we can // dispatch it immediately after obtaining for information w.r.t // exactly *when* if got all its confirmations. if uint32(txConfs) >= msg.numConfirmations { + chainntnfs.Log.Infof("Dispatching %v conf notification, "+ + "height=%v", msg.numConfirmations, currentHeight) msg.finConf <- confDetails return true } // Otherwise, the transaction has only been *partially* confirmed, so // we need to insert it into the confirmation heap. - confsLeft := msg.numConfirmations - uint32(txConfs) - confHeight := uint32(currentHeight) + confsLeft + confHeight := confDetails.BlockHeight + msg.numConfirmations - 1 heapEntry := &confEntry{ msg, confDetails, @@ -524,7 +522,7 @@ chainScan: } heap.Push(n.confHeap, heapEntry) - return false + return true } // notifyBlockEpochs notifies all registered block epoch clients of the newly @@ -574,6 +572,9 @@ func (n *NeutrinoNotifier) notifyConfs(newBlockHeight int32) { nextConf := heap.Pop(n.confHeap).(*confEntry) for nextConf.triggerHeight <= uint32(newBlockHeight) { + chainntnfs.Log.Infof("Dispatching %v conf notification, "+ + "height=%v", nextConf.numConfirmations, newBlockHeight) + nextConf.finConf <- nextConf.initialConfDetails if n.confHeap.Len() == 0 {