Handle fallible commitment point when getting accept_channel

alecchendev · alecchendev · commit 3dec982fe4ab · 2024-12-05T15:58:22.000-08:00
Similar to `open_channel`, if a signer cannot provide a commitment point
immediately, we set a flag to remember we're waiting for a point to send
`accept_channel`. We make sure to get the first two points before moving
on, so when we advance our commitment we always have a point available.
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
@@ -8552,6 +8552,7 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
 pub(super) struct InboundV1Channel<SP: Deref> where SP::Target: SignerProvider {
 	pub context: ChannelContext<SP>,
 	pub unfunded_context: UnfundedChannelContext,
+	pub signer_pending_accept_channel: bool,
 }
 
 /// Fetches the [`ChannelTypeFeatures`] that will be used for a channel built from a given
@@ -8641,15 +8642,17 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
 			unfunded_channel_age_ticks: 0,
 			holder_commitment_point: HolderCommitmentPoint::new(&context.holder_signer, &context.secp_ctx),
 		};
-		let chan = Self { context, unfunded_context };
+		let chan = Self { context, unfunded_context, signer_pending_accept_channel: false };
 		Ok(chan)
 	}
 
 	/// Marks an inbound channel as accepted and generates a [`msgs::AcceptChannel`] message which
 	/// should be sent back to the counterparty node.
 	///
 	/// [`msgs::AcceptChannel`]: crate::ln::msgs::AcceptChannel
-	pub fn accept_inbound_channel(&self) -> msgs::AcceptChannel {
+	pub fn accept_inbound_channel<L: Deref>(&mut self, logger: &L) -> Option<msgs::AcceptChannel>
+	where L::Target: Logger
+	{
 		if self.context.is_outbound() {
 			panic!("Tried to send accept_channel for an outbound channel?");
 		}
@@ -8663,21 +8666,33 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
 			panic!("Tried to send an accept_channel for a channel that has already advanced");
 		}
 
-		self.generate_accept_channel_message()
+		self.generate_accept_channel_message(logger)
 	}
 
 	/// This function is used to explicitly generate a [`msgs::AcceptChannel`] message for an
 	/// inbound channel. If the intention is to accept an inbound channel, use
 	/// [`InboundV1Channel::accept_inbound_channel`] instead.
 	///
 	/// [`msgs::AcceptChannel`]: crate::ln::msgs::AcceptChannel
-	fn generate_accept_channel_message(&self) -> msgs::AcceptChannel {
-		debug_assert!(self.unfunded_context.holder_commitment_point.map(|point| point.is_available()).unwrap_or(false));
-		let first_per_commitment_point = self.unfunded_context.holder_commitment_point
-			.expect("TODO: Handle holder_commitment_point not being set").current_point();
+	fn generate_accept_channel_message<L: Deref>(&mut self, _logger: &L) -> Option<msgs::AcceptChannel>
+	where L::Target: Logger
+	{
+		let first_per_commitment_point = if let Some(holder_commitment_point) = self.unfunded_context.holder_commitment_point {
+			self.signer_pending_accept_channel = false;
+			holder_commitment_point.current_point()
+		} else {
+			#[cfg(not(async_signing))] {
+				panic!("Failed getting commitment point for accept_channel message");
+			}
+			#[cfg(async_signing)] {
+				log_trace!(_logger, "Unable to generate accept_channel message, waiting for commitment point");
+				self.signer_pending_accept_channel = true;
+				return None;
+			}
+		};
 		let keys = self.context.get_holder_pubkeys();
 
-		msgs::AcceptChannel {
+		Some(msgs::AcceptChannel {
 			common_fields: msgs::CommonAcceptChannelFields {
 				temporary_channel_id: self.context.channel_id,
 				dust_limit_satoshis: self.context.holder_dust_limit_satoshis,
@@ -8701,16 +8716,18 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
 			channel_reserve_satoshis: self.context.holder_selected_channel_reserve_satoshis,
 			#[cfg(taproot)]
 			next_local_nonce: None,
-		}
+		})
 	}
 
 	/// Enables the possibility for tests to extract a [`msgs::AcceptChannel`] message for an
 	/// inbound channel without accepting it.
 	///
 	/// [`msgs::AcceptChannel`]: crate::ln::msgs::AcceptChannel
 	#[cfg(test)]
