From 9455b49a5dfb480f814e7fd6dc98afcfda3138d7 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 26798fbbc..99f2944b8 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}; @@ -428,6 +428,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 @@ -1069,7 +1078,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); + 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 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 ce1a88294..0873cf42e 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; @@ -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> -} - /// 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 { prev_local_commitment: Option, prev_local_htlc_sigs: Option>>, local_csv: u16, - remote_tx_cache: RemoteTxCache, remote_csv: u16, key_storage: ChanSigner, @@ -298,16 +287,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)?; @@ -358,29 +337,6 @@ impl Readable for OnchainTxHandler Readable for OnchainTxHandler Readable for OnchainTxHandler 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) -> 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 OnchainTxHandler { 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 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.39.5