Skip to content

Commit bbe88b6

Browse files
jkczyzwpaulino
jkczyz
and
wpaulino
committed
Update next_funding_txid logic for channel_reestablish
The splicing spec updates the logic pertaining to next_funding_txid when handling a channel_reestablish message. Specifically: A receiving node: - if `next_funding_txid` is set: - if `next_funding_txid` matches the latest interactive funding transaction or the current channel funding transaction: - if `next_commitment_number` is equal to the commitment number of the `commitment_signed` message it sent for this funding transaction: - MUST retransmit its `commitment_signed` for that funding transaction. - if it has already received `commitment_signed` and it should sign first, as specified in the [`tx_signatures` requirements](#the-tx_signatures-message): - MUST send its `tx_signatures` for that funding transaction. - if it has already received `tx_signatures` for that funding transaction: - MUST send its `tx_signatures` for that funding transaction. - if it also sets `next_funding_txid` in its own `channel_reestablish`, but the values don't match: - MUST send an `error` and fail the channel. - otherwise: - MUST send `tx_abort` to let the sending node know that they can forget this funding transaction. This commit updates FundedChannel::channel_reestablish accordingly. Co-authored-by: Wilmer Paulino <[email protected]> Co-authored-by: Jeffrey Czyz <[email protected]>
1 parent cbd9bd2 commit bbe88b6

File tree

1 file changed

+97
-77
lines changed

1 file changed

+97
-77
lines changed

‎lightning/src/ln/channel.rs

Lines changed: 97 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -8445,89 +8445,104 @@ where
84458445
.and_then(|_| self.get_channel_ready(logger))
84468446
} else { None };
84478447

