Fix TOB's high severity finding in `OptimismPortal`

packages/contracts-bedrock/.gas-snapshot

@@ -11,7 +11,7 @@ GasBenchMark_OptimismPortal:test_depositTransaction_benchmark() (gas: 74956)
 GasBenchMark_OptimismPortal:test_depositTransaction_benchmark_1() (gas: 35693)
 CrossDomainMessenger_Test:testFuzz_baseGas(uint32) (runs: 256, μ: 20196, ~: 20196)
 CrossDomainMessenger_Test:test_baseGas() (gas: 20054)
-CrossDomainOwnableThroughPortal_Test:test_depositTransaction_crossDomainOwner() (gas: 61809)
+CrossDomainOwnableThroughPortal_Test:test_depositTransaction_crossDomainOwner() (gas: 61815)
 CrossDomainOwnable_Test:test_onlyOwner() (gas: 34861)
 CrossDomainOwnable_Test:test_revertOnlyOwner() (gas: 10530)
 CrossDomainOwnable2_Test:test_onlyOwner() (gas: 77782)
@@ -21,7 +21,7 @@ CrossDomainOwnable2_Test:test_revertOnlyOwner() (gas: 61712)
 DeployerWhitelist_Test:test_owner() (gas: 7516)
 DeployerWhitelist_Test:test_storageSlots() (gas: 33395)
 Encoding_Test:test_decodeVersionedNonce_differential(uint240,uint16) (runs: 256, μ: 13334, ~: 13349)
-Encoding_Test:test_encodeCrossDomainMessage_differential(uint240,uint8,address,address,uint256,uint256,bytes) (runs: 256, μ: 87133, ~: 91570)
+Encoding_Test:test_encodeCrossDomainMessage_differential(uint240,uint8,address,address,uint256,uint256,bytes) (runs: 256, μ: 86806, ~: 91569)
 Encoding_Test:test_encodeDepositTransaction_differential(address,address,uint256,uint256,uint64,bool,bytes,uint256) (runs: 256, μ: 106734, ~: 102596)
 Encoding_Test:test_nonceVersioning(uint240,uint16) (runs: 256, μ: 658, ~: 658)
 FeeVault_Test:test_constructor() (gas: 10623)
@@ -33,10 +33,10 @@ GasPriceOracle_Test:test_overhead() (gas: 10568)
 GasPriceOracle_Test:test_scalar() (gas: 10610)
 GasPriceOracle_Test:test_setGasPriceReverts() (gas: 5888)
 GasPriceOracle_Test:test_setL1BaseFeeReverts() (gas: 5909)
-Hashing_Test:test_hashCrossDomainMessage_differential(uint240,uint16,address,address,uint256,uint256,bytes) (runs: 256, μ: 27668, ~: 27518)
+Hashing_Test:test_hashCrossDomainMessage_differential(uint240,uint16,address,address,uint256,uint256,bytes) (runs: 256, μ: 27677, ~: 27518)
 Hashing_Test:test_hashDepositSource() (gas: 628)
-Hashing_Test:test_hashDepositTransaction_differential(address,address,uint256,uint256,uint64,bytes,uint256) (runs: 256, μ: 66549, ~: 66341)
-Hashing_Test:test_hashOutputRootProof_differential(bytes32,bytes32,bytes32,bytes32) (runs: 256, μ: 73193, ~: 93323)
+Hashing_Test:test_hashDepositTransaction_differential(address,address,uint256,uint256,uint64,bytes,uint256) (runs: 256, μ: 66545, ~: 66341)
+Hashing_Test:test_hashOutputRootProof_differential(bytes32,bytes32,bytes32,bytes32) (runs: 256, μ: 73190, ~: 93323)
 Hashing_Test:test_hashWithdrawal_differential(uint256,address,address,uint256,uint256,bytes) (runs: 256, μ: 23580, ~: 23382)
 L1BlockTest:test_basefee() (gas: 7531)
 L1BlockTest:test_hash() (gas: 7553)
@@ -158,22 +158,23 @@ OptimismPortalUpgradeable_Test:test_initialize_cannotInitImpl_reverts() (gas: 10
 OptimismPortalUpgradeable_Test:test_initialize_cannotInitProxy_reverts() (gas: 15767)
 OptimismPortalUpgradeable_Test:test_params_initValuesOnProxy_success() (gas: 16010)
 OptimismPortalUpgradeable_Test:test_upgradeToAndCall_upgrading_success() (gas: 180435)
-OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_differential_success(address,address,uint256,uint256,bytes) (runs: 256, μ: 224555, ~: 224322)
-OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifOutputRootChanges_reverts() (gas: 192796)
-OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifOutputTimestampIsNotFinalized_reverts() (gas: 195084)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_differential_success(address,address,uint256,uint256,bytes) (runs: 256, μ: 224693, ~: 224460)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifOutputRootChanges_reverts() (gas: 192897)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifOutputTimestampIsNotFinalized_reverts() (gas: 195163)
 OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifWithdrawalNotProven_reverts() (gas: 39589)
-OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifWithdrawalProofNotOldEnough_reverts() (gas: 190538)
-OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onInsufficientGas_reverts() (gas: 192953)
-OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onRecentWithdrawal_reverts() (gas: 173010)
-OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onReentrancy_reverts() (gas: 233210)
-OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onReplay_reverts() (gas: 232682)
-OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_provenWithdrawalHash_success() (gas: 224676)
-OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_targetFails_fails() (gas: 327313)
-OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_timestampLessThanL2OracleStart_reverts() (gas: 191294)
-OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_onInvalidOutputRootProof_reverts() (gas: 81333)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifWithdrawalProofNotOldEnough_reverts() (gas: 190639)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onInsufficientGas_reverts() (gas: 193046)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onRecentWithdrawal_reverts() (gas: 173111)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onReentrancy_reverts() (gas: 233303)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onReplay_reverts() (gas: 232783)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_provenWithdrawalHash_success() (gas: 224777)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_targetFails_fails() (gas: 327459)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_timestampLessThanL2OracleStart_reverts() (gas: 191395)
+OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_onInvalidOutputRootProof_reverts() (gas: 81311)
 OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_onSelfCall_reverts() (gas: 50776)
-OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_oninvalidWithdrawalProof_reverts() (gas: 130105)
-OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_validWithdrawalProof_success() (gas: 176847)
+OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_oninvalidWithdrawalProof_reverts() (gas: 132306)
+OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_replayProve_reverts() (gas: 186207)
+OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_validWithdrawalProof_success() (gas: 176926)
 OptimismPortal_Test:test_OptimismPortalConstructor() (gas: 17298)
 OptimismPortal_Test:test_OptimismPortalReceiveEth_success() (gas: 127483)
 OptimismPortal_Test:test_depositTransaction_NoValueContract_success() (gas: 76706)

packages/contracts-bedrock/contracts/L1/OptimismPortal.sol

@@ -189,6 +189,13 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver {
         // and to prevent replay attacks.
         bytes32 withdrawalHash = Hashing.hashWithdrawal(_tx);
 
+        // Ensure that the withdrawalHash has not already been proven to prevent a malicious party
+        // from censoring the withdrawal.
+        require(
+            provenWithdrawals[withdrawalHash].timestamp == 0,
+            "OptimismPortal: withdrawalHash has already been proven"
+        );
+
         // Verify that the hash of this withdrawal was stored in the L2toL1MessagePasser contract on
         //  L2. If this is true, then we know that this withdrawal was actually triggered on L2
         // and can therefore be relayed on L1.
@@ -202,9 +209,8 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver {
         );
 
         // Designate the withdrawalHash as proven by storing the `outputRoot`, `timestamp`,
-        // and `l2BlockNumber` in the `provenWithdrawals` mapping. A certain withdrawal
-        // can be proved multiple times and thus overwrite a previously stored `ProvenWithdrawal`,
-        // but this is safe due to the replay check in `finalizeWithdrawalTransaction`.
+        // and `l2BlockNumber` in the `provenWithdrawals` mapping. A withdrawalHash can only
+        // be proven one time to prevent a censorship attack.
         provenWithdrawals[withdrawalHash] = ProvenWithdrawal({
             outputRoot: outputRoot,
             timestamp: uint128(block.timestamp),

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

@@ -371,6 +371,27 @@ contract OptimismPortal_FinalizeWithdrawal_Test is Portal_Initializer {
         );
     }
 
+    // Test: proveWithdrawalTransaction reverts if the passed transaction's withdrawalHash has
+    // already been proven.
+    function test_proveWithdrawalTransaction_replayProve_reverts() external {
+        vm.expectEmit(true, true, true, true);
+        emit WithdrawalProven(_withdrawalHash, alice, bob);
+        op.proveWithdrawalTransaction(
+            _defaultTx,
+            _proposedBlockNumber,
+            _outputRootProof,
+            _withdrawalProof
+        );
+
+        vm.expectRevert("OptimismPortal: withdrawalHash has already been proven");
+        op.proveWithdrawalTransaction(
+            _defaultTx,
+            _proposedBlockNumber,
+            _outputRootProof,
+            _withdrawalProof
+        );
+    }
+
     // Test: proveWithdrawalTransaction succeeds and emits the WithdrawalProven event.
     function test_proveWithdrawalTransaction_validWithdrawalProof_success() external {
         vm.expectEmit(true, true, true, true);