-f Dry-up height_timer from OnchainRequest to PackageTemplate

Author: Antoine Riard

lightning/src/chain/channelmonitor.rs

@@ -1582,8 +1582,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, 0);
-					claimable_outpoints.push(OnchainRequest { 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, None);
+					claimable_outpoints.push(OnchainRequest { height_original: height, content: justice_package});
 				}
 			}
 
@@ -1596,8 +1596,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, 0);
-						claimable_outpoints.push(OnchainRequest { 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, None);
+						claimable_outpoints.push(OnchainRequest { height_original: height, content: justice_package});
 					}
 				}
 			}
@@ -1745,8 +1745,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 }, 0);
-								claimable_outpoints.push(OnchainRequest { 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, None);
+								claimable_outpoints.push(OnchainRequest { height_original: /*XXX(ariard) Option<height> */ 0, content: counterparty_package });
 							}
 						}
 					}
@@ -1778,8 +1778,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, 0);
-		let claimable_outpoints = vec!(OnchainRequest { 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, None);
+		let claimable_outpoints = vec!(OnchainRequest { height_original: height, content: justice_package });
 		let outputs = vec![(0, tx.output[0].clone())];
 		(claimable_outpoints, Some((htlc_txid, outputs)))
 	}
@@ -1803,8 +1803,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, 0);
-				claim_requests.push(OnchainRequest { 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, None);
+				claim_requests.push(OnchainRequest { height_original: height, content: htlc_package });
 			}
 		}
 
