Platform: Code4rena
Start Date: 24/03/2023
Pot Size: $49,200 USDC
Total HM: 20
Participants: 246
Period: 6 days
Judge: Picodes
Total Solo HM: 1
Id: 226
League: ETH
Rank: 60/246
Findings: 1
Award: $90.74
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: Rolezn
Also found by: 0x3b, 0xGordita, 0xSmartContract, 0xhacksmithh, 0xnev, 0xpanicError, 4lulz, Angry_Mustache_Man, ArbitraryExecution, Aymen0909, Bason, BlueAlder, EvanW, Franfran, HHK, Haipls, IgorZuk, JCN, KrisApostolov, Madalad, MiksuJak, MiniGlome, RaymondFam, ReyAdmirado, Rickard, Sathish9098, Udsen, adriro, alexzoid, anodaram, arialblack14, c3phas, carlitox477, ch0bu, chaduke, codeslide, d3e4, dicethedev, ernestognw, fatherOfBlocks, georgits, hunter_w3b, inmarelibero, lukris02, mahdirostami, maxper, pavankv, pixpi, rotcivegaf, smaul, tank, tnevler, wen, yac
90.7432 USDC - $90.74
Avoid loops whenever possible, especially nested loops. Instead, use data structures that allow for constant-time lookup or caching. For example, in the stake() function, you could cache the ethPerDerivative value for each derivative to avoid the nested loop that calculates it.
if statements instead of require statements for input validation. require statements are more expensive than if statements because they revert the transaction and refund the remaining gas. For example, in the stake() function, you could replace require(pauseStaking == false, "staking is paused"); with if (pauseStaking) revert("staking is paused").transfer() instead of call() when sending ether to other contracts. transfer() is safer and cheaper than call(). For example, in the stake() function, you could replace derivative.deposit{value: ethAmount}(); with derivative.deposit{value: ethAmount}().transfer(ethAmount).ChangeMinAmount() and ChangeMaxAmount() both emit the same uint256 value, which is the new minAmount or maxAmount. Instead of emitting the value in both events, you could create a single event, such as ChangeAmount(uint256 amount, string message), where message is a string indicating whether it is the minimum or maximum amount being changed.stake() and unstake() function can be removed by using a more efficient way of iterating over the derivatives array, such as a mapping or an array of indexes.stake() and unstake()functions are marked aspayable, which means they can receive Ether along with the function call. However, in the current implementation, the Ether received is immediately processed, so it may not be necessary to mark these functions as payable. You could consider removing the payable` keyword if the Ether received is not needed immediately.rethAddress() function, you could store the RocketStorageInterface instance as a contract-level variable instead of creating a new instance each time the function is called. This would save gas because it would reduce the number of calls to the getAddress() function.swapExactInputSingleHop() function, you could add a check to see if the _minOut parameter is greater than zero. If it is not, you could skip the call to the Uniswap router, saving gas.poolCanDeposit() function, you could store the rocketDepositPool and `rocketProtocolSettings`` instances as contract-level variables instead of creating new instances each time the function is called. This would save gas because it would reduce the number of calls to the getAddress() function.poolCanDeposit function could be made public if it is intended to be called by other contracts. Otherwise, it could be made internal to save gas costs for not creating a new public function entry in the contract ABI.swapExactInputSingleHop function could use IERC20(_tokenIn).transferFrom(msg.sender, address(this), _amountIn) instead of IERC20(_tokenIn).approve(UNISWAP_ROUTER, _amountIn) to save gas costs for approving the token transfer.withdraw() function could be optimized by avoiding the use of the approve function to approve the FRX_ETH_CRV_POOL_ADDRESS to spend the FRX_ETH_ADDRESS tokens. Instead, the contract could hold the FRX_ETH_ADDRESS tokens directly and call the exchange function on the IFrxEthEthPool contract with the FRX_ETH_ADDRESS token allowance set to zero. This would save gas on the additional approve call and the subsequent safeTransferFrom call by the pool contract to transfer the tokens.deposit() function could be optimized by combining the two IERC20 balance checks into a single call to IERC20.balanceOf by passing the contract address as an array of size 1 instead of calling it twice with the same argument.view to the setMaxSlippage() function. If a function does not modify state, it is more gas efficient to declare it as view instead of externalwithdraw() function uses call() to send ETH to the owner of the contract. However, transfer() is a safer and cheaper option for sending ETH as it limits the amount of gas that can be consumed by the recipient contract.revert() function instead of require(): In the withdraw() function, you can use the revert() function instead of require() as it is slightly cheaper in terms of gas usage.#0 - c4-sponsor
2023-04-10T20:02:48Z
elmutt marked the issue as sponsor confirmed
#1 - c4-judge
2023-04-23T19:25:08Z
Picodes marked the issue as grade-a