Split update_holder_per_commitment
authorChris Waterson <waterson@gmail.com>
Wed, 25 Oct 2023 21:38:54 +0000 (14:38 -0700)
committerChris Waterson <waterson@gmail.com>
Wed, 25 Oct 2023 22:10:37 +0000 (15:10 -0700)
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
lightning/src/ln/channelmanager.rs
lightning/src/util/test_channel_signer.rs

index 4b26487c90853002c15e029bfe2f8b532110bf65..4abb39786330d3e5c4af9f07de4a03a30527ac7a 100644 (file)
@@ -1284,7 +1284,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
        }
 
        /// Retrieves the next commitment point and previous commitment secret from the signer.
-       pub fn update_holder_per_commitment<L: Deref>(&mut self, logger: &L) where L::Target: Logger
+       pub fn update_holder_per_commitment_point<L: Deref>(&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<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
                                None
                        }
                };
+       }
+
+       pub fn update_holder_commitment_secret<L: Deref>(&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<SP: Deref> Channel<SP> 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<SP: Deref> Channel<SP> 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<SP: Deref> Channel<SP> 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<SP: Deref> Channel<SP> 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<SP: Deref> Channel<SP> 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<SP: Deref> OutboundV1Channel<SP> 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<SP: Deref> InboundV1Channel<SP> 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<SP: Deref> InboundV1Channel<SP> 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;
index e555b8ef737d02a9aaea09376f61a2f242e586a0..ea5001dd847c84d4498dc66cca77372bc10590bf 100644 (file)
@@ -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()));
                                        }
index 2f1fbc748987b9b1fb51be272433055d393251a0..0f1813ea16ce6207970c9e4413b54fa1aef9cfb3 100644 (file)
@@ -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])