rust-lightning

Migrate `MonitorUpdatingPersister` to an async + async-sync wrapper

As we've done with several other structs, this adds an async
variant of `MonitorUpdatingPersister` and adds an async-sync
wrapper for those using `KVStoreSync`. Unlike with other structs,
we leave `MonitorUpdatingPersister` as the sync variant and make
the new async logic a `MonitorUpdatingPersisterAsync` as the async
monitor updating flow is still considered beta.

This does not yet expose the async monitor updating logic anywhere,
as doing a standard `Persist` async variant would not work for
ensuring the `ChannelManager` and `ChainMonitor` don't block on
async writes or suddenly require a runtime.

Clean up and rustfmt `persist.rs`

Add a test implementation of `FutureSpawner` to track spawned futs

Add async persistence logic in `MonitorUpdatingPersister`

In the next commit we'll add the ability to use an async `KVStore`
as the backing for a `ChainMonitor`. Here we tee this up by adding
an async API to `MonitorUpdatingPersisterAsync`. Its not intended
for public use and is thus only `pub(crate)` but allows spawning
all operations via a generic `FutureSpawner` trait, initiating the
write via the `KVStore` before any `await`s (or async functions).

Because we aren't going to make the `ChannelManager` (or
`ChainMonitor`) fully async, we need a way to alert the
`ChainMonitor` when a persistence completes, but we leave that for
the next commit.

Simplify legacy closed-channel monitor update persistence handling

Pre-0.1, after a channel was closed we generated
`ChannelMonitorUpdate`s with a static `update_id` of `u64::MAX`. In
this case, when using `MonitorUpdatingPersister`, we had to read
the persisted `ChannelMonitor` to figure out what range of monitor
updates to remove from disk. However, now that we have a `list`
method there's no reason to do this anymore, we can just use that.

Simplifying code that we anticipate never hitting anymore is always
a win.

Make `TestStore` async writes actually async, with manual complete

`TestStore` recently got the ability to make async writes, but
wasn't a very useful test as all writes actually completed
immediately. Instead, here, we make the writes actually-async,
forcing the test to mark writes complete as required.

Make `FutureSpawner` only require `Send + Sync` in `std` builds

We only require `Send + Sync` on the `Future`s which are spawned
in std builds, so its weird to require them on the trait itself in
all builds. Instead, make them consistent.

Correct gossip forwarding criteria while doing background sync

If we're doing gossip backfill to a peer, we first forward all our
`channel_announcement`s and `channel_update`s in SCID-order. While
doing so, we don't forward any fresh `channel_update` messages for
any channels which we haven't yet backfilled (as we'll eventually
send the new update anyway, and it might get rejected without the
corresponding `channel_announcement`).

Sadly, our comparison for this was the wrong way, so we actually
*only* forwarded updates which were for channels we haven't yet
backfilled, and dropped updates for channels we already had
backfilled.

Add concrete `NO_*` consts to avoid users having to specify types

When building a background processor without an onion messenger or
liquidity manager, users still need to pass `None` into the
background processor initialization methods. Sadly, Rust will
require a concrete type for the (unused) `Some` variant of the
`Option` passed in, which requires specifying out a lot of type
gook.

Instead, provide some constants which users can pass in trivially.

Fixes #3612

Set HTLC tx version 3 on handling `BumpTransactionEvent::HTLCResolution`

Add forwards-compatibility logic in `LocalHTLCFailureReason` ser

We recently created a `LocalHTLCFailureReason` enum to cover
detailed reasons why an HTLC may have failed, in excess of the
on-the-wire failure encoding.

Sadly, when we did so we introduced a serialization format which
isn't particularly conducive to introducing new failure reasons in
the future. Luckily, `LocalHTLCFailureReason` already has logic
that works for forwards-compatibility - it can be converted to, and
from, the on-the-wire error codes.

Thus, here, we take advantage of that, writing both the on-the-wire
error code as well as a code for the specific case. If we don't
recognize the specific case on deserialization, we'll fall back to
the default case for the on-the-wire code we've written.

We also, of course, include a length-prefixed TLV stream to allow
for additional fields in the future.

Fixes #4087

Document some channel_reestablish spec requirements

Update commitment_signed retransmission for next_funding

The next_funding TLV is included in channel_reestablish when either
tx_signatures or commitment_signed is needed for an interactive-tx
session. This commit largely includes the spec requirements in the
comments, including when to retransmit commitment_signed. But it also
adds an additional check that tx_signatures has not yet been received.

Compute ConstructedTransaction weight from Transaction

Instead of defining a custom function for calculating a transaction's
weight, have ConstructedTransaction hold a Transaction so that its
weight method can be re-used. As a result NegotiatedTxInput no longer
needs to store the transaction inputs.

Remove NegotiatedTxInput::weight

The weight is no longer needed once a ConstructedTransaction is created,
so it doesn't need to be persisted.

Track satisfaction_weight in InteractiveTxInput

Instead of estimating the input weight, track the satisfaction_weight in
InteractiveTxInput. This then can be used compute the transaction weight
without storing individual weights in NegotiatedTxInput in an upcoming
commit.

Use FundingTxInput in InteractiveTxConstructorArgs

