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

Add merkle whitelist voting strategy #384

Merged
merged 10 commits into from
Nov 22, 2022
Merged

Add merkle whitelist voting strategy #384

merged 10 commits into from
Nov 22, 2022

Conversation

Orland0x
Copy link
Contributor

@Orland0x Orland0x commented Nov 14, 2022

closes #382

@Orland0x Orland0x requested a review from pscott November 15, 2022 11:25
Copy link
Contributor

@pscott pscott left a comment

Choose a reason for hiding this comment

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

I think we can add a bit more tests, and I think I'm missing why we need to sort the hashes :)

Also, is there an npm package that can take a hash_function as a parameter to build a merkletree?:)

test/starknet/MerkleProof.test.ts Show resolved Hide resolved
test/starknet/MerkleProof.test.ts Outdated Show resolved Hide resolved
test/starknet/MerkleProof.test.ts Show resolved Hide resolved
test/starknet/MerkleWhitelist.test.ts Show resolved Hide resolved
@Orland0x
Copy link
Contributor Author

I think we can add a bit more tests, and I think I'm missing why we need to sort the hashes :)

Also, is there an npm package that can take a hash_function as a parameter to build a merkletree?:)

If you dont sort the hashes, then you need an additional positional argument for each hash in order to reconstruct the root. So it seems like standard practice to do so.

Yeah there is a lib I tried to used called merkletreejs. I couldn't get it to work with pedersen though. Pedersen is unusual in that it takes 2 numbers and produces the hash whereas most hash functions concatenate the numbers (eg with abi.encode) then hash that single number.

Agreed about the tests - will add more. I basically just copied this implementation, so wasn't too worried about correctness. https://github.com/ncitron/cairo-merkle-distributor

@Orland0x Orland0x requested a review from pscott November 22, 2022 15:20
Copy link
Contributor

@pscott pscott left a comment

Choose a reason for hiding this comment

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

All good ser!

utACK ;) (but CI will test for me :D)

@Orland0x Orland0x merged commit 2e7e70e into develop Nov 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create Whitelist Voting Strategy using a merkle tree
2 participants