Replace is_htlc in InputMaterial by InputDescriptor

Antoine Riard · Antoine Riard · commit 6d8b85c14208 · 2020-03-24T00:11:38.000-04:00
As we cache more and more transaction elements in OnchainTxHandler
we should dry up completly InputMaterial until them being replaced
directly by InputDescriptor
diff --git a/lightning/src/ln/channelmonitor.rs b/lightning/src/ln/channelmonitor.rs
@@ -32,7 +32,7 @@ use ln::msgs::DecodeError;
 use ln::chan_utils;
 use ln::chan_utils::{CounterpartyCommitmentSecrets, HTLCOutputInCommitment, LocalCommitmentTransaction, HTLCType};
 use ln::channelmanager::{HTLCSource, PaymentPreimage, PaymentHash};
-use ln::onchaintx::OnchainTxHandler;
+use ln::onchaintx::{OnchainTxHandler, InputDescriptors};
 use chain::chaininterface::{ChainListener, ChainWatchInterface, BroadcasterInterface, FeeEstimator};
 use chain::transaction::OutPoint;
 use chain::keysinterface::{SpendableOutputDescriptor, ChannelKeys};
@@ -427,7 +427,7 @@ pub(crate) enum InputMaterial {
 		witness_script: Script,
 		pubkey: Option<PublicKey>,
 		key: SecretKey,
-		is_htlc: bool,
+		input_descriptor: InputDescriptors,
 		amount: u64,
 	},
 	RemoteHTLC {
@@ -449,12 +449,12 @@ pub(crate) enum InputMaterial {
 impl Writeable for InputMaterial  {
 	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ::std::io::Error> {
 		match self {
-			&InputMaterial::Revoked { ref witness_script, ref pubkey, ref key, ref is_htlc, ref amount} => {
+			&InputMaterial::Revoked { ref witness_script, ref pubkey, ref key, ref input_descriptor, ref amount} => {
 				writer.write_all(&[0; 1])?;
 				witness_script.write(writer)?;
 				pubkey.write(writer)?;
 				writer.write_all(&key[..])?;
-				is_htlc.write(writer)?;
+				input_descriptor.write(writer)?;
 				writer.write_all(&byte_utils::be64_to_array(*amount))?;
 			},
 			&InputMaterial::RemoteHTLC { ref witness_script, ref key, ref preimage, ref amount, ref locktime } => {
@@ -486,13 +486,13 @@ impl Readable for InputMaterial {
 				let witness_script = Readable::read(reader)?;
 				let pubkey = Readable::read(reader)?;
 				let key = Readable::read(reader)?;
-				let is_htlc = Readable::read(reader)?;
+				let input_descriptor = Readable::read(reader)?;
 				let amount = Readable::read(reader)?;
 				InputMaterial::Revoked {
 					witness_script,
 					pubkey,
 					key,
-					is_htlc,
+					input_descriptor,
 					amount
 				}
 			},
@@ -1471,7 +1471,7 @@ 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 {
-					let witness_data = InputMaterial::Revoked { witness_script: revokeable_redeemscript.clone(), pubkey: Some(revocation_pubkey), key: revocation_key, is_htlc: false, amount: outp.value };
+					let witness_data = InputMaterial::Revoked { witness_script: revokeable_redeemscript.clone(), pubkey: Some(revocation_pubkey), key: revocation_key, input_descriptor: InputDescriptors::RevokedOutput, amount: outp.value };
 					claimable_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});
 				}
 			}
@@ -1486,7 +1486,7 @@ 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)); // Corrupted per_commitment_data, fuck this user
 						}
-						let witness_data = InputMaterial::Revoked { witness_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 { witness_script: expected_script, pubkey: Some(revocation_pubkey), key: revocation_key, input_descriptor: if htlc.offered { InputDescriptors::RevokedOfferedHTLC } else { InputDescriptors::RevokedReceivedHTLC }, amount: tx.output[transaction_output_index as usize].value };
 						claimable_outpoints.push(ClaimRequest { absolute_timelock: htlc.cltv_expiry, aggregable: true, outpoint: BitcoinOutPoint { txid: commitment_txid, vout: transaction_output_index }, witness_data });
 					}
 				}
