Merge pull request #4802 from ethereum-optimism/sc/ctb-hashing-test-cleanup

maint(ctb): clean up Hashing tests

packages/contracts-bedrock/.gas-snapshot

@@ -46,7 +46,7 @@ GovernanceToken_Test:test_mint_fromNotOwner_reverts() (gas: 17052)
 GovernanceToken_Test:test_mint_fromOwner_succeeds() (gas: 108547)
 GovernanceToken_Test:test_transferFrom_succeeds() (gas: 146295)
 GovernanceToken_Test:test_transfer_succeeds() (gas: 138085)
-Hashing_Test:test_hashDepositSource_succeeds() (gas: 650)
+Hashing_hashDepositSource_Test:test_hashDepositSource_succeeds() (gas: 566)
 L1BlockTest:test_basefee_succeeds() (gas: 7553)
 L1BlockTest:test_hash_succeeds() (gas: 7510)
 L1BlockTest:test_number_succeeds() (gas: 7584)

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

@@ -6,20 +6,33 @@ import { Types } from "../libraries/Types.sol";
 import { Hashing } from "../libraries/Hashing.sol";
 import { Encoding } from "../libraries/Encoding.sol";
 
-contract Hashing_Test is CommonTest {
+contract Hashing_hashDepositSource_Test is CommonTest {
     function setUp() external {
         _setUp();
     }
 
+    /**
+     * @notice Tests that hashDepositSource returns the correct hash in a simple case.
+     */
     function test_hashDepositSource_succeeds() external {
-        bytes32 sourceHash = Hashing.hashDepositSource(
-            0xd25df7858efc1778118fb133ac561b138845361626dfb976699c5287ed0f4959,
-            0x1
+        assertEq(
+            Hashing.hashDepositSource(
+                0xd25df7858efc1778118fb133ac561b138845361626dfb976699c5287ed0f4959,
+                0x1
+            ),
+            0xf923fb07134d7d287cb52c770cc619e17e82606c21a875c92f4c63b65280a5cc
         );
+    }
+}
 
-        assertEq(sourceHash, 0xf923fb07134d7d287cb52c770cc619e17e82606c21a875c92f4c63b65280a5cc);
+contract Hashing_hashCrossDomainMessage_Test is CommonTest {
+    function setUp() external {
+        _setUp();
     }
 
+    /**
+     * @notice Tests that hashCrossDomainMessage returns the correct hash in a simple case.
+     */
     function testDiff_hashCrossDomainMessage_succeeds(
         uint240 _nonce,
         uint16 _version,
@@ -29,31 +42,25 @@ contract Hashing_Test is CommonTest {
         uint256 _gasLimit,
         bytes memory _data
     ) external {
-        // Ensure the version is valid
+        // Ensure the version is valid.
         uint16 version = uint16(bound(uint256(_version), 0, 1));
         uint256 nonce = Encoding.encodeVersionedNonce(_nonce, version);
 
-        bytes32 _hash = ffi.hashCrossDomainMessage(
-            nonce,
-            _sender,
-            _target,
-            _value,
-            _gasLimit,
-            _data
-        );
-
-        bytes32 hash = Hashing.hashCrossDomainMessage(
-            nonce,
-            _sender,
-            _target,
-            _value,
-            _gasLimit,
-            _data
+        assertEq(
+            Hashing.hashCrossDomainMessage(nonce, _sender, _target, _value, _gasLimit, _data),
+            ffi.hashCrossDomainMessage(nonce, _sender, _target, _value, _gasLimit, _data)
         );
+    }
+}
 
-        assertEq(hash, _hash);
+contract Hashing_hashWithdrawal_Test is CommonTest {
+    function setUp() external {
+        _setUp();
     }
 
+    /**
+     * @notice Tests that hashWithdrawal returns the correct hash in a simple case.
+     */
     function testDiff_hashWithdrawal_succeeds(
         uint256 _nonce,
         address _sender,
@@ -62,42 +69,56 @@ contract Hashing_Test is CommonTest {
         uint256 _gasLimit,
         bytes memory _data
     ) external {
-        bytes32 hash = Hashing.hashWithdrawal(
-            Types.WithdrawalTransaction(_nonce, _sender, _target, _value, _gasLimit, _data)
+        assertEq(
+            Hashing.hashWithdrawal(
+                Types.WithdrawalTransaction(_nonce, _sender, _target, _value, _gasLimit, _data)
+            ),
+            ffi.hashWithdrawal(_nonce, _sender, _target, _value, _gasLimit, _data)
         );
+    }
+}
 
-        bytes32 _hash = ffi.hashWithdrawal(_nonce, _sender, _target, _value, _gasLimit, _data);
-
-        assertEq(hash, _hash);
+contract Hashing_hashOutputRootProof_Test is CommonTest {
+    function setUp() external {
+        _setUp();
     }
 
+    /**
+     * @notice Tests that hashOutputRootProof returns the correct hash in a simple case.
+     */
     function testDiff_hashOutputRootProof_succeeds(
         bytes32 _version,
         bytes32 _stateRoot,
         bytes32 _messagePasserStorageRoot,
         bytes32 _latestBlockhash
     ) external {
-        Types.OutputRootProof memory proof = Types.OutputRootProof({
-            version: _version,
-            stateRoot: _stateRoot,
-            messagePasserStorageRoot: _messagePasserStorageRoot,
-            latestBlockhash: _latestBlockhash
-        });
-
-        bytes32 hash = Hashing.hashOutputRootProof(proof);
-
-        bytes32 _hash = ffi.hashOutputRootProof(
-            _version,
-            _stateRoot,
-            _messagePasserStorageRoot,
-            _latestBlockhash
+        assertEq(
+            Hashing.hashOutputRootProof(
+                Types.OutputRootProof({
+                    version: _version,
+                    stateRoot: _stateRoot,
+                    messagePasserStorageRoot: _messagePasserStorageRoot,
+                    latestBlockhash: _latestBlockhash
+                })
+            ),
+            ffi.hashOutputRootProof(
+                _version,
+                _stateRoot,
+                _messagePasserStorageRoot,
+                _latestBlockhash
+            )
         );
+    }
+}
 
-        assertEq(hash, _hash);
+contract Hashing_hashDepositTransaction_Test is CommonTest {
+    function setUp() external {
+        _setUp();
     }
 
-    // TODO(tynes): foundry bug cannot serialize
-    // bytes32 as strings with vm.toString
+    /**
+     * @notice Tests that hashDepositTransaction returns the correct hash in a simple case.
+     */
     function testDiff_hashDepositTransaction_succeeds(
         address _from,
         address _to,
@@ -107,30 +128,21 @@ contract Hashing_Test is CommonTest {
         bytes memory _data,
         uint256 _logIndex
     ) external {
-        bytes32 hash = Hashing.hashDepositTransaction(
-            Types.UserDepositTransaction(
-                _from,
-                _to,
-                false, // isCreate
-                _value,
-                _mint,
-                _gas,
-                _data,
-                bytes32(uint256(0)),
-                _logIndex
-            )
+        assertEq(
+            Hashing.hashDepositTransaction(
+                Types.UserDepositTransaction(
+                    _from,
+                    _to,
+                    false, // isCreate
+                    _value,
+                    _mint,
+                    _gas,
+                    _data,
+                    bytes32(uint256(0)),
+                    _logIndex
+                )
+            ),
+            ffi.hashDepositTransaction(_from, _to, _mint, _value, _gas, _data, _logIndex)
         );
-
-        bytes32 _hash = ffi.hashDepositTransaction(
-            _from,
-            _to,
-            _mint,
-            _value,
-            _gas,
-            _data,
-            _logIndex
-        );
-
-        assertEq(hash, _hash);
     }
 }