Merge pull request #2946 from tnull/2024-03-txsync-readd-reorged-output-spends

Tx-Sync: Track spent `WatchedOutput`s and re-add if unconfirmed

Merge pull request #2953 from dunxen/2024-03-fix2941

Fix `ChannelManager::accept_inbound_channel` error handling

Dedup `confirmed_txs` `Vec`

Previously, we would just push to the `confirmed_txs` `Vec`, leading to
redundant `Confirm::transactions_confirmed` calls, especially now that
we re-confirm previously disconnected spends.

Here, we ensure that we don't push additional `ConfirmedTx` entries if
already one with matching `Txid` is present. This not only gets rid of
the spurious `transactions_confirmed` calls (which are harmless), but
more importantly saves us from issuing unnecessary network calls, which
improves latency.

Merge pull request #2934 from TheBlueMatt/2023-03-no-num-traits

Replace the generic `parse_int_be` with a macro called twice

Test ChannelManager::accept_inbound_channel errors

Fix ChannelManager::accept_inbound_channel error handling

Merge pull request #2935 from valentinewallace/2024-03-keysend-to-blinded

Support keysend to blinded paths

Merge pull request #2419 from jurvis/2023-03-interactivetxs

Interactive Transaction Construction

Merge pull request #2887 from benthecarman/htlc-timeout

Add HTLCsTimedOut closing reason

Merge pull request #2951 from jkczyz/2024-03-fix-sender-is-intro-node

Fix sender is the introduction node onion messages

Merge pull request #2924 from tnull/2024-03-add-user-channel-id-to-payment-forwarded

Expose `{prev,next}_user_channel_id` fields in `PaymentForwarded`

Merge pull request #2948 from arik-so/2024-03-public-node-announcement-excess

Publicize all UnsignedNodeAnnouncement fields.

Merge pull request #2949 from valentinewallace/2024-03-dont-write-empty-features

Only write `BlindedHopFeatures` if any features are set

Add HTLCsTimedOut closing reason

Before a force closure from timed out HTLCs was treated the same as when
the user manually force closed the channel. This leads to various UX
issues. This adds a new `ClosureReason` called `HTLCsTimedOut` that
signifies that the closure was caused because the HTLCs timed out. To go
along with this, previously we'd always send "Channel force-closed" when
force closing the channel in the error message which was ambigous, now
we send the force closure reason so the peer can know why the channel
was closed.

Use OnionMessenger::send_onion_message in tests

Use OnionMessenger's public interface in tests whenever possible (i.e.,
when not using any intermediate_nodes in an OnionMessagePath. This
allows us to exercise DefaultMessageRouter, and, in particular that a
path can be found for an unannounced sender when its in the introduction
node.

Use DefaultMessageRouter in functional tests

This helps test cases in DefaultMessageRouter that may not be exercised
now or in the future.

pub(crate) visibility for routing/test_utils.rs

Give pub(crate) visibility to some routing test utilities to facilitate
testing DefaultMessageRouter in functional tests.

Fix sender is the introduction node onion messages

DefaultMessageRouter will form an OnionMessagePath from a BlindedPath
where the sender is the introduction node but only if the sender is
announced. If the sender is unannounced, then DefaultMessageRouter will
fail. While DefaultMessageRouter will only create a blinded path with an
announced introduction node, it may receive one where the introduction
node is unannounced. Don't return an error in this case, as the
OnionMessenger can advance the blinded path by one hop.

This may occur when two nodes have an unannounced channel and one (the
offer creator) wants to use it for payments without an intermediary node
and without putting its node id in the offer.

Include SocketAddress in test node_announcement

Re-order imports

Only write BlindedHopFeatures if any features are set.

Minor space-saving optimization.

Publicize all UnsignedNodeAnnouncement fields.

This is necessary for RGSS tests to create new announcements.

Track spent `WatchedOutput`s and re-add if unconfirmed

Previously, we would track a spending transaction but wouldn't account
for it being reorged out of the chain, in which case we wouldn't monitor
the `WatchedOutput`s until they'd be reloaded on restart.

Here, we keep any `WatchedOutput`s around until their spends are
sufficiently confirmed and only prune them after `ANTI_REORG_DELAY`.

Merge pull request #2945 from valentinewallace/2024-03-optional-blindedhop-features

Make `BlindedHopFeatures` optional per spec

Remove redundant `claiming_channel_id` variable

.. as it's the same as `prev_channel_id` defined a few lines above.

Expose `{prev,next}_user_channel_id` fields in `PaymentForwarded`

This is useful for users that track channels by `user_channel_id`.

For example, in `lightning-liquidity` we currently keep a full
`HashMap<ChanelId, u128>` around *just* to be able to associate
`PaymentForwarded` events with the channels otherwise tracked by
`user_channel_id`.