8448-
if msg.next_local_commitment_number == next_counterparty_commitment_number {
8449-
if required_revoke.is_some() || self.context.signer_pending_revoke_and_ack {
8450-
log_debug!(logger, "Reconnected channel {} with only lost outbound RAA", &self.context.channel_id());
8451-
} else {
8452-
log_debug!(logger, "Reconnected channel {} with no loss", &self.context.channel_id());
8453-
}
8448+
let mut commitment_update = None;
8449+
let mut tx_signatures = None;
8450+
let mut tx_abort = None;
8451+
8452+
// if next_funding_txid is set:
8453+
if let Some(next_funding_txid) = msg.next_funding_txid {
8454+
// - if `next_funding_txid` matches the latest interactive funding transaction
8455+
// or the current channel funding transaction:
8456+
if let Some(session) = &self.interactive_tx_signing_session {
8457+
let our_next_funding_txid = self.maybe_get_next_funding_txid();
8458+
if let Some(our_next_funding_txid) = our_next_funding_txid {
8459+
if our_next_funding_txid != next_funding_txid {
8460+
return Err(ChannelError::close(format!(
8461+
"Unexpected next_funding_txid: {}; expected: {}",
8462+
next_funding_txid, our_next_funding_txid,
8463+
)));
8464+
}
84548465

8455-
// if next_funding_txid is set:
8456-
let (commitment_update, tx_signatures, tx_abort) = if let Some(next_funding_txid) = msg.next_funding_txid {
8457-
if let Some(session) = &self.interactive_tx_signing_session {
8458-
// if next_funding_txid matches the latest interactive funding transaction:
8459-
let our_next_funding_txid = session.unsigned_tx().compute_txid();
8460-
if our_next_funding_txid == next_funding_txid {
8461-
debug_assert_eq!(session.unsigned_tx().compute_txid(), self.maybe_get_next_funding_txid().unwrap());
8462-
8463-
let commitment_update = if !self.context.channel_state.is_their_tx_signatures_sent() && msg.next_local_commitment_number == 0 {
8464-
// if it has not received tx_signatures for that funding transaction AND
8465-
// if next_commitment_number is zero:
8466-
// MUST retransmit its commitment_signed for that funding transaction.
8467-
let commitment_signed = self.context.get_initial_commitment_signed(&self.funding, logger)?;
8468-
Some(msgs::CommitmentUpdate {
8469-
commitment_signed: vec![commitment_signed],
8470-
update_add_htlcs: vec![],
8471-
update_fulfill_htlcs: vec![],
8472-
update_fail_htlcs: vec![],
8473-
update_fail_malformed_htlcs: vec![],
8474-
update_fee: None,
8475-
})
8476-
} else { None };
8466+
if !session.has_received_commitment_signed() {
8467+
self.context.expecting_peer_commitment_signed = true;
8468+
}
8469+
8470+
// - if `next_commitment_number` is equal to the commitment number of the
8471+
// `commitment_signed` message it sent for this funding transaction:
8472+
// - MUST retransmit its `commitment_signed` for that funding transaction.
8473+
if msg.next_local_commitment_number == next_counterparty_commitment_number {
8474+
// `next_counterparty_commitment_number` is guaranteed to always be the
8475+
// commitment number of the `commitment_signed` message we sent for this
8476+
// funding transaction. If they set `next_funding_txid`, then they should
8477+
// not have processed our `tx_signatures` yet, which implies that our state
8478+
// machine is still paused and no updates can happen that would increment
8479+
// our `next_counterparty_commitment_number`.
8480+
//
8481+
// If they did set `next_funding_txid` even after processing our
8482+
// `tx_signatures` erroneously, this may end up resulting in a force close.
8483+
//
84778484
// TODO(dual_funding): For async signing support we need to hold back `tx_signatures` until the `commitment_signed` is ready.
8478-
let tx_signatures = if (
8479-
// if it has not received tx_signatures for that funding transaction AND
8480-
// if it has already received commitment_signed AND it should sign first, as specified in the tx_signatures requirements:
8481-
// MUST send its tx_signatures for that funding transaction.
8482-
!self.context.channel_state.is_their_tx_signatures_sent() && session.has_received_commitment_signed() && session.holder_sends_tx_signatures_first()
8483-
// else if it has already received tx_signatures for that funding transaction:
8484-
// MUST send its tx_signatures for that funding transaction.
8485-
) || self.context.channel_state.is_their_tx_signatures_sent() {
8486-
if self.context.channel_state.is_monitor_update_in_progress() {
8487-
// The `monitor_pending_tx_signatures` field should have already been set in `commitment_signed_initial_v2`
8488-
// if we were up first for signing and had a monitor update in progress, but check again just in case.
8489-
debug_assert!(self.context.monitor_pending_tx_signatures.is_some(), "monitor_pending_tx_signatures should already be set");
8490-
log_debug!(logger, "Not sending tx_signatures: a monitor update is in progress. Setting monitor_pending_tx_signatures.");
8491-
if self.context.monitor_pending_tx_signatures.is_none() {
8492-
self.context.monitor_pending_tx_signatures = session.holder_tx_signatures().clone();
8493-
}
8494-
None
8495-
} else {
8496-
// If `holder_tx_signatures` is `None` here, the `tx_signatures` message will be sent
8497-
// when the holder provides their witnesses as this will queue a `tx_signatures` if the
8498-
// holder must send one.
8499-
session.holder_tx_signatures().clone()
8485+
let commitment_signed = self.context.get_initial_commitment_signed(&self.funding, logger)?;
8486+
commitment_update = Some(msgs::CommitmentUpdate {
8487+
commitment_signed: vec![commitment_signed],
8488+
update_add_htlcs: vec![],
8489+
update_fulfill_htlcs: vec![],
8490+
update_fail_htlcs: vec![],
8491+
update_fail_malformed_htlcs: vec![],
8492+
update_fee: None,
8493+
});
8494+
}
8495+
8496+
// - if it has already received `commitment_signed` and it should sign first,
8497+
// as specified in the [`tx_signatures` requirements](#the-tx_signatures-message):
8498+
// - MUST send its `tx_signatures` for that funding transaction.
8499+
//
8500+
// - if it has already received `tx_signatures` for that funding transaction:
8501+
// - MUST send its `tx_signatures` for that funding transaction.
8502+
if (session.has_received_commitment_signed() && session.holder_sends_tx_signatures_first())
8503+
|| self.context.channel_state.is_their_tx_signatures_sent()
8504+
{
8505+
if self.context.channel_state.is_monitor_update_in_progress() {
8506+
// The `monitor_pending_tx_signatures` field should have already been
8507+
// set in `commitment_signed_initial_v2` if we were up first for signing
8508+
// and had a monitor update in progress.
8509+
if session.holder_sends_tx_signatures_first() {
8510+
debug_assert!(self.context.monitor_pending_tx_signatures.is_some());
85008511
}
85018512
} else {
8502-
None
8503-
};
8504-
if !session.has_received_commitment_signed() {
8505-
self.context.expecting_peer_commitment_signed = true;
8513+
// If `holder_tx_signatures` is `None` here, the `tx_signatures` message
8514+
// will be sent when the user provides their witnesses.
8515+
tx_signatures = session.holder_tx_signatures().clone()
85068516
}
8507-
(commitment_update, tx_signatures, None)
8508-
} else {
8509-
// The `next_funding_txid` does not match the latest interactive funding transaction so we
8510-
// MUST send tx_abort to let the remote know that they can forget this funding transaction.
8511-
(None, None, Some(msgs::TxAbort {
8512-
channel_id: self.context.channel_id(),
8513-
data: format!(
8514-
"next_funding_txid {} does match our latest interactive funding txid {}",
8515-
next_funding_txid, our_next_funding_txid,
8516-
).into_bytes() }))
85178517
}
85188518
} else {
8519-
// We'll just send a `tx_abort` here if we don't have a signing session for this channel
8520-
// on reestablish and tell our peer to just forget about it.
8521-
// Our peer is doing something strange, but it doesn't warrant closing the channel.
8522-
(None, None, Some(msgs::TxAbort {
8519+
// The `next_funding_txid` does not match the latest interactive funding
8520+
// transaction so we MUST send tx_abort to let the remote know that they can
8521+
// forget this funding transaction.
8522+
tx_abort = Some(msgs::TxAbort {
85238523
channel_id: self.context.channel_id(),
8524-
data:
8525-
"No active signing session. The associated funding transaction may have already been broadcast.".as_bytes().to_vec() }))
8524+
data: format!(
8525+
"Unexpected next_funding_txid {}",
8526+
next_funding_txid,
8527+
).into_bytes() });
85268528
}
85278529
} else {
8528-
// Don't send anything related to interactive signing if `next_funding_txid` is not set.
8529-
(None, None, None)
8530-
};
8530+
// We'll just send a `tx_abort` here if we don't have a signing session for this channel
8531+
// on reestablish and tell our peer to just forget about it.
8532+
// Our peer is doing something strange, but it doesn't warrant closing the channel.
8533+
tx_abort = Some(msgs::TxAbort {
8534+
channel_id: self.context.channel_id(),
8535+
data:
8536+
"No active signing session. The associated funding transaction may have already been broadcast.".as_bytes().to_vec() });
8537+
}
8538+
}
8539+
8540+
if msg.next_local_commitment_number == next_counterparty_commitment_number {
8541+
if required_revoke.is_some() || self.context.signer_pending_revoke_and_ack {
8542+
log_debug!(logger, "Reconnected channel {} with only lost outbound RAA", &self.context.channel_id());
8543+
} else {
8544+
log_debug!(logger, "Reconnected channel {} with no loss", &self.context.channel_id());
8545+
}
85318546

85328547
Ok(ReestablishResponses {
85338548
channel_ready, shutdown_msg, announcement_sigs,
@@ -8538,6 +8553,11 @@ where
85388553
tx_abort,
85398554
})
85408555
} else if msg.next_local_commitment_number == next_counterparty_commitment_number - 1 {
8556+
// We've made an update so we must have exchanged `tx_signatures`, implying that
8557+
// `commitment_signed` was also exchanged. However, we may still need to retransmit our
8558+
// `tx_signatures` if the counterparty sent theirs first but didn't get to process ours.
8559+
debug_assert!(commitment_update.is_none());
8560+
85418561
if required_revoke.is_some() || self.context.signer_pending_revoke_and_ack {
85428562
log_debug!(logger, "Reconnected channel {} with lost outbound RAA and lost remote commitment tx", &self.context.channel_id());
85438563
} else {
@@ -8550,8 +8570,8 @@ where
85508570
channel_ready, shutdown_msg, announcement_sigs,
85518571
commitment_update: None, raa: None,
85528572
order: self.context.resend_order.clone(),
8553-
tx_signatures: None,
8554-
tx_abort: None,
8573+
tx_signatures,
8574+
tx_abort,
85558575
})
85568576
} else {
85578577
let commitment_update = if self.context.resend_order == RAACommitmentOrder::RevokeAndACKFirst
@@ -8574,8 +8594,8 @@ where
85748594
channel_ready, shutdown_msg, announcement_sigs,
85758595
raa, commitment_update,
85768596
order: self.context.resend_order.clone(),
8577-
tx_signatures: None,
8578-
tx_abort: None,
8597+
tx_signatures,
8598+
tx_abort,
85798599
})
85808600
}
85818601
} else if msg.next_local_commitment_number < next_counterparty_commitment_number {

0 commit comments

Comments
 (0)