If you are reading from storage more than once, it is cheaper in gas cost to cache the variable in memory: a SLOAD cost 100gas, while MLOAD and MSTORE cost 3 gas.
Calldata instead of memory for RO function parameters
PROBLEM
If a reference type function parameter is read-only, it is cheaper in gas to use calldata instead of memory.
Calldata is a non-modifiable, non-persistent area where function arguments are stored, and behaves mostly like memory.
Try to use calldata as a data location because it will avoid copies and also makes sure that the data cannot be modified.
When the comparison is with a constant storage variable, you can also do the increment in the storage variable declaration
example:
-require(
_feeConfig[1] <= PERFORMANCE_FEE_HARD_CAP,
"performanceFeeStrategist too high"
);
+require(
_feeConfig[1] < PERFORMANCE_FEE_HARD_CAP_PLUS_ONE,
"performanceFeeStrategist too high"
);
and
-uint256 public constant PERFORMANCE_FEE_HARD_CAP = 3_000;
+uint256 public constant PERFORMANCE_FEE_HARD_CAP_PLUS_ONE = 3_001;
Custom Errors
PROBLEM
Custom errors from Solidity 0.8.4 are cheaper than revert strings (cheaper deployment cost and runtime cost when the revert condition is met) while providing the same amount of information, as explained here
Custom errors are defined using the error statement
If a variable is not set/initialized, it is assumed to have the default value (0, false, 0x0 etc depending on the data type).
Explicitly initializing it with its default value is an anti-pattern and wastes gas.
PROOF OF CONCEPT
Instances include:
CitadelMinter.sol
CitadelMinter.sol:152: for (uint256 i = 0; i < numPools; i++) {
CitadelMinter.sol:180: uint256 lockingAmount = 0;
CitadelMinter.sol:181: uint256 stakingAmount = 0;
CitadelMinter.sol:182: uint256 fundingAmount = 0;
Remove explicit initialization for default values.
Event emitting of local variable
PROBLEM
When emitting an event, using a local variable instead of a storage variable saves gas.
PROOF OF CONCEPT
Instances include:
Funding.sol
Funding.sol: 343
SupplySchedule.sol
SupplySchedule.sol: 197
TOOLS USED
Manual Analysis
MITIGATION
In both instances, the storage variable is read multiple times in the function and it is recommended to cache them into memory, then passing these cached variables in the emit statement, as explained in the cache paragraph
Inline functions
PROBLEM
When we define internal functions to perform computation:
The contract’s code size gets bigger
the function call consumes more gas than executing it as an inlined function (part of the code, without the function call)
When it does not affect readability, it is recommended to inline functions in order to save gas
Prefix increments are cheaper than postfix increments.
PROOF OF CONCEPT
Instances include:
CitadelMinter.sol
CitadelMinter.sol:152
CitadelMinter.sol:344
SupplySchedule.sol
SupplySchedule.sol:208
TOOLS USED
Manual Analysis
MITIGATION
change i++ to ++i
Short-circuiting
PROBLEM
Reading from storage costs 100 gas.
When checking conditions using the AND operator, we can save gas by not evaluating the second expression if the first one is false.
PROOF OF CONCEPT
Instances include:
CitadelMinter.sol
CitadelMinter.sol:266
TOOLS USED
Manual Analysis
MITIGATION
Remove the declaration of poolExists line 261 and inline the computation of fundingPools.contains(_pool) line 266 and 273.
Tight Variable Packing
PROBLEM
Solidity contracts have contiguous 32 bytes (256 bits) slots used in storage.
By arranging the variables, it is possible to minimize the number of slots used within a contract's storage and therefore reduce deployment costs.
address type variables are each of 20 bytes size (way less than 32 bytes). However, they here take up a whole 32 bytes slot (they are contiguous).
As bool type variables are of size 1 byte, there's a slot here that can get saved by moving them closer to an address
PROOF OF CONCEPT
Instances include:
Funding.sol
Funding.sol:39:
bool public citadelPriceFlag; /// Flag citadel price for review by guardian if it exceeds min and max bounds;
uint256 public assetDecimalsNormalizationValue;
address public citadelPriceInAssetOracle;
address public saleRecipient;
TOOLS USED
Manual Analysis
MITIGATION
Place citadelPriceFlag after SaleRecipient to save one storage slot
uint256 public assetDecimalsNormalizationValue;
address public citadelPriceInAssetOracle;
address public saleRecipient;
+bool public citadelPriceFlag;
Uint256 instead of Uint8
PROBLEM
Each storage slot has 256 bits.
Whenever a uint smaller than 256 (or even a bool) is pulled from storage, the EVM casts it to a uint256.
Calculations are also unexceptionally performed in uint256 by the EVM.
The default "checked" behavior costs more gas when adding/diving/multiplying, because under-the-hood those checks are implemented as a series of opcodes that, prior to performing the actual arithmetic, check for under/overflow and revert if it is detected.
if it can statically be determined there is no possible way for your arithmetic to under/overflow (such as a condition in an if statement), surrounding the arithmetic in an unchecked block will save gas
PROOF OF CONCEPT
Instances include:
CitadelMinter.sol
CitadelMinter.sol: 152: i bounded by numPools, overflow check unnecessary
CitadelMinter.sol: 344: i bounded by length, overflow check unnecessary
CitadelMinter.sol: 364: totalFundingPoolWeight is greater than or equal to currentPoolWeight, underflow check unnecessary
Funding.sol
Funding.sol: 212: MAX_BPS is less than the maximum discount, underflow check unnecessary
Funding.sol: 236: assetCumulativeFunded is less than assetCap, underflow check unnecessary
KnightingRound.sol
KnightingRound.sol: 198: totalTokenIn + _tokenInAmount is capped by tokenInLimit, overflow check unnecessary
KnightingRound.sol: 250: totalTokenIn is less than tokenInLimit, underflow check unnecessary
StakedCitadel.sol
StakedCitadel.sol: 817: b is less than r, underflow check unnecessary
StakedCitadel.sol: 821: b + diff capped by r (see line 821 and 817), overflow check unnecessary
StakedCitadel.sol: 827: _fee is less than r, underflow check unnecessary
SupplySchedule.sol
SupplySchedule.sol: 208: i is capped by endingEpoch, overflow check unnecessary
TOOLS USED
Manual Analysis
MITIGATION
Place the arithmetic operations in an unchecked block
Gas Report
Table of Contents
Caching storage variables in memory to save gas
PROBLEM
If you are reading from storage more than once, it is cheaper in gas cost to cache the variable in memory: a SLOAD cost 100gas, while MLOAD and MSTORE cost 3 gas.
PROOF OF CONCEPT
Instances include:
CitadelMinter.sol
scope:
mintAndDistribute()citadelTokenis read twiceFunding.sol
scope:
claimAssetToTreasury()assetis read 3 timesscope:
updateCitadelPriceInAsset()minCitadelPriceInAssetis read twicemaxCitadelPriceInAssetis read twiceKnightingRound.sol
scope:
buy()saleStartis read twicetotalTokenInis read twiceguestlistis read twiceStakedCitadel.sol
scope:
depositFor()tokenis read 3 timesscope:
_withdraw()tokenis read 3 timesscope:
_depositForWithAuthorization()guestlistis read twicescope:
setStrategy()strategyis read twicescope:
earn()strategyis read twicescope:
withdraw()vestingis read twiceSupplySchedule.sol
scope:
getMintableDebug()globalStartTimestampis read (5 + endingEpoch - lastEpoch) times: number of reads depending on lastMintTimestamp as it is in a for loopTOOLS USED
Manual Analysis
MITIGATION
cache these storage variables in memory
Calldata instead of memory for RO function parameters
PROBLEM
If a reference type function parameter is read-only, it is cheaper in gas to use calldata instead of memory. Calldata is a non-modifiable, non-persistent area where function arguments are stored, and behaves mostly like memory.
Try to use calldata as a data location because it will avoid copies and also makes sure that the data cannot be modified.
PROOF OF CONCEPT
Instances include:
CitadelToken.sol
scope:
initialize()GlobalAccessControl.sol
scope:
initializeNewRole()StakedCitadel.sol
scope:
initialize()scope:
deposit()scope:
depositAll()scope:
depositFor()scope:
_depositWithAuthorization()scope:
_depositForWithAuthorization()TOOLS USED
Manual Analysis
MITIGATION
Replace
memorywithcalldataComparisons with zero for unsigned integers
PROBLEM
>0is less gas efficient than!0if you enable the optimizer at 10k AND you’re in a require statement. Detailed explanation with the opcodes herePROOF OF CONCEPT
Instances include:
CitadelMinter.sol
scope:
_transferToFundingPools()Funding.sol
scope:
deposit()scope:
sweep()scope:
claimAssetToTreasury()scope:
updateCitadelPriceInAsset()scope:
updateCitadelPriceInAsset()KnightingRound.sol
scope:
function initialize()scope:
buy()scope:
claim()scope:
setSaleDuration()scope:
setTokenOutPrice()scope:
sweep()StakedCitadelVester.sol
scope:
vest()SupplySchedule.sol
scope:
getEpochAtTimestamp()scope:
getMintable()scope:
getMintableDebug()TOOLS USED
Manual Analysis
MITIGATION
Replace
> 0with!0Comparison Operators
PROBLEM
In the EVM, there is no opcode for >= or <=. When using greater than or equal, two operations are performed: > and =.
Using strict comparison operators hence saves gas
PROOF OF CONCEPT
Instances include:
CitadelMinter.sol
Funding.sol
KnightingRound.sol
StakedCitadel.sol
StakedCitadelVester.sol
SupplySchedule.sol
TOOLS USED
Manual Analysis
MITIGATION
Replace
<=with<, and>=with>. Do not forget to increment/decrement the compared variableexample:
When the comparison is with a constant storage variable, you can also do the increment in the storage variable declaration
example:
and
Custom Errors
PROBLEM
Custom errors from Solidity 0.8.4 are cheaper than revert strings (cheaper deployment cost and runtime cost when the revert condition is met) while providing the same amount of information, as explained here
Custom errors are defined using the error statement
PROOF OF CONCEPT
Instances include:
CitadelMinter.sol
Funding.sol
GlobalAccessControl.sol
KnightingRound.sol
StakedCitadel.sol
StakedCitadelVester.sol
SupplySchedule.sol
TOOLS USED
Manual Analysis
MITIGATION
Replace require statements with custom errors.
Default value initialization
PROBLEM
If a variable is not set/initialized, it is assumed to have the default value (0, false, 0x0 etc depending on the data type). Explicitly initializing it with its default value is an anti-pattern and wastes gas.
PROOF OF CONCEPT
Instances include:
CitadelMinter.sol
SupplySchedule.sol
TOOLS USED
Manual Analysis
MITIGATION
Remove explicit initialization for default values.
Event emitting of local variable
PROBLEM
When emitting an event, using a local variable instead of a storage variable saves gas.
PROOF OF CONCEPT
Instances include:
Funding.sol
SupplySchedule.sol
TOOLS USED
Manual Analysis
MITIGATION
In both instances, the storage variable is read multiple times in the function and it is recommended to cache them into memory, then passing these cached variables in the
emitstatement, as explained in the cache paragraphInline functions
PROBLEM
When we define internal functions to perform computation:
When it does not affect readability, it is recommended to inline functions in order to save gas
PROOF OF CONCEPT
Instances include:
StakedCitadel.sol
TOOLS USED
Manual Analysis
MITIGATION
Inline this function where it is called:
Prefix increments
PROBLEM
Prefix increments are cheaper than postfix increments.
PROOF OF CONCEPT
Instances include:
CitadelMinter.sol
SupplySchedule.sol
TOOLS USED
Manual Analysis
MITIGATION
change
i++to++iShort-circuiting
PROBLEM
Reading from storage costs 100 gas. When checking conditions using the AND operator, we can save gas by not evaluating the second expression if the first one is false.
PROOF OF CONCEPT
Instances include:
CitadelMinter.sol
TOOLS USED
Manual Analysis
MITIGATION
poolExistsline 261 and inline the computation offundingPools.contains(_pool)line 266 and 273.Tight Variable Packing
PROBLEM
Solidity contracts have contiguous 32 bytes (256 bits) slots used in storage. By arranging the variables, it is possible to minimize the number of slots used within a contract's storage and therefore reduce deployment costs.
address type variables are each of 20 bytes size (way less than 32 bytes). However, they here take up a whole 32 bytes slot (they are contiguous).
As bool type variables are of size 1 byte, there's a slot here that can get saved by moving them closer to an address
PROOF OF CONCEPT
Instances include:
Funding.sol
TOOLS USED
Manual Analysis
MITIGATION
Place
citadelPriceFlagafterSaleRecipientto save one storage slotUint256 instead of Uint8
PROBLEM
Each storage slot has 256 bits. Whenever a uint smaller than 256 (or even a bool) is pulled from storage, the EVM casts it to a uint256. Calculations are also unexceptionally performed in uint256 by the EVM.
PROOF OF CONCEPT
Instances include
KnightingRound.sol
TOOLS USED
Manual Analysis
MITIGATION
Replace
uint8withuint256Unchecked arithmetic
PROBLEM
The default "checked" behavior costs more gas when adding/diving/multiplying, because under-the-hood those checks are implemented as a series of opcodes that, prior to performing the actual arithmetic, check for under/overflow and revert if it is detected.
if it can statically be determined there is no possible way for your arithmetic to under/overflow (such as a condition in an if statement), surrounding the arithmetic in an
uncheckedblock will save gasPROOF OF CONCEPT
Instances include:
CitadelMinter.sol
Funding.sol
KnightingRound.sol
StakedCitadel.sol
SupplySchedule.sol
TOOLS USED
Manual Analysis
MITIGATION
Place the arithmetic operations in an
uncheckedblock