-
Notifications
You must be signed in to change notification settings - Fork 111
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
Support custom hashing functions #34
Conversation
if (data.format !== 'standard-v1') { | ||
throw new Error(`Unknown format '${data.format}'`); | ||
} | ||
return new StandardMerkleTree( | ||
data.tree.map(hexToBytes), | ||
data.values, | ||
data.leafEncoding, | ||
hashFn ?? standardHash, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an issue. If the tree was built using a non standard hash, an dumped() to a file, how do you know how to load it? What happens if its loaded using no hashFn, or the wrong hashFn?
When we built this library, we felt like dump/load was essential. Using custom hashes breaks some assumption about the ability to dump/load seemlesly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The dump should contain a line that says "hash": "custom"
. load
should have a hashFn
argument that must be present if the dump contains this line. The user is responsible for passing in the correct hash, although we could do a minimal sanity check at load time to error early (i.e., try to hash something and see that it matches an expected value taken from the dump).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something like this ?
Lines 77 to 94 in 5e50904
static load(data: MerkleTreeData<BytesLike>, hashPair?: HashPairFn): SimpleMerkleTree { | |
switch (data.format) { | |
case 'simple-v1': | |
if (hashPair !== undefined) throwError(`Format '${data.format}' does not support custom hashing functions`); | |
break; | |
case 'custom-v1': | |
if (hashPair === undefined) throwError(`Format '${data.format}' requires a hashing function`); | |
// TODO: check that the hash matches the data. | |
break; | |
default: | |
throwError(`Unknown format '${data.format}'`); | |
} | |
return new SimpleMerkleTree( | |
data.tree, | |
data.values.map(({ value, treeIndex }) => ({ value: toHex(value), treeIndex })), | |
hashPair, | |
); | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah something like that.
Also, I'm don't really like the idea that the StandardMerkleTree, which enforces the use of double hashing, is the place where we support custom hashes. I'd much rather put that in the SimpleMerkleTree structure (from #31), where leaf hashes are already left to the user's discression, and where the custom hash would be a use defined Typical example, again, is tornado. Their function hashLeftRight(
IHasher _hasher,
bytes32 _left,
bytes32 _right
) public pure returns (bytes32) {
require(uint256(_left) < FIELD_SIZE, "_left should be inside the field");
require(uint256(_right) < FIELD_SIZE, "_right should be inside the field");
uint256 R = uint256(_left);
uint256 C = 0;
(R, C) = _hasher.MiMCSponge(R, C);
R = addmod(R, uint256(_right), FIELD_SIZE);
(R, C) = _hasher.MiMCSponge(R, C);
return bytes32(R);
}
function _insert(bytes32 _leaf) internal returns (uint32 index) {
// ...
currentLevelHash = hashLeftRight(hasher, left, right);
// ...
} Note that:
|
Closing in favor of #39 |
Description
Add support for custom hashing functions.