Split commitment_signed handling by check-accept

When handling commitment_signed messages, a number of checks are
performed before a ChannelMonitorUpdate is created and returned. Once
splicing is added, these checks need to be performed on the primary
FundingScope and any pending scopes that resulted from splicing or RBF.

This commit splits the handling into a check and accepts methods, taking
&self and &mut self, respectively. This ensures that the ChannelContext
is not modified between checks. Once all funding scopes have been
checked successfully, the accept portion of the code can then execute.

Author: jkczyz

lightning/src/chain/channelmonitor.rs

@@ -529,6 +529,14 @@ impl_writeable_tlv_based_enum_upgradable!(OnchainEvent,
 
 );
 
+/// Partial data from ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo used to simplify the
+/// return type of [`FundedChannel::commitment_signed_check`].
+pub(crate) struct LatestHolderCommitmentTXInfo {
+	pub commitment_tx: HolderCommitmentTransaction,
+	pub htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>,
+	pub nondust_htlc_sources: Vec<HTLCSource>,
+}
+
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub(crate) enum ChannelMonitorUpdateStep {
 	LatestHolderCommitmentTXInfo {

lightning/src/ln/channel.rs

@@ -52,7 +52,7 @@ use crate::ln::chan_utils;
 use crate::ln::onion_utils::HTLCFailReason;
 use crate::chain::BestBlock;
 use crate::chain::chaininterface::{FeeEstimator, ConfirmationTarget, LowerBoundedFeeEstimator, fee_for_weight};
-use crate::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, LATENCY_GRACE_PERIOD_BLOCKS};
+use crate::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, LatestHolderCommitmentTXInfo, LATENCY_GRACE_PERIOD_BLOCKS};
 use crate::chain::transaction::{OutPoint, TransactionData};
 use crate::sign::ecdsa::EcdsaChannelSigner;
 use crate::sign::{EntropySource, ChannelSigner, SignerProvider, NodeSigner, Recipient};
@@ -5494,7 +5494,7 @@ impl<SP: Deref> FundedChannel<SP> where
 		Ok(channel_monitor)
 	}
 
-	pub fn commitment_signed<L: Deref>(&mut self, msg: &msgs::CommitmentSigned, logger: &L) -> Result<Option<ChannelMonitorUpdate>, ChannelError>
+	fn commitment_signed_check<L: Deref>(&self, msg: &msgs::CommitmentSigned, logger: &L) -> Result<LatestHolderCommitmentTXInfo, ChannelError>
 		where L::Target: Logger
 	{
 		if self.context.channel_state.is_quiescent() {
@@ -5622,6 +5622,18 @@ impl<SP: Deref> FundedChannel<SP> where
 		self.context.holder_signer.as_ref().validate_holder_commitment(&holder_commitment_tx, commitment_stats.outbound_htlc_preimages)
 			.map_err(|_| ChannelError::close("Failed to validate our commitment".to_owned()))?;
 
+		Ok(LatestHolderCommitmentTXInfo {
+			commitment_tx: holder_commitment_tx,
+			htlc_outputs: htlcs_and_sigs,
+			nondust_htlc_sources,
+		})
+	}
+
+	pub fn commitment_signed<L: Deref>(&mut self, msg: &msgs::CommitmentSigned, logger: &L) -> Result<Option<ChannelMonitorUpdate>, ChannelError>
+		where L::Target: Logger
+	{
+		let commitment_tx_info = self.commitment_signed_check(msg, logger)?;
+
 		// Update state now that we've passed all the can-fail calls...
 		let mut need_commitment = false;
 		if let &mut Some((_, ref mut update_state)) = &mut self.context.pending_update_fee {
@@ -5661,13 +5673,16 @@ impl<SP: Deref> FundedChannel<SP> where
 			}
 		}
 
+		let LatestHolderCommitmentTXInfo {
+			commitment_tx, htlc_outputs, nondust_htlc_sources,
+		} = commitment_tx_info;
 		self.context.latest_monitor_update_id += 1;
 		let mut monitor_update = ChannelMonitorUpdate {
 			update_id: self.context.latest_monitor_update_id,
 			counterparty_node_id: Some(self.context.counterparty_node_id),
 			updates: vec![ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo {
-				commitment_tx: holder_commitment_tx,
-				htlc_outputs: htlcs_and_sigs,
+				commitment_tx,
+				htlc_outputs,
 				claimed_htlcs,
 				nondust_htlc_sources,
 			}],