Check correct commitment number/point in initial commitment_signed
What the developer wrote
Authored by Jeffrey Czyz
Check correct commitment number/point in initial commitment_signed
When splicing a channel, the initial commitment_signed received should
use the same commitment number and point previously received prior to
splicing the channel. Account for this by checking the commitment_signed
against that one instead, which is now stored separately in
FundedChannel.
What changed, and why it matters
This patch fixes a logic bug in how a Lightning node validates the first commitment signature after a channel splice. Previously, the code checked the new commitment against the wrong commitment number and public key, which could cause the node to reject a valid splice or, in some edge cases, accept an inconsistent state. The fix stores and compares against the commitment number and point that were actually in use before the splice.
Review splice test vectors to ensure the initial commitment_signed after splice is now validated against the correct pre-splice commitment number and point. Consider adding explicit regression tests for splice commitment_signed validation if not already present.
Security signals we found
Incorrect commitment number/point validation during splice
Potential state inconsistency between pre-splice and post-splice commitments
Validation bypass or denial-of-channel risk if wrong commitment point is accepted
Fix is narrowly scoped to a single function and its call sites
Evidence from the diff
In rust-lightning’s channel.rs, validate_commitment_signed previously derived the expected transaction number and commitment point from holder_commitment_point via next_transaction_number()/next_point(). During a splice, the initial commitment_signed should match the pre-splice commitment number and point, not the next ones. The patch changes validate_commitment_signed to accept explicit transaction_number and commitment_point arguments, and updates all three call sites: one splice-specific path now uses current_transaction_number()/current_point(), while the two normal paths continue using next_transaction_number()/next_point().
Changed components
lightning/src/ln/channel.rsFundedChannelvalidate_commitment_signedsplice commitment handlingInspect captured patch +24 / −7
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 3d035b9..b2f88d2 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -4195,7 +4195,7 @@ where
#[rustfmt::skip]
fn validate_commitment_signed<L: Deref>(
- &self, funding: &FundingScope, holder_commitment_point: &HolderCommitmentPoint,
+ &self, funding: &FundingScope, transaction_number: u64, commitment_point: PublicKey,
msg: &msgs::CommitmentSigned, logger: &L,
) -> Result<(HolderCommitmentTransaction, Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)>), ChannelError>
where
@@ -4203,9 +4203,9 @@ where
{
let funding_script = funding.get_funding_redeemscript();
- let commitment_data = self.build_commitment_transaction(funding,
- holder_commitment_point.next_transaction_number(), &holder_commitment_point.next_point(),
- true, false, logger);
+ let commitment_data = self.build_commitment_transaction(
+ funding, transaction_number, &commitment_point, true, false, logger,
+ );
let commitment_txid = {
let trusted_tx = commitment_data.tx.trust();
let bitcoin_tx = trusted_tx.built_transaction();
@@ -7003,9 +7003,15 @@ where
})
.and_then(|funding_negotiation| funding_negotiation.as_funding())
.expect("Funding must exist for negotiated pending splice");
+ let transaction_number = self.holder_commitment_point.current_transaction_number();
+ let commitment_point = self
+ .holder_commitment_point
+ .current_point()
+ .expect("current should be set after receiving the initial commitment_signed");
let (holder_commitment_tx, _) = self.context.validate_commitment_signed(
pending_splice_funding,
- &self.holder_commitment_point,
+ transaction_number,
+ commitment_point,
msg,
logger,
)?;
@@ -7089,9 +7095,17 @@ where
));
}
+ let transaction_number = self.holder_commitment_point.next_transaction_number();
+ let commitment_point = self.holder_commitment_point.next_point();
let update = self
.context
- .validate_commitment_signed(&self.funding, &self.holder_commitment_point, msg, logger)
+ .validate_commitment_signed(
+ &self.funding,
+ transaction_number,
+ commitment_point,
+ msg,
+ logger,
+ )
.map(|(commitment_tx, htlcs_included)| {
let (nondust_htlc_sources, dust_htlcs) =
Self::get_commitment_htlc_data(&htlcs_included);
@@ -7153,9 +7167,12 @@ where
funding_txid
))
})?;
+ let transaction_number = self.holder_commitment_point.next_transaction_number();
+ let commitment_point = self.holder_commitment_point.next_point();
let (commitment_tx, htlcs_included) = self.context.validate_commitment_signed(
funding,
- &self.holder_commitment_point,
+ transaction_number,
+ commitment_point,
msg,
logger,
)?;
Why this scored 57/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.