Skip to content

Commit 28a6ff1

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 ba6bd18 commit 28a6ff1

File tree

1 file changed

+106
-86
lines changed

1 file changed

+106
-86
lines changed

‎lightning/src/ln/channel.rs

Lines changed: 106 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -8813,98 +8813,113 @@ where
88138813
.and_then(|_| self.get_channel_ready(logger))
88148814
} else { None };
88158815

8816-
if msg.next_local_commitment_number == next_counterparty_commitment_number {
8817-
if required_revoke.is_some() || self.context.signer_pending_revoke_and_ack {
8818-
log_debug!(logger, "Reconnected channel {} with only lost outbound RAA", &self.context.channel_id());
8819-
} else {
8820-
log_debug!(logger, "Reconnected channel {} with no loss", &self.context.channel_id());
8821-
}
8816+
let mut commitment_update = None;
8817+
let mut tx_signatures = None;
8818+
let mut tx_abort = None;
8819+
8820+
// if next_funding_txid is set:
8821+
if let Some(next_funding_txid) = msg.next_funding_txid {
8822+
// - if `next_funding_txid` matches the latest interactive funding transaction
8823+
// or the current channel funding transaction:
8824+
if let Some(session) = &self.interactive_tx_signing_session {
8825+
let our_next_funding_txid = self.maybe_get_next_funding_txid();
8826+
if let Some(our_next_funding_txid) = our_next_funding_txid {
8827+
if our_next_funding_txid != next_funding_txid {
8828+
return Err(ChannelError::close(format!(
8829+
"Unexpected next_funding_txid: {}; expected: {}",
8830+
next_funding_txid, our_next_funding_txid,
8831+
)));
8832+
}
88228833

8823-
// if next_funding_txid is set:
8824-
let (commitment_update, tx_signatures, tx_abort) = if let Some(next_funding_txid) = msg.next_funding_txid {
8825-
if let Some(session) = &self.interactive_tx_signing_session {
8826-
// if next_funding_txid matches the latest interactive funding transaction:
8827-
let our_next_funding_txid = session.unsigned_tx().compute_txid();
8828-
if our_next_funding_txid == next_funding_txid {
8829-
debug_assert_eq!(session.unsigned_tx().compute_txid(), self.maybe_get_next_funding_txid().unwrap());
8830-
8831-
let commitment_update = if !self.context.channel_state.is_their_tx_signatures_sent() && msg.next_local_commitment_number == 0 {
8832-
// if it has not received tx_signatures for that funding transaction AND
8833-
// if next_commitment_number is zero:
8834-
// MUST retransmit its commitment_signed for that funding transaction.
8835-
let commitment_signed = self.context.get_initial_commitment_signed_v2(&self.funding, logger)
8836-
// TODO(splicing): Support async signing
8837-
.ok_or_else(|| {
8838-
let message = "Failed to get signatures for new commitment_signed".to_owned();
8839-
ChannelError::Close(
8840-
(
8841-
message.clone(),
8842-
ClosureReason::HolderForceClosed { message, broadcasted_latest_txn: Some(false) },
8843-
)
8844-
)})?;
8845-
Some(msgs::CommitmentUpdate {
8846-
commitment_signed: vec![commitment_signed],
8847-
update_add_htlcs: vec![],
8848-
update_fulfill_htlcs: vec![],
8849-
update_fail_htlcs: vec![],
8850-
update_fail_malformed_htlcs: vec![],
8851-
update_fee: None,
8852-
})
8853-
} else { None };
8834+
if !session.has_received_commitment_signed() {
8835+
self.context.expecting_peer_commitment_signed = true;
8836+
}
8837+
8838+
// - if `next_commitment_number` is equal to the commitment number of the
8839+
// `commitment_signed` message it sent for this funding transaction:
8840+
// - MUST retransmit its `commitment_signed` for that funding transaction.
8841+
if msg.next_local_commitment_number == next_counterparty_commitment_number {
8842+
// `next_counterparty_commitment_number` is guaranteed to always be the
8843+
// commitment number of the `commitment_signed` message we sent for this
8844+
// funding transaction. If they set `next_funding_txid`, then they should
8845+
// not have processed our `tx_signatures` yet, which implies that our state
8846+
// machine is still paused and no updates can happen that would increment
8847+
// our `next_counterparty_commitment_number`.
8848+
//
8849+
// If they did set `next_funding_txid` even after processing our
8850+
// `tx_signatures` erroneously, this may end up resulting in a force close.
8851+
//
88548852
// TODO(dual_funding): For async signing support we need to hold back `tx_signatures` until the `commitment_signed` is ready.
8855-
let tx_signatures = if (
8856-
// if it has not received tx_signatures for that funding transaction AND
8857-
// if it has already received commitment_signed AND it should sign first, as specified in the tx_signatures requirements:
8858-
// MUST send its tx_signatures for that funding transaction.
8859-
!self.context.channel_state.is_their_tx_signatures_sent() && session.has_received_commitment_signed() && session.holder_sends_tx_signatures_first()
8860-
// else if it has already received tx_signatures for that funding transaction:
8861-
// MUST send its tx_signatures for that funding transaction.
8862-
) || self.context.channel_state.is_their_tx_signatures_sent() {
8863-
if self.context.channel_state.is_monitor_update_in_progress() {
8864-
// The `monitor_pending_tx_signatures` field should have already been set in `commitment_signed_initial_v2`
8865-
// if we were up first for signing and had a monitor update in progress, but check again just in case.
8866-
debug_assert!(self.context.monitor_pending_tx_signatures.is_some(), "monitor_pending_tx_signatures should already be set");
8867-
log_debug!(logger, "Not sending tx_signatures: a monitor update is in progress. Setting monitor_pending_tx_signatures.");
8868-
if self.context.monitor_pending_tx_signatures.is_none() {
8869-
self.context.monitor_pending_tx_signatures = session.holder_tx_signatures().clone();
8870-
}
8871-
None
8872-
} else {
8873-
// If `holder_tx_signatures` is `None` here, the `tx_signatures` message will be sent
8874-
// when the holder provides their witnesses as this will queue a `tx_signatures` if the
8875-
// holder must send one.
8876-
session.holder_tx_signatures().clone()
8853+
let commitment_signed = self.context.get_initial_commitment_signed_v2(&self.funding, logger)
8854+
// TODO(splicing): Support async signing
8855+
.ok_or_else(|| {
8856+
let message = "Failed to get signatures for new commitment_signed".to_owned();
8857+
ChannelError::Close(
8858+
(
8859+
message.clone(),
8860+
ClosureReason::HolderForceClosed { message, broadcasted_latest_txn: Some(false) },
8861+
)
8862+
)})?;
8863+
commitment_update = Some(msgs::CommitmentUpdate {
8864+
commitment_signed: vec![commitment_signed],
8865+
update_add_htlcs: vec![],
8866+
update_fulfill_htlcs: vec![],
8867+
update_fail_htlcs: vec![],
8868+
update_fail_malformed_htlcs: vec![],
8869+
update_fee: None,
8870+
});
8871+
}
8872+
8873+
// - if it has already received `commitment_signed` and it should sign first,
8874+
// as specified in the [`tx_signatures` requirements](#the-tx_signatures-message):
8875+
// - MUST send its `tx_signatures` for that funding transaction.
8876+
//
8877+
// - if it has already received `tx_signatures` for that funding transaction:
8878+
// - MUST send its `tx_signatures` for that funding transaction.
8879+
if (session.has_received_commitment_signed() && session.holder_sends_tx_signatures_first())
8880+
|| self.context.channel_state.is_their_tx_signatures_sent()
8881+
{
8882+
if self.context.channel_state.is_monitor_update_in_progress() {
8883+
// The `monitor_pending_tx_signatures` field should have already been
8884+
// set in `initial_commitment_signed_v2` if we were up first for signing
8885+
// and had a monitor update in progress.
8886+
if session.holder_sends_tx_signatures_first() {
8887+
debug_assert!(self.context.monitor_pending_tx_signatures.is_some());
88778888
}
88788889
} else {
8879-
None
8880-
};
8881-
if !session.has_received_commitment_signed() {
8882-
self.context.expecting_peer_commitment_signed = true;
8890+
// If `holder_tx_signatures` is `None` here, the `tx_signatures` message
8891+
// will be sent when the user provides their witnesses.
8892+
tx_signatures = session.holder_tx_signatures().clone()
88838893
}
8884-
(commitment_update, tx_signatures, None)
8885-
} else {
8886-
// The `next_funding_txid` does not match the latest interactive funding transaction so we
8887-
// MUST send tx_abort to let the remote know that they can forget this funding transaction.
8888-
(None, None, Some(msgs::TxAbort {
8889-
channel_id: self.context.channel_id(),
8890-
data: format!(
8891-
"next_funding_txid {} does match our latest interactive funding txid {}",
8892-
next_funding_txid, our_next_funding_txid,
8893-
).into_bytes() }))
88948894
}
88958895
} else {
8896-
// We'll just send a `tx_abort` here if we don't have a signing session for this channel
8897-
// on reestablish and tell our peer to just forget about it.
8898-
// Our peer is doing something strange, but it doesn't warrant closing the channel.
8899-
(None, None, Some(msgs::TxAbort {
8896+
// The `next_funding_txid` does not match the latest interactive funding
8897+
// transaction so we MUST send tx_abort to let the remote know that they can
8898+
// forget this funding transaction.
8899+
tx_abort = Some(msgs::TxAbort {
89008900
channel_id: self.context.channel_id(),
8901-
data:
8902-
"No active signing session. The associated funding transaction may have already been broadcast.".as_bytes().to_vec() }))
8901+
data: format!(
8902+
"Unexpected next_funding_txid {}",
8903+
next_funding_txid,
8904+
).into_bytes() });
89038905
}
89048906
} else {
8905-
// Don't send anything related to interactive signing if `next_funding_txid` is not set.
8906-
(None, None, None)
8907-
};
8907+
// We'll just send a `tx_abort` here if we don't have a signing session for this channel
8908+
// on reestablish and tell our peer to just forget about it.
8909+
// Our peer is doing something strange, but it doesn't warrant closing the channel.
8910+
tx_abort = Some(msgs::TxAbort {
8911+
channel_id: self.context.channel_id(),
8912+
data:
8913+
"No active signing session. The associated funding transaction may have already been broadcast.".as_bytes().to_vec() });
8914+
}
8915+
}
8916+
8917+
if msg.next_local_commitment_number == next_counterparty_commitment_number {
8918+
if required_revoke.is_some() || self.context.signer_pending_revoke_and_ack {
8919+
log_debug!(logger, "Reconnected channel {} with only lost outbound RAA", &self.context.channel_id());
8920+
} else {
8921+
log_debug!(logger, "Reconnected channel {} with no loss", &self.context.channel_id());
8922+
}
89088923

89098924
Ok(ReestablishResponses {
89108925
channel_ready, shutdown_msg, announcement_sigs,
@@ -8915,6 +8930,11 @@ where
89158930
tx_abort,
89168931
})
89178932
} else if msg.next_local_commitment_number == next_counterparty_commitment_number - 1 {
8933+
// We've made an update so we must have exchanged `tx_signatures`, implying that
8934+
// `commitment_signed` was also exchanged. However, we may still need to retransmit our
8935+
// `tx_signatures` if the counterparty sent theirs first but didn't get to process ours.
8936+
debug_assert!(commitment_update.is_none());
8937+
89188938
if required_revoke.is_some() || self.context.signer_pending_revoke_and_ack {
89198939
log_debug!(logger, "Reconnected channel {} with lost outbound RAA and lost remote commitment tx", &self.context.channel_id());
89208940
} else {
@@ -8927,8 +8947,8 @@ where
89278947
channel_ready, shutdown_msg, announcement_sigs,
89288948
commitment_update: None, raa: None,
89298949
order: self.context.resend_order.clone(),
8930-
tx_signatures: None,
8931-
tx_abort: None,
8950+
tx_signatures,
8951+
tx_abort,
89328952
})
89338953
} else {
89348954
let commitment_update = if self.context.resend_order == RAACommitmentOrder::RevokeAndACKFirst
@@ -8951,8 +8971,8 @@ where
89518971
channel_ready, shutdown_msg, announcement_sigs,
89528972
raa, commitment_update,
89538973
order: self.context.resend_order.clone(),
8954-
tx_signatures: None,
8955-
tx_abort: None,
8974+
tx_signatures,
8975+
tx_abort,
89568976
})
89578977
}
89588978
} else if msg.next_local_commitment_number < next_counterparty_commitment_number {

0 commit comments

Comments
 (0)