In this commit, we take an initial step towards converting the existing
breach arbiter and utxo nursery logic into contract resolvers by moving
the files as is, into the `contractcourt` pacakge.
This commit is primarily move only, though we had to massage some
interfaces and config names along the way to make things compile and the
tests run properly.
This fixes a flake I've seen in the wild lately:
```
--- FAIL: TestBlockDifferenceFix (0.01s)
router_test.go:4335: height should have been updated to 5, instead got 4
FAIL
FAIL github.com/lightningnetwork/lnd/routing 3.865s
FAIL
```
We wrap things in an assertion loop to ensure that timing quirks don't
cause the test to fail sporadically.
In this commit, we modify the existing logic that defaults to implicit
commitment type negotiation to support explicit negotiation if the new
feature bit is set. This change allows us to ditch the notion of a
"default" commitment type, as we'll now use feature bits to signal our
understanding of a commiment type, but allow peers to select which
commitment type they actually wish to use.
In addition, this explicit negotiation removes the need for using the
required bit of any commitment types. Instead, if an implementation
wishes to no longer support a commitment type, they should simply stop
advertising the optional bit.
This field will be examined later down the stack along with the set of
feature bits to determine if explicit channel commitment type
negotiation is possible or not.
If these bits are present, then both sides can examine the new
CommitmentType TLV field that's present and use this in place of the
existing implicit commiment type negotiation. With this change, it's now
possible to actually deprecate old unsupported commitment types
properly.
In this commit, we add a new ChannelType field as a new TLV record to
the OpenChannel message. During this change, we make a few tweaks to the
generic TLV encode/decode methods for the ExtraOpaqueData struct to have
it work on the level of tlv.RecordProducer instead of tlv.Record, as
this reduces line noise a bit.
We also partially undo existing logic that would attempt to "prepend"
any new TLV records to the end of the ExtraOpaqueData if one was already
present within the struct. This is based on the assumption that if we've
read a message from disk to order to re-send/transmit it, then the
ExtraOpaqueData is fully populated so we'll write that as is. Otherwise,
a message is being encoded for the first time, and we expect all fields
that are known TLV fields to be specified within the struct itself.
This change required the unit tests to be modified slightly, as we'll
always encode a fresh set of TLV records if none was already specified
within the struct.
In this commit, we add a new TLV record that's intended to be used as an
explicit channel commitment type for a new form of funding negotiation,
and later on a dynamic commitment upgrade protocol. As defined, we have
3 channel types: base (the OG), tweakless, and anchors w/ zero fee
HTLCs. We omit the original variant of anchors as it was never truly
deployed from the PoV of lnd.
In this commit, which builds on top of the prior commit, rather than
using the returned buffer outside of the closure (which means it'll be
copied), we instead use it within the `Submit` closure instead. With the
recent changes to the `brontide` package, we won't allocate any new
buffer when we decrypt, as a result, the `rawMsg` byte slice actually
just slices into the passed `buf` slice (obtained from the pool)`.
With this change, we ensure that the buffer pool can release the slice
back to the pool and eliminate any extra allocations along the way.
In this commit, we implement a simple optimization that dramatically
reduces the number of allocations we need to make when we decrypt a new
message. Before this commit, we would pass in a `nil` value to the
`Decrypt` method which meant that it would always allocate a new
buffers.
Rather than force this behavior, in this commit, we pass in the
ciphertext buffer (with a length of zero), such that the decryption
operation will simply copy the plaintext bytes over the cipher text in
place. This works as the cipher text is always larger than the
plaintext, since the plaintext doesn't have a MAC attached.
The amount the perf increase, amount of allocations, and amount of bytes
allocated are pretty nice:
```
benchmark old ns/op new ns/op delta
BenchmarkReadHeaderAndBody-8 88652 75896 -14.39%
benchmark old allocs new allocs delta
BenchmarkReadHeaderAndBody-8 6 4 -33.33%
benchmark old bytes new bytes delta
BenchmarkReadHeaderAndBody-8 65664 128 -99.81%
```
Here old is without this change, and new with it.
In this commit, we start to optimistically use the new private key cache
that was added to btcwallet. As is, btcwallet will cache the decrypted
account keys for each scope in memory. However, the existing methods
to derive a child key from those account keys requires a write database
transaction, and will re-derive the private key using BIP-32 each time.
The newly added `DeriveFromKeyPathCache` lets us skip all this and
directly use a cache assuming the account info is already cached. The
new logic will try to use this method, but if it fails fall back to the
existing `DeriveFromKeyPath` method. All calls after this will use this
new cached key.
Fixes https://github.com/lightningnetwork/lnd/issues/5125.
Basic benchmark before the btcwallet change and after:
```
benchmark old ns/op new ns/op delta
BenchmarkDerivePrivKey-8 22840583 125 -100.00%
benchmark old allocs new allocs delta
BenchmarkDerivePrivKey-8 89 2 -97.75%
benchmark old bytes new bytes delta
BenchmarkDerivePrivKey-8 10225 24 -99.77%
```
In this commit, we implement a long discussed mechanism to use the
Lightning Network as a redundant block header source. By sending our
latest block header with each ping message, we give peers another source
(outside of the Bitcoin P2P network) they can use to spot check their
chain state. Peers can also use this information to detect if they've
been eclipsed from the traditional Bitcoin P2P network itself.
As is, we only send this data in Ping messages (which are periodically
sent), in the future we could also move to send them as the partial
payload for our pong messages, and also randomize the payload size
requested as well.
In this commit, we fix a bug that would cause attempts to re-use an AMP
invoice to fail. Without this commit, we would only attempt to parse the
payment addr if no invoice was specified, so a user manually specifying the
pubkey of the detonation. The fix is straight forward: always parse the
`pay_addr` field as the user may be attempting to re-use an AMP invoice w/o
open coding each of the sections.
* server: disable DNS boostrap for SigNet nodes
In this commit, we fix a bug that would cause nodes attempting to run
using the signet chain, to waste CPU as they attempted to boostrap to a
non-existent DNS seed. We fix this by ignoring the DNS boostrapper is
signet is active. Along the way, we refactor the conditional sightly to
be more readable, and easily extensible.
* server: extract bootstrap logic into new function w/ unit test
* docs/release-notes: update release notes for signet DNS fix
In this commit, we fix a bug that would cause nodes attempting to run
using the signet chain, to waste CPU as they attempted to boostrap to a
non-existent DNS seed. We fix this by ignoring the DNS boostrapper is
signet is active. Along the way, we refactor the conditional sightly to
be more readable, and easily extensible.
Without this commit, the build may break if a PR commit is created, as
that shows the `PR_NUMBER` field we parse out as "master", reflecting
that its a merge commit into the master branch.
In this commit, we add a simple bash script to parse out the current PR
number from an environment variable in the GH actions context, and use
that to check to see if the PR has been referenced in the release notes
or not. This isn't 100% fool proof, but it should catch most of the
common cases.
In this commit, we attempt to fix a race condition that may occur in the
current AMP and MPP tests.
It appears the following scenario is possible:
* The `mppTestContext` [is used to create 6 channels back to
back](https://github.com/lightningnetwork/lnd/blob/master/lntest/itest/lnd_amp_test.go#L43)
* The method used to create the channel ends up calling
[`openChannelAndAssert`](https://github.com/lightningnetwork/lnd/blob/edd4152682dc8f70e11da541cd3be4692db4b011/lntest/itest/lnd_mpp_test.go#L300)
which'll open the channel, mine 6 blocks, [then ensure that the
channel gets
advertised](https://github.com/lightningnetwork/lnd/blob/edd4152682dc8f70e11da541cd3be4692db4b011/lntest/itest/assertions.go#L78)
* Later on, [we wait for all nodes to hear about all channels on the
network
level](https://github.com/lightningnetwork/lnd/blob/master/lntest/itest/lnd_amp_test.go#L62)
I think the issue here is that we'll potentially already have mined 30
or so blocks before getting to the final nodes, and those nodes may have
already heard about the channel already. This may then cause their
[`lightningNetworkWatcher`](https://github.com/lightningnetwork/lnd/blob/edd4152682dc8f70e11da541cd3be4692db4b011/lntest/node.go#L1213)
goroutine to not properly dispatch this, since it's assumed that the
channel hasn't been announced (or notified) when the method is called.
One solution here is to just check if the channel is already in the
node's graph or not, when we go to register the notification. If we do
this in the same state machine as the watcher, then we ensure if the
channel is already known, the client is immediately notified. One thing
that can help us debug this more in the future is adding additional
logging in some of these helper goroutines so we can more easily track
the control flow.
This commit implements this solution by checking to ensure that the
channel isn't already known in our channel graph before attempting to
wait for its notification, as we may already have missed the
notification before this registration request came in.
In this commit, we fix a regression introduced by a recent bug fix in
this area. Before this change, we'd inspect the error returned by
`processSendError`, and then fail the payment from the PoV of mission
control using the returned error.
A recent refactoring removed `processSendError` and combined the logic
with `tryApplyChannelUpdate` in order to introduce a new
`handleSendError` method that consolidates the logic within the
`shardHandler`. Along the way, the behavior of the prior check was
replicated in the form of a new internal `failPayment` closure. However,
the new function closure ends up returning a `channeldb.FailureReason`
instance, which is actually an `error`.
In the wild, when `SendToRoute` fails due to an error at the
destination, then this new logic caused the `handleSendErorr` method to
fail with an error, returning an unstructured error back to the caller,
instead of the usual payment failure details.
We fix this by no longer checking the `handleSendErorr` for an error as
normal. The `handleSendErorr` function as is will always return an error
of type `*channeldb.FailureReason`, therefore we don't need to treat it
as a normal error. Instead, we check for the type of error returned, and
update the control tower state accordingly.
With this commit, the test added in the prior commit now passes.
Fixes#5477.
In this commit, we modify the existing `TestSendToRouteStructuredError`
test to return an error that doesn't trigger the second chance logic.
Otherwise, we'll get a nil failure result from the mission control
interpretation, meaning we won't exercise the full code path. Instead,
we use a terminal error to ensure that the expected code path is hit.
As is, this test will fail as a recent refactoring causes us to return a
`channeldb.FailureReason` error, since the newly added `handleSendError`
code path in the `SendToRoute` method will return the raw error, rather
than the `shardError`, which is of the expected type.
In this commit, in order to allow the test added in the prior commit to
pass, we'll increment the mockHTLCAmt value by 1 to ensure we never
attempt to add a zero value HTLC.
Fixes#5468
As is, if the remote party proposes a min HTLC of 0 `mSat` to us, then
we won't ever be able to _send outgoing_ in the channel as the
`MayAddOutgoingHtlc` will attempt to add a zero-value HTLC, which isn't
allowed within the protocol.
The default channels created actually already use a min HTLC value of
zero within the tests, so this test case fails as is.
Olaoluwa Osuntokun