Skip to content

Commit dda86a0

Browse files
TheBlueMattTheBlueMatt
authored
Merge pull request #1112 from TheBlueMatt/2021-10-mon-refactors
Hide internals of ChainMonitor behind getter
2 parents da498d7 + 1464671 commit dda86a0

File tree

17 files changed

+436
-415
lines changed

17 files changed

+436
-415
lines changed

‎fuzz/src/chanmon_consistency.rs

+22-22
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ use bitcoin::hashes::sha256::Hash as Sha256;
3030
use bitcoin::hash_types::{BlockHash, WPubkeyHash};
3131

3232
use lightning::chain;
33-
use lightning::chain::{BestBlock, chainmonitor, channelmonitor, Confirm, Watch};
34-
use lightning::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdateErr, MonitorEvent};
33+
use lightning::chain::{BestBlock, ChannelMonitorUpdateErr, chainmonitor, channelmonitor, Confirm, Watch};
34+
use lightning::chain::channelmonitor::{ChannelMonitor, MonitorEvent};
3535
use lightning::chain::transaction::OutPoint;
3636
use lightning::chain::chaininterface::{BroadcasterInterface, ConfirmationTarget, FeeEstimator};
3737
use lightning::chain::keysinterface::{KeysInterface, InMemorySigner};
@@ -99,8 +99,8 @@ impl Writer for VecWriter {
9999
struct TestChainMonitor {
100100
pub logger: Arc<dyn Logger>,
101101
pub keys: Arc<KeyProvider>,
102+
pub persister: Arc<TestPersister>,
102103
pub chain_monitor: Arc<chainmonitor::ChainMonitor<EnforcingSigner, Arc<dyn chain::Filter>, Arc<TestBroadcaster>, Arc<FuzzEstimator>, Arc<dyn Logger>, Arc<TestPersister>>>,
103-
pub update_ret: Mutex<Result<(), channelmonitor::ChannelMonitorUpdateErr>>,
104104
// If we reload a node with an old copy of ChannelMonitors, the ChannelManager deserialization
105105
// logic will automatically force-close our channels for us (as we don't have an up-to-date
106106
// monitor implying we are not able to punish misbehaving counterparties). Because this test
@@ -112,28 +112,27 @@ struct TestChainMonitor {
112112
impl TestChainMonitor {
113113
pub fn new(broadcaster: Arc<TestBroadcaster>, logger: Arc<dyn Logger>, feeest: Arc<FuzzEstimator>, persister: Arc<TestPersister>, keys: Arc<KeyProvider>) -> Self {
114114
Self {
115-
chain_monitor: Arc::new(chainmonitor::ChainMonitor::new(None, broadcaster, logger.clone(), feeest, persister)),
115+
chain_monitor: Arc::new(chainmonitor::ChainMonitor::new(None, broadcaster, logger.clone(), feeest, Arc::clone(&persister))),
116116
logger,
117117
keys,
118-
update_ret: Mutex::new(Ok(())),
118+
persister,
119119
latest_monitors: Mutex::new(HashMap::new()),
120120
should_update_manager: atomic::AtomicBool::new(false),
121121
}
122122
}
123123
}
124124
impl chain::Watch<EnforcingSigner> for TestChainMonitor {
125-
fn watch_channel(&self, funding_txo: OutPoint, monitor: channelmonitor::ChannelMonitor<EnforcingSigner>) -> Result<(), channelmonitor::ChannelMonitorUpdateErr> {
125+
fn watch_channel(&self, funding_txo: OutPoint, monitor: channelmonitor::ChannelMonitor<EnforcingSigner>) -> Result<(), chain::ChannelMonitorUpdateErr> {
126126
let mut ser = VecWriter(Vec::new());
127127
monitor.write(&mut ser).unwrap();
128128
if let Some(_) = self.latest_monitors.lock().unwrap().insert(funding_txo, (monitor.get_latest_update_id(), ser.0)) {
129129
panic!("Already had monitor pre-watch_channel");
130130
}
131131
self.should_update_manager.store(true, atomic::Ordering::Relaxed);
132-
assert!(self.chain_monitor.watch_channel(funding_txo, monitor).is_ok());
133-
self.update_ret.lock().unwrap().clone()
132+
self.chain_monitor.watch_channel(funding_txo, monitor)
134133
}
135134

136-
fn update_channel(&self, funding_txo: OutPoint, update: channelmonitor::ChannelMonitorUpdate) -> Result<(), channelmonitor::ChannelMonitorUpdateErr> {
135+
fn update_channel(&self, funding_txo: OutPoint, update: channelmonitor::ChannelMonitorUpdate) -> Result<(), chain::ChannelMonitorUpdateErr> {
137136
let mut map_lock = self.latest_monitors.lock().unwrap();
138137
let mut map_entry = match map_lock.entry(funding_txo) {
139138
hash_map::Entry::Occupied(entry) => entry,
@@ -146,8 +145,7 @@ impl chain::Watch<EnforcingSigner> for TestChainMonitor {
146145
deserialized_monitor.write(&mut ser).unwrap();
147146
map_entry.insert((update.update_id, ser.0));
148147
self.should_update_manager.store(true, atomic::Ordering::Relaxed);
149-
assert!(self.chain_monitor.update_channel(funding_txo, update).is_ok());
150-
self.update_ret.lock().unwrap().clone()
148+
self.chain_monitor.update_channel(funding_txo, update)
151149
}
152150

153151
fn release_pending_monitor_events(&self) -> Vec<MonitorEvent> {
@@ -346,7 +344,8 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
346344
($node_id: expr, $fee_estimator: expr) => { {
347345
let logger: Arc<dyn Logger> = Arc::new(test_logger::TestLogger::new($node_id.to_string(), out.clone()));
348346
let keys_manager = Arc::new(KeyProvider { node_id: $node_id, rand_bytes_id: atomic::AtomicU32::new(0), enforcement_states: Mutex::new(HashMap::new()) });
349-
let monitor = Arc::new(TestChainMonitor::new(broadcast.clone(), logger.clone(), $fee_estimator.clone(), Arc::new(TestPersister{}), Arc::clone(&keys_manager)));
347+
let monitor = Arc::new(TestChainMonitor::new(broadcast.clone(), logger.clone(), $fee_estimator.clone(),
348+
Arc::new(TestPersister { update_ret: Mutex::new(Ok(())) }), Arc::clone(&keys_manager)));
350349

351350
let mut config = UserConfig::default();
352351
config.channel_options.forwarding_fee_proportional_millionths = 0;
@@ -365,7 +364,8 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
365364
($ser: expr, $node_id: expr, $old_monitors: expr, $keys_manager: expr, $fee_estimator: expr) => { {
366365
let keys_manager = Arc::clone(& $keys_manager);
367366
let logger: Arc<dyn Logger> = Arc::new(test_logger::TestLogger::new($node_id.to_string(), out.clone()));
368-
let chain_monitor = Arc::new(TestChainMonitor::new(broadcast.clone(), logger.clone(), $fee_estimator.clone(), Arc::new(TestPersister{}), Arc::clone(& $keys_manager)));
367+
let chain_monitor = Arc::new(TestChainMonitor::new(broadcast.clone(), logger.clone(), $fee_estimator.clone(),
368+
Arc::new(TestPersister { update_ret: Mutex::new(Ok(())) }), Arc::clone(& $keys_manager)));
369369

370370
let mut config = UserConfig::default();
371371
config.channel_options.forwarding_fee_proportional_millionths = 0;
@@ -846,12 +846,12 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
846846
// bit-twiddling mutations to have similar effects. This is probably overkill, but no
847847
// harm in doing so.
848848

849-
0x00 => *monitor_a.update_ret.lock().unwrap() = Err(ChannelMonitorUpdateErr::TemporaryFailure),
850-
0x01 => *monitor_b.update_ret.lock().unwrap() = Err(ChannelMonitorUpdateErr::TemporaryFailure),
851-
0x02 => *monitor_c.update_ret.lock().unwrap() = Err(ChannelMonitorUpdateErr::TemporaryFailure),
852-
0x04 => *monitor_a.update_ret.lock().unwrap() = Ok(()),
853-
0x05 => *monitor_b.update_ret.lock().unwrap() = Ok(()),
854-
0x06 => *monitor_c.update_ret.lock().unwrap() = Ok(()),
849+
0x00 => *monitor_a.persister.update_ret.lock().unwrap() = Err(ChannelMonitorUpdateErr::TemporaryFailure),
850+
0x01 => *monitor_b.persister.update_ret.lock().unwrap() = Err(ChannelMonitorUpdateErr::TemporaryFailure),
851+
0x02 => *monitor_c.persister.update_ret.lock().unwrap() = Err(ChannelMonitorUpdateErr::TemporaryFailure),
852+
0x04 => *monitor_a.persister.update_ret.lock().unwrap() = Ok(()),
853+
0x05 => *monitor_b.persister.update_ret.lock().unwrap() = Ok(()),
854+
0x06 => *monitor_c.persister.update_ret.lock().unwrap() = Ok(()),
855855

856856
0x08 => {
857857
if let Some((id, _)) = monitor_a.latest_monitors.lock().unwrap().get(&chan_1_funding) {
@@ -1072,9 +1072,9 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
10721072
// after we resolve all pending events.
10731073
// First make sure there are no pending monitor updates, resetting the error state
10741074
// and calling channel_monitor_updated for each monitor.
1075-
*monitor_a.update_ret.lock().unwrap() = Ok(());
1076-
*monitor_b.update_ret.lock().unwrap() = Ok(());
1077-
*monitor_c.update_ret.lock().unwrap() = Ok(());
1075+
*monitor_a.persister.update_ret.lock().unwrap() = Ok(());
1076+
*monitor_b.persister.update_ret.lock().unwrap() = Ok(());
1077+
*monitor_c.persister.update_ret.lock().unwrap() = Ok(());
10781078

10791079
if let Some((id, _)) = monitor_a.latest_monitors.lock().unwrap().get(&chan_1_funding) {
10801080
nodes[0].channel_monitor_updated(&chan_1_funding, *id);

‎fuzz/src/full_stack.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -365,7 +365,8 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
365365
};
366366

367367
let broadcast = Arc::new(TestBroadcaster{ txn_broadcasted: Mutex::new(Vec::new()) });
368-
let monitor = Arc::new(chainmonitor::ChainMonitor::new(None, broadcast.clone(), Arc::clone(&logger), fee_est.clone(), Arc::new(TestPersister{})));
368+
let monitor = Arc::new(chainmonitor::ChainMonitor::new(None, broadcast.clone(), Arc::clone(&logger), fee_est.clone(),
369+
Arc::new(TestPersister { update_ret: Mutex::new(Ok(())) })));
369370

370371
let keys_manager = Arc::new(KeyProvider { node_secret: our_network_key.clone(), counter: AtomicU64::new(0) });
371372
let mut config = UserConfig::default();

‎fuzz/src/utils/test_persister.rs

+12-7
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,19 @@
1-
use lightning::chain::channelmonitor;
1+
use lightning::chain;
2+
use lightning::chain::{chainmonitor, channelmonitor};
23
use lightning::chain::transaction::OutPoint;
34
use lightning::util::enforcing_trait_impls::EnforcingSigner;
45

5-
pub struct TestPersister {}
6-
impl channelmonitor::Persist<EnforcingSigner> for TestPersister {
7-
fn persist_new_channel(&self, _funding_txo: OutPoint, _data: &channelmonitor::ChannelMonitor<EnforcingSigner>) -> Result<(), channelmonitor::ChannelMonitorUpdateErr> {
8-
Ok(())
6+
use std::sync::Mutex;
7+
8+
pub struct TestPersister {
9+
pub update_ret: Mutex<Result<(), chain::ChannelMonitorUpdateErr>>,
10+
}
11+
impl chainmonitor::Persist<EnforcingSigner> for TestPersister {
12+
fn persist_new_channel(&self, _funding_txo: OutPoint, _data: &channelmonitor::ChannelMonitor<EnforcingSigner>) -> Result<(), chain::ChannelMonitorUpdateErr> {
13+
self.update_ret.lock().unwrap().clone()
914
}
1015

11-
fn update_persisted_channel(&self, _funding_txo: OutPoint, _update: &channelmonitor::ChannelMonitorUpdate, _data: &channelmonitor::ChannelMonitor<EnforcingSigner>) -> Result<(), channelmonitor::ChannelMonitorUpdateErr> {
12-
Ok(())
16+
fn update_persisted_channel(&self, _funding_txo: OutPoint, _update: &channelmonitor::ChannelMonitorUpdate, _data: &channelmonitor::ChannelMonitor<EnforcingSigner>) -> Result<(), chain::ChannelMonitorUpdateErr> {
17+
self.update_ret.lock().unwrap().clone()
1318
}
1419
}

‎lightning-background-processor/src/lib.rs

+2-3
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,7 @@
1010

1111
use lightning::chain;
1212
use lightning::chain::chaininterface::{BroadcasterInterface, FeeEstimator};
13-
use lightning::chain::chainmonitor::ChainMonitor;
14-
use lightning::chain::channelmonitor;
13+
use lightning::chain::chainmonitor::{ChainMonitor, Persist};
1514
use lightning::chain::keysinterface::{Sign, KeysInterface};
1615
use lightning::ln::channelmanager::ChannelManager;
1716
use lightning::ln::msgs::{ChannelMessageHandler, RoutingMessageHandler};
@@ -194,7 +193,7 @@ impl BackgroundProcessor {
194193
K::Target: 'static + KeysInterface<Signer = Signer>,
195194
F::Target: 'static + FeeEstimator,
196195
L::Target: 'static + Logger,
197-
P::Target: 'static + channelmonitor::Persist<Signer>,
196+
P::Target: 'static + Persist<Signer>,
198197
CMH::Target: 'static + ChannelMessageHandler,
199198
RMH::Target: 'static + RoutingMessageHandler,
200199
UMH::Target: 'static + CustomMessageHandler,

‎lightning-block-sync/src/init.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@ BlockSourceResult<ValidatedBlockHeader> {
4040
///
4141
/// use lightning::chain;
4242
/// use lightning::chain::Watch;
43+
/// use lightning::chain::chainmonitor;
4344
/// use lightning::chain::chainmonitor::ChainMonitor;
44-
/// use lightning::chain::channelmonitor;
4545
/// use lightning::chain::channelmonitor::ChannelMonitor;
4646
/// use lightning::chain::chaininterface::BroadcasterInterface;
4747
/// use lightning::chain::chaininterface::FeeEstimator;
@@ -65,7 +65,7 @@ BlockSourceResult<ValidatedBlockHeader> {
6565
/// F: FeeEstimator,
6666
/// L: Logger,
6767
/// C: chain::Filter,
68-
/// P: channelmonitor::Persist<S>,
68+
/// P: chainmonitor::Persist<S>,
6969
/// >(
7070
/// block_source: &mut B,
7171
/// chain_monitor: &ChainMonitor<S, &C, &T, &F, &L, &P>,

‎lightning-net-tokio/src/lib.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
//! type Logger = dyn lightning::util::logger::Logger + Send + Sync;
3535
//! type ChainAccess = dyn lightning::chain::Access + Send + Sync;
3636
//! type ChainFilter = dyn lightning::chain::Filter + Send + Sync;
37-
//! type DataPersister = dyn lightning::chain::channelmonitor::Persist<lightning::chain::keysinterface::InMemorySigner> + Send + Sync;
37+
//! type DataPersister = dyn lightning::chain::chainmonitor::Persist<lightning::chain::keysinterface::InMemorySigner> + Send + Sync;
3838
//! type ChainMonitor = lightning::chain::chainmonitor::ChainMonitor<lightning::chain::keysinterface::InMemorySigner, Arc<ChainFilter>, Arc<TxBroadcaster>, Arc<FeeEstimator>, Arc<Logger>, Arc<DataPersister>>;
3939
//! type ChannelManager = Arc<lightning::ln::channelmanager::SimpleArcChannelManager<ChainMonitor, TxBroadcaster, FeeEstimator, Logger>>;
4040
//! type PeerManager = Arc<lightning::ln::peer_handler::SimpleArcPeerManager<lightning_net_tokio::SocketDescriptor, ChainMonitor, TxBroadcaster, FeeEstimator, ChainAccess, Logger>>;

‎lightning-persister/src/lib.rs

+9-8
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ use bitcoin::hashes::hex::{FromHex, ToHex};
1717
use crate::util::DiskWriteable;
1818
use lightning::chain;
1919
use lightning::chain::chaininterface::{BroadcasterInterface, FeeEstimator};
20-
use lightning::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateErr};
21-
use lightning::chain::channelmonitor;
20+
use lightning::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate};
21+
use lightning::chain::chainmonitor;
2222
use lightning::chain::keysinterface::{Sign, KeysInterface};
2323
use lightning::chain::transaction::OutPoint;
2424
use lightning::ln::channelmanager::ChannelManager;
@@ -158,17 +158,17 @@ impl FilesystemPersister {
158158
}
159159
}
160160

161-
impl<ChannelSigner: Sign> channelmonitor::Persist<ChannelSigner> for FilesystemPersister {
162-
fn persist_new_channel(&self, funding_txo: OutPoint, monitor: &ChannelMonitor<ChannelSigner>) -> Result<(), ChannelMonitorUpdateErr> {
161+
impl<ChannelSigner: Sign> chainmonitor::Persist<ChannelSigner> for FilesystemPersister {
162+
fn persist_new_channel(&self, funding_txo: OutPoint, monitor: &ChannelMonitor<ChannelSigner>) -> Result<(), chain::ChannelMonitorUpdateErr> {
163163
let filename = format!("{}_{}", funding_txo.txid.to_hex(), funding_txo.index);
164164
util::write_to_file(self.path_to_monitor_data(), filename, monitor)
165-
.map_err(|_| ChannelMonitorUpdateErr::PermanentFailure)
165+
.map_err(|_| chain::ChannelMonitorUpdateErr::PermanentFailure)
166166
}
167167

168-
fn update_persisted_channel(&self, funding_txo: OutPoint, _update: &ChannelMonitorUpdate, monitor: &ChannelMonitor<ChannelSigner>) -> Result<(), ChannelMonitorUpdateErr> {
168+
fn update_persisted_channel(&self, funding_txo: OutPoint, _update: &ChannelMonitorUpdate, monitor: &ChannelMonitor<ChannelSigner>) -> Result<(), chain::ChannelMonitorUpdateErr> {
169169
let filename = format!("{}_{}", funding_txo.txid.to_hex(), funding_txo.index);
170170
util::write_to_file(self.path_to_monitor_data(), filename, monitor)
171-
.map_err(|_| ChannelMonitorUpdateErr::PermanentFailure)
171+
.map_err(|_| chain::ChannelMonitorUpdateErr::PermanentFailure)
172172
}
173173
}
174174

@@ -180,7 +180,8 @@ mod tests {
180180
use bitcoin::blockdata::block::{Block, BlockHeader};
181181
use bitcoin::hashes::hex::FromHex;
182182
use bitcoin::Txid;
183-
use lightning::chain::channelmonitor::{Persist, ChannelMonitorUpdateErr};
183+
use lightning::chain::ChannelMonitorUpdateErr;
184+
use lightning::chain::chainmonitor::Persist;
184185
use lightning::chain::transaction::OutPoint;
185186
use lightning::{check_closed_broadcast, check_closed_event, check_added_monitors};
186187
use lightning::ln::features::InitFeatures;

0 commit comments

Comments
 (0)