rust-lightning

Allow counterparty tx_abort before handling initial commitment signed

Upon processing the counterparty's initial `commitment_signed` for a
splice, we queue a monitor update with the new commitment transactions
spending the new funding transaction. Once handled, funding transaction
signatures can be exchanged and we must start monitoring the chain for a
possible commitment transaction broadcast spending the new funding
transaction. Aborting the splice negotiation then would therefore be
unsafe, hence why we currently force close in such cases. However, there
is no reason to force close prior to receiving their initial
`commitment_signed`, as this would imply the funding transaction
signatures have yet to be exchanged, thus making a commitment broadcast
spending said transaction impossible allowing us to abort the splice
negotiation safely.

Support capturing `tx_abort` send within channel reestablish tests

Copy async docs from async bump transaction objects to sync ones

When we added the async traits and wrapper structs for the
transaction-bumping logic, we didn't bother copying the
documentation to the sync wrappers as we figured the links
sufficed. Sadly, however, this means that our bindings logic will
have no docs but a broken link to an async object that doesn't
exist.

Instead, here, we copy the docs from the async objects to the sync
ones, at least leaving behind a comment noting that both need
updating whenever one gets updated.

We also fix one bogus link in `Wallet`'s docs.

Provide the same docs in `ChangeDestinationSource` as the sync one

Import `ChannelTransactionParameters` from its real path

Rather than importing `ChannelTransactionParameters` via the `use`
in `lightning::sign`, import it via its real path in
`lightning::sign::ecdsa`. This makes reading the code (incredibly
marginally) simpler, but also makes the bindings generator happy.

Mark async traits and structs no-export

We do not currently support async traits or methods in our
home-grown bindings logic, so here mark them no-export.

Require the high-level sign traits instead of `SignFn` in bindings

Methods like `StaticInvoice::sign` have their own signing-function
traits, so there's no actual need to use `SignFn` directly, its
just a useful generic wrapper. Sadly, because it uses `AsRef`
bounds, which aren't really practical to map in bindings, we can't
really expose it directly in bindings. Because there's an
alternative and its tricky to expose, we simply mark it no-export
here.

Upload CI generated fuzz corpus coverage to codecov

Because each CI job runs on a fresh runner and can't share data
between jobs. We rely on Github Actions upload-artifact and
download-artifact to share the CI generated fuzz corpus, then
replay them in the `contrib/generate_fuzz_coverage.sh` script
to generate the coverage report.

Add draft release notes for 0.2

Add missing documentation for sigs-ready event to `splice_channel`

`ChannelManager::splice_channel` initiates a splice which
ultimately generates a series of events. The most important of
which, `FundingTransactionReadyForSigning` (which must always be
handled, unlike the others), was not documented.

Here we mention the event generation.

Drop useless `vec!` macro calls

`clippy` now complains about more instances of useless `vec!`s, so
we drop them here.

Add `readme` fields to most crates' `Cargo.toml`s

`crates.io` will show any README/README.md which apears in the
crate which is uploaded, but it doesn't consider the README from
the workspace, only if its in the crate itself.

Instead, add the `readme` field so that the workspace README also
gets uploaded.

Remove `Send + Sync` bounds when `no-std`

Add fmt::Debug implementation for FundedChannel

To aid with debugging in tests.

Don't broadcast manual-funded chan closing txn on HTLC timeouts

In 6c5ef049b8d0ec174d7368d48b7b429efffb4a61 we prevented broadcast
of the commitment transactions if the funding transaction has not
yet appeared on-chain for manual-broadcast channels to avoid
spurious bumps or unbroadcastable transactions. However, we missed
the case where a channel is closed due to HTLCs timing out.

Here we fix that by doing the same broadcast-gating when
automatically force-closing a channel due to HTLC timeouts.

Implement Holder HTLC claim chunking for 0FC channels

Otherwise, we could hit the max 10_000vB size limit on V3 transactions
(BIP 431 rule 4).

Also introduce a `max_tx_weight` parameter to `select_confirmed_utxos`.
This constraint makes sure anchor and HTLC transactions in 0FC channels
satisfy the `TRUC_MAX_WEIGHT` and the `TRUC_CHILD_MAX_WEIGHT`
maximums.

Expand the coin-selection algorithm provided for any `T: WalletSource`
to satisfy this new constraint.

Pin `proptest` again to fix CI

.. `proptest` 1.9.0 was just released, now requiring rustc 1.82.

Bump `electrsd` to 0.36.1

We bump our `electrsd` dependency to the latest version, allowing us to
remove the `home` pin.

Doc and comment fixes from #4167

This fixes incorrect docs and comments introduced by
e95ebf8b9a3d43108176e21c8b4c6bd82f3aaabf and
491b6949fec743e34c74c3a472cf0a2cb83d7ab3

Only pause read in `PeerManager` `send_data` not `read_event`