Make blinded hop features optional per spec.

The spec states that if these features are missing, we MUST process the message
as if it were present and contained an empty array.

Merge pull request #2936 from valentinewallace/2024-03-pay-relay-constraints-ser

Fix ser for `PaymentRelay` and `PaymentConstraints`

Merge pull request #2942 from benthecarman/node-id-slice

Add NodeId::from_slice

Add NodeId::from_slice

Merge pull request #2937 from TheBlueMatt/2024-03-no-wake-on-shutdown

Avoid writing `ChannelManager` when hitting lnd bug 6039

Avoid writing `ChannelManager` when hitting lnd bug 6039

When we hit lnd bug 6039, we end up sending error messages to peers
in a loop. This should be fine, but because we used the generic
`PersistenceNotifierGuard::notify_on_drop` lock above the specific
handling, we end up writing `ChannelManager` every time we manage a
round-trip to our peer.

This can add up quite quickly, and isn't actually changing, so we
really need to avoid writing the `ChannelManager` in this case.

Fix ser for PaymentRelay and PaymentConstraints.

Two fields were serialized as u32/u64 when the spec said *tu32/tu64*.
/facepalm.

Implement interactive tx construction protocol via `InteractiveTxConstructor`

This implements the interactive construction protocol described at
https://github.com/lightning/bolts/blob/78e5a6b066d3a8e235931dfc06aa325337874749/02-peer-protocol.md?plain=1#L92.

Our implementation includes a state machine with typed states and transitions
to ensure consumers have compile-time assurances that the protocol is upheld.

States are tracked as in the `StateMachine` enum and can take on all
possible states during the negotiation.

The states are further divided into two categories, namely by the two traits
they implement, either `ReceivedMsgState` or `SentMsgState`.

The defined `StateTransitions` enforce the transitions that `ReceivedMsgState`,
`SentMsgState`, and the `_TxComplete`s can go through.

Co-authored-by: Wilmer Paulino <9447167+wpaulino@users.noreply.github.com>
Co-authored-by: Duncan Dean <git@dunxen.dev>
Co-authored-by: Jurvis Tan <5944973+jurvis@users.noreply.github.com>

Merge pull request #2932 from TheBlueMatt/2023-04-ci-fix

Avoid new "out of disk space" issues in CI

Support receiving keysend payments to blinded paths.

Parse keysend preimages in blinded onion payloads.

Support sending keysend payments to blinded paths.

Replace the generic `parse_int_be` with a macro called twice

`parse_int_be` is generic across integer types and also input
types, but to do so it relies on the `num-traits` crate. There's
not a lot of reason for this now that std has `from_be_bytes`, so
we drop the generic now and replace it with a macro which is called
twice to create two functions, both only supporting conversion from
`u5` arrays.

Use std's `from_be_bytes` rather than our `to_int_be` for int conv

`lightning-invoice` was mostly written before std's `from_be_bytes`
was stabilized, so used its own `to_int_be` utility to do int
conversions from `u8` arrays. Now that the std option has been
stable for quite some time, we should juse use it instead.

Merge pull request #2881 from TheBlueMatt/2024-02-offers-tweak

Small Offers Fixes

Use structured logging where appropriate in `OnionMessenger`

Add `PersistenceNotifierGuard` take to offer/refund payments

This resolves an issue where offer and refund payments get delayed
while we wait for the `invoice_request`/`invoice` onion messages to
get sent. It further ensures we're likely to have the
`ChannelManager` persisted with the new payment info after
initiating the send/receive.

Use `{}`, not `{:?}` for `PublicKey` logging

The `Debug` serialization of `PublicKey`s includes both the X and Y
coordinate, which isn't something most of our users deal with.
Instead, logging using `Display` gives users the keys they're used
to.

Avoid new "out of disk space" issues in CI

Our 1.63 build on Ubuntu has been failing for quite some time
because it runs out of disk space trying to build tests in the last
cfg-flag steps. Thus, we add a few new `cargo clean`s here to fix
it.

Merge pull request #2917 from jkczyz/2024-02-refund-unsupported-chain

Fail `request_refund_payment` for unsupported chain

Fail request_refund_payment for unsupported chain

If a Refund has an unsupported chain, ChannelManager should not send an
invoice as it can't be paid on that chain. Instead, return an error when
calling ChannelManager::request_refund_payment for such refunds.

Update pay_for_offer docs about unsupported chains

Test failing pay_for_offer on an unsupported chain

Merge pull request #2823 from valentinewallace/2024-01-blinded-forwarding-tests

Test blinded forwarding

Merge pull request #2903 from jkczyz/2024-02-bindings-builders

Offers builders for C-bindings

Merge pull request #2926 from tnull/2024-03-derive-eq-for-offer-refund

Impl `PartialEq`/`Eq` for `Offer`/`Refund`

Test that we'll round up blinded intermediate node fees if needed.

See comment in the new test.

Test utils: support intermediate nodes taking 1msat extra fee.

See docs on the new field.

Support fee overpayment by 1 msat in expect_payment_forwarded test util.

See ClaimAlongRouteArgs::allow_1_msat_fee_overpay.

Functionally test min htlc BlindedPayInfo calculation.

Account for prop fee in test util fee calculation.

Use correct min/max htlc in test util for constructing blinded pay params.

In testing, we use channel updates to construct blinded paths and the
{Forward,Receive}Tlvs encoded within. Given a blinded path from node A > B > C,
we currently use channel_update_A->B to construct the payment constraints for
A’s blinded payload.

This is incorrect for setting A's PaymentConstraints::htlc_minimum_msat,
because channel_update_A->B contains the minimum value that *B* will accept,
and we want the constraints to contain the min value that *A* will accept.

This never caused test failures before because min/max htlc values were always
identical in both channel directions.

Therefore, set A’s htlc min/max values to the min/max that A will accept.

Parameterize blinded test util with intro node min/max htlc.

We're currently not setting each node's min/max htlc correctly, see next
commit.

Struct-ify test util pass_along_path args.

Lays groundwork to make pass_along_path easier to adapt without changing a
million callsites.

Test 3-hop blinded path recipient failure.

This tests an intermediate forwarding node being failed back to with malformed
and then failing back themselves with malformed.

Drop error type parameter from SignError

SignError allows implementors of SignFunction to return a custom error
type. Drop this as an unconstrained type causes problems with bindings
and isn't useful unless the caller can take some sort of action based on
different errors.

Fix unused import warning

Support BOLT 12 signing in c_bindings

Replace the Fn trait bound on signing methods with a dedicated trait
since Fn is not supported in bindings. Implement the trait for Fn so
that closures can still be used in Rust.

Add c_bindings version of InvoiceBuilder

Use the macros introduced in the previous commit to define two builders
for each type parameterization of InvoiceBuilder
- InvoiceWithExplicitSigningPubkeyBuilder
- InvoiceWithDerivedSigningPubkeyBuilder

The difference between these and InvoiceBuilder is that these have
methods that take `self` by mutable reference instead of by value and
don't return anything instead returning the modified builder. This is
required because bindings don't support move semantics nor impl blocks
specific to a certain type parameterization. Because of this, the
builder's contents must be cloned when building a Bolt12Invoice.

Keeps InvoiceBuilder defined so that it can be used internally in
ChannelManager's OffersMessageHandler even when compiled for c_bindings.

Implement `Hash` for `Offer` and `Refund`

Impl `PartialEq`/`Eq` for `Offer`/`Refund`

We add custom implementations based on comparing the `bytes` for
`Offer`/`Refund` themselves as this is sufficient and should be faster
than comapring all fields in the worst case.

Merge pull request #2923 from tnull/2024-03-improve-best-block

Refactor `BestBlock` to expose inner fields

Implement `Hash` for `BestBlock`

Merge pull request #2912 from jkczyz/2024-02-prefer-more-connections

Order blinded paths by reliability criteria

Implement `Debug` for `BestBlock`

Refactor `BestBlock` to expose inner fields rather than accessors

.. which is more idiomatic Rust, and easier to handle in (downstream)
bindings.

Macro-ize InvoiceBuilder

InvoiceBuilder is not exported to bindings because it has methods that
take `self` by value and are only implemented for certain type
parameterizations. Define these methods using macros such that different
builders and related methods can be defined for c_bindings.

Merge similar InvoiceBuilder impl blocks

This avoids needing to create additional macros when adding c_bindings
support.

Add c_bindings version of InvoiceRequestBuilder

Use the macros introduced in the previous commit to define two builders
for each type parameterization of InvoiceRequestBuilder
- InvoiceRequestWithExplicitPayerIdBuilder
- InvoiceRequestWithDerivedPayerIdBuilder

The difference between these and InvoiceRequestBuilder is that these
have methods that take `self` by mutable reference instead of by value
and don't return anything instead returning the modified builder. This
is required because bindings don't support move semantics nor impl
blocks specific to a certain type parameterization. Because of this, the
builder's contents must be cloned when building an InvoiceRequest.

Keeps InvoiceRequestBuilder defined so that it can be used internally in
ChannelManager::pay_for_offer even when compiled for c_bindings.

Macro-ize InvoiceRequestBuilder

