Refactor Int64: Introduce canonical zero representation and consistent positivity checks #1735
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Enhance the JSDoc comments for the `negV2` method: - Clarify that it returns a new Int64 instance with the negated value - Provide an example of usage - Add `@see` tags for related methods - Mention the implicit error case - Enhance the JSDoc comments for the `modV2` method: - Clarify the behavior and conditions for the remainder calculation - Note the convention for handling negative numbers - Provide an example of usage - Mention the implicit error case for zero or negative divisor The improved documentation aims to provide clearer explanations, usage examples, and error cases for better understanding and usage of these methods.
…nt64.fromObject to avoid unnecessary checks and improve performance when creating Int64 instances from objects refactor(errors.ts): use Int64.Unsafe.fromObject instead of Int64.fromObject to avoid unnecessary checks and improve performance when creating Int64 instances from objects in error handling feat(int.ts): add Int64.Unsafe.fromObject method to create Int64 instances from objects without checks, and an unsafe instance method to Int64 class for creating instances without checks
…te() method BREAKING CHANGE: The Int64 constructor is now deprecated due to potential security issues with ambiguous zero representation. Use Int64.create() for safe instance creation. feat(Int64): introduce Int64.create() for safe instance creation with canonical zero representation docs(Int64): add warning about security implications of unsafe constructor and recommend using Int64.create()
…presentation The `fromField` method in the `Int64` class has been refactored to ensure a canonical zero representation for all instances, including zero. The changes include: 1. Introducing a new private method `fromFieldUncheckedV2` that checks if the input `Field` is within the valid range for `Int64` and uses `Int64.create` to enforce a canonical zero representation. 2. Adding a new static method `fromFieldV2` that uses `fromFieldUncheckedV2` for constant inputs and creates a witness using `fromFieldUncheckedV2` for variable inputs, proving consistency with the original field. These changes guarantee that all `Int64` instances, including zero, have a consistent representation, eliminating potential ambiguities in zero handling. refactor(Int64): add divV2 method with canonical zero representation A new method `divV2` has been added to the `Int64` class to perform integer division with canonical zero representation. The changes include: 1. The `divV2` method performs the same division operation as `div` but ensures that the result always has a canonical representation, particularly for zero results. 2. It uses `Int64.create` to create the result instance, ensuring a consistent representation for all results, including zero. This method guarantees that all division results, including zero, have a consistent representation, eliminating potential ambiguities in zero handling.
…sitive The isPositiveV2 method is added to the Int64 class to check if the value is strictly greater than zero. It returns true if the value is positive and false otherwise. The method uses the magnitude and sign (sgn) properties of the Int64 instance to determine positivity. It ensures that the value is not equal to zero and has a positive sign. The JSDoc comments provide clear documentation, specifying the behavior and return value of the method. It also includes a remark to clarify that zero is considered non-positive in this context, ensuring consistency with the mathematical definition of "positive" and distinguishing it from other methods that may treat zero as non-negative. This addition enhances the functionality of the Int64 class by providing a convenient way to check for strictly positive values.
MartinMinkov
force-pushed
the
audit/is-positive
branch
from
6c23077
to
ec23c40
Compare
MartinMinkov
commented
Jul 10, 2024
The fromObjectV2 method is added to the Int64 class to provide a more explicit way of creating an Int64 instance from an object. This method takes an object with magnitude and sgn properties, where sgn can be of type Sign or bigint, and returns a new Int64 instance. The addition of this method improves the clarity and readability of the code by explicitly specifying the expected types of the input object properties. It also provides a more controlled way of creating Int64 instances from objects, as it uses the UInt64.from and Sign.fromValue methods to ensure proper conversion of the input values. This change is a refactoring effort to enhance the API of the Int64 class and make it more intuitive and easier to use for developers working with the library.
feat(Int64): introduce Int64.create() for safe instance creation with canonical zero representation feat(Int64): add new V2 methods - fromFieldV2(), fromFieldUncheckedV2(), divV2(), isPositiveV2() deprecate(Int64): deprecate Int64 constructor in favor of Int64.create() deprecate(Int64): deprecate original fromField(), fromFieldUnchecked(), div(), isPositive() methods in favor of V2 versions
…ew implementation docs(Int64): add JSDoc comment to explain the deprecation of the old fromObject method fix(Int64): change the type signature of the deprecated fromObject method to only accept UInt64 and Sign as parameters to match the new implementation and avoid type errors
The unsafe method in the Int64 class has been removed. This method allowed the creation of Int64 instances with ambiguous zero representation, which could lead to potential issues if not handled properly. By removing this method, we ensure that all Int64 instances are created with explicit checks and maintain a consistent and unambiguous representation.
refactor(int.ts): remove unused public method fromFieldV2 The removed methods were intended to provide a canonical zero representation for Int64 instances created from Field values. However, they were not being used in the codebase and added unnecessary complexity. By removing these unused methods, the code becomes cleaner and more focused on the actively used functionality.
The `Int64.isPositive()` method was not deprecated in this changelog, so the mentions of its deprecation were removed for accuracy.
… ambiguity vulnerability
…ct being the original CircuitValue method docs(int.ts): add @deprecated tag to Int64.Unsafe.fromObject method with a suggestion to use Int64.fromObjectV2 instead
…oid zero ambiguity vulnerability" This reverts commit 4740d03.
- Introduce `fromFieldV2()` and `fromFieldUncheckedV2()` methods for creating Int64 instances from Field elements - Add `divV2()` method for performing division on Int64 instances - Include `isPositiveV2()` method to check if an Int64 instance is positive These new methods provide additional functionality and improved performance for working with Int64 values in the SnarkyJS library.
- Introduce `divV2()` method for Int64 division - Deprecate original `div()` method in favor of V2 version refactor(changelog): remove mentions of `fromFieldV2()` and `fromFieldUncheckedV2()` methods The `fromFieldV2()` and `fromFieldUncheckedV2()` methods were mentioned as added features, but they are not present in the final changelog. This commit removes those mentions to avoid confusion.
…nt64.mod() due to incorrect behavior on -0, and recommend using Int64.isPositiveV2() and Int64.modV2() instead for improved security and correctness
docs(CHANGELOG): mention deprecation of `fromObject` method for `Int64` in favor of V2 version
mitschabaude
approved these changes
Jul 12, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
v2 PR: #1742