Merge pull request #3310 from TheBlueMatt/2024-09-unlocked-checksig

Validate `channel_update` signatures without holding a graph lock

Merge pull request #3326 from jkczyz/2024-09-split-router

Decouple `MessageRouter` from `Router`

Validate `channel_update` signatures without holding a graph lock

We often process many gossip messages in parallel across different
peer connections, making the `NetworkGraph` mutexes fairly
contention-sensitive (not to mention the potential that we want to
send a payment and need to find a path to do so).

Because we need to look up a node's public key to validate a
signature on `channel_update` messages, we always need to take a
`NetworkGraph::channels` lock before we can validate the message.

For simplicity, and to avoid taking a lock twice, we'd always
validated the `channel_update` signature while holding the same
lock, but here we address the contention issues by doing a
`channel_update` validation in three stages.

First we take a read lock on `NetworkGraph::channels` and check if
the `channel_update` is new, then release the lock and validate the
message signature, and finally take a write lock, (re-check if the
`channel_update` is new) and update the graph.

De-couple MessageRouter from Router

Now that ChannelManager is parameterized by both a MessageRouter and a
Router, Router implementations no longer need to implement
MessageRouter, too.

Expand DefaultRouter's privacy docs

DefaultRouter::create_blinded_payment_paths may creat a one-hop blinded
path with the recipient as the introduction node. Update the privacy
section of DefaultRouter's docs to indicate this as is done in the docs
for DefaultMessageRouter.

Parameterize ChannelManager with MessageRouter

ChannelManager is parameterized by a Router, which must also implement
MessageRouter. Instead, add a MessageRouter parameter such that the
Router and MessageRouter traits can be de-coupled. This simplifies using
something other than DefaultMessageRouter, which DefaultRouter currently
delegates to.

Merge pull request #3327 from TheBlueMatt/2024-09-local-ci-rustfmt

Only attempt to `rustfmt` files checked into git

Only attempt to `rustfmt` files checked into git

This avoids `rustfmt` failing on Rust files generated by dependent
crates in `target`, eg

```
+ rustfmt --edition 2021 --check ./target/debug/build/thiserror-8230374e07b5c05a/out/probe.rs
Diff in /home/matt/rust-lightning-3/target/debug/build/thiserror-8230374e07b5c05a/out/probe.rs at line 1:

-    #![feature(provide_any)]
+#![feature(provide_any)]

-    use std::any::{Demand, Provider};
+use std::any::{Demand, Provider};

-    fn _f<'a, P: Provider>(p: &'a P, demand: &mut Demand<'a>) {
-        p.provide(demand);
-    }
+fn _f<'a, P: Provider>(p: &'a P, demand: &mut Demand<'a>) {
+ p.provide(demand);
+}
```

Re-order imports

Merge pull request #3289 from tnull/2024-09-fix-rgs-bp-builds

Fix builds of `lightning-rapid-gossip-sync` and `lightning-background-processor` crates

Drop `no-std` feature

We drop the `lightning/no-std` feature and just take
`hashbrown`,`possiblyrandom` and `libm` as required dependencies.

Check workspace members with default features individually in CI

Previously, we would only check the workspace as a whole. This however
would mean that we would check/test crates with `lightning`'s default
features enabled, allowing failures-to-build under the crates own
default features to slip through, if they didn't explicitly enable
`lightning/std`, for example.

Here, we extend the CI to check the workspace as a whole but then run
checks, tests, and doc generation on the workspace members individually,
asserting that all of them build even when not built as part of the same
workspace as `lightning`.

Re-add optional `std` feature dependencies to fix RGS and BP builds

We previously removed these, leading to failure to build under default
features including `std`.

Merge pull request #3321 from valentinewallace/2024-09-fix-main

Fix silent rebase conflict that broke tests

Fix silent rebase conflict that broke tests

429cbe1a060a5ee7abb421c65d2cf5e503909215 merged a PR that renamed
Offer::signing_pubkey to Offer::issuer_signing_pubkey. However, there was a
silent rebase conflict and a test added as part of
1059f5ffc55d3c21bf4d3ae82fe185f588a53116 did not get the memo and used the old
method name, breaking the test build.

Merge pull request #3319 from tnull/2024-09-rustfmt-sync

`rustfmt`: Run on `lightning/src/sync/*`

Merge pull request #3313 from valentinewallace/2024-09-fix-offer-double-pay

Don't pay a duplicate BOLT 12 invoice if `ChannelManager` is stale

Merge pull request #3218 from jkczyz/2024-07-offers-spec-update

Rename `Offer::signing_pubkey` to `Offer::issuer_id`

Fix bug where we double-pay an offer due to stale manager

