]> git.bitcoin.ninja Git - rust-lightning/log
rust-lightning
22 months agoMake the custom message traits cloneable as they're deep in nested structs 2022-12-0.0.113-java-bindings
Matt Corallo [Fri, 24 Sep 2021 18:44:32 +0000 (18:44 +0000)]
Make the custom message traits cloneable as they're deep in nested structs

22 months agoMake ChannelMonitor clonable again
Matt Corallo [Mon, 1 Feb 2021 01:12:50 +0000 (20:12 -0500)]
Make ChannelMonitor clonable again

In general, we'd been moving away from ChannelMonitor being clonable,
   XXXXXXXXXXXXXXXXXXXx

22 months agoMerge pull request #1938 from TheBlueMatt/2022-12-0.0.113-bindings
Matt Corallo [Tue, 10 Jan 2023 17:27:35 +0000 (17:27 +0000)]
Merge pull request #1938 from TheBlueMatt/2022-12-0.0.113-bindings

0.0.113 Bindings Updates

23 months agoUse an explicit `Sign` type on the `ChannelMonitor` read tuple 2022-12-0.0.113-bindings
Matt Corallo [Sat, 24 Dec 2022 04:16:48 +0000 (04:16 +0000)]
Use an explicit `Sign` type on the `ChannelMonitor` read tuple

The bindings currently get confused by the implicit `Sign` type, so
we temporarily remove it on the `impl` here.

23 months agoDon't expose `process_path` as it requires a slice of objects
Matt Corallo [Sat, 24 Dec 2022 04:15:20 +0000 (04:15 +0000)]
Don't expose `process_path` as it requires a slice of objects

23 months agoDrop `EventHandler` support for `Future`s since its nonsense in bindings
Matt Corallo [Thu, 22 Dec 2022 21:54:35 +0000 (21:54 +0000)]
Drop `EventHandler` support for `Future`s since its nonsense in bindings

23 months agoExport Onion Message structs in their respective modules
Matt Corallo [Mon, 19 Dec 2022 21:38:45 +0000 (21:38 +0000)]
Export Onion Message structs in their respective modules

Re-exports in Rust make `use` statements a little shorter, but for
otherwise don't materially change a crate's API. Sadly, the C
bindings generator currently can't figure out re-exports, but it
also exports everything into one global namespace, so it doesn't
matter much anyway.

23 months agoTag `KVStore` `(C-not exported)` as `Writeable` isn't mapped
Matt Corallo [Sun, 26 Jun 2022 18:14:30 +0000 (18:14 +0000)]
Tag `KVStore` `(C-not exported)` as `Writeable` isn't mapped

Currently `Writeable` is mapped manually, making it impossible to
automatically map a trait method that is parameterized by
`Writeable` (as is true for the `write` method on `KVStore`).

Ultimately we'll want to move to automatically mapping `Writeable`
like any other trait (only manually mapping the std `Write` and
`Read` traits), so this is only a candidate for the bindings branch,
not upstream. That may take a few releases, however.

23 months agoRestrict ChannelInfo::as_directed_from visibility
Jeffrey Czyz [Tue, 29 Mar 2022 15:20:39 +0000 (10:20 -0500)]
Restrict ChannelInfo::as_directed_from visibility

Bindings can't handle references in return types, so reduce the
visibility to pub(crate).

23 months agoSimplify type aliasing somewhat around times
Matt Corallo [Tue, 1 Mar 2022 03:47:28 +0000 (03:47 +0000)]
Simplify type aliasing somewhat around times

.. as the current C bindings generator isn't capable of handling
type aliases in generics in type alias definition currently.

23 months agoMake `as_directed_to` non-public
Matt Corallo [Tue, 1 Mar 2022 03:46:52 +0000 (03:46 +0000)]
Make `as_directed_to` non-public

...as the bindings generation does not currently have the ability
to map a reference to a `NodeId` inside a tuple.

23 months ago`#[derive(Clone)]` for `InFlightHtlcs`
Matt Corallo [Mon, 2 Jan 2023 01:07:59 +0000 (01:07 +0000)]
`#[derive(Clone)]` for `InFlightHtlcs`

This is useful for bindings, and generally isn't a bad thing for
users to have access to.

23 months agoEnsure the per-channel key derivation counter doesn't role over
Matt Corallo [Wed, 28 Dec 2022 18:12:29 +0000 (18:12 +0000)]
Ensure the per-channel key derivation counter doesn't role over

Previously, the `derive_channel_keys` derivation ID asserted that
the high bit of the per-channel key derivation counter doesn't
role over as it checked the 31st bit was zero. As we no longer do
that, we should ensure the assertion in `generate_channel_keys_id`
asserts that we don't role over.

23 months agoEnsure `derive_channel_keys` doesn't panic if per-run seed is high
Matt Corallo [Wed, 28 Dec 2022 17:44:33 +0000 (17:44 +0000)]
Ensure `derive_channel_keys` doesn't panic if per-run seed is high

b04d1b868fe28bea2e4c711e6e6d2470d2b98d77 changed the way we
calculate the `channel_keys_id` to include the 128-bit
`user_channel_id` as well, shifting the counter up four bytes and
the `starting_time_nanos` field up into the second four bytes.

