Merge pull request #3212 from dunxen/2024-07-sensible-balances

Accuracy fixes and differentiating sources for `Balance`

Merge pull request #3234 from TheBlueMatt/2024-08-lightning-dep-inv

Swap the dep order between `lightning` and `lightning-invoice`

Merge pull request #3020 from tnull/2024-04-drop-electrsd-autodownload-for-good

Drop `electrsd`'s auto-download feature for good

Enable caching for `bitcoind`/`electrs` in CI

`rustfmt` new files added in the past few commits

The past handful of commits were mostly moving code around, so to
aid reviewers violated our `rustfmt` rules. Here we rectify that by
`rustfmt`'ing the newly-added files.

Prepare to `rustfmt` newly added files

In the next commit we'll `rustfmt` newly-added files, but before
we do so we clean up some code so that the resulting files won't be
quite as absurd. We also exclude the new `invoice_utils.rs` file,
as it needs quite substantial cleanups.

Provide the signer with a full `RawBolt11Invoice` to sign

Now that the `lightning` crate depends on the `lightning-invoice`
crate, there's no reason to have the `sign_invoice` method take raw
base32 field elements as we can now give it a real
`RawBolt11Invoice`, which we do here.

This simplifies the interface and avoids a
serialization-deserialization roundtrip when signing invoices in a
validating signer.

FIxes #3227

Swap the dep order between `lightning` and `lightning-invoice`

`lightning-invoice` previously had a dependency on the entire
`lightning` crate just because it wants to use some of the useful
types from it. This is obviously backwards and leads to some
awkwardness like the BOLT 11 invoice signing API in the `lightning`
crate taking a `[u5]` rather than a `Bolt11Invoice`.

Here we finally rectify this issue, swapping the dependency order
and making `lightning` depend on `lightning-invoice` rather than
the other way around.

This moves various utilities which were in `lightning-invoice` but
relied on `lightning` payment types to make payments to where they
belong (the `lightning` crate), but doesn't bother with integrating
them well in their new home.

Add a `lightning-types` dependency to `lightning-invoice`

`lightning-invoice` currently has a dependency on the entire
`lightning` crate just because it wants to use some of the useful
types from it. This is obviously backwards and leads to some
awkwardness like the BOLT 11 invoice signing API in the `lightning`
crate taking a `[u5]` rather than a `Bolt11Invoice`.

This takes tees us up for the final step, adding a
`lightning-types` dependency to `lightning-invoice` and using it
for imports rather than the `lightning` crate.

Use `check_added_monitors` test utility in invoice utils tests

In a coming commit, the `lightning-invoice::utils` module will move
to the `lightning` crate, causing its tests to be included in the
global lockorder tests done in that crate. This should be fine,
except that the `lightning-invoice::utils` module currently holds
the `added_monitors` lock too long causing lockorder violations.

Instead, this commit replaces the legacy monitors-added test with
the `check_added_monitors` test utility.

Move `UntrustedString` and `PrintableString` to `lightning-types`

`lightning-invoice` currently has a dependency on the entire
`lightning` crate just because it wants to use some of the useful
types from it. This is obviously backwards and leads to some
awkwardness like the BOLT 11 invoice signing API in the `lightning`
crate taking a `[u5]` rather than a `Bolt11Invoice`.

This takes one more step, moving the `UntrustedString` and
`PrintableString` types to `lightning-types`.

Move `Features` into `lightning-types`

`lightning-invoice` currently has a dependency on the entire
`lightning` crate just because it wants to use some of the useful
types from it. This is obviously backwards and leads to some
awkwardness like the BOLT 11 invoice signing API in the `lightning`
crate taking a `[u5]` rather than a `Bolt11Invoice`.

This takes one more step, moving the `Features` types from
`lightning` to `lightning-types`.

Replace usages of `Features::is_subset` and remove it

It turns out all the places we use `Features::is_subset` we could
as well be using `Features::requires_unknown_bits_from`. Further,
in the next commit `Features` will move to a different crate so any
methods which the `lightning` crate uses will need to be public. As
the `is_subset` API is prety confusing (it doesn't consider
optional/required bits, only whether the bits themselves are
strictly a subset) it'd be nice to not have to expose it, which is
enabled here.

Move `Rout{ingFees,eHint{,Hop}}` to `lightning-types`

`lightning-invoice` currently has a dependency on the entire
`lightning` crate just because it wants to use some of the useful
types from it. This is obviously backwards and leads to some
awkwardness like the BOLT 11 invoice signing API in the `lightning`
crate taking a `[u5]` rather than a `Bolt11Invoice`.

