ZcashFoundation / zebra

Zcash - Financial Privacy in Rust 🦓
https://zfnd.org/zebra/
Apache License 2.0
412 stars 106 forks source link

Re-use ECC `sinsemilla` code #7801

Open oxarbitrage opened 1 year ago

oxarbitrage commented 1 year ago

I noticed we have sinsemilla hash function code in Zebra: https://github.com/ZcashFoundation/zebra/blob/main/zebra-chain/src/orchard/sinsemilla.rs and just 1 call to sinsemilla_hash() from one place (https://github.com/ZcashFoundation/zebra/blob/main/zebra-chain/src/orchard/tree.rs#L71).

I was wondering if we should move the sinsemilla code into its own crate/project. I noticed a placeholder crate exist (https://crates.io/crates/sinsemilla) with @str4d as the owner. We can maybe ask permissions, push the code there unless there are other plans for the placeholder.

What other people think?

teor2345 commented 1 year ago

If ECC do end up splitting sinsemilla into its own crate, they will probably use their implementation, which has code for Zero-Knowledge Proof Circuits, and standard Rust: https://docs.rs/halo2_gadgets/latest/halo2_gadgets/sinsemilla/index.html

I think we shouldn't re-implement our own cryptographic functions when they are already available in an audited and tested library. That increases our audit and maintenance load, and the risk of bugs.

We can remove our sinsemilla implementation by replacing zebra_chain::orchard::Node with orchard::tree::MerkleHashOrchard. It uses its own sinsemilla hash implementation internally: https://docs.rs/orchard/latest/orchard/tree/struct.MerkleHashOrchard.html#impl-Hashable-for-MerkleHashOrchard

We need to completely replace the type to avoid expensive cryptographic checks during type conversions. But we can create a type alias to Node, so code outside the module can keep using the old name.

mpguerra commented 8 months ago

@natalieesk this might be somewhat related to #8155

mpguerra commented 1 month ago

related to #8155

str4d commented 1 month ago

Also related to https://github.com/zcash/halo2/issues/827; we want to make our non-circuit Sinsemilla code usable as a dependency without needing the whole of halo2_proofs and halo2_gadgets.