@@ -2077,8 +2077,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, 0);
-			claimable_outpoints.push(OnchainRequest { 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, None);
+			claimable_outpoints.push(OnchainRequest { height_original: height, content: commitment_package });
 			self.pending_monitor_events.push(MonitorEvent::CommitmentTxBroadcasted(self.funding_info.0));
 			let commitment_tx = self.onchain_tx_handler.get_fully_signed_holder_tx(&self.funding_redeemscript);
 			self.holder_tx_signed = true;

lightning/src/chain/onchain_utils.rs

@@ -491,6 +491,9 @@ pub(crate) struct PackageTemplate {
 	// Based feerate of previous broadcast. If resources available (either
 	// output value or utxo bumping).
 	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.
+	height_timer: Option<u32>,
 }
 
 impl PackageTemplate {
@@ -509,6 +512,15 @@ impl PackageTemplate {
 	pub(crate) fn load_feerate(&mut self, new_feerate: u64) {
 		self.feerate_previous = new_feerate;
 	}
+	pub(crate) fn get_timer(&self) -> Option<u32> {
+		if let Some(ref timer) = self.height_timer {
+			return Some(*timer);
+		}
+		None
+	}
+	pub(crate) fn load_timer(&mut self, new_timer: Option<u32>) {
+		self.height_timer = new_timer;
+	}
 	pub(crate) fn outpoints(&self) -> Vec<&BitcoinOutPoint> {
 		let mut outpoints = Vec::with_capacity(self.inputs.len());
 		for (o, _) in self.inputs.iter() {
@@ -523,6 +535,7 @@ impl PackageTemplate {
 				let timelock = self.absolute_timelock;
 				let aggregation = self.aggregation;
 				let feerate_previous = self.feerate_previous;
+				let height_timer = self.height_timer;
 				self.inputs.retain(|outp| {
 					if *split_outp == outp.0 {
 						split_package = Some(PackageTemplate {
@@ -531,6 +544,7 @@ impl PackageTemplate {
 							absolute_timelock: timelock,
 							aggregation,
 							feerate_previous,
+							height_timer,
 						});
 						return false;
 					}
@@ -570,6 +584,7 @@ impl PackageTemplate {
 		if self.feerate_previous > merge_from.feerate_previous {
 			self.feerate_previous = merge_from.feerate_previous;
 		}
+		self.height_timer = cmp::min(self.height_timer, merge_from.height_timer);
 	}
 	pub(crate) fn package_amount(&self) -> u64 {
 		let mut amounts = 0;
@@ -642,15 +657,16 @@ impl PackageTemplate {
 			},
 		}
 	}
-	pub (crate) fn build_package(txid: Txid, vout: u32, input_solving_data: PackageSolvingData, malleability: PackageMalleability, absolute_timelock: u32, aggregation: bool, feerate_previous: u64) -> Self {
+	pub (crate) fn build_package(txid: Txid, vout: u32, input_solving_data: PackageSolvingData, malleability: PackageMalleability, absolute_timelock: u32, aggregation: bool, feerate_previous: u64, height_timer: Option<u32>) -> Self {
 		let mut inputs = Vec::with_capacity(1);
 		inputs.push((BitcoinOutPoint { txid, vout }, input_solving_data));
 		PackageTemplate {
 			inputs,
 			malleability,
 			absolute_timelock,
 			aggregation,
-			feerate_previous
+			feerate_previous,
+			height_timer,
 		}
 	}
 }
@@ -666,6 +682,7 @@ impl Writeable for PackageTemplate {
 		self.absolute_timelock.write(writer)?;
 		self.aggregation.write(writer)?;
 		self.feerate_previous.write(writer)?;
+		self.height_timer.write(writer)?;
 		Ok(())
 	}
 }
@@ -683,12 +700,14 @@ impl Readable for PackageTemplate {
 		let absolute_timelock = Readable::read(reader)?;
 		let aggregation = Readable::read(reader)?;
 		let feerate_previous = Readable::read(reader)?;
+		let height_timer = Readable::read(reader)?;
 		Ok(PackageTemplate {
 			inputs,
 			malleability,
 			absolute_timelock,
 			aggregation,
-			feerate_previous
+			feerate_previous,
+			height_timer,
 		})
 	}
 }
@@ -712,9 +731,6 @@ impl Readable for PackageTemplate {
 /// Content embeds transactions elements to generate transaction. See PackageTemplate.
 #[derive(PartialEq, Clone)]
 pub struct OnchainRequest {
-	// 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>,
 	// Tracked in case of reorg to wipe out now-superflous request.
 	pub(crate) height_original: u32,
 	// Content of request.
@@ -730,7 +746,6 @@ impl OnchainRequest {
 
 impl Writeable for OnchainRequest {
 	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ::std::io::Error> {
-		self.height_timer.write(writer)?;
 		self.height_original.write(writer)?;
 		self.content.write(writer)?;
 
@@ -740,12 +755,10 @@ impl Writeable for OnchainRequest {
 
 impl Readable for OnchainRequest {
 	fn read<R: ::std::io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
-		let height_timer = Readable::read(reader)?;
 		let height_original = Readable::read(reader)?;
 		let content = Readable::read(reader)?;
 
 		Ok(OnchainRequest {
-			height_timer,
 			height_original,
 			content
 		})

lightning/src/chain/onchaintx.rs

@@ -408,7 +408,7 @@ impl<ChannelSigner: Sign> OnchainTxHandler<ChannelSigner> {
 		// height timer expiration (i.e in how many blocks we're going to take action).
 		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.content.load_timer(new_timer);
 				req.content.load_feerate(new_feerate);
 				let txid = tx.txid();
 				for k in req.content.outpoints() {
@@ -522,7 +522,7 @@ impl<ChannelSigner: Sign> OnchainTxHandler<ChannelSigner> {
 
 		// Check if any pending claim request must be rescheduled
 		for (first_claim_txid, ref request) in self.pending_claim_requests.iter() {
-			if let Some(h) = request.height_timer {
+			if let Some(h) = request.content.get_timer() {
 				if h >= height {
 					bump_candidates.insert(*first_claim_txid, (*request).clone());
 				}
@@ -536,7 +536,7 @@ impl<ChannelSigner: Sign> OnchainTxHandler<ChannelSigner> {
 				log_trace!(logger, "Broadcast onchain {}", log_tx!(bump_tx));
 				broadcaster.broadcast_transaction(&bump_tx);
 				if let Some(request) = self.pending_claim_requests.get_mut(first_claim_txid) {
-					request.height_timer = new_timer;
+					request.content.load_timer(new_timer);
 					request.content.load_feerate(new_feerate);
 				}
 			}
@@ -598,7 +598,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.content.load_timer(new_timer);
 				request.content.load_feerate(new_feerate);
 				broadcaster.broadcast_transaction(&bump_tx);
 			}