Jeffrey Czyz [Fri, 16 Dec 2022 20:06:33 +0000 (14:06 -0600)]
Define blinded hop features for use in BOLT 12
BOLT 12 invoices may contain blinded_payinfo for each hop in a blinded
path. Each blinded_payinfo contains features, whose length must be
encoded since there may be multiple hops.
Note these features are also needed in the BOLT 4 encrypted_data_tlv
stream. But since they are a single TLV record, the length must *not* be
encoded there.
Jeffrey Czyz [Fri, 16 Dec 2022 19:35:50 +0000 (13:35 -0600)]
Use explicit WithoutLength for BOLT 12 features
Most BOLT 12 features are used as the value of a TLV record and thus
don't use an explicit length. One exception is the features inside the
blinded payinfo subtype since the TLV record contains a list of them.
However, these features are also used in the BOLT 4 encrypted_data_tlv
TLV stream as a single record, where the length is implicit.
Implement Readable and Writeable for Features wrapped in WithoutLength
such that either serialization can be used where required.
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.
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.
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.
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.
9d7bb73b599a7a9d8468a2f0c54d28f0ce6cf543 broke some capitalization
in docs for `sign_invoice`, which we fix here as well as taking
this opportunity to clean up the `sign_invoice` docs more
generally.
Once ChannelManager supports payment retries, it will make more sense for its
current send_payment method to be named send_payment_with_route because
retrying should be the default. Here we get a head start on this by making the
rename in outbound_payment, but not changing the public interface yet.
Swap pending_outbound_payments for OutboundPayments struct
This allows us to move a lot of outbound payment logic out of ChannelManager
and into the new outbound_payment module, and helps avoid growing
ChannelManager when we add retry logic to it in upcoming work.
Move PendingOutboundPayment to new outbound_payment module
We want to move all outbound payment-related things to this new module, to help
break up ChannelManager so future payment retries work doesn't increase the
size of ChannelManager.
Wilmer Paulino [Tue, 13 Dec 2022 03:00:06 +0000 (19:00 -0800)]
Avoid redundant broadcast of local commitment transaction
This change follows the rationale of commit 62236c7 and addresses the
last remaining redundant local commitment broadcast.
There's no need to broadcast our local commitment transaction if we've
already seen a confirmed one as it'll be immediately rejected as a
duplicate/conflict.
This will also help prevent dispatching spurious events for bumping
commitment and HTLC transactions through anchor outputs since the
dispatch for said events follows the same flow as our usual commitment
broadcast.
Jeffrey Czyz [Tue, 29 Nov 2022 23:30:56 +0000 (18:30 -0500)]
Builder for creating refunds
Add a builder for creating refunds given a payer_id and other required
fields. Other settings are optional and duplicative settings will
override previous settings. Building produces a semantically valid
`invoice_request` message representing the refund, which then may be
communicated out of band (e.g., via QR code).
Define an interface for BOLT 12 refunds (i.e., an `invoice_request`
message without an `offer_node_id`). A refund is more generally an
"offer for money". While it is encoded using the same TLV streams as an
`invoice_request` message, it has different semantics.
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.
Matt Corallo [Tue, 13 Dec 2022 21:41:49 +0000 (21:41 +0000)]
Correct docs on `generate_channel_keys`
03de0598afd032e9fd5dbbaca0aabc6a367ce4f5 appeared to revert updated
docs due to a rebase error. This reverts the docs on
`generate_channel_keys` to the state they were in prior to that
commit, with one additional doc.
Matt Corallo [Tue, 6 Dec 2022 18:16:46 +0000 (18:16 +0000)]
Clarify docs on `provide_channel_parameters`
Its very confusing to say that LDK will call
`provide_channel_parameters` more than once - its true for a
channel, but not for a given instance. Instead, phrase the docs
with reference to a specific instance, which is much clearer.
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.
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.
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.
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.
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).
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.
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).
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.
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`.
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`.
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.
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).