In `derive_channel_keys` we hash the full `channel_keys_id` with an
HD-derived key from our master seed. Previously, that key was
derived with an index of the per-restart counter, re-calculated by
pulling the second four bytes out of the `user_channel_id`. Because
the `channel_keys_id` fields were shifted up four bytes, that is
now a reference to the `starting_time_nanos` value. This should be
fine, the derivation doesn't really add any value here, its all
being hashed anyway, except that derivation IDs must be below 2^31.
This implies that we panic if the user passes a
`starting_time_nanos` which has the high bit set. For those using
the nanosecond part of the current time this isn't an issue - the
value cannot exceed 1_000_000, which does not have the high bit
set, however, some users may use some other per-run seed.

Thus, here we simply drop the high bit from the seed, ensuring we
don't panic. Note that this is backwards compatible as it only
changes the key derivation in cases where we previously panicked.

Ideally we'd drop the derivation entirely, but that would break
backwards compatibility of key derivation.

23 months agoNo-export `&self` methods on non-cloneable enum(s)
Matt Corallo [Fri, 23 Dec 2022 20:44:24 +0000 (20:44 +0000)]
No-export `&self` methods on non-cloneable enum(s)

Specifically, `OnionMessageContents` is a non-cloneable enum, which
isn't stored opaque so we cannot call `&self` methods on it.
Because its methods aren't critical to the API for now, we simply
no-export them rather than trying to work out an alternative
approach.

23 months agoStore an owned `Score` in `ScorerAccountingForInFlightHtlcs`
Matt Corallo [Thu, 22 Dec 2022 21:58:53 +0000 (21:58 +0000)]
Store an owned `Score` in `ScorerAccountingForInFlightHtlcs`

`ScorerAccountingForInFlightHtlcs` generally stores a `Score`
reference generated by calling `LockableScore::lock`, which
actually returns an arbitrary `Score`. Given `Score` is implemented
directly on lock types, it makes sense to simply hold a fully owned
`Score` in `ScorerAccountingForInFlightHtlcs` rather than a mutable
reference to one.

23 months agoMerge pull request #1917 from TheBlueMatt/2022-12-0.0.113 v0.0.113
Arik [Fri, 16 Dec 2022 05:37:58 +0000 (21:37 -0800)]
Merge pull request #1917 from TheBlueMatt/2022-12-0.0.113

Cut 0.0.113

23 months agoBump crate versions to 0.0.113/invoice 0.21 2022-12-0.0.113
Matt Corallo [Thu, 15 Dec 2022 17:30:11 +0000 (17:30 +0000)]
Bump crate versions to 0.0.113/invoice 0.21

23 months agoAdd release notes for 0.0.113
Matt Corallo [Wed, 14 Dec 2022 19:38:54 +0000 (19:38 +0000)]
Add release notes for 0.0.113

Fixes #1890

23 months agoOnly do backtrace builds on stable to fix MSRV break in CI
Matt Corallo [Wed, 14 Dec 2022 19:46:19 +0000 (19:46 +0000)]
Only do backtrace builds on stable to fix MSRV break in CI

23 months agoMerge pull request #1918 from TheBlueMatt/2022-12-one-blinded-path
Matt Corallo [Thu, 15 Dec 2022 00:49:41 +0000 (00:49 +0000)]
Merge pull request #1918 from TheBlueMatt/2022-12-one-blinded-path

Unify blinding nomenclature to call them "paths" not "routes".

23 months agoUpdate references to "blinded route" to "blinded path" 2022-12-one-blinded-path
Matt Corallo [Wed, 14 Dec 2022 21:08:51 +0000 (21:08 +0000)]
Update references to "blinded route" to "blinded path"

Finishing the work from the previous two commits.

23 months agoRename `blinded_route` variables and module to `blinded_path`
Matt Corallo [Wed, 14 Dec 2022 20:49:53 +0000 (20:49 +0000)]
Rename `blinded_route` variables and module to `blinded_path`

Following up on the previous commit, this also renames variables
and the module used to `blinded_path`.

23 months agoUnify blinding nomenclature to call them "paths" not "routes".
Matt Corallo [Wed, 14 Dec 2022 20:45:37 +0000 (20:45 +0000)]
Unify blinding nomenclature to call them "paths" not "routes".

Currently the `onion_message` module exposes the blinded route
object as *both* `BlindedRoute` and `BlindedPath`. This is somewhat
confusing, and given they are really paths, not routes (at least in
the sense that a route could be multi-path, though for OMs they are
not), here we unify to only call them paths.

23 months agoMerge pull request #1915 from TheBlueMatt/2022-12-jit-reload-consistency
Matt Corallo [Tue, 13 Dec 2022 21:08:45 +0000 (21:08 +0000)]
Merge pull request #1915 from TheBlueMatt/2022-12-jit-reload-consistency

Drop forwarded HTLCs which were still pending at persist-time

23 months agoDrop forwarded HTLCs which were still pending at persist-time 2022-12-jit-reload-consistency
Matt Corallo [Tue, 13 Dec 2022 03:27:23 +0000 (03:27 +0000)]
Drop forwarded HTLCs which were still pending at persist-time

