]> git.bitcoin.ninja Git - rust-lightning/log
rust-lightning
13 months agoRemove caching of holder HTLC signatures
Wilmer Paulino [Fri, 13 Oct 2023 20:58:59 +0000 (13:58 -0700)]
Remove caching of holder HTLC signatures

Since we want our holder HTLC signatures to be randomly generated and
not reused, our existing caches are useless now, so we opt to remove
them.

13 months agoUse sign_holder_htlc_transaction to sign non-anchors holder HTLCs
Wilmer Paulino [Fri, 13 Oct 2023 20:52:23 +0000 (13:52 -0700)]
Use sign_holder_htlc_transaction to sign non-anchors holder HTLCs

We want to ensure we use fresh random signatures to prevent certain
classes of transaction replacement attacks at the bitcoin P2P layer.
This was already covered for commitment transactions and zero fee holder
HTLC transactions, but was missing for holder HTLC transactions on
non-anchors channels.

We can easily do this by reusing the existing
`EcdsaChannelSigner::sign_holder_htlc_transaction` method and
circumventing the existing `holder_htlc_sigs/prev_holder_htlc_sigs`
caches, which will be removed in a later commit anyway.

13 months agoProvide missing derivation parameters to OnchainTxHandler
Wilmer Paulino [Fri, 13 Oct 2023 20:47:45 +0000 (13:47 -0700)]
Provide missing derivation parameters to OnchainTxHandler

`OnchainTxHandler` will need to construct `HTLCDescriptor`s for holder
HTLCs, but it did not have access to all of the derivation parameters
that need to be provided.

13 months agoSupport signing non-anchors HTLCs with HTLCDescriptor
Wilmer Paulino [Fri, 13 Oct 2023 20:49:50 +0000 (13:49 -0700)]
Support signing non-anchors HTLCs with HTLCDescriptor

We plan to use `EcdsaChannelSigner::sign_holder_htlc_transaction` to
also sign holder HTLC transactions on non-anchor outputs channels.
`HTLCDescriptor` was only used in an anchor outputs context, so a few
things needed changing, mostly to handle the different scripts and
feerate.

13 months agoMerge pull request #2664 from TheBlueMatt/2023-10-ci-disk-usage
Matt Corallo [Mon, 16 Oct 2023 14:56:34 +0000 (14:56 +0000)]
Merge pull request #2664 from TheBlueMatt/2023-10-ci-disk-usage

Reduce disk usage in CI

13 months agoMerge pull request #2625 from tnull/2023-09-moar-tests-n-fixes
Matt Corallo [Sun, 15 Oct 2023 20:18:56 +0000 (20:18 +0000)]
Merge pull request #2625 from tnull/2023-09-moar-tests-n-fixes

Improve test coverage of #2575 router fixes

13 months agoReduce disk usage in CI 2023-10-ci-disk-usage
Matt Corallo [Sat, 14 Oct 2023 18:41:34 +0000 (18:41 +0000)]
Reduce disk usage in CI

Recently github appears to have reduced the available free disk
space in actions runs, causing CI to fail with out of space errors.
Here we simply run `cargo clean` a few times in CI to reduce our
disk usage somewhat.

13 months agoMerge pull request #2655 from TheBlueMatt/2023-10-no-test-net
Matt Corallo [Sat, 14 Oct 2023 02:10:31 +0000 (02:10 +0000)]
Merge pull request #2655 from TheBlueMatt/2023-10-no-test-net

Replace `lightning-block-sync` test that depended on `foo.com`

13 months agoMerge pull request #2639 from vladimirfomene/fix-final-incorrect-cltv
valentinewallace [Fri, 13 Oct 2023 21:12:11 +0000 (17:12 -0400)]
Merge pull request #2639 from vladimirfomene/fix-final-incorrect-cltv

Fix final incorrect cltv

13 months agofix: use the update_add_htlc's cltv_expiry for comparison
Vladimir Fomene [Mon, 2 Oct 2023 19:05:42 +0000 (22:05 +0300)]
fix: use the update_add_htlc's cltv_expiry for comparison

As noted in BOLT 4, we should be using the update_add_htlc's cltv_expiry,
not the CLTV expiry set by the sender in the onion for this comparison.
See here: https://github.com/lightning/bolts/blob/4dcc377209509b13cf89a4b91fde7d478f5b46d8/04-onion-routing.md?plain=1#L334

13 months agoReplace `lightning-block-sync` test that depended on `foo.com` 2023-10-no-test-net
Matt Corallo [Mon, 9 Oct 2023 03:24:54 +0000 (03:24 +0000)]
Replace `lightning-block-sync` test that depended on `foo.com`

