Skip to content

splice: prevent splice going to onchaind & race prevention - #6564

Merged
rustyrussell merged 2 commits into
ElementsProject:masterfrom
ddustin:ddustin/splice_close_fix
Aug 16, 2023
Merged

splice: prevent splice going to onchaind & race prevention#6564
rustyrussell merged 2 commits into
ElementsProject:masterfrom
ddustin:ddustin/splice_close_fix

Conversation

@ddustin

@ddustin ddustin commented Aug 14, 2023

Copy link
Copy Markdown
Collaborator

Don’t send the funding spend to onchaind if we detect it in inflights (aka. a splice). While we already prevented onchaind_funding_spent from being called directly, the call to wallet_channeltxs_add meant onchaind_funding_spent would be called anyway on restart. This is now fixed.

Additionally there was a potential for a race problem depending on the firing order of the channel depth and and funding spent events.

Instead of requiring these events fire in a specific order, we make a special “memory only” inflight object to prevent the race regardless of firing order.

Fixes #6533

@ddustin ddustin added this to the v23.08 milestone Aug 14, 2023
@ddustin
ddustin force-pushed the ddustin/splice_close_fix branch from 8b76aff to 5869458 Compare August 14, 2023 17:03
@rustyrussell

Copy link
Copy Markdown
Contributor

This broke everything? test_splice.py is now timing out on every run.

You need to write some real tests.

@ddustin
ddustin force-pushed the ddustin/splice_close_fix branch 3 times, most recently from ce20c0c to 0917e8e Compare August 15, 2023 01:36
@ddustin

ddustin commented Aug 15, 2023

Copy link
Copy Markdown
Collaborator Author

Added a regression test 7d54716 that will specifically check for what occured in Issue #6533 (splicing being detected as an incorrect channel close event, saving that event, and firing it upon restart).

@ddustin
ddustin force-pushed the ddustin/splice_close_fix branch 2 times, most recently from 15949ba to 6cd14d4 Compare August 15, 2023 02:38
Don’t send the funding spend to onchaind if we detect it in inflights (aka. a splice). While we already prevented onchaind_funding_spent from being called directly, the call to wallet_channeltxs_add meant onchaind_funding_spent would be called *anyway* on restart. This is now fixed.

Additionally there was a potential for a race problem depending on the firing order of the channel depth and and funding spent events.

Instead of requiring these events fire in a specific order, we make a special “memory only” inflight object to prevent the race regardless of firing order.

Changelog-Fixed: Splice: bugfix for restart related race condition interacting with adversarial close detection.
@ddustin
ddustin force-pushed the ddustin/splice_close_fix branch from 6cd14d4 to 7d54716 Compare August 15, 2023 04:39
@rustyrussell
rustyrussell merged commit c67f1f9 into ElementsProject:master Aug 16, 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.

errors after a restart after a successful splice

2 participants