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

feat: merkle tree verification #31

Closed
wants to merge 5 commits into from
Closed

Conversation

ermvrs
Copy link

@ermvrs ermvrs commented Jun 18, 2023

Here is an example of how to use Merkle Trees with pedersen hash.

Copy link
Collaborator

@enitrat enitrat left a comment

Choose a reason for hiding this comment

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

Thank you for the PR! Great work.

Can you provide a test suite for this? Like it's done in SolidityByExample, to demonstrate how to use this contract in practice?


A Merkle tree is a hash-based data structure that is a generalization of the hash list. It is a tree structure in which each leaf node is a hash of a block of data, and each non-leaf node is a hash of its children. Typically, Merkle trees have a branching factor of 2, meaning that each node has up to 2 children.

If you are familiar with developing whitelists, Merkle trees actively used. In Solidity we use Keccak256 for hashing leaves. As felt252 range is not able to fit a keccak256 hash, we use Pedersen as hash function in this example.
Copy link
Collaborator

Choose a reason for hiding this comment

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

As felt252 range is not able to fit a keccak256 hash, we use Pedersen as hash function in this example.

That is true, however I don't think that's a good reason as we could just use u256s and hash these u256s with keccak together.
I think what matters here is that pedersen is cheaper than keccak, and as you mentioned we can make it work with felts instead of uints

root::write(_root);
}

// Checks is merkle proof verified or not.
Copy link
Collaborator

Choose a reason for hiding this comment

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

// Verifies the given Merkle proof


fn _verify(root:felt252, leaf: felt252, ref proof: Array::<felt252>) -> bool {
let proof_len = proof.len();
let calc_root = _verify_body(leaf, ref proof, proof_len, 0_u32);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of having verify_body as a recursive function, can you make it as a loop instead?

}

fn _verify_body(
leaf: felt252, ref proof: Array::<felt252>, proof_len: u32, index: u32
Copy link
Collaborator

Choose a reason for hiding this comment

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

proof_len is an unrequired argument here as you can simply use proof.len().
also, instead of increasing the index by 1, and decreasing the proof len by 1, you could just call proof.pop_front() on the proof (passed as a Span). and simply pass the proof.

// Checks is merkle proof verified or not.
#[view]
fn verify(account: ContractAddress, mut proof: Array::<felt252>) -> bool {
let account_as_felt: felt252 = account.into();
Copy link
Collaborator

Choose a reason for hiding this comment

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

account is transformed into a felt252 here, and the destination type is explicit. You can just shadow the previous type of account, the type identifier in the variable name is not required.
let account: felt252 = account.into();

fn _verify(root:felt252, leaf: felt252, ref proof: Array::<felt252>) -> bool {
let proof_len = proof.len();
let calc_root = _verify_body(leaf, ref proof, proof_len, 0_u32);
if (calc_root == root) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

you can simplify it to return calc_root == root;

@julio4
Copy link
Contributor

julio4 commented Aug 28, 2023

@ermvrs What is the state of this PR?

@julio4 julio4 closed this Sep 15, 2023
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.

3 participants