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.

Author: waterson

lightning/src/ln/channel.rs

@@ -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;

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()));
 					}

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])