code-423n4 / 2022-06-canto-findings

0 stars 0 forks source link

Gas Optimizations #286

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Avoid initialising variables with default values

When variables are created it contains default values, explicitly initialising with default values (0, address(0), false, ) is not needed

Proof of concept

https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Reservoir.sol#L38

    dripped = 0;

https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-core.sol#L46

    uint public totalSupply = 0;

https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-core.sol#L490

    isPaused = false;

Variable can be converted to immutable

Variable that are initialised during construction and not updated later can be converted to immutable to save gas

Proof of concept

https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Reservoir.sol#L34-37

    dripStart = block.number;
    dripRate = dripRate_;
    token = token_;
    target = target_;

https://github.com/Plex-Engineer/lending-market/blob/755424c1f9ab3f9f0408443e6606f94e4f08a990/contracts/NoteInterest.sol#L22

    address private admin;

Unnecessary variable

Values used once can be directly added to the expression instead of storing in a temporary variable

Proof of concept

token, dripRate_, dripStart_, blockNumber_, target_, drippedNext_ in

    EIP20Interface token_ = token;
    uint reservoirBalance_ = token_.balanceOf(address(this)); // TODO: Verify this is a static call
    uint dripRate_ = dripRate;
    uint dripStart_ = dripStart;
    uint dripped_ = dripped;
    address target_ = target;
    uint blockNumber_ = block.number;

    // Next, calculate intermediate values
    uint dripTotal_ = mul(dripRate_, blockNumber_ - dripStart_, "dripTotal overflow");
    uint deltaDrip_ = sub(dripTotal_, dripped_, "deltaDrip underflow");
    uint toDrip_ = min(reservoirBalance_, deltaDrip_);
    uint drippedNext_ = add(dripped_, toDrip_, "tautological");

    // Finally, write new `dripped` value and transfer tokens to target
    dripped = drippedNext_;
    token_.transfer(target_, toDrip_);

name and symbol in

https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-core.sol#L39

    string public name;
    string public symbol;

Redundant require statement

Since GovernorBravoDelegate.sol uses solidity version 0.8, when addition overflows the execution reverts and require statement is not executed

Proof of concept

https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Governance/GovernorBravoDelegate.sol#L182

    uint c = a + b;
    require(c >= a, "addition overflow");

Mitigation

unchecked can be added to the addition statement

    unchecked { c = a + b; }

Redundant check msg.sender != address(0)

In lender market GovernorBravoDelegate.sol msg.sender is checked for address(0)

Proof of concept

https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Governance/GovernorBravoDelegate.sol#L164

    require(msg.sender == pendingAdmin && msg.sender != address(0), "GovernorBravo:_acceptAdmin: pending admin only");

Mitigation

address(0) check can be removed

> 0 can be replaced with != 0

!= 0 costs less gas compared to > 0 for unsigned integers in require statements with optimizer enabled

Proof of concept

https://github.com/Plex-Engineer/zeroswap/blob/03507a80322112f4f3c723fc68bed0f138702836/contracts/uniswapv2/UniswapV2Pair.sol#L129

https://github.com/Plex-Engineer/zeroswap/blob/03507a80322112f4f3c723fc68bed0f138702836/contracts/uniswapv2/UniswapV2Pair.sol#L138

https://github.com/Plex-Engineer/zeroswap/blob/03507a80322112f4f3c723fc68bed0f138702836/contracts/uniswapv2/UniswapV2Pair.sol#L159

https://github.com/Plex-Engineer/zeroswap/blob/03507a80322112f4f3c723fc68bed0f138702836/contracts/uniswapv2/UniswapV2Pair.sol#L191

https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-core.sol#L253

https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-core.sol#L272

https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-core.sol#L286

https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-core.sol#L303

https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-periphery.sol#L104

    require(amountA > 0, 'BaseV1Router: INSUFFICIENT_AMOUNT');

https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-periphery.sol#L105

    require(reserveA > 0 && reserveB > 0, 'BaseV1Router: INSUFFICIENT_LIQUIDITY');

https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-periphery.sol#L456 https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-periphery.sol#L463

Reduce size of revert strings

Revert string greater than 32 bytes will increase deployment gas as well as runtime gas when the condition is met

Proof of concept

https://github.com/Plex-Engineer/zeroswap/blob/03507a80322112f4f3c723fc68bed0f138702836/contracts/uniswapv2/UniswapV2Pair.sol#L138

L138 : require(liquidity > 0, 'UniswapV2: INSUFFICIENT_LIQUIDITY_MINTED');
L159 :        require(amount0 > 0 && amount1 > 0, 'UniswapV2: INSUFFICIENT_LIQUIDITY_BURNED');
L173        require(amount0Out > 0 || amount1Out > 0, 'UniswapV2: INSUFFICIENT_OUTPUT_AMOUNT');
L175:        require(amount0Out < _reserve0 && amount1Out < _reserve1, 'UniswapV2: INSUFFICIENT_LIQUIDITY');
L191:        require(amount0In > 0 || amount1In > 0, 'UniswapV2: INSUFFICIENT_INPUT_AMOUNT');