InvoiceRequestBuilder is not exported to bindings because it has methods
that take `self` by value and are only implemented for certain type
parameterizations. Define these methods using macros such that different
builders and related methods can be defined for c_bindings.

Merge similar InvoiceRequestBuilder impl blocks

This avoids needing to create additional macros when adding c_bindings
support.

Add c_bindings version of RefundBuilder

Use the macros introduced in the previous commit to define a builder
called RefundMaybeWithDerivedMetadataBuilder.

The difference between this and RefundBuilder is that this has methods
that take `self` by mutable reference instead of by value and don't
return anything instead returning the modified builder. This is required
because bindings don't support move semantics. Because of this, the
builder's contents must be cloned when building a Refund.

Keeps RefundBuilder defined so that it can be used internally in
ChannelManager::create_refund_builder even when compiled for c_bindings.

Macro-ize RefundBuilder

RefundBuilder is not exported to bindings because it has methods that
take `self` by value. Define these methods using macros such that
different builders and related methods can be defined for c_bindings.

Add c_bindings version of OfferBuilder

Use the macros introduced in the previous commit to define two builders
for each type parameterization of OfferBuilder
- OfferWithExplicitMetadataBuilder
- OfferWithDerivedMetadataBuilder

The difference between these and OfferBuilder is that these have methods
that take `self` by mutable reference instead of by value and don't
return anything instead returning the modified builder. This is required
because bindings don't support move semantics nor impl blocks specific
to a certain type parameterization. Because of this, the builder's
contents must be cloned when building an Offer.

Keeps OfferBuilder defined so that it can be used internally in
ChannelManager::create_offer_builder even when compiled for c_bindings.

Merge pull request #2919 from tnull/2024-03-fix-hashtables-rustfmt

`rustfmt`: Fix CI by running `rustfmt` on `util/hash_tables.rs`

Run `rustfmt` on `util/hash_tables.rs`

.. to fix the silent rebase conflict.

Merge pull request #2918 from tnull/2024-03-prefer-from-over-into

Prefer implementing `From` over `Into`

.. as the std library docs state that implementing Into should be avoided:
"One should avoid implementing Into and implement From instead.
Implementing From automatically provides one with an implementation of
Into thanks to the blanket implementation in the standard library."

Merge pull request #2916 from sr-gi/2024-02-29-payment-preimage-hash

util: Adds Into<PaymentHash> for PaymentPreimage

util: Adds Into<PaymentHash> for PaymentPreimage

This seems like a useful interface to have for downstream users

Merge pull request #2915 from TheBlueMatt/2024-02-better-rnf-docs

Update docs on `PaymentFailureReason::RouteNotFound` to add context

Update docs on `PaymentFailureReason::RouteNotFound` to add context

tnull noted on discord that its somewhat unclear to users what
`RouteNotFound` actually means - that we ran out of routes, rather
than could not find a route at all - so the docs are updated here.

Merge pull request #2913 from arik-so/2024/02/serialize-socket-address-vectors

Serialize SocketAddress vectors

Serialize SocketAddress vectors.

Prefer one-hop blinded path to Tor intro nodes

If a node is announced, prefer using a one-hop blinded path with it as
the introduction node to using a two-hop blinded path with a Tor-only
introduction node. The one-hop blinded path is more reliable, thus only
use Tor-only nodes if the recipient is unannounced. And then, prefer
non-Tor-only nodes.

Prefer well-connected nodes for introduction nodes

When forming blinded paths, using a reliable node as the introduction
node is important to ensure onion message reliability. Order blinded
paths by how well-connected the introduction node is as a proxy for
reliability.

For short paths (e.g., when the sender and recipient share an LSP), this
may also result in a scenario where initiating a direct connection isn't
necessary. That is helpful when using RGS since it currently doesn't
include node announcements and thus cannot initiate a direct connection.

Merge pull request #2911 from jkczyz/2024-02-sort-blinded-paths

Prefer non-Tor nodes when creating blinded paths

Prefer non-Tor nodes when creating blinded paths

Tor nodes can have high latency which can have a detrimental effect on
onion message reliability. Prefer using nodes that aren't Tor-only when
creating blinded paths both in offers and in onion message reply paths.

Add NodeInfo::is_tor_only

Add a method to NodeInfo to determine if the node has only announced Tor
addresses. Useful for preferring blinded paths that don't use Tor for
better reliability and improved latency.

Adapt intro node process_pending_htlcs test for non-intro nodes

Macro-ize test code block that causes an intro node error

Will be useful in the next commit for adapting these tests for non-intro node
failures.

Adapt intro node forward checks test for non-intro nodes

Macro-ize test code that causes an intro node error on initial update_add

Will be useful in the next commit for adapting this test for non-intro-node
failures.

Test MPP to 3-hop blinded paths.