DRY Channel::fail_htlc handling on holding cell free.

Merge pull request #2688 from valentinewallace/2023-10-multihop-blinded-recv

Support receiving to multi-hop blinded paths

Merge pull request #2703 from wpaulino/retryable-commitment-broadcast

Refactor commitment broadcast to always go through OnchainTxHandler

Add missing keysend preimage check on inbound onion read.

Add release note for blinded HTLC serialization.

Add test coverage for serialization of malformed HTLCs.

in Channel and ChannelManager.

Add redundant blinded HTLC failure check for posterity.

Although this new check is unreachable right now, it helps prevent potential
future errors where we incorrectly fail blinded HTLCs with an unblinded error.

Make BlindedPath::new_for_payment pub

Because we now support receiving to multi-hop blinded paths.

Fail blinded received HTLCs if they violate PaymentConstraints

.. contained within their encrypted payload.

Test received blinded HTLC failure in process_pending_htlc_forwards

Test successful intercept payment to 2-hop blinded path

Fix blinded recipient fail on Channel error

If a blinded HTLC errors when added to a Channel, such as if the recipient has
already sent a shutdown message, they should malformed-fail backwards with
error code INVALID_ONION_BLINDING and a zeroed out onion hash per BOLT 4.

Fix blinded recipient fail on receive reqs violation

If a blinded HTLC does not satisfy the receiver's requirements, e.g. bad CLTV
or amount, they should malformed-fail backwards with error code
INVALID_ONION_BLINDING and a zeroed out onion hash per BOLt 4.

Fix blinded recipient fail on onion decode failure

If a recipient behind a multihop blinded path fails to decode their onion
payload, they should fail backwards with error code INVALID_ONION_BLINDING and
a zeroed out onion hash per BOLT 4.

Add find_route test util

And use it in the multihop blinded path receive failure test. Will be used in
the next commit to test receiving an invalid blinded final onion payload.

We can't use the existing get_route test util here because blinded payments
rely on the sender adding a random shadow CLTV offset to the final hop; without
this the payment will be failed with cltv-expiry-too-soon.

Fix blinded recipient fail on malformed HTLC

If a blinded recipient to a multihop blinded path needs to fail back a
malformed HTLC, they should use error code INVALID_ONION_BLINDING and a zeroed
out onion hash per BOLT 4.

Test recipient failing an HTLC received to a multi-hop blinded path

Support failing blinded non-intro HTLCs after RAA processing.

If an HTLC fails after its RAA is processed, it is failed back with
ChannelManager::fail_htlc_backwards_internal. This method will now correctly
inform the channel that this HTLC is blinded and to construct an
update_malformed message accordingly.

Tweak initialization of HTLCForwardInfo in fail_htlc_backwards_internal

Makes the next commit adding support for failing blinded HTLCs in said method
easier to read.

ChannelManager: add HTLCForwardInfo variant for blinded non-intro htlcs

Necessary to tell the Channel how to fail these htlcs.

Adapt Channel::fail_htlc for failing with malformed OR update_fail_htlc.

Useful for failing blinded payments back with malformed, and will also be
useful in the future when we move onion decoding into
process_pending_htlc_forwards, after which Channel::fail_htlc will be used for
all malformed htlcs.

Set up Channel::fail_htlc to be able to return update_malformed

Currently it returns only update_fail, but we'll want it to be able to return
update_malformed as well in upcoming commits. We'll use this for correctly
failing blinded received HTLCs backwards with malformed and
invalid_onion_blinding error per BOLT 4.

Channel: add holding cell HTLC variant for blinded HTLCs.

For context, blinded HTLCs where we are not the intro node must always be
failed back with malformed and invalid_onion_blinding error per BOLT 4.

Prior to supporting blinded payments, the only way for an update_malformed to
be returned from Channel was if an onion was actually found to be malformed
during initial update_add processing. This meant that any malformed HTLCs would
never live in the holding cell but instead would be returned directly upon
initial RAA processing.

Now, we need to be able to store these HTLCs in the holding cell because the
HTLC failure necessitating an update_malformed may come long after the RAA is
initially processed, and we may not be a state to send the update_malformed
message at that time.

Therefore, add a new holding cell HTLC variant for blinded non-intro node
HTLCs, which will signal to Channel to fail with malformed and the correct
error code.

Set HTLCPreviousHopData::blinded for blinded received HTLCs.

Will be used in the next commit(s) to let us know to fail blinded received
HTLCs backwards with the malformed and invalid_onion_blinding error per BOLT 4.

Store whether a received HTLC is blinded in PendingHTLCInfo

Useful for knowing to fail back these HTLCs with malformed and
invalid_onion_blinding error per the BOLT 4 spec.

Merge pull request #2787 from jkczyz/2023-12-onion-messenger-assert

Relax `OnionMessenger::peer_disconnected` assertion

Relax OnionMessenger::peer_disconnected assertion

When a peer is connected, OnionMessenger tracks it only if it supports
onion messages. On disconnect, we debug_assert that the peer was in a
state ConnectedPeer, failing when it is in the PendingConnection state.
However, we were mistakenly asserting for peers that we were not
tracking (i.e., that don't support onion messages). Relax the check to
not fail on the latter.

Add test coverage for holder commitment rebroadcast after reorg

Refactor commitment broadcast to always go through OnchainTxHandler

Currently, our holder commitment broadcast only goes through the
`OnchainTxHandler` for anchor outputs channels because we can actually
bump the commitment transaction fees with it. For non-anchor outputs
channels, we would just broadcast once directly via the
`ChannelForceClosed` monitor update, without going through the
`OnchainTxHandler`.

As we add support for async signing, we need to be tolerable to signing
failures. A signing failure of our holder commitment will currently
panic, but once the panic is removed, we must be able to retry signing
once the signer is available. We can easily achieve this via the
existing `OnchainTxHandler::rebroadcast_pending_claims`, but this
requires that we first queue our holder commitment as a claim. This
commit ensures we do so everywhere we need to broadcast a holder
commitment transaction, regardless of the channel type.

Co-authored-by: Rachel Malonson <rachel@lightspark.com>

Cancel previous commitment claims on newly confirmed commitment

Once a commitment transaction is broadcast/confirms, we may need to
claim some of the HTLCs in it. These claims are sent as requests to the
`OnchainTxHandler`, which will bump their feerate as they remain
unconfirmed. When said commitment transaction becomes unconfirmed
though, and another commitment confirms instead, i.e., a reorg happens,
the `OnchainTxHandler` doesn't have any insight into whether these
claims are still valid or not, so it continues attempting to claim the
HTLCs from the previous commitment (now unconfirmed) forever, along with
the HTLCs from the newly confirmed commitment.

Merge pull request #2775 from benthecarman/sign-psbt

Change WalletSource::sign_tx to sign_psbt

Change WalletSource::sign_tx to sign_psbt

Merge pull request #2637 from Sharmalm/2348

Improve block connection logging and filtered txids

Merge pull request #2786 from tnull/2023-12-fix-warnings

Cleanup some warnings

Merge pull request #2782 from TheBlueMatt/2023-12-check-cfg-tags

Add CI test that `#[cfg]` tags are from a defined set

Remove unused `NodeId` in BP tests

Markup packages link as hyperlink

Remove unused `chan_id`

Fix leftover `rustdoc` warnings

.. as these slipped in again.

Merge pull request #2681 from tnull/2023-10-bump-msrv-to-1.63.0

Bump MSRV to rustc 1.63.0

Improving block conenction logging and filtered txids

Implement the Display trait for Outpoint and utilize it in the codebase for monitoring outpoints.
Additionally, add log tracing for best_block_update and confirmed transactions.
solves #2348

Merge pull request #2752 from valentinewallace/2023-11-large-final-onion-payload-fixes

Large final onion payload fixes

Add CI test that `#[cfg]` tags are from a defined set

Rust is fairly relaxed in checking the validity of arguments
passed to #[cfg]. While it should probably be more strict when
checking features, it cannot be strict when checking loose cfg
tags, because those can be anything and are simply passed to rustc
via unconstrained arguments.

Thus, we do it for rustc manually, but scanning all our source and
checking that all our cfg tags match a known cfg tag.

Fixes #2184

Merge pull request #2774 from TheBlueMatt/2023-12-2551-followups

Doc and performance followups to #2551

Error if onion payloads exceed max length on packet construction.

Ensure that if we call construct_onion_packet and friends where payloads are
too large for the allotted packet length, we'll fail to construct. Previously,
senders would happily construct invalid packets by array-shifting the final
node's HMAC out of the packet when adding an intermediate onion layer, causing
the receiver to error with "final payload provided for us as an intermediate
node."

Fix debug panic in onion utils on large custom TLVs or metadata.

We previously assumed that the final node's payload would be ~93 bytes, and had
code to ensure that the filler encoded after that payload is not all 0s. Now
with custom TLVs and metadata supported, the final node's payload may take up
the entire onion packet, so we can't assume that there are 64 bytes of filler
to check.

Pre-calculate heap element scores (retaining RouteGraphNode size)

`RouteGraphNode` currently recalculates scores in its `Ord`
implementation, wasting time while sorting the main Dijkstra's
heap.

Further, some time ago, when implementing the `htlc_maximum_msat`
amount reduction while walking the graph, we added
`PathBuildingHop::was_processed`, looking up the source node in
`dist` each time we pop'ed an element off of the binary heap.
As a result, we now have a reference to our `PathBuildingHop` when
processing a best-node's channels, leading to several fields in
`RouteGraphNode` being entirely redundant.

Here we drop those fields, but add a pre-calculated score field,
as well as force a suboptimal `RouteGraphNode` layout, retaining
its existing 64 byte size.

Without the suboptimal layout, performance is very mixed, but with
it performance is mostly improved, by around 10% in most tests.

Reorder `PathBuildingHop` fields somewhat

Given `PathBuildingHop` is now an even multiple of cache lines, we
can pick which fields "fall off" the cache line we have visible
when dealing with hops, which we do here.

Make `find_route`'s `dist` map elements fit in 128 bytes

We'd previously aggressively cached elements in the
`PathBuildingHop` struct (and its sub-structs), which resulted in a
rather bloated size. This implied cache misses as we read from and
write to multiple cache lines during processing of a single
channel.

Here, we reduce caching in `DirectedChannelInfo`, fitting the
`(NodeId, PathBuildingHop)` tuple in exactly 128 bytes. While this
should fit in a single cache line, it sadly does not generally lie
in only two lines, as glibc returns large buffers from `malloc`
which are very well aligned, plus 16 bytes (for its own allocation
tracking). Thus, we try to avoid reading from the last 16 bytes of
a `PathBuildingHop`, but luckily that isn't super hard.

Note that here we make accessing
`DirectedChannelInfo::effective_capacity` somewhat slower, but
that's okay as its only ever done once per `DirectedChannelInfo`
anyway.

While our routing benchmarks are quite noisy, this appears to
result in between a 5% and 15% performance improvement in the
probabilistic scoring benchmarks.

Make `CandidateRouteHop::PrivateHop::target_node_id` a reference

This avoids bloating `CandidateRouteHop` with a full 33-byte
node_id (and avoids repeated public key serialization when we do
multiple pathfinding passes).

Simplify and make scoring calls in `TestRouter` more complete

`TestRouter` tries to make scoring calls that mimic what an actual
router would do, but the changes in f0ecc3ec73dcdb9303b1bd5ac687a36
failed to make scoring calls for private hints or if we take a
public hop for the last hop.

This fixes those regressions, though no tests currently depend on
this behavior.

Make `CandidateRouteHop` method docs somewhat more descriptive

Fix indentation in `router.rs` broken in a1d15ac1926f70aa5ab4f6686f

Rename `CandidateRouteHop::FirstHop::node_id` and make it a ref

Rather than calling `CandidateRouteHop::FirstHop::node_id` just
`node_id`, we should call it `payer_node_id` to provide more
context.

We also take this opportunity to make it a reference, avoiding
bloating `CandidateRouteHop`.

`#[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.

Fix new unused warnings in `scoring.rs`

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.

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.

Rename `DirectedChannelInfo::outbound` to `from_node_one`

...to give a bit more readability on accessing sites.

Rewrite docs in `CandidateRouteHop` to be somewhat more descriptive

Add variant for non-intro-nodes to BlindedFailure enum

For use in supporting receiving to multi-hop blinded paths.

Test successfully receiving to a multihop blinded path.

Support receiving to multi-hop blinded payment paths.

The only remaining step is to use the update_add blinding point in decoding
inbound onion payloads.

Error handling will be completed in upcoming commits.

Support parsing blinded non-intro onion receive payloads.

Support for receiving to multi-hop blinded payment paths will be completed in
the next commit, sans error handling.

Pass in update add blinding point on onion decode

Will be used to read encrypted_tlvs on non-intro-node onion receipt.

Merge pull request #2760 from TheBlueMatt/2023-11-chan-close-loop

Fix infinite loop when closing a pre-funding channel

Merge pull request #2776 from jkczyz/2023-12-direct-connect-follow-ups

Folllow-ups to #2723

Manually download `bitcoind`/`electrs` for CI tests

Previously, we used the auto-download feature of the
`electrsd`/`bitcoind` crates. While convenient, they unnecessarily
introduced a lot of dependecies (`zip`, `zstd`, `time`, etc.) to our
test environment which needed pinning for the MSRV of 1.63.

Here, we introduce a new `no_download` config flag to the
`lightning-transaction-sync` crate allowing us to disable this
auto-download feature in CI, where we now opt to download the
corresponding binaries manually. We keep the default-auto-download as a
convenience feature for running tests locally though.

Move `lightning-custom-message` to main workspace

Since we now have a consistent MSRV and edition, we can move
`lightning-custom-message` to the main
workspace.

Bump MSRV to rustc 1.63.0 and edition to 2021

.. which is a reasonable common ground, also supported by Debian stable.

Return correct SendSuccess in OnionMessenger

When enqueuing a message for a node already awaiting a connection,
BufferedAwaitingConnection should be returned when a node is not yet
connected as a peer. However, it was only returned when the first
message was enqueued. Any messages enqueued after but before a
connection was established incorrectly returned Buffered.

Rename OnionMessagePath::addresses

The name itself doesn't provide much meaning to what the addresses
correspond to.

Fix create_onion_message return type documentation

Immediately error in `close_channel_internal` if there is no chan

Previously, unfunded channels would be stored outside of
`PeerState::channel_by_id`, and thus if there is no channel when
we look in `PeerState::channel_by_id`, `close_channel_internal`
called `force_close_channel_with_peer` to hunt for unfunded
channels.

However, that is no longer the case, so the call is redundant, and
we can simply return an error instead.

Move pre-funded-channel immediate shutdown logic to the right place

Because a `Funded` `Channel` cannot possibly be pre-funding, the
logic in `ChannelManager::close_channel_internal` to handle
pre-funding channels is in the wrong place.

Rather than being handled inside the `Funded` branch, it should be
in an `else` following it, handling either of the two
`ChannelPhases` outside of `Funded`.

Sadly, because of a previous control flow management `loop {}`, the
existing code will infinite loop, which is fixed here.

Drop unreachable shutdown code in `Channel::get_shutdown`

`Channel` is only a thing for funded channels. Thus, checking if a
channel has not yet been funded is dead code and can simply be
elided.

Limit the scope of `get_funding_created_msg` to outbound channels

Since we no longer use `ChannelContext::get_funding_created_msg`,
it can be moved back into `UnfundedOutboundV1` channels only,
where it realistically belongs.

Move to `Funded` after `funding_signed` rather than on funding

Previously, channels were stored in different maps in `PeerState`
based on whether the funding had been set, keeping the keys across
the maps consistent (pre-funding temporary_channel_ids vs
funding-outpoint-based channel_ids). However, channels are now
stored in a single `channel_by_id` map, making that point moot.

Instead, here, we convert the `ChannelPhase` state transition
boundary to "once we have a `ChannelMonitor`", which makes more
sense now, and was actually the original proposed boundary.

This also requires calling `signer_maybe_unblocked` on a pre-funded
outbound channel, but that nicely also lets us limit the scope of
`FundingCreated` message generation, which we do in the next
commit.

Merge pull request #2723 from jkczyz/2023-11-direct-connect

Direct connect for `OnionMessage` sending

Merge pull request #2691 from wpaulino/refactor-channel-state

Refactor ChannelState to decouple state flags from states

Rename certain flags to align with dual funding

`FundingCreated` and `FundingSent` were mostly named after the
respective `funding_created` and `funding_sent` wire messages. They
include the signature for the initial commitment transaction when
opening a channel. With dual funding, these messages are no longer used,
and instead we rely on the existing `commitment_signed` to exchange
those signatures.

Rename OnionMessageBuffer to OnionMessageRecipient

Remove superfluous space from where clause

Test pending connection onion message buffering

Add tests for onion message buffering checking that messages are cleared
upon disconnection and timed out after MAX_TIMER_TICKS. Also, checks
that ConnectionNeeded events are generated.

Reuse MessengerNode in spec_test_vector

Additional tests will be added needing a similar node struct, so
consolidate its usage.

Call OnionMessageHandler::timer_tick_occurred

lightning-background-processor processes events provided by the
PeerManager's OnionMessageHandler for when a connection is needed. If a
connection is not established in a reasonable amount of time, drop any
buffered onion messages by calling timer_tick_occurred.

Process OnionMessageHandler events in background

OnionMessageHandler implementations now also implement EventsProvider.
Update lightning-background-processor to also process any events the
PeerManager's OnionMessageHandler provides.

Re-order define_run_body macro parameters

Simply to avoid excessive wrapping when possible.

Re-wrap define_run_body macro parameters

Some code hygiene before another parameter is added and rustfmt is
eventually used.

Remove unnecessary BackgroundProcessor type param

Merge pull request #2765 from TheBlueMatt/2023-12-2314-cleanups-1

Post-#2314 Cleanups

Drop buffered messages for timed out nodes

OnionMessenger buffers onion messages for nodes that are pending a
connection. To prevent DoS concerns, add a timer_tick_occurred method to
OnionMessageHandler so that buffered messages can be dropped. This will
be called in lightning-background-processor every 10 seconds.

Make OnionMessageHandler extend EventsProvider

An OnionMessageHandler may buffer messages that can't be sent because
the recipient is not a peer. Have the trait extend EventsProvider so
that implementation so that an Event::ConnectionNeeded can be generated
for any nodes that fall into this category. Also, implement
EventsProvider for OnionMessenger and IgnoringMessageHandler.

Add Event::ConnectionNeeded for onion messages

A MessageRouter may be unable to find a complete path to an onion
message's destination. This could because no such path exists or any
needs on a potential path don't support onion messages. Add an event
that indicates a connection with a node is needed in order to send the
message.

Return socket addresses from DefaultMessageRouter

When there isn't a direct connection with the Destination of an
OnionMessage, look up socket addresses from the NetworkGraph. This is
used to signal to OnionMessenger that a direct connection is needed to
send the message.

Add Option<Vec<SocketAddress>> to OnionMessagePath

MessageRouter::find_path is given a Destination to reach via a set of
peers. If a path cannot be found, it may return a partial path such that
OnionMessenger can signal a direct connection to the first node in the
path is needed. Include a list of socket addresses in the returned
OnionMessagePath to allow OnionMessenger to know how to connect to the
node.

This allows DefaultMessageRouter to use its NetworkGraph to return
socket addresses for gossiped nodes.

Add NetworkGraph reference to DefaultMessageRouter

When buffering onion messages for a node that is not connected as a
peer, it's possible that the node does not exist. Include a NetworkGraph
reference in DefaultMessageRouter so that it can be used to check if the
node actually exists. Otherwise, an malicious node may send an onion
message where the reply path's introduction node doesn't exist. This
would result in buffering messages that may never be delivered.

Buffer onion messages requiring a connection

MessageRouter::find_path returns a path to use when sending an onion
message. If the first node on the path is not connected or does not
support onion messages, sending will fail with InvalidFirstHop. Instead
of failing outright, buffer the message for later sending once the first
node is a connected peer.

Destination in OnionMessenger::send_onion_message

OnionMessenger::send_onion_message takes an OnionMessagePath. This isn't
very useful as it requires finding a path manually. Instead, have the
method take a Destination and use OnionMessenger's MessageRouter to
construct the path. Later, this will allow for buffering messages where
the first node in the path isn't a direct connection.

Merge pull request #2762 from TheBlueMatt/2023-11-htlc-docs

Improve docs on newly-public structs after  #2700