]> git.bitcoin.ninja Git - rust-lightning/commitdiff
Duplicate RemoteTxCache in ChannelMonitor
authorAntoine Riard <ariard@student.42.fr>
Mon, 27 Apr 2020 23:22:10 +0000 (19:22 -0400)
committerAntoine Riard <ariard@student.42.fr>
Tue, 12 May 2020 21:41:33 +0000 (17:41 -0400)
Dry-up remote pubkeys tracking in one struct.

This introduce a duplicate of RemoteTxCache, which is going
to be removed in next commit when OnchainTxHandler version is
removed.

lightning/src/chain/keysinterface.rs
lightning/src/ln/channelmonitor.rs
lightning/src/ln/onchaintx.rs

index 366e78b57092a28e251ec34fc5218bb8750029fb..8cfa2aa7028b5dcdec5d8ba88931ee1e3121dcfb 100644 (file)
@@ -2,7 +2,7 @@
 //! spendable on-chain outputs which the user owns and is responsible for using just as any other
 //! on-chain output which is theirs.
 
-use bitcoin::blockdata::transaction::{Transaction, OutPoint, TxOut, SigHashType};
+use bitcoin::blockdata::transaction::{Transaction, OutPoint, TxOut};
 use bitcoin::blockdata::script::{Script, Builder};
 use bitcoin::blockdata::opcodes;
 use bitcoin::network::constants::Network;
index fae8534dde534a5dbc51fa6d305f4485ef894ff3..1cbea01b19026ec2dc7fac1330f75f1ec7cb4cba 100644 (file)
@@ -31,7 +31,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, InputDescriptors};
+use ln::onchaintx::{OnchainTxHandler, InputDescriptors, RemoteTxCache};
 use chain::chaininterface::{ChainListener, ChainWatchInterface, BroadcasterInterface, FeeEstimator};
 use chain::transaction::OutPoint;
 use chain::keysinterface::{SpendableOutputDescriptor, ChannelKeys};
@@ -425,15 +425,19 @@ struct LocalSignedTx {
 pub(crate) enum InputMaterial {
        Revoked {
                per_commitment_point: PublicKey,
+               remote_delayed_payment_base_key: PublicKey,
+               remote_htlc_base_key: PublicKey,
                per_commitment_key: SecretKey,
                input_descriptor: InputDescriptors,
                amount: u64,
+               htlc: Option<HTLCOutputInCommitment>
        },
        RemoteHTLC {
                per_commitment_point: PublicKey,
+               remote_delayed_payment_base_key: PublicKey,
+               remote_htlc_base_key: PublicKey,
                preimage: Option<PaymentPreimage>,
-               amount: u64,
-               locktime: u32,
+               htlc: HTLCOutputInCommitment
        },
        LocalHTLC {
                preimage: Option<PaymentPreimage>,
@@ -447,19 +451,23 @@ 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 per_commitment_point, ref per_commitment_key, ref input_descriptor, ref amount} => {
+                       &InputMaterial::Revoked { ref per_commitment_point, ref remote_delayed_payment_base_key, ref remote_htlc_base_key, ref per_commitment_key, ref input_descriptor, ref amount, ref htlc} => {
                                writer.write_all(&[0; 1])?;
                                per_commitment_point.write(writer)?;
+                               remote_delayed_payment_base_key.write(writer)?;
+                               remote_htlc_base_key.write(writer)?;
                                writer.write_all(&per_commitment_key[..])?;
                                input_descriptor.write(writer)?;
                                writer.write_all(&byte_utils::be64_to_array(*amount))?;
+                               htlc.write(writer)?;
                        },
-                       &InputMaterial::RemoteHTLC { ref per_commitment_point, ref preimage, ref amount, ref locktime } => {
+                       &InputMaterial::RemoteHTLC { ref per_commitment_point, ref remote_delayed_payment_base_key, ref remote_htlc_base_key, ref preimage, ref htlc} => {
                                writer.write_all(&[1; 1])?;
                                per_commitment_point.write(writer)?;
+                               remote_delayed_payment_base_key.write(writer)?;
+                               remote_htlc_base_key.write(writer)?;
                                preimage.write(writer)?;
-                               writer.write_all(&byte_utils::be64_to_array(*amount))?;
-                               writer.write_all(&byte_utils::be32_to_array(*locktime))?;
+                               htlc.write(writer)?;
                        },
                        &InputMaterial::LocalHTLC { ref preimage, ref amount } => {
                                writer.write_all(&[2; 1])?;
@@ -480,26 +488,34 @@ impl Readable for InputMaterial {
                let input_material = match <u8 as Readable>::read(reader)? {
                        0 => {
                                let per_commitment_point = Readable::read(reader)?;
+                               let remote_delayed_payment_base_key = Readable::read(reader)?;
+                               let remote_htlc_base_key = Readable::read(reader)?;
                                let per_commitment_key = Readable::read(reader)?;
                                let input_descriptor = Readable::read(reader)?;
                                let amount = Readable::read(reader)?;
+                               let htlc = Readable::read(reader)?;
                                InputMaterial::Revoked {
                                        per_commitment_point,
+                                       remote_delayed_payment_base_key,
+                                       remote_htlc_base_key,
                                        per_commitment_key,
                                        input_descriptor,
-                                       amount
+                                       amount,
+                                       htlc
                                }
                        },
                        1 => {
                                let per_commitment_point = Readable::read(reader)?;
+                               let remote_delayed_payment_base_key = Readable::read(reader)?;
+                               let remote_htlc_base_key = Readable::read(reader)?;
                                let preimage = Readable::read(reader)?;
-                               let amount = Readable::read(reader)?;
-                               let locktime = Readable::read(reader)?;
+                               let htlc = Readable::read(reader)?;
                                InputMaterial::RemoteHTLC {
                                        per_commitment_point,
+                                       remote_delayed_payment_base_key,
+                                       remote_htlc_base_key,
                                        preimage,
-                                       amount,
-                                       locktime
+                                       htlc
                                }
                        },
                        2 => {
@@ -707,8 +723,7 @@ pub struct ChannelMonitor<ChanSigner: ChannelKeys> {
        current_remote_commitment_txid: Option<Txid>,
        prev_remote_commitment_txid: Option<Txid>,
 
-       their_htlc_base_key: PublicKey,
-       their_delayed_payment_base_key: PublicKey,
+       remote_tx_cache: RemoteTxCache,
        funding_redeemscript: Script,
        channel_value_satoshis: u64,
        // first is the idx of the first of the two revocation points
@@ -800,8 +815,7 @@ impl<ChanSigner: ChannelKeys> PartialEq for ChannelMonitor<ChanSigner> {
                        self.funding_info != other.funding_info ||
                        self.current_remote_commitment_txid != other.current_remote_commitment_txid ||
                        self.prev_remote_commitment_txid != other.prev_remote_commitment_txid ||
-                       self.their_htlc_base_key != other.their_htlc_base_key ||
-                       self.their_delayed_payment_base_key != other.their_delayed_payment_base_key ||
+                       self.remote_tx_cache != other.remote_tx_cache ||
                        self.funding_redeemscript != other.funding_redeemscript ||
                        self.channel_value_satoshis != other.channel_value_satoshis ||
                        self.their_cur_revocation_points != other.their_cur_revocation_points ||
@@ -869,8 +883,16 @@ impl<ChanSigner: ChannelKeys + Writeable> ChannelMonitor<ChanSigner> {
                self.current_remote_commitment_txid.write(writer)?;
                self.prev_remote_commitment_txid.write(writer)?;
 
-               writer.write_all(&self.their_htlc_base_key.serialize())?;
-               writer.write_all(&self.their_delayed_payment_base_key.serialize())?;
+               self.remote_tx_cache.remote_delayed_payment_base_key.write(writer)?;
+               self.remote_tx_cache.remote_htlc_base_key.write(writer)?;
+               writer.write_all(&byte_utils::be64_to_array(self.remote_tx_cache.per_htlc.len() as u64))?;
+               for (ref txid, ref htlcs) in self.remote_tx_cache.per_htlc.iter() {
+                       writer.write_all(&txid[..])?;
+                       writer.write_all(&byte_utils::be64_to_array(htlcs.len() as u64))?;
+                       for &ref htlc in htlcs.iter() {
+                               htlc.write(writer)?;
+                       }
+               }
                self.funding_redeemscript.write(writer)?;
                self.channel_value_satoshis.write(writer)?;
 
@@ -1025,7 +1047,7 @@ impl<ChanSigner: ChannelKeys + Writeable> ChannelMonitor<ChanSigner> {
 impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
        pub(super) fn new(keys: ChanSigner, shutdown_pubkey: &PublicKey,
                        our_to_self_delay: u16, destination_script: &Script, funding_info: (OutPoint, Script),
-                       their_htlc_base_key: &PublicKey, their_delayed_payment_base_key: &PublicKey,
+                       remote_htlc_base_key: &PublicKey, remote_delayed_payment_base_key: &PublicKey,
                        their_to_self_delay: u16, funding_redeemscript: Script, channel_value_satoshis: u64,
                        commitment_transaction_number_obscure_factor: u64,
                        initial_local_commitment_tx: LocalCommitmentTransaction,
@@ -1037,7 +1059,9 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                let payment_key_hash = WPubkeyHash::hash(&keys.pubkeys().payment_point.serialize());
                let remote_payment_script = Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&payment_key_hash[..]).into_script();
 
-               let mut onchain_tx_handler = OnchainTxHandler::new(destination_script.clone(), keys.clone(), their_to_self_delay, their_delayed_payment_base_key.clone(), their_htlc_base_key.clone(), our_to_self_delay, logger.clone());
+               let remote_tx_cache = RemoteTxCache { remote_delayed_payment_base_key: *remote_delayed_payment_base_key, remote_htlc_base_key: *remote_htlc_base_key, per_htlc: HashMap::new() };
+
+               let mut onchain_tx_handler = OnchainTxHandler::new(destination_script.clone(), keys.clone(), their_to_self_delay, *remote_delayed_payment_base_key, *remote_htlc_base_key ,our_to_self_delay, logger.clone());
 
                let local_tx_sequence = initial_local_commitment_tx.unsigned_tx.input[0].sequence as u64;
                let local_tx_locktime = initial_local_commitment_tx.unsigned_tx.lock_time as u64;
@@ -1072,8 +1096,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                        current_remote_commitment_txid: None,
                        prev_remote_commitment_txid: None,
 
-                       their_htlc_base_key: *their_htlc_base_key,
-                       their_delayed_payment_base_key: *their_delayed_payment_base_key,
+                       remote_tx_cache,
                        funding_redeemscript,
                        channel_value_satoshis: channel_value_satoshis,
                        their_cur_revocation_points: None,
@@ -1205,6 +1228,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                                htlcs.push(htlc.0);
                        }
                }
+               self.remote_tx_cache.per_htlc.insert(new_txid, htlcs.clone());
                self.onchain_tx_handler.provide_latest_remote_tx(new_txid, htlcs);
        }
 
@@ -1413,7 +1437,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                        let per_commitment_key = ignore_error!(SecretKey::from_slice(&secret));
                        let per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key);
                        let revocation_pubkey = ignore_error!(chan_utils::derive_public_revocation_key(&self.secp_ctx, &per_commitment_point, &self.keys.pubkeys().revocation_basepoint));
-                       let delayed_key = ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, &PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key), &self.their_delayed_payment_base_key));
+                       let delayed_key = ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, &PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key), &self.remote_tx_cache.remote_delayed_payment_base_key));
 
                        let revokeable_redeemscript = chan_utils::get_revokeable_redeemscript(&revocation_pubkey, self.our_to_self_delay, &delayed_key);
                        let revokeable_p2wsh = revokeable_redeemscript.to_v0_p2wsh();
@@ -1421,7 +1445,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 { per_commitment_point, per_commitment_key, input_descriptor: InputDescriptors::RevokedOutput, amount: outp.value };
+                                       let witness_data = InputMaterial::Revoked { per_commitment_point, remote_delayed_payment_base_key: self.remote_tx_cache.remote_delayed_payment_base_key, remote_htlc_base_key: self.remote_tx_cache.remote_htlc_base_key, per_commitment_key, input_descriptor: InputDescriptors::RevokedOutput, amount: outp.value, htlc: None };
                                        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});
                                }
                        }