@@ -1671,7 +1671,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 		let redeemscript = chan_utils::get_revokeable_redeemscript(&revocation_pubkey, self.our_to_self_delay, &delayed_key);
 
 		log_trace!(self, "Remote HTLC broadcast {}:{}", htlc_txid, 0);
-		let witness_data = InputMaterial::Revoked { witness_script: redeemscript, pubkey: Some(revocation_pubkey), key: revocation_key, is_htlc: false, amount: tx.output[0].value };
+		let witness_data = InputMaterial::Revoked { witness_script: redeemscript, pubkey: Some(revocation_pubkey), key: revocation_key, input_descriptor: InputDescriptors::RevokedOutput, amount: tx.output[0].value };
 		let claimable_outpoints = vec!(ClaimRequest { absolute_timelock: height + self.our_to_self_delay as u32, aggregable: true, outpoint: BitcoinOutPoint { txid: htlc_txid, vout: 0}, witness_data });
 		(claimable_outpoints, Some((htlc_txid, tx.output.clone())))
 	}
diff --git a/lightning/src/ln/onchaintx.rs b/lightning/src/ln/onchaintx.rs
@@ -18,7 +18,7 @@ use ln::msgs::DecodeError;
 use ln::channelmonitor::{ANTI_REORG_DELAY, CLTV_SHARED_CLAIM_BUFFER, InputMaterial, ClaimRequest};
 use ln::channelmanager::{HTLCSource, PaymentPreimage};
 use ln::chan_utils;
-use ln::chan_utils::{HTLCType, TxCreationKeys, HTLCOutputInCommitment};
+use ln::chan_utils::{TxCreationKeys, HTLCOutputInCommitment};
 use chain::chaininterface::{FeeEstimator, BroadcasterInterface, ConfirmationTarget, MIN_RELAY_FEE_SAT_PER_1000_WEIGHT};
 use chain::keysinterface::ChannelKeys;
 use util::logger::Logger;
@@ -111,15 +111,62 @@ impl Readable for ClaimTxBumpMaterial {
 	}
 }
 
