Platform: Code4rena
Start Date: 30/04/2024
Pot Size: $112,500 USDC
Total HM: 22
Participants: 122
Period: 8 days
Judge: alcueca
Total Solo HM: 1
Id: 372
League: ETH
Rank: 111/122
Findings: 1
Award: $0.00
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: Sathish9098
Also found by: 0x73696d616f, 0xCiphky, 0xmystery, ABAIKUNANBAEV, Bauchibred, BiasedMerc, Fassi_Security, FastChecker, GalloDaSballo, GoatedAudits, K42, KupiaSec, LessDupes, Limbooo, ReadyPlayer2, Rhaydden, SBSecurity, Sabit, Sparrow, WildSniper, ZanyBonzy, adam-idarrha, adeolu, araj, aslanbek, atoko, b0g0, carlitox477, crypticdefense, fyamf, gesha17, gjaldon, grearlake, gumgumzum, hihen, honey-k12, hunter_w3b, inzinko, jesjupyter, jokr, kennedy1030, kind0dev, kinda_very_good, ladboy233, lanrebayode77, oakcobalt, oualidpro, pauliax, rbserver, t0x1c, tapir, underdog, xg, zzykxx
0 USDC - $0.00
Issues not included in the 4naly3er-report.
Total 132 instances over 18 issues:
| ID | Issue | Instances |
|---|---|---|
| [L-01] | Array length is not checked before access its index | 6 |
| [L-02] | Passing abi.encodePacked() with dynamic arguments to a hash can cause collisions | 2 |
| [L-03] | Variables shadowing other definitions | 6 |
| [L-04] | Missing zero address check in constructor | 2 |
| [L-05] | Missing zero address check in initializer | 16 |
| [L-06] | Revert on transfer to the zero address | 3 |
| [L-07] | State variables not limited to reasonable values | 5 |
| [L-08] | address.send() should be replace with address.call() | 1 |
| [L-09] | Contracts are not using their OZ Upgradeable counterparts | 9 |
| [L-10] | Vulnerable versions of packages are being used | 2 |
| [L-11] | Constructor / initialization function lacks parameter validation | 6 |
| [L-12] | Unsafe solidity low-level call can cause gas grief attack | 5 |
| [L-13] | Minting in a loop may lead to a DOS | 2 |
| [L-14] | Functions calling contracts/addresses with transfer hooks should be protected by reentrancy guard | 9 |
| [L-15] | Critical functions should be controlled by time locks | 26 |
| [L-16] | Missing contract existence checks before low-level calls | 1 |
| [L-17] | Tokens may be minted to the zero address | 11 |
| [L-18] | Code does not follow the best practice of check-effects-interaction | 20 |
Total 1087 instances over 73 issues:
| ID | Issue | Instances |
|---|---|---|
| [N-01] | abi.encodePacked() should be replaced with bytes.concat() | 3 |
| [N-02] | Import declarations should import specific identifiers, rather than the whole file | 138 |
| [N-03] | Visibility of state variables is not explicitly defined | 6 |
| [N-04] | Names of private/internal state variables should be prefixed with an underscore | 7 |
| [N-05] | The nonReentrant modifier should occur before all other modifiers | 2 |
| [N-06] | Redundant inheritance specifier | 13 |
| [N-07] | Use of override is unnecessary | 12 |
| [N-08] | Unused structs | 2 |
| [N-09] | Custom errors should be used rather than revert()/require() | 12 |
| [N-10] | Add inline comments for unnamed parameters | 7 |
| [N-11] | Consider providing a ranged getter for array state variables | 3 |
| [N-12] | Consider splitting complex checks into multiple steps | 1 |
| [N-13] | Consider adding a block/deny-list | 8 |
| [N-14] | Constants/Immutables redefined elsewhere | 7 |
| [N-15] | Contracts should each be defined in separate files | 6 |
| [N-16] | Contract name does not match its filename | 10 |
| [N-17] | Consider using AccessControlDefaultAdminRules rather than AccessControl | 1 |
| [N-18] | UPPER_CASE names should be reserved for constant/immutable variables | 4 |
| [N-19] | Consider emitting an event at the end of the constructor | 22 |
| [N-20] | Empty function body without comments | 3 |
| [N-21] | Events are emitted without the sender information | 16 |
| [N-22] | Inconsistent floating version pragma | 4 |
| [N-23] | Function state mutability can be restricted to pure | 5 |
| [N-24] | Imports could be organized more systematically | 20 |
| [N-25] | There is no need to initialize bool variables with false | 3 |
| [N-26] | Interfaces should be indicated with an I prefix in the contract name | 1 |
| [N-27] | Invalid NatSpec comment style | 1 |
| [N-28] | @openzeppelin/contracts should be upgraded to a newer version | 64 |
| [N-29] | Lib @openzeppelin/contracts-upgradeable should be upgraded to a newer version | 40 |
| [N-30] | Expressions for constant values should use immutable rather than constant | 15 |
| [N-31] | Consider moving duplicated strings to constants | 10 |
| [N-32] | Contracts containing only utility functions should be made into libraries | 3 |
| [N-33] | Error names should use CapWords style | 1 |
| [N-34] | Contract name does not follow the Solidity Style Guide | 6 |
| [N-35] | Functions should be named in mixedCase style | 9 |
| [N-36] | Variable names for immutables should use UPPER_CASE_WITH_UNDERSCORES | 2 |
| [N-37] | Named imports of parent contracts are missing | 59 |
| [N-38] | Constants should be put on the left side of comparisons | 86 |
| [N-39] | else-block not required | 4 |
| [N-40] | Large multiples of ten should use scientific notation | 6 |
| [N-41] | High cyclomatic complexity | 1 |
| [N-42] | Typos | 19 |
| [N-43] | Consider bounding input array length | 18 |
| [N-44] | Unnecessary casting | 8 |
| [N-45] | Unused contract variables | 1 |
| [N-46] | Unused import | 25 |
| [N-47] | Unused modifiers | 1 |
| [N-48] | Unused named return | 22 |
| [N-49] | Use delete instead of assigning values to false | 1 |
| [N-50] | Consider using delete rather than assigning zero to clear values | 2 |
| [N-51] | Use the latest Solidity version | 38 |
| [N-52] | Use transfer libraries instead of low level calls to transfer native tokens | 6 |
| [N-53] | Use a struct to encapsulate multiple function parameters | 9 |
| [N-54] | Returning a struct instead of a bunch of variables is better | 7 |
| [N-55] | Events that mark critical parameter changes should contain both the old and the new value | 10 |
| [N-56] | Contract variables should have comments | 4 |
| [N-57] | Missing event when a state variables is set in constructor | 6 |
| [N-58] | Empty bytes check is missing | 10 |
| [N-59] | Don't define functions with the same name in a contract | 2 |
| [N-60] | Interface files should not use fixed compiler versions | 1 |
| [N-61] | Multiple mappings with same keys can be combined into a single struct mapping for readability | 3 |
| [N-62] | Missing event for critical changes | 3 |
| [N-63] | Duplicated require()/revert() checks should be refactored | 22 |
| [N-64] | Consider adding emergency-stop functionality | 12 |
| [N-65] | Avoid the use of sensitive terms | 25 |
| [N-66] | Missing checks for uint state variable assignments | 9 |
| [N-67] | Use the Modern Upgradeable Contract Paradigm | 8 |
| [N-68] | Large or complicated code bases should implement invariant tests | 1 |
| [N-69] | The default value is manually set when it is declared | 37 |
| [N-70] | Contracts should have all public/external functions exposed by interfaces | 23 |
| [N-71] | Top-level declarations should be separated by at least two lines | 89 |
| [N-72] | Consider adding formal verification proofs | 40 |
| [N-73] | Numeric values having to do with time should use time units for readability | 2 |
Accessing the elements of the array without checking or ensuring the validity of the access index in advance. It may result in an unexpected out-of-bounds error, or may result in missing elements when trying to traverse the entire array.
There are 6 instances:
410: if (operatorDelegatorTokenTVLs[0][tokenIndex] < ezETHValue) { 410: if (operatorDelegatorTokenTVLs[0][tokenIndex] < ezETHValue) { 424: operatorDelegatorTokenTVLs[i][tokenIndex] >= ezETHValue 424: operatorDelegatorTokenTVLs[i][tokenIndex] >= ezETHValue 436: if (operatorDelegatorTokenTVLs[i][tokenIndex] >= ezETHValue) { 436: if (operatorDelegatorTokenTVLs[i][tokenIndex] >= ezETHValue) {
abi.encodePacked() with dynamic arguments to a hash can cause collisionsUsing abi.encodePacked can cause hash collisions when used with multiple dynamic arguments. It is recommended to use abi.encode() instead which will pad items to 32 bytes and prevent hash collisions.
There are 2 instances:
147: bytes32 _salt = keccak256(abi.encodePacked(_name, _symbol, msg.sender));
89: bytes32 _salt = keccak256(abi.encodePacked(_name, _symbol, msg.sender));
A variable declaration shadowing any other existing definition is error-prone. It can cause confusion for developers, potentially introduce bugs, and reduce the readability and maintainability.
There are 6 instances:
/// @audit Shadows `state variable _name` 62: string memory _name, /// @audit Shadows `state variable _symbol` 63: string memory _symbol, /// @audit Shadows `state variable _name` 77: string memory _name, /// @audit Shadows `state variable _symbol` 78: string memory _symbol,
/// @audit Shadows `state variable _name` 36: string memory _name, /// @audit Shadows `state variable _symbol` 37: string memory _symbol,
Constructors often take address parameters to initialize important components of a contract, such as owner or linked contracts. However, without a checking, there's a risk that an address parameter could be mistakenly set to the zero address (0x0). This could be due to an error or oversight during contract deployment. A zero address in a crucial role can cause serious issues, as it cannot perform actions like a normal address, and any funds sent to it will be irretrievable. It's therefore crucial to include a zero address check in constructors to prevent such potential problems. If a zero address is detected, the constructor should revert the transaction.
There are 2 instances:
/// @audit `proposers not checked` /// @audit `executors not checked` 87: constructor( 88: uint256 minDelay, 89: address[] memory proposers, 90: address[] memory executors, 91: address admin 92: ) {
Consider adding a zero address check for each address type parameter in initializer.
<details> <summary>There are 16 instances (click to show):</summary>/// @audit `_receiver not checked` /// @audit `_oracle not checked` 75: function initialize( 76: uint256 _currentPrice, 77: IERC20 _xezETH, 78: IERC20 _depositToken, 79: IERC20 _collateralToken, 80: IConnext _connext, 81: bytes32 _swapKey, 82: address _receiver, 83: uint32 _bridgeDestinationDomain, 84: address _bridgeTargetAddress, 85: IRenzoOracleL2 _oracle 86: ) public initializer {
/// @audit `_factory not checked` 61: function initialize( 62: string memory _name, 63: string memory _symbol, 64: address _factory 65: ) public initializer {
/// @audit `_lockboxImplementation not checked` /// @audit `_xerc20Implementation not checked` 54: function initialize( 55: address _lockboxImplementation, 56: address _xerc20Implementation 57: ) public initializer {
/// @audit `_xerc20 not checked` /// @audit `_erc20 not checked` 44: function initialize(address _xerc20, address _erc20, bool _isNative) public initializer {
/// @audit `_factory not checked` /// @audit `_l1Token not checked` /// @audit `_optimismBridge not checked` 35: function initialize( 36: string memory _name, 37: string memory _symbol, 38: address _factory, 39: address _l1Token, 40: address _optimismBridge 41: ) public initializer {
</details>/// @audit `_roleManager not checked` /// @audit `_ezETH not checked` /// @audit `_renzoOracle not checked` /// @audit `_strategyManager not checked` /// @audit `_delegationManager not checked` /// @audit `_depositQueue not checked` 101: function initialize( 102: IRoleManager _roleManager, 103: IEzEthToken _ezETH, 104: IRenzoOracle _renzoOracle, 105: IStrategyManager _strategyManager, 106: IDelegationManager _delegationManager, 107: IDepositQueue _depositQueue 108: ) public initializer {
It's good practice to revert a token transfer transaction if the recipient's address is the zero address. This can prevent unintentional transfers to the zero address due to accidental operations or programming errors. Many token contracts implement such a safeguard, such as OpenZeppelin - ERC20, OpenZeppelin - ERC721.
There are 3 instances:
306: IERC20(_token).safeTransfer(_to, _amount);
490: IERC20(_token).safeTransfer(_to, _amount);
134: ERC20.safeTransfer(_to, _amount);
Consider adding appropriate minimum/maximum value checks to ensure that the following state variables can never be used to excessively harm users, including via griefing.
There are 5 instances:
216: maxDepositTVL = _maxDepositTVL; 714: collateralTokenTvlLimits[_token] = _limit;
405: _minDelay = newDelay;
87: coolDownPeriod = _coolDownPeriod; 132: coolDownPeriod = _newCoolDownPeriod;
address.send() should be replace with address.call()The <address payable>.send() will forward 2300 gas stipend, not adjustable. The limited gas makes it more likely to fail. It is recommended to use address.call{value: amount}() instead, like Address.sendValue()
There is 1 instance:
485: bool success = payable(tx.origin).send(gasRefund);
The non-upgradeable standard version of OpenZeppelin’s library are inherited / used by the contracts. It would be safer to use the upgradeable versions of the library contracts to avoid unexpected behavior.
Where applicable, use the contracts from @openzeppelin/contracts-upgradeable instead of @openzeppelin/contracts. See this for list of available upgradeable contracts
12: import "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";
12: import "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";
4: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
5: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
4: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; 9: import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; 10: import "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol";
</details>6: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; 9: import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
This project is using specific package versions which are vulnerable to the specific CVEs listed below. Consider switching to more recent versions of these packages that don't have these vulnerabilities.
CVE-2023-49798 - HIGH - (@openzeppelin/contracts 4.9.4): OpenZeppelin Contracts is a library for smart contract development. A merge issue when porting the 5.0.1 patch to the 4.9 branch caused a line duplication. In the version of Multicall.sol released in @openzeppelin/contracts@4.9.4 and @openzeppelin/contracts-upgradeable@4.9.4, all subcalls are executed twice. Concretely, this exposes a user to unintentionally duplicate operations like asset transfers. The duplicated delegatecall was removed in version 4.9.5. The 4.9.4 version is marked as deprecated. Users are advised to upgrade. There are no known workarounds for this issue.
CVE-2023-49798 - HIGH - (@openzeppelin/contracts-upgradeable 4.9.4): OpenZeppelin Contracts is a library for smart contract development. A merge issue when porting the 5.0.1 patch to the 4.9 branch caused a line duplication. In the version of Multicall.sol released in @openzeppelin/contracts@4.9.4 and @openzeppelin/contracts-upgradeable@4.9.4, all subcalls are executed twice. Concretely, this exposes a user to unintentionally duplicate operations like asset transfers. The duplicated delegatecall was removed in version 4.9.5. The 4.9.4 version is marked as deprecated. Users are advised to upgrade. There are no known workarounds for this issue.
There are 2 instances:
Constructors and initialization functions play a critical role in contracts by setting important initial states when the contract is first deployed before the system starts. The parameters passed to the constructor and initialization functions directly affect the behavior of the contract / protocol. If incorrect parameters are provided, the system may fail to run, behave abnormally, be unstable, or lack security. Therefore, it's crucial to carefully check each parameter in the constructor and initialization functions. If an exception is found, the transaction should be rolled back.
<details> <summary>There are 6 instances (click to show):</summary>/// @audit `_name` /// @audit `_symbol` 61: function initialize( 62: string memory _name, 63: string memory _symbol, 64: address _factory 65: ) public initializer {
/// @audit `_isNative` 44: function initialize(address _xerc20, address _erc20, bool _isNative) public initializer {
/// @audit `_name` /// @audit `_symbol` 35: function initialize( 36: string memory _name, 37: string memory _symbol, 38: address _factory, 39: address _l1Token, 40: address _optimismBridge 41: ) public initializer {
</details>/// @audit `minDelay` 87: constructor( 88: uint256 minDelay, 89: address[] memory proposers, 90: address[] memory executors, 91: address admin 92: ) {
Using the low-level calls of a solidity address can leave the contract open to gas grief attacks. These attacks occur when the called contract returns a large amount of data.
So when calling an external contract, it is necessary to check the length of the return data before reading/copying it (using returndatasize()).
There are 5 instances:
131: (bool _success, ) = payable(_to).call{ value: _amount }("");
520: (bool success, ) = destination.call{ value: remainingAmount }("");
168: (bool success, ) = feeAddress.call{ value: feeAmount }("");
68: (bool success, ) = rewardDestination.call{ value: balance }("");
369: (bool success, ) = target.call{ value: value }(data);
The signature used seems to require all the minting operations to be done at once. If there are a lot of them, there may be too many to mint in one block.
There are 2 instances:
/// @audit mint by setLimits() 167: for (uint256 _i; _i < _bridgesLength; ++_i) { 168: XERC20(_xerc20).setLimits(_bridges[_i], _minterLimits[_i], _burnerLimits[_i]); 169: }
/// @audit mint by setLimits() 109: for (uint256 _i; _i < _bridgesLength; ++_i) { 110: OptimismMintableXERC20(_xerc20).setLimits( 111: _bridges[_i], 112: _minterLimits[_i], 113: _burnerLimits[_i] 114: ); 115: }
Even if the function follows the best practice of check-effects-interaction, not using a reentrancy guard when there may be transfer hooks opens the users of this protocol up to read-only reentrancy vulnerability with no way to protect them except by block-listing the entire protocol.
<details> <summary>There are 9 instances (click to show):</summary>83: SafeERC20.safeTransferFrom(IERC20(_erc20), msg.sender, address(this), _amount);
295: payable(_to).transfer(_amount); 306: IERC20(_token).safeTransfer(_to, _amount);
479: payable(_to).transfer(_amount); 490: IERC20(_token).safeTransfer(_to, _amount);
134: ERC20.safeTransfer(_to, _amount); 147: ERC20.safeTransferFrom(msg.sender, address(this), _amount);
262: IERC20(token).safeTransfer(feeAddress, feeAmount);
</details>661: _token.safeTransferFrom(msg.sender, address(this), _amount);
It is a good practice to give time for users to react and adjust to critical changes. A timelock provides more guarantees and reduces the level of trust required, thus decreasing risk for users. It also indicates that the project is legitimate (less risk of a malicious owner making a sandwich attack on a user).
<details> <summary>There are 26 instances (click to show):</summary>36: function setOracleAddress(AggregatorV3Interface _oracleAddress) external onlyOwner {
96: function updateXRenzoBridgeL1(address _newXRenzoBridgeL1) external onlyOwner { 134: function setRenzoDeposit(address _newXRenzoDeposit) external onlyOwner {
92: function updateXRenzoBridgeL1(address _newXRenzoBridgeL1) external onlyOwner { 130: function setRenzoDeposit(address _newXRenzoDeposit) external onlyOwner {
466: function setAllowedBridgeSweeper(address _sweeper, bool _allowed) external onlyOwner { 501: function setOraclePriceFeed(IRenzoOracleL2 _oracle) external onlyOwner { 511: function setReceiverPriceFeed(address _receiver) external onlyOwner {
135: function setLimits( 136: address _bridge, 137: uint256 _mintingLimit, 138: uint256 _burningLimit 139: ) external onlyOwner {
106: function setTokenStrategy( 107: IERC20 _token, 108: IStrategy _strategy 109: ) external nonReentrant onlyOperatorDelegatorAdmin { 117: function setDelegateAddress( 118: address _delegateAddress, 119: ISignatureUtils.SignatureWithExpiry memory approverSignatureAndExpiry, 120: bytes32 approverSalt 121: ) external nonReentrant onlyOperatorDelegatorAdmin {
87: function setWithdrawQueue(IWithdrawQueue _withdrawQueue) external onlyRestakeManagerAdmin { 93: function setFeeConfig( 94: address _feeAddress, 95: uint256 _feeBasisPoints 96: ) external onlyRestakeManagerAdmin { 112: function setRestakeManager(IRestakeManager _restakeManager) external onlyRestakeManagerAdmin {
54: function setOracleAddress( 55: IERC20 _token, 56: AggregatorV3Interface _oracleAddress 57: ) external nonReentrant onlyOracleAdmin {
131: function addOperatorDelegator( 132: IOperatorDelegator _newOperatorDelegator, 133: uint256 _allocationBasisPoints 134: ) external onlyRestakeManagerAdmin { 160: function removeOperatorDelegator( 161: IOperatorDelegator _operatorDelegatorToRemove 162: ) external onlyRestakeManagerAdmin { 187: function setOperatorDelegatorAllocation( 188: IOperatorDelegator _operatorDelegator, 189: uint256 _allocationBasisPoints 190: ) external onlyRestakeManagerAdmin { 220: function addCollateralToken(IERC20 _newCollateralToken) external onlyRestakeManagerAdmin { 244: function removeCollateralToken( 245: IERC20 _collateralTokenToRemove 246: ) external onlyRestakeManagerAdmin { 709: function setTokenTvlLimit(IERC20 _token, uint256 _limit) external onlyRestakeManagerAdmin {
72: function setRewardDestination( 73: address _rewardDestination 74: ) external nonReentrant onlyRestakeManagerAdmin {
106: function updateWithdrawBufferTarget( 107: TokenWithdrawBuffer[] calldata _newBufferTarget 108: ) external onlyWithdrawQueueAdmin { 129: function updateCoolDownPeriod(uint256 _newCoolDownPeriod) external onlyWithdrawQueueAdmin { 139: function pause() external onlyWithdrawQueueAdmin {
</details>51: function setPaused(bool _paused) external onlyTokenAdmin {
Low-level calls return success if there is no code present at the specified address. In addition to the zero-address checks, add a check to verify that <address>.code.length > 0.
There is 1 instance:
369: (bool success, ) = target.call{ value: value }(data);
Neither the listed functions, nor _mint() prevent minting to address(0x0)
266: IXERC20(address(xezETH)).mint(msg.sender, xezETHAmount);
96: function mint(address _user, uint256 _amount) public virtual { 152: function mintingMaxLimitOf(address _bridge) public view returns (uint256 _limit) { 174: function mintingCurrentLimitOf(address _bridge) public view returns (uint256 _limit) { 206: uint256 _currentLimit = mintingCurrentLimitOf(_bridge); 232: uint256 _currentLimit = mintingCurrentLimitOf(_bridge);
150: XERC20.mint(_to, _amount);
64: function mint(address _to, uint256 _amount) public override(XERC20, IOptimismMintableERC20) {
572: ezETH.mint(msg.sender, ezETHToMint); 612: ezETH.mint(msg.sender, ezETHToMint);
</details>41: function mint(address to, uint256 amount) external onlyMinterBurner {
Code should follow the best-practice of check-effects-interaction, where state variables are updated before any external calls are made. Doing so prevents a large class of reentrancy bugs.
<details> <summary>There are 20 instances (click to show):</summary>/// @audit `getRate()` is called on line 215 259: ++i; /// @audit `getRate()` is called on line 215 283: ++i;
/// @audit `safeApprove()` is called on line 369 387: amountNextWETH -= fee; /// @audit `withdraw()` is called on line 401 402: feeCollected = address(this).balance - balanceBefore;
/// @audit `setLockbox()` is called on line 208 210: _lockboxRegistry[_xerc20] = _lockbox;
/// @audit `completeQueuedWithdrawal()` is called on line 274 281: queuedShares[address(tokens[i])] -= withdrawal.shares[i]; /// @audit `completeQueuedWithdrawal()` is called on line 274 291: bufferToFill = (balanceOfToken <= bufferToFill) ? balanceOfToken : bufferToFill; /// @audit `completeQueuedWithdrawal()` is called on line 274 294: balanceOfToken -= bufferToFill; /// @audit `completeQueuedWithdrawal()` is called on line 274 312: ++i; /// @audit `stake()` is called on line 355 358: stakedButNotVerifiedEth += msg.value; /// @audit `verifyWithdrawalCredentials()` is called on line 372 385: stakedButNotVerifiedEth -= (validatorCurrentBalanceGwei * GWEI_TO_WEI); /// @audit `verifyWithdrawalCredentials()` is called on line 372 387: ++i; /// @audit `send()` is called on line 485 489: adminGasSpentInWei[tx.origin] -= gasRefund;
/// @audit `_checkAndFillETHWithdrawBuffer()` is called on line 176, triggering an external call - `getBufferDeficit()` 179: totalEarned[address(0x0)] = totalEarned[address(0x0)] + remainingRewards; /// @audit `balanceOf()` is called on line 255 272: totalEarned[address(token)] = totalEarned[address(token)] + balance - feeAmount;
/// @audit `calculateTVLs()` is called on line 504, triggering an external call - `withdrawQueue()` 547: bufferToFill = (_amount <= bufferToFill) ? _amount : bufferToFill; /// @audit `calculateTVLs()` is called on line 504, triggering an external call - `withdrawQueue()` 549: _amount -= bufferToFill;
</details>/// @audit `safeTransferFrom()` is called on line 214 229: amountToRedeem = renzoOracle.lookupTokenAmountFromValue( 230: IERC20(_assetOut), 231: amountToRedeem 232: ); /// @audit `safeTransferFrom()` is called on line 214 239: withdrawRequestNonce++; /// @audit `safeTransferFrom()` is called on line 214 253: claimReserve[_assetOut] += amountToRedeem;
abi.encodePacked() should be replaced with bytes.concat()Solidity version 0.8.4 introduces bytes.concat(), which can be used to replace abi.encodePacked() on bytes/strings. It can make the intended operation clearer, leading to less reviewer confusion.
There are 3 instances:
158: bytes memory _bytecode = abi.encodePacked( 159: _creation, 160: abi.encode(xerc20Implementation, _proxyAdmin, initializeBytecode) 161: ); 201: bytes memory _bytecode = abi.encodePacked( 202: _creation, 203: abi.encode(lockboxImplementation, _proxyAdmin, initializeBytecode) 204: );
100: bytes memory _bytecode = abi.encodePacked( 101: _creation, 102: abi.encode(xerc20Implementation, _proxyAdmin, initializeBytecode) 103: );
Using import declarations of the form import {<identifier_name>} from "some/file.sol" avoids polluting the symbol namespace making flattened files smaller, and speeds up compilation (but does not save any gas).
4: import "./xRenzoBridgeStorage.sol"; 5: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; 6: import "../../Errors/Errors.sol"; 10: import "../Connext/core/IXReceiver.sol"; 11: import "../Connext/core/IWeth.sol"; 12: import "../xERC20/interfaces/IXERC20.sol"; 14: import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";
4: import "./IxRenzoBridge.sol"; 5: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; 6: import "../../IRestakeManager.sol"; 7: import "../xERC20/interfaces/IXERC20Lockbox.sol"; 8: import "../Connext/core/IConnext.sol";
4: import "./RenzoOracleL2Storage.sol"; 5: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; 9: import "../../../Errors/Errors.sol";
4: import "./IRenzoOracleL2.sol"; 5: import "@chainlink/contracts/src/v0.8/interfaces/AggregatorV3Interface.sol";
11: import "../../../Errors/Errors.sol";
8: import "../../../Errors/Errors.sol";
4: import "./xRenzoDepositStorage.sol"; 5: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; 12: import "../../Errors/Errors.sol"; 13: import "../xERC20/interfaces/IXERC20.sol"; 14: import "../Connext/core/IWeth.sol"; 15: import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol"; 16: import "../../RateProvider/IRateProvider.sol";
4: import "./IxRenzoDeposit.sol"; 5: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; 6: import "../Connext/core/IConnext.sol"; 7: import "./Oracle/IRenzoOracleL2.sol";
14: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
11: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; 12: import "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";
9: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
11: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; 12: import "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";
4: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; 5: import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol"; 6: import "../Permissions/IRoleManager.sol"; 7: import "./OperatorDelegatorStorage.sol"; 8: import "../EigenLayer/interfaces/IDelegationManager.sol"; 9: import "../EigenLayer/interfaces/ISignatureUtils.sol"; 10: import "../Errors/Errors.sol";
3: import "../Permissions/IRoleManager.sol"; 4: import "../EigenLayer/interfaces/IStrategy.sol"; 5: import "../EigenLayer/interfaces/IStrategyManager.sol"; 6: import "../EigenLayer/interfaces/IDelegationManager.sol"; 7: import "../EigenLayer/interfaces/IEigenPod.sol"; 8: import "./IOperatorDelegator.sol"; 9: import "../IRestakeManager.sol";
4: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; 5: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; 6: import "./DepositQueueStorage.sol"; 7: import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol"; 8: import "../Errors/Errors.sol";
4: import "../Permissions/IRoleManager.sol"; 5: import "../Withdraw/IWithdrawQueue.sol"; 6: import "../IRestakeManager.sol"; 7: import "./IDepositQueue.sol";
4: import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol"; 5: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; 6: import "./WBETHShimStorage.sol"; 7: import "../../Errors/Errors.sol";
4: import "@chainlink/contracts/src/v0.8/interfaces/AggregatorV3Interface.sol"; 5: import "./IStakedTokenV2.sol";
4: import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol"; 5: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; 6: import "./METHShimStorage.sol"; 7: import "../../Errors/Errors.sol";
4: import "@chainlink/contracts/src/v0.8/interfaces/AggregatorV3Interface.sol"; 5: import "./IMethStaking.sol";
4: import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol"; 5: import "../Permissions/IRoleManager.sol"; 6: import "./RenzoOracleStorage.sol"; 7: import "./IRenzoOracle.sol"; 8: import "../Errors/Errors.sol";
4: import "../Permissions/IRoleManager.sol"; 5: import "@chainlink/contracts/src/v0.8/interfaces/AggregatorV3Interface.sol"; 6: import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
4: import "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol"; 5: import "./IRoleManager.sol"; 6: import "./RoleManagerStorage.sol"; 7: import "../Errors/Errors.sol";
4: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; 5: import "./IRateProvider.sol"; 6: import "../Errors/Errors.sol"; 7: import "./BalancerRateProviderStorage.sol";
4: import "@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol"; 5: import "../IRestakeManager.sol";
4: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; 5: import "@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.sol"; 6: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; 7: import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol"; 8: import "@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol"; 9: import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; 10: import "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol"; 11: import "./RestakeManagerStorage.sol"; 12: import "./EigenLayer/interfaces/IStrategy.sol"; 13: import "./EigenLayer/interfaces/IStrategyManager.sol"; 14: import "./EigenLayer/interfaces/IDelegationManager.sol"; 15: import "./token/IEzEthToken.sol"; 16: import "./IRestakeManager.sol"; 17: import "./Errors/Errors.sol";
4: import "./EigenLayer/interfaces/IStrategy.sol"; 5: import "./EigenLayer/interfaces/IDelegationManager.sol"; 6: import "./EigenLayer/interfaces/IStrategyManager.sol"; 7: import "./token/IEzEthToken.sol"; 8: import "./Delegation/IOperatorDelegator.sol"; 9: import "./Permissions/IRoleManager.sol"; 10: import "./Oracle/IRenzoOracle.sol"; 11: import "./Deposits/IDepositQueue.sol"; 12: import "./IRestakeManager.sol"; 13: import "./Withdraw/IWithdrawQueue.sol";
4: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; 5: import "./RewardHandlerStorage.sol"; 6: import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol"; 7: import "../Errors/Errors.sol";
4: import "../Permissions/IRoleManager.sol";
6: import "@openzeppelin/contracts/access/AccessControl.sol"; 7: import "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol"; 8: import "@openzeppelin/contracts/token/ERC1155/IERC1155Receiver.sol";
4: import "./WithdrawQueueStorage.sol"; 5: import "../Errors/Errors.sol"; 6: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; 7: import "@openzeppelin/contracts-upgradeable/security/PausableUpgradeable.sol"; 8: import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol"; 9: import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
4: import "../Permissions/IRoleManager.sol"; 5: import "../Oracle/IRenzoOracle.sol"; 6: import "../IRestakeManager.sol"; 7: import "../token/IEzEthToken.sol";
4: import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol"; 5: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; 6: import "./IEzEthToken.sol"; 7: import "../Permissions/IRoleManager.sol"; 8: import "./EzEthTokenStorage.sol"; 9: import "../Errors/Errors.sol";
</details>3: import "../Permissions/IRoleManager.sol";
To avoid misunderstandings and unexpected state accesses, it is recommended to explicitly define the visibility of each state variable.
There are 6 instances:
31: address immutable blastStandardBridge; 32: address immutable registry;
20: string constant INVALID_0_INPUT = "Invalid 0 input"; 23: uint256 constant SCALE_FACTOR = 10 ** 18; 26: uint256 constant MAX_TIME_WINDOW = 86400 + 60; // 24 hours + 60 seconds
38: uint256 constant BASIS_POINTS = 100;
private/internal state variables should be prefixed with an underscoreIt is recommended by the Solidity Style Guide.
There are 7 instances:
31: address immutable blastStandardBridge; 32: address immutable registry;
24: uint256 internal constant GWEI_TO_WEI = 1e9;
20: string constant INVALID_0_INPUT = "Invalid 0 input"; 23: uint256 constant SCALE_FACTOR = 10 ** 18; 26: uint256 constant MAX_TIME_WINDOW = 86400 + 60; // 24 hours + 60 seconds
38: uint256 constant BASIS_POINTS = 100;
nonReentrant modifier should occur before all other modifiersThis is a best-practice to protect against reentrancy in other modifiers
There are 2 instances:
210: function sendPrice( 211: CCIPDestinationParam[] calldata _destinationParam, 212: ConnextDestinationParam[] calldata _connextDestinationParam 213: ) external payable onlyPriceFeedSender nonReentrant {
211: function stakeEthFromQueueMulti( 212: IOperatorDelegator[] calldata operatorDelegators, 213: bytes[] calldata pubkeys, 214: bytes[] calldata signatures, 215: bytes32[] calldata depositDataRoots 216: ) external onlyNativeEthRestakeAdmin nonReentrant {
The contracts below already extend the specified contract, so there is no need to list it in the inheritance list again.
<details> <summary>There are 13 instances (click to show):</summary>/// @audit ReentrancyGuardUpgradeable already extends Initializable 16: contract xRenzoBridge is 17: IXReceiver, 18: Initializable, 19: ReentrancyGuardUpgradeable, 20: xRenzoBridgeStorageV1
/// @audit OwnableUpgradeable already extends Initializable 11: contract RenzoOracleL2 is Initializable, OwnableUpgradeable, RenzoOracleL2StorageV1 {
/// @audit OwnableUpgradeable already extends Initializable 27: contract xRenzoDeposit is 28: Initializable, 29: OwnableUpgradeable, 30: ReentrancyGuardUpgradeable, 31: IRateProvider, 32: xRenzoDepositStorageV3
/// @audit ERC20Upgradeable already extends Initializable /// @audit ERC20PermitUpgradeable already extends ERC20Upgradeable 16: contract XERC20 is 17: Initializable, 18: ERC20Upgradeable, 19: OwnableUpgradeable, 20: IXERC20, 21: ERC20PermitUpgradeable
/// @audit XERC20Factory already extends Initializable 14: contract OptimismMintableXERC20Factory is Initializable, XERC20Factory {
/// @audit ReentrancyGuardUpgradeable already extends Initializable 17: contract OperatorDelegator is 18: Initializable, 19: ReentrancyGuardUpgradeable, 20: OperatorDelegatorStorageV4
/// @audit ReentrancyGuardUpgradeable already extends Initializable 10: contract DepositQueue is Initializable, ReentrancyGuardUpgradeable, DepositQueueStorageV2 {
/// @audit ReentrancyGuardUpgradeable already extends Initializable 13: contract RenzoOracle is 14: IRenzoOracle, 15: Initializable, 16: ReentrancyGuardUpgradeable, 17: RenzoOracleStorageV1
/// @audit ReentrancyGuardUpgradeable already extends Initializable 26: contract RestakeManager is Initializable, ReentrancyGuardUpgradeable, RestakeManagerStorageV2 {
/// @audit ReentrancyGuardUpgradeable already extends Initializable 16: contract RewardHandler is Initializable, ReentrancyGuardUpgradeable, RewardHandlerStorageV1 {
/// @audit PausableUpgradeable already extends Initializable 11: contract WithdrawQueue is 12: Initializable, 13: PausableUpgradeable, 14: ReentrancyGuardUpgradeable, 15: WithdrawQueueStorageV1
</details>/// @audit ERC20Upgradeable already extends Initializable 13: contract EzEthToken is Initializable, ERC20Upgradeable, IEzEthToken, EzEthTokenStorageV1 {
override is unnecessaryStarting from Solidity 0.8.8, the override keyword is not required when overriding an interface function, except for the case where the function is defined in multiple bases.
<details> <summary>There are 12 instances (click to show):</summary>66: function _ccipReceive( 67: Client.Any2EVMMessage memory any2EvmMessage 68: ) internal override whenNotPaused {
310: function updatePrice(uint256 _price, uint256 _timestamp) external override { 456: function getRate() external view override returns (uint256) {
48: function supportsInterface( 49: bytes4 interfaceId 50: ) public view override(ERC165Upgradeable) returns (bool) { 56: function remoteToken() public view override returns (address) { 60: function bridge() public view override returns (address) {
411: function onERC721Received( 412: address, 413: address, 414: uint256, 415: bytes memory 416: ) public virtual override returns (bytes4) { 423: function onERC1155Received( 424: address, 425: address, 426: uint256, 427: uint256, 428: bytes memory 429: ) public virtual override returns (bytes4) { 436: function onERC1155BatchReceived( 437: address, 438: address, 439: uint256[] memory, 440: uint256[] memory, 441: bytes memory 442: ) public virtual override returns (bytes4) {
</details>56: function _beforeTokenTransfer( 57: address from, 58: address to, 59: uint256 amount 60: ) internal virtual override { 77: function name() public view virtual override returns (string memory) { 85: function symbol() public view virtual override returns (string memory) {
structsNote that there may be cases where a struct superficially appears to be used, but this is only because there are multiple definitions of the struct in different files. In such cases, the struct definition should be moved into a separate file. The instances below are the unused definitions.
There are 2 instances:
64: struct ExecuteArgs {
9: struct TokenId {
revert()/require()Custom errors are available from solidity version 0.8.4. Custom errors are more easily processed in try-catch blocks, and are easier to re-use and maintain.
There are 12 instances:
267: require(targets.length == values.length, "TimelockController: length mismatch"); 268: require(targets.length == payloads.length, "TimelockController: length mismatch"); 284: require(!isOperation(id), "TimelockController: operation already scheduled"); 285: require(delay >= getMinDelay(), "TimelockController: insufficient delay"); 297: require(isOperationPending(id), "TimelockController: operation cannot be cancelled"); 349: require(targets.length == values.length, "TimelockController: length mismatch"); 350: require(targets.length == payloads.length, "TimelockController: length mismatch"); 370: require(success, "TimelockController: underlying transaction reverted"); 377: require(isOperationReady(id), "TimelockController: operation is not ready"); 378: require( 379: predecessor == bytes32(0) || isOperationDone(predecessor), 380: "TimelockController: missing dependency" 381: ); 388: require(isOperationReady(id), "TimelockController: operation is not ready"); 403: require(msg.sender == address(this), "TimelockController: caller must be timelock");
function func(address a, address) -> function func(address a, address /* b */)
139: function xReceive( 140: bytes32 _transferId, 141: uint256 _amount, 142: address _asset, 143: address _originSender, 144: uint32 _origin, 145: bytes memory 146: ) external nonReentrant returns (bytes memory) {
69: function xReceive( 70: bytes32 _transferId, 71: uint256, 72: address, 73: address _originSender, 74: uint32 _origin, 75: bytes memory _callData 76: ) external onlySource(_originSender, _origin) whenNotPaused returns (bytes memory) {
42: function getRoundData(uint80) external pure returns (uint80, int256, uint256, uint256, uint80) {
42: function getRoundData(uint80) external pure returns (uint80, int256, uint256, uint256, uint80) {
</details>411: function onERC721Received( 412: address, 413: address, 414: uint256, 415: bytes memory 416: ) public virtual override returns (bytes4) { 423: function onERC1155Received( 424: address, 425: address, 426: uint256, 427: uint256, 428: bytes memory 429: ) public virtual override returns (bytes4) { 436: function onERC1155BatchReceived( 437: address, 438: address, 439: uint256[] memory, 440: uint256[] memory, 441: bytes memory 442: ) public virtual override returns (bytes4) {
While the compiler automatically provides a getter for accessing single elements within a public state variable array, it doesn't provide a way to fetch the whole array, or subsets thereof. Consider adding a function to allow the fetching of slices of the array, especially if the contract doesn't already have multicall functionality.
There are 3 instances:
42: IOperatorDelegator[] public operatorDelegators; 49: IERC20[] public collateralTokens;
50: mapping(address => WithdrawRequest[]) public withdrawRequests;
Assign the expression's parts to intermediate local variables, and check against those instead.
There is 1 instance:
111: if ((_baseToken == address(0) && !_isNative) || (_isNative && _baseToken != address(0))) {
Doing so will significantly increase centralization, but will help to prevent hackers from using stolen tokens
<details> <summary>There are 8 instances (click to show):</summary>30: contract LockboxAdapterBlast {
16: contract xRenzoBridge is
27: contract xRenzoDeposit is
11: contract XERC20Lockbox is Initializable, IXERC20Lockbox {
17: contract OperatorDelegator is
10: contract DepositQueue is Initializable, ReentrancyGuardUpgradeable, DepositQueueStorageV2 {
26: contract RestakeManager is Initializable, ReentrancyGuardUpgradeable, RestakeManagerStorageV2 {
</details>11: contract WithdrawQueue is
Consider defining in only one contract so that values cannot become out of sync when only one location is updated. A cheap way to store constants/immutables in a single location is to create an internal constant in a library. If the variable is a local cache of another contract's value, consider making the cache variable internal or private, which will require external users to query the contract with the source of truth, so that callers don't get out of sync.
/// @audit Seen in contracts/Bridge/L2/xRenzoDeposit.sol#37 61: uint8 public constant EXPECTED_DECIMALS = 18;
/// @audit Seen in contracts/Oracle/RenzoOracle.sol#26 13: uint256 public constant MAX_TIME_WINDOW = 86400 + 60; // 24 hours + 60 seconds
/// @audit Seen in contracts/Bridge/L1/xRenzoBridge.sol#61 37: uint8 public constant EXPECTED_DECIMALS = 18;
/// @audit Seen in contracts/Deposits/DepositQueue.sol#13 26: address public constant IS_NATIVE = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE;
/// @audit Seen in contracts/Withdraw/WithdrawQueueStorage.sol#10 13: address public constant IS_NATIVE = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE;
/// @audit Seen in contracts/Bridge/L2/Oracle/RenzoOracleL2.sol#13 26: uint256 constant MAX_TIME_WINDOW = 86400 + 60; // 24 hours + 60 seconds
</details>/// @audit Seen in contracts/Deposits/DepositQueue.sol#13 10: address public constant IS_NATIVE = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE;
Keeping each contract in a separate file makes it easier to work with multiple people, makes the code easier to maintain, and is a common practice on most projects. The following files each contains more than one contract/library/interface.
There are 6 instances:
According to the Solidity Style Guide, contract and library names should also match their filenames.
<details> <summary>There are 10 instances (click to show):</summary>/// @audit Not match filename `xRenzoBridgeStorage.sol` 18: abstract contract xRenzoBridgeStorageV1 is IxRenzoBridge {
/// @audit Not match filename `RenzoOracleL2Storage.sol` 7: abstract contract RenzoOracleL2StorageV1 is IRenzoOracleL2 {
/// @audit Not match filename `CCIPReceiver.sol` 14: contract Receiver is CCIPReceiver, Ownable, Pausable {
/// @audit Not match filename `WBETHShimStorage.sol` 7: abstract contract WBETHShimStorageV1 is AggregatorV3Interface {
/// @audit Not match filename `METHShimStorage.sol` 7: abstract contract METHShimStorageV1 is AggregatorV3Interface {
/// @audit Not match filename `RenzoOracleStorage.sol` 8: abstract contract RenzoOracleStorageV1 {
/// @audit Not match filename `BalancerRateProviderStorage.sol` 7: abstract contract BalancerRateProviderStorageV1 {
/// @audit Not match filename `RewardHandlerStorage.sol` 6: abstract contract RewardHandlerStorageV1 {
/// @audit Not match filename `WithdrawQueueStorage.sol` 9: abstract contract WithdrawQueueStorageV1 {
</details>/// @audit Not match filename `EzEthTokenStorage.sol` 10: contract EzEthTokenStorageV1 {
AccessControlDefaultAdminRules rather than AccessControlThe AccessControlDefaultAdminRules implements multiple security best practices on top of the normal AccessControl rules, so consider using it instead.
There is 1 instance:
25: contract TimelockController is AccessControl, IERC721Receiver, IERC1155Receiver {
constant/immutable variablesIf the variable needs to be different based on which class it comes from, a view/pure function should be used instead (e.g. like this).
There are 4 instances:
31: address public FACTORY;
18: IXERC20 public XERC20; 23: IERC20 public ERC20; 29: bool public IS_NATIVE;
This will allow users to easily exactly pinpoint when and by whom a contract was constructed.
<details> <summary>There are 22 instances (click to show):</summary>39: constructor(address _blastStandardBridge, address _registry) {
65: constructor() {
19: constructor() {
43: constructor( 44: address _router, 45: address _xRenzoBridgeL1, 46: uint64 _ccipEthChainSelector 47: ) CCIPReceiver(_router) {
52: constructor(address _connext, address _xRenzoBridgeL1, uint32 _connextEthChainDomain) {
59: constructor() {
50: constructor() {
44: constructor() {
33: constructor() {
24: constructor() {
19: constructor() {
69: constructor() {
69: constructor() {
18: constructor() {
18: constructor() {
39: constructor() {
17: constructor() {
12: constructor() {
96: constructor() {
33: constructor() {
57: constructor() {
</details>28: constructor() {
Empty function body in solidity is not recommended, consider adding some comments to the body.
There are 3 instances:
313: receive() external payable {}
542: receive() external payable {}
137: receive() external payable {}
When an action is triggered based on a user's action, not being able to filter based on who triggered the action makes event processing a lot more cumbersome. Including the msg.sender the events of these types of action will make events much more useful to end users, especially when msg.sender is not tx.origin.
250: emit MessageSent( 251: messageId, 252: _destinationParam[i].destinationChainSelector, 253: _destinationParam[i]._renzoReceiver, 254: exchangeRate, 255: address(linkToken), 256: fees 257: ); 275: emit ConnextMessageSent( 276: _connextDestinationParam[i].destinationDomainId, 277: _connextDestinationParam[i]._renzoReceiver, 278: exchangeRate, 279: _connextDestinationParam[i].relayerFee 280: );
125: emit LockboxSet(_lockbox);
91: emit XERC20Deployed(_xerc20); 120: emit LockboxDeployed(_lockbox);
126: emit Withdraw(_to, _amount); 151: emit Deposit(_to, _amount);
56: emit XERC20Deployed(_xerc20);
523: emit RewardsForwarded(destination, remainingAmount);
155: emit FullWithdrawalETHReceived(msg.value); 171: emit ProtocolFeesPaid(IERC20(address(0x0)), feeAmount, feeAddress); 182: emit RewardsDeposited(IERC20(address(0x0)), remainingRewards);
404: emit MinDelayChange(_minDelay, newDelay);
</details>183: emit EthBufferFilled(msg.value); 198: emit ERC20BufferFilled(_asset, _amount); 311: emit WithdrawRequestClaimed(_withdrawRequest);
Source files are using different floating version syntax, this is prone to compilation errors, and is not conducive to the code reliability and maintainability.
There are 4 instances:
2: pragma solidity ^0.8.13;
2: pragma solidity >=0.8.4 <0.9.0;
4: pragma solidity ^0.8.0;
2: pragma solidity ^0.8.9;
Function state mutability can be restricted to pure
<details> <summary>There are 5 instances (click to show):</summary>411: function onERC721Received( 412: address, 413: address, 414: uint256, 415: bytes memory 416: ) public virtual override returns (bytes4) { 423: function onERC1155Received( 424: address, 425: address, 426: uint256, 427: uint256, 428: bytes memory 429: ) public virtual override returns (bytes4) { 436: function onERC1155BatchReceived( 437: address, 438: address, 439: uint256[] memory, 440: uint256[] memory, 441: bytes memory 442: ) public virtual override returns (bytes4) {
</details>77: function name() public view virtual override returns (string memory) { 85: function symbol() public view virtual override returns (string memory) {
The contract's interface should be imported first, followed by each of the interfaces it uses, followed by all other files. The examples below do not follow this layout.
<details> <summary>There are 20 instances (click to show):</summary>/// @audit Out of order with the prev import️ ⬆ 5: import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
/// @audit Out of order with the prev import️ ⬆ 7: import { 8: IERC20MetadataUpgradeable 9: } from "@openzeppelin/contracts-upgradeable/token/ERC20/extensions/IERC20MetadataUpgradeable.sol";
/// @audit Out of order with the prev import️ ⬆ 6: import "../../IRestakeManager.sol";
/// @audit Out of order with the prev import️ ⬆ 9: import { 10: IERC20MetadataUpgradeable 11: } from "@openzeppelin/contracts-upgradeable/token/ERC20/extensions/IERC20MetadataUpgradeable.sol"; /// @audit Out of order with the prev import️ ⬆ 13: import "../xERC20/interfaces/IXERC20.sol"; /// @audit Out of order with the prev import️ ⬆ 16: import "../../RateProvider/IRateProvider.sol";
/// @audit Out of order with the prev import️ ⬆ 6: import "../Connext/core/IConnext.sol";
/// @audit Out of order with the prev import️ ⬆ 8: import { IXERC20Lockbox } from "../interfaces/IXERC20Lockbox.sol";
/// @audit Out of order with the prev import️ ⬆ 8: import { IOptimismMintableERC20 } from "../../interfaces/IOptimismMintableERC20.sol";
/// @audit Out of order with the prev import️ ⬆ 6: import "../Permissions/IRoleManager.sol"; /// @audit Out of order with the prev import️ ⬆ 8: import "../EigenLayer/interfaces/IDelegationManager.sol";
/// @audit Out of order with the prev import️ ⬆ 5: import "../Permissions/IRoleManager.sol"; /// @audit Out of order with the prev import️ ⬆ 7: import "./IRenzoOracle.sol";
/// @audit Out of order with the prev import️ ⬆ 5: import "./IRoleManager.sol";
/// @audit Out of order with the prev import️ ⬆ 5: import "./IRateProvider.sol";
/// @audit Out of order with the prev import️ ⬆ 8: import "@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol"; /// @audit Out of order with the prev import️ ⬆ 12: import "./EigenLayer/interfaces/IStrategy.sol";
/// @audit Out of order with the prev import️ ⬆ 7: import "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";
/// @audit Out of order with the prev import️ ⬆ 9: import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
</details>/// @audit Out of order with the prev import️ ⬆ 6: import "./IEzEthToken.sol";
falseSince the bool variables are automatically set to false when created, it is redundant to initialize it with false again.
There are 3 instances:
195: bool foundOd = false; 283: bool withdrawQueueTokenBalanceRecorded = false; 627: bool found = false;
I prefix in the contract nameInterfaces should be indicated with an I prefix in the contract name
There is 1 instance:
17: interface L1StandardBridge {
NatSpec comment in solidity should use /// or /* ... */ syntax.
There is 1 instance:
99: 100: // @dev Given list of tokens and balances, return total value (assumes all lookups are denomintated in same underlying currency)
These contracts import contracts from @openzeppelin/contracts, but they are not using the latest version. Using the latest version ensures contract security with fixes for known vulnerabilities, access to the latest feature updates, and enhanced community support and engagement.
The imported version is ^4.9.3.
4: import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; 5: import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
5: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; 7: import { 14: import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";
5: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
5: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; 6: import {
8: import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol"; 9: import { Pausable } from "@openzeppelin/contracts/security/Pausable.sol";
5: import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol"; 6: import { Pausable } from "@openzeppelin/contracts/security/Pausable.sol";
5: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; 6: import { 9: import { 15: import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";
5: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
5: import { 8: import { 11: import { 14: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
8: import { 11: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; 12: import "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";
5: import { IERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; 6: import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; 7: import { SafeCast } from "@openzeppelin/contracts/utils/math/SafeCast.sol"; 9: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
4: import {
8: import { 11: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; 12: import "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";
4: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; 5: import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";
4: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; 5: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; 7: import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";
4: import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol"; 5: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
4: import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol"; 5: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
4: import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";
6: import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
4: import "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol";
4: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
4: import "@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol";
4: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; 5: import "@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.sol"; 6: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; 7: import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol"; 8: import "@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol"; 9: import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; 10: import "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol";
4: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; 6: import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";
6: import "@openzeppelin/contracts/access/AccessControl.sol"; 7: import "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol"; 8: import "@openzeppelin/contracts/token/ERC1155/IERC1155Receiver.sol";
6: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; 7: import "@openzeppelin/contracts-upgradeable/security/PausableUpgradeable.sol"; 8: import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol"; 9: import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
</details>4: import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol"; 5: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
These contracts import contracts from @openzeppelin/contracts-upgradeable, but they are not using the latest version. Using the latest version ensures contract security with fixes for known vulnerabilities, access to the latest feature updates, and enhanced community support and engagement.
The imported version is ^4.9.3.
5: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; 7: import { 14: import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";
5: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; 6: import {
5: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; 6: import { 9: import { 15: import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";
5: import { 8: import { 11: import { 14: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
8: import { 11: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
9: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
4: import {
8: import { 11: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
5: import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";
4: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; 7: import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";
4: import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol"; 5: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
4: import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol"; 5: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
4: import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";
4: import "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol";
4: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
4: import "@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol";
5: import "@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.sol"; 6: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; 7: import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol"; 8: import "@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol";
4: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; 6: import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";
7: import "@openzeppelin/contracts-upgradeable/security/PausableUpgradeable.sol"; 8: import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";
</details>4: import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol"; 5: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
immutable rather than constantWhile it doesn't save any gas because the compiler knows that developers often make this mistake, it's still best to use the right tool for the task at hand. There is a difference between constant variables and immutable variables, and they should each be used in their appropriate contexts. constants should be used for literal values written into the code, and immutable variables should be used for expressions, or values calculated in, or passed into the constructor.
11: bytes32 public constant RX_ETH_MINTER_BURNER = keccak256("RX_ETH_MINTER_BURNER"); 14: bytes32 public constant OPERATOR_DELEGATOR_ADMIN = keccak256("OPERATOR_DELEGATOR_ADMIN"); 17: bytes32 public constant ORACLE_ADMIN = keccak256("ORACLE_ADMIN"); 20: bytes32 public constant RESTAKE_MANAGER_ADMIN = keccak256("RESTAKE_MANAGER_ADMIN"); 23: bytes32 public constant TOKEN_ADMIN = keccak256("TOKEN_ADMIN"); 26: bytes32 public constant NATIVE_ETH_RESTAKE_ADMIN = keccak256("NATIVE_ETH_RESTAKE_ADMIN"); 29: bytes32 public constant ERC20_REWARD_ADMIN = keccak256("ERC20_REWARD_ADMIN"); 32: bytes32 public constant DEPOSIT_WITHDRAW_PAUSER = keccak256("DEPOSIT_WITHDRAW_PAUSER"); 39: bytes32 public constant BRIDGE_ADMIN = keccak256("BRIDGE_ADMIN"); 42: bytes32 public constant PRICE_FEED_SENDER = keccak256("PRICE_FEED_SENDER"); 47: bytes32 public constant WITHDRAW_QUEUE_ADMIN = keccak256("WITHDRAW_QUEUE_ADMIN");
</details>26: bytes32 public constant TIMELOCK_ADMIN_ROLE = keccak256("TIMELOCK_ADMIN_ROLE"); 27: bytes32 public constant PROPOSER_ROLE = keccak256("PROPOSER_ROLE"); 28: bytes32 public constant EXECUTOR_ROLE = keccak256("EXECUTOR_ROLE"); 29: bytes32 public constant CANCELLER_ROLE = keccak256("CANCELLER_ROLE");
Moving duplicate strings to constants can improve code maintainability and readability.
There are 10 instances:
267: require(targets.length == values.length, "TimelockController: length mismatch"); 268: require(targets.length == payloads.length, "TimelockController: length mismatch"); 349: require(targets.length == values.length, "TimelockController: length mismatch"); 350: require(targets.length == payloads.length, "TimelockController: length mismatch"); 377: require(isOperationReady(id), "TimelockController: operation is not ready"); 388: require(isOperationReady(id), "TimelockController: operation is not ready");
36: __ERC20_init("ezETH", "Renzo Restaked ETH"); 36: __ERC20_init("ezETH", "Renzo Restaked ETH"); 78: return "Renzo Restaked ETH"; 86: return "ezETH";
Consider using a library instead of a contract to provide utility functions.
There are 3 instances:
9: contract RoleManagerStorageV1 { 37: contract RoleManagerStorageV2 is RoleManagerStorageV1 { 45: contract RoleManagerStorageV3 is RoleManagerStorageV2 {
It is recommended by the Solidity Style Guide
There is 1 instance:
15: error OptimismMintableXERC20Factory_NoBridges();
According to the Solidity Style Guide, contracts and libraries should be named using the CapWords style.
There are 6 instances:
16: contract xRenzoBridge is
18: abstract contract xRenzoBridgeStorageV1 is IxRenzoBridge {
27: contract xRenzoDeposit is
9: abstract contract xRenzoDepositStorageV1 is IxRenzoDeposit { 47: abstract contract xRenzoDepositStorageV2 is xRenzoDepositStorageV1 { 52: abstract contract xRenzoDepositStorageV3 is xRenzoDepositStorageV2 {
As the Solidity Style Guide suggests: functions should be named in mixedCase style.
<details> <summary>There are 9 instances (click to show):</summary>96: function updateXRenzoBridgeL1(address _newXRenzoBridgeL1) external onlyOwner { 107: function updateCCIPEthChainSelector(uint64 _newChainSelector) external onlyOwner {
92: function updateXRenzoBridgeL1(address _newXRenzoBridgeL1) external onlyOwner { 103: function updateCCIPEthChainSelector(uint32 _newChainDomain) external onlyOwner {
76: function __XERC20_init(
69: function _getWBETHData()
69: function _getMETHData()
</details>215: function setMaxDepositTVL(uint256 _maxDepositTVL) external onlyRestakeManagerAdmin { 274: function calculateTVLs() public view returns (uint256[][] memory, uint256[] memory, uint256) {
immutables should use UPPER_CASE_WITH_UNDERSCORESFor immutable variable names, each word should use all capital letters, with underscores separating each word (UPPER_CASE_WITH_UNDERSCORES)
There are 2 instances:
31: address immutable blastStandardBridge; 32: address immutable registry;
Consider adding named imports for all parent contracts.
<details> <summary>There are 59 instances (click to show):</summary>/// @audit IXReceiver /// @audit Initializable /// @audit ReentrancyGuardUpgradeable /// @audit xRenzoBridgeStorageV1 16: contract xRenzoBridge is 17: IXReceiver, 18: Initializable, 19: ReentrancyGuardUpgradeable, 20: xRenzoBridgeStorageV1
/// @audit IxRenzoBridge 18: abstract contract xRenzoBridgeStorageV1 is IxRenzoBridge {
/// @audit Initializable /// @audit RenzoOracleL2StorageV1 11: contract RenzoOracleL2 is Initializable, OwnableUpgradeable, RenzoOracleL2StorageV1 {
/// @audit IRenzoOracleL2 7: abstract contract RenzoOracleL2StorageV1 is IRenzoOracleL2 {
/// @audit Initializable /// @audit ReentrancyGuardUpgradeable /// @audit IRateProvider /// @audit xRenzoDepositStorageV3 27: contract xRenzoDeposit is 28: Initializable, 29: OwnableUpgradeable, 30: ReentrancyGuardUpgradeable, 31: IRateProvider, 32: xRenzoDepositStorageV3
/// @audit IxRenzoDeposit 9: abstract contract xRenzoDepositStorageV1 is IxRenzoDeposit {
/// @audit Initializable 16: contract XERC20 is 17: Initializable, 18: ERC20Upgradeable, 19: OwnableUpgradeable, 20: IXERC20, 21: ERC20PermitUpgradeable
/// @audit Initializable 14: contract XERC20Factory is Initializable, IXERC20Factory {
/// @audit Initializable 11: contract XERC20Lockbox is Initializable, IXERC20Lockbox {
/// @audit Initializable 14: contract OptimismMintableXERC20Factory is Initializable, XERC20Factory {
/// @audit Initializable /// @audit ReentrancyGuardUpgradeable /// @audit OperatorDelegatorStorageV4 17: contract OperatorDelegator is 18: Initializable, 19: ReentrancyGuardUpgradeable, 20: OperatorDelegatorStorageV4
/// @audit IOperatorDelegator 16: abstract contract OperatorDelegatorStorageV1 is IOperatorDelegator {
/// @audit Initializable /// @audit ReentrancyGuardUpgradeable /// @audit DepositQueueStorageV2 10: contract DepositQueue is Initializable, ReentrancyGuardUpgradeable, DepositQueueStorageV2 {
/// @audit IDepositQueue 9: abstract contract DepositQueueStorageV1 is IDepositQueue {
/// @audit Initializable /// @audit WBETHShimStorageV1 15: contract WBETHShim is Initializable, WBETHShimStorageV1 {
/// @audit AggregatorV3Interface 7: abstract contract WBETHShimStorageV1 is AggregatorV3Interface {
/// @audit Initializable /// @audit METHShimStorageV1 15: contract METHShim is Initializable, METHShimStorageV1 {
/// @audit AggregatorV3Interface 7: abstract contract METHShimStorageV1 is AggregatorV3Interface {
/// @audit IRenzoOracle /// @audit Initializable /// @audit ReentrancyGuardUpgradeable /// @audit RenzoOracleStorageV1 13: contract RenzoOracle is 14: IRenzoOracle, 15: Initializable, 16: ReentrancyGuardUpgradeable, 17: RenzoOracleStorageV1
/// @audit IRoleManager /// @audit AccessControlUpgradeable /// @audit RoleManagerStorageV3 14: contract RoleManager is IRoleManager, AccessControlUpgradeable, RoleManagerStorageV3 {
/// @audit Initializable /// @audit IRateProvider /// @audit BalancerRateProviderStorageV1 9: contract BalancerRateProvider is Initializable, IRateProvider, BalancerRateProviderStorageV1 {
/// @audit Initializable /// @audit ReentrancyGuardUpgradeable /// @audit RestakeManagerStorageV2 26: contract RestakeManager is Initializable, ReentrancyGuardUpgradeable, RestakeManagerStorageV2 {
/// @audit IRestakeManager 15: abstract contract RestakeManagerStorageV1 is IRestakeManager {
/// @audit Initializable /// @audit ReentrancyGuardUpgradeable /// @audit RewardHandlerStorageV1 16: contract RewardHandler is Initializable, ReentrancyGuardUpgradeable, RewardHandlerStorageV1 {
/// @audit AccessControl /// @audit IERC721Receiver /// @audit IERC1155Receiver 25: contract TimelockController is AccessControl, IERC721Receiver, IERC1155Receiver {
/// @audit Initializable /// @audit PausableUpgradeable /// @audit ReentrancyGuardUpgradeable /// @audit WithdrawQueueStorageV1 11: contract WithdrawQueue is 12: Initializable, 13: PausableUpgradeable, 14: ReentrancyGuardUpgradeable, 15: WithdrawQueueStorageV1
</details>/// @audit Initializable /// @audit ERC20Upgradeable /// @audit IEzEthToken /// @audit EzEthTokenStorageV1 13: contract EzEthToken is Initializable, ERC20Upgradeable, IEzEthToken, EzEthTokenStorageV1 {
Putting constants on the left side of comparison statements is a best practice known as Yoda conditions. Although solidity's static typing system prevents accidental assignments within conditionals, adopting this practice can improve code readability and consistency, especially when working across multiple languages.
<details> <summary>There are 86 instances (click to show):</summary>/// @audit put `address(0)` on the left 41: if (_blastStandardBridge == address(0) || _registry == address(0)) { /// @audit put `address(0)` on the left 41: if (_blastStandardBridge == address(0) || _registry == address(0)) { /// @audit put `0` on the left 65: if (_amount <= 0) { /// @audit put `address(0)` on the left 73: if (xerc20 == address(0) || lockbox == address(0)) { /// @audit put `address(0)` on the left 73: if (xerc20 == address(0) || lockbox == address(0)) {
/// @audit put `EXPECTED_DECIMALS` on the left 100: if (decimals != EXPECTED_DECIMALS) { /// @audit put `EXPECTED_DECIMALS` on the left 104: if (decimals != EXPECTED_DECIMALS) { /// @audit put `EXPECTED_DECIMALS` on the left 108: if (decimals != EXPECTED_DECIMALS) { /// @audit put `EXPECTED_DECIMALS` on the left 112: if (decimals != EXPECTED_DECIMALS) { /// @audit put `0` on the left 158: if (_amount == 0) {
/// @audit put `address(0)` on the left 48: if (_xRenzoBridgeL1 == address(0) || _ccipEthChainSelector == 0) revert InvalidZeroInput(); /// @audit put `0` on the left 48: if (_xRenzoBridgeL1 == address(0) || _ccipEthChainSelector == 0) revert InvalidZeroInput(); /// @audit put `address(0)` on the left 97: if (_newXRenzoBridgeL1 == address(0)) revert InvalidZeroInput(); /// @audit put `0` on the left 108: if (_newChainSelector == 0) revert InvalidZeroInput(); /// @audit put `address(0)` on the left 135: if (_newXRenzoDeposit == address(0)) revert InvalidZeroInput();
/// @audit put `address(0)` on the left 53: if (_xRenzoBridgeL1 == address(0) || _connextEthChainDomain == 0 || _connext == address(0)) /// @audit put `address(0)` on the left 53: if (_xRenzoBridgeL1 == address(0) || _connextEthChainDomain == 0 || _connext == address(0)) /// @audit put `0` on the left 53: if (_xRenzoBridgeL1 == address(0) || _connextEthChainDomain == 0 || _connext == address(0)) /// @audit put `address(0)` on the left 93: if (_newXRenzoBridgeL1 == address(0)) revert InvalidZeroInput(); /// @audit put `0` on the left 104: if (_newChainDomain == 0) revert InvalidZeroInput(); /// @audit put `address(0)` on the left 131: if (_newXRenzoDeposit == address(0)) revert InvalidZeroInput();
/// @audit put `0` on the left 92: _currentPrice == 0 || /// @audit put `0` on the left 97: _swapKey == 0 || /// @audit put `0` on the left 98: _bridgeDestinationDomain == 0 || /// @audit put `address(0)` on the left 99: _bridgeTargetAddress == address(0) /// @audit put `EXPECTED_DECIMALS` on the left 106: if (decimals != EXPECTED_DECIMALS) { /// @audit put `EXPECTED_DECIMALS` on the left 110: if (decimals != EXPECTED_DECIMALS) { /// @audit put `EXPECTED_DECIMALS` on the left 114: if (decimals != EXPECTED_DECIMALS) { /// @audit put `0` on the left 172: if (msg.value == 0) { /// @audit put `0` on the left 186: if (wrappedAmount == 0) { /// @audit put `0` on the left 209: if (_amountIn == 0) { /// @audit put `0` on the left 240: if (amountOut == 0) { /// @audit put `address(0)` on the left 291: if (receiver == address(0) && address(oracle) == address(0)) revert PriceFeedNotAvailable(); /// @audit put `0` on the left 332: if (_price == 0) { /// @audit put `0` on the left 424: if (balance == 0) {
/// @audit put `0` on the left 330: if (_amount == 0) revert IXERC20_INVALID_0_VALUE(); /// @audit put `0` on the left 350: if (_amount == 0) revert IXERC20_INVALID_0_VALUE();
/// @audit put `address(0)` on the left 111: if ((_baseToken == address(0) && !_isNative) || (_isNative && _baseToken != address(0))) { /// @audit put `address(0)` on the left 111: if ((_baseToken == address(0) && !_isNative) || (_isNative && _baseToken != address(0))) { /// @audit put `address(0)` on the left 116: if (_lockboxRegistry[_xerc20] != address(0)) revert IXERC20Factory_LockboxAlreadyDeployed();
/// @audit put `0` on the left 135: if (_baseGasAmountSpent == 0) revert InvalidZeroInput(); /// @audit put `0` on the left 147: if (address(tokenStrategyMapping[token]) == address(0x0) || tokenAmount == 0) /// @audit put `0` on the left 329: queuedShares[address(this)] == 0 /// @audit put `0` on the left 514: if (remainingAmount == 0) {
/// @audit put `address(0x0)` on the left 99: if (_feeAddress == address(0x0)) revert InvalidZeroInput(); /// @audit put `0` on the left 138: if (_amount == 0 || _asset == address(0)) revert InvalidZeroInput(); /// @audit put `address(0)` on the left 138: if (_amount == 0 || _asset == address(0)) revert InvalidZeroInput(); /// @audit put `address(0x0)` on the left 166: if (feeAddress != address(0x0) && feeBasisPoints > 0) { /// @audit put `address(0x0)` on the left 260: if (feeAddress != address(0x0) && feeBasisPoints > 0) {
/// @audit put `0` on the left 77: if (price <= 0) revert InvalidOraclePrice(); /// @audit put `0` on the left 94: if (price <= 0) revert InvalidOraclePrice(); /// @audit put `0` on the left 130: if (_currentValueInProtocol == 0 || _existingEzETHSupply == 0) { /// @audit put `0` on the left 130: if (_currentValueInProtocol == 0 || _existingEzETHSupply == 0) { /// @audit put `0` on the left 146: if (mintAmount == 0) revert InvalidTokenAmount(); /// @audit put `0` on the left 161: if (redeemAmount == 0) revert InvalidTokenAmount();
/// @audit put `0` on the left 37: if (totalSupply == 0 || totalTVL == 0) revert InvalidZeroInput(); /// @audit put `0` on the left 37: if (totalSupply == 0 || totalTVL == 0) revert InvalidZeroInput();
/// @audit put `0` on the left 367: if (operatorDelegators.length == 0) revert NotFound(); /// @audit put `1` on the left 370: if (operatorDelegators.length == 1) { /// @audit put `1` on the left 408: if (operatorDelegators.length == 1) { /// @audit put `0` on the left 510: if (maxDepositTVL != 0 && totalTVL + collateralTokenValue > maxDepositTVL) { /// @audit put `0` on the left 515: if (collateralTokenTvlLimits[_collateralToken] != 0) { /// @audit put `0` on the left 597: if (maxDepositTVL != 0 && totalTVL + msg.value > maxDepositTVL) {
/// @audit put `0` on the left 64: if (balance == 0) {
/// @audit put `address(0)` on the left 102: if (admin != address(0)) { /// @audit put `bytes32(0)` on the left 245: if (salt != bytes32(0)) { /// @audit put `bytes32(0)` on the left 275: if (salt != bytes32(0)) { /// @audit put `bytes32(0)` on the left 379: predecessor == bytes32(0) || isOperationDone(predecessor),
/// @audit put `0` on the left 77: _withdrawalBufferTarget.length == 0 || /// @audit put `0` on the left 78: _coolDownPeriod == 0 /// @audit put `address(0)` on the left 90: _withdrawalBufferTarget[i].asset == address(0) || /// @audit put `0` on the left 91: _withdrawalBufferTarget[i].bufferAmount == 0 /// @audit put `0` on the left 109: if (_newBufferTarget.length == 0) revert InvalidZeroInput(); /// @audit put `address(0)` on the left 111: if (_newBufferTarget[i].asset == address(0) || _newBufferTarget[i].bufferAmount == 0) /// @audit put `0` on the left 111: if (_newBufferTarget[i].asset == address(0) || _newBufferTarget[i].bufferAmount == 0) /// @audit put `0` on the left 130: if (_newCoolDownPeriod == 0) revert InvalidZeroInput(); /// @audit put `IS_NATIVE` on the left 157: if (_asset != IS_NATIVE) { /// @audit put `address(0)` on the left 196: if (_asset == address(0) || _amount == 0) revert InvalidZeroInput(); /// @audit put `0` on the left 196: if (_asset == address(0) || _amount == 0) revert InvalidZeroInput(); /// @audit put `0` on the left 208: if (_amount == 0 || _assetOut == address(0)) revert InvalidZeroInput(); /// @audit put `address(0)` on the left 208: if (_amount == 0 || _assetOut == address(0)) revert InvalidZeroInput(); /// @audit put `0` on the left 211: if (withdrawalBufferTarget[_assetOut] == 0) revert UnsupportedWithdrawAsset(); /// @audit put `IS_NATIVE` on the left 227: if (_assetOut != IS_NATIVE) { /// @audit put `IS_NATIVE` on the left 302: if (_withdrawRequest.collateralToken == IS_NATIVE) {
</details>/// @audit put `address(0)` on the left 69: if (from != address(0) && to != address(0)) { /// @audit put `address(0)` on the left 69: if (from != address(0) && to != address(0)) {
else-block not requiredOne level of nesting can be removed by not having an else block when the if-block always jumps at the end. For example:
if (condition) { body1... return x; } else { body2... }
can be changed to:
<details> <summary>There are 4 instances (click to show):</summary>if (condition) { body1... return x; } body2...
279: if (_amountIn < sweepBatchSize) { 280: return (_amountIn * bridgeFeeShare) / FEE_BASIS; 281: } else { 292: if (address(oracle) != address(0)) { 293: (uint256 oraclePrice, uint256 oracleTimestamp) = oracle.getMintRate(); 294: return 295: oracleTimestamp > lastPriceTimestamp 296: ? (oraclePrice, oracleTimestamp) 297: : (lastPrice, lastPriceTimestamp); 298: } else {
309: if (_limit == _maxLimit) { 310: return _limit; 311: } else if (_timestamp + _DURATION <= block.timestamp) {
</details>157: if (_asset != IS_NATIVE) { 158: return IERC20(_asset).balanceOf(address(this)) - claimReserve[_asset]; 159: } else {
Use a scientific notation rather than decimal literals (e.g. 1e6 instead of 1000000), for better code readability.
There are 6 instances:
/// @audit 200_000 -> 2e5 225: Client.EVMExtraArgsV1({ gasLimit: 200_000 })
/// @audit 10000 -> 1e4 40: uint32 public constant FEE_BASIS = 10000; /// @audit 10_000 -> 1e4 386: uint256 fee = (amountNextWETH * bridgeRouterFeeBps) / 10_000;
/// @audit 10000 -> 1e4 103: if (_feeBasisPoints > 10000) revert OverMaxBasisPoints(); /// @audit 10000 -> 1e4 167: feeAmount = (msg.value * feeBasisPoints) / 10000; /// @audit 10000 -> 1e4 261: feeAmount = (balance * feeBasisPoints) / 10000;
Consider breaking down these blocks into more manageable units, by splitting things into utility functions, by reducing nesting, and by using early returns.
<details> <summary>There is 1 instance (click to show):</summary></details>400: function chooseOperatorDelegatorForWithdraw( 401: uint256 tokenIndex, 402: uint256 ezETHValue, 403: uint256[][] memory operatorDelegatorTokenTVLs, 404: uint256[] memory operatorDelegatorTVLs, 405: uint256 totalTVL 406: ) public view returns (IOperatorDelegator) { 407: // If there is only one operator delegator, try to use it 408: if (operatorDelegators.length == 1) { 409: // If the OD doesn't have the tokens, revert 410: if (operatorDelegatorTokenTVLs[0][tokenIndex] < ezETHValue) { 411: revert NotFound(); 412: } 413: return operatorDelegators[0]; 414: } 415: 416: // Fnd the operator delegator with TVL above the threshold and with enough tokens 417: uint256 odLength = operatorDelegatorTVLs.length; 418: for (uint256 i = 0; i < odLength; ) { 419: if ( 420: operatorDelegatorTVLs[i] > 421: (operatorDelegatorAllocations[operatorDelegators[i]] * totalTVL) / 422: BASIS_POINTS / 423: BASIS_POINTS && 424: operatorDelegatorTokenTVLs[i][tokenIndex] >= ezETHValue 425: ) { 426: return operatorDelegators[i]; 427: } 428: 429: unchecked { 430: ++i; 431: } 432: } 433: 434: // If not found, just find one with enough tokens 435: for (uint256 i = 0; i < odLength; ) { 436: if (operatorDelegatorTokenTVLs[i][tokenIndex] >= ezETHValue) { 437: return operatorDelegators[i]; 438: } 439: 440: unchecked { 441: ++i; 442: } 443: } 444: 445: // This token cannot be withdrawn 446: revert NotFound(); 447: }
All typos should be corrected.
<details> <summary>There are 19 instances (click to show):</summary>/// @audit amonut 171: // Get the amonut of ezETH before the deposit
/// @audit maxmimum 12: /// @dev The maxmimum staleness allowed for a price feed from chainlink
/// @audit Onwer 93: * @dev This should be a permissioned call (onlyOnwer) /// @audit Onwer 104: * @dev This should be a permissioned call (onlyOnwer) /// @audit Onwer 131: * @dev This should be a permissioned call (onlyOnwer)
/// @audit Onwer 89: * @dev This should be a permissioned call (onlyOnwer) /// @audit Onwer 100: * @dev This should be a permissioned call (onlyOnwer) /// @audit Onwer 127: * @dev This should be a permissioned call (onlyOnwer)
/// @audit funcion 160: * @dev This funcion allows anyone to call and deposit the native asset for xezETH /// @audit funcion 195: * @dev This funcion allows anyone to call and deposit collateral for xezETH /// @audit Onwer 508: * @dev This should be permissioned call (onlyOnwer), can be set to address(0) for not configured /// @audit Onwer 518: * @dev This should be a permissioned call (onlyOnwer)
/// @audit legth 202: // set strategies legth for 0th index only
/// @audit bufffer 150: * @dev check and fill ETH withdraw bufffer if required
/// @audit Errror 46: /// @dev Errror when caller does not have ETH Restake Admin role
/// @audit maxmimum 25: /// @dev The maxmimum staleness allowed for a price feed from chainlink
/// @audit Delgator 42: /// @dev Determines if the specified address has permission to update config on the OperatorDelgator Contracts
/// @audit Delgator 13: /// @dev role for granting capability to update config on the OperatorDelgator Contracts
</details>/// @audit mis 194: // Ensure the OD is in the list to prevent mis-configuration
The functions below take in an unbounded array, and make function calls for entries in the array. While the function will revert if it eventually runs out of gas, it may be a nicer user experience to require() that the length of the array is below some reasonable maximum, so that the user doesn't have to use up a full transaction's gas only to see that the transaction reverts.
<details> <summary>There are 18 instances (click to show):</summary>218: for (uint256 i = 0; i < _destinationParam.length; ) { 264: for (uint256 i = 0; i < _connextDestinationParam.length; ) {
167: for (uint256 _i; _i < _bridgesLength; ++_i) {
109: for (uint256 _i; _i < _bridgesLength; ++_i) {
209: for (uint256 i; i < tokens.length; ) { 277: for (uint256 i; i < tokens.length; ) { 381: for (uint256 i = 0; i < validatorFields.length; ) {
227: for (uint256 i = 0; i < arrayLength; ) {
111: for (uint256 i = 0; i < tokenLength; ) {
376: for (uint256 i = 0; i < tvlLength; ) { 418: for (uint256 i = 0; i < odLength; ) { 435: for (uint256 i = 0; i < odLength; ) {
107: for (uint256 i = 0; i < proposers.length; ++i) { 113: for (uint256 i = 0; i < executors.length; ++i) { 272: for (uint256 i = 0; i < targets.length; ++i) { 355: for (uint256 i = 0; i < targets.length; ++i) {
</details>88: for (uint256 i = 0; i < _withdrawalBufferTarget.length; ) { 110: for (uint256 i = 0; i < _newBufferTarget.length; ) {
Unnecessary castings can be removed.
<details> <summary>There are 8 instances (click to show):</summary>/// @audit address(_delegateAddress) 122: if (address(_delegateAddress) == address(0x0)) revert InvalidZeroInput(); /// @audit address(delegateAddress) 123: if (address(delegateAddress) != address(0x0)) revert DelegateAddressAlreadySet();
/// @audit IERC20(token) 255: uint256 balance = IERC20(token).balanceOf(address(this)); /// @audit IERC20(token) 262: IERC20(token).safeTransfer(feeAddress, feeAmount);
/// @audit address(roleManagerAdmin) 23: if (address(roleManagerAdmin) == address(0x0)) revert InvalidZeroInput();
/// @audit address(withdrawQueue) 355: totalTVL += (address(withdrawQueue).balance + totalWithdrawalQueueValue);
</details>/// @audit address(_rewardDestination) 42: if (address(_rewardDestination) == address(0x0)) revert InvalidZeroInput(); /// @audit address(_rewardDestination) 75: if (address(_rewardDestination) == address(0x0)) revert InvalidZeroInput();
The following state variables are defined but not used. It is recommended to check the code for logical omissions that cause them not to be used. If it's determined that they are not needed anywhere, it's best to remove them from the codebase to improve code clarity and minimize confusion.
There is 1 instance:
20: string constant INVALID_0_INPUT = "Invalid 0 input";
The identifier is imported but never used within the file.
<details> <summary>There are 25 instances (click to show):</summary>/// @audit IXERC20 6: import { IXERC20 } from "../../xERC20/interfaces/IXERC20.sol";
/// @audit IRouterClient 9: import { 10: IRouterClient 11: } from "@chainlink/contracts-ccip/src/v0.8/ccip/interfaces/IRouterClient.sol"; /// @audit IRateProvider 12: import { IRateProvider } from "../../RateProvider/IRateProvider.sol"; /// @audit LinkTokenInterface 13: import { 14: LinkTokenInterface 15: } from "@chainlink/contracts/src/v0.8/shared/interfaces/LinkTokenInterface.sol"; /// @audit IRoleManager 16: import { IRoleManager } from "../../Permissions/IRoleManager.sol";
/// @audit OwnableUpgradeable 6: import { 7: OwnableUpgradeable 8: } from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
/// @audit Client 4: import { Client } from "@chainlink/contracts-ccip/src/v0.8/ccip/libraries/Client.sol"; /// @audit CCIPReceiver 5: import { 6: CCIPReceiver 7: } from "@chainlink/contracts-ccip/src/v0.8/ccip/applications/CCIPReceiver.sol"; /// @audit Ownable 8: import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol"; /// @audit Pausable 9: import { Pausable } from "@openzeppelin/contracts/security/Pausable.sol";
/// @audit IXReceiver 4: import { IXReceiver } from "@connext/interfaces/core/IXReceiver.sol"; /// @audit Ownable 5: import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol"; /// @audit Pausable 6: import { Pausable } from "@openzeppelin/contracts/security/Pausable.sol";
/// @audit OwnableUpgradeable 6: import { 7: OwnableUpgradeable 8: } from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
/// @audit IXERC20 4: import { IXERC20 } from "../interfaces/IXERC20.sol"; /// @audit ERC20Upgradeable 5: import { 6: ERC20Upgradeable 7: } from "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol"; /// @audit ERC20PermitUpgradeable 8: import { 9: ERC20PermitUpgradeable 10: } from "@openzeppelin/contracts-upgradeable/token/ERC20/extensions/ERC20PermitUpgradeable.sol"; /// @audit OwnableUpgradeable 11: import { 12: OwnableUpgradeable 13: } from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
/// @audit IXERC20Factory 4: import { IXERC20Factory } from "../interfaces/IXERC20Factory.sol";
/// @audit SafeERC20 6: import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; /// @audit SafeCast 7: import { SafeCast } from "@openzeppelin/contracts/utils/math/SafeCast.sol"; /// @audit IXERC20Lockbox 8: import { IXERC20Lockbox } from "../interfaces/IXERC20Lockbox.sol";
/// @audit ERC165Upgradeable 4: import { 5: ERC165Upgradeable 6: } from "@openzeppelin/contracts-upgradeable/utils/introspection/ERC165Upgradeable.sol";
</details>/// @audit XERC20Factory 5: import { XERC20Factory } from "../XERC20Factory.sol"; /// @audit XERC20Lockbox 6: import { XERC20Lockbox } from "../XERC20Lockbox.sol";
The following modifiers are not used. It is recommended to check the code for logical omissions that cause them not to be used. If it's determined that they are not needed anywhere, it's best to remove them from the codebase to improve code clarity and minimize confusion.
There is 1 instance:
45: modifier onlyRestakeManager() {
Declaring named returns, but not using them, is confusing to the reader. Consider either completely removing them (by declaring just the type without a name), or remove the return statement and do a variable assignment. This would improve the readability of the code, and it may also help reduce regressions during future code refactors.
<details> <summary>There are 22 instances (click to show):</summary>/// @audit shares 143: function deposit( 144: IERC20 token, 145: uint256 tokenAmount 146: ) external nonReentrant onlyRestakeManager returns (uint256 shares) { /// @audit shares 162: function _deposit(IERC20 _token, uint256 _tokenAmount) internal returns (uint256 shares) {
/// @audit roundId /// @audit answer /// @audit startedAt /// @audit updatedAt /// @audit answeredInRound 46: function latestRoundData() 47: external 48: view 49: returns ( 50: uint80 roundId, 51: int256 answer, 52: uint256 startedAt, 53: uint256 updatedAt, 54: uint80 answeredInRound 55: ) /// @audit roundId /// @audit answer /// @audit startedAt /// @audit updatedAt /// @audit answeredInRound 69: function _getWBETHData() 70: internal 71: view 72: returns ( 73: uint80 roundId, 74: int256 answer, 75: uint256 startedAt, 76: uint256 updatedAt, 77: uint80 answeredInRound 78: )
</details>/// @audit roundId /// @audit answer /// @audit startedAt /// @audit updatedAt /// @audit answeredInRound 46: function latestRoundData() 47: external 48: view 49: returns ( 50: uint80 roundId, 51: int256 answer, 52: uint256 startedAt, 53: uint256 updatedAt, 54: uint80 answeredInRound 55: ) /// @audit roundId /// @audit answer /// @audit startedAt /// @audit updatedAt /// @audit answeredInRound 69: function _getMETHData() 70: internal 71: view 72: returns ( 73: uint80 roundId, 74: int256 answer, 75: uint256 startedAt, 76: uint256 updatedAt, 77: uint80 answeredInRound 78: )
falseThe delete keyword more closely matches the semantics of what is being done, and draws more attention to the changing of state, which may lead to a more thorough audit of its associated logic.
There is 1 instance:
117: paused = false;
delete rather than assigning zero to clear valuesThe delete keyword more closely matches the semantics of what is being done, and draws more attention to the changing of state, which may lead to a more thorough audit of its associated logic.
There are 2 instances:
398: bridgeFeeCollected = 0;
168: operatorDelegatorAllocations[_operatorDelegatorToRemove] = 0;
Upgrading to the latest solidity version (0.8.19 for L2s) can optimize gas usage, take advantage of new features and improve overall contract efficiency. Where possible, based on compatibility requirements, it is recommended to use newer/latest solidity version to take advantage of the latest optimizations and features.
<details> <summary>There are 38 instances (click to show):</summary>2: pragma solidity ^0.8.13;
2: pragma solidity 0.8.19;
2: pragma solidity 0.8.19;
2: pragma solidity 0.8.19;
2: pragma solidity 0.8.19;
2: pragma solidity 0.8.19;
2: pragma solidity 0.8.19;
2: pragma solidity 0.8.19;
2: pragma solidity 0.8.19;
2: pragma solidity >=0.8.4 <0.9.0;
2: pragma solidity >=0.8.4 <0.9.0;
2: pragma solidity >=0.8.4 <0.9.0;
2: pragma solidity >=0.8.4 <0.9.0;
2: pragma solidity >=0.8.4 <0.9.0;
2: pragma solidity 0.8.19;
2: pragma solidity 0.8.19;
2: pragma solidity 0.8.19;
2: pragma solidity 0.8.19;
2: pragma solidity 0.8.19;
2: pragma solidity 0.8.19;
2: pragma solidity 0.8.19;
2: pragma solidity 0.8.19;
2: pragma solidity 0.8.19;
2: pragma solidity 0.8.19;
2: pragma solidity 0.8.19;
2: pragma solidity 0.8.19;
2: pragma solidity 0.8.19;
2: pragma solidity 0.8.19;
2: pragma solidity 0.8.19;
2: pragma solidity 0.8.19;
2: pragma solidity 0.8.19;
2: pragma solidity 0.8.19;
2: pragma solidity 0.8.19;
4: pragma solidity ^0.8.0;
2: pragma solidity 0.8.19;
2: pragma solidity 0.8.19;
2: pragma solidity ^0.8.9;
</details>2: pragma solidity 0.8.19;
Consider using SafeTransferLib.safeTransferETH or Address.sendValue for clearer semantic meaning instead of using a low level call.
403: (bool success, ) = payable(msg.sender).call{ value: feeCollected }("");
131: (bool _success, ) = payable(_to).call{ value: _amount }("");
520: (bool success, ) = destination.call{ value: remainingAmount }("");
168: (bool success, ) = feeAddress.call{ value: feeAmount }(""); 286: (bool success, ) = payable(msg.sender).call{ value: gasRefund }("");
</details>68: (bool success, ) = rewardDestination.call{ value: balance }("");
If a function has too many parameters, replacing them with a struct can improve code readability and maintainability, increase reusability, and reduce the likelihood of errors when passing the parameters.
<details> <summary>There are 9 instances (click to show):</summary>201: function hashOperation( 202: address target, 203: uint256 value, 204: bytes calldata data, 205: bytes32 predecessor, 206: bytes32 salt 207: ) public pure virtual returns (bytes32) { 215: function hashOperationBatch( 216: address[] calldata targets, 217: uint256[] calldata values, 218: bytes[] calldata payloads, 219: bytes32 predecessor, 220: bytes32 salt 221: ) public pure virtual returns (bytes32) { 234: function schedule( 235: address target, 236: uint256 value, 237: bytes calldata data, 238: bytes32 predecessor, 239: bytes32 salt, 240: uint256 delay 241: ) public virtual onlyRole(PROPOSER_ROLE) { 259: function scheduleBatch( 260: address[] calldata targets, 261: uint256[] calldata values, 262: bytes[] calldata payloads, 263: bytes32 predecessor, 264: bytes32 salt, 265: uint256 delay 266: ) public virtual onlyRole(PROPOSER_ROLE) { 315: function execute( 316: address target, 317: uint256 value, 318: bytes calldata payload, 319: bytes32 predecessor, 320: bytes32 salt 321: ) public payable virtual onlyRoleOrOpenRole(EXECUTOR_ROLE) { 342: function executeBatch( 343: address[] calldata targets, 344: uint256[] calldata values, 345: bytes[] calldata payloads, 346: bytes32 predecessor, 347: bytes32 salt 348: ) public payable virtual onlyRoleOrOpenRole(EXECUTOR_ROLE) { 423: function onERC1155Received( 424: address, 425: address, 426: uint256, 427: uint256, 428: bytes memory 429: ) public virtual override returns (bytes4) { 436: function onERC1155BatchReceived( 437: address, 438: address, 439: uint256[] memory, 440: uint256[] memory, 441: bytes memory 442: ) public virtual override returns (bytes4) {
</details>64: function initialize( 65: IRoleManager _roleManager, 66: IRestakeManager _restakeManager, 67: IEzEthToken _ezETH, 68: IRenzoOracle _renzoOracle, 69: uint256 _coolDownPeriod, 70: TokenWithdrawBuffer[] calldata _withdrawalBufferTarget 71: ) external initializer {
If a function returns too many variables, replacing them with a struct can improve code readability, maintainability and reusability.
<details> <summary>There are 7 instances (click to show):</summary>42: function getRoundData(uint80) external pure returns (uint80, int256, uint256, uint256, uint80) { 46: function latestRoundData() 47: external 48: view 49: returns ( 50: uint80 roundId, 51: int256 answer, 52: uint256 startedAt, 53: uint256 updatedAt, 54: uint80 answeredInRound 55: ) 69: function _getWBETHData() 70: internal 71: view 72: returns ( 73: uint80 roundId, 74: int256 answer, 75: uint256 startedAt, 76: uint256 updatedAt, 77: uint80 answeredInRound 78: )
42: function getRoundData(uint80) external pure returns (uint80, int256, uint256, uint256, uint80) { 46: function latestRoundData() 47: external 48: view 49: returns ( 50: uint80 roundId, 51: int256 answer, 52: uint256 startedAt, 53: uint256 updatedAt, 54: uint80 answeredInRound 55: ) 69: function _getMETHData() 70: internal 71: view 72: returns ( 73: uint80 roundId, 74: int256 answer, 75: uint256 startedAt, 76: uint256 updatedAt, 77: uint80 answeredInRound 78: )
</details>274: function calculateTVLs() public view returns (uint256[][] memory, uint256[] memory, uint256) {
This should especially be done if the new value is not required to be different from the old value.
<details> <summary>There are 10 instances (click to show):</summary>469: emit BridgeSweeperAddressUpdated(_sweeper, _allowed);
125: emit LockboxSet(_lockbox);
129: emit DelegationAddressUpdated(_delegateAddress);
108: emit FeeConfigUpdated(_feeAddress, _feeBasisPoints); 117: emit RestakeManagerUpdated(_restakeManager);
156: emit OperatorDelegatorAllocationUpdated(_newOperatorDelegator, _allocationBasisPoints); 169: emit OperatorDelegatorAllocationUpdated(_operatorDelegatorToRemove, 0); 211: emit OperatorDelegatorAllocationUpdated(_operatorDelegator, _allocationBasisPoints); 716: emit CollateralTokenTvlUpdated(_token, _limit);
</details>79: emit RewardDestinationUpdated(_rewardDestination);
Consider adding some comments on non-public contract variables to explain what they are supposed to do. This will help for future code reviews.
There are 4 instances:
31: address immutable blastStandardBridge; 32: address immutable registry;
32: mapping(bytes32 => uint256) private _timestamps; 33: uint256 private _minDelay;
The initial states set in a constructor are important and should be recorded in the event.
There are 6 instances:
51: xRenzoBridgeL1 = _xRenzoBridgeL1; 54: ccipEthChainSelector = _ccipEthChainSelector;
57: connext = _connext; 60: xRenzoBridgeL1 = _xRenzoBridgeL1; 63: connextEthChainDomain = _connextEthChainDomain;
117: _minDelay = minDelay;
Passing empty bytes to a function can cause unexpected behavior, such as certain operations failing, producing incorrect results, or wasting gas. It is recommended to check that all byte parameters are not empty.
<details> <summary>There are 10 instances (click to show):</summary>/// @audit _extraData 56: function bridgeTo( 57: address _to, 58: address _erc20, 59: address _remoteToken, 60: uint256 _amount, 61: uint32 _minGasLimit, 62: bytes calldata _extraData 63: ) external {
/// @audit _callData 69: function xReceive( 70: bytes32 _transferId, 71: uint256, 72: address, 73: address _originSender, 74: uint32 _origin, 75: bytes memory _callData 76: ) external onlySource(_originSender, _origin) whenNotPaused returns (bytes memory) {
/// @audit pubkey /// @audit signature 349: function stakeEth( 350: bytes calldata pubkey, 351: bytes calldata signature, 352: bytes32 depositDataRoot 353: ) external payable onlyRestakeManager {
/// @audit pubkey /// @audit signature 187: function stakeEthFromQueue( 188: IOperatorDelegator operatorDelegator, 189: bytes calldata pubkey, 190: bytes calldata signature, 191: bytes32 depositDataRoot 192: ) external onlyNativeEthRestakeAdmin {
/// @audit pubkey /// @audit signature 620: function stakeEthInOperatorDelegator( 621: IOperatorDelegator operatorDelegator, 622: bytes calldata pubkey, 623: bytes calldata signature, 624: bytes32 depositDataRoot 625: ) external payable onlyDepositQueue {
</details>/// @audit data 234: function schedule( 235: address target, 236: uint256 value, 237: bytes calldata data, 238: bytes32 predecessor, 239: bytes32 salt, 240: uint256 delay 241: ) public virtual onlyRole(PROPOSER_ROLE) { /// @audit payload 315: function execute( 316: address target, 317: uint256 value, 318: bytes calldata payload, 319: bytes32 predecessor, 320: bytes32 salt 321: ) public payable virtual onlyRoleOrOpenRole(EXECUTOR_ROLE) {
In Solidity, while function overriding allows for functions with the same name to coexist, it is advisable to avoid this practice to enhance code readability and maintainability. Having multiple functions with the same name, even with different parameters or in inherited contracts, can cause confusion and increase the likelihood of errors during development, testing, and debugging. Using distinct and descriptive function names not only clarifies the purpose and behavior of each function, but also helps prevent unintended function calls or incorrect overriding. By adopting a clear and consistent naming convention, developers can create more comprehensible and maintainable smart contracts.
There are 2 instances:
/// @audit Different function with same name found on line 473 491: function deposit( /// @audit Different function with same name found on line 582 592: function depositETH(uint256 _referralId) public payable nonReentrant notPaused {
Interfaces should not use fixed compiler versions, since they may be used by projects using a different version.
There is 1 instance:
2: pragma solidity 0.8.19;
Well-organized data structures make code reviews easier, which may lead to fewer bugs. Consider combining related mappings into mappings to structs, so it's clear what data is related.
There are 3 instances:
44: mapping(address => uint256) public withdrawalBufferTarget; 47: mapping(address => uint256) public claimReserve; 50: mapping(address => WithdrawRequest[]) public withdrawRequests;
Events should be emitted when critical changes are made to the contracts.
There are 3 instances:
121: function setPaused(bool _paused) external onlyDepositWithdrawPauserAdmin { 122: paused = _paused; 123: } 215: function setMaxDepositTVL(uint256 _maxDepositTVL) external onlyRestakeManagerAdmin { 216: maxDepositTVL = _maxDepositTVL; 217: }
51: function setPaused(bool _paused) external onlyTokenAdmin { 52: paused = _paused; 53: }
require()/revert() checks should be refactoredRefactoring duplicate require()/revert() checks into a modifier or function can make the code more concise, readable and maintainable, and less likely to make errors or omissions when modifying the require() or revert().
/// @audit Duplicated on line 105, 109, 113 101: revert InvalidTokenDecimals(EXPECTED_DECIMALS, decimals);
/// @audit Duplicated on line 111, 115 107: revert InvalidTokenDecimals(EXPECTED_DECIMALS, decimals); /// @audit Duplicated on line 351 346: revert InvalidTimestamp(_timestamp);
/// @audit Duplicated on line 350 330: if (_amount == 0) revert IXERC20_INVALID_0_VALUE(); /// @audit Duplicated on line 354 334: if (_currentLimit < _amount) revert IXERC20_NotHighEnoughLimits();
/// @audit Duplicated on line 92 55: if (!IS_NATIVE) revert IXERC20Lockbox_NotNative(); /// @audit Duplicated on line 80 67: if (IS_NATIVE) revert IXERC20Lockbox_Native();
/// @audit Duplicated on line 271 199: if (tokens.length != tokenAmounts.length) revert MismatchedArrayLengths(); /// @audit Duplicated on line 521 486: if (!success) revert TransferFailed();
/// @audit Duplicated on line 287 169: if (!success) revert TransferFailed();
/// @audit Duplicated on line 90 73: if (address(oracle) == address(0x0)) revert OracleNotFound(); /// @audit Duplicated on line 93 76: if (timestamp < block.timestamp - MAX_TIME_WINDOW) revert OraclePriceExpired(); /// @audit Duplicated on line 94 77: if (price <= 0) revert InvalidOraclePrice();
/// @audit Duplicated on line 224 139: revert AlreadyAdded(); /// @audit Duplicated on line 192 146: if (_allocationBasisPoints > (100 * BASIS_POINTS)) revert OverMaxBasisPoints(); /// @audit Duplicated on line 598 511: revert MaxTVLReached();
</details>/// @audit Duplicated on line 349 267: require(targets.length == values.length, "TimelockController: length mismatch"); /// @audit Duplicated on line 350 268: require(targets.length == payloads.length, "TimelockController: length mismatch"); /// @audit Duplicated on line 388 377: require(isOperationReady(id), "TimelockController: operation is not ready");
Adding a way to quickly halt protocol functionality in an emergency, rather than having to pause individual contracts one-by-one, will make in-progress hack mitigation faster and much less stressful.
<details> <summary>There are 12 instances (click to show):</summary>11: contract RenzoOracleL2 is Initializable, OwnableUpgradeable, RenzoOracleL2StorageV1 {
14: contract Receiver is CCIPReceiver, Ownable, Pausable {
10: contract ConnextReceiver is IXReceiver, Ownable, Pausable {
27: contract xRenzoDeposit is 28: Initializable, 29: OwnableUpgradeable, 30: ReentrancyGuardUpgradeable, 31: IRateProvider, 32: xRenzoDepositStorageV3
16: contract XERC20 is 17: Initializable, 18: ERC20Upgradeable, 19: OwnableUpgradeable, 20: IXERC20, 21: ERC20PermitUpgradeable
11: contract OptimismMintableXERC20 is ERC165Upgradeable, XERC20, IOptimismMintableERC20 {
14: contract RoleManager is IRoleManager, AccessControlUpgradeable, RoleManagerStorageV3 {
9: contract RoleManagerStorageV1 { 37: contract RoleManagerStorageV2 is RoleManagerStorageV1 { 45: contract RoleManagerStorageV3 is RoleManagerStorageV2 {
26: contract RestakeManager is Initializable, ReentrancyGuardUpgradeable, RestakeManagerStorageV2 {
</details>25: contract TimelockController is AccessControl, IERC721Receiver, IERC1155Receiver {
Use alternative variants, e.g. allowlist/denylist instead of whitelist/blacklist.
<details> <summary>There are 25 instances (click to show):</summary>40: // Sanity check 64: // Sanity check 72: // Sanity check
134: * @notice WARNING: This function does NOT whitelist who can send funds from the L2 via Connext. Users should NOT
185: // Sanity check for 0 326: * @dev Sanity checks input values and updates prices 415: // Verify the caller is whitelisted
49: /// @dev Allows only a whitelisted address to configure the contract 61: /// @dev Allows only a whitelisted address to configure the contract
43: /// @dev Allows only a whitelisted address to configure the contract 55: /// @dev Allows only a whitelisted address to trigger native ETH staking 61: /// @dev Allows only a whitelisted address to trigger ERC20 rewards sweeping
97: /// @dev Error when the origin address is not whitelisted
28: /// @dev Allows only a whitelisted address to configure the contract 145: // Sanity check 160: // Sanity check
84: /// @dev Determines if the specified address has permission to set whitelisted origin in xRenzoBridge
38: /// @dev role for granting capability to update whitelisted origin in xRenzoBridge
36: // Sanity check
70: /// @dev Allows only a whitelisted address to configure the contract 76: /// @dev Allows only a whitelisted address to set pause state
17: /// @dev Allows only a whitelisted address to trigger native ETH staking 23: /// @dev Allows only a whitelisted address to configure the contract
</details>14: /// @dev Allows only a whitelisted address to mint or burn ezETH tokens 20: /// @dev Allows only a whitelisted address to pause or unpause the token
Consider whether reasonable bounds checks for variables would be useful.
<details> <summary>There are 9 instances (click to show):</summary>110: ccipEthChainSelector = _newChainSelector;
106: connextEthChainDomain = _newChainDomain;
119: lastPrice = _currentPrice; 143: bridgeDestinationDomain = _bridgeDestinationDomain;
137: baseGasAmountSpent = _baseGasAmountSpent;
216: maxDepositTVL = _maxDepositTVL;
405: _minDelay = newDelay;
</details>87: coolDownPeriod = _coolDownPeriod; 132: coolDownPeriod = _newCoolDownPeriod;
Modern smart contract development often employs upgradeable contract structures, utilizing proxy patterns like OpenZeppelin’s Upgradeable Contracts. This paradigm separates logic and state, allowing developers to amend and enhance the contract's functionality without altering its state or the deployed contract address. Transitioning to this approach enhances long-term maintainability. Resolution: Adopt a well-established proxy pattern for upgradeability, ensuring proper initialization and employing transparent proxies to mitigate potential risks. Embrace comprehensive testing and audit practices, particularly when updating contract logic, to ensure state consistency and security are preserved across upgrades. This ensures your contract remains robust and adaptable to future requirements.
<details> <summary>There are 8 instances (click to show):</summary>30: contract LockboxAdapterBlast {
14: contract Receiver is CCIPReceiver, Ownable, Pausable {
10: contract ConnextReceiver is IXReceiver, Ownable, Pausable {
9: contract RoleManagerStorageV1 { 37: contract RoleManagerStorageV2 is RoleManagerStorageV1 { 45: contract RoleManagerStorageV3 is RoleManagerStorageV2 {
25: contract TimelockController is AccessControl, IERC721Receiver, IERC1155Receiver {
</details>10: contract EzEthTokenStorageV1 {
This includes: large code bases, or code with lots of inline-assembly, complicated math, or complicated interactions between multiple contracts. Invariant fuzzers such as Echidna require the test writer to come up with invariants which should not be violated under any circumstances, and the fuzzer tests various inputs and function calls to ensure that the invariants always hold. Even code with 100% code coverage can still have bugs due to the order of the operations a user performs, and invariant fuzzers may help significantly.
There is 1 instance:
In instances where a new variable is defined, there is no need to set it to it's default value.
<details> <summary>There are 37 instances (click to show):</summary>218: for (uint256 i = 0; i < _destinationParam.length; ) { 264: for (uint256 i = 0; i < _connextDestinationParam.length; ) {
372: uint256 minOut = 0;
176: for (uint256 i = 0; i < strategyLength; i++) { 381: for (uint256 i = 0; i < validatorFields.length; ) { 507: uint256 gasRefunded = 0;
164: uint256 feeAmount = 0; 227: for (uint256 i = 0; i < arrayLength; ) { 257: uint256 feeAmount = 0;
109: uint256 totalValue = 0; 111: for (uint256 i = 0; i < tokenLength; ) {
137: for (uint256 i = 0; i < odLength; ) { 165: for (uint256 i = 0; i < odLength; ) { 197: for (uint256 i = 0; i < odLength; ) { 223: for (uint256 i = 0; i < tokenLength; ) { 249: for (uint256 i = 0; i < tokenLength; ) { 277: uint256 totalTVL = 0; 287: uint256 totalWithdrawalQueueValue = 0; 289: for (uint256 i = 0; i < odLength; ) { 291: uint256 operatorTVL = 0; 299: for (uint256 j = 0; j < tokenLength; ) { 376: for (uint256 i = 0; i < tvlLength; ) { 418: for (uint256 i = 0; i < odLength; ) { 435: for (uint256 i = 0; i < odLength; ) { 454: for (uint256 i = 0; i < tokenLength; ) { 517: uint256 currentTokenTVL = 0; 521: for (uint256 i = 0; i < odLength; ) { 629: for (uint256 i = 0; i < odLength; ) { 676: uint256 totalRewards = 0; 683: for (uint256 i = 0; i < tokenLength; ) { 699: for (uint256 i = 0; i < odLength; ) {
107: for (uint256 i = 0; i < proposers.length; ++i) { 113: for (uint256 i = 0; i < executors.length; ++i) { 272: for (uint256 i = 0; i < targets.length; ++i) { 355: for (uint256 i = 0; i < targets.length; ++i) {
</details>88: for (uint256 i = 0; i < _withdrawalBufferTarget.length; ) { 110: for (uint256 i = 0; i < _newBufferTarget.length; ) {
public/external functions exposed by interfacesAll external/public functions should extend an interface. This is useful to ensure that the whole API is extracted and can be more easily integrated by other projects.
30: contract LockboxAdapterBlast {
16: contract xRenzoBridge is
11: contract RenzoOracleL2 is Initializable, OwnableUpgradeable, RenzoOracleL2StorageV1 {
14: contract Receiver is CCIPReceiver, Ownable, Pausable {
10: contract ConnextReceiver is IXReceiver, Ownable, Pausable {
27: contract xRenzoDeposit is
16: contract XERC20 is
14: contract XERC20Factory is Initializable, IXERC20Factory {
11: contract XERC20Lockbox is Initializable, IXERC20Lockbox {
11: contract OptimismMintableXERC20 is ERC165Upgradeable, XERC20, IOptimismMintableERC20 {
14: contract OptimismMintableXERC20Factory is Initializable, XERC20Factory {
17: contract OperatorDelegator is
10: contract DepositQueue is Initializable, ReentrancyGuardUpgradeable, DepositQueueStorageV2 {
15: contract WBETHShim is Initializable, WBETHShimStorageV1 {
15: contract METHShim is Initializable, METHShimStorageV1 {
13: contract RenzoOracle is
14: contract RoleManager is IRoleManager, AccessControlUpgradeable, RoleManagerStorageV3 {
9: contract BalancerRateProvider is Initializable, IRateProvider, BalancerRateProviderStorageV1 {
26: contract RestakeManager is Initializable, ReentrancyGuardUpgradeable, RestakeManagerStorageV2 {
16: contract RewardHandler is Initializable, ReentrancyGuardUpgradeable, RewardHandlerStorageV1 {
25: contract TimelockController is AccessControl, IERC721Receiver, IERC1155Receiver {
11: contract WithdrawQueue is
</details>13: contract EzEthToken is Initializable, ERC20Upgradeable, IEzEthToken, EzEthTokenStorageV1 {
2: pragma solidity ^0.8.13; 3: 4: import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; 7: import { IXERC20Lockbox } from "../../xERC20/interfaces/IXERC20Lockbox.sol"; 8: 9: interface IXERC20Registry { 15: } 16: 17: interface L1StandardBridge {
2: pragma solidity 0.8.19; 3: 4: import "./xRenzoBridgeStorage.sol"; 14: import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol"; 15: 16: contract xRenzoBridge is 53: } 54: 55: modifier onlyPriceFeedSender() {
2: pragma solidity 0.8.19; 3: 4: import "./IxRenzoBridge.sol"; 16: import { IRoleManager } from "../../Permissions/IRoleManager.sol"; 17: 18: abstract contract xRenzoBridgeStorageV1 is IxRenzoBridge {
2: pragma solidity 0.8.19; 3: 4: import "./RenzoOracleL2Storage.sol"; 9: import "../../../Errors/Errors.sol"; 10: 11: contract RenzoOracleL2 is Initializable, OwnableUpgradeable, RenzoOracleL2StorageV1 { 21: } 22: 23: function initialize(AggregatorV3Interface _oracle) public initializer {
2: pragma solidity 0.8.19; 3: 4: import "./IRenzoOracleL2.sol"; 5: import "@chainlink/contracts/src/v0.8/interfaces/AggregatorV3Interface.sol"; 6: 7: abstract contract RenzoOracleL2StorageV1 is IRenzoOracleL2 {
2: pragma solidity 0.8.19; 3: 4: import { Client } from "@chainlink/contracts-ccip/src/v0.8/ccip/libraries/Client.sol";
2: pragma solidity 0.8.19; 3: 4: import { IXReceiver } from "@connext/interfaces/core/IXReceiver.sol"; 8: import "../../../Errors/Errors.sol"; 9: 10: contract ConnextReceiver is IXReceiver, Ownable, Pausable { 67: } 68: 69: function xReceive(
2: pragma solidity 0.8.19; 3: 4: import "./xRenzoDepositStorage.sol";
2: pragma solidity 0.8.19; 3: 4: import "./IxRenzoDeposit.sol"; 7: import "./Oracle/IRenzoOracleL2.sol"; 8: 9: abstract contract xRenzoDepositStorageV1 is IxRenzoDeposit { 45: } 46: 47: abstract contract xRenzoDepositStorageV2 is xRenzoDepositStorageV1 { 50: } 51: 52: abstract contract xRenzoDepositStorageV3 is xRenzoDepositStorageV2 {
2: pragma solidity >=0.8.4 <0.9.0; 3: 4: import { IXERC20 } from "../interfaces/IXERC20.sol"; 14: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; 15: 16: contract XERC20 is
2: pragma solidity >=0.8.4 <0.9.0; 3: 4: import { IXERC20Factory } from "../interfaces/IXERC20Factory.sol"; 12: import "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol"; 13: 14: contract XERC20Factory is Initializable, IXERC20Factory {
2: pragma solidity >=0.8.4 <0.9.0; 3: 4: import { IXERC20 } from "../interfaces/IXERC20.sol"; 9: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; 10: 11: contract XERC20Lockbox is Initializable, IXERC20Lockbox {
2: pragma solidity >=0.8.4 <0.9.0; 3: 4: import { 9: import { XERC20 } from "../XERC20.sol"; 10: 11: contract OptimismMintableXERC20 is ERC165Upgradeable, XERC20, IOptimismMintableERC20 { 46: } 47: 48: function supportsInterface( 54: } 55: 56: function remoteToken() public view override returns (address) { 58: } 59: 60: function bridge() public view override returns (address) { 62: } 63: 64: function mint(address _to, uint256 _amount) public override(XERC20, IOptimismMintableERC20) { 66: } 67: 68: function burn(address _from, uint256 _amount) public override(XERC20, IOptimismMintableERC20) {
2: pragma solidity >=0.8.4 <0.9.0; 3: 4: import { OptimismMintableXERC20 } from "./OptimismMintableXERC20.sol"; 12: import "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol"; 13: 14: contract OptimismMintableXERC20Factory is Initializable, XERC20Factory {
2: pragma solidity 0.8.19; 3: 4: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; 130: } 131: 132: function setBaseGasAmountSpent(
2: pragma solidity 0.8.19; 3: import "../Permissions/IRoleManager.sol"; 44: } 45: 46: abstract contract OperatorDelegatorStorageV2 is OperatorDelegatorStorageV1 { 49: } 50: 51: abstract contract OperatorDelegatorStorageV3 is OperatorDelegatorStorageV2 { 57: } 58: 59: abstract contract OperatorDelegatorStorageV4 is OperatorDelegatorStorageV3 {
2: pragma solidity 0.8.19; 3: 4: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; 8: import "../Errors/Errors.sol"; 9: 10: contract DepositQueue is Initializable, ReentrancyGuardUpgradeable, DepositQueueStorageV2 {
2: pragma solidity 0.8.19; 3: 4: import "../Permissions/IRoleManager.sol"; 7: import "./IDepositQueue.sol"; 8: 9: abstract contract DepositQueueStorageV1 is IDepositQueue { 24: } 25: 26: abstract contract DepositQueueStorageV2 is DepositQueueStorageV1 {
2: pragma solidity 0.8.19; 3: 4: import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol"; 27: } 28: 29: function decimals() external pure returns (uint8) { 31: } 32: 33: function description() external pure returns (string memory) { 35: } 36: 37: function version() external pure returns (uint256) { 44: } 45: 46: function latestRoundData()
2: pragma solidity 0.8.19; 3: 4: import "@chainlink/contracts/src/v0.8/interfaces/AggregatorV3Interface.sol"; 5: import "./IStakedTokenV2.sol"; 6: 7: abstract contract WBETHShimStorageV1 is AggregatorV3Interface {
2: pragma solidity 0.8.19; 3: 4: import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol"; 27: } 28: 29: function decimals() external pure returns (uint8) { 31: } 32: 33: function description() external pure returns (string memory) { 35: } 36: 37: function version() external pure returns (uint256) { 44: } 45: 46: function latestRoundData()
2: pragma solidity 0.8.19; 3: 4: import "@chainlink/contracts/src/v0.8/interfaces/AggregatorV3Interface.sol"; 5: import "./IMethStaking.sol"; 6: 7: abstract contract METHShimStorageV1 is AggregatorV3Interface {
2: pragma solidity 0.8.19; 3: 4: import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";
2: pragma solidity 0.8.19; 3: 4: import "../Permissions/IRoleManager.sol"; 6: import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; 7: 8: abstract contract RenzoOracleStorageV1 {
2: pragma solidity 0.8.19; 3: 4: import "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol";
43: } 44: 45: contract RoleManagerStorageV3 is RoleManagerStorageV2 {
2: pragma solidity 0.8.19; 3: 4: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; 7: import "./BalancerRateProviderStorage.sol"; 8: 9: contract BalancerRateProvider is Initializable, IRateProvider, BalancerRateProviderStorageV1 {
2: pragma solidity 0.8.19; 3: 4: import "@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol"; 5: import "../IRestakeManager.sol"; 6: 7: abstract contract BalancerRateProviderStorageV1 {
2: pragma solidity 0.8.19; 3: 4: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; 707: } 708: 709: function setTokenTvlLimit(IERC20 _token, uint256 _limit) external onlyRestakeManagerAdmin {
2: pragma solidity 0.8.19; 3: 4: import "./EigenLayer/interfaces/IStrategy.sol"; 13: import "./Withdraw/IWithdrawQueue.sol"; 14: 15: abstract contract RestakeManagerStorageV1 is IRestakeManager { 62: } 63: 64: abstract contract RestakeManagerStorageV2 is RestakeManagerStorageV1 {
2: pragma solidity 0.8.19; 3: 4: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; 60: } 61: 62: function _forwardETH() internal { 70: } 71: 72: function setRewardDestination(
2: pragma solidity 0.8.19; 3: 4: import "../Permissions/IRoleManager.sol"; 4: import "../Permissions/IRoleManager.sol"; 5: 6: abstract contract RewardHandlerStorageV1 {
4: pragma solidity ^0.8.0; 5: 6: import "@openzeppelin/contracts/access/AccessControl.sol";
2: pragma solidity 0.8.19; 3: 4: import "./WithdrawQueueStorage.sol"; 9: import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; 10: 11: contract WithdrawQueue is 48: } 49: 50: modifier onlyDepositQueue() {
2: pragma solidity 0.8.19; 3: 4: import "../Permissions/IRoleManager.sol"; 7: import "../token/IEzEthToken.sol"; 8: 9: abstract contract WithdrawQueueStorageV1 {
2: pragma solidity ^0.8.9; 3: 4: import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol";
</details>2: pragma solidity 0.8.19; 3: import "../Permissions/IRoleManager.sol";
Formal verification is the act of proving or disproving the correctness of intended algorithms underlying a system with respect to a certain formal specification/property/invariant, using formal methods of mathematics.
Some tools that are currently available to perform these tests on smart contracts are SMTChecker and Certora Prover.
<details> <summary>There are 40 instances (click to show):</summary>There are time units for seconds, minutes, hours, days, and weeks, and since they're defined, they should be used.
There are 2 instances:
/// @audit 86400 13: uint256 public constant MAX_TIME_WINDOW = 86400 + 60; // 24 hours + 60 seconds
/// @audit 86400 26: uint256 constant MAX_TIME_WINDOW = 86400 + 60; // 24 hours + 60 seconds
#0 - CloudEllie
2024-05-13T14:02:22Z
#1 - c4-judge
2024-05-24T10:00:45Z
alcueca marked the issue as grade-b