Break up ChannelContext::funding_tx_constructed
What the developer wrote
Authored by Wilmer Paulino
Break up ChannelContext::funding_tx_constructed
This method was originally intended to DRY the logic between
`FundedChannel` and `PendingChannelV2` when handling
`funding_tx_constructed`, though it's not very useful anymore.
What changed, and why it matters
This is a routine code cleanup that removes a helper method and copies its logic directly into the two places that used it. There is no security fix or behavior change visible in the diff.
No security action needed; treat as normal refactoring.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The commit refactors ChannelContext::funding_tx_constructed by inlining its body into the PendingV2Channel and splicing call sites, then deletes the helper. The same operations are still performed in the same order: setting the funding outpoint, asserting no commitment advancement (only for the non-splice path), advancing channel state for the non-splice path, calling get_initial_commitment_signed_v2, and returning an InternalError when signing returns None. The error mapping changes from a propagated AbortReason to an explicit InternalError string, but the failure mode is identical.
Changed components
lightning/src/ln/channel.rsInspect captured patch +31 / −44
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 45a1d98..e7426a0 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -2028,18 +2028,32 @@ where
| NegotiatingFundingFlags::THEIR_INIT_SENT
),
);
+ chan.context.assert_no_commitment_advancement(
+ chan.unfunded_context.transaction_number(),
+ "initial commitment_signed",
+ );
+
+ chan.context.channel_state =
+ ChannelState::FundingNegotiated(FundingNegotiatedFlags::new());
+ chan.funding.channel_transaction_parameters.funding_outpoint =
+ Some(funding_outpoint);
let interactive_tx_constructor = chan
.interactive_tx_constructor
.take()
.expect("PendingV2Channel::interactive_tx_constructor should be set");
- let commitment_signed = chan.context.funding_tx_constructed(
- &mut chan.funding,
- funding_outpoint,
- false,
- chan.unfunded_context.transaction_number(),
- &&logger,
- )?;
+
+ let commitment_signed =
+ chan.context.get_initial_commitment_signed_v2(&chan.funding, &&logger);
+ let commitment_signed = match commitment_signed {
+ Some(commitment_signed) => commitment_signed,
+ // TODO(dual_funding): Support async signing
+ None => {
+ return Err(AbortReason::InternalError(
+ "Failed to compute commitment_signed signatures",
+ ));
+ },
+ };
(interactive_tx_constructor, commitment_signed)
},
@@ -2068,14 +2082,11 @@ where
)
})
.and_then(|(is_initiator, mut funding, interactive_tx_constructor)| {
- match chan.context.funding_tx_constructed(
- &mut funding,
- funding_outpoint,
- true,
- chan.holder_commitment_point.next_transaction_number(),
- &&logger,
- ) {
- Ok(commitment_signed) => {
+ funding.channel_transaction_parameters.funding_outpoint =
+ Some(funding_outpoint);
+ match chan.context.get_initial_commitment_signed_v2(&funding, &&logger)
+ {
+ Some(commitment_signed) => {
// Advance the state
pending_splice.funding_negotiation =
Some(FundingNegotiation::AwaitingSignatures {
@@ -2084,14 +2095,17 @@ where
});
Ok((interactive_tx_constructor, commitment_signed))
},
- Err(e) => {
+ // TODO(splicing): Support async signing
+ None => {
// Restore the taken state for later error handling
pending_splice.funding_negotiation =
Some(FundingNegotiation::ConstructingTransaction {
funding,
interactive_tx_constructor,
});
- Err(e)
+ Err(AbortReason::InternalError(
+ "Failed to compute commitment_signed signatures",
+ ))
},
}
})?
@@ -6189,33 +6203,6 @@ where
Ok(())
}
- #[rustfmt::skip]
- fn funding_tx_constructed<L: Deref>(
- &mut self, funding: &mut FundingScope, funding_outpoint: OutPoint, is_splice: bool,
- holder_commitment_transaction_number: u64, logger: &L,
- ) -> Result<msgs::CommitmentSigned, AbortReason>
- where
- L::Target: Logger
- {
- funding.channel_transaction_parameters.funding_outpoint = Some(funding_outpoint);
-
- if !is_splice {
- self.assert_no_commitment_advancement(holder_commitment_transaction_number, "initial commitment_signed");
- self.channel_state = ChannelState::FundingNegotiated(FundingNegotiatedFlags::new());
- }
-
- let commitment_signed = self.get_initial_commitment_signed_v2(&funding, logger);
- let commitment_signed = match commitment_signed {
- Some(commitment_signed) => commitment_signed,
- // TODO(splicing): Support async signing
- None => {
- return Err(AbortReason::InternalError("Failed to compute commitment_signed signatures"));
- },
- };
-
- Ok(commitment_signed)
- }
-
/// Asserts that the commitment tx numbers have not advanced from their initial number.
fn assert_no_commitment_advancement(
&self, holder_commitment_transaction_number: u64, msg_name: &str,
Why this scored 15/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.