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

pay: Fix a crash when waitblockheight times out #4317

Merged
merged 3 commits into from
Jan 8, 2021

Conversation

cdecker
Copy link
Member

@cdecker cdecker commented Jan 6, 2021

We would plugin_err() if the call to waitblockheight in the
corresponding modifier timed out, instead of returning after
successful sync. We now catch that and terminate the payment on
timeout. Since we only react to errors containing a blockheight from
the last node, not being in sync is fatal for that payment.

Fixes #4269
Fixes #4309
Changelog-Fixed: pay: Fixed an issue where waiting for the blockchain height to sync could time out.

@cdecker cdecker added this to the v0.9.3 milestone Jan 6, 2021
@cdecker cdecker requested a review from ZmnSCPxj January 6, 2021 18:43
Copy link
Contributor

@rustyrussell rustyrussell left a comment

Choose a reason for hiding this comment

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

Ack, except for minor procedural comment. No need for re-ack.

Comment on lines +3936 to +3938


def test_pay_waitblockheight_timeout(node_factory, bitcoind):
Copy link
Contributor

Choose a reason for hiding this comment

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

Please mark this xfail and unmark in next commit, to avoid breaking bisect?

@cdecker
Copy link
Member Author

cdecker commented Jan 7, 2021

ACK 2cd25fb

We need to differentiate timeouts from other results, so make it recognizable.
Fixes ElementsProject#4309
Changelog-Fixed: pay: Fixed an issue where waiting for the blockchain height to sync could time out.
@cdecker cdecker merged commit 4b2efd6 into ElementsProject:master Jan 8, 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.

waitblockheight timeout causes crash. Keysend crashed c-lightning
2 participants