Skip to content

Commit 5e3d8c1

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 4169de1 commit 5e3d8c1

File tree

1 file changed

+103
-84
lines changed

1 file changed

+103
-84
lines changed

‎lightning/src/ln/channel.rs

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

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

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

84998513
Ok(ReestablishResponses {
85008514
channel_ready, shutdown_msg, announcement_sigs,
@@ -8505,6 +8519,11 @@ where
85058519
tx_abort,
85068520
})
85078521
} else if msg.next_local_commitment_number == next_counterparty_commitment_number - 1 {
8522+
// We've made an update so we must have exchanged `tx_signatures`, implying that
8523+
// `commitment_signed` was also exchanged. However, we may still need to retransmit our
8524+
// `tx_signatures` if the counterparty sent theirs first but didn't get to process ours.
8525+
debug_assert!(commitment_update.is_none());
8526+
85088527
if required_revoke.is_some() || self.context.signer_pending_revoke_and_ack {
85098528
log_debug!(logger, "Reconnected channel {} with lost outbound RAA and lost remote commitment tx", &self.context.channel_id());
85108529
} else {
@@ -8517,8 +8536,8 @@ where
85178536
channel_ready, shutdown_msg, announcement_sigs,
85188537
commitment_update: None, raa: None,
85198538
order: self.context.resend_order.clone(),
8520-
tx_signatures: None,
8521-
tx_abort: None,
8539+
tx_signatures,
8540+
tx_abort,
85228541
})
85238542
} else {
85248543
let commitment_update = if self.context.resend_order == RAACommitmentOrder::RevokeAndACKFirst
@@ -8541,8 +8560,8 @@ where
85418560
channel_ready, shutdown_msg, announcement_sigs,
85428561
raa, commitment_update,
85438562
order: self.context.resend_order.clone(),
8544-
tx_signatures: None,
8545-
tx_abort: None,
8563+
tx_signatures,
8564+
tx_abort,
85468565
})
85478566
}
85488567
} else if msg.next_local_commitment_number < next_counterparty_commitment_number {

0 commit comments

Comments
 (0)