If, after forwarding an intercepted payment to our counterparty, we
restart with a ChannelMonitor update having been persisted, but the
corresponding ChannelManager update not having been persisted,
we'll still have the intercepted HTLC in the
`pending_intercepted_htlcs` map on start (and potentially a pending
`HTLCIntercepted` event). This will cause us to allow the user to
handle the forwarded HTLC twice, potentially double-forwarding it.

This builds on 0bb87ddad71d2e33199ebad79e9f709f869f2130, which
provided a preemptive fix for the general relay case (though it was
not an actual issue at the time). We simply check for the HTLCs
having been forwarded on startup and remove them from the map.

Fixes #1858

23 months agoRemove unused fetch in `lightning_invoice` tests.
Matt Corallo [Tue, 13 Dec 2022 01:03:18 +0000 (01:03 +0000)]
Remove unused fetch in `lightning_invoice` tests.

23 months agoMerge pull request #1894 from ssbright/2022-12-custom_payment_hash
Matt Corallo [Tue, 13 Dec 2022 00:00:32 +0000 (00:00 +0000)]
Merge pull request #1894 from ssbright/2022-12-custom_payment_hash

Add invoice constructor with custom payment hash

23 months agoMerge pull request #1900 from tnull/2022-12-improve-confirm-docs
Matt Corallo [Mon, 12 Dec 2022 23:22:52 +0000 (23:22 +0000)]
Merge pull request #1900 from tnull/2022-12-improve-confirm-docs

Improve `Confirm` docs

23 months agoMerge pull request #1892 from tnull/2022-12-spendableoutputdescriptor-doccs
Matt Corallo [Mon, 12 Dec 2022 22:45:00 +0000 (22:45 +0000)]
Merge pull request #1892 from tnull/2022-12-spendableoutputdescriptor-doccs

Clean up docs in `keysinterface.rs`

23 months agoMerge pull request #1907 from TheBlueMatt/2022-12-abandon-crash-reset
Matt Corallo [Mon, 12 Dec 2022 22:16:43 +0000 (22:16 +0000)]
Merge pull request #1907 from TheBlueMatt/2022-12-abandon-crash-reset

Note that abandon_payment does not persist the state update in docs

23 months agoAdd invoice constructor with custom payment hash
ssbright [Mon, 12 Dec 2022 21:08:38 +0000 (13:08 -0800)]
Add invoice constructor with custom payment hash

23 months agoImprove `Confirm` docs
Elias Rohrer [Mon, 5 Dec 2022 16:55:47 +0000 (17:55 +0100)]
Improve `Confirm` docs

23 months agoClean up docs in `keysinterface.rs`
Elias Rohrer [Thu, 1 Dec 2022 14:17:57 +0000 (15:17 +0100)]
Clean up docs in `keysinterface.rs`

23 months agoNote that abandon_payment does not persist the state update in docs 2022-12-abandon-crash-reset
Matt Corallo [Thu, 8 Dec 2022 00:33:15 +0000 (00:33 +0000)]
Note that abandon_payment does not persist the state update in docs

If a user calls `abandon_payment`, then restarts without freshly
persisting the `ChannelManager`, the payment will still be pending
on restart. This was unclear from the docs (and the docs seemed to
imply otherwise). Because this doesn't materially impact the
usability of `abandon_payment` (users shouldn't be called
`retry_payment` on an abandoned one anyway), we simply document it.

Fixes #1804.

23 months agoMerge pull request #1904 from TheBlueMatt/2022-12-1825-followups
Matt Corallo [Mon, 12 Dec 2022 17:58:21 +0000 (17:58 +0000)]
Merge pull request #1904 from TheBlueMatt/2022-12-1825-followups

Trivial Followups to #1825

23 months agoMerge pull request #1738 from jkczyz/2022-09-invoice-request
valentinewallace [Mon, 12 Dec 2022 16:25:07 +0000 (11:25 -0500)]
Merge pull request #1738 from jkczyz/2022-09-invoice-request

BOLT 12 `invoice_request` encoding and building

23 months agoMerge pull request #1906 from wpaulino/prevent-downgrade-from-anchors
Matt Corallo [Mon, 12 Dec 2022 03:11:30 +0000 (03:11 +0000)]
Merge pull request #1906 from wpaulino/prevent-downgrade-from-anchors

Use even types for opt_anchors

23 months agoMerge pull request #1886 from TheBlueMatt/2022-11-claim-relock
Matt Corallo [Mon, 12 Dec 2022 03:10:38 +0000 (03:10 +0000)]
Merge pull request #1886 from TheBlueMatt/2022-11-claim-relock

Relock `channel_state` in for each HTLC in `claim_funds` and lay the groundwork for async event generation

23 months agoAdd BOLT 12 merkle root test for `invoice_request`
Jeffrey Czyz [Mon, 28 Nov 2022 16:20:07 +0000 (11:20 -0500)]
Add BOLT 12 merkle root test for `invoice_request`

A BOLT 12 test vector uses an `invoice_request` message that has a
currency, which aren't supported, so using OfferBuilder::build_unchecked
is required to avoid a panic.

23 months agoInvoice request parsing tests
Jeffrey Czyz [Wed, 16 Nov 2022 22:13:52 +0000 (16:13 -0600)]
Invoice request parsing tests

