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
Rank: 43/63
Findings: 1
Award: $33.33
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: IllIllI
Also found by: 0x1f8b, 0x29A, 0xKitsune, 0xNazgul, Aymen0909, Chom, Deivitto, ElKu, JC, JohnSmith, Kaiziron, Limbooo, MadWookie, Meera, ReyAdmirado, Rohan16, Sm4rty, SooYa, TomJ, Trumpero, Waze, __141345__, ajtra, ak1, antonttc, bulej93, c3phas, cRat1st0s, csanuragjain, defsec, durianSausage, fatherOfBlocks, gogo, hake, hickuphh3, ignacio, joestakey, karanctf, kyteg, m_Rassska, pashov, rajatbeladiya, rbserver, robee, rokinot, samruna, sashik_eth, simon135, tofunmi
33.3324 USDC - $33.33
a = a + b
instead of a += b
MLOAD
<< SLOAD
require()
before some computations, if it makes senseInternal
functions can be inlinedprivate/internal
for constants/immutable
variables instead of public
type(uint256).max
uint
instead of uint8
, uint64
, if it's possiblecalldataload
instead of mload
a = a + b
instead of a += b
<a name="G-01"></a>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;
MLOAD
<< SLOAD
<a name="G-02"></a>MLOAD
costs only 3 units of gas, SLOAD
(warm access) is about 100 units. Therefore, cache, when it's possible.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); ```
require() before some computations, if it makes sense
<a name="G-03"></a>require()
takes some gas for execution, therefore if the statement reverts gas will not be retrieved.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" );
Internal
functions can be inlined<a name="G-04"></a>JUMP
s which costs around 40-50 gas uints. In loops it will save significant amount of gas.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); }
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) {}
private/internal
for constants/immutable
variables instead of public
<a name="G-06"></a>public
instance.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;
Contracts:
file: src/Witch.sol ............................... // Lines: [255-255; 300-300; 358-358; 416-416] require(auction_.start > 0, "Vault not under auction");
type(uint256).max
<a name="G-08"></a>Contracts:
file: src/**/Witch.sol ............................... // Lines: [584-584] if (duration == type(uint32).max) {} // Interpreted as infinite duration
uint
instead of uint8
, uint64
, if it's possible<a name="G-09"></a>error VaultAlreadyUnderAuction(bytes12 vaultId, address witch);
.error VaultAlreadyUnderAuction(bytes32 vaultId, address witch);
.bytes32
doesn't give a huge optimization.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 ) {} ```
calldataload
instead of mload
<a name="G-10"></a>calldataload
, or replace memory
with calldata
. If the args are pretty huge, allocating args in memory will cost a lot.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) {} ```