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

pytest: Stabilize flaky test_splice #6810

Closed
cdecker opened this issue Oct 24, 2023 · 6 comments · Fixed by #6818
Closed

pytest: Stabilize flaky test_splice #6810

cdecker opened this issue Oct 24, 2023 · 6 comments · Fixed by #6818
Assignees

Comments

@cdecker
Copy link
Member

cdecker commented Oct 24, 2023

test_splice was marked flaky via @flaky()
in #6788 because it was flaky. I'm assigning the author with the most
lines in git blame, under the assumption that they'd have the most
context to stabilize the flaky test, but please assign other devs if
they are better suited to fix the stability issue.

Logs

https://github.com/ElementsProject/lightning/actions/runs/6550388862/job/17790327177

ERROR tests/test_splicing.py::test_splice - ValueError: 
Node errors:
 - lightningd-1: had BROKEN messages
Global errors:
2023-10-17T18:08:35.1279101Z lightningd-1 2023-10-17T17:28:56.575Z **BROKEN** 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-gossipd: invalid local_channel_announcement 0bbe022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d5901b0010078c3314666731e339c0b8434f7824797a084ed7ca3655991a672da068e2c44cb53b57b53a296c133bc879109a8931dc31e6913a4bda3d58559b99b95663e6d526de1a52ab38f0c0e2c18c3bc425ab686f61d5c2237a6d67885209ccb1d72a4f92a8021c8d7608e5ad835b604f43c4638331ab8f8a07afe5dcb5b2456d0fe900f679cabaccc6e39dbd95a4dac90e75a258893c3aa3f733d1b8890174d5ddea8003cadffe557773c54d2c07ca1d535c4bf85885f879ae466c16a516e8ffcfec1741088fb5422d3aee7807581c14f8e8cc320d5e0af92818d59fe1c7482e30369bb4680fb9f3da8aa7bc046a20fe834560979c84a128974378fa29f1a716c344e05000006226e46111a0b59caaf126043eb5bbf28c34f3a5e332a1fc7b2b73cf188910f00006d0000010000022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d590266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c0351802e3bd38009866c9da8ec4aa99cc4ea9c6c0dd46df15c61ef0ce1f271291714e5702324266de8403b3ab157a09f1f784d587af61831c998c151bcc21bb74c2b2314b (000100000000000000000000000000000000000000000000000000000000000000000460426164206e6f64655f7369676e61747572655f3120333034343032323037386333333134363636373331653333396330623834333466373832343739376130383465643763613336353539393161363732646130363865326334346362303232303533623537623533613239366331333362633837393130396138393331646333316536393133613462646133643538353539623939623935363633653664353220686173682038393364613434613233333736373834386333363164653438656664323061333166646161303632393366623336643864353937363438393864353731633961206f6e206368616e6e656c5f616e6e6f756e63656d656e7420303130303738633333313436363637333165333339633062383433346637383234373937613038346564376361333635353939316136373264613036386532633434636235336235376235336132393663313333626338373931303961383933316463333165363931336134626461336435383535396239396239353636336536643532366465316135326162333866306330653263313863336263343235616236383666363164356332323337613664363738383532303963636231643732613466393261383032316338643736303865356164383335623630346634336334363338333331616238663861303761666535646362356232343536643066653930306636373963616261636363366533396462643935613464616339306537356132353838393363336161336637333364316238383930313734643564646561383030336361646666653535373737336335346432633037636131643533356334626638353838356638373961653436366331366135313665386666636665633137343130383866623534323264336165653738303735383163313466386538636333323064356530616639323831386435396665316337343832653330333639626234363830666239663364613861613762633034366132306665383334353630393739633834613132383937343337386661323966316137313663333434653035303030303036323236653436313131613062353963616166313236303433656235626266323863333466336135653333326131666337623262373363663138383931306630303030366430303030303130303030303232643232333632306133353961343766663766376163343437633835633436633932336461353333383932323161303035346331316331653363613331643539303236366534353938643164336334313566353732613834383838333062363066376537343465643932333565623062316261393332383362333135633033353138303265336264333830303938363663396461386563346161393963633465613963366330646434366466313563363165663063653166323731323931373134653537303233323432363664653834303362336162313537613039663166373834643538376166363138333163393938633135316263633231626237346332623233313462)

@ddustin
Copy link
Collaborator

ddustin commented Oct 25, 2023

This may be caused by this series of events:

  1. The channel announcement timer fires and starts the process of creating a new channel announcement
  2. splice_mutual_lock occurs, updating the channel scid
  3. The channel announcement finishes, with the old channel scid
  4. The channel scid mismatch causes the signature's to mismatch

