Yield Witch v2 contest - m_Rassska's results

Fixed-rate borrowing and lending on Ethereum

General Information

Platform: Code4rena

Start Date: 14/07/2022

Pot Size: $25,000 USDC

Total HM: 2

Participants: 63

Period: 3 days

Judge: PierrickGT

Total Solo HM: 1

Id: 147

League: ETH

Yield

Findings Distribution

Researcher Performance

Rank: 43/63

Findings: 1

Award: $33.33

🌟 Selected for report: 0

🚀 Solo Findings: 0

Table of contents

Disclaimer<a name="0x0"></a>

  • Please, consider everything described below as a general recommendation. These notes will represent potential possibilities to optimize gas consumption. It's okay, if something is not suitable in your case. Just let me know the reason in the comments section. Enjoy!

[G-01] Consider a = a + b instead of a += b<a name="G-01"></a>

Description:

  • It has an impact on the deployment cost and the cost for distinct transaction as well.

All occurances:

  • Contracts:

      file: contracts/.**./Witch.sol
      ...............................
      
        // Lines: [204-204]
          limits_.sum += auction_.ink;
    
        // Lines: [259-259]
          limits[auction_.ilkId][auction_.baseId].sum -= auction_.ink;
    
        // Lines: [430-430]
          limits_.sum -= auction_.ink;    
    
        // Lines: [443-444]
          auction_.ink -= inkOut.u128();
          auction_.art -= artIn.u128();
    
        // Lines: [450-450]
          limits_.sum -= inkOut.u128();
    
        // Lines: [598-598]
          liquidatorCut -= auctioneerCut;
    

[G-02] Cache state variables, MLOAD << SLOAD<a name="G-02"></a>

Description:

  • MLOAD costs only 3 units of gas, SLOAD(warm access) is about 100 units. Therefore, cache, when it's possible.

