]> git.bitcoin.ninja Git - rust-lightning/commit
Support async get_per_commitment_point, release_commitment_secret
authorChris Waterson <waterson@gmail.com>
Sat, 7 Oct 2023 15:54:07 +0000 (08:54 -0700)
committerChris Waterson <waterson@gmail.com>
Wed, 25 Oct 2023 16:40:31 +0000 (09:40 -0700)
commit6846f14f6fc430a50f454297280bfe7128c6325a
tree22c1f7be490568cfcd4b5ab5c07ba7f7b936bdab
parent1edfdb209732c0dfda2443a33dc1d39c903486f8
Support async get_per_commitment_point, release_commitment_secret

Apologies in advance for the hair-ball. Mostly just wanted to get a 50,000 foot
overview to see if things were headed in the right direction. If things look
okay, I can take a step back and chop this into more modestly-sized PRs.

In general, this PR adds state to the context to allow a `ChannelSigner`
implementation to respond asynchronously to the `get_per_commitment_point`,
`release_commitment_secret`, and `sign_counterparty_commitment` methods, which
are the main signer methods that are called during channel setup and normal
operation.

These changes seem to work as advertised during normal channel operation
(creation and payment exchange), and do not obviously fail during channel
re-establishment or across node restart. That said, there are a lot more test
scenarios to evaluate here.

The details are below.

Adds the RevokeAndAck and RAACommitemnt ordering to the `SignerResumeUpdates`
struct that is returned from `signer_maybe_unblocked`.

Adds `signer_maybe_unblocked` for both inbound and outbound unfunded channel
states. We need these now because `get_per_commitment_point` is asynchronous
-- and necessary for us to proceed out of the unfunded state into normal
operation. Adds appropriate `SignerResumeUpdates` classes for both inbound and
outbound unfunded states.

Maintains `cur_holder_commitment_point` and `prev_holder_commitment_secret` on
the channel context. By making these part of the context state, we can access
them at any point we need them without requiring a signer call to
regenerate. These are updated appropriately throughout the channel state
machine by calling a new context method `update_holder_per_commitment`.

Add several flags to indicate messages that may now be pending the remote
signer unblocking:

- `signer_pending_revoke_and_ack`, set when we're waiting to send the
  revoke-and-ack to the counterparty. This might _not_ just be us waiting on
  the signer; for example, if the commitment order requires sending the RAA
  after the commitment update, then even though we _might_ be able to generate
  the RAA (e.g., because we have the secret), we will not do so while the
  commitment update is pending (e.g., because we're missing the latest point
  or do not have a signed counterparty commitment).

- `signer_pending_channel_ready`, set when we're waiting to send the
  channel-ready to the counterparty.

- `signer_pending_commitment_point`, set when we're waiting for the signer to
  return us the commitment point for the current state.

- `signer_pending_released_secret`, set when we're waiting for the signer to
  release the commitment secret for the previous state.

This state (current commitment point, previous secret, flags) is persisted in
the channel monitor, and restored when the channel is unpickled.

When a monitor update completes, we may still be pending results from the
remote signer. If that is the case, we ensure that we correctly maintain the
above state flags before the channel state is resumed. For example, if the
_monitor_ is pending a commitment signed, but we were not able to retrieve the
commitment update from the signer, then we ensure that
`signer_pending_commitment_update` is set.

When unblocking the signer, we need to ensure that we honor message ordering
constraints. For the commitment update and revoke-and-ack, ensure that we can
honor the context's current `resend_order`. For example, assume we must send
the RAA before the CU, and could potentially send a CU because we have the
commitment point and a signed counterparty commitment transaction. _But_, we
have not yet received the previous state's commitment secret. In this case, we
ensure that no messages are emitted until the commitment secret is released,
at which point the signer-unblocked call will emit both messages.

A similar situation exists with the `channel_ready` message and the
`funding_signed` / `funding_created` messages at channel startup: we make sure
that we don't emit `channel_ready` before the funding message.

There is at least one unsolved problem here: during channel re-establishment,
we need to request an _arbitrary_ commitment point from the signer in order to
verify that the counterparty's giving us a legitimate secret. For the time
being, I've simply commented out this check; however, this is not a viable
solution. There are a few options to consider here:

1. We could require that an asynchronous signer _cache_ the previous
   commitment points, so that any such request must necessarily succeed
   synchronously.

2. We could attempt to factor this method as to admit an asynchronous response
   that would restart the channel re-establish once the commitment point has
   been provided by the signer and the counterparty's secret can be verified.

The former places some additional burden on a remote signer (albeit minimal)
and seems reasonable to me.

As for testing...

For testing asynchronous channel signing, replaces the simple boolean
("everything is on, or everything is off") with flags that let us toggle
different signing methods on and off. This lets us (sort of) simulate the
signer returning responses in an arbitrary order.

Adds a fully-exploded "send a one-hop payment" test to the async test
suite. At each step, simulate the async signer being unavailable, and then
unblocking. Vary the order to check all possible orderings of
`get_per_commitment_point`, `release_commitment_secret`, and
`sign_counterparty_commitment` being provided by the signer.

But there is a lot more to be done here. Many of the odd-ball cases in the PR
_aren't_ covered by unit tests and were instead uncovered by running the code
in situ with an LND counterparty. So there are a lot more tests to write here.
fuzz/src/chanmon_consistency.rs
lightning/src/chain/channelmonitor.rs
lightning/src/chain/onchaintx.rs
lightning/src/ln/async_signer_tests.rs
lightning/src/ln/channel.rs
lightning/src/ln/channelmanager.rs
lightning/src/ln/functional_test_utils.rs
lightning/src/ln/functional_tests.rs
lightning/src/sign/mod.rs
lightning/src/util/test_channel_signer.rs