Skip to content

Commit 59f2ede

Browse files
jkczyzclaude
jkczyz
and
claude
committed
Enforce ChannelTransactionParameters completeness at the type level
ChannelTransactionParameters used Option fields for counterparty parameters and funding outpoint, relying on runtime unwrap/expect calls scattered throughout the codebase. This was fragile since it was easy to call code that assumed populated parameters on a not-yet-funded channel. Split into PartialChannelTransactionParameters (used during negotiation) and ChannelTransactionParameters (fully populated, used once funded). Make FundingScope generic over the parameters type so the compiler enforces which channel phases have complete data, eliminating the need for runtime is_populated() guards in signers and elsewhere. With the type-level distinction in place, also: - Remove the InitialRemoteCommitmentReceiver trait, replacing it with a ChannelContext method that takes complete parameters directly. - Replace unset_funding_info with Channel::unfund(), which properly transitions a funded channel back to unfunded state rather than leaving it in an inconsistent "funded without funding info" state. - Flatten DirectedChannelTransactionParameters to store resolved fields directly instead of computing them on each access. - Concretize methods that only work with complete parameters to take FundingScope<ChannelTransactionParameters> directly, using field access instead of trait-based Option-returning accessors. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent c6c28f7 commit 59f2ede

14 files changed

Lines changed: 967 additions & 740 deletions