@@ -1434,7 +1458,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                                                                tx.output[transaction_output_index as usize].value != htlc.amount_msat / 1000 {
                                                        return (claimable_outpoints, (commitment_txid, watch_outputs)); // Corrupted per_commitment_data, fuck this user
                                                }
-                                               let witness_data = InputMaterial::Revoked { per_commitment_point, per_commitment_key, input_descriptor: if htlc.offered { InputDescriptors::RevokedOfferedHTLC } else { InputDescriptors::RevokedReceivedHTLC }, amount: tx.output[transaction_output_index as usize].value };
+                                               let witness_data = InputMaterial::Revoked { per_commitment_point, remote_delayed_payment_base_key: self.remote_tx_cache.remote_delayed_payment_base_key, remote_htlc_base_key: self.remote_tx_cache.remote_htlc_base_key, per_commitment_key, input_descriptor: if htlc.offered { InputDescriptors::RevokedOfferedHTLC } else { InputDescriptors::RevokedReceivedHTLC }, amount: tx.output[transaction_output_index as usize].value, htlc: Some(htlc.clone()) };
                                                claimable_outpoints.push(ClaimRequest { absolute_timelock: htlc.cltv_expiry, aggregable: true, outpoint: BitcoinOutPoint { txid: commitment_txid, vout: transaction_output_index }, witness_data });
                                        }
                                }
@@ -1569,7 +1593,7 @@ 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 {
-                                                               let witness_data = InputMaterial::RemoteHTLC { per_commitment_point: *revocation_point, preimage, amount: htlc.amount_msat / 1000, locktime: htlc.cltv_expiry };
+                                                               let witness_data = InputMaterial::RemoteHTLC { per_commitment_point: *revocation_point, remote_delayed_payment_base_key: self.remote_tx_cache.remote_delayed_payment_base_key, remote_htlc_base_key: self.remote_tx_cache.remote_htlc_base_key, preimage, htlc: htlc.clone() };
                                                                claimable_outpoints.push(ClaimRequest { absolute_timelock: htlc.cltv_expiry, aggregable, outpoint: BitcoinOutPoint { txid: commitment_txid, vout: transaction_output_index }, witness_data });
                                                        }
                                                }
@@ -1601,7 +1625,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                let per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key);
 
                log_trace!(self, "Remote HTLC broadcast {}:{}", htlc_txid, 0);
-               let witness_data = InputMaterial::Revoked { per_commitment_point, per_commitment_key, input_descriptor: InputDescriptors::RevokedOutput, amount: tx.output[0].value };
+               let witness_data = InputMaterial::Revoked { per_commitment_point, remote_delayed_payment_base_key: self.remote_tx_cache.remote_delayed_payment_base_key, remote_htlc_base_key: self.remote_tx_cache.remote_htlc_base_key,  per_commitment_key, input_descriptor: InputDescriptors::RevokedOutput, amount: tx.output[0].value, htlc: None };
                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())))
        }