Tests for checking invoice_request message semantics when parsing bytes
as defined by BOLT 12.

23 months agoInvoice request building tests
Jeffrey Czyz [Fri, 11 Nov 2022 03:12:58 +0000 (21:12 -0600)]
Invoice request building tests

Tests for checking invoice_request message semantics when building as
defined by BOLT 12.

23 months agoBuilder for creating invoice requests
Jeffrey Czyz [Wed, 31 Aug 2022 15:19:44 +0000 (10:19 -0500)]
Builder for creating invoice requests

Add a builder for creating invoice requests for an offer given a
payer_id. Other settings may be optional depending on the offer and
duplicative settings will override previous settings. Building produces
a semantically valid `invoice_request` message for the offer, which then
can be signed for the payer_id.

23 months agoInvoice request raw byte encoding and decoding
Jeffrey Czyz [Tue, 23 Aug 2022 22:31:46 +0000 (17:31 -0500)]
Invoice request raw byte encoding and decoding

When reading an offer, an `invoice_request` message is sent over the
wire. Implement Writeable for encoding the message and TryFrom for
decoding it by defining in terms of TLV streams. These streams represent
content for the payer metadata (0), reflected `offer` (1-79),
`invoice_request` (80-159), and signature (240).

23 months agoMerkle root hash computation
Jeffrey Czyz [Tue, 9 Aug 2022 22:37:02 +0000 (17:37 -0500)]
Merkle root hash computation

Offers uses a merkle root hash construction for signature calculation
and verification. Add a submodule implementing this so that it can be
used when parsing and signing invoice_request and invoice messages.

23 months agoSchnorr Signature serialization
Jeffrey Czyz [Fri, 24 Jun 2022 21:18:29 +0000 (16:18 -0500)]
Schnorr Signature serialization

BOLT 12 uses Schnorr signatures for signing offers messages, which need
to be serialized.

23 months agoInvoice request message interface and data format
Jeffrey Czyz [Mon, 19 Sep 2022 21:57:46 +0000 (16:57 -0500)]
Invoice request message interface and data format

Define an interface for BOLT 12 `invoice_request` messages. The
underlying format consists of the original bytes and the parsed
contents.

The bytes are later needed when constructing an `invoice` message. This
is because it must mirror all the `offer` and `invoice_request` TLV
records, including unknown ones, which aren't represented in the
contents.

The contents will be used in `invoice` messages to avoid duplication.
Some fields while required in a typical user-pays-merchant flow may not
be necessary in the merchant-pays-user flow (e.g., refund, ATM).

23 months agoAdd second TODO when claiming to mirror the existing TODO on claim fail 2022-11-claim-relock
Matt Corallo [Tue, 6 Dec 2022 21:19:29 +0000 (21:19 +0000)]
Add second TODO when claiming to mirror the existing TODO on claim fail

23 months agoDrop unused link in `claim_funds`
Matt Corallo [Tue, 6 Dec 2022 21:13:35 +0000 (21:13 +0000)]
Drop unused link in `claim_funds`

23 months agoDrop now-unused `ClaimFundsFromHop` enum and replace with an `Err`
Matt Corallo [Wed, 30 Nov 2022 21:48:46 +0000 (21:48 +0000)]
Drop now-unused `ClaimFundsFromHop` enum and replace with an `Err`

23 months agoHandle claim result event generation in claim_funds_from_hop
Matt Corallo [Tue, 6 Dec 2022 21:01:50 +0000 (21:01 +0000)]
Handle claim result event generation in claim_funds_from_hop

Currently `claim_funds` and `claim_funds_internal` call
`claim_funds_from_hop` and then surface and `Event` to the user
informing them of the forwarded/claimed payment based on it's
result. In both places we assume that a claim "completed" even if
a monitor update is being done async.

Instead, here we push that event generation through a
`MonitorUpdateCompletionAction` and a call to
`handle_monitor_update_completion_action`. This will allow us to
hold the event(s) until async monitor updates complete in the
future.

23 months agoDon't hold `channel_state` lock for entire duration of claim_funds
Matt Corallo [Wed, 30 Nov 2022 05:47:16 +0000 (05:47 +0000)]
Don't hold `channel_state` lock for entire duration of claim_funds

When `claim_funds` has to claim multiple HTLCs as a part of a
single MPP payment, it currently does so holding the
`channel_state` lock for the entire duration of the claim loop.
Here we swap that for taking the lock once for each HTLC. This
allows us to be more flexible with locks going forward, and
ultimately isn't a huge change - if our counterparty intends to
force-close a channel, us choosing to ignore it by holding the
`channel_state` lock for the duration of the claim isn't going to
result in a commitment update, it will just result in the preimage
already being in the `ChannelMonitor`.

23 months agoHandle closed-chan HTLC claims in `claim_funds_from_hop`
Matt Corallo [Tue, 6 Dec 2022 20:46:02 +0000 (20:46 +0000)]
Handle closed-chan HTLC claims in `claim_funds_from_hop`

Currently `claim_funds` does all HTLC claims in one `channel_state`
lock, ensuring that we always make claims from channels which are
open. It can thus avoid ever having to generate a
`ChannelMonitorUpdate` containing a preimage for a closed channel,
which we only do in `claim_funds_internal` (for forwarded payments).

In the next commit we'll change the locking of
`claim_funds_from_hop` so that `claim_funds` is no longer under a
single lock but takes a lock for each claim. This allows us to be
more flexible with locks going forward, and ultimately isn't a huge
change - if our counterparty intends to force-close a channel, us
choosing to ignore it by holding the `channel_state` lock for the
duration of the claim isn't going to result in a commitment update,
it will just result in the preimage already being in the
`ChannelMonitor`.

23 months agoAdd support for handling "actions" after a monitor update completes
Matt Corallo [Wed, 30 Nov 2022 18:37:12 +0000 (18:37 +0000)]
Add support for handling "actions" after a monitor update completes

This adds a new enum, `MonitorUpdateCompletionAction` and a method
to execute the "actions". They are intended to be done once a
(potentially-async) `ChannelMonitorUpdate` persistence completes,
however this behavior will be implemented in a future PR. For now,
this adds the relevant infrastructure which will allow us to
prepare `claim_funds` for better monitor async handling.

23 months agoStore pending claims awaiting monitor update in a separate map
Matt Corallo [Tue, 6 Dec 2022 18:33:52 +0000 (18:33 +0000)]
Store pending claims awaiting monitor update in a separate map

In the next commits we'll move to generating `PaymentClaimed`
events while handling `ChannelMonitorUpdate`s rather than directly
in line. Thus, as a prerequisite, here we move to storing the info
required to generate the `PaymentClaimed` event in a separate map.

Note that while this does introduce a new map which is written as
an even value which users cannot opt out of, the map is only filled
in when users use the asynchronous `ChannelMonitor` updates and
after a future PR. As these are still considered beta, breaking
downgrades for such users is considered acceptable in the future PR
(which will likely be one LDK version later).

23 months agoSlightly clarify comment on safety of only processing HTLCs once 2022-12-1825-followups
Matt Corallo [Wed, 7 Dec 2022 23:17:31 +0000 (23:17 +0000)]
Slightly clarify comment on safety of only processing HTLCs once

23 months agoEnsure `Event::BumpTransaction` is forwards-compatible
Matt Corallo [Wed, 7 Dec 2022 00:41:07 +0000 (00:41 +0000)]
Ensure `Event::BumpTransaction` is forwards-compatible

`Event`s with an odd type are ignored by older versions of LDK,
however they rely on the `write_tlv_fields` call to know how many
bytes to read and discard. We were missing that call in our writing
of `Event::BumpTransaction`, which we add here.

23 months agoUse `Witness::push_bitcoin_signature` where relevant
Matt Corallo [Wed, 7 Dec 2022 00:40:38 +0000 (00:40 +0000)]
Use `Witness::push_bitcoin_signature` where relevant

23 months agoDrop excess mut on `OnchainTxHandler::generate_external_htlc_claim`
Matt Corallo [Wed, 7 Dec 2022 00:40:17 +0000 (00:40 +0000)]
Drop excess mut on `OnchainTxHandler::generate_external_htlc_claim`

23 months agoDRY the comparison blocks in `update_claims_view_from_matched_txn`
Matt Corallo [Wed, 7 Dec 2022 00:30:43 +0000 (00:30 +0000)]
DRY the comparison blocks in `update_claims_view_from_matched_txn`

In `update_claims_view_from_matched_txn` we have two different
tx-equivalence checks which do the same thing - both check that the
tx which appeared on chain spent all of the outpoints which we
intended to spend in a given package. While one is more effecient
than the other (but only usable in a subset of cases), the
difference between O(N) and O(N^2) when N is 1-5 is trivial.

Still, it is possible we hit this code with just shy of 900 HTLC
outputs in a channel, and a transaction with a ton of inputs.

While having to spin through a few million entries if our
counterparty wastes a full block isn't really a big deal, we go
ahead and use a sorted vec and binary searches because its trivial.

23 months agoUse `PackageId` rather than `Txid` in `OnchainEvent::Claim`
Matt Corallo [Wed, 7 Dec 2022 00:29:11 +0000 (00:29 +0000)]
Use `PackageId` rather than `Txid` in `OnchainEvent::Claim`

In 19daccf7fb5ea81c8d235c1628a91efe0aa07b96, a `PackageId` type was
added to differentiate between an opaque Id for packages and the
`Txid` type which was being used for that purpose. It, however,
failed to also replace the single inner field in
`OnchainEvent::Claim` which was also a package ID. We do so here.

23 months agoUse even types for opt_anchors
Wilmer Paulino [Wed, 7 Dec 2022 18:30:25 +0000 (10:30 -0800)]
Use even types for opt_anchors

This prevents downgrading to older versions of LDK that are not capable
of supporting anchor channels when the field is serialized (i.e.,
opt_anchors is `Some`).

23 months agoMerge pull request #1863 from TheBlueMatt/2022-11-holding-cell-batch-update
Matt Corallo [Wed, 7 Dec 2022 17:52:04 +0000 (17:52 +0000)]
Merge pull request #1863 from TheBlueMatt/2022-11-holding-cell-batch-update