Our tests should generally not rely on internet access, and should
not rely on the behavior of any given remote server. However, one
of the `endpoint_tests` in `lightning-block-sync::http` relied on
`foo.com` resolving to a single socket address, which both might
change in the future and makes our tests fail without internet.

13 months agoMerge pull request #2650 from TheBlueMatt/2023-10-make-clippy-shut-up
Matt Corallo [Thu, 12 Oct 2023 20:44:46 +0000 (20:44 +0000)]
Merge pull request #2650 from TheBlueMatt/2023-10-make-clippy-shut-up

Make clippy shut up about `PartialOrd` and `Ord` both impl'd

13 months agoMerge pull request #2657 from Fedeparma74/fix-close-channel-deadlock
Matt Corallo [Thu, 12 Oct 2023 18:16:02 +0000 (18:16 +0000)]
Merge pull request #2657 from Fedeparma74/fix-close-channel-deadlock

Fix deadlock when closing an unavailable channel

13 months agoFix deadlock when closing an unavailable channel
Fedeparma74 [Wed, 11 Oct 2023 08:50:28 +0000 (10:50 +0200)]
Fix deadlock when closing an unavailable channel

13 months agoMerge pull request #2652 from tnull/2023-10-deref-achannelmanager
Elias Rohrer [Sun, 8 Oct 2023 07:11:19 +0000 (09:11 +0200)]
Merge pull request #2652 from tnull/2023-10-deref-achannelmanager

13 months agoMerge pull request #2651 from G8XSU/rm-same-feerate-log
valentinewallace [Fri, 6 Oct 2023 23:18:32 +0000 (13:18 -1000)]
Merge pull request #2651 from G8XSU/rm-same-feerate-log

Remove log for not changing feerate when it was equal

13 months agoMerge pull request #2649 from benthecarman/display-outpoint
Matt Corallo [Fri, 6 Oct 2023 19:00:01 +0000 (19:00 +0000)]
Merge pull request #2649 from benthecarman/display-outpoint

Impl Display for Outpoint

13 months agoHave methods take `AChannelManager` as `Deref::Target`
Elias Rohrer [Fri, 6 Oct 2023 17:30:45 +0000 (07:30 -1000)]
Have methods take `AChannelManager` as `Deref::Target`

13 months agoImpl Display for Outpoint
benthecarman [Thu, 5 Oct 2023 19:38:49 +0000 (14:38 -0500)]
Impl Display for Outpoint

13 months agoRemove log for not changing feerate when it was equal
Gursharan Singh [Fri, 6 Oct 2023 01:18:10 +0000 (15:18 -1000)]
Remove log for not changing feerate when it was equal

Log is not required in this case and creates unnecessary log lines at
trace level.

13 months agoMake clippy shut up about `PartialOrd` and `Ord` both impl'd 2023-10-make-clippy-shut-up
Matt Corallo [Thu, 5 Oct 2023 23:57:08 +0000 (23:57 +0000)]
Make clippy shut up about `PartialOrd` and `Ord` both impl'd

Clippy gets mad that we have an implementation of `ParialOrd` and
`Ord` separately, even though both are identical. Making
`ParitalOrd` call `Ord` makes clippy shut up.

13 months agoMerge pull request #2640 from sr-gi/20231003-expose-from-be-bytes
Matt Corallo [Thu, 5 Oct 2023 23:45:59 +0000 (23:45 +0000)]
Merge pull request #2640 from sr-gi/20231003-expose-from-be-bytes

Makes Features::from_be_bytes public

13 months agoMakes Features::from_be_bytes public
Sergi Delgado Segura [Tue, 3 Oct 2023 12:46:18 +0000 (08:46 -0400)]
Makes Features::from_be_bytes public

Downstream projects building Feature<T> are most likely doing so with a
big-endian byte array, however only `from_le_bytes` is exposed.

13 months agoMerge pull request #2645 from TheBlueMatt/2023-10-changelog-dates v0.0.117
Matt Corallo [Wed, 4 Oct 2023 02:16:04 +0000 (02:16 +0000)]
Merge pull request #2645 from TheBlueMatt/2023-10-changelog-dates

Set the timestamp for and provide diff stats for 0.0.117 CHANGELOG

13 months agoSet the timestamp for and provide diff stats for 0.0.117 CHANGELOG 2023-10-changelog-dates
Matt Corallo [Tue, 3 Oct 2023 23:56:58 +0000 (23:56 +0000)]
Set the timestamp for and provide diff stats for 0.0.117 CHANGELOG

13 months agoMerge pull request #2620 from TheBlueMatt/2023-09-cut-0.0.117
Matt Corallo [Tue, 3 Oct 2023 23:16:59 +0000 (23:16 +0000)]
Merge pull request #2620 from TheBlueMatt/2023-09-cut-0.0.117

