Ensure build_commitment_tx and next_local/remote_tx_fee agree on the fee

Author: valentinewallace

lightning/src/ln/channel.rs

@@ -42,6 +42,8 @@ use std;
 use std::default::Default;
 use std::{cmp,mem,fmt};
 use std::ops::Deref;
+#[cfg(any(test, feature = "fuzztarget"))]
+use std::sync::Mutex;
 use bitcoin::hashes::hex::ToHex;
 
 #[cfg(test)]
@@ -386,6 +388,24 @@ pub(super) struct Channel<ChanSigner: ChannelKeys> {
 	commitment_secrets: CounterpartyCommitmentSecrets,
 
 	network_sync: UpdateStatus,
+
+	// We save these values so we can make sure `next_local_commit_tx_fee_msat` and
+	// `next_remote_commit_tx_fee_msat` properly predict what the next commitment transaction fee will
+	// be, by comparing the cached values to the fee of the tranaction generated by
+	// `build_commitment_transaction`.
+	#[cfg(any(test, feature = "fuzztarget"))]
+	next_local_commitment_tx_fee_info_cached: Mutex<Option<CommitmentTxInfoCached>>,
+	#[cfg(any(test, feature = "fuzztarget"))]
+	next_remote_commitment_tx_fee_info_cached: Mutex<Option<CommitmentTxInfoCached>>,
+}
+
+#[cfg(any(test, feature = "fuzztarget"))]
+struct CommitmentTxInfoCached {
+	fee: u64,
+	num_htlcs: usize,
+	next_holder_htlc_id: u64,
+	next_counterparty_htlc_id: u64,
+	feerate: u32,
 }
 
 pub const OUR_MAX_HTLCS: u16 = 50; //TODO
@@ -557,6 +577,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 			commitment_secrets: CounterpartyCommitmentSecrets::new(),
 
 			network_sync: UpdateStatus::Fresh,
+
+			#[cfg(any(test, feature = "fuzztarget"))]
+			next_local_commitment_tx_fee_info_cached: Mutex::new(None),
+			#[cfg(any(test, feature = "fuzztarget"))]
+			next_remote_commitment_tx_fee_info_cached: Mutex::new(None),
 		})
 	}
 
@@ -790,6 +815,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 			commitment_secrets: CounterpartyCommitmentSecrets::new(),
 
 			network_sync: UpdateStatus::Fresh,
+
+			#[cfg(any(test, feature = "fuzztarget"))]
+			next_local_commitment_tx_fee_info_cached: Mutex::new(None),
+			#[cfg(any(test, feature = "fuzztarget"))]
+			next_remote_commitment_tx_fee_info_cached: Mutex::new(None),
 		};
 
 		Ok(chan)
@@ -1740,7 +1770,30 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 			}
 		}
 
-		self.commit_tx_fee_msat(included_htlcs + addl_htlcs)
+		let num_htlcs = included_htlcs + addl_htlcs;
+		let res = self.commit_tx_fee_msat(num_htlcs);
+		#[cfg(any(test, feature = "fuzztarget"))]
+		{
+			let mut num_htlcs = num_htlcs;
+			let mut fee = res;
+			if fee_spike_buffer_htlc.is_some() {
+				num_htlcs -= 1;
+				fee = self.commit_tx_fee_msat(num_htlcs);
+			}
+			let commitment_tx_info = CommitmentTxInfoCached {
+				fee,
+				num_htlcs,
+				next_holder_htlc_id: if locally_offered { self.next_holder_htlc_id + 1 } else {
+					self.next_holder_htlc_id
+				},
+				next_counterparty_htlc_id: if locally_offered { self.next_counterparty_htlc_id } else {
+					self.next_counterparty_htlc_id + 1
+				},
+				feerate: self.feerate_per_kw,
+			};
+			*self.next_local_commitment_tx_fee_info_cached.lock().unwrap() = Some(commitment_tx_info);
+		}
+		res
 	}
 
 	// Get the commitment tx fee for the remote's next commitment transaction based on the number of
@@ -1781,11 +1834,35 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 			match htlc.state {
 				OutboundHTLCState::Committed => included_htlcs += 1,
 				OutboundHTLCState::RemoteRemoved {..} => included_htlcs += 1,
+				OutboundHTLCState::LocalAnnounced { .. } => included_htlcs += 1,
 				_ => {},
 			}
 		}
 
-		self.commit_tx_fee_msat(included_htlcs + addl_htlcs)
+		let num_htlcs = included_htlcs + addl_htlcs;
+		let res = self.commit_tx_fee_msat(num_htlcs);
+		#[cfg(any(test, feature = "fuzztarget"))]
+		{
+			let mut num_htlcs = num_htlcs;
+			let mut fee = res;
+			if fee_spike_buffer_htlc.is_some() {
+				num_htlcs -= 1;
+				fee = self.commit_tx_fee_msat(num_htlcs);
+			}
+			let commitment_tx_info = CommitmentTxInfoCached {
+				fee,
+				num_htlcs,
+				next_holder_htlc_id: if locally_offered { self.next_holder_htlc_id + 1 } else {
+					self.next_holder_htlc_id
+				},
+				next_counterparty_htlc_id: if locally_offered { self.next_counterparty_htlc_id } else {
+					self.next_counterparty_htlc_id + 1
+				},
+				feerate: self.feerate_per_kw,
+			};
+			*self.next_remote_commitment_tx_fee_info_cached.lock().unwrap() = Some(commitment_tx_info);
+		}
+		res
 	}
 
 	pub fn update_add_htlc<F, L: Deref>(&mut self, msg: &msgs::UpdateAddHTLC, mut pending_forward_status: PendingHTLCStatus, create_pending_htlc_status: F, logger: &L) -> Result<(), ChannelError>
@@ -2019,15 +2096,27 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 			(commitment_tx.1, htlcs_cloned, commitment_tx.0, commitment_txid)
 		};
 
+		let total_fee = feerate_per_kw as u64 * (COMMITMENT_TX_BASE_WEIGHT + (num_htlcs as u64) * COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000;
 		//If channel fee was updated by funder confirm funder can afford the new fee rate when applied to the current local commitment transaction
 		if update_fee {
-			let total_fee = feerate_per_kw as u64 * (COMMITMENT_TX_BASE_WEIGHT + (num_htlcs as u64) * COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000;
-
 			let counterparty_reserve_we_require = Channel::<ChanSigner>::get_holder_selected_channel_reserve_satoshis(self.channel_value_satoshis);
 			if self.channel_value_satoshis - self.value_to_self_msat / 1000 < total_fee + counterparty_reserve_we_require {
 				return Err((None, ChannelError::Close("Funding remote cannot afford proposed new fee".to_owned())));
 			}
 		}
+		#[cfg(any(test, feature = "fuzztarget"))]
+		{
+			if self.is_outbound() {
+				let projected_commit_tx_info = self.next_local_commitment_tx_fee_info_cached.lock().unwrap().take();
+				if let Some(info) = projected_commit_tx_info {
+					if info.num_htlcs == num_htlcs && info.next_holder_htlc_id == self.next_holder_htlc_id
+						&& info.next_counterparty_htlc_id == self.next_counterparty_htlc_id
+						&& info.feerate == self.feerate_per_kw {
+							assert_eq!(total_fee, info.fee / 1000);
+						}
+				}
+			}
+		}
 
 		if msg.htlc_signatures.len() != num_htlcs {
 			return Err((None, ChannelError::Close(format!("Got wrong number of HTLC signatures ({}) from remote. It must be {}", msg.htlc_signatures.len(), num_htlcs))));
@@ -3921,6 +4010,22 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 		let counterparty_commitment_txid = counterparty_commitment_tx.0.trust().txid();
 		let (signature, htlc_signatures);
 
+		#[cfg(any(test, feature = "fuzztarget"))]
+		{
+			if !self.is_outbound() {
+				let projected_commit_tx_info = self.next_remote_commitment_tx_fee_info_cached.lock().unwrap().take();
+				if let Some(info) = projected_commit_tx_info {
+					if info.num_htlcs == counterparty_commitment_tx.1
+						&& info.next_holder_htlc_id == self.next_holder_htlc_id
+						&& info.next_counterparty_htlc_id == self.next_counterparty_htlc_id
+						&& info.feerate == self.feerate_per_kw {
+							let actual_fee = self.commit_tx_fee_msat(counterparty_commitment_tx.1);
+							assert_eq!(actual_fee, info.fee);
+						}
+				}
+			}
+		}
+
 		{
 			let mut htlcs = Vec::with_capacity(counterparty_commitment_tx.2.len());
 			for &(ref htlc, _) in counterparty_commitment_tx.2.iter() {
@@ -4511,6 +4616,11 @@ impl<'a, ChanSigner: ChannelKeys, K: Deref> ReadableArgs<&'a K> for Channel<Chan
 			commitment_secrets,
 
 			network_sync: UpdateStatus::Fresh,
+
+			#[cfg(any(test, feature = "fuzztarget"))]
+			next_local_commitment_tx_fee_info_cached: Mutex::new(None),
+			#[cfg(any(test, feature = "fuzztarget"))]
+			next_remote_commitment_tx_fee_info_cached: Mutex::new(None),
 		})
 	}
 }