Skip to content

Change ChannelManager deserialization to return an optional blockhash #810

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions fuzz/src/chanmon_consistency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ impl chain::Watch<EnforcingSigner> for TestChainMonitor {
hash_map::Entry::Occupied(entry) => entry,
hash_map::Entry::Vacant(_) => panic!("Didn't have monitor on update call"),
};
let mut deserialized_monitor = <(BlockHash, channelmonitor::ChannelMonitor<EnforcingSigner>)>::
let mut deserialized_monitor = <(Option<BlockHash>, channelmonitor::ChannelMonitor<EnforcingSigner>)>::
read(&mut Cursor::new(&map_entry.get().1), &OnlyReadsKeysInterface {}).unwrap().1;
deserialized_monitor.update_monitor(&update, &&TestBroadcaster{}, &&FuzzEstimator{}, &self.logger).unwrap();
let mut ser = VecWriter(Vec::new());
Expand Down Expand Up @@ -337,7 +337,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
let mut monitors = HashMap::new();
let mut old_monitors = $old_monitors.latest_monitors.lock().unwrap();
for (outpoint, (update_id, monitor_ser)) in old_monitors.drain() {
monitors.insert(outpoint, <(BlockHash, ChannelMonitor<EnforcingSigner>)>::read(&mut Cursor::new(&monitor_ser), &OnlyReadsKeysInterface {}).expect("Failed to read monitor").1);
monitors.insert(outpoint, <(Option<BlockHash>, ChannelMonitor<EnforcingSigner>)>::read(&mut Cursor::new(&monitor_ser), &OnlyReadsKeysInterface {}).expect("Failed to read monitor").1);
chain_monitor.latest_monitors.lock().unwrap().insert(outpoint, (update_id, monitor_ser));
}
let mut monitor_refs = HashMap::new();
Expand All @@ -355,7 +355,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
channel_monitors: monitor_refs,
};

(<(BlockHash, ChanMan)>::read(&mut Cursor::new(&$ser.0), read_args).expect("Failed to read manager").1, chain_monitor)
(<(Option<BlockHash>, ChanMan)>::read(&mut Cursor::new(&$ser.0), read_args).expect("Failed to read manager").1, chain_monitor)
} }
}

Expand Down
8 changes: 5 additions & 3 deletions fuzz/src/chanmon_deser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,13 @@ impl Writer for VecWriter {

#[inline]
pub fn do_test<Out: test_logger::Output>(data: &[u8], _out: Out) {
if let Ok((latest_block_hash, monitor)) = <(BlockHash, channelmonitor::ChannelMonitor<EnforcingSigner>)>::read(&mut Cursor::new(data), &OnlyReadsKeysInterface {}) {
if let Ok((Some(latest_block_hash), monitor)) = <(Option<BlockHash>, channelmonitor::ChannelMonitor<EnforcingSigner>)>::read(&mut Cursor::new(data), &OnlyReadsKeysInterface {}) {
let mut w = VecWriter(Vec::new());
monitor.write(&mut w).unwrap();
let deserialized_copy = <(BlockHash, channelmonitor::ChannelMonitor<EnforcingSigner>)>::read(&mut Cursor::new(&w.0), &OnlyReadsKeysInterface {}).unwrap();
assert!(latest_block_hash == deserialized_copy.0);
let deserialized_copy = <(Option<BlockHash>, channelmonitor::ChannelMonitor<EnforcingSigner>)>::read(&mut Cursor::new(&w.0), &OnlyReadsKeysInterface {}).unwrap();
if let Some(deserialized) = deserialized_copy.0 {
assert!(latest_block_hash == deserialized);
}
assert!(monitor == deserialized_copy.1);
}
}
Expand Down
17 changes: 10 additions & 7 deletions lightning-block-sync/src/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,12 @@ use lightning::chain;
/// ) {
/// // Read a serialized channel monitor paired with the block hash when it was persisted.
/// let serialized_monitor = "...";
/// let (monitor_block_hash, mut monitor) = <(BlockHash, ChannelMonitor<S>)>::read(
/// let (monitor_block_hash_option, mut monitor) = <(Option<BlockHash>, ChannelMonitor<S>)>::read(
/// &mut Cursor::new(&serialized_monitor), keys_manager).unwrap();
///
/// // Read the channel manager paired with the block hash when it was persisted.
/// let serialized_manager = "...";
/// let (manager_block_hash, mut manager) = {
/// let (manager_block_hash_option, mut manager) = {
/// let read_args = ChannelManagerReadArgs::new(
/// keys_manager,
/// fee_estimator,
Expand All @@ -77,17 +77,20 @@ use lightning::chain;
/// config,
/// vec![&mut monitor],
/// );
/// <(BlockHash, ChannelManager<S, &ChainMonitor<S, &C, &T, &F, &L, &P>, &T, &K, &F, &L>)>::read(
/// <(Option<BlockHash>, ChannelManager<S, &ChainMonitor<S, &C, &T, &F, &L, &P>, &T, &K, &F, &L>)>::read(
/// &mut Cursor::new(&serialized_manager), read_args).unwrap()
/// };
///
/// // Synchronize any channel monitors and the channel manager to be on the best block.
/// let mut cache = UnboundedCache::new();
/// let mut monitor_listener = (RefCell::new(monitor), &*tx_broadcaster, &*fee_estimator, &*logger);
/// let listeners = vec![
/// (monitor_block_hash, &mut monitor_listener as &mut dyn chain::Listen),
/// (manager_block_hash, &mut manager as &mut dyn chain::Listen),
/// ];
/// let mut listeners = vec![];
/// if let Some(monitor_block_hash) = monitor_block_hash_option {
/// listeners.push((monitor_block_hash, &mut monitor_listener as &mut dyn chain::Listen))
/// }
/// if let Some(manager_block_hash) = manager_block_hash_option {
/// listeners.push((manager_block_hash, &mut manager as &mut dyn chain::Listen))
/// }
/// let chain_tip = init::synchronize_listeners(
/// block_source, Network::Bitcoin, &mut cache, listeners).await.unwrap();
///
Expand Down
2 changes: 1 addition & 1 deletion lightning-persister/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ impl FilesystemPersister {
if contents.is_err() { return Err(ChannelMonitorUpdateErr::PermanentFailure); }

if let Ok((_, loaded_monitor)) =
<(BlockHash, ChannelMonitor<Keys::Signer>)>::read(&mut Cursor::new(&contents.unwrap()), keys) {
<(Option<BlockHash>, ChannelMonitor<Keys::Signer>)>::read(&mut Cursor::new(&contents.unwrap()), keys) {
res.insert(OutPoint { txid: txid.unwrap(), index: index.unwrap() }, loaded_monitor);
} else {
return Err(ChannelMonitorUpdateErr::PermanentFailure);
Expand Down
9 changes: 7 additions & 2 deletions lightning/src/chain/channelmonitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2317,7 +2317,7 @@ where
const MAX_ALLOC_SIZE: usize = 64*1024;

impl<'a, Signer: Sign, K: KeysInterface<Signer = Signer>> ReadableArgs<&'a K>
for (BlockHash, ChannelMonitor<Signer>) {
for (Option<BlockHash>, ChannelMonitor<Signer>) {
fn read<R: ::std::io::Read>(reader: &mut R, keys_manager: &'a K) -> Result<Self, DecodeError> {
macro_rules! unwrap_obj {
($key: expr) => {
Expand Down Expand Up @@ -2560,7 +2560,12 @@ impl<'a, Signer: Sign, K: KeysInterface<Signer = Signer>> ReadableArgs<&'a K>
let mut secp_ctx = Secp256k1::new();
secp_ctx.seeded_randomize(&keys_manager.get_secure_random_bytes());

Ok((last_block_hash.clone(), ChannelMonitor {
let mut last_seen_block_hash = Some(last_block_hash.clone());
if last_block_hash == Default::default() {
last_seen_block_hash = None;
}

Ok((last_seen_block_hash, ChannelMonitor {
latest_update_id,
commitment_transaction_number_obscure_factor,

Expand Down
2 changes: 1 addition & 1 deletion lightning/src/ln/chanmon_update_fail_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ fn test_monitor_and_persister_update_fail() {
let monitor = monitors.get(&outpoint).unwrap();
let mut w = test_utils::TestVecWriter(Vec::new());
monitor.write(&mut w).unwrap();
let new_monitor = <(BlockHash, ChannelMonitor<EnforcingSigner>)>::read(
let new_monitor = <(Option<BlockHash>, ChannelMonitor<EnforcingSigner>)>::read(
&mut ::std::io::Cursor::new(&w.0), &test_utils::OnlyReadsKeysInterface {}).unwrap().1;
assert!(new_monitor == *monitor);
let chain_mon = test_utils::TestChainMonitor::new(Some(&chain_source), &chanmon_cfgs[0].tx_broadcaster, &logger, &chanmon_cfgs[0].fee_estimator, &persister, &node_cfgs[0].keys_manager);
Expand Down
12 changes: 8 additions & 4 deletions lightning/src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4006,21 +4006,21 @@ impl<'a, Signer: 'a + Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
// Implement ReadableArgs for an Arc'd ChannelManager to make it a bit easier to work with the
// SipmleArcChannelManager type:
impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
ReadableArgs<ChannelManagerReadArgs<'a, Signer, M, T, K, F, L>> for (BlockHash, Arc<ChannelManager<Signer, M, T, K, F, L>>)
ReadableArgs<ChannelManagerReadArgs<'a, Signer, M, T, K, F, L>> for (Option<BlockHash>, Arc<ChannelManager<Signer, M, T, K, F, L>>)
where M::Target: chain::Watch<Signer>,
T::Target: BroadcasterInterface,
K::Target: KeysInterface<Signer = Signer>,
F::Target: FeeEstimator,
L::Target: Logger,
{
fn read<R: ::std::io::Read>(reader: &mut R, args: ChannelManagerReadArgs<'a, Signer, M, T, K, F, L>) -> Result<Self, DecodeError> {
let (blockhash, chan_manager) = <(BlockHash, ChannelManager<Signer, M, T, K, F, L>)>::read(reader, args)?;
let (blockhash, chan_manager) = <(Option<BlockHash>, ChannelManager<Signer, M, T, K, F, L>)>::read(reader, args)?;
Ok((blockhash, Arc::new(chan_manager)))
}
}

impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
ReadableArgs<ChannelManagerReadArgs<'a, Signer, M, T, K, F, L>> for (BlockHash, ChannelManager<Signer, M, T, K, F, L>)
ReadableArgs<ChannelManagerReadArgs<'a, Signer, M, T, K, F, L>> for (Option<BlockHash>, ChannelManager<Signer, M, T, K, F, L>)
where M::Target: chain::Watch<Signer>,
T::Target: BroadcasterInterface,
K::Target: KeysInterface<Signer = Signer>,
Expand Down Expand Up @@ -4172,7 +4172,11 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
//TODO: Broadcast channel update for closed channels, but only after we've made a
//connection or two.

Ok((last_block_hash.clone(), channel_manager))
let mut last_seen_block_hash = Some(last_block_hash.clone());
if last_block_hash == Default::default() {
last_seen_block_hash = None;
}
Ok((last_seen_block_hash, channel_manager))
}
}

Expand Down
4 changes: 2 additions & 2 deletions lightning/src/ln/functional_test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ impl<'a, 'b, 'c> Drop for Node<'a, 'b, 'c> {
for (_, old_monitor) in old_monitors.iter() {
let mut w = test_utils::TestVecWriter(Vec::new());
old_monitor.write(&mut w).unwrap();
let (_, deserialized_monitor) = <(BlockHash, ChannelMonitor<EnforcingSigner>)>::read(
let (_, deserialized_monitor) = <(Option<BlockHash>, ChannelMonitor<EnforcingSigner>)>::read(
&mut ::std::io::Cursor::new(&w.0), self.keys_manager).unwrap();
deserialized_monitors.push(deserialized_monitor);
}
Expand All @@ -188,7 +188,7 @@ impl<'a, 'b, 'c> Drop for Node<'a, 'b, 'c> {

let mut w = test_utils::TestVecWriter(Vec::new());
self.node.write(&mut w).unwrap();
<(BlockHash, ChannelManager<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>)>::read(&mut ::std::io::Cursor::new(w.0), ChannelManagerReadArgs {
<(Option<BlockHash>, ChannelManager<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>)>::read(&mut ::std::io::Cursor::new(w.0), ChannelManagerReadArgs {
default_config: UserConfig::default(),
keys_manager: self.keys_manager,
fee_estimator: &test_utils::TestFeeEstimator { sat_per_kw: 253 },
Expand Down
Loading