## Findings for the wstETH bridging Op review 1. ENV-setup deploy (it's better to have verifiable transparent source of all addresses and constants) * To fit the transparency requirements before the deployment might be added ".env.optimis.wsteth" and ".env.arbitrum.wsteth" files with fixed variables for wstETH deployment. These files will be used as seed files for the actual .env file 3. https://github.com/lidofinance/lido-l2/tree/develop/contracts/optimism#attack-on-l1crossdomainmessenger Maybe resolve the issue by whitelisting implementations and/or monitoring with Forta (request to the Automation team) 3. Need to remove `IERC20Metadata` (reproducible for the `develop` branch). - Fixed at: https://github.com/lidofinance/lido-l2/commit/2cc16b01823e9dd6ec115e48d04f5f3f6a7a9d17 ```bash > lido-l2@1.0.0 compile > hardhat compile Solidity 0.8.10 is not fully supported yet. You can still use Hardhat, but some features, like stack traces, might not work correctly. Learn more at https://hardhat.org/reference/solidity-support DeclarationError: Identifier not found or not unique. --> contracts/token/interfaces/IERC20Bridged.sol:10:36: | 10 | interface IERC20Bridged is IERC20, IERC20Metadata { | ^^^^^^^^^^^^^^ Error HH600: Compilation failed For more info go to https://hardhat.org/HH600 or run Hardhat with --show-stack-traces ``` 4. Maybe it is worth to add the additional events for `_burn` and `_mint` (optimism `ERC20Core`)? https://github.com/lidofinance/lido-l2/blob/develop/contracts/token/ERC20Core.sol#L138 https://github.com/lidofinance/lido-l2/blob/develop/contracts/token/ERC20Core.sol#L150 - I think such events aren't needed in the `ERC20Core` methods cause it's not the common practice. But we can add `BridgeMinted(address indexed account, uint amount)` and `BridgeBurned(address indexed account, uint amount)` events in the ERC20Bridged contract. Or not add any additional events because we can clearly find when mint and burn happened using the `Transfer(address indexed from, address indexed to, uint256 value)` events. It also will decrease the gas costs, but on L2, it's not so critical. 5. Multichain network name https://github.com/lidofinance/lido-l2/blob/develop/scripts/optimism/finalize-message.ts#L9 Should be "optimism"? - Fixed at: https://github.com/lidofinance/lido-l2/commit/8e46c46c6bb74020efab7b4f3ba34cb51952167b 6. Unclear intention https://github.com/lidofinance/lido-l2/blob/develop/scripts/arbitrum/deploy-gateway.ts#L53 It's slightly vague, why we are moving from step 1 to step 3 (skipping step 2) - Added the names for magic numbers in the script at the commit: https://github.com/lidofinance/lido-l2/commit/b2c671928679c913fe3942ea562df7ad3feb49e3 7. Why do we have chain id #42 for optimism? https://github.com/lidofinance/lido-l2/blob/develop/scripts/optimism/finalize-message.ts#L14 - Fixed at: https://github.com/lidofinance/lido-l2/commit/8e46c46c6bb74020efab7b4f3ba34cb51952167b 8. A few typos: - in `aren` in line `@dev Validates that withdrawals aren enabled` - `recieve` in CrossDomainEnabled.sol - in README: fixed in [git patch](https://gist.github.com/arwer13/4269fbd4e058161f104c496922c337f0) - fixed in: https://github.com/lidofinance/lido-l2/pull/14/commits/04b03b08711ca0e26baa63432129849e1f43f5cb 9. [_setupRole()](https://github.com/lidofinance/lido-l2/blob/082e7eb59de63bd376b30886568813408d04f00b/contracts/BridgingManager.sol#L42) is deprecated according to OpenZeppelin [docs](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/a7818b49a481e867230d4fa49d106bd304c5d95d/contracts/access/AccessControl.sol#L195) 10. Typos: - ['sutisfied' and 'TotkenBridge'](https://github.com/lidofinance/lido-l2/blob/bd962ec998303976bbf51fa1f1833ae79a782719/contracts/optimism/README.md#L715-L715}