Astaria contest - SadBase's results

[G-01] Unused variable can reduce gas usage

Function deposit()

File:PublicVault.sol

Line 251:

function deposit(uint256 amount, address receiver)
    public
    override(ERC4626Cloned)
    whenNotPaused
    returns (uint256)
  {
    VIData storage s = _loadVISlot();
    if (s.allowListEnabled) {
      require(s.allowList[receiver]);
    }

    uint256 assets = totalAssets();

    return super.deposit(amount, receiver);
}

The variable assets are not used and it does not mutate state variables.

Running report on the test cases show a reduction in gas usage

export FORK_URL="https://rpc.ankr.com/eth" && forge snapshot --ffi --fork-url $FORK_URL --fork-block-number 15934974 --diff .gas-snapshot

testBasicPrivateVaultLoan() (gas: 0 (0.000%)) 
testCollateralTokenFileSetup() (gas: 0 (0.000%)) 
testFeesExample() (gas: 0 (0.000%)) 
testIncrementNonceAsStrategistAndDelegate() (gas: 0 (0.000%)) 
testLienTokenFileSetup() (gas: 0 (0.000%)) 
testVaultShutdown() (gas: 0 (0.000%)) 
testClaimFeesAgainstV3Liquidity() (gas: 0 (0.000%)) 
testCannotExceedMinMaxPublicVaultEpochLength() (gas: 0 (0.000%)) 
testCannotRandomAccountIncrementNonce() (gas: 0 (0.000%)) 
testFailInvalidSignature() (gas: 0 (0.000%)) 
testFailSoloLendNotAppraiser() (gas: 0 (0.000%)) 
testInvalidFileData() (gas: 0 (0.000%)) 
testBlockingLiquidationsProcessEpoch() (gas: -609 (-0.010%)) 
testLiquidationNftTransfer() (gas: -608 (-0.014%)) 
testLiquidationPaymentsOverbid() (gas: -609 (-0.015%)) 
testLiquidationNearBoundaryNoWithdraws() (gas: -609 (-0.015%)) 
testLiquidationBoundaryEpochOrdering() (gas: -1218 (-0.017%)) 
testAuctionEnd() (gas: -761 (-0.019%)) 
testBuyoutLienDifferentCollateral() (gas: -761 (-0.022%)) 
testBuyoutLien() (gas: -761 (-0.024%)) 
testLiquidation5050Split() (gas: -1522 (-0.027%)) 
testFailPayLienAfterLiquidate() (gas: -761 (-0.027%)) 
testTwoLoansDiffCollateralSameStack() (gas: -761 (-0.027%)) 
testWithdrawLiquidatedNoBids() (gas: -761 (-0.027%)) 
testFinalAuctionEnd() (gas: -761 (-0.028%)) 
testAuctionEndNoBids() (gas: -761 (-0.029%)) 
testCannotBorrowMoreThanMaxPotentialDebt() (gas: -761 (-0.029%)) 
testCannotCommitToLienPotentialDebtExceedsLiquidationInitialAsk() (gas: -761 (-0.030%)) 
testPublicVaultSlopeIncDecIntegration() (gas: -761 (-0.032%)) 
testBasicPublicVaultLoan() (gas: -761 (-0.033%)) 
testMakeTwoPayments() (gas: -761 (-0.033%)) 
testCannotProcessEpochWithUnliquidatedLien() (gas: -761 (-0.034%)) 
testNewLienExceeds2XEpoch() (gas: -761 (-0.034%)) 
testFailLienDurationZero() (gas: -761 (-0.034%)) 
testFutureLiquidationWithBlockingWithdrawReserve() (gas: -1827 (-0.035%)) 
testReleaseToAddress() (gas: -761 (-0.035%)) 
testCannotLiquidationInitialAsk0() (gas: -761 (-0.037%)) 
testCannotLiquidationInitialAskExceedsAmountBorrowed() (gas: -761 (-0.037%)) 
testCannotBorrowMoreThanMaxAmount() (gas: -761 (-0.038%)) 
testCannotLienRateZero() (gas: -761 (-0.038%)) 
testZeroizedVaultNewLP() (gas: -1522 (-0.041%)) 
testLiquidationAtBoundary() (gas: -3805 (-0.044%)) 
testWithdrawProxy() (gas: -761 (-0.046%)) 
testMultipleVaultsWithLiensOnTheSameCollateral() (gas: -3805 (-0.047%)) 
testEpochProcessionMultipleActors() (gas: -3044 (-0.087%)) 
Overall gas change: -35920 (-1.042%)

Function transferWithdrawReserve()

File:PublicVault.sol
Line 397

function transferWithdrawReserve() public {
    VaultData storage s = _loadStorageSlot();

    if (s.currentEpoch == uint64(0)) {
      return;
    }

    address currentWithdrawProxy = s
      .epochData[s.currentEpoch - 1]
      .withdrawProxy;
    // prevents transfer to a non-existent WithdrawProxy
    // withdrawProxies are indexed by the epoch where they're deployed
    if (currentWithdrawProxy != address(0)) {
      uint256 withdrawBalance = ERC20(asset()).balanceOf(address(this));

      // prevent transfer of more assets then are available
      if (s.withdrawReserve <= withdrawBalance) {
        withdrawBalance = s.withdrawReserve;
        s.withdrawReserve = 0;
      } else {
        unchecked {
          s.withdrawReserve -= withdrawBalance.safeCastTo88();
        }
      }

      ERC20(asset()).safeTransfer(currentWithdrawProxy, withdrawBalance);
      WithdrawProxy(currentWithdrawProxy).increaseWithdrawReserveReceived(
        withdrawBalance
      );
      emit WithdrawReserveTransferred(withdrawBalance);
    }

    address withdrawProxy = s.epochData[s.currentEpoch].withdrawProxy;
    if (
      s.withdrawReserve > 0 &&
      timeToEpochEnd() == 0 &&
      withdrawProxy != address(0)
    ) {
      // address currentWithdrawProxy = s
      //   .epochData[s.currentEpoch - 1]
      //   .withdrawProxy;
      uint256 drainBalance = WithdrawProxy(withdrawProxy).drain(
        s.withdrawReserve,
        s.epochData[s.currentEpoch - 1].withdrawProxy
      );
      unchecked {
        s.withdrawReserve -= drainBalance.safeCastTo88();
      }
      WithdrawProxy(currentWithdrawProxy).increaseWithdrawReserveReceived(
        drainBalance
      );
    }
}

variable currentWithdrawProxy is re-declared even though there are no changes to the address.

Removing the re-declaration of the variable reduces gas usage.

[G-02] Caching of state variables can reduce gas usage

File: PublicVault.sol

function processEpoch() public {
    // check to make sure epoch is over
    if (timeToEpochEnd() > 0) {
      revert InvalidState(InvalidStates.EPOCH_NOT_OVER);
    }
    VaultData storage s = _loadStorageSlot();

    if (s.withdrawReserve > 0) {
      revert InvalidState(InvalidStates.WITHDRAW_RESERVE_NOT_ZERO);
    }

    WithdrawProxy currentWithdrawProxy = WithdrawProxy(
      s.epochData[s.currentEpoch].withdrawProxy
    );

    // split funds from previous WithdrawProxy with PublicVault if hasn't been already
    if (s.currentEpoch != 0) {
      WithdrawProxy previousWithdrawProxy = WithdrawProxy(
        s.epochData[s.currentEpoch - 1].withdrawProxy
      );
      if (
        address(previousWithdrawProxy) != address(0) &&
        previousWithdrawProxy.getFinalAuctionEnd() != 0
      ) {
        previousWithdrawProxy.claim();
      }
    }

    if (s.epochData[s.currentEpoch].liensOpenForEpoch > 0) {
      revert InvalidState(InvalidStates.LIENS_OPEN_FOR_EPOCH_NOT_ZERO);
    }

    // reset liquidationWithdrawRatio to prepare for re calcualtion
    s.liquidationWithdrawRatio = 0;

    // check if there are LPs withdrawing this epoch
    if ((address(currentWithdrawProxy) != address(0))) {
      uint256 proxySupply = currentWithdrawProxy.totalSupply();

      s.liquidationWithdrawRatio = proxySupply
        .mulDivDown(1e18, totalSupply())
        .safeCastTo88();

      currentWithdrawProxy.setWithdrawRatio(s.liquidationWithdrawRatio);
      uint256 expected = currentWithdrawProxy.getExpected();

      unchecked {
        if (totalAssets() > expected) {
          s.withdrawReserve = (totalAssets() - expected)
            .mulWadDown(s.liquidationWithdrawRatio)
            .safeCastTo88();
        } else {
          s.withdrawReserve = 0;
        }
      }
      _setYIntercept(
        s,
        s.yIntercept -
          totalAssets().mulDivDown(s.liquidationWithdrawRatio, 1e18)
      );
      // burn the tokens of the LPs withdrawing
      _burn(address(this), proxySupply);
    }

    // increment epoch
    unchecked {
      s.currentEpoch++;
    }
  }

totalAssets() can be cached and memoized to reduce gas usage

uint256 assets = totalAssets();

unchecked {
	if (assets > expected) {
	  s.withdrawReserve = (assets - expected)
		.mulWadDown(s.liquidationWithdrawRatio)
		.safeCastTo88();
	} else {
	  s.withdrawReserve = 0;
	}
  }
  _setYIntercept(
	s,
	s.yIntercept -
	  assets.mulDivDown(s.liquidationWithdrawRatio, 1e18)
  );
  // burn the tokens of the LPs withdrawing
  _burn(address(this), proxySupply);
}

Running export FORK_URL="https://rpc.ankr.com/eth" && forge snapshot --ffi --fork-url $FORK_URL --fork-block-number 15934974 --diff .gas-snapshot:

testAuctionEnd() (gas: 0 (0.000%)) 
testAuctionEndNoBids() (gas: 0 (0.000%)) 
testBasicPrivateVaultLoan() (gas: 0 (0.000%)) 
testBasicPublicVaultLoan() (gas: 0 (0.000%)) 
testBuyoutLienDifferentCollateral() (gas: 0 (0.000%)) 
testCollateralTokenFileSetup() (gas: 0 (0.000%)) 
testFeesExample() (gas: 0 (0.000%)) 
testFinalAuctionEnd() (gas: 0 (0.000%)) 
testIncrementNonceAsStrategistAndDelegate() (gas: 0 (0.000%)) 
testLienTokenFileSetup() (gas: 0 (0.000%)) 
testLiquidationNftTransfer() (gas: 0 (0.000%)) 
testMakeTwoPayments() (gas: 0 (0.000%)) 
testNewLienExceeds2XEpoch() (gas: 0 (0.000%)) 
testReleaseToAddress() (gas: 0 (0.000%)) 
testTwoLoansDiffCollateralSameStack() (gas: 0 (0.000%)) 
testVaultShutdown() (gas: 0 (0.000%)) 
testClaimFeesAgainstV3Liquidity() (gas: 0 (0.000%)) 
testPublicVaultSlopeIncDecIntegration() (gas: 0 (0.000%)) 
testCannotBorrowMoreThanMaxAmount() (gas: 0 (0.000%)) 
testCannotBorrowMoreThanMaxPotentialDebt() (gas: 0 (0.000%)) 
testCannotCommitToLienPotentialDebtExceedsLiquidationInitialAsk() (gas: 0 (0.000%)) 
testCannotExceedMinMaxPublicVaultEpochLength() (gas: 0 (0.000%)) 
testCannotLienRateZero() (gas: 0 (0.000%)) 
testCannotLiquidationInitialAsk0() (gas: 0 (0.000%)) 
testCannotLiquidationInitialAskExceedsAmountBorrowed() (gas: 0 (0.000%)) 
testCannotProcessEpochWithUnliquidatedLien() (gas: 0 (0.000%)) 
testCannotRandomAccountIncrementNonce() (gas: 0 (0.000%)) 
testFailInvalidSignature() (gas: 0 (0.000%)) 
testFailLienDurationZero() (gas: 0 (0.000%)) 
testFailPayLienAfterLiquidate() (gas: 0 (0.000%)) 
testFailSoloLendNotAppraiser() (gas: 0 (0.000%)) 
testInvalidFileData() (gas: 0 (0.000%)) 
testLiquidationPaymentsOverbid() (gas: -590 (-0.014%)) 
testLiquidationAtBoundary() (gas: -1486 (-0.017%)) 
testBlockingLiquidationsProcessEpoch() (gas: -1189 (-0.020%)) 
testLiquidationNearBoundaryNoWithdraws() (gas: -1189 (-0.029%)) 
testLiquidationBoundaryEpochOrdering() (gas: -2378 (-0.033%)) 
testFutureLiquidationWithBlockingWithdrawReserve() (gas: -1779 (-0.034%)) 
testEpochProcessionMultipleActors() (gas: -1486 (-0.043%)) 
testBuyoutLien() (gas: -1486 (-0.047%)) 
testLiquidation5050Split() (gas: -2972 (-0.052%)) 
testWithdrawLiquidatedNoBids() (gas: -1486 (-0.053%)) 
testMultipleVaultsWithLiensOnTheSameCollateral() (gas: -5944 (-0.073%)) 
testZeroizedVaultNewLP() (gas: -2972 (-0.080%)) 
testWithdrawProxy() (gas: -1486 (-0.089%)) 
Overall gas change: -26443 (-0.585%)

[G-03] Redundant Conditional Checks

File: CollateralToken.sol

function releaseToAddress(uint256 collateralId, address releaseTo)
    public
    releaseCheck(collateralId)
    onlyOwner(collateralId)
{
    CollateralStorage storage s = _loadCollateralSlot();

    if (msg.sender != ownerOf(collateralId)) {
      revert InvalidSender();
    }
    Asset memory underlying = s.idToUnderlying[collateralId];
    address tokenContract = underlying.tokenContract;
    _burn(collateralId);
    delete s.idToUnderlying[collateralId];
    _releaseToAddress(s, underlying, collateralId, releaseTo);
}

The statement of checking msg.sender is redundant with the onlyOwner modifier as they are essentially checking for the same condition.

Removing the conditional statement does not affect the checks but it will reduce gas usage.

testReleaseToAddress() (gas: -429 (-0.020%))