We recently ran into a race condition on macOS where `read_event`
would return `Ok(true)` (implying reads should be paused) due to
many queued outbound messages but before the caller was able to
set the read-pause flag, the `send_data` calls to flush the
buffered messages completed. Thus, when the `read_event` caller got
scheduled again, the buffer was empty and we should be reading, but
it is finally processing the read-pause flag and we end up hanging,
unwilling to read messages and unable to learn that we should start
reading again as there are no messages to `send_data` for.

This should be fairly rare, but not unheard of - the `pause_read`
flag in `read_event` is calculated before handling the last
message, so there's some time between when its calculated and when
its returned. However, that has to race with multiple calls to
`send_data` to send all the pending messages, which all have to
complete before the `read_event` return happens. We've (as far as I
recall) never hit this in prod, but a benchmark HTLC-flood test
managed to hit it somewhat reliably within a few minutes on macOS
and when a synthetic few-ms sleep was added to each message
handling call.

Ultimately this is an issue with the API - we pause reads via a
returned flag but unpause them via a called method, creating two
independent "stream"s of pause/unpauses which can get out of sync.
Thus, here, we stick to a single "stream" of pause-read events from
`PeerManager` to user code via `send_data` calls, dropping the
read-pause flag return from `read_event` entirely.

Technically this adds risk that someone can flood us with enough
messages fast enough to bloat our outbound buffer for a peer before
`PeerManager::process_events` gets called and can flush the pause
flag via `read_event` calls to all descriptors. This isn't ideal
but it should still be relatively hard to do as `process_events`
calls are pretty quick and should be triggered immediately after
each `read_event` call completes.

Assure BroadcasterInterface packages of len > 1 are child-with-parents

Implementations MUST NOT assume any topological order on the
transactions.

While Bitcoin Core v29+ `submitpackage` RPC allows packages of length 1
to be submitted via `submitpackage`, it still requires any package
submitted there to be a `child-with-parents` package.

So we remove the possibility that a batch of transactions passed to
a `BroadcasterInterface` implementation contains unrelated transactions,
or multiple children.

Properly handle funding key rotation during splices

When splicing, we're required by protocol to retain all the
existing keys material except the funding key which we're allowed
to rotate. In the original implementation we acknowledged that but
figured we'd stick with a single `pubkey` method in the
`ChannelSigner` anyway cause adding a specific method for it is
annoying.

Sadly, this was ultimately broken - in `FundingScope::for_splice`,
we called the signer's `new_pubkeys` method (renamed from `pubkeys`
after splicing initially landed), replacing all of the public keys
the `Channel` would use rather than just the funding key. This can
result in commitment signature mismatches if the signer changes any
keys aside from the funding one.

`InMemorySigner` did not do so, however, so we didn't notice the
bug. Luckily-ish, in 189b8ac4a7674bbf623f903dcd144c9d1a24a128 we
started generating a fresh `remote_key` when splicing (at least
when upgrading from 0.1 to 0.2 or when setting `KeysManager` to use
v1 `remote_key` derivation). This breaks splicing cause we can't
communicate the new `remote_key` to the counterparty during the
splicing handshake.

Ultimately this bug is because the API we had didn't communicate to
the signer that we weren't allowed to change anything except the
funding key, and allowed returning a `ChannelPublicKeys` which
would break the channel.

Here we fix this by renaming `new_pubkeys` `pubkeys` again
(partially reverting 9d291e01f98417c2f6b2d4321bbf806464c424a6 but
keeping the changed requirements that `pubkeys` only be called
once) and adding a new `ChannelSigner:new_funding_pubkey` method
specifically for splicing.

We also update `channel.rs` to correctly fetch the new funding
pubkey before sending `splice_init`, storing it in the
`PendingFunding` untl we build a `FundingScope`.

Fix `generated_by_local` arg to build commmitment during splicing

`build_commitment_transaction`'s fifth argument is supposed to be
whether we're the one generating the commitment (i.e. because we're
signing rather than validating the commitment). During splicing,
this doesn't matter because there should be no async HTLC
addition/removal happening so the commitment generated wil be the
same in either case, but its still good to pass the correct bool.

Enforce that `ChanelSigner::pubkeys` is only called once

In the previous commit we partially reverted
9d291e01f98417c2f6b2d4321bbf806464c424a6 renaming
`ChannelSigner::new_pubkeys` to `pubkeys` again, but we still don't
want to go back to requiring that `pubkeys` return the same
contents on each call. Thus, here, we add test logic to check that
`pubkeys` isn't called more than once.

Fix panic when deserializing `Duration`

`Duration::new` adds any nanoseconds in excess of a second to the
second part. This can overflow, however, panicking. In 0.2 we
introduced a few further cases where we store `Duration`s,
specifically some when handling network messages.

Sadly, that introduced a remotely-triggerable crash where someone
can send us, for example, a malicious blinded path context which
can cause us to panic.

Found by the `onion_message` fuzzer