Skip to content

Commit beb1ec0

Browse files
committed
Handle receiving channel_reestablish with next_funding_txid
This follows the the specification closely in branching without being too verbose, so that it should be easy to follow the logic. See: https://github.com/lightning/bolts/blob/aa5207a/02-peer-protocol.md?plain=1#L2520-L2531
1 parent b0b8f25 commit beb1ec0

File tree

3 files changed

+111
-17
lines changed

3 files changed

+111
-17
lines changed

lightning/src/ln/channel.rs

+73-7
Original file line numberDiff line numberDiff line change
@@ -931,6 +931,8 @@ pub(super) struct ReestablishResponses {
931931
pub order: RAACommitmentOrder,
932932
pub announcement_sigs: Option<msgs::AnnouncementSignatures>,
933933
pub shutdown_msg: Option<msgs::Shutdown>,
934+
pub tx_signatures: Option<msgs::TxSignatures>,
935+
pub tx_abort: Option<msgs::TxAbort>,
934936
}
935937

936938
/// The first message we send to our peer after connection
@@ -2088,7 +2090,7 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
20882090

20892091
let mut output_index = None;
20902092
let expected_spk = self.context.get_funding_redeemscript().to_p2wsh();
2091-
for (idx, outp) in signing_session.unsigned_tx.outputs().enumerate() {
2093+
for (idx, outp) in signing_session.unsigned_tx().outputs().enumerate() {
20922094
if outp.script_pubkey() == &expected_spk && outp.value() == self.context.get_value_satoshis() {
20932095
if output_index.is_some() {
20942096
return Err(ChannelError::Close(
@@ -2101,7 +2103,7 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
21012103
}
21022104
}
21032105
let outpoint = if let Some(output_index) = output_index {
2104-
OutPoint { txid: signing_session.unsigned_tx.compute_txid(), index: output_index }
2106+
OutPoint { txid: signing_session.unsigned_tx().compute_txid(), index: output_index }
21052107
} else {
21062108
return Err(ChannelError::Close(
21072109
(
@@ -2116,7 +2118,7 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
21162118
let commitment_signed = self.context.get_initial_commitment_signed(logger);
21172119
let commitment_signed = match commitment_signed {
21182120
Ok(commitment_signed) => {
2119-
self.context.funding_transaction = Some(signing_session.unsigned_tx.build_unsigned_tx());
2121+
self.context.funding_transaction = Some(signing_session.unsigned_tx().build_unsigned_tx());
21202122
commitment_signed
21212123
},
21222124
Err(err) => {
@@ -5961,7 +5963,7 @@ impl<SP: Deref> FundedChannel<SP> where
59615963
}
59625964

59635965
if let Some(ref mut signing_session) = self.interactive_tx_signing_session {
5964-
if msg.tx_hash != signing_session.unsigned_tx.compute_txid() {
5966+
if msg.tx_hash != signing_session.unsigned_tx().compute_txid() {
59655967
return Err(ChannelError::Close(
59665968
(
59675969
"The txid for the transaction does not match".to_string(),
@@ -6591,7 +6593,7 @@ impl<SP: Deref> FundedChannel<SP> where
65916593
}
65926594

65936595
if msg.next_local_commitment_number >= INITIAL_COMMITMENT_NUMBER || msg.next_remote_commitment_number >= INITIAL_COMMITMENT_NUMBER ||
6594-
msg.next_local_commitment_number == 0 {
6596+
msg.next_local_commitment_number == 0 && msg.next_funding_txid.is_none() {
65956597
return Err(ChannelError::close("Peer sent an invalid channel_reestablish to force close in a non-standard way".to_owned()));
65966598
}
65976599

@@ -6655,6 +6657,8 @@ impl<SP: Deref> FundedChannel<SP> where
66556657
raa: None, commitment_update: None,
66566658
order: RAACommitmentOrder::CommitmentFirst,
66576659
shutdown_msg, announcement_sigs,
6660+
tx_signatures: None,
6661+
tx_abort: None,
66586662
});
66596663
}
66606664

@@ -6664,6 +6668,8 @@ impl<SP: Deref> FundedChannel<SP> where
66646668
raa: None, commitment_update: None,
66656669
order: RAACommitmentOrder::CommitmentFirst,
66666670
shutdown_msg, announcement_sigs,
6671+
tx_signatures: None,
6672+
tx_abort: None,
66676673
});
66686674
}
66696675

@@ -6709,11 +6715,67 @@ impl<SP: Deref> FundedChannel<SP> where
67096715
log_debug!(logger, "Reconnected channel {} with no loss", &self.context.channel_id());
67106716
}
67116717

6718+
// if next_funding_txid is set:
6719+
let (commitment_update, tx_signatures, tx_abort) = if let Some(next_funding_txid) = msg.next_funding_txid {
6720+
if let Some(session) = &self.interactive_tx_signing_session {
6721+
// if next_funding_txid matches the latest interactive funding transaction:
6722+
if session.unsigned_tx().compute_txid() == next_funding_txid {
6723+
// if it has not received tx_signatures for that funding transaction:
6724+
if !session.counterparty_sent_tx_signatures() {
6725+
// if next_commitment_number is zero:
6726+
let commitment_update = if msg.next_local_commitment_number == 0 {
6727+
// MUST retransmit its commitment_signed for that funding transaction.
6728+
let commitment_signed = self.context.get_initial_commitment_signed(logger)?;
6729+
Some(msgs::CommitmentUpdate {
6730+
commitment_signed,
6731+
update_add_htlcs: vec![],
6732+
update_fulfill_htlcs: vec![],
6733+
update_fail_htlcs: vec![],
6734+
update_fail_malformed_htlcs: vec![],
6735+
update_fee: None,
6736+
})
6737+
} else { None };
6738+
// if it has already received commitment_signed and it should sign first, as specified in the tx_signatures requirements:
6739+
if session.has_received_commitment_signed() && session.holder_sends_tx_signatures_first() {
6740+
// MUST send its tx_signatures for that funding transaction.
6741+
(commitment_update, session.holder_tx_signatures().clone(), None)
6742+
} else {
6743+
(commitment_update, None, None)
6744+
}
6745+
} else {
6746+
// if it has already received tx_signatures for that funding transaction:
6747+
// MUST send its tx_signatures for that funding transaction.
6748+
(None, session.holder_tx_signatures().clone(), None)
6749+
}
6750+
} else {
6751+
// MUST send tx_abort to let the sending node know that they can forget this funding transaction.
6752+
(None, None, Some(msgs::TxAbort { channel_id: self.context.channel_id(), data: vec![] }))
6753+
}
6754+
} else {
6755+
// Counterparty set `next_funding_txid` at incorrect state.
6756+
// TODO(dual_funding): Should probably error here (or send tx_abort) but not in spec.
6757+
(None, None, None)
6758+
}
6759+
} else {
6760+
// if `next_funding_txid` is not set, and `next_commitment_number` is zero:
6761+
if msg.next_local_commitment_number == 0 {
6762+
// MUST immediately fail the channel and broadcast any relevant latest commitment transaction.
6763+
return Err(ChannelError::close(format!(
6764+
"Peer attempted to reestablish channel expecting a future local commitment transaction: {} (received) vs {} (expected)",
6765+
msg.next_remote_commitment_number,
6766+
our_commitment_transaction
6767+
)));
6768+
}
6769+
(None, None, None)
6770+
};
6771+
67126772
Ok(ReestablishResponses {
67136773
channel_ready, shutdown_msg, announcement_sigs,
67146774
raa: required_revoke,
6715-
commitment_update: None,
6775+
commitment_update,
67166776
order: self.context.resend_order.clone(),
6777+
tx_signatures,
6778+
tx_abort,
67176779
})
67186780
} else if msg.next_local_commitment_number == next_counterparty_commitment_number - 1 {
67196781
if required_revoke.is_some() || self.context.signer_pending_revoke_and_ack {
@@ -6728,6 +6790,8 @@ impl<SP: Deref> FundedChannel<SP> where
67286790
channel_ready, shutdown_msg, announcement_sigs,
67296791
commitment_update: None, raa: None,
67306792
order: self.context.resend_order.clone(),
6793+
tx_signatures: None,
6794+
tx_abort: None,
67316795
})
67326796
} else {
67336797
let commitment_update = if self.context.resend_order == RAACommitmentOrder::RevokeAndACKFirst
@@ -6750,6 +6814,8 @@ impl<SP: Deref> FundedChannel<SP> where
67506814
channel_ready, shutdown_msg, announcement_sigs,
67516815
raa, commitment_update,
67526816
order: self.context.resend_order.clone(),
6817+
tx_signatures: None,
6818+
tx_abort: None,
67536819
})
67546820
}
67556821
} else if msg.next_local_commitment_number < next_counterparty_commitment_number {
@@ -8027,7 +8093,7 @@ impl<SP: Deref> FundedChannel<SP> where
80278093
// to the txid of that interactive transaction, else we MUST NOT set it.
80288094
if let Some(signing_session) = &self.interactive_tx_signing_session {
80298095
// Since we have a signing_session, this implies we've sent an initial `commitment_signed`...
8030-
if !signing_session.counterparty_sent_tx_signatures {
8096+
if !signing_session.counterparty_sent_tx_signatures() {
80318097
// ...but we didn't receive a `tx_signatures` from the counterparty yet.
80328098
Some(self.funding_outpoint().txid)
80338099
} else {

lightning/src/ln/channelmanager.rs

+12-4
Original file line numberDiff line numberDiff line change
@@ -3237,7 +3237,7 @@ macro_rules! handle_monitor_update_completion {
32373237
&mut $peer_state.pending_msg_events, $chan, updates.raa,
32383238
updates.commitment_update, updates.order, updates.accepted_htlcs, updates.pending_update_adds,
32393239
updates.funding_broadcastable, updates.channel_ready,
3240-
updates.announcement_sigs, updates.tx_signatures);
3240+
updates.announcement_sigs, updates.tx_signatures, None);
32413241
if let Some(upd) = channel_update {
32423242
$peer_state.pending_msg_events.push(upd);
32433243
}
@@ -7575,10 +7575,10 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
75757575
pending_forwards: Vec<(PendingHTLCInfo, u64)>, pending_update_adds: Vec<msgs::UpdateAddHTLC>,
75767576
funding_broadcastable: Option<Transaction>,
75777577
channel_ready: Option<msgs::ChannelReady>, announcement_sigs: Option<msgs::AnnouncementSignatures>,
7578-
tx_signatures: Option<msgs::TxSignatures>
7578+
tx_signatures: Option<msgs::TxSignatures>, tx_abort: Option<msgs::TxAbort>,
75797579
) -> (Option<(u64, Option<PublicKey>, OutPoint, ChannelId, u128, Vec<(PendingHTLCInfo, u64)>)>, Option<(u64, Vec<msgs::UpdateAddHTLC>)>) {
75807580
let logger = WithChannelContext::from(&self.logger, &channel.context, None);
7581-
log_trace!(logger, "Handling channel resumption for channel {} with {} RAA, {} commitment update, {} pending forwards, {} pending update_add_htlcs, {}broadcasting funding, {} channel ready, {} announcement, {} tx_signatures",
7581+
log_trace!(logger, "Handling channel resumption for channel {} with {} RAA, {} commitment update, {} pending forwards, {} pending update_add_htlcs, {}broadcasting funding, {} channel ready, {} announcement, {} tx_signatures, {} tx_abort",
75827582
&channel.context.channel_id(),
75837583
if raa.is_some() { "an" } else { "no" },
75847584
if commitment_update.is_some() { "a" } else { "no" },
@@ -7587,6 +7587,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
75877587
if channel_ready.is_some() { "sending" } else { "without" },
75887588
if announcement_sigs.is_some() { "sending" } else { "without" },
75897589
if tx_signatures.is_some() { "sending" } else { "without" },
7590+
if tx_abort.is_some() { "sending" } else { "without" },
75907591
);
75917592

75927593
let counterparty_node_id = channel.context.get_counterparty_node_id();
@@ -7620,6 +7621,12 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
76207621
msg,
76217622
});
76227623
}
7624+
if let Some(msg) = tx_abort {
7625+
pending_msg_events.push(events::MessageSendEvent::SendTxAbort {
7626+
node_id: counterparty_node_id,
7627+
msg,
7628+
});
7629+
}
76237630

76247631
macro_rules! handle_cs { () => {
76257632
if let Some(update) = commitment_update {
@@ -9332,7 +9339,8 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
93329339
let need_lnd_workaround = chan.context.workaround_lnd_bug_4006.take();
93339340
let (htlc_forwards, decode_update_add_htlcs) = self.handle_channel_resumption(
93349341
&mut peer_state.pending_msg_events, chan, responses.raa, responses.commitment_update, responses.order,
9335-
Vec::new(), Vec::new(), None, responses.channel_ready, responses.announcement_sigs, None);
9342+
Vec::new(), Vec::new(), None, responses.channel_ready, responses.announcement_sigs,
9343+
responses.tx_signatures, responses.tx_abort);
93369344
debug_assert!(htlc_forwards.is_none());
93379345
debug_assert!(decode_update_add_htlcs.is_none());
93389346
if let Some(upd) = channel_update {

lightning/src/ln/interactivetxs.rs

+26-6
Original file line numberDiff line numberDiff line change
@@ -289,16 +289,36 @@ impl ConstructedTransaction {
289289
/// https://github.com/lightning/bolts/blob/master/02-peer-protocol.md#sharing-funding-signatures-tx_signatures
290290
#[derive(Debug, Clone, PartialEq)]
291291
pub(crate) struct InteractiveTxSigningSession {
292-
pub unsigned_tx: ConstructedTransaction,
293-
pub counterparty_sent_tx_signatures: bool,
292+
unsigned_tx: ConstructedTransaction,
293+
counterparty_sent_tx_signatures: bool,
294294
holder_sends_tx_signatures_first: bool,
295-
received_commitment_signed: bool,
295+
has_received_commitment_signed: bool,
296296
holder_tx_signatures: Option<TxSignatures>,
297297
}
298298

299299
impl InteractiveTxSigningSession {
300+
pub fn unsigned_tx(&self) -> &ConstructedTransaction {
301+
&self.unsigned_tx
302+
}
303+
304+
pub fn counterparty_sent_tx_signatures(&self) -> bool {
305+
self.counterparty_sent_tx_signatures
306+
}
307+
308+
pub fn holder_sends_tx_signatures_first(&self) -> bool {
309+
self.holder_sends_tx_signatures_first
310+
}
311+
312+
pub fn has_received_commitment_signed(&self) -> bool {
313+
self.has_received_commitment_signed
314+
}
315+
316+
pub fn holder_tx_signatures(&self) -> &Option<TxSignatures> {
317+
&self.holder_tx_signatures
318+
}
319+
300320
pub fn received_commitment_signed(&mut self) -> Option<TxSignatures> {
301-
self.received_commitment_signed = true;
321+
self.has_received_commitment_signed = true;
302322
if self.holder_sends_tx_signatures_first {
303323
self.holder_tx_signatures.clone()
304324
} else {
@@ -307,7 +327,7 @@ impl InteractiveTxSigningSession {
307327
}
308328

309329
pub fn get_tx_signatures(&self) -> Option<TxSignatures> {
310-
if self.received_commitment_signed {
330+
if self.has_received_commitment_signed {
311331
self.holder_tx_signatures.clone()
312332
} else {
313333
None
@@ -988,7 +1008,7 @@ macro_rules! define_state_transitions {
9881008
let signing_session = InteractiveTxSigningSession {
9891009
holder_sends_tx_signatures_first: tx.holder_sends_tx_signatures_first,
9901010
unsigned_tx: tx,
991-
received_commitment_signed: false,
1011+
has_received_commitment_signed: false,
9921012
holder_tx_signatures: None,
9931013
counterparty_sent_tx_signatures: false,
9941014
};

0 commit comments

Comments
 (0)