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

Detect and fix stack overflows! #4751

Conversation

rustyrussell
Copy link
Contributor

@rustyrussell rustyrussell commented Aug 31, 2021

First patch simple enables CI to find the bug, second fixes it. If there's a third, that means we found more!

@cdecker
Copy link
Member

cdecker commented Aug 31, 2021

First patch simple enables CI to find the bug, second fixes it. If there's a third, that means we found more!

Seems to be missing the second patch, then again, CI doesn't seem to have an issue with the stronger verification.

@niftynei
Copy link
Collaborator

The bug was being triggered in a EXPERIMENTAL_DUAL_FUND flow, perhaps the EXPERIMENTAL_FEATURES flag doesn't trigger it?

This would have caught the bug found in the next commit.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell rustyrussell force-pushed the guilt/fix-anchor-outputs-tx-creation-stack-smash branch 2 times, most recently from 08913ed to 2802c42 Compare September 1, 2021 01:26
@rustyrussell
Copy link
Contributor Author

OK, so I changed it to turn on stronger stack detection always, in configure. Indeed, it broke as expected. Then I pushed the fix, and now it's good.

users there (and reliably detected now).

Reported-by: @larsschenk
Reported-by: @nickfarrow
Fixes: ElementsProject#4728
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: EXPERIMENTAL: crash for some users while requesting dual funding leases.
@rustyrussell rustyrussell force-pushed the guilt/fix-anchor-outputs-tx-creation-stack-smash branch from 2802c42 to 401d0c5 Compare September 1, 2021 02:54
@cdecker
Copy link
Member

cdecker commented Sep 2, 2021

ACK 401d0c5

@cdecker cdecker merged commit 372fffa into ElementsProject:master Sep 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants