Skip to content

Commit 4a0e343

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 cc4152f commit 4a0e343

File tree

1 file changed

+104
-84
lines changed

1 file changed

+104
-84
lines changed

‎lightning/src/ln/channel.rs

Lines changed: 104 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -8406,96 +8406,111 @@ where
84068406
.and_then(|_| self.get_channel_ready(logger))
84078407
} else { None };
84088408

8409-
if msg.next_local_commitment_number == next_counterparty_commitment_number {
8410-
if required_revoke.is_some() || self.context.signer_pending_revoke_and_ack {
8411-
log_debug!(logger, "Reconnected channel {} with only lost outbound RAA", &self.context.channel_id());
8412-
} else {
8413-
log_debug!(logger, "Reconnected channel {} with no loss", &self.context.channel_id());
8414-
}
8409+
let mut commitment_update = None;
8410+
let mut tx_signatures = None;
8411+
let mut tx_abort = None;
8412+
8413+
// if next_funding_txid is set:
8414+
if let Some(next_funding_txid) = msg.next_funding_txid {
8415+
// - if `next_funding_txid` matches the latest interactive funding transaction
8416+
// or the current channel funding transaction:
8417+
if let Some(session) = &self.interactive_tx_signing_session {
8418+
let our_next_funding_txid = self.maybe_get_next_funding_txid();
8419+
if let Some(our_next_funding_txid) = our_next_funding_txid {
8420+
if our_next_funding_txid != next_funding_txid {
8421+
return Err(ChannelError::close(format!(
8422+
"Unexpected next_funding_txid: {}; expected: {}",
8423+
next_funding_txid, our_next_funding_txid,
8424+
)));
8425+
}
84158426

8416-
// if next_funding_txid is set:
8417-
let (commitment_update, tx_signatures, tx_abort) = if let Some(next_funding_txid) = msg.next_funding_txid {
8418-
if let Some(session) = &self.interactive_tx_signing_session {
8419-
// if next_funding_txid matches the latest interactive funding transaction:
8420-
let our_next_funding_txid = session.unsigned_tx().compute_txid();
8421-
if our_next_funding_txid == next_funding_txid {
8422-
debug_assert_eq!(session.unsigned_tx().compute_txid(), self.maybe_get_next_funding_txid().unwrap());
8423-
8424-
let commitment_update = if !self.context.channel_state.is_their_tx_signatures_sent() && msg.next_local_commitment_number == 0 {
8425-
// if it has not received tx_signatures for that funding transaction AND
8426-
// if next_commitment_number is zero:
8427-
// MUST retransmit its commitment_signed for that funding transaction.
8428-
let commitment_signed = self.context.get_initial_commitment_signed(&self.funding, logger)
8429-
// TODO(splicing): Support async signing
8430-
.ok_or_else(|| ChannelError::Close(
8431-
(
8432-
"Failed to get signatures for new commitment_signed".to_owned(),
8433-
ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false) },
8434-
)
8435-
))?;
8436-
Some(msgs::CommitmentUpdate {
8437-
commitment_signed: vec![commitment_signed],
8438-
update_add_htlcs: vec![],
8439-
update_fulfill_htlcs: vec![],
8440-
update_fail_htlcs: vec![],
8441-
update_fail_malformed_htlcs: vec![],
8442-
update_fee: None,
8443-
})
8444-
} else { None };
8427+
if !session.has_received_commitment_signed() {
8428+
self.context.expecting_peer_commitment_signed = true;
8429+
}
8430+
8431+
// - if `next_commitment_number` is equal to the commitment number of the
8432+
// `commitment_signed` message it sent for this funding transaction:
8433+
// - MUST retransmit its `commitment_signed` for that funding transaction.
8434+
if msg.next_local_commitment_number == next_counterparty_commitment_number {
8435+
// `next_counterparty_commitment_number` is guaranteed to always be the
8436+
// commitment number of the `commitment_signed` message we sent for this
8437+
// funding transaction. If they set `next_funding_txid`, then they should
8438+
// not have processed our `tx_signatures` yet, which implies that our state
8439+
// machine is still paused and no updates can happen that would increment
8440+
// our `next_counterparty_commitment_number`.
8441+
//
8442+
// If they did set `next_funding_txid` even after processing our
8443+
// `tx_signatures` erroneously, this may end up resulting in a force close.
8444+
//
84458445
// TODO(dual_funding): For async signing support we need to hold back `tx_signatures` until the `commitment_signed` is ready.
8446-
let tx_signatures = if (
8447-
// if it has not received tx_signatures for that funding transaction AND
8448-
// if it has already received commitment_signed AND it should sign first, as specified in the tx_signatures requirements:
8449-
// MUST send its tx_signatures for that funding transaction.
8450-
!self.context.channel_state.is_their_tx_signatures_sent() && session.has_received_commitment_signed() && session.holder_sends_tx_signatures_first()
8451-
// else if it has already received tx_signatures for that funding transaction:
8452-
// MUST send its tx_signatures for that funding transaction.
8453-
) || self.context.channel_state.is_their_tx_signatures_sent() {
8454-
if self.context.channel_state.is_monitor_update_in_progress() {
8455-
// The `monitor_pending_tx_signatures` field should have already been set in `commitment_signed_initial_v2`
8456-
// if we were up first for signing and had a monitor update in progress, but check again just in case.
8457-
debug_assert!(self.context.monitor_pending_tx_signatures.is_some(), "monitor_pending_tx_signatures should already be set");
8458-
log_debug!(logger, "Not sending tx_signatures: a monitor update is in progress. Setting monitor_pending_tx_signatures.");
8459-
if self.context.monitor_pending_tx_signatures.is_none() {
8460-
self.context.monitor_pending_tx_signatures = session.holder_tx_signatures().clone();
8461-
}
8462-
None
8463-
} else {
8464-
// If `holder_tx_signatures` is `None` here, the `tx_signatures` message will be sent
8465-
// when the holder provides their witnesses as this will queue a `tx_signatures` if the
8466-
// holder must send one.
8467-
session.holder_tx_signatures().clone()
8446+
let commitment_signed = self.context.get_initial_commitment_signed(&self.funding, logger)
8447+
// TODO(splicing): Support async signing
8448+
.ok_or_else(|| ChannelError::Close(
8449+
(
8450+
"Failed to get signatures for new commitment_signed".to_owned(),
8451+
ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false) },
8452+
)
8453+
))?;
8454+
commitment_update = Some(msgs::CommitmentUpdate {
8455+
commitment_signed: vec![commitment_signed],
8456+
update_add_htlcs: vec![],
8457+
update_fulfill_htlcs: vec![],
8458+
update_fail_htlcs: vec![],
8459+
update_fail_malformed_htlcs: vec![],
8460+
update_fee: None,
8461+
});
8462+
}
8463+
8464+
// - if it has already received `commitment_signed` and it should sign first,
8465+
// as specified in the [`tx_signatures` requirements](#the-tx_signatures-message):
8466+
// - MUST send its `tx_signatures` for that funding transaction.
8467+
//
8468+
// - if it has already received `tx_signatures` for that funding transaction:
8469+
// - MUST send its `tx_signatures` for that funding transaction.
8470+
if (session.has_received_commitment_signed() && session.holder_sends_tx_signatures_first())
8471+
|| self.context.channel_state.is_their_tx_signatures_sent()
8472+
{
8473+
if self.context.channel_state.is_monitor_update_in_progress() {
8474+
// The `monitor_pending_tx_signatures` field should have already been
8475+
// set in `commitment_signed_initial_v2` if we were up first for signing
8476+
// and had a monitor update in progress.
8477+
if session.holder_sends_tx_signatures_first() {
8478+
debug_assert!(self.context.monitor_pending_tx_signatures.is_some());
84688479
}
84698480
} else {
8470-
None
8471-
};
8472-
if !session.has_received_commitment_signed() {
8473-
self.context.expecting_peer_commitment_signed = true;
8481+
// If `holder_tx_signatures` is `None` here, the `tx_signatures` message
8482+
// will be sent when the user provides their witnesses.
8483+
tx_signatures = session.holder_tx_signatures().clone()
84748484
}
8475-
(commitment_update, tx_signatures, None)
8476-
} else {
8477-
// The `next_funding_txid` does not match the latest interactive funding transaction so we
8478-
// MUST send tx_abort to let the remote know that they can forget this funding transaction.
8479-
(None, None, Some(msgs::TxAbort {
8480-
channel_id: self.context.channel_id(),
8481-
data: format!(
8482-
"next_funding_txid {} does match our latest interactive funding txid {}",
8483-
next_funding_txid, our_next_funding_txid,
8484-
).into_bytes() }))
84858485
}
84868486
} else {
8487-
// We'll just send a `tx_abort` here if we don't have a signing session for this channel
8488-
// on reestablish and tell our peer to just forget about it.
8489-
// Our peer is doing something strange, but it doesn't warrant closing the channel.
8490-
(None, None, Some(msgs::TxAbort {
8487+
// The `next_funding_txid` does not match the latest interactive funding
8488+
// transaction so we MUST send tx_abort to let the remote know that they can
8489+
// forget this funding transaction.
8490+
tx_abort = Some(msgs::TxAbort {
84918491
channel_id: self.context.channel_id(),
8492-
data:
8493-
"No active signing session. The associated funding transaction may have already been broadcast.".as_bytes().to_vec() }))
8492+
data: format!(
8493+
"Unexpected next_funding_txid {}",
8494+
next_funding_txid,
8495+
).into_bytes() });
84948496
}
84958497
} else {
8496-
// Don't send anything related to interactive signing if `next_funding_txid` is not set.
8497-
(None, None, None)
8498-
};
8498+
// We'll just send a `tx_abort` here if we don't have a signing session for this channel
8499+
// on reestablish and tell our peer to just forget about it.
8500+
// Our peer is doing something strange, but it doesn't warrant closing the channel.
8501+
tx_abort = Some(msgs::TxAbort {
8502+
channel_id: self.context.channel_id(),
8503+
data:
8504+
"No active signing session. The associated funding transaction may have already been broadcast.".as_bytes().to_vec() });
8505+
}
8506+
}
8507+
8508+
if msg.next_local_commitment_number == next_counterparty_commitment_number {
8509+
if required_revoke.is_some() || self.context.signer_pending_revoke_and_ack {
8510+
log_debug!(logger, "Reconnected channel {} with only lost outbound RAA", &self.context.channel_id());
8511+
} else {
8512+
log_debug!(logger, "Reconnected channel {} with no loss", &self.context.channel_id());
8513+
}
84998514

85008515
Ok(ReestablishResponses {
85018516
channel_ready, shutdown_msg, announcement_sigs,
@@ -8506,6 +8521,11 @@ where
85068521
tx_abort,
85078522
})
85088523
} else if msg.next_local_commitment_number == next_counterparty_commitment_number - 1 {
8524+
// We've made an update so we must have exchanged `tx_signatures`, implying that
8525+
// `commitment_signed` was also exchanged. However, we may still need to retransmit our
8526+
// `tx_signatures` if the counterparty sent theirs first but didn't get to process ours.
8527+
debug_assert!(commitment_update.is_none());
8528+
85098529
if required_revoke.is_some() || self.context.signer_pending_revoke_and_ack {
85108530
log_debug!(logger, "Reconnected channel {} with lost outbound RAA and lost remote commitment tx", &self.context.channel_id());
85118531
} else {
@@ -8518,8 +8538,8 @@ where
85188538
channel_ready, shutdown_msg, announcement_sigs,
85198539
commitment_update: None, raa: None,
85208540
order: self.context.resend_order.clone(),
8521-
tx_signatures: None,
8522-
tx_abort: None,
8541+
tx_signatures,
8542+
tx_abort,
85238543
})
85248544
} else {
85258545
let commitment_update = if self.context.resend_order == RAACommitmentOrder::RevokeAndACKFirst
@@ -8542,8 +8562,8 @@ where
85428562
channel_ready, shutdown_msg, announcement_sigs,
85438563
raa, commitment_update,
85448564
order: self.context.resend_order.clone(),
8545-
tx_signatures: None,
8546-
tx_abort: None,
8565+
tx_signatures,
8566+
tx_abort,
85478567
})
85488568
}
85498569
} else if msg.next_local_commitment_number < next_counterparty_commitment_number {

0 commit comments

Comments
 (0)