Skip to content

Commit 7165d6d

Browse files
committed
Fix panic on receiving channel_ready after 1st commitment update
`cur_counterparty_commitment_transaction_number` starts at `INITIAL_COMMITMENT_NUMBER`, gets decremented once when the initial `channel_ready` message is received, and gets decremented a second time when the first `revoke_and_ack` is received, revoking the first counterparty commitment point. At this point, `counterparty_prev_commitment_point` points to the non-revoked second commitment point. If we then process a second `channel_ready`, we check the `cur_counterparty_commitment_transaction_number` number, see that if is `INITIAL_COMMITMENT_NUMBER - 2` (i.e. not `- 1`) and assume that the *second* commitment number has been revoked (by `expect`ing `CounterpartyCommitmentSecrets::get_secret` with `INITIAL_COMMITMENT_NUMBER - 1`). This `expect` panic's. As the second commitment point has not yet been revoked, we should fetch it from `counterparty_prev_commitment_point`, which we do here, adding a test which failed on the previous code as well. Found by the `full_stack_target` fuzzer.
1 parent d8a20ed commit 7165d6d

File tree

2 files changed

+12
-0
lines changed

2 files changed

+12
-0
lines changed

lightning/src/ln/channel.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2426,6 +2426,11 @@ impl<Signer: Sign> Channel<Signer> {
24262426
// If they haven't ever sent an updated point, the point they send should match
24272427
// the current one.
24282428
self.counterparty_cur_commitment_point
2429+
} else if self.cur_counterparty_commitment_transaction_number == INITIAL_COMMITMENT_NUMBER - 2 {
2430+
// If we've advanced the commitment number once, the second commitment point is
2431+
// at `counterparty_prev_commitment_point`, which is not yet revoked.
2432+
debug_assert!(self.counterparty_prev_commitment_point.is_some());
2433+
self.counterparty_prev_commitment_point
24292434
} else {
24302435
// If they have sent updated points, channel_ready is always supposed to match
24312436
// their "first" point, which we re-derive here.

lightning/src/ln/priv_short_conf_tests.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,13 @@ fn test_routed_scid_alias() {
245245
check_added_monitors!(nodes[0], 1);
246246

247247
pass_along_route(&nodes[0], &[&[&nodes[1], &nodes[2]]], 100_000, payment_hash, payment_secret);
248+
249+
as_channel_ready.short_channel_id_alias = Some(0xeadbeef);
250+
nodes[2].node.handle_channel_ready(&nodes[1].node.get_our_node_id(), &as_channel_ready);
251+
// Note that we always respond to a channel_ready with a channel_update. Not a lot of reason
252+
// to bother updating that code, so just drop the message here.
253+
get_event_msg!(nodes[2], MessageSendEvent::SendChannelUpdate, nodes[1].node.get_our_node_id());
254+
248255
claim_payment(&nodes[0], &[&nodes[1], &nodes[2]], payment_preimage);
249256

250257
// Now test that if a peer sends us a second channel_ready after the channel is operational we

0 commit comments

Comments
 (0)