Platform: Code4rena
Start Date: 07/07/2023
Pot Size: $121,650 USDC
Total HM: 36
Participants: 111
Period: 7 days
Judge: Picodes
Total Solo HM: 13
Id: 258
League: ETH
Rank: 66/111
Findings: 2
Award: $40.22
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: bin2chen
Also found by: 0x11singh99, 0xWaitress, 0xbepresent, ABAIKUNANBAEV, ArmedGoose, Bauchibred, DadeKuma, GREY-HAWK-REACH, GalloDaSballo, Inspecktor, Jeiwan, Kaysoft, MohammedRizwan, Rolezn, Vagner, alexzoid, alymurtazamemon, ayden, banpaleo5, catellatech, dacian, erebus, eyexploit, fatherOfBlocks, grearlake, joaovwfreire, keccak123, kutugu, lanrebayode77, markus_ether, nadin, naman1778, rvierdiiev, squeaky_cactus, volodya, yixxas
15.9228 USDC - $15.92
Issue | Contexts | |
---|---|---|
LOW‑1 | Approve type(uint256).max not work with some tokens | 1 |
LOW‑2 | Array lengths not checked | 2 |
LOW‑3 | decimals() not part of ERC20 standard | 2 |
Total: 5 contexts over 3 issues
Issue | Contexts | |
---|---|---|
NC‑1 | Consider adding a deny-list | 1 |
NC‑2 | Consistent usage of require vs custom error | 20 |
NC‑3 | Generate perfect code headers for better solidity code layout and readability | 1 |
NC‑4 | Use delete to Clear Variables | 4 |
NC‑5 | Non-external /public function names should begin with an underscore | 38 |
NC‑6 | override function arguments that are unused should have the variable name removed or commented out to avoid compiler warnings | 2 |
NC‑7 | Empty blocks should be removed or emit something | 1 |
NC‑8 | Using underscore at the end of variable name | 13 |
NC‑9 | Use SMTChecker | 1 |
Total: 81 contexts over 9 issues
blacklist
functionNFT thefts have increased recently, so with the addition of hacked NFTs to the platform, NFTs can be converted into liquidity. To prevent this, I recommend adding the blacklist function.
Marketplaces such as Opensea have a blacklist feature that will not list NFTs that have been reported theft, NFT projects such as Manifold have blacklist functions in their smart contracts.
Here is the project example; Manifold
Manifold Contract https://etherscan.io/address/0xe4e4003afe3765aca8149a82fc064c0b125b9e5a#code
modifier nonBlacklistRequired(address extension) { require(!_blacklistedExtensions.contains(extension), "Extension blacklisted"); _; }
Add to Blacklist function and modifier.
type(uint256).max
not work with some tokensSource: https://github.com/d-xo/weird-erc20#revert-on-large-approvals--transfers
File: Vault.sol 677: _asset.safeApprove(address(liquidationPair_), type(uint256).max);
If the length of the arrays are not required to be of the same length, user operations may not be fully executed
File: Claimer.sol function claimPrizes( Vault vault, uint8 tier, address[] calldata winners, uint32[][] calldata prizeIndices, address _feeRecipient ) external returns (uint256 totalFees) {
File: Vault.sol function claimPrizes( uint8 _tier, address[] calldata _winners, uint32[][] calldata _prizeIndices, uint96 _feePerClaim, address _feeRecipient ) external returns (uint256) {
decimals()
not part of ERC20 standarddecimals()
is not part of the official ERC20 standard and might fail for tokens that do not implement it. While in practice it is very unlikely, as usually most of the tokens implement it, this should still be considered as a potential issue.
File: Vault.sol 279: _assetUnit = 10 ** super.decimals()
File: Vault.sol 341: return super.decimals()
Doing so will significantly increase centralization, but will help to prevent hackers from using stolen tokens
File: Vault.sol 110: contract Vault is ERC4626, ERC20Permit, ILiquidationSource, Ownable
Consider using the same approach throughout the codebase to improve the consistency of the code.
File: Vault.sol 266: if (address(twabController_) == address(0)) revert TwabControllerZeroAddress(); 267: if (address(yieldVault_) == address(0)) revert YieldVaultZeroAddress(); 268: if (address(prizePool_) == address(0)) revert PrizePoolZeroAddress(); 269: if (address(owner_) == address(0)) revert OwnerZeroAddress(); 396: if (_shares > _yieldFeeTotalSupply) revert YieldFeeGTAvailable(_shares, _yieldFeeTotalSupply); 409: revert DepositMoreThanMax(_receiver, _assets, maxDeposit(_receiver)); 515: revert WithdrawMoreThanMax(_owner, _assets, maxWithdraw(_owner)); 529: if (_shares > maxRedeem(_owner)) revert RedeemMoreThanMax(_owner, _shares, maxRedeem(_owner)); 559: revert LiquidationCallerNotLP(msg.sender, address(_liquidationPair)); 561: revert LiquidationTokenInNotPrizeToken(_tokenIn, address(_prizePool.prizeToken())); 563: revert LiquidationTokenOutNotVaultShare(_tokenOut, address(this)); 564: if (_amountOut == 0) revert LiquidationAmountOutZero(); 568: revert LiquidationAmountOutGTYield(_amountOut, _liquidableYield); 591: if (_token != _liquidationPair.tokenIn()) revert TargetTokenNotSupported(_token); 614: if (msg.sender != _claimer) revert CallerNotClaimer(msg.sender, _claimer); 668: if (address(liquidationPair_) == address(0)) revert LPZeroAddress(); 830: if (_token != address(this)) revert LiquidationTokenOutNotVaultShare(_token, address(this)); 972: if (_shares > maxMint(_receiver)) revert MintMoreThanMax(_receiver, _shares, maxMint(_receiver)); 1200: if (!_isVaultCollateralized()) revert VaultUnderCollateralized(); 1220: revert YieldFeePercentageGTPrecision(yieldFeePercentage_, FEE_PRECISION);
It is recommended to use pre-made headers for Solidity code layout and readability: https://github.com/transmissions11/headers
/*////////////////////////////////////////////////////////////// TESTING 123 //////////////////////////////////////////////////////////////*/
File: Vault.sol 4: Vault.sol
delete
to Clear Variablesdelete a
assigns the initial value for the type to a
. i.e. for integers it is equivalent to a = 0
, but it can also be used on arrays, where it assigns a dynamic array of length zero or a static array of the same length with all elements reset. For structs, it assigns a struct with all members reset. Similarly, it can also be used to set an address to zero address. It has no effect on whole mappings though (as the keys of mappings may be arbitrary and are generally unknown). However, individual keys and what they map to can be deleted: If a
is a mapping, then delete a[x]
will delete the value stored at x
.
The delete
key better conveys the intention and is also more idiomatic. Consider replacing assignments of zero with delete
statements.
File: PrizePool.sol 369: claimCount = 0; 370: canaryClaimCount = 0; 371: largestTierClaimed = 0;
File: TwabLib.sol 229: index = 0;
external
/public
function names should begin with an underscoreAccording to the Solidity Style Guide, Non-external
/public
function names should begin with an <a href="https://docs.soliditylang.org/en/latest/style-guide.html#underscore-prefix-for-non-external-functions-and-variables">underscore</a>
File: LinearVRGDALib.sol 16: function getDecayConstant(UD2x18 _priceDeltaScale) internal pure returns (SD59x18) {
File: LinearVRGDALib.sol 24: function getPerTimeUnit( uint256 _count, uint256 _durationSeconds ) internal pure returns (SD59x18) {
File: LinearVRGDALib.sol 39: function getVRGDAPrice( uint256 _targetPrice, uint256 _timeSinceStart, uint256 _sold, SD59x18 _perTimeUnit, SD59x18 _decayConstant ) internal pure returns (uint256) {
File: LinearVRGDALib.sol 102: function getMaximumPriceDeltaScale( uint256 _minFee, uint256 _maxFee, uint256 _time ) internal pure returns (UD2x18) {
File: DrawAccumulatorLib.sol 64: function add( Accumulator storage accumulator, uint256 _amount, uint16 _drawId, SD59x18 _alpha ) internal returns (bool) {
File: DrawAccumulatorLib.sol 120: function getTotalRemaining( Accumulator storage accumulator, uint16 _startDrawId, SD59x18 _alpha ) internal view returns (uint256) {
File: DrawAccumulatorLib.sol 141: function newestDrawId(Accumulator storage accumulator) internal view returns (uint256) {
File: DrawAccumulatorLib.sol 151: function newestObservation( Accumulator storage accumulator ) internal view returns (Observation memory) {
File: DrawAccumulatorLib.sol 166: function getDisbursedBetween( Accumulator storage _accumulator, uint16 _startDrawId, uint16 _endDrawId, SD59x18 _alpha ) internal view returns (uint256) {
File: DrawAccumulatorLib.sol 342: function computeIndices( RingBufferInfo memory ringBufferInfo ) internal pure returns (Pair32 memory) {
File: DrawAccumulatorLib.sol 364: function readDrawIds( Accumulator storage accumulator, Pair32 memory indices ) internal view returns (Pair32 memory) {
File: DrawAccumulatorLib.sol 380: function integrateInf(SD59x18 _alpha, uint _x, uint _k) internal pure returns (uint256) {
File: DrawAccumulatorLib.sol 390: function integrate( SD59x18 _alpha, uint _start, uint _end, uint _k ) internal pure returns (uint256) {
File: DrawAccumulatorLib.sol 406: function computeC(SD59x18 _alpha, uint _x, uint _k) internal pure returns (SD59x18) {
File: DrawAccumulatorLib.sol 420: function binarySearch( uint16[366] storage _drawRingBuffer, uint16 _oldestIndex, uint16 _newestIndex, uint16 _cardinality, uint16 _targetLastClosedDrawId ) internal view returns ( uint16 beforeOrAtIndex, uint16 beforeOrAtDrawId, uint16 afterOrAtIndex, uint16 afterOrAtDrawId ) {
File: TierCalculationLib.sol 17: function getTierOdds( uint8 _tier, uint8 _numberOfTiers, uint16 _grandPrizePeriod ) internal pure returns (SD59x18) {
File: TierCalculationLib.sol 32: function estimatePrizeFrequencyInDraws(SD59x18 _tierOdds) internal pure returns (uint256) {
File: TierCalculationLib.sol 39: function prizeCount(uint8 _tier) internal pure returns (uint256) {
File: TierCalculationLib.sol 52: function canaryPrizeCount( uint8 _numberOfTiers, uint8 _canaryShares, uint8 _reserveShares, uint8 _tierShares ) internal pure returns (UD60x18) {
File: TierCalculationLib.sol 73: function isWinner( uint256 _userSpecificRandomNumber, uint128 _userTwab, uint128 _vaultTwabTotalSupply, SD59x18 _vaultContributionFraction, SD59x18 _tierOdds ) internal pure returns (bool) {
File: TierCalculationLib.sol 104: function calculatePseudoRandomNumber( address _user, uint8 _tier, uint32 _prizeIndex, uint256 _winningRandomNumber ) internal pure returns (uint256) {
File: TierCalculationLib.sol 118: function calculateWinningZone( uint256 _userTwab, SD59x18 _vaultContributionFraction, SD59x18 _tierOdds ) internal pure returns (uint256) {
File: TierCalculationLib.sol 133: function estimatedClaimCount( uint8 _numberOfTiers, uint16 _grandPrizePeriod ) internal pure returns (uint32) {
File: ObservationLib.sol 55: function binarySearch( Observation[MAX_CARDINALITY] storage _observations, uint24 _newestObservationIndex, uint24 _oldestObservationIndex, uint32 _target, uint16 _cardinality, uint32 _time ) internal view returns (Observation memory beforeOrAt, Observation memory afterOrAt) {
File: OverflowSafeComparatorLib.sol 15: function lt(uint32 _a, uint32 _b, uint32 _timestamp) internal pure returns (bool) {
File: OverflowSafeComparatorLib.sol 31: function lte(uint32 _a, uint32 _b, uint32 _timestamp) internal pure returns (bool) {
File: OverflowSafeComparatorLib.sol 47: function checkedSub(uint32 _a, uint32 _b, uint32 _timestamp) internal pure returns (uint32) {
File: TwabLib.sol 75: function increaseBalances( uint32 PERIOD_LENGTH, uint32 PERIOD_OFFSET, Account storage _account, uint96 _amount, uint96 _delegateAmount ) internal returns (ObservationLib.Observation memory observation, bool isNew, bool isObservationRecorded) {
File: TwabLib.sol 144: function decreaseBalances( uint32 PERIOD_LENGTH, uint32 PERIOD_OFFSET, Account storage _account, uint96 _amount, uint96 _delegateAmount, string memory _revertMessage ) internal returns (ObservationLib.Observation memory observation, bool isNew, bool isObservationRecorded) {
File: TwabLib.sol 223: function getOldestObservation( ObservationLib.Observation[MAX_CARDINALITY] storage _observations, AccountDetails memory _accountDetails ) internal view returns (uint16 index, ObservationLib.Observation memory observation) {
File: TwabLib.sol 244: function getNewestObservation( ObservationLib.Observation[MAX_CARDINALITY] storage _observations, AccountDetails memory _accountDetails ) internal view returns (uint16 index, ObservationLib.Observation memory observation) {
File: TwabLib.sol 263: function getBalanceAt( uint32 PERIOD_OFFSET, ObservationLib.Observation[MAX_CARDINALITY] storage _observations, AccountDetails memory _accountDetails, uint32 _targetTime ) internal view returns (uint256) {
File: TwabLib.sol 288: function getTwabBetween( uint32 PERIOD_OFFSET, ObservationLib.Observation[MAX_CARDINALITY] storage _observations, AccountDetails memory _accountDetails, uint32 _startTime, uint32 _endTime ) internal view returns (uint256) {
File: TwabLib.sol 418: function getTimestampPeriod( uint32 PERIOD_LENGTH, uint32 PERIOD_OFFSET, uint32 _timestamp ) internal pure returns (uint32 period) {
File: TwabLib.sol 456: function getPreviousOrAtObservation( uint32 PERIOD_OFFSET, ObservationLib.Observation[MAX_CARDINALITY] storage _observations, AccountDetails memory _accountDetails, uint32 _targetTime ) internal view returns (ObservationLib.Observation memory prevOrAtObservation) {
File: TwabLib.sol 544: function getNextOrNewestObservation( uint32 PERIOD_OFFSET, ObservationLib.Observation[MAX_CARDINALITY] storage _observations, AccountDetails memory _accountDetails, uint32 _targetTime ) internal view returns (ObservationLib.Observation memory nextOrNewestObservation) {
File: TwabLib.sol 663: function isTimeSafe( uint32 PERIOD_LENGTH, uint32 PERIOD_OFFSET, ObservationLib.Observation[MAX_CARDINALITY] storage _observations, AccountDetails memory _accountDetails, uint32 _time ) internal view returns (bool) {
</details>File: TwabLib.sol 743: function isTimeRangeSafe( uint32 PERIOD_LENGTH, uint32 PERIOD_OFFSET, ObservationLib.Observation[MAX_CARDINALITY] storage _observations, AccountDetails memory _accountDetails, uint32 _startTime, uint32 _endTime ) internal view returns (bool) {
override
function arguments that are unused should have the variable name removed or commented out to avoid compiler warningsFile: Vault.sol 375: function maxDeposit : address
File: Vault.sol 383: function maxMint : address
File: Vault.sol 7: constructor( IERC20 _asset, string memory _name, string memory _symbol, TwabController twabController_, IERC4626 yieldVault_, PrizePool prizePool_, address claimer_, address yieldFeeRecipient_, uint256 yieldFeePercentage_, address _owner ) Vault( _asset, _name, _symbol, twabController_, yieldVault_, prizePool_, claimer_, yieldFeeRecipient_, yieldFeePercentage_, _owner ) {}
The use of underscore at the end of the variable name is uncommon and also suggests that the variable name was not completely changed. Consider refactoring variableName_
to variableName
.
File: PrizePool.sol 368: winningRandomNumber_; 372: openDrawStartedAt_;
File: DrawAccumulatorLib.sol 84: newestDrawId_;
File: Vault.sol 271: twabController_; 272: yieldVault_; 273: prizePool_; 646: claimer_; 1210: claimer_; 679: liquidationPair_; 696: yieldFeePercentage_; 1222: yieldFeePercentage_; 709: yieldFeeRecipient_; 1230: yieldFeeRecipient_;
The highest tier of smart contract behavior assurance is formal mathematical verification. All assertions that are made are guaranteed to be true across all inputs → The quality of your asserts is the quality of your verification
https://twitter.com/0xOwenThurm/status/1614359896350425088?t=dbG9gHFigBX85Rv29lOjIQ&s=19
#0 - c4-judge
2023-07-18T19:16:08Z
Picodes marked the issue as grade-b
#1 - PierrickGT
2023-09-07T23:41:20Z
LOW-1, 3: has been removed LOW-2: no need to check array length
NC-1: the protocol is permissionless, we use tokenlists in the front-end to vet Vaults but don't enforce it at the Solidity level NC-2: has been fixed NC-3: useless recommandation NC-4: we would lose in code legibility NC-5: no need to prefix internal functions with an underscore for libraries NC-6: has been fixed NC-7: this is a mock contract NC-8: not uncommon at all NC-9: we are already using invariant and fuzz tests
🌟 Selected for report: c3phas
Also found by: 0x11singh99, 0xAnah, 0xn006e7, LeoS, Rageur, Raihan, ReyAdmirado, Rolezn, SAAJ, SAQ, SM3_SS, Udsen, alymurtazamemon, hunter_w3b, koxuan, naman1778, petrichor, ybansal2403
24.2984 USDC - $24.30
Issue | Contexts | Estimated Gas Saved | |
---|---|---|---|
GAS‑1 | abi.encode() is less efficient than abi.encodepacked() | 1 | 53 |
GAS‑2 | Use assembly to emit events | 26 | 988 |
GAS‑3 | Counting down in for statements is more gas efficient | 7 | 1799 |
GAS‑4 | Use assembly to write address storage values | 22 | 1628 |
GAS‑5 | Multiple accesses of a mapping/array should use a local variable cache | 9 | 720 |
GAS‑6 | Remove forge-std import | 1 | 100 |
GAS‑7 | The result of a function call should be cached rather than re-calling the function | 12 | 600 |
GAS‑8 | Superfluous event fields | 1 | 34 |
GAS‑9 | Use nested if and avoid multiple check combinations | 12 | 72 |
GAS‑10 | Using XOR (^) and AND (&) bitwise equivalents | 196 | 2548 |
Total: 301 contexts over 10 issues
abi.encode()
is less efficient than abi.encodepacked()
See for more information: https://github.com/ConnorBlockchain/Solidity-Encode-Gas-Comparison
File: TierCalculationLib.sol 110: return uint256(keccak256(abi.encode(_user, _tier, _prizeIndex, _winningRandomNumber)));
contract GasTest is DSTest { Contract0 c0; Contract1 c1; function setUp() public { c0 = new Contract0(); c1 = new Contract1(); } function testGas() public { c0.not_optimized(); c1.optimized(); } } contract Contract0 { string a = "Code4rena"; function not_optimized() public returns(bytes32){ return keccak256(abi.encode(a)); } } contract Contract1 { string a = "Code4rena"; function optimized() public returns(bytes32){ return keccak256(abi.encodePacked(a)); } }
Contract0 contract | |||||
---|---|---|---|---|---|
Deployment Cost | Deployment Size | ||||
101871 | 683 | ||||
Function Name | min | avg | median | max | # calls |
not_optimized | 2661 | 2661 | 2661 | 2661 | 1 |
Contract1 contract | |||||
---|---|---|---|---|---|
Deployment Cost | Deployment Size | ||||
99465 | 671 | ||||
Function Name | min | avg | median | max | # calls |
optimized | 2608 | 2608 | 2608 | 2608 | 1 |
We can use assembly to emit events efficiently by utilizing scratch space
and the free memory pointer
. This will allow us to potentially avoid memory expansion costs.
Note: In order to do this optimization safely, we will need to cache and restore the free memory pointer.
For example, for a generic emit
event for eventSentAmountExample
:
// uint256 id, uint256 value, uint256 amount emit eventSentAmountExample(id, value, amount);
We can use the following assembly emit events:
assembly { let memptr := mload(0x40) mstore(0x00, calldataload(0x44)) mstore(0x20, calldataload(0xa4)) mstore(0x40, amount) log1( 0x00, 0x60, // keccak256("eventSentAmountExample(uint256,uint256,uint256)") 0xa622cf392588fbf2cd020ff96b2f4ebd9c76d7a4bc7f3e6b2f18012312e76bc3 ) mstore(0x40, memptr) }
File: PrizePool.sol 305: emit DrawManagerSet(_drawManager);
File: PrizePool.sol 328: emit ContributePrizeTokens(_prizeVault, lastClosedDrawId + 1, _amount);
File: PrizePool.sol 341: emit WithdrawReserve(_to, _amount);
File: PrizePool.sol 454: emit IncreaseClaimRewards(_feeRecipient, _fee);
File: PrizePool.sol 492: emit WithdrawClaimRewards(_to, _amount, _available);
File: PrizePool.sol 501: emit IncreaseReserve(msg.sender, _amount);
File: TieredLiquidityDistributor.sol 539: emit ReserveConsumed(excess);
File: TwabController.sol 663: emit Delegated(_vault, _from, _to);
File: TwabController.sol 688: emit IncreasedBalance(_vault, _user, _amount, _delegateAmount);
File: TwabController.sol 731: emit DecreasedBalance(_vault, _user, _amount, _delegateAmount);
File: TwabController.sol 773: emit DecreasedTotalSupply(_vault, _amount, _delegateAmount);
File: TwabController.sol 807: emit IncreasedTotalSupply(_vault, _amount, _delegateAmount);
File: Vault.sol 401: emit MintYieldFee(msg.sender, _recipient, _shares);
File: Vault.sol 645: emit ClaimerSet(_previousClaimer, claimer_);
File: Vault.sol 656: emit SetHooks(msg.sender, hooks);
File: Vault.sol 681: emit LiquidationPairSet(liquidationPair_);
File: Vault.sol 695: emit YieldFeePercentageSet(_previousYieldFeePercentage, yieldFeePercentage_);
File: Vault.sol 708: emit YieldFeeRecipientSet(_previousYieldFeeRecipient, yieldFeeRecipient_);
File: Vault.sol 962: emit Deposit(_caller, _receiver, _assets, _shares);
File: Vault.sol 991: emit Sponsor(msg.sender, _receiver, _assets, _shares);
File: Vault.sol 1029: emit Withdraw(_caller, _receiver, _owner, _assets, _shares);
File: Vault.sol 1111: emit RecordedExchangeRate(_lastRecordedExchangeRate);
File: Vault.sol 1126: emit Transfer(address(0), _receiver, _shares);
File: Vault.sol 1142: emit Transfer(_owner, address(0), _shares);
File: Vault.sol 1157: emit Transfer(_from, _to, _shares);
</details>File: VaultFactory.sol 83: emit NewFactoryVault(_vault, VaultFactory(address(this)));
for
statements is more gas efficientCounting down is more gas efficient than counting up because neither we are making zero variable to non-zero variable and also we will get gas refund in the last transaction when making non-zero to zero variable.
File: Claimer.sol 68: for (uint i = 0; i < winners.length; i++) {
File: Claimer.sol 144: for (uint i = 0; i < _claimCount; i++) {
File: TieredLiquidityDistributor.sol 361: for (uint8 i = start; i < end; i++) {
File: TieredLiquidityDistributor.sol 630: for (uint8 i = start; i < end; i++) {
File: TierCalculationLib.sol 138: for (uint8 i = 0; i < _numberOfTiers; i++) {
</details>File: Vault.sol 618: for (uint w = 0; w < _winners.length; w++) { 620: for (uint p = 0; p < prizeIndicesLength; p++) {
contract GasTest is DSTest { Contract0 c0; Contract1 c1; function setUp() public { c0 = new Contract0(); c1 = new Contract1(); } function testGas() public { c0.AddNum(); c1.AddNum(); } } contract Contract0 { uint256 num = 3; function AddNum() public { uint256 _num = num; for(uint i=0;i<=9;i++){ _num = _num +1; } num = _num; } } contract Contract1 { uint256 num = 3; function AddNum() public { uint256 _num = num; for(uint i=9;i>=0;i--){ _num = _num +1; } num = _num; } }
Contract0 contract | |||||
---|---|---|---|---|---|
Deployment Cost | Deployment Size | ||||
77011 | 311 | ||||
Function Name | min | avg | median | max | # calls |
AddNum | 7040 | 7040 | 7040 | 7040 | 1 |
Contract1 contract | |||||
---|---|---|---|---|---|
Deployment Cost | Deployment Size | ||||
73811 | 295 | ||||
Function Name | min | avg | median | max | # calls |
AddNum | 3819 | 3819 | 3819 | 3819 | 1 |
assembly
to write address storage valuesFile: PrizePool.sol 357: _numTiers = numberOfTiers;
File: PrizePool.sol 608: _numTiers = numberOfTiers;
File: PrizePool.sol 358: _nextNumberOfTiers = _numTiers;
File: PrizePool.sol 609: _nextNumberOfTiers = _numTiers;
File: PrizePool.sol 368: _winningRandomNumber = winningRandomNumber_;
File: PrizePool.sol 372: _lastClosedDrawStartedAt = openDrawStartedAt_;
File: PrizePool.sol 683: _numberOfTiers = numberOfTiers;
File: PrizePool.sol 851: _numberOfTiers = numberOfTiers;
File: PrizePool.sol 782: _claimExpansionThreshold = claimExpansionThreshold;
File: TieredLiquidityDistributor.sol 473: _lastClosedDrawId = lastClosedDrawId;
File: TieredLiquidityDistributor.sol 647: _numTiers = numberOfTiers;
File: TwabController.sol 598: _userDelegate = _user;
File: Vault.sol 271: _twabController = twabController_;
File: Vault.sol 272: _yieldVault = yieldVault_;
File: Vault.sol 273: _prizePool = prizePool_;
File: Vault.sol 642: _previousClaimer = _claimer;
File: Vault.sol 679: _liquidationPair = liquidationPair_;
File: Vault.sol 692: _previousYieldFeePercentage = _yieldFeePercentage;
File: Vault.sol 705: _previousYieldFeeRecipient = _yieldFeeRecipient;
File: Vault.sol 1210: _claimer = claimer_;
File: Vault.sol 1222: _yieldFeePercentage = yieldFeePercentage_;
File: Vault.sol 1230: _yieldFeeRecipient = yieldFeeRecipient_;
Caching a mapping's value in a local storage or calldata variable when the value is accessed multiple times saves ~42 gas per access due to not having to perform the same offset calculation every time. Help the Optimizer by saving a storage variable's reference instead of repeatedly fetching it
To help the optimizer,declare a storage type variable and use it instead of repeatedly fetching the reference in a map or an array. As an example, instead of repeatedly calling someMap[someIndex]
, save its reference like this: SomeStruct storage someStruct = someMap[someIndex]
and use it.
File: PrizePool.sol 434: if (claimedPrizes[msg.sender][_winner][lastClosedDrawId][_tier][_prizeIndex]) { 438: claimedPrizes[msg.sender][_winner][lastClosedDrawId][_tier][_prizeIndex] = true;
File: PrizePool.sol 484: uint256 _available = claimerRewards[msg.sender]; 490: claimerRewards[msg.sender] -= _amount;
File: DrawAccumulatorLib.sol 82: Observation memory newestObservation_ = accumulator.observations[newestDrawId_]; 134: Observation memory newestObservation_ = accumulator.observations[newestDrawId_]; 106: accumulator.observations[newestDrawId_] = Observation({
File: TwabLib.sol 698: period != _getTimestampPeriod(PERIOD_LENGTH, PERIOD_OFFSET, _observations[0].timestamp) 700: : _time >= _observations[0].timestamp;
forge-std
It's used to print the values of variables while running tests to help debug and see what's happening inside your contracts but since it's a development tool, it serves no purpose on mainnet.
Also, the remember to remove the usage of calls that use forge-std
when removing of the import of forge-std
.
File: PrizePool.sol 4: import "forge-std/console2.sol";
External calls are expensive. Results of external function calls should be cached rather than call them multiple times. Consider caching the following:
File: PrizePool.sol 320: smoothing.intoSD59x18() 326: smoothing.intoSD59x18() 529: smoothing.intoSD59x18() 545: smoothing.intoSD59x18() 715: smoothing.intoSD59x18() 728: smoothing.intoSD59x18() 821: smoothing.intoSD59x18() 907: smoothing.intoSD59x18()
File: PrizePool.sol 353: if (block.timestamp < _openDrawEndsAt()) { 354: revert DrawNotFinished(_openDrawEndsAt(), uint64(block.timestamp));
File: Vault.sol 560: if (_tokenIn != address(_prizePool.prizeToken())) 561: revert LiquidationTokenInNotPrizeToken(_tokenIn, address(_prizePool.prizeToken()));
block.number
and block.timestamp
are added to the event information by default, so adding them manually will waste additional gas.
File: Vault.sol 191: event Sponsor(address indexed caller, address indexed receiver, uint256 assets, uint256 shares);
if
and avoid multiple check combinationsUsing nested if
, is cheaper than using &&
multiple check combinations. There are more advantages, such as easier to read code and better coverage reports.
File: DrawAccumulatorLib.sol 454: if (targetAtOrAfter && _targetLastClosedDrawId <= afterOrAtDrawId) {
File: TwabController.sol 530: if (!_isFromDelegate && _fromDelegate != SPONSORSHIP_ADDRESS) { 561: if (!_isToDelegate && _toDelegate != SPONSORSHIP_ADDRESS) {
File: TwabController.sol 619: if (_fromDelegate != address(0) && _fromDelegate != SPONSORSHIP_ADDRESS) { 630: if (_toDelegate != address(0) && _toDelegate != SPONSORSHIP_ADDRESS) {
File: ObservationLib.sol 88: if (targetAfterOrAt && _target.lte(afterOrAt.timestamp, _time)) {
File: OverflowSafeComparatorLib.sol 17: if (_a <= _timestamp && _b <= _timestamp) return _a < _b;
File: OverflowSafeComparatorLib.sol 33: if (_a <= _timestamp && _b <= _timestamp) return _a <= _b;
File: OverflowSafeComparatorLib.sol 50: if (_a <= _timestamp && _b <= _timestamp) return _a - _b;
File: Vault.sol 325: if (_availableYield != 0 && _yieldFeePercentage != 0) {
File: Vault.sol 580: if (_vaultAssets != 0 && _amountOut >= _vaultAssets) {
</details>File: Vault.sol 1182: if (_totalSupplyAmount != 0 && _withdrawableAssets != 0) {
contract GasTest is DSTest { Contract0 c0; Contract1 c1; function setUp() public { c0 = new Contract0(); c1 = new Contract1(); } function testGas() public { c0.checkAge(19); c1.checkAgeOptimized(19); } } contract Contract0 { function checkAge(uint8 _age) public returns(string memory){ if(_age>18 && _age<22){ return "Eligible"; } } } contract Contract1 { function checkAgeOptimized(uint8 _age) public returns(string memory){ if(_age>18){ if(_age<22){ return "Eligible"; } } } }
Contract0 contract | |||||
---|---|---|---|---|---|
Deployment Cost | Deployment Size | ||||
76923 | 416 | ||||
Function Name | min | avg | median | max | # calls |
checkAge | 651 | 651 | 651 | 651 | 1 |
Contract1 contract | |||||
---|---|---|---|---|---|
Deployment Cost | Deployment Size | ||||
76323 | 413 | ||||
Function Name | min | avg | median | max | # calls |
checkAgeOptimized | 645 | 645 | 645 | 645 | 1 |
Given 4 variables a, b, c and d represented as such:
0 0 0 0 0 1 1 0 <- a 0 1 1 0 0 1 1 0 <- b 0 0 0 0 0 0 0 0 <- c 1 1 1 1 1 1 1 1 <- d
To have a == b means that every 0 and 1 match on both variables. Meaning that a XOR (operator ^) would evaluate to 0 ((a ^ b) == 0), as it excludes by definition any equalities.Now, if a != b, this means that there’s at least somewhere a 1 and a 0 not matching between a and b, making (a ^ b) != 0.Both formulas are logically equivalent and using the XOR bitwise operator costs actually the same amount of gas.However, it is much cheaper to use the bitwise OR operator (|) than comparing the truthy or falsy values.These are logically equivalent too, as the OR bitwise operator (|) would result in a 1 somewhere if any value is not 0 between the XOR (^) statements, meaning if any XOR (^) statement verifies that its arguments are different.
File: Claimer.sol 134: if (_claimCount == 0) {
File: LinearVRGDALib.sol 75: if (expResult == 0) {
File: PrizePool.sol 350: if (winningRandomNumber_ == 0) {
File: PrizePool.sol 763: (lastClosedDrawId == 0 ? 1 : 2) *
File: PrizePool.sol 896: if (_lastClosedDrawId == 0) {
File: TieredLiquidityDistributor.sol 605: return _tier == _numberOfTiers - 1;
File: TieredLiquidityDistributor.sol 731: if (numTiers == 3) { 733: } else if (numTiers == 4) { 735: } else if (numTiers == 5) { 737: } else if (numTiers == 6) { 739: } else if (numTiers == 7) { 741: } else if (numTiers == 8) { 743: } else if (numTiers == 9) { 745: } else if (numTiers == 10) { 747: } else if (numTiers == 11) { 749: } else if (numTiers == 12) { 751: } else if (numTiers == 13) { 753: } else if (numTiers == 14) { 755: } else if (numTiers == 15) {
File: TieredLiquidityDistributor.sol 765: if (numTiers == 3) { 767: } else if (numTiers == 4) { 769: } else if (numTiers == 5) { 771: } else if (numTiers == 6) { 773: } else if (numTiers == 7) { 775: } else if (numTiers == 8) { 777: } else if (numTiers == 9) { 779: } else if (numTiers == 10) { 781: } else if (numTiers == 11) { 783: } else if (numTiers == 12) { 785: } else if (numTiers == 13) { 787: } else if (numTiers == 14) { 789: } else if (numTiers == 15) {
File: DrawAccumulatorLib.sol 70: if (_drawId == 0) {
File: DrawAccumulatorLib.sol 126: if (ringBufferInfo.cardinality == 0) {
File: DrawAccumulatorLib.sol 178: if (ringBufferInfo.cardinality == 0) {
File: TierCalculationLib.sol 80: if (_vaultTwabTotalSupply == 0) {
File: TwabController.sol 516: if (_from == _to) { 524: bool _isFromDelegate = _fromDelegate == _from; 538: _to == address(0) || 539: (_toDelegate == SPONSORSHIP_ADDRESS && _fromDelegate != SPONSORSHIP_ADDRESS) 544: _to == address(0) ? _amount : 0, 545: (_to == address(0) && _fromDelegate != SPONSORSHIP_ADDRESS) || 539: (_toDelegate == SPONSORSHIP_ADDRESS && _fromDelegate != SPONSORSHIP_ADDRESS) 555: bool _isToDelegate = _toDelegate == _to; 569: _from == address(0) || 570: (_fromDelegate == SPONSORSHIP_ADDRESS && _toDelegate != SPONSORSHIP_ADDRESS)
File: TwabController.sol 597: if (_userDelegate == address(0)) {
File: TwabController.sol 624: if (_toDelegate == address(0) || _toDelegate == SPONSORSHIP_ADDRESS) { 635: if (_fromDelegate == address(0) || _fromDelegate == SPONSORSHIP_ADDRESS) {
File: TwabController.sol 650: if (_to == _currentDelegate) {
File: ObservationLib.sol 78: if (beforeOrAtTimestamp == 0) {
File: TwabLib.sol 367: if (newestObservation.timestamp == currentTime) { 381: if (currentPeriod == 0 || currentPeriod > newestObservationPeriod) {
File: TwabLib.sol 485: if (_accountDetails.cardinality == 0) { 497: if (_accountDetails.cardinality == 1) { 529: if (afterOrAtObservation.timestamp == _targetTime) {
File: TwabLib.sol 572: if (_accountDetails.cardinality == 0) { 587: if (_accountDetails.cardinality == 1) {
File: TwabLib.sol 690: if (_accountDetails.cardinality == 0) { 696: if (_accountDetails.cardinality == 1) {
File: Vault.sol 564: if (_amountOut == 0) revert LiquidationAmountOutZero();
File: Vault.sol 871: (_assets == 0 || _exchangeRate == 0)
</details>File: Vault.sol 902: (_shares == 0 || _exchangeRate == 0)
contract GasTest is DSTest { Contract0 c0; Contract1 c1; function setUp() public { c0 = new Contract0(); c1 = new Contract1(); } function testGas() public { c0.not_optimized(1,2); c1.optimized(1,2); } } contract Contract0 { function not_optimized(uint8 a,uint8 b) public returns(bool){ return ((a==1) || (b==1)); } } contract Contract1 { function optimized(uint8 a,uint8 b) public returns(bool){ return ((a ^ 1) & (b ^ 1)) == 0; } }
Contract0 contract | |||||
---|---|---|---|---|---|
Deployment Cost | Deployment Size | ||||
46099 | 261 | ||||
Function Name | min | avg | median | max | # calls |
not_optimized | 456 | 456 | 456 | 456 | 1 |
Contract1 contract | |||||
---|---|---|---|---|---|
Deployment Cost | Deployment Size | ||||
42493 | 243 | ||||
Function Name | min | avg | median | max | # calls |
optimized | 430 | 430 | 430 | 430 | 1 |
#0 - c4-judge
2023-07-18T19:00:30Z
Picodes marked the issue as grade-b
#1 - PierrickGT
2023-09-07T23:31:53Z
GAS-1: we can't use abi.encodePacked
GAS-2, 3, 4: we would lose in code legibility
GAS-5: irrelevant, values are being updated
GAS-6, 7: has been fixed
GAS-8: irrelevant suggestion
GAS-9, 10: we would lose in code legibility