Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gas Optimizations #12

Open
code423n4 opened this issue Mar 21, 2022 · 1 comment
Open

Gas Optimizations #12

code423n4 opened this issue Mar 21, 2022 · 1 comment
Labels
bug Something isn't working G (Gas Optimization)

Comments

@code423n4
Copy link
Contributor

Gas Report

Table of Contents:

Foreword

  • Storage-reading optimizations

The code can be optimized by minimising the number of SLOADs. SLOADs are expensive (100 gas) compared to MLOADs/MSTOREs (3 gas). In the paragraphs below, please see the @audit-issue tags in the pieces of code's comments for more information about SLOADs that could be saved by caching the mentioned storage variables in memory variables.

  • @audit tags

The code is annotated at multiple places with //@audit comments to pinpoint the issues. Please, pay attention to them for more details.

File: xMPL.sol

function scheduleMigration()

File: xMPL.sol
76:     function scheduleMigration(address migrator_, address newAsset_) external override onlyOwner {
77:         require(migrator_ != address(0), "xMPL:SM:INVALID_MIGRATOR");
78:         require(newAsset_ != address(0), "xMPL:SM:INVALID_NEW_ASSET");
79: 
80:         scheduledMigrationTimestamp = block.timestamp + MINIMUM_MIGRATION_DELAY;
81:         scheduledMigrator           = migrator_;
82:         scheduledNewAsset           = newAsset_;
83: 
84:         emit MigrationScheduled(asset, newAsset_, migrator_, scheduledMigrationTimestamp); //@audit cache block.timestamp + MINIMUM_MIGRATION_DELAY
85:     }

Cache block.timestamp + MINIMUM_MIGRATION_DELAY in memory to save gas

It's possible to save 1 SLOAD by saving block.timestamp + MINIMUM_MIGRATION_DELAY in a memory variable and use this variable at L80 and L84 instead of emitting the storage variable at L84.

File: RevenueDistributionToken.sol

function updateVestingSchedule()

File: RevenueDistributionToken.sol
79:     function updateVestingSchedule(uint256 vestingPeriod_) external override returns (uint256 issuanceRate_, uint256 freeAssets_) {
80:         require(msg.sender == owner, "RDT:UVS:NOT_OWNER");
81:         require(totalSupply != 0,    "RDT:UVS:ZERO_SUPPLY");
82: 
83:         // Update "y-intercept" to reflect current available asset.
84:         freeAssets_ = freeAssets = totalAssets();
85: 
86:         // Calculate slope.
87:         issuanceRate_ = issuanceRate = ((ERC20(asset).balanceOf(address(this)) - freeAssets_) * precision) / vestingPeriod_;
88: 
89:         // Update timestamp and period finish.
90:         vestingPeriodFinish = (lastUpdated = block.timestamp) + vestingPeriod_; //@audit cache calculation
91: 
92:         emit VestingScheduleUpdated(msg.sender, vestingPeriodFinish, issuanceRate); //@audit emit issuanceRate_  //@audit use cached calculation
93:     }

Cache (lastUpdated = block.timestamp) + vestingPeriod_ in memory to save gas

It's possible to save 1 SLOAD by saving (lastUpdated = block.timestamp) + vestingPeriod_ in a memory variable and use this variable at L90 and L92 instead of emitting the storage variable vestingPeriodFinish at L92.

Emit the memory variable issuanceRate_ instead of the storage variable issuanceRate L92

This can save 1 SLOAD

function totalAssets()

File: RevenueDistributionToken.sol
258:     function totalAssets() public view override returns (uint256 totalManagedAssets_) {
259:         if (issuanceRate == 0) return freeAssets; //@audit issuanceRate SLOAD 1
260: 
261:         uint256 vestingTimePassed =
262:             block.timestamp > vestingPeriodFinish ? //@audit vestingPeriodFinish SLOAD 1
263:                 vestingPeriodFinish - lastUpdated :  //@audit vestingPeriodFinish SLOAD 2
264:                 block.timestamp - lastUpdated;
265: 
266:         return ((issuanceRate * vestingTimePassed) / precision) + freeAssets; //@audit issuanceRate SLOAD 2
267:     }

Cache issuanceRate

Caching this in memory can save around 1 SLOAD

Cache vestingPeriodFinish

Caching this in memory can save around 1 SLOAD

General recommendations

Comparisons

> 0 is less efficient than != 0 for unsigned integers (with proof)

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

Proof: While it may seem that > 0 is cheaper than !=, this is only true without the optimizer enabled and outside a require statement. If you enable the optimizer at 10k AND you're in a require statement, this will save gas. You can see this tweet for more proofs: https://twitter.com/gzeon/status/1485428085885640706

I suggest changing > 0 with != 0 here:

mpl-migration/contracts/Migrator.sol:25:        require(amount_ > uint256(0),                                              "M:M:ZERO_AMOUNT");

Also, please enable the Optimizer.

For-Loops

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.

Here, I suggest storing the array's length in a variable before the for-loop, and use it instead:

File: MapleLoanInternals.sol:258:         for (uint256 i; i < calls_.length; ++i) {

Increments can be unchecked

In Solidity 0.8+, there's a default overflow check on unsigned integers. It's possible to uncheck this in for-loops and save some gas at each iteration, but at the cost of some code readability, as this uncheck cannot be made inline.

ethereum/solidity#10695

Instances include:

File: MapleLoanInternals.sol:258:         for (uint256 i; i < calls_.length; ++i) {

The code would go from:

for (uint256 i; i < numIterations; ++i) {  
 // ...  
}  

to:

for (uint256 i; i < numIterations;) {  
 // ...  
 unchecked { ++i; }  
}  

The risk of overflow is inexistant for a uint256 here.

@JGcarv
Copy link
Collaborator

JGcarv commented Mar 23, 2022

We will address the following:

  • RevenueDistributionToken.sol function totalAssets()
  • Migrator.sol changing > 0 with != 0 here:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working G (Gas Optimization)
Projects
None yet
Development

No branches or pull requests

2 participants