From b1a27f2abf337d16d036b442e21636a2bfc07d98 Mon Sep 17 00:00:00 2001 From: Chris Waterson Date: Wed, 25 Oct 2023 14:38:54 -0700 Subject: [PATCH] Split update_holder_per_commitment Split `update_holder_per_commitment` into two parts: 1. `update_holder_per_commitment_point`, which we call to retrieve a new commitment point. 2. `update_holder_commitment_secret`, which we call when we're ready to release the commitment secret. This delays releasing the secret until we actually need it for the revoke-and-ack. By doing this, we restore the signer check to its original condition, as well. --- lightning/src/ln/channel.rs | 31 ++++++++++++++++------- lightning/src/ln/channelmanager.rs | 3 ++- lightning/src/util/test_channel_signer.rs | 2 +- 3 files changed, 25 insertions(+), 11 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 4b26487c9..4abb39786 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1284,7 +1284,7 @@ impl ChannelContext where SP::Target: SignerProvider { } /// Retrieves the next commitment point and previous commitment secret from the signer. - pub fn update_holder_per_commitment(&mut self, logger: &L) where L::Target: Logger + pub fn update_holder_per_commitment_point(&mut self, logger: &L) where L::Target: Logger { let transaction_number = self.cur_holder_commitment_transaction_number; let signer = self.holder_signer.as_ref(); @@ -1309,6 +1309,12 @@ impl ChannelContext where SP::Target: SignerProvider { None } }; + } + + pub fn update_holder_commitment_secret(&mut self, logger: &L) where L::Target: Logger + { + let transaction_number = self.cur_holder_commitment_transaction_number; + let signer = self.holder_signer.as_ref(); let releasing_transaction_number = transaction_number + 2; if releasing_transaction_number <= INITIAL_COMMITMENT_NUMBER { @@ -2842,7 +2848,7 @@ impl Channel where self.context.channel_state = ChannelState::FundingSent as u32; } self.context.cur_holder_commitment_transaction_number -= 1; - self.context.update_holder_per_commitment(logger); + self.context.update_holder_per_commitment_point(logger); self.context.cur_counterparty_commitment_transaction_number -= 1; log_info!(logger, "Received funding_signed from peer for channel {}", &self.context.channel_id()); @@ -3355,7 +3361,7 @@ impl Channel where }; self.context.cur_holder_commitment_transaction_number -= 1; - self.context.update_holder_per_commitment(logger); + self.context.update_holder_per_commitment_point(logger); // Note that if we need_commitment & !AwaitingRemoteRevoke we'll call // build_commitment_no_status_check() next which will reset this to RAAFirst. @@ -4045,6 +4051,7 @@ impl Channel where } let raa = if self.context.monitor_pending_revoke_and_ack { + self.context.update_holder_commitment_secret(logger); self.get_last_revoke_and_ack(logger).or_else(|| { log_trace!(logger, "Monitor was pending RAA, but RAA is not available; setting signer_pending_revoke_and_ack"); self.context.signer_pending_revoke_and_ack = true; @@ -4138,9 +4145,14 @@ impl Channel where log_trace!(logger, "Signing unblocked in channel {} at sequence {}", &self.context.channel_id(), self.context.cur_holder_commitment_transaction_number); - if self.context.signer_pending_commitment_point || self.context.signer_pending_released_secret { - log_trace!(logger, "Attempting to update holder per-commitment for pending commitment point and secret..."); - self.context.update_holder_per_commitment(logger); + if self.context.signer_pending_commitment_point { + log_trace!(logger, "Attempting to update holder per-commitment point..."); + self.context.update_holder_per_commitment_point(logger); + } + + if self.context.signer_pending_released_secret { + log_trace!(logger, "Attempting to update holder commitment secret..."); + self.context.update_holder_commitment_secret(logger); } if self.context.channel_state & (ChannelState::PeerDisconnected as u32) != 0 { @@ -4494,6 +4506,7 @@ impl Channel where self.context.monitor_pending_revoke_and_ack = true; None } else { + self.context.update_holder_commitment_secret(logger); self.get_last_revoke_and_ack(logger).map(|raa| { if self.context.signer_pending_revoke_and_ack { log_trace!(logger, "Generated RAA for channel_reestablish; clearing signer_pending_revoke_and_ack"); @@ -6667,7 +6680,7 @@ impl OutboundV1Channel where SP::Target: SignerProvider { where L::Target: Logger { let open_channel = if self.signer_pending_open_channel { - self.context.update_holder_per_commitment(logger); + self.context.update_holder_per_commitment_point(logger); self.get_open_channel(chain_hash.clone()).map(|msg| { log_trace!(logger, "Clearing signer_pending_open_channel"); self.signer_pending_open_channel = false; @@ -7176,7 +7189,7 @@ impl InboundV1Channel where SP::Target: SignerProvider { self.context.channel_id = funding_txo.to_channel_id(); self.context.cur_counterparty_commitment_transaction_number -= 1; self.context.cur_holder_commitment_transaction_number -= 1; - self.context.update_holder_per_commitment(logger); + self.context.update_holder_per_commitment_point(logger); let (counterparty_initial_commitment_tx, funding_signed) = self.context.get_funding_signed_msg(logger); @@ -7224,7 +7237,7 @@ impl InboundV1Channel where SP::Target: SignerProvider { where L::Target: Logger { let accept_channel = if self.signer_pending_accept_channel { - self.context.update_holder_per_commitment(logger); + self.context.update_holder_per_commitment_point(logger); self.generate_accept_channel_message().map(|msg| { log_trace!(logger, "Clearing signer_pending_accept_channel"); self.signer_pending_accept_channel = false; diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index e555b8ef7..ea5001dd8 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -10142,7 +10142,8 @@ where log_info!(args.logger, "Successfully loaded channel {} at update_id {} against monitor at update id {}", &channel.context.channel_id(), channel.context.get_latest_monitor_update_id(), monitor.get_latest_update_id()); - channel.context.update_holder_per_commitment(&args.logger); + channel.context.update_holder_per_commitment_point(&args.logger); + channel.context.update_holder_commitment_secret(&args.logger); if let Some(short_channel_id) = channel.context.get_short_channel_id() { short_to_chan_info.insert(short_channel_id, (channel.context.get_counterparty_node_id(), channel.context.channel_id())); } diff --git a/lightning/src/util/test_channel_signer.rs b/lightning/src/util/test_channel_signer.rs index 2f1fbc748..0f1813ea1 100644 --- a/lightning/src/util/test_channel_signer.rs +++ b/lightning/src/util/test_channel_signer.rs @@ -238,7 +238,7 @@ impl EcdsaChannelSigner for TestChannelSigner { let trusted_tx = self.verify_holder_commitment_tx(commitment_tx, secp_ctx); let state = self.state.lock().unwrap(); let commitment_number = trusted_tx.commitment_number(); - if state.last_holder_revoked_commitment != commitment_number && state.last_holder_revoked_commitment - 1 != commitment_number { + if state.last_holder_revoked_commitment - 1 != commitment_number && state.last_holder_revoked_commitment - 2 != commitment_number { if !self.disable_revocation_policy_check { panic!("can only sign the next two unrevoked commitment numbers, revoked={} vs requested={} for {}", state.last_holder_revoked_commitment, commitment_number, self.inner.commitment_seed[0]) -- 2.39.5