From 1ec0c14fce2766c803bf6de04e71b7e0766e51c2 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Fri, 8 Nov 2019 20:12:13 -0500 Subject: [PATCH] multi: remove listeners field and method from ChainWatchInterface This includes the purpose of this PR, which is to remove the circular reference created by ChainListeners self-adding themselves to their ChainWatchInterface's `listeners` field. --- .../fuzz/fuzz_targets/chanmon_fail_consistency.rs | 2 +- lightning/fuzz/fuzz_targets/full_stack_target.rs | 2 +- lightning/src/chain/chaininterface.rs | 11 ----------- lightning/src/ln/channelmanager.rs | 13 +++---------- lightning/src/ln/channelmonitor.rs | 3 +-- lightning/src/ln/functional_test_utils.rs | 2 +- lightning/src/ln/functional_tests.rs | 3 --- 7 files changed, 7 insertions(+), 29 deletions(-) diff --git a/lightning/fuzz/fuzz_targets/chanmon_fail_consistency.rs b/lightning/fuzz/fuzz_targets/chanmon_fail_consistency.rs index 74b1e5d14..33b7e2633 100644 --- a/lightning/fuzz/fuzz_targets/chanmon_fail_consistency.rs +++ b/lightning/fuzz/fuzz_targets/chanmon_fail_consistency.rs @@ -193,7 +193,7 @@ pub fn do_test(data: &[u8]) { config.channel_options.fee_proportional_millionths = 0; config.channel_options.announced_channel = true; config.peer_channel_config_limits.min_dust_limit_satoshis = 0; - (ChannelManager::new(Network::Bitcoin, fee_est.clone(), monitor.clone(), watch.clone(), broadcast.clone(), Arc::clone(&logger), keys_manager.clone(), config, 0).unwrap(), + (ChannelManager::new(Network::Bitcoin, fee_est.clone(), monitor.clone(), broadcast.clone(), Arc::clone(&logger), keys_manager.clone(), config, 0).unwrap(), monitor) } } } diff --git a/lightning/fuzz/fuzz_targets/full_stack_target.rs b/lightning/fuzz/fuzz_targets/full_stack_target.rs index 41ab473fd..f9f2e3bb0 100644 --- a/lightning/fuzz/fuzz_targets/full_stack_target.rs +++ b/lightning/fuzz/fuzz_targets/full_stack_target.rs @@ -331,7 +331,7 @@ pub fn do_test(data: &[u8], logger: &Arc) { config.channel_options.fee_proportional_millionths = slice_to_be32(get_slice!(4)); config.channel_options.announced_channel = get_slice!(1)[0] != 0; config.peer_channel_config_limits.min_dust_limit_satoshis = 0; - let channelmanager = ChannelManager::new(Network::Bitcoin, fee_est.clone(), monitor.clone(), watch.clone(), broadcast.clone(), Arc::clone(&logger), keys_manager.clone(), config, 0).unwrap(); + let channelmanager = ChannelManager::new(Network::Bitcoin, fee_est.clone(), monitor.clone(), broadcast.clone(), Arc::clone(&logger), keys_manager.clone(), config, 0).unwrap(); let router = Arc::new(Router::new(PublicKey::from_secret_key(&Secp256k1::signing_only(), &keys_manager.get_node_secret()), watch.clone(), Arc::clone(&logger))); let peers = RefCell::new([false; 256]); diff --git a/lightning/src/chain/chaininterface.rs b/lightning/src/chain/chaininterface.rs index c0330fb29..24c9d7100 100644 --- a/lightning/src/chain/chaininterface.rs +++ b/lightning/src/chain/chaininterface.rs @@ -45,11 +45,6 @@ pub trait ChainWatchInterface: Sync + Send { /// Indicates that a listener needs to see all transactions. fn watch_all_txn(&self); - /// Register the given listener to receive events. Only a weak pointer is provided and the - /// registration should be freed once that pointer expires. - fn register_listener(&self, listener: Weak); - //TODO: unregister - /// Gets the script and value in satoshis for a given unspent transaction output given a /// short_channel_id (aka unspent_tx_output_identier). For BTC/tBTC channels the top three /// bytes are the block height, the next 3 the transaction index within the block, and the @@ -204,7 +199,6 @@ impl ChainWatchedUtil { pub struct ChainWatchInterfaceUtil { network: Network, watched: Mutex, - listeners: Mutex>>, reentered: AtomicUsize, logger: Arc, } @@ -232,11 +226,6 @@ impl ChainWatchInterface for ChainWatchInterfaceUtil { } } - fn register_listener(&self, listener: Weak) { - let mut vec = self.listeners.lock().unwrap(); - vec.push(listener); - } - fn get_chain_utxo(&self, genesis_hash: Sha256dHash, _unspent_tx_output_identifier: u64) -> Result<(Script, u64), ChainError> { if genesis_hash != genesis_block(self.network).header.bitcoin_hash() { return Err(ChainError::NotWatched); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 5452e365d..f368a3a20 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -25,7 +25,7 @@ use secp256k1::Secp256k1; use secp256k1::ecdh::SharedSecret; use secp256k1; -use chain::chaininterface::{BroadcasterInterface,ChainListener,ChainWatchInterface,FeeEstimator}; +use chain::chaininterface::{BroadcasterInterface,ChainListener,FeeEstimator}; use chain::transaction::OutPoint; use ln::channel::{Channel, ChannelError}; use ln::channelmonitor::{ChannelMonitor, ChannelMonitorUpdateErr, ManyChannelMonitor, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY}; @@ -323,7 +323,6 @@ pub struct ChannelManager { genesis_hash: Sha256dHash, fee_estimator: Arc, monitor: Arc, - chain_monitor: Arc, tx_broadcaster: Arc, #[cfg(test)] @@ -596,7 +595,6 @@ impl ChannelManager { genesis_hash: genesis_block(network).header.bitcoin_hash(), fee_estimator: feeest.clone(), monitor: monitor.clone(), - chain_monitor, tx_broadcaster, latest_block_height: AtomicUsize::new(current_blockchain_height), @@ -619,8 +617,7 @@ impl ChannelManager { logger, }); - let weak_res = Arc::downgrade(&res); - res.chain_monitor.register_listener(weak_res); + Ok(res) } @@ -3142,10 +3139,7 @@ pub struct ChannelManagerReadArgs<'a> { /// you have deserialized ChannelMonitors separately and will add them to your /// ManyChannelMonitor after deserializing this ChannelManager. pub monitor: Arc, - /// The ChainWatchInterface for use in the ChannelManager in the future. - /// - /// No calls to the ChainWatchInterface will be made during deserialization. - pub chain_monitor: Arc, + /// The BroadcasterInterface which will be used in the ChannelManager in the future and may be /// used to broadcast the latest local commitment transactions of channels which must be /// force-closed during deserialization. @@ -3248,7 +3242,6 @@ impl<'a, R : ::std::io::Read> ReadableArgs> for (S genesis_hash, fee_estimator: args.fee_estimator, monitor: args.monitor, - chain_monitor: args.chain_monitor, tx_broadcaster: args.tx_broadcaster, latest_block_height: AtomicUsize::new(latest_block_height as usize), diff --git a/lightning/src/ln/channelmonitor.rs b/lightning/src/ln/channelmonitor.rs index a31c5bc50..dc385f6b6 100644 --- a/lightning/src/ln/channelmonitor.rs +++ b/lightning/src/ln/channelmonitor.rs @@ -223,8 +223,7 @@ impl SimpleManyChannelMonitor logger, fee_estimator: feeest, }); - let weak_res = Arc::downgrade(&res); - res.chain_monitor.register_listener(weak_res); + res } diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 7e776227a..303cde08d 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -844,7 +844,7 @@ pub fn create_network(node_count: usize, node_config: &[Option]) -> let mut default_config = UserConfig::new(); default_config.channel_options.announced_channel = true; default_config.peer_channel_config_limits.force_announced_channel_preference = false; - let node = ChannelManager::new(Network::Testnet, feeest.clone(), chan_monitor.clone(), chain_monitor.clone(), tx_broadcaster.clone(), Arc::clone(&logger), keys_manager.clone(), if node_config[i].is_some() { node_config[i].clone().unwrap() } else { default_config }, 0).unwrap(); + let node = ChannelManager::new(Network::Testnet, feeest.clone(), chan_monitor.clone(), tx_broadcaster.clone(), Arc::clone(&logger), keys_manager.clone(), if node_config[i].is_some() { node_config[i].clone().unwrap() } else { default_config }, 0).unwrap(); let router = Router::new(PublicKey::from_secret_key(&secp_ctx, &keys_manager.get_node_secret()), chain_monitor.clone(), Arc::clone(&logger)); nodes.push(Node { chain_monitor, tx_broadcaster, chan_monitor, node, router, keys_manager, node_seed: seed, network_payment_count: payment_count.clone(), diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index c147195bf..91829db81 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -3398,7 +3398,6 @@ fn test_no_txn_manager_serialize_deserialize() { keys_manager, fee_estimator: Arc::new(test_utils::TestFeeEstimator { sat_per_kw: 253 }), monitor: nodes[0].chan_monitor.clone(), - chain_monitor: nodes[0].chain_monitor.clone(), tx_broadcaster: nodes[0].tx_broadcaster.clone(), logger: Arc::new(test_utils::TestLogger::new()), channel_monitors: &channel_monitors, @@ -3524,7 +3523,6 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() { keys_manager, fee_estimator: Arc::new(test_utils::TestFeeEstimator { sat_per_kw: 253 }), monitor: nodes[0].chan_monitor.clone(), - chain_monitor: nodes[0].chain_monitor.clone(), tx_broadcaster: nodes[0].tx_broadcaster.clone(), logger: Arc::new(test_utils::TestLogger::new()), channel_monitors: &node_0_monitors.iter().map(|monitor| { (monitor.get_funding_txo().unwrap(), monitor) }).collect(), @@ -6131,7 +6129,6 @@ fn test_data_loss_protect() { keys_manager: Arc::new(keysinterface::KeysManager::new(&nodes[0].node_seed, Network::Testnet, Arc::clone(&logger), 42, 21)), fee_estimator: feeest.clone(), monitor: monitor.clone(), - chain_monitor: chain_monitor.clone(), logger: Arc::clone(&logger), tx_broadcaster, default_config: UserConfig::new(), -- 2.39.5