rust-lightning

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

Tests: take netgraph locks after peer_state

Previously we were taking the NetworkGraph::{channels,nodes} locks before
ChannelManager::per_peer_state's peer_state locks in some tests, which violated
a lock order requirement we have in ChannelManager to take netgraph locks
*after* peer_state locks. See OffersMessageFlow::path_for_release_htlc which is
called while a peer_state lock is held and takes netgraph locks while creating
blinded paths.

Test async send

Add testing for sending payments from an often-offline sender to an
often-offline recipient.

Send held_htlc_available with counterparty reply path

As part of supporting sending payments as an often-offline sender, the sender
needs to send held_htlc_available onion messages such that the reply path to
the message terminates at their always-online channel counterparty that is
holding the HTLC. That way when the recipient responds with release_held_htlc,
the sender's counterparty will receive that message.

After laying groundwork over some past commits, here we as an async sender send
held_htlc_available messages using reply paths created by our always-online
channel counterparty.

Set UpdateAddHTLC::hold_htlc for offline payees

As part of supporting sending payments as an often-offline sender, the sender
needs to be able to set a flag in their update_add_htlc message indicating that
the HTLC should be held until receipt of a release_held_htlc onion message from
the often-offline payment recipient.

The prior commits laid groundwork to finally set the flag here in this commit.

See-also BOLTs PR 989

Support held_htlc_available counterparty reply path

As part of supporting sending payments as an often-offline sender, the sender
needs to send held_htlc_available onion messages such that the reply path to
the message terminates at their always-online channel counterparty that is
holding the HTLC. That way when the recipient responds with release_held_htlc,
the sender's counterparty will receive that message.

Here we lay some groundwork for using a counterparty-created reply path when
sending held_htlc_available as an async sender in the next commit.

Conditionally advertise htlc_hold feature

Now that we support the feature of sending payments as an often-offline sender
to an often-offline recipient, including the sender's LSP-side, we can start
conditionally advertising the feature bit to other nodes on the network.

Also remove an outdated comment, in a previous version of the code we had more
logic to check if we're an always-online node before advertising the feature
bit but it's since been removed.

Add util to gather channels that support htlc_hold

Returns a list of channels where our counterparty supports
InitFeatures::supports_htlc_hold, or an error if there are none. Useful for
sending async payments to StaticInvoices.

Add config flag for held outbound HTLCs

If the flag is set to true, then if we as an often-offline payer receive a
static invoice to pay, we will attempt to hold the corresponding outbound HTLCs
with our next-hop channel counterparty(s) that support the feature. This
allows our node to go offline once the HTLCs are locked in even though the
recipient may not yet be online to receive them.

Immediately archive `ChannelMonitor`s for inbound unfuned channels

If a peer opens a channel to us, but never actually broadcasts the
funding transaction, we'll still keep a `ChannelMonitor` around for
the channel. While we maybe shouldn't do this either, when the
channel ultimately times out 2016 blocks later, we should at least
immediately archive the `ChannelMonitor`, which we do here.

Fixes #3384

Add hold_htlcs param to pay_route_internal

As part of supporting sending payments as an often-offline sender, the sender
needs to be able to set a flag in their update_add_htlc message indicating that
the HTLC should be held until receipt of a release_held_htlc onion message from
the often-offline payment recipient.

We don't yet ever set this flag, but lay the groundwork by including the
parameter in the pay_route method.

See-also <https://github.com/lightning/bolts/pull/989>

Extract send_to_static_invoice util

Makes an upcoming commit cleaner.

In the util we allow manually passing in the first_hops that should be used
when paying a static invoice, which is useful for only passing in next-hop
channels that support the hold_htlcs feature.

Add hold_htlc param to internal Channel APIs

As part of supporting sending payments as an often-offline sender, the sender
needs to be able to set a flag in their update_add_htlc message indicating that
the HTLC should be held until receipt of a release_held_htlc onion message from
the often-offline payment recipient.

We don't yet ever set this flag, but lay the groundwork by including a
parameter for it in internal channel send_htlc APIs, including serializing the
field when the parameter is set.

See-also BOLTs PR 989

Persist hold_htlc flags in Channel

As part of supporting sending payments as an often-offline sender, the sender
needs to be able to set a flag in their update_add_htlc message indicating that
the HTLC should be held until receipt of a release_held_htlc onion message from
the often-offline payment recipient.

We don't yet ever set this flag, but lay the groundwork by including the field
in Channel serialization.

See-also BOLTs PR 989

Add init_features to list_channels filter callback

Useful to filter for channel peers that support a specific feature, in this
case the hold_htlc feature, in upcoming commits.

Actually remove forward<>intercepted htlcs lock dep

8513341b1f7c39da3e6ad4521d4161362b0f4562 purported to remove a lock dependency
between ChannelManager::pending_intercepted_htlcs and ::forward_htlcs. However,
in practice the dependency remained because the locks of the two maps were
still held at the same time in some cases. Here we fix this.

Send `update_add_htlc` messages after HTLC removal messages

While nodes are generally supposed to validate commitment
transactions after the `commitent_signed` and not while HTLCs are
being added/removed, we don't. This can make a commitment update
where we use HTLC balance claimed with a fulfill to send new HTLCs,
which is perfectly valid, being rejected. While we shouldn't
currently generate any such commitments, we might want to in the
future, and on the off-chance that we do, or where such a
commitment would result in a dust threshold overrun, its always
safter to add new HTLCs to a commitment only after we've removed
any HTLCs we're going to remove, which we do here.

Deprecate `pay_for_offer_from_human_readable_name`

While its cool to have a super trivial API to initiate payment to
a `HumanReadableName`, most wallets ultimately support multiple
payment instruction formats (eg on-chain) or multiple HRN
resolution schemes (eg LN-Address). Thus, we generally don't expect
this API to get a lot of use.

Meanwhile, we've now ended up with both a
`ChannelManager::pay_for_offer_from_hrn` and
`ChannelManager::pay_for_ofer_from_human_readable_name`, which is
incredibly confusing.

Here, we deprecate `pay_for_offer_from_human_readable_name`,
pushing folks to the `bitcoin-payment-instructions` crate instead.
Because it has outbound payment states associated with it it may be
some time before we can fully remove all the pieces of it, sadly.

Add struct-level logging to OutboundPayments

Previously, OutboundPayments methods required borrowed logger
arguments since Logger lacked a Clone bound. This prevented
logging in all desired places, limiting trace statements for
debugging. This change initializes OutboundPayments with a
cloned Logger, simplifying method signatures and improving
logging flexibility.