search

code-423n4 / 2022-06-yieldy-findings

0 stars 0 forks source link

Gas Optimizations #175

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Executive Summary

Below are listed gas savings refactorings along with the estimated gas saved.

Reducing SLOADS is by far the best way to save gas on this codebase, for that reason most of the advice is based on that

Minor gas savings are also listed, but reducing SLOADs should be prioritized especially as it will save gas for end users

Optimized setCurvePool - 4400 gas

https://github.com/code-423n4/2022-06-yieldy/blob/524f3b83522125fb7d4677fa7a7e5ba5a2c0fe67/src/contracts/Staking.sol#L157-L158

    function setCurvePool(address _curvePool) external onlyOwner {

Because the external address is not verified against a registry (e.g. the curve registry 0x0000000022D53366457F9d5E68Ec105046FC4383), the validation could be sidestepped by a malicious owner.

Because of that setToAndFromCurve() could be replaced by just inputting the pool indexes, saving the STATICALL and SLOADS which, beside the extra small math, should save 2.1k 2 (SLOAD) + 100 2 (STATICALL) = 4400 gas

Make Cowswap Fields Constants - 2.1k * 2 = 4200 gas

https://github.com/code-423n4/2022-06-yieldy/blob/524f3b83522125fb7d4677fa7a7e5ba5a2c0fe67/src/contracts/Staking.sol#L73-L74

        COW_SETTLEMENT = 0x9008D19f58AAbD9eD0D60971565AA8510560ab41;
        COW_RELAYER = 0xC92E8bdf79f0507f65a392b0ab4667716BFE0110;

These values are hardcoded in the initializer, it would be better to hardcode them as constants

Will save 2.1k each time you read them

Storage Read when function param is cheaper - 100 gas

https://github.com/code-423n4/2022-06-yieldy/blob/524f3b83522125fb7d4677fa7a7e5ba5a2c0fe67/src/contracts/LiquidityReserve.sol#L82

The function is receiving a new _stakingContract but using the storage value to set approvals

Change

  IERC20Upgradeable(rewardToken).approve(
      stakingContract,
      type(uint256).max
  );

To

  IERC20Upgradeable(rewardToken).approve(
      _stakingContract,
      type(uint256).max
  );

Double SLOAD when MSTORE would be chaper (94 gas * 3) - 282 gas

Double SLOAD when you already have cached value in memory - 97 gas

In both isntances you check with _isClaimAvailable but you already cached Claim memory info = warmUpInfo[_recipient]. To avoid an additional SLOAD (100 gas), you could just pass the info to the function

https://github.com/code-423n4/2022-06-yieldy/blob/524f3b83522125fb7d4677fa7a7e5ba5a2c0fe67/src/contracts/Staking.sol#L465-L467

    function claim(address _recipient) public {
        Claim memory info = warmUpInfo[_recipient];
        if (_isClaimAvailable(_recipient)) {

Also applies here https://github.com/code-423n4/2022-06-yieldy/blob/524f3b83522125fb7d4677fa7a7e5ba5a2c0fe67/src/contracts/Staking.sol#L425-L430

        Claim storage info = warmUpInfo[_recipient];

        // if claim is available then auto claim tokens
        if (_isClaimAvailable(_recipient)) {
            claim(_recipient);
        }

Refactor to 

_isClaimAvailable(info)

Double SLOAD - 94 gas

https://github.com/code-423n4/2022-06-yieldy/blob/524f3b83522125fb7d4677fa7a7e5ba5a2c0fe67/src/contracts/BatchRequests.sol#L50-L59

    function canBatchContractByIndex(uint256 _index)
        external
        view
        returns (address, bool)
    {
        return (
            contracts[_index],
            IStaking(contracts[_index]).canBatchTransactions()
        );
    }

Would recommend refactoring to 

address contract = contracts[_index];
  return (
      contract,
      IStaking(contract).canBatchTransactions()
  );

Which will save 94 gas

Expensive read from storage - 191 gas per iteration

https://github.com/code-423n4/2022-06-yieldy/blob/524f3b83522125fb7d4677fa7a7e5ba5a2c0fe67/src/contracts/BatchRequests.sol#L14-L27

    function sendWithdrawalRequests() external {
        uint256 contractsLength = contracts.length;
        for (uint256 i; i < contractsLength; ) {
            if (
                contracts[i] != address(0) &&
                IStaking(contracts[i]).canBatchTransactions()
            ) {
                IStaking(contracts[i]).sendWithdrawalRequests();
            }
            unchecked {
                ++i;
            }
        }
    }

We could instead cache the value

address contract = contracts[i];

Saving 94 gas on every iteration which returns false And saving another 97 gas when doing the sendWithdrawalRequests

For a total of 191 gas per iteration because we use Memory intead of Storage

Same deal - reading contracts from storage instead of caching in memory - 94 gas per loop

https://github.com/code-423n4/2022-06-yieldy/blob/524f3b83522125fb7d4677fa7a7e5ba5a2c0fe67/src/contracts/BatchRequests.sol#L29-L44

    function canBatchContracts() external view returns (Batch[] memory) {
        uint256 contractsLength = contracts.length;
        Batch[] memory batch = new Batch[](contractsLength);
        for (uint256 i; i < contractsLength; ) {
            bool canBatch = IStaking(contracts[i]).canBatchTransactions();
            batch[i] = Batch(contracts[i], canBatch);
            unchecked {
                ++i;
            }
        }
        return batch;
    }

We can save 94 gas per iteration here as well

Additional SLOAD when you already cached the value - 97 gas

https://github.com/code-423n4/2022-06-yieldy/blob/524f3b83522125fb7d4677fa7a7e5ba5a2c0fe67/src/contracts/Staking.sol#L319-L321

    function _requestWithdrawalFromTokemak(uint256 _amount) internal {
        ITokePool tokePoolContract = ITokePool(TOKE_POOL);
        uint256 balance = ITokePool(TOKE_POOL).balanceOf(address(this));

You can just use tokePoolContract at this point, saving 97 gas

Similarly you can use a memory cache here as well: https://github.com/code-423n4/2022-06-yieldy/blob/524f3b83522125fb7d4677fa7a7e5ba5a2c0fe67/src/contracts/Staking.sol#L144-L147

        uint256 totalTokeAmount = IERC20Upgradeable(TOKE_TOKEN).balanceOf(
            address(this)
        );
        IERC20Upgradeable(TOKE_TOKEN).safeTransfer(

Unnecessary MSTORE - 6 gas * 2 = 12 gas

https://github.com/code-423n4/2022-06-yieldy/blob/524f3b83522125fb7d4677fa7a7e5ba5a2c0fe67/src/contracts/Staking.sol#L329-L345

    function _depositToTokemak(uint256 _amount) internal {
        ITokePool tokePoolContract = ITokePool(TOKE_POOL);
        tokePoolContract.deposit(_amount);
    }
    function _getTokemakBalance() internal view returns (uint256) {
        ITokePool tokePoolContract = ITokePool(TOKE_POOL);
        return tokePoolContract.balanceOf(address(this));
    }

You're storing the casted version of the address, but you could simply cast the storage value, avoiding the extra 6 gas

Change to:

    function _getTokemakBalance() internal view returns (uint256) {
        return ITokePool(TOKE_POOL).balanceOf(address(this));
    }

Additional instance: https://github.com/code-423n4/2022-06-yieldy/blob/524f3b83522125fb7d4677fa7a7e5ba5a2c0fe67/src/contracts/Staking.sol#L120-L121

        ITokeReward tokeRewardContract = ITokeReward(TOKE_REWARD);

Avoid reading from storage if you have a memory copy - 97 * 11 = 1067 gas

All fields in initialize are being read from storage, each read costing an additional 97 gas over just reading from the memory parameters

https://github.com/code-423n4/2022-06-yieldy/blob/524f3b83522125fb7d4677fa7a7e5ba5a2c0fe67/src/contracts/Staking.sol#L77-L92


        if (CURVE_POOL != address(0)) {
            IERC20(TOKE_POOL).approve(CURVE_POOL, type(uint256).max);
            setToAndFromCurve();
        }

        IERC20(STAKING_TOKEN).approve(TOKE_POOL, type(uint256).max);
        IERC20Upgradeable(YIELDY_TOKEN).approve(
            LIQUIDITY_RESERVE,
            type(uint256).max
        );
        IERC20Upgradeable(YIELDY_TOKEN).approve(
            LIQUIDITY_RESERVE,
            type(uint256).max
        );
        IERC20Upgradeable(TOKE_TOKEN).approve(COW_RELAYER, type(uint256).max);

Can be changed to _curvePool != address(0) etc.. Each SLOAD is 100 gas vs the CALLDATALOAD which costs 3 gas

There's 11 ALL_CAPS storage vars, so the refactoring will save 1067 gas on the initializer call

moose-code commented 2 years ago

Very nice and practical report quantifying savings and focusing on what can actually move the needle.