In this commit, we address a bug where it's possible that we still
attempt to manually scan for a transaction to determine whether it's
been included in the chain even after successfully checking the txindex
and not finding it there. Now, we'll short-circuit this process by
exiting early if the txindex lookup was successful but the transaction
in question was not found. Otherwise, we'll fall back to the manual
scan.
In this commit, we extend the different ChainNotifier implementations to
cache height hints for our spend events. Each outpoint we've requested a
spend notification for will have its initial height hint cached. We then
increment this height hint at every new block for unspent outpoints.
This allows us to retrieve the *exact* height at which the outpoint has
been spent. By doing this, we optimize the different ChainNotifier
implementations since they will no longer have to rescan forward (and
possibly fetch blocks in the neutrino/pruned node case) from the initial
height hint.
In this commit, we alter the different chain notifiers to query their
height hint cache before registering a confimation notification. We do
this as it's possible that the cache has a higher height hint, which
can potentially reduce the amount of blocked fetched when attempting
historical dispatches.
This resolves the situation where a notifier's chain backend skips a series of blocks, causing the notifier to need to dispatch historical block notifications to clients.
Additionally, if the current notifier's best block has been reorged out, this logic enables the notifier to rewind to the common ancestor between the current chain and the outdated best block and dispatches notifications from the ancestor.
This prevents the situation where we notify clients about a newly connected block, and then the block connection itself fails. We also want to set our best block in between connecting the block and notifying clients, in case a client makes queries about the new block they have received.
If the chain backend misses telling the notifier about a series of disconnected blocks, the notifier is now able to disconnect the tip to its new best block.
If a client passes in their best known block when registering for block notifications, check to see if it's behind our best block. If so, dispatch the missed block notifications to the client.
This is necessary because clients that persist their best known block can miss new blocks while registering for notifications.
Clients can optionally pass their best block known into RegisterBlockEpochNtfn. This enables the notifiers to catch up clients on blocks they may have missed.
In this commit, we modify the way to handle historical spend dispatches
to ensure that we don't block the client for very old rescans. Rather
than blocking and waiting for the rescan to finish (which may take
minutes in the worst case), we'll now instead launch a goroutine to
handle the async response of the rescan.
Before this commit, we relied on the need of full nodes to enable the
transaction index. This allowed us to fetch historical details about
transactions in order to register and dispatch confirmation and spend
notifications.
This commit allows us to drop that requirement by providing a fallback
method to use when the transaction index is not enabled. This fallback
method relies on manually scanning blocks for the transactions
requested, starting from the earliest height the transactions could have
been included in, to the current height in the chain.
This commit fixes a recently introduced bug in the btcdnotifier, where
we would skip all spend clients waiting for a confirmed spend in
txUpdates. The regular case where a spend is included in a new block was
correctly handled in onBlockConnected, but the txUpdates queue is also
used for confirmed spends during rescans, which we would miss. This
commit fixes that by checking if the tx update is confirmed or
unconfirmed, and acts accordingly.
In this commit, we add a new Updates channel to our ConfirmationEvent
struct. This channel will be used to deliver updates to a subscriber of
a confirmation notification. Updates will be delivered at every
incremental height of the chain with the number of confirmations
remaining for the transaction to be considered confirmed by the
subscriber.
This commit adds a boolean to RegisterSpendNtfn, giving the caller the
option to only register for notifications on confirmed spends. This is
implemented for the btcd backend using logic similar to what is in used
for Neutrino, paving the way for later unifying them.
This commit moves the call to the btcd backend to start watching an
outpoint for spentness to after we have recorded the outpoint in our
list of clients. This is done to avoid a race that could occur if btcd
quicly sent a spend notification before we had been able to record it in
our map, essentially losing it.
In this commit, we fix a lingering bug related to the way that we
deliver block epoch notifications to end users. Before this commit, we
would launch a new goroutine for *each block*. This was done in order
to ensure that the notification dispatch wouldn’t block the main
goroutine that was dispatching the notifications. This method archived
the goal, but had a nasty side effect that the goroutines could be
re-ordered during scheduling, meaning that in the case of fast
successive blocks, then notifications would be delivered out of order.
Receiving out of order notifications is either disallowed, or can cause
sub-systems that rely on these notifications to get into weird states.
In order to fix this issue, we’ll no longer launch a new goroutine to
deliver each notification to an awaiting client. Instead, each client
will now gain a concurrent in-order queue for notification delivery.
Due to the internal design of chainntnfs.ConcurrentQueue, the caller
should never block, yet the receivers will receive notifications in
order. This change solves the re-ordering issue and also minimizes the
number of goroutines that we’ll create in order to deliver block epoch
notifications.
In this commit, we fix a race condition related to the way we attempt
to query to see if an outpoint has already been spent by the time it’s
registered within the ChainNotifier. If the transaction creating the
outpoint hasn’t made it into the mempool by the time we execute the
GetTxOut call, then we’ll attempt to query for the transaction itself.
In this case, if we query for the transaction, then the block hash
field will be empty as it hasn’t yet made it into a block. Under the
previous logic, we’d then attempt to force a rescan. This is an issue
as the forced rescan will fail since it’ll try to fetch the block hash
of all zeroes.
In this commit, we fix this issue by only entering this “fallback to
rescan” logic iff, the transaction has actually been mined.
In the historical dispatch of btcdnotify, the dispatcher checks if a
transaction has been included in a block. If this check happens before
the notifier has processed the update, it's possible that the
currentHeight of the notifier and the currentHeight of the chain might
be out of sync which causes an off by one error when calculating a
target height for the transaction confirmation. This change uses the
height of the block the transaction was found in, rather than the
currentHeight that's known by the notifier to eliminate this.
This race condition can occur if a transaction is included in a block
right when a notification is being added to the notifier for it AND when
the confirmation requires > 1 confirmations. In this case, the
confirmation gets added to the confirmation heap twice.
This commit fixes a prior bug in the logic for registering a new spend
notification. Previously, if the transaction wasn’t found in the
mempool or already confirmed within the chain, then
GetRawTransactionVerbose would return an error which would cause the
function itself to exit with an error.
This issue would then cause the server to be unable to start up as the
breach arbiter would be unable to register for spend notifications for
all the channels that it needed to be watching.
We fix this error simply by recognizing the particular JSON-RPC error
that will be returned in this scenario and treating it as a benign
error.
This commit fixes a prior mishandled error when attempting historical
confirmation dispatches. In the prior version of this code fragment, if
the transaction under the spotlight wasn’t found within the mempool, or
already in the chain, then an error would be returned by
b.chainConn.GetRawTransactionVerbose, which would case the function to
exit with an error. This behavior was incorrect, as during transaction
re-broadcasts, it was possible for transaction not yet to be a member
of either set.
We fix this issue by ensuring that we treat the JSON error code as a
benign error and continue with the notification registration.
This commit fixes to distinct bugs in the way we previously dipatched
notifications for transactions which needed a historical dispatch.
Previously we would compare transactions when scanning the block using
the `tx.Hash` field. This was incorrect has the `Hash` field is
actually the wtxid, not the txid which should be the item being
compared. We fix this within the second bug fix by actually using the
txid to find the proper transaction.
The second fix has to due with a slight race condition which led to an
off-by-one error when dispatching the historical confirmation. If while
we were dispatching the confirmation, a new block was found, then we
could calculate the wrong block height (off by one) as we were using
the ‘currentHeight’ instead the height of the block which included the
transaction.
