Skip to content

fix: set correct overflow range#3

Merged
nikkolasg merged 1 commit into
mainfrom
fix/include-modulus-as-overflow
Sep 5, 2024
Merged

fix: set correct overflow range#3
nikkolasg merged 1 commit into
mainfrom
fix/include-modulus-as-overflow

Conversation

@Insun35

@Insun35 Insun35 commented Sep 3, 2024

Copy link
Copy Markdown

No description provided.

@silathdiir silathdiir left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think this fix is correct, good catch!
May be also valuable to send a PR to the upstream https://github.com/0xPolygonZero/plonky2-ecdsa. I found its code also has this bug, they may also help us confirm. Wdyt?

@Insun35

Insun35 commented Sep 4, 2024

Copy link
Copy Markdown
Author

I think this fix is correct, good catch! May be also valuable to send a PR to the upstream https://github.com/0xPolygonZero/plonky2-ecdsa. I found its code also has this bug, they may also help us confirm. Wdyt?

I just looked at it and realized there is already a fix PR for this issue😂. Thanks for suggesting though

@nikkolasg nikkolasg merged commit 1e8a320 into main Sep 5, 2024
@silathdiir silathdiir deleted the fix/include-modulus-as-overflow branch October 3, 2024 03:01
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.

3 participants