Because we pass in the same config into multiple notifiers, we sometimes
get a data race in the unit tests. By creating a copy of the config we
avoid that.
With the chainntnfs.NewMiner now being optimized for not creating
nodes with colliding ports, we use it in all unit tests that spin up
temporary miners.
This commit fixes a number of issues with our temporary bitcoind nodes
that we spin up for unit tests:
- We didn't remove the node's temporary data dir after tests finished.
- We used random ports which lead to unwanted port collisions.
- We used ipc:// unix sockets for ZMQ which currently isn't supported
in bitcoind v26.0 due to a regression. Since we can reliably create
new non-colliding ports now, we should use TCP for ZMQ anyway.
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.
