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
  1. 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)
  2. Need to remove IERC20Metadata (reproducible for the develop branch).
> 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                                                             
  1. 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.
  1. Multichain network name
    https://github.com/lidofinance/lido-l2/blob/develop/scripts/optimism/finalize-message.ts#L9
    Should be "optimism"?
  1. 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)
  1. Why do we have chain id #42 for optimism?
    https://github.com/lidofinance/lido-l2/blob/develop/scripts/optimism/finalize-message.ts#L14
  1. A few typos:
    • in aren in line @dev Validates that withdrawals aren enabled
    • recieve in CrossDomainEnabled.sol
    • in README: fixed in git patch
  1. _setupRole() is deprecated according to OpenZeppelin docs

  2. Typos: