← Watch feed
Fail interactive-tx negotiation on abort
Public commit record
68/100 · Adequate
What the developer wrote
Authored by Wilmer Paulino
Fail interactive-tx negotiation on abort
This commit reworks all interactive transaction construction methods to
mark the negotiation as failed upon a local/remote `TxAbort`, ensuring
the `InteractiveTxConstructor` is consumed. Along the way, we refactor
the handling of `tx_complete` such that we only have a single call into
the `Channel` from the `ChannelManager`.
✓ Descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context
What changed, and why it matters
This commit is a defensive refactor of how Lightning transaction negotiations are aborted. It makes sure that when either side cancels an interactive funding/splicing negotiation, the internal constructor object is consumed (taken away) so it cannot be reused. It also centralizes the handling of the final 'tx_complete' message so the channel manager calls the channel object only once. The change appears to close a state-handling gap where a failed negotiation might leave stale state behind, but the commit itself does not describe a specific vulnerability or assign a CVE.
Recommended action
Treat as a hardening/state-correctness improvement rather than an urgent security fix. Reviewers should verify that fail_interactive_tx_negotiation is invoked on every error path that can leave the constructor in a bad state, and that the new tx_complete flow does not skip required signature validation or commitment advancement checks. Monitor project release notes for any later security classification.
Security signals we found
01
State cleanup on abort: interactive constructor is taken/consumed, preventing reuse after failure
02
Centralized tx_complete handling reduces duplicated state transitions between ChannelManager and Channel
03
Removal of public as_unfunded_v2_mut and narrowing of interactive_tx_constructor_mut/funding_tx_constructed visibility
04
Error propagation changed from raw TxAbort messages to typed AbortReason, with conversion to TxAbort happening in one place
05
Channel no longer force-closes on funding_tx_constructed failure for unfunded channels; instead sends TxAbort
06
No CVE, advisory, or researcher attribution present in commit or supplied references
Evidence from the diff
The patch moves interactive-transaction message handling (tx_add_input, tx_add_output, tx_remove_input, tx_remove_output, tx_complete, tx_abort) from ChannelManager into new Channel methods. A new fail_interactive_tx_negotiation helper takes (consumes) the InteractiveTxConstructor or pending splice funding negotiation on any abort, returning a TxAbort. tx_complete now returns an enum (HandleTxCompleteValue) and, when negotiation completes, calls funding_tx_constructed internally, producing the CommitmentSigned in one place. Error paths in funding_tx_constructed now return AbortReason instead of raw TxAbort, and ChannelManager no longer closes unfunded channels on tx_complete failure. A splicing test is updated to expect only a TxAbort after an incomplete splice negotiation.
Changed components
lightning/src/ln/channel.rslightning/src/ln/channelmanager.rslightning/src/ln/interactivetxs.rslightning/src/ln/splicing_tests.rsInspect captured patch +252 / −228
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 1065803..c14a571 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -63,12 +63,11 @@ use crate::ln::funding::FundingTxInput;
#[cfg(splicing)]
use crate::ln::funding::SpliceContribution;
#[cfg(splicing)]
+use crate::ln::interactivetxs::calculate_change_output_value;
use crate::ln::interactivetxs::{
- calculate_change_output_value, AbortReason, InteractiveTxMessageSend,
-};
-use crate::ln::interactivetxs::{
- get_output_weight, InteractiveTxConstructor, InteractiveTxConstructorArgs,
- InteractiveTxSigningSession, SharedOwnedInput, SharedOwnedOutput, TX_COMMON_FIELDS_WEIGHT,
+ get_output_weight, AbortReason, HandleTxCompleteValue, InteractiveTxConstructor,
+ InteractiveTxConstructorArgs, InteractiveTxMessageSend, InteractiveTxSigningSession,
+ SharedOwnedInput, SharedOwnedOutput, TX_COMMON_FIELDS_WEIGHT,
};
use crate::ln::msgs;
use crate::ln::msgs::{ClosingSigned, ClosingSignedFeeRange, DecodeError, OnionErrorPacket};
@@ -1597,14 +1596,6 @@ where
}
}
- pub fn as_unfunded_v2_mut(&mut self) -> Option<&mut PendingV2Channel<SP>> {
- if let ChannelPhase::UnfundedV2(channel) = &mut self.phase {
- Some(channel)
- } else {
- None
- }
- }
-
#[rustfmt::skip]
pub fn signer_maybe_unblocked<L: Deref>(
&mut self, chain_hash: ChainHash, logger: &L,
@@ -1739,7 +1730,7 @@ where
}
}
- pub fn interactive_tx_constructor_mut(&mut self) -> Option<&mut InteractiveTxConstructor> {
+ fn interactive_tx_constructor_mut(&mut self) -> Option<&mut InteractiveTxConstructor> {
match &mut self.phase {
ChannelPhase::UnfundedV2(chan) => chan.interactive_tx_constructor.as_mut(),
#[cfg(splicing)]
@@ -1748,6 +1739,195 @@ where
}
}
+ fn fail_interactive_tx_negotiation<L: Deref>(
+ &mut self, reason: AbortReason, logger: &L,
+ ) -> msgs::TxAbort
+ where
+ L::Target: Logger,
+ {
+ let logger = WithChannelContext::from(logger, &self.context(), None);
+ log_info!(logger, "Failed interactive transaction negotiation: {reason}");
+
+ let _interactive_tx_constructor = match &mut self.phase {
+ ChannelPhase::Undefined => unreachable!(),
+ ChannelPhase::UnfundedOutboundV1(_) | ChannelPhase::UnfundedInboundV1(_) => None,
+ ChannelPhase::UnfundedV2(pending_v2_channel) => {
+ pending_v2_channel.interactive_tx_constructor.take()
+ },
+ #[cfg(not(splicing))]
+ ChannelPhase::Funded(_) => unreachable!(),
+ #[cfg(splicing)]
+ ChannelPhase::Funded(funded_channel) => funded_channel
+ .pending_splice
+ .as_mut()
+ .and_then(|pending_splice| pending_splice.funding_negotiation.take())
+ .and_then(|funding_negotiation| {
+ if let FundingNegotiation::ConstructingTransaction(
+ _,
+ interactive_tx_constructor,
+ ) = funding_negotiation
+ {
+ Some(interactive_tx_constructor)
+ } else {
+ None
+ }
+ }),
+ };
+
+ reason.into_tx_abort_msg(self.context().channel_id)
+ }
+
+ pub fn tx_add_input<L: Deref>(
+ &mut self, msg: &msgs::TxAddInput, logger: &L,
+ ) -> Result<InteractiveTxMessageSend, msgs::TxAbort>
+ where
+ L::Target: Logger,
+ {
+ match self.interactive_tx_constructor_mut() {
+ Some(interactive_tx_constructor) => interactive_tx_constructor.handle_tx_add_input(msg),
+ None => Err(AbortReason::InternalError(
+ "Received unexpected interactive transaction negotiation message",
+ )),
+ }
+ .map_err(|abort_reason| self.fail_interactive_tx_negotiation(abort_reason, logger))
+ }
+
+ pub fn tx_add_output<L: Deref>(
+ &mut self, msg: &msgs::TxAddOutput, logger: &L,
+ ) -> Result<InteractiveTxMessageSend, msgs::TxAbort>
+ where
+ L::Target: Logger,
+ {
+ match self.interactive_tx_constructor_mut() {
+ Some(interactive_tx_constructor) => {
+ interactive_tx_constructor.handle_tx_add_output(msg)
+ },
+ None => Err(AbortReason::InternalError(
+ "Received unexpected interactive transaction negotiation message",
+ )),
+ }
+ .map_err(|abort_reason| self.fail_interactive_tx_negotiation(abort_reason, logger))
+ }
+
+ pub fn tx_remove_input<L: Deref>(
+ &mut self, msg: &msgs::TxRemoveInput, logger: &L,
+ ) -> Result<InteractiveTxMessageSend, msgs::TxAbort>
+ where
+ L::Target: Logger,
+ {
+ match self.interactive_tx_constructor_mut() {
+ Some(interactive_tx_constructor) => {
+ interactive_tx_constructor.handle_tx_remove_input(msg)
+ },
+ None => Err(AbortReason::InternalError(
+ "Received unexpected interactive transaction negotiation message",
+ )),
+ }
+ .map_err(|abort_reason| self.fail_interactive_tx_negotiation(abort_reason, logger))
+ }
+
+ pub fn tx_remove_output<L: Deref>(
+ &mut self, msg: &msgs::TxRemoveOutput, logger: &L,
+ ) -> Result<InteractiveTxMessageSend, msgs::TxAbort>
+ where
+ L::Target: Logger,
+ {
+ match self.interactive_tx_constructor_mut() {
+ Some(interactive_tx_constructor) => {
+ interactive_tx_constructor.handle_tx_remove_output(msg)
+ },
+ None => Err(AbortReason::InternalError(
+ "Received unexpected interactive transaction negotiation message",
+ )),
+ }
+ .map_err(|abort_reason| self.fail_interactive_tx_negotiation(abort_reason, logger))
+ }
+
+ pub fn tx_complete<L: Deref>(
+ &mut self, msg: &msgs::TxComplete, logger: &L,
+ ) -> Result<(Option<InteractiveTxMessageSend>, Option<msgs::CommitmentSigned>), msgs::TxAbort>
+ where
+ L::Target: Logger,
+ {
+ let tx_complete_action = match self.interactive_tx_constructor_mut() {
+ Some(interactive_tx_constructor) => interactive_tx_constructor.handle_tx_complete(msg),
+ None => Err(AbortReason::InternalError(
+ "Received unexpected interactive transaction negotiation message",
+ )),
+ }
+ .map_err(|abort_reason| self.fail_interactive_tx_negotiation(abort_reason, logger))?;
+
+ let (interactive_tx_msg_send, negotiation_complete) = match tx_complete_action {
+ HandleTxCompleteValue::SendTxMessage(interactive_tx_msg_send) => {
+ (Some(interactive_tx_msg_send), false)
+ },
+ HandleTxCompleteValue::SendTxComplete(
+ interactive_tx_msg_send,
+ negotiation_complete,
+ ) => (Some(interactive_tx_msg_send), negotiation_complete),
+ HandleTxCompleteValue::NegotiationComplete => (None, true),
+ };
+ if !negotiation_complete {
+ return Ok((interactive_tx_msg_send, None));
+ }
+
+ let commitment_signed = self
+ .funding_tx_constructed(logger)
+ .map_err(|abort_reason| self.fail_interactive_tx_negotiation(abort_reason, logger))?;
+ Ok((interactive_tx_msg_send, Some(commitment_signed)))
+ }
+
+ pub fn tx_abort<L: Deref>(
+ &mut self, msg: &msgs::TxAbort, logger: &L,
+ ) -> Result<Option<msgs::TxAbort>, ChannelError>
+ where
+ L::Target: Logger,
+ {
+ // This checks for and resets the interactive negotiation state by `take()`ing it from the channel.
+ // The existence of the `tx_constructor` indicates that we have not moved into the signing
+ // phase for this interactively constructed transaction and hence we have not exchanged
+ // `tx_signatures`. Either way, we never close the channel upon receiving a `tx_abort`:
+ // https://github.com/lightning/bolts/blob/247e83d/02-peer-protocol.md?plain=1#L574-L576
+ let should_ack = match &mut self.phase {
+ ChannelPhase::Undefined => unreachable!(),
+ ChannelPhase::UnfundedOutboundV1(_) | ChannelPhase::UnfundedInboundV1(_) => {
+ let err = "Got an unexpected tx_abort message: This is an unfunded channel created with V1 channel establishment";
+ return Err(ChannelError::Warn(err.into()));
+ },
+ ChannelPhase::UnfundedV2(pending_v2_channel) => {
+ pending_v2_channel.interactive_tx_constructor.take().is_some()
+ },
+ #[cfg(not(splicing))]
+ ChannelPhase::Funded(_) => {
+ let err = "Got an unexpected tx_abort message: This is an funded channel and splicing is not supported";
+ return Err(ChannelError::Warn(err.into()));
+ },
+ #[cfg(splicing)]
+ ChannelPhase::Funded(funded_channel) => funded_channel
+ .pending_splice
+ .as_mut()
+ .and_then(|pending_splice| pending_splice.funding_negotiation.take())
+ .is_some(),
+ };
+
+ // NOTE: Since at this point we have not sent a `tx_abort` message for this negotiation
+ // previously (tx_constructor was `Some`), we need to echo back a tx_abort message according
+ // to the spec:
+ // https://github.com/lightning/bolts/blob/247e83d/02-peer-protocol.md?plain=1#L560-L561
+ // For rationale why we echo back `tx_abort`:
+ // https://github.com/lightning/bolts/blob/247e83d/02-peer-protocol.md?plain=1#L578-L580
+ Ok(should_ack.then(|| {
+ let logger = WithChannelContext::from(logger, &self.context(), None);
+ let reason =
+ types::string::UntrustedString(String::from_utf8_lossy(&msg.data).to_string());
+ log_info!(logger, "Counterparty failed interactive transaction negotiation: {reason}");
+ msgs::TxAbort {
+ channel_id: msg.channel_id,
+ data: "Acknowledged tx_abort".to_string().into_bytes(),
+ }
+ }))
+ }
+
#[rustfmt::skip]
pub fn funding_signed<L: Deref>(
&mut self, msg: &msgs::FundingSigned, best_block: BestBlock, signer_provider: &SP, logger: &L
@@ -1780,9 +1960,9 @@ where
result.map(|monitor| (self.as_funded_mut().expect("Channel should be funded"), monitor))
}
- pub fn funding_tx_constructed<L: Deref>(
+ fn funding_tx_constructed<L: Deref>(
&mut self, logger: &L,
- ) -> Result<msgs::CommitmentSigned, msgs::TxAbort>
+ ) -> Result<msgs::CommitmentSigned, AbortReason>
where
L::Target: Logger,
{
@@ -1837,17 +2017,15 @@ where
}
}
- return Err(msgs::TxAbort {
- channel_id: chan.context.channel_id(),
- data: "Got a tx_complete message in an invalid state".to_owned().into_bytes(),
- });
+ return Err(AbortReason::InternalError(
+ "Got a tx_complete message in an invalid state",
+ ));
},
_ => {
debug_assert!(false);
- return Err(msgs::TxAbort {
- channel_id: self.context().channel_id(),
- data: "Got a tx_complete message in an invalid phase".to_owned().into_bytes(),
- });
+ return Err(AbortReason::InternalError(
+ "Got a tx_complete message in an invalid phase",
+ ));
},
}
}
@@ -5855,7 +6033,7 @@ where
fn funding_tx_constructed<L: Deref>(
&mut self, funding: &mut FundingScope, signing_session: &mut InteractiveTxSigningSession,
is_splice: bool, holder_commitment_transaction_number: u64, logger: &L
- ) -> Result<msgs::CommitmentSigned, msgs::TxAbort>
+ ) -> Result<msgs::CommitmentSigned, AbortReason>
where
L::Target: Logger
{
@@ -5864,10 +6042,7 @@ where
for (idx, outp) in signing_session.unsigned_tx().outputs().enumerate() {
if outp.script_pubkey() == &expected_spk && outp.value() == funding.get_value_satoshis() {
if output_index.is_some() {
- return Err(msgs::TxAbort {
- channel_id: self.channel_id(),
- data: "Multiple outputs matched the expected script and value".to_owned().into_bytes(),
- });
+ return Err(AbortReason::DuplicateFundingOutput);
}
output_index = Some(idx as u16);
}
@@ -5875,10 +6050,7 @@ where
let outpoint = if let Some(output_index) = output_index {
OutPoint { txid: signing_session.unsigned_tx().compute_txid(), index: output_index }
} else {
- return Err(msgs::TxAbort {
- channel_id: self.channel_id(),
- data: "No output matched the funding script_pubkey".to_owned().into_bytes(),
- });
+ return Err(AbortReason::MissingFundingOutput);
};
funding
.channel_transaction_parameters.funding_outpoint = Some(outpoint);
@@ -5892,10 +6064,7 @@ where
self.counterparty_next_commitment_transaction_number,
);
// TODO(splicing) Forced error, as the use case is not complete
- return Err(msgs::TxAbort {
- channel_id: self.channel_id(),
- data: "Splicing not yet supported".to_owned().into_bytes(),
- });
+ return Err(AbortReason::InternalError("Splicing not yet supported"));
} else {
self.assert_no_commitment_advancement(holder_commitment_transaction_number, "initial commitment_signed");
}
@@ -5906,10 +6075,7 @@ where
// TODO(splicing): Support async signing
None => {
funding.channel_transaction_parameters.funding_outpoint = None;
- return Err(msgs::TxAbort {
- channel_id: self.channel_id(),
- data: "Failed to get signature for commitment_signed".to_owned().into_bytes(),
- });
+ return Err(AbortReason::InternalError("Failed to compute commitment_signed signatures"));
},
};
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index dfc10e8..90e6a30 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -67,7 +67,7 @@ use crate::ln::channel_state::ChannelDetails;
#[cfg(splicing)]
use crate::ln::funding::SpliceContribution;
use crate::ln::inbound_payment;
-use crate::ln::interactivetxs::{HandleTxCompleteResult, InteractiveTxMessageSendResult};
+use crate::ln::interactivetxs::InteractiveTxMessageSend;
use crate::ln::msgs;
use crate::ln::msgs::{
BaseMessageHandler, ChannelMessageHandler, CommitmentUpdate, DecodeError, LightningError,
@@ -9869,7 +9869,9 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
}
}
- fn internal_tx_msg<HandleTxMsgFn: Fn(&mut Channel<SP>) -> Option<MessageSendEvent>>(
+ fn internal_tx_msg<
+ HandleTxMsgFn: Fn(&mut Channel<SP>) -> Result<InteractiveTxMessageSend, msgs::TxAbort>,
+ >(
&self, counterparty_node_id: &PublicKey, channel_id: ChannelId,
tx_msg_handler: HandleTxMsgFn,
) -> Result<(), MsgHandleErrInternal> {
@@ -9887,10 +9889,12 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
hash_map::Entry::Occupied(mut chan_entry) => {
let channel = chan_entry.get_mut();
let msg_send_event = match tx_msg_handler(channel) {
- Some(msg_send_event) => msg_send_event,
- None => {
- let err = ChannelError::Warn("Received unexpected interactive transaction negotiation message".to_owned());
- return Err(MsgHandleErrInternal::from_chan_no_close(err, channel_id))
+ Ok(msg_send) => msg_send.into_msg_send_event(*counterparty_node_id),
+ Err(tx_abort) => {
+ MessageSendEvent::SendTxAbort {
+ node_id: *counterparty_node_id,
+ msg: tx_abort,
+ }
},
};
peer_state.pending_msg_events.push(msg_send_event);
@@ -9909,15 +9913,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
&self, counterparty_node_id: PublicKey, msg: &msgs::TxAddInput,
) -> Result<(), MsgHandleErrInternal> {
self.internal_tx_msg(&counterparty_node_id, msg.channel_id, |channel: &mut Channel<SP>| {
- Some(
- InteractiveTxMessageSendResult(
- channel
- .interactive_tx_constructor_mut()?
- .handle_tx_add_input(msg)
- .map_err(|reason| reason.into_tx_abort_msg(msg.channel_id)),
- )
- .into_msg_send_event(counterparty_node_id),
- )
+ channel.tx_add_input(msg, &self.logger)
})
}
@@ -9925,15 +9921,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
&self, counterparty_node_id: PublicKey, msg: &msgs::TxAddOutput,
) -> Result<(), MsgHandleErrInternal> {
self.internal_tx_msg(&counterparty_node_id, msg.channel_id, |channel: &mut Channel<SP>| {
- Some(
- InteractiveTxMessageSendResult(
- channel
- .interactive_tx_constructor_mut()?
- .handle_tx_add_output(msg)
- .map_err(|reason| reason.into_tx_abort_msg(msg.channel_id)),
- )
- .into_msg_send_event(counterparty_node_id),
- )
+ channel.tx_add_output(msg, &self.logger)
})
}
@@ -9941,15 +9929,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
&self, counterparty_node_id: PublicKey, msg: &msgs::TxRemoveInput,
) -> Result<(), MsgHandleErrInternal> {
self.internal_tx_msg(&counterparty_node_id, msg.channel_id, |channel: &mut Channel<SP>| {
- Some(
- InteractiveTxMessageSendResult(
- channel
- .interactive_tx_constructor_mut()?
- .handle_tx_remove_input(msg)
- .map_err(|reason| reason.into_tx_abort_msg(msg.channel_id)),
- )
- .into_msg_send_event(counterparty_node_id),
- )
+ channel.tx_remove_input(msg, &self.logger)
})
}
@@ -9957,15 +9937,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
&self, counterparty_node_id: PublicKey, msg: &msgs::TxRemoveOutput,
) -> Result<(), MsgHandleErrInternal> {
self.internal_tx_msg(&counterparty_node_id, msg.channel_id, |channel: &mut Channel<SP>| {
- Some(
- InteractiveTxMessageSendResult(
- channel
- .interactive_tx_constructor_mut()?
- .handle_tx_remove_output(msg)
- .map_err(|reason| reason.into_tx_abort_msg(msg.channel_id)),
- )
- .into_msg_send_event(counterparty_node_id),
- )
+ channel.tx_remove_output(msg, &self.logger)
})
}
@@ -9983,57 +9955,34 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
let peer_state = &mut *peer_state_lock;
match peer_state.channel_by_id.entry(msg.channel_id) {
hash_map::Entry::Occupied(mut chan_entry) => {
- let (msg_send_event_opt, negotiation_complete) = match chan_entry.get_mut().interactive_tx_constructor_mut() {
- Some(interactive_tx_constructor) => {
- HandleTxCompleteResult(
- interactive_tx_constructor
- .handle_tx_complete(msg)
- .map_err(|reason| reason.into_tx_abort_msg(msg.channel_id)),
- )
- .into_msg_send_event(counterparty_node_id)
+ let chan = chan_entry.get_mut();
+ match chan.tx_complete(msg, &self.logger) {
+ Ok((interactive_tx_msg_send, commitment_signed)) => {
+ if let Some(interactive_tx_msg_send) = interactive_tx_msg_send {
+ let msg_send_event = interactive_tx_msg_send.into_msg_send_event(counterparty_node_id);
+ peer_state.pending_msg_events.push(msg_send_event);
+ };
+ if let Some(commitment_signed) = commitment_signed {
+ peer_state.pending_msg_events.push(MessageSendEvent::UpdateHTLCs {
+ node_id: counterparty_node_id,
+ channel_id: msg.channel_id,
+ updates: CommitmentUpdate {
+ commitment_signed: vec![commitment_signed],
+ update_add_htlcs: vec![],
+ update_fulfill_htlcs: vec![],
+ update_fail_htlcs: vec![],
+ update_fail_malformed_htlcs: vec![],
+ update_fee: None,
+ },
+ });
+ }
},
- None => {
- let err = ChannelError::Warn("Received unexpected tx_complete message".to_owned());
- return Err(MsgHandleErrInternal::from_chan_no_close(err, msg.channel_id))
+ Err(tx_abort) => {
+ peer_state.pending_msg_events.push(MessageSendEvent::SendTxAbort {
+ node_id: counterparty_node_id,
+ msg: tx_abort,
+ });
},
- };
- if let Some(msg_send_event) = msg_send_event_opt {
- peer_state.pending_msg_events.push(msg_send_event);
- };
- if negotiation_complete {
- let commitment_signed = match chan_entry
- .get_mut()
- .funding_tx_constructed(&self.logger)
- {
- Ok(commitment_signed) => commitment_signed,
- Err(tx_abort) => {
- if chan_entry.get().is_funded() {
- peer_state.pending_msg_events.push(MessageSendEvent::SendTxAbort {
- node_id: counterparty_node_id,
- msg: tx_abort,
- });
- return Ok(());
- } else {
- let msg = String::from_utf8(tx_abort.data)
- .expect("tx_abort data should contain valid UTF-8");
- let reason = ClosureReason::ProcessingError { err: msg.clone() };
- let err = ChannelError::Close((msg, reason));
- try_channel_entry!(self, peer_state, Err(err), chan_entry)
- }
- },
- };
- peer_state.pending_msg_events.push(MessageSendEvent::UpdateHTLCs {
- node_id: counterparty_node_id,
- channel_id: msg.channel_id,
- updates: CommitmentUpdate {
- commitment_signed: vec![commitment_signed],
- update_add_htlcs: vec![],
- update_fulfill_htlcs: vec![],
- update_fail_htlcs: vec![],
- update_fail_malformed_htlcs: vec![],
- update_fee: None,
- },
- });
}
Ok(())
},
@@ -10105,41 +10054,8 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
let peer_state = &mut *peer_state_lock;
match peer_state.channel_by_id.entry(msg.channel_id) {
hash_map::Entry::Occupied(mut chan_entry) => {
- let tx_constructor = match chan_entry.get_mut().as_unfunded_v2_mut() {
- Some(chan) => &mut chan.interactive_tx_constructor,
- None => if chan_entry.get().is_funded() {
- // TODO(splicing)/TODO(RBF): We'll also be doing interactive tx construction
- // for a "Channel::Funded" when we want to bump the fee on an interactively
- // constructed funding tx or during splicing. For now we send an error as we would
- // never ack an RBF attempt or a splice for now:
- try_channel_entry!(self, peer_state, Err(ChannelError::Warn(
- "Got an unexpected tx_abort message: After initial funding transaction is signed, \
- splicing and RBF attempts of interactive funding transactions are not supported yet so \
- we don't have any negotiation in progress".into(),
- )), chan_entry)
- } else {
- try_channel_entry!(self, peer_state, Err(ChannelError::Warn(
- "Got an unexpected tx_abort message: This is an unfunded channel created with V1 channel \
- establishment".into(),
- )), chan_entry)
- },
- };
- // This checks for and resets the interactive negotiation state by `take()`ing it from the channel.
- // The existence of the `tx_constructor` indicates that we have not moved into the signing
- // phase for this interactively constructed transaction and hence we have not exchanged
- // `tx_signatures`. Either way, we never close the channel upon receiving a `tx_abort`:
- // https://github.com/lightning/bolts/blob/247e83d/02-peer-protocol.md?plain=1#L574-L576
- if tx_constructor.take().is_some() {
- let msg = msgs::TxAbort {
- channel_id: msg.channel_id,
- data: "Acknowledged tx_abort".to_string().into_bytes(),
- };
- // NOTE: Since at this point we have not sent a `tx_abort` message for this negotiation
- // previously (tx_constructor was `Some`), we need to echo back a tx_abort message according
- // to the spec:
- // https://github.com/lightning/bolts/blob/247e83d/02-peer-protocol.md?plain=1#L560-L561
- // For rationale why we echo back `tx_abort`:
- // https://github.com/lightning/bolts/blob/247e83d/02-peer-protocol.md?plain=1#L578-L580
+ let res = chan_entry.get_mut().tx_abort(msg, &self.logger);
+ if let Some(msg) = try_channel_entry!(self, peer_state, res, chan_entry) {
peer_state.pending_msg_events.push(MessageSendEvent::SendTxAbort {
node_id: *counterparty_node_id,
msg,
diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs
index ca46801..5212773 100644
--- a/lightning/src/ln/interactivetxs.rs
+++ b/lightning/src/ln/interactivetxs.rs
@@ -89,7 +89,7 @@ impl SerialIdExt for SerialId {
}
}
-#[derive(Debug, Clone, PartialEq)]
+#[derive(Debug, Clone, Copy, PartialEq)]
pub(crate) enum AbortReason {
InvalidStateTransition,
UnexpectedCounterpartyMessage,
@@ -1852,23 +1852,6 @@ impl InteractiveTxMessageSend {
}
}
-pub(super) struct InteractiveTxMessageSendResult(
- pub Result<InteractiveTxMessageSend, msgs::TxAbort>,
-);
-
-impl InteractiveTxMessageSendResult {
- pub fn into_msg_send_event(self, counterparty_node_id: PublicKey) -> MessageSendEvent {
- match self.0 {
- Ok(interactive_tx_msg_send) => {
- interactive_tx_msg_send.into_msg_send_event(counterparty_node_id)
- },
- Err(tx_abort_msg) => {
- MessageSendEvent::SendTxAbort { node_id: counterparty_node_id, msg: tx_abort_msg }
- },
- }
- }
-}
-
// This macro executes a state machine transition based on a provided action.
macro_rules! do_state_transition {
($self: ident, $transition: ident, $msg: expr) => {{
@@ -1901,43 +1884,6 @@ pub(super) enum HandleTxCompleteValue {
NegotiationComplete,
}
-impl HandleTxCompleteValue {
- pub fn into_msg_send_event(
- self, counterparty_node_id: PublicKey,
- ) -> (Option<MessageSendEvent>, bool) {
- match self {
- HandleTxCompleteValue::SendTxMessage(msg) => {
- (Some(msg.into_msg_send_event(counterparty_node_id)), false)
- },
- HandleTxCompleteValue::SendTxComplete(msg, negotiation_complete) => {
- (Some(msg.into_msg_send_event(counterparty_node_id)), negotiation_complete)
- },
- HandleTxCompleteValue::NegotiationComplete => (None, true),
- }
- }
-}
-
-pub(super) struct HandleTxCompleteResult(pub Result<HandleTxCompleteValue, msgs::TxAbort>);
-
-impl HandleTxCompleteResult {
- pub fn into_msg_send_event(
- self, counterparty_node_id: PublicKey,
- ) -> (Option<MessageSendEvent>, bool) {
- match self.0 {
- Ok(interactive_tx_msg_send) => {
- interactive_tx_msg_send.into_msg_send_event(counterparty_node_id)
- },
- Err(tx_abort_msg) => (
- Some(MessageSendEvent::SendTxAbort {
- node_id: counterparty_node_id,
- msg: tx_abort_msg,
- }),
- false,
- ),
- }
- }
-}
-
pub(super) struct InteractiveTxConstructorArgs<'a, ES: Deref>
where
ES::Target: EntropySource,
diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs
index b60903d..bec2f4c 100644
--- a/lightning/src/ln/splicing_tests.rs
+++ b/lightning/src/ln/splicing_tests.rs
@@ -278,14 +278,10 @@ fn test_v1_splice_in() {
.node
.handle_tx_complete(acceptor_node.node.get_our_node_id(), &tx_complete_msg);
let events = initiator_node.node.get_and_clear_pending_msg_events();
- assert_eq!(events.len(), 2);
+ assert_eq!(events.len(), 1);
match events[0] {
- MessageSendEvent::SendTxComplete { .. } => {},
- _ => panic!("Unexpected event {:?}", events[0]),
- }
- match events[1] {
MessageSendEvent::SendTxAbort { .. } => {},
- _ => panic!("Unexpected event {:?}", events[1]),
+ _ => panic!("Unexpected event {:?}", events[0]),
}
// TODO(splicing): Continue with commitment flow, new tx confirmation, and shutdown
Risk score
Our methodology →Why this scored 42/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.