search

NethermindEth / cairo-vm-go

A virtual machine for Cairo written in Go
MIT License
71 stars 43 forks source link

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

Open rodrigo-pino opened 2 months ago

rodrigo-pino commented 2 months ago

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.

TAdev0 commented 2 months ago

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

rodrigo-pino commented 2 months ago

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 commented 2 months ago

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

TAdev0 commented 2 months ago

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

TAdev0 commented 2 weeks ago

@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?