Skip to content

Commit 770640f

Browse files
jkczyzwpaulino
andcommitted
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 ac4e17e commit 770640f

File tree

1 file changed

+101
-81
lines changed

1 file changed

+101
-81
lines changed

lightning/src/ln/channel.rs

Lines changed: 101 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -8779,91 +8779,106 @@ where
87798779
self.get_channel_ready(logger)
87808780
} else { None };
87818781

8782-
if msg.next_local_commitment_number == next_counterparty_commitment_number {
8783-
if required_revoke.is_some() || self.context.signer_pending_revoke_and_ack {
8784-
log_debug!(logger, "Reconnected channel {} with only lost outbound RAA", &self.context.channel_id());
8785-
} else {
8786-
log_debug!(logger, "Reconnected channel {} with no loss", &self.context.channel_id());
8787-
}
8782+
let mut commitment_update = None;
8783+
let mut tx_signatures = None;
8784+
let mut tx_abort = None;
8785+
8786+
// if next_funding_txid is set:
8787+
if let Some(next_funding_txid) = msg.next_funding_txid {
8788+
// - if `next_funding_txid` matches the latest interactive funding transaction
8789+
// or the current channel funding transaction:
8790+
if let Some(session) = &self.interactive_tx_signing_session {
8791+
let our_next_funding_txid = self.maybe_get_next_funding_txid();
8792+
if let Some(our_next_funding_txid) = our_next_funding_txid {
8793+
if our_next_funding_txid != next_funding_txid {
8794+
return Err(ChannelError::close(format!(
8795+
"Unexpected next_funding_txid: {}; expected: {}",
8796+
next_funding_txid, our_next_funding_txid,
8797+
)));
8798+
}
87888799

8789-
// if next_funding_txid is set:
8790-
let (commitment_update, tx_signatures, tx_abort) = if let Some(next_funding_txid) = msg.next_funding_txid {
8791-
if let Some(session) = &self.interactive_tx_signing_session {
8792-
// if next_funding_txid matches the latest interactive funding transaction:
8793-
let our_next_funding_txid = session.unsigned_tx().compute_txid();
8794-
if our_next_funding_txid == next_funding_txid {
8795-
debug_assert_eq!(session.unsigned_tx().compute_txid(), self.maybe_get_next_funding_txid().unwrap());
8796-
8797-
let commitment_update = if !self.context.channel_state.is_their_tx_signatures_sent() && msg.next_local_commitment_number == 0 {
8798-
// if it has not received tx_signatures for that funding transaction AND
8799-
// if next_commitment_number is zero:
8800-
// MUST retransmit its commitment_signed for that funding transaction.
8801-
let commitment_signed = self.context.get_initial_commitment_signed_v2(&self.funding, logger)
8802-
// TODO(splicing): Support async signing
8803-
.ok_or_else(|| {
8804-
let message = "Failed to get signatures for new commitment_signed".to_owned();
8805-
ChannelError::Close(
8806-
(
8807-
message.clone(),
8808-
ClosureReason::HolderForceClosed { message, broadcasted_latest_txn: Some(false) },
8809-
)
8810-
)})?;
8811-
Some(msgs::CommitmentUpdate {
8812-
commitment_signed: vec![commitment_signed],
8813-
update_add_htlcs: vec![],
8814-
update_fulfill_htlcs: vec![],
8815-
update_fail_htlcs: vec![],
8816-
update_fail_malformed_htlcs: vec![],
8817-
update_fee: None,
8818-
})
8819-
} else { None };
8820-
let tx_signatures = if (
8821-
// if it has not received tx_signatures for that funding transaction AND
8822-
// if it has already received commitment_signed AND it should sign first, as specified in the tx_signatures requirements:
8823-
// MUST send its tx_signatures for that funding transaction.
8824-
!self.context.channel_state.is_their_tx_signatures_sent() && session.has_received_commitment_signed() && session.holder_sends_tx_signatures_first()
8825-
// else if it has already received tx_signatures for that funding transaction:
8826-
// MUST send its tx_signatures for that funding transaction.
8827-
) || self.context.channel_state.is_their_tx_signatures_sent() {
8828-
// If `holder_tx_signatures` is `None` here, the `tx_signatures` message will be sent
8829-
// when the holder provides their witnesses as this will queue a `tx_signatures` if the
8830-
// holder must send one.
8831-
if session.holder_tx_signatures().is_none() {
8832-
log_debug!(logger, "Waiting for funding transaction signatures to be provided");
8833-
None
8834-
} else {
8835-
session.holder_tx_signatures().clone()
8836-
}
8800+
if !session.has_received_commitment_signed() {
8801+
self.context.expecting_peer_commitment_signed = true;
8802+
}
8803+
8804+
// - if `next_commitment_number` is equal to the commitment number of the
8805+
// `commitment_signed` message it sent for this funding transaction:
8806+
// - MUST retransmit its `commitment_signed` for that funding transaction.
8807+
if msg.next_local_commitment_number == next_counterparty_commitment_number {
8808+
// `next_counterparty_commitment_number` is guaranteed to always be the
8809+
// commitment number of the `commitment_signed` message we sent for this
8810+
// funding transaction. If they set `next_funding_txid`, then they should
8811+
// not have processed our `tx_signatures` yet, which implies that our state
8812+
// machine is still paused and no updates can happen that would increment
8813+
// our `next_counterparty_commitment_number`.
8814+
//
8815+
// If they did set `next_funding_txid` even after processing our
8816+
// `tx_signatures` erroneously, this may end up resulting in a force close.
8817+
let commitment_signed = self.context.get_initial_commitment_signed_v2(&self.funding, logger)
8818+
// TODO(splicing): Support async signing
8819+
.ok_or_else(|| {
8820+
let message = "Failed to get signatures for new commitment_signed".to_owned();
8821+
ChannelError::Close(
8822+
(
8823+
message.clone(),
8824+
ClosureReason::HolderForceClosed { message, broadcasted_latest_txn: Some(false) },
8825+
)
8826+
)})?;
8827+
commitment_update = Some(msgs::CommitmentUpdate {
8828+
commitment_signed: vec![commitment_signed],
8829+
update_add_htlcs: vec![],
8830+
update_fulfill_htlcs: vec![],
8831+
update_fail_htlcs: vec![],
8832+
update_fail_malformed_htlcs: vec![],
8833+
update_fee: None,
8834+
});
8835+
}
8836+
8837+
// - if it has already received `commitment_signed` and it should sign first
8838+
// - MUST send its `tx_signatures` for that funding transaction.
8839+
//
8840+
// - if it has already received `tx_signatures` for that funding transaction:
8841+
// - MUST send its `tx_signatures` for that funding transaction.
8842+
if (session.has_received_commitment_signed() && session.holder_sends_tx_signatures_first())
8843+
|| self.context.channel_state.is_their_tx_signatures_sent()
8844+
{
8845+
// If `holder_tx_signatures` is `None` here, the `tx_signatures` message will be sent
8846+
// when the holder provides their witnesses as this will queue a `tx_signatures` if the
8847+
// holder must send one.
8848+
if session.holder_tx_signatures().is_none() {
8849+
log_debug!(logger, "Waiting for funding transaction signatures to be provided");
88378850
} else {
8838-
None
8839-
};
8840-
if !session.has_received_commitment_signed() {
8841-
self.context.expecting_peer_commitment_signed = true;
8851+
tx_signatures = session.holder_tx_signatures().clone();
88428852
}
8843-
(commitment_update, tx_signatures, None)
8844-
} else {
8845-
// The `next_funding_txid` does not match the latest interactive funding transaction so we
8846-
// MUST send tx_abort to let the remote know that they can forget this funding transaction.
8847-
(None, None, Some(msgs::TxAbort {
8848-
channel_id: self.context.channel_id(),
8849-
data: format!(
8850-
"next_funding_txid {} does match our latest interactive funding txid {}",
8851-
next_funding_txid, our_next_funding_txid,
8852-
).into_bytes() }))
88538853
}
88548854
} else {
8855-
// We'll just send a `tx_abort` here if we don't have a signing session for this channel
8856-
// on reestablish and tell our peer to just forget about it.
8857-
// Our peer is doing something strange, but it doesn't warrant closing the channel.
8858-
(None, None, Some(msgs::TxAbort {
8855+
// The `next_funding_txid` does not match the latest interactive funding
8856+
// transaction so we MUST send tx_abort to let the remote know that they can
8857+
// forget this funding transaction.
8858+
tx_abort = Some(msgs::TxAbort {
88598859
channel_id: self.context.channel_id(),
8860-
data:
8861-
"No active signing session. The associated funding transaction may have already been broadcast.".as_bytes().to_vec() }))
8860+
data: format!(
8861+
"Unexpected next_funding_txid {}",
8862+
next_funding_txid,
8863+
).into_bytes() });
88628864
}
88638865
} else {
8864-
// Don't send anything related to interactive signing if `next_funding_txid` is not set.
8865-
(None, None, None)
8866-
};
8866+
// We'll just send a `tx_abort` here if we don't have a signing session for this channel
8867+
// on reestablish and tell our peer to just forget about it.
8868+
// Our peer is doing something strange, but it doesn't warrant closing the channel.
8869+
tx_abort = Some(msgs::TxAbort {
8870+
channel_id: self.context.channel_id(),
8871+
data:
8872+
"No active signing session. The associated funding transaction may have already been broadcast.".as_bytes().to_vec() });
8873+
}
8874+
}
8875+
8876+
if msg.next_local_commitment_number == next_counterparty_commitment_number {
8877+
if required_revoke.is_some() || self.context.signer_pending_revoke_and_ack {
8878+
log_debug!(logger, "Reconnected channel {} with only lost outbound RAA", &self.context.channel_id());
8879+
} else {
8880+
log_debug!(logger, "Reconnected channel {} with no loss", &self.context.channel_id());
8881+
}
88678882

88688883
Ok(ReestablishResponses {
88698884
channel_ready, shutdown_msg, announcement_sigs,
@@ -8874,6 +8889,11 @@ where
88748889
tx_abort,
88758890
})
88768891
} else if msg.next_local_commitment_number == next_counterparty_commitment_number - 1 {
8892+
// We've made an update so we must have exchanged `tx_signatures`, implying that
8893+
// `commitment_signed` was also exchanged. However, we may still need to retransmit our
8894+
// `tx_signatures` if the counterparty sent theirs first but didn't get to process ours.
8895+
debug_assert!(commitment_update.is_none());
8896+
88778897
if required_revoke.is_some() || self.context.signer_pending_revoke_and_ack {
88788898
log_debug!(logger, "Reconnected channel {} with lost outbound RAA and lost remote commitment tx", &self.context.channel_id());
88798899
} else {
@@ -8886,8 +8906,8 @@ where
88868906
channel_ready, shutdown_msg, announcement_sigs,
88878907
commitment_update: None, raa: None,
88888908
order: self.context.resend_order.clone(),
8889-
tx_signatures: None,
8890-
tx_abort: None,
8909+
tx_signatures,
8910+
tx_abort,
88918911
})
88928912
} else {
88938913
let commitment_update = if self.context.resend_order == RAACommitmentOrder::RevokeAndACKFirst
@@ -8910,8 +8930,8 @@ where
89108930
channel_ready, shutdown_msg, announcement_sigs,
89118931
raa, commitment_update,
89128932
order: self.context.resend_order.clone(),
8913-
tx_signatures: None,
8914-
tx_abort: None,
8933+
tx_signatures,
8934+
tx_abort,
89158935
})
89168936
}
89178937
} else if msg.next_local_commitment_number < next_counterparty_commitment_number {

0 commit comments

Comments
 (0)