rust-lightning

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

Test async send

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

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.

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.

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.

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

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.

Speed up `remove_stale_channels_and_tracking` nontrivially

During startup, the lightning protocol forces us to fetch a ton of
gossip for channels where there is a `channel_update` in only one
direction. We then have to wait around a while until we can prune
the crap cause we don't know when the gossip sync has completed.

Sadly, doing a large prune via `remove_stale_channels_and_tracking`
is somewhat slow. Removing a large portion of our graph currently
takes a bit more than 7.5 seconds on an i9-14900K, which can
ultimately ~hang a node with a few less GHz ~forever.

The bulk of this time is in our `IndexedMap` removals, where we
walk the entire `keys` `Vec` to remove the entry, then shift it
down after removing.

In the previous commit we shifted to a bulk removal model for
channels, here we do the same for nodes. This reduces the same test
to around 340 milliseconds on the same hardware.

Speed up `remove_stale_channels_and_tracking` nontrivially

During startup, the lightning protocol forces us to fetch a ton of
gossip for channels where there is a `channel_update` in only one
direction. We then have to wait around a while until we can prune
the crap cause we don't know when the gossip sync has completed.

Sadly, doing a large prune via `remove_stale_channels_and_tracking`
is somewhat slow. Removing a large portion of our graph currently
takes a bit more than 7.5 seconds on an i9-14900K, which can
ultimately ~hang a node with a few less GHz ~forever.

The bulk of this time is in our `IndexedMap` removals, where we
walk the entire `keys` `Vec` to remove the entry, then shift it
down after removing.

Here we shift to a bulk removal model when removing channels, doing
a single `Vec` iterate + shift. This reduces the same test to
around 1.38 seconds on the same hardware.

Don't auto-fail offers payments pre-HTLC lock in

Previously, we had a bug that particularly affected async payments where if an
outbound payment was in the state {Static}InvoiceReceived and there was a call
to process_pending_htlc_forwards, the payment would be automatically abandoned.
We would behave correctly and avoid abandoning if the payment was awaiting an
invoice, but not if the payment had an invoice but the HTLCs weren't yet locked
in.

ln: remove test flag from `negotiate_anchor_zero_fee_commitments`