Petite Spruce Mammoth - Gas optimization in `AccountFacetImpl.sol`

[sherlock-admin4]

Petite Spruce Mammoth

Low/Info

Gas optimization in AccountFacetImpl.sol

Summary

While reviewing the AccountFacetImpl contract, several areas for gas optimization were identified. These optimizations aim to reduce redundant storage reads, minimize external calls, and improve general efficiency. General Gas Optimizations:

1. Accessing storage multiple times within the same function is expensive. Cache frequently accessed variables in memory when possible.
2. Performing decimal conversions multiple times in the same function increases gas costs. It is more efficient to calculate these once and reuse the result.
3. Combining multiple external calls into one can reduce gas, as each external call involves overhead.

4. safeTransfer and safeTransferFrom calls are gas-heavy. These should be minimized and optimized when possible. We can optimize withdraw function using these techniques. Original code: https://github.com/sherlock-audit/2024-09-symmio-v0-8-4-update-contest/blob/main/protocol-core/contracts/facets/Account/AccountFacetImpl.sol#L26-L36 Optimized code:

   function withdraw(address user, uint256 amount) internal {
   AccountStorage.Layout storage accountLayout = AccountStorage.layout();
   GlobalAppStorage.Layout storage appLayout = GlobalAppStorage.layout();
   
   // Cache withdraw cooldown and collateral decimals to avoid redundant storage reads
   uint256 userCooldown = accountLayout.withdrawCooldown[msg.sender];
   uint256 deallocateCooldown = MAStorage.layout().deallocateCooldown;
   
   require(
       block.timestamp >= userCooldown + deallocateCooldown,
       "AccountFacet: Cooldown hasn't reached"
   );
   
   // Cache the collateral decimals to avoid repeated external calls
   uint8 collateralDecimals = IERC20Metadata(appLayout.collateral).decimals();
   uint256 amountWith18Decimals = (amount * 1e18) / (10 ** collateralDecimals);
   
   // Perform subtraction once and cache result
   uint256 userBalance = accountLayout.balances[msg.sender];
   require(userBalance >= amountWith18Decimals, "AccountFacet: Insufficient balance");
   accountLayout.balances[msg.sender] = userBalance - amountWith18Decimals;
   
   // External transfer
   IERC20(appLayout.collateral).safeTransfer(user, amount);
   }

5. Instead of repeatedly accessing storage, we cache the withdrawCooldown and deallocateCooldown values, reducing gas consumption.
6. IERC20Metadata(appLayout.collateral).decimals() call is external and costly. By caching this once in the collateralDecimals variable, we avoid making this call multiple times.
7. Instead of subtracting from balances[msg.sender] directly each time, we cache the balance, perform the subtraction in memory, and update storage once. This saves gas by reducing the number of storage operations.
8. The ERC20 transfer is performed after all state changes are made. This is not only a security measure but also ensures that we only make the external call once, saving gas.

Root Cause

No response

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

Expected gas savings:

1. Minimizing redundant reads and writes from storage can save significant gas, especially when done frequently.
2. Each external call avoided saves roughly 700-2600 gas, depending on the network conditions and contract design.
3. Reducing the number of IERC20Metadata calls saves considerable gas by minimizing expensive external calls.

PoC

We can implement a test case to benchmark the gas usage of both the original and optimized withdraw function. Below is an example of how you can write a PoC to measure the gas savings.

Example test in hardhat:

const { expect } = require("chai");
const { ethers } = require("hardhat");

describe("Gas Optimization PoC", function () {
  let accountFacet, collateral, user;

  before(async function () {
    // Deploy mock contracts and set up
    const Collateral = await ethers.getContractFactory("MockERC20");
    collateral = await Collateral.deploy("Test Collateral", "TC", 18);
    await collateral.deployed();

    const AccountFacet = await ethers.getContractFactory("AccountFacetImpl");
    accountFacet = await AccountFacet.deploy();
    await accountFacet.deployed();

    [user] = await ethers.getSigners();

    // Deposit some initial amount for testing
    await collateral.mint(user.address, ethers.utils.parseUnits("1000", 18));
    await collateral.approve(accountFacet.address, ethers.utils.parseUnits("1000", 18));
    await accountFacet.deposit(user.address, ethers.utils.parseUnits("500", 18));
  });

  it("should benchmark gas usage for original vs optimized withdraw", async function () {
    // Measure gas usage for original withdraw
    const originalTx = await accountFacet.withdraw(user.address, ethers.utils.parseUnits("10", 18));
    const originalReceipt = await originalTx.wait();
    console.log("Original Withdraw Gas Usage:", originalReceipt.gasUsed.toString());

    // Measure gas usage for optimized withdraw
    const optimizedTx = await accountFacet.optimizedWithdraw(user.address, ethers.utils.parseUnits("10", 18));
    const optimizedReceipt = await optimizedTx.wait();
    console.log("Optimized Withdraw Gas Usage:", optimizedReceipt.gasUsed.toString());

    expect(optimizedReceipt.gasUsed).to.be.below(originalReceipt.gasUsed);
  });
});

Mitigation

No response