-#[derive(PartialEq)]
-pub(super) enum InputDescriptors {
+#[derive(PartialEq, Clone, Copy)]
+pub(crate) enum InputDescriptors {
 	RevokedOfferedHTLC,
 	RevokedReceivedHTLC,
 	OfferedHTLC,
 	ReceivedHTLC,
 	RevokedOutput, // either a revoked to_local output on commitment tx, a revoked HTLC-Timeout output or a revoked HTLC-Success output
 }
 
+impl Writeable for InputDescriptors {
+	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ::std::io::Error> {
+		match self {
+			&InputDescriptors::RevokedOfferedHTLC => {
+				writer.write_all(&[0; 1])?;
+			},
+			&InputDescriptors::RevokedReceivedHTLC => {
+				writer.write_all(&[1; 1])?;
+			},
+			&InputDescriptors::OfferedHTLC => {
+				writer.write_all(&[2; 1])?;
+			},
+			&InputDescriptors::ReceivedHTLC => {
+				writer.write_all(&[3; 1])?;
+			}
+			&InputDescriptors::RevokedOutput => {
+				writer.write_all(&[4; 1])?;
+			}
+		}
+		Ok(())
+	}
+}
+
+impl Readable for InputDescriptors {
+	fn read<R: ::std::io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
+		let input_descriptor = match <u8 as Readable>::read(reader)? {
+			0 => {
+				InputDescriptors::RevokedOfferedHTLC
+			},
+			1 => {
+				InputDescriptors::RevokedReceivedHTLC
+			},
+			2 => {
+				InputDescriptors::OfferedHTLC
+			},
+			3 => {
+				InputDescriptors::ReceivedHTLC
+			},
+			4 => {
+				InputDescriptors::RevokedOutput
+			}
+			_ => return Err(DecodeError::InvalidValue),
+		};
+		Ok(input_descriptor)
+	}
+}
+
 macro_rules! subtract_high_prio_fee {
 	($self: ident, $fee_estimator: expr, $value: expr, $predicted_weight: expr, $used_feerate: expr) => {
 		{
@@ -579,8 +626,8 @@ impl<ChanSigner: ChannelKeys> OnchainTxHandler<ChanSigner> {
 		let mut dynamic_fee = true;
 		for per_outp_material in cached_claim_datas.per_input_material.values() {
 			match per_outp_material {
-				&InputMaterial::Revoked { ref witness_script, ref is_htlc, ref amount, .. } => {
-					inputs_witnesses_weight += Self::get_witnesses_weight(if !is_htlc { &[InputDescriptors::RevokedOutput] } else if HTLCType::scriptlen_to_htlctype(witness_script.len()) == Some(HTLCType::OfferedHTLC) { &[InputDescriptors::RevokedOfferedHTLC] } else if HTLCType::scriptlen_to_htlctype(witness_script.len()) == Some(HTLCType::AcceptedHTLC) { &[InputDescriptors::RevokedReceivedHTLC] } else { unreachable!() });
+				&InputMaterial::Revoked { ref input_descriptor, ref amount, .. } => {
+					inputs_witnesses_weight += Self::get_witnesses_weight(&[*input_descriptor]);
 					amt += *amount;
 				},
 				&InputMaterial::RemoteHTLC { ref preimage, ref amount, .. } => {
@@ -618,19 +665,19 @@ impl<ChanSigner: ChannelKeys> OnchainTxHandler<ChanSigner> {
 
 			for (i, (outp, per_outp_material)) in cached_claim_datas.per_input_material.iter().enumerate() {
 				match per_outp_material {
-					&InputMaterial::Revoked { ref witness_script, ref pubkey, ref key, ref is_htlc, ref amount } => {
+					&InputMaterial::Revoked { ref witness_script, ref pubkey, ref key, ref input_descriptor, ref amount } => {
 						let sighash_parts = bip143::SighashComponents::new(&bumped_tx);
 						let sighash = hash_to_message!(&sighash_parts.sighash_all(&bumped_tx.input[i], &witness_script, *amount)[..]);
 						let sig = self.secp_ctx.sign(&sighash, &key);
 						bumped_tx.input[i].witness.push(sig.serialize_der().to_vec());
 						bumped_tx.input[i].witness[0].push(SigHashType::All as u8);
-						if *is_htlc {
+						if *input_descriptor !=  InputDescriptors::RevokedOutput {
 							bumped_tx.input[i].witness.push(pubkey.unwrap().clone().serialize().to_vec());
 						} else {
 							bumped_tx.input[i].witness.push(vec!(1));
 						}
 						bumped_tx.input[i].witness.push(witness_script.clone().into_bytes());
-						log_trace!(self, "Going to broadcast Penalty Transaction {} claiming revoked {} output {} from {} with new feerate {}...", bumped_tx.txid(), if !is_htlc { "to_local" } else if HTLCType::scriptlen_to_htlctype(witness_script.len()) == Some(HTLCType::OfferedHTLC) { "offered" } else if HTLCType::scriptlen_to_htlctype(witness_script.len()) == Some(HTLCType::AcceptedHTLC) { "received" } else { "" }, outp.vout, outp.txid, new_feerate);
+						log_trace!(self, "Going to broadcast Penalty Transaction {} claiming revoked {} output {} from {} with new feerate {}...", bumped_tx.txid(), if *input_descriptor == InputDescriptors::RevokedOutput { "to_local" } else if *input_descriptor == InputDescriptors::RevokedOfferedHTLC { "offered" } else if *input_descriptor == InputDescriptors::RevokedReceivedHTLC { "received" } else { "" }, outp.vout, outp.txid, new_feerate);
 					},
 					&InputMaterial::RemoteHTLC { ref witness_script, ref key, ref preimage, ref amount, ref locktime } => {
 						if !preimage.is_some() { bumped_tx.lock_time = *locktime }; // Right now we don't aggregate time-locked transaction, if we do we should set lock_time before to avoid breaking hash computation
