Skip to content

Commit 98ec3e4

Browse files
jkczyzclaude
andcommitted
Return InteractiveTxMsgError from splice_init and tx_init_rbf
The prior two commits manually intercepted ChannelError::Abort in the channelmanager handlers for splice_init and tx_init_rbf to exit quiescence before returning, since the channel methods didn't signal this themselves. The interactive TX message handlers already solved this by returning InteractiveTxMsgError which bundles exited_quiescence into the error type. Apply the same pattern: change splice_init and tx_init_rbf to return InteractiveTxMsgError, adding a quiescent_negotiation_err helper on FundedChannel that exits quiescence for Abort errors and passes through other variants unchanged. Extract handle_interactive_tx_msg_err in channelmanager to deduplicate the error handling across internal_tx_msg, internal_splice_init, internal_tx_init_rbf, and internal_tx_complete. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent bd5b04d commit 98ec3e4

3 files changed

Lines changed: 255 additions & 164 deletions

File tree

lightning/src/ln/channel.rs

Lines changed: 78 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -12723,9 +12723,7 @@ where
1272312723
}
1272412724

1272512725
/// Checks during handling splice_init
12726-
pub fn validate_splice_init(
12727-
&self, msg: &msgs::SpliceInit, our_funding_contribution: SignedAmount,
12728-
) -> Result<FundingScope, ChannelError> {
12726+
pub fn validate_splice_init(&self, msg: &msgs::SpliceInit) -> Result<(), ChannelError> {
1272912727
if self.holder_commitment_point.current_point().is_none() {
1273012728
return Err(ChannelError::WarnAndDisconnect(format!(
1273112729
"Channel {} commitment point needs to be advanced once before spliced",
@@ -12762,32 +12760,7 @@ where
1276212760
)));
1276312761
}
1276412762

12765-
self.validate_splice_contributions(our_funding_contribution, their_funding_contribution)
12766-
.map_err(|e| ChannelError::WarnAndDisconnect(e))?;
12767-
12768-
// Rotate the pubkeys using the prev_funding_txid as a tweak
12769-
let prev_funding_txid = self.funding.get_funding_txid();
12770-
let funding_pubkey = match prev_funding_txid {
12771-
None => {
12772-
debug_assert!(false);
12773-
self.funding.get_holder_pubkeys().funding_pubkey
12774-
},
12775-
Some(prev_funding_txid) => self
12776-
.context
12777-
.holder_signer
12778-
.new_funding_pubkey(prev_funding_txid, &self.context.secp_ctx),
12779-
};
12780-
let mut new_keys = self.funding.get_holder_pubkeys().clone();
12781-
new_keys.funding_pubkey = funding_pubkey;
12782-
12783-
Ok(FundingScope::for_splice(
12784-
&self.funding,
12785-
&self.context,
12786-
our_funding_contribution,
12787-
their_funding_contribution,
12788-
msg.funding_pubkey,
12789-
new_keys,
12790-
))
12763+
Ok(())
1279112764
}
1279212765

1279312766
fn validate_splice_contributions(
@@ -12927,17 +12900,46 @@ where
1292712900
pub(crate) fn splice_init<ES: EntropySource, L: Logger>(
1292812901
&mut self, msg: &msgs::SpliceInit, entropy_source: &ES, holder_node_id: &PublicKey,
1292912902
logger: &L,
12930-
) -> Result<msgs::SpliceAck, ChannelError> {
12903+
) -> Result<msgs::SpliceAck, InteractiveTxMsgError> {
12904+
self.validate_splice_init(msg).map_err(|e| self.quiescent_negotiation_err(e))?;
12905+
1293112906
let feerate = FeeRate::from_sat_per_kwu(msg.funding_feerate_per_kw as u64);
12932-
let (our_funding_contribution, holder_balance) =
12933-
self.resolve_queued_contribution(feerate, logger)?;
12907+
let (queued_net_value, holder_balance) = self
12908+
.resolve_queued_contribution(feerate, logger)
12909+
.map_err(|e| self.quiescent_negotiation_err(e))?;
12910+
12911+
let our_funding_contribution = queued_net_value.unwrap_or(SignedAmount::ZERO);
12912+
let their_funding_contribution = SignedAmount::from_sat(msg.funding_contribution_satoshis);
12913+
self.validate_splice_contributions(our_funding_contribution, their_funding_contribution)
12914+
.map_err(|e| self.quiescent_negotiation_err(ChannelError::WarnAndDisconnect(e)))?;
1293412915

12935-
let splice_funding =
12936-
self.validate_splice_init(msg, our_funding_contribution.unwrap_or(SignedAmount::ZERO))?;
12916+
// Rotate the pubkeys using the prev_funding_txid as a tweak
12917+
let prev_funding_txid = self.funding.get_funding_txid();
12918+
let funding_pubkey = match prev_funding_txid {
12919+
None => {
12920+
debug_assert!(false);
12921+
self.funding.get_holder_pubkeys().funding_pubkey
12922+
},
12923+
Some(prev_funding_txid) => self
12924+
.context
12925+
.holder_signer
12926+
.new_funding_pubkey(prev_funding_txid, &self.context.secp_ctx),
12927+
};
12928+
let mut holder_pubkeys = self.funding.get_holder_pubkeys().clone();
12929+
holder_pubkeys.funding_pubkey = funding_pubkey;
12930+
12931+
let splice_funding = FundingScope::for_splice(
12932+
&self.funding,
12933+
&self.context,
12934+
our_funding_contribution,
12935+
their_funding_contribution,
12936+
msg.funding_pubkey,
12937+
holder_pubkeys,
12938+
);
1293712939

1293812940
// Adjust for the feerate and clone so we can store it for future RBF re-use.
1293912941
let (adjusted_contribution, our_funding_inputs, our_funding_outputs) =
12940-
if our_funding_contribution.is_some() {
12942+
if queued_net_value.is_some() {
1294112943
let adjusted_contribution = self
1294212944
.take_queued_funding_contribution()
1294312945
.expect("queued_funding_contribution was Some")
@@ -12948,7 +12950,6 @@ where
1294812950
} else {
1294912951
(None, Default::default(), Default::default())
1295012952
};
12951-
let our_funding_contribution = our_funding_contribution.unwrap_or(SignedAmount::ZERO);
1295212953

1295312954
log_info!(
1295412955
logger,
@@ -12991,9 +12992,8 @@ where
1299112992

1299212993
/// Checks during handling tx_init_rbf for an existing splice
1299312994
fn validate_tx_init_rbf<F: FeeEstimator>(
12994-
&self, msg: &msgs::TxInitRbf, our_funding_contribution: SignedAmount,
12995-
fee_estimator: &LowerBoundedFeeEstimator<F>,
12996-
) -> Result<FundingScope, ChannelError> {
12995+
&self, msg: &msgs::TxInitRbf, fee_estimator: &LowerBoundedFeeEstimator<F>,
12996+
) -> Result<(ChannelPublicKeys, PublicKey), ChannelError> {
1299712997
if self.holder_commitment_point.current_point().is_none() {
1299812998
return Err(ChannelError::WarnAndDisconnect(format!(
1299912999
"Channel {} commitment point needs to be advanced once before RBF",
@@ -13059,36 +13059,26 @@ where
1305913059
return Err(ChannelError::Abort(AbortReason::InsufficientRbfFeerate));
1306013060
}
1306113061

13062-
let their_funding_contribution = match msg.funding_output_contribution {
13063-
Some(value) => SignedAmount::from_sat(value),
13064-
None => SignedAmount::ZERO,
13065-
};
13066-
13067-
self.validate_splice_contributions(our_funding_contribution, their_funding_contribution)
13068-
.map_err(|e| ChannelError::WarnAndDisconnect(e))?;
13069-
1307013062
// Reuse funding pubkeys from the last negotiated candidate since all RBF candidates
1307113063
// for the same splice share the same funding output script.
13072-
let holder_pubkeys = last_candidate.get_holder_pubkeys().clone();
13073-
let counterparty_funding_pubkey = *last_candidate.counterparty_funding_pubkey();
13074-
13075-
Ok(FundingScope::for_splice(
13076-
&self.funding,
13077-
&self.context,
13078-
our_funding_contribution,
13079-
their_funding_contribution,
13080-
counterparty_funding_pubkey,
13081-
holder_pubkeys,
13064+
Ok((
13065+
last_candidate.get_holder_pubkeys().clone(),
13066+
*last_candidate.counterparty_funding_pubkey(),
1308213067
))
1308313068
}
1308413069

1308513070
pub(crate) fn tx_init_rbf<ES: EntropySource, F: FeeEstimator, L: Logger>(
1308613071
&mut self, msg: &msgs::TxInitRbf, entropy_source: &ES, holder_node_id: &PublicKey,
1308713072
fee_estimator: &LowerBoundedFeeEstimator<F>, logger: &L,
13088-
) -> Result<msgs::TxAckRbf, ChannelError> {
13073+
) -> Result<msgs::TxAckRbf, InteractiveTxMsgError> {
13074+
let (holder_pubkeys, counterparty_funding_pubkey) = self
13075+
.validate_tx_init_rbf(msg, fee_estimator)
13076+
.map_err(|e| self.quiescent_negotiation_err(e))?;
13077+
1308913078
let feerate = FeeRate::from_sat_per_kwu(msg.feerate_sat_per_1000_weight as u64);
13090-
let (queued_net_value, holder_balance) =
13091-
self.resolve_queued_contribution(feerate, logger)?;
13079+
let (queued_net_value, holder_balance) = self
13080+
.resolve_queued_contribution(feerate, logger)
13081+
.map_err(|e| self.quiescent_negotiation_err(e))?;
1309213082

1309313083
// If no queued contribution, try prior contribution from previous negotiation.
1309413084
// Failing here means the RBF would erase our splice — reject it.
@@ -13105,19 +13095,31 @@ where
1310513095
prior
1310613096
.net_value_for_acceptor_at_feerate(feerate, holder_balance)
1310713097
.map_err(|_| ChannelError::Abort(AbortReason::InsufficientRbfFeerate))
13108-
})?;
13098+
})
13099+
.map_err(|e| self.quiescent_negotiation_err(e))?;
1310913100
Some(net_value)
1311013101
} else {
1311113102
None
1311213103
};
1311313104

1311413105
let our_funding_contribution = queued_net_value.or(prior_net_value);
13106+
let our_funding_contribution = our_funding_contribution.unwrap_or(SignedAmount::ZERO);
1311513107

13116-
let rbf_funding = self.validate_tx_init_rbf(
13117-
msg,
13118-
our_funding_contribution.unwrap_or(SignedAmount::ZERO),
13119-
fee_estimator,
13120-
)?;
13108+
let their_funding_contribution = match msg.funding_output_contribution {
13109+
Some(value) => SignedAmount::from_sat(value),
13110+
None => SignedAmount::ZERO,
13111+
};
13112+
self.validate_splice_contributions(our_funding_contribution, their_funding_contribution)
13113+
.map_err(|e| self.quiescent_negotiation_err(ChannelError::WarnAndDisconnect(e)))?;
13114+
13115+
let rbf_funding = FundingScope::for_splice(
13116+
&self.funding,
13117+
&self.context,
13118+
our_funding_contribution,
13119+
their_funding_contribution,
13120+
counterparty_funding_pubkey,
13121+
holder_pubkeys,
13122+
);
1312113123

1312213124
// Consume the appropriate contribution source.
1312313125
let (our_funding_inputs, our_funding_outputs) = if queued_net_value.is_some() {
@@ -13154,8 +13156,6 @@ where
1315413156
Default::default()
1315513157
};
1315613158

13157-
let our_funding_contribution = our_funding_contribution.unwrap_or(SignedAmount::ZERO);
13158-
1315913159
log_info!(
1316013160
logger,
1316113161
"Starting RBF funding negotiation for channel {} after receiving tx_init_rbf; channel value: {} sats",
@@ -14285,6 +14285,16 @@ where
1428514285
was_quiescent
1428614286
}
1428714287

14288+
fn quiescent_negotiation_err(&mut self, err: ChannelError) -> InteractiveTxMsgError {
14289+
let exited_quiescence = if matches!(err, ChannelError::Abort(_)) {
14290+
debug_assert!(self.context.channel_state.is_quiescent());
14291+
self.exit_quiescence()
14292+
} else {
14293+
false
14294+
};
14295+
InteractiveTxMsgError { err, splice_funding_failed: None, exited_quiescence }
14296+
}
14297+
1428814298
pub fn remove_legacy_scids_before_block(&mut self, height: u32) -> alloc::vec::Drain<'_, u64> {
1428914299
let end = self
1429014300
.funding

0 commit comments

Comments
 (0)