Skip to content

Conversation

tnull
Copy link
Contributor

@tnull tnull commented Jul 21, 2023

A channel's short_channel_id is currently only set when the funding transaction is confirmed via transactions_confirmed, which might be well after the channel initially becomes usable, e.g., in the 0conf case.

Previously we would panic due to a reachable unwrap when receiving a counterparty's announcement_signatures message for a 0conf channel pending confirmation on-chain.

Here we fix this bug by avoiding unsafe unwraps and just erroring out and ignoring the announcement_signatures message if the short_channel_id hasn't been set yet.

tnull added 2 commits July 21, 2023 09:54
A channel's `short_channel_id` is currently only set when the funding
transaction is confirmed via `transactions_confirmed`, which might be
well after the channel initally becomes usable, e.g., in the 0conf case.

Previously we would panic due to a reachable `unwrap` when receiving a
counterparty's `announcement_signatures` message for a 0conf channel
pending confirmation on-chain.

Here we fix this bug by avoiding unsafe `unwrap`s and just erroring out
and ignoring the announcement_signatures message if the `short_channel_id`
hasn't been set yet.
While this is currently not reachable, it's still cleaner to
avoid the `unwrap` and return `None` if `short_channel_id` hasn't been
set yet.
@tnull tnull added this to the 0.0.116 milestone Jul 21, 2023
Copy link
Contributor

@jkczyz jkczyz left a comment

Choose a reason for hiding this comment

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

Tested ACK. New test fails without the fix.

Copy link
Collaborator

@TheBlueMatt TheBlueMatt left a comment

Choose a reason for hiding this comment

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

LGTM.

@TheBlueMatt TheBlueMatt merged commit d7e3320 into lightningdevkit:main Jul 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants