Matt Corallo [Wed, 6 Dec 2023 01:17:48 +0000 (01:17 +0000)]
`#[inline]` `CandidateRouteHop` accessors
These are used in the performance-critical routing and scoring
operations, which may happen outside of our crate. Thus, we really
need to allow downstream crates to inline these accessors into
their code, which we do here.
Matt Corallo [Wed, 6 Dec 2023 01:13:33 +0000 (01:13 +0000)]
Privatise `CandidateRouteHop::short_channel_id` as its a footgun
Short channel "ID"s are not globally unique when they come from a
BOLT 11 route hint or a first hop (which can be an outbound SCID
alias). In those cases, its rather confusing that we have a
`short_channel_id` method which mixes them all together, and even
more confusing that we have a `CandidateHopId` which is not, in
fact returning a unique identifier.
In our routing logic this is mostly fine - the cost of a collision
isn't super high and we should still do just fine finding a route,
however the same can't be true for downstream users, as they may or
may not rely on the apparent guarantees.
Thus, here, we privatise the SCID and id accessors.
Matt Corallo [Wed, 6 Dec 2023 17:48:51 +0000 (17:48 +0000)]
Fix and re-enable the `remembers_historical_failures` test
f0ecc3ec73dcdb9303b1bd5ac687a361decce2dd introduced a regression in
the `remembers_historical_failures` test, and disabled it by simply
removing the `#[test]` annotation. This fixes the test and marks it
as a test again.
jbesraa [Sat, 26 Aug 2023 15:06:45 +0000 (18:06 +0300)]
Add `source` and `target` fn's to `CandidateRouteHop`
We add `CandidateRouteHop::source` and
`CandidateRouteHop::source` functions to point
to current and next hops in route respectively.
As we have now `source` and `target`
available in `CandidateRouteHop` we also
remove `CandidateRouteHop::id` inputs
so now they are consumed from `self.target`
and `self.source` functions.
In the `add_entry` macro we also remove `source`
and `target` arguments in favor of `candidate`
of type `CandidateRouteHop` that holds the
needed info.
jbesraa [Tue, 5 Dec 2023 18:56:51 +0000 (20:56 +0200)]
Add `outbound` flag to `DirectedChannelInfo`
If `outband` flag is set to true then `ChannelInfo::node_one`
is forwarding a payment to target `ChannelInfo::node_two`.
If `outband` flag is set to false then `ChannelInfo::node_two`
is forwarding a payment to target `ChannelInfo::node_one`.
Matt Corallo [Tue, 28 Nov 2023 23:19:39 +0000 (23:19 +0000)]
Provide inbound HTLC preimages to the `EcdsaChannelSigner`
The VLS signer has a desire to see preimages for resolved forwarded
HTLCs when they are first claimed by us, even if that claim was for
the inbound edge (where claiming strictly increases our balance).
Luckily, providing that information is rather trivial, which we do
here.
Elias Rohrer [Mon, 4 Dec 2023 14:39:13 +0000 (15:39 +0100)]
Log additional details when ignoring first hops
Users are often confused when we fail to find a route due to some
requirements on the first hop are not being met. While we now take note
and log such candidates, we still previously required users to check
additional details to figure out why exactly the router refused to route
over a particular first hop.
Here, we add additional TRACE logging, in particular for
`ChannelDetails::next_outbound_htlc_limit_msat` and
`ChannelDetails::next_outbound_htlc_minimum_msat` when they are
relevant.
Matt Corallo [Sun, 3 Dec 2023 19:09:32 +0000 (19:09 +0000)]
Doc that `Record::node_id` may be missing even with `channel_id`
There are various place where we log something related to a channel
but fail to fill in the channel's counterparty information. This is
somewhat surprising, given channel counterparty information is
always known, but simply is sometimes not readily accessible to LDK
when a log is printed.
Matt Corallo [Sat, 2 Dec 2023 19:13:02 +0000 (19:13 +0000)]
Marginally optimize test logging
973636bd2ab2ba35fb8b9703f1d5d0e72f069cdc introduced a new `HashMap`
in the `TestLogger` but then did lookups by iterating the entire
map. This fixes that, and also takes this opportunity to stop
allocating new `String`s for the module to store each log entry in
the `TestLogger`
Jeffrey Czyz [Wed, 29 Nov 2023 16:51:33 +0000 (10:51 -0600)]
Refactor ChainMonitor::update_channel error case
Move the handling of ChannelMonitorUpdateStatus::UnrecoverableError to
the point of error to avoid needing an unwrap later when re-wrapping the
logger.
Jeffrey Czyz [Tue, 21 Nov 2023 16:47:12 +0000 (10:47 -0600)]
Add semantics to logger::Records
Include optional peer and channel ids to logger::Record. This will be
used by wrappers around Logger in order to provide more context (e.g.,
the peer that sent a message, the channel an operation is pertaining to,
etc.). Implementations of Logger can include this as metadata to aid in
searching logs.
Matt Corallo [Thu, 30 Nov 2023 23:35:43 +0000 (23:35 +0000)]
Drop unnecessary SIMD subtraction in ChaCha20 `round`
While its all constant arithmetic to calculate the shift, which
LLVM likely optimizes out for us, there's no reason to do it four
times, which just makes the code harder to read.
Set HTLCPreviousHopData::blinded on intro node forward.
Useful so we know to fail back blinded HTLCs where we are the intro node with
the invalid_onion_blinding error per BOLT 4.
We don't set this field for blinded received HTLCs because we don't support
receiving to multi-hop blinded paths yet, and there's no point in setting it
for HTLCs received to 1-hop blinded paths because per the spec they should fail
back using an unblinded error code.
A blinding point is provided in update_add_htlc messages if we are relaying or
receiving a payment within a blinded path, to decrypt the onion routing packet
and the recipient-provided encrypted payload within. Will be used in upcoming
commits.
A blinding point is provided in update_add_htlc messages if we are relaying or
receiving a payment within a blinded path, to decrypt the onion routing packet
and the recipient-provided encrypted payload within. Will be used in upcoming
commits.
Arik Sosman [Mon, 28 Aug 2023 23:06:41 +0000 (16:06 -0700)]
Move ECDSA-specific signers into ecdsa.rs
To separate out the logic in the `sign` module, which will start to be
convoluted with multiple signer types, we're splitting out each signer
type into its own submodule, following the taproot.rs example from a
previous commit.
Arik Sosman [Mon, 6 Nov 2023 05:51:15 +0000 (21:51 -0800)]
Reparametrize ChannelSignerType by SignerProvider.
ChannelSignerType is an enum that contains variants of all currently
supported signer types. Given that those signer types are enumerated
as associated types in multiple places, it is prudent to denote one
type as the authority on signer types.
SignerProvider seemed like the best option. Thus, instead of
ChannelSignerType declaring the associated types itself, it simply
uses their definitions from SignerProvider.
Arik Sosman [Mon, 6 Nov 2023 05:36:59 +0000 (21:36 -0800)]
Add TaprootSigner variant to SignerProvider.
Previously, SignerProvider was not laid out to support multiple signer
types. However, with the distinction between ECDSA and Taproot signers,
we now need to account for SignerProviders needing to support both.
This approach does mean that if ever we introduced another signer type
in the future, all implementers of SignerProvider would need to add it
as an associated type, and would also need to write a set of dummy
implementations for any Signer trait they do not wish to support.
For the time being, the TaprootSigner associated type is cfg-gated.
Arik Sosman [Sun, 7 May 2023 04:13:53 +0000 (21:13 -0700)]
Introduce TaprootSigner trait.
For Taproot support, we need to define an alternative trait to
EcdsaChannelSigner. This trait will be implemented by all signers
that wish to support Taproot channels.
Matt Corallo [Thu, 9 Nov 2023 04:14:15 +0000 (04:14 +0000)]
Handle missing case in reestablish local commitment number checks
If we're behind exactly one commitment (which we've revoked), we'd
previously force-close the channel, guaranteeing we'll lose funds
as the counterparty has our latest local commitment state's
revocation secret.
While this shouldn't happen because users should never lose data,
sometimes issues happen, and we should ensure we always panic.
Further, `test_data_loss_protect` is updated to test this case.
Matt Corallo [Thu, 9 Nov 2023 03:28:45 +0000 (03:28 +0000)]
Clean up error messages and conditionals in reestablish handling
When we reestablish there are generally always 4 conditions for
both local and remote commitment transactions:
* we're stale and have possibly lost data
* we're ahead and the peer has lost data
* we're caught up
* we're nearly caught up and need to retransmit one update.
In especially the local commitment case we had a mess of different
comparisons, which is improved here. Further, the error messages
are clarified and include more information.
Matt Corallo [Mon, 27 Nov 2023 21:37:42 +0000 (21:37 +0000)]
Add `channel_keys_id` to `SpendableOutputDescriptor::StaticOutput`
In 7f0fd868ad4e8072440f1eb79e78894de1629157, `channel_keys_id` was
added as an argument to `SignerProvider::get_destination_script`,
allowing implementors to generate a new script for each channel.
This is great, however users then have no way to re-derive the
corresponding private key when they ultimately receive a
`SpendableOutputDescriptor::StaticOutput`. Instead, they have to
track all the addresses as they derive them separately. In many
cases this is fine, but we should support both deployments, which
we do here by simply including the missing `channel_keys_id` for
the user.
olegkubrakov [Tue, 14 Nov 2023 18:08:25 +0000 (10:08 -0800)]
Implement struct wrappers for channel key types to avoid confusion.
Currently all channel keys and their basepoints exist uniformly as
`PublicKey` type, which not only makes in harder for a developer to
distinguish those entities, but also does not engage the language
type system to check if the correct key is being used in any
particular function.
Having struct wrappers around keys also enables more nuanced
semantics allowing to express Lightning Protocol rules in language.
For example, the code allows to derive `HtlcKey` from
`HtlcBasepoint` and not from `PaymentBasepoint`.
This change is transparent for channel monitors that will use the
internal public key of a wrapper.
Payment, DelayedPayment, HTLC and Revocation basepoints and their
derived keys are now wrapped into a specific struct that make it
distinguishable for the Rust type system. Functions that require a
specific key or basepoint should not use generic Public Key, but
require a specific key wrapper struct to engage Rust type
verification system and make it more clear for developers which
key is used.
Matt Corallo [Sun, 26 Nov 2023 19:09:06 +0000 (19:09 +0000)]
Remove now-redundant checks in BOLT12 `Invoice` fallback addresses
Now that we use the `rust-bitcoin` `WitnessProgram` to check our
addresses, we can just rely on it, rather than checking the program
length and version.
Matt Corallo [Sun, 26 Nov 2023 19:07:10 +0000 (19:07 +0000)]
Drop panic if `rust-bitcoin` adds a new `Network`
`rust-bitcoin` 0.30 added `#[non_exhaustive]` to the `Network`
enum, allowing them to "add support" for a new network type without
a major version change in the future. When upgrading, we added a
simple `unreachable` for the general match arm, which would break
in a minor version change of `rust-bitcoin`.
While it seems [possible rust-bitcoin will change
this](https://github.com/rust-bitcoin/rust-bitcoin/issues/2225),
we still shouldn't ba panicking, which we drop here in favor of a
`debug_assert`ion, and a default value.