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.
`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.
We give some more information while reducing the log levels to make the
logging less spammy.
We also convert one safe-to-unwrap case from returning an error to
unwrapping the value.
- If a path within a route passes through the same channelID twice,
that shows the path is looped and will be rejected by nodes.
- Add a check to explicitly reject such payment before trying to send
them.
In other languages (Java and C#, notably), overriding `Eq` without
overriding `Hash` can lead to surprising or broken behavior. Even
in Rust, its usually the case that you actually want both. Here we
add missing `Hash` derivations for P2P messages, to at least
address the first pile of warnings the C# compiler dumps.
Implementation of standard traits on arrays longer than 32 elements
was shipped in rustc 1.47, which is below our MSRV of 1.48 and we
can use to remove some unnecessary manual implementation of
`PartialEq` on `OnionPacket`.
If we remove an HTLC (or fee update), commit, and receive our
counterparty's `revoke_and_ack`, we remove all knowledge of said
HTLC (or fee update). However, the latest local commitment
transaction that we can broadcast still contains the HTLC (or old
fee), thus we are not eligible for initiating the `closing_signed`
negotiation if we're shutting down and are generally expecting a
counterparty `commitment_signed` immediately.
Because we don't have any tracking of these updates in the `Channel`
(only the `ChannelMonitor` is aware of the HTLC being in our latest
local commitment transaction), we'd previously send a
`closing_signed` too early, causing LDK<->LDK channels with an HTLC
pending towards the channel initiator at the time of `shutdown` to
always fail to cooperatively close.
To fix this race, we add an additional unpersisted bool to
`Channel` and use that to gate sending the initial `closing_signed`.
Quite a while ago we added checks for the total current dust
exposure on a channel to explicitly limit dust inflation attacks.
When we did this, we kept the existing upper bound on the channel's
feerate in place. However, these two things are redundant - the
point of the feerate upper bound is to prevent dust inflation, and
it does so in a crude way that can cause spurious force-closures.
Here we simply drop the upper bound entirely, relying on the dust
inflation limit to prevent dust inflation instead.
`lightning-invoice` was historically responsible for actually
paying invoices, handling retries and everything. However, that
turned out to be buggy and hard to maintain, so the payment logic
was eventually moved into `ChannelManager`. However, the old
utilites remain.
Because our payment logic has a number of tunable parameters and
there are different ways to pay a BOLT11 invoice, we ended up with
six different methods to pay or probe a BOLT11 invoice, with more
requested as various options still were not exposed.
Instead, here, we replace all six methods with two simple ones
which return the arguments which need to be passed to
`ChannelManager`. Those arguments can be further tweaked before
passing them on, allowing more flexibility.
This breaks backwards compatibility with versions of LDK prior to
0.0.113 as they expect to always read signer data.
This also substantially reduces allocations during `ChannelManager`
serialization, as we currently don't pre-allocate the `Vec` that
the signer gets written in to. We could alternatively pre-allocate
that `Vec`, but we've been set up to skip the write entirely for a
while, and 0.0.113 was released nearly a year ago. Users
downgrading to LDK 0.0.112 and before at this point should not be
expected.
When we check gossip message signatures, there's no reason to
serialize out the full gossip message before hashing, and it
generates a lot of allocations during the initial startup when we
fetch the full gossip from peers.
Whenever we go to send bytes to a peer, we need to construct a
waker for tokio to call back into if we need to finish sending
later. That waker needs some reference to the peer's read task to
wake it up, hidden behind a single `*const ()`. To do this, we'd
previously simply stored a `Box<tokio::mpsc::Sender>` in that
pointer, which requires a `clone` for each waker construction. This
leads to substantial malloc traffic.
Instead, here, we replace this box with an `Arc`, leaving a single
`tokio::mpsc::Sender` floating around and simply change the
refcounts whenever we construct a new waker, which we can do
without allocations.
When we forward gossip messages, we store them in a separate buffer
before we encrypt them (and commit to the order in which they'll
appear on the wire). Rather than storing that buffer encoded with
no headroom, requiring re-allocating to add the message length and
two MAC blocks, we here add the headroom prior to pushing it into
the gossip buffer, avoiding an allocation.
The data loss protect test was panicking in a message assertion
which should be passing, but because the test was marked only
`#[should_panic]` it was being treated as a successful outcome.
Instead, we use `catch_unwind` on exactly the line we expect to
panic to ensure we are hitting the right one.