Lean on the holding cell when batch-forwarding/failing HTLCs

23 months agoMerge pull request #1825 from wpaulino/anchors-bump-htlc-resolution-event
Matt Corallo [Wed, 7 Dec 2022 05:51:27 +0000 (05:51 +0000)]
Merge pull request #1825 from wpaulino/anchors-bump-htlc-resolution-event

Introduce new BumpTransactionEvent variant HTLCResolution

23 months agoExtend BaseSign with HTLC output signing support for external claims
Wilmer Paulino [Wed, 30 Nov 2022 22:05:02 +0000 (14:05 -0800)]
Extend BaseSign with HTLC output signing support for external claims

23 months agoYield BumpHTLCResolution events
Wilmer Paulino [Wed, 31 Aug 2022 19:03:35 +0000 (12:03 -0700)]
Yield BumpHTLCResolution events

23 months agoExpose HTLC transaction construction helpers
Wilmer Paulino [Wed, 30 Nov 2022 22:03:45 +0000 (14:03 -0800)]
Expose HTLC transaction construction helpers

23 months agoRename set_equality within update_claims_view_from_matched_txn
Wilmer Paulino [Tue, 22 Nov 2022 01:22:06 +0000 (17:22 -0800)]
Rename set_equality within update_claims_view_from_matched_txn

23 months agoGenerate ClaimEvent for HolderHTLCOutput inputs from anchor channels
Wilmer Paulino [Wed, 30 Nov 2022 22:03:26 +0000 (14:03 -0800)]
Generate ClaimEvent for HolderHTLCOutput inputs from anchor channels

23 months agoIntroduce internal package ID to track pending claims
Wilmer Paulino [Mon, 28 Nov 2022 15:47:44 +0000 (07:47 -0800)]
Introduce internal package ID to track pending claims

Now that our txids will no longer be stable for package claims that
require external funds to be allocated, we transition to a 32-byte array
identifier to remain compatible with them.

23 months agoSupport HolderHTLCOutput inputs from anchor channels
Wilmer Paulino [Wed, 31 Aug 2022 18:56:59 +0000 (11:56 -0700)]
Support HolderHTLCOutput inputs from anchor channels

23 months agoSpecify amount units in HolderHTLCOutput
Wilmer Paulino [Wed, 13 Jul 2022 17:27:45 +0000 (10:27 -0700)]
Specify amount units in HolderHTLCOutput

This is only a name change, there is no change in behavior.

23 months agoUpdate HTLC transaction detection from revoked counterparty commitments
Wilmer Paulino [Tue, 22 Nov 2022 01:11:09 +0000 (17:11 -0800)]
Update HTLC transaction detection from revoked counterparty commitments

Previously, this method assumed that all HTLC transactions have 1 input
and 1 output, with the sole input having a witness of 5 elements. This
will no longer be the case for HTLC transactions on channels with
anchors outputs since additional inputs and outputs can be attached to
them to allow fee bumping.

23 months agoTrack HTLC resolving transaction to determine input index
Wilmer Paulino [Thu, 25 Aug 2022 20:09:29 +0000 (13:09 -0700)]
Track HTLC resolving transaction to determine input index

23 months agoMerge pull request #1895 from TheBlueMatt/2022-12-fix-missing-data
Matt Corallo [Tue, 6 Dec 2022 22:46:04 +0000 (22:46 +0000)]
Merge pull request #1895 from TheBlueMatt/2022-12-fix-missing-data

Fix some onion errors and assert their length is correct

23 months agoCorrectly handle any `UPDATE` errors to phandom invoices 2022-12-fix-missing-data
Matt Corallo [Fri, 2 Dec 2022 21:12:47 +0000 (21:12 +0000)]
Correctly handle any `UPDATE` errors to phandom invoices

If we try to send any onion error with the `UPDATE` flag in
response to a phantom receipt, we should always swap it for
something generic that doesn't require a `channel_update` in it.
Here we use `temporary_node_failure`.

Test provided by Valentine Wallace <vwallace@protonmail.com>

23 months agoReplace `build_first_hop_failure_packet` with `HTLCFailReason`
Matt Corallo [Thu, 1 Dec 2022 23:30:04 +0000 (23:30 +0000)]
Replace `build_first_hop_failure_packet` with `HTLCFailReason`

This ensures we always hit our new debug assertions while building
failure packets in the immediately-fail pipeline while processing
an inbound HTLC.

23 months agoUse `temporary_node_failure` for a phantom HTLC with bogus CLTV
Matt Corallo [Thu, 1 Dec 2022 23:39:28 +0000 (23:39 +0000)]
Use `temporary_node_failure` for a phantom HTLC with bogus CLTV

When we receive a phantom HTLC with a bogus/modified CLTV, we
should fail back with `incorrect_cltv_expiry`, but that requires a
`channel_update`, which we cannot generate for a phantom HTLC which
has no corresponding channel. Thus, instead, we have to fall back
to `incorrect_cltv_expiry`.

Fixes #1879

23 months agoAssert that all onion error messages are correct len in tests
Matt Corallo [Thu, 1 Dec 2022 20:31:52 +0000 (20:31 +0000)]
Assert that all onion error messages are correct len in tests

