Yield Witch v2 contest - cRat1st0s's results

Yield-Witch-v2-code4rena Report

• Yield-Witch-v2-code4rena Report
  • QA Report
  • Files Description Table
  • Issues found
    • [N-01]: Typos
      • Impact
      • Code Affected and Mitigation
      • Tools used

QA Report

Files Description Table

Issues found

[N-01]: Typos

Impact

None.

Code Affected and Mitigation

diff --git a/contracts/Witch.sol b/contracts/Witch.sol
index f98dd6a..92ca566 100644
--- a/contracts/Witch.sol
+++ b/contracts/Witch.sol
@@ -217,7 +217,7 @@ contract Witch is AccessControl {
         emit Auctioned(vaultId, uint32(block.timestamp));
     }
 
-    /// @dev Calculates the auction initial values, the 2 non-trivial values are how much art must be repayed
+    /// @dev Calculates the auction initial values, the 2 non-trivial values are how much art must be repaid
     /// and what's the max ink that will be offered in exchange. For the realtime amount of ink that's on offer
     /// use `_calcPayout`
     function _calcAuction(
@@ -264,7 +264,7 @@ contract Witch is AccessControl {
     }
 
     /// @dev Moves the vault ownership back to the original owner & clean internal state.
-    /// Useful as a method so it can be overriden by specialised witches that may need to do extra accounting or notify 3rd parties
+    /// Useful as a method so it can be overridden by specialised witches that may need to do extra accounting or notify 3rd parties
     function _auctionEnded(bytes12 vaultId, address owner) internal virtual {
         cauldron.give(vaultId, owner);
         delete auctions[vaultId];
@@ -382,7 +382,7 @@ contract Witch is AccessControl {
         _collateralBought(vaultId, to, liquidatorCut + auctioneerCut, artIn);
     }
 
-    /// @dev transfers funds from the ilkJoin to the liquidator (and potentially the auctioneer if they're differente people)
+    /// @dev transfers funds from the ilkJoin to the liquidator (and potentially the auctioneer if they're different people)
     function _payInk(
         DataTypes.Auction memory auction_,
         address to,
@@ -459,7 +459,7 @@ contract Witch is AccessControl {
     }
 
     /// @dev Logs that a certain amount of a vault was liquidated
-    /// Useful as a method so it can be overriden by specialised witches that may need to do extra accounting or notify 3rd parties
+    /// Useful as a method so it can be overridden by specialised witches that may need to do extra accounting or notify 3rd parties
     function _collateralBought(
         bytes12 vaultId,
         address buyer,
@@ -517,7 +517,7 @@ contract Witch is AccessControl {
 
 
     */
-    /// @dev quoutes hoy much ink a liquidator is expected to get if it repays an `artIn` amount
+    /// @dev Quotes how much ink a liquidator is expected to get if it repays an `artIn` amount
     /// Works for both Auctioned and ToBeAuctioned vaults
     /// @param vaultId The vault to get a quote for
     /// @param to Address that would get the collateral bought

Tools used

VS Code

Yield-Witch-v2-code4rena Report

Gas Optimizations

Files Description Table

Issues found

[G-01]: Use Custom Errors

Impact

Less expensive and able to use dynamic information in them.

Code Affected and Mitigation:

diff --git a/contracts/Witch.sol b/contracts/Witch.sol
index f98dd6a..cc66687 100644
--- a/contracts/Witch.sol
+++ b/contracts/Witch.sol
@@ -26,6 +26,18 @@ contract Witch is AccessControl {
     error VaultAlreadyUnderAuction(bytes12 vaultId, address witch);
     error VaultNotLiquidable(bytes12 vaultId, bytes6 ilkId, bytes6 baseId);
     error AuctioneerRewardTooHigh(uint128 max, uint128 actual);
+    error Unrecognized();
+    error InitialOfferAbove100Per();
+    error ProportionAbove100Per();
+    error InitialOfferBelow1Per();
+    error ProportionBelow1Per();
+    error NotUndercollateralized();
+    error CollateralLimitReached();
+    error VaultNotUnderAuction();
+    error Undercollateralized();
+    error NotEnoughBought();
+    error JoinNotFound();
+    error LeavesDust();
 
     event Auctioned(bytes12 indexed vaultId, uint256 indexed start);
     event Cancelled(bytes12 indexed vaultId);
@@ -81,7 +93,10 @@ contract Witch is AccessControl {
     /// @param param Name of parameter to set (must be "ladle")
     /// @param value Address of new ladle
     function point(bytes32 param, address value) external auth {
-        require(param == "ladle", "Unrecognized");
+        // require(param == "ladle", "Unrecognized");
+        if(param != "ladle"){
+            revert Unrecognized();
+        }
         ladle = ILadle(value);
         emit Point(param, value);
     }
@@ -99,13 +114,25 @@ contract Witch is AccessControl {
         uint64 proportion,
         uint64 initialOffer
     ) external auth {
-        require(initialOffer <= 1e18, "InitialOffer above 100%");
-        require(proportion <= 1e18, "Proportion above 100%");
-        require(
-            initialOffer == 0 || initialOffer >= 0.01e18,
-            "InitialOffer below 1%"
-        );
-        require(proportion >= 0.01e18, "Proportion below 1%");
+        // require(initialOffer <= 1e18, "InitialOffer above 100%");
+        if(initialOffer > 1e18) {
+            revert InitialOfferAbove100Per();
+        }
+        // require(proportion <= 1e18, "Proportion above 100%");
+        if(proportion > 1e18) {
+            revert ProportionAbove100Per();
+        }
+        // require(
+            // initialOffer == 0 || initialOffer >= 0.01e18,
+            // "InitialOffer below 1%"
+        // );
+        if(initialOffer != 0 && initialOffer < 0.01e18) {
+            revert InitialOfferBelow1Per();
+        }
+        // require(proportion >= 0.01e18, "Proportion below 1%");
+        if(proportion < 0.01e18) {
+            revert ProportionBelow1Per();
+        }
         lines[ilkId][baseId] = DataTypes.Line({
             duration: duration,
             proportion: proportion,
@@ -186,7 +213,10 @@ contract Witch is AccessControl {
             revert VaultNotLiquidable(vaultId, vault.ilkId, series.baseId);
         }
 
-        require(cauldron.level(vaultId) < 0, "Not undercollateralized");
+        // require(cauldron.level(vaultId) < 0, "Not undercollateralized");
+        if(cauldron.level(vaultId) >= 0){
+            revert NotUndercollateralized();
+        }
 
         DataTypes.Balances memory balances = cauldron.balances(vaultId);
         DataTypes.Debt memory debt = cauldron.debt(series.baseId, vault.ilkId);
@@ -197,7 +227,10 @@ contract Witch is AccessControl {
         DataTypes.Limits memory limits_ = limits[vault.ilkId][
             series.baseId
         ];
-        require(limits_.sum <= limits_.max, "Collateral limit reached");
+        // require(limits_.sum <= limits_.max, "Collateral limit reached");
+        if(limits_.sum > limits_.max) {
+            revert CollateralLimitReached();
+        }
 
         auction_ = _calcAuction(vault, series, to, balances, debt);
 
@@ -252,8 +285,14 @@ contract Witch is AccessControl {
     /// @param vaultId Id of vault to return
     function cancel(bytes12 vaultId) external {
         DataTypes.Auction storage auction_ = auctions[vaultId];
-        require(auction_.start > 0, "Vault not under auction");
-        require(cauldron.level(vaultId) >= 0, "Undercollateralized");
+        // require(auction_.start > 0, "Vault not under auction");
+        if(auction_.start <= 0){
+            revert VaultNotUnderAuction();
+        }
+        // require(cauldron.level(vaultId) >= 0, "Undercollateralized");
+        if(cauldron.level(vaultId) < 0) {
+            revert Undercollateralized();
+        }
 
         // Update concurrent collateral under auction
         limits[auction_.ilkId][auction_.baseId].sum -= auction_.ink;
@@ -297,7 +336,10 @@ contract Witch is AccessControl {
         )
     {
         DataTypes.Auction memory auction_ = auctions[vaultId];
-        require(auction_.start > 0, "Vault not under auction");
+        // require(auction_.start > 0, "Vault not under auction");
+        if(auction_.start <= 0){
+            revert VaultNotUnderAuction();
+        }
 
         // Find out how much debt is being repaid
         uint128 artIn = uint128(
@@ -310,7 +352,10 @@ contract Witch is AccessControl {
 
         // Calculate the collateral to be sold
         (liquidatorCut, auctioneerCut) = _calcPayout(auction_, to, artIn);
-        require(liquidatorCut >= minInkOut, "Not enough bought");
+        // require(liquidatorCut >= minInkOut, "Not enough bought");
+        if(liquidatorCut < minInkOut){
+            revert NotEnoughBought();
+        }
 
         // Update Cauldron and local auction data
         _updateAccounting(
@@ -325,7 +370,11 @@ contract Witch is AccessControl {
         if (baseIn != 0) {
             // Take underlying from liquidator
             IJoin baseJoin = ladle.joins(auction_.baseId);
-            require(baseJoin != IJoin(address(0)), "Join not found");
+            // require(baseJoin != IJoin(address(0)), "Join not found");
+            if (baseJoin == IJoin(address(0)))
+            {
+                revert JoinNotFound();
+            }
             baseJoin.join(msg.sender, baseIn.u128());
         }
 
@@ -355,14 +404,20 @@ contract Witch is AccessControl {
         )
     {
         DataTypes.Auction memory auction_ = auctions[vaultId];
-        require(auction_.start > 0, "Vault not under auction");
+        // require(auction_.start > 0, "Vault not under auction");
+        if(auction_.start <= 0){
+            revert VaultNotUnderAuction();
+        }
 
         // If offering too much fyToken, take only the necessary.
         artIn = maxArtIn > auction_.art ? auction_.art : maxArtIn;
 
         // Calculate the collateral to be sold
         (liquidatorCut, auctioneerCut) = _calcPayout(auction_, to, artIn);
-        require(liquidatorCut >= minInkOut, "Not enough bought");
+        // require(liquidatorCut >= minInkOut, "Not enough bought");
+        if(liquidatorCut < minInkOut){
+            revert NotEnoughBought();
+        }
 
         // Update Cauldron and local auction data
         _updateAccounting(
@@ -392,7 +447,11 @@ contract Witch is AccessControl {
         // If liquidatorCut is 0, then auctioneerCut is 0 too, so no need to double check
         if (liquidatorCut > 0) {
             IJoin ilkJoin = ladle.joins(auction_.ilkId);
-            require(ilkJoin != IJoin(address(0)), "Join not found");
+            // require(ilkJoin != IJoin(address(0)), "Join not found");
+            if (ilkJoin == IJoin(address(0)))
+            {
+                revert JoinNotFound();
+            }
 
             // Pay auctioneer's cut if necessary
             if (auctioneerCut > 0) {
@@ -413,7 +472,10 @@ contract Witch is AccessControl {
         uint256 artIn
     ) internal {
         // Duplicate check, but guarantees data integrity
-        require(auction_.start > 0, "Vault not under auction");
+        // require(auction_.start > 0, "Vault not under auction");
+        if(auction_.start <= 0){
+            revert VaultNotUnderAuction();
+        }
 
         // Update concurrent collateral under auction
         DataTypes.Limits memory limits_ = limits[auction_.ilkId][
@@ -434,10 +496,13 @@ contract Witch is AccessControl {
                     auction_.baseId,
                     auction_.ilkId
                 );
-                require(
-                    auction_.art - artIn >= debt.min * (10debt.dec),
-                    "Leaves dust"
-                );
+                // require(
+                    // auction_.art - artIn >= debt.min * (10debt.dec),
+                    // "Leaves dust"
+                // );
+                if(auction_.art - artIn >= debt.min * (10debt.dec)) {
+                    revert LeavesDust();
+                }
 
                 // Update the auction
                 auction_.ink -= inkOut.u128();

Proof of Concept

Before changes

After changes

Tools used

Foundry / VS Code