-	pub fn get_accept_channel_message(&self) -> msgs::AcceptChannel {
-		self.generate_accept_channel_message()
+	pub fn get_accept_channel_message<L: Deref>(&mut self, logger: &L) -> Option<msgs::AcceptChannel>
+	where L::Target: Logger
+	{
+		self.generate_accept_channel_message(logger)
 	}
 
 	pub fn funding_created<L: Deref>(
@@ -8769,6 +8786,29 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
 
 		Ok((channel, funding_signed, channel_monitor))
 	}
+
+	/// Indicates that the signer may have some signatures for us, so we should retry if we're
+	/// blocked.
+	#[allow(unused)]
+	pub fn signer_maybe_unblocked<L: Deref>(&mut self, logger: &L) -> Option<msgs::AcceptChannel>
+	where L::Target: Logger
+	{
+		if self.unfunded_context.holder_commitment_point.is_none() {
+			self.unfunded_context.holder_commitment_point = HolderCommitmentPoint::new(&self.context.holder_signer, &self.context.secp_ctx);
+		}
+		if let Some(ref mut point) = self.unfunded_context.holder_commitment_point {
+			if !point.is_available() {
+				point.try_resolve_pending(&self.context.holder_signer, &self.context.secp_ctx, logger);
+			}
+		}
+		match self.unfunded_context.holder_commitment_point {
+			Some(ref mut point) if point.is_available() && self.signer_pending_accept_channel => {
+				log_trace!(logger, "Attempting to generate accept_channel...");
+				self.generate_accept_channel_message(logger)
+			}
+			_ => None
+		}
+	}
 }
 
 // A not-yet-funded outbound (from holder) channel using V2 channel establishment.
@@ -10388,10 +10428,10 @@ mod tests {
 		// Make sure A's dust limit is as we expect.
 		let open_channel_msg = node_a_chan.get_open_channel(ChainHash::using_genesis_block(network), &&logger).unwrap();
 		let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[7; 32]).unwrap());
-		let node_b_chan = InboundV1Channel::<&TestKeysInterface>::new(&feeest, &&keys_provider, &&keys_provider, node_b_node_id, &channelmanager::provided_channel_type_features(&config), &channelmanager::provided_init_features(&config), &open_channel_msg, 7, &config, 0, &&logger, /*is_0conf=*/false).unwrap();
+		let mut node_b_chan = InboundV1Channel::<&TestKeysInterface>::new(&feeest, &&keys_provider, &&keys_provider, node_b_node_id, &channelmanager::provided_channel_type_features(&config), &channelmanager::provided_init_features(&config), &open_channel_msg, 7, &config, 0, &&logger, /*is_0conf=*/false).unwrap();
 
 		// Node B --> Node A: accept channel, explicitly setting B's dust limit.
-		let mut accept_channel_msg = node_b_chan.accept_inbound_channel();
+		let mut accept_channel_msg = node_b_chan.accept_inbound_channel(&&logger).unwrap();
 		accept_channel_msg.common_fields.dust_limit_satoshis = 546;
 		node_a_chan.accept_channel(&accept_channel_msg, &config.channel_handshake_limits, &channelmanager::provided_init_features(&config)).unwrap();
 		node_a_chan.context.holder_dust_limit_satoshis = 1560;
@@ -10520,10 +10560,10 @@ mod tests {
 		// Create Node B's channel by receiving Node A's open_channel message
 		let open_channel_msg = node_a_chan.get_open_channel(chain_hash, &&logger).unwrap();
 		let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[7; 32]).unwrap());
-		let node_b_chan = InboundV1Channel::<&TestKeysInterface>::new(&feeest, &&keys_provider, &&keys_provider, node_b_node_id, &channelmanager::provided_channel_type_features(&config), &channelmanager::provided_init_features(&config), &open_channel_msg, 7, &config, 0, &&logger, /*is_0conf=*/false).unwrap();
+		let mut node_b_chan = InboundV1Channel::<&TestKeysInterface>::new(&feeest, &&keys_provider, &&keys_provider, node_b_node_id, &channelmanager::provided_channel_type_features(&config), &channelmanager::provided_init_features(&config), &open_channel_msg, 7, &config, 0, &&logger, /*is_0conf=*/false).unwrap();
 
 		// Node B --> Node A: accept channel
-		let accept_channel_msg = node_b_chan.accept_inbound_channel();
+		let accept_channel_msg = node_b_chan.accept_inbound_channel(&&logger).unwrap();
 		node_a_chan.accept_channel(&accept_channel_msg, &config.channel_handshake_limits, &channelmanager::provided_init_features(&config)).unwrap();
 
 		// Node A --> Node B: funding created
@@ -10707,10 +10747,10 @@ mod tests {
 		// Make sure A's dust limit is as we expect.
 		let open_channel_msg = node_a_chan.get_open_channel(ChainHash::using_genesis_block(network), &&logger).unwrap();
 		let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[7; 32]).unwrap());
