Require re-proving against a different game (#9944)

split req

packages/contracts-bedrock/semver-lock.json

@@ -36,8 +36,8 @@
     "sourceCodeHash": "0xf549ae16033b63e7cb3e032898a6495e1a13090dc8dd1422f7f650076ae973f8"
   },
   "src/L1/OptimismPortal2.sol": {
-    "initCodeHash": "0xd2947b6ce4c5976d550c84d72f14ead2c92f27449edc98ca8ef92b3e9c02b367",
-    "sourceCodeHash": "0x477df849308119f516e21e3c44e72422c1afa919fad5351be5046d3ec9f0e11f"
+    "initCodeHash": "0x3501f67714a63fe67ce1b7530bcaa2593f149f595a702034837006494ee8cb64",
+    "sourceCodeHash": "0x11451fdf6a9c60331fb7413d946dae556410a64bcf761fe48f82e56812743424"
   },
   "src/L1/ProtocolVersions.sol": {
     "initCodeHash": "0x72cd467e8bcf019c02675d72ab762e088bcc9cc0f1a4e9f587fa4589f7fdd1b8",

packages/contracts-bedrock/src/L1/OptimismPortal2.sol

@@ -117,8 +117,8 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver {
     }
 
     /// @notice Semantic version.
-    /// @custom:semver 3.3.0
-    string public constant version = "3.3.0";
+    /// @custom:semver 3.4.0
+    string public constant version = "3.4.0";
 
     /// @notice Constructs the OptimismPortal contract.
     constructor(
@@ -267,6 +267,13 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver {
         bytes32 withdrawalHash = Hashing.hashWithdrawal(_tx);
         ProvenWithdrawal memory provenWithdrawal = provenWithdrawals[withdrawalHash];
 
+        // We do not allow for proving withdrawals against dispute games that have resolved against the favor
+        // of the root claim.
+        require(
+            gameProxy.status() != GameStatus.CHALLENGER_WINS,
+            "OptimismPortal: cannot prove against invalid dispute games"
+        );
+
         // We generally want to prevent users from proving the same withdrawal multiple times
         // because each successive proof will update the timestamp. A malicious user can take
         // advantage of this to prevent other users from finalizing their withdrawal. However,

packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol

@@ -446,9 +446,9 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest {
         });
     }
 
-    /// @dev Tests that `proveWithdrawalTransaction` reverts when the withdrawal has already
-    ///      been proven.
-    function test_proveWithdrawalTransaction_replayProve_reverts() external {
+    /// @dev Tests that `proveWithdrawalTransaction` reverts when the withdrawal has already been proven, and the
+    ///      re-prove attempt is for the same dispute game.
+    function test_proveWithdrawalTransaction_replayProve_sameGame_reverts() external {
         vm.expectEmit(true, true, true, true);
         emit WithdrawalProven(_withdrawalHash, alice, bob);
         optimismPortal2.proveWithdrawalTransaction({
@@ -469,6 +469,64 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest {
         });
     }
 
+    /// @dev Tests that `proveWithdrawalTransaction` reverts when the withdrawal has already been proven, and the first
+    ///      game is currently being disputed, is otherwise not invalid, and has not been blacklisted.
+    function test_proveWithdrawalTransaction_replayProve_differentGameFirstGameGood_reverts() external {
+        vm.expectEmit(true, true, true, true);
+        emit WithdrawalProven(_withdrawalHash, alice, bob);
+        optimismPortal2.proveWithdrawalTransaction({
+            _tx: _defaultTx,
+            _disputeGameIndex: _proposedGameIndex,
+            _outputRootProof: _outputRootProof,
+            _withdrawalProof: _withdrawalProof
+        });
+
+        // Create a new dispute game, but don't mock anything about the first game.
+        disputeGameFactory.create(
+            optimismPortal2.respectedGameType(), Claim.wrap(_outputRoot), abi.encode(_proposedBlockNumber + 1)
+        );
+        _proposedGameIndex = disputeGameFactory.gameCount() - 1;
+
+        vm.expectRevert(
+            "OptimismPortal: withdrawal hash has already been proven, and the old dispute game is not invalid"
+        );
+        optimismPortal2.proveWithdrawalTransaction({
+            _tx: _defaultTx,
+            _disputeGameIndex: _proposedGameIndex,
+            _outputRootProof: _outputRootProof,
+            _withdrawalProof: _withdrawalProof
+        });
+    }
+
+    /// @dev Tests that `proveWithdrawalTransaction` reverts when the withdrawal has already been proven, and the new
+    ///      game has the `CHALLENGER_WINS` status.
+    function test_proveWithdrawalTransaction_replayProve_differentGameChallengerWins_reverts() external {
+        vm.expectEmit(true, true, true, true);
+        emit WithdrawalProven(_withdrawalHash, alice, bob);
+        optimismPortal2.proveWithdrawalTransaction({
+            _tx: _defaultTx,
+            _disputeGameIndex: _proposedGameIndex,
+            _outputRootProof: _outputRootProof,
+            _withdrawalProof: _withdrawalProof
+        });
+
+        // Create a new dispute game, and mock both games to be CHALLENGER_WINS.
+        IDisputeGame game2 = disputeGameFactory.create(
+            optimismPortal2.respectedGameType(), Claim.wrap(_outputRoot), abi.encode(_proposedBlockNumber + 1)
+        );
+        _proposedGameIndex = disputeGameFactory.gameCount() - 1;
+        vm.mockCall(address(game), abi.encodeCall(game.status, ()), abi.encode(GameStatus.CHALLENGER_WINS));
+        vm.mockCall(address(game2), abi.encodeCall(game.status, ()), abi.encode(GameStatus.CHALLENGER_WINS));
+
+        vm.expectRevert("OptimismPortal: cannot prove against invalid dispute games");
+        optimismPortal2.proveWithdrawalTransaction({
+            _tx: _defaultTx,
+            _disputeGameIndex: _proposedGameIndex,
+            _outputRootProof: _outputRootProof,
+            _withdrawalProof: _withdrawalProof
+        });
+    }
+
     /// @dev Tests that `proveWithdrawalTransaction` reverts if the dispute game being proven against is not of the
     ///      respected game type.
     function test_proveWithdrawalTransaction_badGameType_reverts() external {
@@ -503,6 +561,14 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest {
         vm.prank(optimismPortal2.guardian());
         optimismPortal2.blacklistDisputeGame(IDisputeGame(address(game)));
 
+        // Mock the status of the dispute game we just proved against to be CHALLENGER_WINS.
+        vm.mockCall(address(game), abi.encodeCall(game.status, ()), abi.encode(GameStatus.CHALLENGER_WINS));
+        // Create a new game to re-prove against
+        disputeGameFactory.create(
+            optimismPortal2.respectedGameType(), Claim.wrap(_outputRoot), abi.encode(_proposedBlockNumber + 1)
+        );
+        _proposedGameIndex = disputeGameFactory.gameCount() - 1;
+
         vm.expectEmit(true, true, true, true);
         emit WithdrawalProven(_withdrawalHash, alice, bob);
         optimismPortal2.proveWithdrawalTransaction({
@@ -525,7 +591,13 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest {
             _withdrawalProof: _withdrawalProof
         });
 
+        // Mock the status of the dispute game we just proved against to be CHALLENGER_WINS.
         vm.mockCall(address(game), abi.encodeCall(game.status, ()), abi.encode(GameStatus.CHALLENGER_WINS));
+        // Create a new game to re-prove against
+        disputeGameFactory.create(
+            optimismPortal2.respectedGameType(), Claim.wrap(_outputRoot), abi.encode(_proposedBlockNumber + 1)
+        );
+        _proposedGameIndex = disputeGameFactory.gameCount() - 1;
 
         vm.expectEmit(true, true, true, true);
         emit WithdrawalProven(_withdrawalHash, alice, bob);