Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions GUIDELINES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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`).
Expand All @@ -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
28 changes: 14 additions & 14 deletions contracts/account/utils/draft-ERC7579Utils.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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))
);
}

Expand All @@ -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}.
Expand All @@ -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}.
Expand All @@ -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.
Expand All @@ -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") {
Expand Down
6 changes: 3 additions & 3 deletions contracts/metatx/ERC2771Forwarder.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the guidelines this are acceptable as decimal zero. Not a big deal.

gasLeft := gas()
}

Expand Down Expand Up @@ -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;
Expand Down
16 changes: 8 additions & 8 deletions contracts/proxy/Clones.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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;
}
Expand All @@ -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",
Expand Down
10 changes: 5 additions & 5 deletions contracts/proxy/Proxy.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
}
}
Expand Down
12 changes: 6 additions & 6 deletions contracts/token/ERC20/utils/SafeERC20.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion contracts/utils/Bytes.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
20 changes: 10 additions & 10 deletions contracts/utils/Create2.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
Loading