zcash_history: Implement v2 history tree with Orchard support
#401
Conversation
Each Zcash epoch (between two network upgrades) has a separate history tree, making it easy to switch the node data format at network upgrades. This commit enables the general tree logic to be shared across history tree versions.
str4d
force-pushed
the
368-zip-221-orchard
branch
from
1c35977
to
e84bb87
Compare
V2 is implemented as a wrapper around V1, so this simply expands the tested code.
Codecov Report
@@ Coverage Diff @@
## master #401 +/- ##
==========================================
- Coverage 62.89% 62.59% -0.31%
==========================================
Files 86 87 +1
Lines 8407 8465 +58
==========================================
+ Hits 5288 5299 +11
- Misses 3119 3166 +47
Continue to review full report at Codecov.
|
| 32 + // start Orchard tree root | ||
| 32 + // end Orchard tree root | ||
| 9; // Orchard tx count (compact uint) | ||
| // = total of 244 |
There was a problem hiding this comment.
In Zebra we're using MAX_NODE_DATA_SIZE to create fixed-size byte arrays.
So between merging this PR and NU5 activation, we would have some unused trailing zero bytes. But the RAM and disk impact is minimal because we only store one copy of the cache. So we just need to check that we're not expecting to read or write the whole array every time.
@conradoplg does ZcashFoundation/zebra#2227 assume that we're reading or writing every node data byte?
There was a problem hiding this comment.
This constant is intentionally the upper bound on the node data encoding, to reflect the fact that some of the data types are compact sizes, and thus variable width. This constant is used for e.g. predictable array sizes crossing the FFI in zcashd.
There was a problem hiding this comment.
ZcashFoundation/zebra#2227 just hands it over to librustzcash (zcash_history::NodeData::from_bytes) and doesn't assume anything else
| pub struct V2 { | ||
| /// The V1 node data retained in V2. | ||
| pub v1: NodeData, |
There was a problem hiding this comment.
This also looks good for Zebra.
We can take our block_to_history_node function that produces a zcash_history::NodeData, and wrap it in a block_to_history_node_v2 function that returns zcash_history::V2:
https://github.com/ZcashFoundation/zebra/pull/2227/files#diff-ccc89b2ef027c45e027753030442d684005ebf402fcc69be39c70ee6ed193773R230
We might want to rename the legacy function to _v1, but we can do that in our Orchard-specific PR.
There was a problem hiding this comment.
As an aside, adding new fields is a lot simpler than deleting fields.
We deleted the per-spend anchor for sapling using marker traits, and it got slightly complicated:
Conversion from shared to per-spend anchors:
https://github.com/ZcashFoundation/zebra/blob/4aecf0360724e50d51f31ab15e86277e5b8bbd32/zebra-chain/src/sapling/shielded_data.rs#L267-L291
But having a shared implementation is still really nice.
There was a problem hiding this comment.
Yeah, there will likely be some duplication if we ever change or delete a field (e.g. when Sapling is eventually tuned off). But that's unlikely to happen for quite some time.
| pub struct Tree<V: Version> { | ||
| stored: HashMap<u32, Entry<V>>, | ||
|
|
||
| // This can grow indefinitely if `Tree` is misused as a self-contained data structure | ||
| generated: Vec<Entry>, | ||
| generated: Vec<Entry<V>>, |
There was a problem hiding this comment.
This change looks good for Zebra, we'd just add the same type parameter to our Tree, and pass it to the inner zcash_history::Tree.
We add support for multiple history tree versions with a marker trait, and add marker structs for the two currently-defined history tree versions.
Closes #368.