Merge pull request #6016 from ethereum-optimism/clabby/ctb/optimize-bytes

feat(ctb): `toNibbles` optimization.

packages/contracts-bedrock/.gas-snapshot

@@ -2,9 +2,9 @@ Bytes_slice_Test:test_slice_acrossMultipleWords_works() (gas: 9413)
 Bytes_slice_Test:test_slice_acrossWords_works() (gas: 1430)
 Bytes_slice_Test:test_slice_fromNonZeroIdx_works() (gas: 17240)
 Bytes_slice_Test:test_slice_fromZeroIdx_works() (gas: 20826)
-Bytes_toNibbles_Test:test_toNibbles_expectedResult128Bytes_works() (gas: 129874)
-Bytes_toNibbles_Test:test_toNibbles_expectedResult5Bytes_works() (gas: 6132)
-Bytes_toNibbles_Test:test_toNibbles_zeroLengthInput_works() (gas: 944)
+Bytes_toNibbles_Test:test_toNibbles_expectedResult128Bytes_works() (gas: 78882)
+Bytes_toNibbles_Test:test_toNibbles_expectedResult5Bytes_works() (gas: 3992)
+Bytes_toNibbles_Test:test_toNibbles_zeroLengthInput_works() (gas: 823)
 CrossDomainMessenger_BaseGas_Test:test_baseGas_succeeds() (gas: 20412)
 CrossDomainOwnable2_Test:test_onlyOwner_notMessenger_reverts() (gas: 8416)
 CrossDomainOwnable2_Test:test_onlyOwner_notOwner2_reverts() (gas: 57515)
@@ -43,7 +43,7 @@ GasBenchMark_L1StandardBridge_Finalize:test_finalizeETHWithdrawal_benchmark() (g
 GasBenchMark_L2OutputOracle:test_proposeL2Output_benchmark() (gas: 88513)
 GasBenchMark_OptimismPortal:test_depositTransaction_benchmark() (gas: 75225)
 GasBenchMark_OptimismPortal:test_depositTransaction_benchmark_1() (gas: 75662)
-GasBenchMark_OptimismPortal:test_proveWithdrawalTransaction_benchmark() (gas: 169237)
+GasBenchMark_OptimismPortal:test_proveWithdrawalTransaction_benchmark() (gas: 143113)
 GasPriceOracle_Test:test_baseFee_succeeds() (gas: 8325)
 GasPriceOracle_Test:test_decimals_succeeds() (gas: 6167)
 GasPriceOracle_Test:test_gasPrice_succeeds() (gas: 8294)
@@ -207,25 +207,25 @@ LegacyERC20ETH_Test:test_transferFrom_doesNotExist_reverts() (gas: 12957)
 LegacyERC20ETH_Test:test_transfer_doesNotExist_reverts() (gas: 10755)
 LegacyMessagePasser_Test:test_passMessageToL1_succeeds() (gas: 34524)
 MerkleTrie_get_Test:test_get_corruptedProof_reverts() (gas: 5736)
-MerkleTrie_get_Test:test_get_extraProofElements_reverts() (gas: 60631)
+MerkleTrie_get_Test:test_get_extraProofElements_reverts() (gas: 58975)
 MerkleTrie_get_Test:test_get_invalidDataRemainder_reverts() (gas: 35852)
-MerkleTrie_get_Test:test_get_invalidInternalNodeHash_reverts() (gas: 50810)
-MerkleTrie_get_Test:test_get_nonexistentKey1_reverts() (gas: 59671)
-MerkleTrie_get_Test:test_get_nonexistentKey2_reverts() (gas: 23385)
-MerkleTrie_get_Test:test_get_smallerPathThanKey1_reverts() (gas: 53525)
-MerkleTrie_get_Test:test_get_smallerPathThanKey2_reverts() (gas: 55006)
-MerkleTrie_get_Test:test_get_validProof10_succeeds() (gas: 50593)
-MerkleTrie_get_Test:test_get_validProof1_succeeds() (gas: 61688)
-MerkleTrie_get_Test:test_get_validProof2_succeeds() (gas: 71579)
-MerkleTrie_get_Test:test_get_validProof3_succeeds() (gas: 32827)
-MerkleTrie_get_Test:test_get_validProof4_succeeds() (gas: 23623)
-MerkleTrie_get_Test:test_get_validProof5_succeeds() (gas: 84262)
-MerkleTrie_get_Test:test_get_validProof6_succeeds() (gas: 73021)
-MerkleTrie_get_Test:test_get_validProof7_succeeds() (gas: 79719)
-MerkleTrie_get_Test:test_get_validProof8_succeeds() (gas: 50550)
-MerkleTrie_get_Test:test_get_validProof9_succeeds() (gas: 50550)
-MerkleTrie_get_Test:test_get_wrongKeyProof_reverts() (gas: 53871)
-MerkleTrie_get_Test:test_get_zeroBranchValueLength_reverts() (gas: 43248)
+MerkleTrie_get_Test:test_get_invalidInternalNodeHash_reverts() (gas: 49706)
+MerkleTrie_get_Test:test_get_nonexistentKey1_reverts() (gas: 54839)
+MerkleTrie_get_Test:test_get_nonexistentKey2_reverts() (gas: 16723)
+MerkleTrie_get_Test:test_get_smallerPathThanKey1_reverts() (gas: 51472)
+MerkleTrie_get_Test:test_get_smallerPathThanKey2_reverts() (gas: 52953)
+MerkleTrie_get_Test:test_get_validProof10_succeeds() (gas: 48937)
+MerkleTrie_get_Test:test_get_validProof1_succeeds() (gas: 56062)
+MerkleTrie_get_Test:test_get_validProof2_succeeds() (gas: 65953)
+MerkleTrie_get_Test:test_get_validProof3_succeeds() (gas: 27356)
+MerkleTrie_get_Test:test_get_validProof4_succeeds() (gas: 18152)
+MerkleTrie_get_Test:test_get_validProof5_succeeds() (gas: 80776)
+MerkleTrie_get_Test:test_get_validProof6_succeeds() (gas: 69535)
+MerkleTrie_get_Test:test_get_validProof7_succeeds() (gas: 76233)
+MerkleTrie_get_Test:test_get_validProof8_succeeds() (gas: 48894)
+MerkleTrie_get_Test:test_get_validProof9_succeeds() (gas: 48894)
+MerkleTrie_get_Test:test_get_wrongKeyProof_reverts() (gas: 50782)
+MerkleTrie_get_Test:test_get_zeroBranchValueLength_reverts() (gas: 41747)
 MerkleTrie_get_Test:test_get_zeroLengthKey_reverts() (gas: 3632)
 MintManager_constructor_Test:test_constructor_succeeds() (gas: 10579)
 MintManager_mint_Test:test_mint_afterPeriodElapsed_succeeds() (gas: 148117)
@@ -265,26 +265,26 @@ OptimismPortalUpgradeable_Test:test_initialize_cannotInitImpl_reverts() (gas: 10
 OptimismPortalUpgradeable_Test:test_initialize_cannotInitProxy_reverts() (gas: 15918)
 OptimismPortalUpgradeable_Test:test_params_initValuesOnProxy_succeeds() (gas: 21774)
 OptimismPortalUpgradeable_Test:test_upgradeToAndCall_upgrading_succeeds() (gas: 180547)
-OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifOutputRootChanges_reverts() (gas: 204022)
-OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifOutputTimestampIsNotFinalized_reverts() (gas: 207475)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifOutputRootChanges_reverts() (gas: 177898)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifOutputTimestampIsNotFinalized_reverts() (gas: 181351)
 OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifWithdrawalNotProven_reverts() (gas: 41731)
-OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifWithdrawalProofNotOldEnough_reverts() (gas: 199419)
-OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onInsufficientGas_reverts() (gas: 205848)
-OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onRecentWithdrawal_reverts() (gas: 180184)
-OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onReentrancy_reverts() (gas: 243823)
-OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onReplay_reverts() (gas: 245561)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifWithdrawalProofNotOldEnough_reverts() (gas: 173295)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onInsufficientGas_reverts() (gas: 179724)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onRecentWithdrawal_reverts() (gas: 154060)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onReentrancy_reverts() (gas: 217699)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onReplay_reverts() (gas: 219437)
 OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_paused_reverts() (gas: 53510)
-OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_provenWithdrawalHash_succeeds() (gas: 234996)
-OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_targetFails_fails() (gas: 8797746687696163867)
-OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_timestampLessThanL2OracleStart_reverts() (gas: 196997)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_provenWithdrawalHash_succeeds() (gas: 208872)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_targetFails_fails() (gas: 8797746687696162662)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_timestampLessThanL2OracleStart_reverts() (gas: 170873)
 OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_onInvalidOutputRootProof_reverts() (gas: 85690)
-OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_onInvalidWithdrawalProof_reverts() (gas: 137350)
+OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_onInvalidWithdrawalProof_reverts() (gas: 111226)
 OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_onSelfCall_reverts() (gas: 52947)
 OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_paused_reverts() (gas: 73673)
-OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_replayProveChangedOutputRootAndOutputIndex_succeeds() (gas: 346739)
-OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_replayProveChangedOutputRoot_succeeds() (gas: 279571)
-OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_replayProve_reverts() (gas: 192548)
-OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_validWithdrawalProof_succeeds() (gas: 180486)
+OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_replayProveChangedOutputRootAndOutputIndex_succeeds() (gas: 294491)
+OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_replayProveChangedOutputRoot_succeeds() (gas: 227323)
+OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_replayProve_reverts() (gas: 166424)
+OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_validWithdrawalProof_succeeds() (gas: 154362)
 OptimismPortal_Test:test_constructor_succeeds() (gas: 19402)
 OptimismPortal_Test:test_depositTransaction_contractCreation_reverts() (gas: 14342)
 OptimismPortal_Test:test_depositTransaction_createWithZeroValueForContract_succeeds() (gas: 76726)

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

@@ -140,7 +140,7 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver {
     }
 
     /**
-     * @custom:semver 1.6.0
+     * @custom:semver 1.7.0
      *
      * @param _l2Oracle                  Address of the L2OutputOracle contract.
      * @param _guardian                  Address that can pause deposits and withdrawals.
@@ -152,7 +152,7 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver {
         address _guardian,
         bool _paused,
         SystemConfig _config
-    ) Semver(1, 6, 0) {
+    ) Semver(1, 7, 0) {
         L2_ORACLE = _l2Oracle;
         GUARDIAN = _guardian;
         SYSTEM_CONFIG = _config;

packages/contracts-bedrock/contracts/libraries/Bytes.sol

@@ -112,20 +112,50 @@ library Bytes {
      * @return Resulting nibble array.
      */
     function toNibbles(bytes memory _bytes) internal pure returns (bytes memory) {
-        uint256 bytesLength = _bytes.length;
-        bytes memory nibbles = new bytes(bytesLength * 2);
-        bytes1 b;
-
-        for (uint256 i = 0; i < bytesLength; ) {
-            b = _bytes[i];
-            nibbles[i * 2] = b >> 4;
-            nibbles[i * 2 + 1] = b & 0x0f;
-            unchecked {
-                ++i;
+        bytes memory _nibbles;
+        assembly {
+            // Grab a free memory offset for the new array
+            _nibbles := mload(0x40)
+
+            // Load the length of the passed bytes array from memory
+            let bytesLength := mload(_bytes)
+
+            // Calculate the length of the new nibble array
+            // This is the length of the input array times 2
+            let nibblesLength := shl(0x01, bytesLength)
+
+            // Update the free memory pointer to allocate memory for the new array.
+            // To do this, we add the length of the new array + 32 bytes for the array length
+            // rounded up to the nearest 32 byte boundary to the current free memory pointer.
+            mstore(0x40, add(_nibbles, and(not(0x1F), add(nibblesLength, 0x3F))))
+
+            // Store the length of the new array in memory
+            mstore(_nibbles, nibblesLength)
+
+            // 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))
             }
         }
-
-        return nibbles;
+        return _nibbles;
     }
 
     /**

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

@@ -182,49 +182,6 @@ contract Bytes_slice_Test is Test {
 }
 
 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)))
-
-                // 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.
@@ -272,11 +229,49 @@ contract Bytes_toNibbles_Test is Test {
     }
 
     /**
-     * @notice Test that the `toNibbles` function in the `Bytes` library is equivalent to the Yul
-     *         implementation.
+     * @notice Tests that the `toNibbles` function correctly updates the free memory pointer depending
+     *         on the length of the resulting array.
      */
-    function testDiff_toNibbles_succeeds(bytes memory _input) public {
-        assertEq(Bytes.toNibbles(_input), _toNibblesYul(_input));
+    function testFuzz_toNibbles_memorySafety_succeeds(bytes memory _input) public {
+        // Grab the free memory pointer before the `toNibbles` operation
+        uint64 initPtr;
+        assembly {
+            initPtr := mload(0x40)
+        }
+        uint64 expectedPtr = uint64(initPtr + 0x20 + ((_input.length * 2 + 0x1F) & ~uint256(0x1F)));
+
+        // Ensure that all memory outside of the expected range is safe.
+        vm.expectSafeMemory(initPtr, expectedPtr);
+
+        // Pull out each individual nibble from the input bytes array
+        bytes memory nibbles = Bytes.toNibbles(_input);
+
+        // Grab the free memory pointer after the `toNibbles` operation
+        uint64 finalPtr;
+        assembly {
+            finalPtr := mload(0x40)
+        }
+
+        // The free memory pointer should have been updated properly
+        if (_input.length == 0) {
+            // If the input length is zero, only 32 bytes of memory should have been allocated.
+            assertEq(finalPtr, initPtr + 0x20);
+        } else {
+            // If the input length is greater than zero, the memory allocated should be the
+            // length of the input * 2 + 32 bytes for the length field.
+            //
+            // Note that we use a slightly less efficient, but equivalent method of rounding
+            // up `_length` to the next multiple of 32 than is used in the `toNibbles` function.
+            // This is to diff test the method used in `toNibbles`.
+            uint64 _expectedPtr = uint64(initPtr + 0x20 + (((_input.length * 2 + 0x1F) >> 5) << 5));
+            assertEq(finalPtr, _expectedPtr);
+
+            // Sanity check for equivalence of the rounding methods.
+            assertEq(_expectedPtr, expectedPtr);
+        }
+
+        // The nibbles length should be equal to `_length * 2`
+        assertEq(nibbles.length, _input.length << 1);
     }
 }