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`

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.

Macro-ize OfferBuilder

OfferBuilder 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 pull request #2909 from benthecarman/inv-pk-helper

Add helper function to properly get invoice pubkey

Merge pull request #2877 from tnull/2024-02-start-rustfmt-journey

`rustfmt`: Add CI scripts and format `onion_utils.rs`

Add helper function to properly get invoice pubkey

Merge pull request #2908 from TheBlueMatt/2024-02-drop-useless-refs

Drop unnecessary int reference in SCID conversion utilities

Merge pull request #2905 from tnull/2024-02-expose-init-features

Refactor `PeerManager::get_peer_node_ids` in favor of `list_peers`/`peer_by_node_id` returning additional information

Drop unnecessary int reference in SCID conversion utilities

Merge pull request #2770 from dunxen/2023-12-dualfundingpreliminaries

Preliminary refactoring & structure for dual-funded channels

Introduce `PeerManager::list_peers` and `peer_by_node_id`

.. returning `PeerDetails` rather than tuples of peer-associated values.

Previously, we wouldn't offer any way to retrieve the features a
peer provided in their `Init` message.

Here, we allow to retrieve them via a new `PeerDetails` struct,
side-by-side with `SocketAddress`es and a bool indicating the direction
of the peer connection.

Merge pull request #2898 from tnull/2024-02-ignore-RUSTSEC-2021-0145

Have CI's `cargo audit` ignore `RUSTSEC-2021-0125`

Add V2 `ChannelPhase` variants

Add maybe_handle_error_without_close for OutboundV2Channel

Add `OutboundV2Channel` struct

Create ChannelContext constructor for outbound channels

Add `InboundV2Channel` struct

Create ChannelContext constructor for inbound channels

Add `DualFundingChannelContext` struct

Test builds with #[cfg(dual_funding)] enabled

Add V2 constructors to `ChannelId`

Merge pull request #2891 from TheBlueMatt/2024-02-no-ahash

Drop the `ahash` dependency

Have CI's `cargo audit` ignore `RUSTSEC-2021-0125`

This advisory is only relevant for a downstream dependency of
`criterion`, which we currently don't want to bump in order to continue
benchmarking with our MSRV 1.63.0.

We therefore just add it to our ignore list for now.

Merge pull request #2897 from TheBlueMatt/2024-02-fix-route-ser

Fix `Route` serialization round-trip

Drop `lightning-invoice` dependency on hashbrown`

Drop `ahash` dependency in favor of core's `SipHasher`

https://github.com/tkaitchuck/aHash/pull/196 bumped the MSRV of
`ahash` in a patch release, which makes it rather difficult for us
to have it as a dependency.

Further, it seems that `ahash` hasn't been particularly robust in
the past, notably
https://github.com/tkaitchuck/aHash/issues/163 and
https://github.com/tkaitchuck/aHash/issues/166.

Luckily, `core` provides `SipHasher` even on no-std (sadly its
SipHash-2-4 unlike the SipHash-1-3 used by the `DefaultHasher` in
`std`). Thus, we drop the `ahash` dependency entirely here and
simply wrap `SipHasher` for our `no-std` HashMaps.

Add a crate which wraps `getrandom` but always compiles

In the next commit we'll drop the `ahash` dependency in favor of
directly calling `getrandom` to seed our hash tables. However,
we'd like to depend on `getrandom` only on certain platforms *and*
only when certain features (no-std) are set.

This introduces an indirection crate to do so, allowing us to
depend on it only when `no-std` is set but only depending on
`getrandom` on platforms which it supports.

Test `Route` serialization round-trip

This adds testing for the previous two commits by testing that all
routes generated in testing are able to survive a serialization
round-trip.

Fix `Route` serialization round-trip

When the `max_total_routing_fee_msat` parameter was added to
`RouteParameters`, the serialization used `map` to get the max fee,
accidentally writing an `Option<Option<u64>>`, but then read it as
an `Option<u64>`. Thus, any `Route`s with a `route_params` written
will fail to be read back.

Luckily, this is an incredibly rarely-used bit of code, so only one
user managed to hit it.

Fix blinded path serialization in `Route`

