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

Update @zk-kit/lean-imt #2184

Merged
merged 3 commits into from
Dec 10, 2024
Merged

Update @zk-kit/lean-imt #2184

merged 3 commits into from
Dec 10, 2024

Conversation

artwyman
Copy link
Member

Just a cleanliness update while I check for changes in our various dependencies. Here's the story behind the updated comment, for posterity:

I originally planned to use 0 as the hash value for PODNull, but decided against it because LeanIMT behaved badly. Turns out that behavior was a bug, fixed here: privacy-scaling-explorations/zk-kit#355

However, the test for that bug points out that people might assume that 0 has special meaning in many situations (including using it to mean removal from a Merkle tree), so it's still a good idea to use a unique non-zero value for PODNull's hash. In this PR I've updated the comment on the related utest to describe the general reasoning, rather than point to LeanIMT in particular.

@artwyman artwyman requested a review from rrrliu December 10, 2024 01:26
@artwyman artwyman self-assigned this Dec 10, 2024
…/lean-imt-update

# Conflicts:
#	packages/lib/pod/package.json
#	yarn.lock
@artwyman artwyman enabled auto-merge December 10, 2024 02:34
@artwyman artwyman added this pull request to the merge queue Dec 10, 2024
Merged via the queue into main with commit 77f5c78 Dec 10, 2024
1 check passed
@artwyman artwyman deleted the artwyman/lean-imt-update branch December 10, 2024 03:42
@artwyman
Copy link
Member Author

Oh, I got wrapped up in the process of cleaning up tasks and forgot to actually wait for review on this one. Sorry @rrrliu. I think the risk is minimal so I'll leave it for review-after-merge instead of revering, but let me know if you have concerns.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant