From 87337eb55a46dc8e4795b10dbd1a7e4f4fb59044 Mon Sep 17 00:00:00 2001 From: Martin Minkov Date: Wed, 3 Jul 2024 09:48:34 -0700 Subject: [PATCH 1/3] refactor(nullifier): deprecate isUnused, assertUnused, and setUsed methods 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. --- src/lib/provable/crypto/nullifier.ts | 50 +++++++++++++++++++++++----- 1 file changed, 41 insertions(+), 9 deletions(-) diff --git a/src/lib/provable/crypto/nullifier.ts b/src/lib/provable/crypto/nullifier.ts index 29a714006d..266c489fa9 100644 --- a/src/lib/provable/crypto/nullifier.ts +++ b/src/lib/provable/crypto/nullifier.ts @@ -96,17 +96,31 @@ class Nullifier extends Struct({ return Poseidon.hash(Group.toFields(this.public.nullifier)); } + /** + * @deprecated This method uses the deprecated {@link MerkleMapWitness.computeRootAndKey} which may be vulnerable to hash collisions in key indices. Use {@link isUnusedV2} instead, which utilizes the safer {@link MerkleMapWitness.computeRootAndKeyV2} method. + */ + isUnused(witness: MerkleMapWitness, root: Field) { + let [newRoot, key] = witness.computeRootAndKey(Field(0)); + key.assertEquals(this.key()); + let isUnused = newRoot.equals(root); + + let isUsed = witness.computeRootAndKey(Field(1))[0].equals(root); + // prove that our Merkle witness is correct + isUsed.or(isUnused).assertTrue(); + return isUnused; // if this is false, `isUsed` is true because of the check before + } + /** * Returns the state of the Nullifier. * * @example * ```ts * // returns a Bool based on whether or not the nullifier has been used before - * let isUnused = nullifier.isUnused(); + * let isUnused = nullifier.isUnusedV2(); * ``` */ - isUnused(witness: MerkleMapWitness, root: Field) { - let [newRoot, key] = witness.computeRootAndKey(Field(0)); + isUnusedV2(witness: MerkleMapWitness, root: Field) { + let [newRoot, key] = witness.computeRootAndKeyV2(Field(0)); key.assertEquals(this.key()); let isUnused = newRoot.equals(root); @@ -116,32 +130,50 @@ class Nullifier extends Struct({ return isUnused; // if this is false, `isUsed` is true because of the check before } + /** + * @deprecated This method uses the deprecated {@link MerkleMapWitness.computeRootAndKey} which may be vulnerable to hash collisions in key indices. Use {@link assertUnusedV2} instead, which utilizes the safer {@link MerkleMapWitness.computeRootAndKeyV2} method. + */ + assertUnused(witness: MerkleMapWitness, root: Field) { + let [impliedRoot, key] = witness.computeRootAndKey(Field(0)); + this.key().assertEquals(key); + impliedRoot.assertEquals(root); + } + /** * Checks if the Nullifier has been used before. * * @example * ```ts * // asserts that the nullifier has not been used before, throws an error otherwise - * nullifier.assertUnused(); + * nullifier.assertUnusedV2(); * ``` */ - assertUnused(witness: MerkleMapWitness, root: Field) { - let [impliedRoot, key] = witness.computeRootAndKey(Field(0)); + assertUnusedV2(witness: MerkleMapWitness, root: Field) { + let [impliedRoot, key] = witness.computeRootAndKeyV2(Field(0)); this.key().assertEquals(key); impliedRoot.assertEquals(root); } + /** + * @deprecated This method uses the deprecated {@link MerkleMapWitness.computeRootAndKey} which may be vulnerable to hash collisions in key indices. Use {@link setUsedV2} instead, which utilizes the safer {@link MerkleMapWitness.computeRootAndKeyV2} method. + */ + setUsed(witness: MerkleMapWitness) { + let [newRoot, key] = witness.computeRootAndKey(Field(1)); + key.assertEquals(this.key()); + return newRoot; + } + /** * Sets the Nullifier, returns the new Merkle root. * * @example * ```ts * // calculates the new root of the Merkle tree in which the nullifier is set to used - * let newRoot = nullifier.setUsed(witness); + * let newRoot = nullifier.setUsedV2(witness); * ``` */ - setUsed(witness: MerkleMapWitness) { - let [newRoot, key] = witness.computeRootAndKey(Field(1)); + setUsedV2(witness: MerkleMapWitness) { + let [newRoot, key] = witness.computeRootAndKeyV2(Field(1)); key.assertEquals(this.key()); return newRoot; } From 3076ec29e4b94b3c5222d6cc8fda5a3800bbc2be Mon Sep 17 00:00:00 2001 From: Martin Minkov Date: Wed, 3 Jul 2024 09:51:33 -0700 Subject: [PATCH 2/3] refactor(nullifier): update nullifier methods to use V2 versions 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. --- src/examples/nullifier.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/examples/nullifier.ts b/src/examples/nullifier.ts index 2ef4413c46..55c0dbdfdc 100644 --- a/src/examples/nullifier.ts +++ b/src/examples/nullifier.ts @@ -28,10 +28,10 @@ class PayoutOnlyOnce extends SmartContract { ); // we compute the current root and make sure the entry is set to 0 (= unused) - nullifier.assertUnused(nullifierWitness, nullifierRoot); + nullifier.assertUnusedV2(nullifierWitness, nullifierRoot); // we set the nullifier to 1 (= used) and calculate the new root - let newRoot = nullifier.setUsed(nullifierWitness); + let newRoot = nullifier.setUsedV2(nullifierWitness); // we update the on-chain root this.nullifierRoot.set(newRoot); From fb1e4fb53ea2e1ab97962a4090187d1289cdde1e Mon Sep 17 00:00:00 2001 From: Martin Minkov Date: Wed, 3 Jul 2024 09:54:31 -0700 Subject: [PATCH 3/3] feat: add new V2 methods for nullifier operations 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. --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7cc1cf260b..44fc05c058 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,11 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Added - `ForeignField`-based representation of scalars via `ScalarField` https://github.com/o1-labs/o1js/pull/1705 +- Introduced new V2 methods for nullifier operations: `isUnusedV2()`, `assertUnusedV2()`, and `setUsedV2()` https://github.com/o1-labs/o1js/pull/1715 + +### Deprecated + +- Deprecated `Nullifier.isUnused()`, `Nullifier.assertUnused()`, and `Nullifier.setUsed()` methods https://github.com/o1-labs/o1js/pull/1715 ## [1.4.0](https://github.com/o1-labs/o1js/compare/40c597775...ed198f305) - 2024-06-25