code-423n4 / 2022-03-maple-findings

0 stars 0 forks source link

Gas Optimizations #20

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Gas Optimizations

An array's length should be cached to save gas in for-loops

Reading array length at each iteration of the loop takes 6 gas (3 for mload and 3 to place memory_offset) in the stack.

Caching the array length in the stack saves around 3 gas per iteration.

Findings

MapleLoanInternals.sol#L258

Recommended mitigation steps

uint length = calls_.length;

for (uint256 i; i < length; ++i) {
    ( bool success, ) = refinancer_.delegatecall(calls_[i]);
    require(success, "MLI:ANT:FAILED");
}

Use unchecked {} primitive within for loops

Given the use of Solidity compiler >= 0.8.0, there are default arithmetic checks for mathematical operations which consume additional gas for such checks internally. In expressions where we are absolutely sure of no overflows/underflows, one can use the unchecked primitive to wrap such expressions to avoid checks and save gas.

Findings

MapleLoanInternals.sol#L258

Recommended mitigation steps

Change to

for (uint256 i; i < calls_.length;) {
    ( bool success, ) = refinancer_.delegatecall(calls_[i]);
    require(success, "MLI:ANT:FAILED");

    unchecked { ++i; }
}

Use assignment = instead of +=

Named return variable lateInterest_ is unused before line MapleLoanInternals.sol#L589, therefore it's not necessary to use +=.

Findings

MapleLoanInternals.sol#L589

Recommended mitigation steps

Change to assignment (=) to save gas.

lateInterest_ = _getInterest(principal_, interestRate_ + lateInterestPremium_, fullDaysLate);
lateInterest_ += (lateFeeRate_ * principal_) / SCALED_ONE;

Unused named returns can be removed

Description

Removing unused named return variables can reduce gas usage and improve code clarity.

Recommended mitigation steps

Remove the unused named return variables or use them instead of creating additional variables.

Findings

RevenueDistributionToken.sol

#L193 unnecessary named return apr_

#L196 unnecessary named return balanceOfAssets_

#L258 unnecessary named return totalManagedAssets_

#L284 unnecessary named return result_

MapleLoan.sol

#L244 unnecessary named return collateral_

#L274-400

MapleLoanInternals.sol

#L204 unnecessary named return proposedRefinanceCommitment_

#L397 unnecessary named return isMaintained_

#L434 unnecessary named return unaccountedAmount_

#L442 unnecessary named return mapleGlobals_

#L456 unnecessary named return collateral_

#L495 unnecessary named return interest_

#L594 unnecessary named return periodicInterestRate_

#L599 unnecessary named return refinanceCommitment_

lucas-manuel commented 2 years ago

Gas Optimizations

An array's length should be cached to save gas in for-loops

Reading array length at each iteration of the loop takes 6 gas (3 for mload and 3 to place memory_offset) in the stack.

Caching the array length in the stack saves around 3 gas per iteration.

Findings

MapleLoanInternals.sol#L258

Recommended mitigation steps

uint length = calls_.length;

for (uint256 i; i < length; ++i) {
    ( bool success, ) = refinancer_.delegatecall(calls_[i]);
    require(success, "MLI:ANT:FAILED");
}

Use unchecked {} primitive within for loops

Given the use of Solidity compiler >= 0.8.0, there are default arithmetic checks for mathematical operations which consume additional gas for such checks internally. In expressions where we are absolutely sure of no overflows/underflows, one can use the unchecked primitive to wrap such expressions to avoid checks and save gas.

Findings

MapleLoanInternals.sol#L258

Recommended mitigation steps

Change to

for (uint256 i; i < calls_.length;) {
    ( bool success, ) = refinancer_.delegatecall(calls_[i]);
    require(success, "MLI:ANT:FAILED");

    unchecked { ++i; }
}

Duplicates

lucas-manuel commented 2 years ago

Use assignment = instead of +=

Named return variable lateInterest_ is unused before line MapleLoanInternals.sol#L589, therefore it's not necessary to use +=.

Findings

MapleLoanInternals.sol#L589

Recommended mitigation steps

Change to assignment (=) to save gas.

lateInterest_ = _getInterest(principal_, interestRate_ + lateInterestPremium_, fullDaysLate);
lateInterest_ += (lateFeeRate_ * principal_) / SCALED_ONE;

Won't implement, less clean

lucas-manuel commented 2 years ago

Findings

RevenueDistributionToken.sol

#L193 unnecessary named return apr_

#L196 unnecessary named return balanceOfAssets_

#L258 unnecessary named return totalManagedAssets_

#L284 unnecessary named return result_

MapleLoan.sol

#L244 unnecessary named return collateral_

#L274-400

MapleLoanInternals.sol

#L204 unnecessary named return proposedRefinanceCommitment_

#L397 unnecessary named return isMaintained_

#L434 unnecessary named return unaccountedAmount_

#L442 unnecessary named return mapleGlobals_

#L456 unnecessary named return collateral_

#L495 unnecessary named return interest_

#L594 unnecessary named return periodicInterestRate_

#L599 unnecessary named return refinanceCommitment_

Will change apr and balanceOfAssets tonot use return, but this isn't really related to this issue.

lucas-manuel commented 2 years ago

1 and 2 are confirmed but duplicates 3 and 4 are acknowledged