From 3c107fac4d3661ce3869c1a05f2f23c3e9efc611 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 27 Nov 2024 12:49:47 +0100 Subject: [PATCH 1/3] Rewrite assembly slot offset for consistencty --- contracts/account/utils/draft-ERC7579Utils.sol | 2 +- contracts/token/ERC1155/utils/ERC1155Utils.sol | 4 ++-- contracts/token/ERC20/utils/ERC1363Utils.sol | 4 ++-- contracts/token/ERC721/utils/ERC721Utils.sol | 2 +- contracts/utils/Address.sol | 3 +-- contracts/utils/Strings.sol | 4 ++-- 6 files changed, 9 insertions(+), 10 deletions(-) diff --git a/contracts/account/utils/draft-ERC7579Utils.sol b/contracts/account/utils/draft-ERC7579Utils.sol index 652f5554592..8cd46105aee 100644 --- a/contracts/account/utils/draft-ERC7579Utils.sol +++ b/contracts/account/utils/draft-ERC7579Utils.sol @@ -172,7 +172,7 @@ library ERC7579Utils { assembly ("memory-safe") { let ptr := add(executionCalldata.offset, calldataload(executionCalldata.offset)) // Extract the ERC7579 Executions - executionBatch.offset := add(ptr, 32) + executionBatch.offset := add(ptr, 0x20) executionBatch.length := calldataload(ptr) } } diff --git a/contracts/token/ERC1155/utils/ERC1155Utils.sol b/contracts/token/ERC1155/utils/ERC1155Utils.sol index 371cd86ba46..8796064fc61 100644 --- a/contracts/token/ERC1155/utils/ERC1155Utils.sol +++ b/contracts/token/ERC1155/utils/ERC1155Utils.sol @@ -42,7 +42,7 @@ library ERC1155Utils { revert IERC1155Errors.ERC1155InvalidReceiver(to); } else { assembly ("memory-safe") { - revert(add(32, reason), mload(reason)) + revert(add(reason, 0x20), mload(reason)) } } } @@ -79,7 +79,7 @@ library ERC1155Utils { revert IERC1155Errors.ERC1155InvalidReceiver(to); } else { assembly ("memory-safe") { - revert(add(32, reason), mload(reason)) + revert(add(reason, 0x20), mload(reason)) } } } diff --git a/contracts/token/ERC20/utils/ERC1363Utils.sol b/contracts/token/ERC20/utils/ERC1363Utils.sol index 6ba26901eb0..e2ce200809a 100644 --- a/contracts/token/ERC20/utils/ERC1363Utils.sol +++ b/contracts/token/ERC20/utils/ERC1363Utils.sol @@ -53,7 +53,7 @@ library ERC1363Utils { revert ERC1363InvalidReceiver(to); } else { assembly ("memory-safe") { - revert(add(32, reason), mload(reason)) + revert(add(reason, 0x20), mload(reason)) } } } @@ -87,7 +87,7 @@ library ERC1363Utils { revert ERC1363InvalidSpender(spender); } else { assembly ("memory-safe") { - revert(add(32, reason), mload(reason)) + revert(add(reason, 0x20), mload(reason)) } } } diff --git a/contracts/token/ERC721/utils/ERC721Utils.sol b/contracts/token/ERC721/utils/ERC721Utils.sol index 2fd091afd67..4ca31a2af7e 100644 --- a/contracts/token/ERC721/utils/ERC721Utils.sol +++ b/contracts/token/ERC721/utils/ERC721Utils.sol @@ -41,7 +41,7 @@ library ERC721Utils { revert IERC721Errors.ERC721InvalidReceiver(to); } else { assembly ("memory-safe") { - revert(add(32, reason), mload(reason)) + revert(add(reason, 0x20), mload(reason)) } } } diff --git a/contracts/utils/Address.sol b/contracts/utils/Address.sol index a1c8af296ce..b6b10ffb6fc 100644 --- a/contracts/utils/Address.sol +++ b/contracts/utils/Address.sol @@ -140,8 +140,7 @@ library Address { if (returndata.length > 0) { // The easiest way to bubble the revert reason is using memory via assembly assembly ("memory-safe") { - let returndata_size := mload(returndata) - revert(add(32, returndata), returndata_size) + revert(add(returndata, 0x20), mload(returndata)) } } else { revert Errors.FailedCall(); diff --git a/contracts/utils/Strings.sol b/contracts/utils/Strings.sol index 161a2b576d7..d290334bf44 100644 --- a/contracts/utils/Strings.sol +++ b/contracts/utils/Strings.sol @@ -40,7 +40,7 @@ library Strings { string memory buffer = new string(length); uint256 ptr; assembly ("memory-safe") { - ptr := add(buffer, add(32, length)) + ptr := add(buffer, add(length, 0x20)) } while (true) { ptr--; @@ -431,7 +431,7 @@ library Strings { function _unsafeReadBytesOffset(bytes memory buffer, uint256 offset) private pure returns (bytes32 value) { // This is not memory safe in the general case, but all calls to this private function are within bounds. assembly ("memory-safe") { - value := mload(add(buffer, add(0x20, offset))) + value := mload(add(buffer, add(offset, 0x20))) } } } From 715222075497e10c15eae54ac5338c1200ee66c4 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 27 Nov 2024 12:55:04 +0100 Subject: [PATCH 2/3] Apply suggestions from code review --- contracts/utils/Strings.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/utils/Strings.sol b/contracts/utils/Strings.sol index d290334bf44..4cad8a2c5a0 100644 --- a/contracts/utils/Strings.sol +++ b/contracts/utils/Strings.sol @@ -40,7 +40,7 @@ library Strings { string memory buffer = new string(length); uint256 ptr; assembly ("memory-safe") { - ptr := add(buffer, add(length, 0x20)) + ptr := add(add(buffer, 0x20), length) } while (true) { ptr--; @@ -431,7 +431,7 @@ library Strings { function _unsafeReadBytesOffset(bytes memory buffer, uint256 offset) private pure returns (bytes32 value) { // This is not memory safe in the general case, but all calls to this private function are within bounds. assembly ("memory-safe") { - value := mload(add(buffer, add(offset, 0x20))) + value := mload(add(add(buffer, 0x20), offset)) } } } From abe20dfd70667b9625240398d53104b6c95a4dda Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 27 Nov 2024 16:33:23 +0100 Subject: [PATCH 3/3] more consistency --- contracts/governance/Governor.sol | 2 +- contracts/utils/Bytes.sol | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/contracts/governance/Governor.sol b/contracts/governance/Governor.sol index 390411556b0..ea0d5c73582 100644 --- a/contracts/governance/Governor.sol +++ b/contracts/governance/Governor.sol @@ -827,7 +827,7 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72 function _unsafeReadBytesOffset(bytes memory buffer, uint256 offset) private pure returns (bytes32 value) { // This is not memory safe in the general case, but all calls to this private function are within bounds. assembly ("memory-safe") { - value := mload(add(buffer, add(0x20, offset))) + value := mload(add(add(buffer, 0x20), offset)) } } } diff --git a/contracts/utils/Bytes.sol b/contracts/utils/Bytes.sol index cf0cb8fcd7f..56ac20b90e1 100644 --- a/contracts/utils/Bytes.sol +++ b/contracts/utils/Bytes.sol @@ -92,7 +92,7 @@ library Bytes { // allocate and copy bytes memory result = new bytes(end - start); assembly ("memory-safe") { - mcopy(add(result, 0x20), add(buffer, add(start, 0x20)), sub(end, start)) + mcopy(add(result, 0x20), add(add(buffer, 0x20), start), sub(end, start)) } return result; @@ -107,7 +107,7 @@ library Bytes { function _unsafeReadBytesOffset(bytes memory buffer, uint256 offset) private pure returns (bytes32 value) { // This is not memory safe in the general case, but all calls to this private function are within bounds. assembly ("memory-safe") { - value := mload(add(buffer, add(0x20, offset))) + value := mload(add(add(buffer, 0x20), offset)) } } }