Exposing ChannelPhase in ChannelManager has led to verbose match
statements, which need to be modified each time a ChannelPhase is added.
Making ChannelPhase an implementation detail of Channel would help avoid
this.
As a step in this direction, define a conversion from OutboundV1Channel
to ChannelPhase (to be renamed Channel).
Exposing ChannelPhase in ChannelManager has led to verbose match
statements, which need to be modified each time a ChannelPhase is added.
Making ChannelPhase an implementation detail of Channel would help avoid
this.
As a step in this direction, define a conversion from Channel (to be
renamed FundedChannel) to ChannelPhase (to be renamed Channel).
Exposing ChannelPhase in ChannelManager has led to verbose match
statements, which need to be modified each time a ChannelPhase is added.
Making ChannelPhase an implementation detail of Channel would help avoid
this.
As a step in this direction, update ChannelManager methods to use
methods on ChannelPhase for obtaining the appropriate V1 channel types.
Exposing ChannelPhase in ChannelManager has led to verbose match
statements, which need to be modified each time a ChannelPhase is added.
Making ChannelPhase an implementation detail of Channel would help avoid
this.
As a step in this direction, update the convert_chan_phase_err macro to
use ChannelPhase::as_funded_mut instead.
Exposing ChannelPhase in ChannelManager has led to verbose match
statements, which need to be modified each time a ChannelPhase is added.
Making ChannelPhase an implementation detail of Channel would help avoid
this.
As a step in this direction, update ChannelManager methods to use
ChannelPhase::as_unfunded_v2_mut and ChannelPhase::into_unfunded_v2
methods.
Exposing ChannelPhase in ChannelManager has led to verbose match
statements, which need to be modified each time a ChannelPhase is added.
Making ChannelPhase an implementation detail of Channel would help avoid
this.
As a step in this direction, update ChannelManager::timer_tick_occurred
to use ChannelPhase::as_funded_mut and a new
ChannelPhase::unfunded_context_mut method.
Exposing ChannelPhase in ChannelManager has led to verbose match
statements, which need to be modified each time a ChannelPhase is added.
Making ChannelPhase an implementation detail of Channel would help avoid
this.
As a step in this direction, update ChannelManager::handle_error to use
a new ChannelPhase::maybe_handle_error_without_close.
Exposing ChannelPhase in ChannelManager has led to verbose match
statements, which need to be modified each time a ChannelPhase is added.
Making ChannelPhase an implementation detail of Channel would help avoid
this.
As a step in this direction, update ChannelManager::peer_connected to
use ChannelPhase::as_funded_mut and a new
ChannelPhase::maybe_get_open_channel method.
Exposing ChannelPhase in ChannelManager has led to verbose match
statements, which need to be modified each time a ChannelPhase is added.
Making ChannelPhase an implementation detail of Channel would help avoid
this.
As a step in this direction, update ChannelManager::peer_disconnected to
use ChannelPhase::as_funded_mut and a new ChannelPhase::is_resumable
method.
Exposing ChannelPhase in ChannelManager has led to verbose match
statements, which need to be modified each time a ChannelPhase is added.
Making ChannelPhase an implementation detail of Channel would help avoid
this.
As a step in this direction, update ChannelManager::signer_unblocked to
use ChannelPhase::as_funded and a new method on ChannelPhase dispatching
to each variant's signer_maybe_unblocked method.
Exposing ChannelPhase in ChannelManager has led to verbose match
statements, which need to be modified each time a ChannelPhase is added.
Making ChannelPhase an implementation detail of Channel would help avoid
this.
As a step in this direction, update ChannelManager::internal_tx_abort to
use ChannelPhase::is_funded and a new ChannelPhase::as_unfunded_v2_mut
method.
Exposing ChannelPhase in ChannelManager has led to verbose match
statements, which need to be modified each time a ChannelPhase is added.
Making ChannelPhase an implementation detail of Channel would help avoid
this.
As a step in this direction, rewrite ChannelManager's
unfunded_channel_count method to use ChannelPhase::as_funded and a new
ChannelPhase::as_unfunded_v2 method.
Exposing ChannelPhase in ChannelManager has led to verbose match
statements, which need to be modified each time a ChannelPhase is added.
Making ChannelPhase an implementation detail of Channel would help avoid
this.
As a step in this direction, introduce ChannelPhase::is_funded for use
in ChannelManager when a Channel (to be later renamed FundedChannel)
needs to be tested for.
Exposing ChannelPhase in ChannelManager has led to verbose match
statements, which need to be modified each time a ChannelPhase is added.
Making ChannelPhase an implementation detail of Channel would help avoid
this.
As a step in this direction, introduce ChannelPhase::as_funded and
ChannelPhase::as_funded_mut for use in ChannelManager when a Channel (to
be later renamed FundedChannel) is needed.
Pending v2 channels will need to be broken up into separate phases for
constructing and signing the funding transaction. To avoid increasing
the number of phases, combine the InboundV2Channel and OutboundV2Channel
types so that the can be used in one phase. Whether the channel is
inbound or outbound can be inferred from the ChannelContext.
We should update the return types on the signing methods here as
well, but we should at least start by documenting which methods are
async and which are not.
Once we complete async support for `get_per_commitment_point`, we
can change the return types as most things in the channel signing
traits will be finalized.
bcaba29f92 started returning
pre-built `Route`s from the router in the `chanmon_consistency`
fuzzer. In doing so, it didn't properly fill in the `route_parms`
field which is expected to match the requested parameters. This
causes a debug assertion when sending.
Here we fix this by setting the correct `route_params`.
Now that the core features required for `async_signing` are in
place, we can go ahead and expose it publicly (rather than behind a
a `cfg`-flag). We still don't have full async support for
`get_per_commitment_point`, but only one case in channel
reconnection remains. The overall logic may still have some
hiccups, but its been in use in production at a major LDK user for
some time now. Thus, it doesn't really make sense to hide behind a
`cfg`-flag, even if the feature is only 99% complete. Further, the
new paths exposed are very restricted to signing operations that
run async, so the risk for existing users should be incredibly low.
This moves the common `if during_startup { push background event }
else { apply ChannelMonitorUpdate }` pattern by simply inlining it
in `handle_new_monitor_update`.
One of the largest gaps in our async persistence functionality has
been preimage (claim) updates to closed channels. Here we finally
implement support for this (for updates which are generated during
startup).
Thanks to all the work we've built up over the past many commits,
this is a fairly straightforward patch, removing the
immediate-completion logic from `claim_mpp_part` and adding the
required in-flight tracking logic to
`apply_post_close_monitor_update`.
Like in the during-runtime case in the previous commit, we sadly
can't use the `handle_new_monitor_update` macro wholesale as it
handles the `Channel` resumption as well which we don't do here.
On startup, we walk the preimages and payment HTLC sets on all our
`ChannelMonitor`s, re-claiming all payments which we recently
claimed. This ensures all HTLCs in any claimed payments are claimed
across all channels.
In doing so, we expect to see the same payment multiple times,
after all it may have been received as multiple HTLCs across
multiple channels. In such cases, there's no reason to redundantly
claim the same set of HTLCs again and again. In the current code,
doing so may lead to redundant `PaymentClaimed` events, and in a
coming commit will instead cause an assertion failure.
One of the largest gaps in our async persistence functionality has
been preimage (claim) updates to closed channels. Here we finally
implement support for this (for updates at runtime).
Thanks to all the work we've built up over the past many commits,
this is a well-contained patch within `claim_mpp_part`, pushing
the generated `ChannelMonitorUpdate`s through the same pipeline we
use for open channels.
Sadly we can't use the `handle_new_monitor_update` macro wholesale
as it handles the `Channel` resumption as well which we don't do
here.
In d1c340a0e1 we added support in
`handle_new_monitor_update!` for handling updates without dropping
locks.
In the coming commits we'll start handling `ChannelMonitorUpdate`s
"like normal" for updates against closed channels. Here we set up
the first step by adding a new `POST_CHANNEL_CLOSE` variant on
`handle_new_monitor_update!` which attempts to handle the
`ChannelMonitorUpdate` and handles completion actions if it
finishes immediately, just like the pre-close variant.
In c99d3d785d we added a new
`apply_post_close_monitor_update` method which takes a
`ChannelMonitorUpdate` (possibly) for a channel which has been
closed, sets the `update_id` to the right value to keep our updates
well-ordered, and then applies it.
Setting the `update_id` at application time here is fine - updates
don't really have an order after the channel has been closed, they
can be applied in any order - and was done for practical reasons
as calculating the right `update_id` at generation time takes a
bit more work on startup, and was impossible without new
assumptions during claim.
In the previous commit we added exactly the new assumption we need
at claiming (as it's required for the next few commits anyway), so
now the only thing stopping us is the extra complexity.
In the coming commits, we'll move to tracking post-close
`ChannelMonitorUpdate`s as in-flight like any other updates, which
requires having an `update_id` at generation-time so that we know
what updates are still in-flight.
Thus, we go ahead and eat the complexity here, creating
`update_id`s when the `ChannelMonitorUpdate`s are generated for
closed-channel updates, like we do for channels which are still
live.
We also ensure that we always insert `ChannelMonitorUpdate`s in the
pending updates set when we push the background event, avoiding a
race where we push an update as a background event, then while its
processing another update finishes and the post-update actions get
run.
Here we make a test that disables a channel signer's ability
to return commitment points upon being first derived for a channel.
We also fit in a couple cleanups: removing a comment referencing a
previous design with a `HolderCommitmentPoint::Uninitialized` variant,
as well as adding coverage for updating channel maps in async closing
signed.
Here we handle the case where our signer is pending the next commitment
point when we try to send channel ready. We set a flag to remember to
send this message when our signer is unblocked. This follows the same
general pattern as everywhere else where we're waiting on a commitment
point from the signer in order to send a message.
Similar to `open_channel`, if a signer cannot provide a commitment point
immediately, we set a flag to remember we're waiting for a point to send
`accept_channel`. We make sure to get the first two points before moving
on, so when we advance our commitment we always have a point available.
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from #3152.
In the event that a signer cannot provide a commitment point
immediately, we set a flag to remember we're waiting for this before we
can send `open_channel`. We make sure to get the first two commitment
points, so when we advance commitments, we always have a commitment
point available.
When initializing a context, we set the `signer_pending_open_channel`
flag to false, and leave setting this flag for where we attempt to
generate a message.
When checking to send messages when a signer is unblocked, we must
handle both when we haven't gotten any commitment point, as well as when
we've gotten the first but not the second point.
Following a previous commit adding `HolderCommitmentPoint` elsewhere, we
make the transition to use those commitment points and remove the
existing one.
We are choosing to move the `HolderCommitmentPoint` (the struct that
tracks commitment points retrieved from the signer + the commitment
number) to handle channel establishment, where we have no commitment
point at all. Previously we introduced this struct to track when we were
pending a commitment point (because of an async signer) during normal
channel operation, which assumed we always had a commitment point to
start out with.
Intiially we tried to add an `Uninitialized` variant
that held no points, but that meant that we needed to handle that case
everywhere which left a bunch of scary/unnecessary unwraps/expects.
Instead, we just hold an optional `HolderCommitmentPoint` struct
on our unfunded channels, and a non-optional `HolderCommitmentPoint`
for funded channels.
This commit starts that transition. A following commit will remove the
holder commitment point from the current `ChannelContext`.
This also makes small fixups to the comments on the
HolderCommitmentPoint variants.
Now that we track the latest `ChannelMonitorUpdate::update_id` for
each closed channel in
`PeerState::closed_channel_monitor_update_ids`, we should always
have a `PeerState` entry for the channel counterparty any time we
go to claim an HTLC on a channel, even if its closed.
Here we make this a hard assertion as we'll need to access that
`PeerState` in the coming commits to track in-flight updates
against closed channels.
UnknownPaymentContext is used when payment::ReceiveTlvs doesn't contain
a PaymentContext. This is only needed for a legacy BlindedPaymentPath.
Since these paths a short-lived, UnknownPaymentContext is no longer
needed. Remove it and require that payment::ReceiveTlvs always contains
a PaymentContext.
Any such path would fail authentication since the payment::ReceiveTlvs
would be missing an HMAC and Nonce, so this is a good time to remove
UnknownPaymentContext.
When receiving a payment over a BlindedPaymentPath, a PaymentContext is
included but was not authenticated. The previous commit adds an HMAC of
the payment::ReceiveTlvs (which contains the PaymentContext) and the
nonce used to create the HMAC. This commit verifies the authenticity
when parsing the InboundOnionPayload. This prevents a malicious actor
from for forging it.
In order to authenticate a PaymentContext, an HMAC and Nonce must be
included along with it in payment::ReceiveTlvs. Compute the HMAC when
constructing a BlindedPaymentPath and include it in the recipient's
BlindedPaymentTlvs. Authentication will be added in an upcoming commit.
Now that NodeSigner::get_inbound_payment_key returns an ExpandedKey
instead of KeyMaterial, the latter is no longer needed. Remove
KeyMaterial and replace its uses with [u8; 32].
NodeSinger::get_inbound_payment_key_material returns KeyMaterial, which
is used for constructing an ExpandedKey. Change the trait to return an
ExpandedKey directly instead. This allows for direct access to the
ExpandedKey when a NodeSigner referenced is available. Otherwise, it
would either need to be reconstructed or passed in separately.
When receiving a PaymentContext from a blinded payment, the context must
be authenticated. Otherwise, the context can be forged and would appear
within a PaymentPurpose. Add functions for constructing and verifying an
HMAC for the ReceiveTlvs, which contains the PaymentContext.
