Skip to content

Commit fd9673f

Browse files
TheBlueMattTheBlueMatt
committed
Constify 2**48-1 and add some additional comments in Channel
1 parent d60b6c7 commit fd9673f

File tree

1 file changed

+33
-22
lines changed

1 file changed

+33
-22
lines changed

‎src/ln/channel.rs

+33-22
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,8 @@ enum ChannelState {
256256
}
257257
const BOTH_SIDES_SHUTDOWN_MASK: u32 = (ChannelState::LocalShutdownSent as u32 | ChannelState::RemoteShutdownSent as u32);
258258

259+
const INITIAL_COMMITMENT_NUMBER: u64 = (1 << 48) - 1;
260+
259261
// TODO: We should refactor this to be an Inbound/OutboundChannel until initial setup handshaking
260262
// has been completed, and then turn into a Channel to get compiler-time enforcement of things like
261263
// calling channel_id() before we're set up or things like get_outbound_funding_signed on an
@@ -436,8 +438,8 @@ impl Channel {
436438
channel_value_satoshis: channel_value_satoshis,
437439

438440
local_keys: chan_keys,
439-
cur_local_commitment_transaction_number: (1 << 48) - 1,
440-
cur_remote_commitment_transaction_number: (1 << 48) - 1,
441+
cur_local_commitment_transaction_number: INITIAL_COMMITMENT_NUMBER,
442+
cur_remote_commitment_transaction_number: INITIAL_COMMITMENT_NUMBER,
441443
value_to_self_msat: channel_value_satoshis * 1000 - push_msat,
442444
pending_inbound_htlcs: Vec::new(),
443445
pending_outbound_htlcs: Vec::new(),
@@ -594,8 +596,8 @@ impl Channel {
594596
announce_publicly: their_announce,
595597

596598
local_keys: chan_keys,
597-
cur_local_commitment_transaction_number: (1 << 48) - 1,
598-
cur_remote_commitment_transaction_number: (1 << 48) - 1,
599+
cur_local_commitment_transaction_number: INITIAL_COMMITMENT_NUMBER,
600+
cur_remote_commitment_transaction_number: INITIAL_COMMITMENT_NUMBER,
599601
value_to_self_msat: msg.push_msat,
600602
pending_inbound_htlcs: Vec::new(),
601603
pending_outbound_htlcs: Vec::new(),
@@ -693,7 +695,7 @@ impl Channel {
693695
/// which peer generated this transaction and "to whom" this transaction flows.
694696
#[inline]
695697
fn build_commitment_transaction(&self, commitment_number: u64, keys: &TxCreationKeys, local: bool, generated_by_local: bool) -> (Transaction, Vec<HTLCOutputInCommitment>) {
696-
let obscured_commitment_transaction_number = self.get_commitment_transaction_number_obscure_factor() ^ (0xffffffffffff - commitment_number);
698+
let obscured_commitment_transaction_number = self.get_commitment_transaction_number_obscure_factor() ^ (INITIAL_COMMITMENT_NUMBER - commitment_number);
697699

698700
let txins = {
699701
let mut ins: Vec<TxIn> = Vec::new();
@@ -1303,7 +1305,9 @@ impl Channel {
13031305
// channel.
13041306
return Err(HandleError{err: "Received funding_created after we got the channel!", action: Some(msgs::ErrorAction::SendErrorMessage {msg: msgs::ErrorMessage {channel_id: self.channel_id, data: "Received funding_created after we got the channel!".to_string()}})});
13051307
}
1306-
if self.channel_monitor.get_min_seen_secret() != (1 << 48) || self.cur_remote_commitment_transaction_number != (1 << 48) - 1 || self.cur_local_commitment_transaction_number != (1 << 48) - 1 {
1308+
if self.channel_monitor.get_min_seen_secret() != (1 << 48) ||
1309+
self.cur_remote_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER ||
1310+
self.cur_local_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER {
13071311
panic!("Should not have advanced channel commitment tx numbers prior to funding_created");
13081312
}
13091313

@@ -1342,7 +1346,9 @@ impl Channel {
13421346
if self.channel_state != ChannelState::FundingCreated as u32 {
13431347
return Err(HandleError{err: "Received funding_signed in strange state!", action: None});
13441348
}
1345-
if self.channel_monitor.get_min_seen_secret() != (1 << 48) || self.cur_remote_commitment_transaction_number != (1 << 48) - 2 || self.cur_local_commitment_transaction_number != (1 << 48) - 1 {
1349+
if self.channel_monitor.get_min_seen_secret() != (1 << 48) ||
1350+
self.cur_remote_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER - 1 ||
1351+
self.cur_local_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER {
13461352
panic!("Should not have advanced channel commitment tx numbers prior to funding_created");
13471353
}
13481354

@@ -1375,8 +1381,9 @@ impl Channel {
13751381
self.channel_state = ChannelState::ChannelFunded as u32 | (self.channel_state & BOTH_SIDES_SHUTDOWN_MASK);
13761382
self.channel_update_count += 1;
13771383
} else if self.channel_state & (ChannelState::ChannelFunded as u32) != 0 &&
1378-
self.cur_local_commitment_transaction_number == (1 << 48) - 2 &&
1379-
self.cur_remote_commitment_transaction_number == (1 << 48) - 2 {
1384+
// Note that funding_signed/funding_created will have decremented both by 1!
1385+
self.cur_local_commitment_transaction_number == INITIAL_COMMITMENT_NUMBER - 1 &&
1386+
self.cur_remote_commitment_transaction_number == INITIAL_COMMITMENT_NUMBER - 1 {
13801387
if self.their_cur_commitment_point != Some(msg.next_per_commitment_point) {
13811388
return Err(HandleError{err: "Peer sent a reconnect funding_locked with a different point", action: None});
13821389
}
@@ -1908,8 +1915,8 @@ impl Channel {
19081915
return Err(HandleError{err: "Peer sent a loose channel_reestablish not after reconnect", action: Some(msgs::ErrorAction::SendErrorMessage{msg: msgs::ErrorMessage{data: "Peer sent a loose channel_reestablish not after reconnect".to_string(), channel_id: msg.channel_id}})});
19091916
}
19101917

1911-
if msg.next_local_commitment_number == 0 || msg.next_local_commitment_number >= 0xffffffffffff ||
1912-
msg.next_remote_commitment_number == 0 || msg.next_remote_commitment_number >= 0xffffffffffff {
1918+
if msg.next_local_commitment_number == 0 || msg.next_local_commitment_number >= INITIAL_COMMITMENT_NUMBER ||
1919+
msg.next_remote_commitment_number == 0 || msg.next_remote_commitment_number >= INITIAL_COMMITMENT_NUMBER {
19131920
return Err(HandleError{err: "Peer send garbage channel_reestablish", action: Some(msgs::ErrorAction::SendErrorMessage{msg: msgs::ErrorMessage{data: "Peer send garbage channel_reestablish".to_string(), channel_id: msg.channel_id}})});
19141921
}
19151922

@@ -1918,8 +1925,10 @@ impl Channel {
19181925
self.channel_state &= !(ChannelState::PeerDisconnected as u32);
19191926

19201927
let mut required_revoke = None;
1921-
if msg.next_remote_commitment_number == 0xffffffffffff - self.cur_local_commitment_transaction_number {
1922-
} else if msg.next_remote_commitment_number == 0xfffffffffffe - self.cur_local_commitment_transaction_number {
1928+
if msg.next_remote_commitment_number == INITIAL_COMMITMENT_NUMBER - self.cur_local_commitment_transaction_number {
1929+
// Remote isn't waiting on any RevokeAndACK from us!
1930+
// Note that if we need to repeat our FundingLocked we'll do that in the next if block.
1931+
} else if msg.next_remote_commitment_number == (INITIAL_COMMITMENT_NUMBER - 1) - self.cur_local_commitment_transaction_number {
19231932
let next_per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &self.build_local_commitment_secret(self.cur_local_commitment_transaction_number));
19241933
let per_commitment_secret = chan_utils::build_commitment_secret(self.local_keys.commitment_seed, self.cur_local_commitment_transaction_number + 2);
19251934
required_revoke = Some(msgs::RevokeAndACK {
@@ -1931,8 +1940,8 @@ impl Channel {
19311940
return Err(HandleError{err: "Peer attempted to reestablish channel with a very old local commitment transaction", action: Some(msgs::ErrorAction::SendErrorMessage{msg: msgs::ErrorMessage{data: "Peer attempted to reestablish channel with a very old remote commitment transaction".to_string(), channel_id: msg.channel_id}})});
19321941
}
19331942

1934-
if msg.next_local_commitment_number == 0xffffffffffff - self.cur_remote_commitment_transaction_number {
1935-
if msg.next_remote_commitment_number == 0xffffffffffff - self.cur_local_commitment_transaction_number {
1943+
if msg.next_local_commitment_number == INITIAL_COMMITMENT_NUMBER - self.cur_remote_commitment_transaction_number {
1944+
if msg.next_remote_commitment_number == INITIAL_COMMITMENT_NUMBER - self.cur_local_commitment_transaction_number {
19361945
log_debug!(self, "Reconnected channel {} with no lost commitment txn", log_bytes!(self.channel_id()));
19371946
if msg.next_local_commitment_number == 1 && msg.next_remote_commitment_number == 1 {
19381947
let next_per_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
@@ -1964,7 +1973,7 @@ impl Channel {
19641973
} else {
19651974
return Ok((None, required_revoke, None, None));
19661975
}
1967-
} else if msg.next_local_commitment_number == 0xfffffffffffe - self.cur_remote_commitment_transaction_number {
1976+
} else if msg.next_local_commitment_number == (INITIAL_COMMITMENT_NUMBER - 1) - self.cur_remote_commitment_transaction_number {
19681977
return Ok((None, required_revoke,
19691978
Some(msgs::CommitmentUpdate {
19701979
update_add_htlcs: Vec::new(),
@@ -2389,7 +2398,7 @@ impl Channel {
23892398
panic!("Cannot generate an open_channel after we've moved forward");
23902399
}
23912400

2392-
if self.cur_local_commitment_transaction_number != (1 << 48) - 1 {
2401+
if self.cur_local_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER {
23932402
panic!("Tried to send an open_channel for a channel that has already advanced");
23942403
}
23952404

@@ -2425,7 +2434,7 @@ impl Channel {
24252434
if self.channel_state != (ChannelState::OurInitSent as u32) | (ChannelState::TheirInitSent as u32) {
24262435
panic!("Tried to send accept_channel after channel had moved forward");
24272436
}
2428-
if self.cur_local_commitment_transaction_number != (1 << 48) - 1 {
2437+
if self.cur_local_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER {
24292438
panic!("Tried to send an accept_channel for a channel that has already advanced");
24302439
}
24312440

@@ -2474,7 +2483,9 @@ impl Channel {
24742483
if self.channel_state != (ChannelState::OurInitSent as u32 | ChannelState::TheirInitSent as u32) {
24752484
panic!("Tried to get a funding_created messsage at a time other than immediately after initial handshake completion (or tried to get funding_created twice)");
24762485
}
2477-
if self.channel_monitor.get_min_seen_secret() != (1 << 48) || self.cur_remote_commitment_transaction_number != (1 << 48) - 1 || self.cur_local_commitment_transaction_number != (1 << 48) - 1 {
2486+
if self.channel_monitor.get_min_seen_secret() != (1 << 48) ||
2487+
self.cur_remote_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER ||
2488+
self.cur_local_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER {
24782489
panic!("Should not have advanced channel commitment tx numbers prior to funding_created");
24792490
}
24802491

@@ -2551,8 +2562,8 @@ impl Channel {
25512562
assert_eq!(self.channel_state & ChannelState::PeerDisconnected as u32, ChannelState::PeerDisconnected as u32);
25522563
msgs::ChannelReestablish {
25532564
channel_id: self.channel_id(),
2554-
next_local_commitment_number: 0xffffffffffff - self.cur_local_commitment_transaction_number,
2555-
next_remote_commitment_number: 0xffffffffffff - self.cur_remote_commitment_transaction_number,
2565+
next_local_commitment_number: INITIAL_COMMITMENT_NUMBER - self.cur_local_commitment_transaction_number,
2566+
next_remote_commitment_number: INITIAL_COMMITMENT_NUMBER - self.cur_remote_commitment_transaction_number,
25562567
data_loss_protect: None,
25572568
}
25582569
}
@@ -2910,7 +2921,7 @@ mod tests {
29102921

29112922
macro_rules! test_commitment {
29122923
( $their_sig_hex: expr, $our_sig_hex: expr, $tx_hex: expr) => {
2913-
unsigned_tx = chan.build_commitment_transaction(0xffffffffffff - 42, &keys, true, false);
2924+
unsigned_tx = chan.build_commitment_transaction(INITIAL_COMMITMENT_NUMBER - 42, &keys, true, false);
29142925
let their_signature = Signature::from_der(&secp_ctx, &hex::decode($their_sig_hex).unwrap()[..]).unwrap();
29152926
let sighash = Message::from_slice(&bip143::SighashComponents::new(&unsigned_tx.0).sighash_all(&unsigned_tx.0.input[0], &chan.get_funding_redeemscript(), chan.channel_value_satoshis)[..]).unwrap();
29162927
secp_ctx.verify(&sighash, &their_signature, &chan.their_funding_pubkey.unwrap()).unwrap();

0 commit comments

Comments
 (0)