Skip to content

Commit 7f7fee5

Browse files
committed
Read monitors from our KeysInterface in chanmon_consistency_fuzz
If the fuzz target is failing due to a channel force-close, the immediately-visible error is that we're signing a stale state. This is because the ChannelMonitorUpdateStep::ChannelForceClosed event results in a signature in the test clone which was deserialized using a OnlyReadsKeysInterface. Instead, we need to deserialize using the full KeysInterface instance.
1 parent eb481d3 commit 7f7fee5

File tree

1 file changed

+16
-11
lines changed

1 file changed

+16
-11
lines changed

fuzz/src/chanmon_consistency.rs

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,7 @@ use bitcoin::hashes::sha256::Hash as Sha256;
2929
use bitcoin::hash_types::{BlockHash, WPubkeyHash};
3030

3131
use lightning::chain;
32-
use lightning::chain::chainmonitor;
33-
use lightning::chain::channelmonitor;
32+
use lightning::chain::{chainmonitor, channelmonitor, Watch};
3433
use lightning::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdateErr, MonitorEvent};
3534
use lightning::chain::transaction::OutPoint;
3635
use lightning::chain::chaininterface::{BroadcasterInterface, ConfirmationTarget, FeeEstimator};
@@ -45,7 +44,6 @@ use lightning::util::logger::Logger;
4544
use lightning::util::config::UserConfig;
4645
use lightning::util::events::{EventsProvider, MessageSendEventsProvider};
4746
use lightning::util::ser::{Readable, ReadableArgs, Writeable, Writer};
48-
use lightning::util::test_utils::OnlyReadsKeysInterface;
4947
use lightning::routing::router::{Route, RouteHop};
5048

5149