All occurances:

  • Contracts:

      file: src/Witch.sol
      ...............................
    
        // Lines: [254-261]
          // Comment: cache auction_ to get MLOAD for auction_.start, auction_.ilkId, auction_.baseId, auction_.ink, auction_.owner; 
    
              DataTypes.Auction storage auction_ = auctions[vaultId];
              require(auction_.start > 0, "Vault not under auction");
              require(cauldron.level(vaultId) >= 0, "Undercollateralized");
    
              // Update concurrent collateral under auction
              limits[auction_.ilkId][auction_.baseId].sum -= auction_.ink;
    
              _auctionEnded(vaultId, auction_.owner);
        ```

[G-03] Add require() before some computations, if it makes sense<a name="G-03"></a>

Description:

  • Everyting above require() takes some gas for execution, therefore if the statement reverts gas will not be retrieved.

All occurances:

  • Contracts:

      file: src/Witch.sol
      ...............................
      
        // Lines: [189-189]
          require(cauldron.level(vaultId) < 0, "Not undercollateralized");
    
        // Lines: [256-256]
          require(cauldron.level(vaultId) >= 0, "Undercollateralized");
    
        // Lines: [437-440]
          require(
              auction_.art - artIn >= debt.min * (10**debt.dec),
              "Leaves dust"
          );
    

[G-04] Internal functions can be inlined<a name="G-04"></a>

Description:

  • It takes some extra JUMPs which costs around 40-50 gas uints. In loops it will save significant amount of gas.

All occurances:

  • Contracts:

      file: src/Witch.sol
      ...............................
    
        // Lines: [214-218]
            function _auctionStarted(bytes12 vaultId) internal virtual {
                // The Witch is now in control of the vault under auction
                cauldron.give(vaultId, address(this));
                emit Auctioned(vaultId, uint32(block.timestamp));
            }
    
        // Lines: [268-272]
            function _auctionEnded(bytes12 vaultId, address owner) internal virtual {
                cauldron.give(vaultId, owner);
                delete auctions[vaultId];
                emit Ended(vaultId);
            }
      
        // Lines: [463-470]
          function _collateralBought(
              bytes12 vaultId,
              address buyer,
              uint256 ink,
              uint256 art
          ) internal virtual {
              emit Bought(vaultId, buyer, ink, art);
          }

[G-05] Unused the named return variables<a name="G-05"></a>

Description:

  • Unnecessary gas usage.

All occurances:

  • Contracts:

      file: src/Witch.sol
      ...............................
      
        // Lines: [176-179]
            function auction(bytes12 vaultId, address to)
                external
                returns (DataTypes.Auction memory auction_)
            {}
    
        // Lines: [286-298]
            function payBase(
              bytes12 vaultId,
              address to,
              uint128 minInkOut,
              uint128 maxBaseIn
          )
              external
              returns (
                  uint256 liquidatorCut,
                  uint256 auctioneerCut,
                  uint256 baseIn
              )
          {}
    
        // Lines: [344-356]
            function payFYToken(
                bytes12 vaultId,
                address to,
                uint128 minInkOut,
                uint128 maxArtIn
            )
                external
                returns (
                    uint256 liquidatorCut,
                    uint256 auctioneerCut,
                    uint256 artIn
                )
            {}
    
        // Lines: [528-540]
            function calcPayout(
                bytes12 vaultId,
                address to,
                uint256 maxArtIn
            )
                external
                view
                returns (
                    uint256 liquidatorCut,
                    uint256 auctioneerCut,
                    uint256 artIn
                )
            {}
    
        // Lines: [562-566]
            function _calcPayout(
                DataTypes.Auction memory auction_,
                address to,
                uint256 artIn
            ) internal view returns (uint256 liquidatorCut, uint256 auctioneerCut) {}
    
    

[G-06] Use private/internal for constants/immutable variables instead of public<a name="G-06"></a>

Description:

  • Optimization comes from not creating a getter function for each public instance.

All occurances:

  • Contracts:

      file: src/**/Witch.sol
      ...............................
      
        // Lines: [59-60]
          ICauldron public immutable cauldron;
          ILadle public ladle;
    
        // Lines: [63-63]
          uint128 public auctioneerReward = 0.01e18;
    
        // Lines: [63-69]
          mapping(bytes12 => DataTypes.Auction) public auctions;
          mapping(bytes6 => mapping(bytes6 => DataTypes.Line)) public lines;
          mapping(bytes6 => mapping(bytes6 => DataTypes.Limits)) public limits;
          mapping(address => bool) public otherWitches;
          mapping(bytes6 => mapping(bytes6 => bool)) public ignoredPairs;
    

[G-07] Define modifier/function to cover the same require statements<a name="G-07"></a>

Description:

  • Optimization relies on the deployemnt cost.

All occurances:

  • Contracts:

      file: src/Witch.sol
      ...............................
      
        // Lines: [255-255; 300-300; 358-358; 416-416]
          require(auction_.start > 0, "Vault not under auction");

[G-08] Use const values instead of type(uint256).max<a name="G-08"></a>

Description:

  • Not sure about readability, but it might be tangible in loops.

All occurances:

  • Contracts:

      file: src/**/Witch.sol
      ...............................
      
        // Lines: [584-584]
          if (duration == type(uint32).max) {}     // Interpreted as infinite duration

[G-09] Use uint instead of uint8, uint64, if it's possible<a name="G-09"></a>

Description:

  • I deployed a contract with only a single error and function and compared between:
    • error VaultAlreadyUnderAuction(bytes12 vaultId, address witch);.
    • error VaultAlreadyUnderAuction(bytes32 vaultId, address witch);.
  • The second one is saving about ~10k units of gas for deployment and 10k per each transaction.
  • However, defaining further errors with !bytes32 doesn't give a huge optimization.
  • The stack slots are 32bytes, just use 32bytes, if you are not trying to tight up the storage.

All occurances:

  • Contracts:

      file: src/**/Witch.sol
      ...............................
      
        // Lines: [26-38]
          error VaultAlreadyUnderAuction(bytes12 vaultId, address witch);
          error VaultNotLiquidable(bytes12 vaultId, bytes6 ilkId, bytes6 baseId);
          error AuctioneerRewardTooHigh(uint128 max, uint128 actual);
    
          event Auctioned(bytes12 indexed vaultId, uint256 indexed start);
          event Cancelled(bytes12 indexed vaultId);
          event Ended(bytes12 indexed vaultId);
          event Bought(
              bytes12 indexed vaultId,
              address indexed buyer,
              uint256 ink,
              uint256 art
          );
    
        // Lines: [42-57]
          event Point(bytes32 indexed param, address indexed value);
          event LineSet(
              bytes6 indexed ilkId,
              bytes6 indexed baseId,
              uint32 duration,
              uint64 proportion,
              uint64 initialOffer
          );
          event LimitSet(bytes6 indexed ilkId, bytes6 indexed baseId, uint128 max);
          event AnotherWitchSet(address indexed value, bool isWitch);
          event IgnoredPairSet(
              bytes6 indexed ilkId,
              bytes6 indexed baseId,
              bool ignore
          );
          event AuctioneerRewardSet(uint128 auctioneerReward);
    
          
        // Lines: [63-63]
          uint128 public auctioneerReward = 0.01e18;
    
        // Lines: [63-69]
          mapping(bytes12 => DataTypes.Auction) public auctions;
          mapping(bytes6 => mapping(bytes6 => DataTypes.Line)) public lines;
          mapping(bytes6 => mapping(bytes6 => DataTypes.Limits)) public limits;
          mapping(address => bool) public otherWitches;
          mapping(bytes6 => mapping(bytes6 => bool)) public ignoredPairs;
    
        // Lines: [95-101]
            function setLine(
                bytes6 ilkId,
                bytes6 baseId,
                uint32 duration,
                uint64 proportion,
                uint64 initialOffer
            ) external auth {}
    
        // Lines: [126-130]
            function setLimit(
                bytes6 ilkId,
                bytes6 baseId,
                uint128 max
            ) external auth {}
    
        // Lines: [150-154]
            function setIgnoredPair(
                bytes6 ilkId,
                bytes6 baseId,
                bool ignore
            ) external auth {}
    
        // Lines: [161-161]
            function setAuctioneerReward(uint128 auctioneerReward_) external auth {}
    
        // Lines: [176-179]
            function auction(bytes12 vaultId, address to)
                external
                returns (DataTypes.Auction memory auction_)
            {}
    
        // Lines: [214-214]
            function _auctionStarted(bytes12 vaultId) internal virtual {}
    
        // Lines: [232-232]
            uint128 art = uint256(balances.art).wmul(line.proportion).u128();
    
        // Lines: [234-236]
            uint128 ink = (art == balances.art)
              ? balances.ink
              : uint256(balances.ink).wmul(line.proportion).u128();
    
        // Lines: [253-253]
            function cancel(bytes12 vaultId) external {}
    
        // Lines: [268-268]
            function _auctionEnded(bytes12 vaultId, address owner) internal virtual {}
    
        // Lines: [286-295]
            function payBase(
                bytes12 vaultId,
                address to,
                uint128 minInkOut,
                uint128 maxBaseIn
            )
                external
                returns (
                    uint256 liquidatorCut,
                    uint256 auctioneerCut,
                    uint256 baseIn
                )
            {}
    
        // Lines: [303-305]
            uint128 artIn = uint128(
              cauldron.debtFromBase(auction_.seriesId, maxBaseIn)
            );
    
        // Lines: [344-356]
            function payFYToken(
                bytes12 vaultId,
                address to,
                uint128 minInkOut,
                uint128 maxArtIn
            )
                external
                returns (
                    uint256 liquidatorCut,
                    uint256 auctioneerCut,
                    uint256 artIn
                )
            {}
    
        // Lines: [409-414]
            function _updateAccounting(
                bytes12 vaultId,
                DataTypes.Auction memory auction_,
                uint256 inkOut,
                uint256 artIn
            ) internal {}
    
        // Lines: [463-468]
            function _collateralBought(
                bytes12 vaultId,
                address buyer,
                uint256 ink,
                uint256 art
            ) internal virtual {}
    
        // Lines: [528-540]
            function calcPayout(
                bytes12 vaultId,
                address to,
                uint256 maxArtIn
            )
                external
                view
                returns (
                    uint256 liquidatorCut,
                    uint256 auctioneerCut,
                    uint256 artIn
                )
            {}
    
      ```

[G-10] Use calldataload instead of mload<a name="G-10"></a>

Description:

  • After Berlin hard fork, to load a non-zero byte from calldata dropped from 64 units of gas to 16 units, therefore if you do not modify args, use a calldata instead of memory. Here you need to explicitly specify calldataload, or replace memory with calldata. If the args are pretty huge, allocating args in memory will cost a lot.

All occurances:

  • Contracts:

      file: src/**/Witch.sol
      ...............................
      
        // Lines: [223-229]
          function _calcAuction(
              DataTypes.Vault memory vault,
              DataTypes.Series memory series,
              address to,
              DataTypes.Balances memory balances,
              DataTypes.Debt memory debt
          ) internal view returns (DataTypes.Auction memory) {}
    
        // Lines: [386-391]
            function _payInk(
                DataTypes.Auction memory auction_,
                address to,
                uint256 liquidatorCut,
                uint256 auctioneerCut
            ) internal {}
    
    
        // Lines: [562-566]
            function _calcPayout(
                DataTypes.Auction memory auction_,
                address to,
                uint256 artIn
            ) internal view returns (uint256 liquidatorCut, uint256 auctioneerCut) {}
    
    
      ```

Kudos for the quality of the code! It's pretty easy to explore

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter