Skip to content

Commit 35a311c

Browse files
valentinewallacevalentinewallace
committed
Change ChannelManager deserialization to return an optional blockhash
If the ChannelManager never receives any blocks, it'll return a default blockhash on deserialization. It's preferable for this to be an Option instead.
1 parent e77b160 commit 35a311c

File tree

5 files changed

+22
-16
lines changed

5 files changed

+22
-16
lines changed

‎fuzz/src/chanmon_consistency.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
355355
channel_monitors: monitor_refs,
356356
};
357357

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

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ use lightning::chain;
6767
///
6868
/// // Read the channel manager paired with the block hash when it was persisted.
6969
/// let serialized_manager = "...";
70-
/// let (manager_block_hash, mut manager) = {
70+
/// let (manager_block_hash_option, mut manager) = {
7171
/// let read_args = ChannelManagerReadArgs::new(
7272
/// keys_manager,
7373
/// fee_estimator,
@@ -77,17 +77,19 @@ use lightning::chain;
7777
/// config,
7878
/// vec![&mut monitor],
7979
/// );
80-
/// <(BlockHash, ChannelManager<S, &ChainMonitor<S, &C, &T, &F, &L, &P>, &T, &K, &F, &L>)>::read(
80+
/// <(Option<BlockHash>, ChannelManager<S, &ChainMonitor<S, &C, &T, &F, &L, &P>, &T, &K, &F, &L>)>::read(
8181
/// &mut Cursor::new(&serialized_manager), read_args).unwrap()
8282
/// };
8383
///
8484
/// // Synchronize any channel monitors and the channel manager to be on the best block.
8585
/// let mut cache = UnboundedCache::new();
8686
/// let mut monitor_listener = (RefCell::new(monitor), &*tx_broadcaster, &*fee_estimator, &*logger);
87-
/// let listeners = vec![
87+
/// let mut listeners = vec![
8888
/// (monitor_block_hash, &mut monitor_listener as &mut dyn chain::Listen),
89-
/// (manager_block_hash, &mut manager as &mut dyn chain::Listen),
9089
/// ];
90+
/// if let Some(manager_block_hash) = manager_block_hash_option {
91+
/// listeners.push((manager_block_hash, &mut manager as &mut dyn chain::Listen))
92+
/// }
9193
/// let chain_tip = init::synchronize_listeners(
9294
/// block_source, Network::Bitcoin, &mut cache, listeners).await.unwrap();
9395
///

‎lightning/src/ln/channelmanager.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4005,21 +4005,21 @@ impl<'a, Signer: 'a + Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
40054005
// Implement ReadableArgs for an Arc'd ChannelManager to make it a bit easier to work with the
40064006
// SipmleArcChannelManager type:
40074007
impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
4008-
ReadableArgs<ChannelManagerReadArgs<'a, Signer, M, T, K, F, L>> for (BlockHash, Arc<ChannelManager<Signer, M, T, K, F, L>>)
4008+
ReadableArgs<ChannelManagerReadArgs<'a, Signer, M, T, K, F, L>> for (Option<BlockHash>, Arc<ChannelManager<Signer, M, T, K, F, L>>)
40094009
where M::Target: chain::Watch<Signer>,
40104010
T::Target: BroadcasterInterface,
40114011
K::Target: KeysInterface<Signer = Signer>,
40124012
F::Target: FeeEstimator,
40134013
L::Target: Logger,
40144014
{
40154015
fn read<R: ::std::io::Read>(reader: &mut R, args: ChannelManagerReadArgs<'a, Signer, M, T, K, F, L>) -> Result<Self, DecodeError> {
4016-
let (blockhash, chan_manager) = <(BlockHash, ChannelManager<Signer, M, T, K, F, L>)>::read(reader, args)?;
4016+
let (blockhash, chan_manager) = <(Option<BlockHash>, ChannelManager<Signer, M, T, K, F, L>)>::read(reader, args)?;
40174017
Ok((blockhash, Arc::new(chan_manager)))
40184018
}
40194019
}
40204020

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

4171-
Ok((last_block_hash.clone(), channel_manager))
4171+
let mut last_seen_block_hash = Some(last_block_hash.clone());
4172+
if last_block_hash == Default::default() {
4173+
last_seen_block_hash = None;
4174+
}
4175+
Ok((last_seen_block_hash, channel_manager))
41724176
}
41734177
}
41744178