This takes one more step, moving the routing types
`lightning-invoice` uses into `lightning-types`.

Move `Payment{Hash,Preimage,Secret}` into a new crate

`lightning-invoice` currently has a dependency on the entire
`lightning` crate just because it wants to use some of the useful
types from it. This is obviously backwards and leads to some
awkwardness like the BOLT 11 invoice signing API in the `lightning`
crate taking a `[u5]` rather than a `Bolt11Invoice`.

This is the first step towards fixing that - moving the common
types we need into a new `lightning-types` crate which both can
depend on.

Since we're using a new crate and can't depend on the existing
`lightning` hex utility to implement `Display`, we also take this
opportunity to switch to the new `Display` impl macro in
`hex_conservative`.

Test claimable balance is expected for forwarded/outbound payments

Indicate source of balances

Introduce the `BalanceSource` enum to differentiate between force-close,
coop-close, and HTLCs in `Balance::ClaimableAwaitingConfirmations`.

Explain how rounded millisat values are included in overall balance

Test rounded msat balances

Include rounded msat balances in `Balance::ClaimableOnChannelClose`

If we're gonna push users towards using `Balance` to determine
their current balances, we really need to provide more information,
including msat balances.

Here we add rounded-out msat balances to the pre-close balance
information

Add tx fee information to `Balance::ClaimableOnChannelClose`

`Balance::ClaimableOnChannelClose` excludes the commitment
transaction fee, which makes it hard to use for current balance
calculation. Here we add it, setting the value to zero for inbound
channels (i.e. ones for which we don't pay the fee).

Move commitment tx fee calculation helpers to `chan_utils`

These don't really belong in `channel` as they're now used in other
parts of the codebase.

Drop `chan_utils` self-import

There's no reason to `use` a module within that module to refer to
that module...

Include an `outbound_payment` flag in `MaybeTimeoutClaimableHTLC`

When the user is fetching their current balances after forwarding a
payment (before it clears), they'll see a
`MaybePreimageClaimableHTLC` and a `MaybeTimeoutClaimableHTLC` but
if they sum up their balance using
`Balance::claimable_amount_satoshis` neither will be included.

Obviously, exactly one of the two balances should be included - one
of the two resolutions should happen in our favor. This causes our
visible balance to fluctuate up and down by the full value of any
HTLCs we're in the middle of forwarding, which is incredibly
confusing to see. If we want to stop the fluctuations, we need to
pick one of the two balances to include. The obvious candidate is
`MaybeTimeoutClaimableHTLC` as it is the lower of the two, and
represents our balance without the fee we'd receive from the
forward.

Sadly, if we always include it, we'll end up also including any
HTLCs which we've sent but which haven't yet been claimed by their
recipient, which is the wrong behavior.

Luckily, we have access to the `Option<HTLCSource>` while walking
HTLCs, which allows us to add an `outbound_payment` flag to
`MaybeTimeoutClaimableHTLC`. This allows us to only include
forwarded payments in `claimable_amount_satoshis`.

Sadly, even with this in place our balance still fluctuates by the
changes in the commitment transaction fees we have to pay during
forwarding, but addressing that is left for later.

Upgrade all github actions to `v4` variants

Fix misc `shellcheck` complaints

Have `shellcheck` check all scripts in `contrib`

Have `shellcheck` check all scripts in `ci`

Drop `electrsd` autodownload feature for good

... which requires a bunch of unnecessary dev dependencies, e.g., `zip`.

Instead we lean on the `download_bitcoind_electrs.sh` script also for
local testing.

Move CI downloading `bitcoind`/`electrsd` to dedicated shell script

... allowing it to be sourced locally before running
`lightning-transaction-sync` tests.

Merge pull request #3210 from tcharding/07-31-rm-hex-dep

Remove explicit dependency on hex-conservative

Merge pull request #3236 from TheBlueMatt/2024-08-unclonable-graph

Stop relying on a `Clone`able `NetworkGraph` ref in `DefaultRouter`

Stop relying on a `Clone`able `NetworkGraph` ref in `DefaultRouter`

While there's not really much harm in requiring a `Clone`able
reference (they almost always are), it does make our bindings
struggle a bit as they don't support multi-trait bounds (as it
would require synthesizing a new C trait, which the bindings don't
do automatically). Luckily, there's really no reason for it, and we
can just call the `DefaultMessageRouter` directly when we want to
route a message.

We've carried this patch for a while on the bindings branch, but
there's not a strong reason it can't go upstream.

Merge pull request #3214 from TheBlueMatt/2024-07-chan-by-val

