Matt Corallo [Wed, 23 Aug 2023 03:30:56 +0000 (03:30 +0000)]
Remove redundant payment preimag hashing in HTLC claim pipeline
Currently, when we receive an HTLC claim from a peer, we first hash
the preimage they gave us before removing the HTLC, then
immediately pass the preimage to the inbound channel and hash the
preimage again before removing the HTLC and sending our peer an
`update_fulfill_htlc`. This second hash is actually only asserted
on, never used in any meaningful way as we have the htlc data
present in the same code.
Here we simply drop this second hash and move it into a
`debug_assert`.
Matt Corallo [Wed, 23 Aug 2023 02:57:35 +0000 (02:57 +0000)]
Include payment hash in more early payment logs
If a user has issues with a payment, the most obvious thing they'll
do is check logs for the payment hash. Thus, we should ensure our
logs that show a payment's lifecycle include the payment hash and
are emitted (a) as soon as LDK learns of the payment, (b) once the
payment goes out to the peer (which is already reasonably covered
in the commitment transaction building logs) and (c) when the
payment ultimately is fulfilled or fails.
Rather than using a holder_signer of a specific
signer type in Channel and ChannelContext, this
allows us to hold an enum such that depending on
the type of channel, the appropriate signer could
be held in its respective variant.
Doing so required the reparametrization of Channel
from using a Signer to using the SignerProvider
trait. This percolated down to the ChannelManager
and multiple tests.
Now, when accessign various signer methods, there
is a distinction between accessing methods defined
for all signers on ChannelSigner, and accessing
type-specific methods using accessors such as
`as_ecdsa`.
jbesraa [Mon, 21 Aug 2023 19:45:02 +0000 (22:45 +0300)]
Add channel_id to SpendableOutputs event
This will make it possible to
link between SpendableOuts and ChannelMonitor
- change channel_id to option so we dont break upgrade
- remove unused channel_id
- document channel_id
- extract channel id dynamically to pass test
- use contains to check channel_id in test as the events are not ordered
- update docs framing
- specify ldk version channel_id will be introduced in
Co-authored-by: Elias Rohrer <dev@tnull.de>
Update lightning/src/events/mod.rs
Jeffrey Czyz [Mon, 27 Feb 2023 18:10:32 +0000 (12:10 -0600)]
Support signing BOLT 12 invoices in NodeSigner
BOLT 12 messages need to be signed in the following scenarios:
- constructing an InvoiceRequest after scanning an Offer,
- constructing an Invoice after scanning a Refund, and
- constructing an Invoice when handling an InvoiceRequest.
Extend the NodeSigner trait to support signing BOLT 12 invoices such
that it can be used in the latter contexts. The method could be used
in an OffersMessageHandler.
Jeffrey Czyz [Wed, 16 Aug 2023 21:35:16 +0000 (16:35 -0500)]
Expose Offer/InvoiceRequest methods in Invoice
Bolt12Invoice can either be for an Offer (via an InvoiceRequest) or a
Refund. It wraps those types, so expose their methods on both
Bolt12Invoice and UnsignedBolt12Invoice.
Since Refund does not have all the Offer/InvoiceRequest methods, use an
Option return type such that None can returned for refund-based
invoices.
For methods that are duplicated between Offer/InvoiceRequest and
Bolt12Invoice, prefer the (non-Option, if applicable) method from
Bolt12Invoice (e.g., amount_msats, signing_pubkey).
Jeffrey Czyz [Tue, 15 Aug 2023 13:24:40 +0000 (08:24 -0500)]
Macro-ize InvoiceRequest accessors for reuse
Various messages wrap InvoiceRequestContents, which shouldn't be exposed
as it is an implementation detail. Define a macro for InvoiceRequest
accessor methods so that these messages can also define them.
Jeffrey Czyz [Tue, 15 Aug 2023 12:45:06 +0000 (07:45 -0500)]
Macro-ize Offer accessors for reuse
InvoiceRequest wraps OfferContents, which shouldn't be exposed as it is
an implementation detail. Define a macro for Offer accessor methods so
that InvoiceRequest and UnsignedInvoiceRequest can also define them.
Jeffrey Czyz [Fri, 11 Aug 2023 18:11:14 +0000 (13:11 -0500)]
Wrap KeyPair by DerivedSigningPubkey
InvoiceBuilder is parameterized by a SigningPubkeyStrategy, either
ExplicitSigningPubkey and DerivedSigningPubkey. It also holds an
Option<KeyPair>, which may be None and Some for those strategies,
respectively. This leads to methods for InvoiceBuilder parameterized by
DerivedSigningPubkey needing to blindly unwrap the Option<KeyPair>.
Instead, have DerivedSigningPubkey wrap KeyPair.
Jeffrey Czyz [Mon, 27 Feb 2023 20:23:05 +0000 (14:23 -0600)]
TaggedHash for BOLT 12 signing function
The function used to sign BOLT 12 messages only takes a message digest.
This doesn't allow signers to independently verify the message before
signing nor does it allow them to derive the necessary signing keys, if
needed.
Introduce a TaggedHash wrapper for a message digest, which each unsigned
BOLT 12 message type constructs upon initialization. Change the signing
function to take AsRef<TaggedHash>, which each unsigned type implements.
This allows the signing function to take any unsigned message and obtain
its tagged hash.
Chris Waterson [Sun, 6 Aug 2023 22:39:21 +0000 (15:39 -0700)]
Provide the HTLCs that settled a payment.
Creates a new `events::ClaimedHTLC` struct that contains the relevant
information about a claimed HTLC; e.g., the channel it arrived on, its ID, the
amount of the HTLC, the overall amount of the payment, etc. Adds appropriate
serialization support.
Adds a `Vec<events::ClaimedHTLC>` to the `ClaimingPayment`
structure. Populates this when creating the struct by converting the
`payment.htlcs` (which are `ClaimingHTLC` structs) into `event::ClaimedHTLC`
structs. This is a straightforward transformation.
Adds a `Vec<events::ClaimedHTLC>` to the `events::Event::PaymentClaimed`
enum. This is populated directly from the `ClaimingPayment`'s `htlcs` vec.
Blinded paths: support constructing onion keys + handling onion errors
We don't bother actually parsing errors from within a blinded path, since all
errors should be wiped by the introduction node by the time it gets back to us
(the sender).
Matt Corallo [Thu, 17 Aug 2023 22:34:24 +0000 (22:34 +0000)]
Work around LND bug 6039
LND hasn't properly handled shutdown messages ever, and
force-closes any time we send one while HTLCs are still present.
The issue is tracked at
https://github.com/lightningnetwork/lnd/issues/6039 and has had
multiple patches to fix it but none so far have managed to land
upstream. The issue appears to be very low priority for the LND
team despite being marked "P1".
We're not going to bother handling this in a sensible way, instead
simply repeated the Shutdown message on repeat until morale
improves.
Matt Corallo [Fri, 28 Jul 2023 05:30:24 +0000 (05:30 +0000)]
Delay RAA-after-next processing until PaymentSent is are handled
In 0ad1f4c943bdc9037d0c43d1b74c745befa065f0 we fixed a nasty bug
where a failure to persist a `ChannelManager` faster than a
`ChannelMonitor` could result in the loss of a `PaymentSent` event,
eventually resulting in a `PaymentFailed` instead!
As noted in that commit, there's still some risk, though its been
substantially reduced - if we receive an `update_fulfill_htlc`
message for an outbound payment, and persist the initial removal
`ChannelMonitorUpdate`, then respond with our own
`commitment_signed` + `revoke_and_ack`, followed by receiving our
peer's final `revoke_and_ack`, and then persist the
`ChannelMonitorUpdate` generated from that, all prior to completing
a `ChannelManager` persistence, we'll still forget the HTLC and
eventually trigger a `PaymentFailed` rather than the correct
`PaymentSent`.
Here we fully fix the issue by delaying the final
`ChannelMonitorUpdate` persistence until the `PaymentSent` event
has been processed and document the fact that a spurious
`PaymentFailed` event can still be generated for a sent payment.
The original fix in 0ad1f4c943bdc9037d0c43d1b74c745befa065f0 is
still incredibly useful here, allowing us to avoid blocking the
first `ChannelMonitorUpdate` until the event processing completes,
as this would cause us to add event-processing delay in our general
commitment update latency. Instead, we ultimately race the user
handling the `PaymentSent` event with how long it takes our
`revoke_and_ack` + `commitment_signed` to make it to our
counterparty and receive the response `revoke_and_ack`. This should
give the user plenty of time to handle the event before we need to
make progress.
Sadly, because we change our `ChannelMonitorUpdate` semantics, this
change requires a number of test changes, avoiding checking for a
post-RAA `ChannelMonitorUpdate` until after we process a
`PaymentSent` event. Note that this does not apply to payments we
learned the preimage for on-chain - ensuring `PaymentSent` events
from such resolutions will be addressed in a future PR. Thus, tests
which resolve payments on-chain switch to a direct call to the
`expect_payment_sent` function with the claim-expected flag unset.
Elias Rohrer [Thu, 17 Aug 2023 08:50:23 +0000 (10:50 +0200)]
Don't require import of internal macros
Commit f560320b introduced changes that require users of
`impl_writeable_tlv_based`/`impl_writeable_tlv_based_enum` to import
`_encode_varint_length_prefixed_tlv` and `alloc` separately.
Here, we take care of the necessary imports in
`_encode_varint_length_prefixed_tlv` itself, allowing users to just
import the `impl_writeable_tlv_based` variant they need.
Alec Chen [Wed, 16 Aug 2023 19:02:21 +0000 (14:02 -0500)]
Address custom HTLC TLV fixups
Don't collect iterators to compare, minorly simplify encoding the
keysend TLV, combine the _encode_tlv_stream variants to check that the
ordering of TLVs is correct including custom TLVs.
Matt Corallo [Tue, 15 Aug 2023 19:19:03 +0000 (19:19 +0000)]
Correct test struct initialization ordering
When reloading a node in the test framework, we end up with a new
`ChannelManager` that has references to various test util structs.
In order for the tests to compile reliably in the face of unrelated
changes, those test structs need to always be initialized before
both the new but also the original `ChannelManager`.
Matt Corallo [Tue, 15 Aug 2023 22:31:31 +0000 (22:31 +0000)]
Drop now-unused `outbound_scid_alias` param to channel constructor
01847277b957ec94129141a7e7439ae539c094f1 switched around the logic
for inbound channel construction to assign the outbound SCID alias
after constructing the `InboundV1Channel` object. Thus, the SCID
alias argument is now unused, and we remove it here.
Matt Corallo [Tue, 15 Aug 2023 22:22:45 +0000 (22:22 +0000)]
Ensure we wipe pending un-accepted channel requests on err/discon.
If we have a pending inbound un-accepted channel but receive an
error message for it from our peer, or our peer disconnects, we
should remove the pending entry, ensuring any attempts to accept
it fail.
Matt Corallo [Tue, 15 Aug 2023 19:17:31 +0000 (19:17 +0000)]
Correct lifetimes on `_reload_node`
For some reason an unrelated PR caused all our tests with
`reload_node` calls to fail to compile. This is due, in part, to
the lifetimes on `reload_node` implying that the new and original
`ChannelManager` (or some of the structs they reference) must live
for the same lifetime.
This fixes that issue by correcting the lifetimes to be consistent
across `Node` and `_reload_node`.
Willem Van Lint [Sun, 6 Aug 2023 00:26:49 +0000 (17:26 -0700)]
Remove AvailableBalances::balance_msat
The ChannelMonitor::get_claimable_balances method provides a more
straightforward approach to the balance of a channel, which satisfies
most use cases. The computation of AvailableBalances::balance_msat is
complex and originally had a different purpose that is not applicable
anymore.
Chris Waterson [Tue, 18 Jul 2023 14:47:44 +0000 (07:47 -0700)]
Wait to create a channel until after accepting.
Create a new table in 'peer_state' to maintain unaccepted inbound
channels; i.e., a channel for which we've received an 'open_channel'
message but that user code has not yet confirmed for acceptance. When
user code accepts the channel (e.g. via 'accept_inbound_channel'),
create the channel object and as before.
Currently, the 'open_channel' message eagerly creates an
InboundV1Channel object before determining if the channel should be
accepted. Because this happens /before/ the channel has been assigned
a user identity (which happens in the handler for OpenChannelRequest),
the channel is assigned a random user identity. As part of the
creation process, the channel's cryptographic material is initialized,
which then uses this randomly generated value for the user's channel
identity e.g. in SignerProvider::generate_channel_keys_id.
By delaying the creation of the InboundV1Channel until /after/ the
channel has been accepted, we ensure that we defer cryptographic
initialization until we have given the user the opportunity to assign
an identity to the channel.
Alec Chen [Thu, 8 Jun 2023 17:08:25 +0000 (12:08 -0500)]
Enforce explicit claims on payments with even custom TLVs
Because we don't know which custom TLV type numbers the user is
expecting (and it would be cumbersome for them to tell us), instead of
failing unknown even custom TLVs on deserialization, we accept all
custom TLVs, and pass them to the user to check whether they recognize
them and choose to fail back if they don't. However, a user may not
check for custom TLVs, in which case we should reject any even custom
TLVs as unknown.
This commit makes sure a user must explicitly accept a payment with
even custom TLVs, by (1) making the default
`ChannelManager::claim_funds` fail if the payment had even custom TLVs
and (2) adding a new function
`ChannelManager::claim_funds_with_known_custom_tlvs` that accepts them.
This commit also refactors our custom TLVs test and updates various
documentation to account for this.
Alec Chen [Thu, 8 Jun 2023 04:17:09 +0000 (23:17 -0500)]
Add `FailureCode::InvalidOnionPayload` variant
When a user decodes custom TLVs, if they fail to recognize even type
numbers they should fail back with the correct failure code and fail
data. This new variant adds the proper failure variant for the user to
pass into `ChannelManager::fail_htlc_backwards_with_reason`.
Note that the enum discriminants were removed because when adding a
struct variant we can no longer make use of the discriminant through
casting like we previously did, and instead have to manually define the
associated failure code anyway.
Alec Chen [Fri, 19 May 2023 20:37:47 +0000 (15:37 -0500)]
Drop non-matching custom TLVs when receiving MPP
Upon receiving multiple payment parts with custom TLVs, we fail payments
if they have any non-matching or missing even TLVs, and otherwise just
drop non-matching TLVs if they're odd.
Alec Chen [Fri, 19 May 2023 04:16:29 +0000 (23:16 -0500)]
De/serialize custom TLVs on `{Inbound,Outbound}OnionPayload`
When serialized, the TLVs in `OutboundOnionPayload`, unlike a normal
TLV stream, are prefixed with the length of the stream. To allow a user
to add arbitrary custom TLVs, we aren't able to communicate to our
serialization macros exactly which fields to expect, so this commit
adds new macro variants to allow appending an extra set of bytes (and
modifying the prefixed length accordingly).
Because the keysend preimage TLV has a type number in the custom type
range, and a user's TLVs may have type numbers above and/or below
keysend's type number, and because TLV streams must be serialized in
increasing order by type number, this commit also ensures the keysend
TLV is properly sorted/serialized amongst the custom TLVs.
Alec Chen [Tue, 16 May 2023 22:56:28 +0000 (17:56 -0500)]
Allow users to provide custom TLVs through `RecipientOnionFields`
Custom TLVs allow users to send extra application-specific data with
a payment. These have the additional flexibility compared to
`payment_metadata` that they don't have to reflect recipient generated
data provided in an invoice, in which `payment_metadata` could be
reused.
We ensure provided type numbers are unique, increasing, and within the
experimental range with the `RecipientOnionFields::with_custom_tlvs`
method.
Jeffrey Czyz [Fri, 21 Oct 2022 05:12:42 +0000 (00:12 -0500)]
Smooth out channel liquidity bounds decay
Decaying the channel liquidity bounds by a half life can result in a
large decrease, which may have an oscillating affect on whether a
channel is retried. Approximate an additional three-quarter life when
half of the next half life has passed to help smooth out the decay.
Vladimir Fomene [Fri, 30 Jun 2023 10:03:26 +0000 (13:03 +0300)]
Add counterparty_node_id & channel_capacity to ChannelClosed
The current ChannelClosed event does not let
you know the counterparty during a channel close
event. This change adds the counterparty_node_id
and the channel_capacity to the ChannelClosed event.
This helps users to have more context during a
channel close event. Solves #2343
Matt Corallo [Tue, 8 Aug 2023 04:19:51 +0000 (04:19 +0000)]
Make CI build -> test flows test -> build.
Doing `cargo test` causes us to build both the crate(s) themselves
and the test binaries, which depend on the main builds. However, it
can start building the test code while the actual program code for
the main crate(s) themselves are being built, making a
build -> test flow slightly slower than test -> build.
Its not really a huge deal, but I'm using `ci/ci-tests.sh` more
locally and it meaningfully changes the time-to-test-run.
Matt Corallo [Tue, 8 Aug 2023 04:15:20 +0000 (04:15 +0000)]
Scope payment preimage in do_test_keysend_payments
b0d4ab8cf8c93740674a00546be38a1a5f0a83c3 fixed a nasty bug where
we'd failed to include the payment preimage in keysend onions at
all. Ultimately, this was a test failure - the existing test suite
should which did keysend payments were not structured in a way that
would fail in this case, instead using the same preimage variable
both for sending and receiving.
Here we improve the main keysend test tweaked by b0d4ab8cf8c9374067
to make absolutely sure it cannot work if the preimage doesn't come
from the onion. We make the payment preimage on the sending side a
variable inside a scope which only exists for the send call. Once
that scope completes the payment preimage only exists in the
sending `ChannelManager`, which must have put it in the onion in
order for the receiving node to have it.
Receive payment onions as new InboundPayload instead of OnionHopData
To support route blinding, we want to split OnionHopData into two separate
structs, one for inbound onions and one for outbound onions. This is because
blinded payloads change the fields present in the onion hop data struct based
on whether we're sending vs receiving (outbound onions include encrypted blobs,
inbound onions can decrypt those blobs and contain the decrypted fields
themselves).
In upcoming commits, we'll add variants for blinded payloads to the new
InboundPayload enum.