Merge pull request #2801 from valentinewallace/2023-12-rb-groundwork-followups

#2128 follow-ups

Merge pull request #2820 from TheBlueMatt/2024-01-fuzz-ooo-monitor-updates

Test individual monitor update compl in chanmon_consistency fuzzer

Merge pull request #2821 from TheBlueMatt/2024-01-om-direct-export

Expose `onion_message` items directly rather than via re-exports

Merge pull request #2791 from valentinewallace/2023-12-multihop-recv-followups

Follow-ups to #2688

Expose `onion_message` items directly rather than via re-exports

When we originally added the `onion_message` module, there weren't
a lot of public items in it, and it didn't make a lot of sense to
export the whole sub-module structure publicly. So, instead, we
exported the public items via re-exports directly in the
`onion_message` top-level module. However, as time went on, more
and more things entered the module, which left the top-level module
rather cluttered.

Worse, in 0.0.119, we exposed
`onion_message::messenger::SendSuccess` via the return type of
`send_message`, but forgot to re-export the enum itself, making
it impossible to actually use from external code.

Here we address both issues and simply replace the re-export with
the underlying sub-module structure.

Merge pull request #2809 from TheBlueMatt/2023-12-closing-event-cleanup-fixes

Clean Up Funding Error Handling and shutdown

Rename parameter from err_packet to err_contents.

This name is more accurate since the method has been generalized to support
malformed HTLCs.

Remove outdated comment.

Normalize order of (sha256_of_onion, failure_code) in trait.

This helps avoid destructuring the tuple.

DRY malformed HTLC handling during htlc batch processing.

Fix logger usage during batched htlc processing of malforms.

Introduced due to a rebase error.

Clean up code DRY'd in previous commit.

Rename OutboundOnionPayload::BlindedReceive::amt_msat to be more descriptive.

Rename OutboundOnionPayload::Receive::amt_msat to be more descriptive.

Rename InboundOnionPayload::BlindedReceive::amt_msat to be more descriptive.

Rename InboundOnionPayload::Receive::amt_msat to be more descriptive.

Rename OutboundOnionPayload::BlindedReceive::outgoing_cltv_value.

There is no outgoing HTLC for received HTLCs, so rename to be more accurate.

Rename OutboundOnionPayload::Receive::outgoing_cltv_value.

There is no outgoing HTLC for received HTLCs, so rename to be more accurate.

Rename InboundOnionPayload::BlindedReceive::outgoing_cltv_value.

There is no outgoing HTLC for received HTLCs, so rename to be more accurate.

Rename InboundOnionPayload::Receive::outgoing_cltv_value

.. since there is no outgoing cltv for received HTLCs.

Rename outgoing_cltv_value internal var.

There is no outgoing CLTV for received HTLCs, so this new var makes more sense.

Rename InboundOnionErr to InboundHTLCErr.

The prior name seems to reference onion decode errors specifically, when in
fact the error contents are generic failure codes for any error that occurs
during HTLC receipt.

Remove trailing whitespace.

Merge pull request #2748 from TheBlueMatt/2023-11-2675-followups

Doc and style followups from #2675

Update docs on `HTLC` and `DelayedPayment` keys for clarity

This also adds required linebreaks to keep the docs to a reasonable
width.

Drop the unused `PaymentKey` type

935a716cc6c4fada075e2b740a70bb1b7b349d49 added new wrappers for the
various channel keys, including a payment_key. However, the
`payment_key` has been unused in lightning since the introduction
(and broad requiring) of the `static_remotekey` feature.

Thus, we simply remove it (and an incredibly stale TODO) here.

Update docs for slightly more clarity on channel key derivation

Merge pull request #2807 from Jossec101/log-errors-do-accept-inbound-channel

Log the errors before we return them from ChannelManager::do_accept_inbound_channel

Test individual monitor update compl in chanmon_consistency fuzzer

