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:
./notional-coop/notional-wrapped-fcash/contracts/wfCashERC4626.sol:23: require(underlyingExternal > 0, "Must Settle");
./notional-coop/notional-wrapped-fcash/contracts/wfCashLogic.sol:211: require(opts.receiver != address(0), "Receiver is zero address");
2. > 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:
./notional-coop/notional-wrapped-fcash/contracts/wfCashBase.sol:37: require(cashGroup.maxMarketIndex > 0, "Invalid currency");
4. ++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:
250: for(uint256 i = 0; i < modules.length; i++) {266: for(uint256 i = 0; i < modules.length; i++) {405: for(uint256 i = 0; i < positionsLength; i++) {617: for(uint256 i = 0; i < positionsLength; i++) {622: numFCashPositions++;630: for(uint256 i = 0; i < positionsLength; i++) {635: j++;
5. 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:
./notional-coop/notional-wrapped-fcash/contracts/lib/Constants.sol:6: address internal constant ETH_ADDRESS = address(0);NotionalTradeModule.sol 50: address internal constant ETH_ADDRESS = address(0);
NotionalTradeModule.sol:
250: for(uint256 i = 0; i < modules.length; i++){266: for(uint256 i = 0; i < modules.length; i++) {405: for(uint256 i = 0; i < positionsLength; i++) {617: for(uint256 i = 0; i < positionsLength; i++) {630: for(uint256 i = 0; i < positionsLength; i++) {519: uint32 minImpliedRate = 0;
6. change constant to immutable
Due to how constant variables are implemented (replacements at compile-time), an expression assigned to a constant variable is recomputed each time that the variable is used, which wastes some gas.
If the variable was immutable instead: the calculation would only be done once at deploy time (in the constructor), and then the result would be saved and read directly at runtime rather than being recalculated.
Consequences: each usage of a "constant" costs ~100gas more on each access (it is still a little better than storing the result in storage, but not much..). since these are not real constants, they can't be referenced from a real constant environment (e.g. from assembly, or from another library )
.
./notional-coop/notional-wrapped-fcash/contracts/proxy/WrappedfCashFactory.sol:12: bytes32 public constant SALT = 0;
`
Change these expressions from constant to immutable and implement the calculation in the constructor or hardcode these values in the constants and add a comment to say how the value was calculated.
7. chane string to bytes[x] because the amount of chars of constant
make it bytes20 instead of string to save 12bytes of gas every time its read.
8.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
ex: 585: require(_paymentToken == assetToken, "Token is neither asset nor underlying token");
IMPACT
Require statements including conditions with the && operator can be broken down in multiple require statements to save gas.
instances include :
wfCashLogic.sol:116wfCashLogic.sol:129
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:
./notional-coop/notional-wrapped-fcash/contracts/wfCashERC4626.sol:23: require(underlyingExternal > 0, "Must Settle");./notional-coop/notional-wrapped-fcash/contracts/wfCashLogic.sol:211: require(opts.receiver != address(0), "Receiver is zero address");./notional-coop/notional-wrapped-fcash/contracts/wfCashLogic.sol:225: require(0 < cashBalance, "Negative Cash Balance");wfCashLogic:57 wfCashLogic:116 wfCashLogic:211 wfCashLogic:225 wfCashERC4626.sol#L23 wfCashBase.sol#L37 wfCashBase.sol#L40 NotionalTradeModule.sol#L234 NotionalTradeModule.sol#L269 NotionalTradeModule.sol#L280 NotionalTradeModule.sol#L378 NotionalTradeModule.sol#L449 NotionalTradeModule.sol#L485 NotionalTradeModule.sol#L169 NotionalTradeModule.sol#L199 NotionalTradeModule.sol#L378 NotionalTradeModule.sol#L573
2. > 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:
./notional-coop/notional-wrapped-fcash/contracts/wfCashBase.sol:37: require(cashGroup.maxMarketIndex > 0, "Invalid currency");4. ++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:
250: for(uint256 i = 0; i < modules.length; i++) {266: for(uint256 i = 0; i < modules.length; i++) {405: for(uint256 i = 0; i < positionsLength; i++) {617: for(uint256 i = 0; i < positionsLength; i++) {622: numFCashPositions++;630: for(uint256 i = 0; i < positionsLength; i++) {635: j++;5. 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:
./notional-coop/notional-wrapped-fcash/contracts/lib/Constants.sol:6: address internal constant ETH_ADDRESS = address(0);NotionalTradeModule.sol 50: address internal constant ETH_ADDRESS = address(0);NotionalTradeModule.sol:
250: for(uint256 i = 0; i < modules.length; i++){266: for(uint256 i = 0; i < modules.length; i++) {405: for(uint256 i = 0; i < positionsLength; i++) {617: for(uint256 i = 0; i < positionsLength; i++) {630: for(uint256 i = 0; i < positionsLength; i++) {519: uint32 minImpliedRate = 0;6. change constant to immutable
Due to how constant variables are implemented (replacements at compile-time), an expression assigned to a constant variable is recomputed each time that the variable is used, which wastes some gas.
If the variable was immutable instead: the calculation would only be done once at deploy time (in the constructor), and then the result would be saved and read directly at runtime rather than being recalculated. Consequences: each usage of a "constant" costs ~100gas more on each access (it is still a little better than storing the result in storage, but not much..). since these are not real constants, they can't be referenced from a real constant environment (e.g. from assembly, or from another library ) .
/notional-coop/notional-wrapped-fcash/contracts/lib/Constants.sol:45: uint16 internal constant MAX_CURRENCIES = uint16(UNMASK_FLAGS);./notional-coop/notional-wrapped-fcash/contracts/proxy/WrappedfCashFactory.sol:12: bytes32 public constant SALT = 0;` Change these expressions from constant to immutable and implement the calculation in the constructor or hardcode these values in the constants and add a comment to say how the value was calculated.7. chane string to bytes[x] because the amount of chars of constant
make it bytes20 instead of string to save 12bytes of gas every time its read.
NotionalTradeModule.sol 111: string constant internal DEFAULT_ISSUANCE_MODULE_NAME = "DefaultIssuanceModule";8.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 ex: 585:
require(_paymentToken == assetToken, "Token is neither asset nor underlying token");NotionalTradeModule.sol#L169 NotionalTradeModule.sol#L199 NotionalTradeModule.sol#L378 NotionalTradeModule.sol#L573
9. REQUIRE INSTEAD OF &&
IMPACT Require statements including conditions with the && operator can be broken down in multiple require statements to save gas. instances include :
wfCashLogic.sol:116wfCashLogic.sol:129