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

Refactor Nullifier methods to use safer MerkleMapWitness operations #1715

Merged
merged 3 commits into from
Jul 3, 2024

Conversation

MartinMinkov
Copy link
Contributor

No description provided.

@MartinMinkov MartinMinkov marked this pull request as ready for review July 2, 2024 20:04
@MartinMinkov MartinMinkov marked this pull request as draft July 2, 2024 20:04
@MartinMinkov MartinMinkov changed the title refactor(nullifier.ts, merkle-map.test.ts): Use computeRootAndKeyV2 Introduce NullifierV2 that uses computeRootAndKeyV2 Jul 2, 2024
@MartinMinkov MartinMinkov marked this pull request as ready for review July 2, 2024 21:45
Copy link
Contributor

@mitschabaude mitschabaude left a comment

Choose a reason for hiding this comment

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

Wouldn't it cause less code duplication to only deprecate the individual methods that use computeRootAndKey?

Many people might not even use those methods so they can just continue to use Nullifier without disruption

@mitschabaude
Copy link
Contributor

So I'm saying, I don't see why we have to add another version of the class in this case

@MartinMinkov MartinMinkov changed the title Introduce NullifierV2 that uses computeRootAndKeyV2 Refactor Nullifier methods to use safer MerkleMapWitness operations Jul 3, 2024
@MartinMinkov
Copy link
Contributor Author

So I'm saying, I don't see why we have to add another version of the class in this case

Yeah, that was my mistake. Changed the PR to this approach instead!

…thods in favor of V2 versions

feat(nullifier): add isUnusedV2, assertUnusedV2, and setUsedV2 methods that use the safer MerkleMapWitness.computeRootAndKeyV2
docs(nullifier): add deprecation notices to isUnused, assertUnused, and setUsed methods, and update examples to use V2 versions

The original methods used the deprecated MerkleMapWitness.computeRootAndKey which may be vulnerable to hash collisions in key indices. The new V2 methods utilize the safer MerkleMapWitness.computeRootAndKeyV2 to mitigate this issue and improve the security of the nullifier operations.
The nullifier methods `assertUnused` and `setUsed` have been updated to their V2 counterparts `assertUnusedV2` and `setUsedV2` respectively. This change is made to use the latest versions of these methods which likely include performance improvements, bug fixes, or additional functionality compared to the previous versions.
The new methods `isUnusedV2()`, `assertUnusedV2()`, and `setUsedV2()` have been introduced to provide improved functionality for nullifier operations.

BREAKING CHANGE: deprecate Nullifier.isUnused(), Nullifier.assertUnused(), and Nullifier.setUsed() methods

The existing `Nullifier.isUnused()`, `Nullifier.assertUnused()`, and `Nullifier.setUsed()` methods have been deprecated in favor of the new V2 methods. These deprecated methods will be removed in a future release. Please update your code to use the new V2 methods instead.
@MartinMinkov MartinMinkov merged commit e8f64bb into main Jul 3, 2024
14 checks passed
@MartinMinkov MartinMinkov deleted the feat/fix-nullifier branch July 3, 2024 18:34
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.

2 participants