https://github.com/Plex-Engineer/zeroswap/blob/03507a80322112f4f3c723fc68bed0f138702836/contracts/uniswapv2/UniswapV2Router02.sol#L52

L52 :                require(amountBOptimal >= amountBMin, 'UniswapV2Router: INSUFFICIENT_B_AMOUNT');
L57 :                require(amountAOptimal >= amountAMin, 'UniswapV2Router: INSUFFICIENT_A_AMOUNT');
L118 :        require(amountA >= amountAMin, 'UniswapV2Router: INSUFFICIENT_A_AMOUNT');
L119 :        require(amountB >= amountBMin, 'UniswapV2Router: INSUFFICIENT_B_AMOUNT');
L233 :        require(amounts[amounts.length - 1] >= amountOutMin, 'UniswapV2Router: INSUFFICIENT_OUTPUT_AMOUNT');
L247 :        require(amounts[0] <= amountInMax, 'UniswapV2Router: EXCESSIVE_INPUT_AMOUNT');
L263 :        require(amounts[amounts.length - 1] >= amountOutMin, 'UniswapV2Router: INSUFFICIENT_OUTPUT_AMOUNT');
L277 :        require(amounts[0] <= amountInMax, 'UniswapV2Router: EXCESSIVE_INPUT_AMOUNT');
L294 :        require(amounts[amounts.length - 1] >= amountOutMin, 'UniswapV2Router: INSUFFICIENT_OUTPUT_AMOUNT');
L312 :        require(amounts[0] <= msg.value, 'UniswapV2Router: EXCESSIVE_INPUT_AMOUNT');
L354 :        require(
            IERC20Uniswap(path[path.length - 1]).balanceOf(to).sub(balanceBefore) >= amountOutMin,
            'UniswapV2Router: INSUFFICIENT_OUTPUT_AMOUNT'
        );
L377 :                require(
            IERC20Uniswap(path[path.length - 1]).balanceOf(to).sub(balanceBefore) >= amountOutMin,
            'UniswapV2Router: INSUFFICIENT_OUTPUT_AMOUNT'
        );
L398 :              require(amountOut >= amountOutMin, 'UniswapV2Router: INSUFFICIENT_OUTPUT_AMOUNT');

https://github.com/Plex-Engineer/zeroswap/blob/03507a80322112f4f3c723fc68bed0f138702836/contracts/uniswapv2/UniswapV2Oracle.sol#L46

L46 :    require(granularity_ > 1, 'SlidingWindowOracle: GRANULARITY');

L49 :        require(
            (periodSize = windowSize_ / granularity_) * granularity_ == windowSize_,
            'SlidingWindowOracle: WINDOW_NOT_EVENLY_DIVISIBLE'
        );
L133 :        require(timeElapsed <= windowSize, 'SlidingWindowOracle: MISSING_HISTORICAL_OBSERVATION');

L135 :        require(timeElapsed >= windowSize - periodSize * 2, 'SlidingWindowOracle: UNEXPECTED_TIME_ELAPSED');

https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-periphery.sol#L86

L86 :    require(tokenA != tokenB, 'BaseV1Router: IDENTICAL_ADDRESSES');
L104 :    require(amountA > 0, 'BaseV1Router: INSUFFICIENT_AMOUNT');
L105 :    require(reserveA > 0 && reserveB > 0, 'BaseV1Router: INSUFFICIENT_LIQUIDITY');
L295 :    require(amountA >= amountAMin, 'BaseV1Router: INSUFFICIENT_A_AMOUNT');
L296 :    require(amountB >= amountBMin, 'BaseV1Router: INSUFFICIENT_B_AMOUNT');
L387 :    require(amounts[amounts.length - 1] >= amountOutMin, 'BaseV1Router: INSUFFICIENT_OUTPUT_AMOUNT');
L402 :        require(amounts[amounts.length - 1] >= amountOutMin, 'BaseV1Router: INSUFFICIENT_OUTPUT_AMOUNT');
L417 :        require(amounts[amounts.length - 1] >= amountOutMin, 'BaseV1Router: INSUFFICIENT_OUTPUT_AMOUNT');
L430 :        require(amounts[amounts.length - 1] >= amountOutMin, 'BaseV1Router: INSUFFICIENT_OUTPUT_AMOUNT');
L452 :        require(success, 'TransferHelper: ETH_TRANSFER_FAILED');

https://github.com/Plex-Engineer/lending-market/blob/755424c1f9ab3f9f0408443e6606f94e4f08a990/contracts/Governance/GovernorBravoDelegate.sol#L87

