Structurify claim request handed between detection/reaction

Antoine Riard · Antoine Riard · commit 0c89682c0b5b · 2020-02-18T12:54:19.000-05:00
diff --git a/lightning/src/ln/channelmonitor.rs b/lightning/src/ln/channelmonitor.rs
@@ -499,6 +499,28 @@ impl<R: ::std::io::Read> Readable<R> for InputMaterial {
 	}
 }
 
+/// ClaimRequest is a descriptor structure to communicate between detection
+/// and reaction module. They are generated by ChannelMonitor while parsing
+/// onchain txn leaked from a channel and handed over to OnchainTxHandler which
+/// is responsible for opportunistic aggregation, selecting and enforcing
+/// bumping logic, building and signing transactions.
+pub(crate) struct ClaimRequest {
+	// Block height before which claiming is exclusive to one party,
+	// after reaching it, claiming may be contentious.
+	pub(crate) absolute_timelock: u32,
+	// Timeout tx must have nLocktime set which means aggregating multiple
+	// ones must take the higher nLocktime among them to satisfy all of them.
+	// Sadly it has few pitfalls, a) it takes longuer to get fund back b) CLTV_DELTA
+	// of a sooner-HTLC could be swallowed by the highest nLocktime of the HTLC set.
+	// Do simplify we mark them as non-aggregable.
+	pub(crate) aggregable: bool,
+	// Basic bitcoin outpoint (txid, vout)
+	pub(crate) outpoint: BitcoinOutPoint,
+	// Following outpoint type, set of data needed to generate transaction digest
+	// and satisfy witness program.
+	pub(crate) witness_data: InputMaterial
+}
+
 /// Upon discovering of some classes of onchain tx by ChannelMonitor, we may have to take actions on it
 /// once they mature to enough confirmations (ANTI_REORG_DELAY)
 #[derive(Clone, PartialEq)]
@@ -1231,7 +1253,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 	/// HTLC-Success/HTLC-Timeout transactions.
 	/// Return updates for HTLC pending in the channel and failed automatically by the broadcast of
 	/// revoked remote commitment tx
