From 5dfb2b3e7d39bf53556b5b20d37fcc75e3acf755 Mon Sep 17 00:00:00 2001 From: ernestognw Date: Wed, 19 Jun 2024 21:43:14 -0600 Subject: [PATCH 01/56] Implement LowLevelCall library --- contracts/access/manager/AuthorityUtils.sol | 25 ++-- contracts/token/ERC20/extensions/ERC4626.sol | 30 ++-- contracts/token/ERC20/utils/SafeERC20.sol | 8 +- contracts/utils/Address.sol | 3 +- contracts/utils/LowLevelCall.sol | 140 ++++++++++++++++++ contracts/utils/Memory.sol | 19 +++ .../utils/cryptography/SignatureChecker.sol | 17 ++- 7 files changed, 209 insertions(+), 33 deletions(-) create mode 100644 contracts/utils/LowLevelCall.sol create mode 100644 contracts/utils/Memory.sol diff --git a/contracts/access/manager/AuthorityUtils.sol b/contracts/access/manager/AuthorityUtils.sol index fb3018ca805..c35bb31f141 100644 --- a/contracts/access/manager/AuthorityUtils.sol +++ b/contracts/access/manager/AuthorityUtils.sol @@ -4,6 +4,8 @@ pragma solidity ^0.8.20; import {IAuthority} from "./IAuthority.sol"; +import {Memory} from "../../utils/Memory.sol"; +import {LowLevelCall} from "../../utils/LowLevelCall.sol"; library AuthorityUtils { /** @@ -17,16 +19,21 @@ library AuthorityUtils { address target, bytes4 selector ) internal view returns (bool immediate, uint32 delay) { - (bool success, bytes memory data) = authority.staticcall( - abi.encodeCall(IAuthority.canCall, (caller, target, selector)) + Memory.Pointer ptr = Memory.saveFreePointer(); + bytes memory params = abi.encodeCall(IAuthority.canCall, (caller, target, selector)); + (bool success, bytes32 immediateWord, bytes32 delayWord) = LowLevelCall.staticCallReturnBytes32Tuple( + authority, + params ); - if (success) { - if (data.length >= 0x40) { - (immediate, delay) = abi.decode(data, (bool, uint32)); - } else if (data.length >= 0x20) { - immediate = abi.decode(data, (bool)); - } + Memory.loadFreePointer(ptr); + + if (!success) { + return (false, 0); } - return (immediate, delay); + + return ( + uint256(immediateWord) != 0, + uint32(uint256(delayWord)) // Intentional overflow to truncate the higher 224 bits + ); } } diff --git a/contracts/token/ERC20/extensions/ERC4626.sol b/contracts/token/ERC20/extensions/ERC4626.sol index c71b14ad48c..1c63174c155 100644 --- a/contracts/token/ERC20/extensions/ERC4626.sol +++ b/contracts/token/ERC20/extensions/ERC4626.sol @@ -7,6 +7,8 @@ import {IERC20, IERC20Metadata, ERC20} from "../ERC20.sol"; import {SafeERC20} from "../utils/SafeERC20.sol"; import {IERC4626} from "../../../interfaces/IERC4626.sol"; import {Math} from "../../../utils/math/Math.sol"; +import {Memory} from "../../../utils/Memory.sol"; +import {LowLevelCall} from "../../../utils/LowLevelCall.sol"; /** * @dev Implementation of the ERC-4626 "Tokenized Vault Standard" as defined in @@ -75,25 +77,21 @@ abstract contract ERC4626 is ERC20, IERC4626 { * @dev Set the underlying asset contract. This must be an ERC20-compatible contract (ERC-20 or ERC-777). */ constructor(IERC20 asset_) { - (bool success, uint8 assetDecimals) = _tryGetAssetDecimals(asset_); - _underlyingDecimals = success ? assetDecimals : 18; + _underlyingDecimals = _tryGetAssetDecimalsWithFallback(asset_, 18); _asset = asset_; } - /** - * @dev Attempts to fetch the asset decimals. A return value of false indicates that the attempt failed in some way. - */ - function _tryGetAssetDecimals(IERC20 asset_) private view returns (bool, uint8) { - (bool success, bytes memory encodedDecimals) = address(asset_).staticcall( - abi.encodeCall(IERC20Metadata.decimals, ()) - ); - if (success && encodedDecimals.length >= 32) { - uint256 returnedDecimals = abi.decode(encodedDecimals, (uint256)); - if (returnedDecimals <= type(uint8).max) { - return (true, uint8(returnedDecimals)); - } - } - return (false, 0); + function _tryGetAssetDecimalsWithFallback(IERC20 asset_, uint8 defaultValue) private view returns (uint8) { + Memory.Pointer ptr = Memory.saveFreePointer(); + bytes memory params = abi.encodeCall(IERC20Metadata.decimals, ()); + + (bool success, bytes32 rawValue) = LowLevelCall.staticCallReturnBytes32(address(asset_), params); + uint256 length = LowLevelCall.returnDataSize(); + uint256 value = uint256(rawValue); + + Memory.loadFreePointer(ptr); + + return uint8(Math.ternary(success && length >= 0x20 && value <= type(uint8).max, value, defaultValue)); } /** diff --git a/contracts/token/ERC20/utils/SafeERC20.sol b/contracts/token/ERC20/utils/SafeERC20.sol index 58f9fcf4d68..08bae09ce06 100644 --- a/contracts/token/ERC20/utils/SafeERC20.sol +++ b/contracts/token/ERC20/utils/SafeERC20.sol @@ -6,6 +6,7 @@ pragma solidity ^0.8.20; import {IERC20} from "../IERC20.sol"; import {IERC1363} from "../../../interfaces/IERC1363.sol"; import {Address} from "../../../utils/Address.sol"; +import {Memory} from "../../../utils/Memory.sol"; /** * @title SafeERC20 @@ -34,7 +35,9 @@ library SafeERC20 { * non-reverting calls are assumed to be successful. */ function safeTransfer(IERC20 token, address to, uint256 value) internal { + Memory.Pointer ptr = Memory.saveFreePointer(); _callOptionalReturn(token, abi.encodeCall(token.transfer, (to, value))); + Memory.loadFreePointer(ptr); } /** @@ -42,7 +45,9 @@ library SafeERC20 { * calling contract. If `token` returns no value, non-reverting calls are assumed to be successful. */ function safeTransferFrom(IERC20 token, address from, address to, uint256 value) internal { + Memory.Pointer ptr = Memory.saveFreePointer(); _callOptionalReturn(token, abi.encodeCall(token.transferFrom, (from, to, value))); + Memory.loadFreePointer(ptr); } /** @@ -74,12 +79,13 @@ library SafeERC20 { * to be set to zero before setting it to a non-zero value, such as USDT. */ function forceApprove(IERC20 token, address spender, uint256 value) internal { + Memory.Pointer ptr = Memory.saveFreePointer(); bytes memory approvalCall = abi.encodeCall(token.approve, (spender, value)); - if (!_callOptionalReturnBool(token, approvalCall)) { _callOptionalReturn(token, abi.encodeCall(token.approve, (spender, 0))); _callOptionalReturn(token, approvalCall); } + Memory.loadFreePointer(ptr); } /** diff --git a/contracts/utils/Address.sol b/contracts/utils/Address.sol index 53a3c442049..bb371e9fa28 100644 --- a/contracts/utils/Address.sol +++ b/contracts/utils/Address.sol @@ -4,6 +4,7 @@ pragma solidity ^0.8.20; import {Errors} from "./Errors.sol"; +import {LowLevelCall} from "./LowLevelCall.sol"; /** * @dev Collection of functions related to the address type @@ -35,7 +36,7 @@ library Address { revert Errors.InsufficientBalance(address(this).balance, amount); } - (bool success, ) = recipient.call{value: amount}(""); + bool success = LowLevelCall.callRaw(recipient, amount, ""); if (!success) { revert Errors.FailedCall(); } diff --git a/contracts/utils/LowLevelCall.sol b/contracts/utils/LowLevelCall.sol new file mode 100644 index 00000000000..42c7f24ba9c --- /dev/null +++ b/contracts/utils/LowLevelCall.sol @@ -0,0 +1,140 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +import {Errors} from "./Errors.sol"; + +/** + * @dev Library of low level call functions that implement different calling strategies to deal with the return data. + */ +library LowLevelCall { + /// === CALL === + + /// @dev Performs a Solidity function call using a low level `call` and ignoring the return data. + function callRaw(address target, uint256 value, bytes memory data) internal returns (bool success) { + assembly ("memory-safe") { + success := call(gas(), target, value, add(data, 0x20), mload(data), 0, 0) + } + } + + /// @dev Performs a Solidity function call using a low level `call` and returns the first 32 bytes of the result + /// in the scratch space of memory. Useful for functions that return a single-word value. + /// + /// WARNING: Do not assume that the result is zero if `success` is false. Memory can be already allocated + /// and this function doesn't zero it out. + function callReturnBytes32( + address target, + uint256 value, + bytes memory data + ) internal returns (bool success, bytes32 result) { + assembly ("memory-safe") { + success := call(gas(), target, value, add(data, 0x20), mload(data), 0, 0x20) + result := mload(0) + } + } + + /// @dev Performs a Solidity function call using a low level `call` and returns the first 64 bytes of the result + /// in the scratch space of memory. Useful for functions that return a tuple of single-word values values. + /// + /// WARNING: Do not assume that the results are zero if `success` is false. Memory can be already allocated + /// and this function doesn't zero it out. + function callReturnBytes32Tuple( + address target, + uint256 value, + bytes memory data + ) internal returns (bool success, bytes32 result1, bytes32 result2) { + assembly ("memory-safe") { + success := call(gas(), target, value, add(data, 0x20), mload(data), 0, 0x40) + result1 := mload(0) + result2 := mload(0x20) + } + } + + /// @dev Performs a Solidity function call using a low level `call` and writes the result to the memory location + /// specified by `resultPtr`. + /// + /// IMPORTANT: This function assumes that the length of the memory array is stored in the first 32 bytes of the array and uses it for truncating + /// returndata if it's longer than the allocated memory to avoid corrupting to further places in memory. The `resultPtr` should be a + /// memory location that is already allocated with a predefined length. + /// + /// WARNING: Do not use if writing to `resultPtr` is not safe according to + /// the https://docs.soliditylang.org/en/latest/assembly.html#memory-safety[Solidity documentation]. + function callReturnOverride( + address target, + uint256 value, + bytes memory data, + bytes memory resultPtr + ) internal returns (bool success) { + assembly ("memory-safe") { + let maxSize := mload(resultPtr) + success := call(gas(), target, value, add(data, 0x20), mload(data), resultPtr, maxSize) + } + } + + /// === STATICCALL === + + /// @dev Performs a Solidity function call using a low level `staticcall` and ignoring the return data. + function staticCallRaw(address target, bytes memory data) internal view returns (bool success) { + assembly ("memory-safe") { + success := staticcall(gas(), target, add(data, 0x20), mload(data), 0, 0) + } + } + + /// @dev Performs a Solidity function call using a low level `staticcall` and returns the first 32 bytes of the result + /// in the scratch space of memory. Useful for functions that return a single-word value. + /// + /// WARNING: Do not assume that the result is zero if `success` is false. Memory can be already allocated + /// and this function doesn't zero it out. + function staticCallReturnBytes32( + address target, + bytes memory data + ) internal view returns (bool success, bytes32 result) { + assembly ("memory-safe") { + success := staticcall(gas(), target, add(data, 0x20), mload(data), 0, 0x20) + result := mload(0) + } + } + + /// @dev Performs a Solidity function call using a low level `staticcall` and returns the first 64 bytes of the result + /// in the scratch space of memory. Useful for functions that return a tuple of single-word values values. + /// + /// WARNING: Do not assume that the results are zero if `success` is false. Memory can be already allocated + /// and this function doesn't zero it out. + function staticCallReturnBytes32Tuple( + address target, + bytes memory data + ) internal view returns (bool success, bytes32 result1, bytes32 result2) { + assembly ("memory-safe") { + success := staticcall(gas(), target, add(data, 0x20), mload(data), 0, 0x40) + result1 := mload(0) + result2 := mload(0x20) + } + } + + /// @dev Performs a Solidity function call using a low level `staticcall` and writes the result to the memory location + /// specified by `resultPtr`. + /// + /// IMPORTANT: This function assumes that the length of the memory array is stored in the first 32 bytes of the array and uses it for truncating + /// returndata if it's longer than the allocated memory to avoid corrupting to further places in memory. The `resultPtr` should be a + /// memory location that is already allocated with a predefined length. + /// + /// WARNING: Do not use if writing to `resultPtr` is not safe according to + /// the https://docs.soliditylang.org/en/latest/assembly.html#memory-safety[Solidity documentation]. + function staticCallReturnOverride( + address target, + bytes memory data, + bytes memory resultPtr + ) internal view returns (bool success) { + assembly ("memory-safe") { + let maxSize := mload(resultPtr) + success := staticcall(gas(), target, add(data, 0x20), mload(data), resultPtr, maxSize) + } + } + + /// @dev Returns the size of the return data buffer. + function returnDataSize() internal pure returns (uint256 size) { + assembly ("memory-safe") { + size := returndatasize() + } + } +} diff --git a/contracts/utils/Memory.sol b/contracts/utils/Memory.sol new file mode 100644 index 00000000000..3f0ea05f70c --- /dev/null +++ b/contracts/utils/Memory.sol @@ -0,0 +1,19 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +library Memory { + type Pointer is bytes32; + + function saveFreePointer() internal pure returns (Pointer ptr) { + assembly ("memory-safe") { + ptr := mload(0x40) + } + } + + function loadFreePointer(Pointer ptr) internal pure { + assembly ("memory-safe") { + mstore(0x40, ptr) + } + } +} diff --git a/contracts/utils/cryptography/SignatureChecker.sol b/contracts/utils/cryptography/SignatureChecker.sol index 9aaa2e0716c..b0d8c3c644e 100644 --- a/contracts/utils/cryptography/SignatureChecker.sol +++ b/contracts/utils/cryptography/SignatureChecker.sol @@ -5,6 +5,8 @@ pragma solidity ^0.8.20; import {ECDSA} from "./ECDSA.sol"; import {IERC1271} from "../../interfaces/IERC1271.sol"; +import {Memory} from "../Memory.sol"; +import {LowLevelCall} from "../LowLevelCall.sol"; /** * @dev Signature verification helper that can be used instead of `ECDSA.recover` to seamlessly support both ECDSA @@ -40,11 +42,14 @@ library SignatureChecker { bytes32 hash, bytes memory signature ) internal view returns (bool) { - (bool success, bytes memory result) = signer.staticcall( - abi.encodeCall(IERC1271.isValidSignature, (hash, signature)) - ); - return (success && - result.length >= 32 && - abi.decode(result, (bytes32)) == bytes32(IERC1271.isValidSignature.selector)); + bytes4 magic = IERC1271.isValidSignature.selector; + + Memory.Pointer ptr = Memory.saveFreePointer(); + bytes memory params = abi.encodeCall(IERC1271.isValidSignature, (hash, signature)); + (bool success, bytes32 result) = LowLevelCall.staticCallReturnBytes32(signer, params); + uint256 length = LowLevelCall.returnDataSize(); + Memory.loadFreePointer(ptr); + + return success && length >= 32 && result == bytes32(magic); } } From e8e8438b860b8f68c487fe3324012279e05125e0 Mon Sep 17 00:00:00 2001 From: ernestognw Date: Wed, 19 Jun 2024 21:52:39 -0600 Subject: [PATCH 02/56] Add comments to Memory library --- contracts/utils/Memory.sol | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/contracts/utils/Memory.sol b/contracts/utils/Memory.sol index 3f0ea05f70c..f959a88bf7c 100644 --- a/contracts/utils/Memory.sol +++ b/contracts/utils/Memory.sol @@ -2,15 +2,20 @@ pragma solidity ^0.8.20; +/// @dev Memory utility library. library Memory { type Pointer is bytes32; + /// @dev Returns a memory pointer to the current free memory pointer. function saveFreePointer() internal pure returns (Pointer ptr) { assembly ("memory-safe") { ptr := mload(0x40) } } + /// @dev Sets the free memory pointer to a specific value. + /// + /// WARNING: Everything after the pointer may be overwritten. function loadFreePointer(Pointer ptr) internal pure { assembly ("memory-safe") { mstore(0x40, ptr) From 8e1cc9c38b36116ec9ea75b9abcd8f0fb1d9c2aa Mon Sep 17 00:00:00 2001 From: ernestognw Date: Sat, 22 Jun 2024 14:50:15 -0600 Subject: [PATCH 03/56] Update names --- contracts/access/manager/AuthorityUtils.sol | 2 +- contracts/token/ERC20/extensions/ERC4626.sol | 2 +- contracts/utils/LowLevelCall.sol | 49 ++----------------- .../utils/cryptography/SignatureChecker.sol | 2 +- 4 files changed, 7 insertions(+), 48 deletions(-) diff --git a/contracts/access/manager/AuthorityUtils.sol b/contracts/access/manager/AuthorityUtils.sol index c35bb31f141..4f9b6c90c94 100644 --- a/contracts/access/manager/AuthorityUtils.sol +++ b/contracts/access/manager/AuthorityUtils.sol @@ -21,7 +21,7 @@ library AuthorityUtils { ) internal view returns (bool immediate, uint32 delay) { Memory.Pointer ptr = Memory.saveFreePointer(); bytes memory params = abi.encodeCall(IAuthority.canCall, (caller, target, selector)); - (bool success, bytes32 immediateWord, bytes32 delayWord) = LowLevelCall.staticCallReturnBytes32Tuple( + (bool success, bytes32 immediateWord, bytes32 delayWord) = LowLevelCall.staticcallReturnScratchBytes32Pair( authority, params ); diff --git a/contracts/token/ERC20/extensions/ERC4626.sol b/contracts/token/ERC20/extensions/ERC4626.sol index 1c63174c155..c5f70604bcf 100644 --- a/contracts/token/ERC20/extensions/ERC4626.sol +++ b/contracts/token/ERC20/extensions/ERC4626.sol @@ -85,7 +85,7 @@ abstract contract ERC4626 is ERC20, IERC4626 { Memory.Pointer ptr = Memory.saveFreePointer(); bytes memory params = abi.encodeCall(IERC20Metadata.decimals, ()); - (bool success, bytes32 rawValue) = LowLevelCall.staticCallReturnBytes32(address(asset_), params); + (bool success, bytes32 rawValue) = LowLevelCall.staticcallReturnScratchBytes32(address(asset_), params); uint256 length = LowLevelCall.returnDataSize(); uint256 value = uint256(rawValue); diff --git a/contracts/utils/LowLevelCall.sol b/contracts/utils/LowLevelCall.sol index 42c7f24ba9c..03bc6015bba 100644 --- a/contracts/utils/LowLevelCall.sol +++ b/contracts/utils/LowLevelCall.sol @@ -22,7 +22,7 @@ library LowLevelCall { /// /// WARNING: Do not assume that the result is zero if `success` is false. Memory can be already allocated /// and this function doesn't zero it out. - function callReturnBytes32( + function callReturnScratchBytes32( address target, uint256 value, bytes memory data @@ -38,7 +38,7 @@ library LowLevelCall { /// /// WARNING: Do not assume that the results are zero if `success` is false. Memory can be already allocated /// and this function doesn't zero it out. - function callReturnBytes32Tuple( + function callReturnScratchBytes32Pair( address target, uint256 value, bytes memory data @@ -50,27 +50,6 @@ library LowLevelCall { } } - /// @dev Performs a Solidity function call using a low level `call` and writes the result to the memory location - /// specified by `resultPtr`. - /// - /// IMPORTANT: This function assumes that the length of the memory array is stored in the first 32 bytes of the array and uses it for truncating - /// returndata if it's longer than the allocated memory to avoid corrupting to further places in memory. The `resultPtr` should be a - /// memory location that is already allocated with a predefined length. - /// - /// WARNING: Do not use if writing to `resultPtr` is not safe according to - /// the https://docs.soliditylang.org/en/latest/assembly.html#memory-safety[Solidity documentation]. - function callReturnOverride( - address target, - uint256 value, - bytes memory data, - bytes memory resultPtr - ) internal returns (bool success) { - assembly ("memory-safe") { - let maxSize := mload(resultPtr) - success := call(gas(), target, value, add(data, 0x20), mload(data), resultPtr, maxSize) - } - } - /// === STATICCALL === /// @dev Performs a Solidity function call using a low level `staticcall` and ignoring the return data. @@ -85,7 +64,7 @@ library LowLevelCall { /// /// WARNING: Do not assume that the result is zero if `success` is false. Memory can be already allocated /// and this function doesn't zero it out. - function staticCallReturnBytes32( + function staticcallReturnScratchBytes32( address target, bytes memory data ) internal view returns (bool success, bytes32 result) { @@ -100,7 +79,7 @@ library LowLevelCall { /// /// WARNING: Do not assume that the results are zero if `success` is false. Memory can be already allocated /// and this function doesn't zero it out. - function staticCallReturnBytes32Tuple( + function staticcallReturnScratchBytes32Pair( address target, bytes memory data ) internal view returns (bool success, bytes32 result1, bytes32 result2) { @@ -111,26 +90,6 @@ library LowLevelCall { } } - /// @dev Performs a Solidity function call using a low level `staticcall` and writes the result to the memory location - /// specified by `resultPtr`. - /// - /// IMPORTANT: This function assumes that the length of the memory array is stored in the first 32 bytes of the array and uses it for truncating - /// returndata if it's longer than the allocated memory to avoid corrupting to further places in memory. The `resultPtr` should be a - /// memory location that is already allocated with a predefined length. - /// - /// WARNING: Do not use if writing to `resultPtr` is not safe according to - /// the https://docs.soliditylang.org/en/latest/assembly.html#memory-safety[Solidity documentation]. - function staticCallReturnOverride( - address target, - bytes memory data, - bytes memory resultPtr - ) internal view returns (bool success) { - assembly ("memory-safe") { - let maxSize := mload(resultPtr) - success := staticcall(gas(), target, add(data, 0x20), mload(data), resultPtr, maxSize) - } - } - /// @dev Returns the size of the return data buffer. function returnDataSize() internal pure returns (uint256 size) { assembly ("memory-safe") { diff --git a/contracts/utils/cryptography/SignatureChecker.sol b/contracts/utils/cryptography/SignatureChecker.sol index b0d8c3c644e..9e1d0d3e29f 100644 --- a/contracts/utils/cryptography/SignatureChecker.sol +++ b/contracts/utils/cryptography/SignatureChecker.sol @@ -46,7 +46,7 @@ library SignatureChecker { Memory.Pointer ptr = Memory.saveFreePointer(); bytes memory params = abi.encodeCall(IERC1271.isValidSignature, (hash, signature)); - (bool success, bytes32 result) = LowLevelCall.staticCallReturnBytes32(signer, params); + (bool success, bytes32 result) = LowLevelCall.staticcallReturnScratchBytes32(signer, params); uint256 length = LowLevelCall.returnDataSize(); Memory.loadFreePointer(ptr); From 7c7be9ae95d0ec9e2f467e4bda1d2af5db57331d Mon Sep 17 00:00:00 2001 From: ernestognw Date: Sat, 22 Jun 2024 14:51:16 -0600 Subject: [PATCH 04/56] Update name --- contracts/utils/LowLevelCall.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/utils/LowLevelCall.sol b/contracts/utils/LowLevelCall.sol index 03bc6015bba..f2d76f1f18c 100644 --- a/contracts/utils/LowLevelCall.sol +++ b/contracts/utils/LowLevelCall.sol @@ -53,7 +53,7 @@ library LowLevelCall { /// === STATICCALL === /// @dev Performs a Solidity function call using a low level `staticcall` and ignoring the return data. - function staticCallRaw(address target, bytes memory data) internal view returns (bool success) { + function staticcallRaw(address target, bytes memory data) internal view returns (bool success) { assembly ("memory-safe") { success := staticcall(gas(), target, add(data, 0x20), mload(data), 0, 0) } From 4ad1b2358cdcde70dc87fea4b758d2c1137cd3e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernesto=20Garc=C3=ADa?= Date: Mon, 24 Jun 2024 18:38:28 -0600 Subject: [PATCH 05/56] Apply suggestions from code review --- contracts/utils/LowLevelCall.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/utils/LowLevelCall.sol b/contracts/utils/LowLevelCall.sol index f2d76f1f18c..086c9ff65f3 100644 --- a/contracts/utils/LowLevelCall.sol +++ b/contracts/utils/LowLevelCall.sol @@ -34,7 +34,7 @@ library LowLevelCall { } /// @dev Performs a Solidity function call using a low level `call` and returns the first 64 bytes of the result - /// in the scratch space of memory. Useful for functions that return a tuple of single-word values values. + /// in the scratch space of memory. Useful for functions that return a tuple of single-word values. /// /// WARNING: Do not assume that the results are zero if `success` is false. Memory can be already allocated /// and this function doesn't zero it out. @@ -75,7 +75,7 @@ library LowLevelCall { } /// @dev Performs a Solidity function call using a low level `staticcall` and returns the first 64 bytes of the result - /// in the scratch space of memory. Useful for functions that return a tuple of single-word values values. + /// in the scratch space of memory. Useful for functions that return a tuple of single-word values. /// /// WARNING: Do not assume that the results are zero if `success` is false. Memory can be already allocated /// and this function doesn't zero it out. From 5240323a19eaf01f5f9cfe811ee0c7fffe08ed94 Mon Sep 17 00:00:00 2001 From: ernestognw Date: Tue, 25 Jun 2024 10:44:47 -0600 Subject: [PATCH 06/56] Add LowLevelCall to SafeERC20 --- contracts/token/ERC20/utils/SafeERC20.sol | 30 +++++++------------ .../utils/cryptography/SignatureChecker.sol | 4 +-- 2 files changed, 12 insertions(+), 22 deletions(-) diff --git a/contracts/token/ERC20/utils/SafeERC20.sol b/contracts/token/ERC20/utils/SafeERC20.sol index a5a3b31d173..92109af755a 100644 --- a/contracts/token/ERC20/utils/SafeERC20.sol +++ b/contracts/token/ERC20/utils/SafeERC20.sol @@ -7,6 +7,7 @@ import {IERC20} from "../IERC20.sol"; import {IERC1363} from "../../../interfaces/IERC1363.sol"; import {Address} from "../../../utils/Address.sol"; import {Memory} from "../../../utils/Memory.sol"; +import {LowLevelCall} from "../../../utils/LowLevelCall.sol"; /** * @title SafeERC20 @@ -150,21 +151,18 @@ library SafeERC20 { * This is a variant of {_callOptionalReturnBool} that reverts if call fails to meet the requirements. */ function _callOptionalReturn(IERC20 token, bytes memory data) private { - uint256 returnSize; - uint256 returnValue; + (bool success, bytes32 returnValue) = LowLevelCall.callReturnScratchBytes32(address(token), 0, data); + uint256 returnSize = LowLevelCall.returnDataSize(); + assembly ("memory-safe") { - let success := call(gas(), token, 0, add(data, 0x20), mload(data), 0, 0x20) - // bubble errors if iszero(success) { - let ptr := mload(0x40) - returndatacopy(ptr, 0, returndatasize()) - revert(ptr, returndatasize()) + // Bubble up revert reason + returndatacopy(data, 0, returnSize) + revert(data, returnSize) } - returnSize := returndatasize() - returnValue := mload(0) } - if (returnSize == 0 ? address(token).code.length == 0 : returnValue != 1) { + if (returnSize == 0 ? address(token).code.length == 0 : uint256(returnValue) != 1) { revert SafeERC20FailedOperation(address(token)); } } @@ -178,14 +176,8 @@ library SafeERC20 { * This is a variant of {_callOptionalReturn} that silently catches all reverts and returns a bool instead. */ function _callOptionalReturnBool(IERC20 token, bytes memory data) private returns (bool) { - bool success; - uint256 returnSize; - uint256 returnValue; - assembly ("memory-safe") { - success := call(gas(), token, 0, add(data, 0x20), mload(data), 0, 0x20) - returnSize := returndatasize() - returnValue := mload(0) - } - return success && (returnSize == 0 ? address(token).code.length > 0 : returnValue == 1); + (bool success, bytes32 returnValue) = LowLevelCall.callReturnScratchBytes32(address(token), 0, data); + uint256 returnSize = LowLevelCall.returnDataSize(); + return success && (returnSize == 0 ? address(token).code.length > 0 : uint256(returnValue) == 1); } } diff --git a/contracts/utils/cryptography/SignatureChecker.sol b/contracts/utils/cryptography/SignatureChecker.sol index 9e1d0d3e29f..1e1982e8881 100644 --- a/contracts/utils/cryptography/SignatureChecker.sol +++ b/contracts/utils/cryptography/SignatureChecker.sol @@ -42,14 +42,12 @@ library SignatureChecker { bytes32 hash, bytes memory signature ) internal view returns (bool) { - bytes4 magic = IERC1271.isValidSignature.selector; - Memory.Pointer ptr = Memory.saveFreePointer(); bytes memory params = abi.encodeCall(IERC1271.isValidSignature, (hash, signature)); (bool success, bytes32 result) = LowLevelCall.staticcallReturnScratchBytes32(signer, params); uint256 length = LowLevelCall.returnDataSize(); Memory.loadFreePointer(ptr); - return success && length >= 32 && result == bytes32(magic); + return success && length >= 32 && result == bytes32(IERC1271.isValidSignature.selector); } } From 58b4a96c06fc678220c936d41dd46b4f9188124e Mon Sep 17 00:00:00 2001 From: ernestognw Date: Tue, 25 Jun 2024 10:53:07 -0600 Subject: [PATCH 07/56] Add value versions to call --- contracts/token/ERC20/utils/SafeERC20.sol | 4 ++-- contracts/utils/Address.sol | 2 +- contracts/utils/LowLevelCall.sol | 27 ++++++++++++++++++++--- 3 files changed, 27 insertions(+), 6 deletions(-) diff --git a/contracts/token/ERC20/utils/SafeERC20.sol b/contracts/token/ERC20/utils/SafeERC20.sol index 92109af755a..acacf6b29c1 100644 --- a/contracts/token/ERC20/utils/SafeERC20.sol +++ b/contracts/token/ERC20/utils/SafeERC20.sol @@ -151,7 +151,7 @@ library SafeERC20 { * This is a variant of {_callOptionalReturnBool} that reverts if call fails to meet the requirements. */ function _callOptionalReturn(IERC20 token, bytes memory data) private { - (bool success, bytes32 returnValue) = LowLevelCall.callReturnScratchBytes32(address(token), 0, data); + (bool success, bytes32 returnValue) = LowLevelCall.callReturnScratchBytes32(address(token), data); uint256 returnSize = LowLevelCall.returnDataSize(); assembly ("memory-safe") { @@ -176,7 +176,7 @@ library SafeERC20 { * This is a variant of {_callOptionalReturn} that silently catches all reverts and returns a bool instead. */ function _callOptionalReturnBool(IERC20 token, bytes memory data) private returns (bool) { - (bool success, bytes32 returnValue) = LowLevelCall.callReturnScratchBytes32(address(token), 0, data); + (bool success, bytes32 returnValue) = LowLevelCall.callReturnScratchBytes32(address(token), data); uint256 returnSize = LowLevelCall.returnDataSize(); return success && (returnSize == 0 ? address(token).code.length > 0 : uint256(returnValue) == 1); } diff --git a/contracts/utils/Address.sol b/contracts/utils/Address.sol index bb371e9fa28..099a3d5405e 100644 --- a/contracts/utils/Address.sol +++ b/contracts/utils/Address.sol @@ -36,7 +36,7 @@ library Address { revert Errors.InsufficientBalance(address(this).balance, amount); } - bool success = LowLevelCall.callRaw(recipient, amount, ""); + bool success = LowLevelCall.callRaw(recipient, "", amount); if (!success) { revert Errors.FailedCall(); } diff --git a/contracts/utils/LowLevelCall.sol b/contracts/utils/LowLevelCall.sol index 086c9ff65f3..2659c5bedd7 100644 --- a/contracts/utils/LowLevelCall.sol +++ b/contracts/utils/LowLevelCall.sol @@ -11,7 +11,12 @@ library LowLevelCall { /// === CALL === /// @dev Performs a Solidity function call using a low level `call` and ignoring the return data. - function callRaw(address target, uint256 value, bytes memory data) internal returns (bool success) { + function callRaw(address target, bytes memory data) internal returns (bool success) { + return callRaw(target, data, 0); + } + + /// @dev Same as {callRaw}, but allows to specify the value to be sent in the call. + function callRaw(address target, bytes memory data, uint256 value) internal returns (bool success) { assembly ("memory-safe") { success := call(gas(), target, value, add(data, 0x20), mload(data), 0, 0) } @@ -24,8 +29,16 @@ library LowLevelCall { /// and this function doesn't zero it out. function callReturnScratchBytes32( address target, - uint256 value, bytes memory data + ) internal returns (bool success, bytes32 result) { + return callReturnScratchBytes32(target, data, 0); + } + + /// @dev Same as {callReturnScratchBytes32}, but allows to specify the value to be sent in the call. + function callReturnScratchBytes32( + address target, + bytes memory data, + uint256 value ) internal returns (bool success, bytes32 result) { assembly ("memory-safe") { success := call(gas(), target, value, add(data, 0x20), mload(data), 0, 0x20) @@ -40,8 +53,16 @@ library LowLevelCall { /// and this function doesn't zero it out. function callReturnScratchBytes32Pair( address target, - uint256 value, bytes memory data + ) internal returns (bool success, bytes32 result1, bytes32 result2) { + return callReturnScratchBytes32Pair(target, data, 0); + } + + /// @dev Same as {callReturnScratchBytes32Pair}, but allows to specify the value to be sent in the call. + function callReturnScratchBytes32Pair( + address target, + bytes memory data, + uint256 value ) internal returns (bool success, bytes32 result1, bytes32 result2) { assembly ("memory-safe") { success := call(gas(), target, value, add(data, 0x20), mload(data), 0, 0x40) From bbb6aa16dc1e9e0ca97c2ec3eb49ec614c1aeeae Mon Sep 17 00:00:00 2001 From: ernestognw Date: Wed, 26 Jun 2024 14:37:52 -0600 Subject: [PATCH 08/56] Add documentation --- contracts/access/manager/AuthorityUtils.sol | 2 +- contracts/token/ERC20/extensions/ERC4626.sol | 2 +- contracts/token/ERC20/utils/SafeERC20.sol | 4 +- contracts/utils/LowLevelCall.sol | 26 +++++------ contracts/utils/README.adoc | 3 ++ .../utils/cryptography/SignatureChecker.sol | 2 +- docs/modules/ROOT/pages/utilities.adoc | 44 +++++++++++++++++++ 7 files changed, 65 insertions(+), 18 deletions(-) diff --git a/contracts/access/manager/AuthorityUtils.sol b/contracts/access/manager/AuthorityUtils.sol index 4f9b6c90c94..d476f053d25 100644 --- a/contracts/access/manager/AuthorityUtils.sol +++ b/contracts/access/manager/AuthorityUtils.sol @@ -21,7 +21,7 @@ library AuthorityUtils { ) internal view returns (bool immediate, uint32 delay) { Memory.Pointer ptr = Memory.saveFreePointer(); bytes memory params = abi.encodeCall(IAuthority.canCall, (caller, target, selector)); - (bool success, bytes32 immediateWord, bytes32 delayWord) = LowLevelCall.staticcallReturnScratchBytes32Pair( + (bool success, bytes32 immediateWord, bytes32 delayWord) = LowLevelCall.staticcallReturnBytes32Pair( authority, params ); diff --git a/contracts/token/ERC20/extensions/ERC4626.sol b/contracts/token/ERC20/extensions/ERC4626.sol index c5f70604bcf..4d8c1064383 100644 --- a/contracts/token/ERC20/extensions/ERC4626.sol +++ b/contracts/token/ERC20/extensions/ERC4626.sol @@ -85,7 +85,7 @@ abstract contract ERC4626 is ERC20, IERC4626 { Memory.Pointer ptr = Memory.saveFreePointer(); bytes memory params = abi.encodeCall(IERC20Metadata.decimals, ()); - (bool success, bytes32 rawValue) = LowLevelCall.staticcallReturnScratchBytes32(address(asset_), params); + (bool success, bytes32 rawValue) = LowLevelCall.staticcallReturnBytes32(address(asset_), params); uint256 length = LowLevelCall.returnDataSize(); uint256 value = uint256(rawValue); diff --git a/contracts/token/ERC20/utils/SafeERC20.sol b/contracts/token/ERC20/utils/SafeERC20.sol index acacf6b29c1..41a21d32a7d 100644 --- a/contracts/token/ERC20/utils/SafeERC20.sol +++ b/contracts/token/ERC20/utils/SafeERC20.sol @@ -151,7 +151,7 @@ library SafeERC20 { * This is a variant of {_callOptionalReturnBool} that reverts if call fails to meet the requirements. */ function _callOptionalReturn(IERC20 token, bytes memory data) private { - (bool success, bytes32 returnValue) = LowLevelCall.callReturnScratchBytes32(address(token), data); + (bool success, bytes32 returnValue) = LowLevelCall.callReturnBytes32(address(token), data); uint256 returnSize = LowLevelCall.returnDataSize(); assembly ("memory-safe") { @@ -176,7 +176,7 @@ library SafeERC20 { * This is a variant of {_callOptionalReturn} that silently catches all reverts and returns a bool instead. */ function _callOptionalReturnBool(IERC20 token, bytes memory data) private returns (bool) { - (bool success, bytes32 returnValue) = LowLevelCall.callReturnScratchBytes32(address(token), data); + (bool success, bytes32 returnValue) = LowLevelCall.callReturnBytes32(address(token), data); uint256 returnSize = LowLevelCall.returnDataSize(); return success && (returnSize == 0 ? address(token).code.length > 0 : uint256(returnValue) == 1); } diff --git a/contracts/utils/LowLevelCall.sol b/contracts/utils/LowLevelCall.sol index 2659c5bedd7..bf552a3e78a 100644 --- a/contracts/utils/LowLevelCall.sol +++ b/contracts/utils/LowLevelCall.sol @@ -6,6 +6,9 @@ import {Errors} from "./Errors.sol"; /** * @dev Library of low level call functions that implement different calling strategies to deal with the return data. + * + * WARNING: Using this library requires an advanced understanding of Solidity and how the EVM works. It is recommended + * to use the {Address} library instead. */ library LowLevelCall { /// === CALL === @@ -27,15 +30,12 @@ library LowLevelCall { /// /// WARNING: Do not assume that the result is zero if `success` is false. Memory can be already allocated /// and this function doesn't zero it out. - function callReturnScratchBytes32( - address target, - bytes memory data - ) internal returns (bool success, bytes32 result) { - return callReturnScratchBytes32(target, data, 0); + function callReturnBytes32(address target, bytes memory data) internal returns (bool success, bytes32 result) { + return callReturnBytes32(target, data, 0); } - /// @dev Same as {callReturnScratchBytes32}, but allows to specify the value to be sent in the call. - function callReturnScratchBytes32( + /// @dev Same as {callReturnBytes32}, but allows to specify the value to be sent in the call. + function callReturnBytes32( address target, bytes memory data, uint256 value @@ -51,15 +51,15 @@ library LowLevelCall { /// /// WARNING: Do not assume that the results are zero if `success` is false. Memory can be already allocated /// and this function doesn't zero it out. - function callReturnScratchBytes32Pair( + function callReturnBytes32Pair( address target, bytes memory data ) internal returns (bool success, bytes32 result1, bytes32 result2) { - return callReturnScratchBytes32Pair(target, data, 0); + return callReturnBytes32Pair(target, data, 0); } - /// @dev Same as {callReturnScratchBytes32Pair}, but allows to specify the value to be sent in the call. - function callReturnScratchBytes32Pair( + /// @dev Same as {callReturnBytes32Pair}, but allows to specify the value to be sent in the call. + function callReturnBytes32Pair( address target, bytes memory data, uint256 value @@ -85,7 +85,7 @@ library LowLevelCall { /// /// WARNING: Do not assume that the result is zero if `success` is false. Memory can be already allocated /// and this function doesn't zero it out. - function staticcallReturnScratchBytes32( + function staticcallReturnBytes32( address target, bytes memory data ) internal view returns (bool success, bytes32 result) { @@ -100,7 +100,7 @@ library LowLevelCall { /// /// WARNING: Do not assume that the results are zero if `success` is false. Memory can be already allocated /// and this function doesn't zero it out. - function staticcallReturnScratchBytes32Pair( + function staticcallReturnBytes32Pair( address target, bytes memory data ) internal view returns (bool success, bytes32 result1, bytes32 result2) { diff --git a/contracts/utils/README.adoc b/contracts/utils/README.adoc index 056ad3331ac..f1bec98fd6d 100644 --- a/contracts/utils/README.adoc +++ b/contracts/utils/README.adoc @@ -36,6 +36,7 @@ Miscellaneous contracts and libraries containing utility functions you can use t * {Context}: An utility for abstracting the sender and calldata in the current execution context. * {Packing}: A library for packing and unpacking multiple values into bytes32 * {Panic}: A library to revert with https://docs.soliditylang.org/en/v0.8.20/control-structures.html#panic-via-assert-and-error-via-require[Solidity panic codes]. + * {LowLevelCall}: Collection of functions to perform calls with low-level assembly. [NOTE] ==== @@ -127,3 +128,5 @@ Ethereum contracts have no native concept of an interface, so applications must {{Packing}} {{Panic}} + +{{LowLevelCall}} diff --git a/contracts/utils/cryptography/SignatureChecker.sol b/contracts/utils/cryptography/SignatureChecker.sol index 1e1982e8881..10bcc0fff76 100644 --- a/contracts/utils/cryptography/SignatureChecker.sol +++ b/contracts/utils/cryptography/SignatureChecker.sol @@ -44,7 +44,7 @@ library SignatureChecker { ) internal view returns (bool) { Memory.Pointer ptr = Memory.saveFreePointer(); bytes memory params = abi.encodeCall(IERC1271.isValidSignature, (hash, signature)); - (bool success, bytes32 result) = LowLevelCall.staticcallReturnScratchBytes32(signer, params); + (bool success, bytes32 result) = LowLevelCall.staticcallReturnBytes32(signer, params); uint256 length = LowLevelCall.returnDataSize(); Memory.loadFreePointer(ptr); diff --git a/docs/modules/ROOT/pages/utilities.adoc b/docs/modules/ROOT/pages/utilities.adoc index 506a581dfad..2f39c610d07 100644 --- a/docs/modules/ROOT/pages/utilities.adoc +++ b/docs/modules/ROOT/pages/utilities.adoc @@ -318,3 +318,47 @@ await instance.multicall([ instance.interface.encodeFunctionData("bar") ]); ---- + +=== LowLevelCall + +The `LowLevelCall` library contains a set of functions to perform external calls with low-level assembly, allowing them to deal with the callee's `returndata` in different ways. This is especially useful to make a call in a way that is safe against return bombing (i.e. the callee allocates too much memory using a long returndata). + +The functions in the library efficiently allocates a fixed sized of the `returndata` up to 64 bytes. You can either ignore the returned data, or get 1 or 2 `bytes32` values. + +[source,solidity] +---- +using LowLevelCall for address; + +function _foo(address target, bytes memory data) internal { + bool success; + bytes32 returnValue1; + bytes32 returnValue2; + + // Ignore return data + success = target.callRaw(data); + + // Copy only 32 bytes from return data + (success, returnValue1) = target.callReturnBytes32(data); + + // Copy two (32 bytes) EVM words from returndata + (success, returnValue1, returnValue2) = target.callReturnBytes32Pair(data); +} +---- + +There are cases where you would like to check the size of the returned data, either to make sure it fits the expected size, or to check it before loading it to memory. In those case, the library also includes a function to separately check the return data size: + + +[source,solidity] +---- +using LowLevelCall for address; + +function _foo(address target, bytes memory data) internal returns (bool returnBool) { + if (!target.callRaw(data)) { + // Unsuccessful call + return false; + } + + // As long as the contract returned data, its content doesn't matter + return LowLevelCall.returnDataSize() >= 32; +} +---- From 044fbbef08a6e1ccaca7916c0f49094ac0756823 Mon Sep 17 00:00:00 2001 From: ernestognw Date: Wed, 26 Jun 2024 14:40:52 -0600 Subject: [PATCH 09/56] Add changeset --- .changeset/sharp-scissors-drum.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/sharp-scissors-drum.md diff --git a/.changeset/sharp-scissors-drum.md b/.changeset/sharp-scissors-drum.md new file mode 100644 index 00000000000..b701eccf3fa --- /dev/null +++ b/.changeset/sharp-scissors-drum.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': minor +--- + +`LowLevelCall`: Add a library to perform low-level calls and deal with the `returndata` more granularly. From 85ce0785941c4ca89766bdfb2caf9897f271f216 Mon Sep 17 00:00:00 2001 From: ernestognw Date: Wed, 26 Jun 2024 16:28:32 -0600 Subject: [PATCH 10/56] Add LowLevelCall tests --- contracts/mocks/CallReceiverMock.sol | 10 ++ test/utils/LowLevelCall.test.js | 204 +++++++++++++++++++++++++++ 2 files changed, 214 insertions(+) create mode 100644 test/utils/LowLevelCall.test.js diff --git a/contracts/mocks/CallReceiverMock.sol b/contracts/mocks/CallReceiverMock.sol index e371c7db800..496386c4a7c 100644 --- a/contracts/mocks/CallReceiverMock.sol +++ b/contracts/mocks/CallReceiverMock.sol @@ -24,6 +24,12 @@ contract CallReceiverMock { return "0x1234"; } + function mockFunctionWithArgsReturn(uint256 a, uint256 b) public payable returns (uint256, uint256) { + emit MockFunctionCalledWithArgs(a, b); + + return (a, b); + } + function mockFunctionNonPayable() public returns (string memory) { emit MockFunctionCalled(); @@ -34,6 +40,10 @@ contract CallReceiverMock { return "0x1234"; } + function mockStaticFunctionWithArgsReturn(uint256 a, uint256 b) public pure returns (uint256, uint256) { + return (a, b); + } + function mockFunctionRevertsNoReason() public payable { revert(); } diff --git a/test/utils/LowLevelCall.test.js b/test/utils/LowLevelCall.test.js new file mode 100644 index 00000000000..d83b485cba0 --- /dev/null +++ b/test/utils/LowLevelCall.test.js @@ -0,0 +1,204 @@ +const { ethers } = require('hardhat'); +const { expect } = require('chai'); +const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers'); + +async function fixture() { + const [recipient, other] = await ethers.getSigners(); + + const mock = await ethers.deployContract('$LowLevelCall'); + const target = await ethers.deployContract('CallReceiverMock'); + const targetEther = await ethers.deployContract('EtherReceiverMock'); + + return { recipient, other, mock, target, targetEther, value: BigInt(1e18) }; +} + +describe('LowLevelCall', function () { + beforeEach(async function () { + Object.assign(this, await loadFixture(fixture)); + }); + + describe('callRaw', function () { + beforeEach(async function () { + this.call = this.target.interface.encodeFunctionData('mockFunction'); + }); + + it('calls the requested function and returns true', async function () { + await expect(this.mock.$callRaw(this.target, this.call)) + .to.emit(this.target, 'MockFunctionCalled') + .to.emit(this.mock, 'return$callRaw_address_bytes') + .withArgs(true); + }); + + it('calls the requested function with value and returns true', async function () { + await this.other.sendTransaction({ to: this.mock, value: this.value }); + + const tx = this.mock['$callRaw(address,bytes,uint256)'](this.target, this.call, this.value); + await expect(tx).to.changeEtherBalance(this.target, this.value); + await expect(tx).to.emit(this.mock, 'return$callRaw_address_bytes_uint256').withArgs(true); + }); + + it("calls the requested function and returns false if the caller doesn't have enough balance", async function () { + const tx = this.mock['$callRaw(address,bytes,uint256)'](this.target, this.call, this.value); + await expect(tx).to.not.changeEtherBalance(this.target, this.value); + await expect(tx).to.emit(this.mock, 'return$callRaw_address_bytes_uint256').withArgs(false); + }); + + it('calls the requested function and returns false if the subcall reverts', async function () { + const call = this.target.interface.encodeFunctionData('mockFunctionRevertsNoReason'); + const tx = await this.mock.$callRaw(this.target, call); + expect(tx).to.emit(this.mock, 'return$callRaw_address_bytes').withArgs(false); + }); + }); + + describe('callReturnBytes32', function () { + beforeEach(async function () { + this.returnValue = ethers.id('returnDataBytes32'); + this.call = this.target.interface.encodeFunctionData('mockFunctionWithArgsReturn', [ + this.returnValue, + ethers.ZeroHash, + ]); + }); + + it('calls the requested function and returns true', async function () { + await expect(this.mock.$callReturnBytes32(this.target, this.call)) + .to.emit(this.target, 'MockFunctionCalledWithArgs') + .withArgs(this.returnValue, ethers.ZeroHash) + .to.emit(this.mock, 'return$callReturnBytes32_address_bytes') + .withArgs(true, this.returnValue); + }); + + it('calls the requested function with value and returns true', async function () { + await this.other.sendTransaction({ to: this.mock, value: this.value }); + + const tx = this.mock['$callReturnBytes32(address,bytes,uint256)'](this.target, this.call, this.value); + await expect(tx).to.changeEtherBalance(this.target, this.value); + await expect(tx) + .to.emit(this.mock, 'return$callReturnBytes32_address_bytes_uint256') + .withArgs(true, this.returnValue); + }); + + it("calls the requested function and returns false if the caller doesn't have enough balance", async function () { + const tx = this.mock['$callReturnBytes32(address,bytes,uint256)'](this.target, this.call, this.value); + await expect(tx).to.not.changeEtherBalance(this.target, this.value); + await expect(tx) + .to.emit(this.mock, 'return$callReturnBytes32_address_bytes_uint256') + .withArgs(false, ethers.ZeroHash); + }); + + it('calls the requested function and returns false if the subcall reverts', async function () { + const call = this.target.interface.encodeFunctionData('mockFunctionRevertsNoReason'); + const tx = await this.mock.$callReturnBytes32(this.target, call); + expect(tx).to.emit(this.mock, 'return$callReturnBytes32_address_bytes').withArgs(false, ethers.ZeroHash); + }); + }); + + describe('callReturnBytes32Pair', function () { + beforeEach(async function () { + this.returnValue1 = ethers.id('returnDataBytes32Pair1'); + this.returnValue2 = ethers.id('returnDataBytes32Pair2'); + this.call = this.target.interface.encodeFunctionData('mockFunctionWithArgsReturn', [ + this.returnValue1, + this.returnValue2, + ]); + }); + + it('calls the requested function and returns true', async function () { + await expect(this.mock.$callReturnBytes32Pair(this.target, this.call)) + .to.emit(this.target, 'MockFunctionCalledWithArgs') + .withArgs(this.returnValue1, this.returnValue2) + .to.emit(this.mock, 'return$callReturnBytes32Pair_address_bytes') + .withArgs(true, this.returnValue1, this.returnValue2); + }); + + it('calls the requested function with value and returns true', async function () { + await this.other.sendTransaction({ to: this.mock, value: this.value }); + + const tx = this.mock['$callReturnBytes32Pair(address,bytes,uint256)'](this.target, this.call, this.value); + await expect(tx).to.changeEtherBalance(this.target, this.value); + await expect(tx) + .to.emit(this.mock, 'return$callReturnBytes32Pair_address_bytes_uint256') + .withArgs(true, this.returnValue1, this.returnValue2); + }); + + it("calls the requested function and returns false if the caller doesn't have enough balance", async function () { + const tx = this.mock['$callReturnBytes32Pair(address,bytes,uint256)'](this.target, this.call, this.value); + await expect(tx).to.not.changeEtherBalance(this.target, this.value); + await expect(tx) + .to.emit(this.mock, 'return$callReturnBytes32Pair_address_bytes_uint256') + .withArgs(false, ethers.ZeroHash, ethers.ZeroHash); + }); + + it('calls the requested function and returns false if the subcall reverts', async function () { + const call = this.target.interface.encodeFunctionData('mockFunctionRevertsNoReason'); + const tx = await this.mock.$callReturnBytes32Pair(this.target, call); + expect(tx) + .to.emit(this.mock, 'return$callReturnBytes32Pair_address_bytes') + .withArgs(false, ethers.ZeroHash, ethers.ZeroHash); + }); + }); + + describe('staticcallRaw', function () { + it('calls the requested function and returns true', async function () { + const call = this.target.interface.encodeFunctionData('mockStaticFunction'); + expect(await this.mock.$staticcallRaw(this.target, call)).to.equal(true); + }); + + it('calls the requested function and returns false if the subcall reverts', async function () { + const interface = new ethers.Interface(['function mockFunctionDoesNotExist()']); + + const call = interface.encodeFunctionData('mockFunctionDoesNotExist'); + expect(await this.mock.$staticcallRaw(this.target, call)).to.equal(false); + }); + }); + + describe('staticcallReturnBytes32', function () { + beforeEach(async function () { + this.returnValue = ethers.id('returnDataBytes32'); + }); + + it('calls the requested function and returns true', async function () { + const call = this.target.interface.encodeFunctionData('mockStaticFunctionWithArgsReturn', [ + this.returnValue, + ethers.ZeroHash, + ]); + expect(await this.mock.$staticcallReturnBytes32(this.target, call)).to.deep.equal([true, this.returnValue]); + }); + + it('calls the requested function and returns false if the subcall reverts', async function () { + const interface = new ethers.Interface(['function mockFunctionDoesNotExist()']); + + const call = interface.encodeFunctionData('mockFunctionDoesNotExist'); + expect(await this.mock.$staticcallReturnBytes32(this.target, call)).to.deep.equal([false, ethers.ZeroHash]); + }); + }); + + describe('staticcallReturnBytes32Pair', function () { + beforeEach(async function () { + this.returnValue1 = ethers.id('returnDataBytes32Pair1'); + this.returnValue2 = ethers.id('returnDataBytes32Pair2'); + }); + + it('calls the requested function and returns true', async function () { + const call = this.target.interface.encodeFunctionData('mockStaticFunctionWithArgsReturn', [ + this.returnValue1, + this.returnValue2, + ]); + expect(await this.mock.$staticcallReturnBytes32Pair(this.target, call)).to.deep.equal([ + true, + this.returnValue1, + this.returnValue2, + ]); + }); + + it('calls the requested function and returns false if the subcall reverts', async function () { + const interface = new ethers.Interface(['function mockFunctionDoesNotExist()']); + + const call = interface.encodeFunctionData('mockFunctionDoesNotExist'); + expect(await this.mock.$staticcallReturnBytes32Pair(this.target, call)).to.deep.equal([ + false, + ethers.ZeroHash, + ethers.ZeroHash, + ]); + }); + }); +}); From bb1a5554f1ca4bb3a815ba871879744c5edfce47 Mon Sep 17 00:00:00 2001 From: ernestognw Date: Wed, 26 Jun 2024 16:53:57 -0600 Subject: [PATCH 11/56] Add tests to memory --- contracts/access/manager/AuthorityUtils.sol | 4 +-- contracts/mocks/MemoryMock.sol | 29 +++++++++++++++++++ contracts/token/ERC20/extensions/ERC4626.sol | 4 +-- contracts/token/ERC20/utils/SafeERC20.sol | 12 ++++---- contracts/utils/Memory.sol | 4 +-- .../utils/cryptography/SignatureChecker.sol | 4 +-- test/utils/Memory.test.js | 23 +++++++++++++++ 7 files changed, 66 insertions(+), 14 deletions(-) create mode 100644 contracts/mocks/MemoryMock.sol create mode 100644 test/utils/Memory.test.js diff --git a/contracts/access/manager/AuthorityUtils.sol b/contracts/access/manager/AuthorityUtils.sol index d476f053d25..d6caeff3c61 100644 --- a/contracts/access/manager/AuthorityUtils.sol +++ b/contracts/access/manager/AuthorityUtils.sol @@ -19,13 +19,13 @@ library AuthorityUtils { address target, bytes4 selector ) internal view returns (bool immediate, uint32 delay) { - Memory.Pointer ptr = Memory.saveFreePointer(); + Memory.Pointer ptr = Memory.getFreePointer(); bytes memory params = abi.encodeCall(IAuthority.canCall, (caller, target, selector)); (bool success, bytes32 immediateWord, bytes32 delayWord) = LowLevelCall.staticcallReturnBytes32Pair( authority, params ); - Memory.loadFreePointer(ptr); + Memory.setFreePointer(ptr); if (!success) { return (false, 0); diff --git a/contracts/mocks/MemoryMock.sol b/contracts/mocks/MemoryMock.sol new file mode 100644 index 00000000000..34b7bd628ca --- /dev/null +++ b/contracts/mocks/MemoryMock.sol @@ -0,0 +1,29 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +import {Memory} from "../utils/Memory.sol"; + +contract MemoryMock { + bytes32 private _ptr; + + modifier _rememberPtr() { + assembly { + mstore(0x40, sload(_ptr.slot)) + } + _; + } + + function _setPointer(bytes32 ptr) public { + _ptr = ptr; + } + + function $setFreePointer(bytes32 ptr) public { + _setPointer(ptr); + return Memory.setFreePointer(Memory.Pointer.wrap(ptr)); + } + + function $getFreePointer() public view _rememberPtr returns (bytes32) { + return Memory.Pointer.unwrap(Memory.getFreePointer()); + } +} diff --git a/contracts/token/ERC20/extensions/ERC4626.sol b/contracts/token/ERC20/extensions/ERC4626.sol index 4d8c1064383..e1604792fc6 100644 --- a/contracts/token/ERC20/extensions/ERC4626.sol +++ b/contracts/token/ERC20/extensions/ERC4626.sol @@ -82,14 +82,14 @@ abstract contract ERC4626 is ERC20, IERC4626 { } function _tryGetAssetDecimalsWithFallback(IERC20 asset_, uint8 defaultValue) private view returns (uint8) { - Memory.Pointer ptr = Memory.saveFreePointer(); + Memory.Pointer ptr = Memory.getFreePointer(); bytes memory params = abi.encodeCall(IERC20Metadata.decimals, ()); (bool success, bytes32 rawValue) = LowLevelCall.staticcallReturnBytes32(address(asset_), params); uint256 length = LowLevelCall.returnDataSize(); uint256 value = uint256(rawValue); - Memory.loadFreePointer(ptr); + Memory.setFreePointer(ptr); return uint8(Math.ternary(success && length >= 0x20 && value <= type(uint8).max, value, defaultValue)); } diff --git a/contracts/token/ERC20/utils/SafeERC20.sol b/contracts/token/ERC20/utils/SafeERC20.sol index 41a21d32a7d..16e40305ee1 100644 --- a/contracts/token/ERC20/utils/SafeERC20.sol +++ b/contracts/token/ERC20/utils/SafeERC20.sol @@ -34,9 +34,9 @@ library SafeERC20 { * non-reverting calls are assumed to be successful. */ function safeTransfer(IERC20 token, address to, uint256 value) internal { - Memory.Pointer ptr = Memory.saveFreePointer(); + Memory.Pointer ptr = Memory.getFreePointer(); _callOptionalReturn(token, abi.encodeCall(token.transfer, (to, value))); - Memory.loadFreePointer(ptr); + Memory.setFreePointer(ptr); } /** @@ -44,9 +44,9 @@ library SafeERC20 { * calling contract. If `token` returns no value, non-reverting calls are assumed to be successful. */ function safeTransferFrom(IERC20 token, address from, address to, uint256 value) internal { - Memory.Pointer ptr = Memory.saveFreePointer(); + Memory.Pointer ptr = Memory.getFreePointer(); _callOptionalReturn(token, abi.encodeCall(token.transferFrom, (from, to, value))); - Memory.loadFreePointer(ptr); + Memory.setFreePointer(ptr); } /** @@ -78,13 +78,13 @@ library SafeERC20 { * to be set to zero before setting it to a non-zero value, such as USDT. */ function forceApprove(IERC20 token, address spender, uint256 value) internal { - Memory.Pointer ptr = Memory.saveFreePointer(); + Memory.Pointer ptr = Memory.getFreePointer(); bytes memory approvalCall = abi.encodeCall(token.approve, (spender, value)); if (!_callOptionalReturnBool(token, approvalCall)) { _callOptionalReturn(token, abi.encodeCall(token.approve, (spender, 0))); _callOptionalReturn(token, approvalCall); } - Memory.loadFreePointer(ptr); + Memory.setFreePointer(ptr); } /** diff --git a/contracts/utils/Memory.sol b/contracts/utils/Memory.sol index f959a88bf7c..a26e6516e7b 100644 --- a/contracts/utils/Memory.sol +++ b/contracts/utils/Memory.sol @@ -7,7 +7,7 @@ library Memory { type Pointer is bytes32; /// @dev Returns a memory pointer to the current free memory pointer. - function saveFreePointer() internal pure returns (Pointer ptr) { + function getFreePointer() internal pure returns (Pointer ptr) { assembly ("memory-safe") { ptr := mload(0x40) } @@ -16,7 +16,7 @@ library Memory { /// @dev Sets the free memory pointer to a specific value. /// /// WARNING: Everything after the pointer may be overwritten. - function loadFreePointer(Pointer ptr) internal pure { + function setFreePointer(Pointer ptr) internal pure { assembly ("memory-safe") { mstore(0x40, ptr) } diff --git a/contracts/utils/cryptography/SignatureChecker.sol b/contracts/utils/cryptography/SignatureChecker.sol index 10bcc0fff76..2a80d1017d4 100644 --- a/contracts/utils/cryptography/SignatureChecker.sol +++ b/contracts/utils/cryptography/SignatureChecker.sol @@ -42,11 +42,11 @@ library SignatureChecker { bytes32 hash, bytes memory signature ) internal view returns (bool) { - Memory.Pointer ptr = Memory.saveFreePointer(); + Memory.Pointer ptr = Memory.getFreePointer(); bytes memory params = abi.encodeCall(IERC1271.isValidSignature, (hash, signature)); (bool success, bytes32 result) = LowLevelCall.staticcallReturnBytes32(signer, params); uint256 length = LowLevelCall.returnDataSize(); - Memory.loadFreePointer(ptr); + Memory.setFreePointer(ptr); return success && length >= 32 && result == bytes32(IERC1271.isValidSignature.selector); } diff --git a/test/utils/Memory.test.js b/test/utils/Memory.test.js new file mode 100644 index 00000000000..523bf886d26 --- /dev/null +++ b/test/utils/Memory.test.js @@ -0,0 +1,23 @@ +const { ethers } = require('hardhat'); +const { expect } = require('chai'); +const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers'); + +async function fixture() { + const mock = await ethers.deployContract('MemoryMock'); + + return { mock }; +} + +describe('Memory', function () { + beforeEach(async function () { + Object.assign(this, await loadFixture(fixture)); + }); + + describe('free pointer', function () { + it('returns the new memory pointer after it has been altered', async function () { + const ptr = '0x00000000000000000000000000000000000000000000000000000000000000a0'; + await this.mock.$setFreePointer(ptr); + expect(await this.mock.$getFreePointer()).to.eq(ptr); + }); + }); +}); From cf31c38ae2ec5c3b8009ee6b608fe86f5327ef14 Mon Sep 17 00:00:00 2001 From: ernestognw Date: Wed, 26 Jun 2024 16:55:09 -0600 Subject: [PATCH 12/56] Add missing check --- .changeset/dull-students-eat.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/dull-students-eat.md diff --git a/.changeset/dull-students-eat.md b/.changeset/dull-students-eat.md new file mode 100644 index 00000000000..94c4fc21ef2 --- /dev/null +++ b/.changeset/dull-students-eat.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': minor +--- + +`Memory`: Add library with utilities to manipulate memory From 7d4196bc0205470a9f9f8fd13b69d121efbcec63 Mon Sep 17 00:00:00 2001 From: ernestognw Date: Wed, 26 Jun 2024 18:06:53 -0600 Subject: [PATCH 13/56] Add to stateless --- contracts/mocks/Stateless.sol | 2 ++ 1 file changed, 2 insertions(+) diff --git a/contracts/mocks/Stateless.sol b/contracts/mocks/Stateless.sol index 6bf78babc7d..ba9984464e8 100644 --- a/contracts/mocks/Stateless.sol +++ b/contracts/mocks/Stateless.sol @@ -22,7 +22,9 @@ import {ERC165} from "../utils/introspection/ERC165.sol"; import {ERC165Checker} from "../utils/introspection/ERC165Checker.sol"; import {ERC1967Utils} from "../proxy/ERC1967/ERC1967Utils.sol"; import {ERC721Holder} from "../token/ERC721/utils/ERC721Holder.sol"; +import {LowLevelCall} from "../utils/LowLevelCall.sol"; import {Math} from "../utils/math/Math.sol"; +import {Memory} from "../utils/Memory.sol"; import {MerkleProof} from "../utils/cryptography/MerkleProof.sol"; import {MessageHashUtils} from "../utils/cryptography/MessageHashUtils.sol"; import {Panic} from "../utils/Panic.sol"; From 0323b38cc0aa8b6b92d2be20a8328e79ed4c0590 Mon Sep 17 00:00:00 2001 From: ernestognw Date: Thu, 27 Jun 2024 16:35:43 -0600 Subject: [PATCH 14/56] Try to fix tests --- contracts/mocks/MemoryMock.sol | 10 +++++----- test/utils/LowLevelCall.test.js | 10 +++++----- test/utils/Memory.test.js | 4 ++-- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/contracts/mocks/MemoryMock.sol b/contracts/mocks/MemoryMock.sol index 34b7bd628ca..128f66c0c11 100644 --- a/contracts/mocks/MemoryMock.sol +++ b/contracts/mocks/MemoryMock.sol @@ -8,8 +8,8 @@ contract MemoryMock { bytes32 private _ptr; modifier _rememberPtr() { - assembly { - mstore(0x40, sload(_ptr.slot)) + assembly ("memory-safe") { + mstore(0x00, sload(_ptr.slot)) } _; } @@ -18,12 +18,12 @@ contract MemoryMock { _ptr = ptr; } - function $setFreePointer(bytes32 ptr) public { + function _setFreePointer(bytes32 ptr) public { _setPointer(ptr); - return Memory.setFreePointer(Memory.Pointer.wrap(ptr)); + return Memory.setFreePointer(ptr); } - function $getFreePointer() public view _rememberPtr returns (bytes32) { + function _getFreePointer() public view _rememberPtr returns (bytes32) { return Memory.Pointer.unwrap(Memory.getFreePointer()); } } diff --git a/test/utils/LowLevelCall.test.js b/test/utils/LowLevelCall.test.js index d83b485cba0..717ace7d607 100644 --- a/test/utils/LowLevelCall.test.js +++ b/test/utils/LowLevelCall.test.js @@ -18,7 +18,7 @@ describe('LowLevelCall', function () { }); describe('callRaw', function () { - beforeEach(async function () { + beforeEach(function () { this.call = this.target.interface.encodeFunctionData('mockFunction'); }); @@ -51,7 +51,7 @@ describe('LowLevelCall', function () { }); describe('callReturnBytes32', function () { - beforeEach(async function () { + beforeEach(function () { this.returnValue = ethers.id('returnDataBytes32'); this.call = this.target.interface.encodeFunctionData('mockFunctionWithArgsReturn', [ this.returnValue, @@ -93,7 +93,7 @@ describe('LowLevelCall', function () { }); describe('callReturnBytes32Pair', function () { - beforeEach(async function () { + beforeEach(function () { this.returnValue1 = ethers.id('returnDataBytes32Pair1'); this.returnValue2 = ethers.id('returnDataBytes32Pair2'); this.call = this.target.interface.encodeFunctionData('mockFunctionWithArgsReturn', [ @@ -152,7 +152,7 @@ describe('LowLevelCall', function () { }); describe('staticcallReturnBytes32', function () { - beforeEach(async function () { + beforeEach(function () { this.returnValue = ethers.id('returnDataBytes32'); }); @@ -173,7 +173,7 @@ describe('LowLevelCall', function () { }); describe('staticcallReturnBytes32Pair', function () { - beforeEach(async function () { + beforeEach(function () { this.returnValue1 = ethers.id('returnDataBytes32Pair1'); this.returnValue2 = ethers.id('returnDataBytes32Pair2'); }); diff --git a/test/utils/Memory.test.js b/test/utils/Memory.test.js index 523bf886d26..9ae373ce411 100644 --- a/test/utils/Memory.test.js +++ b/test/utils/Memory.test.js @@ -16,8 +16,8 @@ describe('Memory', function () { describe('free pointer', function () { it('returns the new memory pointer after it has been altered', async function () { const ptr = '0x00000000000000000000000000000000000000000000000000000000000000a0'; - await this.mock.$setFreePointer(ptr); - expect(await this.mock.$getFreePointer()).to.eq(ptr); + await this.mock._setFreePointer(ptr); + expect(await this.mock._getFreePointer()).to.eq(ptr); }); }); }); From 28889bd553d7577100f5d418a51ef06026134efb Mon Sep 17 00:00:00 2001 From: ernestognw Date: Thu, 27 Jun 2024 16:49:24 -0600 Subject: [PATCH 15/56] Rollback --- contracts/mocks/MemoryMock.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/mocks/MemoryMock.sol b/contracts/mocks/MemoryMock.sol index 128f66c0c11..6494581a4ec 100644 --- a/contracts/mocks/MemoryMock.sol +++ b/contracts/mocks/MemoryMock.sol @@ -20,7 +20,7 @@ contract MemoryMock { function _setFreePointer(bytes32 ptr) public { _setPointer(ptr); - return Memory.setFreePointer(ptr); + return Memory.setFreePointer(Memory.Pointer.wrap(ptr)); } function _getFreePointer() public view _rememberPtr returns (bytes32) { From 39edc84984ce9a163e62ad6252120a5a070fcab9 Mon Sep 17 00:00:00 2001 From: ernestognw Date: Thu, 27 Jun 2024 17:25:57 -0600 Subject: [PATCH 16/56] FV for Memory --- contracts/mocks/MemoryMock.sol | 29 ----------------------------- contracts/utils/Memory.sol | 10 ++++++++++ test/utils/Memory.t.sol | 16 ++++++++++++++++ test/utils/Memory.test.js | 23 ----------------------- 4 files changed, 26 insertions(+), 52 deletions(-) delete mode 100644 contracts/mocks/MemoryMock.sol create mode 100644 test/utils/Memory.t.sol delete mode 100644 test/utils/Memory.test.js diff --git a/contracts/mocks/MemoryMock.sol b/contracts/mocks/MemoryMock.sol deleted file mode 100644 index 6494581a4ec..00000000000 --- a/contracts/mocks/MemoryMock.sol +++ /dev/null @@ -1,29 +0,0 @@ -// SPDX-License-Identifier: MIT - -pragma solidity ^0.8.20; - -import {Memory} from "../utils/Memory.sol"; - -contract MemoryMock { - bytes32 private _ptr; - - modifier _rememberPtr() { - assembly ("memory-safe") { - mstore(0x00, sload(_ptr.slot)) - } - _; - } - - function _setPointer(bytes32 ptr) public { - _ptr = ptr; - } - - function _setFreePointer(bytes32 ptr) public { - _setPointer(ptr); - return Memory.setFreePointer(Memory.Pointer.wrap(ptr)); - } - - function _getFreePointer() public view _rememberPtr returns (bytes32) { - return Memory.Pointer.unwrap(Memory.getFreePointer()); - } -} diff --git a/contracts/utils/Memory.sol b/contracts/utils/Memory.sol index a26e6516e7b..a0fc881e318 100644 --- a/contracts/utils/Memory.sol +++ b/contracts/utils/Memory.sol @@ -21,4 +21,14 @@ library Memory { mstore(0x40, ptr) } } + + /// @dev Pointer to `bytes32`. + function asBytes32(Pointer ptr) internal pure returns (bytes32) { + return Pointer.unwrap(ptr); + } + + /// @dev `bytes32` to pointer. + function asPointer(bytes32 value) internal pure returns (Pointer) { + return Pointer.wrap(value); + } } diff --git a/test/utils/Memory.t.sol b/test/utils/Memory.t.sol new file mode 100644 index 00000000000..793b1b6f3fc --- /dev/null +++ b/test/utils/Memory.t.sol @@ -0,0 +1,16 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +import {Test} from "forge-std/Test.sol"; +import {Memory} from "@openzeppelin/contracts/utils/Memory.sol"; + +contract MemoryTest is Test { + using Memory for *; + + function testSymbolicGetSetFreePointer(uint256 ptr) public { + Memory.Pointer memoryPtr = Memory.asPointer(bytes32(ptr)); + Memory.setFreePointer(memoryPtr); + assertEq(Memory.getFreePointer().asBytes32(), memoryPtr.asBytes32()); + } +} diff --git a/test/utils/Memory.test.js b/test/utils/Memory.test.js deleted file mode 100644 index 9ae373ce411..00000000000 --- a/test/utils/Memory.test.js +++ /dev/null @@ -1,23 +0,0 @@ -const { ethers } = require('hardhat'); -const { expect } = require('chai'); -const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers'); - -async function fixture() { - const mock = await ethers.deployContract('MemoryMock'); - - return { mock }; -} - -describe('Memory', function () { - beforeEach(async function () { - Object.assign(this, await loadFixture(fixture)); - }); - - describe('free pointer', function () { - it('returns the new memory pointer after it has been altered', async function () { - const ptr = '0x00000000000000000000000000000000000000000000000000000000000000a0'; - await this.mock._setFreePointer(ptr); - expect(await this.mock._getFreePointer()).to.eq(ptr); - }); - }); -}); From a5918deaa77248e2e0383ba7ae2c1eadc2747598 Mon Sep 17 00:00:00 2001 From: ernestognw Date: Thu, 27 Jun 2024 17:27:24 -0600 Subject: [PATCH 17/56] Simplify --- test/utils/Memory.t.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/utils/Memory.t.sol b/test/utils/Memory.t.sol index 793b1b6f3fc..9beb3096597 100644 --- a/test/utils/Memory.t.sol +++ b/test/utils/Memory.t.sol @@ -8,8 +8,8 @@ import {Memory} from "@openzeppelin/contracts/utils/Memory.sol"; contract MemoryTest is Test { using Memory for *; - function testSymbolicGetSetFreePointer(uint256 ptr) public { - Memory.Pointer memoryPtr = Memory.asPointer(bytes32(ptr)); + function testSymbolicGetSetFreePointer(bytes32 ptr) public { + Memory.Pointer memoryPtr = Memory.asPointer(ptr); Memory.setFreePointer(memoryPtr); assertEq(Memory.getFreePointer().asBytes32(), memoryPtr.asBytes32()); } From 29e0c7a097f897e86860dbc587ed4ffb7de1a78b Mon Sep 17 00:00:00 2001 From: ernestognw Date: Thu, 27 Jun 2024 17:29:58 -0600 Subject: [PATCH 18/56] Simplify --- test/utils/Memory.t.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/utils/Memory.t.sol b/test/utils/Memory.t.sol index 9beb3096597..4cc60b88f9c 100644 --- a/test/utils/Memory.t.sol +++ b/test/utils/Memory.t.sol @@ -9,7 +9,7 @@ contract MemoryTest is Test { using Memory for *; function testSymbolicGetSetFreePointer(bytes32 ptr) public { - Memory.Pointer memoryPtr = Memory.asPointer(ptr); + Memory.Pointer memoryPtr = ptr.asPointer(); Memory.setFreePointer(memoryPtr); assertEq(Memory.getFreePointer().asBytes32(), memoryPtr.asBytes32()); } From 652df3f16014086621902545c3ab08b8da9b8e0f Mon Sep 17 00:00:00 2001 From: ernestognw Date: Thu, 27 Jun 2024 18:09:40 -0600 Subject: [PATCH 19/56] Fix coverage --- test/utils/cryptography/Memory.test.js | 41 ++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 test/utils/cryptography/Memory.test.js diff --git a/test/utils/cryptography/Memory.test.js b/test/utils/cryptography/Memory.test.js new file mode 100644 index 00000000000..5698728dcfd --- /dev/null +++ b/test/utils/cryptography/Memory.test.js @@ -0,0 +1,41 @@ +const { ethers } = require('hardhat'); +const { expect } = require('chai'); +const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers'); + +async function fixture() { + const mock = await ethers.deployContract('$Memory'); + + return { mock }; +} + +describe('Memory', function () { + beforeEach(async function () { + Object.assign(this, await loadFixture(fixture)); + }); + + describe('free pointer', function () { + it('sets memory pointer', async function () { + const ptr = '0x00000000000000000000000000000000000000000000000000000000000000a0'; + expect(await this.mock.$setFreePointer(ptr)).to.not.be.reverted; + }); + + it('gets memory pointer', async function () { + expect(await this.mock.$getFreePointer()).to.equal( + // Default pointer + '0x0000000000000000000000000000000000000000000000000000000000000080', + ); + }); + + it('asBytes32', async function () { + const ptr = ethers.toBeHex('0x1234', 32); + await this.mock.$setFreePointer(ptr); + expect(await this.mock.$asBytes32(ptr)).to.equal(ptr); + }); + + it('asPointer', async function () { + const ptr = ethers.toBeHex('0x1234', 32); + await this.mock.$setFreePointer(ptr); + expect(await this.mock.$asPointer(ptr)).to.equal(ptr); + }); + }); +}); From 9eb5f1cac83afe4d0000ea5d0a9aaf296a8c21d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernesto=20Garc=C3=ADa?= Date: Wed, 4 Sep 2024 11:13:57 -0600 Subject: [PATCH 20/56] Add memory utils --- .changeset/dull-students-eat.md | 5 ++++ contracts/utils/Memory.sol | 34 +++++++++++++++++++++++++++ contracts/utils/README.adoc | 1 + test/utils/Memory.t.sol | 16 +++++++++++++ test/utils/Memory.test.js | 41 +++++++++++++++++++++++++++++++++ 5 files changed, 97 insertions(+) create mode 100644 .changeset/dull-students-eat.md create mode 100644 contracts/utils/Memory.sol create mode 100644 test/utils/Memory.t.sol create mode 100644 test/utils/Memory.test.js diff --git a/.changeset/dull-students-eat.md b/.changeset/dull-students-eat.md new file mode 100644 index 00000000000..94c4fc21ef2 --- /dev/null +++ b/.changeset/dull-students-eat.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': minor +--- + +`Memory`: Add library with utilities to manipulate memory diff --git a/contracts/utils/Memory.sol b/contracts/utils/Memory.sol new file mode 100644 index 00000000000..a0fc881e318 --- /dev/null +++ b/contracts/utils/Memory.sol @@ -0,0 +1,34 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +/// @dev Memory utility library. +library Memory { + type Pointer is bytes32; + + /// @dev Returns a memory pointer to the current free memory pointer. + function getFreePointer() internal pure returns (Pointer ptr) { + assembly ("memory-safe") { + ptr := mload(0x40) + } + } + + /// @dev Sets the free memory pointer to a specific value. + /// + /// WARNING: Everything after the pointer may be overwritten. + function setFreePointer(Pointer ptr) internal pure { + assembly ("memory-safe") { + mstore(0x40, ptr) + } + } + + /// @dev Pointer to `bytes32`. + function asBytes32(Pointer ptr) internal pure returns (bytes32) { + return Pointer.unwrap(ptr); + } + + /// @dev `bytes32` to pointer. + function asPointer(bytes32 value) internal pure returns (Pointer) { + return Pointer.wrap(value); + } +} diff --git a/contracts/utils/README.adoc b/contracts/utils/README.adoc index 0ef3e5387c8..87af4fd4b7b 100644 --- a/contracts/utils/README.adoc +++ b/contracts/utils/README.adoc @@ -40,6 +40,7 @@ Miscellaneous contracts and libraries containing utility functions you can use t * {Packing}: A library for packing and unpacking multiple values into bytes32 * {Panic}: A library to revert with https://docs.soliditylang.org/en/v0.8.20/control-structures.html#panic-via-assert-and-error-via-require[Solidity panic codes]. * {Comparators}: A library that contains comparator functions to use with with the {Heap} library. + * {Memory}: A utility library to manipulate memory. [NOTE] ==== diff --git a/test/utils/Memory.t.sol b/test/utils/Memory.t.sol new file mode 100644 index 00000000000..4cc60b88f9c --- /dev/null +++ b/test/utils/Memory.t.sol @@ -0,0 +1,16 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +import {Test} from "forge-std/Test.sol"; +import {Memory} from "@openzeppelin/contracts/utils/Memory.sol"; + +contract MemoryTest is Test { + using Memory for *; + + function testSymbolicGetSetFreePointer(bytes32 ptr) public { + Memory.Pointer memoryPtr = ptr.asPointer(); + Memory.setFreePointer(memoryPtr); + assertEq(Memory.getFreePointer().asBytes32(), memoryPtr.asBytes32()); + } +} diff --git a/test/utils/Memory.test.js b/test/utils/Memory.test.js new file mode 100644 index 00000000000..5698728dcfd --- /dev/null +++ b/test/utils/Memory.test.js @@ -0,0 +1,41 @@ +const { ethers } = require('hardhat'); +const { expect } = require('chai'); +const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers'); + +async function fixture() { + const mock = await ethers.deployContract('$Memory'); + + return { mock }; +} + +describe('Memory', function () { + beforeEach(async function () { + Object.assign(this, await loadFixture(fixture)); + }); + + describe('free pointer', function () { + it('sets memory pointer', async function () { + const ptr = '0x00000000000000000000000000000000000000000000000000000000000000a0'; + expect(await this.mock.$setFreePointer(ptr)).to.not.be.reverted; + }); + + it('gets memory pointer', async function () { + expect(await this.mock.$getFreePointer()).to.equal( + // Default pointer + '0x0000000000000000000000000000000000000000000000000000000000000080', + ); + }); + + it('asBytes32', async function () { + const ptr = ethers.toBeHex('0x1234', 32); + await this.mock.$setFreePointer(ptr); + expect(await this.mock.$asBytes32(ptr)).to.equal(ptr); + }); + + it('asPointer', async function () { + const ptr = ethers.toBeHex('0x1234', 32); + await this.mock.$setFreePointer(ptr); + expect(await this.mock.$asPointer(ptr)).to.equal(ptr); + }); + }); +}); From 60d33d415ba69415060be55167e7e36778b79cec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernesto=20Garc=C3=ADa?= Date: Wed, 4 Sep 2024 11:35:51 -0600 Subject: [PATCH 21/56] Move memory tests --- test/utils/{cryptography => }/Memory.test.js | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/utils/{cryptography => }/Memory.test.js (100%) diff --git a/test/utils/cryptography/Memory.test.js b/test/utils/Memory.test.js similarity index 100% rename from test/utils/cryptography/Memory.test.js rename to test/utils/Memory.test.js From 2d397f467f202ccc1dc3c2b240a9f3353f13af83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernesto=20Garc=C3=ADa?= Date: Wed, 4 Sep 2024 11:43:04 -0600 Subject: [PATCH 22/56] Fix tests upgradeable --- contracts/mocks/Stateless.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/contracts/mocks/Stateless.sol b/contracts/mocks/Stateless.sol index 846c77d98e8..a96dd48cc87 100644 --- a/contracts/mocks/Stateless.sol +++ b/contracts/mocks/Stateless.sol @@ -37,6 +37,7 @@ import {SignatureChecker} from "../utils/cryptography/SignatureChecker.sol"; import {SignedMath} from "../utils/math/SignedMath.sol"; import {StorageSlot} from "../utils/StorageSlot.sol"; import {Strings} from "../utils/Strings.sol"; +import {Memory} from "../utils/Memory.sol"; import {Time} from "../utils/types/Time.sol"; contract Dummy1234 {} From 2a0fb7e5db92a563991ff7b447596b8191d22381 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernesto=20Garc=C3=ADa?= Date: Thu, 5 Sep 2024 12:21:13 -0600 Subject: [PATCH 23/56] Add docs --- contracts/utils/Memory.sol | 8 +++++- docs/modules/ROOT/pages/utilities.adoc | 36 +++++++++++++++++++++++++- test/utils/Memory.t.sol | 5 ++-- 3 files changed, 44 insertions(+), 5 deletions(-) diff --git a/contracts/utils/Memory.sol b/contracts/utils/Memory.sol index a0fc881e318..abb6f100bc6 100644 --- a/contracts/utils/Memory.sol +++ b/contracts/utils/Memory.sol @@ -2,7 +2,13 @@ pragma solidity ^0.8.20; -/// @dev Memory utility library. +/// @dev Utilities to manipulate memory. +/// +/// Memory is a contiguous and dynamic byte array in which Solidity stores non-primitive types. +/// This library provides functions to manipulate pointers to this dynamic array. +/// +/// WARNING: When manipulating memory, make sure to follow the Solidity documentation +/// guidelines for https://docs.soliditylang.org/en/v0.8.20/assembly.html#memory-safety[Memory Safety]. library Memory { type Pointer is bytes32; diff --git a/docs/modules/ROOT/pages/utilities.adoc b/docs/modules/ROOT/pages/utilities.adoc index b8afec4eabd..d1cf470d60a 100644 --- a/docs/modules/ROOT/pages/utilities.adoc +++ b/docs/modules/ROOT/pages/utilities.adoc @@ -189,7 +189,7 @@ Some use cases require more powerful data structures than arrays and mappings of - xref:api:utils.adoc#EnumerableSet[`EnumerableSet`]: A https://en.wikipedia.org/wiki/Set_(abstract_data_type)[set] with enumeration capabilities. - xref:api:utils.adoc#EnumerableMap[`EnumerableMap`]: A `mapping` variant with enumeration capabilities. - xref:api:utils.adoc#MerkleTree[`MerkleTree`]: An on-chain https://wikipedia.org/wiki/Merkle_Tree[Merkle Tree] with helper functions. -- xref:api:utils.adoc#Heap.sol[`Heap`]: A +- xref:api:utils.adoc#Heap.sol[`Heap`]: A https://en.wikipedia.org/wiki/Binary_heap[binary heap] to store elements with priority defined by a compartor function. The `Enumerable*` structures are similar to mappings in that they store and remove elements in constant time and don't allow for repeated entries, but they also support _enumeration_, which means you can easily query all stored entries both on and off-chain. @@ -386,3 +386,37 @@ await instance.multicall([ instance.interface.encodeFunctionData("bar") ]); ---- + +=== Memory + +The `Memory` library provides functions for advanced use cases that require granular memory management. A common use case is to avoid unnecessary memory expansion costs when iterating over a section of the code that allocates new memory. Consider the following example: + +[source,solidity] +---- +function callFoo(address target) internal { + bytes memory callData = abi.encodeWithSelector( + bytes4(keccak256("foo()")) + ) + (bool success, /* bytes memory returndata */) = target.call(callData); + require(success); +} +---- + +Note the function allocates memory for both the `callData` argument and for the returndata even if it's ignored. As such, it may be desirable to reset the free memory pointer after the end of the function. + +[source,solidity] +---- +function callFoo(address target) internal { + Memory.Pointer ptr = Memory.getFreePointer(); // Cache pointer + bytes memory callData = abi.encodeWithSelector( + bytes4(keccak256("foo()")) + ) + (bool success, /* bytes memory returndata */) = target.call(callData); + require(success); + Memory.setFreePointer(ptr); // Reset pointer +} +---- + +In this way, new memory will be allocated in the space where the `returndata` and `callData` used to be, potentially reducing memory expansion costs by shrinking the its size at the end of the transaction and resulting in gas savings. + +IMPORTANT: By default, Solidity handles memory safely. Using this library without understanding how memory works may be dangerous. Consider thoroughly reading the Solidity documentation about the https://docs.soliditylang.org/en/v0.8.20/internals/layout_in_memory.html[memory layout] and how the language defines https://docs.soliditylang.org/en/v0.8.20/assembly.html#memory-safety[memory safety]. diff --git a/test/utils/Memory.t.sol b/test/utils/Memory.t.sol index 4cc60b88f9c..99cb75fb095 100644 --- a/test/utils/Memory.t.sol +++ b/test/utils/Memory.t.sol @@ -9,8 +9,7 @@ contract MemoryTest is Test { using Memory for *; function testSymbolicGetSetFreePointer(bytes32 ptr) public { - Memory.Pointer memoryPtr = ptr.asPointer(); - Memory.setFreePointer(memoryPtr); - assertEq(Memory.getFreePointer().asBytes32(), memoryPtr.asBytes32()); + Memory.setFreePointer(ptr.asPointer()); + assertEq(Memory.getFreePointer().asBytes32(), ptr); } } From a7e61c3bd521822e7a8c932c5596c6f3cea8739e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernesto=20Garc=C3=ADa?= Date: Thu, 5 Sep 2024 12:32:19 -0600 Subject: [PATCH 24/56] Make use of the library --- contracts/access/manager/AuthorityUtils.sol | 3 +++ contracts/token/ERC20/extensions/ERC4626.sol | 3 +++ contracts/token/ERC20/utils/SafeERC20.sol | 7 +++++++ contracts/utils/cryptography/SignatureChecker.sol | 6 +++++- 4 files changed, 18 insertions(+), 1 deletion(-) diff --git a/contracts/access/manager/AuthorityUtils.sol b/contracts/access/manager/AuthorityUtils.sol index fb3018ca805..4cc77123716 100644 --- a/contracts/access/manager/AuthorityUtils.sol +++ b/contracts/access/manager/AuthorityUtils.sol @@ -4,6 +4,7 @@ pragma solidity ^0.8.20; import {IAuthority} from "./IAuthority.sol"; +import {Memory} from "../../utils/Memory.sol"; library AuthorityUtils { /** @@ -17,6 +18,7 @@ library AuthorityUtils { address target, bytes4 selector ) internal view returns (bool immediate, uint32 delay) { + Memory.Pointer ptr = Memory.getFreePointer(); (bool success, bytes memory data) = authority.staticcall( abi.encodeCall(IAuthority.canCall, (caller, target, selector)) ); @@ -27,6 +29,7 @@ library AuthorityUtils { immediate = abi.decode(data, (bool)); } } + Memory.setFreePointer(ptr); return (immediate, delay); } } diff --git a/contracts/token/ERC20/extensions/ERC4626.sol b/contracts/token/ERC20/extensions/ERC4626.sol index c71b14ad48c..121d729bc96 100644 --- a/contracts/token/ERC20/extensions/ERC4626.sol +++ b/contracts/token/ERC20/extensions/ERC4626.sol @@ -7,6 +7,7 @@ import {IERC20, IERC20Metadata, ERC20} from "../ERC20.sol"; import {SafeERC20} from "../utils/SafeERC20.sol"; import {IERC4626} from "../../../interfaces/IERC4626.sol"; import {Math} from "../../../utils/math/Math.sol"; +import {Memory} from "../../../utils/Memory.sol"; /** * @dev Implementation of the ERC-4626 "Tokenized Vault Standard" as defined in @@ -84,6 +85,7 @@ abstract contract ERC4626 is ERC20, IERC4626 { * @dev Attempts to fetch the asset decimals. A return value of false indicates that the attempt failed in some way. */ function _tryGetAssetDecimals(IERC20 asset_) private view returns (bool, uint8) { + Memory.Pointer ptr = Memory.getFreePointer(); (bool success, bytes memory encodedDecimals) = address(asset_).staticcall( abi.encodeCall(IERC20Metadata.decimals, ()) ); @@ -93,6 +95,7 @@ abstract contract ERC4626 is ERC20, IERC4626 { return (true, uint8(returnedDecimals)); } } + Memory.setFreePointer(ptr); return (false, 0); } diff --git a/contracts/token/ERC20/utils/SafeERC20.sol b/contracts/token/ERC20/utils/SafeERC20.sol index ed41fb042c9..4d06ded819d 100644 --- a/contracts/token/ERC20/utils/SafeERC20.sol +++ b/contracts/token/ERC20/utils/SafeERC20.sol @@ -6,6 +6,7 @@ pragma solidity ^0.8.20; import {IERC20} from "../IERC20.sol"; import {IERC1363} from "../../../interfaces/IERC1363.sol"; import {Address} from "../../../utils/Address.sol"; +import {Memory} from "../../../utils/Memory.sol"; /** * @title SafeERC20 @@ -32,7 +33,9 @@ library SafeERC20 { * non-reverting calls are assumed to be successful. */ function safeTransfer(IERC20 token, address to, uint256 value) internal { + Memory.Pointer ptr = Memory.getFreePointer(); _callOptionalReturn(token, abi.encodeCall(token.transfer, (to, value))); + Memory.setFreePointer(ptr); } /** @@ -40,7 +43,9 @@ library SafeERC20 { * calling contract. If `token` returns no value, non-reverting calls are assumed to be successful. */ function safeTransferFrom(IERC20 token, address from, address to, uint256 value) internal { + Memory.Pointer ptr = Memory.getFreePointer(); _callOptionalReturn(token, abi.encodeCall(token.transferFrom, (from, to, value))); + Memory.setFreePointer(ptr); } /** @@ -72,12 +77,14 @@ library SafeERC20 { * to be set to zero before setting it to a non-zero value, such as USDT. */ function forceApprove(IERC20 token, address spender, uint256 value) internal { + Memory.Pointer ptr = Memory.getFreePointer(); bytes memory approvalCall = abi.encodeCall(token.approve, (spender, value)); if (!_callOptionalReturnBool(token, approvalCall)) { _callOptionalReturn(token, abi.encodeCall(token.approve, (spender, 0))); _callOptionalReturn(token, approvalCall); } + Memory.setFreePointer(ptr); } /** diff --git a/contracts/utils/cryptography/SignatureChecker.sol b/contracts/utils/cryptography/SignatureChecker.sol index 9aaa2e0716c..16e038d2d87 100644 --- a/contracts/utils/cryptography/SignatureChecker.sol +++ b/contracts/utils/cryptography/SignatureChecker.sol @@ -5,6 +5,7 @@ pragma solidity ^0.8.20; import {ECDSA} from "./ECDSA.sol"; import {IERC1271} from "../../interfaces/IERC1271.sol"; +import {Memory} from "../Memory.sol"; /** * @dev Signature verification helper that can be used instead of `ECDSA.recover` to seamlessly support both ECDSA @@ -40,11 +41,14 @@ library SignatureChecker { bytes32 hash, bytes memory signature ) internal view returns (bool) { + Memory.Pointer ptr = Memory.getFreePointer(); (bool success, bytes memory result) = signer.staticcall( abi.encodeCall(IERC1271.isValidSignature, (hash, signature)) ); - return (success && + bool valid = (success && result.length >= 32 && abi.decode(result, (bytes32)) == bytes32(IERC1271.isValidSignature.selector)); + Memory.setFreePointer(ptr); + return valid; } } From 1aae8bbd7e77f20600c4661a56a1d1d95ceb76b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernesto=20Garc=C3=ADa?= Date: Tue, 8 Oct 2024 23:48:04 -0600 Subject: [PATCH 25/56] Update docs/modules/ROOT/pages/utilities.adoc Co-authored-by: Hadrien Croubois --- docs/modules/ROOT/pages/utilities.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/modules/ROOT/pages/utilities.adoc b/docs/modules/ROOT/pages/utilities.adoc index d1cf470d60a..84ede5a4b5e 100644 --- a/docs/modules/ROOT/pages/utilities.adoc +++ b/docs/modules/ROOT/pages/utilities.adoc @@ -417,6 +417,6 @@ function callFoo(address target) internal { } ---- -In this way, new memory will be allocated in the space where the `returndata` and `callData` used to be, potentially reducing memory expansion costs by shrinking the its size at the end of the transaction and resulting in gas savings. +This way, memory is allocated to accommodate the `callData`, and the `returndata` is freed. This allows other memory operations to reuse that space, thus reducing the memory expansion costs of these operations. In particular, this allows many `callFoo` to be performed in a loop with limited memory expansion costs. IMPORTANT: By default, Solidity handles memory safely. Using this library without understanding how memory works may be dangerous. Consider thoroughly reading the Solidity documentation about the https://docs.soliditylang.org/en/v0.8.20/internals/layout_in_memory.html[memory layout] and how the language defines https://docs.soliditylang.org/en/v0.8.20/assembly.html#memory-safety[memory safety]. From d514606a9cba7c1fc081427b5d7dd19017f26f9c Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 6 Mar 2025 17:14:59 +0100 Subject: [PATCH 26/56] fix tests --- test/utils/Memory.t.sol | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/utils/Memory.t.sol b/test/utils/Memory.t.sol index 99cb75fb095..0affe3234c4 100644 --- a/test/utils/Memory.t.sol +++ b/test/utils/Memory.t.sol @@ -8,7 +8,11 @@ import {Memory} from "@openzeppelin/contracts/utils/Memory.sol"; contract MemoryTest is Test { using Memory for *; - function testSymbolicGetSetFreePointer(bytes32 ptr) public { + function testSymbolicGetSetFreePointer(uint256 seed) public pure { + // - first 0x80 bytes are reserved (scratch + FMP + zero) + // - moving the free memory pointer to far causes OOG errors + bytes32 ptr = bytes32(bound(seed, 0x80, type(uint24).max)); + Memory.setFreePointer(ptr.asPointer()); assertEq(Memory.getFreePointer().asBytes32(), ptr); } From 14fa04ef86a0c78af70fae316d7cee2cae71304c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernesto=20Garc=C3=ADa?= Date: Wed, 7 May 2025 12:30:10 -0600 Subject: [PATCH 27/56] Update contracts/utils/Memory.sol Co-authored-by: Arr00 <13561405+arr00@users.noreply.github.com> --- contracts/utils/Memory.sol | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/contracts/utils/Memory.sol b/contracts/utils/Memory.sol index abb6f100bc6..33842a6eb4d 100644 --- a/contracts/utils/Memory.sol +++ b/contracts/utils/Memory.sol @@ -2,13 +2,15 @@ pragma solidity ^0.8.20; -/// @dev Utilities to manipulate memory. -/// -/// Memory is a contiguous and dynamic byte array in which Solidity stores non-primitive types. -/// This library provides functions to manipulate pointers to this dynamic array. -/// -/// WARNING: When manipulating memory, make sure to follow the Solidity documentation -/// guidelines for https://docs.soliditylang.org/en/v0.8.20/assembly.html#memory-safety[Memory Safety]. +/** + * @dev Utilities to manipulate memory. + * + * Memory is a contiguous and dynamic byte array in which Solidity stores non-primitive types. + * This library provides functions to manipulate pointers to this dynamic array. + * + * WARNING: When manipulating memory, make sure to follow the Solidity documentation + * guidelines for https://docs.soliditylang.org/en/v0.8.20/assembly.html#memory-safety[Memory Safety]. + */ library Memory { type Pointer is bytes32; From d0d55fcc356d813d5563687a229ee6710e12d5aa Mon Sep 17 00:00:00 2001 From: Arr00 <13561405+arr00@users.noreply.github.com> Date: Wed, 7 May 2025 14:32:03 -0400 Subject: [PATCH 28/56] Update contracts/utils/Memory.sol --- contracts/utils/Memory.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/utils/Memory.sol b/contracts/utils/Memory.sol index 33842a6eb4d..e5cc0e06cc8 100644 --- a/contracts/utils/Memory.sol +++ b/contracts/utils/Memory.sol @@ -3,7 +3,7 @@ pragma solidity ^0.8.20; /** - * @dev Utilities to manipulate memory. + * @dev Utilities to manipulate memory. * * Memory is a contiguous and dynamic byte array in which Solidity stores non-primitive types. * This library provides functions to manipulate pointers to this dynamic array. From e38691d1ab280b399211c5c4d2413eb54d388cf4 Mon Sep 17 00:00:00 2001 From: ernestognw Date: Sun, 8 Jun 2025 13:03:38 -0600 Subject: [PATCH 29/56] Enhance LowLevelCall and Memory utils and usage --- contracts/access/manager/AuthorityUtils.sol | 2 - contracts/token/ERC20/extensions/ERC4626.sol | 5 +- contracts/token/ERC20/utils/SafeERC20.sol | 8 +-- contracts/utils/Address.sol | 8 +-- contracts/utils/Create2.sol | 11 ++-- contracts/utils/LowLevelCall.sol | 32 +++++++++++ contracts/utils/Memory.sol | 54 ++++++++++++++++++- .../utils/cryptography/SignatureChecker.sol | 6 +-- 8 files changed, 98 insertions(+), 28 deletions(-) diff --git a/contracts/access/manager/AuthorityUtils.sol b/contracts/access/manager/AuthorityUtils.sol index dd9cc7ee85b..8b0470968b9 100644 --- a/contracts/access/manager/AuthorityUtils.sol +++ b/contracts/access/manager/AuthorityUtils.sol @@ -4,8 +4,6 @@ pragma solidity ^0.8.20; import {IAuthority} from "./IAuthority.sol"; -import {Memory} from "../../utils/Memory.sol"; -import {LowLevelCall} from "../../utils/LowLevelCall.sol"; library AuthorityUtils { /** diff --git a/contracts/token/ERC20/extensions/ERC4626.sol b/contracts/token/ERC20/extensions/ERC4626.sol index f8f150c87b5..6e6a57c305d 100644 --- a/contracts/token/ERC20/extensions/ERC4626.sol +++ b/contracts/token/ERC20/extensions/ERC4626.sol @@ -7,8 +7,6 @@ import {IERC20, IERC20Metadata, ERC20} from "../ERC20.sol"; import {SafeERC20} from "../utils/SafeERC20.sol"; import {IERC4626} from "../../../interfaces/IERC4626.sol"; import {Math} from "../../../utils/math/Math.sol"; -import {Memory} from "../../../utils/Memory.sol"; -import {LowLevelCall} from "../../../utils/LowLevelCall.sol"; /** * @dev Implementation of the ERC-4626 "Tokenized Vault Standard" as defined in @@ -77,7 +75,8 @@ abstract contract ERC4626 is ERC20, IERC4626 { * @dev Set the underlying asset contract. This must be an ERC20-compatible contract (ERC-20 or ERC-777). */ constructor(IERC20 asset_) { - _underlyingDecimals = _tryGetAssetDecimalsWithFallback(asset_, 18); + (bool success, uint8 assetDecimals) = _tryGetAssetDecimals(asset_); + _underlyingDecimals = success ? assetDecimals : 18; _asset = asset_; } diff --git a/contracts/token/ERC20/utils/SafeERC20.sol b/contracts/token/ERC20/utils/SafeERC20.sol index c29dda4e70d..2de490d4c18 100644 --- a/contracts/token/ERC20/utils/SafeERC20.sol +++ b/contracts/token/ERC20/utils/SafeERC20.sol @@ -182,12 +182,8 @@ library SafeERC20 { (bool success, bytes32 returnValue) = LowLevelCall.callReturnBytes32(address(token), data); uint256 returnSize = LowLevelCall.returnDataSize(); - assembly ("memory-safe") { - if iszero(success) { - // Bubble up revert reason - returndatacopy(data, 0, returnSize) - revert(data, returnSize) - } + if (!success) { + LowLevelCall.bubbleRevert(data); } if (returnSize == 0 ? address(token).code.length == 0 : uint256(returnValue) != 1) { diff --git a/contracts/utils/Address.sol b/contracts/utils/Address.sol index 6ba4e46f4c6..058872f9d3d 100644 --- a/contracts/utils/Address.sol +++ b/contracts/utils/Address.sol @@ -5,11 +5,14 @@ pragma solidity ^0.8.20; import {Errors} from "./Errors.sol"; import {LowLevelCall} from "./LowLevelCall.sol"; +import {Memory} from "./Memory.sol"; /** * @dev Collection of functions related to the address type */ library Address { + using Memory for *; + /** * @dev There's no code at `target` (it is not a contract). */ @@ -140,10 +143,7 @@ library Address { function _revert(bytes memory returndata) private pure { // Look for revert reason and bubble it up if present if (returndata.length > 0) { - // The easiest way to bubble the revert reason is using memory via assembly - assembly ("memory-safe") { - revert(add(returndata, 0x20), mload(returndata)) - } + LowLevelCall.bubbleRevert(returndata.asPointer().addOffset(0x20).asBytes32()); } else { revert Errors.FailedCall(); } diff --git a/contracts/utils/Create2.sol b/contracts/utils/Create2.sol index ffd39d9a46c..edd4a3e49aa 100644 --- a/contracts/utils/Create2.sol +++ b/contracts/utils/Create2.sol @@ -4,6 +4,8 @@ pragma solidity ^0.8.20; import {Errors} from "./Errors.sol"; +import {LowLevelCall} from "./LowLevelCall.sol"; +import {Memory} from "./Memory.sol"; /** * @dev Helper to make usage of the `CREATE2` EVM opcode easier and safer. @@ -43,15 +45,10 @@ library Create2 { } assembly ("memory-safe") { addr := create2(amount, add(bytecode, 0x20), mload(bytecode), salt) - // if no address was created, and returndata is not empty, bubble revert - if and(iszero(addr), not(iszero(returndatasize()))) { - let p := mload(0x40) - returndatacopy(p, 0, returndatasize()) - revert(p, returndatasize()) - } } if (addr == address(0)) { - revert Errors.FailedDeployment(); + if (LowLevelCall.returnDataSize() != 0) LowLevelCall.bubbleRevert(LowLevelCall.returnData()); + else revert Errors.FailedDeployment(); } } diff --git a/contracts/utils/LowLevelCall.sol b/contracts/utils/LowLevelCall.sol index bf552a3e78a..d51d0bfc576 100644 --- a/contracts/utils/LowLevelCall.sol +++ b/contracts/utils/LowLevelCall.sol @@ -3,6 +3,7 @@ pragma solidity ^0.8.20; import {Errors} from "./Errors.sol"; +import {Memory} from "./Memory.sol"; /** * @dev Library of low level call functions that implement different calling strategies to deal with the return data. @@ -11,6 +12,8 @@ import {Errors} from "./Errors.sol"; * to use the {Address} library instead. */ library LowLevelCall { + using Memory for *; + /// === CALL === /// @dev Performs a Solidity function call using a low level `call` and ignoring the return data. @@ -117,4 +120,33 @@ library LowLevelCall { size := returndatasize() } } + + /// @dev Returns the return data buffer. + function returnData() internal pure returns (bytes memory data) { + uint256 size = returnDataSize(); + assembly ("memory-safe") { + data := mload(0x40) + returndatacopy(data, 0, size) + } + } + + /// === BUBBLE UP REVERT REASON === + + /** + * @dev Bubbles up the revert reason. This function is useful to bubble up the revert reason + * from a low level call. Developers can provide any bytes memory pointer to the revert reason + * since the function will revert regardless of the pointer value. + */ + function bubbleRevert(bytes memory ptr) internal pure { + bubbleRevert(ptr.asPointer().asBytes32()); + } + + /// @dev Same as {bubbleRevert-bytes}, but for `bytes32` pointers. + function bubbleRevert(bytes32 ptr) internal pure { + uint256 size = returnDataSize(); + assembly ("memory-safe") { + returndatacopy(ptr, 0, size) + revert(ptr, size) + } + } } diff --git a/contracts/utils/Memory.sol b/contracts/utils/Memory.sol index a0fc881e318..5435e9ff5fe 100644 --- a/contracts/utils/Memory.sol +++ b/contracts/utils/Memory.sol @@ -2,7 +2,15 @@ pragma solidity ^0.8.20; -/// @dev Memory utility library. +/** + * @dev Utilities to manipulate memory. + * + * Memory is a contiguous and dynamic byte array in which Solidity stores non-primitive types. + * This library provides functions to manipulate pointers to this dynamic array. + * + * WARNING: When manipulating memory, make sure to follow the Solidity documentation + * guidelines for https://docs.soliditylang.org/en/v0.8.20/assembly.html#memory-safety[Memory Safety]. + */ library Memory { type Pointer is bytes32; @@ -22,6 +30,41 @@ library Memory { } } + /// @dev Returns a memory pointer to the content of a buffer. Skips the length word. + function contentPointer(bytes memory buffer) internal pure returns (Pointer) { + bytes32 ptr; + assembly ("memory-safe") { + ptr := add(buffer, 32) + } + return asPointer(ptr); + } + + /// @dev Copies `length` bytes from `srcPtr` to `destPtr`. + function copy(Pointer destPtr, Pointer srcPtr, uint256 length) internal pure { + assembly ("memory-safe") { + mcopy(destPtr, srcPtr, length) + } + } + + /// @dev Extracts a byte from a memory pointer. + function extractByte(Pointer ptr) internal pure returns (bytes1 v) { + assembly ("memory-safe") { + v := byte(0, mload(ptr)) + } + } + + /// @dev Extracts a word from a memory pointer. + function extractWord(Pointer ptr) internal pure returns (uint256 v) { + assembly ("memory-safe") { + v := mload(ptr) + } + } + + /// @dev Adds an offset to a memory pointer. + function addOffset(Pointer ptr, uint256 offset) internal pure returns (Pointer) { + return asPointer(bytes32(uint256(asBytes32(ptr)) + offset)); + } + /// @dev Pointer to `bytes32`. function asBytes32(Pointer ptr) internal pure returns (bytes32) { return Pointer.unwrap(ptr); @@ -31,4 +74,13 @@ library Memory { function asPointer(bytes32 value) internal pure returns (Pointer) { return Pointer.wrap(value); } + + /// @dev `bytes` to pointer. + function asPointer(bytes memory value) internal pure returns (Pointer) { + bytes32 ptr; + assembly ("memory-safe") { + ptr := value + } + return asPointer(ptr); + } } diff --git a/contracts/utils/cryptography/SignatureChecker.sol b/contracts/utils/cryptography/SignatureChecker.sol index 1cfb01b7abd..374e3b7f773 100644 --- a/contracts/utils/cryptography/SignatureChecker.sol +++ b/contracts/utils/cryptography/SignatureChecker.sol @@ -5,7 +5,6 @@ pragma solidity ^0.8.24; import {ECDSA} from "./ECDSA.sol"; import {IERC1271} from "../../interfaces/IERC1271.sol"; -import {Memory} from "../Memory.sol"; import {LowLevelCall} from "../LowLevelCall.sol"; import {IERC7913SignatureVerifier} from "../../interfaces/IERC7913.sol"; import {Bytes} from "../../utils/Bytes.sol"; @@ -52,13 +51,10 @@ library SignatureChecker { bytes32 hash, bytes memory signature ) internal view returns (bool) { - Memory.Pointer ptr = Memory.getFreePointer(); bytes memory params = abi.encodeCall(IERC1271.isValidSignature, (hash, signature)); (bool success, bytes32 result) = LowLevelCall.staticcallReturnBytes32(signer, params); - uint256 length = LowLevelCall.returnDataSize(); - Memory.setFreePointer(ptr); - return success && length >= 32 && result == bytes32(IERC1271.isValidSignature.selector); + return success && LowLevelCall.returnDataSize() >= 32 && result == bytes32(IERC1271.isValidSignature.selector); } /** From ac92bb41b6ccb71120b40601e63bb9f490d89ead Mon Sep 17 00:00:00 2001 From: ernestognw Date: Sun, 8 Jun 2025 13:12:54 -0600 Subject: [PATCH 30/56] up --- contracts/access/manager/AuthorityUtils.sol | 4 ---- contracts/token/ERC20/extensions/ERC4626.sol | 3 --- contracts/token/ERC20/utils/SafeERC20.sol | 7 ------- contracts/utils/cryptography/SignatureChecker.sol | 6 +----- 4 files changed, 1 insertion(+), 19 deletions(-) diff --git a/contracts/access/manager/AuthorityUtils.sol b/contracts/access/manager/AuthorityUtils.sol index 5aeed4f6285..8b0470968b9 100644 --- a/contracts/access/manager/AuthorityUtils.sol +++ b/contracts/access/manager/AuthorityUtils.sol @@ -4,7 +4,6 @@ pragma solidity ^0.8.20; import {IAuthority} from "./IAuthority.sol"; -import {Memory} from "../../utils/Memory.sol"; library AuthorityUtils { /** @@ -18,7 +17,6 @@ library AuthorityUtils { address target, bytes4 selector ) internal view returns (bool immediate, uint32 delay) { - Memory.Pointer ptr = Memory.getFreePointer(); bytes memory data = abi.encodeCall(IAuthority.canCall, (caller, target, selector)); assembly ("memory-safe") { @@ -34,7 +32,5 @@ library AuthorityUtils { delay := mul(delay, iszero(shr(32, delay))) } } - - Memory.setFreePointer(ptr); } } diff --git a/contracts/token/ERC20/extensions/ERC4626.sol b/contracts/token/ERC20/extensions/ERC4626.sol index d5b8bcb9888..6e6a57c305d 100644 --- a/contracts/token/ERC20/extensions/ERC4626.sol +++ b/contracts/token/ERC20/extensions/ERC4626.sol @@ -7,7 +7,6 @@ import {IERC20, IERC20Metadata, ERC20} from "../ERC20.sol"; import {SafeERC20} from "../utils/SafeERC20.sol"; import {IERC4626} from "../../../interfaces/IERC4626.sol"; import {Math} from "../../../utils/math/Math.sol"; -import {Memory} from "../../../utils/Memory.sol"; /** * @dev Implementation of the ERC-4626 "Tokenized Vault Standard" as defined in @@ -85,7 +84,6 @@ abstract contract ERC4626 is ERC20, IERC4626 { * @dev Attempts to fetch the asset decimals. A return value of false indicates that the attempt failed in some way. */ function _tryGetAssetDecimals(IERC20 asset_) private view returns (bool ok, uint8 assetDecimals) { - Memory.Pointer ptr = Memory.getFreePointer(); (bool success, bytes memory encodedDecimals) = address(asset_).staticcall( abi.encodeCall(IERC20Metadata.decimals, ()) ); @@ -95,7 +93,6 @@ abstract contract ERC4626 is ERC20, IERC4626 { return (true, uint8(returnedDecimals)); } } - Memory.setFreePointer(ptr); return (false, 0); } diff --git a/contracts/token/ERC20/utils/SafeERC20.sol b/contracts/token/ERC20/utils/SafeERC20.sol index bcd17bc0111..883e8d30c97 100644 --- a/contracts/token/ERC20/utils/SafeERC20.sol +++ b/contracts/token/ERC20/utils/SafeERC20.sol @@ -5,7 +5,6 @@ pragma solidity ^0.8.20; import {IERC20} from "../IERC20.sol"; import {IERC1363} from "../../../interfaces/IERC1363.sol"; -import {Memory} from "../../../utils/Memory.sol"; /** * @title SafeERC20 @@ -32,9 +31,7 @@ library SafeERC20 { * non-reverting calls are assumed to be successful. */ function safeTransfer(IERC20 token, address to, uint256 value) internal { - Memory.Pointer ptr = Memory.getFreePointer(); _callOptionalReturn(token, abi.encodeCall(token.transfer, (to, value))); - Memory.setFreePointer(ptr); } /** @@ -42,9 +39,7 @@ library SafeERC20 { * calling contract. If `token` returns no value, non-reverting calls are assumed to be successful. */ function safeTransferFrom(IERC20 token, address from, address to, uint256 value) internal { - Memory.Pointer ptr = Memory.getFreePointer(); _callOptionalReturn(token, abi.encodeCall(token.transferFrom, (from, to, value))); - Memory.setFreePointer(ptr); } /** @@ -104,14 +99,12 @@ library SafeERC20 { * set here. */ function forceApprove(IERC20 token, address spender, uint256 value) internal { - Memory.Pointer ptr = Memory.getFreePointer(); bytes memory approvalCall = abi.encodeCall(token.approve, (spender, value)); if (!_callOptionalReturnBool(token, approvalCall)) { _callOptionalReturn(token, abi.encodeCall(token.approve, (spender, 0))); _callOptionalReturn(token, approvalCall); } - Memory.setFreePointer(ptr); } /** diff --git a/contracts/utils/cryptography/SignatureChecker.sol b/contracts/utils/cryptography/SignatureChecker.sol index 1e8991a6bb9..261372f0c3d 100644 --- a/contracts/utils/cryptography/SignatureChecker.sol +++ b/contracts/utils/cryptography/SignatureChecker.sol @@ -5,7 +5,6 @@ pragma solidity ^0.8.24; import {ECDSA} from "./ECDSA.sol"; import {IERC1271} from "../../interfaces/IERC1271.sol"; -import {Memory} from "../Memory.sol"; import {IERC7913SignatureVerifier} from "../../interfaces/IERC7913.sol"; import {Bytes} from "../../utils/Bytes.sol"; @@ -51,15 +50,12 @@ library SignatureChecker { bytes32 hash, bytes memory signature ) internal view returns (bool) { - Memory.Pointer ptr = Memory.getFreePointer(); (bool success, bytes memory result) = signer.staticcall( abi.encodeCall(IERC1271.isValidSignature, (hash, signature)) ); - bool valid = (success && + return (success && result.length >= 32 && abi.decode(result, (bytes32)) == bytes32(IERC1271.isValidSignature.selector)); - Memory.setFreePointer(ptr); - return valid; } /** From 6bb96d5fcb850b240e4ae8a05473db943c379890 Mon Sep 17 00:00:00 2001 From: ernestognw Date: Sun, 8 Jun 2025 15:35:11 -0600 Subject: [PATCH 31/56] WIP: Add more Memory functions --- contracts/utils/Bytes.sol | 7 ++++ contracts/utils/Memory.sol | 66 ++++++++++++++++++++++++++++++++++--- contracts/utils/Strings.sol | 3 +- test/utils/Bytes.t.sol | 12 +++++++ test/utils/Memory.t.sol | 43 +++++++++++++++++++++--- 5 files changed, 121 insertions(+), 10 deletions(-) create mode 100644 test/utils/Bytes.t.sol diff --git a/contracts/utils/Bytes.sol b/contracts/utils/Bytes.sol index 1234b845513..f8c3fb2ebfa 100644 --- a/contracts/utils/Bytes.sol +++ b/contracts/utils/Bytes.sol @@ -99,6 +99,13 @@ library Bytes { return result; } + /** + * @dev Returns true if the two byte buffers are equal. + */ + function equal(bytes memory a, bytes memory b) internal pure returns (bool) { + return a.length == b.length && keccak256(a) == keccak256(b); + } + /** * @dev Reads a bytes32 from a bytes array without bounds checking. * diff --git a/contracts/utils/Memory.sol b/contracts/utils/Memory.sol index e5cc0e06cc8..2d8d76e85ce 100644 --- a/contracts/utils/Memory.sol +++ b/contracts/utils/Memory.sol @@ -14,14 +14,14 @@ pragma solidity ^0.8.20; library Memory { type Pointer is bytes32; - /// @dev Returns a memory pointer to the current free memory pointer. + /// @dev Returns a `Pointer` to the current free `Pointer`. function getFreePointer() internal pure returns (Pointer ptr) { assembly ("memory-safe") { ptr := mload(0x40) } } - /// @dev Sets the free memory pointer to a specific value. + /// @dev Sets the free `Pointer` to a specific value. /// /// WARNING: Everything after the pointer may be overwritten. function setFreePointer(Pointer ptr) internal pure { @@ -30,13 +30,71 @@ library Memory { } } - /// @dev Pointer to `bytes32`. + /// @dev Returns a `Pointer` to the content of a `bytes` buffer. Skips the length word. + function contentPointer(bytes memory buffer) internal pure returns (Pointer) { + return addOffset(asPointer(buffer), 32); + } + + /** + * @dev Copies `length` bytes from `srcPtr` to `destPtr`. Equivalent to https://www.evm.codes/?fork=cancun#5e[`mcopy`]. + * + * WARNING: Reading or writing beyond the allocated memory bounds of either pointer + * will result in undefined behavior and potential memory corruption. + */ + function copy(Pointer destPtr, Pointer srcPtr, uint256 length) internal pure { + assembly ("memory-safe") { + mcopy(destPtr, srcPtr, length) + } + } + + /// @dev Extracts a `bytes1` from a `Pointer`. `offset` starts from the most significant byte. + function extractByte(Pointer ptr, uint256 offset) internal pure returns (bytes1 v) { + bytes32 word = extractWord(ptr); + assembly ("memory-safe") { + v := byte(offset, word) + } + } + + /// @dev Extracts a `bytes32` from a `Pointer`. + function extractWord(Pointer ptr) internal pure returns (bytes32 v) { + assembly ("memory-safe") { + v := mload(ptr) + } + } + + /// @dev Adds an offset to a `Pointer`. + function addOffset(Pointer ptr, uint256 offset) internal pure returns (Pointer) { + return asPointer(bytes32(asUint256(ptr) + offset)); + } + + /// @dev `Pointer` to `bytes32`. function asBytes32(Pointer ptr) internal pure returns (bytes32) { return Pointer.unwrap(ptr); } - /// @dev `bytes32` to pointer. + /// @dev `Pointer` to `uint256`. + function asUint256(Pointer ptr) internal pure returns (uint256) { + return uint256(asBytes32(ptr)); + } + + /// @dev `bytes32` to `Pointer`. function asPointer(bytes32 value) internal pure returns (Pointer) { return Pointer.wrap(value); } + + /// @dev `bytes` to `Pointer`. + function asPointer(bytes memory value) internal pure returns (Pointer) { + bytes32 ptr; + assembly ("memory-safe") { + ptr := value + } + return asPointer(ptr); + } + + /// @dev `Pointer` to `bytes`. + function asBytes(Pointer ptr) internal pure returns (bytes memory b) { + assembly ("memory-safe") { + b := ptr + } + } } diff --git a/contracts/utils/Strings.sol b/contracts/utils/Strings.sol index 4cc597646f2..a865bfbc785 100644 --- a/contracts/utils/Strings.sol +++ b/contracts/utils/Strings.sol @@ -6,6 +6,7 @@ pragma solidity ^0.8.20; import {Math} from "./math/Math.sol"; import {SafeCast} from "./math/SafeCast.sol"; import {SignedMath} from "./math/SignedMath.sol"; +import {Bytes} from "./Bytes.sol"; /** * @dev String operations. @@ -132,7 +133,7 @@ library Strings { * @dev Returns true if the two strings are equal. */ function equal(string memory a, string memory b) internal pure returns (bool) { - return bytes(a).length == bytes(b).length && keccak256(bytes(a)) == keccak256(bytes(b)); + return Bytes.equal(bytes(a), bytes(b)); } /** diff --git a/test/utils/Bytes.t.sol b/test/utils/Bytes.t.sol new file mode 100644 index 00000000000..6b2d7b5cad3 --- /dev/null +++ b/test/utils/Bytes.t.sol @@ -0,0 +1,12 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +import {Test} from "forge-std/Test.sol"; +import {Bytes} from "@openzeppelin/contracts/utils/Bytes.sol"; + +contract BytesTest is Test { + function testSymbolicEqual(bytes memory a, bytes memory b) public pure { + assertEq(Bytes.equal(a, b), Bytes.equal(a, b)); + } +} diff --git a/test/utils/Memory.t.sol b/test/utils/Memory.t.sol index 0affe3234c4..8964c164523 100644 --- a/test/utils/Memory.t.sol +++ b/test/utils/Memory.t.sol @@ -4,16 +4,49 @@ pragma solidity ^0.8.20; import {Test} from "forge-std/Test.sol"; import {Memory} from "@openzeppelin/contracts/utils/Memory.sol"; +import {Bytes} from "@openzeppelin/contracts/utils/Bytes.sol"; contract MemoryTest is Test { using Memory for *; - function testSymbolicGetSetFreePointer(uint256 seed) public pure { - // - first 0x80 bytes are reserved (scratch + FMP + zero) - // - moving the free memory pointer to far causes OOG errors - bytes32 ptr = bytes32(bound(seed, 0x80, type(uint24).max)); + // - first 0x80 bytes are reserved (scratch + FMP + zero) + uint256 constant START_PTR = 0x80; + // - moving the free memory pointer to far causes OOG errors + uint256 constant END_PTR = type(uint24).max; - Memory.setFreePointer(ptr.asPointer()); + function testGetSetFreePointer(uint256 seed) public pure { + bytes32 ptr = bytes32(bound(seed, START_PTR, END_PTR)); + ptr.asPointer().setFreePointer(); assertEq(Memory.getFreePointer().asBytes32(), ptr); } + + function testSymbolicContentPointer(uint256 seed) public pure { + Memory.Pointer ptr = bytes32(bound(seed, START_PTR, END_PTR)).asPointer(); + assertEq(ptr.asBytes().contentPointer().asBytes32(), ptr.addOffset(32).asBytes32()); + } + + // function testCopy(bytes memory data, uint256 destSeed) public pure { + // uint256 upperPtr = data.asPointer().asUint256() + data.length; + // Memory.Pointer destPtr = bytes32(bound(destSeed, upperPtr, upperPtr + 100)).asPointer(); + // Memory.copy(data.asPointer(), destPtr, data.length + 32); + // for (uint256 i = 0; i < data.length; i++) { + // assertEq(data[i], destPtr.asBytes()[i]); + // } + // } + + function testExtractByte(uint256 seed, uint256 index) public pure { + index = bound(index, 0, 31); + Memory.Pointer ptr = bytes32(bound(seed, START_PTR, END_PTR)).asPointer(); + assertEq(ptr.extractByte(index), bytes1(ptr.asBytes32() >> (256 - index * 8))); + } + + // function testExtractWord(uint256 seed) public pure { + // Memory.Pointer ptr = bytes32(bound(seed, START_PTR, END_PTR)).asPointer(); + // assertEq(ptr.extractWord(), ptr.asBytes32()); + // } + + // function testAddOffset(uint256 seed, uint256 offset) public pure { + // Memory.Pointer ptr = bytes32(bound(seed, START_PTR, END_PTR)).asPointer(); + // assertEq(ptr.addOffset(offset).asUint256(), ptr.asUint256() + offset); + // } } From 860e5a819701d49f4cbb8f13ca6306e1853e6939 Mon Sep 17 00:00:00 2001 From: ernestognw Date: Sun, 8 Jun 2025 16:50:42 -0600 Subject: [PATCH 32/56] up --- contracts/utils/Memory.sol | 16 +++++--- test/utils/Memory.test.js | 78 +++++++++++++++++++++++++++++++------- 2 files changed, 74 insertions(+), 20 deletions(-) diff --git a/contracts/utils/Memory.sol b/contracts/utils/Memory.sol index 2d8d76e85ce..891754f94d1 100644 --- a/contracts/utils/Memory.sol +++ b/contracts/utils/Memory.sol @@ -47,7 +47,11 @@ library Memory { } } - /// @dev Extracts a `bytes1` from a `Pointer`. `offset` starts from the most significant byte. + /** + * @dev Extracts a `bytes1` from a `Pointer`. `offset` starts from the most significant byte. + * + * NOTE: Will return `0x00` if `offset` is larger or equal to `32`. + */ function extractByte(Pointer ptr, uint256 offset) internal pure returns (bytes1 v) { bytes32 word = extractWord(ptr); assembly ("memory-safe") { @@ -67,22 +71,22 @@ library Memory { return asPointer(bytes32(asUint256(ptr) + offset)); } - /// @dev `Pointer` to `bytes32`. + /// @dev `Pointer` to `bytes32`. Expects a pointer to a properly ABI-encoded `bytes` object. function asBytes32(Pointer ptr) internal pure returns (bytes32) { return Pointer.unwrap(ptr); } - /// @dev `Pointer` to `uint256`. + /// @dev `Pointer` to `uint256`. Expects a pointer to a properly ABI-encoded `bytes` object. function asUint256(Pointer ptr) internal pure returns (uint256) { return uint256(asBytes32(ptr)); } - /// @dev `bytes32` to `Pointer`. + /// @dev `bytes32` to `Pointer`. Expects a pointer to a properly ABI-encoded `bytes` object. function asPointer(bytes32 value) internal pure returns (Pointer) { return Pointer.wrap(value); } - /// @dev `bytes` to `Pointer`. + /// @dev Returns a `Pointer` to the `value`'s header (i.e. includes the length word). function asPointer(bytes memory value) internal pure returns (Pointer) { bytes32 ptr; assembly ("memory-safe") { @@ -91,7 +95,7 @@ library Memory { return asPointer(ptr); } - /// @dev `Pointer` to `bytes`. + /// @dev `Pointer` to `bytes`. Expects a pointer to a properly ABI-encoded `bytes` object. function asBytes(Pointer ptr) internal pure returns (bytes memory b) { assembly ("memory-safe") { b := ptr diff --git a/test/utils/Memory.test.js b/test/utils/Memory.test.js index 5698728dcfd..c6ae6ba2d76 100644 --- a/test/utils/Memory.test.js +++ b/test/utils/Memory.test.js @@ -14,28 +14,78 @@ describe('Memory', function () { }); describe('free pointer', function () { - it('sets memory pointer', async function () { - const ptr = '0x00000000000000000000000000000000000000000000000000000000000000a0'; - expect(await this.mock.$setFreePointer(ptr)).to.not.be.reverted; + it('sets free memory pointer', async function () { + const ptr = ethers.toBeHex(0xa0, 32); + await expect(this.mock.$setFreePointer(ptr)).to.not.be.reverted; }); - it('gets memory pointer', async function () { - expect(await this.mock.$getFreePointer()).to.equal( - // Default pointer - '0x0000000000000000000000000000000000000000000000000000000000000080', + it('gets free memory pointer', async function () { + await expect(this.mock.$getFreePointer()).to.eventually.equal( + ethers.toBeHex(0x80, 32), // Default pointer ); }); + }); - it('asBytes32', async function () { - const ptr = ethers.toBeHex('0x1234', 32); - await this.mock.$setFreePointer(ptr); - expect(await this.mock.$asBytes32(ptr)).to.equal(ptr); + it('extractWord extracts a word', async function () { + const ptr = await this.mock.$getFreePointer(); + await expect(this.mock.$extractWord(ptr)).to.eventually.equal(ethers.toBeHex(0, 32)); + }); + + it('extractByte extracts a byte', async function () { + const ptr = await this.mock.$getFreePointer(); + await expect(this.mock.$extractByte(ptr, 0)).to.eventually.equal(ethers.toBeHex(0, 1)); + }); + + it('contentPointer', async function () { + const data = ethers.toUtf8Bytes('hello world'); + const result = await this.mock.$contentPointer(data); + expect(result).to.equal(ethers.toBeHex(0xa0, 32)); // 0x80 is the default free pointer (length) + }); + + describe('addOffset', function () { + it('addOffset', async function () { + const basePtr = ethers.toBeHex(0x80, 32); + const offset = 32; + const expectedPtr = ethers.toBeHex(0xa0, 32); + + await expect(this.mock.$addOffset(basePtr, offset)).to.eventually.equal(expectedPtr); }); - it('asPointer', async function () { + it('addOffsetwraps around', async function () { + const basePtr = ethers.toBeHex(0x80, 32); + const offset = 256; + const expectedPtr = ethers.toBeHex(0x180, 32); + await expect(this.mock.$addOffset(basePtr, offset)).to.eventually.equal(expectedPtr); + }); + }); + + describe('pointer conversions', function () { + it('asBytes32 / asPointer', async function () { const ptr = ethers.toBeHex('0x1234', 32); - await this.mock.$setFreePointer(ptr); - expect(await this.mock.$asPointer(ptr)).to.equal(ptr); + await expect(this.mock.$asBytes32(ptr)).to.eventually.equal(ptr); + await expect(this.mock.$asPointer(ethers.Typed.bytes32(ptr))).to.eventually.equal(ptr); + }); + + it('asBytes / asPointer', async function () { + const ptr = await this.mock.$asPointer(ethers.Typed.bytes(ethers.toUtf8Bytes('hello world'))); + expect(ptr).to.equal(ethers.toBeHex(0x80, 32)); // Default free pointer + await expect(this.mock.$asBytes(ptr)).to.eventually.equal(ethers.toBeHex(0x20, 32)); + }); + + it('asUint256', async function () { + const value = 0x1234; + const ptr = ethers.toBeHex(value, 32); + await expect(this.mock.$asUint256(ptr)).to.eventually.equal(value); + }); + }); + + describe('memory operations', function () { + it('copy', async function () { + await expect(this.mock.$copy(ethers.toBeHex(0x80, 32), ethers.toBeHex(0xc0, 32), 32)).to.not.be.reverted; + }); + + it('copy with zero length', async function () { + await expect(this.mock.$copy(ethers.toBeHex(0x80, 32), ethers.toBeHex(0xc0, 32), 0)).to.not.be.reverted; }); }); }); From ecdb768fc852cc37608f4f9842cd1c9703bfa19a Mon Sep 17 00:00:00 2001 From: ernestognw Date: Sun, 8 Jun 2025 17:01:02 -0600 Subject: [PATCH 33/56] revert --- contracts/utils/Bytes.sol | 7 ------- contracts/utils/Strings.sol | 3 +-- test/utils/Bytes.t.sol | 12 ------------ 3 files changed, 1 insertion(+), 21 deletions(-) delete mode 100644 test/utils/Bytes.t.sol diff --git a/contracts/utils/Bytes.sol b/contracts/utils/Bytes.sol index f8c3fb2ebfa..1234b845513 100644 --- a/contracts/utils/Bytes.sol +++ b/contracts/utils/Bytes.sol @@ -99,13 +99,6 @@ library Bytes { return result; } - /** - * @dev Returns true if the two byte buffers are equal. - */ - function equal(bytes memory a, bytes memory b) internal pure returns (bool) { - return a.length == b.length && keccak256(a) == keccak256(b); - } - /** * @dev Reads a bytes32 from a bytes array without bounds checking. * diff --git a/contracts/utils/Strings.sol b/contracts/utils/Strings.sol index a865bfbc785..4cc597646f2 100644 --- a/contracts/utils/Strings.sol +++ b/contracts/utils/Strings.sol @@ -6,7 +6,6 @@ pragma solidity ^0.8.20; import {Math} from "./math/Math.sol"; import {SafeCast} from "./math/SafeCast.sol"; import {SignedMath} from "./math/SignedMath.sol"; -import {Bytes} from "./Bytes.sol"; /** * @dev String operations. @@ -133,7 +132,7 @@ library Strings { * @dev Returns true if the two strings are equal. */ function equal(string memory a, string memory b) internal pure returns (bool) { - return Bytes.equal(bytes(a), bytes(b)); + return bytes(a).length == bytes(b).length && keccak256(bytes(a)) == keccak256(bytes(b)); } /** diff --git a/test/utils/Bytes.t.sol b/test/utils/Bytes.t.sol deleted file mode 100644 index 6b2d7b5cad3..00000000000 --- a/test/utils/Bytes.t.sol +++ /dev/null @@ -1,12 +0,0 @@ -// SPDX-License-Identifier: MIT - -pragma solidity ^0.8.20; - -import {Test} from "forge-std/Test.sol"; -import {Bytes} from "@openzeppelin/contracts/utils/Bytes.sol"; - -contract BytesTest is Test { - function testSymbolicEqual(bytes memory a, bytes memory b) public pure { - assertEq(Bytes.equal(a, b), Bytes.equal(a, b)); - } -} From 95907aa286018d536920104790000c5f8ac6b478 Mon Sep 17 00:00:00 2001 From: ernestognw Date: Sun, 8 Jun 2025 17:11:23 -0600 Subject: [PATCH 34/56] Update docs --- docs/modules/ROOT/pages/utilities.adoc | 49 +++++++++++++++++--------- 1 file changed, 32 insertions(+), 17 deletions(-) diff --git a/docs/modules/ROOT/pages/utilities.adoc b/docs/modules/ROOT/pages/utilities.adoc index 24c79276bc8..68358f8ec2a 100644 --- a/docs/modules/ROOT/pages/utilities.adoc +++ b/docs/modules/ROOT/pages/utilities.adoc @@ -463,37 +463,52 @@ await instance.multicall([ === Memory -The `Memory` library provides functions for advanced use cases that require granular memory management. A common use case is to avoid unnecessary memory expansion costs when iterating over a section of the code that allocates new memory. Consider the following example: +The `Memory` library provides functions for advanced use cases that require granular memory management. A common use case is to avoid unnecessary memory expansion costs when performing repeated operations that allocate memory in a loop. Consider the following example: [source,solidity] ---- -function callFoo(address target) internal { - bytes memory callData = abi.encodeWithSelector( - bytes4(keccak256("foo()")) - ) - (bool success, /* bytes memory returndata */) = target.call(callData); - require(success); +function processMultipleItems(uint256[] memory items) internal { + for (uint256 i = 0; i < items.length; i++) { + bytes memory tempData = abi.encode(items[i], block.timestamp); + // Process tempData... + } } ---- -Note the function allocates memory for both the `callData` argument and for the returndata even if it's ignored. As such, it may be desirable to reset the free memory pointer after the end of the function. +Note that each iteration allocates new memory for `tempData`, causing the memory to expand continuously. This can be optimized by resetting the memory pointer between iterations: [source,solidity] ---- -function callFoo(address target) internal { +function processMultipleItems(uint256[] memory items) internal { Memory.Pointer ptr = Memory.getFreePointer(); // Cache pointer - bytes memory callData = abi.encodeWithSelector( - bytes4(keccak256("foo()")) - ) - (bool success, /* bytes memory returndata */) = target.call(callData); - require(success); - Memory.setFreePointer(ptr); // Reset pointer + for (uint256 i = 0; i < items.length; i++) { + bytes memory tempData = abi.encode(items[i], block.timestamp); + // Process tempData... + Memory.setFreePointer(ptr); // Reset pointer for reuse + } } ---- -This way, memory is allocated to accommodate the `callData`, and the `returndata` is freed. This allows other memory operations to reuse that space, thus reducing the memory expansion costs of these operations. In particular, this allows many `callFoo` to be performed in a loop with limited memory expansion costs. +This way, memory allocated for `tempData` in each iteration is reused, significantly reducing memory expansion costs when processing many items. -IMPORTANT: By default, Solidity handles memory safely. Using this library without understanding how memory works may be dangerous. Consider thoroughly reading the Solidity documentation about the https://docs.soliditylang.org/en/v0.8.20/internals/layout_in_memory.html[memory layout] and how the language defines https://docs.soliditylang.org/en/v0.8.20/assembly.html#memory-safety[memory safety]. +==== Copying memory buffers + +The `Memory` library provides a `copy` function that allows copying data between memory locations. This is useful when you need to extract a segment of data from a larger buffer or when you want to avoid unnecessary memory allocations. The following example demonstrates how to copy a segment of data from a source buffer: + +[source,solidity] +---- +function copyDataSegment(bytes memory source, uint256 offset, uint256 length) + internal pure returns (bytes memory result) { + + result = new bytes(length); + Memory.Pointer srcPtr = Memory.addOffset(Memory.contentPointer(source), offset); + Memory.Pointer destPtr = Memory.contentPointer(result); + + Memory.copy(destPtr, srcPtr, length); +} +---- + +IMPORTANT: Manual memory management increases gas costs and prevents compiler optimizations. Only use these functions after profiling confirms they're necessary. By default, Solidity handles memory safely - using this library without understanding memory layout and safety may be dangerous. See the https://docs.soliditylang.org/en/v0.8.20/internals/layout_in_memory.html[memory layout] and https://docs.soliditylang.org/en/v0.8.20/assembly.html#memory-safety[memory safety] documentation for details. === Historical Block Hashes From 124cceee184dc01c1d50301e7ef46a686696d988 Mon Sep 17 00:00:00 2001 From: ernestognw Date: Sun, 8 Jun 2025 17:12:00 -0600 Subject: [PATCH 35/56] Nit --- docs/modules/ROOT/pages/utilities.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/modules/ROOT/pages/utilities.adoc b/docs/modules/ROOT/pages/utilities.adoc index 68358f8ec2a..6d42ddc914d 100644 --- a/docs/modules/ROOT/pages/utilities.adoc +++ b/docs/modules/ROOT/pages/utilities.adoc @@ -463,7 +463,7 @@ await instance.multicall([ === Memory -The `Memory` library provides functions for advanced use cases that require granular memory management. A common use case is to avoid unnecessary memory expansion costs when performing repeated operations that allocate memory in a loop. Consider the following example: +The xref:api:utils.adoc#Memory[`Memory`] library provides functions for advanced use cases that require granular memory management. A common use case is to avoid unnecessary memory expansion costs when performing repeated operations that allocate memory in a loop. Consider the following example: [source,solidity] ---- From c3237dfbaa2cbb9855179d7fc689dbec4cbaa080 Mon Sep 17 00:00:00 2001 From: ernestognw Date: Sun, 8 Jun 2025 18:07:37 -0600 Subject: [PATCH 36/56] Finish fuzz tests and FV --- test/utils/Memory.t.sol | 53 ++++++++++++++++++++++++++--------------- 1 file changed, 34 insertions(+), 19 deletions(-) diff --git a/test/utils/Memory.t.sol b/test/utils/Memory.t.sol index 8964c164523..dcdc015ea28 100644 --- a/test/utils/Memory.t.sol +++ b/test/utils/Memory.t.sol @@ -4,7 +4,6 @@ pragma solidity ^0.8.20; import {Test} from "forge-std/Test.sol"; import {Memory} from "@openzeppelin/contracts/utils/Memory.sol"; -import {Bytes} from "@openzeppelin/contracts/utils/Bytes.sol"; contract MemoryTest is Test { using Memory for *; @@ -25,28 +24,44 @@ contract MemoryTest is Test { assertEq(ptr.asBytes().contentPointer().asBytes32(), ptr.addOffset(32).asBytes32()); } - // function testCopy(bytes memory data, uint256 destSeed) public pure { - // uint256 upperPtr = data.asPointer().asUint256() + data.length; - // Memory.Pointer destPtr = bytes32(bound(destSeed, upperPtr, upperPtr + 100)).asPointer(); - // Memory.copy(data.asPointer(), destPtr, data.length + 32); - // for (uint256 i = 0; i < data.length; i++) { - // assertEq(data[i], destPtr.asBytes()[i]); - // } - // } + function testCopy(bytes memory data, uint256 destSeed) public pure { + uint256 minDestPtr = Memory.getFreePointer().asUint256(); + Memory.Pointer destPtr = bytes32(bound(destSeed, minDestPtr, minDestPtr + END_PTR)).asPointer(); + destPtr.addOffset(data.length + 32).setFreePointer(); + destPtr.copy(data.asPointer(), data.length + 32); + bytes memory copiedData = destPtr.asBytes(); + assertEq(data.length, copiedData.length); + for (uint256 i = 0; i < data.length; i++) { + assertEq(data[i], copiedData[i]); + } + } - function testExtractByte(uint256 seed, uint256 index) public pure { + function testExtractByte(uint256 seed, uint256 index, bytes32 value) public pure { index = bound(index, 0, 31); Memory.Pointer ptr = bytes32(bound(seed, START_PTR, END_PTR)).asPointer(); - assertEq(ptr.extractByte(index), bytes1(ptr.asBytes32() >> (256 - index * 8))); + + assembly ("memory-safe") { + mstore(ptr, value) + } + + bytes1 expected; + assembly ("memory-safe") { + expected := byte(index, value) + } + assertEq(ptr.extractByte(index), expected); } - // function testExtractWord(uint256 seed) public pure { - // Memory.Pointer ptr = bytes32(bound(seed, START_PTR, END_PTR)).asPointer(); - // assertEq(ptr.extractWord(), ptr.asBytes32()); - // } + function testExtractWord(uint256 seed, bytes32 value) public pure { + Memory.Pointer ptr = bytes32(bound(seed, START_PTR, END_PTR)).asPointer(); + assembly ("memory-safe") { + mstore(ptr, value) + } + assertEq(ptr.extractWord(), value); + } - // function testAddOffset(uint256 seed, uint256 offset) public pure { - // Memory.Pointer ptr = bytes32(bound(seed, START_PTR, END_PTR)).asPointer(); - // assertEq(ptr.addOffset(offset).asUint256(), ptr.asUint256() + offset); - // } + function testSymbolicAddOffset(uint256 seed, uint256 offset) public pure { + offset = bound(offset, 0, type(uint256).max - END_PTR); + Memory.Pointer ptr = bytes32(bound(seed, START_PTR, END_PTR)).asPointer(); + assertEq(ptr.addOffset(offset).asUint256(), ptr.asUint256() + offset); + } } From 27f0a9b2926df3170c208914a0c027abe5ef936d Mon Sep 17 00:00:00 2001 From: ernestognw Date: Sun, 8 Jun 2025 18:31:49 -0600 Subject: [PATCH 37/56] up --- contracts/utils/Memory.sol | 6 +++--- test/utils/Memory.t.sol | 8 ++++---- test/utils/Memory.test.js | 8 ++++---- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/contracts/utils/Memory.sol b/contracts/utils/Memory.sol index 891754f94d1..84071f4d16b 100644 --- a/contracts/utils/Memory.sol +++ b/contracts/utils/Memory.sol @@ -52,15 +52,15 @@ library Memory { * * NOTE: Will return `0x00` if `offset` is larger or equal to `32`. */ - function extractByte(Pointer ptr, uint256 offset) internal pure returns (bytes1 v) { - bytes32 word = extractWord(ptr); + function loadByte(Pointer ptr, uint256 offset) internal pure returns (bytes1 v) { + bytes32 word = load(ptr); assembly ("memory-safe") { v := byte(offset, word) } } /// @dev Extracts a `bytes32` from a `Pointer`. - function extractWord(Pointer ptr) internal pure returns (bytes32 v) { + function load(Pointer ptr) internal pure returns (bytes32 v) { assembly ("memory-safe") { v := mload(ptr) } diff --git a/test/utils/Memory.t.sol b/test/utils/Memory.t.sol index dcdc015ea28..3a663d2c95d 100644 --- a/test/utils/Memory.t.sol +++ b/test/utils/Memory.t.sol @@ -36,7 +36,7 @@ contract MemoryTest is Test { } } - function testExtractByte(uint256 seed, uint256 index, bytes32 value) public pure { + function testLoadByte(uint256 seed, uint256 index, bytes32 value) public pure { index = bound(index, 0, 31); Memory.Pointer ptr = bytes32(bound(seed, START_PTR, END_PTR)).asPointer(); @@ -48,15 +48,15 @@ contract MemoryTest is Test { assembly ("memory-safe") { expected := byte(index, value) } - assertEq(ptr.extractByte(index), expected); + assertEq(ptr.loadByte(index), expected); } - function testExtractWord(uint256 seed, bytes32 value) public pure { + function testLoad(uint256 seed, bytes32 value) public pure { Memory.Pointer ptr = bytes32(bound(seed, START_PTR, END_PTR)).asPointer(); assembly ("memory-safe") { mstore(ptr, value) } - assertEq(ptr.extractWord(), value); + assertEq(ptr.load(), value); } function testSymbolicAddOffset(uint256 seed, uint256 offset) public pure { diff --git a/test/utils/Memory.test.js b/test/utils/Memory.test.js index c6ae6ba2d76..7b675d40672 100644 --- a/test/utils/Memory.test.js +++ b/test/utils/Memory.test.js @@ -26,14 +26,14 @@ describe('Memory', function () { }); }); - it('extractWord extracts a word', async function () { + it('load extracts a word', async function () { const ptr = await this.mock.$getFreePointer(); - await expect(this.mock.$extractWord(ptr)).to.eventually.equal(ethers.toBeHex(0, 32)); + await expect(this.mock.$load(ptr)).to.eventually.equal(ethers.toBeHex(0, 32)); }); - it('extractByte extracts a byte', async function () { + it('loadByte extracts a byte', async function () { const ptr = await this.mock.$getFreePointer(); - await expect(this.mock.$extractByte(ptr, 0)).to.eventually.equal(ethers.toBeHex(0, 1)); + await expect(this.mock.$loadByte(ptr, 0)).to.eventually.equal(ethers.toBeHex(0, 1)); }); it('contentPointer', async function () { From 848fc064ee70ee5b312eea307b61fa3bb55222f4 Mon Sep 17 00:00:00 2001 From: ernestognw Date: Sun, 8 Jun 2025 18:34:32 -0600 Subject: [PATCH 38/56] up --- test/utils/LowLevelCall.test.js | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/test/utils/LowLevelCall.test.js b/test/utils/LowLevelCall.test.js index 717ace7d607..75586a05a11 100644 --- a/test/utils/LowLevelCall.test.js +++ b/test/utils/LowLevelCall.test.js @@ -140,14 +140,14 @@ describe('LowLevelCall', function () { describe('staticcallRaw', function () { it('calls the requested function and returns true', async function () { const call = this.target.interface.encodeFunctionData('mockStaticFunction'); - expect(await this.mock.$staticcallRaw(this.target, call)).to.equal(true); + await expect(this.mock.$staticcallRaw(this.target, call)).to.eventually.equal(true); }); it('calls the requested function and returns false if the subcall reverts', async function () { - const interface = new ethers.Interface(['function mockFunctionDoesNotExist()']); + const iface = new ethers.Interface(['function mockFunctionDoesNotExist()']); - const call = interface.encodeFunctionData('mockFunctionDoesNotExist'); - expect(await this.mock.$staticcallRaw(this.target, call)).to.equal(false); + const call = iface.encodeFunctionData('mockFunctionDoesNotExist'); + await expect(this.mock.$staticcallRaw(this.target, call)).to.eventually.equal(false); }); }); @@ -161,14 +161,20 @@ describe('LowLevelCall', function () { this.returnValue, ethers.ZeroHash, ]); - expect(await this.mock.$staticcallReturnBytes32(this.target, call)).to.deep.equal([true, this.returnValue]); + await expect(this.mock.$staticcallReturnBytes32(this.target, call)).to.eventually.deep.equal([ + true, + this.returnValue, + ]); }); it('calls the requested function and returns false if the subcall reverts', async function () { - const interface = new ethers.Interface(['function mockFunctionDoesNotExist()']); + const iface = new ethers.Interface(['function mockFunctionDoesNotExist()']); - const call = interface.encodeFunctionData('mockFunctionDoesNotExist'); - expect(await this.mock.$staticcallReturnBytes32(this.target, call)).to.deep.equal([false, ethers.ZeroHash]); + const call = iface.encodeFunctionData('mockFunctionDoesNotExist'); + await expect(this.mock.$staticcallReturnBytes32(this.target, call)).to.eventually.deep.equal([ + false, + ethers.ZeroHash, + ]); }); }); @@ -183,7 +189,7 @@ describe('LowLevelCall', function () { this.returnValue1, this.returnValue2, ]); - expect(await this.mock.$staticcallReturnBytes32Pair(this.target, call)).to.deep.equal([ + await expect(this.mock.$staticcallReturnBytes32Pair(this.target, call)).to.eventually.deep.equal([ true, this.returnValue1, this.returnValue2, @@ -191,10 +197,10 @@ describe('LowLevelCall', function () { }); it('calls the requested function and returns false if the subcall reverts', async function () { - const interface = new ethers.Interface(['function mockFunctionDoesNotExist()']); + const iface = new ethers.Interface(['function mockFunctionDoesNotExist()']); - const call = interface.encodeFunctionData('mockFunctionDoesNotExist'); - expect(await this.mock.$staticcallReturnBytes32Pair(this.target, call)).to.deep.equal([ + const call = iface.encodeFunctionData('mockFunctionDoesNotExist'); + await expect(this.mock.$staticcallReturnBytes32Pair(this.target, call)).to.eventually.deep.equal([ false, ethers.ZeroHash, ethers.ZeroHash, From a213518543e2cf444d8f2b7ceb1fbdae530fce67 Mon Sep 17 00:00:00 2001 From: ernestognw Date: Sun, 8 Jun 2025 18:41:34 -0600 Subject: [PATCH 39/56] up --- docs/modules/ROOT/pages/utilities.adoc | 31 +++++++++++--------------- test/utils/LowLevelCall.test.js | 14 ++++-------- 2 files changed, 17 insertions(+), 28 deletions(-) diff --git a/docs/modules/ROOT/pages/utilities.adoc b/docs/modules/ROOT/pages/utilities.adoc index 704df552f4b..e748aa0d5f6 100644 --- a/docs/modules/ROOT/pages/utilities.adoc +++ b/docs/modules/ROOT/pages/utilities.adoc @@ -461,46 +461,41 @@ await instance.multicall([ ]); ---- -=== LowLevelCall +=== Low-level Calls -The `LowLevelCall` library contains a set of functions to perform external calls with low-level assembly, allowing them to deal with the callee's `returndata` in different ways. This is especially useful to make a call in a way that is safe against return bombing (i.e. the callee allocates too much memory using a long returndata). +The xref:api:utils.adoc#LowLevelCall[`LowLevelCall`] library provides low-level external calls with fixed-size return data handling, protecting against return bombing attacks where callees allocate excessive memory. -The functions in the library efficiently allocates a fixed sized of the `returndata` up to 64 bytes. You can either ignore the returned data, or get 1 or 2 `bytes32` values. +The library efficiently handles return data up to 64 bytes, allowing you to ignore it entirely or extract 1-2 `bytes32` values: [source,solidity] ---- using LowLevelCall for address; -function _foo(address target, bytes memory data) internal { +function example(address target, bytes memory data) internal { bool success; - bytes32 returnValue1; - bytes32 returnValue2; + bytes32 result1; + bytes32 result2; // Ignore return data success = target.callRaw(data); - // Copy only 32 bytes from return data - (success, returnValue1) = target.callReturnBytes32(data); + // Extract single 32-byte value + (success, result1) = target.callReturnBytes32(data); - // Copy two (32 bytes) EVM words from returndata - (success, returnValue1, returnValue2) = target.callReturnBytes32Pair(data); + // Extract two 32-byte values + (success, result1, result2) = target.callReturnBytes32Pair(data); } ---- -There are cases where you would like to check the size of the returned data, either to make sure it fits the expected size, or to check it before loading it to memory. In those case, the library also includes a function to separately check the return data size: - +You can also check return data size before processing: [source,solidity] ---- -using LowLevelCall for address; - -function _foo(address target, bytes memory data) internal returns (bool returnBool) { +function checkReturnSize(address target, bytes memory data) internal returns (bool) { if (!target.callRaw(data)) { - // Unsuccessful call return false; } - - // As long as the contract returned data, its content doesn't matter + return LowLevelCall.returnDataSize() >= 32; } ---- diff --git a/test/utils/LowLevelCall.test.js b/test/utils/LowLevelCall.test.js index 75586a05a11..5d44c6150a0 100644 --- a/test/utils/LowLevelCall.test.js +++ b/test/utils/LowLevelCall.test.js @@ -161,20 +161,14 @@ describe('LowLevelCall', function () { this.returnValue, ethers.ZeroHash, ]); - await expect(this.mock.$staticcallReturnBytes32(this.target, call)).to.eventually.deep.equal([ - true, - this.returnValue, - ]); + expect(await this.mock.$staticcallReturnBytes32(this.target, call)).to.deep.equal([true, this.returnValue]); }); it('calls the requested function and returns false if the subcall reverts', async function () { const iface = new ethers.Interface(['function mockFunctionDoesNotExist()']); const call = iface.encodeFunctionData('mockFunctionDoesNotExist'); - await expect(this.mock.$staticcallReturnBytes32(this.target, call)).to.eventually.deep.equal([ - false, - ethers.ZeroHash, - ]); + expect(await this.mock.$staticcallReturnBytes32(this.target, call)).to.deep.equal([false, ethers.ZeroHash]); }); }); @@ -189,7 +183,7 @@ describe('LowLevelCall', function () { this.returnValue1, this.returnValue2, ]); - await expect(this.mock.$staticcallReturnBytes32Pair(this.target, call)).to.eventually.deep.equal([ + expect(await this.mock.$staticcallReturnBytes32Pair(this.target, call)).to.deep.equal([ true, this.returnValue1, this.returnValue2, @@ -200,7 +194,7 @@ describe('LowLevelCall', function () { const iface = new ethers.Interface(['function mockFunctionDoesNotExist()']); const call = iface.encodeFunctionData('mockFunctionDoesNotExist'); - await expect(this.mock.$staticcallReturnBytes32Pair(this.target, call)).to.eventually.deep.equal([ + expect(await this.mock.$staticcallReturnBytes32Pair(this.target, call)).to.deep.equal([ false, ethers.ZeroHash, ethers.ZeroHash, From 4182f3256eeb7471b8e6ddb6302c6701c589cf99 Mon Sep 17 00:00:00 2001 From: ernestognw Date: Sun, 8 Jun 2025 18:55:17 -0600 Subject: [PATCH 40/56] Use LowLevelCall more --- contracts/account/Account.sol | 4 ++-- contracts/account/extensions/AccountERC7579.sol | 8 +++++--- contracts/proxy/transparent/ProxyAdmin.sol | 7 ++++++- contracts/token/ERC20/extensions/ERC4626.sol | 8 +++++--- 4 files changed, 18 insertions(+), 9 deletions(-) diff --git a/contracts/account/Account.sol b/contracts/account/Account.sol index 687d5d9902e..32d3ddb2cd7 100644 --- a/contracts/account/Account.sol +++ b/contracts/account/Account.sol @@ -5,6 +5,7 @@ pragma solidity ^0.8.20; import {PackedUserOperation, IAccount, IEntryPoint} from "../interfaces/draft-IERC4337.sol"; import {ERC4337Utils} from "./utils/draft-ERC4337Utils.sol"; import {AbstractSigner} from "../utils/cryptography/signers/AbstractSigner.sol"; +import {LowLevelCall} from "../utils/LowLevelCall.sol"; /** * @dev A simple ERC4337 account implementation. This base implementation only includes the minimal logic to process @@ -112,8 +113,7 @@ abstract contract Account is AbstractSigner, IAccount { */ function _payPrefund(uint256 missingAccountFunds) internal virtual { if (missingAccountFunds > 0) { - (bool success, ) = payable(msg.sender).call{value: missingAccountFunds}(""); - success; // Silence warning. The entrypoint should validate the result. + LowLevelCall.callRaw(msg.sender, "", missingAccountFunds); // The entrypoint should validate the result. } } diff --git a/contracts/account/extensions/AccountERC7579.sol b/contracts/account/extensions/AccountERC7579.sol index e554bb96b0f..41c5b9e7a45 100644 --- a/contracts/account/extensions/AccountERC7579.sol +++ b/contracts/account/extensions/AccountERC7579.sol @@ -7,6 +7,8 @@ import {IERC1271} from "../../interfaces/IERC1271.sol"; import {IERC7579Module, IERC7579Validator, IERC7579Execution, IERC7579AccountConfig, IERC7579ModuleConfig, MODULE_TYPE_VALIDATOR, MODULE_TYPE_EXECUTOR, MODULE_TYPE_FALLBACK} from "../../interfaces/draft-IERC7579.sol"; import {ERC7579Utils, Mode, CallType, ExecType} from "../../account/utils/draft-ERC7579Utils.sol"; import {EnumerableSet} from "../../utils/structs/EnumerableSet.sol"; +import {LowLevelCall} from "../../utils/LowLevelCall.sol"; +import {Memory} from "../../utils/Memory.sol"; import {Bytes} from "../../utils/Bytes.sol"; import {Packing} from "../../utils/Packing.sol"; import {Address} from "../../utils/Address.sol"; @@ -50,6 +52,8 @@ abstract contract AccountERC7579 is Account, IERC1271, IERC7579Execution, IERC75 using ERC7579Utils for *; using EnumerableSet for *; using Packing for bytes32; + using LowLevelCall for *; + using Memory for *; EnumerableSet.AddressSet private _validators; EnumerableSet.AddressSet private _executors; @@ -311,9 +315,7 @@ abstract contract AccountERC7579 is Account, IERC1271, IERC7579Execution, IERC75 if (success) return returndata; - assembly ("memory-safe") { - revert(add(returndata, 0x20), mload(returndata)) - } + returndata.asPointer().addOffset(0x20).asBytes().bubbleRevert(); } /// @dev Returns the fallback handler for the given selector. Returns `address(0)` if not installed. diff --git a/contracts/proxy/transparent/ProxyAdmin.sol b/contracts/proxy/transparent/ProxyAdmin.sol index eefd49a80e6..cc41b8c27fd 100644 --- a/contracts/proxy/transparent/ProxyAdmin.sol +++ b/contracts/proxy/transparent/ProxyAdmin.sol @@ -5,6 +5,7 @@ pragma solidity ^0.8.22; import {ITransparentUpgradeableProxy} from "./TransparentUpgradeableProxy.sol"; import {Ownable} from "../../access/Ownable.sol"; +import {LowLevelCall} from "../../utils/LowLevelCall.sol"; /** * @dev This is an auxiliary contract meant to be assigned as the admin of a {TransparentUpgradeableProxy}. For an @@ -40,6 +41,10 @@ contract ProxyAdmin is Ownable { address implementation, bytes memory data ) public payable virtual onlyOwner { - proxy.upgradeToAndCall{value: msg.value}(implementation, data); + LowLevelCall.callRaw( + address(proxy), + abi.encodeCall(ITransparentUpgradeableProxy.upgradeToAndCall, (implementation, data)), + msg.value + ); } } diff --git a/contracts/token/ERC20/extensions/ERC4626.sol b/contracts/token/ERC20/extensions/ERC4626.sol index 6e6a57c305d..59587a15855 100644 --- a/contracts/token/ERC20/extensions/ERC4626.sol +++ b/contracts/token/ERC20/extensions/ERC4626.sol @@ -6,6 +6,7 @@ pragma solidity ^0.8.20; import {IERC20, IERC20Metadata, ERC20} from "../ERC20.sol"; import {SafeERC20} from "../utils/SafeERC20.sol"; import {IERC4626} from "../../../interfaces/IERC4626.sol"; +import {LowLevelCall} from "../../../utils/LowLevelCall.sol"; import {Math} from "../../../utils/math/Math.sol"; /** @@ -84,11 +85,12 @@ abstract contract ERC4626 is ERC20, IERC4626 { * @dev Attempts to fetch the asset decimals. A return value of false indicates that the attempt failed in some way. */ function _tryGetAssetDecimals(IERC20 asset_) private view returns (bool ok, uint8 assetDecimals) { - (bool success, bytes memory encodedDecimals) = address(asset_).staticcall( + (bool success, bytes32 encodedDecimals) = LowLevelCall.staticcallReturnBytes32( + address(asset_), abi.encodeCall(IERC20Metadata.decimals, ()) ); - if (success && encodedDecimals.length >= 32) { - uint256 returnedDecimals = abi.decode(encodedDecimals, (uint256)); + if (success && LowLevelCall.returnDataSize() >= 32) { + uint256 returnedDecimals = uint256(encodedDecimals); if (returnedDecimals <= type(uint8).max) { return (true, uint8(returnedDecimals)); } From ad16f66b40ab40b977b824a47316a0ae430b4d97 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 9 Jul 2025 22:56:51 +0200 Subject: [PATCH 41/56] Update following team discussion --- contracts/account/Account.sol | 2 +- .../extensions/draft-AccountERC7579.sol | 12 +- contracts/proxy/transparent/ProxyAdmin.sol | 6 +- contracts/token/ERC20/extensions/ERC4626.sol | 6 +- contracts/token/ERC20/utils/SafeERC20.sol | 16 ++- contracts/utils/Address.sol | 50 ++++++-- contracts/utils/LowLevelCall.sol | 117 +++++++----------- contracts/utils/Memory.sol | 72 ----------- .../utils/cryptography/SignatureChecker.sol | 2 +- test/utils/Memory.t.sol | 46 ------- test/utils/Memory.test.js | 63 ---------- 11 files changed, 105 insertions(+), 287 deletions(-) diff --git a/contracts/account/Account.sol b/contracts/account/Account.sol index 32d3ddb2cd7..6079ac39e3c 100644 --- a/contracts/account/Account.sol +++ b/contracts/account/Account.sol @@ -113,7 +113,7 @@ abstract contract Account is AbstractSigner, IAccount { */ function _payPrefund(uint256 missingAccountFunds) internal virtual { if (missingAccountFunds > 0) { - LowLevelCall.callRaw(msg.sender, "", missingAccountFunds); // The entrypoint should validate the result. + LowLevelCall.callNoReturn(msg.sender, missingAccountFunds, ""); // The entrypoint should validate the result. } } diff --git a/contracts/account/extensions/draft-AccountERC7579.sol b/contracts/account/extensions/draft-AccountERC7579.sol index c71197fffbd..f96260658ce 100644 --- a/contracts/account/extensions/draft-AccountERC7579.sol +++ b/contracts/account/extensions/draft-AccountERC7579.sol @@ -17,10 +17,8 @@ import { import {ERC7579Utils, Mode, CallType, ExecType} from "../../account/utils/draft-ERC7579Utils.sol"; import {EnumerableSet} from "../../utils/structs/EnumerableSet.sol"; import {LowLevelCall} from "../../utils/LowLevelCall.sol"; -import {Memory} from "../../utils/Memory.sol"; import {Bytes} from "../../utils/Bytes.sol"; import {Packing} from "../../utils/Packing.sol"; -import {Address} from "../../utils/Address.sol"; import {Calldata} from "../../utils/Calldata.sol"; import {Account} from "../Account.sol"; @@ -62,7 +60,6 @@ abstract contract AccountERC7579 is Account, IERC1271, IERC7579Execution, IERC75 using EnumerableSet for *; using Packing for bytes32; using LowLevelCall for *; - using Memory for *; EnumerableSet.AddressSet private _validators; EnumerableSet.AddressSet private _executors; @@ -321,10 +318,11 @@ abstract contract AccountERC7579 is Account, IERC1271, IERC7579Execution, IERC75 (bool success, bytes memory returndata) = handler.call{value: msg.value}( abi.encodePacked(msg.data, msg.sender) ); - - if (success) return returndata; - - returndata.asPointer().addOffset(0x20).asBytes().bubbleRevert(); + if (success) { + return returndata; + } else { + LowLevelCall.bubbleRevert(returndata); + } } /// @dev Returns the fallback handler for the given selector. Returns `address(0)` if not installed. diff --git a/contracts/proxy/transparent/ProxyAdmin.sol b/contracts/proxy/transparent/ProxyAdmin.sol index cc41b8c27fd..46b9b62cbc5 100644 --- a/contracts/proxy/transparent/ProxyAdmin.sol +++ b/contracts/proxy/transparent/ProxyAdmin.sol @@ -41,10 +41,10 @@ contract ProxyAdmin is Ownable { address implementation, bytes memory data ) public payable virtual onlyOwner { - LowLevelCall.callRaw( + LowLevelCall.callNoReturn( address(proxy), - abi.encodeCall(ITransparentUpgradeableProxy.upgradeToAndCall, (implementation, data)), - msg.value + msg.value, + abi.encodeCall(ITransparentUpgradeableProxy.upgradeToAndCall, (implementation, data)) ); } } diff --git a/contracts/token/ERC20/extensions/ERC4626.sol b/contracts/token/ERC20/extensions/ERC4626.sol index 59587a15855..393c095cc1d 100644 --- a/contracts/token/ERC20/extensions/ERC4626.sol +++ b/contracts/token/ERC20/extensions/ERC4626.sol @@ -7,6 +7,7 @@ import {IERC20, IERC20Metadata, ERC20} from "../ERC20.sol"; import {SafeERC20} from "../utils/SafeERC20.sol"; import {IERC4626} from "../../../interfaces/IERC4626.sol"; import {LowLevelCall} from "../../../utils/LowLevelCall.sol"; +import {Memory} from "../../../utils/Memory.sol"; import {Math} from "../../../utils/math/Math.sol"; /** @@ -85,16 +86,19 @@ abstract contract ERC4626 is ERC20, IERC4626 { * @dev Attempts to fetch the asset decimals. A return value of false indicates that the attempt failed in some way. */ function _tryGetAssetDecimals(IERC20 asset_) private view returns (bool ok, uint8 assetDecimals) { - (bool success, bytes32 encodedDecimals) = LowLevelCall.staticcallReturnBytes32( + Memory.Pointer ptr = Memory.getFreePointer(); + (bool success, bytes32 encodedDecimals, ) = LowLevelCall.staticcallReturn64Bytes( address(asset_), abi.encodeCall(IERC20Metadata.decimals, ()) ); if (success && LowLevelCall.returnDataSize() >= 32) { uint256 returnedDecimals = uint256(encodedDecimals); if (returnedDecimals <= type(uint8).max) { + Memory.setFreePointer(ptr); return (true, uint8(returnedDecimals)); } } + Memory.setFreePointer(ptr); return (false, 0); } diff --git a/contracts/token/ERC20/utils/SafeERC20.sol b/contracts/token/ERC20/utils/SafeERC20.sol index 2de490d4c18..8699c8ad1ee 100644 --- a/contracts/token/ERC20/utils/SafeERC20.sol +++ b/contracts/token/ERC20/utils/SafeERC20.sol @@ -179,14 +179,11 @@ library SafeERC20 { * This is a variant of {_callOptionalReturnBool} that reverts if call fails to meet the requirements. */ function _callOptionalReturn(IERC20 token, bytes memory data) private { - (bool success, bytes32 returnValue) = LowLevelCall.callReturnBytes32(address(token), data); - uint256 returnSize = LowLevelCall.returnDataSize(); + (bool success, bytes32 returnValue, ) = LowLevelCall.callReturn64Bytes(address(token), data); if (!success) { - LowLevelCall.bubbleRevert(data); - } - - if (returnSize == 0 ? address(token).code.length == 0 : uint256(returnValue) != 1) { + LowLevelCall.bubbleRevert(); + } else if (LowLevelCall.returnDataSize() == 0 ? address(token).code.length == 0 : uint256(returnValue) != 1) { revert SafeERC20FailedOperation(address(token)); } } @@ -200,8 +197,9 @@ library SafeERC20 { * This is a variant of {_callOptionalReturn} that silently catches all reverts and returns a bool instead. */ function _callOptionalReturnBool(IERC20 token, bytes memory data) private returns (bool) { - (bool success, bytes32 returnValue) = LowLevelCall.callReturnBytes32(address(token), data); - uint256 returnSize = LowLevelCall.returnDataSize(); - return success && (returnSize == 0 ? address(token).code.length > 0 : uint256(returnValue) == 1); + (bool success, bytes32 returnValue, ) = LowLevelCall.callReturn64Bytes(address(token), data); + return + success && + (LowLevelCall.returnDataSize() == 0 ? address(token).code.length > 0 : uint256(returnValue) == 1); } } diff --git a/contracts/utils/Address.sol b/contracts/utils/Address.sol index 058872f9d3d..7588833f0d2 100644 --- a/contracts/utils/Address.sol +++ b/contracts/utils/Address.sol @@ -38,11 +38,13 @@ library Address { if (address(this).balance < amount) { revert Errors.InsufficientBalance(address(this).balance, amount); } - - bool success = LowLevelCall.callRaw(recipient, "", amount); - bytes memory returndata = LowLevelCall.returnData(); - if (!success) { - _revert(returndata); + if (LowLevelCall.callNoReturn(recipient, amount, "")) { + // call succesfull, nothing to do + return; + } else if (LowLevelCall.returnDataSize() == 0) { + revert Errors.FailedCall(); + } else { + LowLevelCall.bubbleRevert(); } } @@ -81,8 +83,16 @@ library Address { if (address(this).balance < value) { revert Errors.InsufficientBalance(address(this).balance, value); } - (bool success, bytes memory returndata) = target.call{value: value}(data); - return verifyCallResultFromTarget(target, success, returndata); + bool success = LowLevelCall.callNoReturn(target, value, data); + if (success && (LowLevelCall.returnDataSize() > 0 || target.code.length > 0)) { + return LowLevelCall.returnData(); + } else if (success) { + revert AddressEmptyCode(target); + } else if (LowLevelCall.returnDataSize() > 0) { + LowLevelCall.bubbleRevert(); + } else { + revert Errors.FailedCall(); + } } /** @@ -90,8 +100,16 @@ library Address { * but performing a static call. */ function functionStaticCall(address target, bytes memory data) internal view returns (bytes memory) { - (bool success, bytes memory returndata) = target.staticcall(data); - return verifyCallResultFromTarget(target, success, returndata); + bool success = LowLevelCall.staticcallNoReturn(target, data); + if (success && (LowLevelCall.returnDataSize() > 0 || target.code.length > 0)) { + return LowLevelCall.returnData(); + } else if (success) { + revert AddressEmptyCode(target); + } else if (LowLevelCall.returnDataSize() > 0) { + LowLevelCall.bubbleRevert(); + } else { + revert Errors.FailedCall(); + } } /** @@ -99,8 +117,16 @@ library Address { * but performing a delegate call. */ function functionDelegateCall(address target, bytes memory data) internal returns (bytes memory) { - (bool success, bytes memory returndata) = target.delegatecall(data); - return verifyCallResultFromTarget(target, success, returndata); + bool success = LowLevelCall.delegatecallNoReturn(target, data); + if (success && (LowLevelCall.returnDataSize() > 0 || target.code.length > 0)) { + return LowLevelCall.returnData(); + } else if (success) { + revert AddressEmptyCode(target); + } else if (LowLevelCall.returnDataSize() > 0) { + LowLevelCall.bubbleRevert(); + } else { + revert Errors.FailedCall(); + } } /** @@ -143,7 +169,7 @@ library Address { function _revert(bytes memory returndata) private pure { // Look for revert reason and bubble it up if present if (returndata.length > 0) { - LowLevelCall.bubbleRevert(returndata.asPointer().addOffset(0x20).asBytes32()); + LowLevelCall.bubbleRevert(returndata); } else { revert Errors.FailedCall(); } diff --git a/contracts/utils/LowLevelCall.sol b/contracts/utils/LowLevelCall.sol index d51d0bfc576..7d8ed5ea1c2 100644 --- a/contracts/utils/LowLevelCall.sol +++ b/contracts/utils/LowLevelCall.sol @@ -2,9 +2,6 @@ pragma solidity ^0.8.20; -import {Errors} from "./Errors.sol"; -import {Memory} from "./Memory.sol"; - /** * @dev Library of low level call functions that implement different calling strategies to deal with the return data. * @@ -12,104 +9,85 @@ import {Memory} from "./Memory.sol"; * to use the {Address} library instead. */ library LowLevelCall { - using Memory for *; - - /// === CALL === - /// @dev Performs a Solidity function call using a low level `call` and ignoring the return data. - function callRaw(address target, bytes memory data) internal returns (bool success) { - return callRaw(target, data, 0); + function callNoReturn(address target, bytes memory data) internal returns (bool success) { + return callNoReturn(target, 0, data); } - /// @dev Same as {callRaw}, but allows to specify the value to be sent in the call. - function callRaw(address target, bytes memory data, uint256 value) internal returns (bool success) { + /// @dev Same as {callNoReturn}, but allows to specify the value to be sent in the call. + function callNoReturn(address target, uint256 value, bytes memory data) internal returns (bool success) { assembly ("memory-safe") { success := call(gas(), target, value, add(data, 0x20), mload(data), 0, 0) } } - /// @dev Performs a Solidity function call using a low level `call` and returns the first 32 bytes of the result - /// in the scratch space of memory. Useful for functions that return a single-word value. - /// - /// WARNING: Do not assume that the result is zero if `success` is false. Memory can be already allocated - /// and this function doesn't zero it out. - function callReturnBytes32(address target, bytes memory data) internal returns (bool success, bytes32 result) { - return callReturnBytes32(target, data, 0); - } - - /// @dev Same as {callReturnBytes32}, but allows to specify the value to be sent in the call. - function callReturnBytes32( - address target, - bytes memory data, - uint256 value - ) internal returns (bool success, bytes32 result) { - assembly ("memory-safe") { - success := call(gas(), target, value, add(data, 0x20), mload(data), 0, 0x20) - result := mload(0) - } - } - /// @dev Performs a Solidity function call using a low level `call` and returns the first 64 bytes of the result /// in the scratch space of memory. Useful for functions that return a tuple of single-word values. /// /// WARNING: Do not assume that the results are zero if `success` is false. Memory can be already allocated /// and this function doesn't zero it out. - function callReturnBytes32Pair( + function callReturn64Bytes( address target, bytes memory data ) internal returns (bool success, bytes32 result1, bytes32 result2) { - return callReturnBytes32Pair(target, data, 0); + return callReturn64Bytes(target, 0, data); } /// @dev Same as {callReturnBytes32Pair}, but allows to specify the value to be sent in the call. - function callReturnBytes32Pair( + function callReturn64Bytes( address target, - bytes memory data, - uint256 value + uint256 value, + bytes memory data ) internal returns (bool success, bytes32 result1, bytes32 result2) { assembly ("memory-safe") { success := call(gas(), target, value, add(data, 0x20), mload(data), 0, 0x40) - result1 := mload(0) + result1 := mload(0x00) result2 := mload(0x20) } } - /// === STATICCALL === - /// @dev Performs a Solidity function call using a low level `staticcall` and ignoring the return data. - function staticcallRaw(address target, bytes memory data) internal view returns (bool success) { + function staticcallNoReturn(address target, bytes memory data) internal view returns (bool success) { assembly ("memory-safe") { success := staticcall(gas(), target, add(data, 0x20), mload(data), 0, 0) } } - /// @dev Performs a Solidity function call using a low level `staticcall` and returns the first 32 bytes of the result - /// in the scratch space of memory. Useful for functions that return a single-word value. + /// @dev Performs a Solidity function call using a low level `staticcall` and returns the first 64 bytes of the result + /// in the scratch space of memory. Useful for functions that return a tuple of single-word values. /// - /// WARNING: Do not assume that the result is zero if `success` is false. Memory can be already allocated + /// WARNING: Do not assume that the results are zero if `success` is false. Memory can be already allocated /// and this function doesn't zero it out. - function staticcallReturnBytes32( + function staticcallReturn64Bytes( address target, bytes memory data - ) internal view returns (bool success, bytes32 result) { + ) internal view returns (bool success, bytes32 result1, bytes32 result2) { assembly ("memory-safe") { - success := staticcall(gas(), target, add(data, 0x20), mload(data), 0, 0x20) - result := mload(0) + success := staticcall(gas(), target, add(data, 0x20), mload(data), 0, 0x40) + result1 := mload(0x00) + result2 := mload(0x20) } } - /// @dev Performs a Solidity function call using a low level `staticcall` and returns the first 64 bytes of the result + /// @dev Performs a Solidity function call using a low level `delegatecall` and ignoring the return data. + function delegatecallNoReturn(address target, bytes memory data) internal returns (bool success) { + assembly ("memory-safe") { + success := delegatecall(gas(), target, add(data, 0x20), mload(data), 0, 0) + } + } + + /// @dev Performs a Solidity function call using a low level `delegatecall` and returns the first 64 bytes of the result /// in the scratch space of memory. Useful for functions that return a tuple of single-word values. /// /// WARNING: Do not assume that the results are zero if `success` is false. Memory can be already allocated /// and this function doesn't zero it out. - function staticcallReturnBytes32Pair( + function delegatecallReturn64Bytes( address target, bytes memory data - ) internal view returns (bool success, bytes32 result1, bytes32 result2) { + ) internal returns (bool success, bytes32 result1, bytes32 result2) { assembly ("memory-safe") { - success := staticcall(gas(), target, add(data, 0x20), mload(data), 0, 0x40) - result1 := mload(0) + success := delegatecall(gas(), target, add(data, 0x20), mload(data), 0, 0x40) + result1 := mload(0x00) result2 := mload(0x20) } } @@ -121,32 +99,27 @@ library LowLevelCall { } } - /// @dev Returns the return data buffer. - function returnData() internal pure returns (bytes memory data) { - uint256 size = returnDataSize(); + /// @dev Returns a buffer containing the return data from the last call. + function returnData() internal pure returns (bytes memory result) { assembly ("memory-safe") { - data := mload(0x40) - returndatacopy(data, 0, size) + result := mload(0x40) + mstore(result, returndatasize()) + returndatacopy(add(result, 0x20), 0, returndatasize()) + mstore(0x40, add(result, add(0x20, returndatasize()))) } } - /// === BUBBLE UP REVERT REASON === - - /** - * @dev Bubbles up the revert reason. This function is useful to bubble up the revert reason - * from a low level call. Developers can provide any bytes memory pointer to the revert reason - * since the function will revert regardless of the pointer value. - */ - function bubbleRevert(bytes memory ptr) internal pure { - bubbleRevert(ptr.asPointer().asBytes32()); + /// @dev Revert with the return data from the last call. + function bubbleRevert() internal pure { + assembly ("memory-safe") { + returndatacopy(0, 0, returndatasize()) + revert(0, returndatasize()) + } } - /// @dev Same as {bubbleRevert-bytes}, but for `bytes32` pointers. - function bubbleRevert(bytes32 ptr) internal pure { - uint256 size = returnDataSize(); + function bubbleRevert(bytes memory returndata) internal pure { assembly ("memory-safe") { - returndatacopy(ptr, 0, size) - revert(ptr, size) + revert(add(returndata, 0x20), mload(returndata)) } } } diff --git a/contracts/utils/Memory.sol b/contracts/utils/Memory.sol index 84071f4d16b..7348126df06 100644 --- a/contracts/utils/Memory.sol +++ b/contracts/utils/Memory.sol @@ -29,76 +29,4 @@ library Memory { mstore(0x40, ptr) } } - - /// @dev Returns a `Pointer` to the content of a `bytes` buffer. Skips the length word. - function contentPointer(bytes memory buffer) internal pure returns (Pointer) { - return addOffset(asPointer(buffer), 32); - } - - /** - * @dev Copies `length` bytes from `srcPtr` to `destPtr`. Equivalent to https://www.evm.codes/?fork=cancun#5e[`mcopy`]. - * - * WARNING: Reading or writing beyond the allocated memory bounds of either pointer - * will result in undefined behavior and potential memory corruption. - */ - function copy(Pointer destPtr, Pointer srcPtr, uint256 length) internal pure { - assembly ("memory-safe") { - mcopy(destPtr, srcPtr, length) - } - } - - /** - * @dev Extracts a `bytes1` from a `Pointer`. `offset` starts from the most significant byte. - * - * NOTE: Will return `0x00` if `offset` is larger or equal to `32`. - */ - function loadByte(Pointer ptr, uint256 offset) internal pure returns (bytes1 v) { - bytes32 word = load(ptr); - assembly ("memory-safe") { - v := byte(offset, word) - } - } - - /// @dev Extracts a `bytes32` from a `Pointer`. - function load(Pointer ptr) internal pure returns (bytes32 v) { - assembly ("memory-safe") { - v := mload(ptr) - } - } - - /// @dev Adds an offset to a `Pointer`. - function addOffset(Pointer ptr, uint256 offset) internal pure returns (Pointer) { - return asPointer(bytes32(asUint256(ptr) + offset)); - } - - /// @dev `Pointer` to `bytes32`. Expects a pointer to a properly ABI-encoded `bytes` object. - function asBytes32(Pointer ptr) internal pure returns (bytes32) { - return Pointer.unwrap(ptr); - } - - /// @dev `Pointer` to `uint256`. Expects a pointer to a properly ABI-encoded `bytes` object. - function asUint256(Pointer ptr) internal pure returns (uint256) { - return uint256(asBytes32(ptr)); - } - - /// @dev `bytes32` to `Pointer`. Expects a pointer to a properly ABI-encoded `bytes` object. - function asPointer(bytes32 value) internal pure returns (Pointer) { - return Pointer.wrap(value); - } - - /// @dev Returns a `Pointer` to the `value`'s header (i.e. includes the length word). - function asPointer(bytes memory value) internal pure returns (Pointer) { - bytes32 ptr; - assembly ("memory-safe") { - ptr := value - } - return asPointer(ptr); - } - - /// @dev `Pointer` to `bytes`. Expects a pointer to a properly ABI-encoded `bytes` object. - function asBytes(Pointer ptr) internal pure returns (bytes memory b) { - assembly ("memory-safe") { - b := ptr - } - } } diff --git a/contracts/utils/cryptography/SignatureChecker.sol b/contracts/utils/cryptography/SignatureChecker.sol index 5961720514a..0729d120053 100644 --- a/contracts/utils/cryptography/SignatureChecker.sol +++ b/contracts/utils/cryptography/SignatureChecker.sol @@ -52,7 +52,7 @@ library SignatureChecker { bytes memory signature ) internal view returns (bool) { bytes memory params = abi.encodeCall(IERC1271.isValidSignature, (hash, signature)); - (bool success, bytes32 result) = LowLevelCall.staticcallReturnBytes32(signer, params); + (bool success, bytes32 result, ) = LowLevelCall.staticcallReturn64Bytes(signer, params); return success && LowLevelCall.returnDataSize() >= 32 && result == bytes32(IERC1271.isValidSignature.selector); } diff --git a/test/utils/Memory.t.sol b/test/utils/Memory.t.sol index 3a663d2c95d..02ef900993d 100644 --- a/test/utils/Memory.t.sol +++ b/test/utils/Memory.t.sol @@ -18,50 +18,4 @@ contract MemoryTest is Test { ptr.asPointer().setFreePointer(); assertEq(Memory.getFreePointer().asBytes32(), ptr); } - - function testSymbolicContentPointer(uint256 seed) public pure { - Memory.Pointer ptr = bytes32(bound(seed, START_PTR, END_PTR)).asPointer(); - assertEq(ptr.asBytes().contentPointer().asBytes32(), ptr.addOffset(32).asBytes32()); - } - - function testCopy(bytes memory data, uint256 destSeed) public pure { - uint256 minDestPtr = Memory.getFreePointer().asUint256(); - Memory.Pointer destPtr = bytes32(bound(destSeed, minDestPtr, minDestPtr + END_PTR)).asPointer(); - destPtr.addOffset(data.length + 32).setFreePointer(); - destPtr.copy(data.asPointer(), data.length + 32); - bytes memory copiedData = destPtr.asBytes(); - assertEq(data.length, copiedData.length); - for (uint256 i = 0; i < data.length; i++) { - assertEq(data[i], copiedData[i]); - } - } - - function testLoadByte(uint256 seed, uint256 index, bytes32 value) public pure { - index = bound(index, 0, 31); - Memory.Pointer ptr = bytes32(bound(seed, START_PTR, END_PTR)).asPointer(); - - assembly ("memory-safe") { - mstore(ptr, value) - } - - bytes1 expected; - assembly ("memory-safe") { - expected := byte(index, value) - } - assertEq(ptr.loadByte(index), expected); - } - - function testLoad(uint256 seed, bytes32 value) public pure { - Memory.Pointer ptr = bytes32(bound(seed, START_PTR, END_PTR)).asPointer(); - assembly ("memory-safe") { - mstore(ptr, value) - } - assertEq(ptr.load(), value); - } - - function testSymbolicAddOffset(uint256 seed, uint256 offset) public pure { - offset = bound(offset, 0, type(uint256).max - END_PTR); - Memory.Pointer ptr = bytes32(bound(seed, START_PTR, END_PTR)).asPointer(); - assertEq(ptr.addOffset(offset).asUint256(), ptr.asUint256() + offset); - } } diff --git a/test/utils/Memory.test.js b/test/utils/Memory.test.js index 7b675d40672..c82a54a05ab 100644 --- a/test/utils/Memory.test.js +++ b/test/utils/Memory.test.js @@ -25,67 +25,4 @@ describe('Memory', function () { ); }); }); - - it('load extracts a word', async function () { - const ptr = await this.mock.$getFreePointer(); - await expect(this.mock.$load(ptr)).to.eventually.equal(ethers.toBeHex(0, 32)); - }); - - it('loadByte extracts a byte', async function () { - const ptr = await this.mock.$getFreePointer(); - await expect(this.mock.$loadByte(ptr, 0)).to.eventually.equal(ethers.toBeHex(0, 1)); - }); - - it('contentPointer', async function () { - const data = ethers.toUtf8Bytes('hello world'); - const result = await this.mock.$contentPointer(data); - expect(result).to.equal(ethers.toBeHex(0xa0, 32)); // 0x80 is the default free pointer (length) - }); - - describe('addOffset', function () { - it('addOffset', async function () { - const basePtr = ethers.toBeHex(0x80, 32); - const offset = 32; - const expectedPtr = ethers.toBeHex(0xa0, 32); - - await expect(this.mock.$addOffset(basePtr, offset)).to.eventually.equal(expectedPtr); - }); - - it('addOffsetwraps around', async function () { - const basePtr = ethers.toBeHex(0x80, 32); - const offset = 256; - const expectedPtr = ethers.toBeHex(0x180, 32); - await expect(this.mock.$addOffset(basePtr, offset)).to.eventually.equal(expectedPtr); - }); - }); - - describe('pointer conversions', function () { - it('asBytes32 / asPointer', async function () { - const ptr = ethers.toBeHex('0x1234', 32); - await expect(this.mock.$asBytes32(ptr)).to.eventually.equal(ptr); - await expect(this.mock.$asPointer(ethers.Typed.bytes32(ptr))).to.eventually.equal(ptr); - }); - - it('asBytes / asPointer', async function () { - const ptr = await this.mock.$asPointer(ethers.Typed.bytes(ethers.toUtf8Bytes('hello world'))); - expect(ptr).to.equal(ethers.toBeHex(0x80, 32)); // Default free pointer - await expect(this.mock.$asBytes(ptr)).to.eventually.equal(ethers.toBeHex(0x20, 32)); - }); - - it('asUint256', async function () { - const value = 0x1234; - const ptr = ethers.toBeHex(value, 32); - await expect(this.mock.$asUint256(ptr)).to.eventually.equal(value); - }); - }); - - describe('memory operations', function () { - it('copy', async function () { - await expect(this.mock.$copy(ethers.toBeHex(0x80, 32), ethers.toBeHex(0xc0, 32), 32)).to.not.be.reverted; - }); - - it('copy with zero length', async function () { - await expect(this.mock.$copy(ethers.toBeHex(0x80, 32), ethers.toBeHex(0xc0, 32), 0)).to.not.be.reverted; - }); - }); }); From d1d6412712764988c7b2eec9235c0095607d38f0 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 9 Jul 2025 23:04:57 +0200 Subject: [PATCH 42/56] update --- contracts/token/ERC20/extensions/ERC4626.sol | 6 +++--- contracts/token/ERC20/utils/SafeERC20.sol | 12 ++++++------ contracts/utils/Address.sol | 2 +- contracts/utils/Memory.sol | 14 ++++++++++++-- test/utils/Memory.t.sol | 6 +++--- 5 files changed, 25 insertions(+), 15 deletions(-) diff --git a/contracts/token/ERC20/extensions/ERC4626.sol b/contracts/token/ERC20/extensions/ERC4626.sol index 393c095cc1d..073ba72d96f 100644 --- a/contracts/token/ERC20/extensions/ERC4626.sol +++ b/contracts/token/ERC20/extensions/ERC4626.sol @@ -86,7 +86,7 @@ abstract contract ERC4626 is ERC20, IERC4626 { * @dev Attempts to fetch the asset decimals. A return value of false indicates that the attempt failed in some way. */ function _tryGetAssetDecimals(IERC20 asset_) private view returns (bool ok, uint8 assetDecimals) { - Memory.Pointer ptr = Memory.getFreePointer(); + Memory.Pointer ptr = Memory.getFMP(); (bool success, bytes32 encodedDecimals, ) = LowLevelCall.staticcallReturn64Bytes( address(asset_), abi.encodeCall(IERC20Metadata.decimals, ()) @@ -94,11 +94,11 @@ abstract contract ERC4626 is ERC20, IERC4626 { if (success && LowLevelCall.returnDataSize() >= 32) { uint256 returnedDecimals = uint256(encodedDecimals); if (returnedDecimals <= type(uint8).max) { - Memory.setFreePointer(ptr); + Memory.setFMP(ptr); return (true, uint8(returnedDecimals)); } } - Memory.setFreePointer(ptr); + Memory.setFMP(ptr); return (false, 0); } diff --git a/contracts/token/ERC20/utils/SafeERC20.sol b/contracts/token/ERC20/utils/SafeERC20.sol index 8699c8ad1ee..2ace77b963e 100644 --- a/contracts/token/ERC20/utils/SafeERC20.sol +++ b/contracts/token/ERC20/utils/SafeERC20.sol @@ -34,9 +34,9 @@ library SafeERC20 { * non-reverting calls are assumed to be successful. */ function safeTransfer(IERC20 token, address to, uint256 value) internal { - Memory.Pointer ptr = Memory.getFreePointer(); + Memory.Pointer ptr = Memory.getFMP(); _callOptionalReturn(token, abi.encodeCall(token.transfer, (to, value))); - Memory.setFreePointer(ptr); + Memory.setFMP(ptr); } /** @@ -44,9 +44,9 @@ library SafeERC20 { * calling contract. If `token` returns no value, non-reverting calls are assumed to be successful. */ function safeTransferFrom(IERC20 token, address from, address to, uint256 value) internal { - Memory.Pointer ptr = Memory.getFreePointer(); + Memory.Pointer ptr = Memory.getFMP(); _callOptionalReturn(token, abi.encodeCall(token.transferFrom, (from, to, value))); - Memory.setFreePointer(ptr); + Memory.setFMP(ptr); } /** @@ -106,13 +106,13 @@ library SafeERC20 { * set here. */ function forceApprove(IERC20 token, address spender, uint256 value) internal { - Memory.Pointer ptr = Memory.getFreePointer(); + Memory.Pointer ptr = Memory.getFMP(); bytes memory approvalCall = abi.encodeCall(token.approve, (spender, value)); if (!_callOptionalReturnBool(token, approvalCall)) { _callOptionalReturn(token, abi.encodeCall(token.approve, (spender, 0))); _callOptionalReturn(token, approvalCall); } - Memory.setFreePointer(ptr); + Memory.setFMP(ptr); } /** diff --git a/contracts/utils/Address.sol b/contracts/utils/Address.sol index 7588833f0d2..90a28786e4b 100644 --- a/contracts/utils/Address.sol +++ b/contracts/utils/Address.sol @@ -39,7 +39,7 @@ library Address { revert Errors.InsufficientBalance(address(this).balance, amount); } if (LowLevelCall.callNoReturn(recipient, amount, "")) { - // call succesfull, nothing to do + // call successful, nothing to do return; } else if (LowLevelCall.returnDataSize() == 0) { revert Errors.FailedCall(); diff --git a/contracts/utils/Memory.sol b/contracts/utils/Memory.sol index 7348126df06..fdd1175d603 100644 --- a/contracts/utils/Memory.sol +++ b/contracts/utils/Memory.sol @@ -15,7 +15,7 @@ library Memory { type Pointer is bytes32; /// @dev Returns a `Pointer` to the current free `Pointer`. - function getFreePointer() internal pure returns (Pointer ptr) { + function getFMP() internal pure returns (Pointer ptr) { assembly ("memory-safe") { ptr := mload(0x40) } @@ -24,9 +24,19 @@ library Memory { /// @dev Sets the free `Pointer` to a specific value. /// /// WARNING: Everything after the pointer may be overwritten. - function setFreePointer(Pointer ptr) internal pure { + function setFMP(Pointer ptr) internal pure { assembly ("memory-safe") { mstore(0x40, ptr) } } + + /// @dev `Pointer` to `bytes32`. Expects a pointer to a properly ABI-encoded `bytes` object. + function asBytes32(Pointer ptr) internal pure returns (bytes32) { + return Pointer.unwrap(ptr); + } + + /// @dev `bytes32` to `Pointer`. Expects a pointer to a properly ABI-encoded `bytes` object. + function asPointer(bytes32 value) internal pure returns (Pointer) { + return Pointer.wrap(value); + } } diff --git a/test/utils/Memory.t.sol b/test/utils/Memory.t.sol index 02ef900993d..5d553ee1523 100644 --- a/test/utils/Memory.t.sol +++ b/test/utils/Memory.t.sol @@ -13,9 +13,9 @@ contract MemoryTest is Test { // - moving the free memory pointer to far causes OOG errors uint256 constant END_PTR = type(uint24).max; - function testGetSetFreePointer(uint256 seed) public pure { + function testGetsetFreeMemoryPointer(uint256 seed) public pure { bytes32 ptr = bytes32(bound(seed, START_PTR, END_PTR)); - ptr.asPointer().setFreePointer(); - assertEq(Memory.getFreePointer().asBytes32(), ptr); + ptr.asPointer().setFMP(); + assertEq(Memory.getFMP().asBytes32(), ptr); } } From 716cd3f03528ad5f8c962f5d871ab5e82be90221 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 9 Jul 2025 23:09:07 +0200 Subject: [PATCH 43/56] update --- test/utils/Memory.test.js | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/test/utils/Memory.test.js b/test/utils/Memory.test.js index c82a54a05ab..36d7a205209 100644 --- a/test/utils/Memory.test.js +++ b/test/utils/Memory.test.js @@ -13,16 +13,27 @@ describe('Memory', function () { Object.assign(this, await loadFixture(fixture)); }); - describe('free pointer', function () { + describe('free memory pointer', function () { it('sets free memory pointer', async function () { - const ptr = ethers.toBeHex(0xa0, 32); - await expect(this.mock.$setFreePointer(ptr)).to.not.be.reverted; + const ptr = '0x00000000000000000000000000000000000000000000000000000000000000a0'; + await expect(this.mock.$setFMP(ptr)).to.not.be.reverted; }); it('gets free memory pointer', async function () { - await expect(this.mock.$getFreePointer()).to.eventually.equal( - ethers.toBeHex(0x80, 32), // Default pointer + await expect(this.mock.$getFMP()).to.eventually.equal( + // Default pointer + '0x0000000000000000000000000000000000000000000000000000000000000080', ); }); + + it('asBytes32', async function () { + const ptr = '0x0000000000000000000000000000000000000000000000000000000000001234'; + await expect(this.mock.$asBytes32(ptr)).to.eventually.equal(ptr); + }); + + it('asPointer', async function () { + const ptr = '0x0000000000000000000000000000000000000000000000000000000000001234'; + await expect(this.mock.$asPointer(ptr)).to.eventually.equal(ptr); + }); }); }); From b7ce6dd18b356acd749c5d7758862ae1364cfd3c Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 10 Jul 2025 10:14:05 +0200 Subject: [PATCH 44/56] cleanup and testing --- .../extensions/draft-AccountERC7579.sol | 10 +- contracts/mocks/CallReceiverMock.sol | 28 +- contracts/mocks/Stateless.sol | 2 +- contracts/proxy/transparent/ProxyAdmin.sol | 7 +- contracts/token/ERC20/extensions/ERC4626.sol | 6 +- contracts/token/ERC20/utils/SafeERC20.sol | 12 +- contracts/utils/Address.sol | 3 - contracts/utils/Create2.sol | 1 - contracts/utils/README.adoc | 9 +- test/utils/LowLevelCall.test.js | 408 ++++++++++-------- 10 files changed, 263 insertions(+), 223 deletions(-) diff --git a/contracts/account/extensions/draft-AccountERC7579.sol b/contracts/account/extensions/draft-AccountERC7579.sol index f96260658ce..ba024c1e197 100644 --- a/contracts/account/extensions/draft-AccountERC7579.sol +++ b/contracts/account/extensions/draft-AccountERC7579.sol @@ -59,7 +59,6 @@ abstract contract AccountERC7579 is Account, IERC1271, IERC7579Execution, IERC75 using ERC7579Utils for *; using EnumerableSet for *; using Packing for bytes32; - using LowLevelCall for *; EnumerableSet.AddressSet private _validators; EnumerableSet.AddressSet private _executors; @@ -315,13 +314,10 @@ abstract contract AccountERC7579 is Account, IERC1271, IERC7579Execution, IERC75 // From https://eips.ethereum.org/EIPS/eip-7579#fallback[ERC-7579 specifications]: // - MUST utilize ERC-2771 to add the original msg.sender to the calldata sent to the fallback handler // - MUST use call to invoke the fallback handler - (bool success, bytes memory returndata) = handler.call{value: msg.value}( - abi.encodePacked(msg.data, msg.sender) - ); - if (success) { - return returndata; + if (LowLevelCall.callNoReturn(handler, msg.value, abi.encodePacked(msg.data, msg.sender))) { + return LowLevelCall.returnData(); } else { - LowLevelCall.bubbleRevert(returndata); + LowLevelCall.bubbleRevert(); } } diff --git a/contracts/mocks/CallReceiverMock.sol b/contracts/mocks/CallReceiverMock.sol index 145f633ba4e..4f16ae24176 100644 --- a/contracts/mocks/CallReceiverMock.sol +++ b/contracts/mocks/CallReceiverMock.sol @@ -7,31 +7,37 @@ contract CallReceiverMock { event MockFunctionCalledWithArgs(uint256 a, uint256 b); event MockFunctionCalledExtra(address caller, uint256 value); + uint256 public nbCalls; uint256[] private _array; - function mockFunction() public payable returns (string memory) { + modifier countCall() { + ++nbCalls; + _; + } + + function mockFunction() public payable countCall returns (string memory) { emit MockFunctionCalled(); return "0x1234"; } - function mockFunctionEmptyReturn() public payable { + function mockFunctionEmptyReturn() public payable countCall { emit MockFunctionCalled(); } - function mockFunctionWithArgs(uint256 a, uint256 b) public payable returns (string memory) { + function mockFunctionWithArgs(uint256 a, uint256 b) public payable countCall returns (string memory) { emit MockFunctionCalledWithArgs(a, b); return "0x1234"; } - function mockFunctionWithArgsReturn(uint256 a, uint256 b) public payable returns (uint256, uint256) { + function mockFunctionWithArgsReturn(uint256 a, uint256 b) public payable countCall returns (uint256, uint256) { emit MockFunctionCalledWithArgs(a, b); return (a, b); } - function mockFunctionNonPayable() public returns (string memory) { + function mockFunctionNonPayable() public countCall returns (string memory) { emit MockFunctionCalled(); return "0x1234"; @@ -45,32 +51,32 @@ contract CallReceiverMock { return (a, b); } - function mockFunctionRevertsNoReason() public payable { + function mockFunctionRevertsNoReason() public payable countCall { revert(); } - function mockFunctionRevertsReason() public payable { + function mockFunctionRevertsReason() public payable countCall { revert("CallReceiverMock: reverting"); } - function mockFunctionThrows() public payable { + function mockFunctionThrows() public payable countCall { assert(false); } - function mockFunctionOutOfGas() public payable { + function mockFunctionOutOfGas() public payable countCall { for (uint256 i = 0; ; ++i) { _array.push(i); } } - function mockFunctionWritesStorage(bytes32 slot, bytes32 value) public returns (string memory) { + function mockFunctionWritesStorage(bytes32 slot, bytes32 value) public countCall returns (string memory) { assembly { sstore(slot, value) } return "0x1234"; } - function mockFunctionExtra() public payable { + function mockFunctionExtra() public payable countCall { emit MockFunctionCalledExtra(msg.sender, msg.value); } } diff --git a/contracts/mocks/Stateless.sol b/contracts/mocks/Stateless.sol index 0626727bec5..21c1b1e0596 100644 --- a/contracts/mocks/Stateless.sol +++ b/contracts/mocks/Stateless.sol @@ -25,7 +25,6 @@ import {EnumerableSet} from "../utils/structs/EnumerableSet.sol"; import {ERC165} from "../utils/introspection/ERC165.sol"; import {ERC165Checker} from "../utils/introspection/ERC165Checker.sol"; import {ERC721Holder} from "../token/ERC721/utils/ERC721Holder.sol"; -import {LowLevelCall} from "../utils/LowLevelCall.sol"; import {ERC1155Holder} from "../token/ERC1155/utils/ERC1155Holder.sol"; import {ERC1967Utils} from "../proxy/ERC1967/ERC1967Utils.sol"; import {ERC4337Utils} from "../account/utils/draft-ERC4337Utils.sol"; @@ -34,6 +33,7 @@ import {ERC7913P256Verifier} from "../utils/cryptography/verifiers/ERC7913P256Ve import {ERC7913RSAVerifier} from "../utils/cryptography/verifiers/ERC7913RSAVerifier.sol"; import {Heap} from "../utils/structs/Heap.sol"; import {InteroperableAddress} from "../utils/draft-InteroperableAddress.sol"; +import {LowLevelCall} from "../utils/LowLevelCall.sol"; import {Math} from "../utils/math/Math.sol"; import {Memory} from "../utils/Memory.sol"; import {MerkleProof} from "../utils/cryptography/MerkleProof.sol"; diff --git a/contracts/proxy/transparent/ProxyAdmin.sol b/contracts/proxy/transparent/ProxyAdmin.sol index 46b9b62cbc5..eefd49a80e6 100644 --- a/contracts/proxy/transparent/ProxyAdmin.sol +++ b/contracts/proxy/transparent/ProxyAdmin.sol @@ -5,7 +5,6 @@ pragma solidity ^0.8.22; import {ITransparentUpgradeableProxy} from "./TransparentUpgradeableProxy.sol"; import {Ownable} from "../../access/Ownable.sol"; -import {LowLevelCall} from "../../utils/LowLevelCall.sol"; /** * @dev This is an auxiliary contract meant to be assigned as the admin of a {TransparentUpgradeableProxy}. For an @@ -41,10 +40,6 @@ contract ProxyAdmin is Ownable { address implementation, bytes memory data ) public payable virtual onlyOwner { - LowLevelCall.callNoReturn( - address(proxy), - msg.value, - abi.encodeCall(ITransparentUpgradeableProxy.upgradeToAndCall, (implementation, data)) - ); + proxy.upgradeToAndCall{value: msg.value}(implementation, data); } } diff --git a/contracts/token/ERC20/extensions/ERC4626.sol b/contracts/token/ERC20/extensions/ERC4626.sol index 073ba72d96f..46f66952b9e 100644 --- a/contracts/token/ERC20/extensions/ERC4626.sol +++ b/contracts/token/ERC20/extensions/ERC4626.sol @@ -86,7 +86,7 @@ abstract contract ERC4626 is ERC20, IERC4626 { * @dev Attempts to fetch the asset decimals. A return value of false indicates that the attempt failed in some way. */ function _tryGetAssetDecimals(IERC20 asset_) private view returns (bool ok, uint8 assetDecimals) { - Memory.Pointer ptr = Memory.getFMP(); + Memory.Pointer ptr = Memory.getFreeMemoryPointer(); (bool success, bytes32 encodedDecimals, ) = LowLevelCall.staticcallReturn64Bytes( address(asset_), abi.encodeCall(IERC20Metadata.decimals, ()) @@ -94,11 +94,11 @@ abstract contract ERC4626 is ERC20, IERC4626 { if (success && LowLevelCall.returnDataSize() >= 32) { uint256 returnedDecimals = uint256(encodedDecimals); if (returnedDecimals <= type(uint8).max) { - Memory.setFMP(ptr); + Memory.setFreeMemoryPointer(ptr); return (true, uint8(returnedDecimals)); } } - Memory.setFMP(ptr); + Memory.setFreeMemoryPointer(ptr); return (false, 0); } diff --git a/contracts/token/ERC20/utils/SafeERC20.sol b/contracts/token/ERC20/utils/SafeERC20.sol index 2ace77b963e..b8d777f7a65 100644 --- a/contracts/token/ERC20/utils/SafeERC20.sol +++ b/contracts/token/ERC20/utils/SafeERC20.sol @@ -34,9 +34,9 @@ library SafeERC20 { * non-reverting calls are assumed to be successful. */ function safeTransfer(IERC20 token, address to, uint256 value) internal { - Memory.Pointer ptr = Memory.getFMP(); + Memory.Pointer ptr = Memory.getFreeMemoryPointer(); _callOptionalReturn(token, abi.encodeCall(token.transfer, (to, value))); - Memory.setFMP(ptr); + Memory.setFreeMemoryPointer(ptr); } /** @@ -44,9 +44,9 @@ library SafeERC20 { * calling contract. If `token` returns no value, non-reverting calls are assumed to be successful. */ function safeTransferFrom(IERC20 token, address from, address to, uint256 value) internal { - Memory.Pointer ptr = Memory.getFMP(); + Memory.Pointer ptr = Memory.getFreeMemoryPointer(); _callOptionalReturn(token, abi.encodeCall(token.transferFrom, (from, to, value))); - Memory.setFMP(ptr); + Memory.setFreeMemoryPointer(ptr); } /** @@ -106,13 +106,13 @@ library SafeERC20 { * set here. */ function forceApprove(IERC20 token, address spender, uint256 value) internal { - Memory.Pointer ptr = Memory.getFMP(); + Memory.Pointer ptr = Memory.getFreeMemoryPointer(); bytes memory approvalCall = abi.encodeCall(token.approve, (spender, value)); if (!_callOptionalReturnBool(token, approvalCall)) { _callOptionalReturn(token, abi.encodeCall(token.approve, (spender, 0))); _callOptionalReturn(token, approvalCall); } - Memory.setFMP(ptr); + Memory.setFreeMemoryPointer(ptr); } /** diff --git a/contracts/utils/Address.sol b/contracts/utils/Address.sol index 90a28786e4b..95e7fb4f8f6 100644 --- a/contracts/utils/Address.sol +++ b/contracts/utils/Address.sol @@ -5,14 +5,11 @@ pragma solidity ^0.8.20; import {Errors} from "./Errors.sol"; import {LowLevelCall} from "./LowLevelCall.sol"; -import {Memory} from "./Memory.sol"; /** * @dev Collection of functions related to the address type */ library Address { - using Memory for *; - /** * @dev There's no code at `target` (it is not a contract). */ diff --git a/contracts/utils/Create2.sol b/contracts/utils/Create2.sol index edd4a3e49aa..bca7464c832 100644 --- a/contracts/utils/Create2.sol +++ b/contracts/utils/Create2.sol @@ -5,7 +5,6 @@ pragma solidity ^0.8.20; import {Errors} from "./Errors.sol"; import {LowLevelCall} from "./LowLevelCall.sol"; -import {Memory} from "./Memory.sol"; /** * @dev Helper to make usage of the `CREATE2` EVM opcode easier and safer. diff --git a/contracts/utils/README.adoc b/contracts/utils/README.adoc index 4bd139650ab..e557d9da5f1 100644 --- a/contracts/utils/README.adoc +++ b/contracts/utils/README.adoc @@ -36,11 +36,10 @@ Miscellaneous contracts and libraries containing utility functions you can use t * {Context}: A utility for abstracting the sender and calldata in the current execution context. * {Packing}: A library for packing and unpacking multiple values into bytes32 * {Panic}: A library to revert with https://docs.soliditylang.org/en/v0.8.20/control-structures.html#panic-via-assert-and-error-via-require[Solidity panic codes]. - * {LowLevelCall}: Collection of functions to perform calls with low-level assembly. * {Comparators}: A library that contains comparator functions to use with the {Heap} library. * {CAIP2}, {CAIP10}: Libraries for formatting and parsing CAIP-2 and CAIP-10 identifiers. - * {Memory}: A utility library to manipulate memory. * {InteroperableAddress}: Library for formatting and parsing ERC-7930 interoperable addresses. + * {LowLevelCall}: Collection of functions to perform calls with low-level assembly. * {Memory}: A utility library to manipulate memory. * {Blockhash}: A library for accessing historical block hashes beyond the standard 256 block limit utilizing EIP-2935's historical blockhash functionality. * {Time}: A library that provides helpers for manipulating time-related objects, including a `Delay` type. @@ -132,18 +131,16 @@ Ethereum contracts have no native concept of an interface, so applications must {{Panic}} -{{LowLevelCall}} - {{Comparators}} {{CAIP2}} {{CAIP10}} -{{Memory}} - {{InteroperableAddress}} +{{LowLevelCall}} + {{Memory}} {{Blockhash}} diff --git a/test/utils/LowLevelCall.test.js b/test/utils/LowLevelCall.test.js index 5d44c6150a0..6e4dec3b164 100644 --- a/test/utils/LowLevelCall.test.js +++ b/test/utils/LowLevelCall.test.js @@ -2,14 +2,17 @@ const { ethers } = require('hardhat'); const { expect } = require('chai'); const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers'); +const value = ethers.parseEther('1'); +const returnValue1 = ethers.id('hello'); +const returnValue2 = ethers.id('world'); + async function fixture() { - const [recipient, other] = await ethers.getSigners(); + const [account] = await ethers.getSigners(); const mock = await ethers.deployContract('$LowLevelCall'); const target = await ethers.deployContract('CallReceiverMock'); - const targetEther = await ethers.deployContract('EtherReceiverMock'); - return { recipient, other, mock, target, targetEther, value: BigInt(1e18) }; + return { account, mock, target }; } describe('LowLevelCall', function () { @@ -17,188 +20,235 @@ describe('LowLevelCall', function () { Object.assign(this, await loadFixture(fixture)); }); - describe('callRaw', function () { - beforeEach(function () { - this.call = this.target.interface.encodeFunctionData('mockFunction'); - }); - - it('calls the requested function and returns true', async function () { - await expect(this.mock.$callRaw(this.target, this.call)) - .to.emit(this.target, 'MockFunctionCalled') - .to.emit(this.mock, 'return$callRaw_address_bytes') - .withArgs(true); - }); - - it('calls the requested function with value and returns true', async function () { - await this.other.sendTransaction({ to: this.mock, value: this.value }); - - const tx = this.mock['$callRaw(address,bytes,uint256)'](this.target, this.call, this.value); - await expect(tx).to.changeEtherBalance(this.target, this.value); - await expect(tx).to.emit(this.mock, 'return$callRaw_address_bytes_uint256').withArgs(true); - }); - - it("calls the requested function and returns false if the caller doesn't have enough balance", async function () { - const tx = this.mock['$callRaw(address,bytes,uint256)'](this.target, this.call, this.value); - await expect(tx).to.not.changeEtherBalance(this.target, this.value); - await expect(tx).to.emit(this.mock, 'return$callRaw_address_bytes_uint256').withArgs(false); - }); - - it('calls the requested function and returns false if the subcall reverts', async function () { - const call = this.target.interface.encodeFunctionData('mockFunctionRevertsNoReason'); - const tx = await this.mock.$callRaw(this.target, call); - expect(tx).to.emit(this.mock, 'return$callRaw_address_bytes').withArgs(false); - }); - }); - - describe('callReturnBytes32', function () { - beforeEach(function () { - this.returnValue = ethers.id('returnDataBytes32'); - this.call = this.target.interface.encodeFunctionData('mockFunctionWithArgsReturn', [ - this.returnValue, - ethers.ZeroHash, - ]); - }); - - it('calls the requested function and returns true', async function () { - await expect(this.mock.$callReturnBytes32(this.target, this.call)) - .to.emit(this.target, 'MockFunctionCalledWithArgs') - .withArgs(this.returnValue, ethers.ZeroHash) - .to.emit(this.mock, 'return$callReturnBytes32_address_bytes') - .withArgs(true, this.returnValue); - }); - - it('calls the requested function with value and returns true', async function () { - await this.other.sendTransaction({ to: this.mock, value: this.value }); - - const tx = this.mock['$callReturnBytes32(address,bytes,uint256)'](this.target, this.call, this.value); - await expect(tx).to.changeEtherBalance(this.target, this.value); - await expect(tx) - .to.emit(this.mock, 'return$callReturnBytes32_address_bytes_uint256') - .withArgs(true, this.returnValue); - }); - - it("calls the requested function and returns false if the caller doesn't have enough balance", async function () { - const tx = this.mock['$callReturnBytes32(address,bytes,uint256)'](this.target, this.call, this.value); - await expect(tx).to.not.changeEtherBalance(this.target, this.value); - await expect(tx) - .to.emit(this.mock, 'return$callReturnBytes32_address_bytes_uint256') - .withArgs(false, ethers.ZeroHash); - }); - - it('calls the requested function and returns false if the subcall reverts', async function () { - const call = this.target.interface.encodeFunctionData('mockFunctionRevertsNoReason'); - const tx = await this.mock.$callReturnBytes32(this.target, call); - expect(tx).to.emit(this.mock, 'return$callReturnBytes32_address_bytes').withArgs(false, ethers.ZeroHash); - }); - }); - - describe('callReturnBytes32Pair', function () { - beforeEach(function () { - this.returnValue1 = ethers.id('returnDataBytes32Pair1'); - this.returnValue2 = ethers.id('returnDataBytes32Pair2'); - this.call = this.target.interface.encodeFunctionData('mockFunctionWithArgsReturn', [ - this.returnValue1, - this.returnValue2, - ]); - }); - - it('calls the requested function and returns true', async function () { - await expect(this.mock.$callReturnBytes32Pair(this.target, this.call)) - .to.emit(this.target, 'MockFunctionCalledWithArgs') - .withArgs(this.returnValue1, this.returnValue2) - .to.emit(this.mock, 'return$callReturnBytes32Pair_address_bytes') - .withArgs(true, this.returnValue1, this.returnValue2); - }); - - it('calls the requested function with value and returns true', async function () { - await this.other.sendTransaction({ to: this.mock, value: this.value }); - - const tx = this.mock['$callReturnBytes32Pair(address,bytes,uint256)'](this.target, this.call, this.value); - await expect(tx).to.changeEtherBalance(this.target, this.value); - await expect(tx) - .to.emit(this.mock, 'return$callReturnBytes32Pair_address_bytes_uint256') - .withArgs(true, this.returnValue1, this.returnValue2); - }); - - it("calls the requested function and returns false if the caller doesn't have enough balance", async function () { - const tx = this.mock['$callReturnBytes32Pair(address,bytes,uint256)'](this.target, this.call, this.value); - await expect(tx).to.not.changeEtherBalance(this.target, this.value); - await expect(tx) - .to.emit(this.mock, 'return$callReturnBytes32Pair_address_bytes_uint256') - .withArgs(false, ethers.ZeroHash, ethers.ZeroHash); - }); - - it('calls the requested function and returns false if the subcall reverts', async function () { - const call = this.target.interface.encodeFunctionData('mockFunctionRevertsNoReason'); - const tx = await this.mock.$callReturnBytes32Pair(this.target, call); - expect(tx) - .to.emit(this.mock, 'return$callReturnBytes32Pair_address_bytes') - .withArgs(false, ethers.ZeroHash, ethers.ZeroHash); + describe('call', function () { + describe('without any return', function () { + it('calls the requested function and returns true', async function () { + await expect(this.target.nbCalls()).to.eventually.equal(0); + + await expect(this.mock.$callNoReturn(this.target, this.target.interface.encodeFunctionData('mockFunction'))) + .to.emit(this.target, 'MockFunctionCalled') + .to.emit(this.mock, 'return$callNoReturn_address_bytes') + .withArgs(true); + + await expect(this.target.nbCalls()).to.eventually.equal(1); + }); + + it('calls the requested function with value and returns true', async function () { + await this.account.sendTransaction({ to: this.mock, value }); + + await expect(this.target.nbCalls()).to.eventually.equal(0); + + const tx = this.mock.$callNoReturn( + this.target, + ethers.Typed.uint256(value), + this.target.interface.encodeFunctionData('mockFunction'), + ); + await expect(tx).to.changeEtherBalances([this.mock, this.target], [-value, value]); + await expect(tx).to.emit(this.mock, 'return$callNoReturn_address_uint256_bytes').withArgs(true); + + await expect(this.target.nbCalls()).to.eventually.equal(1); + }); + + it("calls the requested function and returns false if the caller doesn't have enough balance", async function () { + await expect(this.target.nbCalls()).to.eventually.equal(0); + + const tx = this.mock.$callNoReturn( + this.target, + ethers.Typed.uint256(value), + this.target.interface.encodeFunctionData('mockFunction'), + ); + await expect(tx).to.changeEtherBalances([this.mock, this.target], [0n, 0n]); + await expect(tx).to.emit(this.mock, 'return$callNoReturn_address_uint256_bytes').withArgs(false); + + // reverted call do not count + await expect(this.target.nbCalls()).to.eventually.equal(0); + }); + + it('calls the requested function and returns false if the subcall reverts', async function () { + await expect(this.target.nbCalls()).to.eventually.equal(0); + + const tx = this.mock.$callNoReturn( + this.target, + this.target.interface.encodeFunctionData('mockFunctionRevertsNoReason'), + ); + await expect(tx).to.emit(this.mock, 'return$callNoReturn_address_bytes').withArgs(false); + + // reverted call do not count + await expect(this.target.nbCalls()).to.eventually.equal(0); + }); + }); + + describe('with 64 bytes return in the scratch space', function () { + it('calls the requested function and returns true', async function () { + await expect(this.target.nbCalls()).to.eventually.equal(0); + + await expect( + this.mock.$callReturn64Bytes( + this.target, + this.target.interface.encodeFunctionData('mockFunctionWithArgsReturn', [returnValue1, returnValue2]), + ), + ) + .to.emit(this.target, 'MockFunctionCalledWithArgs') + .withArgs(returnValue1, returnValue2) + .to.emit(this.mock, 'return$callReturn64Bytes_address_bytes') + .withArgs(true, returnValue1, returnValue2); + + await expect(this.target.nbCalls()).to.eventually.equal(1); + }); + + it('calls the requested function with value and returns true', async function () { + await this.account.sendTransaction({ to: this.mock, value }); + + await expect(this.target.nbCalls()).to.eventually.equal(0); + + const tx = this.mock.$callReturn64Bytes( + this.target, + ethers.Typed.uint256(value), + this.target.interface.encodeFunctionData('mockFunctionWithArgsReturn', [returnValue1, returnValue2]), + ); + await expect(tx).to.changeEtherBalances([this.mock, this.target], [-value, value]); + await expect(tx) + .to.emit(this.mock, 'return$callReturn64Bytes_address_uint256_bytes') + .withArgs(true, returnValue1, returnValue2); + + await expect(this.target.nbCalls()).to.eventually.equal(1); + }); + + it("calls the requested function and returns false if the caller doesn't have enough balance", async function () { + await expect(this.target.nbCalls()).to.eventually.equal(0); + + const tx = this.mock.$callReturn64Bytes( + this.target, + ethers.Typed.uint256(value), + this.target.interface.encodeFunctionData('mockFunctionWithArgsReturn', [returnValue1, returnValue2]), + ); + await expect(tx).to.changeEtherBalances([this.mock, this.target], [0n, 0n]); + await expect(tx) + .to.emit(this.mock, 'return$callReturn64Bytes_address_uint256_bytes') + .withArgs(false, ethers.ZeroHash, ethers.ZeroHash); + + // reverted call do not count + await expect(this.target.nbCalls()).to.eventually.equal(0); + }); + + it('calls the requested function and returns false if the subcall reverts', async function () { + await expect(this.target.nbCalls()).to.eventually.equal(0); + + const tx = this.mock.$callReturn64Bytes( + this.target, + this.target.interface.encodeFunctionData('mockFunctionRevertsNoReason'), + ); + await expect(tx) + .to.emit(this.mock, 'return$callReturn64Bytes_address_bytes') + .withArgs(false, ethers.ZeroHash, ethers.ZeroHash); + + // reverted call do not count + await expect(this.target.nbCalls()).to.eventually.equal(0); + }); }); }); - describe('staticcallRaw', function () { - it('calls the requested function and returns true', async function () { - const call = this.target.interface.encodeFunctionData('mockStaticFunction'); - await expect(this.mock.$staticcallRaw(this.target, call)).to.eventually.equal(true); - }); - - it('calls the requested function and returns false if the subcall reverts', async function () { - const iface = new ethers.Interface(['function mockFunctionDoesNotExist()']); - - const call = iface.encodeFunctionData('mockFunctionDoesNotExist'); - await expect(this.mock.$staticcallRaw(this.target, call)).to.eventually.equal(false); + describe('staticcall', function () { + describe('without any return', function () { + it('calls the requested function and returns true', async function () { + await expect( + this.mock.$staticcallNoReturn(this.target, this.target.interface.encodeFunctionData('mockStaticFunction')), + ).to.eventually.equal(true); + }); + + it('calls the requested function and returns false if the subcall reverts', async function () { + await expect( + this.mock.$staticcallNoReturn( + this.target, + this.target.interface.encodeFunctionData('mockFunctionRevertsNoReason'), + ), + ).to.eventually.equal(false); + }); + }); + + describe('with 64 bytes return in the scratch space', function () { + it('calls the requested function and returns true', async function () { + await expect( + this.mock.$staticcallReturn64Bytes( + this.target, + this.target.interface.encodeFunctionData('mockStaticFunctionWithArgsReturn', [returnValue1, returnValue2]), + ), + ).to.eventually.deep.equal([true, returnValue1, returnValue2]); + }); + + it('calls the requested function and returns false if the subcall reverts', async function () { + await expect( + this.mock.$staticcallReturn64Bytes( + this.target, + this.target.interface.encodeFunctionData('mockFunctionRevertsNoReason'), + ), + ).to.eventually.deep.equal([false, ethers.ZeroHash, ethers.ZeroHash]); + }); }); }); - describe('staticcallReturnBytes32', function () { - beforeEach(function () { - this.returnValue = ethers.id('returnDataBytes32'); - }); - - it('calls the requested function and returns true', async function () { - const call = this.target.interface.encodeFunctionData('mockStaticFunctionWithArgsReturn', [ - this.returnValue, - ethers.ZeroHash, - ]); - expect(await this.mock.$staticcallReturnBytes32(this.target, call)).to.deep.equal([true, this.returnValue]); - }); - - it('calls the requested function and returns false if the subcall reverts', async function () { - const iface = new ethers.Interface(['function mockFunctionDoesNotExist()']); - - const call = iface.encodeFunctionData('mockFunctionDoesNotExist'); - expect(await this.mock.$staticcallReturnBytes32(this.target, call)).to.deep.equal([false, ethers.ZeroHash]); - }); - }); - - describe('staticcallReturnBytes32Pair', function () { - beforeEach(function () { - this.returnValue1 = ethers.id('returnDataBytes32Pair1'); - this.returnValue2 = ethers.id('returnDataBytes32Pair2'); - }); - - it('calls the requested function and returns true', async function () { - const call = this.target.interface.encodeFunctionData('mockStaticFunctionWithArgsReturn', [ - this.returnValue1, - this.returnValue2, - ]); - expect(await this.mock.$staticcallReturnBytes32Pair(this.target, call)).to.deep.equal([ - true, - this.returnValue1, - this.returnValue2, - ]); - }); - - it('calls the requested function and returns false if the subcall reverts', async function () { - const iface = new ethers.Interface(['function mockFunctionDoesNotExist()']); - - const call = iface.encodeFunctionData('mockFunctionDoesNotExist'); - expect(await this.mock.$staticcallReturnBytes32Pair(this.target, call)).to.deep.equal([ - false, - ethers.ZeroHash, - ethers.ZeroHash, - ]); + describe('delegate', function () { + describe('without any return', function () { + it('calls the requested function and returns true', async function () { + await expect(ethers.provider.getStorage(this.mock, 0)).to.eventually.equal(ethers.toBeHex(0, 32)); + + await expect( + this.mock.$delegatecallNoReturn(this.target, this.target.interface.encodeFunctionData('mockFunction')), + ) + .to.emit(this.mock, 'return$delegatecallNoReturn') + .withArgs(true); + + await expect(ethers.provider.getStorage(this.mock, 0)).to.eventually.equal(ethers.toBeHex(1, 32)); + }); + + it('calls the requested function and returns false if the subcall reverts', async function () { + await expect(ethers.provider.getStorage(this.mock, 0)).to.eventually.equal(ethers.toBeHex(0, 32)); + + await expect( + this.mock.$delegatecallNoReturn( + this.target, + this.target.interface.encodeFunctionData('mockFunctionRevertsNoReason'), + ), + ) + .to.emit(this.mock, 'return$delegatecallNoReturn') + .withArgs(false); + + // reverted call do not count + await expect(ethers.provider.getStorage(this.mock, 0)).to.eventually.equal(ethers.toBeHex(0, 32)); + }); + }); + + describe('with 64 bytes return in the scratch space', function () { + it('calls the requested function and returns true', async function () { + await expect(ethers.provider.getStorage(this.mock, 0)).to.eventually.equal(ethers.toBeHex(0, 32)); + + await expect( + this.mock.$delegatecallReturn64Bytes( + this.target, + this.target.interface.encodeFunctionData('mockFunctionWithArgsReturn', [returnValue1, returnValue2]), + ), + ) + .to.emit(this.mock, 'return$delegatecallReturn64Bytes') + .withArgs(true, returnValue1, returnValue2); + + await expect(ethers.provider.getStorage(this.mock, 0)).to.eventually.equal(ethers.toBeHex(1, 32)); + }); + + it('calls the requested function and returns false if the subcall reverts', async function () { + await expect(ethers.provider.getStorage(this.mock, 0)).to.eventually.equal(ethers.toBeHex(0, 32)); + + await expect( + this.mock.$delegatecallReturn64Bytes( + this.target, + this.target.interface.encodeFunctionData('mockFunctionRevertsNoReason'), + ), + ) + .to.emit(this.mock, 'return$delegatecallReturn64Bytes') + .withArgs(false, ethers.ZeroHash, ethers.ZeroHash); + + // reverted call do not count + await expect(ethers.provider.getStorage(this.mock, 0)).to.eventually.equal(ethers.toBeHex(0, 32)); + }); }); }); }); From 2a4cc06be89156260cae011501e97774fb78266a Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 10 Jul 2025 10:35:54 +0200 Subject: [PATCH 45/56] simplify --- contracts/token/ERC20/extensions/ERC4626.sol | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/contracts/token/ERC20/extensions/ERC4626.sol b/contracts/token/ERC20/extensions/ERC4626.sol index 46f66952b9e..9385b49aed4 100644 --- a/contracts/token/ERC20/extensions/ERC4626.sol +++ b/contracts/token/ERC20/extensions/ERC4626.sol @@ -87,19 +87,16 @@ abstract contract ERC4626 is ERC20, IERC4626 { */ function _tryGetAssetDecimals(IERC20 asset_) private view returns (bool ok, uint8 assetDecimals) { Memory.Pointer ptr = Memory.getFreeMemoryPointer(); - (bool success, bytes32 encodedDecimals, ) = LowLevelCall.staticcallReturn64Bytes( + (bool success, bytes32 returnedDecimals, ) = LowLevelCall.staticcallReturn64Bytes( address(asset_), abi.encodeCall(IERC20Metadata.decimals, ()) ); - if (success && LowLevelCall.returnDataSize() >= 32) { - uint256 returnedDecimals = uint256(encodedDecimals); - if (returnedDecimals <= type(uint8).max) { - Memory.setFreeMemoryPointer(ptr); - return (true, uint8(returnedDecimals)); - } - } Memory.setFreeMemoryPointer(ptr); - return (false, 0); + + return + (success && LowLevelCall.returnDataSize() >= 32 && uint256(returnedDecimals) <= type(uint8).max) + ? (true, uint8(uint256(returnedDecimals))) + : (false, 0); } /** From a05e964773397055c2f2571565d43a5057e7ac7c Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 10 Jul 2025 10:39:00 +0200 Subject: [PATCH 46/56] Update Create2.sol --- contracts/utils/Create2.sol | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/contracts/utils/Create2.sol b/contracts/utils/Create2.sol index bca7464c832..596dd36bc3f 100644 --- a/contracts/utils/Create2.sol +++ b/contracts/utils/Create2.sol @@ -46,8 +46,11 @@ library Create2 { addr := create2(amount, add(bytecode, 0x20), mload(bytecode), salt) } if (addr == address(0)) { - if (LowLevelCall.returnDataSize() != 0) LowLevelCall.bubbleRevert(LowLevelCall.returnData()); - else revert Errors.FailedDeployment(); + if (LowLevelCall.returnDataSize() == 0) { + revert Errors.FailedDeployment(); + } else { + LowLevelCall.bubbleRevert(); + } } } From 45e8d661e402400c4728c21ec6aeab334df7848b Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 10 Jul 2025 10:41:07 +0200 Subject: [PATCH 47/56] Update SignatureChecker.sol --- contracts/utils/cryptography/SignatureChecker.sol | 3 +++ 1 file changed, 3 insertions(+) diff --git a/contracts/utils/cryptography/SignatureChecker.sol b/contracts/utils/cryptography/SignatureChecker.sol index 0729d120053..35a9a452dcc 100644 --- a/contracts/utils/cryptography/SignatureChecker.sol +++ b/contracts/utils/cryptography/SignatureChecker.sol @@ -6,6 +6,7 @@ pragma solidity ^0.8.24; import {ECDSA} from "./ECDSA.sol"; import {IERC1271} from "../../interfaces/IERC1271.sol"; import {LowLevelCall} from "../LowLevelCall.sol"; +import {Memory} from "../Memory.sol"; import {IERC7913SignatureVerifier} from "../../interfaces/IERC7913.sol"; import {Bytes} from "../../utils/Bytes.sol"; @@ -51,8 +52,10 @@ library SignatureChecker { bytes32 hash, bytes memory signature ) internal view returns (bool) { + Memory.Pointer ptr = Memory.getFreeMemoryPointer(); bytes memory params = abi.encodeCall(IERC1271.isValidSignature, (hash, signature)); (bool success, bytes32 result, ) = LowLevelCall.staticcallReturn64Bytes(signer, params); + Memory.setFreeMemoryPointer(ptr); return success && LowLevelCall.returnDataSize() >= 32 && result == bytes32(IERC1271.isValidSignature.selector); } From 0a3b2ccd57e7f225b457e3caa8585760d553e109 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 10 Jul 2025 10:45:49 +0200 Subject: [PATCH 48/56] Update utilities.adoc --- docs/modules/ROOT/pages/utilities.adoc | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/docs/modules/ROOT/pages/utilities.adoc b/docs/modules/ROOT/pages/utilities.adoc index bf7956464fa..0c9966d29fc 100644 --- a/docs/modules/ROOT/pages/utilities.adoc +++ b/docs/modules/ROOT/pages/utilities.adoc @@ -477,13 +477,13 @@ function example(address target, bytes memory data) internal { bytes32 result2; // Ignore return data - success = target.callRaw(data); + success = target.callNoReturn(data); // Extract single 32-byte value - (success, result1) = target.callReturnBytes32(data); + (success, result1, ) = target.callReturn64Bytes(data); // Extract two 32-byte values - (success, result1, result2) = target.callReturnBytes32Pair(data); + (success, result1, result2) = target.callReturn64Bytes(data); } ---- @@ -491,12 +491,16 @@ You can also check return data size before processing: [source,solidity] ---- -function checkReturnSize(address target, bytes memory data) internal returns (bool) { - if (!target.callRaw(data)) { - return false; +function checkReturnSize(address target, bytes memory data) internal returns (uint256 value, uint256 otherValue) { + (bool success, bytes32 result1, bytes32 result2) = target.callReturn64Bytes(data); + + if (!success || LowLevelCall.returnDataSize() < 32) { + return (0, 0); + } else if (LowLevelCall.returnDataSize() < 64) { + return (uint256(result1), 0); + } else { + return (uint256(result1), uint256(result2)); } - - return LowLevelCall.returnDataSize() >= 32; } ---- From b547cf57a323484940f6525b8e881b419b046b3a Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 10 Jul 2025 12:34:11 +0200 Subject: [PATCH 49/56] fix testing logic --- contracts/mocks/CallReceiverMock.sol | 58 ++++++++++++++--------- contracts/utils/Create2.sol | 2 +- test/utils/LowLevelCall.test.js | 70 +++++++--------------------- 3 files changed, 54 insertions(+), 76 deletions(-) diff --git a/contracts/mocks/CallReceiverMock.sol b/contracts/mocks/CallReceiverMock.sol index 4f16ae24176..8d699ef5f54 100644 --- a/contracts/mocks/CallReceiverMock.sol +++ b/contracts/mocks/CallReceiverMock.sol @@ -7,37 +7,56 @@ contract CallReceiverMock { event MockFunctionCalledWithArgs(uint256 a, uint256 b); event MockFunctionCalledExtra(address caller, uint256 value); - uint256 public nbCalls; uint256[] private _array; - modifier countCall() { - ++nbCalls; - _; - } - - function mockFunction() public payable countCall returns (string memory) { + function mockFunction() public payable returns (string memory) { emit MockFunctionCalled(); + return "0x1234"; + } + function mockFunctionWritesStorage(bytes32 slot, bytes32 value) public returns (string memory) { + assembly ("memory-safe") { + sstore(slot, value) + } return "0x1234"; } - function mockFunctionEmptyReturn() public payable countCall { + function mockFunctionEmptyReturn() public payable { + emit MockFunctionCalled(); + } + + function mockFunctionEmptyReturnWritesStorage(bytes32 slot, bytes32 value) public payable { + assembly ("memory-safe") { + sstore(slot, value) + } emit MockFunctionCalled(); } - function mockFunctionWithArgs(uint256 a, uint256 b) public payable countCall returns (string memory) { + function mockFunctionWithArgs(uint256 a, uint256 b) public payable returns (string memory) { emit MockFunctionCalledWithArgs(a, b); return "0x1234"; } - function mockFunctionWithArgsReturn(uint256 a, uint256 b) public payable countCall returns (uint256, uint256) { + function mockFunctionWithArgsReturn(uint256 a, uint256 b) public payable returns (uint256, uint256) { emit MockFunctionCalledWithArgs(a, b); + return (a, b); + } + function mockFunctionWithArgsReturnWritesStorage( + bytes32 slot, + bytes32 value, + uint256 a, + uint256 b + ) public payable returns (uint256, uint256) { + assembly ("memory-safe") { + sstore(slot, value) + } + emit MockFunctionCalledWithArgs(a, b); return (a, b); } - function mockFunctionNonPayable() public countCall returns (string memory) { + function mockFunctionNonPayable() public returns (string memory) { emit MockFunctionCalled(); return "0x1234"; @@ -51,32 +70,25 @@ contract CallReceiverMock { return (a, b); } - function mockFunctionRevertsNoReason() public payable countCall { + function mockFunctionRevertsNoReason() public payable { revert(); } - function mockFunctionRevertsReason() public payable countCall { + function mockFunctionRevertsReason() public payable { revert("CallReceiverMock: reverting"); } - function mockFunctionThrows() public payable countCall { + function mockFunctionThrows() public payable { assert(false); } - function mockFunctionOutOfGas() public payable countCall { + function mockFunctionOutOfGas() public payable { for (uint256 i = 0; ; ++i) { _array.push(i); } } - function mockFunctionWritesStorage(bytes32 slot, bytes32 value) public countCall returns (string memory) { - assembly { - sstore(slot, value) - } - return "0x1234"; - } - - function mockFunctionExtra() public payable countCall { + function mockFunctionExtra() public payable { emit MockFunctionCalledExtra(msg.sender, msg.value); } } diff --git a/contracts/utils/Create2.sol b/contracts/utils/Create2.sol index 596dd36bc3f..8a42c8c6aaa 100644 --- a/contracts/utils/Create2.sol +++ b/contracts/utils/Create2.sol @@ -50,7 +50,7 @@ library Create2 { revert Errors.FailedDeployment(); } else { LowLevelCall.bubbleRevert(); - } + } } } diff --git a/test/utils/LowLevelCall.test.js b/test/utils/LowLevelCall.test.js index 6e4dec3b164..971c725c4d5 100644 --- a/test/utils/LowLevelCall.test.js +++ b/test/utils/LowLevelCall.test.js @@ -5,6 +5,8 @@ const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers'); const value = ethers.parseEther('1'); const returnValue1 = ethers.id('hello'); const returnValue2 = ethers.id('world'); +const storageSlot = ethers.id('location'); +const storageValue = ethers.id('data'); async function fixture() { const [account] = await ethers.getSigners(); @@ -23,21 +25,15 @@ describe('LowLevelCall', function () { describe('call', function () { describe('without any return', function () { it('calls the requested function and returns true', async function () { - await expect(this.target.nbCalls()).to.eventually.equal(0); - await expect(this.mock.$callNoReturn(this.target, this.target.interface.encodeFunctionData('mockFunction'))) .to.emit(this.target, 'MockFunctionCalled') .to.emit(this.mock, 'return$callNoReturn_address_bytes') .withArgs(true); - - await expect(this.target.nbCalls()).to.eventually.equal(1); }); it('calls the requested function with value and returns true', async function () { await this.account.sendTransaction({ to: this.mock, value }); - await expect(this.target.nbCalls()).to.eventually.equal(0); - const tx = this.mock.$callNoReturn( this.target, ethers.Typed.uint256(value), @@ -45,13 +41,9 @@ describe('LowLevelCall', function () { ); await expect(tx).to.changeEtherBalances([this.mock, this.target], [-value, value]); await expect(tx).to.emit(this.mock, 'return$callNoReturn_address_uint256_bytes').withArgs(true); - - await expect(this.target.nbCalls()).to.eventually.equal(1); }); it("calls the requested function and returns false if the caller doesn't have enough balance", async function () { - await expect(this.target.nbCalls()).to.eventually.equal(0); - const tx = this.mock.$callNoReturn( this.target, ethers.Typed.uint256(value), @@ -59,29 +51,19 @@ describe('LowLevelCall', function () { ); await expect(tx).to.changeEtherBalances([this.mock, this.target], [0n, 0n]); await expect(tx).to.emit(this.mock, 'return$callNoReturn_address_uint256_bytes').withArgs(false); - - // reverted call do not count - await expect(this.target.nbCalls()).to.eventually.equal(0); }); it('calls the requested function and returns false if the subcall reverts', async function () { - await expect(this.target.nbCalls()).to.eventually.equal(0); - const tx = this.mock.$callNoReturn( this.target, this.target.interface.encodeFunctionData('mockFunctionRevertsNoReason'), ); await expect(tx).to.emit(this.mock, 'return$callNoReturn_address_bytes').withArgs(false); - - // reverted call do not count - await expect(this.target.nbCalls()).to.eventually.equal(0); }); }); describe('with 64 bytes return in the scratch space', function () { it('calls the requested function and returns true', async function () { - await expect(this.target.nbCalls()).to.eventually.equal(0); - await expect( this.mock.$callReturn64Bytes( this.target, @@ -92,15 +74,11 @@ describe('LowLevelCall', function () { .withArgs(returnValue1, returnValue2) .to.emit(this.mock, 'return$callReturn64Bytes_address_bytes') .withArgs(true, returnValue1, returnValue2); - - await expect(this.target.nbCalls()).to.eventually.equal(1); }); it('calls the requested function with value and returns true', async function () { await this.account.sendTransaction({ to: this.mock, value }); - await expect(this.target.nbCalls()).to.eventually.equal(0); - const tx = this.mock.$callReturn64Bytes( this.target, ethers.Typed.uint256(value), @@ -110,13 +88,9 @@ describe('LowLevelCall', function () { await expect(tx) .to.emit(this.mock, 'return$callReturn64Bytes_address_uint256_bytes') .withArgs(true, returnValue1, returnValue2); - - await expect(this.target.nbCalls()).to.eventually.equal(1); }); it("calls the requested function and returns false if the caller doesn't have enough balance", async function () { - await expect(this.target.nbCalls()).to.eventually.equal(0); - const tx = this.mock.$callReturn64Bytes( this.target, ethers.Typed.uint256(value), @@ -126,14 +100,9 @@ describe('LowLevelCall', function () { await expect(tx) .to.emit(this.mock, 'return$callReturn64Bytes_address_uint256_bytes') .withArgs(false, ethers.ZeroHash, ethers.ZeroHash); - - // reverted call do not count - await expect(this.target.nbCalls()).to.eventually.equal(0); }); it('calls the requested function and returns false if the subcall reverts', async function () { - await expect(this.target.nbCalls()).to.eventually.equal(0); - const tx = this.mock.$callReturn64Bytes( this.target, this.target.interface.encodeFunctionData('mockFunctionRevertsNoReason'), @@ -141,9 +110,6 @@ describe('LowLevelCall', function () { await expect(tx) .to.emit(this.mock, 'return$callReturn64Bytes_address_bytes') .withArgs(false, ethers.ZeroHash, ethers.ZeroHash); - - // reverted call do not count - await expect(this.target.nbCalls()).to.eventually.equal(0); }); }); }); @@ -190,20 +156,22 @@ describe('LowLevelCall', function () { describe('delegate', function () { describe('without any return', function () { it('calls the requested function and returns true', async function () { - await expect(ethers.provider.getStorage(this.mock, 0)).to.eventually.equal(ethers.toBeHex(0, 32)); + await expect(ethers.provider.getStorage(this.mock, storageSlot)).to.eventually.equal(ethers.ZeroHash); await expect( - this.mock.$delegatecallNoReturn(this.target, this.target.interface.encodeFunctionData('mockFunction')), + this.mock.$delegatecallNoReturn( + this.target, + this.target.interface.encodeFunctionData('mockFunctionWritesStorage', [storageSlot, storageValue]), + ), ) .to.emit(this.mock, 'return$delegatecallNoReturn') .withArgs(true); - await expect(ethers.provider.getStorage(this.mock, 0)).to.eventually.equal(ethers.toBeHex(1, 32)); + // check delegate call set storage correctly + await expect(ethers.provider.getStorage(this.mock, storageSlot)).to.eventually.equal(storageValue); }); it('calls the requested function and returns false if the subcall reverts', async function () { - await expect(ethers.provider.getStorage(this.mock, 0)).to.eventually.equal(ethers.toBeHex(0, 32)); - await expect( this.mock.$delegatecallNoReturn( this.target, @@ -212,31 +180,32 @@ describe('LowLevelCall', function () { ) .to.emit(this.mock, 'return$delegatecallNoReturn') .withArgs(false); - - // reverted call do not count - await expect(ethers.provider.getStorage(this.mock, 0)).to.eventually.equal(ethers.toBeHex(0, 32)); }); }); describe('with 64 bytes return in the scratch space', function () { it('calls the requested function and returns true', async function () { - await expect(ethers.provider.getStorage(this.mock, 0)).to.eventually.equal(ethers.toBeHex(0, 32)); + await expect(ethers.provider.getStorage(this.mock, storageSlot)).to.eventually.equal(ethers.ZeroHash); await expect( this.mock.$delegatecallReturn64Bytes( this.target, - this.target.interface.encodeFunctionData('mockFunctionWithArgsReturn', [returnValue1, returnValue2]), + this.target.interface.encodeFunctionData('mockFunctionWithArgsReturnWritesStorage', [ + storageSlot, + storageValue, + returnValue1, + returnValue2, + ]), ), ) .to.emit(this.mock, 'return$delegatecallReturn64Bytes') .withArgs(true, returnValue1, returnValue2); - await expect(ethers.provider.getStorage(this.mock, 0)).to.eventually.equal(ethers.toBeHex(1, 32)); + // check delegate call set storage correctly + await expect(ethers.provider.getStorage(this.mock, storageSlot)).to.eventually.equal(storageValue); }); it('calls the requested function and returns false if the subcall reverts', async function () { - await expect(ethers.provider.getStorage(this.mock, 0)).to.eventually.equal(ethers.toBeHex(0, 32)); - await expect( this.mock.$delegatecallReturn64Bytes( this.target, @@ -245,9 +214,6 @@ describe('LowLevelCall', function () { ) .to.emit(this.mock, 'return$delegatecallReturn64Bytes') .withArgs(false, ethers.ZeroHash, ethers.ZeroHash); - - // reverted call do not count - await expect(ethers.provider.getStorage(this.mock, 0)).to.eventually.equal(ethers.toBeHex(0, 32)); }); }); }); From 51a3c5086961d6489cc1803a4601dcc5e8db6ee6 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 14 Jul 2025 23:22:14 +0200 Subject: [PATCH 50/56] cleanup --- contracts/token/ERC20/utils/SafeERC20.sol | 4 ---- contracts/utils/cryptography/SignatureChecker.sol | 2 -- 2 files changed, 6 deletions(-) diff --git a/contracts/token/ERC20/utils/SafeERC20.sol b/contracts/token/ERC20/utils/SafeERC20.sol index 24d61431964..60631d26b61 100644 --- a/contracts/token/ERC20/utils/SafeERC20.sol +++ b/contracts/token/ERC20/utils/SafeERC20.sol @@ -5,9 +5,6 @@ pragma solidity ^0.8.20; import {IERC20} from "../IERC20.sol"; import {IERC1363} from "../../../interfaces/IERC1363.sol"; -import {Address} from "../../../utils/Address.sol"; -import {LowLevelCall} from "../../../utils/LowLevelCall.sol"; -import {Memory} from "../../../utils/Memory.sol"; /** * @title SafeERC20 @@ -110,7 +107,6 @@ library SafeERC20 { if (!_safeApprove(token, spender, 0, true)) revert SafeERC20FailedOperation(address(token)); if (!_safeApprove(token, spender, value, true)) revert SafeERC20FailedOperation(address(token)); } - Memory.setFreeMemoryPointer(ptr); } /** diff --git a/contracts/utils/cryptography/SignatureChecker.sol b/contracts/utils/cryptography/SignatureChecker.sol index 653f999af4a..4e5d665cbca 100644 --- a/contracts/utils/cryptography/SignatureChecker.sol +++ b/contracts/utils/cryptography/SignatureChecker.sol @@ -5,8 +5,6 @@ pragma solidity ^0.8.24; import {ECDSA} from "./ECDSA.sol"; import {IERC1271} from "../../interfaces/IERC1271.sol"; -import {LowLevelCall} from "../LowLevelCall.sol"; -import {Memory} from "../Memory.sol"; import {IERC7913SignatureVerifier} from "../../interfaces/IERC7913.sol"; import {Bytes} from "../../utils/Bytes.sol"; From d79627b3a6f5c3248eb2d50565e41eadcb438873 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Fri, 22 Aug 2025 14:16:50 +0200 Subject: [PATCH 51/56] consistency --- contracts/utils/Address.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contracts/utils/Address.sol b/contracts/utils/Address.sol index 95e7fb4f8f6..3fcbc982845 100644 --- a/contracts/utils/Address.sol +++ b/contracts/utils/Address.sol @@ -38,10 +38,10 @@ library Address { if (LowLevelCall.callNoReturn(recipient, amount, "")) { // call successful, nothing to do return; - } else if (LowLevelCall.returnDataSize() == 0) { - revert Errors.FailedCall(); - } else { + } else if (LowLevelCall.returnDataSize() > 0) { LowLevelCall.bubbleRevert(); + } else { + revert Errors.FailedCall(); } } From 0227ab4a830331b3e19cf3fda140c3ea7fa77085 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Fri, 22 Aug 2025 14:23:08 +0200 Subject: [PATCH 52/56] memory-safety --- contracts/utils/LowLevelCall.sol | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/contracts/utils/LowLevelCall.sol b/contracts/utils/LowLevelCall.sol index 7d8ed5ea1c2..ff73cfec2b0 100644 --- a/contracts/utils/LowLevelCall.sol +++ b/contracts/utils/LowLevelCall.sol @@ -112,8 +112,9 @@ library LowLevelCall { /// @dev Revert with the return data from the last call. function bubbleRevert() internal pure { assembly ("memory-safe") { - returndatacopy(0, 0, returndatasize()) - revert(0, returndatasize()) + let fmp := mload(0x40) + returndatacopy(fmp, 0, returndatasize()) + revert(fmp, returndatasize()) } } From 6a74179b6a75d3b214439c5cec5ec5678c2b77c7 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 27 Aug 2025 15:17:59 +0200 Subject: [PATCH 53/56] deprecate + unit test for verifyCallResultFromTarget --- contracts/utils/Address.sol | 33 +++++++++------------- test/utils/Address.test.js | 55 +++++++++++++++++++++++++++++++++++-- 2 files changed, 66 insertions(+), 22 deletions(-) diff --git a/contracts/utils/Address.sol b/contracts/utils/Address.sol index 8661ce2d8ce..f1ccfcfd7ef 100644 --- a/contracts/utils/Address.sol +++ b/contracts/utils/Address.sol @@ -130,21 +130,24 @@ library Address { * @dev Tool to verify that a low level call to smart-contract was successful, and reverts if the target * was not a contract or bubbling up the revert reason (falling back to {Errors.FailedCall}) in case * of an unsuccessful call. + * + * NOTE: This function is DEPRECATED and may be remove in the next major release. */ function verifyCallResultFromTarget( address target, bool success, bytes memory returndata ) internal view returns (bytes memory) { - if (!success) { - _revert(returndata); - } else { - // only check if target is a contract if the call was successful and the return data is empty - // otherwise we already know that it was a contract - if (returndata.length == 0 && target.code.length == 0) { - revert AddressEmptyCode(target); - } + // only check if target is a contract if the call was successful and the return data is empty + // otherwise we already know that it was a contract + if (success && (returndata.length > 0 || target.code.length > 0)) { return returndata; + } else if (success) { + revert AddressEmptyCode(target); + } else if (returndata.length > 0) { + LowLevelCall.bubbleRevert(returndata); + } else { + revert Errors.FailedCall(); } } @@ -153,19 +156,9 @@ library Address { * revert reason or with a default {Errors.FailedCall} error. */ function verifyCallResult(bool success, bytes memory returndata) internal pure returns (bytes memory) { - if (!success) { - _revert(returndata); - } else { + if (success) { return returndata; - } - } - - /** - * @dev Reverts with returndata if present. Otherwise reverts with {Errors.FailedCall}. - */ - function _revert(bytes memory returndata) private pure { - // Look for revert reason and bubble it up if present - if (returndata.length > 0) { + } else if (returndata.length > 0) { LowLevelCall.bubbleRevert(returndata); } else { revert Errors.FailedCall(); diff --git a/test/utils/Address.test.js b/test/utils/Address.test.js index 8307a923e69..2335c223722 100644 --- a/test/utils/Address.test.js +++ b/test/utils/Address.test.js @@ -5,6 +5,9 @@ const { PANIC_CODES } = require('@nomicfoundation/hardhat-chai-matchers/panic'); const coder = ethers.AbiCoder.defaultAbiCoder(); +const fakeContract = { interface: ethers.Interface.from(['error SomeCustomErrorWithoutArgs()']) }; +const returndata = fakeContract.interface.encodeErrorResult('SomeCustomErrorWithoutArgs'); + async function fixture() { const [recipient, other] = await ethers.getSigners(); @@ -274,8 +277,56 @@ describe('Address', function () { describe('verifyCallResult', function () { it('returns returndata on success', async function () { - const returndata = '0x123abc'; - expect(await this.mock.$verifyCallResult(true, returndata)).to.equal(returndata); + await expect(this.mock.$verifyCallResult(true, returndata)).to.eventually.equal(returndata); + }); + + it('bubble returndata on failure', async function () { + await expect(this.mock.$verifyCallResult(false, returndata)).to.be.revertedWithCustomError( + fakeContract, + 'SomeCustomErrorWithoutArgs', + ); + }); + + it('standard error on failure without returndata', async function () { + await expect(this.mock.$verifyCallResult(false, '0x')).to.be.revertedWithCustomError(this.mock, 'FailedCall'); + }); + }); + + describe('verifyCallResultFromTarget', function () { + it('success with non-empty returndata', async function () { + await expect(this.mock.$verifyCallResultFromTarget(this.mock, true, returndata)).to.eventually.equal(returndata); + await expect(this.mock.$verifyCallResultFromTarget(this.recipient, true, returndata)).to.eventually.equal( + returndata, + ); + }); + + it('success with empty returndata', async function () { + await expect(this.mock.$verifyCallResultFromTarget(this.mock, true, '0x')).to.eventually.equal('0x'); + await expect(this.mock.$verifyCallResultFromTarget(this.recipient, true, '0x')) + .to.be.revertedWithCustomError(this.mock, 'AddressEmptyCode') + .withArgs(this.recipient); + }); + + it('failure with non-empty returndata', async function () { + await expect(this.mock.$verifyCallResultFromTarget(this.mock, false, returndata)).to.revertedWithCustomError( + fakeContract, + 'SomeCustomErrorWithoutArgs', + ); + await expect(this.mock.$verifyCallResultFromTarget(this.recipient, false, returndata)).to.revertedWithCustomError( + fakeContract, + 'SomeCustomErrorWithoutArgs', + ); + }); + + it('failure with empty returndata', async function () { + await expect(this.mock.$verifyCallResultFromTarget(this.mock, false, '0x')).to.be.revertedWithCustomError( + this.mock, + 'FailedCall', + ); + await expect(this.mock.$verifyCallResultFromTarget(this.recipient, false, '0x')).to.be.revertedWithCustomError( + this.mock, + 'FailedCall', + ); }); }); }); From ab88c0fd91501a543c98a6db71eb21d2256f6f46 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 2 Sep 2025 15:43:28 +0200 Subject: [PATCH 54/56] test return on faillure --- test/utils/LowLevelCall.test.js | 47 +++++++++++++++++++++++++++++---- 1 file changed, 42 insertions(+), 5 deletions(-) diff --git a/test/utils/LowLevelCall.test.js b/test/utils/LowLevelCall.test.js index 971c725c4d5..c0f06a45e1d 100644 --- a/test/utils/LowLevelCall.test.js +++ b/test/utils/LowLevelCall.test.js @@ -103,14 +103,34 @@ describe('LowLevelCall', function () { }); it('calls the requested function and returns false if the subcall reverts', async function () { - const tx = this.mock.$callReturn64Bytes( - this.target, - this.target.interface.encodeFunctionData('mockFunctionRevertsNoReason'), - ); - await expect(tx) + await expect( + this.mock.$callReturn64Bytes( + this.target, + this.target.interface.encodeFunctionData('mockFunctionRevertsNoReason'), + ), + ) .to.emit(this.mock, 'return$callReturn64Bytes_address_bytes') .withArgs(false, ethers.ZeroHash, ethers.ZeroHash); }); + + it('returns the first 64 bytes of the revert reason or custom error if the subcall reverts', async function () { + const encoded = ethers.Interface.from(['error Error(string)']).encodeErrorResult('Error', [ + 'CallReceiverMock: reverting', + ]); + + await expect( + this.mock.$callReturn64Bytes( + this.target, + this.target.interface.encodeFunctionData('mockFunctionRevertsReason'), + ), + ) + .to.emit(this.mock, 'return$callReturn64Bytes_address_bytes') + .withArgs( + false, + ethers.hexlify(ethers.getBytes(encoded).slice(0x00, 0x20)), + ethers.hexlify(ethers.getBytes(encoded).slice(0x20, 0x40)), + ); + }); }); }); @@ -150,6 +170,23 @@ describe('LowLevelCall', function () { ), ).to.eventually.deep.equal([false, ethers.ZeroHash, ethers.ZeroHash]); }); + + it('returns the first 64 bytes of the revert reason or custom error if the subcall reverts', async function () { + const encoded = ethers.Interface.from(['error Error(string)']).encodeErrorResult('Error', [ + 'CallReceiverMock: reverting', + ]); + + await expect( + this.mock.$staticcallReturn64Bytes( + this.target, + this.target.interface.encodeFunctionData('mockFunctionRevertsReason'), + ), + ).to.eventually.deep.equal([ + false, + ethers.hexlify(ethers.getBytes(encoded).slice(0x00, 0x20)), + ethers.hexlify(ethers.getBytes(encoded).slice(0x20, 0x40)), + ]); + }); }); }); From 2b28554c22202b06f81b64814ac3784daa6b0115 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 2 Sep 2025 15:55:32 +0200 Subject: [PATCH 55/56] use normal space --- docs/modules/ROOT/pages/utilities.adoc | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/docs/modules/ROOT/pages/utilities.adoc b/docs/modules/ROOT/pages/utilities.adoc index 0c9966d29fc..0f1b2280d5b 100644 --- a/docs/modules/ROOT/pages/utilities.adoc +++ b/docs/modules/ROOT/pages/utilities.adoc @@ -73,7 +73,7 @@ IMPORTANT: The P256 library only allows for `s` values in the lower order of the ==== RSA -RSA is a public-key cryptosystem that was popularized by corporate and governmental public key infrastructures (https://en.wikipedia.org/wiki/Public_key_infrastructure[PKIs]) and https://en.wikipedia.org/wiki/Domain_Name_System_Security_Extensions[DNSSEC]. +RSA is a public-key cryptosystem that was popularized by corporate and governmental public key infrastructures (https://en.wikipedia.org/wiki/Public_key_infrastructure[PKIs]) and https://en.wikipedia.org/wiki/Domain_Name_System_Security_Extensions[DNSSEC]. This cryptosystem consists of using a private key that's the product of 2 large prime numbers. The message is signed by applying a modular exponentiation to its hash (commonly SHA256), where both the exponent and modulus compose the public key of the signer. @@ -310,10 +310,10 @@ function push(bytes32 leaf) public /* onlyOwner */ { function _hashFn(bytes32 a, bytes32 b) internal view returns(bytes32) { // Custom hash function implementation - // Kept as an internal implementation detail to + // Kept as an internal implementation detail to // guarantee the same function is always used } ----- +---- === Using a Heap @@ -494,7 +494,7 @@ You can also check return data size before processing: function checkReturnSize(address target, bytes memory data) internal returns (uint256 value, uint256 otherValue) { (bool success, bytes32 result1, bytes32 result2) = target.callReturn64Bytes(data); - if (!success || LowLevelCall.returnDataSize() < 32) { + if (!success || LowLevelCall.returnDataSize() < 32) { return (0, 0); } else if (LowLevelCall.returnDataSize() < 64) { return (uint256(result1), 0); @@ -561,7 +561,7 @@ The xref:api:utils.adoc#Time[`Time`] library provides helpers for manipulating t One of its key features is the `Delay` type, which represents a duration that can automatically change its value at a specified point in the future while maintaining delay guarantees. For example, when reducing a delay value (e.g., from 7 days to 1 day), the change only takes effect after the difference between the old and new delay (i.e. a 6 days) or a minimum setback period, preventing an attacker who gains admin access from immediately reducing security timeouts and executing sensitive operations. This is particularly useful for governance and security mechanisms where timelock periods need to be enforced. -Consider this example for using and safely updating Delays: +Consider this example for using and safely updating Delays: [source,solidity] ---- // SPDX-License-Identifier: MIT @@ -571,7 +571,7 @@ import {Time} from "contracts/utils/types/Time.sol"; contract MyDelayedContract { using Time for *; - + Time.Delay private _delay; constructor() { @@ -582,10 +582,10 @@ contract MyDelayedContract { // Get the current `_delay` value, respecting any pending delay changes if they've taken effect uint32 currentDelay = _delay.get(); uint48 executionTime = Time.timestamp() + currentDelay; - + // ... schedule the operation at `executionTime` } - + function execute(bytes32 operationId) external { uint48 executionTime = getExecutionTime(operationId); require(executionTime > 0, "Operation not scheduled"); @@ -593,19 +593,19 @@ contract MyDelayedContract { // ... execute the operation } - + // Update the delay with `Time`'s safety mechanism function updateDelay(uint32 newDelay) external { (Time.Delay updatedDelay, uint48 effect) = _delay.withUpdate( newDelay, // The new delay value 5 days // Minimum setback if reducing the delay ); - + _delay = updatedDelay; // ... emit events } - + // Get complete delay details including pending changes function getDelayDetails() external view returns ( uint32 currentValue, // The current delay value From 0a80296f2395c316b032d15b08d1c275b1b5d9e9 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 2 Sep 2025 15:59:56 +0200 Subject: [PATCH 56/56] typo --- contracts/utils/Address.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/utils/Address.sol b/contracts/utils/Address.sol index f1ccfcfd7ef..52b5a0a7044 100644 --- a/contracts/utils/Address.sol +++ b/contracts/utils/Address.sol @@ -131,7 +131,7 @@ library Address { * was not a contract or bubbling up the revert reason (falling back to {Errors.FailedCall}) in case * of an unsuccessful call. * - * NOTE: This function is DEPRECATED and may be remove in the next major release. + * NOTE: This function is DEPRECATED and may be removed in the next major release. */ function verifyCallResultFromTarget( address target,