When users do async monitor updating, it may not be the case that
all pending monitors will complete updating at once. Thus, we
should fuzz monitor updates completing out of order, which we do
here.

Merge pull request #2817 from tnull/2024-01-time-in-electrum

Feature-gate `time` use also in `ElectrumSyncClient`

Improved error logging in ChannelManager::do_accept_inbound_channel

This refactor ensures that all API errors in do_accept_inbound_channel
are logged before they are returned.

Feature-gate `time` use also in `ElectrumSyncClient`

A previous commit introduced the `time` feature to gate the use of
`SystemTime` dependent APIs in `EsploraSyncClient`. It however omitted
doing the same for the Electrum side of things. Here, we address this
oversight.

Merge pull request #2802 from TheBlueMatt/2023-12-no-pub-enum

Move `CandidateRouteHop` enum variant fields into structs

Merge pull request #2808 from TheBlueMatt/2023-12-fuzzing-fixes-1

Fix reachable unwrap on non-channel_type manual channel acceptance

If we receive an `OpenChannel` message without a `channel_type`
with `manually_accept_inbound_channels` set, we will `unwrap()`
`None`.

This is uncommon these days as most nodes support `channel_type`,
but sadly is rather trivial for a peer to hit for those with manual
channel acceptance enabled.

Reported in and fixes #2804. Luckily, the updated
`full_stack_target` has no issue reaching this issue quickly.

Merge pull request #2799 from benthecarman/tx-sync-wasm

Don't call system time in no-std

Move `CandidateRouteHop` enum variant fields into structs

The bindings generator struggles a bit with the references in enum
variant fields in `CandidateRouteHop`. While we could probably fix
this, its much eaiser (and less risky) to inline the enum variant
fields from `CandidateRouteHop` into structs. This also lets us
make some of the fields non-public, which seems better at least for
the opaque `hint_idx` in the blinded paths.

Fix handling of duplicate initial `ChannelMonitor` writing

In e06484b0f44155e647ff29810d2f187967e45813, we added specific
handling for outbound-channel initial monitor updates failing -
in such a case we have a counterparty who tried to open a second
channel with the same funding info we just gave them, causing us
to force-close our outbound channel as it shows up as
duplicate-funding. Its largely harmless as it leads to a spurious
force-closure of a channel with a peer doing something absurd,
however it causes the `full_stack_target` fuzzer to fail.

Sadly, in 574c77e7bc95fd8dea5a8058b6b35996cc99db8d, as we were
dropping handling of `PermanentFailure` handling for updates, we
accidentally dropped handling for initial updates as well.

Here we fix the issue (again) and add a test.

Do not panic if a peer learns our funding info before we fund

We'd previously assumed that LDK would receive
`funding_transaction_generated` prior to our peer learning the txid
and panicked if the peer tried to open a redundant channel to us
with the same funding outpoint.

While this assumption is generally safe, some users may have
out-of-band protocols where they notify their LSP about a funding
outpoint first, or this may be violated in the future with
collaborative transaction construction protocols, i.e. the upcoming
dual-funding protocol.

Drop now-unused fields from `MsgHandleErrInternal`

Move ChannelClosed generation into finish_close_channel

Currently the channel shutdown sequence has a number of steps which
all the shutdown callsites have to call. Because many shutdown
cases are rare error cases, its relatively easy to miss a call and
leave users without `Event`s or miss some important cleanup.

One of those steps, calling `issue_channel_close_events`, is rather
easy to remove, as it only generates two events, which can simply
be moved to another shutdown step.

Here we remove `issue_channel_close_events` by moving
`ChannelClosed` event generation into `finish_force_close_channel`.

Move DiscardFunding generation into finish_close_channel

Currently the channel shutdown sequence has a number of steps which
all the shutdown callsites have to call. Because many shutdown
cases are rare error cases, its relatively easy to miss a call and
leave users without `Event`s or miss some important cleanup.

