Skip to content

Commit 959b553

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 a4bfe3a commit 959b553

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
@@ -12595,13 +12595,15 @@ where
1259512595
pub(crate) fn splice_init<ES: EntropySource, L: Logger>(
1259612596
&mut self, msg: &msgs::SpliceInit, entropy_source: &ES, holder_node_id: &PublicKey,
1259712597
logger: &L,
12598-
) -> Result<msgs::SpliceAck, ChannelError> {
12598+
) -> Result<msgs::SpliceAck, InteractiveTxMsgError> {
1259912599
let feerate = FeeRate::from_sat_per_kwu(msg.funding_feerate_per_kw as u64);
12600-
let (our_funding_contribution, holder_balance) =
12601-
self.resolve_queued_contribution(feerate, logger)?;
12600+
let (our_funding_contribution, holder_balance) = self
12601+
.resolve_queued_contribution(feerate, logger)
12602+
.map_err(|e| self.quiescent_negotiation_err(e))?;
1260212603

12603-
let splice_funding =
12604-
self.validate_splice_init(msg, our_funding_contribution.unwrap_or(SignedAmount::ZERO))?;
12604+
let splice_funding = self
12605+
.validate_splice_init(msg, our_funding_contribution.unwrap_or(SignedAmount::ZERO))
12606+
.map_err(|e| self.quiescent_negotiation_err(e))?;
1260512607

1260612608
// Adjust for the feerate and clone so we can store it for future RBF re-use.
1260712609
let (adjusted_contribution, our_funding_inputs, our_funding_outputs) =
@@ -12753,10 +12755,11 @@ where
1275312755
pub(crate) fn tx_init_rbf<ES: EntropySource, F: FeeEstimator, L: Logger>(
1275412756
&mut self, msg: &msgs::TxInitRbf, entropy_source: &ES, holder_node_id: &PublicKey,
1275512757
fee_estimator: &LowerBoundedFeeEstimator<F>, logger: &L,
12756-
) -> Result<msgs::TxAckRbf, ChannelError> {
12758+
) -> Result<msgs::TxAckRbf, InteractiveTxMsgError> {
1275712759
let feerate = FeeRate::from_sat_per_kwu(msg.feerate_sat_per_1000_weight as u64);
12758-
let (queued_net_value, holder_balance) =
12759-
self.resolve_queued_contribution(feerate, logger)?;
12760+
let (queued_net_value, holder_balance) = self
12761+
.resolve_queued_contribution(feerate, logger)
12762+
.map_err(|e| self.quiescent_negotiation_err(e))?;
1276012763

1276112764
// If no queued contribution, try prior contribution from previous negotiation.
1276212765
// Failing here means the RBF would erase our splice — reject it.
@@ -12773,19 +12776,22 @@ where
1277312776
prior
1277412777
.net_value_for_acceptor_at_feerate(feerate, holder_balance)
1277512778
.map_err(|_| ChannelError::Abort(AbortReason::InsufficientRbfFeerate))
12776-
})?;
12779+
})
12780+
.map_err(|e| self.quiescent_negotiation_err(e))?;
1277712781
Some(net_value)
1277812782
} else {
1277912783
None
1278012784
};
1278112785

1278212786
let our_funding_contribution = queued_net_value.or(prior_net_value);
1278312787

12784-
let rbf_funding = self.validate_tx_init_rbf(
12785-
msg,
12786-
our_funding_contribution.unwrap_or(SignedAmount::ZERO),
12787-
fee_estimator,
12788-
)?;
12788+
let rbf_funding = self
12789+
.validate_tx_init_rbf(
12790+
msg,
12791+
our_funding_contribution.unwrap_or(SignedAmount::ZERO),
12792+
fee_estimator,
12793+
)
12794+
.map_err(|e| self.quiescent_negotiation_err(e))?;
1278912795

1279012796
// Consume the appropriate contribution source.
1279112797
let (our_funding_inputs, our_funding_outputs) = if queued_net_value.is_some() {
@@ -13961,6 +13967,12 @@ where
1396113967
was_quiescent
1396213968
}
1396313969

