From 3c75ae3bbef153db494eba2de31a3a8cd04aab98 Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Mon, 27 Apr 2020 20:07:17 -0400 Subject: [PATCH] Dedup RemoteTxCache by removing OnchainTxHandler copy 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 | 16 ++++++-- lightning/src/ln/onchaintx.rs | 60 +----------------------------- 2 files changed, 14 insertions(+), 62 deletions(-) diff --git a/lightning/src/ln/channelmonitor.rs b/lightning/src/ln/channelmonitor.rs index 1cbea01b..4e4d7b5a 100644 --- a/lightning/src/ln/channelmonitor.rs +++ b/lightning/src/ln/channelmonitor.rs @@ -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, Option)>, } +/// 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> +} + /// 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 ChannelMonitor { 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 ChannelMonitor { 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 diff --git a/lightning/src/ln/onchaintx.rs b/lightning/src/ln/onchaintx.rs index b5efaf93..e3e267d3 100644 --- a/lightning/src/ln/onchaintx.rs +++ b/lightning/src/ln/onchaintx.rs @@ -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> -} - /// 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 { prev_local_commitment: Option, prev_local_htlc_sigs: Option>>, local_csv: u16, - remote_tx_cache: RemoteTxCache, remote_csv: u16, key_storage: ChanSigner, @@ -300,16 +289,6 @@ impl OnchainTxHandler { 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 ReadableArgs> 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 ReadableArgs> 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 ReadableArgs> for OnchainTx } impl OnchainTxHandler { - 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) -> Self { + pub(super) fn new(destination_script: Script, keys: ChanSigner, local_csv: u16, remote_csv: u16, logger: Arc) -> 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 OnchainTxHandler { 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 OnchainTxHandler { } } - pub(super) fn provide_latest_remote_tx(&mut self, commitment_txid: Txid, htlcs: Vec) { - 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 { if let Some(ref mut local_commitment) = self.local_commitment { -- 2.30.2