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

Replace secp_utils usage of big.Int with uint256 library. #353

Open
rodrigo-pino opened this issue Apr 15, 2024 · 6 comments
Open

Replace secp_utils usage of big.Int with uint256 library. #353

rodrigo-pino opened this issue Apr 15, 2024 · 6 comments
Labels
Difficulty: easy Easy - good first issue Duration: under a day Should only take a few hours optimization Modification of the code that can improve performance

Comments

@rodrigo-pino
Copy link
Contributor

rodrigo-pino commented Apr 15, 2024

In ./pkg/hintrunner/utils/secp_utils.go there are utilities for elliptic curves. They used big.Int unnecessarily because all values are smaller than 2**256 - 1.

The task is to replace all use of big.Int with uint256 which is waaaaaaay faster.

@rodrigo-pino rodrigo-pino added Difficulty: easy Easy - good first issue Duration: under a day Should only take a few hours labels Apr 15, 2024
@TAdev0
Copy link
Contributor

TAdev0 commented Apr 15, 2024

Hi @rodrigo-pino may i be assigned to this issue? thanks!

@rodrigo-pino
Copy link
Contributor Author

Hi @TAdev0 is yours. This are the things you need to do:

  1. Swap the arithmetic that uses big.Int for uint256.
  2. Output should be actual values and not references to them. (a.k.a. uint256 instead of *uint256)
  3. The functions getBaseBig & getSecPBig should return a constant and not perform any operation. Currently, they are: (i) creating a big int and (ii) setting it up using a string. They should return a uint256 without any of these operations. Also, it makes no sense they return a boolean as a second output.

Tell me if you need help with anything!

@rodrigo-pino
Copy link
Contributor Author

We are using github.com/holiman/uint256 as our uint256 library which is already part of the VM packages

@TAdev0
Copy link
Contributor

TAdev0 commented Apr 17, 2024

Thanks a lot for explanations, will be useful as this is my first Go contribution :)

@TAdev0 TAdev0 added the blocked Depends on another to be solved first label May 23, 2024
@TAdev0
Copy link
Contributor

TAdev0 commented Jun 22, 2024

@rodrigo-pino @cicr99 now all hints are implemented, we can discuss the scope of this issue
should we replace big.Int absolutely everywhere its possible with uint256?

@TAdev0 TAdev0 removed their assignment Jun 25, 2024
@TAdev0 TAdev0 removed the blocked Depends on another to be solved first label Jun 25, 2024
@TAdev0 TAdev0 added the optimization Modification of the code that can improve performance label Jul 11, 2024
@Tchisom17
Copy link

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

I have a strong background in backend software engineering with expertise in languages such as Go and Solidity. My experience with optimizing code for performance and efficiency, especially in systems where every millisecond counts, aligns well with the task at hand. Additionally, I have worked with cryptographic primitives, which include elliptic curve cryptography, making me familiar with the operations and optimizations necessary in this domain.

How I plan on tackling this issue

To approach the problem of replacing big.Int with uint256, I would first ensure I understand the precise usage of big.Int in the current codebase, particularly how it handles elliptic curve operations in secp_utils.go. Given that big.Int is a general-purpose library for handling arbitrarily large integers, it adds unnecessary overhead when dealing with values smaller than 2**256 - 1. This is where uint256 can provide a significant performance boost due to its fixed-size nature, which allows for more efficient operations at the CPU level.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Difficulty: easy Easy - good first issue Duration: under a day Should only take a few hours optimization Modification of the code that can improve performance
Projects
None yet
Development

No branches or pull requests

3 participants