-	fn check_spend_remote_transaction(&mut self, tx: &Transaction, height: u32) -> (HashMap<Sha256dHash, Vec<(u32, bool, BitcoinOutPoint, InputMaterial)>>, (Sha256dHash, Vec<TxOut>), Option<SpendableOutputDescriptor>) {
+	fn check_spend_remote_transaction(&mut self, tx: &Transaction, height: u32) -> (HashMap<Sha256dHash, Vec<ClaimRequest>>, (Sha256dHash, Vec<TxOut>), Option<SpendableOutputDescriptor>) {
 		// Most secp and related errors trying to create keys means we have no hope of constructing
 		// a spend transaction...so we return no transactions to broadcast
 		let mut claimable_outpoints = HashMap::new();
@@ -1286,7 +1308,8 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 			// First, process non-htlc outputs (to_local & to_remote)
 			for (idx, outp) in tx.output.iter().enumerate() {
 				if outp.script_pubkey == revokeable_p2wsh {
-					outpoints.push((height + self.our_to_self_delay as u32, true, BitcoinOutPoint { txid: commitment_txid, vout: idx as u32 }, InputMaterial::Revoked { script: revokeable_redeemscript.clone(), pubkey: Some(revocation_pubkey), key: revocation_key, is_htlc: false, amount: outp.value }));
+					let witness_data = InputMaterial::Revoked { script: revokeable_redeemscript.clone(), pubkey: Some(revocation_pubkey), key: revocation_key, is_htlc: false, amount: outp.value };
+					outpoints.push(ClaimRequest { absolute_timelock: height + self.our_to_self_delay as u32, aggregable: true, outpoint: BitcoinOutPoint { txid: commitment_txid, vout: idx as u32 }, witness_data});
 				} else if Some(&outp.script_pubkey) == local_payment_p2wpkh.as_ref() {
 					spendable_descriptor = Some(SpendableOutputDescriptor::DynamicOutputP2WPKH {
 						outpoint: BitcoinOutPoint { txid: commitment_txid, vout: idx as u32 },
@@ -1306,7 +1329,8 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 								tx.output[transaction_output_index as usize].script_pubkey != expected_script.to_v0_p2wsh() {
 							return (claimable_outpoints, (commitment_txid, watch_outputs), spendable_descriptor); // Corrupted per_commitment_data, fuck this user
 						}
-						outpoints.push((htlc.cltv_expiry, true, BitcoinOutPoint { txid: commitment_txid, vout: transaction_output_index }, InputMaterial::Revoked { script: expected_script, pubkey: Some(revocation_pubkey), key: revocation_key, is_htlc: true, amount: tx.output[transaction_output_index as usize].value }));
+						let witness_data = InputMaterial::Revoked { script: expected_script, pubkey: Some(revocation_pubkey), key: revocation_key, is_htlc: true, amount: tx.output[transaction_output_index as usize].value };
+						outpoints.push(ClaimRequest { absolute_timelock: htlc.cltv_expiry, aggregable: true, outpoint: BitcoinOutPoint { txid: commitment_txid, vout: transaction_output_index }, witness_data });
 					}
 				}
 			}
@@ -1469,7 +1493,8 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 							let preimage = if htlc.offered { if let Some(p) = self.payment_preimages.get(&htlc.payment_hash) { Some(*p) } else { None } } else { None };
 							let aggregable = if !htlc.offered { false } else { true };
 							if preimage.is_some() || !htlc.offered {
-								outpoints.push((htlc.cltv_expiry, aggregable, BitcoinOutPoint { txid: commitment_txid, vout: transaction_output_index }, InputMaterial::RemoteHTLC { script: expected_script, key: htlc_privkey, preimage, amount: htlc.amount_msat / 1000, locktime: htlc.cltv_expiry }));
+								let witness_data = InputMaterial::RemoteHTLC { script: expected_script, key: htlc_privkey, preimage, amount: htlc.amount_msat / 1000, locktime: htlc.cltv_expiry };
+								outpoints.push(ClaimRequest { absolute_timelock: htlc.cltv_expiry, aggregable, outpoint: BitcoinOutPoint { txid: commitment_txid, vout: transaction_output_index }, witness_data });
 							}
 						}
 					}
@@ -1491,7 +1516,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 	}
 
 	/// Attempts to claim a remote HTLC-Success/HTLC-Timeout's outputs using the revocation key
