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

Fix an incorrect assertion when the block locator is at the tip #2789

Merged
merged 7 commits into from
Sep 27, 2021

Conversation

teor2345
Copy link
Contributor

@teor2345 teor2345 commented Sep 22, 2021

Motivation

Zebra panics when it's at the tip, and it's asked for a block locator for that tip.
(This might depend on whether there's a stop height, and whether integer checks are optimised out.)

This might have been triggered by receiving block hash gossips from the new Zebra code in PR #2729.

Solution

  • Skip the assertion when the response is empty
  • Add tests for BlockLocator, FindBlockHashes, and FindBlockHeaders
  • Add a specific test for this bug

Review

Anyone can review this panic fix, but I think @oxarbitrage might have looked at this code recently?

Reviewer Checklist

  • Code implements Specs and Designs
  • Tests for Expected Behaviour
  • Tests for Errors

@teor2345 teor2345 added C-bug Category: This is a bug A-rust Area: Updates to Rust code P-Medium C-security Category: Security issues I-panic Zebra panics with an internal error message I-invalid-data Zebra relies on invalid or untrusted data, or sends invalid data labels Sep 22, 2021
@teor2345 teor2345 added this to the 2021 Sprint 19 milestone Sep 22, 2021
@teor2345 teor2345 self-assigned this Sep 22, 2021
@teor2345 teor2345 force-pushed the block-locator-tip-panic branch from 76e4fe9 to e1227d3 Compare September 23, 2021 04:02
teor2345 added a commit that referenced this pull request Sep 23, 2021
@teor2345 teor2345 marked this pull request as ready for review September 23, 2021 05:16
@teor2345
Copy link
Contributor Author

teor2345 commented Sep 23, 2021

The macOS failure looks like a spurious timing issue:

Message: Opening database "/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/zebrad_tests.qiCrlWIEdGK7/state/state/v10/mainnet" failed: Error { message: "IO error: While lock file: /var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/zebrad_tests.qiCrlWIEdGK7/state/state/v10/mainnet/LOCK: Resource temporarily unavailable" }. Hint: Check if another zebrad process is running. Try changing the state cache_dir in the Zebra config.

https://github.com/ZcashFoundation/zebra/pull/2789/checks?check_run_id=3692472245#step:10:1326

oxarbitrage
oxarbitrage previously approved these changes Sep 27, 2021
Copy link
Contributor

@oxarbitrage oxarbitrage left a comment

Choose a reason for hiding this comment

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

Looks good. Just a minor suggestion in a doc comment.

zebra-state/src/service.rs Outdated Show resolved Hide resolved
zebra-state/src/service.rs Show resolved Hide resolved
Co-authored-by: Alfredo Garcia <[email protected]>
@teor2345 teor2345 enabled auto-merge (squash) September 27, 2021 21:57
@teor2345 teor2345 merged commit 4567701 into main Sep 27, 2021
@teor2345 teor2345 deleted the block-locator-tip-panic branch September 27, 2021 22:43
@mpguerra mpguerra mentioned this pull request Apr 11, 2023
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rust Area: Updates to Rust code C-bug Category: This is a bug C-security Category: Security issues I-invalid-data Zebra relies on invalid or untrusted data, or sends invalid data I-panic Zebra panics with an internal error message
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants