Merge pull request #3297 from TheBlueMatt/2024-09-cleanup-package

Cleanup `PackageTemplate`a bit

Add a test for the fee-bump rate of timeout HTLC claims on cp txn

In a previous commit we updated the fee-bump-rate of claims against
HTLC timeouts on counterparty commitment transactions so that
instead of immediately attempting to bump every block we consider
the fact that we actually have at least `MIN_CLTV_EXPIRY_DELTA`
blocks to do so, and bumping at the appropriate rate given that.

Here we test that by adding an extra check to an existing test
that we do not bump in the very next block after the HTLC timeout
claim was initially broadcasted.

Set correct `counterparty_spendable_height` for outb local HTLCs

For outbound HTLCs, the counterparty can spend the output
immediately. This fixes the `counterparty_spendable_height` in the
`PackageTemplate` claiming outbound HTLCs on local commitment
transactions, which was previously spuriously set to the HTLC
timeout (at which point *we* can claim the HTLC).

Rename `soonest_conf_deadline` to `counterparty_spendable_height`

This renames the field in `PackageTemplate` which describes the
height at which a counterparty can make a claim to an output to
match its actual use.

Previously it had been set based on when a counterparty can claim
an output but also used for other purposes. In the previous commit
we cleaned up its use for fee-bumping-rate, so here we can rename
it as it is now only used as the `counteraprty_spendable_height`.

Clean up `PackageTemplate::get_height_timer` to consider type

`PackageTemplate::get_height_timer` is used to decide when to next
bump our feerate on claims which need to make it on chain within
some window. It does so by comparing the current height with some
deadline and increasing the bump rate as the deadline approaches.

However, the deadline used is the `counterparty_spendable_height`,
which is the height at which the counterparty might be able to
spend the same output, irrespective of why. This doesn't make sense
for all output types, for example outbound HTLCs are spendable by
our counteraprty immediately (by revealing the preimage), but we
don't need to get our HTLC timeout claims confirmed immedaitely,
as we actually have `MIN_CLTV_EXPIRY` blocks before the inbound
edge of a forwarded HTLC becomes claimable by our (other)
counterparty.

Thus, here, we adapt `get_height_timer` to look at the type of
output being claimed, and adjust the rate at which we bump the fee
according to the real deadline.

Stop passing current height to `PackageTemplate::build_package`

Now that we don't store the confirmation height of the inputs
being spent, passing the current height to
`PackageTemplate::build_package` is useless - we only use it to set
the height at which we should next bump the fee, but we just want
it to be "next block", so we might as well use `0` and avoid the
extra argument. Further, in one case we were already passing `0`,
so passing the argument is just confusing as we can't rely on it
being set.

