Legacy withdrawals can be relayed twice, causing double spending of bridged assets
`L2CrossDomainMessenger.relayMessage` checks that legacy messages have not been relayed by reading from the `successfulMessages` state variable, however the contract's storage will wiped during the migration to Bedrock and `successfulMessages` will be empty after the deployment of the contract. The check will always pass, even if a legacy message have already been relayed using its v0 hash. As a result, random withdrawal messages, as well as messages from malicious actors, can be relayed multiple times during the migration: first, as legacy v0 messages (before the migration); then, as Bedrock v1 messages (during the migration).\\nL2CrossDomainMessenger inherits from CrossDomainMessenger, which inherits from `CrossDomainMessengerLegacySpacer0`, `CrossDomainMessengerLegacySpacer1`, assuming that the contract will be deployed at an address with existing state-the two spacer contracts are needed to "skip" the slots occupied by previous implementations of the contract.\\nDuring the migration, legacy (i.e. pre-Bedrock) withdrawal messages will be converted to Bedrock messages-they're expected to call the `relayMessage` function of `L2CrossDomainMessenger`. The `L2CrossDomainMessenger.relayMessage` function checks that the relayed legacy message haven't been relayed already:\\n```\\n// If the message is version 0, then it's a migrated legacy withdrawal. We therefore need\\n// to check that the legacy version of the message has not already been relayed.\\nif (version == 0) {\\n    bytes32 oldHash = Hashing.hashCrossDomainMessageV0(_target, _sender, _message, _nonce);\\n    require(\\n        successfulMessages[oldHash] == false,\\n        "CrossDomainMessenger: legacy withdrawal already relayed"\\n    );\\n}\\n```\\n\\nIt reads a V0 message hash from the `successfulMessages` state variable, assuming that the content of the variable is preserved during the migration. However, the state and storage of all predeployed contracts is wiped during the migration:\\n```\\n// We need to wipe the storage of every predeployed contract EXCEPT for the GovernanceToken,\\n// WETH9, the DeployerWhitelist, the LegacyMessagePasser, and LegacyERC20ETH. We have verified\\n// that none of the legacy storage (other than the aforementioned contracts) is accessible and\\n// therefore can be safely removed from the database. Storage must be wiped before anything\\n// else or the ERC-1967 proxy storage slots will be removed.\\nif err := WipePredeployStorage(db); err != nil {\\n    return nil, fmt.Errorf("cannot wipe storage: %w", err)\\n}\\n```\\n\\nAlso notice that withdrawals are migrated after predeploys were wiped and deployed-predeploys will have empty storage at the time withdrawals are migrated.\\nMoreover, if we check the code at the `L2CrossDomainMessenger` address of the current version of Optimism, we'll see that the contract's storage layout is different from the layout of the `CrossDomainMessengerLegacySpacer0` and `CrossDomainMessengerLegacySpacer1` contracts: there are no gaps and other spacer slots; `successfulMessages` is the second slot of the contract. Thus, even if there were no wiping, the `successfulMessages` mapping of the new `L2CrossDomainMessenger` contract would still be empty.
Consider cleaning up the storage layout of `L1CrossDomainMessenger`, `L2CrossDomainMessenger` and other proxied contracts.\\nIn the PreCheckWithdrawals function, consider reading withdrawal hashes from the `successfulMessages` mapping of the old `L2CrossDomainMessenger` contract and checking if the values are set. Successful withdrawals should be skipped at this point to filter out legacy withdrawals that have already been relayed.\\nConsider removing the check from the `relayMessage` function, since the check will be useless due to the empty state of the contract.
Withdrawal messages can be relayed twice: once right before and once during the migration. ETH and ERC20 tokens can be withdrawn twice, which is basically double spending of bridged assets.
```\\n// If the message is version 0, then it's a migrated legacy withdrawal. We therefore need\\n// to check that the legacy version of the message has not already been relayed.\\nif (version == 0) {\\n    bytes32 oldHash = Hashing.hashCrossDomainMessageV0(_target, _sender, _message, _nonce);\\n    require(\\n        successfulMessages[oldHash] == false,\\n        "CrossDomainMessenger: legacy withdrawal already relayed"\\n    );\\n}\\n```\\n