-
Notifications
You must be signed in to change notification settings - Fork 115
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(db): use the correct state version for databases without a state version file #7385
Conversation
@Mergifyio update. |
✅ Branch has been successfully updated |
This assert fails https://github.com/zcashfoundation/zebra/blob/f08cd9fbc6d01f1bae0d9e5b54047a053f36a4e7/zebra-state/src/service/finalized_state/disk_format/upgrade.rs#L481-L486. It looks like |
11f2f4c
to
c442321
Compare
That's my mistake. As soon as RocksDB creates the database directory, we provide 25.0.0 as the database version, even though the database is empty and newly created. We need to open RocksDB first, because we depend on its lock to protect the version file. We could delete the "newly created" code path entirely to simplify things, if you want. Because now it's equivalent to an upgrade from 25.0.0 to the current version, but without any data in the database. (We might want to keep logging it differently, to avoid confusing users.) But that's out of scope for this PR, so let's open a ticket if there's enough interest. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could delete the "newly created" code path entirely to simplify things, if you want.
I'm fine with leaving it as it is.
Motivation
When Zebra opens a state cache that has never been upgraded, it was reporting the cache as a newly created state, then marking it with the current state version. This incorrectly skips all format upgrades.
Instead, all upgrades should be performed on the state.
Specifications
https://doc.rust-lang.org/std/fs/struct.Metadata.html#method.is_dir
Complex Code or Requirements
We missed this bug because there weren't enough tests for state version changes or state upgrades. But there are tests in PR #7379 now.
Solution
If there's a database directory wit no version file, report its version as
25.0.0
. (Compatible but with no upgrades.)Review
This bug fix blocks all the finalized state format changes.
Reviewer Checklist