Dedup RemoteTxCache by removing OnchainTxHandler copy
authorAntoine Riard <ariard@student.42.fr>
Tue, 28 Apr 2020 00:07:17 +0000 (20:07 -0400)
committerAntoine Riard <ariard@student.42.fr>
Thu, 28 May 2020 08:21:47 +0000 (04:21 -0400)
RemoteTxCache was providing all data needed at transaction
signature for any remote HTLC transaction or justice transaction.
This move was making the API between OnchainTxHandle akward and
scope of responsibilites with ChannelMonitor unclear.

Instead scope OnchainTxHandler to transaction-finalization, fee-bumping
and broadcast only.

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

index 26798fbbc56e84b16066aec524e7d4bd8ee5af18..99f2944b84cae4084d9c35f634670767744b3ffe 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, RemoteTxCache};
+use ln::onchaintx::{OnchainTxHandler, InputDescriptors};
 use chain::chaininterface::{ChainListener, ChainWatchInterface, BroadcasterInterface, FeeEstimator};
 use chain::transaction::OutPoint;
 use chain::keysinterface::{SpendableOutputDescriptor, ChannelKeys};
@@ -428,6 +428,15 @@ struct LocalSignedTx {
        htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>,
 }
 
+/// Cache remote basepoint to compute any transaction on
+/// remote outputs, either justice or preimage/timeout transactions.
+#[derive(PartialEq)]
+struct RemoteTxCache {
+       remote_delayed_payment_base_key: PublicKey,
+       remote_htlc_base_key: PublicKey,
+       per_htlc: HashMap<Txid, Vec<HTLCOutputInCommitment>>
+}
+
 /// When ChannelMonitor discovers an onchain outpoint being a step of a channel and that it needs
 /// to generate a tx to push channel state forward, we cache outpoint-solving tx material to build
 /// a new bumped one in case of lenghty confirmation delay
@@ -1069,7 +1078,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 
                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);
+               let mut onchain_tx_handler = OnchainTxHandler::new(destination_script.clone(), keys.clone(), their_to_self_delay, our_to_self_delay);
 
                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;
@@ -1235,8 +1244,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);
+               self.remote_tx_cache.per_htlc.insert(new_txid, htlcs);
        }
 
        /// Informs this monitor of the latest local (ie broadcastable) commitment transaction. The
index ce1a882948f27da253ad84b985be14f652359f62..0873cf42e093c11d663706b79d679516bb405ebf 100644 (file)
@@ -11,13 +11,12 @@ use bitcoin::hash_types::Txid;
 
 use bitcoin::secp256k1::{Secp256k1, Signature};
 use bitcoin::secp256k1;
-use bitcoin::secp256k1::key::PublicKey;
 
 use ln::msgs::DecodeError;
 use ln::channelmonitor::{ANTI_REORG_DELAY, CLTV_SHARED_CLAIM_BUFFER, InputMaterial, ClaimRequest};
 use ln::channelmanager::PaymentPreimage;
 use ln::chan_utils;
-use ln::chan_utils::{TxCreationKeys, LocalCommitmentTransaction, HTLCOutputInCommitment};
+use ln::chan_utils::{TxCreationKeys, LocalCommitmentTransaction};
 use chain::chaininterface::{FeeEstimator, BroadcasterInterface, ConfirmationTarget, MIN_RELAY_FEE_SAT_PER_1000_WEIGHT};
 use chain::keysinterface::ChannelKeys;
 use util::logger::Logger;
@@ -48,15 +47,6 @@ enum OnchainEvent {
        }
 }
 
-/// Cache remote basepoint to compute any transaction on
-/// remote outputs, either justice or preimage/timeout transactions.
-#[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
 #[derive(Clone, PartialEq)]
 pub struct ClaimTxBumpMaterial {
@@ -251,7 +241,6 @@ pub struct OnchainTxHandler<ChanSigner: ChannelKeys> {
        prev_local_commitment: Option<LocalCommitmentTransaction>,
        prev_local_htlc_sigs: Option<Vec<Option<(usize, Signature)>>>,
        local_csv: u16,
-       remote_tx_cache: RemoteTxCache,
        remote_csv: u16,
 
        key_storage: ChanSigner,
@@ -298,16 +287,6 @@ impl<ChanSigner: ChannelKeys + Writeable> OnchainTxHandler<ChanSigner> {
 
                self.local_csv.write(writer)?;
 
-               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.remote_csv.write(writer)?;
 
                self.key_storage.write(writer)?;
@@ -358,29 +337,6 @@ impl<ChanSigner: ChannelKeys + Readable> Readable for OnchainTxHandler<ChanSigne
 
                let local_csv = 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 remote_csv = Readable::read(reader)?;
 
                let key_storage = Readable::read(reader)?;
@@ -435,7 +391,6 @@ impl<ChanSigner: ChannelKeys + Readable> Readable for OnchainTxHandler<ChanSigne
                        prev_local_commitment,
                        prev_local_htlc_sigs,
                        local_csv,
-                       remote_tx_cache,
                        remote_csv,
                        key_storage,
                        claimable_outpoints,
@@ -447,16 +402,10 @@ impl<ChanSigner: ChannelKeys + Readable> Readable for OnchainTxHandler<ChanSigne
 }
 
 impl<ChanSigner: ChannelKeys> OnchainTxHandler<ChanSigner> {
-       pub(super) fn new(destination_script: Script, keys: ChanSigner, local_csv: u16, remote_delayed_payment_base_key: PublicKey, remote_htlc_base_key: PublicKey, remote_csv: u16) -> Self {
+       pub(super) fn new(destination_script: Script, keys: ChanSigner, local_csv: u16, remote_csv: u16) -> Self {
 
                let key_storage = keys;
 
-               let remote_tx_cache = RemoteTxCache {
-                       remote_delayed_payment_base_key,
-                       remote_htlc_base_key,
-                       per_htlc: HashMap::new(),
-               };
-
                OnchainTxHandler {
                        destination_script,
                        local_commitment: None,
@@ -464,7 +413,6 @@ impl<ChanSigner: ChannelKeys> OnchainTxHandler<ChanSigner> {
                        prev_local_commitment: None,
                        prev_local_htlc_sigs: None,
                        local_csv,
-                       remote_tx_cache,
                        remote_csv,
                        key_storage,
                        pending_claim_requests: HashMap::new(),
@@ -989,10 +937,6 @@ impl<ChanSigner: ChannelKeys> OnchainTxHandler<ChanSigner> {
                }
        }
 
-       pub(super) fn provide_latest_remote_tx(&mut self, commitment_txid: Txid, htlcs: Vec<HTLCOutputInCommitment>) {
-               self.remote_tx_cache.per_htlc.insert(commitment_txid, htlcs);
-       }
-
        #[cfg(test)]
        pub(super) fn get_fully_signed_copy_local_tx(&mut self, funding_redeemscript: &Script) -> Option<Transaction> {
                if let Some(ref mut local_commitment) = self.local_commitment {