‎lightning/src/chain/channelmonitor.rs‎

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,8 @@ use crate::chain::{BestBlock, WatchedOutput};
4646
use crate::events::bump_transaction::{AnchorDescriptor, BumpTransactionEvent};
4747
use crate::events::{ClosureReason, Event, EventHandler, ReplayEvent};
4848
use crate::ln::chan_utils::{
49-
self, ChannelTransactionParameters, CommitmentTransaction, CounterpartyCommitmentSecrets,
50-
HTLCClaim, HTLCOutputInCommitment, HolderCommitmentTransaction,
49+
self, ChannelTransactionParameters, ChannelTransactionParametersAccess, CommitmentTransaction,
50+
CounterpartyCommitmentSecrets, HTLCClaim, HTLCOutputInCommitment, HolderCommitmentTransaction,
5151
};
5252
use crate::ln::channel::INITIAL_COMMITMENT_NUMBER;
5353
use crate::ln::channel_keys::{
@@ -118,9 +118,7 @@ impl ChannelMonitorUpdate {
118118
) -> impl Iterator<Item = (OutPoint, ScriptBuf)> + '_ {
119119
self.updates.iter().filter_map(|update| match update {
120120
ChannelMonitorUpdateStep::RenegotiatedFunding { channel_parameters, .. } => {
121-
let funding_outpoint = channel_parameters
122-
.funding_outpoint
123-
.expect("Renegotiated funding must always have known outpoint");
121+
let funding_outpoint = channel_parameters.funding_outpoint;
124122
let funding_script = channel_parameters.make_funding_redeemscript().to_p2wsh();
125123
Some((funding_outpoint, funding_script))
126124
},
@@ -1170,8 +1168,7 @@ struct FundingScope {
11701168

11711169
impl FundingScope {
11721170
fn funding_outpoint(&self) -> OutPoint {
1173-
let funding_outpoint = self.channel_parameters.funding_outpoint.as_ref();
1174-
*funding_outpoint.expect("Funding outpoint must be set for active monitor")
1171+
self.channel_parameters.funding_outpoint
11751172
}
11761173

11771174
fn funding_txid(&self) -> Txid {
@@ -1868,7 +1865,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
18681865
&channel_parameters.channel_type_features, &holder_pubkeys.payment_point
18691866
);
18701867

1871-
let counterparty_channel_parameters = channel_parameters.counterparty_parameters.as_ref().unwrap();
1868+
let counterparty_channel_parameters = &channel_parameters.counterparty_parameters;
18721869
let counterparty_delayed_payment_base_key = counterparty_channel_parameters.pubkeys.delayed_payment_basepoint;
18731870
let counterparty_htlc_base_key = counterparty_channel_parameters.pubkeys.htlc_basepoint;
18741871
let counterparty_commitment_params = CounterpartyCommitmentParameters { counterparty_delayed_payment_base_key, counterparty_htlc_base_key, on_counterparty_tx_csv };
@@ -1885,8 +1882,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
18851882
initial_holder_commitment_tx.clone(), secp_ctx,
18861883
);
18871884

1888-
let funding_outpoint = channel_parameters.funding_outpoint
1889-
.expect("Funding outpoint must be known during initialization");
1885+
let funding_outpoint = channel_parameters.funding_outpoint;
18901886
let funding_redeem_script = channel_parameters.make_funding_redeemscript();
18911887
let funding_script = funding_redeem_script.to_p2wsh();
18921888
let mut outputs_to_watch = new_hash_map();
@@ -4231,7 +4227,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
42314227
channel_parameters, holder_commitment_tx, counterparty_commitment_tx,
42324228
} => {
42334229
log_trace!(logger, "Updating ChannelMonitor with alternative holder and counterparty commitment transactions for funding txid {}",
4234-
channel_parameters.funding_outpoint.unwrap().txid);
4230+
channel_parameters.funding_outpoint.txid);
42354231
if let Err(_) = self.renegotiated_funding(
42364232
logger, channel_parameters, holder_commitment_tx, counterparty_commitment_tx,
42374233
) {
@@ -4345,7 +4341,6 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
43454341
#[rustfmt::skip]
43464342
fn get_funding_txo(&self) -> OutPoint {
43474343
self.funding.channel_parameters.funding_outpoint
4348-
.expect("Funding outpoint must be set for active monitor")
43494344
}
43504345

43514346
/// Returns the P2WSH script we are currently monitoring the chain for spends. This will change
@@ -6981,11 +6976,11 @@ mod tests {
69816976
holder_pubkeys: keys.pubkeys(&secp_ctx),
69826977
holder_selected_contest_delay: 66,
69836978
is_outbound_from_holder: true,
6984-
counterparty_parameters: Some(CounterpartyChannelTransactionParameters {
6979+
counterparty_parameters: CounterpartyChannelTransactionParameters {
69856980
pubkeys: counterparty_pubkeys,
69866981
selected_contest_delay: 67,
6987-
}),
6988-
funding_outpoint: Some(funding_outpoint),
6982+
},
6983+
funding_outpoint,
69896984
splice_parent_funding_txid: None,
69906985
channel_type_features: ChannelTypeFeatures::only_static_remote_key(),
69916986
channel_value_satoshis: 0,
@@ -7244,11 +7239,11 @@ mod tests {
72447239
holder_pubkeys: keys.pubkeys(&secp_ctx),
72457240
holder_selected_contest_delay: 66,
72467241
is_outbound_from_holder: true,
7247-
counterparty_parameters: Some(CounterpartyChannelTransactionParameters {
7242+
counterparty_parameters: CounterpartyChannelTransactionParameters {
72487243
pubkeys: counterparty_pubkeys,
72497244
selected_contest_delay: 67,
7250-
}),
7251-
funding_outpoint: Some(funding_outpoint),
7245+
},
7246+
funding_outpoint,
72527247
splice_parent_funding_txid: None,
72537248
channel_type_features: ChannelTypeFeatures::only_static_remote_key(),
72547249
channel_value_satoshis: 0,

‎lightning/src/chain/onchaintx.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1354,11 +1354,11 @@ mod tests {
13541354
holder_pubkeys: signer.pubkeys(&secp_ctx),
13551355
holder_selected_contest_delay: 66,
13561356
is_outbound_from_holder: true,
1357-
counterparty_parameters: Some(CounterpartyChannelTransactionParameters {
1357+
counterparty_parameters: CounterpartyChannelTransactionParameters {
13581358
pubkeys: counterparty_pubkeys,
13591359
selected_contest_delay: 67,
1360-
}),
1361-
funding_outpoint: Some(funding_outpoint),
1360+
},
1361+
funding_outpoint,
13621362
splice_parent_funding_txid: None,
13631363
channel_type_features: ChannelTypeFeatures::only_static_remote_key(),
13641364
channel_value_satoshis: 0,

‎lightning/src/chain/package.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ use crate::chain::channelmonitor::COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
3131
use crate::chain::onchaintx::{FeerateStrategy, OnchainTxHandler};
3232
use crate::chain::transaction::MaybeSignedTransaction;
3333
use crate::ln::chan_utils::{
34-
self, ChannelTransactionParameters, HTLCOutputInCommitment, HolderCommitmentTransaction,
35-
TxCreationKeys,
34+
self, ChannelTransactionParameters, ChannelTransactionParametersAccess, HTLCOutputInCommitment,
35+
HolderCommitmentTransaction, TxCreationKeys,
3636
};
3737
use crate::ln::channel_keys::{DelayedPaymentBasepoint, HtlcBasepoint};
3838
use crate::ln::channelmanager::MIN_CLTV_EXPIRY_DELTA;
@@ -1892,7 +1892,7 @@ mod tests {
18921892
payment_hash: PaymentHash::from(preimage),
18931893
transaction_output_index: None,
18941894
};
1895-
let funding_outpoint = channel_parameters.funding_outpoint.unwrap();
1895+
let funding_outpoint = channel_parameters.funding_outpoint;
18961896
let commitment_tx = HolderCommitmentTransaction::dummy(0, funding_outpoint, vec![htlc.clone()]);
18971897
let trusted_tx = commitment_tx.trust();
18981898
PackageSolvingData::HolderHTLCOutput(HolderHTLCOutput::build(
@@ -1929,7 +1929,7 @@ mod tests {
19291929
payment_hash: PaymentHash::from(PaymentPreimage([2;32])),
19301930
transaction_output_index: None,
19311931
};
1932-
let funding_outpoint = channel_parameters.funding_outpoint.unwrap();
1932+
let funding_outpoint = channel_parameters.funding_outpoint;
19331933
let commitment_tx = HolderCommitmentTransaction::dummy(0, funding_outpoint, vec![htlc.clone()]);
19341934
let trusted_tx = commitment_tx.trust();
19351935
PackageSolvingData::HolderHTLCOutput(HolderHTLCOutput::build(
@@ -1957,7 +1957,7 @@ mod tests {
19571957
macro_rules! dumb_funding_output {
19581958
() => {{
19591959
let mut channel_parameters = ChannelTransactionParameters::test_dummy(0);
1960-
let funding_outpoint = channel_parameters.funding_outpoint.unwrap();
1960+
let funding_outpoint = channel_parameters.funding_outpoint;
19611961
let commitment_tx = HolderCommitmentTransaction::dummy(0, funding_outpoint, Vec::new());
19621962
channel_parameters.channel_type_features = ChannelTypeFeatures::only_static_remote_key();
19631963
PackageSolvingData::HolderFundingOutput(HolderFundingOutput::build(

‎lightning/src/ln/async_signer_tests.rs‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1244,12 +1244,11 @@ fn do_test_closing_signed(extra_closing_signed: bool, reconnect: bool) {
12441244
let per_peer_state = nodes[1].node.per_peer_state.read().unwrap();
12451245
let mut chan_lock = per_peer_state.get(&node_a_id).unwrap().lock().unwrap();
12461246
let channel = chan_lock.channel_by_id.get_mut(&chan_id).unwrap();
1247-
let (funding, context) = channel.funding_and_context_mut();
1248-
1249-
let signer = context.get_mut_signer().as_mut_ecdsa().unwrap();
1247+
let funded = channel.as_funded_mut().unwrap();
1248+
let signer = funded.context.get_mut_signer().as_mut_ecdsa().unwrap();
12501249
let signature = signer
12511250
.sign_closing_transaction(
1252-
&funding.channel_transaction_parameters,
1251+
&funded.funding.channel_transaction_parameters,
12531252
&closing_tx_2,
12541253
&Secp256k1::new(),
12551254
)

0 commit comments

Comments
 (0)