-f Dry-up feerate_previous from OnchainRequest to PackageTemplate

Author: Antoine Riard

lightning/src/chain/channelmonitor.rs

@@ -1462,8 +1462,8 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 			for (idx, outp) in tx.output.iter().enumerate() {
 				if outp.script_pubkey == revokeable_p2wsh {
 					let revk_outp = RevokedOutput::build(per_commitment_point, per_commitment_key, self.counterparty_tx_cache.counterparty_delayed_payment_base_key, self.counterparty_tx_cache.counterparty_htlc_base_key, InputDescriptors::RevokedOutput, outp.value, None, self.counterparty_tx_cache.on_counterparty_tx_csv);
-					let justice_package = PackageTemplate::build_package(commitment_txid, idx as u32, PackageSolvingData::RevokedOutput(revk_outp), PackageMalleability::Malleable, height + self.counterparty_tx_cache.on_counterparty_tx_csv as u32, true);
-					claimable_outpoints.push(OnchainRequest { feerate_previous: 0, height_timer: None, height_original: height, content: justice_package});
+					let justice_package = PackageTemplate::build_package(commitment_txid, idx as u32, PackageSolvingData::RevokedOutput(revk_outp), PackageMalleability::Malleable, height + self.counterparty_tx_cache.on_counterparty_tx_csv as u32, true, 0);
+					claimable_outpoints.push(OnchainRequest { height_timer: None, height_original: height, content: justice_package});
 				}
 			}
 
@@ -1476,8 +1476,8 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 							return (claimable_outpoints, (commitment_txid, watch_outputs)); // Corrupted per_commitment_data, fuck this user
 						}
 						let revk_outp = RevokedOutput::build(per_commitment_point, per_commitment_key, self.counterparty_tx_cache.counterparty_delayed_payment_base_key, self.counterparty_tx_cache.counterparty_htlc_base_key, if htlc.offered { InputDescriptors::RevokedOfferedHTLC } else { InputDescriptors::RevokedReceivedHTLC }, htlc.amount_msat / 1000, Some(htlc.clone()), self.counterparty_tx_cache.on_counterparty_tx_csv);
-						let justice_package = PackageTemplate::build_package(commitment_txid, transaction_output_index, PackageSolvingData::RevokedOutput(revk_outp), PackageMalleability::Malleable, htlc.cltv_expiry, true);
-						claimable_outpoints.push(OnchainRequest { feerate_previous: 0, height_timer: None, height_original: height, content: justice_package});
+						let justice_package = PackageTemplate::build_package(commitment_txid, transaction_output_index, PackageSolvingData::RevokedOutput(revk_outp), PackageMalleability::Malleable, htlc.cltv_expiry, true, 0);
+						claimable_outpoints.push(OnchainRequest { height_timer: None, height_original: height, content: justice_package});
 					}
 				}
 			}
@@ -1626,8 +1626,8 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 							let preimage = if htlc.offered { if let Some(p) = self.payment_preimages.get(&htlc.payment_hash) { Some(*p) } else { None } } else { None };
 							if preimage.is_some() || !htlc.offered {
 								let counterparty_htlc_outp = CounterpartyHTLCOutput::build(*revocation_point, self.counterparty_tx_cache.counterparty_delayed_payment_base_key, self.counterparty_tx_cache.counterparty_htlc_base_key, preimage, htlc.clone());
-								let counterparty_package = PackageTemplate::build_package(commitment_txid, transaction_output_index, PackageSolvingData::CounterpartyHTLCOutput(counterparty_htlc_outp), PackageMalleability::Malleable, htlc.cltv_expiry, if !htlc.offered { false } else { true });
-								claimable_outpoints.push(OnchainRequest { feerate_previous: 0, height_timer: None, height_original: /*XXX(ariard) Option<height> */ 0, content: counterparty_package });
+								let counterparty_package = PackageTemplate::build_package(commitment_txid, transaction_output_index, PackageSolvingData::CounterpartyHTLCOutput(counterparty_htlc_outp), PackageMalleability::Malleable, htlc.cltv_expiry, if !htlc.offered { false } else { true }, 0);
+								claimable_outpoints.push(OnchainRequest { height_timer: None, height_original: /*XXX(ariard) Option<height> */ 0, content: counterparty_package });
 							}
 						}
 					}
@@ -1659,8 +1659,8 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 
 		log_trace!(logger, "Counterparty HTLC broadcast {}:{}", htlc_txid, 0);
 		let revk_outp = RevokedOutput::build(per_commitment_point, per_commitment_key, self.counterparty_tx_cache.counterparty_delayed_payment_base_key, self.counterparty_tx_cache.counterparty_htlc_base_key, InputDescriptors::RevokedOutput, tx.output[0].value, None, self.counterparty_tx_cache.on_counterparty_tx_csv);