When we're constructing an HTLCFailReason, we should check that we
set the data to at least the correct length for the given failure
code, which we do here.

23 months agoCorrectly include the `sha256_hash_of_onion` field in BADONION errs
Matt Corallo [Thu, 1 Dec 2022 20:30:45 +0000 (20:30 +0000)]
Correctly include the `sha256_hash_of_onion` field in BADONION errs

The spec mandates that we copy the `sha256_hash_of_onion` field
from the `UpdateFailMalformedHTLC` message into the error message
we send back to the sender, however we simply ignored it. Here we
copy it into the message correctly.

23 months agoDrop the stale `final_expiry_too_soon` error code
Matt Corallo [Thu, 1 Dec 2022 20:25:33 +0000 (20:25 +0000)]
Drop the stale `final_expiry_too_soon` error code

This replaces `final_expiry_too_soon` with
`incorrect_or_unknown_payment` as was done in
https://github.com/lightning/bolts/pull/608. Note that the
rationale for this (that it may expose whether you are the final
recipient for the payment or not) does not currently apply to us -
we don't apply different final CLTV values to different payments.
However, we might in the future, and this will make us slightly
more consistent with other nodes.

23 months agoEncapsulate `HTLCFailReason` to not expose struct variants
Matt Corallo [Thu, 1 Dec 2022 19:28:32 +0000 (19:28 +0000)]
Encapsulate `HTLCFailReason` to not expose struct variants

Now that `HTLCFailReason` is opaque and in `onion_utils`, we should
encapsulate it so that `ChannelManager` can no longer directly
access its inner fields.

23 months agoMove `HTLCFailReason` to `onion_utils`
Matt Corallo [Thu, 1 Dec 2022 19:20:19 +0000 (19:20 +0000)]
Move `HTLCFailReason` to `onion_utils`

Now that it's entirely abstracted, there's no reason for
`HTLCFailReason` to be in `channelmanager`, it's really an
onion-level abstraction.

23 months agoMerge pull request #1902 from tnull/2022-12-payment-received-renaming-follow-up
Matt Corallo [Tue, 6 Dec 2022 19:07:09 +0000 (19:07 +0000)]
Merge pull request #1902 from tnull/2022-12-payment-received-renaming-follow-up

Also rename variables referring to `PaymentClaimable`

23 months agoMove `claimable_htlcs` to a struct for more fields in the same mutex
Matt Corallo [Tue, 6 Dec 2022 18:51:49 +0000 (18:51 +0000)]
Move `claimable_htlcs` to a struct for more fields in the same mutex

23 months agoCorrect confusing docs on `Channel` methods 2022-11-holding-cell-batch-update
Matt Corallo [Mon, 28 Nov 2022 23:51:55 +0000 (23:51 +0000)]
Correct confusing docs on `Channel` methods

The methods return `Ok(())` always, they just happen to never
return in the case of a duplicate claim if debug assertions are
enabled.

23 months agoLean on the holding cell for commitments when updating fees
Matt Corallo [Mon, 21 Nov 2022 01:13:52 +0000 (01:13 +0000)]
Lean on the holding cell for commitments when updating fees

Like the previous commit, here we update the update_fee+commit
logic to simply push the fee update into the holding cell and then
use the standard holding-cell-freeing codepaths to actually send
the commitment update. This removes a substantial amount of code,
reducing redundant codepaths and keeping channel state machine
logic in channel.rs.

23 months agoFree the holding cells during background timer ticks
Matt Corallo [Mon, 21 Nov 2022 01:22:51 +0000 (01:22 +0000)]
Free the holding cells during background timer ticks

We currently free the channel holding cells in
`get_and_clear_pending_msg_events`, blocking outbound messages
while we do so. This is fine, but may block the message pipeline
longer than we need to. In the next commit we'll push
timer-originating channel fee updates out through the holding cell
pipeline, leaning more on that freeing in the future.

Thus, to avoid a regression in message time, here we clear the
holding cell after processing all timer events. This also avoids
needing to change tests in the next commit.

23 months agoLean on the holding cell when batch-forwarding/failing HTLCs
Matt Corallo [Sat, 19 Nov 2022 00:00:28 +0000 (00:00 +0000)]
Lean on the holding cell when batch-forwarding/failing HTLCs

When we batch HTLC updates, we currently do the explicit queueing
plus the commitment generation in the `ChannelManager`. This is a
bit strange as its ultimately really a `Channel` responsibility to
generate commitments at the correct time, with the abstraction
leaking into `ChannelManager` with the `send_htlc` and
`get_update_fail_htlc` method docs having clear comments about
how `send_commitment` MUST be called prior to calling other
`Channel` methods.

Luckily `Channel` already has an update queue - the holding cell.
Thus, we can trivially rewrite the batch update logic as inserting
the desired updates into the holding cell and then asking all
channels to clear their holding cells.

23 months agoMerge pull request #1867 from wpaulino/remove-signer-persistence
Matt Corallo [Tue, 6 Dec 2022 18:13:49 +0000 (18:13 +0000)]
Merge pull request #1867 from wpaulino/remove-signer-persistence

Re-derive signers instead of persisting them