One of those steps, calling `issue_channel_close_events`, is rather
easy to remove, as it only generates two events, which can simply
be moved to another shutdown step.

Here we move the first of the two events, `DiscardFunding`, into
`finish_force_close_channel`.

Consider `MONITOR_UPDATE_IN_PROGRESS` as unbroadcasted funding

If we promote our channel to `AwaitingChannelReady` after adding
funding info, but still have `MONITOR_UPDATE_IN_PROGRESS` set, we
haven't broadcasted the funding transaction yet and thus should
return values from `unbroadcasted_funding[_txid]` and generate a
`DiscardFunding` event.

Fix dust buffer feerate calculation overflow

If a peer provides a feerate which nears `u32::MAX`, we may
overflow calculating the dust buffer feerate, leading to spuriously
keeping non-anchor channels open when they should be force-closed.

Fix debug assertion on opening a channel with a disconnected peer

If we try to open a channel with a peer that is disconnected (but
with which we have some other channels), we'll end up with an
unfunded channel which will lead to a panic when the peer
reconnects. Here we drop this debug assertion without bother to add
a new test, given this behavior will change in a PR very soon.

Fix `REVOKEABLE_REDEEMSCRIPT_MAX_LENGTH` for contest delays >0x7fff

When contest delays are >= 0x8000, script pushes require an extra
byte to avoid being interpreted as a negative int. Thus, for
channels with CSV delays longer than ~7.5 months we may generate
transactions with slightly too little fee. This isn't really a huge
deal, but we should prefer to be conservative here, and slightly
too high fee in the general case is better than slightly too little
fee in other cases.

Stop including dust values in feerate affordability checks

When we or our counterparty are updating the fees on the channel,
we currently check that the resulting balance is sufficient not
only to meet the reserve threshold, but also not push it below
dust. This isn't required in the BOLTs and may lead to spurious
force-closures (which would be a bit safer, but reserve should
always exceed the dust threshold).

Worse, the current logic is broken - it compares the output value
in *billionths of satoshis* to the dust limit in satoshis. Thus,
the code is borderline dead anyway, but can overflow for channels
with several million Bitcoin, causing the fuzzer to get mad (and
lead to spurious force-closures for few-billion-dollar channels).

Fix `Feature` eq + hash to ignore excess zero bytes

If we get a `Feature` object which has excess zero bytes, we
shouldn't consider it a different `Feature` from another with the
same bits set, but no excess zero bytes. Here we fix both the
`Hash` and `PartialEq` implementation for `Features` to ignore
excess zero bytes.

Merge pull request #2788 from tnull/2023-12-enforce-no-warnings-ci

Deny warnings in CI

Don't call system time in unless feature enabled

Allow `unused_variables` for one `_test_vectors` method

Allow unused imports in `lightning-invoice` prelude

Drop unused `windows` imports in `FilesystemStore` tests

Drop unenforced bound in trait alias

... to fix:
```
  error: bounds on generic parameters are not enforced in type aliases
   --> lightning/src/onion_message/messenger.rs:267:33
```

Fix unused import warnings in `c_bindings`

Fix unused imports in `lightning-invoice` no-std tests

Drop unused `sync` module from `lightning-invoice`

Fix unused (import) warnings in `no-std` builds

Fix redundant import warning in BP's `futures`

Feature-gate `GetUtxosResponse` in `lightning-block-sync`

.. as it's only used by the REST client.

Deny warnings in CI

Since we recently got rid of our last build/test/doc warnings, we now
deny warnings via `-D warnings` in CI, enforcing no new ones are
introduced.

Merge pull request #2794 from TheBlueMatt/2023-12-cut-119

Cut 0.0.119

Bump versions to 0.0.119/`lightning-invoice` 0.27

Add CHANGELOG entries for 0.0.119

Merge pull request #2798 from TheBlueMatt/2023-12-119-bindings-upstream

Small API cleanups pre-0.0.119