This fixes the following bug:
- An outbound payment is AwaitingInvoice
- We receive an invoice and lock the HTLCs into the relevant ChannelMonitors
- The monitors are successfully persisted, but the ChannelManager fails to
  persist, so the outbound payment remains AwaitingInvoice
- We restart, causing the channels to close due to a stale ChannelManager
- We receive a duplicate invoice, and attempt to pay it again due to the
  payment still being AwaitingInvoice in the stale ChannelManager

After the fix for this, we will notice that the payment is already locked into
the monitor on startup and transition the incorrectly-AwaitingInvoice payment
to Retryable, which prevents double-paying on duplicate invoice receipt.

`rustfmt`: Drop `sync/mod.rs` from exclusion list

`rustfmt`: Run on `sync/mod.rs`

`rustfmt`: Drop `sync/test_lockorder_checks.rs` from exclusion list

`rustfmt`: Run on `sync/test_lockorder_checks.rs`

`rustfmt`: Drop `sync/nostd_sync.rs` from exclusion list

`rustfmt`: Run on `sync/nostd_sync.rs`

`rustfmt`: Drop `sync/fairrwlock.rs` from exclusion list

`rustfmt`: Run on `sync/fairrwlock.rs`

`rustfmt`: Drop `sync/debug_sync.rs` from exclusion list

`rustfmt`: Run on `sync/debug_sync.rs`

Rename Refund::payer_id

For consistency with Offer::issuer_signing_pubkey, rename
Refund::payer_id to use "signing_pubkey" instead of "id".

Rename InvoiceRequest::payer_id

For consistency with Offer::issuer_signing_pubkey, rename
InvoiceRequest::payer_id to use "signing_pubkey" instead of "id".

Update offer test vectors from latest spec changes

Use unsigned invoice types in docs

Add issuer_signing_pubkey to Bolt12Invoice

Useful for determining if the signing_pubkey is the
issuer_signing_pubkey or is from a blinded path.

Re-write Offer::issuer_signing_pubkey documentation

Rename Offer::signing_pubkey to Offer::issuer_signing_pubkey

The spec was recently changed to use offer_issuer_id instead of
offer_node_id. LDK always used signing_pubkey to avoid confusion with a
node_id. Rename it to issuer_signing_pubkey now as InvoiceRequest and
Bolt12Invoice will have similarly named methods in upcoming commits.

Move monitor<>outbound_payments startup htlc syncing code.

Move the code that ensures that HTLCs locked into ChannelMonitors are
synchronized with the ChannelManager's OutboundPayments store to the
outbound_payments module.

This is useful both because ChannelManager::read is very long/confusing method,
so it's nice to encapsulate some of its functionality, and because we need to
fix an existing bug in this logic where we may risk double-paying an offer due
to outbound_payments being stale on startup. See the next commit for this
bugfix.

Test utils: use full import path in reload_node macro.

Merge pull request #3315 from tnull/2024-09-rustfmt-crypto

`rustfmt`: Run on `crypto` module

`rustfmt`: Drop `crypto/mod.rs` from exclusion list

`rustfmt`: Run on `crypto/mod.rs`

`rustfmt`: Drop `crypto/utils.rs` from exclusion list

`rustfmt`: Run on `crypto/utils.rs`

`rustfmt`: Drop `crypto/streams.rs` from exclusion list

`rustfmt`: Run on `crypto/streams.rs`

`rustfmt`: Drop `crypto/poly1305.rs` from exclusion list

`rustfmt`: Run on `crypto/poly1305.rs`

`rustfmt`: Drop `crypto/chacha20poly1305.rs` from exclusion list

`rustfmt`: Run on `crypto/chacha20poly1305rfc.rs`

`rustfmt`: Drop `crypto/chacha20.rs` from exclusion list

`rustfmt`: Run on `crypto/chacha20.rs`

Merge pull request #3314 from G8XSU/event-logging

Add logging around event processing.

Merge pull request #3140 from valentinewallace/2024-06-pay-static-invoice

Support paying static invoices

Add logging around event processing.

Currently we don't have any visibility if BackgroundProcessor
takes considerably more of time to process events, adding logs
to help debug such issues.

Remove payment_release_secret from async payments messages.

This field isn't necessary because we already authenticate the messages via the
blinded reply paths payment_id, nonce and HMAC.

Document PendingOutboundPayment::{Static}InvoiceReceived semantics.

While these variants may sound similar, they are very different. One is so
temporary it's never even persisted to disk, the other is a state we will stay
in for hours or days. See added docs for more info.

Add new Bolt12PaymentError for failed blinded path creation.

Currently used when initiating an async payment via held_htlc_available OM. This
OM needs a reply path back to us, so use this error for our invoice_error OM if
we fail to create said reply path.

Verify inbound ReleaseHeldHtlc messages via hmac.

See AsyncPaymentsContext::hmac, but this prevents the recipient from
deanonymizing us. Without this, if they are able to guess the correct payment
id, then they could create a blinded path to us and confirm our identity.

We also move the PAYMENT_HASH_HMAC_INPUT const to use &[7; 16], which is safe
because this const was added since the last release. This ordering reads more
smoothly.

Rename Payment{Hash,Id} hmac creation/verification methods for offers.

We want to specify that these methods are only to be used in an outbound offers
payment context, because we'll be adding similar methods for the outbound async
payments context in upcoming commits.

Don't trigger manager persistence on unexpected release_htlc message.

If someone sends us an unexpected or duplicate release_held_htlc onion message,
we should simply ignore it and not persist the entire ChannelManager in
response.

Correct docs on payment id in RecentPaymentDetails.

Support abandoning pending outbound async payments.

Async payments may have very high expires because we may be waiting for days
for the recipient to come online, so it's important that users be able to
abandon these payments early if needed.

Timeout expired outbound async payments.

Support sending async payments as an always-online sender.

Async receive is not yet supported.

Here we process inbound release_htlc onion messages, check that they actually
correspond to one of our outbound payments, and actually forward the HTLCs.
Valid release_htlc receipt indicates that the recipient has now come online to
receive.

Split off send_payment_for_bolt12_invoice_internal util.

This new util will be able to send to both static and non-static BOLT 12
invoices.

Set max path len on receipt of static invoice.

Because we may receive a static invoice to pay days before the recipient
actually comes back online to receive the payment, it's good to do as many
checks as we can up-front. Here we ensure that the blinded paths provided
in the invoice won't cause us to exceed the maximum onion packet size.

Error on static invoice with unknown required features.

DRY handling when initiating payment to BOLT 12 invoice.

Support initiating an async payment to a static invoice.

Supported when the sender is an always-online node. Here we send the initial
held_htlc_available onion message upon receipt of a static invoice, next we'll
need to actually send HTLCs upon getting a response to said OM.

Release pending async payments to PeerManager.

Pass full message context into ChanMan blinded path util.

Allows us to repurpose this util to create blinded paths in an async payments
message context while maintaining support for the offers context.

Store async payment data in PendingOutboundPayment.

Adds a pending outbound payment variant for async payments, which indicates
that we have received a static invoice to pay and have generated a keysend preimage
for the eventual payment. When the recipient comes back online, we'll
transition from this new state to Retryable and actually forward the HTLCs.

Support creating PaymentParameters from static invoices.

Support checking that a static invoice matches an outbound invreq.

Useful for ensuring that an inbound static invoice matches one of our outbound
invreqs, otherwise it is an unexpected invoice and should be ignored and not
paid.

Merge pull request #3243 from dunxen/2024-08-reremove-balancemsat

Remove AvailableBalances::balance_msat

Merge pull request #3302 from TheBlueMatt/2024-09-atomic-cleanups

Simplify and fix AtomicCounter

Store AsyncPaymentsMessages for later sending

Upcoming commits will support sending and receiving held_htlc_available and
release_held_htlc messages. These messages need to be enqueued so that they can
be released in ChannelManager's implementation of AsyncPaymentsMessageHandler
to OnionMessenger for sending.

Pass context into held_htlc_available message handling.

Useful for using the payment_id within to look up the corresponding outbound
async payment so we know we can safely release the HTLCs to the now-onlinen
recipient.

Add MessageContext for async payments.

This context will be used in reply paths for outbound held_htlc_available
messages, so we can authenticate the corresponding release_held_htlc messages.

Tweak debug_assert message for parsed onion messages.

More specific.

Merge pull request #3010 from shaavan/issue2836

Introduce Retry InvoiceRequest Flow

Rename `AtomicCounter::get_increment` to simply `next`

Its a counter, `next` is super clear, `get_increment` is a bit
less so.

Simplify and fix `AtomicCounter`

`AtomicCounter` was slightly race-y on 32-bit platforms because it
increments the high `AtomicUsize` independently from the low
`AtomicUsize`, leading to a potential race where another thread
could observe the low increment but not the high increment and see
a value of 0 twice.

This isn't a big deal because (a) most platforms are 64-bit these
days, (b) 32-bit platforms aren't super likely to have their
counter overflow 32 bits anyway, and (c) the two writes are
back-to-back so having another thread read during that window is
very unlikely.

However, we can also optimize the counter somewhat by using the
`target_has_atomic = "64"` cfg flag, which we do here, allowing us
to use `AtomicU64` even on 32-bit platforms where 64-bit atomics
are available.

This changes some test behavior slightly, which requires
adaptation.

Fixes #3000

Introduce handle_message_received test

- Add a test to verify the functionality of the handle_message_received
  function.
- Ensure the test covers scenarios where InvoiceRequest messages are retried
  for PendingOutboundPayments after a simulated connection loss.