If this is the case than two solutions are: allowing a grace period of channel announcements with the stale channel scid to be ignored gracefully (as we do in the peer protocol), or introduce a method to cancel an ongoing channel announcement creation when we splice_mutual_lock 🤔

@ddustin
Copy link
Collaborator

ddustin commented Oct 25, 2023

Relevant bit of log:
lightningd-2 2023-10-17T17:28:56.490Z DEBUG 0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518-gossipd: Bad gossip order: WIRE_CHANNEL_UPDATE before announcement 109x1x0/1

@ddustin
Copy link
Collaborator

ddustin commented Oct 25, 2023

BROKEN messages decoded:

Bad node_signature_1 3044022078c3314666731e339c0b8434f7824797a084ed7ca3655991a672da068e2c44cb022053b57b53a296c133bc879109a8931dc31e6913a4bda3d58559b99b95663e6d52
hash 893da44a233767848c361de48efd20a31fdaa06293fb36d8d59764898d571c9a
on channel_announcement 010078c3314666731e339c0b8434f7824797a084ed7ca3655991a672da068e2c44cb53b57b53a296c133bc879109a8931dc31e6913a4bda3d58559b99b95663e6d526de1a52ab38f0c0e2c18c3bc425ab686f61d5c2237a6d67885209ccb1d72a4f92a8021c8d7608e5ad835b604f43c4638331ab8f8a07afe5dcb5b2456d0fe900f679cabaccc6e39dbd95a4dac90e75a258893c3aa3f733d1b8890174d5ddea8003cadffe557773c54d2c07ca1d535c4bf85885f879ae466c16a516e8ffcfec1741088fb5422d3aee7807581c14f8e8cc320d5e0af92818d59fe1c7482e30369bb4680fb9f3da8aa7bc046a20fe834560979c84a128974378fa29f1a716c344e05000006226e46111a0b59caaf126043eb5bbf28c34f3a5e332a1fc7b2b73cf188910f00006d0000010000022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d590266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c0351802e3bd38009866c9da8ec4aa99cc4ea9c6c0dd46df15c61ef0ce1f271291714e5702324266de8403b3ab157a09f1f784d587af61831c998c151bcc21bb74c2b2314b
WIRE_CHANNEL_ANNOUNCEMENT:
node_signature_1=3044022078c3314666731e339c0b8434f7824797a084ed7ca3655991a672da068e2c44cb022053b57b53a296c133bc879109a8931dc31e6913a4bda3d58559b99b95663e6d52
node_signature_2=304402206de1a52ab38f0c0e2c18c3bc425ab686f61d5c2237a6d67885209ccb1d72a4f902202a8021c8d7608e5ad835b604f43c4638331ab8f8a07afe5dcb5b2456d0fe900f
bitcoin_signature_1=30440220679cabaccc6e39dbd95a4dac90e75a258893c3aa3f733d1b8890174d5ddea80002203cadffe557773c54d2c07ca1d535c4bf85885f879ae466c16a516e8ffcfec174
bitcoin_signature_2=304402201088fb5422d3aee7807581c14f8e8cc320d5e0af92818d59fe1c7482e30369bb02204680fb9f3da8aa7bc046a20fe834560979c84a128974378fa29f1a716c344e05
features=[]
chain_hash=0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206
short_channel_id=109x1x0
node_id_1=022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59
node_id_2=0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518
bitcoin_key_1=02e3bd38009866c9da8ec4aa99cc4ea9c6c0dd46df15c61ef0ce1f271291714e57
bitcoin_key_2=02324266de8403b3ab157a09f1f784d587af61831c998c151bcc21bb74c2b2314b

@ddustin
Copy link
Collaborator

ddustin commented Oct 25, 2023

Interestingly, since gossipd doesn't flag this as an error, the rest of the test operates perfectly. The second channel announcement message goes through successfully and everything continues on peacefully.

@ddustin
Copy link
Collaborator

ddustin commented Oct 25, 2023

Thanks for finding that log 🙏.

PR #6818 should address the flakiness found in this log.

If you come across a different log of a flake run, would love to get a copy of it.

I believe the PR will fix test_commit_crash_splice referenced in #6788 as well, but can't be certain without catching more logs 👀.

@cdecker
Copy link
Member Author

cdecker commented Oct 26, 2023

Thanks @ddustin I will try and see if I can find a log example of test_commit_crash_splice and link it in #6788

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 a pull request may close this issue.

2 participants