-		let node_b_chan = InboundV1Channel::<&TestKeysInterface>::new(&feeest, &&keys_provider, &&keys_provider, node_b_node_id, &channelmanager::provided_channel_type_features(&config), &channelmanager::provided_init_features(&config), &open_channel_msg, 7, &config, 0, &&logger, /*is_0conf=*/false).unwrap();
+		let mut node_b_chan = InboundV1Channel::<&TestKeysInterface>::new(&feeest, &&keys_provider, &&keys_provider, node_b_node_id, &channelmanager::provided_channel_type_features(&config), &channelmanager::provided_init_features(&config), &open_channel_msg, 7, &config, 0, &&logger, /*is_0conf=*/false).unwrap();
 
 		// Node B --> Node A: accept channel, explicitly setting B's dust limit.
-		let mut accept_channel_msg = node_b_chan.accept_inbound_channel();
+		let mut accept_channel_msg = node_b_chan.accept_inbound_channel(&&logger).unwrap();
 		accept_channel_msg.common_fields.dust_limit_satoshis = 546;
 		node_a_chan.accept_channel(&accept_channel_msg, &config.channel_handshake_limits, &channelmanager::provided_init_features(&config)).unwrap();
 		node_a_chan.context.holder_dust_limit_satoshis = 1560;