L87 :        require(state(proposalId) == ProposalState.Queued, "GovernorBravo::execute: proposal can only be executed if it is queued");
L78 :    require(!timelock.queuedTransactions(keccak256(abi.encode(target, value, signature, data, eta))), "GovernorBravo::queueOrRevertInternal: identical proposal action already queued at eta");

L25 :    require(address(timelock) == address(0), "GovernorBravo::initialize: can only initialize once");
L26 :    require(msg.sender == admin, "GovernorBravo::initialize: admin only");
L27 :    require(timelock_ != address(0), "GovernorBravo::initialize: invalid timelock address");

https://github.com/Plex-Engineer/lending-market/blob/755424c1f9ab3f9f0408443e6606f94e4f08a990/contracts/Governance/GovernorBravoDelegate.sol#L45

    require(unigovProposal.targets.length == unigovProposal.values.length && 
            unigovProposal.targets.length == unigovProposal.signatures.length && 
            unigovProposal.targets.length == unigovProposal.calldatas.length, 
            "GovernorBravo::propose: proposal function information arity mismatch");
    require(unigovProposal.targets.length != 0, "GovernorBravo::propose: must provide actions");
    require(unigovProposal.targets.length <= proposalMaxOperations, "GovernorBravo::propose: too many actions");

https://github.com/Plex-Engineer/lending-market/blob/755424c1f9ab3f9f0408443e6606f94e4f08a990/contracts/Accountant/AccountantDelegate.sol#L60

L17 :  require(msg.sender == admin, "AccountantDelegate::initialize: only admin can call this function");
L18 :   require(noteAddress_ != address(0), "AccountantDelegate::initialize: note Address invalid");
L29 : require(note.balanceOf(msg.sender) == note._initialSupply(), "AccountantDelegate::initiatlize: Accountant has not received payment");
L48 : require(msg.sender == address(cnote), "AccountantDelegate::supplyMarket: Only the CNote contract can supply market");
L60 : require(msg.sender == address(cnote), "AccountantDelegate::redeemMarket: Only the CNote contract can redeem market");
L83 : require(cNoteConverted >= noteDifferential, "Note Loaned to LendingMarket must increase in value");

++i is more efficient than i++

pre-increment is more efficient than post-increment for unsigned integers, and unchecked can be added to avoid the default underflow/overflow protections introduced in 0.8 to save more gas

https://github.com/Plex-Engineer/zeroswap/blob/03507a80322112f4f3c723fc68bed0f138702836/contracts/uniswapv2/UniswapV2Router02.sol#L214

    for (uint i; i < path.length - 1; i++) {

https://github.com/Plex-Engineer/zeroswap/blob/03507a80322112f4f3c723fc68bed0f138702836/contracts/uniswapv2/UniswapV2Router02.sol#L323

    for (uint i; i < path.length - 1; i++) {

https://github.com/Plex-Engineer/zeroswap/blob/03507a80322112f4f3c723fc68bed0f138702836/contracts/uniswapv2/UniswapV2Oracle.sol#L83

    for (uint i = pairObservations[pair].length; i < granularity; i++) {

https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-core.sol#L207

    for (uint i = 0; i < _prices.length; i++) {

Cache array length in loops

Array length can be cached and used in the for-loops instead of querying in every iteration

Proof of concept

https://github.com/Plex-Engineer/zeroswap/blob/03507a80322112f4f3c723fc68bed0f138702836/contracts/uniswapv2/UniswapV2Router02.sol#L214

    for (uint i; i < path.length - 1; i++) {

https://github.com/Plex-Engineer/zeroswap/blob/03507a80322112f4f3c723fc68bed0f138702836/contracts/uniswapv2/UniswapV2Router02.sol#L323

    for (uint i; i < path.length - 1; i++) {

https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-core.sol#L207

    for (uint i = 0; i < _prices.length; i++) {

https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-periphery.sol#L136

    for (uint i = 0; i < routes.length; i++) {

https://github.com/Plex-Engineer/lending-market/blob/755424c1f9ab3f9f0408443e6606f94e4f08a990/contracts/Governance/GovernorBravoDelegate.sol#L90

    for (uint i = 0; i < proposal.targets.length; i++) {

https://github.com/Plex-Engineer/lending-market/blob/755424c1f9ab3f9f0408443e6606f94e4f08a990/contracts/Comptroller.sol#L1106

    for (uint i = 0; i < affectedUsers.length; ++i) {

https://github.com/Plex-Engineer/lending-market/blob/755424c1f9ab3f9f0408443e6606f94e4f08a990/contracts/Comptroller.sol#L959

    for (uint i = 0; i < allMarkets.length; i ++) {
GalloDaSballo commented 2 years ago

10.5k for immutables

Rest is less than 1k