Make `funding_transaction_generated` take a `ChannelId` by value

Use bitcoin::hex instead of bitcoin::hashes::hex

Use the `hex-conservative` crate directly from `bitcoin` instead of from
`hashes`. Although it makes no real difference it is slightly more clear
and more terse.

Remove explicit dependency on hex-conservative

The `hex` crate is re-exported by `rust-bitcoin` so we can get it from
there instead of explicitly depending on it. Doing so reduces the
maintenance burden and helps reduce the likelyhood of getting two
versions in the dependency graph.

Merge pull request #3220 from dunxen/2024-08-config-param-consistency

Set default ChannelHandshakeLimits::min_funding_satoshis to 1000

Set default ChannelHandshakeLimits::min_funding_satoshis to 1000

The original default value of 0 was inconsistent with the minimum requirement
of 1000 satoshis in ChannelHandshakeConfig::their_channel_reserve_proportional_millionths.

Merge pull request #3194 from tnull/2024-07-om-event-notifier

Add `Notifier` to `OnionMessenger`

Merge pull request #3215 from tnull/2024-08-protect-against-merkle-leaf-node-weakness

tx-sync: Protect against Core's Merkle leaf node weakness

Protect against Core's Merkle leaf node weakness

Bitcoin Core's Merkle tree implementation has no way to discern between
internal and leaf node entries. As a consequence it is susceptible to an
attacker injecting additional transactions by crafting 64-byte
transactions matching an inner Merkle node's hash (see
https://web.archive.org/web/20240329003521/https://bitslog.com/2018/06/09/leaf-node-weakness-in-bitcoin-merkle-tree-design/).

To protect against this (highly unlikely attack vector), we check that
the transaction isn't 64 bytes in length, and skip it otherwise.

Merge pull request #3230 from TheBlueMatt/2024-08-old-doc-tweaks

Minor doc tweaks to `MonitorHolder`

Add `Notifier` to `OnionMessenger`

Merge pull request #3231 from tcharding/08-08-rm-blockdata

Stop using `blockdata` module

Merge pull request #3232 from cooltexture1/fix-#3209

Small fix for #3209

Fix #3209

There was a issue with the ci/check-compiles.sh.
It would return a warning due to links not being enclosed in <>.
Fixed the issue by enclosing the links.

Run contrib/run-rustfmt.sh

No other changes other than those introduced by the script.

Do not use bitcoin::blockdata

The `rust-bitcoin` project is working towards making the public API
separate from the directory structure; eventually the
`bitcoin::blockdata` will go away, to make maintenance easier here stop
using the `blockdata` module.

Do not run the formatter, so as to make review easier. This patch was
created mechanically using:

search-and-replace bitcoin::blockdata bitcoin

and having defined

```bash
search-and-replace () {
        if (($# != 2))
        then
                echo "Usage: $0 <this> <that>"
                return
        fi
        local this="$1"
        local that="$2"
        for file in $(git grep -l "$this")
        do
                perl -pi -e "s/$this/$that/g" "$file"
        done
}
```

Minor doc tweaks to `MonitorHolder`

036c31c9d0b6a243fa33aa5f8d5148d2ca065617 introduced some minor doc
changes but failed to imrpove the docs that is was changing fully
which this does. Suggested by @tnull in review.

Merge pull request #3226 from TheBlueMatt/2024-08-rustfmt-contrib

Add a script to automatically `rustfmt` all required files

Force locale in `rustfmt` shell scripts that rely on sort order

Merge pull request #3219 from dunxen/2024-08-PR2989-followups

Fix remaining feedback and other nits for 2989

Merge pull request #3225 from TheBlueMatt/2024-08-3019-followups

Clean up `CommonOpenChannelFields` docs slightly

Merge pull request #3224 from TheBlueMatt/2024-08-deprecate-send_payment

Mark ChannelManager::send_payment_with_route as deprecated and take `Route` by value

Add a script to automatically `rustfmt` all required files

As we now require `rustfmt` pass on a subset of our files, its
helpful to have a script which will automatically format any
required files so that contributors don't need to think too hard
about it.

Clean up `CommonOpenChannelFields` docs slightly

Merge pull request #3019 from benthecarman/is-public

Add more information to OpenChannelRequest Event

Merge pull request #3202 from jkczyz/2024-07-blinded-path-auth-follow-up

Follow-ups to #3139

Make `funding_transaction_generated` take a `ChannelId` by value

`ChannelId` is just a 32-byte array, so there's not a lot of value
in passing it by reference to `funding_transaction_generated`,
which we fix here.

This is also nice for bindings as languages like Java can better
analyze whether the `ChannelManager` ends up with a reference to
the `ChannelId`.

Make `send_payment_with_route` take `Route` by value

Now that `ChannelManager::send_payment_with_route` is deprecated,
we don't care too much about making it as effecient as possible, so
there's not much cost to making it take `Route` by value. This
avoids bindings being unsure if the by-reference `Route` passed
needs to outlive the `ChannelManager` itself or if it only needs to
outlive the method call, creating some call overhead by forcing a
`Route::clone`, but avoiding a memory leak.

Mark `ChannelManager::send_payment_with_route` as deprecated

We probably should have done this long ago a release or two after
adding `send_payment`, but we didn't and the second best time is
now.

`send_payment_with_route` has particularly hard to use retry
semantics that make it unsuitable for real use. Once we get the
last of our users off of it, we'll want to remove it (or at least
mark it test-only), but we should start by deprecating it.

Fix remaining feedback and other nits for 2989

Merge pull request #3216 from tnull/2024-08-pin-tokio-in-ci

Pin `tokio` in CI to fix MSRV

Pin `tokio` in CI to fix MSRV

The recently released `tokio` 1.39 bumped their MSRV to rustc 1.70.
Here, we pin the `tokio` version to 1.38 for users that require to
maintain our MSRV of rustc 1.63.

Replace use of OffersContext::Unknown with None

Now that ChannelManager uses a known OffersContext when creating blinded
paths, OffersContext::Unknown is no longer needed. Remove it and update
OffersMessageHandler to us an Option, which is more idiomatic for
signifying whether a message was delivered with or without an
OffersContext.

Include payment hash in Bolt12Invoice reply path

Instead of using OffersContext::Unknown for the Bolt12Invoice reply path
use OffersContext::InboundPayment to include the payment hash.
OffersContext::Unknown will be removed in another commit.

Include payment hash when logging invoice handling

Include payment hash when logging InvoiceError

By including the payment hash from the invoice in an onion message's
reply path, it can be used when logging errors as additional context.

Use BlindedPath::new_for_payment in fuzz tests

When creating a Bolt12Invoice in fuzz tests, use
BlindedPath::new_for_payment instead of BlindedPath::new_for_message.
This way PaymentContext is used instead of MessageContext, as is more
realistic though should not affect the test. This allows us to remove
OffersContext::Unknown.

Use different iv_bytes for blinded path metadata

Best practice is to use different IV bytes for different contexts.
Update Offer and Refund metadata computation to use different IV bytes
when the metadata is included in a blinded path. For invoice requests,
the metatdata will always be in the blinded path, so it remains the
same.

Delay adding iv_bytes to MetadataMaterial::hmac

In an upcoming commit, the iv_bytes used in MetadataMaterial will vary
depending on when whether a blinded path is included in the
corresponding message. Delay adding into MetadataMaterial::hmac as
otherwise the HmacEngine would need to be re-initialized using an
ExpandedKey, which won't be readily available.

Result from Bolt12Invoice::verify_using_payer_data

Use a Result return type instead of a bool when verifying a
Bolt12Invoice. This way ignoring the result will produce a compiler
warning.

Bolt12Invoice::is_for_refund_without_paths tests

Merge pull request #3189 from tnull/2024-07-rustfmt-lightning-net-tokio

 `rustfmt`: Run on `lightning-net-tokio`

Merge pull request #3171 from jkczyz/2024-07-propagate-error

Include routing failures in `Bolt12PaymentError`

Add more information to OpenChannelRequest Event

Merge pull request #3203 from lexe-app/pr/description-as-inner

lightning-invoice: Add `Description::as_inner`

lightning-invoice: Add `Description::as_inner`

Merge pull request #3196 from TheBlueMatt/2024-07-monitor-ordering

Ensure `ChannelMonitorUpdate`s are ordered with full monitor writes

Merge pull request #3199 from TheBlueMatt/2024-07-funding-check

Enforce segwit inputs for all "safe" funding transactions

Merge pull request #3139 from jkczyz/2024-06-blinded-path-auth

Authenticate use of offer blinded paths

Merge pull request #3197 from TheBlueMatt/2024-07-stupid-locked-connect

[block-sync] Don't hold client-cache lock while connecting

Enforce segwit inputs for all "safe" funding transactions

8403755a2a524beb9f6c8951f51dd60f7c54c912 introduced a separate path
for funding a channel without a full funding transaction, relying
on users to manually broadcast the funding tx. One of the major
things that makes this path less safe is that for other paths we're
supposed to validate that all inputs have witnesses, making the
funding transaction (likely) txid-non-malleable.

However, in one of several rewrites of that commit the funding tx
tests ended up getting elided in some call paths, which is fixed
here.

Add pending changelog for BOLT12 authentication

Rename Bolt12Invoice::verify

Elide nonce from payer metadata

InvoiceRequest and Refund have payer_metadata which consists of an
encrypted payment id and a nonce used to derive its signing keys and
authenticate any corresponding invoices. Now that the blinded paths
include this data in their OffersContext, remove the nonce as it is now
redundant. Keep the encrypted payment id as some data is needed in the
payer metadata according to the spec. This saves space and prevents
de-anonymization attacks as along as the nonce isn't revealed.

Correct docs

Include OffersContext in Event::InvoiceReceived

When authenticating that an invoice is for a valid invoice request, the
payer metadata is needed. Some of this data will be removed in the next
commit and instead be included in the message context of the request's
reply path. Add this data to Event::InvoiceReceived so that asynchronous
invoice handling can verify properly.

Authenticate Bolt12Invoice using BlindedPath data

When a Bolt12Invoice is handled with an OfferContext, use both the
containing payment_id and nonce to verify that it is for a pending
outbound payment. Previously, the nonce the payment_id were taken from
the payer_metadata and the latter was compared against the payment_id
in the OfferContext. The payer_metadata thus no longer needs to include
either when a blinded path is used. However, some payer_metadata will
still be needed as per the spec.

Add nonce to OffersContext::OutboundPayment

To authenticate that a Bolt12Invoice is for a valid InvoiceRequest or
Refund, include the nonce from the payer_metadata in the InvoiceRequest
reply path or Refund::paths, respectively. This can be used to prevent
de-anonymization attacks where an attacker sends invoices using
self-constructed paths to nodes near the blinded paths' introduction
nodes.

Add Bolt12Invoice::verify_using_payer_data

Invoices are authenticated by checking the payer metadata in the
corresponding invoice request or refund. For all invoices requests and
for refunds using blinded paths, this will be the encrypted payment id
and a 128-bit nonce. Allows checking the unencrypted payment id and
nonce explicitly instead of the payer metadata. This will be used by an
upcoming change that includes the payment id and nonce in the invoice
request's reply path and the refund's blinded paths instead of
completely in the payer metadata, which mitigates de-anonymization
attacks.

Pass Nonce directly to RefundBuilder

When using RefundBuilder::deriving_payer_id, the nonce generated needs
to be the same one included in any RefundBuilder::paths. This is because
the nonce is used along with the refund TLVs to derive a payer id and
will soon be used to authenticate any invoices.

Pass Nonce directly to InvoiceRequestBuilder

When using InvoiceRequestBuilder::deriving_payer_id, the nonce generated
needs to be the same one included in any reply path. This is because the
nonce is used along with the invoice request TLVs to derive a payer id.
While this data is also included in the payer_metadata, including it in
the blinded path would allow reducing the amount of data needed there to
just enough to provide entropy (i.e., 16 bytes). This is more important
for Refund because it can be transmitted via a QR code. But using the
same payer_metadata structure for both InvoiceRequest and Refund would
be beneficial to avoid more code.

Add failure tests for offer message authentication

Don't send InvoiceError on failed authentication

When an invoice or invoice request cannot be authenticated from an
OffersContext, simply do not respond instead of sending an InvoiceError
message. According to BOLT4, messages sent over a blinded path not
intended for its use MUST be ignored.

Authenticate Bolt12Invoice using OfferContext

When a Bolt12Invoice is handled with an OfferContext, use the
containing payment_id to verify that it is for a pending outbound
payment. Only invoices for refunds without any blinded paths can be
verified without an OfferContext.

Add docs to Metadata::without_keys

Rename InvoiceRequest::verify

Elide metadata from Offer with derived keys

When an Offer uses blinded paths, its metadata consists of a nonce used
to derive its signing keys. Now that the blinded paths contain this
nonce, elide the metadata as it is now redundant. This saves space and
also makes it impossible to derive the signing keys if an invoice
request is received with the incorrect nonce. The nonce shouldn't be
revealed in this case either to prevent de-anonymization attacks.

Authenticate InvoiceRequest using OfferContext

When an InvoiceRequest is handled with an OfferContext, use the
containing nonce to verify that it is for a valid Offer. Otherwise, fall
back to using Offer::metadata, which also contains the nonce. The latter
is useful for supporting offers without blinded paths or those created
prior to including an OffersContext in their blinded paths.