@@ -87,6 +85,7 @@ impl Writer for VecWriter {
8785

8886
struct TestChainMonitor {
8987
pub logger: Arc<dyn Logger>,
88+
pub keys: Arc<KeyProvider>,
9089
pub chain_monitor: Arc<chainmonitor::ChainMonitor<EnforcingChannelKeys, Arc<dyn chain::Filter>, Arc<TestBroadcaster>, Arc<FuzzEstimator>, Arc<dyn Logger>, Arc<TestPersister>>>,
9190
pub update_ret: Mutex<Result<(), channelmonitor::ChannelMonitorUpdateErr>>,
9291
// If we reload a node with an old copy of ChannelMonitors, the ChannelManager deserialization
@@ -98,17 +97,18 @@ struct TestChainMonitor {
9897
pub should_update_manager: atomic::AtomicBool,
9998
}
10099
impl TestChainMonitor {
101-
pub fn new(broadcaster: Arc<TestBroadcaster>, logger: Arc<dyn Logger>, feeest: Arc<FuzzEstimator>, persister: Arc<TestPersister>) -> Self {
100+
pub fn new(broadcaster: Arc<TestBroadcaster>, logger: Arc<dyn Logger>, feeest: Arc<FuzzEstimator>, persister: Arc<TestPersister>, keys: Arc<KeyProvider>) -> Self {
102101
Self {
103102
chain_monitor: Arc::new(chainmonitor::ChainMonitor::new(None, broadcaster, logger.clone(), feeest, persister)),
104103
logger,
104+
keys,
105105
update_ret: Mutex::new(Ok(())),
106106
latest_monitors: Mutex::new(HashMap::new()),
107107
should_update_manager: atomic::AtomicBool::new(false),
108108
}
109109
}
110110
}
111-
impl chain::Watch for TestChainMonitor {
111+
impl Watch for TestChainMonitor {
112112
type Keys = EnforcingChannelKeys;
113113

114114
fn watch_channel(&self, funding_txo: OutPoint, monitor: channelmonitor::ChannelMonitor<EnforcingChannelKeys>) -> Result<(), channelmonitor::ChannelMonitorUpdateErr> {
@@ -129,12 +129,13 @@ impl chain::Watch for TestChainMonitor {
129129
hash_map::Entry::Vacant(_) => panic!("Didn't have monitor on update call"),
130130
};
131131
let mut deserialized_monitor = <(BlockHash, channelmonitor::ChannelMonitor<EnforcingChannelKeys>)>::
132-
read(&mut Cursor::new(&map_entry.get().1), &OnlyReadsKeysInterface {}).unwrap().1;
132+
read(&mut Cursor::new(&map_entry.get().1), &*self.keys).unwrap().1;
133133
deserialized_monitor.update_monitor(&update, &&TestBroadcaster{}, &&FuzzEstimator{}, &self.logger).unwrap();
134134
let mut ser = VecWriter(Vec::new());
135135
deserialized_monitor.write(&mut ser).unwrap();
136136
map_entry.insert((update.update_id, ser.0));
137137
self.should_update_manager.store(true, atomic::Ordering::Relaxed);
138+
assert!(self.chain_monitor.update_channel(funding_txo, update).is_ok());
138139
self.update_ret.lock().unwrap().clone()
139140
}
140141

@@ -313,9 +314,9 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
313314
macro_rules! make_node {
314315
($node_id: expr) => { {
315316
let logger: Arc<dyn Logger> = Arc::new(test_logger::TestLogger::new($node_id.to_string(), out.clone()));
316-
let monitor = Arc::new(TestChainMonitor::new(broadcast.clone(), logger.clone(), fee_est.clone(), Arc::new(TestPersister{})));
317-
318317
let keys_manager = Arc::new(KeyProvider { node_id: $node_id, rand_bytes_id: atomic::AtomicU8::new(0), revoked_commitments: Mutex::new(HashMap::new()) });
318+
let monitor = Arc::new(TestChainMonitor::new(broadcast.clone(), logger.clone(), fee_est.clone(), Arc::new(TestPersister{}), Arc::clone(&keys_manager)));
319+
319320
let mut config = UserConfig::default();
320321
config.channel_options.fee_proportional_millionths = 0;
321322
config.channel_options.announced_channel = true;
@@ -329,7 +330,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
329330
($ser: expr, $node_id: expr, $old_monitors: expr, $keys_manager: expr) => { {
330331
let keys_manager = Arc::clone(& $keys_manager);
331332
let logger: Arc<dyn Logger> = Arc::new(test_logger::TestLogger::new($node_id.to_string(), out.clone()));
332-
let chain_monitor = Arc::new(TestChainMonitor::new(broadcast.clone(), logger.clone(), fee_est.clone(), Arc::new(TestPersister{})));
333+
let chain_monitor = Arc::new(TestChainMonitor::new(broadcast.clone(), logger.clone(), fee_est.clone(), Arc::new(TestPersister{}), Arc::clone(& $keys_manager)));
333334

334335
let mut config = UserConfig::default();
335336
config.channel_options.fee_proportional_millionths = 0;
@@ -339,7 +340,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
339340
let mut monitors = HashMap::new();
340341
let mut old_monitors = $old_monitors.latest_monitors.lock().unwrap();
341342
for (outpoint, (update_id, monitor_ser)) in old_monitors.drain() {
342-
monitors.insert(outpoint, <(BlockHash, ChannelMonitor<EnforcingChannelKeys>)>::read(&mut Cursor::new(&monitor_ser), &OnlyReadsKeysInterface {}).expect("Failed to read monitor").1);
343+
monitors.insert(outpoint, <(BlockHash, ChannelMonitor<EnforcingChannelKeys>)>::read(&mut Cursor::new(&monitor_ser), &*$keys_manager).expect("Failed to read monitor").1);
343344
chain_monitor.latest_monitors.lock().unwrap().insert(outpoint, (update_id, monitor_ser));
344345
}
345346
let mut monitor_refs = HashMap::new();
@@ -357,7 +358,11 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
357358
channel_monitors: monitor_refs,
358359
};
359360

360-
(<(BlockHash, ChanMan)>::read(&mut Cursor::new(&$ser.0), read_args).expect("Failed to read manager").1, chain_monitor)
361+
let res = (<(BlockHash, ChanMan)>::read(&mut Cursor::new(&$ser.0), read_args).expect("Failed to read manager").1, chain_monitor.clone());
362+
for (funding_txo, mon) in monitors.drain() {
363+
assert!(chain_monitor.chain_monitor.watch_channel(funding_txo, mon).is_ok());
364+
}
365+
res
361366
} }
362367
}
363368

0 commit comments

Comments
 (0)