‎lightning/src/ln/functional_test_utils.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ impl<'a, 'b, 'c> Drop for Node<'a, 'b, 'c> {
188188

189189
let mut w = test_utils::TestVecWriter(Vec::new());
190190
self.node.write(&mut w).unwrap();
191-
<(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 {
191+
<(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 {
192192
default_config: UserConfig::default(),
193193
keys_manager: self.keys_manager,
194194
fee_estimator: &test_utils::TestFeeEstimator { sat_per_kw: 253 },

‎lightning/src/ln/functional_tests.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4332,7 +4332,7 @@ fn test_no_txn_manager_serialize_deserialize() {
43324332
let (_, nodes_0_deserialized_tmp) = {
43334333
let mut channel_monitors = HashMap::new();
43344334
channel_monitors.insert(chan_0_monitor.get_funding_txo().0, &mut chan_0_monitor);
4335-
<(BlockHash, ChannelManager<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>)>::read(&mut nodes_0_read, ChannelManagerReadArgs {
4335+
<(Option<BlockHash>, ChannelManager<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>)>::read(&mut nodes_0_read, ChannelManagerReadArgs {
43364336
default_config: config,
43374337
keys_manager,
43384338
fee_estimator: &fee_estimator,
@@ -4441,7 +4441,7 @@ fn test_manager_serialize_deserialize_events() {
44414441
let (_, nodes_0_deserialized_tmp) = {
44424442
let mut channel_monitors = HashMap::new();
44434443
channel_monitors.insert(chan_0_monitor.get_funding_txo().0, &mut chan_0_monitor);
4444-
<(BlockHash, ChannelManager<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>)>::read(&mut nodes_0_read, ChannelManagerReadArgs {
4444+
<(Option<BlockHash>, ChannelManager<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>)>::read(&mut nodes_0_read, ChannelManagerReadArgs {
44454445
default_config: config,
44464446
keys_manager,
44474447
fee_estimator: &fee_estimator,
@@ -4532,7 +4532,7 @@ fn test_simple_manager_serialize_deserialize() {
45324532
let (_, nodes_0_deserialized_tmp) = {
45334533
let mut channel_monitors = HashMap::new();
45344534
channel_monitors.insert(chan_0_monitor.get_funding_txo().0, &mut chan_0_monitor);
4535-
<(BlockHash, ChannelManager<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>)>::read(&mut nodes_0_read, ChannelManagerReadArgs {
4535+
<(Option<BlockHash>, ChannelManager<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>)>::read(&mut nodes_0_read, ChannelManagerReadArgs {
45364536
default_config: UserConfig::default(),
45374537
keys_manager,
45384538
fee_estimator: &fee_estimator,
@@ -4623,7 +4623,7 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() {
46234623

46244624
let mut nodes_0_read = &nodes_0_serialized[..];
46254625
if let Err(msgs::DecodeError::InvalidValue) =
4626-
<(BlockHash, ChannelManager<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>)>::read(&mut nodes_0_read, ChannelManagerReadArgs {
4626+
<(Option<BlockHash>, ChannelManager<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>)>::read(&mut nodes_0_read, ChannelManagerReadArgs {
46274627
default_config: UserConfig::default(),
46284628
keys_manager,
46294629
fee_estimator: &fee_estimator,
@@ -4637,7 +4637,7 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() {
46374637

46384638
let mut nodes_0_read = &nodes_0_serialized[..];
46394639
let (_, nodes_0_deserialized_tmp) =
4640-
<(BlockHash, ChannelManager<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>)>::read(&mut nodes_0_read, ChannelManagerReadArgs {
4640+
<(Option<BlockHash>, ChannelManager<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>)>::read(&mut nodes_0_read, ChannelManagerReadArgs {
46414641
default_config: UserConfig::default(),
46424642
keys_manager,
46434643
fee_estimator: &fee_estimator,
@@ -7498,7 +7498,7 @@ fn test_data_loss_protect() {
74987498
node_state_0 = {
74997499
let mut channel_monitors = HashMap::new();
75007500
channel_monitors.insert(OutPoint { txid: chan.3.txid(), index: 0 }, &mut chain_monitor);
7501-
<(BlockHash, ChannelManager<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>)>::read(&mut ::std::io::Cursor::new(previous_node_state), ChannelManagerReadArgs {
7501+
<(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(previous_node_state), ChannelManagerReadArgs {
75027502
keys_manager: keys_manager,
75037503
fee_estimator: &fee_estimator,
75047504
chain_monitor: &monitor,

0 commit comments

Comments
 (0)