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

Fix and finite field operations in bindings/crypto module #1765

Merged
merged 1 commit into from
Jul 31, 2024

Conversation

MartinMinkov
Copy link
Contributor

@MartinMinkov MartinMinkov commented Jul 22, 2024

Description

🔗 bindings PR: o1-labs/o1js-bindings#287

This pull request addresses issues and has a slight optimization change in the finite field operations.

The following tasks were accomplished:

  1. Fixed isSquare and sqrt methods to ensure input is taken modulo p
  2. Optimized the equal method to avoid unnecessary modulo operations

Additional Information

These changes ensure that the finite field operations work correctly for all input values and improve performance by reducing unnecessary computations. The optimization in the equal method is particularly beneficial for frequently used operations.

It also addresses the issue found in the audit where with this code:

import { createForeignField } from 'o1js';

class Field17 extends createForeignField(17n) {}

// This works fine
console.log(Field17.Bigint.sqrt(0n));
// this runs forever  normally --- with the changes it prints 0n
console.log(Field17.Bigint.sqrt(17n));

@MartinMinkov MartinMinkov force-pushed the audit/field-curve-canon branch from 1ffef08 to 760714e Compare July 23, 2024 15:58
@MartinMinkov MartinMinkov changed the title Add canonicalize function for bindings/../finite-field.ts Fix and finite field operations in bindings/crypto module Jul 23, 2024
@MartinMinkov MartinMinkov force-pushed the audit/field-curve-canon branch 4 times, most recently from bc64277 to 379a942 Compare July 29, 2024 17:14
@MartinMinkov MartinMinkov force-pushed the audit/field-curve-canon branch from 379a942 to 11a0ba7 Compare July 30, 2024 17:05
@MartinMinkov MartinMinkov force-pushed the audit/field-curve-canon branch from 11a0ba7 to 836b760 Compare July 30, 2024 17:13
@MartinMinkov MartinMinkov merged commit df830d2 into main Jul 31, 2024
23 of 24 checks passed
@MartinMinkov MartinMinkov deleted the audit/field-curve-canon branch July 31, 2024 17:02
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