-	fn check_spend_remote_htlc(&mut self, tx: &Transaction, commitment_number: u64, height: u32) -> HashMap<Sha256dHash, Vec<(u32, bool, BitcoinOutPoint, InputMaterial)>> {
+	fn check_spend_remote_htlc(&mut self, tx: &Transaction, commitment_number: u64, height: u32) -> HashMap<Sha256dHash, Vec<ClaimRequest>> {
 		//TODO: send back new outputs to guarantee pending_claim_request consistency
 		if tx.input.len() != 1 || tx.output.len() != 1 || tx.input[0].witness.len() != 5 {
 			return HashMap::new()
@@ -1524,7 +1549,8 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 		let htlc_txid = tx.txid(); //TODO: This is gonna be a performance bottleneck for watchtowers!
 
 		log_trace!(self, "Remote HTLC broadcast {}:{}", htlc_txid, 0);
-		let outpoints = vec!((height + self.our_to_self_delay as u32, true, BitcoinOutPoint { txid: htlc_txid, vout: 0}, InputMaterial::Revoked { script: redeemscript, pubkey: Some(revocation_pubkey), key: revocation_key, is_htlc: false, amount: tx.output[0].value }));
+		let witness_data = InputMaterial::Revoked { script: redeemscript, pubkey: Some(revocation_pubkey), key: revocation_key, is_htlc: false, amount: tx.output[0].value };
+		let outpoints = vec!(ClaimRequest { absolute_timelock: height + self.our_to_self_delay as u32, aggregable: true, outpoint: BitcoinOutPoint { txid: htlc_txid, vout: 0}, witness_data });
 		let mut claimable_outpoints = HashMap::with_capacity(1);
 		claimable_outpoints.insert(htlc_txid, outpoints);
 		claimable_outpoints
@@ -1793,7 +1819,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 		let mut watch_outputs = Vec::new();
 		let mut spendable_outputs = Vec::new();
 		let mut htlc_updated = Vec::new();
-		let mut claimable_outpoints = HashMap::new();
+		let mut claim_requests = HashMap::new();
 		for tx in txn_matched {
 			if tx.input.len() == 1 {
 				// Assuming our keys were not leaked (in which case we're screwed no matter what),
@@ -1811,14 +1837,14 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 				};
 				if funding_txo.is_none() || (prevout.txid == funding_txo.as_ref().unwrap().0.txid && prevout.vout == funding_txo.as_ref().unwrap().0.index as u32) {
 					if (tx.input[0].sequence >> 8*3) as u8 == 0x80 && (tx.lock_time >> 8*3) as u8 == 0x20 {
-						let (mut new_outpoints, new_outputs, spendable_output) = self.check_spend_remote_transaction(&tx, height);
+						let (mut new_claim_requests, new_outputs, spendable_output) = self.check_spend_remote_transaction(&tx, height);
 						if !new_outputs.1.is_empty() {
 							watch_outputs.push(new_outputs);
 						}
 						if let Some(spendable_output) = spendable_output {
 							spendable_outputs.push(spendable_output);
 						}
-						if new_outpoints.is_empty() {
+						if new_claim_requests.is_empty() {
 							let (local_txn, mut spendable_output, new_outputs) = self.check_spend_local_transaction(&tx, height);
 							spendable_outputs.append(&mut spendable_output);
 							for tx in local_txn.iter() {
@@ -1829,23 +1855,23 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 								watch_outputs.push(new_outputs);
 							}
 						}
-						for (k, v) in new_outpoints.drain() {
-							claimable_outpoints.insert(k, v);
+						for (k, v) in new_claim_requests.drain() {
+							claim_requests.insert(k, v);
 						}
 					}
-					if !funding_txo.is_none() && claimable_outpoints.is_empty() {
+					if !funding_txo.is_none() && claim_requests.is_empty() {
 						if let Some(spendable_output) = self.check_spend_closing_transaction(&tx) {
 							spendable_outputs.push(spendable_output);
 						}
 					}
 				} else {
 					if let Some(&(commitment_number, _)) = self.remote_commitment_txn_on_chain.get(&prevout.txid) {
-						let mut new_outpoints = self.check_spend_remote_htlc(&tx, commitment_number, height);
-						for (k, v) in new_outpoints.drain() {
-							claimable_outpoints.insert(k, v);
+						let mut new_claim_requests = self.check_spend_remote_htlc(&tx, commitment_number, height);
+						for (k, v) in new_claim_requests.drain() {
+							claim_requests.insert(k, v);
 						}
-						for (k, v) in new_outpoints.drain() {
-							claimable_outpoints.insert(k, v);
+						for (k, v) in new_claim_requests.drain() {
+							claim_requests.insert(k, v);
 						}
 					}
 				}
@@ -1902,7 +1928,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 			}
 		}
 
-		let mut spendable_output = self.onchain_tx_handler.block_connected(txn_matched, claimable_outpoints, height, broadcaster, fee_estimator);
+		let mut spendable_output = self.onchain_tx_handler.block_connected(txn_matched, claim_requests, height, broadcaster, fee_estimator);
 		spendable_outputs.append(&mut spendable_output);
 
 		self.last_block_hash = block_hash.clone();
diff --git a/lightning/src/ln/onchaintx.rs b/lightning/src/ln/onchaintx.rs
@@ -13,7 +13,7 @@ use bitcoin_hashes::sha256d::Hash as Sha256dHash;
 use secp256k1::Secp256k1;
 
 use ln::msgs::DecodeError;
-use ln::channelmonitor::{ANTI_REORG_DELAY, CLTV_SHARED_CLAIM_BUFFER, InputMaterial};
+use ln::channelmonitor::{ANTI_REORG_DELAY, CLTV_SHARED_CLAIM_BUFFER, InputMaterial, ClaimRequest};
 use ln::chan_utils::HTLCType;
 use chain::chaininterface::{FeeEstimator, BroadcasterInterface, ConfirmationTarget, MIN_RELAY_FEE_SAT_PER_1000_WEIGHT};
 use chain::keysinterface::SpendableOutputDescriptor;
@@ -467,29 +467,29 @@ impl OnchainTxHandler {
 		Some((new_timer, new_feerate, bumped_tx))
 	}
 
-	pub(crate) fn block_connected(&mut self, txn_matched: &[&Transaction], claimable_outpoints: HashMap<Sha256dHash, Vec<(u32, bool, BitcoinOutPoint, InputMaterial)>>, height: u32, broadcaster: &BroadcasterInterface, fee_estimator: &FeeEstimator) -> Vec<SpendableOutputDescriptor> {
+	pub(crate) fn block_connected(&mut self, txn_matched: &[&Transaction], claim_requests: HashMap<Sha256dHash, Vec<ClaimRequest>>, height: u32, broadcaster: &BroadcasterInterface, fee_estimator: &FeeEstimator) -> Vec<SpendableOutputDescriptor> {
 		let mut new_claims = Vec::new();
 		let mut aggregated_claim = HashMap::new();
 		let mut aggregated_soonest = ::std::u32::MAX;
 		let mut new_pending_claim_requests = HashMap::with_capacity(new_claims.len());
-		let mut new_claimable_outpoints = HashMap::with_capacity(claimable_outpoints.len());
+		let mut new_claimable_outpoints = HashMap::with_capacity(claim_requests.len());
 		let mut spendable_outputs = Vec::new();
 
 		// Try to aggregate outputs if they're 1) belong to same parent tx, 2) their
 		// timelock expiration isn't imminent (<= CLTV_SHARED_CLAIM_BUFFER).
-		for (_, siblings_outpoints) in claimable_outpoints {
-			for outp in siblings_outpoints {
+		for (_, siblings_requests) in claim_requests {
+			for req in siblings_requests {
 				// Don't claim a outpoint twice that would be bad for privacy and may uselessly lock a CPFP input for a while
-				if let Some(_) = self.claimable_outpoints.get(&outp.2) { log_trace!(self, "Bouncing off outpoint {}:{}, already registered its claiming request", outp.2.txid, outp.2.vout); } else {
-					log_trace!(self, "Test if outpoint can be aggregated with expiration {} against {}", outp.0, height + CLTV_SHARED_CLAIM_BUFFER);
-					if outp.0 <= height + CLTV_SHARED_CLAIM_BUFFER || !outp.1 { // Don't aggregate if outpoint absolute timelock is soon or marked as non-aggregable
+				if let Some(_) = self.claimable_outpoints.get(&req.outpoint) { log_trace!(self, "Bouncing off outpoint {}:{}, already registered its claiming request", req.outpoint.txid, req.outpoint.vout); } else {
+					log_trace!(self, "Test if outpoint can be aggregated with expiration {} against {}", req.absolute_timelock, height + CLTV_SHARED_CLAIM_BUFFER);
+					if req.absolute_timelock <= height + CLTV_SHARED_CLAIM_BUFFER || !req.aggregable { // Don't aggregate if outpoint absolute timelock is soon or marked as non-aggregable
 						let mut single_input = HashMap::new();
-						single_input.insert(outp.2, outp.3);
-						new_claims.push((outp.0, single_input));
+						single_input.insert(req.outpoint, req.witness_data);
+						new_claims.push((req.absolute_timelock, single_input));
 					} else {
-						aggregated_claim.insert(outp.2, outp.3);
-						if outp.0 < aggregated_soonest {
-							aggregated_soonest = outp.0;
+						aggregated_claim.insert(req.outpoint, req.witness_data);
+						if req.absolute_timelock < aggregated_soonest {
+							aggregated_soonest = req.absolute_timelock;
 						}
 					}
 				}