23 months agoRename variables referring to `PaymentClaimable`
Elias Rohrer [Tue, 6 Dec 2022 09:47:07 +0000 (10:47 +0100)]
Rename variables referring to `PaymentClaimable`

2 years agoMerge pull request #1857 from TheBlueMatt/2022-11-reload-htlc
Matt Corallo [Mon, 5 Dec 2022 22:54:08 +0000 (22:54 +0000)]
Merge pull request #1857 from TheBlueMatt/2022-11-reload-htlc

Fail HTLCs which were removed from a channel but not persisted

2 years agoFail HTLCs which were removed from a channel but not persisted 2022-11-reload-htlc
Matt Corallo [Wed, 16 Nov 2022 02:20:03 +0000 (02:20 +0000)]
Fail HTLCs which were removed from a channel but not persisted

When a channel is force-closed, if a `ChannelMonitor` update is
completed but a `ChannelManager` persist has not yet happened,
HTLCs which were removed in the latest (persisted) `ChannelMonitor`
update will not be failed even though they do not appear in the
commitment transaction which went on chain. This is because the
`ChannelManager` thinks the `ChannelMonitor` is responsible for
them (as it is stale), but the `ChannelMonitor` has no knowledge of
the HTLC at all (as it is not stale).

The fix for this is relatively simple - we need to check for this
specific case and fail back such HTLCs when deserializing a
`ChannelManager`

2 years agoAvoid attempting to forward to a closed chan on stale-data reload
Matt Corallo [Thu, 17 Nov 2022 05:55:45 +0000 (05:55 +0000)]
Avoid attempting to forward to a closed chan on stale-data reload

If, after forwarding a payment to our counterparty, we restart with
a ChannelMonitor update having been persisted, but the
corresponding ChannelManager update was not persisted, we'll still
have the forwarded HTLC in the `forward_htlcs` map on start. This
will cause us to generate a (spurious) `PendingHTLCsForwardable`
event. However, when we go to forward said HTLC, we'll notice the
channel has been closed and leave it up to the `ChannelMontior` to
finalize the HTLC.

This is all fine today - we won't lose any funds, we'll just
generate an excess forwardable event and then fail to forward.
However, in the future when we allow for forward-time channel
changes this could break. Thus, its worth adding tests for this
behavior today, and, while we're at it, removing the spurious
forwardable HTLCs event.

2 years agoExpose the full set of outbound HTLCs from a `ChannelMonitor`
Matt Corallo [Tue, 15 Nov 2022 23:35:31 +0000 (23:35 +0000)]
Expose the full set of outbound HTLCs from a `ChannelMonitor`

This expands the outbound-HTLC-listing support in `ChannelMonitor`
to include not only the set of outbound HTLCs which have not yet
been resolved but to also include the full set of HTLCs which the
`ChannelMonitor` is currently able to to or has already finalized.

This will be used in the next commit to fail-back HTLCs which were
removed from a channel in the ChannelMonitor but not in a Channel.
Using the existing `get_pending_outbound_htlcs` for this purpose is
subtly broken - if the channel is already closed, an HTLC fail may
have completed on chain and is no longer "pending" to the monitor,
but the fail event is still in the monitor waiting to be handed
back to the `ChannelMonitor` when polled.

2 years agoRemove unnecessary byte_utils helpers
Wilmer Paulino [Thu, 1 Dec 2022 22:45:46 +0000 (14:45 -0800)]
Remove unnecessary byte_utils helpers

Now that to_be_bytes is available under our current MSRV of 1.41, we
can use it instead of our own version.

2 years agoDrop Clone requirement from Sign
Wilmer Paulino [Tue, 29 Nov 2022 17:06:31 +0000 (09:06 -0800)]
Drop Clone requirement from Sign

Now that we opt to always re-derive channel secrets whenever required,
we can drop the Clone requirement from Sign.

2 years agoAvoid use of OnlyReadsKeysInterface
Wilmer Paulino [Mon, 21 Nov 2022 21:34:22 +0000 (13:34 -0800)]
Avoid use of OnlyReadsKeysInterface

Since `ChannelMonitor`s will now re-derive signers rather than
persisting them, we can no longer use the OnlyReadsKeysInterface
concrete implementation.

2 years agoRe-derive signers upon deserializing OnchainTxHandler
Wilmer Paulino [Mon, 21 Nov 2022 20:49:05 +0000 (12:49 -0800)]
Re-derive signers upon deserializing OnchainTxHandler

Similar to the previous commit, we introduce a new serialization version
that doesn't store a monitor's signer. Since the monitor already knows
of a channel's `channel_keys_id`, there's no need to store any new data
to re-derive all private key material for said channel.

2 years agoRe-derive signers upon deserializing Channel
Wilmer Paulino [Mon, 21 Nov 2022 20:47:41 +0000 (12:47 -0800)]
Re-derive signers upon deserializing Channel

To do so, we introduce a new serialization version that doesn't store a
channel's signer, and instead stores its signer's `channel_keys_id`.
This is a unique identifier that can be provided to our `KeysInterface`
to re-derive all private key material for said channel.

We choose to not upgrade the minimum compatible serialization version
until a later time, which will also remove any signer serialization
logic on implementations of `KeysInterface` and `Sign`.