diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index e637f44d3cf..9df76b1c099 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -2525,7 +2525,7 @@ where // Our commitment numbers start at 2^48-1 and count down, whereas the ones used in transaction // generation start at 0 and count up...this simplifies some parts of implementation at the // cost of others, but should really just be changed. - cur_counterparty_commitment_transaction_number: u64, + counterparty_next_commitment_transaction_number: u64, pending_inbound_htlcs: Vec, pending_outbound_htlcs: Vec, holding_cell_htlc_updates: Vec, @@ -2669,8 +2669,8 @@ where is_manual_broadcast: bool, is_batch_funding: Option<()>, - counterparty_cur_commitment_point: Option, - counterparty_prev_commitment_point: Option, + counterparty_next_commitment_point: Option, + counterparty_current_commitment_point: Option, counterparty_node_id: PublicKey, counterparty_shutdown_scriptpubkey: Option, @@ -2820,8 +2820,8 @@ where }; let context = self.context(); let commitment_data = context.build_commitment_transaction(self.funding(), - context.cur_counterparty_commitment_transaction_number, - &context.counterparty_cur_commitment_point.unwrap(), false, false, logger); + context.counterparty_next_commitment_transaction_number, + &context.counterparty_next_commitment_point.unwrap(), false, false, logger); let counterparty_initial_commitment_tx = commitment_data.tx; let counterparty_trusted_tx = counterparty_initial_commitment_tx.trust(); let counterparty_initial_bitcoin_tx = counterparty_trusted_tx.built_transaction(); @@ -2881,7 +2881,7 @@ where counterparty_initial_commitment_tx.clone(), ); - self.context_mut().cur_counterparty_commitment_transaction_number -= 1; + self.context_mut().counterparty_next_commitment_transaction_number -= 1; Ok((channel_monitor, counterparty_initial_commitment_tx)) } @@ -3246,7 +3246,7 @@ where shutdown_scriptpubkey, destination_script, - cur_counterparty_commitment_transaction_number: INITIAL_COMMITMENT_NUMBER, + counterparty_next_commitment_transaction_number: INITIAL_COMMITMENT_NUMBER, pending_inbound_htlcs: Vec::new(), pending_outbound_htlcs: Vec::new(), @@ -3297,8 +3297,8 @@ where is_batch_funding: None, - counterparty_cur_commitment_point: Some(open_channel_fields.first_per_commitment_point), - counterparty_prev_commitment_point: None, + counterparty_next_commitment_point: Some(open_channel_fields.first_per_commitment_point), + counterparty_current_commitment_point: None, counterparty_node_id, counterparty_shutdown_scriptpubkey, @@ -3482,7 +3482,7 @@ where shutdown_scriptpubkey, destination_script, - cur_counterparty_commitment_transaction_number: INITIAL_COMMITMENT_NUMBER, + counterparty_next_commitment_transaction_number: INITIAL_COMMITMENT_NUMBER, pending_inbound_htlcs: Vec::new(), pending_outbound_htlcs: Vec::new(), @@ -3535,8 +3535,8 @@ where is_batch_funding: None, - counterparty_cur_commitment_point: None, - counterparty_prev_commitment_point: None, + counterparty_next_commitment_point: None, + counterparty_current_commitment_point: None, counterparty_node_id, counterparty_shutdown_scriptpubkey: None, @@ -3915,7 +3915,7 @@ where pubkeys: counterparty_pubkeys, }); - self.counterparty_cur_commitment_point = Some(common_fields.first_per_commitment_point); + self.counterparty_next_commitment_point = Some(common_fields.first_per_commitment_point); self.counterparty_shutdown_scriptpubkey = counterparty_shutdown_scriptpubkey; self.channel_state = ChannelState::NegotiatingFunding( @@ -5558,7 +5558,7 @@ where if is_splice { debug_assert_eq!( holder_commitment_transaction_number, - self.cur_counterparty_commitment_transaction_number, + self.counterparty_next_commitment_transaction_number, ); // TODO(splicing) Forced error, as the use case is not complete return Err(msgs::TxAbort { @@ -5590,7 +5590,7 @@ where &self, holder_commitment_transaction_number: u64, msg_name: &str, ) { if self.commitment_secrets.get_min_seen_secret() != (1 << 48) - || self.cur_counterparty_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER + || self.counterparty_next_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER || holder_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER { debug_assert!( @@ -5608,13 +5608,13 @@ where SP::Target: SignerProvider, L::Target: Logger, { - let mut commitment_number = self.cur_counterparty_commitment_transaction_number; - let mut commitment_point = self.counterparty_cur_commitment_point.unwrap(); + let mut commitment_number = self.counterparty_next_commitment_transaction_number; + let mut commitment_point = self.counterparty_next_commitment_point.unwrap(); // Use the previous commitment number and point when splicing since they shouldn't change. if commitment_number != INITIAL_COMMITMENT_NUMBER { commitment_number += 1; - commitment_point = self.counterparty_prev_commitment_point.unwrap(); + commitment_point = self.counterparty_current_commitment_point.unwrap(); } let commitment_data = self.build_commitment_transaction( @@ -5682,13 +5682,13 @@ where #[cfg(all(test))] pub fn get_initial_counterparty_commitment_signature_for_test( &mut self, funding: &mut FundingScope, logger: &L, - counterparty_cur_commitment_point_override: PublicKey, + counterparty_next_commitment_point_override: PublicKey, ) -> Option where SP::Target: SignerProvider, L::Target: Logger, { - self.counterparty_cur_commitment_point = Some(counterparty_cur_commitment_point_override); + self.counterparty_next_commitment_point = Some(counterparty_next_commitment_point_override); self.get_initial_counterparty_commitment_signature(funding, logger) } @@ -6776,15 +6776,15 @@ where // They probably disconnected/reconnected and re-sent the channel_ready, which is // required, or they're sending a fresh SCID alias. let expected_point = - if self.context.cur_counterparty_commitment_transaction_number == INITIAL_COMMITMENT_NUMBER - 1 { + if self.context.counterparty_next_commitment_transaction_number == INITIAL_COMMITMENT_NUMBER - 1 { // If they haven't ever sent an updated point, the point they send should match - // the current one. - self.context.counterparty_cur_commitment_point - } else if self.context.cur_counterparty_commitment_transaction_number == INITIAL_COMMITMENT_NUMBER - 2 { + // the next one. + self.context.counterparty_next_commitment_point + } else if self.context.counterparty_next_commitment_transaction_number == INITIAL_COMMITMENT_NUMBER - 2 { // If we've advanced the commitment number once, the second commitment point is - // at `counterparty_prev_commitment_point`, which is not yet revoked. - debug_assert!(self.context.counterparty_prev_commitment_point.is_some()); - self.context.counterparty_prev_commitment_point + // at `counterparty_current_commitment_point`, which is not yet revoked. + debug_assert!(self.context.counterparty_current_commitment_point.is_some()); + self.context.counterparty_current_commitment_point } else { // If they have sent updated points, channel_ready is always supposed to match // their "first" point, which we re-derive here. @@ -6798,8 +6798,8 @@ where return Ok(None); } - self.context.counterparty_prev_commitment_point = self.context.counterparty_cur_commitment_point; - self.context.counterparty_cur_commitment_point = Some(msg.next_per_commitment_point); + self.context.counterparty_current_commitment_point = self.context.counterparty_next_commitment_point; + self.context.counterparty_next_commitment_point = Some(msg.next_per_commitment_point); // Clear any interactive signing session. self.interactive_tx_signing_session = None; @@ -7027,8 +7027,8 @@ where .context .build_commitment_transaction( pending_splice_funding, - self.context.cur_counterparty_commitment_transaction_number + 1, - &self.context.counterparty_prev_commitment_point.unwrap(), + self.context.counterparty_next_commitment_transaction_number + 1, + &self.context.counterparty_current_commitment_point.unwrap(), false, false, logger, @@ -7615,11 +7615,11 @@ where "Peer provided an invalid per_commitment_secret".to_owned() ); - if let Some(counterparty_prev_commitment_point) = - self.context.counterparty_prev_commitment_point + if let Some(counterparty_current_commitment_point) = + self.context.counterparty_current_commitment_point { if PublicKey::from_secret_key(&self.context.secp_ctx, &secret) - != counterparty_prev_commitment_point + != counterparty_current_commitment_point { return Err(ChannelError::close("Got a revoke commitment secret which didn't correspond to their current pubkey".to_owned())); } @@ -7650,7 +7650,7 @@ where ChannelSignerType::Ecdsa(ecdsa) => { ecdsa .validate_counterparty_revocation( - self.context.cur_counterparty_commitment_transaction_number + 1, + self.context.counterparty_next_commitment_transaction_number + 1, &secret, ) .map_err(|_| { @@ -7665,7 +7665,7 @@ where self.context .commitment_secrets .provide_secret( - self.context.cur_counterparty_commitment_transaction_number + 1, + self.context.counterparty_next_commitment_transaction_number + 1, msg.per_commitment_secret, ) .map_err(|_| { @@ -7675,7 +7675,7 @@ where let mut monitor_update = ChannelMonitorUpdate { update_id: self.context.latest_monitor_update_id, updates: vec![ChannelMonitorUpdateStep::CommitmentSecret { - idx: self.context.cur_counterparty_commitment_transaction_number + 1, + idx: self.context.counterparty_next_commitment_transaction_number + 1, secret: msg.per_commitment_secret, }], channel_id: Some(self.context.channel_id()), @@ -7687,10 +7687,10 @@ where // channel based on that, but stepping stuff here should be safe either way. self.context.channel_state.clear_awaiting_remote_revoke(); self.mark_response_received(); - self.context.counterparty_prev_commitment_point = - self.context.counterparty_cur_commitment_point; - self.context.counterparty_cur_commitment_point = Some(msg.next_per_commitment_point); - self.context.cur_counterparty_commitment_transaction_number -= 1; + self.context.counterparty_current_commitment_point = + self.context.counterparty_next_commitment_point; + self.context.counterparty_next_commitment_point = Some(msg.next_per_commitment_point); + self.context.counterparty_next_commitment_transaction_number -= 1; if self.context.announcement_sigs_state == AnnouncementSigsState::Committed { self.context.announcement_sigs_state = AnnouncementSigsState::PeerReceived; @@ -8429,8 +8429,11 @@ where } let funding_signed = if self.context.signer_pending_funding && !self.funding.is_outbound() { let commitment_data = self.context.build_commitment_transaction(&self.funding, - self.context.cur_counterparty_commitment_transaction_number + 1, - &self.context.counterparty_cur_commitment_point.unwrap(), false, false, logger); + // The previous transaction number (i.e., when adding 1) is used because this field + // is advanced when handling funding_created, but the point is not advanced until + // handling channel_ready. + self.context.counterparty_next_commitment_transaction_number + 1, + &self.context.counterparty_next_commitment_point.unwrap(), false, false, logger); let counterparty_initial_commitment_tx = commitment_data.tx; self.context.get_funding_signed_msg(&self.funding.channel_transaction_parameters, logger, counterparty_initial_commitment_tx) } else { None }; @@ -8795,12 +8798,12 @@ where ))); }; - // We increment cur_counterparty_commitment_transaction_number only upon receipt of + // We increment counterparty_next_commitment_transaction_number only upon receipt of // revoke_and_ack, not on sending commitment_signed, so we add one if have // AwaitingRemoteRevoke set, which indicates we sent a commitment_signed but haven't gotten // the corresponding revoke_and_ack back yet. let is_awaiting_remote_revoke = self.context.channel_state.is_awaiting_remote_revoke(); - let next_counterparty_commitment_number = INITIAL_COMMITMENT_NUMBER - self.context.cur_counterparty_commitment_transaction_number + if is_awaiting_remote_revoke { 1 } else { 0 }; + let next_counterparty_commitment_number = INITIAL_COMMITMENT_NUMBER - self.context.counterparty_next_commitment_transaction_number + if is_awaiting_remote_revoke { 1 } else { 0 }; let channel_ready = if msg.next_local_commitment_number == 1 && INITIAL_COMMITMENT_NUMBER - self.holder_commitment_point.next_transaction_number() == 1 { // We should never have to worry about MonitorUpdateInProgress resending ChannelReady @@ -9631,12 +9634,12 @@ where } pub fn get_cur_counterparty_commitment_transaction_number(&self) -> u64 { - self.context.cur_counterparty_commitment_transaction_number + 1 + self.context.counterparty_next_commitment_transaction_number + 1 - if self.context.channel_state.is_awaiting_remote_revoke() { 1 } else { 0 } } pub fn get_revoked_counterparty_commitment_transaction_number(&self) -> u64 { - let ret = self.context.cur_counterparty_commitment_transaction_number + 2; + let ret = self.context.counterparty_next_commitment_transaction_number + 2; debug_assert_eq!(self.context.commitment_secrets.get_min_seen_secret(), ret); ret } @@ -9754,7 +9757,7 @@ where return true; } if self.holder_commitment_point.next_transaction_number() == INITIAL_COMMITMENT_NUMBER - 1 && - self.context.cur_counterparty_commitment_transaction_number == INITIAL_COMMITMENT_NUMBER - 1 { + self.context.counterparty_next_commitment_transaction_number == INITIAL_COMMITMENT_NUMBER - 1 { // If we're a 0-conf channel, we'll move beyond AwaitingChannelReady immediately even while // waiting for the initial monitor persistence. Thus, we check if our commitment // transaction numbers have both been iterated only exactly once (for the @@ -10541,7 +10544,7 @@ where #[rustfmt::skip] fn get_channel_reestablish(&mut self, logger: &L) -> msgs::ChannelReestablish where L::Target: Logger { assert!(self.context.channel_state.is_peer_disconnected()); - assert_ne!(self.context.cur_counterparty_commitment_transaction_number, INITIAL_COMMITMENT_NUMBER); + assert_ne!(self.context.counterparty_next_commitment_transaction_number, INITIAL_COMMITMENT_NUMBER); // This is generally the first function which gets called on any given channel once we're // up and running normally. Thus, we take this opportunity to attempt to resolve the // `holder_commitment_point` to get any keys which we are currently missing. @@ -10556,8 +10559,8 @@ where // valid, and valid in fuzzing mode's arbitrary validity criteria: let mut pk = [2; 33]; pk[1] = 0xff; let dummy_pubkey = PublicKey::from_slice(&pk).unwrap(); - let remote_last_secret = if self.context.cur_counterparty_commitment_transaction_number + 1 < INITIAL_COMMITMENT_NUMBER { - let remote_last_secret = self.context.commitment_secrets.get_secret(self.context.cur_counterparty_commitment_transaction_number + 2).unwrap(); + let remote_last_secret = if self.context.counterparty_next_commitment_transaction_number + 1 < INITIAL_COMMITMENT_NUMBER { + let remote_last_secret = self.context.commitment_secrets.get_secret(self.context.counterparty_next_commitment_transaction_number + 2).unwrap(); log_trace!(logger, "Enough info to generate a Data Loss Protect with per_commitment_secret {} for channel {}", log_bytes!(remote_last_secret), &self.context.channel_id()); remote_last_secret } else { @@ -10580,10 +10583,10 @@ where // receive, however we track it by the next commitment number for a remote transaction // (which is one further, as they always revoke previous commitment transaction, not // the one we send) so we have to decrement by 1. Note that if - // cur_counterparty_commitment_transaction_number is INITIAL_COMMITMENT_NUMBER we will have + // counterparty_next_commitment_transaction_number is INITIAL_COMMITMENT_NUMBER we will have // dropped this channel on disconnect as it hasn't yet reached AwaitingChannelReady so we can't // overflow here. - next_remote_commitment_number: INITIAL_COMMITMENT_NUMBER - self.context.cur_counterparty_commitment_transaction_number - 1, + next_remote_commitment_number: INITIAL_COMMITMENT_NUMBER - self.context.counterparty_next_commitment_transaction_number - 1, your_last_per_commitment_secret: remote_last_secret, my_current_per_commitment_point: dummy_pubkey, next_funding_txid: self.maybe_get_next_funding_txid(), @@ -11202,8 +11205,8 @@ where ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { commitment_txid: counterparty_commitment_tx.trust().txid(), htlc_outputs, - commitment_number: self.context.cur_counterparty_commitment_transaction_number, - their_per_commitment_point: self.context.counterparty_cur_commitment_point.unwrap(), + commitment_number: self.context.counterparty_next_commitment_transaction_number, + their_per_commitment_point: self.context.counterparty_next_commitment_point.unwrap(), feerate_per_kw: Some(counterparty_commitment_tx.feerate_per_kw()), to_broadcaster_value_sat: Some(counterparty_commitment_tx.to_broadcaster_value_sat()), to_countersignatory_value_sat: Some(counterparty_commitment_tx.to_countersignatory_value_sat()), @@ -11259,8 +11262,8 @@ where L::Target: Logger, { let commitment_data = self.context.build_commitment_transaction( - funding, self.context.cur_counterparty_commitment_transaction_number, - &self.context.counterparty_cur_commitment_point.unwrap(), false, true, logger, + funding, self.context.counterparty_next_commitment_transaction_number, + &self.context.counterparty_next_commitment_point.unwrap(), false, true, logger, ); let counterparty_commitment_tx = commitment_data.tx; @@ -11310,8 +11313,8 @@ where self.build_commitment_no_state_update(funding, logger); let commitment_data = self.context.build_commitment_transaction( - funding, self.context.cur_counterparty_commitment_transaction_number, - &self.context.counterparty_cur_commitment_point.unwrap(), false, true, logger, + funding, self.context.counterparty_next_commitment_transaction_number, + &self.context.counterparty_next_commitment_point.unwrap(), false, true, logger, ); let counterparty_commitment_tx = commitment_data.tx; @@ -11879,8 +11882,8 @@ where #[rustfmt::skip] fn get_funding_created_msg(&mut self, logger: &L) -> Option where L::Target: Logger { let commitment_data = self.context.build_commitment_transaction(&self.funding, - self.context.cur_counterparty_commitment_transaction_number, - &self.context.counterparty_cur_commitment_point.unwrap(), false, false, logger); + self.context.counterparty_next_commitment_transaction_number, + &self.context.counterparty_next_commitment_point.unwrap(), false, false, logger); let counterparty_initial_commitment_tx = commitment_data.tx; let signature = match &self.context.holder_signer { // TODO (taproot|arik): move match into calling method for Taproot @@ -12910,7 +12913,7 @@ where self.context.destination_script.write(writer)?; self.holder_commitment_point.next_transaction_number().write(writer)?; - self.context.cur_counterparty_commitment_transaction_number.write(writer)?; + self.context.counterparty_next_commitment_transaction_number.write(writer)?; self.funding.value_to_self_msat.write(writer)?; let mut dropped_inbound_htlcs = 0; @@ -13167,8 +13170,8 @@ where self.funding.channel_transaction_parameters.write(writer)?; self.funding.funding_transaction.write(writer)?; - self.context.counterparty_cur_commitment_point.write(writer)?; - self.context.counterparty_prev_commitment_point.write(writer)?; + self.context.counterparty_next_commitment_point.write(writer)?; + self.context.counterparty_current_commitment_point.write(writer)?; self.context.counterparty_node_id.write(writer)?; self.context.counterparty_shutdown_scriptpubkey.write(writer)?; @@ -13341,7 +13344,7 @@ where let destination_script = Readable::read(reader)?; let holder_commitment_next_transaction_number = Readable::read(reader)?; - let cur_counterparty_commitment_transaction_number = Readable::read(reader)?; + let counterparty_next_commitment_transaction_number = Readable::read(reader)?; let value_to_self_msat = Readable::read(reader)?; let pending_inbound_htlc_count: u64 = Readable::read(reader)?; @@ -13568,9 +13571,9 @@ where ReadableArgs::>::read(reader, Some(channel_value_satoshis))?; let funding_transaction: Option = Readable::read(reader)?; - let counterparty_cur_commitment_point = Readable::read(reader)?; + let counterparty_next_commitment_point = Readable::read(reader)?; - let counterparty_prev_commitment_point = Readable::read(reader)?; + let counterparty_current_commitment_point = Readable::read(reader)?; let counterparty_node_id = Readable::read(reader)?; let counterparty_shutdown_scriptpubkey = Readable::read(reader)?; @@ -13973,7 +13976,7 @@ where shutdown_scriptpubkey, destination_script, - cur_counterparty_commitment_transaction_number, + counterparty_next_commitment_transaction_number, holder_max_accepted_htlcs, pending_inbound_htlcs, @@ -14025,8 +14028,8 @@ where is_batch_funding, - counterparty_cur_commitment_point, - counterparty_prev_commitment_point, + counterparty_next_commitment_point, + counterparty_current_commitment_point, counterparty_node_id, counterparty_shutdown_scriptpubkey,