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

verifyProof probably should return false for any key if the tree is empty #343

Open
0xDatapunk opened this issue Oct 7, 2024 · 3 comments
Labels
audit 🔍 This issue is related to an audit. bug 🐛 Something isn't working good first issue Good for newcomers

Comments

@0xDatapunk
Copy link
Collaborator

0xDatapunk commented Oct 7, 2024

verifyProof(merkleProof: MerkleProof): boolean {

not sure if it is as designed, but verifyProof return True for any key with an empty tree. It makes better sense if false is returned:

verifyProof(merkleProof: MerkleProof): boolean {
    if (this.root === this.zeroNode) {
        // Empty tree case
        return merkleProof.root === this.zeroNode && 
               merkleProof.siblings.length === 0 &&
               merkleProof.matchingEntry === undefined &&
               merkleProof.membership === false;
    }
    // ... rest of the method ...
}```
@github-project-automation github-project-automation bot moved this to 📋 Backlog in ZK-Kit Oct 7, 2024
@cedoor cedoor added bug 🐛 Something isn't working audit 🔍 This issue is related to an audit. labels Oct 14, 2024
@cedoor
Copy link
Member

cedoor commented Oct 24, 2024

A proof of non-membership in a empty tree should still be true tho. Wdyt @0xDatapunk ?

@cedoor cedoor added the good first issue Good for newcomers label Oct 24, 2024
@0xDatapunk
Copy link
Collaborator Author

yes, and the code above should return true for non-membership.

@cedoor
Copy link
Member

cedoor commented Oct 24, 2024

Not sure about this merkleProof.matchingEntry === undefined -> should a matchingEntry should still be defined?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
audit 🔍 This issue is related to an audit. bug 🐛 Something isn't working good first issue Good for newcomers
Projects
Status: 📋 Backlog
Development

No branches or pull requests

2 participants