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

Re-use ECC sinsemilla code #7801

Open
oxarbitrage opened this issue Oct 23, 2023 · 4 comments
Open

Re-use ECC sinsemilla code #7801

oxarbitrage opened this issue Oct 23, 2023 · 4 comments
Labels
A-compatibility Area: Compatibility with other nodes or wallets, or standard rules A-cryptography Area: Cryptography related E-help-wanted Call for participation: Help is requested to fix this issue.

Comments

@oxarbitrage
Copy link
Contributor

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?

@oxarbitrage oxarbitrage added P-Low ❄️ A-cryptography Area: Cryptography related A-compatibility Area: Compatibility with other nodes or wallets, or standard rules labels Oct 23, 2023
@teor2345
Copy link
Collaborator

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.

@teor2345 teor2345 changed the title Move sinsemilla code to its own project/crate ? Re-use ECC sinsemilla code Jan 1, 2024
@teor2345 teor2345 added the E-help-wanted Call for participation: Help is requested to fix this issue. label Jan 1, 2024
@mpguerra
Copy link
Contributor

mpguerra commented Mar 6, 2024

@natalieesk this might be somewhat related to #8155

@mpguerra
Copy link
Contributor

related to #8155

@str4d
Copy link
Contributor

str4d commented Oct 14, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-compatibility Area: Compatibility with other nodes or wallets, or standard rules A-cryptography Area: Cryptography related E-help-wanted Call for participation: Help is requested to fix this issue.
Projects
Status: Product Backlog
Development

No branches or pull requests

4 participants