Note that this does remove an assertion that we never merge
packages that were crated at different heights, and in the future
we may wish to do that (as there's no specific reason not to), but
we do not currently change the behavior.

Drop unused `PackageTemplate::height_original`

This has never been used, and its set to a fixed value of zero for
HTLCs on local commitment transactions making it impossible to rely
on so might as well remove it.

Merge pull request #3330 from tnull/2024-09-add-macros-crate

Add `lightning-macros` crate and drop `bdk_macros` dependency

Merge pull request #3371 from tnull/2024-10-allow-to-set-payment-and-description-hash

Invoice utils: allow to set both description hash and payment hash

Update `lightning-transaction-sync` to use macros crate

Add `lightning-macros` crate

Previously, we used the `bdk_macros` dependency for some simple proc
macros in `lightning-transaction-sync`. However, post-1.0 BDK doesn't
further maintain this crate and will at some point probably yank it
together with the old `bdk` crate that was split up.

Here, we create a new crate for utility proc macros and ~~steal~~ add
what we currently use (slightly modified for the latest `syn` version's
API though). In the future we may want to expand this crate, e.g., for
some `maybe_async` macros in the context of an `async KVStore`
implementation.

Rename `PackageTemplate::timelock` `counteraprty_spendable_height`

This function was very confusing - its used to determine by when
we have to stop aggregating this claim with others as it starts to
be at risk of pinning due to the counterparty's ability to spend
the output.

It is not ever used as a timelock for a transaction, and thus its
name is very confusing.

Instead we rename it `counterparty_spendable_height`.

Rename claim cleaning match bool for accuracy

We don't actually care if a confirmed transaction claimed other
outputs, only that it claimed a superset of the outputs in the
pending claim we're looking at. Thus, the variable to detect that
is renamed `is_claim_subset_of_tx` instead of `are_sets_equal`.

Merge pull request #3316 from optout21/signer_extend

[Splicing] Signer extended with method to sign prev funding transaction input

Invoice utils: allow to set both description hash and payment hash

Signer extended with method to sign prev funding transaction input

Merge pull request #3329 from arik-so/monitor_archive_docs_followup

Document monitor archival idempotency requirement (#3276 followup)

Merge pull request #3363 from G8XSU/event-logging-2

Revert "Add logging around event processing."

Merge pull request #3366 from TheBlueMatt/2024-10-0.0.125-relnotes

Add CHANGELOG entry for 0.0.125

Add CHANGELOG entry for 0.0.125

Revert "Add logging around event processing."

This reverts commit 85eb8145fba1dbf3b9348d9142cc105ee13db33b.
Logging here can be overly verbose and moreover in case of event
handling failure, we loop back without any added delay.

Merge pull request #3354 from tnull/2024-10-fix-block-init-synchronize

`lightning-block-sync`: Fix `synchronize_listeners` always calling default implementation

Document monitor archival idempotency requirement.

Merge pull request #3349 from TheBlueMatt/2024-10-3270-followups

Minor #3270 Followups

Merge pull request #3353 from tnull/2024-10-simplify-block-sync

Drop unnecessary `Result` in `RpcClient::new`

Fix `synchronize_listeners` calling default implementation

Previously, the `ChainListenerSet` `Listen` implementation wouldn't
forward to the listeners `block_connected` implementation outside of
tests. This would result in the default implementation of
`Listen::block_connected` being used and the listeners implementation
never being called.

Drop unnecessary `Result` in `RestClient::new`

.. as it's infallible

Drop unnecessary `Result` in `RpcClient::new`

.. as it's infallible

Merge pull request #3339 from arik-so/lint-script-file

Create script file for easy local linting

Create script file for easy local linting

Merge pull request #3346 from TheBlueMatt/2024-10-dns-feature-flag

Add support for parsing the dns_resolver feature bit

Merge pull request #3235 from Mirebella/add-local-balance-msats

Add `last_local_balance_msats` field

Merge pull request #3351 from carlaKC/arch-update-keys-interface

docs/trivial: update diagram to reflect changes to KeysInterface

Add `Event::ChannelClosed::last_local_balance_msats`

Users commonly want to know what their balance was when a channel
was closed, which this provides in a somewhat simplified manner.

It does not consider pending HTLCs and will always overstate our
balance by transaction fees.

arch: update diagram to reflect changes to KeysInterface

KeysInterface was split into EntropySource, SignerProvider and
NodeSigner in #1930.

Hold a reference to byte arrays when serializing to bech32

When we serialize from a byte array to bech32 in
`lightning-invoice`, we can either copy the array itself into the
iterator or hold a reference to the array and iterate through that.

In aa2f6b47df312f026213d0ceaaff20ffe955c377 we opted to copy the
array into the iterator, which is fine for the current array sizes
we're working with, but does result in additional memory on the
stack if, in the future, we end up writing large arrays.

Instead, here, we switch to using the slice serialization code when
writing arrays, (very marginally) reducing code size and reducing
stack usage.

Marginally reduce allocations in `lightning-invoice`

In aa2f6b47df312f026213d0ceaaff20ffe955c377 we refactored
`lightning-invoice` de/serialization to use the new version of
`bech32`, but in order to keep the public API the same we
introduced one allocation we could have skipped.

Instead, here, we replace the public `Utf8Error` with
`FromUtf8Error` which contains the original data which failed
conversion, removing an allocation in the process.

Check that the HRPs generated in BOLT 11 `RawHrp` are always valid

...in `debug_assertions`.

Drop one unnecessary allocation added in aa2f6b47df312f026213d0ceaa

In aa2f6b47df312f026213d0ceaaff20ffe955c377 we refactored
`lightning-invoice` de/serialization to use the new version of
`bech32`, but ended up adding one unnecessary allocation in our
offers logic, which we drop here.

Marginally reduce allocations in `lightning-invoice`

In aa2f6b47df312f026213d0ceaaff20ffe955c377 we refactored
`lightning-invoice` de/serialization to use the new version of
`bech32`, also reducing some trivial unnecessary allocations when
we did so.

Here we drop a few additional allocations which came up in review.

Merge pull request #3270 from optout21/bech32-iterser

Upgrade bech32 dependency (iterative)

Upgrade bech32 dependency, bech32 serialization improvements

Allow a `DNSResolverMessageHandler` to set `dns_resolver` feature

A `DNSResolverMessageHandler` which handles resolution requests
should want the `NodeFeatures` included in the node's
`node_announcement` to include `dns_resolver` to indicate to the
world that it provides that service. Here we enable this by
requesting extra feature flags from the `DNSResolverMessageHandler`
in the features `OnionMessenger`, in turn, provides to
`PeerManager` (which builds the `node_announcement`).

Add support for parsing the `dns_resolver` feature bit

This feature bit is used to indicate that a node will make DNS
queries on behalf of onion message senders, returning DNSSEC TXT
proofs for the requested names.

It is used to signal support for bLIP 32 resolution and can be used
to find nodes from which we can try to resolve BIP 32 HRNs.

Merge pull request #3179 from TheBlueMatt/2024-07-human-readable-names-resolution-1

Add the core functionality required to resolve Human Readable Names

Merge pull request #3301 from dunxen/2024-9-fixnevertypefallback

Add an explicit_type TLV syntax for avoiding certain cases of type inference

Add Bolt11InvoiceFeatures serialization tests

Add an `explicit_type` TLV syntax for avoiding certain cases of type inference

This new syntax is used to fix "dependency on fallback of ! -> ()".
This avoids cases where code compiles with a fallback of the
never type leading to the unit type. The behaviour in Rust edition 2024
would make this a compile error.

See: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/builtin/static.DEPENDENCY_ON_UNIT_NEVER_TYPE_FALLBACK.html#

Merge pull request #3324 from tnull/2024-09-rustfmt-util-1

`rustfmt`: Run on `util/*` (1/2)

Merge pull request #3311 from TheBlueMatt/2024-09-3010-followups

Quick #3010 followups

Call `ChannelMessageHandler::message_received` without peer lock

While `message_received` purports to be called on every message,
prior to the message, doing so on `Init` messages means we have to
call `message_received` while holding the per-peer mutex, which
can cause some lock contention.

Instead, here, we call `message_received` after processing `Init`
messages (which is probably more useful anyway - the peer isn't
really "connected" until we've processed the `Init` messages),
allowing us to call it unlocked.

Check that we aren't reading a second message in BOLT 12 retry test

`creates_and_pays_for_offer_with_retry` intends to check that we
re-send a BOLT 12 `invoice_request` in response to a
`message_received` call, but doesn't actually test that there were
no messages in the outbound buffer after the initial send, which we
do here.

Add the core functionality required to resolve Human Readable Names

This adds a new utility struct, `OMNameResolver`, which implements
the core functionality required to resolve Human Readable Names,
namely generating `DNSSECQuery` onion messages, tracking the state
of requests, and ultimately receiving and verifying `DNSSECProof`
onion messages.

It tracks pending requests with a `PaymentId`, allowing for easy
integration into `ChannelManager` in a coming commit - mapping
received proofs to `PaymentId`s which we can then complete by
handing them `Offer`s to pay.

It does not, directly, implement `DNSResolverMessageHandler`, but
an implementation of `DNSResolverMessageHandler` becomes trivial
with `OMNameResolver` handling the inbound messages and creating
the messages to send.

Allow `_` in `Hostname`s

These are perfectly fine and are relied on by BIP 353, so we need
to ensure we allow them.

Add a type to track `HumanReadableName`s

BIP 353 `HumanReadableName`s are represented as `₿user@domain` and
can be resolved using DNS into a `bitcoin:` URI. In the next
commit, we will add such a resolver using onion messages to fetch
records from the DNS, which will rely on this new type to get name
information from outside LDK.

Parse and handle `DNSResolverMessage`s in `OnionMessenger`

This adds the requisite message parsing and handling code for the
new DNSSEC messages to `OnionMessenger`.

Add DNS(SEC) query and proof messages and onion message handler

This creates the initial DNSSEC proof and query messages in a new
module in `onion_message`, as well as a new message handler to
handle them.

In the coming commits, a default implementation will be added which
verifies DNSSEC proofs which can be used to resolve BIP 353 URIs
without relying on anything outside of the lightning network.

Add a `MessageContext::DNSResolution` to protect against probing

When we make a DNSSEC query with a reply path, we don't want to
allow the DNS resolver to attempt to respond to various nodes to
try to detect (through timining or other analysis) whether we were
the one who made the query. Thus, we need to include a nonce in the
context in our reply path, which we set up here by creating a new
context type for DNS resolutions.

Merge pull request #3341 from TheBlueMatt/2024-09-gossip-rustfmt-cleanup

Minor `gossip.rs` `rustfmt` cleanups

Minor `gossip.rs` `rustfmt` cleanups

Just a few minor updates to `gossip.rs` to reduce code verticality
and simplify things a bit.

Merge pull request #3336 from tnull/2024-09-rustfmt-router

`rustfmt`: Run on the `routing` directory (1/3)

Merge pull request #3334 from arik-so/lint_fixes

Fix linter complaints

`rustfmt`: Drop `routing/gossip.rs` from exclusion list

`rustfmt`: Run on `routing/gossip.rs`

Suppress linter warnings

Allow `map_err` and `dependency_on_unit_never_type_fallback` pending
a future MSRV increase.

Merge pull request #3303 from TheBlueMatt/2024-09-inbound-payment-id

Add a `PaymentId` for inbound payments

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.

Add a `PaymentId` for inbound payments

We expect our users to have fully idempotent `Event` handling as we
may replay events on restart for one of a number of reasons. This
isn't a big deal as long as all our events have some kind of
identifier users can use to check if the `Event` has already been
handled.

For outbound payments, this is the `PaymentId` they provide in the
send methods, however for inbound payments we don't have a great
option.

`PaymentHash` largely suffices - users can simply always claim in
response to a `PaymentClaimable` of sufficient value and treat a
`PaymentClaimed` event as duplicate any time they see a second one
for the same `PaymentHash`. This mostly works, but may result in
accepting duplicative payments if someone (incorrectly) pays twice
for the same `PaymentHash`.

Users could also fail for duplicative `PaymentClaimable` events of
the same `PaymentHash`, but doing so may result in spuriously
failing a payment if the `PaymentClaimable` event is a replay and
they never saw a corresponding `PaymentClaimed` event.

While none of this will result in spuriously thinking they've been
paid when they have not, it does result in some pretty awkward
semantics which we'd rather avoid our users having to deal with.

Instead, here, we add a new `PaymentId` which is simply an HMAC of
the HTLCs (as Channel ID, HTLC ID pairs) which were included in the
payment.

Add an `inbound_payment_id_secret` to `ChannelManager`

In the next commit we'll start generating `PaymentId`s for inbound
payments randomly by HMAC'ing the HTLC set of the payment. Here we
start by defining the HMAC secret for these HMACs.

This requires one small test adaptation and a full_stack_target
fuzz change because it changes the RNG consumption.

Do not check the ordering of HTLCs in `PaymentClaim[able,ed]`

In the next commit we'll change the order of HTLCs in
`PaymentClaim[able,ed]` events. This shouldn't break anything, but
our current functional tests check that the HTLCs are provided in
the order they expect (the order they were received). Instead, here
we only validate that each claimed HTLC matches one expected path.

`rustfmt`: Drop `util/persist.rs` from exclusion list

`rustfmt`: Run on `util/persist.rs`

`rustfmt`: Prepare `util/persist.rs`

`rustfmt`: Run on `util/message_signing.rs`

`rustfmt`: Drop `util/macro_logger.rs` from exclusion list

`rustfmt`: Run on `util/macro_logger.rs`

`rustfmt`: Drop `util/logger.rs` from exclusion list

`rustfmt`: Run on `util/logger.rs`

`rustfmt`: Drop `util/indexed_map.rs` from exclusion list

`rustfmt`: Run on `util/indexed_map.rs`

`rustfmt`: Drop `util/fuzz_wrappers.rs` from exclusion list

`rustfmt`: Run on `util/fuzz_wrappers.rs`

`rustfmt`: Drop `util/errors.rs` from exclusion list

`rustfmt`: Run on `util/errors.rs`

`rustfmt`: Drop `util/config.rs` from exclusion list

`rustfmt`: Run on `util/config.rs`

`rustfmt`: Drop `util/byte_utils.rs` from exclusion list

`rustfmt`: Run on `util/byte_utils.rs`

`rustfmt`: Drop `util/base32.rs` from exclusion list

`rustfmt`: Run on `util/base32.rs`

`rustfmt`: Drop `util/atomic_counter.rs` from exclusion list

`rustfmt`: Run on `util/atomic_counter.rs`

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