`Route`'s blinded_path serialization logic writes a blinded path
`Option` per path hop, however on read we (correctly) only read one
blinded path `Option` per path. This causes serialization of
`Route`s with blinded paths to fail to round-trip.

Here we fix this by writing blinded paths per path.

Drop the `fails_paying_for_bolt12_invoice` test

`fails_paying_for_bolt12_invoice` tests that we fail to send a
payment if the router returns `Ok` but includes a bogus route (one
with 0-length paths). While this marginally increases our test
coverage, in the next commit we'll be testing that all routes
round-trip serialization, which fails here as bogus routes are not
supported in deserialization.

Because this isn't particularly critical test coverage, we simply
opt to drop the test entirely here.

Remove `onion_utils.rs` from exclusion list

.. now continously checking its formatting in CI.

Run `rustfmt` on `onion_utils.rs`

Add `rustfmt::skip` and small adjustments where needed

Merge pull request #2894 from TheBlueMatt/2024-02-future-poll-leak

Never store more than one StdWaker per live Future

Never store more than one `StdWaker` per live `Future`

When an `std::future::Future` is `poll()`ed, we're only supposed to
use the latest `Waker` provided. However, we currently push an
`StdWaker` onto our callback list every time `poll` is called,
waking every `Waker` but also using more and more memory until the
`Future` itself is woken.

Here we fix this by removing any `StdWaker`s stored for a given
`Future` when it is `drop`ped or prior to pushing a new `StdWaker`
onto the list when `poll`ed.

Sadly, the introduction of a `Drop` impl for `Future` means we
can't trivially destructure the struct any longer, causing a few
methods to need to take `Future`s by reference rather than
ownership and `clone` a few `Arc`s.

Fixes #2874

Give `Future`s for a `FutureState` an idx and track `StdWaker` idxn

When an `std::future::Future` is `poll()`ed, we're only supposed to
use the latest `Waker` provided. However, we currently push an
`StdWaker` onto our callback list every time `poll` is called,
waking every `Waker` but also using more and more memory until the
`Future` itself is woken.

Here we take a step towards fixing this by giving each `Future` a
unique index and storing which `Future` an `StdWaker` came from in
the callback list. This sets us up to deduplicate `StdWaker`s by
`Future`s in the next commit.

Split lists of `Waker` and directly-registered `Future` callbacks

In the next commit we'll fix a memory leak due to keeping too many
`std::task::Waker` callbacks in `FutureState` from redundant `poll`
calls, but first we need to split handling of `StdWaker`-based
future wake callbacks from normal ones, which we do here.

Merge pull request #2895 from TheBlueMatt/2024-02-logging-tweaks

Minor Logging tweaks

Add `rustfmt` instructions to CONTRIBUTING.md

Add `rustfmt` to CI, ignore all files

We add the previously discussed `rustfmt.toml` and enforce it in CI for
any files that are not contained in an exclusion list.

To start, we add all current Rust files to this exclusion list. This
means that formatter rules will be enforced for any newly introduced
files, and we'll then start going through the codebase file-by-file,
removing them from the list as we go.

Merge pull request #2883 from tnull/2024-02-dyn-kvstore-blanket-impls

Add blanket `Persist`/`Persister` impls for `dyn KVStore + Send + Sync`

Opportunistically skip log in `update_claims_view_from_matched_txn`

On each block, for each `ChannelMonitor`, we log two status
statements in `OnChainTx::update_claims_view_from_matched_txn`.
This can add up to quite a bit, and is generally not very
interesting when we don't actually do anything if there's no claims
to bump.

Here we drop both logs if we have no claims to work with, but
retain it if we process any claims.

Opportunistically skip log in `update_claims_view_from_requests`

On each block, for each `ChannelMonitor`, we log a status statement
in `OnChainTx::update_claims_view_from_requests`. This can add up
to quite a bit, and is generally not very interesting when we don't
actually do anything if there's no claims to bump.

Here we drop the log if we have no claims to work with, but retain
it if we process any claims.

Drop some "Channel does not qualify for a feerate change" logs

On a high-traffic/channel node, `Channel .* does not qualify for a
feerate change.*` is our most common log, and it doesn't provide
much useful information. It's logged in two cases - (a) where the
estimator feerate is less than the current channel feerate but not
by more than half twice and (b) where we'd like to update the
channel feerate but the peer is disconnected or channel not
available for updates.

Because these conditions can persist and we log them once a minute
the volume of logs can add up quickly. Here we simply remove the
log in case (a), though leave (b) as its anticipated to be somewhat
quieter and does indicate a persistent issue that should be
addressed (possibly by closing the channel).

Make peers sending gossip out of order logging less scary

Multiple times we've had users wonder why they see `Error handling
message from.*; ignoring: Couldn't find channel for update` in
their logs and wonder if its related to their channel
force-closing. While this does indicate a peer is sending us gossip
our of order (and thus misbehaving), its not relevant to channel
operation and the logged message and level should indicate that.

Thus, here, we move the level to Gossip and add "gossip" between
"handling" and "message" (so it reads "Error handling gossip
message from.*").

Fixes #2471

Merge pull request #2861 from tnull/2024-01-introduce-cargo-audit

Introduce CI workflow running `cargo audit`

Merge pull request #2816 from wpaulino/retryable-holder-sigs

Allow holder commitment and HTLC signature requests to fail

Merge pull request #2892 from TheBlueMatt/2024-02-destination-eq

Add further standard derives to various onion message structs

Introduce CI workflow running `cargo audit`

In order to continuously monitor our dependencies for security
vulnerabilities, we introduce a new CI job that will use `cargo audit`
to check for any known vulnerabilities.

This job is run on a daily schedule. For each new advisory, a new issue
will be created.

Add further standard derives to various onion message structs

Merge pull request #2888 from TheBlueMatt/2024-02-destination-eq

Implement `Debug`/`PartialEq`/`Eq` for `Destination`

Drop `reqwest` pin

.. since a version with fixed MSRV was released by now.

Move `lightning-transaction-sync` to main workspace

.. so it's actually included in the audit.

Disable `lightning-transaction-sync` integration tests on Windows

.. as the `electrsd` crate doesn't support it.

While we previously did so in our CI script, we now also `cfg`-gate the
tests and dependencies for easier handling.

Implement `Debug`/`PartialEq`/`Eq` for `Destination`

Merge pull request #2850 from TheBlueMatt/2024-01-fuzz-gossip

Merge pull request #2886 from TheBlueMatt/2024-02-actually-fix-build

 Fix silent merge conflict introduced in d3ddf15 @TheBlueMatt

Replace spaces with tabs in msgs.rs

Fix silent merge conflict introduced in d3ddf15357589ed10f4c844dc22

Merge pull request #2885 from benthecarman/tiny-opt

Worlds smallest optimization

Worlds smallest optimization

Preallocate for 8 items in the vec. I chose this value for

1. features
2. description
3. payment hash
4. expire time
5. min_final_cltv
6. payment secret
7. route hint
8. for the memes

Merge pull request #2871 from dunxen/2024-02-msgcommonfields

Combine common fields for OpenChannel and AcceptChannel V1 & V2

Allow holder commitment and HTLC signature requests to fail

As part of the ongoing async signer work, our holder signatures must
also be capable of being obtained asynchronously. We expose a new
`ChannelMonitor::signer_unblocked` method to retry pending onchain
claims by re-signing and rebroadcasting transactions. Unfortunately, we
cannot retry said claims without them being registered first, so if
we're not able to obtain the signature synchronously, we must return the
transaction as unsigned and ensure it is not broadcast.

Introduce FeerateStrategy enum for onchain claims

This refactors the existing `force_feerate_bump` flag into an enum as we
plan to introduce a new flag/enum variant in a future commit.

Rework get_latest_holder_commitment_txn to broadcast for users

This method is meant to be used as a last resort when a user is forced
to broadcast the current state, even if it is stale, in an attempt to
claim their funds in the channel. Previously, we'd return the commitment
and HTLC transactions such that they broadcast them themselves. Doing so
required a different code path, one which was not tested, to obtain
these transactions than our usual path when force closing. It's not
worth maintaining both, and it's much simpler for us to broadcast
instead.