ZIP-221: integrate MMR tree from librustcash (without Orchard)

[conradoplg]

Motivation

ZIP-221 specifies that the hashFinalSaplingRoot field of the header is a commitment to the root of a MMR tree, therefore Zebra must support it.

Solution

librustzcash has code for that in zcash_history, though not for the whole logic. It has a Tree class that contains the logic for adding / removing nodes to the tree, but it's not supposed to be used as a long-term data structure - zcashd instantiates it every time a node is added or removed.

This PR creates a wrapper over it.

The code in this pull request has:

• Documentation Comments
• Unit Tests and Property Tests

(will add tests after moving from draft)

Review

@teor2345 will probably want to review it.

This draft PR only adds the wrapping over librustzcash; it still does not handle the logic to keep track of what should be stored.

Draft PR points that I'd like feedback on:

• Are some of the expects OK? If an error should be returned, how should I wrap it? (I guess I shouldn't return a librustzcash error. I tried using thiserror but couldn't get it to work, so I thought it would be better to confirm which approach to use beforehand)
• Is the librustzcash wrapping enough for this PR? (i.e. should I add the remaining logic in a separate PR?)
• Are the method names OK? I copied librustzcash, but I considered using push/pop (similar to Vec)
• Should the remaining logic be added in the same file / class, or maybe in other place of zebra_chain?

Questions about the design in #2132

• "use HashMap<u32, Entry>": this is what would be persisted/serialized, and I'd add / remove nodes on it based on the Tree operations, right? Are there any guidelines on how that should be serialized?

Related Issues

Part of #2132

Follow Up Work

Needs Orchard support which is blocked by librustzcash: zcash/librustzcash#368

[teor2345]

This all looks good so far, just a bunch of tweaks and suggestions.

[conradoplg]

I've addressed most of the issues; still need to change to return Result where needed

[teor2345]

Just some quick suggestions before I tackle the larger questions on the PR.

[teor2345]

Draft PR points that I'd like feedback on:

• Are some of the expects OK? If an error should be returned, how should I wrap it? (I guess I shouldn't return a librustzcash error. I tried using thiserror but couldn't get it to work, so I thought it would be better to confirm which approach to use beforehand)

In the primitives module, it's ok to return a librustzcash error type.

We'll wrap it in our own error types in the calling code in zebra_state.

• Is the librustzcash wrapping enough for this PR? (i.e. should I add the remaining logic in a separate PR?)

Yes, let's do separate PRs for each ticket. And any changes within a ticket that are easily split up.

But if there are any tests you can run on this code, let's add them in this PR. The zcash_test crate has block test vectors for Heartwood and Canopy, for both Mainnet and Testnet:
https://github.com/ZcashFoundation/zebra/blob/main/zebra-test/src/vectors/block.rs#L78

Here are some tests you can do:

1. The root of a new tree, after adding mainnet heartwood activation 903_000, matches the commitment field in the header of 903_001
2. Similarly for mainnet canopy, testnet heartwood, and testnet canopy

• Are the method names OK? I copied librustzcash, but I considered using push/pop (similar to Vec)

For primitives, let's match the method names from the library we're wrapping.

• Should the remaining logic be added in the same file / class, or maybe in other place of zebra_chain?

Let's put primitive operations in zebra_chain, so they're easier to upstream:

• MMR data type
• Pruning old blocks before serialization

The other changes should go in zebra_state, because they are tightly integrated with the NonFinalizedState and FinalizedState implementations.

Questions about the design in #2132

• "use HashMap<u32, Entry>": this is what would be persisted/serialized, and I'd add / remove nodes on it based on the Tree operations, right?

This is the type that will be cloned for each chain in the NonFinalizedState, and then serialized for the best chain in the FinalizedState. (Multiple chains happen around every 1 in 300 blocks. They are typically very short.)

We might want a tuple struct wrapper around HashMap<u32, Entry>. That will make it self-documenting, and easier to implement serialization on.

Are there any guidelines on how that should be serialized?

Here's our current serialization design, you should update it in the serialization PR:
https://github.com/ZcashFoundation/zebra/blob/main/book/src/dev/rfcs/0005-state-updates.md#rocksdb-data-structures

You'll probably want to serialize it with BE32(height) keys, and [u8] values from Entry.write. If we use height, then all our indexes will be unique, because the finalized state only stores a single block at each height.

We can calculate height using index + network_upgrade.activation_height(network). So you might want to store the network_upgrade in the Zebra Tree, and add an activation_height method.

[conradoplg]

Another round of fixes. I had to make a small refactoring because I was mixing up Node and Entry. An Entry contains a Node and the indices of the left/right children, and Tree::new requires Entrys and not Nodes.

I've created a wrapper for Entry that stores it encoded as a buffer vector. However that may be not needed (you can derive the children indices, no need to store them), but for simplicity I chose this route for now.

Tests are not complete either - see next comment I'll post

[conradoplg]

1. The root of a new tree, after adding mainnet heartwood activation 903_000, matches the commitment field in the header of 903_001

To do that I'll need the Sapling root for that block, not sure how I'll get that. I'll ask in the chat.

[teor2345]

1. The root of a new tree, after adding mainnet heartwood activation 903_000, matches the commitment field in the header of 903_001

To do that I'll need the Sapling root for that block, not sure how I'll get that. I'll ask in the chat.

The final sapling roots for all post-sapling test vectors are in PR #2243

[teor2345]

This looks good, just a few minor changes to tidy up the code.

[conradoplg]

New round of improvements!

[str4d]

FYI I've implemented the NU5 tree format: zcash/librustzcash#401 (feedback welcome, particularly re: how it interacts with this PR).

[teor2345]

This looks like a good API, let's merge so we can start work on the state changes.

@conradoplg do you think we need to make any changes now to prepare for the librustzcash Orchard changes?
https://github.com/zcash/librustzcash/pull/401/files

We can leave all the changes to the Orchard-specific PR, unless they impact the state design.

[conradoplg]

@teor2345 I believe that the only changes to the API of this module will be adding a type parameter to Tree and adding the Orchard tree root hash parameter wherever the Sapling root is being passed. Also I'll probably have a better understanding of any required API adjustments after starting working on the state part. So I'll open an issue for the Orchard part and we can prioritize it if any bigger changes are required