-		let justice_package = PackageTemplate::build_package(htlc_txid, 0, PackageSolvingData::RevokedOutput(revk_outp), PackageMalleability::Malleable, height + self.counterparty_tx_cache.on_counterparty_tx_csv as u32, true);
-		let claimable_outpoints = vec!(OnchainRequest { feerate_previous: 0, height_timer: None, height_original: height, content: justice_package });
+		let justice_package = PackageTemplate::build_package(htlc_txid, 0, PackageSolvingData::RevokedOutput(revk_outp), PackageMalleability::Malleable, height + self.counterparty_tx_cache.on_counterparty_tx_csv as u32, true, 0);
+		let claimable_outpoints = vec!(OnchainRequest { height_timer: None, height_original: height, content: justice_package });
 		let outputs = vec![(0, tx.output[0].clone())];
 		(claimable_outpoints, Some((htlc_txid, outputs)))
 	}
@@ -1684,8 +1684,8 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 						continue;
 					}
 				} else { None }, htlc.amount_msat);
-				let htlc_package = PackageTemplate::build_package(holder_tx.txid, transaction_output_index, PackageSolvingData::HolderHTLCOutput(htlc_output), PackageMalleability::Untractable, height, false);
-				claim_requests.push(OnchainRequest { feerate_previous: 0, height_timer: None, height_original: height, content: htlc_package });
+				let htlc_package = PackageTemplate::build_package(holder_tx.txid, transaction_output_index, PackageSolvingData::HolderHTLCOutput(htlc_output), PackageMalleability::Untractable, height, false, 0);
+				claim_requests.push(OnchainRequest { height_timer: None, height_original: height, content: htlc_package });
 			}
 		}
 
@@ -1893,8 +1893,8 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 		let should_broadcast = self.would_broadcast_at_height(height, &logger);
 		if should_broadcast {
 			let funding_outp = HolderFundingOutput::build(self.funding_redeemscript.clone());
-			let commitment_package = PackageTemplate::build_package(self.funding_info.0.txid.clone(), self.funding_info.0.index as u32, PackageSolvingData::HolderFundingOutput(funding_outp), PackageMalleability::Untractable, height, false);
-			claimable_outpoints.push(OnchainRequest { feerate_previous: 0, height_timer: None, height_original: height, content: commitment_package });
+			let commitment_package = PackageTemplate::build_package(self.funding_info.0.txid.clone(), self.funding_info.0.index as u32, PackageSolvingData::HolderFundingOutput(funding_outp), PackageMalleability::Untractable, height, false, 0);
+			claimable_outpoints.push(OnchainRequest { height_timer: None, height_original: height, content: commitment_package });
 		}
 		if should_broadcast {
 			self.pending_monitor_events.push(MonitorEvent::CommitmentTxBroadcasted(self.funding_info.0));

lightning/src/chain/onchain_utils.rs

@@ -488,7 +488,9 @@ pub(crate) struct PackageTemplate {
 	// of a sooner-HTLC could be swallowed by the highest nLocktime of the HTLC set.
 	// To simplify we mark them as non-aggregable.
 	aggregation: bool,
-
+	// Based feerate of previous broadcast. If resources available (either
+	// output value or utxo bumping).
+	feerate_previous: u64,
 }
 
 impl PackageTemplate {
@@ -501,6 +503,12 @@ impl PackageTemplate {
 	pub(crate) fn aggregation(&self) -> bool {
 		self.aggregation
 	}
+	pub(crate) fn get_feerate(&self) -> u64 {
+		self.feerate_previous
+	}
+	pub(crate) fn load_feerate(&mut self, new_feerate: u64) {
+		self.feerate_previous = new_feerate;
+	}
 	pub(crate) fn outpoints(&self) -> Vec<&BitcoinOutPoint> {
 		let mut outpoints = Vec::with_capacity(self.inputs.len());
 		for (o, _) in self.inputs.iter() {
@@ -514,13 +522,15 @@ impl PackageTemplate {
 				let mut split_package = None;
 				let timelock = self.absolute_timelock;
 				let aggregation = self.aggregation;
+				let feerate_previous = self.feerate_previous;
 				self.inputs.retain(|outp| {
 					if *split_outp == outp.0 {
 						split_package = Some(PackageTemplate {
 							inputs: vec![(outp.0, outp.1.clone())],
 							malleability: PackageMalleability::Malleable,
 							absolute_timelock: timelock,
 							aggregation,
+							feerate_previous,
 						});
 						return false;
 					}
@@ -557,6 +567,9 @@ impl PackageTemplate {
 		if self.absolute_timelock > merge_from.absolute_timelock {
 			self.absolute_timelock = merge_from.absolute_timelock;
 		}
+		if self.feerate_previous > merge_from.feerate_previous {
+			self.feerate_previous = merge_from.feerate_previous;
+		}
 	}
 	pub(crate) fn package_amount(&self) -> u64 {
 		let mut amounts = 0;
@@ -629,14 +642,15 @@ impl PackageTemplate {
 			},
 		}
 	}
-	pub (crate) fn build_package(txid: Txid, vout: u32, input_solving_data: PackageSolvingData, malleability: PackageMalleability, absolute_timelock: u32, aggregation: bool) -> Self {
+	pub (crate) fn build_package(txid: Txid, vout: u32, input_solving_data: PackageSolvingData, malleability: PackageMalleability, absolute_timelock: u32, aggregation: bool, feerate_previous: u64) -> Self {
 		let mut inputs = Vec::with_capacity(1);
 		inputs.push((BitcoinOutPoint { txid, vout }, input_solving_data));
 		PackageTemplate {
 			inputs,
 			malleability,
 			absolute_timelock,
-			aggregation
+			aggregation,
+			feerate_previous
 		}
 	}
 }
@@ -651,6 +665,7 @@ impl Writeable for PackageTemplate {
 		self.malleability.write(writer)?;
 		self.absolute_timelock.write(writer)?;
 		self.aggregation.write(writer)?;
+		self.feerate_previous.write(writer)?;
 		Ok(())
 	}
 }
@@ -667,11 +682,13 @@ impl Readable for PackageTemplate {
 		let malleability = Readable::read(reader)?;
 		let absolute_timelock = Readable::read(reader)?;
 		let aggregation = Readable::read(reader)?;
+		let feerate_previous = Readable::read(reader)?;
 		Ok(PackageTemplate {
 			inputs,
 			malleability,
 			absolute_timelock,
-			aggregation
+			aggregation,
+			feerate_previous
 		})
 	}
 }
@@ -695,9 +712,6 @@ impl Readable for PackageTemplate {
 /// Content embeds transactions elements to generate transaction. See PackageTemplate.
 #[derive(PartialEq, Clone)]
 pub struct OnchainRequest {
-	// Based feerate of previous broadcast. If resources available (either
-	// output value or utxo bumping).
-	pub(crate) feerate_previous: u64,
 	// At every block tick, used to check if pending claiming tx is taking too
 	// much time for confirmation and we need to bump it.
 	pub(crate) height_timer: Option<u32>,
@@ -716,7 +730,6 @@ impl OnchainRequest {
 
 impl Writeable for OnchainRequest {
 	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ::std::io::Error> {
-		self.feerate_previous.write(writer)?;
 		self.height_timer.write(writer)?;
 		self.height_original.write(writer)?;
 		self.content.write(writer)?;
@@ -727,13 +740,11 @@ impl Writeable for OnchainRequest {
 
 impl Readable for OnchainRequest {
 	fn read<R: ::std::io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
-		let feerate_previous = Readable::read(reader)?;
 		let height_timer = Readable::read(reader)?;
 		let height_original = Readable::read(reader)?;
 		let content = Readable::read(reader)?;
 
 		Ok(OnchainRequest {
-			feerate_previous,
 			height_timer,
 			height_original,
 			content

lightning/src/chain/onchaintx.rs

@@ -336,7 +336,7 @@ impl<ChannelSigner: Sign> OnchainTxHandler<ChannelSigner> {
 		let amt = cached_request.content.package_amount();
 		if cached_request.content.is_malleable() {
 			let predicted_weight = cached_request.content.package_weight(&self.destination_script);
-			if let Some((output_value, new_feerate)) = onchain_utils::compute_output_value(predicted_weight, amt, cached_request.feerate_previous, fee_estimator, logger) {
+			if let Some((output_value, new_feerate)) = onchain_utils::compute_output_value(predicted_weight, amt, cached_request.content.get_feerate(), fee_estimator, logger) {
 				assert!(new_feerate != 0);
 
 				let transaction = cached_request.content.finalize_package(self, output_value, self.destination_script.clone(), logger).unwrap();
@@ -394,7 +394,7 @@ impl<ChannelSigner: Sign> OnchainTxHandler<ChannelSigner> {
 		for mut req in preprocessed_requests {
 			if let Some((new_timer, new_feerate, tx)) = self.generate_claim_tx(height, &req, &*fee_estimator, &*logger) {
 				req.height_timer = new_timer;
-				req.feerate_previous = new_feerate;
+				req.content.load_feerate(new_feerate);
 				let txid = tx.txid();
 				for k in req.content.outpoints() {
 					log_trace!(logger, "Registering claiming request for {}:{}", k.txid, k.vout);
@@ -524,7 +524,7 @@ impl<ChannelSigner: Sign> OnchainTxHandler<ChannelSigner> {
 				broadcaster.broadcast_transaction(&bump_tx);
 				if let Some(request) = self.pending_claim_requests.get_mut(first_claim_txid) {
 					request.height_timer = new_timer;
-					request.feerate_previous = new_feerate;
+					request.content.load_feerate(new_feerate);
 				}
 			}
 		}
@@ -558,7 +558,7 @@ impl<ChannelSigner: Sign> OnchainTxHandler<ChannelSigner> {
 		for (_, request) in bump_candidates.iter_mut() {
 			if let Some((new_timer, new_feerate, bump_tx)) = self.generate_claim_tx(height, &request, &&*fee_estimator, &&*logger) {
 				request.height_timer = new_timer;
-				request.feerate_previous = new_feerate;
+				request.content.load_feerate(new_feerate);
 				broadcaster.broadcast_transaction(&bump_tx);
 			}
 		}