@@ -2192,8 +2216,29 @@ impl<ChanSigner: ChannelKeys + Readable> ReadableArgs<Arc<Logger>> for (BlockHas
                let current_remote_commitment_txid = Readable::read(reader)?;
                let prev_remote_commitment_txid = Readable::read(reader)?;
 
-               let their_htlc_base_key = Readable::read(reader)?;
-               let their_delayed_payment_base_key = Readable::read(reader)?;
+               let remote_tx_cache = {
+                       let remote_delayed_payment_base_key = Readable::read(reader)?;
+                       let remote_htlc_base_key = Readable::read(reader)?;
+                       let per_htlc_len: u64 = Readable::read(reader)?;
+                       let mut per_htlc = HashMap::with_capacity(cmp::min(per_htlc_len as usize, MAX_ALLOC_SIZE / 64));
+                       for _  in 0..per_htlc_len {
+                               let txid: Txid = Readable::read(reader)?;
+                               let htlcs_count: u64 = Readable::read(reader)?;
+                               let mut htlcs = Vec::with_capacity(cmp::min(htlcs_count as usize, MAX_ALLOC_SIZE / 32));
+                               for _ in 0..htlcs_count {
+                                       let htlc = Readable::read(reader)?;
+                                       htlcs.push(htlc);
+                               }
+                               if let Some(_) = per_htlc.insert(txid, htlcs) {
+                                       return Err(DecodeError::InvalidValue);
+                               }
+                       }
+                       RemoteTxCache {
+                               remote_delayed_payment_base_key,
+                               remote_htlc_base_key,
+                               per_htlc,
+                       }
+               };
                let funding_redeemscript = Readable::read(reader)?;
                let channel_value_satoshis = Readable::read(reader)?;
 
@@ -2402,8 +2447,7 @@ impl<ChanSigner: ChannelKeys + Readable> ReadableArgs<Arc<Logger>> for (BlockHas
                        current_remote_commitment_txid,
                        prev_remote_commitment_txid,
 
-                       their_htlc_base_key,
-                       their_delayed_payment_base_key,
+                       remote_tx_cache,
                        funding_redeemscript,
                        channel_value_satoshis,
                        their_cur_revocation_points,
index 6af53c73e43dba56684f3383800ec0fb1e10381c..b5efaf9380f8e7e4e3a2836d036cd40c890ba566 100644 (file)
@@ -51,10 +51,11 @@ enum OnchainEvent {
 
 /// Cache remote basepoint to compute any transaction on
 /// remote outputs, either justice or preimage/timeout transactions.
-struct RemoteTxCache {
-       remote_delayed_payment_base_key: PublicKey,
-       remote_htlc_base_key: PublicKey,
-       per_htlc: HashMap<Txid, Vec<(HTLCOutputInCommitment)>>
+#[derive(PartialEq)]
+pub(super) struct RemoteTxCache {
+       pub(super) remote_delayed_payment_base_key: PublicKey,
+       pub(super) remote_htlc_base_key: PublicKey,
+       pub(super) per_htlc: HashMap<Txid, Vec<(HTLCOutputInCommitment)>>
 }
 
 /// Higher-level cache structure needed to re-generate bumped claim txn if needed
@@ -599,9 +600,9 @@ impl<ChanSigner: ChannelKeys> OnchainTxHandler<ChanSigner> {
                                        inputs_witnesses_weight += Self::get_witnesses_weight(&[*input_descriptor]);
                                        amt += *amount;
                                },
-                               &InputMaterial::RemoteHTLC { ref preimage, ref amount, .. } => {
+                               &InputMaterial::RemoteHTLC { ref preimage, ref htlc, .. } => {
                                        inputs_witnesses_weight += Self::get_witnesses_weight(if preimage.is_some() { &[InputDescriptors::OfferedHTLC] } else { &[InputDescriptors::ReceivedHTLC] });
-                                       amt += *amount;
+                                       amt += htlc.amount_msat / 1000;
                                },
                                &InputMaterial::LocalHTLC { .. } => {
                                        dynamic_fee = false;
@@ -634,33 +635,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 per_commitment_point, ref per_commitment_key, ref input_descriptor, ref amount } => {
-                                               if let Ok(chan_keys) = TxCreationKeys::new(&self.secp_ctx, &per_commitment_point, &self.remote_tx_cache.remote_delayed_payment_base_key, &self.remote_tx_cache.remote_htlc_base_key, &self.key_storage.pubkeys().revocation_basepoint, &self.key_storage.pubkeys().htlc_basepoint) {
-
-                                                       let mut this_htlc = None;
-                                                       if *input_descriptor != InputDescriptors::RevokedOutput {
-                                                               if let Some(htlcs) = self.remote_tx_cache.per_htlc.get(&outp.txid) {
-                                                                       for htlc in htlcs {
-                                                                               if htlc.transaction_output_index.unwrap() == outp.vout {
-                                                                                       this_htlc = Some(htlc);
-                                                                               }
-                                                                       }
-                                                               }
-                                                       }
+                                       &InputMaterial::Revoked { ref per_commitment_point, ref remote_delayed_payment_base_key, ref remote_htlc_base_key, ref per_commitment_key, ref input_descriptor, ref amount, ref htlc } => {
+                                               if let Ok(chan_keys) = TxCreationKeys::new(&self.secp_ctx, &per_commitment_point, remote_delayed_payment_base_key, remote_htlc_base_key, &self.key_storage.pubkeys().revocation_basepoint, &self.key_storage.pubkeys().htlc_basepoint) {
 
-                                                       let witness_script = if *input_descriptor != InputDescriptors::RevokedOutput && this_htlc.is_some() {
-                                                               chan_utils::get_htlc_redeemscript_with_explicit_keys(&this_htlc.unwrap(), &chan_keys.a_htlc_key, &chan_keys.b_htlc_key, &chan_keys.revocation_key)
-                                                       } else if *input_descriptor != InputDescriptors::RevokedOutput {
-                                                               return None;
+                                                       let witness_script = if let Some(ref htlc) = *htlc {
+                                                               chan_utils::get_htlc_redeemscript_with_explicit_keys(&htlc, &chan_keys.a_htlc_key, &chan_keys.b_htlc_key, &chan_keys.revocation_key)
                                                        } else {
                                                                chan_utils::get_revokeable_redeemscript(&chan_keys.revocation_key, self.remote_csv, &chan_keys.a_delayed_payment_key)
                                                        };
 
-                                                       let is_htlc = *input_descriptor != InputDescriptors::RevokedOutput;
-                                                       if let Ok(sig) = self.key_storage.sign_justice_transaction(&bumped_tx, i, &witness_script, *amount, &per_commitment_key, &chan_keys.revocation_key, is_htlc,  &self.secp_ctx) {
+                                                       if let Ok(sig) = self.key_storage.sign_justice_transaction(&bumped_tx, i, &witness_script, *amount, &per_commitment_key, &chan_keys.revocation_key, htlc.is_some(),  &self.secp_ctx) {
                                                                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 htlc.is_some() {
                                                                        bumped_tx.input[i].witness.push(chan_keys.revocation_key.clone().serialize().to_vec());
                                                                } else {
                                                                        bumped_tx.input[i].witness.push(vec!(1));
@@ -672,21 +659,12 @@ impl<ChanSigner: ChannelKeys> OnchainTxHandler<ChanSigner> {
                                                        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 per_commitment_point, ref preimage, ref amount, ref locktime } => {
-                                               if let Ok(chan_keys) = TxCreationKeys::new(&self.secp_ctx, &per_commitment_point, &self.remote_tx_cache.remote_delayed_payment_base_key, &self.remote_tx_cache.remote_htlc_base_key, &self.key_storage.pubkeys().revocation_basepoint, &self.key_storage.pubkeys().htlc_basepoint) {
-                                                       let mut this_htlc = None;
-                                                       if let Some(htlcs) = self.remote_tx_cache.per_htlc.get(&outp.txid) {
-                                                               for htlc in htlcs {
-                                                                       if htlc.transaction_output_index.unwrap() == outp.vout {
-                                                                               this_htlc = Some(htlc);
-                                                                       }
-                                                               }
-                                                       }
-                                                       if this_htlc.is_none() { return None; }
-                                                       let witness_script = chan_utils::get_htlc_redeemscript_with_explicit_keys(&this_htlc.unwrap(), &chan_keys.a_htlc_key, &chan_keys.b_htlc_key, &chan_keys.revocation_key);
+                                       &InputMaterial::RemoteHTLC { ref per_commitment_point, ref remote_delayed_payment_base_key, ref remote_htlc_base_key, ref preimage, ref htlc } => {
+                                               if let Ok(chan_keys) = TxCreationKeys::new(&self.secp_ctx, &per_commitment_point, remote_delayed_payment_base_key, remote_htlc_base_key, &self.key_storage.pubkeys().revocation_basepoint, &self.key_storage.pubkeys().htlc_basepoint) {
+                                                       let witness_script = chan_utils::get_htlc_redeemscript_with_explicit_keys(&htlc, &chan_keys.a_htlc_key, &chan_keys.b_htlc_key, &chan_keys.revocation_key);
 
-                                                       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
-                                                       if let Ok(sig) = self.key_storage.sign_remote_htlc_transaction(&bumped_tx, i, &witness_script, *amount, &per_commitment_point, preimage, &self.secp_ctx) {
+                                                       if !preimage.is_some() { bumped_tx.lock_time = htlc.cltv_expiry }; // Right now we don't aggregate time-locked transaction, if we do we should set lock_time before to avoid breaking hash computation
+                                                       if let Ok(sig) = self.key_storage.sign_remote_htlc_transaction(&bumped_tx, i, &witness_script, htlc.amount_msat / 1000, &per_commitment_point, preimage, &self.secp_ctx) {
                                                                bumped_tx.input[i].witness.push(sig.serialize_der().to_vec());
                                                                bumped_tx.input[i].witness[0].push(SigHashType::All as u8);
                                                                if let &Some(preimage) = preimage {