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. diff --git a/contracts/account/Account.sol b/contracts/account/Account.sol index 19c64d7a608..0767d5b7c9b 100644 --- a/contracts/account/Account.sol +++ b/contracts/account/Account.sol @@ -6,6 +6,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 @@ -113,8 +114,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.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 12ac9df1747..4d5a849f7d8 100644 --- a/contracts/account/extensions/draft-AccountERC7579.sol +++ b/contracts/account/extensions/draft-AccountERC7579.sol @@ -17,6 +17,7 @@ import { } 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 {Bytes} from "../../utils/Bytes.sol"; import {Packing} from "../../utils/Packing.sol"; import {Calldata} from "../../utils/Calldata.sol"; @@ -314,14 +315,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; - - assembly ("memory-safe") { - revert(add(returndata, 0x20), mload(returndata)) + if (LowLevelCall.callNoReturn(handler, msg.value, abi.encodePacked(msg.data, msg.sender))) { + return LowLevelCall.returnData(); + } else { + LowLevelCall.bubbleRevert(); } } diff --git a/contracts/mocks/CallReceiverMock.sol b/contracts/mocks/CallReceiverMock.sol index 8b5ec7adb0f..8d699ef5f54 100644 --- a/contracts/mocks/CallReceiverMock.sol +++ b/contracts/mocks/CallReceiverMock.sol @@ -11,7 +11,13 @@ contract CallReceiverMock { 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"; } @@ -19,12 +25,37 @@ contract CallReceiverMock { 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 returns (string memory) { emit MockFunctionCalledWithArgs(a, b); return "0x1234"; } + 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 returns (string memory) { emit MockFunctionCalled(); @@ -35,6 +66,10 @@ contract CallReceiverMock { return "0x1234"; } + function mockStaticFunctionWithArgsReturn(uint256 a, uint256 b) public pure returns (uint256, uint256) { + return (a, b); + } + function mockFunctionRevertsNoReason() public payable { revert(); } @@ -53,13 +88,6 @@ contract CallReceiverMock { } } - function mockFunctionWritesStorage(bytes32 slot, bytes32 value) public returns (string memory) { - assembly { - sstore(slot, value) - } - return "0x1234"; - } - function mockFunctionExtra() public payable { emit MockFunctionCalledExtra(msg.sender, msg.value); } diff --git a/contracts/mocks/Stateless.sol b/contracts/mocks/Stateless.sol index fd0cf398eb0..21c1b1e0596 100644 --- a/contracts/mocks/Stateless.sol +++ b/contracts/mocks/Stateless.sol @@ -33,7 +33,9 @@ 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"; import {MessageHashUtils} from "../utils/cryptography/MessageHashUtils.sol"; import {Nonces} from "../utils/Nonces.sol"; @@ -49,7 +51,6 @@ 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 {} diff --git a/contracts/token/ERC20/extensions/ERC4626.sol b/contracts/token/ERC20/extensions/ERC4626.sol index fd8231acf2e..adc906b1ef4 100644 --- a/contracts/token/ERC20/extensions/ERC4626.sol +++ b/contracts/token/ERC20/extensions/ERC4626.sol @@ -6,6 +6,8 @@ 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 {Memory} from "../../../utils/Memory.sol"; import {Math} from "../../../utils/math/Math.sol"; /** @@ -84,16 +86,17 @@ 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( + Memory.Pointer ptr = Memory.getFreeMemoryPointer(); + (bool success, bytes32 returnedDecimals, ) = LowLevelCall.staticcallReturn64Bytes( + address(asset_), 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); + Memory.setFreeMemoryPointer(ptr); + + return + (success && LowLevelCall.returnDataSize() >= 32 && uint256(returnedDecimals) <= type(uint8).max) + ? (true, uint8(uint256(returnedDecimals))) + : (false, 0); } /** diff --git a/contracts/utils/Address.sol b/contracts/utils/Address.sol index cb90227d502..52b5a0a7044 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 @@ -34,10 +35,13 @@ library Address { if (address(this).balance < amount) { revert Errors.InsufficientBalance(address(this).balance, amount); } - - (bool success, bytes memory returndata) = recipient.call{value: amount}(""); - if (!success) { - _revert(returndata); + if (LowLevelCall.callNoReturn(recipient, amount, "")) { + // call successful, nothing to do + return; + } else if (LowLevelCall.returnDataSize() > 0) { + LowLevelCall.bubbleRevert(); + } else { + revert Errors.FailedCall(); } } @@ -76,8 +80,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(); + } } /** @@ -85,8 +97,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(); + } } /** @@ -94,29 +114,40 @@ 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(); + } } /** * @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 removed 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(); } } @@ -125,23 +156,10 @@ 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) { - // The easiest way to bubble the revert reason is using memory via assembly - assembly ("memory-safe") { - revert(add(returndata, 0x20), mload(returndata)) - } + } else if (returndata.length > 0) { + LowLevelCall.bubbleRevert(returndata); } else { revert Errors.FailedCall(); } diff --git a/contracts/utils/Create2.sol b/contracts/utils/Create2.sol index ffd39d9a46c..8a42c8c6aaa 100644 --- a/contracts/utils/Create2.sol +++ b/contracts/utils/Create2.sol @@ -4,6 +4,7 @@ pragma solidity ^0.8.20; import {Errors} from "./Errors.sol"; +import {LowLevelCall} from "./LowLevelCall.sol"; /** * @dev Helper to make usage of the `CREATE2` EVM opcode easier and safer. @@ -43,15 +44,13 @@ 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) { + revert Errors.FailedDeployment(); + } else { + LowLevelCall.bubbleRevert(); + } } } diff --git a/contracts/utils/LowLevelCall.sol b/contracts/utils/LowLevelCall.sol new file mode 100644 index 00000000000..ff73cfec2b0 --- /dev/null +++ b/contracts/utils/LowLevelCall.sol @@ -0,0 +1,126 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +/** + * @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 { + /// @dev Performs a Solidity function call using a low level `call` and ignoring the return data. + function callNoReturn(address target, bytes memory data) internal returns (bool success) { + return callNoReturn(target, 0, data); + } + + /// @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 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 callReturn64Bytes( + address target, + bytes memory data + ) internal returns (bool success, bytes32 result1, bytes32 result2) { + return callReturn64Bytes(target, 0, data); + } + + /// @dev Same as {callReturnBytes32Pair}, but allows to specify the value to be sent in the call. + function callReturn64Bytes( + 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(0x00) + result2 := mload(0x20) + } + } + + /// @dev Performs a Solidity function call using a low level `staticcall` and ignoring the return data. + 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 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 staticcallReturn64Bytes( + 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(0x00) + result2 := mload(0x20) + } + } + + /// @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 delegatecallReturn64Bytes( + address target, + bytes memory data + ) internal returns (bool success, bytes32 result1, bytes32 result2) { + assembly ("memory-safe") { + success := delegatecall(gas(), target, add(data, 0x20), mload(data), 0, 0x40) + result1 := mload(0x00) + result2 := mload(0x20) + } + } + + /// @dev Returns the size of the return data buffer. + function returnDataSize() internal pure returns (uint256 size) { + assembly ("memory-safe") { + 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") { + result := mload(0x40) + mstore(result, returndatasize()) + returndatacopy(add(result, 0x20), 0, returndatasize()) + mstore(0x40, add(result, add(0x20, returndatasize()))) + } + } + + /// @dev Revert with the return data from the last call. + function bubbleRevert() internal pure { + assembly ("memory-safe") { + let fmp := mload(0x40) + returndatacopy(fmp, 0, returndatasize()) + revert(fmp, returndatasize()) + } + } + + function bubbleRevert(bytes memory returndata) internal pure { + assembly ("memory-safe") { + revert(add(returndata, 0x20), mload(returndata)) + } + } +} diff --git a/contracts/utils/README.adoc b/contracts/utils/README.adoc index a536e3b51f8..e557d9da5f1 100644 --- a/contracts/utils/README.adoc +++ b/contracts/utils/README.adoc @@ -38,8 +38,9 @@ Miscellaneous contracts and libraries containing utility functions you can use t * {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 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. @@ -136,10 +137,12 @@ Ethereum contracts have no native concept of an interface, so applications must {{CAIP10}} -{{Memory}} - {{InteroperableAddress}} +{{LowLevelCall}} + +{{Memory}} + {{Blockhash}} {{Time}} diff --git a/docs/modules/ROOT/pages/utilities.adoc b/docs/modules/ROOT/pages/utilities.adoc index f0114726f2f..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 @@ -461,6 +461,49 @@ await instance.multicall([ ]); ---- +=== Low-level Calls + +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 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 example(address target, bytes memory data) internal { + bool success; + bytes32 result1; + bytes32 result2; + + // Ignore return data + success = target.callNoReturn(data); + + // Extract single 32-byte value + (success, result1, ) = target.callReturn64Bytes(data); + + // Extract two 32-byte values + (success, result1, result2) = target.callReturn64Bytes(data); +} +---- + +You can also check return data size before processing: + +[source,solidity] +---- +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)); + } +} +---- + === Memory 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: @@ -518,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 @@ -528,7 +571,7 @@ import {Time} from "contracts/utils/types/Time.sol"; contract MyDelayedContract { using Time for *; - + Time.Delay private _delay; constructor() { @@ -539,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"); @@ -550,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 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', + ); }); }); }); diff --git a/test/utils/LowLevelCall.test.js b/test/utils/LowLevelCall.test.js new file mode 100644 index 00000000000..c0f06a45e1d --- /dev/null +++ b/test/utils/LowLevelCall.test.js @@ -0,0 +1,257 @@ +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'); +const storageSlot = ethers.id('location'); +const storageValue = ethers.id('data'); + +async function fixture() { + const [account] = await ethers.getSigners(); + + const mock = await ethers.deployContract('$LowLevelCall'); + const target = await ethers.deployContract('CallReceiverMock'); + + return { account, mock, target }; +} + +describe('LowLevelCall', function () { + beforeEach(async function () { + Object.assign(this, await loadFixture(fixture)); + }); + + describe('call', function () { + describe('without any return', function () { + it('calls the requested function and returns true', async function () { + 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); + }); + + it('calls the requested function with value and returns true', async function () { + await this.account.sendTransaction({ to: this.mock, value }); + + 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); + }); + + it("calls the requested function and returns false if the caller doesn't have enough balance", async function () { + 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); + }); + + it('calls the requested function and returns false if the subcall reverts', async function () { + 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); + }); + }); + + describe('with 64 bytes return in the scratch space', function () { + it('calls the requested function and returns true', async function () { + 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); + }); + + it('calls the requested function with value and returns true', async function () { + await this.account.sendTransaction({ to: this.mock, value }); + + 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); + }); + + it("calls the requested function and returns false if the caller doesn't have enough balance", async function () { + 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); + }); + + it('calls the requested function and returns false if the subcall reverts', async function () { + 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)), + ); + }); + }); + }); + + 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]); + }); + + 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)), + ]); + }); + }); + }); + + 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, storageSlot)).to.eventually.equal(ethers.ZeroHash); + + await expect( + this.mock.$delegatecallNoReturn( + this.target, + this.target.interface.encodeFunctionData('mockFunctionWritesStorage', [storageSlot, storageValue]), + ), + ) + .to.emit(this.mock, 'return$delegatecallNoReturn') + .withArgs(true); + + // 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( + this.mock.$delegatecallNoReturn( + this.target, + this.target.interface.encodeFunctionData('mockFunctionRevertsNoReason'), + ), + ) + .to.emit(this.mock, 'return$delegatecallNoReturn') + .withArgs(false); + }); + }); + + 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, storageSlot)).to.eventually.equal(ethers.ZeroHash); + + await expect( + this.mock.$delegatecallReturn64Bytes( + this.target, + this.target.interface.encodeFunctionData('mockFunctionWithArgsReturnWritesStorage', [ + storageSlot, + storageValue, + returnValue1, + returnValue2, + ]), + ), + ) + .to.emit(this.mock, 'return$delegatecallReturn64Bytes') + .withArgs(true, returnValue1, returnValue2); + + // 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( + this.mock.$delegatecallReturn64Bytes( + this.target, + this.target.interface.encodeFunctionData('mockFunctionRevertsNoReason'), + ), + ) + .to.emit(this.mock, 'return$delegatecallReturn64Bytes') + .withArgs(false, ethers.ZeroHash, ethers.ZeroHash); + }); + }); + }); +});