Skip to content

Commit a9e5607

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 725be21 commit a9e5607

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
@@ -8417,89 +8417,104 @@ where
84178417
.and_then(|_| self.get_channel_ready(logger))
84188418
} else { None };
84198419

8420-
if msg.next_local_commitment_number == next_counterparty_commitment_number {
8421-
if required_revoke.is_some() || self.context.signer_pending_revoke_and_ack {
8422-
log_debug!(logger, "Reconnected channel {} with only lost outbound RAA", &self.context.channel_id());
8423-
} else {
8424-
log_debug!(logger, "Reconnected channel {} with no loss", &self.context.channel_id());
8425-
}
8420+
let mut commitment_update = None;
8421+
let mut tx_signatures = None;
8422+
let mut tx_abort = None;
8423+
8424+
// if next_funding_txid is set:
8425+
if let Some(next_funding_txid) = msg.next_funding_txid {
8426+
// - if `next_funding_txid` matches the latest interactive funding transaction
8427+
// or the current channel funding transaction:
8428+
if let Some(session) = &self.interactive_tx_signing_session {
8429+
let our_next_funding_txid = self.maybe_get_next_funding_txid();
8430+
if let Some(our_next_funding_txid) = our_next_funding_txid {
8431+
if our_next_funding_txid != next_funding_txid {
8432+
return Err(ChannelError::close(format!(
8433+
"Unexpected next_funding_txid: {}; expected: {}",
8434+
next_funding_txid, our_next_funding_txid,
8435+
)));
8436+
}
84268437

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

85048519
Ok(ReestablishResponses {
85058520
channel_ready, shutdown_msg, announcement_sigs,
@@ -8510,6 +8525,11 @@ where
85108525
tx_abort,
85118526
})
85128527
} else if msg.next_local_commitment_number == next_counterparty_commitment_number - 1 {
8528+
// We've made an update so we must have exchanged `tx_signatures`, implying that
8529+
// `commitment_signed` was also exchanged. However, we may still need to retransmit our
8530+
// `tx_signatures` if the counterparty sent theirs first but didn't get to process ours.
8531+
debug_assert!(commitment_update.is_none());
8532+
85138533
if required_revoke.is_some() || self.context.signer_pending_revoke_and_ack {
85148534
log_debug!(logger, "Reconnected channel {} with lost outbound RAA and lost remote commitment tx", &self.context.channel_id());
85158535
} else {
@@ -8522,8 +8542,8 @@ where
85228542
channel_ready, shutdown_msg, announcement_sigs,
85238543
commitment_update: None, raa: None,
85248544
order: self.context.resend_order.clone(),
8525-
tx_signatures: None,
8526-
tx_abort: None,
8545+
tx_signatures,
8546+
tx_abort,
85278547
})
85288548
} else {
85298549
let commitment_update = if self.context.resend_order == RAACommitmentOrder::RevokeAndACKFirst
@@ -8546,8 +8566,8 @@ where
85468566
channel_ready, shutdown_msg, announcement_sigs,
85478567
raa, commitment_update,
85488568
order: self.context.resend_order.clone(),
8549-
tx_signatures: None,
8550-
tx_abort: None,
8569+
tx_signatures,
8570+
tx_abort,
85518571
})
85528572
}
85538573
} else if msg.next_local_commitment_number < next_counterparty_commitment_number {

0 commit comments

Comments
 (0)