InteractiveTxConstructor::new converts inputs given in channel.rs to
InputOwned. Instead of converting FundingTxInput in channel.rs to the
type needed by that constructor, pass in FundingTxInput and have it
converted directly to InputOwned there. This saves one conversion and
Vec allocation. It also lets us use the satisfaction_weight in an
upcoming refactor.

Simplify test_trampoline_unblinded_receive

No need to construct unused blinded hop data or hardcode session privs/prng
seeds.

Compute trampoline session_priv from outer session_priv

This simplifies the code and makes it more straightforward to test unblinded
trampoline receives where we need to compute the trampoline session_priv when
manually creating the inner onion. (The trampoline onion needs to be manually
created because LDK does not natively support sending to unblinded trampolines,
just receiving.)

Debug-Assert that there are no queued messages when a peer connects

In the previous two commits we stopped enqueueing messages for
disconnected peers. Here we add some basic test coverage of these
changes by asserting that there are no queued messages for a peer
when they (re-)connect.

We also add a few assertions that a peer is connected when we push
a message onto the queu, in cases where we expect a peer to be
connected.

Stop enqueueing error messages for disconnected peers

If a channel is failed while a peer is disconnected, we'll still
have a `PeerState` for that peer. Historically, we haven't bothered
to check if a peer is actually connected before we push the `error`
message onto the `PeerState::pending_msg_events` queue, leaving us
sending messages into the void.

This is generally not an issue as
`ChannelManager::get_and_clear_pending_msg_events` should be called
very regularly, removing these messages and then dropping them as
`PeerManager` won't have anything to do with them. Further, when
the the message is an `error`, if a peer happens to connect between
when we push the message and when `get_and_clear_pending_msg_events`
is called the worst that happens is they get the `error` message
we'd end up sending them when they try to reestablish the channel
anyway.

Still, its awkward to leave the `error`s lying around in a message
queue for a disconnected peer, so we remove them here.

Stop enqueueing messages for disconnected peers

If a channel operation occurs while a peer is disconnected, we'll
still have a `PeerState` for that peer. Historically, we haven't
bothered to check if a peer is actually connected before we push
new messages onto the `PeerState::pending_msg_events` field,
leaving us sending messages into the void.

This is generally not an issue as
`ChannelManager::get_and_clear_pending_msg_events` should be called
very regularly, removing these messages and then dropping them as
`PeerManager` won't have anything to do with them.

Still, there is a race condition here - if a peer manages to
connect between the generation of one such message and when
`get_and_clear_pending_msg_events` is called, we may enqueue a
message to the peer which makes no sense and could lead to a
spurious channel closure (especially in the case of an async
`ChannelMonitorUpdate` completion or async signing operation, which
often lead to normal channel message generation).

Further, if a peer is slow to send their `channel_reestablish`
message after connection this race could be substantially more
likely, as such normal channel messages may be nonsense until we've
completed the reestablish dance (i.e. the later reestablish dance
may lead us to re-send the same messages again immediately).

Here we remove most of the cases where we enqueue messages for
disconnected peers.

Note that we differentiate between two different checks for
connected-ness - for cases where we're sending an `error` or gossip
messages, we allow the messages to be enqueued if the peer is
connected at all. For most other cases, we only allow messages to
be enqueued if the peer is connected *and* the channel has
completed its reestablish dance (if required, i.e. the channel is
"connected").

Correct `msgs_sent_since_pong` tracking for gossip forwards

When we forward gossip messages, we use the already-encoded copy
we have, pushing it onto the `gossip_broadcast_buffer`. When we
have free socket buffer space, and there are no non-gossip messages
pending, we'll remove it from the `gossip_broadcast_buffer` and
push it onto the normal `pending_outbound_buffer` socket queue.

Separately, we use `msgs_sent_since_pong` to ensure that our peer
is being responsive and our messages are getting through with
minimal latency. After we send 32 messages (if we've already
received the `pong` for the last `ping` wensent), we send an extra
`ping` and will only queue up an additional 32 forwarded gossip
messages before we start dropping forwarded gossip on the floor.

Previously, we (arguably) incorrectly incremented
`msgs_sent_since_pong` when we pushed a message onto the
`gossip_broadcast_buffer`, not when it was actually queued up in
our `pending_outbound_buffer` immediate socket queue. This is
fairly strange - the point of `msgs_sent_since_pong` is to keep
peer latency low, we already independently enforce memory limits to
ensure we drop forwarded gossip messages if a peer's message queues
have grown too large. This means we potentially disconnect a peer
for not draining the backlog forwarded-gossip socket queue fast
enough, rather than simply dropping gossip.

While this shouldn't be a big deal in practice, I do see (mostly
Tor) peers disconnect on my node occasionally, and while its likely
due to Tor circuits hanging and a disconnect is needed, the
incorrect tracking here makes state analysis difficult as there are
nearly always a flow of enough gossip messages to "send to the
peer". Its also possible that this allows incredibly low bandwidth
connections to stay afloat more durably.

fix: Onion v3 parse/format order and render lowercase

Tor v3 addresses are base32 of 35 bytes laid out as:
[32-byte ed25519 pubkey][2-byte checksum big-endian][1-byte version]

see: https://spec.torproject.org/rend-spec/encoding-onion-addresses.html

* Parsing now slices 0..32 = pubkey, 32..34 = checksum (BE), 34 = version.
* Display now appends pubkey + checksum + version and lowercases base32.
* Adds/updates test vector accordingly.

Fix lint CI job