Skip to content

Merkle proof library with tests and docs #260

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.

Sign up for GitHub

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

Merged
frangio merged 7 commits into from
Sep 18, 2017
Merged

Merkle proof library with tests and docs #260

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3c4d0d0
MerkleProof library and initial stubbed out tests
yondonfu Jun 14, 2017
2e0bd06
Add tests, docs and MerkleTree helper
yondonfu Jun 15, 2017
30e2023
Fix indentation in MerkleProof.sol and remove mock contract
yondonfu Jun 15, 2017
bc3db5d
Fix weird indent issue for inline assembly
yondonfu Jun 15, 2017
863ad48
Check proof length multiple of 32. Use keccak256 instead of sha3
yondonfu Jul 28, 2017
24323d3
MerkleTree util class hashes elements
yondonfu Aug 7, 2017
bcda5bf
Merge branch 'master' into feature/merkleproof
yondonfu Sep 18, 2017
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
Prev Previous commit
Next Next commit
Fix weird indent issue for inline assembly
  • Loading branch information
@yondonfu
yondonfu committed Jun 15, 2017
commit bc3db5d4c11991b103899fc1fa266a2939c02135
4 changes: 2 additions & 2 deletions contracts/MerkleProof.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ library MerkleProof {
for (uint256 i = 32; i <= _proof.length; i += 32) {
assembly {
// Load the current element of the proof
proofElement := mload(add(_proof, i))
}
proofElement := mload(add(_proof, i))
Copy link
Contributor

@spalladino spalladino Jul 26, 2017 •
edited
Loading

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

@yondonfu yondonfu Jul 27, 2017

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 •
edited
Loading

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)
Expand Down