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

1 stars 0 forks source link

Gas Optimizations #195

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

1.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). ex of instances include: https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/BaseConnextFacet.sol#L38 https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol#L72 in Stableswap.sol : 125,102,89,90,96,97,98,177,167, https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/libraries/AmplificationUtils.sol#L84-L86 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/libraries/AmplificationUtils.sol#L92-L94 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/libraries/AmplificationUtils.sol#L111 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/libraries/LibDiamond.sol#L191-L193

2. you can add free variables

address is 160 bits you have 96 bits left over for another variable instead of it getting wasted make a uninitlized variable which dosnt cost any more gas https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/BridgeFacet.sol#L35 https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol#L55-L59 you can make this variable in a struct uitn256 instead of uint32 because uint256 take up less gas. because uint32 need to get converted to that. https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/libraries/ConnextMessage.sol#L30

also save a sslot of gas by putting struct order like this to save gas

address ,bool,address,address bool=1byte it can get added to address to make it one slot instead of taking up one slot https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol#L55 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/libraries/LibConnextStorage.sol#L47-L54

3. Mark functions as payable when users can't mistakenly send ETH

impact

Functions marked as payable are 24 gas cheaper than their counterpart (in non-payable functions, Solidity adds an extra check to ensure msg.value is zero). When users can't mistakenly send ETH to a function (as an example, when there's an onlyOwner modifier or alike), it is safe to mark it as payable

proof of concept

Functions with onlyOwner modifier that aren't payable yet: instances include: https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/BridgeFacet.sol#L233 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/BridgeFacet.sol#L242 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/BridgeFacet.sol#L250 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/ProposedOwnableFacet.sol#L128 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/ProposedOwnableFacet.sol#L142 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/ProposedOwnableFacet.sol#L162 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/ProposedOwnableFacet.sol#L175 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/ProposedOwnableFacet.sol#L203 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/ProposedOwnableFacet.sol#L217 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/ProposedOwnableFacet.sol#L253 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/ProposedOwnableFacet.sol#L258 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/RelayerFacet.sol#L88 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/RelayerFacet.sol#L101 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/RelayerFacet.sol#L112 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/RoutersFacet.sol#L263 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/RoutersFacet.sol#L293 https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/facets/StableSwapFacet.sol#L502 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/helpers/LPToken.sol#L34 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/helpers/ProposedOwnableUpgradeable.sol#L155

make these public functions, exeternal to save gas

https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/promise/PromiseRouter.sol#L146

4. save gas by instead of += 1 which cost more gas then using ++var

https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/BridgeFacet.sol#L395

5. ++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 instances include: NotionalTradeModule.sol: https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/BridgeFacet.sol#L747 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/BridgeFacet.sol#L843 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/BridgeFacet.sol#L1004 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/DiamondLoupeFacet.sol#L31 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/RelayerFacet.sol#L144 https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/facets/StableSwapFacet.sol#L415 https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol#L176 https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/helpers/StableSwap.sol#L81 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/libraries/LibDiamond.sol#L153 https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/libraries/SwapUtils.sol#L405 https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/libraries/SwapUtils.sol#L425 https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/libraries/SwapUtils.sol#L558 https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/libraries/SwapUtils.sol#L591 https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/libraries/SwapUtils.sol#L844 https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/libraries/SwapUtils.sol#L924 https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/libraries/SwapUtils.sol#L1014 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/relayer-fee/libraries/RelayerFeeMessage.sol#L85

6. use encodepacked instead of encode

because encode puts zeros in the encoding if not filled and it wastes gas https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/BridgeFacet.sol#L918

7. No need to explicitly initialize variables with default values

If a variable is not set/initialized, it is assumed to have the default value (0 for uint, false for bool, address(0) for address...). Explicitly initializing it with its default value is an anti-pattern and wastes gas. instances include: https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/facets/StableSwapFacet.sol#L415 https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/facets/StableSwapFacet.sol#L451-L452 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/VersionFacet.sol#L16 https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol#L176 https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/helpers/StableSwap.sol#L81 https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/libraries/SwapUtils.sol#L405 https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/libraries/SwapUtils.sol#L558 https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/libraries/SwapUtils.sol#L591 https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/libraries/SwapUtils.sol#L844 https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/libraries/SwapUtils.sol#L924

is sstore save 20_000 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/upgrade-initializers/DiamondInit.sol#L70

uint Var is anything greater or equal to zero saves gas to make the condition != 0 in require statement

https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol#L150 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/libraries/AmplificationUtils.sol#L86 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/libraries/LibDiamond.sol#L121 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/libraries/LibDiamond.sol#L139

Using bools for storage incurs overhead

// Booleans are more expensive than uint256 or any type that takes up a full // word because each write operation emits an extra SLOAD to first read the // slot's contents, replace the bits taken up by the boolean, and then write // back. This is the compiler's defense against contract upgrades and // pointer aliasing, and it cannot be disabled. https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/helpers/ProposedOwnableUpgradeable.sol#L57 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/helpers/ProposedOwnableUpgradeable.sol#L54

REQUIRE INSTEAD OF &&

IMPACT

Require statements including conditions with the && operator can be broken down in multiple require statements to save gas. instances include : https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/helpers/StableSwap.sol#L85

use 1e18 instead of 10**18 to save gas

10 ** 18 can be changed to 1e18 to avoid unnecessary arithmetic operation and save gas. https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/libraries/AmplificationUtils.sol#L22 https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/libraries/SwapUtils.sol#L103-L113

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. 1 byte for character https://githubcom/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/libraries/AmplificationUtils.sol#L86 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/libraries/LibDiamond.sol#L66 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/libraries/LibDiamond.sol#L123 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/libraries/LibDiamond.sol#L132 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/libraries/LibDiamond.sol#L139 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/libraries/LibDiamond.sol#L141 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/libraries/LibDiamond.sol#L150 https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/libraries/LibDiamond.sol#L191-L193 https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/libraries/SwapUtils.sol#L790

make constant value ,compute keccak off chain to save gas and then assign it.

https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/libraries/LibDiamond.sol#L14

Unchecking arithmetics operations that can’t underflow/overflow

Solidity version 0.8+ comes with implicit overflow and underflow checks on unsigned integers. When an overflow or an underflow isn’t possible (as an example, when a comparison is made before the arithmetic operation), some gas can be saved by using an unchecked block: Checked or Unchecked Arithmetic. I suggest wrapping with an unchecked block: Same thing with second unchecked because total can't overflow amount cant overflow https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/libraries/SwapUtils.sol#L405 https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/libraries/SwapUtils.sol#L425 https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/libraries/SwapUtils.sol#L558 https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/libraries/SwapUtils.sol#L1055