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

Merkle proof library with tests and docs #260

Merged
merged 7 commits into from
Sep 18, 2017
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions contracts/MerkleProof.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
pragma solidity ^0.4.11;

/*
* @title MerkleProof
* @dev Merkle proof verification
* @note Based on https://github.com/ameensol/merkle-tree-solidity/blob/master/src/MerkleProof.sol
*/
library MerkleProof {
/*
* @dev Verifies a Merkle proof proving the existence of a leaf in a Merkle tree. Assumes that each pair of leaves
* and each pair of pre-images is sorted.
* @param _proof Merkle proof containing sibling hashes on the branch from the leaf to the root of the Merkle tree
* @param _root Merkle root
* @param _leaf Leaf of Merkle tree
*/
function verifyProof(bytes _proof, bytes32 _root, bytes32 _leaf) constant returns (bool) {
bytes32 proofElement;
bytes32 computedHash = _leaf;
Copy link
Contributor

Choose a reason for hiding this comment

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

Comparing this implementation to https://github.com/raiden-network/raiden/blob/master/raiden/smart_contracts/NettingChannelLibrary.sol#L268, it seems the only difference is whether to hash the leaf element before entering the loop.

I guess the difference is on how you build the tree: is it the tree who hashes every element initially to form the leaves, or should the tree's user do it (as is the case here)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it makes sense for _leaf to be the hash of the leaf element instead of the actual leaf element since it avoids an additional keccak256 operation on-chain - the more logic that can be pushed off-chain the better imo. Then, the comments/documentation can make it clearer that _leaf should be the hash of the leaf element and not the actual leaf element.


for (uint256 i = 32; i <= _proof.length; i += 32) {
assembly {
// Load the current element of the proof
proofElement := mload(add(_proof, i))
Copy link
Contributor

@spalladino spalladino Jul 26, 2017

Choose a reason for hiding this comment

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

WDYT about adding a check that proof.length is actually multiple of 4, so we don't risk loading into memory unexpected data in this line? I'm not quite sure about what parts of memory would be readable this way, though most likely it's the rest of the params, so this shouldn't be an issue; but it's a cheap check and it doesn't hurt to be extra careful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree that a check for proof.length just to be careful makes sense. Though, I think the check should be for if proof.length is a multiple of 32 since proof is supposed to be an array of bytes32 hashes?

Copy link
Contributor

@spalladino spalladino Jul 28, 2017

Choose a reason for hiding this comment

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

Absolutely, my bad! The check is for 32 as you say.

}

if (computedHash < proofElement) {
// Hash(current computed hash + current element of the proof)
computedHash = sha3(computedHash, proofElement);
} else {
// Hash(current element of the proof + current computed hash)
computedHash = sha3(proofElement, computedHash);
}
}

// Check if the computed hash (root) is equal to the provided root
return computedHash == _root;
}
}
9 changes: 9 additions & 0 deletions docs/source/merkleproof.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
MerkleProof
=============================================

Merkle proof verification for leaves of a Merkle tree.

verifyProof(bytes _proof, bytes32 _root, bytes32 _leaf) internal constant returns (bool)
"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""

Verifies a Merkle proof proving the existence of a leaf in a Merkle tree. Assumes that each pair of leaves and each pair of pre-images is sorted.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
"babel-register": "^6.23.0",
"coveralls": "^2.13.1",
"ethereumjs-testrpc": "^3.0.2",
"ethereumjs-util": "^5.1.2",
"mocha-lcov-reporter": "^1.3.0",
"solidity-coverage": "^0.1.0",
"truffle": "3.2.2"
Expand Down
43 changes: 43 additions & 0 deletions test/MerkleProof.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
var MerkleProof = artifacts.require("./MerkleProof.sol");

import { sha3 } from "ethereumjs-util";
import MerkleTree from "./helpers/merkleTree.js";

contract('MerkleProof', function(accounts) {
let merkleProof;

before(async function() {
merkleProof = await MerkleProof.new();
});

describe("verifyProof", function() {
it("should return true for a valid Merkle proof", async function() {
const elements = ["a", "b", "c", "d"].map(el => sha3(el));
Copy link
Contributor

Choose a reason for hiding this comment

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

See the comment above. Shouldn't the MerkleTree class be responsible for initially hashing the elements?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, I think that makes sense from a usability standpoint. A user can generate a merkle tree off-chain by passing in an array of elements and then when the user wants to submit a proof the user uses the same merkle tree to generate a proof and submits it along with the hash of the element to be proved

const merkleTree = new MerkleTree(elements);

const root = merkleTree.getHexRoot();

const proof = merkleTree.getHexProof(elements[0]);

const leaf = merkleTree.bufToHex(elements[0]);

const result = await merkleProof.verifyProof(proof, root, leaf);
assert.isOk(result, "verifyProof did not return true for a valid proof");
});

it("should return false for an invalid Merkle proof", async function() {
const elements = ["a", "b", "c"].map(el => sha3(el));
const merkleTree = new MerkleTree(elements);

const root = merkleTree.getHexRoot();

const proof = merkleTree.getHexProof(elements[0]);
const badProof = proof.slice(0, proof.length - 32);

const leaf = merkleTree.bufToHex(elements[0]);

const result = await merkleProof.verifyProof(badProof, root, leaf);
assert.isNotOk(result, "verifyProof did not return false for an invalid proof");
});
});
});
135 changes: 135 additions & 0 deletions test/helpers/merkleTree.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
import { sha3 } from "ethereumjs-util";

export default class MerkleTree {
constructor(elements) {
// Filter empty strings
this.elements = elements.filter(el => el);

// Check if elements are 32 byte buffers
if (this.elements.some(el => el.length !== 32 || !Buffer.isBuffer(el))) {
throw new Error("Elements must be 32 byte buffers");
}

// Deduplicate elements
this.elements = this.bufDedup(this.elements);
// Sort elements
this.elements.sort(Buffer.compare);

// Create layers
this.layers = this.getLayers(this.elements);
}

getLayers(elements) {
if (elements.length == 0) {
return [[""]];
}

const layers = [];
layers.push(elements);

// Get next layer until we reach the root
while (layers[layers.length - 1].length > 1) {
layers.push(this.getNextLayer(layers[layers.length - 1]));
}

return layers;
}

getNextLayer(elements) {
return elements.reduce((layer, el, idx, arr) => {
if (idx % 2 === 0) {
// Hash the current element with its pair element
layer.push(this.combinedHash(el, arr[idx + 1]));
}

return layer;
}, []);
}

combinedHash(first, second) {
if (!first) { return second; }
if (!second) { return first; }

return sha3(this.sortAndConcat(first, second));
}

getRoot() {
return this.layers[this.layers.length - 1][0];
}

getHexRoot() {
return this.bufToHex(this.getRoot());
}

getProof(el) {
let idx = this.bufIndexOf(el, this.elements);

if (idx === -1) {
throw new Error("Element does not exist in Merkle tree");
}

return this.layers.reduce((proof, layer) => {
const pairElement = this.getPairElement(idx, layer);

if (pairElement) {
proof.push(pairElement);
}

idx = Math.floor(idx / 2);

return proof;
}, []);
}

getHexProof(el) {
const proof = this.getProof(el);

return this.bufArrToHex(proof);
}

getPairElement(idx, layer) {
const pairIdx = idx % 2 === 0 ? idx + 1 : idx - 1;

if (pairIdx < layer.length) {
return layer[pairIdx];
} else {
return null;
}
}

bufIndexOf(el, arr) {
for (let i = 0; i < arr.length; i++) {
if (el.equals(arr[i])) {
return i;
}
}

return -1;
}

bufDedup(elements) {
return elements.filter((el, idx) => {
return this.bufIndexOf(el, elements) === idx;
});
}

bufToHex(el) {
if (!Buffer.isBuffer(el)) {
throw new Error("Element is not a buffer");
}

return "0x" + el.toString("hex");
}

bufArrToHex(arr) {
if (arr.some(el => !Buffer.isBuffer(el))) {
throw new Error("Array is not an array of buffers");
}

return "0x" + arr.map(el => el.toString("hex")).join("");
}

sortAndConcat(...args) {
return Buffer.concat([...args].sort(Buffer.compare));
}
}