Cut 0.0.117

13 months agoBump crate versions to 0.0.117/invoice 0.25 2023-09-cut-0.0.117
Matt Corallo [Tue, 3 Oct 2023 17:15:45 +0000 (17:15 +0000)]
Bump crate versions to 0.0.117/invoice 0.25

13 months agoAdd 0.0.117 release notes
Matt Corallo [Fri, 29 Sep 2023 02:50:27 +0000 (02:50 +0000)]
Add 0.0.117 release notes

13 months agoMerge pull request #2631 from TheBlueMatt/2023-09-pm-no-refs-reqd
Matt Corallo [Tue, 3 Oct 2023 17:12:19 +0000 (17:12 +0000)]
Merge pull request #2631 from TheBlueMatt/2023-09-pm-no-refs-reqd

Fix `Simple*PeerManager` to not require refs to the `UtxoLookup`

13 months agoMerge pull request #2634 from TheBlueMatt/2023-09-claimable-unwrap
Matt Corallo [Tue, 3 Oct 2023 17:12:08 +0000 (17:12 +0000)]
Merge pull request #2634 from TheBlueMatt/2023-09-claimable-unwrap

Avoid unwrap'ing `channel_parameters` in to_counterparty signing

13 months agoClean up at least some lifetimes on `SimpleRefPeerManager` 2023-09-pm-no-refs-reqd
Matt Corallo [Tue, 3 Oct 2023 03:52:53 +0000 (03:52 +0000)]
Clean up at least some lifetimes on `SimpleRefPeerManager`

Rather than simply a, b, c, d...we at least use names for a few
things, also splitting the reused 'f lifetime.

13 months agoNote when we're allowed to unwrap channel parameters in docs 2023-09-claimable-unwrap
Matt Corallo [Tue, 3 Oct 2023 04:32:40 +0000 (04:32 +0000)]
Note when we're allowed to unwrap channel parameters in docs

This further documents the parameter-fetching utilities in
`InMemorySigner` to hopefully make them more robust against future
spurious `unwrap`s.

13 months agoMake `InMemorySigner` parameter-fetching utilities return `Option`s
Matt Corallo [Tue, 3 Oct 2023 04:12:28 +0000 (04:12 +0000)]
Make `InMemorySigner` parameter-fetching utilities return `Option`s

In the previous commit we fixed a bug where we were spuriously
(indirectly) `unwrap`ing the `channel_parameters` in
`InMemorySigner`. Here we make such bugs much less likely in the
future by having the utilities which do the `unwrap`ing internally
return `Option`s instead.

This makes the `unwrap`s clear at the callsite.

13 months agoMerge pull request #2595 from TheBlueMatt/2023-09-117-bindings-part1
Matt Corallo [Tue, 3 Oct 2023 03:56:46 +0000 (03:56 +0000)]
Merge pull request #2595 from TheBlueMatt/2023-09-117-bindings-part1

Various cleanups to make bindings generation simpler

13 months agoMerge pull request #2632 from TheBlueMatt/2023-09-writeable-rwlock
Matt Corallo [Tue, 3 Oct 2023 03:45:03 +0000 (03:45 +0000)]
Merge pull request #2632 from TheBlueMatt/2023-09-writeable-rwlock

Implement Readable/Writeable for RwLock wrappers

13 months agoMerge pull request #2633 from TheBlueMatt/2023-09-expose-onion-parse
Jeffrey Czyz [Tue, 3 Oct 2023 01:16:13 +0000 (20:16 -0500)]
Merge pull request #2633 from TheBlueMatt/2023-09-expose-onion-parse

Expose `parse_onion_address` publicly in `no-std`

13 months agoAvoid unwrap'ing `channel_parameters` in to_counterparty signing
Matt Corallo [Sun, 1 Oct 2023 00:15:22 +0000 (00:15 +0000)]
Avoid unwrap'ing `channel_parameters` in to_counterparty signing

Previously, `StaticPaymentOutputDescriptor`s did not include
`channel_parameters` for the signer. As a result, when going to
spend old `StaticPaymentOutputDescriptor`s,
`InMemorySigner::sign_counterparty_payment_input` may be called
with `channel_parameters` set to `None`. This should be fine, but
in fa2a2efef47b5da48ac7a239f3d8a835a7f28164 we started relying on
it (indirectly via `channel_features`) for signing. This caused an
`unwrap` when spending old output descriptors.

This is fixed here by simply avoiding the unwrap and assuming old
`StaticPaymentOutputDescriptor`s represent non-anchor channels.

13 months agoRemove unused `mem::drop` which drops a reference 2023-09-117-bindings-part1
Matt Corallo [Sun, 1 Oct 2023 00:00:37 +0000 (00:00 +0000)]
Remove unused `mem::drop` which drops a reference