13970+
fn quiescent_negotiation_err(&mut self, err: ChannelError) -> InteractiveTxMsgError {
13971+
let exited_quiescence =
13972+
if matches!(err, ChannelError::Abort(_)) { self.exit_quiescence() } else { false };
13973+
InteractiveTxMsgError { err, splice_funding_failed: None, exited_quiescence }
13974+
}
13975+
1396413976
pub fn remove_legacy_scids_before_block(&mut self, height: u32) -> alloc::vec::Drain<'_, u64> {
1396513977
let end = self
1396613978
.funding

lightning/src/ln/channelmanager.rs

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

11676+
fn handle_interactive_tx_msg_err(
11677+
&self, err: InteractiveTxMsgError, channel_id: ChannelId, counterparty_node_id: &PublicKey,
11678+
user_channel_id: u128,
11679+
) -> MsgHandleErrInternal {
11680+
if let Some(splice_funding_failed) = err.splice_funding_failed {
11681+
let pending_events = &mut self.pending_events.lock().unwrap();
11682+
pending_events.push_back((
11683+
events::Event::SpliceFailed {
11684+
channel_id,
11685+
counterparty_node_id: *counterparty_node_id,
11686+
user_channel_id,
11687+
abandoned_funding_txo: splice_funding_failed.funding_txo,
11688+
channel_type: splice_funding_failed.channel_type.clone(),
11689+
},
11690+
None,
11691+
));
11692+
pending_events.push_back((
11693+
events::Event::DiscardFunding {
11694+
channel_id,
11695+
funding_info: FundingInfo::Contribution {
11696+
inputs: splice_funding_failed.contributed_inputs,
11697+
outputs: splice_funding_failed.contributed_outputs,
11698+
},
11699+
},
11700+
None,
11701+
));
11702+
}
11703+
debug_assert!(!err.exited_quiescence || matches!(err.err, ChannelError::Abort(_)));
11704+
11705+
MsgHandleErrInternal::from_chan_no_close(err.err, channel_id)
11706+
.with_exited_quiescence(err.exited_quiescence)
11707+
}
11708+
1167611709
fn internal_tx_msg<
1167711710
HandleTxMsgFn: Fn(&mut Channel<SP>) -> Result<InteractiveTxMessageSend, InteractiveTxMsgError>,
1167811711
>(
@@ -11694,38 +11727,14 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1169411727
peer_state.pending_msg_events.push(msg_send_event);
1169511728
Ok(())
1169611729
},
11697-
Err(InteractiveTxMsgError {
11698-
err,
11699-
splice_funding_failed,
11700-
exited_quiescence,
11701-
}) => {
11702-
if let Some(splice_funding_failed) = splice_funding_failed {
11703-
let pending_events = &mut self.pending_events.lock().unwrap();
11704-
pending_events.push_back((
11705-
events::Event::SpliceFailed {
11706-
channel_id,
11707-
counterparty_node_id: *counterparty_node_id,
11708-
user_channel_id: channel.context().get_user_id(),
11709-
abandoned_funding_txo: splice_funding_failed.funding_txo,
11710-
channel_type: splice_funding_failed.channel_type.clone(),
11711-
},
11712-
None,
11713-
));
11714-
pending_events.push_back((
11715-
events::Event::DiscardFunding {
11716-
channel_id,
11717-
funding_info: FundingInfo::Contribution {
11718-
inputs: splice_funding_failed.contributed_inputs,
11719-
outputs: splice_funding_failed.contributed_outputs,
11720-
},
11721-
},
11722-
None,
11723-
));
11724-
}
11725-
debug_assert!(!exited_quiescence || matches!(err, ChannelError::Abort(_)));
11726-
11727-
Err(MsgHandleErrInternal::from_chan_no_close(err, channel_id)
11728-
.with_exited_quiescence(exited_quiescence))
11730+
Err(err) => {
11731+
let user_channel_id = channel.context().get_user_id();
11732+
Err(self.handle_interactive_tx_msg_err(
11733+
err,
11734+
channel_id,
11735+
counterparty_node_id,
11736+
user_channel_id,
11737+
))
1172911738
},
1173011739
}
1173111740
},
@@ -13100,27 +13109,30 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1310013109
}
1310113110

