Merge pull request #4801 from ethereum-optimism/sc/ctb-bytes-test-cleanup

maint(ctb): clean up Alias and Bytes tests

packages/contracts-bedrock/.gas-snapshot

@@ -9,13 +9,13 @@ GasBenchMark_L2OutputOracle:test_proposeL2Output_benchmark() (gas: 88513)
 GasBenchMark_OptimismPortal:test_depositTransaction_benchmark() (gas: 74998)
 GasBenchMark_OptimismPortal:test_depositTransaction_benchmark_1() (gas: 36156)
 GasBenchMark_OptimismPortal:test_proveWithdrawalTransaction_benchmark() (gas: 167187)
-Bytes_Test:test_slice_acrossMultipleWords_works() (gas: 9391)
-Bytes_Test:test_slice_acrossWords_works() (gas: 1397)
-Bytes_Test:test_slice_fromNonZeroIdx_works() (gas: 17218)
-Bytes_Test:test_slice_fromZeroIdx_works() (gas: 20826)
-Bytes_Test:test_toNibbles_expectedResult128Bytes_works() (gas: 129885)
-Bytes_Test:test_toNibbles_expectedResult5Bytes_works() (gas: 6132)
-Bytes_Test:test_toNibbles_zeroLengthInput_works() (gas: 966)
+Bytes_slice_Test:test_slice_acrossMultipleWords_works() (gas: 9357)
+Bytes_slice_Test:test_slice_acrossWords_works() (gas: 1396)
+Bytes_slice_Test:test_slice_fromNonZeroIdx_works() (gas: 17154)
+Bytes_slice_Test:test_slice_fromZeroIdx_works() (gas: 20671)
+Bytes_toNibbles_Test:test_toNibbles_expectedResult128Bytes_works() (gas: 129830)
+Bytes_toNibbles_Test:test_toNibbles_expectedResult5Bytes_works() (gas: 6088)
+Bytes_toNibbles_Test:test_toNibbles_zeroLengthInput_works() (gas: 944)
 CrossDomainMessenger_BaseGas_Test:test_baseGas_succeeds() (gas: 20120)
 CrossDomainOwnableThroughPortal_Test:test_depositTransaction_crossDomainOwner_succeeds() (gas: 61882)
 CrossDomainOwnable_Test:test_onlyOwner_notOwner_reverts() (gas: 10530)

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

@@ -4,7 +4,10 @@ pragma solidity 0.8.15;
 import { Test } from "forge-std/Test.sol";
 import { AddressAliasHelper } from "../vendor/AddressAliasHelper.sol";
 
