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

(De)Serialize NoteCommitmentTrees using supported serialization format instead of bincode #7383

Closed
4 tasks
Tracked by #6642
oxarbitrage opened this issue Aug 24, 2023 · 1 comment
Closed
4 tasks
Tracked by #6642
Labels
A-rust Area: Updates to Rust code A-state Area: State / database changes C-tech-debt Category: Code maintainability issues

Comments

@oxarbitrage
Copy link
Contributor

oxarbitrage commented Aug 24, 2023

Scheduling

This ticket is blocked by the #7392 state bug fix.

Motivation

Apparently (@teor2345 mentioned me this, came up in conversations with @upbqdn), if we replace bincode with the supported serialization format in for example sprout, sapling and orchard implementations the resulting database might be smaller. And it will be easier to maintain the code.

We might need to add as_bytes() and from_bytes() methods to sprout::NoteCommitmentTree, sapling::NoteCommitmentTree and orchard::NoteCommitmentTree to do this, using:

Suggested Solution

  • Make the needed changes to the database format
  • Update database version
  • Check the serialization/deserialization produces the same tree structures, by synchronizing the chain up to tip, then comparing the tree roots using the RPCs (or any other way). The comparison can be done with an older Zebra version, or with zcashd.

Optional

  • Compare the database size with a previous version without the changes (may need extra space in your computer to hold the 2 versions).
@oxarbitrage oxarbitrage added P-Low ❄️ A-state Area: State / database changes A-rust Area: Updates to Rust code labels Aug 24, 2023
@teor2345 teor2345 added the C-tech-debt Category: Code maintainability issues label Aug 25, 2023
@teor2345
Copy link
Contributor

This is also a technical debt issue. Currently we're maintaining code for a legacy incrementalmerkletree bincode-compatible serialization format, conversion code, and code to use the current version of incrementalmerkletree.

If we used the supported serialization formats, we could delete the duplicate code and data structures, and remove zebra-state's dependency on bincode and serde.

@mpguerra mpguerra closed this as not planned Won't fix, can't repro, duplicate, stale Oct 14, 2024
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 A-state Area: State / database changes C-tech-debt Category: Code maintainability issues
Projects
Status: Done
Development

No branches or pull requests

3 participants