1310213111
if let Some(ref mut funded_channel) = chan_entry.get_mut().as_funded_mut() {
13103-
let init_res = funded_channel.splice_init(
13112+
let user_channel_id = funded_channel.context.get_user_id();
13113+
match funded_channel.splice_init(
1310413114
msg,
1310513115
&self.entropy_source,
1310613116
&self.get_our_node_id(),
1310713117
&self.logger,
13108-
);
13109-
if let Err(ChannelError::Abort(_)) = &init_res {
13110-
funded_channel.exit_quiescence();
13111-
let chan_id = funded_channel.context.channel_id();
13112-
let res = MsgHandleErrInternal::from_chan_no_close(
13113-
init_res.unwrap_err(),
13114-
chan_id,
13115-
);
13116-
return Err(res.with_exited_quiescence(true));
13118+
) {
13119+
Ok(splice_ack_msg) => {
13120+
peer_state.pending_msg_events.push(MessageSendEvent::SendSpliceAck {
13121+
node_id: *counterparty_node_id,
13122+
msg: splice_ack_msg,
13123+
});
13124+
Ok(())
13125+
},
13126+
Err(err) => {
13127+
debug_assert!(err.splice_funding_failed.is_none());
13128+
Err(self.handle_interactive_tx_msg_err(
13129+
err,
13130+
msg.channel_id,
13131+
counterparty_node_id,
13132+
user_channel_id,
13133+
))
13134+
},
1311713135
}
13118-
let splice_ack_msg = try_channel_entry!(self, peer_state, init_res, chan_entry);
13119-
peer_state.pending_msg_events.push(MessageSendEvent::SendSpliceAck {
13120-
node_id: *counterparty_node_id,
13121-
msg: splice_ack_msg,
13122-
});
13123-
Ok(())
1312413136
} else {
1312513137
try_channel_entry!(
1312613138
self,
@@ -13153,28 +13165,31 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1315313165
},
1315413166
hash_map::Entry::Occupied(mut chan_entry) => {
1315513167
if let Some(ref mut funded_channel) = chan_entry.get_mut().as_funded_mut() {
13156-
let init_res = funded_channel.tx_init_rbf(
13168+
let user_channel_id = funded_channel.context.get_user_id();
13169+
match funded_channel.tx_init_rbf(
1315713170
msg,
1315813171
&self.entropy_source,
1315913172
&self.get_our_node_id(),
1316013173
&self.fee_estimator,
1316113174
&self.logger,
13162-
);
13163-
if let Err(ChannelError::Abort(_)) = &init_res {
13164-
funded_channel.exit_quiescence();
13165-
let chan_id = funded_channel.context.channel_id();
13166-
let res = MsgHandleErrInternal::from_chan_no_close(
13167-
init_res.unwrap_err(),
13168-
chan_id,
13169-
);
13170-
return Err(res.with_exited_quiescence(true));
13175+
) {
13176+
Ok(tx_ack_rbf_msg) => {
13177+
peer_state.pending_msg_events.push(MessageSendEvent::SendTxAckRbf {
13178+
node_id: *counterparty_node_id,
13179+
msg: tx_ack_rbf_msg,
13180+
});
13181+
Ok(())
13182+
},
13183+
Err(err) => {
13184+
debug_assert!(err.splice_funding_failed.is_none());
13185+
Err(self.handle_interactive_tx_msg_err(
13186+
err,
13187+
msg.channel_id,
13188+
counterparty_node_id,
13189+
user_channel_id,
13190+
))
13191+
},
1317113192
}
13172-
let tx_ack_rbf_msg = try_channel_entry!(self, peer_state, init_res, chan_entry);
13173-
peer_state.pending_msg_events.push(MessageSendEvent::SendTxAckRbf {
13174-
node_id: *counterparty_node_id,
13175-
msg: tx_ack_rbf_msg,
13176-
});
13177-
Ok(())
1317813193
} else {
1317913194
try_channel_entry!(
1318013195
self,

0 commit comments

Comments
 (0)