Introduce message_received in ChannelMessageHandler

- Introduce the `message_received` function to manage the
  behavior when a message is received from any peer.
- This function is used within `ChannelManager` to retry `InvoiceRequest`
  messages if we haven't received the corresponding invoice yet.
- This change makes the offer communication robust against sudden
  connection drops where the initial attempt to send the message
  might have failed.

Introduce enqueue_invoice_request Function

1. Separate the logic of forming `invoice_request` messages from
   `invoice_request` and `reply_paths` and enqueueing them into a
   separate function.
2. This logic will be reused in the following commit when reforming
   `invoice_request` messages for retrying.

Introduce RetryableInvoiceRequest in AwaitingInvoice

1. To enable the retry of the Invoice Request message, it's necessary
   to store the essential data required to recreate the message.
2. A new struct is introduced to manage this data, ensuring the
   InvoiceRequest message can be reliably recreated for retries.
3. The addition of an `awaiting_invoice` flag allows tracking of
   retryable invoice requests, preventing the need to lock the
   `pending_outbound_payment` mutex.

Refactor `channel_update` processing logic into local fns

In the next commit we'll move to checking `channel_update`s in
three steps - first we check if the `channel_update` is new and the
latest for a channel, then we check the signature, and finally we
update our local state. This allows us to avoid holding a lock on
`NetworkGraph::channels` while validating the message signature.

Here we do a quick prefactor to make that simpler - moving the
validation logic of `channel_update` that we'll do in step one (and
repeat in step three) into a local function. We also take this
opportunity to do one static check unlocked which we had been doing
while holding a `channel` lock.

Merge pull request #3291 from tnull/2024-09-add-their-node-id-routing-msg-handler

Add node id to remaining `RoutingMessageHandler::handle_` methods

Merge pull request #3163 from shaavan/invoice_reply_path

Introduce Reply Paths for BOLT12 Invoice in Offers Flow.

Don't rely on route-fetching rand in `blinded_path_with_custom_tlv`

`blinded_path_with_custom_tlv` indirectly relied on route CLTV
randomization when sending because nodes were at substantially
different block heights after setup. Instead we make sure all nodes
are at the same height which makes the test more robust.

Drop `Debug` on `InMemorySigner` (and `EntropySource`)

`InMemorySigner` has various private keys in it which makes
`Debug` either useless or dangerous (because most keys won't log
anything, but if they did we'd risk logging private key material).

Refactor: Take `their_node_id` by value across all handler interfaces

In order to maintain interface consistency, we refactor all message
handler interfaces to take `PublicKey` rather than `&PublicKey`, as the
difference in efficiency should be negigible and the former is easier to
handle in binding languages.

Over time, we also want to move (no pun intended) towards all messaging
interfaces using move semantics, so dropping the reference for
`PublicKey` is the first step in this direction.

Add node id to remaining `RoutingMessageHandler::handle_` methods

Previously, some `RoutingMessageHandler::handle_` methods (in particular
the ones handling node and channel announcements, as well as channel
updates, omitted the `their_node_id` argument. This didn't allow
implementors to discern *who* sent a particular method.

Here, we add `their_node_id: Option<&PublicKey>` to have them learn who
sent a message, and set `None` if our own node it the originator of a
broadcast operation.

Update Offers Test to Verify BOLT12 Invoice Reply Paths

1. Updated the Offers Test to check the reply paths in BOLT12 Invoices.
2. Changed the `extract_invoice` return type from `Option<BlindedMessagePath>`
   to `BlindedMessagePath` since all BOLT12Invoices now have a corresponding
   reply path by default.

Add reply_path to BOLT12Invoices in Offers Flow

1. Introduced reply_path in BOLT12Invoices to address a gap in error handling.
   Previously, if a BOLT12Invoice sent in the offers flow generated an Invoice Error,
   the payer had no way to send this error back to the payee.
2. By adding a reply_path to the Invoice Message, the payer can now communicate
   any errors back to the payee, ensuring better error handling and communication
   within the offers flow.

Add HMAC, and nonce to OffersContext::InboundPayment

Introduce HMAC and nonce calculation when sending Invoice with
reply path, so that if we receive InvoiceError back for the
corresponding Invoice we can verify the payment hash before logging it.

Merge pull request #3309 from TheBlueMatt/2024-09-always-print-why

Always print why we disconnect explicitly

Merge pull request #3305 from TheBlueMatt/2024-09-no-redundant-gossip-validation

Avoid redundant `{channel,node}_announcement` signature checks

Always print why we disconnect explicitly

...and drop the old, confusing, "protocol error" message when we
get a duplicate connection from a peer.

This should slightly further disambiguate disconnection reasons,
which are a key debugging tool for assigning blame.