|
Issue |
Instances |
| L-1 |
Do not use deprecated library functions |
6 |
| L-2 |
Empty Function Body - Consider commenting why |
10 |
| L-3 |
Return values of approve() not checked |
2 |
| L-4 |
Unsafe ERC20 operation(s) |
8 |
| - |
:- |
:-: |
| GAS-1 |
Using bools for storage incurs overhead |
1 |
| GAS-2 |
Cache array length outside of loop |
2 |
| GAS-3 |
Use Custom Errors |
33 |
| GAS-4 |
Functions guaranteed to revert when called by normal users can be marked payable |
15 |
| GAS-5 |
Use != 0 instead of > 0 for unsigned integer comparison |
7 |
| GAS-6 |
stETH should be set as immutable |
1 |
[L-1] Do not use deprecated library functions
Openzeppelin has deprecated several functions and replaced with newer versions. Please consult https://docs.openzeppelin.com/
-
Found in contracts/L2TokenReceiver.sol Line: 43
TransferHelper.safeApprove(params.tokenIn, router, 0);
-
Found in contracts/L2TokenReceiver.sol Line: 44
TransferHelper.safeApprove(params.tokenIn, nonfungiblePositionManager, 0);
-
Found in contracts/L2TokenReceiver.sol Line: 48
TransferHelper.safeApprove(params.tokenOut, nonfungiblePositionManager, 0);
-
Found in contracts/L2TokenReceiver.sol Line: 123
TransferHelper.safeApprove(newParams_.tokenIn, router, type(uint256).max);
-
Found in contracts/L2TokenReceiver.sol Line: 124
TransferHelper.safeApprove(newParams_.tokenIn, nonfungiblePositionManager, type(uint256).max);
-
Found in contracts/L2TokenReceiver.sol Line: 126
TransferHelper.safeApprove(newParams_.tokenOut, nonfungiblePositionManager, type(uint256).max);
[L-2] Empty Function Body - Consider commenting why
Instances (10):
File: L1Sender.sol
140: function _authorizeUpgrade(address) internal view override onlyOwner {}
File: L2MessageReceiver.sol
108: function _authorizeUpgrade(address) internal view override onlyOwner {}
File: L2TokenReceiver.sol
131: function _authorizeUpgrade(address) internal view override onlyOwner {}
File: MOR.sol
11: constructor(uint256 cap_) ERC20("MOR", "MOR") ERC20Capped(cap_) {}
File: mock/DistributionV2.sol
36: function _authorizeUpgrade(address) internal view override {}
File: mock/L1SenderV2.sol
11: function _authorizeUpgrade(address) internal view override {}
File: mock/L2MessageReceiverV2.sol
11: function _authorizeUpgrade(address) internal view override {}
File: mock/L2TokenReceiverV2.sol
11: function _authorizeUpgrade(address) internal view override {}
File: mock/NonfungiblePositionManagerMock.sol
9: ) external payable returns (uint128 liquidity, uint256 amount0, uint256 amount1) {}
30: {}
[L-3] Return values of approve() not checked
Not all IERC20 implementations revert() when there's a failure in approve(). The function signature has a boolean return value and they indicate errors that way instead. By not checking the return value, operations that should have marked as failed, may potentially go through without actually approving anything
Instances (2):
File: L1Sender.sol
91: IERC20(oldToken_).approve(IGatewayRouter(oldGateway_).getGateway(oldToken_), 0);
95: IERC20(newToken_).approve(IGatewayRouter(newGateway_).getGateway(newToken_), type(uint256).max);
[L-4] Unsafe ERC20 Operations should not be used
ERC20 functions may not behave as expected. For example: return values are not always meaningful. It is recommended to use OpenZeppelin's SafeERC20 library.
Instances (8):
-
Found in contracts/L1Sender.sol Line: 69
IERC20(unwrappedDepositToken).approve(oldToken_, 0);
-
Found in contracts/L1Sender.sol Line: 76
IERC20(unwrappedToken_).approve(newToken_, type(uint256).max);
-
Found in contracts/L1Sender.sol Line: 91
IERC20(oldToken_).approve(IGatewayRouter(oldGateway_).getGateway(oldToken_), 0);
-
Found in contracts/L1Sender.sol Line: 95
IERC20(newToken_).approve(IGatewayRouter(newGateway_).getGateway(newToken_), type(uint256).max);
-
Found in contracts/mock/GatewayRouterMock.sol Line: 15
IERC20(_token).transferFrom(msg.sender, _to, _amount);
-
Found in contracts/mock/SwapRouterMock.sol Line: 9
IERC20(params_.tokenIn).transferFrom(msg.sender, address(this), params_.amountIn);
-
Found in contracts/mock/SwapRouterMock.sol Line: 10
IERC20(params_.tokenOut).transfer(params_.recipient, params_.amountIn);
-
Found in contracts/mock/tokens/WStETHMock.sol Line: 25
stETH.transferFrom(msg.sender, address(this), stETHAmount_);
[GAS-1] Using bools for storage incurs overhead
Use uint256(1) and uint256(2) for true/false to avoid a Gwarmaccess (100 gas), and to avoid Gsset (20000 gas) when changing from ‘false’ to ‘true’, after having been ‘true’ in the past. See source.
Instances (1):
File: Distribution.sol
18: bool public isNotUpgradeable;
[GAS-2] Cache array length outside of loop
If not cached, the solidity compiler will always read the length of the array during each iteration. That is, if it is a storage array, this is an extra sload operation (100 additional extra gas for each iteration except for the first) and if it is a memory array, this is an extra mload operation (3 additional gas for each iteration except for the first).
Instances (2):
File: Distribution.sol
62: for (uint256 i; i < poolsInfo_.length; ++i) {
133: for (uint256 i; i < users_.length; ++i) {
[GAS-3] Use Custom Errors
Source
Instead of using error strings, to reduce deployment and runtime cost, you should use Custom Errors. This would save both deployment and runtime cost.
Instances (33):
File: Distribution.sol
37: require(_poolExists(poolId_), "DS: pool doesn't exist");
42: require(pools[poolId_].isPublic, "DS: pool isn't public");
74: require(pool_.payoutStart > block.timestamp, "DS: invalid payout start value");
84: require(pools[poolId_].isPublic == pool_.isPublic, "DS: invalid pool type");
117: require(pool_.decreaseInterval > 0, "DS: invalid decrease interval");
128: require(!pools[poolId_].isPublic, "DS: pool is public");
129: require(users_.length == amounts_.length, "DS: invalid length");
159: require(block.timestamp > pool.payoutStart + pool.claimLockPeriod, "DS: pool claim is locked");
163: require(pendingRewards_ > 0, "DS: nothing to claim");
195: require(amount_ > 0, "DS: nothing to stake");
209: require(userData.deposited + amount_ >= pool.minimalStake, "DS: amount too low");
235: require(deposited_ > 0, "DS: user isn't staked");
257: require(amount_ > 0, "DS: nothing to withdraw");
258: require(newDeposited_ >= pool.minimalStake || newDeposited_ == 0, "DS: invalid withdraw amount");
325: require(overplus_ > 0, "DS: overplus is zero");
349: require(!isNotUpgradeable, "DS: upgrade isn't available");
File: L1Sender.sol
24: require(_msgSender() == distribution, "L1S: invalid sender");
54: require(newConfig_.receiver != address(0), "L1S: invalid receiver");
File: L2MessageReceiver.sol
37: require(_msgSender() == config.gateway, "L2MR: invalid gateway");
47: require(_msgSender() == address(this), "L2MR: invalid caller");
59: require(payloadHash_ != bytes32(0), "L2MR: no stored message");
60: require(keccak256(payload_) == payloadHash_, "L2MR: invalid payload");
95: require(senderChainId_ == config.senderChainId, "L2MR: invalid sender chain ID");
101: require(sender_ == config.sender, "L2MR: invalid sender address");
File: L2TokenReceiver.sol
120: require(newParams_.tokenIn != address(0), "L2TR: invalid tokenIn");
121: require(newParams_.tokenOut != address(0), "L2TR: invalid tokenOut");
File: mock/tokens/StETHMock.sol
20: require(_amount <= 1000 * (10 ** decimals()), "StETHMock: amount is too big");
76: require(_sender != address(0), "TRANSFER_FROM_ZERO_ADDR");
77: require(_recipient != address(0), "TRANSFER_TO_ZERO_ADDR");
78: require(_recipient != address(this), "TRANSFER_TO_STETH_CONTRACT");
81: require(_sharesAmount <= currentSenderShares, "BALANCE_EXCEEDED");
88: require(_recipient != address(0), "MINT_TO_ZERO_ADDR");
File: mock/tokens/WStETHMock.sol
23: require(stETHAmount_ > 0, "wstETH: can't wrap zero stETH");
[GAS-4] Functions guaranteed to revert when called by normal users can be marked payable
If a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided.
Instances (15):
File: Distribution.sol
73: function createPool(Pool calldata pool_) public onlyOwner {
82: function editPool(uint256 poolId_, Pool calldata pool_) external onlyOwner poolExists(poolId_) {
344: function removeUpgradeability() external onlyOwner {
348: function _authorizeUpgrade(address) internal view override onlyOwner {
File: L1Sender.sol
45: function setDistribution(address distribution_) public onlyOwner {
49: function setRewardTokenConfig(RewardTokenConfig calldata newConfig_) public onlyOwner {
53: function setDepositTokenConfig(DepositTokenConfig calldata newConfig_) public onlyOwner {
140: function _authorizeUpgrade(address) internal view override onlyOwner {}
File: L2MessageReceiver.sol
26: function setParams(address rewardToken_, Config calldata config_) external onlyOwner {
108: function _authorizeUpgrade(address) internal view override onlyOwner {}
File: L2TokenReceiver.sol
41: function editParams(SwapParams memory newParams_) external onlyOwner {
54: function swap(uint256 amountIn_, uint256 amountOutMinimum_) external onlyOwner returns (uint256) {
131: function _authorizeUpgrade(address) internal view override onlyOwner {}
File: MOR.sol
24: function mint(address account_, uint256 amount_) external onlyOwner {
File: mock/tokens/StETHMock.sol
29: function setTotalPooledEther(uint256 _totalPooledEther) external onlyOwner {
[GAS-5] Use != 0 instead of > 0 for unsigned integer comparison
Instances (7):
File: Distribution.sol
117: require(pool_.decreaseInterval > 0, "DS: invalid decrease interval");
163: require(pendingRewards_ > 0, "DS: nothing to claim");
195: require(amount_ > 0, "DS: nothing to stake");
235: require(deposited_ > 0, "DS: user isn't staked");
257: require(amount_ > 0, "DS: nothing to withdraw");
325: require(overplus_ > 0, "DS: overplus is zero");
File: mock/tokens/WStETHMock.sol
23: require(stETHAmount_ > 0, "wstETH: can't wrap zero stETH");
[GAS-6] stETH should be set as immutable
File: mock/tokens/WStETHMock.sol
9: IStETH public stETH;
