Commit Graph

69 Commits

Author SHA1 Message Date
Olaoluwa Osuntokun
6781b17056
chainntnfs: update bitcoind and btcd backends to match new spend ntfn API 2018-07-31 21:28:53 -07:00
Olaoluwa Osuntokun
e93149e576
chainntnfs/btcdnotify: update RegisterConfirmationsNtfn to take pkScript 2018-07-31 21:28:49 -07:00
Wilmer Paulino
12816a910d chainntnfs: make historical confirmation rescans async 2018-07-31 18:23:25 -07:00
Wilmer Paulino
c43506dee9 chainntnfs: add unique ID field to track conf ntfns within notifier 2018-07-31 18:23:25 -07:00
Johan T. Halseth
3808105dcd
chainntnfs/btcdnotify: remove all mempool spend clients 2018-07-22 23:09:08 +02:00
Johan T. Halseth
44d7b84df0
chainntnfs: remove mempool option from RegisterSpendNtfn 2018-07-22 23:09:08 +02:00
Olaoluwa Osuntokun
274687471a
chainntnfs/btcdnotify: switch to async rescan call for historical spend ntfns
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.
2018-07-18 01:31:49 -07:00
Olaoluwa Osuntokun
6f60f139f4 multi: switch over import paths from roasbeef/* to btcsuite/* 2018-07-13 17:05:39 -07:00
Wilmer Paulino
13fb866574
chainntnfs: remove txindex requirement when registering notfications
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.
2018-04-17 14:59:51 -04:00
Johan T. Halseth
5d6dd90d18
chainntnfs/btcdnotify: correctly notify on confirmed rescan spends
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.
2018-04-16 20:09:08 +02:00
Olaoluwa Osuntokun
86ad6d318e
Merge pull request #889 from wpaulino/chainnotifier-updates
chainntnfs: add incremental update notifications within ChainNotifier
2018-04-12 19:21:26 -07:00
Wilmer Paulino
486694a84e
chainntnfs: add Updates channel field to ConfirmationEvent
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.
2018-04-06 00:30:19 -04:00
Johan T. Halseth
cc17bc1636
chainntnfs/btcd: add logic for mempool argument to RegisterSpendNtfn
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.
2018-04-03 11:24:07 +02:00
Johan T. Halseth
b08fc05390
chainntnfs/btcd: move NotifySpent to after recording the outpoint
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.
2018-03-28 10:21:08 +02:00
Olaoluwa Osuntokun
9c18c3d9a4
chainntnfs: ensure all block epoch notifications are sent *in order*
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.
2018-02-09 16:13:28 -08:00
Olaoluwa Osuntokun
bf6001074c
chainntnfs: fix spend notification registration race condition
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.
2018-01-28 14:48:56 -08:00
Jim Posen
bc7c834362 chainntnfs: Fix stylistic issues. 2017-12-14 19:16:15 -08:00
Jim Posen
280e264e8c chainntnfs: Implement quit signal in TxConfNotifier. 2017-12-14 19:16:15 -08:00
Jim Posen
4405dac4d0 chainntnfs/btcd: Refactor BtcdNotifier to use TxConfNotifier. 2017-12-14 19:16:15 -08:00
Jim Posen
0cac7e80d4 chainntnfs/btcd: Handle block disconnects with chainUpdate.
This does not implement full handling of block disconnections, but
ensures that all chain updates are processed in order.
2017-12-14 19:16:15 -08:00
Olaoluwa Osuntokun
839ce0689e
chainntnfs/btcdnotify: add additional logic when dispatching ntfns 2017-12-07 19:09:53 -08:00
Jim Posen
0297042a8d chainntnfs: Use the new ConcurrentQueue in btcd notifier. 2017-11-16 15:15:22 -08:00
Sam Lewis
1f95b660b9 chainntfns: Fix off by 1 block height error
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.
2017-11-13 22:36:25 -08:00
Sam Lewis
93981a85c0 chainntnfs: Fix btcdnotify dispatch race condition
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.
2017-11-13 22:36:25 -08:00
Olaoluwa Osuntokun
5044cd6468
chainntnfs/btcdnotify: recognize JSON-RPC error in RegisterSpendNtfn
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.
2017-09-12 17:11:47 +02:00
Olaoluwa Osuntokun
ed7eae819a
chainntnfs/btcdnotify: don't error if tx not found for historical conf dispatch
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.
2017-09-12 17:06:58 +02:00
Philip Hayes
f0aa186a56 channeldb+utxonursery+lnwire: use lnwire's OutPoint,TxOut serialization 2017-08-25 17:56:50 -07:00
Olaoluwa Osuntokun
9f0efddc20
multi: switch from btcrpcclient to rpcclient 2017-08-24 18:54:24 -07:00
Conner Fromknecht
d1e797451d chainntnfs/btcd+neutrino: close spendChan after send 2017-08-03 17:32:44 -07:00
Olaoluwa Osuntokun
399d9c974f
chainntnfs: ensure ntfn cancellation loop will exit
This commit fixes a slight bug introduced. We now ensure that the
cancel loop always exists if the ChainNotifier has been signaled for a
quit.
2017-08-01 17:14:11 -07:00
Conner Fromknecht
9f85eadde1 chainntnfs/btcd+neutrino: sync epoch cancel 2017-07-31 21:44:23 -07:00
Conner Fromknecht
a9b1af4c73 chainntnfs/btcd+neurtino: unify + sync ntfn cancels 2017-07-31 21:44:23 -07:00
Olaoluwa Osuntokun
c47047c0b8
chainntnfs: extend interface test to ensure initial conf height is correct 2017-07-11 17:38:43 -07:00
Olaoluwa Osuntokun
54cb8a05cd
chainntnfs: fix vet error, don't pass lock by value 2017-06-07 17:07:33 -07:00
Olaoluwa Osuntokun
625d57aea6
chainntnfs/btcdnotify: ensure block epoch coroutines exit before closing ntfn channel 2017-06-07 17:04:32 -07:00
Olaoluwa Osuntokun
0050108391
chainntnfs/btcdnotify: fix off-by one error when setting spending height 2017-06-05 19:07:48 -07:00
Olaoluwa Osuntokun
49e27c77dd
chainntnfs/btcdnotify: modify logging level from error to warn 2017-05-16 18:46:55 -07:00
Olaoluwa Osuntokun
dfd9f6d1ca
chainntnfs/btcdnotify: fix historical confirmation dispatch bugs
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.
2017-05-15 17:47:07 -07:00
Olaoluwa Osuntokun
2f0639f1af
chainntnfs: add heightHint to RegisterSpendNtfn+ RegisterConfirmationsNtfn
This commit modifies two of the main methods in the ChainNotifier
interface to be more light client friendly. In order to do so, we now
tack on an extra parameter to the methods: heightHint. This value
represents the earliest known height that the chain should be scanned
when attempting to do a dispatch from historical data.

All tests have also been updated to use these new parameters properly
when excising the expected behavior of each interface implementation.
2017-05-11 15:20:38 -07:00
Olaoluwa Osuntokun
45bbeb8f84
chainntnfs/btcdnotify: properly include SpendingHeight in SpendDetails
This commit modifies the btcdnotify implementation of the ChainNotifier
interface to properly include the height in which the watched output
was spent in the SpendDetail sent as a notification.

The set of tests have also been updated to assert that the proper
spending height is included in received notification.
2017-05-11 15:20:36 -07:00
Olaoluwa Osuntokun
4b15310c08
chainntfns/btcdnotify: eliminate block epoch race condition, use diff cancel channel
This commit fixes a race condition that was uncovered by the race
condition detector surrounding cancelling active block epoch
notifications. Previously we would close the main notification channel
for each client, at tine this would cause a read/write race condition
if an active grouting was attempting to dispatch a notification. We now
fix this use by using a distinct channel for signaling cancellation to
the active grouting, and another to signal cancellation to any
notification observers.
2017-05-05 15:53:20 -07:00
Olaoluwa Osuntokun
a9b8da7c3f
chainntfns/btcdnotify: log error when unable to query for transaction 2017-05-04 17:38:52 -07:00
Olaoluwa Osuntokun
c575107607
chaintnfs/btcdnotify: make cancellation of block epoch synchronous
This commit is meant to fix an occasional flake in the interrogation
tests cause by the async nature of the cancellation of block epoch
notifications. This commit modifies the cancellation to now be fully
synchronous which should eliminate this flake.
2017-04-26 21:17:35 -07:00
Olaoluwa Osuntokun
419c2ac206
chainntnfs/btcdnotify: fix race condition for block epoch clients
This commit fixes a race condition that was introduced while fixing a
lingering bug in the logic to notify block epoch clients. The race
condition would happen as by removing the default case in the select
statement, it was now possible for the client’s block epoch client to
be closed while the routine was attempting a send on it.

We now eliminate this race condition possibility by adding a wait group
to all goroutines launched to dispatch a block epoch notification. With
this modification, the Stop() goroutine will now wait for all other
goroutine to exit before closing the block epoch channels of all
currently registered clients.
2017-04-04 14:20:16 +02:00
Olaoluwa Osuntokun
608b9d96d1
chainntnfs/btcdnotify: fix dropped block epoch notification bug
This commit a bug introduced in the chain notifier while we were
limiting the usage of mutexes within the package. In a prior commit a
default case was introduced in the select statement in order to avoid
the possibility of the main goroutine blocking when dispatching block
epoch notification.

In order to avoid this potentially disastrous bug, we now instead
launch a new goroutine for each client to ensure that all notifications
are reliably dispatched.
2017-04-01 20:13:58 +02:00
Andrey Samokhvalov
ee2379775c lnd: fix golint warning which requires to add additional comments 2017-03-13 16:30:23 -07:00
Andrey Samokhvalov
fd97a4bd19 lnd: partially fix golint warnings 2017-03-13 16:30:23 -07:00
Andrey Samokhvalov
143a6e01bb lnd: fix unconvert warnings 2017-03-13 16:30:23 -07:00
Olaoluwa Osuntokun
6c81dfad61
chainntfns/btcdnotifier: fix race condition in notifyBlockEpochs
This commit fixes a race condition in the notifyBlockEpochs detected by
the race condition detector. Previously the notifyBlockEpochs function
could cause a race condition when a new caller was either cancelling an
existing notification intent or creating a new one.

We fix this issue by making the call to notifyBlockEpochs synchronous
rather than asynchronous. An alternative would be to add a mutex
guarding the map state. The channel itself is buffered with a good
margin, so there shouldn’t be a huge impact.
2017-02-22 14:51:48 -08:00
Olaoluwa Osuntokun
73cc28d5fb
chainntnfs/btcdnotify: implement spend+epoch ntfn cancellations
This commit minifies the BtcdNotifier concrete implementation of the
ChainNotifier interface to allow callers to optionally cancel an
outstanding block epoch or spend notificaiton intent.

To do this efficiently, we now give each notification intent a unique
ID based on if it’s an epoch intent or a spend intent. We then use this
ID to reference back to the original un-dispatched notification intent
when the caller wishes to cancel the intent.
2017-02-21 01:42:53 -08:00