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.

Sign up for GitHub

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

Jump to bottom

Gas Optimizations #46

Open
code423n4 opened this issue Jun 29, 2022 · 1 comment
Open

Gas Optimizations #46

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

Comments

@code423n4
Copy link
Contributor

2022-06-NewBlockchain-v2 Report

Files Description Table

File Name SHA-1 Hash
2022-06-NewBlockchain-v2/lending-market-v2/contracts/Governance/GovernorBravoDelegate.sol 307e3d50b6b6a8af9dc2708049078ce78af6f83d
2022-06-NewBlockchain-v2/lending-market-v2/contracts/Stableswap/BaseV1-core.sol 1a4c315f37e8ec7b1d274e6fba2e69dbda742d4e
2022-06-NewBlockchain-v2/lending-market-v2/contracts/Stableswap/BaseV1-periphery.sol f232643764224059e5d24ab1334fa4a32191e7ef

Gas Optimizations

[G-01]: Arithmetics: Shift Right instead of Dividing by 2

Impact

While the DIV opcode uses 5 gas, the SHR opcode only uses 3 gas (https://www.evm.codes/).
Furthermore, Solidity’s division operation also includes a division-by-0 prevention which is bypassed using shifting.

Affected Code:

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Stableswap/BaseV1-core.sol::23 => uint x = y / 2 + 1;

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Stableswap/BaseV1-core.sol::26 => x = (y / x + x) / 2;

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Stableswap/BaseV1-periphery.sol::48 => uint x = y / 2 + 1;

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Stableswap/BaseV1-periphery.sol::51 => x = (y / x + x) / 2;

Mitigation

Replace / 2 with >> 1 .

Tools used

VS Code

[G-02]: Variables: No need to explicitly initialize variables with default values

Impact

If a variable is not set/initialized, it is assumed to have the default value (0, false, 0x0 etc depending on the data type). If you explicitly initialize it with its default value, you are just wasting gas.

Code Affected:

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Stableswap/BaseV1-core.sol::226 => uint nextIndex = 0;

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Stableswap/BaseV1-core.sol::227 => uint index = 0;

Mitigation

Do not initialize variables with default values.

Tools used

VS Code

[G-03]: For-Loops: Pre-increments cost less gas compared to post-increments

Impact

Pre-increments cost less gas compared to post-increments.

Code Affected:

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Stableswap/BaseV1-core.sol::210 => for (uint i = 0; i < _prices.length; i++) {

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Stableswap/BaseV1-core.sol::340 => for (uint i = 0; i < 255; i++) {

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Stableswap/BaseV1-periphery.sol::154 => for (uint i = 0; i < routes.length; i++) {

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Stableswap/BaseV1-periphery.sol::380 => for (uint i = 0; i < routes.length; i++) {

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Comptroller.sol::131 => for (uint i = 0; i < len; i++) {

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Comptroller.sol::211 => for (uint i = 0; i < len; i++) {

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Comptroller.sol::742 => for (uint i = 0; i < assets.length; i++) {

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Comptroller.sol::1010 => for(uint i = 0; i < numMarkets; i++) {

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Comptroller.sol::1352 => for (uint i = 0; i < cTokens.length; i++) {

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Governance/GovernorBravoDelegate.sol::66 => for (uint i = 0; i < newProposal.targets.length; i++) {

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Governance/GovernorBravoDelegate.sol::88 => for (uint i = 0; i < proposal.targets.length; i++) {

Mitigation

Change i++ to ++i.

Tools used

VS Code

[G-04]: For-Loops: Increments can be unchecked

Impact

In Solidity 0.8+, there’s a default overflow check on unsigned integers.

Code Affected:

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Stableswap/BaseV1-core.sol::210 => for (uint i = 0; i < _prices.length; i++) {

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Stableswap/BaseV1-core.sol::340 => for (uint i = 0; i < 255; i++) {

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Stableswap/BaseV1-periphery.sol::154 => for (uint i = 0; i < routes.length; i++) {

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Stableswap/BaseV1-periphery.sol::380 => for (uint i = 0; i < routes.length; i++) {

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Comptroller.sol::131 => for (uint i = 0; i < len; i++) {

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Comptroller.sol::211 => for (uint i = 0; i < len; i++) {

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Comptroller.sol::742 => for (uint i = 0; i < assets.length; i++) {

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Comptroller.sol::1010 => for(uint i = 0; i < numMarkets; i++) {

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Comptroller.sol::1352 => for (uint i = 0; i < cTokens.length; i++) {

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Governance/GovernorBravoDelegate.sol::66 => for (uint i = 0; i < newProposal.targets.length; i++) {

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Governance/GovernorBravoDelegate.sol::88 => for (uint i = 0; i < proposal.targets.length; i++) {

Mitigation

One example is the code would go from:

for (uint i = 0; i < _prices.length; i++) {
    priceAverageCumulative += _prices[i];
}

to:

for (uint i = 0; i < _prices.length;) {
    priceAverageCumulative += _prices[i];
    unchecked { i++; }
}

Tools used

VS Code

[G-05]: For-Loops: No need to explicitly initialize variables with default values

Impact

If a variable is not set/initialized, it is assumed to have the default value (0, false, 0x0 etc depending on the data type). If you explicitly initialize it with its default value, you are just wasting gas.

Code Affected:

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Stableswap/BaseV1-core.sol::210 => for (uint i = 0; i < _prices.length; i++) {

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Stableswap/BaseV1-core.sol::340 => for (uint i = 0; i < 255; i++) {

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Stableswap/BaseV1-periphery.sol::154 => for (uint i = 0; i < routes.length; i++) {

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Stableswap/BaseV1-periphery.sol::380 => for (uint i = 0; i < routes.length; i++) {

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Comptroller.sol::131 => for (uint i = 0; i < len; i++) {

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Comptroller.sol::211 => for (uint i = 0; i < len; i++) {

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Comptroller.sol::742 => for (uint i = 0; i < assets.length; i++) {

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Comptroller.sol::1010 => for(uint i = 0; i < numMarkets; i++) {

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Comptroller.sol::1352 => for (uint i = 0; i < cTokens.length; i++) {

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Governance/GovernorBravoDelegate.sol::66 => for (uint i = 0; i < newProposal.targets.length; i++) {

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Governance/GovernorBravoDelegate.sol::88 => for (uint i = 0; i < proposal.targets.length; i++) {

Mitigation

Do not initialize variables with default values.

Tools used

VS Code

[G-06]: Comparisons: Use != 0 rather than > 0 for unsigned integers in require() statements

Impact

When the optimizer is enabled, gas is wasted by doing a greater-than operation, rather than a not-equals operation inside require() statements. When using !=, the optimizer is able to avoid the EQ, ISZERO, and associated operations, by relying on the JUMPI that comes afterwards, which itself checks for zero.

Affected Code:

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Stableswap/BaseV1-core.sol::256 => require(liquidity > 0, 'ILM'); // BaseV1: INSUFFICIENT_LIQUIDITY_MINTED

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Stableswap/BaseV1-core.sol::275 => require(amount0 > 0 && amount1 > 0, 'ILB'); // BaseV1: INSUFFICIENT_LIQUIDITY_BURNED

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Stableswap/BaseV1-core.sol::289 => require(amount0Out > 0 || amount1Out > 0, 'IOA'); // BaseV1: INSUFFICIENT_OUTPUT_AMOUNT

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Stableswap/BaseV1-core.sol::306 => require(amount0In > 0 || amount1In > 0, 'IIA'); // BaseV1: INSUFFICIENT_INPUT_AMOUNT

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Stableswap/BaseV1-periphery.sol::122 => require(amountA > 0, "BaseV1Router: INSUFFICIENT_AMOUNT");

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Stableswap/BaseV1-periphery.sol::123 => require(reserveA > 0 && reserveB > 0, "BaseV1Router: INSUFFICIENT_LIQUIDITY");

Mitigation

Use != 0 rather than > 0 for unsigned integers in require() statements.

Tools used

VS Code

[G-07]: Comparisons: Boolean Comparisons

Impact

Comparing to a constant (true or false) is a bit more expensive than directly checking the returned boolean value.

Code Affected:

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Comptroller.sol::154 => if (marketToJoin.accountMembership[borrower] == true) {

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Comptroller.sol::1355 => if (borrowers == true) {

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Comptroller.sol::1362 => if (suppliers == true) {

Mitigation

Remove the == true.

Tools used

VS Code

[G-08]: OPEN TODOS

Impact

Code architecture, incentives, and error handling/reporting questions/issues should be resolved before deployment. It is recommended to avoid open TODOs as they may indicate programming errors that still need to be fixed.

Code Affected:

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Comptroller.sol::1237 => // TODO: Don't distribute supplier COMP if the user is not in the supplier market.

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Comptroller.sol::1276 => // TODO: Don't distribute supplier COMP if the user is not in the borrower market.

Tools used

VS Code

[G-09]: Require instead of &&

Impact

Require statements including conditions with the && operator can be broken down in multiple require statements to save gas.

Code Affected:

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Stableswap/BaseV1-core.sol::275 => require(amount0 > 0 && amount1 > 0, 'ILB'); // BaseV1: INSUFFICIENT_LIQUIDITY_BURNED

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Stableswap/BaseV1-core.sol::291 => require(amount0Out < _reserve0 && amount1Out < _reserve1, 'IL'); // BaseV1: INSUFFICIENT_LIQUIDITY

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Stableswap/BaseV1-core.sol::297 => require(to != _token0 && to != _token1, 'IT'); // BaseV1: INVALID_TO

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Stableswap/BaseV1-core.sol::434 => require(recoveredAddress != address(0) && recoveredAddress == owner, 'BaseV1: INVALID_SIGNATURE');

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Stableswap/BaseV1-core.sol::471 => require(success && (data.length == 0 || abi.decode(data, (bool))));

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Stableswap/BaseV1-periphery.sol::123 => require(reserveA > 0 && reserveB > 0, "BaseV1Router: INSUFFICIENT_LIQUIDITY");

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Stableswap/BaseV1-periphery.sol::477 => require(success && (data.length == 0 || abi.decode(data, (bool))));

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Comptroller.sol::1008 => require(numMarkets != 0 && numMarkets == numBorrowCaps, "invalid input");

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Comptroller.sol::1416 => require(numTokens == supplySpeeds.length && numTokens == borrowSpeeds.length, "Comptroller::_setCompSpeeds invalid input");

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Governance/GovernorBravoDelegate.sol::42 => require(unigovProposal.targets.length == unigovProposal.values.length &&

2022-06-NewBlockchain-v2/lending-market-v2/contracts/Governance/GovernorBravoDelegate.sol::43 => unigovProposal.targets.length == unigovProposal.signatures.length &&

Mitigation

Breakdown each condition in a separate require statement.

Tools used

VS Code
@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels Jun 29, 2022
code423n4 added a commit that referenced this issue Jun 29, 2022
@code423n4
cRat1st0s issue #46
ef60dd8
@GalloDaSballo
Copy link
Collaborator

No immutables so all the findings will save less than 200 gas

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

No branches or pull requests
2 participants
@GalloDaSballo @code423n4