Merge pull request #2795 from TheBlueMatt/2023-11-robuster-chan-to-peer

 Move channel -> peer tracking to OutPoints from Channel IDs

Fix dev MSRV build of lightning-transaction-sync

...due to a transitive dependency of the `bitcoind` crate.

Use correct default value when comparing to `htlc_maximum_msat`

62f866965436fff1a8e98ee655a8a6dcbb8716c1 added two
`htlc_maximum_msat.unwrap_or`s, but used a default value of 0,
spuriously causing all HTLCs to fail if we don't have an htlc
maximum value. This should be mostly harmless, but we should fix it
anyway.

Make `FinalOnionHopData` public

In 4b5db8c3ce, `channelmanager::PendingHTLCRouting` was made
public, exposing a `FinalOnionHopData` field to the world. However,
`FinalOnionHopData` was left crate-private, making the enum
impossible to construct.

There isn't a strong reason for this (even though the
`FinalOnionHopData` API is somewhat confusing, being separated from
the rest of the onion structs), so we expose it here.

Drop explicit `bitcoin_hashes` dependency in `lightning-invoice`

Since `lightning-invoice` now depends on the `bitcoin` crate
directly, also depending on the `bitcoin_hashes` crate is redundant
and just means we confuse users by setting the `std` flag only on
`bitcoin`. Thus, we drop the explicit dependency here and replace
it with `bitcoin::hashes`.

Un-export the `PrivateRoute` inner field as there are invariants

When we make the `PrivateRoute` inner `RouteHint` `pub`, we failed
to note that the `PrivateRoute::new` constructor actually verifies
a length invariant. Thus, we un-export the inner field and force
users to go back through the `new` fn.

Merge pull request #2781 from jkczyz/2023-09-multihop-paths

Multi-hop `BlindedPath` creation interface

Fix build warning

Use one-hop blinded paths only for announced nodes

To avoid exposing a node's identity in a blinded path, only create
one-hop blinded paths if the node has been announced, and thus has
public channels. Otherwise, there is no way to route a payment to the
node, exposing its identity needlessly.

DRY up OffersMessage::InvoiceRequest handling

Multi-hop blinded payment paths in ChannelManager

When constructing blinded payment paths for Bolt12Invoice, delegate to
Router::create_blinded_payment_paths which may produce multi-hop blinded
paths. Fallback to one-hop blinded paths if the Router fails or returns
no paths.

Add create_blinded_payment_paths to Router

The Router trait is used to find a Route for paying a node. Expand the
interface with a create_blinded_payment paths method for creating such
paths to a recipient node.

Provide an implementation for DefaultRouter that creates two-hop
blinded paths where the recipient's peers serve as the introduction
nodes.

CounterpartyForwardingInfo to PaymentRelay mapping

Fix broken doc link in create_refund_builder

Multi-hop blinded paths in ChannelManager

When constructing blinded paths for Offer and Refund, delegate to
MessageRouter::create_blinded_paths which may produce multi-hop blinded
paths. Fallback to one-hop blinded paths if the MessageRouter fails or
returns no paths.

Likewise, do the same for InvoiceRequest and Bolt12Invoice reply paths.

Use CLTV_FAR_FAR_AWAY in PaymentConstraints

When finding a route through a blinded path, a random CLTV offset may be
added to the path in order to preserve privacy. This needs to be
accounted for in the blinded path's PaymentConstraints. Add
CLTV_FAR_FAR_AWAY to the max_cltv_expiry constraint to allow for such
offsets.

Consistently clean up when failing in `internal_funding_created`

When we fail to accept a counterparty's funding for various
reasons, we should ensure we call the correct cleanup methods in
`internal_funding_created` to remove the temporary data for the
channel in our various internal structs (primarily the SCID alias
map).

This adds the missing cleanup, using `convert_chan_phase_err`
consistently in all the error paths.

This also ensures we get a `ChannelClosed` event when relevant.

Fix create_one_hop_blinded_payment_path docs

Require any Router also implements MessageRouter

ChannelManager is parameterized by a Router in order to find routes when
sending and retrying payments. For the offers flow, it needs to be able
to construct blinded paths (e.g., in the offer and in reply paths).
Instead of adding yet another parameter to ChannelManager, require that
any Router also implements MessageRouter. Implement this for
DefaultRouter by delegating to a DefaultMessageRouter.

Add create_blinded_paths to MessageRouter

The MessageRouter trait is used to find an OnionMessagePath to a
Destination (e.g., to a BlindedPath). Expand the interface with a
create_blinded_paths method for creating such paths to a recipient.
Provide a default implementation creating two-hop blinded paths where
the recipient's peers serve as introduction nodes.

Add RouteBlinding feature flag

The RouteBlinding feature flag is signals support for relaying payments
over blinded paths. It is used for paying BOLT 12 invoices, which are
required to included at least one blinded path.

Re-order AnchorsZeroFeeHtlcTx in module docs/tests

Move AnchorsZeroFeeHtlcTx after Wumbo to keep order by feature bit.
Also, update setting order and comment in tests.

Merge pull request #2656 from TheBlueMatt/2023-09-scoring-decay-timer

Stop decaying liquidity information during scoring

Merge pull request #2796 from yellowred/fix/bitcoind_parse_txid

Fixes bug that causes incorrect conversion of JsonValue to Txid.

Merge pull request #2779 from G8XSU/2706-stop

Stop cleaning monitor updates on new block connect

Fixes bug with incorrect conversion of JsonValue to Txid.

The bug happens when there is a call to a bitcoind that expects a transaction id as a response and
results in a corrupted Txid being returned.

Stop cleaning monitor updates on new block connect

Previously, we used to cleanup monitor updates at both consolidation
threshold and new block connects. With this change we will only
cleanup when our consolidation criteria is met. Also, we remove
monitor read from cleanup logic, in case of update consolidation.
Note: In case of channel-closing monitor update, we still need to
read the old monitor before persisting the new one in order to
determine the cleanup range.

Merge pull request #2792 from TheBlueMatt/2023-12-no-async-signing

cfg-gate async signing logic

Drop fake time advancing in scoring tests

Now that we use explicit times passed to decay methods, there's no
reason to make calls to `SinceEpoch::advance` in scoring tests.

Drop half-life-based bucket decay in `update_history_buckets`

Because we decay the bucket information in the background, there's
not much reason to try to decay them immediately prior to updating,
and in removing that we can also clean up a good bit of dead code,
which we do here.

Make scorer decay + persistence more frequent

There's some edge cases in our scoring when the information really
should be decayed but hasn't yet been prior to an update. Rather
than try to fix them exactly, we instead decay the scorer a bit
more often, which largely solves them but also gives us a bit more
accurate bounds on our channels, allowing us to reuse channels at
a similar amount to what just failed immediately, but at a
substantial penalty.

Drop warning about mixing `no-std` and `std` `ProbabilisticScorer`s

Now that the serialization format of `no-std` and `std`
`ProbabilisticScorer`s both just use `Duration` since UNIX epoch
and don't care about time except when decaying, we don't need to
warn users to not mix the scorers across `no-std` and `std` flags.

Fixes #2539

Add a benchmark for decaying a 100k channel scorer's liquidity info

This is a good gut-check to ensure we don't end up taking a ton of
time decaying channel liquidity info.

It currently clocks in around 1.25ms on an i7-1360P.

Drop now-trivial `decayed_offset_msat` helper utility

As we now no longer decay bounds information when fetching them,
there is no need to have a decaying-fetching helper utility.

Drop now-unused `T: Time` bound on `ProbabilisticScorer`

Now that we don't access time via the `Time` trait in
`ProbabilisticScorer`, we can finally drop the `Time` bound
entirely, removing the `ProbabilisticScorerUsingTime` and type
alias indirection and replacing it with a simple struct.