Fix the following uint test flake,
```
--- FAIL: TestHistoricalConfDetailsTxIndex (0.00s)
--- FAIL: TestHistoricalConfDetailsTxIndex/rpc_polling_enabled (1.16s)
bitcoind_test.go:174: should have found the transaction within the mempool, but did not: TxNotFoundIndex
FAIL
```
Noticed this while updating to latest master in another project:
If there are two notifications for the same output being registered and
the first one does _NOT_ request the block to be included, then the
second one also will not receive the block, even if it explicitly
requests it.
This is caused by the block being removed from the original confirmation
set instead of a copy (as it is done in NotifyHeight and
UpdateConfDetails).
In the NewBitcoindBackend test util function, ensure that the
`rpcpolling` param is switched on in the correct order. Also switch back
to using `ioutil.TempDir` as it seems that the zmq conn strings created
from `t.TempDir()` result in an invalid conn string.
This commit moves the `HeightHintCache` implementation to the
`channeldb` package and inverts the dependency relation between
`chainntnfs` and `channeldb`.
Many packages depend on channeldb for type definitions,
interfaces, etc. `chainntnfs` is an example of that. `chainntnfs`
defines the `SpendHintCache` and `ConfirmHintCache` interfaces but
it also implments them (`HeightHintCache` struct). The implementation
uses logic that should not leak from channeldb (ex: bucket paths).
This makes our code highly coupled + it would not allow us to use any
of these interfaces in a package that is imported by `channeldb`
(circular dependency).
This commit makes sure that no loop variables or other temporary
variables are accessed directly in a goroutine but are instead passed
into the goroutine through a parameter. This makes sure a copy of the
value is put on the stack and is not changed while the outside loop
continues.
In this commit, we add a new option for the existing confirmation
notification system that optionally allows the caller to specify that a
block should be included as well.
The only quirk w/ the implementation here is the neutrino backend:
usually we get filtered blocks, we so need to first fetch the block
again so we can deliver the full block to the notifier. On the notifier
end, it'll only be checking for the transactions we care about, to
sending a full block doesn't affect the correctness.
We also extend the `testBatchConfirmationNotification` test to assert
that a block is only included if the caller specifies it.
Because Taproot key spend only spends don't allow us to re-construct the
spent pkScript from the witness alone, we cannot support registering
spend notifications for v1 pkScripts only. We instead require the
outpoint to be specified. This commit makes it possible to only match by
outpoint and also adds an itest for it.
This commit was previously split into the following parts to ease
review:
- 2d746f68: replace imports
- 4008f0fd: use ecdsa.Signature
- 849e33d1: remove btcec.S256()
- b8f6ebbd: use v2 library correctly
- fa80bca9: bump go modules
With go 1.17 a change to the build flags was implemented:
https://go.googlesource.com/proposal/+/master/design/draft-gobuild.md
The formatter now automatically adds the forward-compatible build tag
format and the linter checks for them, so we need to include them in our
code.
This commit fixes a buggy scenario where:
- a spend of the desired outpoint occurs
- RegisterSpend is called, not immediately notifying
- caller performs a historical dispatch, calling UpdateSpendDetails
- caller is notified on the Spend channel
- re-org occurs
- caller is not notified of the re-org
We fix this by correctly populating the spendsByHeight map when
dispatchSpendDetails is called. This mirrors the confirmation case.
After the error is received on the filter update errChan, update the
EndHeight if we're performing a historical scan. If a block was mined
after the call to RegisterConf/RegisterSpend but before the filter was
updated, then the block would not have the filter applied. This means
that a block containing the desired conf/spend parameters would be
undetected. Fix this by ensuring the historical scan also includes this
height, as it would previously not be included.
Moves filter handling logic for connecting blocks to the
connectFilteredBlock method. The disconnect logic now lives in
disconnectFilteredBlock. After updating the filter, drainChainUpdates
is called which will drain everything from the chainUpdates chan and
apply all updates by calling either connectFilteredBlock or
disconnectFilteredBlock. This will allow callers to update their
EndHeight if performing a historical dispatch, as blocks up to this
height may not have had the filter applied.
ConcurrentQueue has internal structures so if a filterUpdate exists
in it, the idea of draining the queue may not work reliably. The
update may exist in the ConcurrentQueue but may not be available via
ChanOut() when we're ready to drain the ConcurrentQueue. Fix this by
using a regular buffered chan, which will either have the update or
not have the update. Its size is set to 100 as our tests may generate
quite a bit of updates.
This mirrors the logic for the spend case. This prevents a double
dispatch scenario when combined with a later commit. Otherwise, the
confirmation would linger in the buffer which is not ideal.