13 months agoFix new unused variable warning in `test_utils.rs`
Matt Corallo [Sat, 30 Sep 2023 17:47:45 +0000 (17:47 +0000)]
Fix new unused variable warning in `test_utils.rs`

13 months agoUse `crate::prelude::*` to load `Vec`, rather than `crate::Vec`
Matt Corallo [Thu, 28 Sep 2023 02:45:00 +0000 (02:45 +0000)]
Use `crate::prelude::*` to load `Vec`, rather than `crate::Vec`

This is kinda dumb, but the bindings get confused when referring
to `Vec` absolutely in a `use` statement, and there's no reason not
to load our prelude everywhere.

13 months agoDrop various bounds on types passed to `MonitorUpdatingPersister`
Matt Corallo [Wed, 27 Sep 2023 23:02:50 +0000 (23:02 +0000)]
Drop various bounds on types passed to `MonitorUpdatingPersister`

The new `MonitorUpdatingPersister` has a few redundant type bounds
(re-specified on functions after having been specified on the
struct itself), which we remove here.

Further, it requires a `Deref<FeeEstimator>` which is `Clone`able.
This is generally fine in rust, but annoying in bindings, so we
simply elide it in favor if a `&Deref<FeeEstimator>`.

13 months agoMake `create_onion_message` a freestanding function
Matt Corallo [Mon, 25 Sep 2023 22:10:27 +0000 (22:10 +0000)]
Make `create_onion_message` a freestanding function

The new `create_onion_message` function in `OnionMessenger` is hard
to handle - it has various generic requirements indirectly via the
struct, but they're not bounded by any of the method parameters.
Thus, you can't simply call `OnionMessenger::create_onion_message`,
as various bounds are not specified.

Instead, we move it to a freestanding function so that it can be
called directly without explicitly setting bounds.

13 months agoMark `AChannelManager` no-export
Matt Corallo [Mon, 25 Sep 2023 22:14:04 +0000 (22:14 +0000)]
Mark `AChannelManager` no-export

The trait itself has no purpose for bindings, as all structs are
concretized anyway. Further, the bindings have specific handling
for generic bounding traits like this.

13 months agoMark `SpendableOutputDescriptor::to_psbt_input` as no-export
Matt Corallo [Sun, 24 Sep 2023 23:41:10 +0000 (23:41 +0000)]
Mark `SpendableOutputDescriptor::to_psbt_input` as no-export

Its honestly likely not all that useful as its not materially
interoperable with other PSBT libraries. Instead, users should
simply fetch the full PSBT and use the inputs from it as they see
fit.

13 months agoDrop unnecessary crate:: prefix when accessing `bitcoin` in macro
Matt Corallo [Sat, 23 Sep 2023 23:34:33 +0000 (23:34 +0000)]
Drop unnecessary crate:: prefix when accessing `bitcoin` in macro

Unexported macros don't need to use the `$crate` prefix.

13 months agoAvoid ref to a `Vec` when accessing custom onion TLVs
Matt Corallo [Fri, 22 Sep 2023 01:44:57 +0000 (01:44 +0000)]
Avoid ref to a `Vec` when accessing custom onion TLVs

The bindings can't easily return a reference to a `Vec`, so we
instead split the implementation into vec/slice depending on the
`c_bindings` flag.

13 months agoAvoid blanket impls in bindings builds
Matt Corallo [Mon, 25 Sep 2023 01:05:45 +0000 (01:05 +0000)]
Avoid blanket impls in bindings builds

The C bindings implements `Deref` for all traits, so having a
blanket `Deref` implementation ends up conflicting with this.

13 months agoSimplify score bounding with a unified type
Matt Corallo [Sun, 24 Sep 2023 23:59:13 +0000 (23:59 +0000)]
Simplify score bounding with a unified type

