Skip to content

Fix spurious panic on receipt of a block while awaiting funding #1711

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

Merged
merged 1 commit into from
Sep 9, 2022
Merged
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
5 changes: 4 additions & 1 deletion lightning/src/ln/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4755,6 +4755,9 @@ impl<Signer: Sign> Channel<Signer> {
}

fn check_get_channel_ready(&mut self, height: u32) -> Option<msgs::ChannelReady> {
// Called:
// * always when a new block/transactions are confirmed with the new height
// * when funding is signed with a height of 0
if self.funding_tx_confirmation_height == 0 && self.minimum_depth != Some(0) {
return None;
}
Expand All @@ -4780,7 +4783,7 @@ impl<Signer: Sign> Channel<Signer> {
// We got a reorg but not enough to trigger a force close, just ignore.
false
} else {
if self.channel_state < ChannelState::ChannelFunded as u32 {
if self.funding_tx_confirmation_height != 0 && self.channel_state < ChannelState::ChannelFunded as u32 {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: comment could be stronger here it's preventing to hit the panic when we receive a transactions_confirmed() in the 0conf case as raised in pre-review.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean the top comment? I'm less worried about a new panic here, and more worried about someone breaking the logic. Its all a bit hairy, so good to be aware of the cases we have to handle correctly, panic or not.

// We should never see a funding transaction on-chain until we've received
// funding_signed (if we're an outbound channel), or seen funding_generated (if we're
// an inbound channel - before that we have no known funding TXID). The fuzzer,
Expand Down
42 changes: 42 additions & 0 deletions lightning/src/ln/priv_short_conf_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1021,3 +1021,45 @@ fn test_zero_conf_accept_reject() {
_ => panic!(),
}
}

#[test]
fn test_connect_before_funding() {
// Tests for a particularly dumb explicit panic that existed prior to 0.0.111 for 0conf
// channels. If we received a block while awaiting funding for 0-conf channels we'd hit an
// explicit panic when deciding if we should broadcast our channel_ready message.
let chanmon_cfgs = create_chanmon_cfgs(2);
let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);

let mut manually_accept_conf = test_default_channel_config();
manually_accept_conf.manually_accept_inbound_channels = true;

let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, Some(manually_accept_conf)]);
let nodes = create_network(2, &node_cfgs, &node_chanmgrs);

nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100_000, 10_001, 42, None).unwrap();
let open_channel = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id());

nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), InitFeatures::known(), &open_channel);
let events = nodes[1].node.get_and_clear_pending_events();
assert_eq!(events.len(), 1);
match events[0] {
Event::OpenChannelRequest { temporary_channel_id, .. } => {
nodes[1].node.accept_inbound_channel_from_trusted_peer_0conf(&temporary_channel_id, &nodes[0].node.get_our_node_id(), 0).unwrap();
},
_ => panic!("Unexpected event"),
};

let mut accept_channel = get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id());
assert_eq!(accept_channel.minimum_depth, 0);
nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), InitFeatures::known(), &accept_channel);

let events = nodes[0].node.get_and_clear_pending_events();
assert_eq!(events.len(), 1);
match events[0] {
Event::FundingGenerationReady { .. } => {},
_ => panic!("Unexpected event"),
}

connect_blocks(&nodes[0], 1);
connect_blocks(&nodes[1], 1);
}