Skip to content

Commit 2379686

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, and internal_tx_init_rbf. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 0d4c84f commit 2379686

File tree

2 files changed

+105
-78
lines changed

2 files changed

+105
-78
lines changed

lightning/src/ln/channel.rs

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12890,13 +12890,15 @@ where
1289012890
pub(crate) fn splice_init<ES: EntropySource, L: Logger>(
1289112891
&mut self, msg: &msgs::SpliceInit, entropy_source: &ES, holder_node_id: &PublicKey,
1289212892
logger: &L,
12893-
) -> Result<msgs::SpliceAck, ChannelError> {
12893+
) -> Result<msgs::SpliceAck, InteractiveTxMsgError> {
1289412894
let feerate = FeeRate::from_sat_per_kwu(msg.funding_feerate_per_kw as u64);
12895-
let (our_funding_contribution, holder_balance) =
12896-
self.resolve_queued_contribution(feerate, logger)?;
12895+
let (our_funding_contribution, holder_balance) = self
12896+
.resolve_queued_contribution(feerate, logger)
12897+
.map_err(|e| self.quiescent_negotiation_err(e))?;
1289712898

12898-
let splice_funding =
12899-
self.validate_splice_init(msg, our_funding_contribution.unwrap_or(SignedAmount::ZERO))?;
12899+
let splice_funding = self
12900+
.validate_splice_init(msg, our_funding_contribution.unwrap_or(SignedAmount::ZERO))
12901+
.map_err(|e| self.quiescent_negotiation_err(e))?;
1290012902

1290112903
// Adjust for the feerate and clone so we can store it for future RBF re-use.
1290212904
let (adjusted_contribution, our_funding_inputs, our_funding_outputs) =
@@ -13048,10 +13050,11 @@ where
1304813050
pub(crate) fn tx_init_rbf<ES: EntropySource, F: FeeEstimator, L: Logger>(
1304913051
&mut self, msg: &msgs::TxInitRbf, entropy_source: &ES, holder_node_id: &PublicKey,
1305013052
fee_estimator: &LowerBoundedFeeEstimator<F>, logger: &L,
13051-
) -> Result<msgs::TxAckRbf, ChannelError> {
13053+
) -> Result<msgs::TxAckRbf, InteractiveTxMsgError> {
1305213054
let feerate = FeeRate::from_sat_per_kwu(msg.feerate_sat_per_1000_weight as u64);
13053-
let (queued_net_value, holder_balance) =
13054-
self.resolve_queued_contribution(feerate, logger)?;
13055+
let (queued_net_value, holder_balance) = self
13056+
.resolve_queued_contribution(feerate, logger)
13057+
.map_err(|e| self.quiescent_negotiation_err(e))?;
1305513058

1305613059
// If no queued contribution, try prior contribution from previous negotiation.
1305713060
// Failing here means the RBF would erase our splice — reject it.
@@ -13068,19 +13071,22 @@ where
1306813071
prior
1306913072
.net_value_for_acceptor_at_feerate(feerate, holder_balance)
1307013073
.map_err(|_| ChannelError::Abort(AbortReason::InsufficientRbfFeerate))
13071-
})?;
13074+
})
13075+
.map_err(|e| self.quiescent_negotiation_err(e))?;
1307213076
Some(net_value)
1307313077
} else {
1307413078
None
1307513079
};
1307613080

1307713081
let our_funding_contribution = queued_net_value.or(prior_net_value);
1307813082

13079-
let rbf_funding = self.validate_tx_init_rbf(
13080-
msg,
13081-
our_funding_contribution.unwrap_or(SignedAmount::ZERO),
13082-
fee_estimator,
13083-
)?;
13083+
let rbf_funding = self
13084+
.validate_tx_init_rbf(
13085+
msg,
13086+
our_funding_contribution.unwrap_or(SignedAmount::ZERO),
13087+
fee_estimator,
13088+
)
13089+
.map_err(|e| self.quiescent_negotiation_err(e))?;
1308413090

1308513091
// Consume the appropriate contribution source.
1308613092
let (our_funding_inputs, our_funding_outputs) = if queued_net_value.is_some() {
@@ -14249,6 +14255,12 @@ where
1424914255
was_quiescent
1425014256
}
1425114257

14258+
fn quiescent_negotiation_err(&mut self, err: ChannelError) -> InteractiveTxMsgError {
14259+
let exited_quiescence =
14260+
if matches!(err, ChannelError::Abort(_)) { self.exit_quiescence() } else { false };
14261+
InteractiveTxMsgError { err, splice_funding_failed: None, exited_quiescence }
14262+
}
14263+
1425214264
pub fn remove_legacy_scids_before_block(&mut self, height: u32) -> alloc::vec::Drain<'_, u64> {
1425314265
let end = self
1425414266
.funding

lightning/src/ln/channelmanager.rs

Lines changed: 79 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -11823,6 +11823,39 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1182311823
}
1182411824
}
1182511825

11826+
fn handle_interactive_tx_msg_err(
11827+
&self, err: InteractiveTxMsgError, channel_id: ChannelId, counterparty_node_id: &PublicKey,
11828+
user_channel_id: u128,
11829+
) -> MsgHandleErrInternal {
11830+
if let Some(splice_funding_failed) = err.splice_funding_failed {
11831+
let pending_events = &mut self.pending_events.lock().unwrap();
11832+
pending_events.push_back((
11833+
events::Event::SpliceFailed {
11834+
channel_id,
11835+
counterparty_node_id: *counterparty_node_id,
11836+
user_channel_id,
11837+
abandoned_funding_txo: splice_funding_failed.funding_txo,
11838+
channel_type: splice_funding_failed.channel_type.clone(),
11839+
},
11840+
None,
11841+
));
11842+
pending_events.push_back((
11843+
events::Event::DiscardFunding {
11844+
channel_id,
11845+
funding_info: FundingInfo::Contribution {
11846+
inputs: splice_funding_failed.contributed_inputs,
11847+
outputs: splice_funding_failed.contributed_outputs,
11848+
},
11849+
},
11850+
None,
11851+
));
11852+
}
11853+
debug_assert!(!err.exited_quiescence || matches!(err.err, ChannelError::Abort(_)));
11854+
11855+
MsgHandleErrInternal::from_chan_no_close(err.err, channel_id)
11856+
.with_exited_quiescence(err.exited_quiescence)
11857+
}
11858+
1182611859
fn internal_tx_msg<
1182711860
HandleTxMsgFn: Fn(&mut Channel<SP>) -> Result<InteractiveTxMessageSend, InteractiveTxMsgError>,
1182811861
>(
@@ -11844,38 +11877,14 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1184411877
peer_state.pending_msg_events.push(msg_send_event);
1184511878
Ok(())
1184611879
},
11847-
Err(InteractiveTxMsgError {
11848-
err,
11849-
splice_funding_failed,
11850-
exited_quiescence,
11851-
}) => {
11852-
if let Some(splice_funding_failed) = splice_funding_failed {
11853-
let pending_events = &mut self.pending_events.lock().unwrap();
11854-
pending_events.push_back((
11855-
events::Event::SpliceFailed {
11856-
channel_id,
11857-
counterparty_node_id: *counterparty_node_id,
11858-
user_channel_id: channel.context().get_user_id(),
11859-
abandoned_funding_txo: splice_funding_failed.funding_txo,
11860-
channel_type: splice_funding_failed.channel_type.clone(),
11861-
},
11862-
None,
11863-
));
11864-
pending_events.push_back((
11865-
events::Event::DiscardFunding {
11866-
channel_id,
11867-
funding_info: FundingInfo::Contribution {
11868-
inputs: splice_funding_failed.contributed_inputs,
11869-
outputs: splice_funding_failed.contributed_outputs,
11870-
},
11871-
},
11872-
None,
11873-
));
11874-
}
11875-
debug_assert!(!exited_quiescence || matches!(err, ChannelError::Abort(_)));
11876-
11877-
Err(MsgHandleErrInternal::from_chan_no_close(err, channel_id)
11878-
.with_exited_quiescence(exited_quiescence))
11880+
Err(err) => {
11881+
let user_channel_id = channel.context().get_user_id();
11882+
Err(self.handle_interactive_tx_msg_err(
11883+
err,
11884+
channel_id,
11885+
counterparty_node_id,
11886+
user_channel_id,
11887+
))
1187911888
},
1188011889
}
1188111890
},
@@ -13258,27 +13267,30 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1325813267
}
1325913268

1326013269
if let Some(ref mut funded_channel) = chan_entry.get_mut().as_funded_mut() {
13261-
let init_res = funded_channel.splice_init(
13270+
let user_channel_id = funded_channel.context.get_user_id();
13271+
match funded_channel.splice_init(
1326213272
msg,
1326313273
&self.entropy_source,
1326413274
&self.get_our_node_id(),
1326513275
&self.logger,
13266-
);
13267-
if let Err(ChannelError::Abort(_)) = &init_res {
13268-
funded_channel.exit_quiescence();
13269-
let chan_id = funded_channel.context.channel_id();
13270-
let res = MsgHandleErrInternal::from_chan_no_close(
13271-
init_res.unwrap_err(),
13272-
chan_id,
13273-
);
13274-
return Err(res.with_exited_quiescence(true));
13276+
) {
13277+
Ok(splice_ack_msg) => {
13278+
peer_state.pending_msg_events.push(MessageSendEvent::SendSpliceAck {
13279+
node_id: *counterparty_node_id,
13280+
msg: splice_ack_msg,
13281+
});
13282+
Ok(())
13283+
},
13284+
Err(err) => {
13285+
debug_assert!(err.splice_funding_failed.is_none());
13286+
Err(self.handle_interactive_tx_msg_err(
13287+
err,
13288+
msg.channel_id,
13289+
counterparty_node_id,
13290+
user_channel_id,
13291+
))
13292+
},
1327513293
}
13276-
let splice_ack_msg = try_channel_entry!(self, peer_state, init_res, chan_entry);
13277-
peer_state.pending_msg_events.push(MessageSendEvent::SendSpliceAck {
13278-
node_id: *counterparty_node_id,
13279-
msg: splice_ack_msg,
13280-
});
13281-
Ok(())
1328213294
} else {
1328313295
try_channel_entry!(
1328413296
self,
@@ -13311,28 +13323,31 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1331113323
},
1331213324
hash_map::Entry::Occupied(mut chan_entry) => {
1331313325
if let Some(ref mut funded_channel) = chan_entry.get_mut().as_funded_mut() {
13314-
let init_res = funded_channel.tx_init_rbf(
13326+
let user_channel_id = funded_channel.context.get_user_id();
13327+
match funded_channel.tx_init_rbf(
1331513328
msg,
1331613329
&self.entropy_source,
1331713330
&self.get_our_node_id(),
1331813331
&self.fee_estimator,
1331913332
&self.logger,
13320-
);
13321-
if let Err(ChannelError::Abort(_)) = &init_res {
13322-
funded_channel.exit_quiescence();
13323-
let chan_id = funded_channel.context.channel_id();
13324-
let res = MsgHandleErrInternal::from_chan_no_close(
13325-
init_res.unwrap_err(),
13326-
chan_id,
13327-
);
13328-
return Err(res.with_exited_quiescence(true));
13333+
) {
13334+
Ok(tx_ack_rbf_msg) => {
13335+
peer_state.pending_msg_events.push(MessageSendEvent::SendTxAckRbf {
13336+
node_id: *counterparty_node_id,
13337+
msg: tx_ack_rbf_msg,
13338+
});
13339+
Ok(())
13340+
},
13341+
Err(err) => {
13342+
debug_assert!(err.splice_funding_failed.is_none());
13343+
Err(self.handle_interactive_tx_msg_err(
13344+
err,
13345+
msg.channel_id,
13346+
counterparty_node_id,
13347+
user_channel_id,
13348+
))
13349+
},
1332913350
}
13330-
let tx_ack_rbf_msg = try_channel_entry!(self, peer_state, init_res, chan_entry);
13331-
peer_state.pending_msg_events.push(MessageSendEvent::SendTxAckRbf {
13332-
node_id: *counterparty_node_id,
13333-
msg: tx_ack_rbf_msg,
13334-
});
13335-
Ok(())
1333613351
} else {
1333713352
try_channel_entry!(
1333813353
self,

0 commit comments

Comments
 (0)