Merge pull request #4562 from ethereum-optimism/sc/ctb-block-legacy-replays

feat(ctb): check for relayed legacy messages

op-chain-ops/crossdomain/migrate.go

@@ -63,7 +63,10 @@ func MigrateWithdrawal(withdrawal *LegacyWithdrawal, l1CrossDomainMessenger *com
 		return nil, err
 	}
 
-	versionedNonce := EncodeVersionedNonce(withdrawal.Nonce, common.Big1)
+	// Migrated withdrawals are specified as version 0. Both the
+	// L2ToL1MessagePasser and the CrossDomainMessenger use the same
+	// versioning scheme. Both should be set to version 0
+	versionedNonce := EncodeVersionedNonce(withdrawal.Nonce, new(big.Int))
 	// Encode the call to `relayMessage` on the `CrossDomainMessenger`.
 	// The minGasLimit can safely be 0 here.
 	data, err := abi.Pack(
@@ -83,7 +86,7 @@ func MigrateWithdrawal(withdrawal *LegacyWithdrawal, l1CrossDomainMessenger *com
 	gasLimit := uint64(len(data)*16 + 200_000)
 
 	w := NewWithdrawal(
-		withdrawal.Nonce,
+		versionedNonce,
 		&predeploys.L2CrossDomainMessengerAddr,
 		l1CrossDomainMessenger,
 		value,

packages/contracts-bedrock/.gas-snapshot

@@ -21,9 +21,9 @@ CrossDomainOwnableThroughPortal_Test:test_depositTransaction_crossDomainOwner_su
 CrossDomainOwnable_Test:test_onlyOwner_notOwner_reverts() (gas: 10530)
 CrossDomainOwnable_Test:test_onlyOwner_succeeds() (gas: 34861)
 CrossDomainOwnable2_Test:test_onlyOwner_notMessenger_reverts() (gas: 8416)
-CrossDomainOwnable2_Test:test_onlyOwner_notOwner2_reverts() (gas: 61754)
+CrossDomainOwnable2_Test:test_onlyOwner_notOwner2_reverts() (gas: 61783)
 CrossDomainOwnable2_Test:test_onlyOwner_notOwner_reverts() (gas: 16588)
-CrossDomainOwnable2_Test:test_onlyOwner_succeeds() (gas: 77782)
+CrossDomainOwnable2_Test:test_onlyOwner_succeeds() (gas: 77811)
 DeployerWhitelist_Test:test_owner_succeeds() (gas: 7538)
 DeployerWhitelist_Test:test_storageSlots_succeeds() (gas: 33395)
 FeeVault_Test:test_constructor_succeeds() (gas: 10647)
@@ -55,22 +55,27 @@ L1BlockTest:test_updateValues_succeeds() (gas: 60481)
 L1BlockNumberTest:test_fallback_succeeds() (gas: 18633)
 L1BlockNumberTest:test_getL1BlockNumber_succeeds() (gas: 10603)
 L1BlockNumberTest:test_receive_succeeds() (gas: 25340)
-L1CrossDomainMessenger_Test:test_messageVersion_succeeds() (gas: 24716)
-L1CrossDomainMessenger_Test:test_pause_callerIsNotOwner_reverts() (gas: 24539)
-L1CrossDomainMessenger_Test:test_pause_succeeds() (gas: 52964)
-L1CrossDomainMessenger_Test:test_relayMessage_paused_reverts() (gas: 60498)
-L1CrossDomainMessenger_Test:test_relayMessage_reentrancy_reverts() (gas: 190902)
-L1CrossDomainMessenger_Test:test_relayMessage_retryAfterFailure_succeeds() (gas: 197256)
-L1CrossDomainMessenger_Test:test_relayMessage_succeeds() (gas: 73617)
-L1CrossDomainMessenger_Test:test_relayMessage_toSystemContract_reverts() (gas: 65858)
-L1CrossDomainMessenger_Test:test_relayMessage_v0_reverts() (gas: 33214)
-L1CrossDomainMessenger_Test:test_replayMessage_withValue_reverts() (gas: 38214)
-L1CrossDomainMessenger_Test:test_sendMessage_succeeds() (gas: 299523)
-L1CrossDomainMessenger_Test:test_sendMessage_twice_succeeds() (gas: 1490457)
-L1CrossDomainMessenger_Test:test_unpause_callerIsNotOwner_reverts() (gas: 24472)
-L1CrossDomainMessenger_Test:test_unpause_succeeds() (gas: 45149)
-L1CrossDomainMessenger_Test:test_xDomainMessageSender_reset_succeeds() (gas: 84036)
-L1CrossDomainMessenger_Test:test_xDomainSender_notSet_reverts() (gas: 24252)
+L1CrossDomainMessenger_Test:test_messageVersion_succeeds() (gas: 24759)
+L1CrossDomainMessenger_Test:test_pause_callerIsNotOwner_reverts() (gas: 24517)
+L1CrossDomainMessenger_Test:test_pause_succeeds() (gas: 52942)
+L1CrossDomainMessenger_Test:test_relayMessage_legacyOldReplay_reverts() (gas: 56662)
+L1CrossDomainMessenger_Test:test_relayMessage_legacyRetryAfterFailureThenSuccess_reverts() (gas: 212695)
+L1CrossDomainMessenger_Test:test_relayMessage_legacyRetryAfterFailure_succeeds() (gas: 203380)
+L1CrossDomainMessenger_Test:test_relayMessage_legacyRetryAfterSuccess_reverts() (gas: 126553)
+L1CrossDomainMessenger_Test:test_relayMessage_legacy_succeeds() (gas: 76688)
+L1CrossDomainMessenger_Test:test_relayMessage_paused_reverts() (gas: 60521)
+L1CrossDomainMessenger_Test:test_relayMessage_reentrancy_reverts() (gas: 190932)
+L1CrossDomainMessenger_Test:test_relayMessage_retryAfterFailure_succeeds() (gas: 197320)
+L1CrossDomainMessenger_Test:test_relayMessage_succeeds() (gas: 73646)
+L1CrossDomainMessenger_Test:test_relayMessage_toSystemContract_reverts() (gas: 65917)
+L1CrossDomainMessenger_Test:test_relayMessage_v2_reverts() (gas: 19545)
+L1CrossDomainMessenger_Test:test_replayMessage_withValue_reverts() (gas: 38265)
+L1CrossDomainMessenger_Test:test_sendMessage_succeeds() (gas: 299568)
+L1CrossDomainMessenger_Test:test_sendMessage_twice_succeeds() (gas: 1490458)
+L1CrossDomainMessenger_Test:test_unpause_callerIsNotOwner_reverts() (gas: 24516)
+L1CrossDomainMessenger_Test:test_unpause_succeeds() (gas: 45185)
+L1CrossDomainMessenger_Test:test_xDomainMessageSender_reset_succeeds() (gas: 84065)
+L1CrossDomainMessenger_Test:test_xDomainSender_notSet_reverts() (gas: 24274)
 L1ERC721Bridge_Test:test_bridgeERC721To_localTokenZeroAddress_reverts() (gas: 52730)
 L1ERC721Bridge_Test:test_bridgeERC721To_remoteTokenZeroAddress_reverts() (gas: 27332)
 L1ERC721Bridge_Test:test_bridgeERC721To_succeeds() (gas: 354722)
@@ -103,17 +108,17 @@ L1StandardBridge_Getter_Test:test_getters_succeeds() (gas: 31449)
 L1StandardBridge_Initialize_Test:test_initialize_succeeds() (gas: 22005)
 L1StandardBridge_Receive_Test:test_receive_succeeds() (gas: 514475)
 L2CrossDomainMessenger_Test:test_messageVersion_succeeds() (gas: 8367)
-L2CrossDomainMessenger_Test:test_pause_notOwner_reverts() (gas: 10859)
+L2CrossDomainMessenger_Test:test_pause_notOwner_reverts() (gas: 10837)
 L2CrossDomainMessenger_Test:test_pause_succeeds() (gas: 31846)
 L2CrossDomainMessenger_Test:test_relayMessage_paused_reverts() (gas: 41596)
-L2CrossDomainMessenger_Test:test_relayMessage_reentrancy_reverts() (gas: 167787)
-L2CrossDomainMessenger_Test:test_relayMessage_retry_succeeds() (gas: 168313)
-L2CrossDomainMessenger_Test:test_relayMessage_succeeds() (gas: 53183)
-L2CrossDomainMessenger_Test:test_relayMessage_toSystemContract_reverts() (gas: 36194)
-L2CrossDomainMessenger_Test:test_relayMessage_v0_reverts() (gas: 18870)
+L2CrossDomainMessenger_Test:test_relayMessage_reentrancy_reverts() (gas: 167794)
+L2CrossDomainMessenger_Test:test_relayMessage_retry_succeeds() (gas: 168371)
+L2CrossDomainMessenger_Test:test_relayMessage_succeeds() (gas: 53212)
+L2CrossDomainMessenger_Test:test_relayMessage_toSystemContract_reverts() (gas: 36246)
+L2CrossDomainMessenger_Test:test_relayMessage_v2_reverts() (gas: 18913)
 L2CrossDomainMessenger_Test:test_sendMessage_succeeds() (gas: 122533)
 L2CrossDomainMessenger_Test:test_sendMessage_twice_succeeds() (gas: 134671)
-L2CrossDomainMessenger_Test:test_xDomainMessageSender_reset_succeeds() (gas: 52594)
+L2CrossDomainMessenger_Test:test_xDomainMessageSender_reset_succeeds() (gas: 52623)
 L2CrossDomainMessenger_Test:test_xDomainSender_senderNotSet_reverts() (gas: 10524)
 L2ERC721Bridge_Test:test_bridgeERC721To_localTokenZeroAddress_reverts() (gas: 26432)
 L2ERC721Bridge_Test:test_bridgeERC721To_remoteTokenZeroAddress_reverts() (gas: 21748)

packages/contracts-bedrock/contracts/test/L2CrossDomainMessenger.t.sol

@@ -93,18 +93,20 @@ contract L2CrossDomainMessenger_Test is Messenger_Initializer {
         L2Messenger.xDomainMessageSender();
     }
 
-    function test_relayMessage_v0_reverts() external {
+    function test_relayMessage_v2_reverts() external {
         address target = address(0xabcd);
         address sender = address(L1Messenger);
         address caller = AddressAliasHelper.applyL1ToL2Alias(address(L1Messenger));
 
-        vm.prank(caller);
-
+        // Expect a revert.
         vm.expectRevert(
-            "CrossDomainMessenger: only version 1 messages are supported after the Bedrock upgrade"
+            "CrossDomainMessenger: only version 0 or 1 messages are supported at this time"
         );
+
+        // Try to relay a v2 message.
+        vm.prank(caller);
         L2Messenger.relayMessage(
-            0, // nonce
+            Encoding.encodeVersionedNonce(0, 2), // nonce
             sender,
             target,
             0, // value

packages/contracts-bedrock/contracts/universal/CrossDomainMessenger.sol

@@ -263,15 +263,23 @@ abstract contract CrossDomainMessenger is
         bytes calldata _message
     ) external payable nonReentrant whenNotPaused {
         (, uint16 version) = Encoding.decodeVersionedNonce(_nonce);
+        require(
+            version < 2,
+            "CrossDomainMessenger: only version 0 or 1 messages are supported at this time"
+        );
 
-        // Block any messages that aren't version 1. All version 0 messages have been guaranteed to
-        // be relayed OR have been migrated to version 1 messages. Version 0 messages do not commit
-        // to the value or minGasLimit fields, which can create unexpected issues for end-users.
+        // If the message is version 0, then it's a migrated legacy withdrawal. We therefore need
+        // to check that the legacy version of the message has not already been relayed.
+        if (version == 0) {
+            bytes32 oldHash = Hashing.hashCrossDomainMessageV0(_target, _sender, _message, _nonce);
             require(
-            version == 1,
-            "CrossDomainMessenger: only version 1 messages are supported after the Bedrock upgrade"
+                successfulMessages[oldHash] == false,
+                "CrossDomainMessenger: legacy withdrawal already relayed"
             );
+        }
 
+        // We use the v1 message hash as the unique identifier for the message because it commits
+        // to the value and minimum gas limit of the message.
         bytes32 versionedHash = Hashing.hashCrossDomainMessageV1(
             _nonce,
             _sender,