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

New PoseidonHash/RAMT/SMT #33

Closed
wants to merge 34 commits into from

Conversation

DanieleDiBenedetto
Copy link
Collaborator

@DanieleDiBenedetto DanieleDiBenedetto commented Jun 25, 2020

Closes #14 and also #39

api/Cargo.toml Outdated
algebra = {version = "0.1.0", git = "https://github.com/ZencashOfficial/ginger-lib.git", branch = "development"}
primitives = {version = "0.1.0", features = ["merkle_tree", "signature", "vrf"], git = "https://github.com/ZencashOfficial/ginger-lib.git", branch = "development"}
proof-systems = {version = "0.1.0", features = ["groth16"], git = "https://github.com/ZencashOfficial/ginger-lib.git", branch = "development"}
algebra = {version = "0.1.0", git = "https://github.com/ZencashOfficial/ginger-lib.git", branch = "updatable_poseidon"}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once UpdatablePoseidon branch will be merged in GingerLib we must change this dependency.

r1cs-core = {version = "0.1.0", git = "https://github.com/ZencashOfficial/ginger-lib.git", branch = "development"}
r1cs-std = {version = "0.1.0", git = "https://github.com/ZencashOfficial/ginger-lib.git", branch = "development"}
proof-systems = {version = "0.1.0", features = ["groth16"], git = "https://github.com/ZencashOfficial/ginger-lib.git", branch = "development"}
algebra = {version = "0.1.0", git = "https://github.com/ZencashOfficial/ginger-lib.git", branch = "updatable_poseidon"}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once UpdatablePoseidon branch will be merged in GingerLib we must change this dependency.

@@ -5,7 +5,7 @@ authors = ["DanieleDiBenedetto <[email protected]>"]
edition = "2018"

[dependencies]
algebra = {version = "0.1.0", git = "https://github.com/ZencashOfficial/ginger-lib.git", branch = "development"}
primitives = {version = "0.1.0", features = ["vrf"], git = "https://github.com/ZencashOfficial/ginger-lib.git", branch = "development"}
algebra = {version = "0.1.0", git = "https://github.com/ZencashOfficial/ginger-lib.git", branch = "updatable_poseidon"}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once UpdatablePoseidon branch will be merged in GingerLib we must change this dependency.

@DanieleDiBenedetto DanieleDiBenedetto linked an issue Jun 25, 2020 that may be closed by this pull request
@DanieleDiBenedetto DanieleDiBenedetto marked this pull request as draft August 6, 2020 17:11
@DanieleDiBenedetto DanieleDiBenedetto changed the title Updatable Poseidon Hash New PoseidonHash/RAMT/SMT Aug 24, 2020
@DanieleDiBenedetto DanieleDiBenedetto marked this pull request as ready for review August 24, 2020 11:33
api/src/ginger_calls.rs Outdated Show resolved Hide resolved
api/src/ginger_calls.rs Outdated Show resolved Hide resolved
- Modified function signature 'RandomAccessMerkleTree.init(height)' to 'RandomAccessMerkleTree.init(height, processing_step)';
- Modified class name 'RandomAccessMerkleTree' to 'FieldBasedOptimizedMHT';
- Added possibility, for all Merkle Trees, to create a Merkle Path;
- Introduced new MerklePath class containing a verify() function;
- Modified, for all Merkle Trees, the relationship between height (h) and number of leaves (l). Previously it was l = 2^(h - 1) now it's l = 2^h;
- Some useful refactoring;
- Other minor changes to sync with GingerLib's 'smt' branch
return nativeGetPosition(leaf);
}

private native boolean nativeIsPositionEmpty(long position);

public boolean isPositionEmpty(long position){
if (merkleTreePointer == 0)
throw new IllegalArgumentException("merkleTreePointer must be not null.");
throw new IllegalArgumentException("BigMerkleTree instance was freed.");
return nativeIsPositionEmpty(position);
}

private native void nativeAddLeaf(FieldElement leaf, long position);

public void addLeaf(FieldElement leaf, long position){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, what about this method? Are you going to remove position arg?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about this one. From an usage perspective can be misleading, but probably for testing and debugging it will be useful

this.merklePathPointer = merklePathPointer;
}

private native boolean nativeVerify(int merkleTreeHeight, FieldElement leaf, FieldElement root);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need the merkleTreeHeight here? Is it not enough just to apply the path elements to the leaf one by one and then compare it with the root?

Copy link
Collaborator Author

@DanieleDiBenedetto DanieleDiBenedetto Sep 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to check also that the path is of the correct length, otherwise I can just pass as a leaf a children of the root and the method will pass anyway.
As using the height as an explicit parameter, instead, is a result of keeping the height as a free parameter to instantiate the merkle trees. Hardcoding the height in the rust side of the cryptolib means that if we want to modify the height of a tree, we should release a new version of the cryptolib; or if a sidechain creator wants to declare a new tree, he has to modify explicitly the zendoo-sc-cryptolib too (even if, probably, he has to do it anyway)

@DanieleDiBenedetto DanieleDiBenedetto linked an issue Oct 5, 2020 that may be closed by this pull request
@@ -347,7 +391,12 @@ pub fn vrf_prove(msg: &FieldElement, sk: &VRFSk, pk: &VRFPk) -> Result<(VRFProof
let mut hash_input = Vec::new();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are these 3 lines still needed?

((1 + FIELD_SIZE) * path.get_length()) + 1
}

pub fn apply(path: &GingerMHTPath, leaf: &FieldElement) -> FieldElement
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move to ginger-lib FieldBasedMHTPath

@DanieleDiBenedetto DanieleDiBenedetto deleted the updatable_poseidon branch March 23, 2023 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Better explain the building process Poseidon Hash/RAMT/SMT
3 participants