rdubois-crypto / FreshCryptoLib

Cryptographic Primitives for Blockchain Systems (solidity, cairo, C and rust)
MIT License
125 stars 22 forks source link

Reduced `FCL_Elliptic_ZZ.ecdsa_verify` gas cost ( -3249 gas ) #17

Closed jayden-sudo closed 1 year ago

jayden-sudo commented 1 year ago

With the code equivalent, executing FCL_Elliptic_ZZ.ecdsa_verify reduced gas cost by 3249 gas:

Gas
Before 212970 gas
After 209721 gas

Gas Optimization Records for ecdsa_verify:

  1. Changed from calling ModExp to staticcall ModExp, reducing the number of parameters.

    1. The function was also changed to view.
  2. Changed the call ModExp gas from not(0) to a constant max_uint256, reducing opcode calls.

  3. Replaced uint256[6] memory pointer with pointer := mload(0x40), reducing calls like MSTORE(0x40).

  4. Performed assembly optimizations.

    1. 
      
      // after if and(and(gt(x, 0), gt(y, 0)),and(gt(p, x), gt(p, y))) ```
    2. 
      
      // after `if and(iszero(scalar_u), iszero(scalar_v)) { return(X, 0x20)}` ```
    3. 
      
      // after switch x case 1{} case 2{} ```
    4. 
      
      // after iszero(x) ```
  5. Add ecAff_add_affinepoint :ecAff_add(gx, gy, Q0, Q1) -> ecAff_add_affinepoint(Q0, Q1)

  6. Calldata cache, avoid frequent call of calldataload

    1. 
      function f(uint256[2] calldata rs) returns (bool success){ 
      // before 
      if (rs[0] == 0 || rs[0] >= n || rs[1] == 0 || rs[1] >= n){ return false; } 
      // after 
      uint256 r = rs[0]; 
      uint256 s = rs[1]; 
      assembly{
      if or( or(iszero(r), iszero(s)), iszero(and(gt(n, r), gt(n, s))) ) { 
          return(success, 0x20) // return false 
      }
      }
      } ```
  7. inline point on curve check

    1. 
      // after assembly { // if(Q0==0 || Q0==0 || Q0>=p || Q1>=p){ return false; } if or( or(iszero(Q0), iszero(Q1)),iszero(and(gt(p, Q0), gt(p, Q1))) ) { return(success, 0x20) // return false } let LHS := mulmod(Q1, Q1, p) // y^2 let RHS := addmod(mulmod(mulmod(Q0, Q0, p), Q0, p), mulmod(Q0, a, p), p) // x^3+ax RHS := addmod(RHS, b, p) // x^3 + a*x + b
       if sub(LHS, RHS) {         // if LHS != RHS
           return(success, 0x20)  //  return false  (not on curve)
       }
      }
  8. Inline FCL_nModInv(s)

    1. 
      // after
      // inlined FCL_nModInv(s)
      uint256 sInv;
      assembly {
         sInv := ...
      }
rdubois-crypto commented 1 year ago
jayden-sudo commented 1 year ago
  • Inlining is important inside of a loop, when it is only a single call, the benefit is negligible and will render audit much complicated compared to gain.
  • It is hard to read the PR because of the indentation changes, there is a linter in the makefile in WebAuthn_forge: make lint-write (i might have forgotten to use it, will check and will PR a lint only change, separating linting from code modification)

Yes, thank you for the response. I completely agree with your suggestion. So, I just submitted a version that aligns with your advice: #18