rdubois-crypto / FreshCryptoLib

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

Reduced FCL_Elliptic_ZZ.ecdsa_verify gas cost ( -2600 gas ) #18

Closed jayden-sudo closed 9 months ago

jayden-sudo commented 9 months ago

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

Gas
Before 212970 gas
After 210370 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. Replaced uint256[6] memory pointer with pointer := mload(0x40), reducing calls like MSTORE(0x40).

  3. Performed assembly optimizations.

    1. 
      // before 
      if eq(x,1){} if eq(x,2)
      // after
      switch x case 1{} case 2{} ```
    2. 
      // before
      eq(x, 0)
      // after
      iszero(x) ```
  4. 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]; 
      
      } ```
rdubois-crypto commented 9 months ago

This is ok for me, just a question for the switch case intead of nested if, did u test a positive impact of the gas cost ? I remember avoiding them due to lesser efficiency?

jayden-sudo commented 9 months ago

This is ok for me, just a question for the switch case intead of nested if, did u test a positive impact of the gas cost ? I remember avoiding them due to lesser efficiency?

Using switch may lead to higher gas consumption, but it's not entirely accurate. For example, consider the following program:

// SPDX-License-Identifier: GPL-3.0
pragma solidity ^0.8.21;

contract Test {
    function testSwitch(uint256 i) public view returns (uint256 res){
        uint256 gas = gasleft();
        assembly{
            switch i
            case 1 {
                    i := add(i,2)
            }
            case 2 {
                    i := add(i,3)
            }
            case 3 {
                    i := add(i,4)
            }
        }
        return gas - gasleft();
    }

    function testIF(uint256 i) public view returns (uint256 res){
        uint256 gas = gasleft();
        assembly{
            if eq(i,1){
                i := add(i,2)
            }
            if eq(i,2){
                i := add(i,3)
            }
            if eq(i,3){
                i := add(i,4)
            }
        }
        return gas - gasleft();
    }
}

When viaIR is false and optimizer is false:

When viaIR is true and optimizer is false:

Especially when viaIR is true, using switch often results in higher gas efficiency. In fact, the gas efficiency improvement in this PR mainly relies on this change.

But this is indeed confusing. If the first if condition can be satisfied, then using switch is clearly superior. If there are many branches in the switch, and often the if conditions are only satisfied later, using if might be better (of course, in such cases, we can optimize by reorder the conditions to prioritize the most frequently hit one at the top of the switch).

rdubois-crypto commented 9 months ago

Please provide two distincts PR, with and without the switch case. view one is ok, switch will depend on the concrete bench (not toy example).

jayden-sudo commented 9 months ago

Please provide two distincts PR, with and without the switch case. view one is ok, switch will depend on the concrete bench (not toy example).

Okay, I'll submit it soon 🫡

jayden-sudo commented 9 months ago

Please provide two distincts PR, with and without the switch case. view one is ok, switch will depend on the concrete bench (not toy example).

without 'switch': #22 , only 0.3kgas saved. (hope this PR helps you complete the gas efficiency comparison)

rdubois-crypto commented 9 months ago

Closed with adoption of same contributor no switch PR version. (switch case only PR can be submitted now).

jayden-sudo commented 9 months ago

Closed with adoption of same contributor no switch PR version. (switch case only PR can be submitted now).

Thank you. Honestly, changing "if" to "switch" and saving 2K gas perplexed me as well. Before submitting the PR( if -> switch), I'll examine the compiled OP code, identify the reasons for saving 2K gas, ensure that this gas efficiency improvement is sustainable, and provide detailed explanations for the gas efficiency improvements in the PR.