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>
Tue, 12 May 2020 21:41:33 +0000 (17:41 -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 1cbea01b19026ec2dc7fac1330f75f1ec7cb4cba..4e4d7b5a2337ae3187b60e5d470a94038bb4f1d3 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};
@@ -418,6 +418,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
@@ -1061,7 +1070,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, logger.clone());
+               let mut onchain_tx_handler = OnchainTxHandler::new(destination_script.clone(), keys.clone(), their_to_self_delay, 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;
@@ -1228,8 +1237,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 b5efaf9380f8e7e4e3a2836d036cd40c890ba566..e3e267d3275b3e29126f7035cf1d0057cc5fd494 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;
@@ -49,15 +48,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 {
@@ -252,7 +242,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,
@@ -300,16 +289,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)?;
@@ -360,29 +339,6 @@ impl<ChanSigner: ChannelKeys + Readable> ReadableArgs<Arc<Logger>> for OnchainTx
 
                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)?;
@@ -437,7 +393,6 @@ impl<ChanSigner: ChannelKeys + Readable> ReadableArgs<Arc<Logger>> for OnchainTx
                        prev_local_commitment,
                        prev_local_htlc_sigs,
                        local_csv,
-                       remote_tx_cache,
                        remote_csv,
                        key_storage,
                        claimable_outpoints,
@@ -450,16 +405,10 @@ impl<ChanSigner: ChannelKeys + Readable> ReadableArgs<Arc<Logger>> for OnchainTx
 }
 
 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, logger: Arc<Logger>) -> Self {
+       pub(super) fn new(destination_script: Script, keys: ChanSigner, local_csv: u16, remote_csv: u16, logger: Arc<Logger>) -> 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,
@@ -467,7 +416,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(),
@@ -990,10 +938,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 {