From 82ada7a339ab5f0f2181e4ab82fddfda1b078d48 Mon Sep 17 00:00:00 2001 From: yonada Date: Thu, 25 Apr 2024 16:02:38 +0100 Subject: [PATCH] feat(world-modules): call with signature supports ERC1271 (#2559) --- packages/world-modules/gas-report.json | 2 +- .../modules/callwithsignature/IERC1271.sol | 17 +++++++ .../IUnstable_CallWithSignatureErrors.sol | 2 +- .../callwithsignature/SignatureChecker.sol | 50 +++++++++++++++++++ .../validateCallWithSignature.sol | 7 ++- .../test/CallWithSignatureModule.t.sol | 9 +--- 6 files changed, 74 insertions(+), 13 deletions(-) create mode 100644 packages/world-modules/src/modules/callwithsignature/IERC1271.sol create mode 100644 packages/world-modules/src/modules/callwithsignature/SignatureChecker.sol diff --git a/packages/world-modules/gas-report.json b/packages/world-modules/gas-report.json index 853c07d2d5..fbcb13e891 100644 --- a/packages/world-modules/gas-report.json +++ b/packages/world-modules/gas-report.json @@ -9,7 +9,7 @@ "file": "test/CallWithSignatureModule.t.sol", "test": "testRegisterDelegationWithSignature", "name": "register an unlimited delegation with signature", - "gasUsed": 135913 + "gasUsed": 138532 }, { "file": "test/ERC20.t.sol", diff --git a/packages/world-modules/src/modules/callwithsignature/IERC1271.sol b/packages/world-modules/src/modules/callwithsignature/IERC1271.sol new file mode 100644 index 0000000000..a1fd71358a --- /dev/null +++ b/packages/world-modules/src/modules/callwithsignature/IERC1271.sol @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: MIT +// OpenZeppelin Contracts (last updated v5.0.0) (interfaces/IERC1271.sol) + +pragma solidity ^0.8.24; + +/** + * @dev Interface of the ERC-1271 standard signature validation method for + * contracts as defined in https://eips.ethereum.org/EIPS/eip-1271[ERC-1271]. + */ +interface IERC1271 { + /** + * @dev Should return whether the signature provided is valid for the provided data + * @param hash Hash of the data to be signed + * @param signature Signature byte array associated with _data + */ + function isValidSignature(bytes32 hash, bytes memory signature) external view returns (bytes4 magicValue); +} diff --git a/packages/world-modules/src/modules/callwithsignature/IUnstable_CallWithSignatureErrors.sol b/packages/world-modules/src/modules/callwithsignature/IUnstable_CallWithSignatureErrors.sol index 5923338ca8..37bd5e962d 100644 --- a/packages/world-modules/src/modules/callwithsignature/IUnstable_CallWithSignatureErrors.sol +++ b/packages/world-modules/src/modules/callwithsignature/IUnstable_CallWithSignatureErrors.sol @@ -5,5 +5,5 @@ interface IUnstable_CallWithSignatureErrors { /** * @dev Mismatched signature. */ - error InvalidSignature(address signer); + error InvalidSignature(); } diff --git a/packages/world-modules/src/modules/callwithsignature/SignatureChecker.sol b/packages/world-modules/src/modules/callwithsignature/SignatureChecker.sol new file mode 100644 index 0000000000..cbadd6d679 --- /dev/null +++ b/packages/world-modules/src/modules/callwithsignature/SignatureChecker.sol @@ -0,0 +1,50 @@ +// SPDX-License-Identifier: MIT +// OpenZeppelin Contracts (last updated v5.0.0) (utils/cryptography/SignatureChecker.sol) + +pragma solidity ^0.8.24; + +import { ECDSA } from "./ECDSA.sol"; +import { IERC1271 } from "./IERC1271.sol"; + +/** + * @dev Signature verification helper that can be used instead of `ECDSA.recover` to seamlessly support both ECDSA + * signatures from externally owned accounts (EOAs) as well as ERC-1271 signatures from smart contract wallets like + * Argent and Safe Wallet (previously Gnosis Safe). + */ +library SignatureChecker { + /** + * @dev Checks if a signature is valid for a given signer and data hash. If the signer is a smart contract, the + * signature is validated against that smart contract using ERC-1271, otherwise it's validated using `ECDSA.recover`. + * + * NOTE: Unlike ECDSA signatures, contract signatures are revocable, and the outcome of this function can thus + * change through time. It could return true at block N and false at block N+1 (or the opposite). + */ + function isValidSignatureNow(address signer, bytes32 hash, bytes memory signature) internal view returns (bool) { + if (signer.code.length == 0) { + (address recovered, ECDSA.RecoverError err, ) = ECDSA.tryRecover(hash, signature); + return err == ECDSA.RecoverError.NoError && recovered == signer; + } else { + return isValidERC1271SignatureNow(signer, hash, signature); + } + } + + /** + * @dev Checks if a signature is valid for a given signer and data hash. The signature is validated + * against the signer smart contract using ERC-1271. + * + * NOTE: Unlike ECDSA signatures, contract signatures are revocable, and the outcome of this function can thus + * change through time. It could return true at block N and false at block N+1 (or the opposite). + */ + function isValidERC1271SignatureNow( + address signer, + 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)); + } +} diff --git a/packages/world-modules/src/modules/callwithsignature/validateCallWithSignature.sol b/packages/world-modules/src/modules/callwithsignature/validateCallWithSignature.sol index 6156ee7914..3e6e4cd904 100644 --- a/packages/world-modules/src/modules/callwithsignature/validateCallWithSignature.sol +++ b/packages/world-modules/src/modules/callwithsignature/validateCallWithSignature.sol @@ -6,6 +6,7 @@ import { CallWithSignatureNonces } from "./tables/CallWithSignatureNonces.sol"; import { getSignedMessageHash } from "./getSignedMessageHash.sol"; import { ECDSA } from "./ECDSA.sol"; import { IUnstable_CallWithSignatureErrors } from "./IUnstable_CallWithSignatureErrors.sol"; +import { SignatureChecker } from "./SignatureChecker.sol"; /** * @notice Verifies the given system call corresponds to the given signature. @@ -24,9 +25,7 @@ function validateCallWithSignature( uint256 nonce = CallWithSignatureNonces._get(signer); bytes32 hash = getSignedMessageHash(signer, systemId, callData, nonce, WorldContextConsumerLib._world()); - // If the message was not signed by the delegator or is invalid, revert - address recoveredSigner = ECDSA.recover(hash, signature); - if (recoveredSigner != signer) { - revert IUnstable_CallWithSignatureErrors.InvalidSignature(recoveredSigner); + if (!SignatureChecker.isValidSignatureNow(signer, hash, signature)) { + revert IUnstable_CallWithSignatureErrors.InvalidSignature(); } } diff --git a/packages/world-modules/test/CallWithSignatureModule.t.sol b/packages/world-modules/test/CallWithSignatureModule.t.sol index 42d4a68a24..ac51fe069b 100644 --- a/packages/world-modules/test/CallWithSignatureModule.t.sol +++ b/packages/world-modules/test/CallWithSignatureModule.t.sol @@ -66,7 +66,7 @@ contract Unstable_CallWithSignatureModuleTest is Test, GasReporter { bytes memory signature = abi.encodePacked(r, s, v); // Attempt to register a limited delegation using an empty signature - vm.expectRevert(abi.encodeWithSelector(ECDSA.ECDSAInvalidSignatureLength.selector, 0)); + vm.expectRevert(abi.encodeWithSelector(IUnstable_CallWithSignatureErrors.InvalidSignature.selector)); Unstable_CallWithSignatureSystem(address(world)).callWithSignature( delegator, REGISTRATION_SYSTEM_ID, @@ -101,12 +101,7 @@ contract Unstable_CallWithSignatureModuleTest is Test, GasReporter { world.callFrom(delegator, systemId, abi.encodeCall(WorldTestSystem.msgSender, ())); // Attempt to register a limited delegation using an old signature - vm.expectRevert( - abi.encodeWithSelector( - IUnstable_CallWithSignatureErrors.InvalidSignature.selector, - 0x65F0D93280688178Ec3F55bBd6f6088290639Bfb - ) - ); + vm.expectRevert(abi.encodeWithSelector(IUnstable_CallWithSignatureErrors.InvalidSignature.selector)); Unstable_CallWithSignatureSystem(address(world)).callWithSignature( delegator, REGISTRATION_SYSTEM_ID,