@@ -10780,11 +10820,11 @@ mod tests {
 		let mut outbound_chan = OutboundV1Channel::<&TestKeysInterface>::new(
 			&feeest, &&keys_provider, &&keys_provider, node_b_node_id, &features, 10000000, 100000, 42, &config, 0, 42, None, &logger
 		).unwrap();
-		let inbound_chan = InboundV1Channel::<&TestKeysInterface>::new(
+		let mut inbound_chan = InboundV1Channel::<&TestKeysInterface>::new(
 			&feeest, &&keys_provider, &&keys_provider, node_b_node_id, &channelmanager::provided_channel_type_features(&config),
 			&features, &outbound_chan.get_open_channel(ChainHash::using_genesis_block(network), &&logger).unwrap(), 7, &config, 0, &&logger, false
 		).unwrap();
-		outbound_chan.accept_channel(&inbound_chan.get_accept_channel_message(), &config.channel_handshake_limits, &features).unwrap();
+		outbound_chan.accept_channel(&inbound_chan.get_accept_channel_message(&&logger).unwrap(), &config.channel_handshake_limits, &features).unwrap();
 		let tx = Transaction { version: Version::ONE, lock_time: LockTime::ZERO, input: Vec::new(), output: vec![TxOut {
 			value: Amount::from_sat(10000000), script_pubkey: outbound_chan.context.get_funding_redeemscript(),
 		}]};
@@ -11836,13 +11876,13 @@ mod tests {
 
 		let open_channel_msg = channel_a.get_open_channel(ChainHash::using_genesis_block(network), &&logger).unwrap();
 
-		let channel_b = InboundV1Channel::<&TestKeysInterface>::new(
+		let mut channel_b = InboundV1Channel::<&TestKeysInterface>::new(
 			&fee_estimator, &&keys_provider, &&keys_provider, node_id_a,
 			&channelmanager::provided_channel_type_features(&config), &channelmanager::provided_init_features(&config),
 			&open_channel_msg, 7, &config, 0, &&logger, /*is_0conf=*/false
 		).unwrap();
 
-		let mut accept_channel_msg = channel_b.get_accept_channel_message();
+		let mut accept_channel_msg = channel_b.get_accept_channel_message(&&logger).unwrap();
 		accept_channel_msg.common_fields.channel_type = Some(simple_anchors_channel_type.clone());
 
 		let res = channel_a.accept_channel(
@@ -11887,7 +11927,7 @@ mod tests {
 
 		let open_channel_msg = node_a_chan.get_open_channel(ChainHash::using_genesis_block(network), &&logger).unwrap();
 		let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[7; 32]).unwrap());
-		let node_b_chan = InboundV1Channel::<&TestKeysInterface>::new(
+		let mut node_b_chan = InboundV1Channel::<&TestKeysInterface>::new(
 			&feeest,
 			&&keys_provider,
 			&&keys_provider,
@@ -11902,7 +11942,7 @@ mod tests {
 			true,  // Allow node b to send a 0conf channel_ready.
 		).unwrap();
 
-		let accept_channel_msg = node_b_chan.accept_inbound_channel();
+		let accept_channel_msg = node_b_chan.accept_inbound_channel(&&logger).unwrap();
 		node_a_chan.accept_channel(
 			&accept_channel_msg,
 			&config.channel_handshake_limits,
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
@@ -7690,11 +7690,14 @@ where
 							&self.channel_type_features(), &peer_state.latest_features, &open_channel_msg,
 							user_channel_id, &self.default_configuration, best_block_height, &self.logger, accept_0conf
 						).map_err(|err| MsgHandleErrInternal::from_chan_no_close(err, *temporary_channel_id)
-						).map(|channel| {
-							let message_send_event = events::MessageSendEvent::SendAcceptChannel {
-								node_id: *counterparty_node_id,
-								msg: channel.accept_inbound_channel(),
-							};
+						).map(|mut channel| {
+							let logger = WithChannelContext::from(&self.logger, &channel.context, None);
+							let message_send_event = channel.accept_inbound_channel(&&logger).map(|msg| {
+								events::MessageSendEvent::SendAcceptChannel {
+									node_id: *counterparty_node_id,
+									msg,
+								}
+							});
 							(*temporary_channel_id, ChannelPhase::UnfundedInboundV1(channel), message_send_event)
 						})
 					},
@@ -7715,7 +7718,7 @@ where
 								node_id: channel.context.get_counterparty_node_id(),
 								msg: channel.accept_inbound_dual_funded_channel()
 							};
-							(channel.context.channel_id(), ChannelPhase::UnfundedInboundV2(channel), message_send_event)
+							(channel.context.channel_id(), ChannelPhase::UnfundedInboundV2(channel), Some(message_send_event))
 						})
 					},
 				}
@@ -7783,7 +7786,9 @@ where
 		let outbound_scid_alias = self.create_and_insert_outbound_scid_alias();
 		channel_phase.context_mut().set_outbound_scid_alias(outbound_scid_alias);
 
-		peer_state.pending_msg_events.push(message_send_event);
+		if let Some(message_send_event) = message_send_event {
+			peer_state.pending_msg_events.push(message_send_event);
+		}
 		peer_state.channel_by_id.insert(channel_id, channel_phase);
 
 		Ok(())
@@ -7959,15 +7964,18 @@ where
 
 		let (mut channel_phase, message_send_event) = match msg {
 			OpenChannelMessageRef::V1(msg) => {
-				let channel = InboundV1Channel::new(
+				let mut channel = InboundV1Channel::new(
 					&self.fee_estimator, &self.entropy_source, &self.signer_provider, *counterparty_node_id,
 					&self.channel_type_features(), &peer_state.latest_features, msg, user_channel_id,
 					&self.default_configuration, best_block_height, &self.logger, /*is_0conf=*/false
 				).map_err(|e| MsgHandleErrInternal::from_chan_no_close(e, msg.common_fields.temporary_channel_id))?;
-				let message_send_event = events::MessageSendEvent::SendAcceptChannel {
-					node_id: *counterparty_node_id,
-					msg: channel.accept_inbound_channel(),
-				};
+				let logger = WithChannelContext::from(&self.logger, &channel.context, None);
+				let message_send_event = channel.accept_inbound_channel(&&logger).map(|msg| {
+					events::MessageSendEvent::SendAcceptChannel {
+						node_id: *counterparty_node_id,
+						msg,
+					}
+				});
 				(ChannelPhase::UnfundedInboundV1(channel), message_send_event)
 			},
 			OpenChannelMessageRef::V2(msg) => {
@@ -7980,14 +7988,16 @@ where
 					node_id: *counterparty_node_id,
 					msg: channel.accept_inbound_dual_funded_channel(),
 				};
-				(ChannelPhase::UnfundedInboundV2(channel), message_send_event)
+				(ChannelPhase::UnfundedInboundV2(channel), Some(message_send_event))
 			},
 		};
 
 		let outbound_scid_alias = self.create_and_insert_outbound_scid_alias();
 		channel_phase.context_mut().set_outbound_scid_alias(outbound_scid_alias);
 
-		peer_state.pending_msg_events.push(message_send_event);
+		if let Some(message_send_event) = message_send_event {
+			peer_state.pending_msg_events.push(message_send_event);
+		}
 		peer_state.channel_by_id.insert(channel_phase.context().channel_id(), channel_phase);
 
 		Ok(())
@@ -9521,7 +9531,17 @@ where
 					}
 					None
 				}
-				ChannelPhase::UnfundedInboundV1(_) | ChannelPhase::UnfundedInboundV2(_) | ChannelPhase::UnfundedOutboundV2(_) => None,
+				ChannelPhase::UnfundedInboundV1(chan) => {
+					let logger = WithChannelContext::from(&self.logger, &chan.context, None);
+					if let Some(msg) = chan.signer_maybe_unblocked(&&logger) {
+						pending_msg_events.push(events::MessageSendEvent::SendAcceptChannel {
+							node_id,
+							msg,
+						});
+					}
+					None
+				},
+				ChannelPhase::UnfundedInboundV2(_) | ChannelPhase::UnfundedOutboundV2(_) => None,
 			}
 		};
 
