Skip to content
This repository has been archived by the owner on Jan 19, 2021. It is now read-only.

Fix bug in verifyProof #51

Merged
merged 1 commit into from
Aug 27, 2018
Merged

Conversation

no2chem
Copy link
Contributor

@no2chem no2chem commented Aug 21, 2018

This PR fixes a bug in verify proof where "Error: Unexpected end of proof" if the tree contains an extension node with an embedded branch node.

A simple example of this bug is below:

const Tree = require('merkle-patricia-tree');
const tree = new Tree();
tree.put('a', 'a', (err) => {
    tree.put('b', 'b', (err) => {
         Tree.prove(tree, 'a', (err, proof) => {
            Tree.verifyProof(tree.root, 'a', proof, (err, val) => {
                // err = "Error: Unexpected end of proof"
                // expected : val === 'a' 
            }
        } 
    }
}

Fixes #52.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 80.791% when pulling ceefb10 on no2chem:verifyBranchFix into bc42cde on ethereumjs:master.

@no2chem
Copy link
Contributor Author

no2chem commented Aug 21, 2018

It seems that the coveralls configuration for this project doesn't reflect proof.js.

This is probably because L8 from package.json only runs coverage for ./test/index.js.

Copy link
Member

@holgerd77 holgerd77 left a comment

Choose a reason for hiding this comment

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

Ok, this looks good, will approve now, @no2chem thanks again for the PR.

Do you depend on this for another library and need this as a release or does this has some time and can be released in a combined release a bit later?

@holgerd77 holgerd77 merged commit 22e5f27 into ethereumjs:master Aug 27, 2018
@no2chem
Copy link
Contributor Author

no2chem commented Aug 28, 2018

@holgerd77 Thanks! No, we don't have a particular need for it right now, so it can be combined as a release sometime later.

@no2chem no2chem deleted the verifyBranchFix branch September 5, 2018 20:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Verify Proof fails if there is an extension with an embedded branch
3 participants