Platform: Code4rena
Start Date: 25/01/2022
Pot Size: $50,000 USDT
Total HM: 17
Participants: 39
Period: 3 days
Judge: LSDan
Total Solo HM: 9
Id: 79
League: ETH
Rank: 4/39
Findings: 3
Award: $2,523.08
🌟 Selected for report: 24
🚀 Solo Findings: 0
🌟 Selected for report: cccz
Also found by: 0x1f8b, Dravee, TomFrenchBlockchain, UncleGrandpa925, WatchPug, bobi, byterocket, hack3r-0m, sirhashalot
Dravee
Silent failures (lack of failure detection / revert in case of failure).
ERC20 transfer()/transferFrom() do not return booleans: Contracts compiled with solc >= 0.4.22 interacting with such functions will revert. Use OpenZeppelin's SafeERC20 (wrappers around ERC20 operations that throw on failure when the token contract implementation returns false. Tokens that return no value and instead revert or throw on failure are also supported with non-reverting calls assumed to be successful. Adds safeTransfer, safeTransferFrom, safeApprove, safeDecreaseAllowance, and safeIncreaseAllowance)
Instances include:
contracts\LaunchEvent.sol:458: token.transfer(msg.sender, amount); contracts\LaunchEvent.sol:464: pair.transfer(msg.sender, balance); contracts\LaunchEvent.sol:490: token.transfer(msg.sender, amount); contracts\LaunchEvent.sol:514: token.transfer(issuer, balance); contracts\LaunchEvent.sol:538: token.transfer(penaltyCollector, excessToken); contracts\LaunchEvent.sol:543: WAVAX.transfer(penaltyCollector, excessWavax); contracts\RocketJoeFactory.sol:133: IERC20(_token).transferFrom(msg.sender, launchEvent, _tokenAmount); contracts\RocketJoeStaking.sol:184: rJoe.transfer(_to, rJoeBal); contracts\RocketJoeStaking.sol:186: rJoe.transfer(_to, _amount);
While some of those are known tokens, several others are arbitrary ERC20 tokens. safeTransfer/safeTransferFrom would be a good practice here
VS Code
Consider using safeTransfer/safeTransferFrom consistently.
#0 - cryptofish7
2022-02-10T21:45:59Z
Duplicate of #12
#1 - dmvt
2022-02-22T10:50:00Z
This could result in a loss of funds given the right external conditions.
2 — Med (M): vulns have a risk of 2 and are considered “Medium” severity when assets are not at direct risk, but the function of the protocol or its availability could be impacted, or leak value with a hypothetical attack path with stated assumptions, but external requirements.
🌟 Selected for report: Dravee
518.9746 USDT - $518.97
Dravee
A division by 0 could occur
There are no checks that the denominator is != 0 here:
File: LaunchEvent.sol 392: uint256 tokenAllocated = tokenReserve; 393: 394: // Adjust the amount of tokens sent to the pool if floor price not met 395: if ( 396: floorPrice > (wavaxReserve * 10**token.decimals()) / tokenAllocated 397: ) {
tokenReserve (uint256 tokenAllocated = tokenReserve;) can be equal to 0 according to this comment: https://github.com/code-423n4/2022-01-trader-joe/blob/main/contracts/LaunchEvent.sol#L106
Therefore this could happen
VS Code
Check for tokenAllocated != 0 before this division
#0 - cryptofish7
2022-01-31T23:18:43Z
🌟 Selected for report: Dravee
518.9746 USDT - $518.97
Dravee
These checks should be inclusive:
contracts\LaunchEvent.sol:182: block.timestamp > phase3Start + issuerTimelock, contracts\LaunchEvent.sol:187: block.timestamp > phase3Start + userTimelock, contracts\LaunchEvent.sol:250: _issuerTimelock > _userTimelock, contracts\LaunchEvent.sol:254: _auctionStart > block.timestamp, contracts\LaunchEvent.sol:291: if (block.timestamp < auctionStart || auctionStart == 0) { contracts\LaunchEvent.sol:293: } else if (block.timestamp < auctionStart + PHASE_ONE_DURATION) { contracts\LaunchEvent.sol:554: if (timeElapsed < PHASE_ONE_NO_FEE_DURATION) { contracts\LaunchEvent.sol:556: } else if (timeElapsed < PHASE_ONE_DURATION) {
Also, strict inequalities add a check of non equality which costs around 3 gas, therefore inclusive checks are cheaper gas-wise. Use them when possible.
🌟 Selected for report: Dravee
518.9746 USDT - $518.97
Dravee
From Slither:
LaunchEvent (contracts/LaunchEvent.sol#19-624) should inherit from ILaunchEvent (contracts/interfaces/ILaunchEvent.sol#5-18) RocketJoeToken (contracts/RocketJoeToken.sol#12-67) should inherit from IRocketJoeToken (contracts/interfaces/IRocketJoeToken.sol#9-105) Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#missing-inheritance
51.0749 USDT - $51.07
Dravee
approve() will fail for certain token implementations that do not return a boolean value. It is recommended to use OpenZeppelin's SafeERC20's safeApprove().
https://github.com/code-423n4/2022-01-trader-joe/blob/main/contracts/LaunchEvent.sol#L407-L408
File: LaunchEvent.sol 407: WAVAX.approve(address(router), wavaxReserve); 408: token.approve(address(router), tokenAllocated);
VS Code
Use OpenZeppelin's SafeERC20's safeApprove().
#0 - cryptofish7
2022-02-01T00:45:01Z
We replaced router.addLiquidity() with pair.mint() so approve not necessary anymore
🌟 Selected for report: sirhashalot
Also found by: 0v3rf10w, 0x1f8b, Dravee, UncleGrandpa925, cccz, defsec, gzeon
Dravee
Transferring tokens to the zero address is usually prohibited to accidentally avoid "burning" tokens by sending them to an unrecoverable zero address.
The penaltyCollector address is set without a 0 address check:
File: RocketJoeFactory.sol 167: function setPenaltyCollector(address _penaltyCollector) 168: external 169: override 170: onlyOwner 171: { 172: penaltyCollector = _penaltyCollector; 173: emit SetPenaltyCollector(_penaltyCollector); 174: }
If this mistake happens, the following transfer functions, which do not check for penaltyCollector being different from the default 0 address either, would burn excess tokens:
contracts\LaunchEvent.sol:538: token.transfer(penaltyCollector, excessToken); contracts\LaunchEvent.sol:543: WAVAX.transfer(penaltyCollector, excessWavax);
VS Code
Add a check to prevent accidentally burning tokens
#0 - cryptofish7
2022-02-10T21:52:18Z
Duplicate of #263
Dravee
!= 0 costs less gas compared to > 0 for unsigned integers in require statements with the optimizer enabled (6 gas)
> 0 is used in the following location(s):
contracts\LaunchEvent.sol:314: msg.value > 0, contracts\LaunchEvent.sol:338: if (rJoeNeeded > 0) { contracts\LaunchEvent.sol:355: require(_amount > 0, "LaunchEvent: invalid withdraw amount"); contracts\LaunchEvent.sol:370: if (feeAmount > 0) { contracts\LaunchEvent.sol:390: require(wavaxReserve > 0, "LaunchEvent: no wavax balance"); contracts\LaunchEvent.sol:455: if (tokenReserve > 0) { contracts\LaunchEvent.sol:486: require(amount > 0, "LaunchEvent: caller has no incentive to claim"); contracts\LaunchEvent.sol:498: user.balance > 0, contracts\LaunchEvent.sol:537: if (excessToken > 0) { contracts\LaunchEvent.sol:542: if (excessWavax > 0) { contracts\LaunchEvent.sol:547: if (excessAvax > 0) _safeTransferAVAX(penaltyCollector, excessAvax); contracts\RocketJoeFactory.sol:119: _tokenAmount > 0, contracts\RocketJoeStaking.sol:101: if (user.amount > 0) {
VS Code
Change > 0 with != 0.
#0 - cryptofish7
2022-02-10T21:45:02Z
Duplicate of #240
🌟 Selected for report: WatchPug
Also found by: Czar102, Dravee, Jujic, Meta0xNull, byterocket, defsec, p4st13r4, pauliax, robee, sirhashalot
Dravee
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.
Revert strings > 32 bytes are here:
contracts\LaunchEvent.sol:165: "LaunchEvent: you can't send AVAX directly to this contract" contracts\LaunchEvent.sol:183: "LaunchEvent: can't withdraw before issuer's timelock" contracts\LaunchEvent.sol:188: "LaunchEvent: can't withdraw before user's timelock" contracts\LaunchEvent.sol:239: "LaunchEvent: maxWithdrawPenalty too big" contracts\LaunchEvent.sol:243: "LaunchEvent: fixedWithdrawPenalty too big" contracts\LaunchEvent.sol:247: "LaunchEvent: can't lock user LP for more than 7 days" contracts\LaunchEvent.sol:251: "LaunchEvent: issuer can't withdraw before users" contracts\LaunchEvent.sol:255: "LaunchEvent: start of phase 1 cannot be in the past" contracts\LaunchEvent.sol:315: "LaunchEvent: expected non-zero AVAX to deposit" contracts\LaunchEvent.sol:322: "LaunchEvent: amount exceeds max allocation" contracts\LaunchEvent.sol:359: "LaunchEvent: withdrawn amount exceeds balance" contracts\LaunchEvent.sol:388: "LaunchEvent: liquid pair already exists" contracts\LaunchEvent.sol:444: "LaunchEvent: liquidity already withdrawn" contracts\LaunchEvent.sol:474: "LaunchEvent: incentives already withdrawn" contracts\LaunchEvent.sol:499: "LaunchEvent: expected user to have non-zero balance to perform emergency withdraw" contracts\LaunchEvent.sol:523: "LaunchEvent: caller is not RocketJoeFactory owner" contracts\RocketJoeFactory.sol:60: "RJFactory: Addresses can't be null address" contracts\RocketJoeFactory.sol:113: "RJFactory: token has already been issued" contracts\RocketJoeFactory.sol:120: "RJFactory: token amount needs to be greater than 0" contracts\RocketJoeFactory.sol:127: "RJFactory: liquid pair already exists" contracts\RocketJoeFactory.sol:208: "RJFactory: phase one duration lower than no fee duration" contracts\RocketJoeFactory.sol:225: "RJFactory: no fee duration bigger than phase one duration" contracts\RocketJoeStaking.sol:67: "RocketJoeStaking: rJOE minting needs to start after the current timestamp" contracts\RocketJoeStaking.sol:120: "RocketJoeStaking: withdraw amount exceeds balance" contracts\RocketJoeToken.sol:19: "RocketJoeToken: caller is not a RJLaunchEvent" contracts\RocketJoeToken.sol:28: "RocketJoeToken: already initialized" contracts\RocketJoeToken.sol:64: "RocketJoeToken: can't send token"
Visual Studio Code
Shorten the revert strings to fit in 32 bytes.
#0 - cryptofish7
2022-02-10T21:48:11Z
Duplicate of #242
🌟 Selected for report: Czar102
Also found by: Dravee, byterocket, d4rk
Dravee
A division by 2 can be calculated by shifting one to the right.
While the DIV opcode uses 5 gas, the SHR opcode only uses 3 gas. Furthermore, Solidity's division operation also includes a division-by-0 prevention which is bypassed using shifting.
Instances include:
contracts\LaunchEvent.sol:451: balance = lpSupply / 2; contracts\LaunchEvent.sol:585: return (user.balance * lpSupply) / wavaxAllocated / 2;
VS Code
Replace / 2 with >> 1
#0 - cryptofish7
2022-02-10T21:45:28Z
Duplicate of #271
🌟 Selected for report: Dravee
39.7792 USDT - $39.78
Dravee
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.
Functions with the onlyOwner modifier that aren't payable yet:
contracts\RocketJoeFactory.sol:159: function setRJoe(address _rJoe) external override onlyOwner { contracts\RocketJoeFactory.sol:167: function setPenaltyCollector(address _penaltyCollector) contracts\RocketJoeFactory.sol:178: function setRouter(address _router) external override onlyOwner { contracts\RocketJoeFactory.sol:185: function setFactory(address _factory) external override onlyOwner { contracts\RocketJoeFactory.sol:192: function setRJoePerAvax(uint256 _rJoePerAvax) external override onlyOwner { contracts\RocketJoeFactory.sol:200: function setPhaseDuration(uint256 _phaseNumber, uint256 _duration) contracts\RocketJoeFactory.sol:218: function setPhaseOneNoFeeDuration(uint256 _noFeeDuration) contracts\RocketJoeStaking.sol:151: function updateEmissionRate(uint256 _rJoePerSec) external onlyOwner { contracts\RocketJoeToken.sol:37: function mint(address _to, uint256 _amount) external onlyOwner {
VS Code
Mark as payable the functions that have a "safe for users" modifier
🌟 Selected for report: WatchPug
Also found by: Dravee, Jujic, Rhynorater, TomFrenchBlockchain, defsec, hyh, ye0lde
Dravee
Increased gas cost due to unnecessary automatic underflow checks.
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, or the operation doesn't depend on user input), some gas can be saved by using an unchecked block.
https://docs.soliditylang.org/en/v0.8.10/control-structures.html#checked-or-unchecked-arithmetic
These lines are the ones I found that can't underflow:
contracts\LaunchEvent.sol:329: rJoeNeeded = getRJoeAmount(newAllocation - user.allocation); contracts\LaunchEvent.sol:361: user.balance -= _amount; contracts\LaunchEvent.sol:558: ((timeElapsed - PHASE_ONE_NO_FEE_DURATION) * contracts\LaunchEvent.sol:560: uint256(PHASE_ONE_DURATION - PHASE_ONE_NO_FEE_DURATION); contracts\RocketJoeStaking.sol:87: uint256 multiplier = block.timestamp - lastRewardTimestamp; contracts\RocketJoeStaking.sol:129: user.amount = user.amount - _amount; contracts\RocketJoeStaking.sol:167: uint256 multiplier = block.timestamp - lastRewardTimestamp;
VS Code
Uncheck arithmetic operations when the risk of underflow or overflow is already contained by wrapping them in an unchecked block
#0 - cryptofish7
2022-02-10T21:44:34Z
Duplicate of #233
🌟 Selected for report: Dravee
39.7792 USDT - $39.78
Dravee
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.
In LaunchEvent.sol, the order of variables is this way:
address public issuer; //@audit-info 20 bytes < 32 bytes uint256 public auctionStart; uint256 public PHASE_ONE_DURATION; uint256 public PHASE_ONE_NO_FEE_DURATION; uint256 public PHASE_TWO_DURATION; uint256 public tokenIncentivesPercent; uint256 public floorPrice; uint256 public userTimelock; uint256 public issuerTimelock; uint256 public maxWithdrawPenalty; uint256 public fixedWithdrawPenalty; IRocketJoeToken public rJoe; uint256 public rJoePerAvax; IWAVAX private WAVAX; IERC20Metadata public token; IJoeRouter02 private router; IJoeFactory private factory; IRocketJoeFactory public rocketJoeFactory; bool private initialized; //@audit-info 1 byte < 32 bytes, should be moved bool public stopped; //@audit-info 1 byte < 32 bytes, should be moved uint256 public maxAllocation; mapping(address => UserInfo) public getUserInfo; IJoePair public pair; uint256 private wavaxAllocated; uint256 private lpSupply; uint256 private tokenReserve; uint256 private tokenIncentivesBalance; uint256 private tokenIncentivesForUsers; uint256 private tokenIncentiveIssuerRefund; uint256 private wavaxReserve;
As we can see, the first variable is an address, which is a 20 bytes size variable (way less than 32 bytes). However, due to it's wrong position, it will take up a whole 32 bytes slot.
As a bool type variable is of size 1 byte, it would be better to make them contiguous so that they are packed together in a 32 bytes slot.
I suggest the following:
address public issuer; //@audit-info SLOT 1 bool private initialized; //@audit-info SLOT 1 bool public stopped; //@audit-info SLOT 1 uint256 public auctionStart; //@audit-info SLOT 2 uint256 public PHASE_ONE_DURATION; uint256 public PHASE_ONE_NO_FEE_DURATION; uint256 public PHASE_TWO_DURATION; uint256 public tokenIncentivesPercent; uint256 public floorPrice; uint256 public userTimelock; uint256 public issuerTimelock; uint256 public maxWithdrawPenalty; uint256 public fixedWithdrawPenalty; IRocketJoeToken public rJoe; uint256 public rJoePerAvax; IWAVAX private WAVAX; IERC20Metadata public token; IJoeRouter02 private router; IJoeFactory private factory; IRocketJoeFactory public rocketJoeFactory; uint256 public maxAllocation; mapping(address => UserInfo) public getUserInfo; IJoePair public pair; uint256 private wavaxAllocated; uint256 private lpSupply; uint256 private tokenReserve; uint256 private tokenIncentivesBalance; uint256 private tokenIncentivesForUsers; uint256 private tokenIncentiveIssuerRefund; uint256 private wavaxReserve;
🌟 Selected for report: sirhashalot
Also found by: Dravee, Rhynorater, robee
Dravee
Public functions that are never called by the contract should be declared external to save gas.
Instances include:
withdrawAVAX(uint256) should be declared external: - LaunchEvent.withdrawAVAX(uint256) (contracts/LaunchEvent.sol#349-373) initialize(IERC20Upgradeable,RocketJoeToken,uint256,uint256) should be declared external: - RocketJoeStaking.initialize(IERC20Upgradeable,RocketJoeToken,uint256,uint256) (contracts/RocketJoeStaking.sol#57-76) Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#public-function-that-could-be-declared-external
Slither
Change the visibility to external
#0 - cryptofish7
2022-02-11T01:03:48Z
Duplicate of #262
🌟 Selected for report: Dravee
39.7792 USDT - $39.78
Dravee
Checking non-zero transfer values can avoid an external call to save gas.
Instances missing a non-zero value check before transfer:
contracts\LaunchEvent.sol:464: pair.transfer(msg.sender, balance); contracts\LaunchEvent.sol:514: token.transfer(issuer, balance); contracts\RocketJoeStaking.sol:145: joe.safeTransfer(address(msg.sender), _amount); contracts\RocketJoeStaking.sol:184: rJoe.transfer(_to, rJoeBal); contracts\RocketJoeStaking.sol:186: rJoe.transfer(_to, _amount);
VS Code
Check if transfer amount > 0 before executing the transfer
🌟 Selected for report: Dravee
39.7792 USDT - $39.78
Dravee
Strict inequalities add a check of non equality which costs around 3 gas.
The following should use an inclusive inequality to save gas:
File: RocketJoeStaking.sol 181: function _safeRJoeTransfer(address _to, uint256 _amount) internal { 182: uint256 rJoeBal = rJoe.balanceOf(address(this)); 183: if (_amount > rJoeBal) { //@audit should be >= to save gas 184: rJoe.transfer(_to, rJoeBal); 185: } else { 186: rJoe.transfer(_to, _amount); 187: } 188: }
VS Code
Use >= or <= instead of > and < when possible.
🌟 Selected for report: Dravee
39.7792 USDT - $39.78
Dravee
SLOADs are expensive (~100 gas) compared to MLOADs/MSTOREs (~3 gas). Minimizing them can save gas.
The code in the title's function is as such (see @audit comments):
File: LaunchEvent.sol 395: if ( 396: floorPrice > (wavaxReserve * 10**token.decimals()) / tokenAllocated //@audit 1st (wavaxReserve * 10**token.decimals()) 397: ) { 398: tokenAllocated = (wavaxReserve * 10**token.decimals()) / floorPrice; //@audit 2nd (wavaxReserve * 10**token.decimals())
Here, wavaxReserve and token are storage variables and there are even 2 external calls (token.decimals()).
The code can be optimized by caching the calculation.
Here's what I suggest:
File: LaunchEvent.sol 395: uint256 numerator = (wavaxReserve * 10**token.decimals()); //@audit caching 396: if ( 397: floorPrice > numerator / tokenAllocated //@audit MLOAD 398: ) { 399: tokenAllocated = numerator / floorPrice; //@audit MLOAD
VS Code
Use caching
🌟 Selected for report: Dravee
39.7792 USDT - $39.78
Dravee
SLOADs are expensive (~100 gas) compared to MLOADs/MSTOREs (~3 gas). Minimizing them can save gas.
The code is as such (see @audit-info comments):
File: LaunchEvent.sol 438: function withdrawLiquidity() external isStopped(false) timelockElapsed { 439: require(address(pair) != address(0), "LaunchEvent: pair not created"); //@audit-info SLOAD 440: 441: UserInfo storage user = getUserInfo[msg.sender]; 442: require( 443: !user.hasWithdrawnPair, 444: "LaunchEvent: liquidity already withdrawn" 445: ); 446: 447: uint256 balance = pairBalance(msg.sender); 448: user.hasWithdrawnPair = true; 449: 450: if (msg.sender == issuer) { 451: balance = lpSupply / 2; 452: 453: emit IssuerLiquidityWithdrawn(msg.sender, address(pair), balance); //@audit-info SLOAD 454: 455: if (tokenReserve > 0) { 456: uint256 amount = tokenReserve; 457: tokenReserve = 0; 458: token.transfer(msg.sender, amount); 459: } 460: } else { 461: emit UserLiquidityWithdrawn(msg.sender, address(pair), balance); //@audit-info SLOAD 462: } 463: 464: pair.transfer(msg.sender, balance); 465: }
1 SLOAD could be saved by caching address(pair)
VS Code
Use caching on address(pair)
🌟 Selected for report: Dravee
39.7792 USDT - $39.78
Dravee
SLOADs are expensive (~100 gas) compared to MLOADs/MSTOREs (~3 gas). Minimizing them can save gas.
The code is as such (see @audit-info comments):
File: LaunchEvent.sol 497: require( 498: user.balance > 0, //@audit-info : SLOAD. 499: "LaunchEvent: expected user to have non-zero balance to perform emergency withdraw" 500: ); 501: 502: uint256 balance = user.balance; //@audit SLOAD. Should be moved before the above require statement
1 SLOAD could be saved by caching user.balance earlier:
File: LaunchEvent.sol 497: uint256 balance = user.balance; 498: require( 499: balance > 0, //@audit-info : MLOAD. 500: "LaunchEvent: expected user to have non-zero balance to perform emergency withdraw" 501: );
VS Code
Move uint256 balance = user.balance; before the require statement.
While it indeed costs 1 MLOAD (3 gas) to make the check this way, I don't believe that it would take more than 33 miss for 1 pass. Therefore, there's no reason for the use of 2 SLOADs here
🌟 Selected for report: Dravee
39.7792 USDT - $39.78
Dravee
Increased gas cost (1 MSTORE and 1 MLOAD)
_phaseThreeStartTime is a variable used only once. A comment should suffice instead of a variable (see @audit-info):
File: RocketJoeFactory.sol 232: function _emitLaunchedEvent( 233: address _issuer, 234: address _token, 235: uint256 _phaseOneStartTime 236: ) internal { 237: uint256 _phaseTwoStartTime = _phaseOneStartTime + PHASE_ONE_DURATION; 238: uint256 _phaseThreeStartTime = _phaseTwoStartTime + PHASE_TWO_DURATION; //@audit-info _phaseThreeStartTime is used only once 239: 240: emit RJLaunchEventCreated( 241: _issuer, 242: _token, 243: _phaseOneStartTime, 244: _phaseTwoStartTime, 245: _phaseThreeStartTime, //@audit-info _phaseThreeStartTime's only use 246: rJoe, 247: rJoePerAvax 248: ); 249: }
VS Code
Do not store this data in a variable. Use _phaseTwoStartTime + PHASE_TWO_DURATION directly as the emitted argument
🌟 Selected for report: Dravee
39.7792 USDT - $39.78
Dravee
SLOADs are expensive (~100 gas) compared to MLOADs/MSTOREs (~3 gas). Minimizing them can save gas.
The code is as such (see @audit-info comments):
File: RocketJoeStaking.sol 096: function deposit(uint256 _amount) external { 097: UserInfo storage user = userInfo[msg.sender]; 098: 099: updatePool(); 100: 101: if (user.amount > 0) {//@audit-info SLOAD 102: uint256 pending = (user.amount * accRJoePerShare) / //@audit-info SLOAD 103: PRECISION - 104: user.rewardDebt; 105: _safeRJoeTransfer(msg.sender, pending); 106: } 107: user.amount = user.amount + _amount;//@audit-info SLOAD 108: user.rewardDebt = (user.amount * accRJoePerShare) / PRECISION;//@audit-info SLOAD 109: 110: joe.safeTransferFrom(address(msg.sender), address(this), _amount); 111: emit Deposit(msg.sender, _amount); 112: }
VS Code
Cache user.amount and use it for calculation.
I'd suggest:
File: RocketJoeStaking.sol 096: function deposit(uint256 _amount) external { 097: UserInfo storage user = userInfo[msg.sender]; 098: 099: updatePool(); 100: uint256 _userAmount = user.amount;//@audit-info caching 101: if (_userAmount > 0) {//@audit-info MLOAD 102: uint256 pending = (_userAmount * accRJoePerShare) / //@audit-info MLOAD 103: PRECISION - 104: user.rewardDebt; 105: _safeRJoeTransfer(msg.sender, pending); 106: } 107: _userAmount += _amount; // @audit-info memory calculation 108: user.amount = _userAmount;//@audit-info MLOAD optimization 109: user.rewardDebt = (_userAmount * accRJoePerShare) / PRECISION;//@audit-info MLOAD 110: 111: joe.safeTransferFrom(address(msg.sender), address(this), _amount); 112: emit Deposit(msg.sender, _amount); 113: }
🌟 Selected for report: Dravee
39.7792 USDT - $39.78
Dravee
SLOADs are expensive (~100 gas) compared to MLOADs/MSTOREs (~3 gas). Minimizing them can save gas.
The code is as such (see @audit-info comments):
File: RocketJoeStaking.sol 116: function withdraw(uint256 _amount) external { 117: UserInfo storage user = userInfo[msg.sender]; 118: require( 119: user.amount >= _amount, //@audit-info SLOAD 120: "RocketJoeStaking: withdraw amount exceeds balance" 121: ); 122: 123: updatePool(); 124: 125: uint256 pending = (user.amount * accRJoePerShare) / //@audit-info SLOAD 126: PRECISION - 127: user.rewardDebt; 128: 129: user.amount = user.amount - _amount;//@audit-info SLOAD 130: user.rewardDebt = (user.amount * accRJoePerShare) / PRECISION;//@audit-info SLOAD 131: 132: _safeRJoeTransfer(msg.sender, pending); 133: joe.safeTransfer(address(msg.sender), _amount); 134: emit Withdraw(msg.sender, _amount); 135: }
VS Code
Cache user.amount and use it for calculation.
I'd suggest:
File: RocketJoeStaking.sol 116: function withdraw(uint256 _amount) external { 117: UserInfo storage user = userInfo[msg.sender]; 118: uint256 _userAmount = user.amount;//@audit-info caching 119: require( 120: _userAmount >= _amount, //@audit-info MLOAD 121: "RocketJoeStaking: withdraw amount exceeds balance" 122: ); 123: 124: updatePool(); 125: 126: uint256 pending = (_userAmount * accRJoePerShare) / //@audit-info MLOAD 127: PRECISION - 128: user.rewardDebt; 129: 130: _userAmount -= _amount;//@audit-info memory calculation 131: user.amount = _userAmount;//@audit-info MLOAD optimization 132: user.rewardDebt = (_userAmount * accRJoePerShare) / PRECISION;//@audit-info MLOAD 133: 134: _safeRJoeTransfer(msg.sender, pending); 135: joe.safeTransfer(address(msg.sender), _amount); 136: emit Withdraw(msg.sender, _amount); 137: }
🌟 Selected for report: Dravee
39.7792 USDT - $39.78
Dravee
SLOADs are expensive (~100 gas) compared to MLOADs/MSTOREs (~3 gas). Minimizing them can save gas.
The code is as such (see @audit-info comments):
File: RocketJoeStaking.sol 126: uint256 pending = (_userAmount * accRJoePerShare) / //@audit accRJoePerShare SLOAD 1 127: PRECISION - 128: user.rewardDebt; 129: 130: user.amount = user.amount - _amount; 131: user.rewardDebt = (user.amount * accRJoePerShare) / PRECISION;//@audit-info accRJoePerShare SLOAD 2
By caching accRJoePerShare in memory and using it, it's possible to save 1 SLOAD
VS Code
Cache accRJoePerShare
🌟 Selected for report: Dravee
39.7792 USDT - $39.78
Dravee
SLOADs are expensive (~100 gas) compared to MLOADs/MSTOREs (~3 gas). Minimizing them can save gas.
The code is as such (see @audit-info comments):
File: RocketJoeStaking.sol 101: if (user.amount > 0) { 102: uint256 pending = (user.amount * accRJoePerShare) / //@audit accRJoePerShare SLOAD 1 103: PRECISION - 104: user.rewardDebt; 105: _safeRJoeTransfer(msg.sender, pending); 106: } 107: user.amount = user.amount + _amount; 108: user.rewardDebt = (user.amount * accRJoePerShare) / PRECISION; //@audit-info accRJoePerShare SLOAD 2
By caching accRJoePerShare in memory and using it, it's possible to save 1 SLOAD
VS Code
Cache accRJoePerShare
🌟 Selected for report: Dravee
39.7792 USDT - $39.78
Dravee
SLOADs are expensive (~100 gas) compared to MLOADs/MSTOREs (~3 gas). Minimizing them can save gas.
The code is as such (see @audit comments):
File: RocketJoeStaking.sol 159: function updatePool() public { 160: if (block.timestamp <= lastRewardTimestamp) { //@audit lastRewardTimestamp SLOAD 1 161: return; 162: } 163: uint256 joeSupply = joe.balanceOf(address(this)); 164: if (joeSupply == 0) { 165: lastRewardTimestamp = block.timestamp; 166: return; 167: } 168: uint256 multiplier = block.timestamp - lastRewardTimestamp;//@audit lastRewardTimestamp SLOAD 2 169: uint256 rJoeReward = multiplier * rJoePerSec; 170: accRJoePerShare = 171: accRJoePerShare + 172: (rJoeReward * PRECISION) / 173: joeSupply; 174: lastRewardTimestamp = block.timestamp; 175: 176: rJoe.mint(address(this), rJoeReward); 177: }
By caching lastRewardTimestamp in memory and using it, it's possible to save 1 SLOAD
VS Code
Cache the value in a variable
🌟 Selected for report: Dravee
39.7792 USDT - $39.78
Dravee
SLOADs are expensive (~100 gas) compared to MLOADs/MSTOREs (~3 gas). Minimizing them can save gas.
The code is as such (see @audit comments):
File: RocketJoeFactory.sol 117: require(_token != wavax, "RJFactory: token can't be wavax"); //@audit wavax SLOAD 1 118: require( 119: _tokenAmount > 0, 120: "RJFactory: token amount needs to be greater than 0" 121: ); 122: require( 123: IJoeFactory(factory).getPair(_token, wavax) == address(0) || //@audit wavax SLOAD 2 124: IJoePair(IJoeFactory(factory).getPair(_token, wavax)) //@audit wavax SLOAD 3 125: .totalSupply() == 126: 0, 127: "RJFactory: liquid pair already exists" 128: );
By caching this in memory and using it, it's possible to save 2 SLOADs
VS Code
Cache the value in a variable
🌟 Selected for report: Dravee
39.7792 USDT - $39.78
Dravee
SLOADs are expensive (~100 gas) compared to MLOADs/MSTOREs (~3 gas). Minimizing them can save gas.
The code is as such (see @audit comments):
File: LaunchEvent.sol 290: function currentPhase() public view returns (Phase) { 291: if (block.timestamp < auctionStart || auctionStart == 0) { //@audit auctionStart SLOAD 1 and 2 292: return Phase.NotStarted; 293: } else if (block.timestamp < auctionStart + PHASE_ONE_DURATION) { //@audit auctionStart SLOAD 3 and PHASE_ONE_DURATION SLOAD 1 294: return Phase.PhaseOne; 295: } else if ( 296: block.timestamp < 297: auctionStart + PHASE_ONE_DURATION + PHASE_TWO_DURATION //@audit auctionStart SLOAD 4 and PHASE_ONE_DURATION SLOAD 2 298: ) { 299: return Phase.PhaseTwo; 300: } 301: return Phase.PhaseThree; 302: }
VS Code
By caching these in memory, it's possible to save 4 SLOADs
🌟 Selected for report: Dravee
39.7792 USDT - $39.78
Dravee
SLOADs are expensive (~100 gas) compared to MLOADs/MSTOREs (~3 gas). Minimizing them can save gas.
The code is as such (see @audit comments):
File: LaunchEvent.sol 377: function createPair() external isStopped(false) atPhase(Phase.PhaseThree) { 378: (address wavaxAddress, address tokenAddress) = ( 379: address(WAVAX), 380: address(token) 381: ); 382: require( 383: factory.getPair(wavaxAddress, tokenAddress) == address(0) || 384: IJoePair( 385: IJoeFactory(factory).getPair(wavaxAddress, tokenAddress) 386: ).totalSupply() == 387: 0, 388: "LaunchEvent: liquid pair already exists" 389: ); 390: require(wavaxReserve > 0, "LaunchEvent: no wavax balance");//@audit wavaxReserve SLOAD 1 391: 392: uint256 tokenAllocated = tokenReserve; 393: 394: // Adjust the amount of tokens sent to the pool if floor price not met 395: if ( 396: floorPrice > (wavaxReserve * 10**token.decimals()) / tokenAllocated //@audit wavaxReserve SLOAD 2 397: ) { 398: tokenAllocated = (wavaxReserve * 10**token.decimals()) / floorPrice; //@audit wavaxReserve SLOAD 3 399: tokenIncentivesForUsers = 400: (tokenIncentivesForUsers * tokenAllocated) / 401: tokenReserve; 402: tokenIncentiveIssuerRefund = 403: tokenIncentivesBalance - 404: tokenIncentivesForUsers; 405: } 406: 407: WAVAX.approve(address(router), wavaxReserve); //@audit wavaxReserve SLOAD 4 408: token.approve(address(router), tokenAllocated); 409: 410: /// We can't trust the output cause of reflect tokens 411: (, , lpSupply) = router.addLiquidity( 412: wavaxAddress, // tokenA 413: tokenAddress, // tokenB 414: wavaxReserve, // amountADesired //@audit wavaxReserve SLOAD 5 415: tokenAllocated, // amountBDesired 416: wavaxReserve, // amountAMin //@audit wavaxReserve SLOAD 6 417: tokenAllocated, // amountBMin 418: address(this), // to 419: block.timestamp // deadline 420: ); 421: 422: pair = IJoePair(factory.getPair(tokenAddress, wavaxAddress)); 423: wavaxAllocated = wavaxReserve; //@audit wavaxReserve SLOAD 7 424: wavaxReserve = 0; 425: 426: tokenReserve -= tokenAllocated; 427: 428: emit LiquidityPoolCreated( 429: address(pair), 430: tokenAddress, 431: wavaxAddress, 432: tokenAllocated, 433: wavaxAllocated 434: ); 435: }
By caching this in memory and using it, it's possible to save 6 SLOADs
VS Code
Cache the value in a variable
🌟 Selected for report: Dravee
39.7792 USDT - $39.78
Dravee
SLOADs are expensive (~100 gas) compared to MLOADs/MSTOREs (~3 gas). Minimizing them can save gas.
The code is as such (see @audit comments):
File: LaunchEvent.sol 455: if (tokenReserve > 0) { //@audit tokenReserve SLOAD 1 456: uint256 amount = tokenReserve; //@audit tokenReserve SLOAD 2 457: tokenReserve = 0; 458: token.transfer(msg.sender, amount); 459: }
Here, uint256 amount = tokenReserve should be moved before the if statement to save 1 SLOAD at the cost of 1 MSTORE and 1 MLOAD. If the chance for tokenReserve == 0 is less that 1 in 16, it would be worth it to keep things the way they are. If not, use caching
VS Code
Cache the value in a variable
🌟 Selected for report: Dravee
39.7792 USDT - $39.78
Dravee
SLOADs are expensive (~100 gas) compared to MLOADs/MSTOREs (~3 gas). Minimizing them can save gas.
The code is as such (see @audit comments):
File: LaunchEvent.sol 494: function emergencyWithdraw() external isStopped(true) { 495: if (msg.sender != issuer) { //@audit issuer SLOAD 1 496: UserInfo storage user = getUserInfo[msg.sender]; 497: require( 498: user.balance > 0, 499: "LaunchEvent: expected user to have non-zero balance to perform emergency withdraw" 500: ); 501: 502: uint256 balance = user.balance; 503: user.balance = 0; 504: wavaxReserve -= balance; 505: WAVAX.withdraw(balance); 506: 507: _safeTransferAVAX(msg.sender, balance); 508: 509: emit AvaxEmergencyWithdraw(msg.sender, balance); 510: } else { 511: uint256 balance = tokenReserve + tokenIncentivesBalance; 512: tokenReserve = 0; 513: tokenIncentivesBalance = 0; 514: token.transfer(issuer, balance); //@audit issuer SLOAD 2 515: emit TokenEmergencyWithdraw(msg.sender, balance); 516: } 517: }
By caching this in memory and using it, it's possible to save 1 SLOAD
VS Code
Cache the value in a variable
🌟 Selected for report: Dravee
39.7792 USDT - $39.78
Dravee
SLOADs are expensive (~100 gas) compared to MLOADs/MSTOREs (~3 gas). Minimizing them can save gas.
The code is as such (see @audit comments):
File: LaunchEvent.sol 552: function getPenalty() public view returns (uint256) { 553: uint256 timeElapsed = block.timestamp - auctionStart; 554: if (timeElapsed < PHASE_ONE_NO_FEE_DURATION) { //@audit PHASE_ONE_NO_FEE_DURATION SLOAD 1 555: return 0; 556: } else if (timeElapsed < PHASE_ONE_DURATION) { //@audit PHASE_ONE_DURATION SLOAD 1 557: return 558: ((timeElapsed - PHASE_ONE_NO_FEE_DURATION) * //@audit PHASE_ONE_NO_FEE_DURATION SLOAD 2 559: maxWithdrawPenalty) / 560: uint256(PHASE_ONE_DURATION - PHASE_ONE_NO_FEE_DURATION); //@audit PHASE_ONE_DURATION SLOAD 2 and PHASE_ONE_NO_FEE_DURATION SLOAD 3 561: } 562: return fixedWithdrawPenalty; 563: }
VS Code
By caching these in memory, it's possible to save 3 SLOADs
🌟 Selected for report: Dravee
39.7792 USDT - $39.78
Dravee
SLOADs are expensive (~100 gas) compared to MLOADs/MSTOREs (~3 gas). Minimizing them can save gas.
The code is as such (see @audit comments):
File: LaunchEvent.sol 580: function pairBalance(address _user) public view returns (uint256) { 581: UserInfo memory user = getUserInfo[_user]; 582: if (wavaxAllocated == 0 || user.hasWithdrawnPair) { //@audit wavaxAllocated SLOAD 1 583: return 0; 584: } 585: return (user.balance * lpSupply) / wavaxAllocated / 2; //@audit wavaxAllocated SLOAD 2 586: }
By caching this in memory and using it, it's possible to save 1 SLOAD
VS Code
Cache the value in a variable
Dravee
Using newer compiler versions and the optimizer give gas optimizations. Also, additional safety checks are available for free.
The advantages here are:
Instances include:
contracts\LaunchEvent.sol:4:pragma solidity ^0.8.0; contracts\RocketJoeFactory.sol:4:pragma solidity ^0.8.0; contracts\RocketJoeStaking.sol:3:pragma solidity ^0.8.0; contracts\RocketJoeToken.sol:3:pragma solidity ^0.8.0;
VS Code
Consider upgrading pragma to at least 0.8.4.
#0 - cryptofish7
2022-02-10T21:50:02Z
Duplicate of #181
🌟 Selected for report: WatchPug
Also found by: Dravee, TomFrenchBlockchain, wuwe1
Dravee
Reduce contract size by removing Ownable given that its functionalities are not used.
LaunchEvent.sol inherits Ownable but the only place where a line of code relates to Ownable is the following:
contracts\LaunchEvent.sol:522: msg.sender == Ownable(address(rocketJoeFactory)).owner(),
LaunchEvent.sol doesn't need to inherit Ownable for that.
VS Code
Remove Ownable inheritance. The import is, however, useful, therefore it should remain.
#0 - cryptofish7
2022-02-10T21:43:38Z
Duplicate of #241