In a few places we require a unified scorer, which implements both
`ScoreLookUp` and `ScoreUpdate`. Rather than double-bounding (which
the bindings generator can't handle directly), we use a top-level
`Score` trait which requires both and is implemented for all
implementers of both supertraits.

13 months agoRemove unnecessary bounds in scoring
Matt Corallo [Thu, 21 Sep 2023 22:54:14 +0000 (22:54 +0000)]
Remove unnecessary bounds in scoring

In our scoring logic we have a handful of unnecessary bounds,
leading to extra diff in the bindings branch when those bounds have
to be removed as well as a few cases where bindings generation
simply gets confused.

Here we remove a number of bounds across the scoring traits and
impls, cleaning things up and simplifying bindings changes.

13 months agoUse `Default::default()` to construct `()` as score-updating param
Matt Corallo [Thu, 21 Sep 2023 22:22:18 +0000 (22:22 +0000)]
Use `Default::default()` to construct `()` as score-updating param

In 6b0d94a3029f74de3a7542cbba0d48c2f7e5866b we switched most tests
to `Default::default()` for scoring parameters passed to
route-fetching. Here we do the same for the scoring parameters when
passed to score-updating.

13 months agoExpose `parse_onion_address` publicly in `no-std` 2023-09-expose-onion-parse
Matt Corallo [Sat, 30 Sep 2023 18:05:10 +0000 (18:05 +0000)]
Expose `parse_onion_address` publicly in `no-std`

The reason for having a separate `parse_onion_address` from
`FromStr` is to have an onion parsing function in `no-std`, but
when we added it we forgot to make it public. We do this here, as
well as fix a few compilation warnings in `no-std`.

13 months agoSwitch `Simple*ChannelManager` locks around `Score` to `RwLock` 2023-09-writeable-rwlock
Matt Corallo [Sat, 30 Sep 2023 17:40:42 +0000 (17:40 +0000)]
Switch `Simple*ChannelManager` locks around `Score` to `RwLock`

This switches the locks used around `ProbabilisticScorer` in
`Simple*ChannelManager` type aliases to `RwLock`.

13 months agoFix `Simple*PeerManager` to not require refs to the `UtxoLookup`
Matt Corallo [Sat, 30 Sep 2023 17:32:28 +0000 (17:32 +0000)]
Fix `Simple*PeerManager` to not require refs to the `UtxoLookup`

`UtxoLookup` doesn't strictly need to be referenced from the
`PeerManager`, and in fact the new `GossipVerifier` in
`lightning-block-sync` requires itself to be owned by the
`PeerManager` (for circular type reasons).

This allows us to use `lightning-block-sync`'s `GossipVerifier`
with `SimpleArcPeerManager` in ldk-sample.

13 months agoImplement `Readable`/`Writeable` for `RwLock` wrappers
Matt Corallo [Sat, 30 Sep 2023 17:35:21 +0000 (17:35 +0000)]
Implement `Readable`/`Writeable` for `RwLock` wrappers

We now support separate R/W locks in `LockableScore`, which allow
us to do routefinding in parallel, however in order to support
`WriteableScore` for such users we need to implement `Writeable`
for `RwLock` wrappers around `Writeable` types, which we do here.

13 months agoMerge pull request #2630 from TheBlueMatt/2023-09-117-rc1 v0.0.117-rc1
Matt Corallo [Sat, 30 Sep 2023 00:38:27 +0000 (00:38 +0000)]
Merge pull request #2630 from TheBlueMatt/2023-09-117-rc1

Bump crate versions to 0.0.117-rc1/invoice 0.25-rc1

13 months agoMerge pull request #2610 from wpaulino/missing-htlc-claim-balance
Matt Corallo [Fri, 29 Sep 2023 23:55:04 +0000 (23:55 +0000)]
Merge pull request #2610 from wpaulino/missing-htlc-claim-balance

Fix matching of second-stage HTLC claim in get_htlc_balance

13 months agoMerge pull request #2621 from G8XSU/dont-persist-erroneous-update
Matt Corallo [Fri, 29 Sep 2023 23:45:57 +0000 (23:45 +0000)]
Merge pull request #2621 from G8XSU/dont-persist-erroneous-update

Persist entire monitor if there is an error while applying monitor_update

13 months agoBump crate versions to 0.0.117-rc1/invoice 0.25-rc1 2023-09-117-rc1
Matt Corallo [Fri, 29 Sep 2023 23:39:18 +0000 (23:39 +0000)]
Bump crate versions to 0.0.117-rc1/invoice 0.25-rc1

13 months agoFix wrong node broadcaster in test_yield_anchors_events
Wilmer Paulino [Fri, 29 Sep 2023 23:00:35 +0000 (16:00 -0700)]
Fix wrong node broadcaster in test_yield_anchors_events

13 months agoAdd anchors coverage to test_revoked_counterparty_aggregated_claims
Wilmer Paulino [Tue, 26 Sep 2023 20:05:16 +0000 (13:05 -0700)]
Add anchors coverage to test_revoked_counterparty_aggregated_claims

13 months agoAdd anchors coverage to test_revoked_counterparty_htlc_tx_balances
Wilmer Paulino [Tue, 26 Sep 2023 18:59:12 +0000 (11:59 -0700)]
Add anchors coverage to test_revoked_counterparty_htlc_tx_balances

13 months agoAdd update_persisted_channel doc to indicate rare case for None update
Gursharan Singh [Fri, 29 Sep 2023 20:41:18 +0000 (13:41 -0700)]
Add update_persisted_channel doc to indicate rare case for None update

13 months agoPersist full monitor if there is an error while applying monitor_update
Gursharan Singh [Fri, 29 Sep 2023 03:30:53 +0000 (20:30 -0700)]
Persist full monitor if there is an error while applying monitor_update

Motivation: When there is an error while applying monitor_update to a
channel_monitor,  we don't want to persist a 'monitor_update' which
results in a failure to apply later while reading 'channel_monitor' with
updates from storage. Instead, we should persist the entire 'channel_monitor'
here.

13 months agoCorrectly mark chain_sync updates in test_utils
Gursharan Singh [Fri, 29 Sep 2023 04:18:47 +0000 (21:18 -0700)]
Correctly mark chain_sync updates in test_utils

We were incorrectly marking updates as chain_sync
or not in test_utils based on whether monitor_update
is None or not. Instead, use UpdateOrigin to determine it.

13 months agoAdd anchors coverage to test_revoked_counterparty_commitment_balances
Wilmer Paulino [Tue, 26 Sep 2023 17:31:09 +0000 (10:31 -0700)]
Add anchors coverage to test_revoked_counterparty_commitment_balances

13 months agoAdd anchors coverage to test_balances_on_local_commitment_htlcs
Wilmer Paulino [Tue, 26 Sep 2023 00:01:31 +0000 (17:01 -0700)]
Add anchors coverage to test_balances_on_local_commitment_htlcs

13 months agoAdd anchors coverage to test_claim_value_force_close
Wilmer Paulino [Mon, 25 Sep 2023 23:59:05 +0000 (16:59 -0700)]
Add anchors coverage to test_claim_value_force_close

13 months agoAdd anchors coverage to chanmon_claim_value_coop_close
Wilmer Paulino [Wed, 27 Sep 2023 21:11:35 +0000 (14:11 -0700)]
Add anchors coverage to chanmon_claim_value_coop_close

13 months agoAdd test util to handle bump HTLC events
Wilmer Paulino [Tue, 26 Sep 2023 18:59:42 +0000 (11:59 -0700)]
Add test util to handle bump HTLC events

13 months agoFix matching of second-stage HTLC claim in get_htlc_balance
Matt Corallo [Sun, 24 Sep 2023 02:32:08 +0000 (02:32 +0000)]
Fix matching of second-stage HTLC claim in get_htlc_balance

We incorrectly assumed that the descriptor's output index from
second-stage HTLC transaction would always match the HTLC's output index
in the commitment transaction. This doesn't make any sense though, we
need to make sure we map the descriptor to it's corresponding HTLC in
the commitment. Instead, we check that the transaction from which the
descriptor originated from spends the HTLC in question.

Note that pre-anchors, second-stage HTLC transactions are always 1
input-1 output, so previously we would only match if the HTLC was the
first output in the commitment transaction. Post-anchors, they are
malleable, so  we can aggregate multiple HTLC claims into a single
transaction making this even more likely to happen. Unfortunately, we
lacked proper coverage in this area so the bug went unnoticed. To
address this, we aim to extend our existing coverage of
`get_claimable_balances` to anchor outputs channels in the following
commits.

13 months agoMerge pull request #2629 from jkczyz/2023-09-invreqfailed
Matt Corallo [Fri, 29 Sep 2023 22:42:50 +0000 (22:42 +0000)]
Merge pull request #2629 from jkczyz/2023-09-invreqfailed

Config-guard `Event::InvoiceRequestFailed`

13 months agoMerge pull request #2605 from wpaulino/anchors-monitor-track-to-remote-script
Matt Corallo [Fri, 29 Sep 2023 22:06:58 +0000 (22:06 +0000)]
Merge pull request #2605 from wpaulino/anchors-monitor-track-to-remote-script

Use correct to_remote script in counterparty commitments

13 months agoRemove unused imports
Jeffrey Czyz [Fri, 29 Sep 2023 21:04:21 +0000 (14:04 -0700)]
Remove unused imports

13 months agoFix build warning for unused method
Jeffrey Czyz [Fri, 29 Sep 2023 21:03:39 +0000 (14:03 -0700)]
Fix build warning for unused method

13 months agoRemove an unnecessary enumerate
Jeffrey Czyz [Fri, 29 Sep 2023 20:58:34 +0000 (13:58 -0700)]
Remove an unnecessary enumerate

13 months agoConfig-guard Event::InvoiceRequestFailed
Jeffrey Czyz [Fri, 29 Sep 2023 20:47:25 +0000 (13:47 -0700)]
Config-guard Event::InvoiceRequestFailed

The event cannot be generated publicly, so remove it for now to avoid
users needing to handle it.

13 months agoMerge pull request #2628 from wvanlint/fix_multiple_shutdown_results
Matt Corallo [Fri, 29 Sep 2023 21:25:51 +0000 (21:25 +0000)]
Merge pull request #2628 from wvanlint/fix_multiple_shutdown_results

Fix handling multiple ShutdownResults

13 months agoExpose witness_script for StaticPaymentOutputDescriptor
Wilmer Paulino [Thu, 28 Sep 2023 19:31:43 +0000 (12:31 -0700)]
Expose witness_script for StaticPaymentOutputDescriptor

13 months agoFix incorrect anchors `counterparty_payment_script` upon deserialization
Wilmer Paulino [Thu, 28 Sep 2023 19:12:19 +0000 (12:12 -0700)]
Fix incorrect anchors `counterparty_payment_script` upon deserialization

13 months agoMerge pull request #2622 from wpaulino/funding-and-commitment-tx-confirm-same-block
Matt Corallo [Fri, 29 Sep 2023 21:06:55 +0000 (21:06 +0000)]
Merge pull request #2622 from wpaulino/funding-and-commitment-tx-confirm-same-block

Avoid early return upon confirmation of channel funding

13 months agoMerge pull request #2626 from TheBlueMatt/2023-09-revert-2476
Matt Corallo [Fri, 29 Sep 2023 21:06:41 +0000 (21:06 +0000)]
Merge pull request #2626 from TheBlueMatt/2023-09-revert-2476

Revert "Remove AvailableBalances::balance_msat"

13 months agoFix off-by-one max witness estimate for P2WPKH StaticPaymentDescriptor
Wilmer Paulino [Thu, 28 Sep 2023 16:07:10 +0000 (09:07 -0700)]
Fix off-by-one max witness estimate for P2WPKH StaticPaymentDescriptor

We were not accounting for the extra byte denoting the number of items
in the witness stack.

13 months agoSupport signing to_remote anchors variant for StaticPaymentOutput
Wilmer Paulino [Mon, 25 Sep 2023 23:55:22 +0000 (16:55 -0700)]
Support signing to_remote anchors variant for StaticPaymentOutput

`to_remote` outputs on commitment transactions with anchor outputs have
an additional `1 CSV` constraint on its spending condition,
transitioning away from the previous P2WPKH script to a P2WSH.

Since our `ChannelMonitor` was never updated to track the proper
`to_remote` script on anchor outputs channels, we also missed updating
our signer to handle the new script changes.

13 months agoUse correct to_remote script in counterparty commitments
Wilmer Paulino [Mon, 25 Sep 2023 23:53:42 +0000 (16:53 -0700)]
Use correct to_remote script in counterparty commitments

While our commitment transactions did use the correct `to_remote`
script, the `ChannelMonitor`'s was not as it is tracked separately. This
would lead to users never receiving an `Event::SpendableOutputs` with a
`StaticPaymentOutput` descriptor to claim the funds.

Luckily, any users affected which had channel closures confirmed by a
counterparty commitment just need to replay the closing transaction to
receive the event.

13 months agoMerge pull request #2624 from wpaulino/2609-follow-up
Matt Corallo [Fri, 29 Sep 2023 20:07:16 +0000 (20:07 +0000)]
Merge pull request #2624 from wpaulino/2609-follow-up

Address 2609 follow-up comments

13 months agoFix handling multiple ShutdownResults
Willem Van Lint [Fri, 29 Sep 2023 19:06:36 +0000 (12:06 -0700)]
Fix handling multiple ShutdownResults

13 months agoMerge pull request #2623 from wpaulino/htlc-claim-receive-preimage-after-close
Matt Corallo [Fri, 29 Sep 2023 18:53:44 +0000 (18:53 +0000)]
Merge pull request #2623 from wpaulino/htlc-claim-receive-preimage-after-close

Claim HTLCs with preimage from currently confirmed commitment

13 months agoAvoid early return upon confirmation of channel funding
Wilmer Paulino [Fri, 22 Sep 2023 17:39:10 +0000 (10:39 -0700)]
Avoid early return upon confirmation of channel funding

This early return is only possible if the channel requires a single
confirmation, allowing a `channel_ready` message to go out. This can be
problematic though if a commitment transaction (specifically from the
counterparty, as the channel would be immediately closed if a local
commitment is broadcast) also confirms within the same block. The
`ChannelMonitor` will detect both, but it won't inform the
`ChannelManager` at all. Luckily, while the channel still is considered
open to the `ChannelManager`, the `ChannelMonitor` will reject any
further updates to the channel state.

13 months agoRevert "Remove AvailableBalances::balance_msat" 2023-09-revert-2476
Matt Corallo [Fri, 29 Sep 2023 18:32:25 +0000 (18:32 +0000)]
Revert "Remove AvailableBalances::balance_msat"

While removing the `balance_msat` field absolutely makes sense -
it is, at best, confusing - we really need a solid replacement for
it before we can do so. While one such replacement is in progress,
it is not complete and we'd like to not block our current release
on its completion.

This reverts commit ef5be580f51d6756612fea710516c0297203f4dc.

13 months agoTest we consider route hints if we are the src of the first hop
Elias Rohrer [Fri, 29 Sep 2023 14:44:20 +0000 (16:44 +0200)]
Test we consider route hints if we are the src of the first hop

Previously, we would only consider route hints if the entry point was
in our first hops or in the network graph. We fixed this by also
considering hints if our own node ID was the first src.

Here, we add test coverage for this behavior.

13 months agoTest we prefer first hops over route hints
Elias Rohrer [Fri, 29 Sep 2023 14:09:44 +0000 (16:09 +0200)]
Test we prefer first hops over route hints

We previously added logic that would avoid adding superflous candidates
for route hints if we detect that we have a first hop for this channel.

Here we add test coverage that we actually prefer the first hop over the
route hint, but still consider the remaining hints.

13 months agoOnly yield DelayedPaymentOutput descriptors once their delay expires
Wilmer Paulino [Fri, 29 Sep 2023 16:40:34 +0000 (09:40 -0700)]
Only yield DelayedPaymentOutput descriptors once their delay expires

Otherwise, we could give users a descriptor ahead of time that will
result in an invalid transaction spend/broadcast.

13 months agoMerge pull request #2617 from wpaulino/no-persist-same-channel-update
Matt Corallo [Fri, 29 Sep 2023 17:39:49 +0000 (17:39 +0000)]
Merge pull request #2617 from wpaulino/no-persist-same-channel-update

Avoid persisting on same counterparty's ChannelUpdate

13 months agoMerge pull request #2591 from TheBlueMatt/2023-09-2562-followups
Elias Rohrer [Fri, 29 Sep 2023 17:08:36 +0000 (19:08 +0200)]
Merge pull request #2591 from TheBlueMatt/2023-09-2562-followups

Doc and comment followups to #2562

13 months agoNote required levels of descendant transactions in get_spendable_outputs
Wilmer Paulino [Fri, 29 Sep 2023 16:16:48 +0000 (09:16 -0700)]
Note required levels of descendant transactions in get_spendable_outputs

Three levels of descendant transactions starting from the channel's
funding transaction should cover all potential spendable outputs.

The first level covers the commitment transaction.

The second level covers the to_self claims, to_remote claims,
second-stage HTLC claims and justice transactions.

The third levels covers the justice transactions on second-stage HTLCs,
and to_self claims on second-stage HTLCs.

13 months agoTest preimage claim after reorg of counterparty commitment
Wilmer Paulino [Mon, 14 Aug 2023 19:12:54 +0000 (12:12 -0700)]
Test preimage claim after reorg of counterparty commitment

This test adds coverage for receiving a preimage after seeing a
counterparty commitment confirm, followed by a reorg and the
confirmation of a different commitment instead.

The first test covers the case where a holder commitment confirms after
the counterparty commitment reorg.

The second test covers the case where a previous counterparty commitment
confirms after the latest counterparty commitment reorg.

13 months agoClaim HTLCs with preimage from currently confirmed commitment
Wilmer Paulino [Fri, 28 Jul 2023 22:32:29 +0000 (15:32 -0700)]
Claim HTLCs with preimage from currently confirmed commitment

We should always claim HTLCs from the currently confirmed commitment,
rather than always claiming from the latest or previous counterparty
commitment if we've seen either confirm onchain at a prior point.

13 months agoAvoid persisting on same counterparty's ChannelUpdate
Wilmer Paulino [Thu, 28 Sep 2023 23:02:25 +0000 (16:02 -0700)]
Avoid persisting on same counterparty's ChannelUpdate

Some nodes may rebroadcast their `ChannelUpdate` to their counterparty
on every connection establishment, which leads to us doing an additional
persist most of the time when nothing has changed. Now, we'll only
persist if we receive an update that changes anything.

13 months agoAssert equality of route params in tests
Elias Rohrer [Fri, 29 Sep 2023 06:53:19 +0000 (08:53 +0200)]
Assert equality of route params in tests

Previously we only asserted the `final_value_msat` matches. Looking at
it again we can _of course_ assert the full equality of looked-for and
included route params after all (duh, not sure what I was thinking...).

This cleans up the prior misunderstanding and fixes a bunch of tests
that would now fail otherwise.

13 months agoMerge pull request #2609 from wpaulino/monitor-get-spendable-output
Matt Corallo [Fri, 29 Sep 2023 01:29:47 +0000 (01:29 +0000)]
Merge pull request #2609 from wpaulino/monitor-get-spendable-output

Allow retrieval of SpendableOutputDescriptors from relevant transactions