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

change(db): Make the first stable release forward-compatible with planned state changes #6813

Merged
merged 10 commits into from
Jun 6, 2023

Conversation

teor2345
Copy link
Contributor

@teor2345 teor2345 commented Jun 5, 2023

Motivation

We're going to change how the state database is implemented in ticket #6642, after the first stable release.

It would be nice if all stable Zebra versions could read those states, even if they have some format changes.

Close #6746

Specifications

https://docs.rs/rocksdb/latest/rocksdb/struct.DBCommon.html#method.list_cf
https://docs.rs/rocksdb/latest/rocksdb/struct.DBIteratorWithThreadMode.html

Complex Code or Requirements

This is all fairly straightforward, the tricky part will be tickets #6642 and #4784, where we actually change the state format.

Solution

Testing

These changes are covered by existing state and RPC tests.

I don't think we should have specific forward compatibility tests, we can just support it on a best-effort basis.

Review

This is a routine change that we'd like to get in the first stable release.

Reviewer Checklist

  • Will the PR name make sense to users?
    • Does it need extra CHANGELOG info? (new features, breaking changes, large changes)
  • Are the PR labels correct?
  • Does the code do what the ticket and PR says?
    • Does it change concurrent code, unsafe code, or consensus rules?
  • How do you know it works? Does it have tests?

Follow Up Work

Actually change the state format.

@teor2345 teor2345 added C-enhancement Category: This is an improvement P-Medium ⚡ A-state Area: State / database changes labels Jun 5, 2023
@teor2345 teor2345 self-assigned this Jun 5, 2023
@teor2345 teor2345 requested a review from a team as a code owner June 5, 2023 00:57
@teor2345 teor2345 requested review from oxarbitrage and removed request for a team June 5, 2023 00:57
@codecov
Copy link

codecov bot commented Jun 5, 2023

Codecov Report

Merging #6813 (050a08b) into main (628ddd6) will increase coverage by 0.18%.
The diff coverage is 85.90%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6813      +/-   ##
==========================================
+ Coverage   77.97%   78.15%   +0.18%     
==========================================
  Files         308      308              
  Lines       41023    41113      +90     
==========================================
+ Hits        31988    32133     +145     
+ Misses       9035     8980      -55     

oxarbitrage
oxarbitrage previously approved these changes Jun 5, 2023
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.

This is very clear, thank you for making it that way. Looks great.

zebra-state/src/constants.rs Show resolved Hide resolved
zebra-state/src/constants.rs Outdated Show resolved Hide resolved
zebra-state/src/constants.rs Outdated Show resolved Hide resolved
zebra-state/src/constants.rs Outdated Show resolved Hide resolved
@teor2345
Copy link
Contributor Author

teor2345 commented Jun 5, 2023

This PR is ready for re-review

@teor2345
Copy link
Contributor Author

teor2345 commented Jun 6, 2023

Unrelated failure, possibly #6763 but in the RPC code:

{"app":"lightwalletd","error":"Post "[http://127.0.0.1:41291](http://127.0.0.1:41291/)": read tcp 127.0.0.1:45786-\u003e127.0.0.1:41291: read: connection reset by peer","level":"fatal","msg":"error zcashd getbestblockhash rpc","time":"2023-06-06T01:10:56Z"}

https://github.com/ZcashFoundation/zebra/actions/runs/5182998267/jobs/9341002724?pr=6813#step:8:426

@teor2345
Copy link
Contributor Author

teor2345 commented Jun 6, 2023

@Mergifyio update

@mergify
Copy link
Contributor

mergify bot commented Jun 6, 2023

update

✅ Branch has been successfully updated

mergify bot added a commit that referenced this pull request Jun 6, 2023
mergify bot added a commit that referenced this pull request Jun 6, 2023
teor2345 added a commit that referenced this pull request Jun 6, 2023
@mergify mergify bot merged commit 355f123 into main Jun 6, 2023
@mergify mergify bot deleted the db-forward-compat branch June 6, 2023 21:18
@teor2345 teor2345 mentioned this pull request Jun 7, 2023
41 tasks
mergify bot added a commit that referenced this pull request Jun 7, 2023
…#6814)

* Implement minor and patch database format versions

* Log and update database format versions when opening database

* Refactor the current list of column families into a constant

* Open all available column families, including from future Zebra versions

* Refactor note commitment tree lookups to go through the height methods

* Make Sapling/Orchard note commitment tree lookup forwards compatible

* Ignore errors reading column family lists from disk

* Update format version comments and TODOs

* Correctly log newly created database formats

* Fix a new cargo lint about resolver versions

* cargo clippy --fix --all-features --all-targets

* cargo fmt --all

* Add missing tokio feature in the state, revealed by the new resolver

* Add missing dev dependencies in zebra-node-services

* Add a missing `tokio` feature from PR #6813

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-state Area: State / database changes C-enhancement Category: This is an improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make the first stable release forward-compatible with planned state changes
2 participants