Add guidelines for numeric literals (#5936)

Co-authored-by: ernestognw <[email protected]>

Author: Amxx

GUIDELINES.md

@@ -117,7 +117,7 @@ In addition to the official Solidity Style Guide we have a number of other conve
 
   Some standards (e.g. ERC-20) use present tense, and in those cases the
   standard specification is used.
-  
+
 * Interface names should have a capital I prefix.
 
   ```solidity
@@ -141,7 +141,7 @@ In addition to the official Solidity Style Guide we have a number of other conve
 * Unchecked arithmetic blocks should contain comments explaining why overflow is guaranteed not to happen. If the reason is immediately apparent from the line above the unchecked block, the comment may be omitted.
 
 * Custom errors should be declared following the [EIP-6093](https://eips.ethereum.org/EIPS/eip-6093) rationale whenever reasonable. Also, consider the following:
-  
+
   * The domain prefix should be picked in the following order:
     1. Use `ERC<number>` if the error is a violation of an ERC specification.
     2. Use the name of the underlying component where it belongs (eg. `Governor`, `ECDSA`, or `Timelock`).
@@ -153,3 +153,18 @@ In addition to the official Solidity Style Guide we have a number of other conve
     4. Declare the error in an extension if the error only happens in such extension or child contracts.
 
   * Custom error names should not be declared twice along the library to avoid duplicated identifier declarations when inheriting from multiple contracts.
+
+* Numeric literals should use appropriate formats based on their purpose to improve code readability:
+
+  **Memory-related operations (use hexadecimal):**
+  * Memory locations: `mload(64)` → `mload(0x40)`
+  * Memory offsets: `mstore(add(ptr, 32), value)` → `mstore(add(ptr, 0x20), value)`
+  * Memory lengths: `keccak256(ptr, 85)` → `keccak256(ptr, 0x55)`
+
+  **Bit operations (use decimal):**
+  * Shift amounts: `shl(0x80, value)` → `shl(128, value)`
+  * Bit masks and positions should use decimal when representing bit counts
+
+  **Exceptions:**
+  * Trivially small values (1, 2) may use decimal even in memory operations: `ptr := add(ptr, 1)`
+  * In `call`/`staticcall`/`delegatecall`, decimal zero is acceptable when both location and length are zero

contracts/account/utils/draft-ERC7579Utils.sol

@@ -126,10 +126,10 @@ library ERC7579Utils {
         Mode mode
     ) internal pure returns (CallType callType, ExecType execType, ModeSelector selector, ModePayload payload) {
         return (
-            CallType.wrap(Packing.extract_32_1(Mode.unwrap(mode), 0)),
-            ExecType.wrap(Packing.extract_32_1(Mode.unwrap(mode), 1)),
-            ModeSelector.wrap(Packing.extract_32_4(Mode.unwrap(mode), 6)),
-            ModePayload.wrap(Packing.extract_32_22(Mode.unwrap(mode), 10))
+            CallType.wrap(Packing.extract_32_1(Mode.unwrap(mode), 0x00)),
+            ExecType.wrap(Packing.extract_32_1(Mode.unwrap(mode), 0x01)),
+            ModeSelector.wrap(Packing.extract_32_4(Mode.unwrap(mode), 0x06)),
+            ModePayload.wrap(Packing.extract_32_22(Mode.unwrap(mode), 0x0a))
         );
     }
 
@@ -146,9 +146,9 @@ library ERC7579Utils {
     function decodeSingle(
         bytes calldata executionCalldata
     ) internal pure returns (address target, uint256 value, bytes calldata callData) {
-        target = address(bytes20(executionCalldata[0:20]));
-        value = uint256(bytes32(executionCalldata[20:52]));
-        callData = executionCalldata[52:];
+        target = address(bytes20(executionCalldata[0x00:0x14]));
+        value = uint256(bytes32(executionCalldata[0x14:0x34]));
+        callData = executionCalldata[0x34:];
     }
 
     /// @dev Encodes a delegate call execution. See {decodeDelegate}.
@@ -163,8 +163,8 @@ library ERC7579Utils {
     function decodeDelegate(
         bytes calldata executionCalldata
     ) internal pure returns (address target, bytes calldata callData) {
-        target = address(bytes20(executionCalldata[0:20]));
-        callData = executionCalldata[20:];
+        target = address(bytes20(executionCalldata[0:0x14]));
+        callData = executionCalldata[0x14:];
     }
 
     /// @dev Encodes a batch of executions. See {decodeBatch}.
@@ -180,17 +180,17 @@ library ERC7579Utils {
             uint256 bufferLength = executionCalldata.length;
 
             // Check executionCalldata is not empty.
-            if (bufferLength < 32) revert ERC7579DecodingError();
+            if (bufferLength < 0x20) revert ERC7579DecodingError();
 
             // Get the offset of the array (pointer to the array length).
-            uint256 arrayLengthOffset = uint256(bytes32(executionCalldata[0:32]));
+            uint256 arrayLengthOffset = uint256(bytes32(executionCalldata[0x00:0x20]));
 
             // The array length (at arrayLengthOffset) should be 32 bytes long. We check that this is within the
             // buffer bounds. Since we know bufferLength is at least 32, we can subtract with no overflow risk.
-            if (arrayLengthOffset > bufferLength - 32) revert ERC7579DecodingError();
+            if (arrayLengthOffset > bufferLength - 0x20) revert ERC7579DecodingError();
 
             // Get the array length. arrayLengthOffset + 32 is bounded by bufferLength so it does not overflow.
-            uint256 arrayLength = uint256(bytes32(executionCalldata[arrayLengthOffset:arrayLengthOffset + 32]));
+            uint256 arrayLength = uint256(bytes32(executionCalldata[arrayLengthOffset:arrayLengthOffset + 0x20]));
 
             // Check that the buffer is long enough to store the array elements as "offset pointer":
             // - each element of the array is an "offset pointer" to the data.
@@ -200,7 +200,7 @@ library ERC7579Utils {
             //
             // Since we know bufferLength is at least arrayLengthOffset + 32, we can subtract with no overflow risk.
             // Solidity limits length of such arrays to 2**64-1, this guarantees `arrayLength * 32` does not overflow.
-            if (arrayLength > type(uint64).max || bufferLength - arrayLengthOffset - 32 < arrayLength * 32)
+            if (arrayLength > type(uint64).max || bufferLength - arrayLengthOffset - 0x20 < arrayLength * 0x20)
                 revert ERC7579DecodingError();
 
             assembly ("memory-safe") {

contracts/metatx/ERC2771Forwarder.sol

@@ -287,7 +287,7 @@ contract ERC2771Forwarder is EIP712, Nonces {
             uint256 gasLeft;
 
             assembly ("memory-safe") {
-                success := call(reqGas, to, value, add(data, 0x20), mload(data), 0, 0)
+                success := call(reqGas, to, value, add(data, 0x20), mload(data), 0x00, 0x00)
                 gasLeft := gas()
             }
 
@@ -318,9 +318,9 @@ contract ERC2771Forwarder is EIP712, Nonces {
             // |-----------|----------|--------------------------------------------------------------------|
             // |           |          |                                                           result ↓ |
             // | 0x00:0x1F | selector | 0x0000000000000000000000000000000000000000000000000000000000000001 |
-            success := staticcall(gas(), target, add(encodedParams, 0x20), mload(encodedParams), 0, 0x20)
+            success := staticcall(gas(), target, add(encodedParams, 0x20), mload(encodedParams), 0x00, 0x20)
             returnSize := returndatasize()
-            returnValue := mload(0)
+            returnValue := mload(0x00)
         }
 
         return success && returnSize >= 0x20 && returnValue > 0;

contracts/proxy/Clones.sol

@@ -51,9 +51,9 @@ library Clones {
         assembly ("memory-safe") {
             // Cleans the upper 96 bits of the `implementation` word, then packs the first 3 bytes
             // of the `implementation` address with the bytecode before the address.
-            mstore(0x00, or(shr(0xe8, shl(0x60, implementation)), 0x3d602d80600a3d3981f3363d3d373d3d3d363d73000000))
+            mstore(0x00, or(shr(232, shl(96, implementation)), 0x3d602d80600a3d3981f3363d3d373d3d3d363d73000000))
             // Packs the remaining 17 bytes of `implementation` with the bytecode after the address.
-            mstore(0x20, or(shl(0x78, implementation), 0x5af43d82803e903d91602b57fd5bf3))
+            mstore(0x20, or(shl(120, implementation), 0x5af43d82803e903d91602b57fd5bf3))
             instance := create(value, 0x09, 0x37)
         }
         if (instance == address(0)) {
@@ -98,9 +98,9 @@ library Clones {
         assembly ("memory-safe") {
             // Cleans the upper 96 bits of the `implementation` word, then packs the first 3 bytes
             // of the `implementation` address with the bytecode before the address.
-            mstore(0x00, or(shr(0xe8, shl(0x60, implementation)), 0x3d602d80600a3d3981f3363d3d373d3d3d363d73000000))
+            mstore(0x00, or(shr(232, shl(96, implementation)), 0x3d602d80600a3d3981f3363d3d373d3d3d363d73000000))
             // Packs the remaining 17 bytes of `implementation` with the bytecode after the address.
-            mstore(0x20, or(shl(0x78, implementation), 0x5af43d82803e903d91602b57fd5bf3))
+            mstore(0x20, or(shl(120, implementation), 0x5af43d82803e903d91602b57fd5bf3))
             instance := create2(value, 0x09, 0x37, salt)
         }
         if (instance == address(0)) {
@@ -259,9 +259,9 @@ library Clones {
      *   function should only be used to check addresses that are known to be clones.
      */
     function fetchCloneArgs(address instance) internal view returns (bytes memory) {
-        bytes memory result = new bytes(instance.code.length - 45); // revert if length is too short
+        bytes memory result = new bytes(instance.code.length - 0x2d); // revert if length is too short
         assembly ("memory-safe") {
-            extcodecopy(instance, add(result, 32), 45, mload(result))
+            extcodecopy(instance, add(result, 0x20), 0x2d, mload(result))
         }
         return result;
     }
@@ -280,11 +280,11 @@ library Clones {
         address implementation,
         bytes memory args
     ) private pure returns (bytes memory) {
-        if (args.length > 24531) revert CloneArgumentsTooLong();
+        if (args.length > 0x5fd3) revert CloneArgumentsTooLong();
         return
             abi.encodePacked(
                 hex"61",
-                uint16(args.length + 45),
+                uint16(args.length + 0x2d),
                 hex"3d81600a3d39f3363d3d373d3d3d363d73",
                 implementation,
                 hex"5af43d82803e903d91602b57fd5bf3",

contracts/proxy/Proxy.sol

@@ -24,22 +24,22 @@ abstract contract Proxy {
             // Copy msg.data. We take full control of memory in this inline assembly
             // block because it will not return to Solidity code. We overwrite the
             // Solidity scratch pad at memory position 0.
-            calldatacopy(0, 0, calldatasize())
+            calldatacopy(0x00, 0x00, calldatasize())
 
             // Call the implementation.
             // out and outsize are 0 because we don't know the size yet.
-            let result := delegatecall(gas(), implementation, 0, calldatasize(), 0, 0)
+            let result := delegatecall(gas(), implementation, 0x00, calldatasize(), 0x00, 0x00)
 
             // Copy the returned data.
-            returndatacopy(0, 0, returndatasize())
+            returndatacopy(0x00, 0x00, returndatasize())
 
             switch result
             // delegatecall returns 0 on error.
             case 0 {
-                revert(0, returndatasize())
+                revert(0x00, returndatasize())
             }
             default {
-                return(0, returndatasize())
+                return(0x00, returndatasize())
             }
         }
     }

contracts/token/ERC20/utils/SafeERC20.sol

@@ -181,13 +181,13 @@ library SafeERC20 {
             mstore(0x00, selector)
             mstore(0x04, and(to, shr(96, not(0))))
             mstore(0x24, value)
-            success := call(gas(), token, 0, 0, 0x44, 0, 0x20)
+            success := call(gas(), token, 0, 0x00, 0x44, 0x00, 0x20)
             // if call success and return is true, all is good.
             // otherwise (not success or return is not true), we need to perform further checks
             if iszero(and(success, eq(mload(0x00), 1))) {
                 // if the call was a failure and bubble is enabled, bubble the error
                 if and(iszero(success), bubble) {
-                    returndatacopy(fmp, 0, returndatasize())
+                    returndatacopy(fmp, 0x00, returndatasize())
                     revert(fmp, returndatasize())
                 }
                 // if the return value is not true, then the call is only successful if:
@@ -224,13 +224,13 @@ library SafeERC20 {
             mstore(0x04, and(from, shr(96, not(0))))
             mstore(0x24, and(to, shr(96, not(0))))
             mstore(0x44, value)
-            success := call(gas(), token, 0, 0, 0x64, 0, 0x20)
+            success := call(gas(), token, 0, 0x00, 0x64, 0x00, 0x20)
             // if call success and return is true, all is good.
             // otherwise (not success or return is not true), we need to perform further checks
             if iszero(and(success, eq(mload(0x00), 1))) {
                 // if the call was a failure and bubble is enabled, bubble the error
                 if and(iszero(success), bubble) {
-                    returndatacopy(fmp, 0, returndatasize())
+                    returndatacopy(fmp, 0x00, returndatasize())
                     revert(fmp, returndatasize())
                 }
                 // if the return value is not true, then the call is only successful if:
@@ -260,13 +260,13 @@ library SafeERC20 {
             mstore(0x00, selector)
             mstore(0x04, and(spender, shr(96, not(0))))
             mstore(0x24, value)
-            success := call(gas(), token, 0, 0, 0x44, 0, 0x20)
+            success := call(gas(), token, 0, 0x00, 0x44, 0x00, 0x20)
             // if call success and return is true, all is good.
             // otherwise (not success or return is not true), we need to perform further checks
             if iszero(and(success, eq(mload(0x00), 1))) {
                 // if the call was a failure and bubble is enabled, bubble the error
                 if and(iszero(success), bubble) {
-                    returndatacopy(fmp, 0, returndatasize())
+                    returndatacopy(fmp, 0x00, returndatasize())
                     revert(fmp, returndatasize())
                 }
                 // if the return value is not true, then the call is only successful if:

contracts/utils/Bytes.sol

@@ -223,7 +223,7 @@ library Bytes {
      * if the buffer is all zeros.
      */
     function clz(bytes memory buffer) internal pure returns (uint256) {
-        for (uint256 i = 0; i < buffer.length; i += 32) {
+        for (uint256 i = 0; i < buffer.length; i += 0x20) {
             bytes32 chunk = _unsafeReadBytesOffset(buffer, i);
             if (chunk != bytes32(0)) {
                 return Math.min(8 * i + Math.clz(uint256(chunk)), 8 * buffer.length);

contracts/utils/Create2.sol

@@ -70,22 +70,22 @@ library Create2 {
         assembly ("memory-safe") {
             let ptr := mload(0x40) // Get free memory pointer
 
-            // |                   | ↓ ptr ...  ↓ ptr + 0x0B (start) ...  ↓ ptr + 0x20 ...  ↓ ptr + 0x40 ...   |
-            // |-------------------|---------------------------------------------------------------------------|
-            // | bytecodeHash      |                                                        CCCCCCCCCCCCC...CC |
-            // | salt              |                                      BBBBBBBBBBBBB...BB                   |
-            // | deployer          | 000000...0000AAAAAAAAAAAAAAAAAAA...AA                                     |
-            // | 0xFF              |            FF                                                             |
-            // |-------------------|---------------------------------------------------------------------------|
-            // | memory            | 000000...00FFAAAAAAAAAAAAAAAAAAA...AABBBBBBBBBBBBB...BBCCCCCCCCCCCCC...CC |
-            // | keccak(start, 85) |            ↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑ |
+            // |                     | ↓ ptr ...  ↓ ptr + 0x0B (start) ...  ↓ ptr + 0x20 ...  ↓ ptr + 0x40 ...   |
+            // |---------------------|---------------------------------------------------------------------------|
+            // | bytecodeHash        |                                                        CCCCCCCCCCCCC...CC |
+            // | salt                |                                      BBBBBBBBBBBBB...BB                   |
+            // | deployer            | 000000...0000AAAAAAAAAAAAAAAAAAA...AA                                     |
+            // | 0xFF                |            FF                                                             |
+            // |---------------------|---------------------------------------------------------------------------|
+            // | memory              | 000000...00FFAAAAAAAAAAAAAAAAAAA...AABBBBBBBBBBBBB...BBCCCCCCCCCCCCC...CC |
+            // | keccak(start, 0x55) |            ↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑ |
 
             mstore(add(ptr, 0x40), bytecodeHash)
             mstore(add(ptr, 0x20), salt)
             mstore(ptr, deployer) // Right-aligned with 12 preceding garbage bytes
             let start := add(ptr, 0x0b) // The hashed data starts at the final garbage byte which we will set to 0xff
             mstore8(start, 0xff)
-            addr := and(keccak256(start, 85), 0xffffffffffffffffffffffffffffffffffffffff)
+            addr := and(keccak256(start, 0x55), 0xffffffffffffffffffffffffffffffffffffffff)
         }
     }
 }