From 34f36d5ffecc8920d7a6e5ab92c9d71171580e15 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 27 Sep 2023 23:02:50 +0000 Subject: [PATCH] Drop various bounds on types passed to `MonitorUpdatingPersister` The new `MonitorUpdatingPersister` has a few redundant type bounds (re-specified on functions after having been specified on the struct itself), which we remove here. Further, it requires a `Deref` which is `Clone`able. This is generally fine in rust, but annoying in bindings, so we simply elide it in favor if a `&Deref`. --- fuzz/src/chanmon_consistency.rs | 2 +- lightning/src/chain/chainmonitor.rs | 2 +- lightning/src/chain/channelmonitor.rs | 8 +++--- lightning/src/util/persist.rs | 38 +++++++++++---------------- 4 files changed, 21 insertions(+), 29 deletions(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index 6a2007165..2a1b9e9a7 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -155,7 +155,7 @@ impl chain::Watch for TestChainMonitor { }; let deserialized_monitor = <(BlockHash, channelmonitor::ChannelMonitor)>:: read(&mut Cursor::new(&map_entry.get().1), (&*self.keys, &*self.keys)).unwrap().1; - deserialized_monitor.update_monitor(update, &&TestBroadcaster{}, &FuzzEstimator { ret_val: atomic::AtomicU32::new(253) }, &self.logger).unwrap(); + deserialized_monitor.update_monitor(update, &&TestBroadcaster{}, &&FuzzEstimator { ret_val: atomic::AtomicU32::new(253) }, &self.logger).unwrap(); let mut ser = VecWriter(Vec::new()); deserialized_monitor.write(&mut ser).unwrap(); map_entry.insert((update.update_id, ser.0)); diff --git a/lightning/src/chain/chainmonitor.rs b/lightning/src/chain/chainmonitor.rs index 261c0471c..aa9508d23 100644 --- a/lightning/src/chain/chainmonitor.rs +++ b/lightning/src/chain/chainmonitor.rs @@ -767,7 +767,7 @@ where C::Target: chain::Filter, Some(monitor_state) => { let monitor = &monitor_state.monitor; log_trace!(self.logger, "Updating ChannelMonitor for channel {}", log_funding_info!(monitor)); - let update_res = monitor.update_monitor(update, &self.broadcaster, &*self.fee_estimator, &self.logger); + let update_res = monitor.update_monitor(update, &self.broadcaster, &self.fee_estimator, &self.logger); let update_id = MonitorUpdateId::from_monitor_update(update); let mut pending_monitor_updates = monitor_state.pending_monitor_updates.lock().unwrap(); diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index a0ac1657e..bd0c15484 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -1311,7 +1311,7 @@ impl ChannelMonitor { &self, updates: &ChannelMonitorUpdate, broadcaster: &B, - fee_estimator: F, + fee_estimator: &F, logger: &L, ) -> Result<(), ()> where @@ -2615,7 +2615,7 @@ impl ChannelMonitorImpl { self.pending_monitor_events.push(MonitorEvent::HolderForceClosed(self.funding_info.0)); } - pub fn update_monitor(&mut self, updates: &ChannelMonitorUpdate, broadcaster: &B, fee_estimator: F, logger: &L) -> Result<(), ()> + pub fn update_monitor(&mut self, updates: &ChannelMonitorUpdate, broadcaster: &B, fee_estimator: &F, logger: &L) -> Result<(), ()> where B::Target: BroadcasterInterface, F::Target: FeeEstimator, L::Target: Logger, @@ -2655,7 +2655,7 @@ impl ChannelMonitorImpl { panic!("Attempted to apply ChannelMonitorUpdates out of order, check the update_id before passing an update to update_monitor!"); } let mut ret = Ok(()); - let bounded_fee_estimator = LowerBoundedFeeEstimator::new(&*fee_estimator); + let bounded_fee_estimator = LowerBoundedFeeEstimator::new(&**fee_estimator); for update in updates.updates.iter() { match update { ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { commitment_tx, htlc_outputs, claimed_htlcs, nondust_htlc_sources } => { @@ -4581,7 +4581,7 @@ mod tests { let broadcaster = TestBroadcaster::with_blocks(Arc::clone(&nodes[1].blocks)); assert!( - pre_update_monitor.update_monitor(&replay_update, &&broadcaster, &chanmon_cfgs[1].fee_estimator, &nodes[1].logger) + pre_update_monitor.update_monitor(&replay_update, &&broadcaster, &&chanmon_cfgs[1].fee_estimator, &nodes[1].logger) .is_err()); // Even though we error'd on the first update, we should still have generated an HTLC claim // transaction diff --git a/lightning/src/util/persist.rs b/lightning/src/util/persist.rs index 2a022c37c..35e473c42 100644 --- a/lightning/src/util/persist.rs +++ b/lightning/src/util/persist.rs @@ -397,11 +397,7 @@ where pub fn new( kv_store: K, logger: L, maximum_pending_updates: u64, entropy_source: ES, signer_provider: SP, - ) -> Self - where - ES::Target: EntropySource + Sized, - SP::Target: SignerProvider + Sized, - { + ) -> Self { MonitorUpdatingPersister { kv_store, logger, @@ -416,12 +412,10 @@ where /// It is extremely important that your [`KVStore::read`] implementation uses the /// [`io::ErrorKind::NotFound`] variant correctly. For more information, please see the /// documentation for [`MonitorUpdatingPersister`]. - pub fn read_all_channel_monitors_with_updates( - &self, broadcaster: B, fee_estimator: F, + pub fn read_all_channel_monitors_with_updates( + &self, broadcaster: &B, fee_estimator: &F, ) -> Result::Signer>)>, io::Error> where - ES::Target: EntropySource + Sized, - SP::Target: SignerProvider + Sized, B::Target: BroadcasterInterface, F::Target: FeeEstimator, { @@ -432,8 +426,8 @@ where let mut res = Vec::with_capacity(monitor_list.len()); for monitor_key in monitor_list { res.push(self.read_channel_monitor_with_updates( - &broadcaster, - fee_estimator.clone(), + broadcaster, + fee_estimator, monitor_key, )?) } @@ -457,12 +451,10 @@ where /// /// Loading a large number of monitors will be faster if done in parallel. You can use this /// function to accomplish this. Take care to limit the number of parallel readers. - pub fn read_channel_monitor_with_updates( - &self, broadcaster: &B, fee_estimator: F, monitor_key: String, + pub fn read_channel_monitor_with_updates( + &self, broadcaster: &B, fee_estimator: &F, monitor_key: String, ) -> Result<(BlockHash, ChannelMonitor<::Signer>), io::Error> where - ES::Target: EntropySource + Sized, - SP::Target: SignerProvider + Sized, B::Target: BroadcasterInterface, F::Target: FeeEstimator, { @@ -484,7 +476,7 @@ where Err(err) => return Err(err), }; - monitor.update_monitor(&update, broadcaster, fee_estimator.clone(), &self.logger) + monitor.update_monitor(&update, broadcaster, fee_estimator, &self.logger) .map_err(|e| { log_error!( self.logger, @@ -949,17 +941,17 @@ mod tests { // Check that the persisted channel data is empty before any channels are // open. let mut persisted_chan_data_0 = persister_0.read_all_channel_monitors_with_updates( - broadcaster_0, &chanmon_cfgs[0].fee_estimator).unwrap(); + &broadcaster_0, &&chanmon_cfgs[0].fee_estimator).unwrap(); assert_eq!(persisted_chan_data_0.len(), 0); let mut persisted_chan_data_1 = persister_1.read_all_channel_monitors_with_updates( - broadcaster_1, &chanmon_cfgs[1].fee_estimator).unwrap(); + &broadcaster_1, &&chanmon_cfgs[1].fee_estimator).unwrap(); assert_eq!(persisted_chan_data_1.len(), 0); // Helper to make sure the channel is on the expected update ID. macro_rules! check_persisted_data { ($expected_update_id: expr) => { persisted_chan_data_0 = persister_0.read_all_channel_monitors_with_updates( - broadcaster_0, &chanmon_cfgs[0].fee_estimator).unwrap(); + &broadcaster_0, &&chanmon_cfgs[0].fee_estimator).unwrap(); // check that we stored only one monitor assert_eq!(persisted_chan_data_0.len(), 1); for (_, mon) in persisted_chan_data_0.iter() { @@ -978,7 +970,7 @@ mod tests { } } persisted_chan_data_1 = persister_1.read_all_channel_monitors_with_updates( - broadcaster_1, &chanmon_cfgs[1].fee_estimator).unwrap(); + &broadcaster_1, &&chanmon_cfgs[1].fee_estimator).unwrap(); assert_eq!(persisted_chan_data_1.len(), 1); for (_, mon) in persisted_chan_data_1.iter() { assert_eq!(mon.get_latest_update_id(), $expected_update_id); @@ -1043,7 +1035,7 @@ mod tests { check_persisted_data!(CLOSED_CHANNEL_UPDATE_ID); // Make sure the expected number of stale updates is present. - let persisted_chan_data = persister_0.read_all_channel_monitors_with_updates(broadcaster_0, &chanmon_cfgs[0].fee_estimator).unwrap(); + let persisted_chan_data = persister_0.read_all_channel_monitors_with_updates(&broadcaster_0, &&chanmon_cfgs[0].fee_estimator).unwrap(); let (_, monitor) = &persisted_chan_data[0]; let monitor_name = MonitorName::from(monitor.get_funding_txo().0); // The channel should have 0 updates, as it wrote a full monitor and consolidated. @@ -1151,7 +1143,7 @@ mod tests { // Check that the persisted channel data is empty before any channels are // open. - let persisted_chan_data = persister_0.read_all_channel_monitors_with_updates(broadcaster_0, &chanmon_cfgs[0].fee_estimator).unwrap(); + let persisted_chan_data = persister_0.read_all_channel_monitors_with_updates(&broadcaster_0, &&chanmon_cfgs[0].fee_estimator).unwrap(); assert_eq!(persisted_chan_data.len(), 0); // Create some initial channel @@ -1162,7 +1154,7 @@ mod tests { send_payment(&nodes[1], &vec![&nodes[0]][..], 4_000_000); // Get the monitor and make a fake stale update at update_id=1 (lowest height of an update possible) - let persisted_chan_data = persister_0.read_all_channel_monitors_with_updates(broadcaster_0, &chanmon_cfgs[0].fee_estimator).unwrap(); + let persisted_chan_data = persister_0.read_all_channel_monitors_with_updates(&broadcaster_0, &&chanmon_cfgs[0].fee_estimator).unwrap(); let (_, monitor) = &persisted_chan_data[0]; let monitor_name = MonitorName::from(monitor.get_funding_txo().0); persister_0 -- 2.39.5