Skip to content
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

close: Do not attempt a unilateral close when we see another close on-chain #7447

Merged

Conversation

cdecker
Copy link
Member

@cdecker cdecker commented Jul 2, 2024

In Greenlight we have an issue with channels that close, and the
signer got desynced from the Node due to a non-atomic update which
bumped the VLS signer commitment number, but the corresponding bump
got lost on CLN due to preemption. The problem here is that the signer
will not sign off on any future states, so the channel is unusable and
needs to be closed. If the peer closes the channel however, the last-
ditch attempt of publishing the last state, i.e., signing the n-1th
commitment TX, when VLS believes we are at the nth, won't work.

To get past this situation we selectively skip the unilateral close
attempt on our end, by setting passive=true when calling
drop_to_chain. This is safe because the way we got to this place is
by seeing our counterparty's unilateral close confirmed in a block we
are processing, therefore it is already too late to publish the
unilateral close anyway (it conflicts with the counterparty's close).

Changelog-Changed: close: We no longer attempt to publish a unilateral close that'd fail anyway when we witness a close onchain.

@cdecker cdecker self-assigned this Jul 2, 2024
@cdecker cdecker requested a review from rustyrussell July 2, 2024 14:58
@cdecker cdecker marked this pull request as draft July 2, 2024 14:59
@cdecker cdecker force-pushed the 20240702-skip-unilateral-on-onchain branch 2 times, most recently from fa4c915 to 6733a52 Compare July 3, 2024 14:18
@cdecker
Copy link
Member Author

cdecker commented Jul 13, 2024

I have removed the bip32_pubkey check before starting onchaind. @rustyrussell IIRC this was put in place in order to guard against the cosmic-ray-bit-flip in derived addresses we've heard stories from the LND team. I think this check could be added to onchaind such that it's the part that locks up, would you like me to re-add that check there?

@cdecker cdecker marked this pull request as ready for review July 19, 2024 08:19
@cdecker cdecker force-pushed the 20240702-skip-unilateral-on-onchain branch from b674556 to 73621ac Compare July 22, 2024 13:12
@cdecker
Copy link
Member Author

cdecker commented Jul 22, 2024

The passive argument was a bit weird, now it's called rebroadcast as that's closer to what we're actually trying to do.

cdecker added 3 commits July 29, 2024 16:50
Changelog-Changed: close: We no longer attempt to publish a unilateral close that'd fail anyway when we witness a close onchain.
The signer may not be present at this time. If we want to keep the
check to protect against bit flips we should move it into `onchaind`
where it doesn't matter as much that the signer may be slow to
respond.
@cdecker cdecker force-pushed the 20240702-skip-unilateral-on-onchain branch from e21471d to 47fea4f Compare July 29, 2024 14:50
@cdecker cdecker enabled auto-merge (rebase) July 29, 2024 14:52
@cdecker cdecker merged commit 10acbff into ElementsProject:master Jul 29, 2024
8 of 12 checks passed
@cdecker cdecker deleted the 20240702-skip-unilateral-on-onchain branch July 30, 2024 09:40
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.

1 participant