-contract AddressAliasHelper_Test is Test {
+contract AddressAliasHelper_applyAndUndo_Test is Test {
+    /**
+     * @notice Tests that applying and then undoing an alias results in the original address.
+     */
     function testFuzz_applyAndUndo_succeeds(address _address) external {
         address aliased = AddressAliasHelper.applyL1ToL2Alias(_address);
         address unaliased = AddressAliasHelper.undoL1ToL2Alias(aliased);

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

@@ -3,10 +3,10 @@ pragma solidity 0.8.15;
 import { Test } from "forge-std/Test.sol";
 import { Bytes } from "../libraries/Bytes.sol";
 
-/// @title BytesTest
-contract Bytes_Test is Test {
-    /// @dev Tests that the `slice` function works as expected when starting from
-    /// index 0.
+contract Bytes_slice_Test is Test {
+    /**
+     * @notice Tests that the `slice` function works as expected when starting from index 0.
+     */
     function test_slice_fromZeroIdx_works() public {
         bytes memory input = hex"11223344556677889900";
 
@@ -24,8 +24,10 @@ contract Bytes_Test is Test {
         assertEq(Bytes.slice(input, 0, 10), hex"11223344556677889900");
     }
 
-    /// @dev Tests that the `slice` function works as expected when starting from
-    /// indexes [1, 9] with lengths [1, 9], in reverse order.
+    /**
+     * @notice Tests that the `slice` function works as expected when starting from indices [1, 9]
+     *         with lengths [1, 9], in reverse order.
+     */
     function test_slice_fromNonZeroIdx_works() public {
         bytes memory input = hex"11223344556677889900";
 
@@ -42,10 +44,11 @@ contract Bytes_Test is Test {
         assertEq(Bytes.slice(input, 1, 9), hex"223344556677889900");
     }
 
-    /// @dev Tests that the `slice` function works as expected when slicing between
-    /// multiple words in memory. In this case, we test that a 2 byte slice between
-    /// the 32nd byte of the first word and the 1st byte of the second word is
-    /// correct.
+    /**
+     * @notice Tests that the `slice` function works as expected when slicing between multiple words
+     *         in memory. In this case, we test that a 2 byte slice between the 32nd byte of the
+     *         first word and the 1st byte of the second word is correct.
+     */
     function test_slice_acrossWords_works() public {
         bytes
             memory input = hex"00000000000000000000000000000000000000000000000000000000000000112200000000000000000000000000000000000000000000000000000000000000";
@@ -53,9 +56,11 @@ contract Bytes_Test is Test {
         assertEq(Bytes.slice(input, 31, 2), hex"1122");
     }
 
-    /// @dev Tests that the `slice` function works as expected when slicing between
-    /// multiple words in memory. In this case, we test that a 34 byte slice between
-    /// 3 separate words returns the correct result.
+    /**
+     * @notice Tests that the `slice` function works as expected when slicing between multiple
+     *         words in memory. In this case, we test that a 34 byte slice between 3 separate words
+     *         returns the correct result.
+     */
     function test_slice_acrossMultipleWords_works() public {
         bytes
             memory input = hex"000000000000000000000000000000000000000000000000000000000000001122FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF1100000000000000000000000000000000000000000000000000000000000000";
@@ -65,8 +70,10 @@ contract Bytes_Test is Test {
         assertEq(Bytes.slice(input, 31, 34), expected);
     }
 
-    /// @dev Tests that, when given an input bytes array of length `n`,
-    /// the `slice` function will always revert if `_start + _length > n`.
+    /**
+     * @notice Tests that, when given an input bytes array of length `n`, the `slice` function will
+     *         always revert if `_start + _length > n`.
+     */
     function testFuzz_slice_outOfBounds_reverts(
         bytes memory _input,
         uint256 _start,
@@ -81,8 +88,10 @@ contract Bytes_Test is Test {
         Bytes.slice(_input, _start, _length);
     }
 
-    /// @dev Tests that, when given a length `n` that is greater than `type(uint256).max - 31`,
-    /// the `slice` function reverts.
+    /**
+     * @notice Tests that, when given a length `n` that is greater than `type(uint256).max - 31`,
+     *         the `slice` function reverts.
+     */
     function testFuzz_slice_lengthOverflows_reverts(
         bytes memory _input,
         uint256 _start,
@@ -95,8 +104,10 @@ contract Bytes_Test is Test {
         Bytes.slice(_input, _start, _length);
     }
 
-    /// @dev Tests that, when given a length `n` that is greater than `type(uint256).max - 31`,
-    /// the `slice` function reverts.
+    /**
+     * @notice Tests that, when given a start index `n` that is greater than
+     *         `type(uint256).max - n`, the `slice` function reverts.
+     */
     function testFuzz_slice_rangeOverflows_reverts(
         bytes memory _input,
         uint256 _start,
@@ -111,9 +122,56 @@ contract Bytes_Test is Test {
         vm.expectRevert("slice_overflow");
         Bytes.slice(_input, _start, _length);
     }
+}
+
+contract Bytes_toNibbles_Test is Test {
+    /**
+     * @notice Diffs the test Solidity version of `toNibbles` against the Yul version.
+     *
+     * @param _bytes The `bytes` array to convert to nibbles.
+     *
+     * @return Yul version of `toNibbles` applied to `_bytes`.
+     */
+    function _toNibblesYul(bytes memory _bytes) internal pure returns (bytes memory) {
+        // Allocate memory for the `nibbles` array.
+        bytes memory nibbles = new bytes(_bytes.length << 1);
+
+        assembly {
+            // Load the length of the passed bytes array from memory
+            let bytesLength := mload(_bytes)
+
+            // Store the memory offset of the _bytes array's contents on the stack
+            let bytesStart := add(_bytes, 0x20)
+
+            // Store the memory offset of the nibbles array's contents on the stack
+            let nibblesStart := add(nibbles, 0x20)
+
+            // Loop through each byte in the input array
+            for {
+                let i := 0x00
+            } lt(i, bytesLength) {
+                i := add(i, 0x01)
+            } {
+                // Get the starting offset of the next 2 bytes in the nibbles array
+                let offset := add(nibblesStart, shl(0x01, i))
+
+                // Load the byte at the current index within the `_bytes` array
+                let b := byte(0x00, mload(add(bytesStart, i)))
 
-    /// @dev Tests that, given an input of 5 bytes, the `toNibbles` function returns
-    /// an array of 10 nibbles corresponding to the input data.
+                // Pull out the first nibble and store it in the new array
+                mstore8(offset, shr(0x04, b))
+                // Pull out the second nibble and store it in the new array
+                mstore8(add(offset, 0x01), and(b, 0x0F))
+            }
+        }
+
+        return nibbles;
+    }
+
+    /**
+     * @notice Tests that, given an input of 5 bytes, the `toNibbles` function returns an array of
+     *         10 nibbles corresponding to the input data.
+     */
     function test_toNibbles_expectedResult5Bytes_works() public {
         bytes memory input = hex"1234567890";
         bytes memory expected = hex"01020304050607080900";
@@ -124,10 +182,11 @@ contract Bytes_Test is Test {
         assertEq(actual, expected);
     }
 
-    /// @dev Tests that, given an input of 128 bytes, the `toNibbles` function returns
-    /// an array of 256 nibbles corresponding to the input data.
-    /// This test exists to ensure that, given a large input, the `toNibbles` function
-    /// works as expected.
+    /**
+     * @notice Tests that, given an input of 128 bytes, the `toNibbles` function returns an array
+     *         of 256 nibbles corresponding to the input data. This test exists to ensure that,
+     *         given a large input, the `toNibbles` function works as expected.
+     */
     function test_toNibbles_expectedResult128Bytes_works() public {
         bytes
             memory input = hex"000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f303132333435363738393a3b3c3d3e3f404142434445464748494a4b4c4d4e4f505152535455565758595a5b5c5d5e5f606162636465666768696a6b6c6d6e6f707172737475767778797a7b7c7d7e7f";
@@ -140,8 +199,10 @@ contract Bytes_Test is Test {
         assertEq(actual, expected);
     }
 
-    /// @dev Tests that, given an input of 0 bytes, the `toNibbles` function returns
-    /// a zero length array.
+    /**
+     * @notice Tests that, given an input of 0 bytes, the `toNibbles` function returns a zero
+     *         length array.
+     */
     function test_toNibbles_zeroLengthInput_works() public {
         bytes memory input = hex"";
         bytes memory expected = hex"";
@@ -153,30 +214,24 @@ contract Bytes_Test is Test {
         assertEq(actual, expected);
     }
 
-    /// @dev Test that the `toNibbles` function in the `Bytes` library is equivalent to the
-    /// Yul implementation.
+    /**
+     * @notice Test that the `toNibbles` function in the `Bytes` library is equivalent to the Yul
+     *         implementation.
+     */
     function testDiff_toNibbles_succeeds(bytes memory _input) public {
-        assertEq(Bytes.toNibbles(_input), toNibblesYul(_input));
-    }
-
-    /// @dev Test that the `equal` function in the `Bytes` library returns `false` if given
-    /// two non-equal byte arrays.
-    function testFuzz_equal_notEqual_works(bytes memory _a, bytes memory _b) public {
-        vm.assume(!manualEq(_a, _b));
-        assertFalse(Bytes.equal(_a, _b));
+        assertEq(Bytes.toNibbles(_input), _toNibblesYul(_input));
     }
+}
 
-    /// @dev Test whether or not the `equal` function in the `Bytes` library is equivalent
-    /// to manually checking equality of the two dynamic `bytes` arrays in memory.
-    function testDiff_equal_works(bytes memory _a, bytes memory _b) public {
-        assertEq(Bytes.equal(_a, _b), manualEq(_a, _b));
-    }
-
-    ////////////////////////////////////////////////////////////////
-    //                          HELPERS                           //
-    ////////////////////////////////////////////////////////////////
-
-    /// @dev Utility function to manually check equality of two dynamic `bytes` arrays in memory.
+contract Bytes_equal_Test is Test {
+    /**
+     * @notice Manually checks equality of two dynamic `bytes` arrays in memory.
+     *
+     * @param _a The first `bytes` array to compare.
+     * @param _b The second `bytes` array to compare.
+     *
+     * @return True if the two `bytes` arrays are equal in memory.
+     */
     function manualEq(bytes memory _a, bytes memory _b) internal pure returns (bool) {
         bool _eq;
         assembly {
@@ -191,40 +246,20 @@ contract Bytes_Test is Test {
         return _eq;
     }
 
-    /// @dev Utility function to diff test Solidity version of `toNibbles`
-    function toNibblesYul(bytes memory _bytes) internal pure returns (bytes memory) {
-        // Allocate memory for the `nibbles` array.
-        bytes memory nibbles = new bytes(_bytes.length << 1);
-
-        assembly {
-            // Load the length of the passed bytes array from memory
-            let bytesLength := mload(_bytes)
-
-            // Store the memory offset of the _bytes array's contents on the stack
-            let bytesStart := add(_bytes, 0x20)
-
-            // Store the memory offset of the nibbles array's contents on the stack
-            let nibblesStart := add(nibbles, 0x20)
-
-            // Loop through each byte in the input array
-            for {
-                let i := 0x00
-            } lt(i, bytesLength) {
-                i := add(i, 0x01)
-            } {
-                // Get the starting offset of the next 2 bytes in the nibbles array
-                let offset := add(nibblesStart, shl(0x01, i))
-
-                // Load the byte at the current index within the `_bytes` array
-                let b := byte(0x00, mload(add(bytesStart, i)))
-
-                // Pull out the first nibble and store it in the new array
-                mstore8(offset, shr(0x04, b))
-                // Pull out the second nibble and store it in the new array
-                mstore8(add(offset, 0x01), and(b, 0x0F))
-            }
+    /**
+     * @notice Tests that the `equal` function in the `Bytes` library returns `false` if given two
+     *         non-equal byte arrays.
+     */
+    function testFuzz_equal_notEqual_works(bytes memory _a, bytes memory _b) public {
+        vm.assume(!manualEq(_a, _b));
+        assertFalse(Bytes.equal(_a, _b));
     }
 
-        return nibbles;
+    /**
+     * @notice Test whether or not the `equal` function in the `Bytes` library is equivalent to
+     *         manually checking equality of the two dynamic `bytes` arrays in memory.
+     */
+    function testDiff_equal_works(bytes memory _a, bytes memory _b) public {
+        assertEq(Bytes.equal(_a, _b), manualEq(_a, _b));
     }
 }