code-423n4 / 2022-05-velodrome-findings

0 stars 0 forks source link

Gas Optimizations #30

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Delete optimization

Use delete instead of set to default value (false or 0)

Affected source code:

Use require instead of assert

The assert() and require() functions are a part of the error handling aspect in Solidity. Solidity makes use of state-reverting error handling exceptions. This means all changes made to the contract on that call or any sub-calls are undone if an error is thrown. It also flags an error.

They are quite similar as both check for conditions and if they are not met, would throw an error.

The big difference between the two is that the assert() function when false, uses up all the remaining gas and reverts all the changes made.

Meanwhile, a require() function when false, also reverts back all the changes made to the contract but does refund all the remaining gas fees we offered to pay. This is the most common Solidity function used by developers for debugging and error handling.

Affected source code:

Use constants

In the RewardsDistributor contract the WEEK*WEEK operation is performed multiple times, storing this result in a constant is a good way to save computational time and gas.

Affected source code:

Logic improvements

Below is a series of changes in the logic that will improve the cost of gas.

_mint(msg.sender, 0); is not needed because it doesn't produce any change, so it's better to remove this line.

Affected source code:

A method for return the block.number and is not used like you can see here:

Affected source code:

Using the right type it's possible to save slots between calls, decimals must be uint8 according to ERC20, so it's better to use this type in our methods.

Change decimals0 and decimals1 from uint256 to uint8:

Affected source code:

msg.sender is sent twice, it is better to avoid concatenating it by using abi.encode.

ILayerZeroEndpoint(endpoint).send{value: msg.value}(
    optimismChainId,
    abi.encodePacked(optimismReceiver),
    abi.encode(msg.sender, amount),
    payable(msg.sender),
    zroPaymentAddress,
    zroTransactionParams
);

Affected source code:

Remove unused args

bytes memory /*params*/:

uint256, /* proposalId */, uint256[] memory, /* values */ and bytes32 /*descriptionHash*/:

++i costs less gas compared to i++ or i += 1

++i costs less gas compared to i++ or i += 1 for unsigned integer, as pre-increment is cheaper (about 5 gas per iteration). This statement is true even with the optimizer enabled.

i++ increments i and returns the initial value of i. Which means:

uint i = 1;
i++; // == 1 but i == 2

But ++i returns the actual incremented value:

uint i = 1;
++i; // == 2 and i == 2 too, so no need for a temporary variable

In the first case, the compiler has to create a temporary variable (when used) for returning 1 instead of 2 I suggest using ++i instead of i++ to increment the value of an uint variable. Same thing for --i and i--

Affected source code:

Reduce the size of error messages (Long revert Strings)

Shortening revert strings to fit in 32 bytes will decrease deployment time gas and will decrease runtime gas when the revert condition is met.

Revert strings that are longer than 32 bytes require at least one additional mstore, along with additional overhead for computing memory offset, etc.

I suggest shortening the revert strings to fit in 32 bytes, or that using custom errors as described next (require pragma upgrade).

Use Custom Errors instead of Revert Strings to save Gas

Custom errors from Solidity 0.8.4 are cheaper than revert strings (cheaper deployment cost and runtime cost when the revert condition is met)

Source Custom Errors in Solidity:

Starting from Solidity v0.8.4, there is a convenient and gas-efficient way to explain to users why an operation failed through the use of custom errors. Until now, you could already use strings to give more information about failures (e.g., revert("Insufficient funds.");), but they are rather expensive, especially when it comes to deploy cost, and it is difficult to use dynamic information in them.

Custom errors are defined using the error statement, which can be used inside and outside of contracts (including interfaces and libraries).

Affected source code:

Avoid multiple casting

Use the enum VoteType instead of casting the uint8:

Affected source code:

Gas saving using immutable.

It's possible to avoid storage access a save gas using immutable keyword for the following variables:

Affected source code:

Use variable caching

Reduce storage or memory array access. Caching the value inside a loop is a good practice to spend less gas.

routes[i]:

GalloDaSballo commented 2 years ago

Delete optimization

Doesn't save gas

 Use require instead of assert

Valid but also can't quantify run-time savings

Use constants

Disagree with the low quality finding, X / WEEK * WEEK is using integer division to segment the math

_mint(msg.sender, 0);

Agree, I believe this will save 8 gas for the function call and perhaps 6 gas on the 0 check 14 gas

A method for return the block.number and is not used like you can see here:

Disagree as the function is external

Using the right type it's possible to save slots between calls, decimals must be uint8 according to ERC20, so it's better to use this type in our methods.

Disagree especially in lack of POC, smaller size numbers will require masking and other checks, making them less efficient

msg.sender is sent twice, it is better to avoid concatenating it by using abi.encode.

Disagree as that's the function signature

Remove unused args

Disagree as that's the function signature

++i costs less gas compared to i++ or i += 1

Saves 5 gas per instance

36 * 5 = 180

Use the enum VoteType instead of casting the uint8:

Valid QA but not valid gas wise

Gas saving using immutable.

Valid gas saving In lack of additional details will award on SLOAD per variable

4 * 2100 = 8400

Use variable caching

No explanation = no points

Overall the report feels very automated and could have just contained the immutable and ++i savings and be shorter but as effective

Total gas saved: 8594