Gas Optimizations

[c4-submissions]

See the markdown file with the details of this report here.

[141345]

3 r 4 nc

Defer writing to state variables(Save 2 SSTORE - 4400 Gas)
nc

Defer 1 SSTORE to later (Save 2200 Gas in case of reverts)
d

Avoid making an sstore if we might revert in the execution(Save 1 SSTORE: 2200 Gas if we revert)
d

Avoid an sstore in case of a revert(Save 2200 Gas)
d

Optimize function _addFunctions
r

Optimize function deposit(): Avoid making an external call in case of a revert
d

Function deposit() can be optimized by reordering the order of executions(avoid making external calls if we can make early reverts)
d

Making use of the modifier in this function increases gas cost due to the external call that can be avoided in case we revert on parameter validation
d

Reorder the checks to have cheaper checks first(Save 1 SLOAD in case of a revert - 2100 Gas)
d

Save an entire SLOAD by optimizing the order of execution(saves gas if we revert - 2100 Gas)
d

The fallback function Should validate msg.data.length first which could help avoid making a state load
d

Avoid reading state variables earlier on if we might revert on a different cheaper check(Save 1 SLOAD in case of a revert - 2100 Gas)
d

Cache totalDepositedAmountPerUser[_l1Token][_depositor] to a local variable
r

The whole thing should be done in assembly
r

Don't cache if using once
nc

No need to cache deriveL2GasPrice(_gasPrice, _l2GasPerPubdataByteLimit);
d

Evaluate the simple arithmetic offline
i

Unnecessary casting
nc

Caching calldata length increases gas
nc

[c4-pre-sort]

141345 marked the issue as sufficient quality report

[c4-judge]

GalloDaSballo marked the issue as grade-b

[lsaudit]

The whole thing should be done in assembly

I've run this test in Remix-IDE:

function testAAA(address _to, uint _amount) public
    {
      uint gas = gasleft();
     bool callSuccess;
     assembly {
            callSuccess := call(gas(), _to, _amount, 0, 0, 0, 0)
        }
      console.log(gas - gasleft());
    }
    
    function testBBB(address _to, uint _amount) public
    {
      uint gas = gasleft();
      assembly
       {

          if iszero(call(gas(), _to, _amount, 0, 0, 0, 0)){
                mstore(0x00,0x40678658)
 }
       }
      console.log(gas - gasleft());
    }

     
} 

testAAA() returned 6835, while testBBB() returned more gas: 6850.
IMHO, it's invalid, could anyone take another look if this is valid, please?

Don't cache if using once, No need to cache deriveL2GasPrice(_gasPrice, _l2GasPerPubdataByteLimit);

There are multiple of instances related to this issue. E.g., in my report I've identified about 40 variables which shoudn't be cached when they are used only once: #338

Honestly, it would be extremely unfair, if my report with more than 40 redundant variables was classified as 1 x NC, and this report - with 2 redundant variables only - the same, as 1 x NC.

This issue does not describe even 10% of the instances with above problem. There are multiple of other instances (please check my linked report) which should be also mentioned here. I do not think that this issue classifies as NC, since it's too cursory. A lot of instances are missing.

Caching calldata length increases gas

I created a simple PoC in Remix-IDE:

   function AAA(
        address[] calldata _callers,
        address[] calldata _targets,
        bytes4[] calldata _functionSigs,
        bool[] calldata _enables
    ) external  {
        uint gas = gasleft();
       uint256 callersLength = _callers.length;

        // The size of arrays should be equal
        require(callersLength == _targets.length, "yw");
        require(callersLength == _functionSigs.length, "yx");
        require(callersLength == _enables.length, "yy");

        for (uint256 i = 0; i < callersLength; i = i++) {
           uint x;
        }
      console.log(gas - gasleft());

    }

   function BBB(
        address[] calldata _callers,
        address[] calldata _targets,
        bytes4[] calldata _functionSigs,
        bool[] calldata _enables
    ) external  {
        uint gas = gasleft();
       

        // The size of arrays should be equal
        require(_callers.length == _targets.length, "yw");
        require(_callers.length == _functionSigs.length, "yx");
        require(_callers.length == _enables.length, "yy");

        for (uint256 i = 0; i < _callers.length; i = i++) {
           uint x;
        }
      console.log(gas - gasleft());

    }

AAA([],[],[],[]): 152 gas
BBB([],[],[],[]): 166 gas

IMHO, it's invalid, but I'd like to ask anyone to take another look into it

[GalloDaSballo]

I think this was judge fine

wrt assembly you must test in context and with optimizer on

Because with assembly you can skip fixing the fmp you should save gas + some unchecked math

Also your test is messing up the dispatcher
you must test on 2 separate contracts with the same function name

Recommend you use foundry

Not changing the score here