Skip to content

Commit 4b189bd

Browse files
committed
Allow deserialization of new Channels before we've seen a block
Previously, if we have a live ChannelManager (that has seen blocks) and we open a new Channel, if we serialize that ChannelManager before a new block comes in, we'll fail to deserialize it. This is the result of an overly-ambigious last_block_connected check which would see 0s for the new channel but the previous block for the ChannelManager as a whole. We add a new test which catches this error as well as hopefully getting some test coverage for other similar issues in the future.
1 parent ed2a5fd commit 4b189bd

File tree

2 files changed

+99
-1
lines changed

2 files changed

+99
-1
lines changed

lightning/src/ln/channelmanager.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3310,7 +3310,7 @@ impl<'a, R : ::std::io::Read, ChanSigner: ChannelKeys + Readable<R>, M: Deref> R
33103310
let mut short_to_id = HashMap::with_capacity(cmp::min(channel_count as usize, 128));
33113311
for _ in 0..channel_count {
33123312
let mut channel: Channel<ChanSigner> = ReadableArgs::read(reader, args.logger.clone())?;
3313-
if channel.last_block_connected != last_block_hash {
3313+
if channel.last_block_connected != Default::default() && channel.last_block_connected != last_block_hash {
33143314
return Err(DecodeError::InvalidValue);
33153315
}
33163316

lightning/src/ln/functional_tests.rs

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,104 @@ fn test_1_conf_open() {
413413
}
414414
}
415415

416+
fn do_test_sanity_on_in_flight_opens(steps: u8) {
417+
// Previously, we had issues deserializing channels when we hadn't connected the first block
418+
// after creation. To catch that and similar issues, we lean on the Node::drop impl to test
419+
// serialization round-trips and simply do steps towards opening a channel and then drop the
420+
// Node objects.
421+
422+
let node_cfgs = create_node_cfgs(2);
423+
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
424+
let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
425+
426+
if steps & 0b1000_0000 != 0{
427+
let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
428+
nodes[0].block_notifier.block_connected_checked(&header, 1, &Vec::new(), &[0; 0]);
429+
nodes[1].block_notifier.block_connected_checked(&header, 1, &Vec::new(), &[0; 0]);
430+
}
431+
432+
if steps & 0x0f == 0 { return; }
433+
nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100000, 10001, 42).unwrap();
434+
let open_channel = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id());
435+
436+
if steps & 0x0f == 1 { return; }
437+
nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), InitFeatures::supported(), &open_channel);
438+
let accept_channel = get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id());
439+
440+
if steps & 0x0f == 2 { return; }
441+
nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), InitFeatures::supported(), &accept_channel);
442+
443+
let (temporary_channel_id, tx, funding_output) = create_funding_transaction(&nodes[0], 100000, 42);
444+
445+
if steps & 0x0f == 3 { return; }
446+
{
447+
nodes[0].node.funding_transaction_generated(&temporary_channel_id, funding_output);
448+
let mut added_monitors = nodes[0].chan_monitor.added_monitors.lock().unwrap();
449+
assert_eq!(added_monitors.len(), 1);
450+
assert_eq!(added_monitors[0].0, funding_output);
451+
added_monitors.clear();
452+
}
453+
let funding_created = get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id());
454+
455+
if steps & 0x0f == 4 { return; }
456+
nodes[1].node.handle_funding_created(&nodes[0].node.get_our_node_id(), &funding_created);
457+
{
458+
let mut added_monitors = nodes[1].chan_monitor.added_monitors.lock().unwrap();
459+
assert_eq!(added_monitors.len(), 1);
460+
assert_eq!(added_monitors[0].0, funding_output);
461+
added_monitors.clear();
462+
}
463+
let funding_signed = get_event_msg!(nodes[1], MessageSendEvent::SendFundingSigned, nodes[0].node.get_our_node_id());
464+
465+
if steps & 0x0f == 5 { return; }
466+
nodes[0].node.handle_funding_signed(&nodes[1].node.get_our_node_id(), &funding_signed);
467+
{
468+
let mut added_monitors = nodes[0].chan_monitor.added_monitors.lock().unwrap();
469+
assert_eq!(added_monitors.len(), 1);
470+
assert_eq!(added_monitors[0].0, funding_output);
471+
added_monitors.clear();
472+
}
473+
474+
let events_4 = nodes[0].node.get_and_clear_pending_events();
475+
assert_eq!(events_4.len(), 1);
476+
match events_4[0] {
477+
Event::FundingBroadcastSafe { ref funding_txo, user_channel_id } => {
478+
assert_eq!(user_channel_id, 42);
479+
assert_eq!(*funding_txo, funding_output);
480+
},
481+
_ => panic!("Unexpected event"),
482+
};
483+
484+
if steps & 0x0f == 6 { return; }
485+
create_chan_between_nodes_with_value_confirm_first(&nodes[0], &nodes[1], &tx);
486+
487+
if steps & 0x0f == 7 { return; }
488+
confirm_transaction(&nodes[0].block_notifier, &nodes[0].chain_monitor, &tx, tx.version);
489+
create_chan_between_nodes_with_value_confirm_second(&nodes[1], &nodes[0]);
490+
}
491+
492+
#[test]
493+
fn test_sanity_on_in_flight_opens() {
494+
do_test_sanity_on_in_flight_opens(0);
495+
do_test_sanity_on_in_flight_opens(0 | 0b1000_0000);
496+
do_test_sanity_on_in_flight_opens(1);
497+
do_test_sanity_on_in_flight_opens(1 | 0b1000_0000);
498+
do_test_sanity_on_in_flight_opens(2);
499+
do_test_sanity_on_in_flight_opens(2 | 0b1000_0000);
500+
do_test_sanity_on_in_flight_opens(3);
501+
do_test_sanity_on_in_flight_opens(3 | 0b1000_0000);
502+
do_test_sanity_on_in_flight_opens(4);
503+
do_test_sanity_on_in_flight_opens(4 | 0b1000_0000);
504+
do_test_sanity_on_in_flight_opens(5);
505+
do_test_sanity_on_in_flight_opens(5 | 0b1000_0000);
506+
do_test_sanity_on_in_flight_opens(6);
507+
do_test_sanity_on_in_flight_opens(6 | 0b1000_0000);
508+
do_test_sanity_on_in_flight_opens(7);
509+
do_test_sanity_on_in_flight_opens(7 | 0b1000_0000);
510+
do_test_sanity_on_in_flight_opens(8);
511+
do_test_sanity_on_in_flight_opens(8 | 0b1000_0000);
512+
}
513+
416514
#[test]
417515
fn test_update_fee_vanilla() {
418516
let node_cfgs = create_node_cfgs(2);

0 commit comments

Comments
 (0)