From 1c97739c1fa97941a71b99eb6b9ed096eb085b51 Mon Sep 17 00:00:00 2001 From: ernestognw Date: Fri, 2 May 2025 15:42:17 -0600 Subject: [PATCH 1/8] Test ethers 6.13.6-beta.1 --- package-lock.json | 8 ++++---- package.json | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package-lock.json b/package-lock.json index a82155c7608..a8cdbf4de21 100644 --- a/package-lock.json +++ b/package-lock.json @@ -24,7 +24,7 @@ "chai": "^4.2.0", "eslint": "^9.0.0", "eslint-config-prettier": "^10.0.0", - "ethers": "^6.13.4", + "ethers": "^6.13.6-beta.1", "glob": "^11.0.0", "globals": "^16.0.0", "graphlib": "^2.1.8", @@ -4483,9 +4483,9 @@ } }, "node_modules/ethers": { - "version": "6.13.7", - "resolved": "https://registry.npmjs.org/ethers/-/ethers-6.13.7.tgz", - "integrity": "sha512-qbaJ0uIrjh+huP1Lad2f2QtzW5dcqSVjIzVH6yWB4dKoMuj2WqYz5aMeeQTCNpAKgTJBM5J9vcc2cYJ23UAimQ==", + "version": "6.13.6-beta.1", + "resolved": "https://registry.npmjs.org/ethers/-/ethers-6.13.6-beta.1.tgz", + "integrity": "sha512-sJZklf+m7QrlzYnOFbR0qHPqgYHeevbY98VIhzvnSdzhJVN/nNV/skKc/4wjyxbWRhK9t7r6ENcwUwLPjfxTLw==", "dev": true, "funding": [ { diff --git a/package.json b/package.json index d2f7dec6c6d..635290f6205 100644 --- a/package.json +++ b/package.json @@ -66,7 +66,7 @@ "chai": "^4.2.0", "eslint": "^9.0.0", "eslint-config-prettier": "^10.0.0", - "ethers": "^6.13.4", + "ethers": "^6.13.6-beta.1", "glob": "^11.0.0", "globals": "^16.0.0", "graphlib": "^2.1.8", From 6b1bbd8d7bb6096597b478c443fde4d6b51e1c6a Mon Sep 17 00:00:00 2001 From: ernestognw Date: Fri, 2 May 2025 15:44:49 -0600 Subject: [PATCH 2/8] up --- package-lock.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index a8cdbf4de21..1ac37124f2e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4483,9 +4483,9 @@ } }, "node_modules/ethers": { - "version": "6.13.6-beta.1", - "resolved": "https://registry.npmjs.org/ethers/-/ethers-6.13.6-beta.1.tgz", - "integrity": "sha512-sJZklf+m7QrlzYnOFbR0qHPqgYHeevbY98VIhzvnSdzhJVN/nNV/skKc/4wjyxbWRhK9t7r6ENcwUwLPjfxTLw==", + "version": "6.13.7", + "resolved": "https://registry.npmjs.org/ethers/-/ethers-6.13.7.tgz", + "integrity": "sha512-qbaJ0uIrjh+huP1Lad2f2QtzW5dcqSVjIzVH6yWB4dKoMuj2WqYz5aMeeQTCNpAKgTJBM5J9vcc2cYJ23UAimQ==", "dev": true, "funding": [ { From 19fe4c525d96d9d8baa2b9bf48be25b3c0384b44 Mon Sep 17 00:00:00 2001 From: ernestognw Date: Fri, 2 May 2025 15:52:18 -0600 Subject: [PATCH 3/8] up --- package-lock.json | 8 ++++---- package.json | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package-lock.json b/package-lock.json index 1ac37124f2e..e1b7018388f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -24,7 +24,7 @@ "chai": "^4.2.0", "eslint": "^9.0.0", "eslint-config-prettier": "^10.0.0", - "ethers": "^6.13.6-beta.1", + "ethers": "6.13.6-beta.1", "glob": "^11.0.0", "globals": "^16.0.0", "graphlib": "^2.1.8", @@ -4483,9 +4483,9 @@ } }, "node_modules/ethers": { - "version": "6.13.7", - "resolved": "https://registry.npmjs.org/ethers/-/ethers-6.13.7.tgz", - "integrity": "sha512-qbaJ0uIrjh+huP1Lad2f2QtzW5dcqSVjIzVH6yWB4dKoMuj2WqYz5aMeeQTCNpAKgTJBM5J9vcc2cYJ23UAimQ==", + "version": "6.13.6-beta.1", + "resolved": "https://registry.npmjs.org/ethers/-/ethers-6.13.6-beta.1.tgz", + "integrity": "sha512-sJZklf+m7QrlzYnOFbR0qHPqgYHeevbY98VIhzvnSdzhJVN/nNV/skKc/4wjyxbWRhK9t7r6ENcwUwLPjfxTLw==", "dev": true, "funding": [ { diff --git a/package.json b/package.json index 635290f6205..64ee144be16 100644 --- a/package.json +++ b/package.json @@ -66,7 +66,7 @@ "chai": "^4.2.0", "eslint": "^9.0.0", "eslint-config-prettier": "^10.0.0", - "ethers": "^6.13.6-beta.1", + "ethers": "6.13.6-beta.1", "glob": "^11.0.0", "globals": "^16.0.0", "graphlib": "^2.1.8", From c39d5f5755c64b3c0ff7fc666c615e58f6135f2a Mon Sep 17 00:00:00 2001 From: ernestognw Date: Sat, 3 May 2025 00:43:58 -0600 Subject: [PATCH 4/8] Tweak workflows --- .github/actions/setup/action.yml | 3 ++- .github/workflows/checks.yml | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/actions/setup/action.yml b/.github/actions/setup/action.yml index 3c5fc602e13..fe8a85b3f55 100644 --- a/.github/actions/setup/action.yml +++ b/.github/actions/setup/action.yml @@ -13,7 +13,8 @@ runs: path: '**/node_modules' key: npm-v3-${{ hashFiles('**/package-lock.json') }} - name: Install dependencies - run: npm ci + ## TODO: Remove when EIP-7702 authorizations are enabled in latest non-beta ethers version + run: npm ci --legacy-peer-deps shell: bash if: steps.cache.outputs.cache-hit != 'true' - name: Install Foundry diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index 6aca7f30cb4..cba9894b3b9 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -118,6 +118,8 @@ jobs: - uses: actions/checkout@v4 - name: Set up environment uses: ./.github/actions/setup + ## TODO: Remove when EIP-7702 authorizations are enabled in latest non-beta ethers version + - run: rm package-lock.json package.json # Dependencies already installed - uses: crytic/slither-action@v0.4.1 codespell: From 54f632a588bd9b306d42c51d3e7613625809b665 Mon Sep 17 00:00:00 2001 From: ernestognw Date: Sat, 3 May 2025 00:46:34 -0600 Subject: [PATCH 5/8] Use Solidity 0.8.27 as default and set default EVM to prague --- foundry.toml | 2 +- hardhat.config.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/foundry.toml b/foundry.toml index 7a2e8a60942..ea8b1fadd88 100644 --- a/foundry.toml +++ b/foundry.toml @@ -1,5 +1,5 @@ [profile.default] -solc_version = '0.8.24' +solc_version = '0.8.27' evm_version = 'prague' optimizer = true optimizer-runs = 200 diff --git a/hardhat.config.js b/hardhat.config.js index 30c19ca6d5b..17ebf45eeb7 100644 --- a/hardhat.config.js +++ b/hardhat.config.js @@ -18,7 +18,7 @@ const { argv } = require('yargs/yargs')() compiler: { alias: 'compileVersion', type: 'string', - default: '0.8.24', + default: '0.8.27', }, src: { alias: 'source', @@ -38,7 +38,7 @@ const { argv } = require('yargs/yargs')() evm: { alias: 'evmVersion', type: 'string', - default: 'cancun', + default: 'prague', }, // Extra modules coverage: { From 1f4992f11e64ccf64ab72d22a078d953ff3c30f4 Mon Sep 17 00:00:00 2001 From: ernestognw Date: Sat, 3 May 2025 01:02:00 -0600 Subject: [PATCH 6/8] Add ERC7739 and ERC7379Utils --- .changeset/lucky-donuts-scream.md | 5 + .changeset/proud-tables-sip.md | 5 + .../mocks/utils/cryptography/ERC7739Mock.sol | 28 +++ .../utils/cryptography/AbstractSigner.sol | 22 ++ contracts/utils/cryptography/ERC7739.sol | 98 +++++++++ contracts/utils/cryptography/ERC7739Utils.sol | 206 ++++++++++++++++++ test/helpers/erc7739.js | 118 ++++++++++ test/utils/cryptography/ERC1271.behavior.js | 111 ++++++++++ test/utils/cryptography/ERC7739.test.js | 13 ++ test/utils/cryptography/ERC7739Utils.test.js | 203 +++++++++++++++++ 10 files changed, 809 insertions(+) create mode 100644 .changeset/lucky-donuts-scream.md create mode 100644 .changeset/proud-tables-sip.md create mode 100644 contracts/mocks/utils/cryptography/ERC7739Mock.sol create mode 100644 contracts/utils/cryptography/AbstractSigner.sol create mode 100644 contracts/utils/cryptography/ERC7739.sol create mode 100644 contracts/utils/cryptography/ERC7739Utils.sol create mode 100644 test/helpers/erc7739.js create mode 100644 test/utils/cryptography/ERC1271.behavior.js create mode 100644 test/utils/cryptography/ERC7739.test.js create mode 100644 test/utils/cryptography/ERC7739Utils.test.js diff --git a/.changeset/lucky-donuts-scream.md b/.changeset/lucky-donuts-scream.md new file mode 100644 index 00000000000..aaeb29a5e35 --- /dev/null +++ b/.changeset/lucky-donuts-scream.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': minor +--- + +`ERC7739`: An abstract contract to validate signatures following the rehashing scheme from `ERC7739Utils`. diff --git a/.changeset/proud-tables-sip.md b/.changeset/proud-tables-sip.md new file mode 100644 index 00000000000..5a199d0ef41 --- /dev/null +++ b/.changeset/proud-tables-sip.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': minor +--- + +`ERC7739Utils`: Add a library that implements a defensive rehashing mechanism to prevent replayability of smart contract signatures based on the ERC-7739. diff --git a/contracts/mocks/utils/cryptography/ERC7739Mock.sol b/contracts/mocks/utils/cryptography/ERC7739Mock.sol new file mode 100644 index 00000000000..eaf487227f7 --- /dev/null +++ b/contracts/mocks/utils/cryptography/ERC7739Mock.sol @@ -0,0 +1,28 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +import {ECDSA} from "../../../utils/cryptography/ECDSA.sol"; +import {EIP712} from "../../../utils/cryptography/EIP712.sol"; +import {ERC7739} from "../../../utils/cryptography/ERC7739.sol"; +import {AbstractSigner} from "../../../utils/cryptography/AbstractSigner.sol"; + +contract ERC7739ECDSAMock is AbstractSigner, ERC7739 { + address private _signer; + + constructor(address signerAddr) EIP712("ERC7739ECDSA", "1") { + _signer = signerAddr; + } + + function signer() public view virtual returns (address) { + return _signer; + } + + function _rawSignatureValidation( + bytes32 hash, + bytes calldata signature + ) internal view virtual override returns (bool) { + (address recovered, ECDSA.RecoverError err, ) = ECDSA.tryRecover(hash, signature); + return signer() == recovered && err == ECDSA.RecoverError.NoError; + } +} diff --git a/contracts/utils/cryptography/AbstractSigner.sol b/contracts/utils/cryptography/AbstractSigner.sol new file mode 100644 index 00000000000..40803518c68 --- /dev/null +++ b/contracts/utils/cryptography/AbstractSigner.sol @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +/** + * @dev Abstract contract for signature validation. + * + * Developers must implement {_rawSignatureValidation} and use it as the lowest-level signature validation mechanism. + * + * @custom:stateless + */ +abstract contract AbstractSigner { + /** + * @dev Signature validation algorithm. + * + * WARNING: Implementing a signature validation algorithm is a security-sensitive operation as it involves + * cryptographic verification. It is important to review and test thoroughly before deployment. Consider + * using one of the signature verification libraries (xref:api:utils#ECDSA[ECDSA], xref:api:utils#P256[P256] + * or xref:api:utils#RSA[RSA]). + */ + function _rawSignatureValidation(bytes32 hash, bytes calldata signature) internal view virtual returns (bool); +} diff --git a/contracts/utils/cryptography/ERC7739.sol b/contracts/utils/cryptography/ERC7739.sol new file mode 100644 index 00000000000..eb4f47f37c3 --- /dev/null +++ b/contracts/utils/cryptography/ERC7739.sol @@ -0,0 +1,98 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +import {IERC1271} from "../../interfaces/IERC1271.sol"; +import {EIP712} from "../cryptography/EIP712.sol"; +import {MessageHashUtils} from "../cryptography/MessageHashUtils.sol"; +import {ShortStrings} from "../ShortStrings.sol"; +import {AbstractSigner} from "./AbstractSigner.sol"; +import {ERC7739Utils} from "./ERC7739Utils.sol"; + +/** + * @dev Validates signatures wrapping the message hash in a nested EIP712 type. See {ERC7739Utils}. + * + * Linking the signature to the EIP-712 domain separator is a security measure to prevent signature replay across different + * EIP-712 domains (e.g. a single offchain owner of multiple contracts). + * + * This contract requires implementing the {_rawSignatureValidation} function, which passes the wrapped message hash, + * which may be either an typed data or a personal sign nested type. + * + * NOTE: xref:api:utils#EIP712[EIP-712] uses xref:api:utils#ShortStrings[ShortStrings] to optimize gas + * costs for short strings (up to 31 characters). Consider that strings longer than that will use storage, + * which may limit the ability of the signer to be used within the ERC-4337 validation phase (due to + * https://eips.ethereum.org/EIPS/eip-7562#storage-rules[ERC-7562 storage access rules]). + */ +abstract contract ERC7739 is AbstractSigner, EIP712, IERC1271 { + using ERC7739Utils for *; + using MessageHashUtils for bytes32; + + /** + * @dev Attempts validating the signature in a nested EIP-712 type. + * + * A nested EIP-712 type might be presented in 2 different ways: + * + * - As a nested EIP-712 typed data + * - As a _personal_ signature (an EIP-712 mimic of the `eth_personalSign` for a smart contract) + */ + function isValidSignature(bytes32 hash, bytes calldata signature) public view virtual returns (bytes4 result) { + // For the hash `0x7739773977397739773977397739773977397739773977397739773977397739` and an empty signature, + // we return the magic value `0x77390001` as it's assumed impossible to find a preimage for it that can be used + // maliciously. Useful for simulation purposes and to validate whether the contract supports ERC-7739. + return + (_isValidNestedTypedDataSignature(hash, signature) || _isValidNestedPersonalSignSignature(hash, signature)) + ? IERC1271.isValidSignature.selector + : (hash == 0x7739773977397739773977397739773977397739773977397739773977397739 && signature.length == 0) + ? bytes4(0x77390001) + : bytes4(0xffffffff); + } + + /** + * @dev Nested personal signature verification. + */ + function _isValidNestedPersonalSignSignature(bytes32 hash, bytes calldata signature) private view returns (bool) { + return _rawSignatureValidation(_domainSeparatorV4().toTypedDataHash(hash.personalSignStructHash()), signature); + } + + /** + * @dev Nested EIP-712 typed data verification. + */ + function _isValidNestedTypedDataSignature( + bytes32 hash, + bytes calldata encodedSignature + ) private view returns (bool) { + // decode signature + ( + bytes calldata signature, + bytes32 appSeparator, + bytes32 contentsHash, + string calldata contentsDescr + ) = encodedSignature.decodeTypedDataSig(); + + ( + , + string memory name, + string memory version, + uint256 chainId, + address verifyingContract, + bytes32 salt, + + ) = eip712Domain(); + + // Check that contentHash and separator are correct + // Rebuild nested hash + return + hash == appSeparator.toTypedDataHash(contentsHash) && + bytes(contentsDescr).length != 0 && + _rawSignatureValidation( + appSeparator.toTypedDataHash( + ERC7739Utils.typedDataSignStructHash( + contentsDescr, + contentsHash, + abi.encode(keccak256(bytes(name)), keccak256(bytes(version)), chainId, verifyingContract, salt) + ) + ), + signature + ); + } +} diff --git a/contracts/utils/cryptography/ERC7739Utils.sol b/contracts/utils/cryptography/ERC7739Utils.sol new file mode 100644 index 00000000000..e7b720ea0c9 --- /dev/null +++ b/contracts/utils/cryptography/ERC7739Utils.sol @@ -0,0 +1,206 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +import {Calldata} from "../Calldata.sol"; + +/** + * @dev Utilities to process https://ercs.ethereum.org/ERCS/erc-7739[ERC-7739] typed data signatures + * that are specific to an EIP-712 domain. + * + * This library provides methods to wrap, unwrap and operate over typed data signatures with a defensive + * rehashing mechanism that includes the application's xref:api:utils#EIP712-_domainSeparatorV4[EIP-712] + * and preserves readability of the signed content using an EIP-712 nested approach. + * + * A smart contract domain can validate a signature for a typed data structure in two ways: + * + * - As an application validating a typed data signature. See {typedDataSignStructHash}. + * - As a smart contract validating a raw message signature. See {personalSignStructHash}. + * + * NOTE: A provider for a smart contract wallet would need to return this signature as the + * result of a call to `personal_sign` or `eth_signTypedData`, and this may be unsupported by + * API clients that expect a return value of 129 bytes, or specifically the `r,s,v` parameters + * of an xref:api:utils#ECDSA[ECDSA] signature, as is for example specified for + * xref:api:utils#EIP712[EIP-712]. + */ +library ERC7739Utils { + /** + * @dev An EIP-712 type to represent "personal" signatures + * (i.e. mimic of `personal_sign` for smart contracts). + */ + bytes32 private constant PERSONAL_SIGN_TYPEHASH = keccak256("PersonalSign(bytes prefixed)"); + + /** + * @dev Nest a signature for a given EIP-712 type into a nested signature for the domain of the app. + * + * Counterpart of {decodeTypedDataSig} to extract the original signature and the nested components. + */ + function encodeTypedDataSig( + bytes memory signature, + bytes32 appSeparator, + bytes32 contentsHash, + string memory contentsDescr + ) internal pure returns (bytes memory) { + return + abi.encodePacked(signature, appSeparator, contentsHash, contentsDescr, uint16(bytes(contentsDescr).length)); + } + + /** + * @dev Parses a nested signature into its components. + * + * Constructed as follows: + * + * `signature ‖ APP_DOMAIN_SEPARATOR ‖ contentsHash ‖ contentsDescr ‖ uint16(contentsDescr.length)` + * + * - `signature` is the signature for the (ERC-7739) nested struct hash. This signature indirectly signs over the + * original "contents" hash (from the app) and the account's domain separator. + * - `APP_DOMAIN_SEPARATOR` is the EIP-712 {EIP712-_domainSeparatorV4} of the application smart contract that is + * requesting the signature verification (though ERC-1271). + * - `contentsHash` is the hash of the underlying data structure or message. + * - `contentsDescr` is a descriptor of the "contents" part of the the EIP-712 type of the nested signature. + * + * NOTE: This function returns empty if the input format is invalid instead of reverting. + * data instead. + */ + function decodeTypedDataSig( + bytes calldata encodedSignature + ) + internal + pure + returns (bytes calldata signature, bytes32 appSeparator, bytes32 contentsHash, string calldata contentsDescr) + { + unchecked { + uint256 sigLength = encodedSignature.length; + + // 66 bytes = contentsDescrLength (2 bytes) + contentsHash (32 bytes) + APP_DOMAIN_SEPARATOR (32 bytes). + if (sigLength < 66) return (Calldata.emptyBytes(), 0, 0, Calldata.emptyString()); + + uint256 contentsDescrEnd = sigLength - 2; // Last 2 bytes + uint256 contentsDescrLength = uint16(bytes2(encodedSignature[contentsDescrEnd:])); + + // Check for space for `contentsDescr` in addition to the 66 bytes documented above + if (sigLength < 66 + contentsDescrLength) return (Calldata.emptyBytes(), 0, 0, Calldata.emptyString()); + + uint256 contentsHashEnd = contentsDescrEnd - contentsDescrLength; + uint256 separatorEnd = contentsHashEnd - 32; + uint256 signatureEnd = separatorEnd - 32; + + signature = encodedSignature[:signatureEnd]; + appSeparator = bytes32(encodedSignature[signatureEnd:separatorEnd]); + contentsHash = bytes32(encodedSignature[separatorEnd:contentsHashEnd]); + contentsDescr = string(encodedSignature[contentsHashEnd:contentsDescrEnd]); + } + } + + /** + * @dev Nests an `ERC-191` digest into a `PersonalSign` EIP-712 struct, and returns the corresponding struct hash. + * This struct hash must be combined with a domain separator, using {MessageHashUtils-toTypedDataHash} before + * being verified/recovered. + * + * This is used to simulates the `personal_sign` RPC method in the context of smart contracts. + */ + function personalSignStructHash(bytes32 contents) internal pure returns (bytes32) { + return keccak256(abi.encode(PERSONAL_SIGN_TYPEHASH, contents)); + } + + /** + * @dev Nests an `EIP-712` hash (`contents`) into a `TypedDataSign` EIP-712 struct, and returns the corresponding + * struct hash. This struct hash must be combined with a domain separator, using {MessageHashUtils-toTypedDataHash} + * before being verified/recovered. + */ + function typedDataSignStructHash( + string calldata contentsName, + string calldata contentsType, + bytes32 contentsHash, + bytes memory domainBytes + ) internal pure returns (bytes32 result) { + return + bytes(contentsName).length == 0 + ? bytes32(0) + : keccak256( + abi.encodePacked(typedDataSignTypehash(contentsName, contentsType), contentsHash, domainBytes) + ); + } + + /** + * @dev Variant of {typedDataSignStructHash-string-string-bytes32-bytes} that takes a content descriptor + * and decodes the `contentsName` and `contentsType` out of it. + */ + function typedDataSignStructHash( + string calldata contentsDescr, + bytes32 contentsHash, + bytes memory domainBytes + ) internal pure returns (bytes32 result) { + (string calldata contentsName, string calldata contentsType) = decodeContentsDescr(contentsDescr); + + return typedDataSignStructHash(contentsName, contentsType, contentsHash, domainBytes); + } + + /** + * @dev Compute the EIP-712 typehash of the `TypedDataSign` structure for a given type (and typename). + */ + function typedDataSignTypehash( + string calldata contentsName, + string calldata contentsType + ) internal pure returns (bytes32) { + return + keccak256( + abi.encodePacked( + "TypedDataSign(", + contentsName, + " contents,string name,string version,uint256 chainId,address verifyingContract,bytes32 salt)", + contentsType + ) + ); + } + + /** + * @dev Parse the type name out of the ERC-7739 contents type description. Supports both the implicit and explicit + * modes. + * + * Following ERC-7739 specifications, a `contentsName` is considered invalid if it's empty or it contains + * any of the following bytes , )\x00 + * + * If the `contentsType` is invalid, this returns an empty string. Otherwise, the return string has non-zero + * length. + */ + function decodeContentsDescr( + string calldata contentsDescr + ) internal pure returns (string calldata contentsName, string calldata contentsType) { + bytes calldata buffer = bytes(contentsDescr); + if (buffer.length == 0) { + // pass through (fail) + } else if (buffer[buffer.length - 1] == bytes1(")")) { + // Implicit mode: read contentsName from the beginning, and keep the complete descr + for (uint256 i = 0; i < buffer.length; ++i) { + bytes1 current = buffer[i]; + if (current == bytes1("(")) { + // if name is empty - passthrough (fail) + if (i == 0) break; + // we found the end of the contentsName + return (string(buffer[:i]), contentsDescr); + } else if (_isForbiddenChar(current)) { + // we found an invalid character (forbidden) - passthrough (fail) + break; + } + } + } else { + // Explicit mode: read contentsName from the end, and remove it from the descr + for (uint256 i = buffer.length; i > 0; --i) { + bytes1 current = buffer[i - 1]; + if (current == bytes1(")")) { + // we found the end of the contentsName + return (string(buffer[i:]), string(buffer[:i])); + } else if (_isForbiddenChar(current)) { + // we found an invalid character (forbidden) - passthrough (fail) + break; + } + } + } + return (Calldata.emptyString(), Calldata.emptyString()); + } + + function _isForbiddenChar(bytes1 char) private pure returns (bool) { + return char == 0x00 || char == bytes1(" ") || char == bytes1(",") || char == bytes1("(") || char == bytes1(")"); + } +} diff --git a/test/helpers/erc7739.js b/test/helpers/erc7739.js new file mode 100644 index 00000000000..5a489de71e9 --- /dev/null +++ b/test/helpers/erc7739.js @@ -0,0 +1,118 @@ +const { ethers } = require('hardhat'); +const { formatType } = require('./eip712'); + +const PersonalSign = formatType({ prefixed: 'bytes' }); +const TypedDataSign = contentsTypeName => + formatType({ + contents: contentsTypeName, + name: 'string', + version: 'string', + chainId: 'uint256', + verifyingContract: 'address', + salt: 'bytes32', + }); + +class ERC7739Signer extends ethers.AbstractSigner { + #signer; + #domain; + + constructor(signer, domain) { + super(signer.provider); + this.#signer = signer; + this.#domain = domain; + } + + static from(signer, domain) { + return new this(signer, domain); + } + + get signingKey() { + return this.#signer.signingKey; + } + + get privateKey() { + return this.#signer.privateKey; + } + + async getAddress() { + return this.#signer.getAddress(); + } + + connect(provider) { + this.#signer.connect(provider); + } + + async signTransaction(tx) { + return this.#signer.signTransaction(tx); + } + + async signMessage(message) { + return this.#signer.signTypedData(this.#domain, { PersonalSign }, ERC4337Utils.preparePersonalSign(message)); + } + + async signTypedData(domain, types, value) { + const { allTypes, contentsTypeName, contentsDescr } = ERC4337Utils.getContentsDetail(types); + + return Promise.resolve( + this.#signer.signTypedData(domain, allTypes, ERC4337Utils.prepareSignTypedData(value, this.#domain)), + ).then(signature => + ethers.concat([ + signature, + ethers.TypedDataEncoder.hashDomain(domain), // appDomainSeparator + ethers.TypedDataEncoder.hashStruct(contentsTypeName, types, value), // contentsHash + ethers.toUtf8Bytes(contentsDescr), + ethers.toBeHex(contentsDescr.length, 2), + ]), + ); + } +} + +class ERC4337Utils { + static preparePersonalSign(message) { + return { + prefixed: ethers.concat([ + ethers.toUtf8Bytes(ethers.MessagePrefix), + ethers.toUtf8Bytes(String(message.length)), + typeof message === 'string' ? ethers.toUtf8Bytes(message) : message, + ]), + }; + } + + static prepareSignTypedData(contents, signerDomain) { + return { + name: signerDomain.name ?? '', + version: signerDomain.version ?? '', + chainId: signerDomain.chainId ?? 0, + verifyingContract: signerDomain.verifyingContract ?? ethers.ZeroAddress, + salt: signerDomain.salt ?? ethers.ZeroHash, + contents, + }; + } + + static getContentsDetail(contentsTypes, contentsTypeName = Object.keys(contentsTypes).at(0)) { + // Examples values + // + // contentsTypeName B + // typedDataSignType TypedDataSign(B contents,...)A(uint256 v)B(Z z)Z(A a) + // contentsType A(uint256 v)B(Z z)Z(A a) + // contentsDescr A(uint256 v)B(Z z)Z(A a)B + const allTypes = { TypedDataSign: TypedDataSign(contentsTypeName), ...contentsTypes }; + const typedDataSignType = ethers.TypedDataEncoder.from(allTypes).encodeType('TypedDataSign'); + const contentsType = typedDataSignType.slice(typedDataSignType.indexOf(')') + 1); // Remove TypedDataSign (first object) + const contentsDescr = contentsType + (contentsType.startsWith(contentsTypeName) ? '' : contentsTypeName); + + return { + allTypes, + contentsTypes, + contentsTypeName, + contentsDescr, + }; + } +} + +module.exports = { + ERC7739Signer, + ERC4337Utils, + PersonalSign, + TypedDataSign, +}; diff --git a/test/utils/cryptography/ERC1271.behavior.js b/test/utils/cryptography/ERC1271.behavior.js new file mode 100644 index 00000000000..ef3e668028e --- /dev/null +++ b/test/utils/cryptography/ERC1271.behavior.js @@ -0,0 +1,111 @@ +const { ethers } = require('hardhat'); +const { expect } = require('chai'); +const { Permit, formatType, getDomain } = require('../../helpers/eip712'); +const { ERC7739Signer } = require('../../helpers/erc7739'); + +function shouldBehaveLikeERC1271({ erc7739 = false } = {}) { + const MAGIC_VALUE = '0x1626ba7e'; + + describe(`supports ERC-${erc7739 ? 7739 : 1271}`, function () { + beforeEach(async function () { + // if deploy function is present, check that code is already in place + if (this.mock.deploy) { + await ethers.provider.getCode(this.mock.address).then(code => code != '0x' || this.mock.deploy()); + } + this._signer = erc7739 + ? new ERC7739Signer(this.signer, this.domain ?? (await getDomain(this.mock))) + : this.signer; + }); + + describe('PersonalSign', function () { + it('returns true for a valid personal signature', async function () { + const text = 'Hello, world!'; + + const hash = ethers.hashMessage(text); + const signature = await this._signer.signMessage(text); + + await expect(this.mock.isValidSignature(hash, signature)).to.eventually.equal(MAGIC_VALUE); + }); + + it('returns false for an invalid personal signature', async function () { + const message = 'Message the app expects'; + const otherMessage = 'Message signed is different'; + + const hash = ethers.hashMessage(message); + const signature = await this._signer.signMessage(otherMessage); + + await expect(this.mock.isValidSignature(hash, signature)).to.eventually.not.equal(MAGIC_VALUE); + }); + }); + + describe('TypedDataSign', function () { + beforeEach(async function () { + // Dummy app domain, different from the ERC7739's domain + // Note the difference of format (signer domain doesn't include a salt, but app domain does) + this.appDomain = { + name: 'SomeApp', + version: '1', + chainId: await ethers.provider.getNetwork().then(({ chainId }) => chainId), + verifyingContract: '0xe7f1725E7734CE288F8367e1Bb143E90bb3F0512', + salt: '0x02cb3d8cb5e8928c9c6de41e935e16a4e28b2d54e7e7ba47e99f16071efab785', + }; + }); + + it('returns true for a valid typed data signature', async function () { + const contents = { + owner: '0x1ab5E417d9AF00f1ca9d159007e12c401337a4bb', + spender: '0xD68E96620804446c4B1faB3103A08C98d4A8F55f', + value: 1_000_000n, + nonce: 0n, + deadline: ethers.MaxUint256, + }; + + const hash = ethers.TypedDataEncoder.hash(this.appDomain, { Permit }, contents); + const signature = await this._signer.signTypedData(this.appDomain, { Permit }, contents); + + await expect(this.mock.isValidSignature(hash, signature)).to.eventually.equal(MAGIC_VALUE); + }); + + it('returns true for valid typed data signature (nested types)', async function () { + const contentsTypes = { + B: formatType({ z: 'Z' }), + Z: formatType({ a: 'A' }), + A: formatType({ v: 'uint256' }), + }; + + const contents = { z: { a: { v: 1n } } }; + + const hash = ethers.TypedDataEncoder.hash(this.appDomain, contentsTypes, contents); + const signature = await this._signer.signTypedData(this.appDomain, contentsTypes, contents); + + await expect(this.mock.isValidSignature(hash, signature)).to.eventually.equal(MAGIC_VALUE); + }); + + it('returns false for an invalid typed data signature', async function () { + const contents = { + owner: '0x1ab5E417d9AF00f1ca9d159007e12c401337a4bb', + spender: '0xD68E96620804446c4B1faB3103A08C98d4A8F55f', + value: 1_000_000n, + nonce: 0n, + deadline: ethers.MaxUint256, + }; + + const hash = ethers.TypedDataEncoder.hash(this.appDomain, { Permit }, contents); + // message signed by the user is for a lower amount. + const signature = await this._signer.signTypedData(this.appDomain, { Permit }, { ...contents, value: 1_000n }); + + await expect(this.mock.isValidSignature(hash, signature)).to.eventually.not.equal(MAGIC_VALUE); + }); + }); + + erc7739 && + it('support ERC-7739 detection', async function () { + const hash = '0x7739773977397739773977397739773977397739773977397739773977397739'; + await expect(this.mock.isValidSignature(hash, '0x')).to.eventually.equal('0x77390001'); + }); + }); +} + +module.exports = { + shouldBehaveLikeERC1271, +}; diff --git a/test/utils/cryptography/ERC7739.test.js b/test/utils/cryptography/ERC7739.test.js new file mode 100644 index 00000000000..663bc52cb91 --- /dev/null +++ b/test/utils/cryptography/ERC7739.test.js @@ -0,0 +1,13 @@ +const { ethers } = require('hardhat'); +const { shouldBehaveLikeERC1271 } = require('./ERC1271.behavior'); + +describe('ERC7739', function () { + describe('for an ECDSA signer', function () { + before(async function () { + this.signer = ethers.Wallet.createRandom(); + this.mock = await ethers.deployContract('ERC7739ECDSAMock', [this.signer.address]); + }); + + shouldBehaveLikeERC1271({ erc7739: true }); + }); +}); diff --git a/test/utils/cryptography/ERC7739Utils.test.js b/test/utils/cryptography/ERC7739Utils.test.js new file mode 100644 index 00000000000..93e382df611 --- /dev/null +++ b/test/utils/cryptography/ERC7739Utils.test.js @@ -0,0 +1,203 @@ +const { expect } = require('chai'); +const { ethers } = require('hardhat'); +const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers'); + +const { Permit } = require('../../helpers/eip712'); +const { ERC4337Utils, PersonalSign } = require('../../helpers/erc7739'); + +const details = ERC4337Utils.getContentsDetail({ Permit }); + +const fixture = async () => { + const mock = await ethers.deployContract('$ERC7739Utils'); + const domain = { + name: 'SomeDomain', + version: '1', + chainId: await ethers.provider.getNetwork().then(({ chainId }) => chainId), + verifyingContract: '0xe7f1725E7734CE288F8367e1Bb143E90bb3F0512', + }; + const otherDomain = { + name: 'SomeOtherDomain', + version: '2', + chainId: await ethers.provider.getNetwork().then(({ chainId }) => chainId), + verifyingContract: '0x92C32cadBc39A15212505B5530aA765c441F306f', + }; + const permit = { + owner: '0x1ab5E417d9AF00f1ca9d159007e12c401337a4bb', + spender: '0xD68E96620804446c4B1faB3103A08C98d4A8F55f', + value: 1_000_000n, + nonce: 0n, + deadline: ethers.MaxUint256, + }; + return { mock, domain, otherDomain, permit }; +}; + +describe('ERC7739Utils', function () { + beforeEach(async function () { + Object.assign(this, await loadFixture(fixture)); + }); + + describe('encodeTypedDataSig', function () { + it('wraps a typed data signature', async function () { + const signature = ethers.randomBytes(65); + const appSeparator = ethers.id('SomeApp'); + const contentsHash = ethers.id('SomeData'); + const contentsDescr = 'SomeType()'; + const encoded = ethers.concat([ + signature, + appSeparator, + contentsHash, + ethers.toUtf8Bytes(contentsDescr), + ethers.toBeHex(contentsDescr.length, 2), + ]); + + await expect( + this.mock.$encodeTypedDataSig(signature, appSeparator, contentsHash, contentsDescr), + ).to.eventually.equal(encoded); + }); + }); + + describe('decodeTypedDataSig', function () { + it('unwraps a typed data signature', async function () { + const signature = ethers.randomBytes(65); + const appSeparator = ethers.id('SomeApp'); + const contentsHash = ethers.id('SomeData'); + const contentsDescr = 'SomeType()'; + const encoded = ethers.concat([ + signature, + appSeparator, + contentsHash, + ethers.toUtf8Bytes(contentsDescr), + ethers.toBeHex(contentsDescr.length, 2), + ]); + + await expect(this.mock.$decodeTypedDataSig(encoded)).to.eventually.deep.equal([ + ethers.hexlify(signature), + appSeparator, + contentsHash, + contentsDescr, + ]); + }); + + it('returns default empty values if the signature is too short', async function () { + const encoded = ethers.randomBytes(65); // DOMAIN_SEPARATOR (32 bytes) + CONTENTS (32 bytes) + CONTENTS_TYPE_LENGTH (2 bytes) - 1 + await expect(this.mock.$decodeTypedDataSig(encoded)).to.eventually.deep.equal([ + '0x', + ethers.ZeroHash, + ethers.ZeroHash, + '', + ]); + }); + + it('returns default empty values if the length is invalid', async function () { + const encoded = ethers.concat([ethers.randomBytes(64), '0x3f']); // Can't be less than 64 bytes + await expect(this.mock.$decodeTypedDataSig(encoded)).to.eventually.deep.equal([ + '0x', + ethers.ZeroHash, + ethers.ZeroHash, + '', + ]); + }); + }); + + describe('personalSignStructhash', function () { + it('should produce a personal signature EIP-712 nested type', async function () { + const text = 'Hello, world!'; + + await expect(this.mock.$personalSignStructHash(ethers.hashMessage(text))).to.eventually.equal( + ethers.TypedDataEncoder.hashStruct('PersonalSign', { PersonalSign }, ERC4337Utils.preparePersonalSign(text)), + ); + }); + }); + + describe('typedDataSignStructHash', function () { + it('should match the typed data nested struct hash', async function () { + const message = ERC4337Utils.prepareSignTypedData(this.permit, this.domain); + + const contentsHash = ethers.TypedDataEncoder.hashStruct('Permit', { Permit }, this.permit); + const hash = ethers.TypedDataEncoder.hashStruct('TypedDataSign', details.allTypes, message); + + const domainBytes = ethers.AbiCoder.defaultAbiCoder().encode( + ['bytes32', 'bytes32', 'uint256', 'address', 'bytes32'], + [ + ethers.id(this.domain.name), + ethers.id(this.domain.version), + this.domain.chainId, + this.domain.verifyingContract, + ethers.ZeroHash, + ], + ); + + await expect( + this.mock.$typedDataSignStructHash( + details.contentsTypeName, + ethers.Typed.string(details.contentsDescr), + contentsHash, + domainBytes, + ), + ).to.eventually.equal(hash); + await expect( + this.mock.$typedDataSignStructHash(details.contentsDescr, contentsHash, domainBytes), + ).to.eventually.equal(hash); + }); + }); + + describe('typedDataSignTypehash', function () { + it('should match', async function () { + const typedDataSignType = ethers.TypedDataEncoder.from(details.allTypes).encodeType('TypedDataSign'); + + await expect( + this.mock.$typedDataSignTypehash( + details.contentsTypeName, + typedDataSignType.slice(typedDataSignType.indexOf(')') + 1), + ), + ).to.eventually.equal(ethers.keccak256(ethers.toUtf8Bytes(typedDataSignType))); + }); + }); + + describe('decodeContentsDescr', function () { + const forbiddenChars = ', )\x00'; + + for (const { descr, contentsDescr, contentTypeName, contentType } of [].concat( + { + descr: 'should parse a valid descriptor (implicit)', + contentsDescr: 'SomeType(address foo,uint256 bar)', + contentTypeName: 'SomeType', + }, + { + descr: 'should parse a valid descriptor (explicit)', + contentsDescr: 'A(C c)B(A a)C(uint256 v)B', + contentTypeName: 'B', + contentType: 'A(C c)B(A a)C(uint256 v)', + }, + { descr: 'should return nothing for an empty descriptor', contentsDescr: '', contentTypeName: null }, + { descr: 'should return nothing if no [(] is present', contentsDescr: 'SomeType', contentTypeName: null }, + { + descr: 'should return nothing if starts with [(] (implicit)', + contentsDescr: '(SomeType(address foo,uint256 bar)', + contentTypeName: null, + }, + { + descr: 'should return nothing if starts with [(] (explicit)', + contentsDescr: '(SomeType(address foo,uint256 bar)(SomeType', + contentTypeName: null, + }, + forbiddenChars.split('').map(char => ({ + descr: `should return nothing if contains [${char}] (implicit)`, + contentsDescr: `SomeType${char}(address foo,uint256 bar)`, + contentTypeName: null, + })), + forbiddenChars.split('').map(char => ({ + descr: `should return nothing if contains [${char}] (explicit)`, + contentsDescr: `SomeType${char}(address foo,uint256 bar)SomeType${char}`, + contentTypeName: null, + })), + )) { + it(descr, async function () { + await expect(this.mock.$decodeContentsDescr(contentsDescr)).to.eventually.deep.equal([ + contentTypeName ?? '', + contentTypeName ? (contentType ?? contentsDescr) : '', + ]); + }); + } + }); +}); From 58c794e3f4caf91421cd4d643b5465dd513e282f Mon Sep 17 00:00:00 2001 From: ernestognw Date: Sat, 3 May 2025 01:04:29 -0600 Subject: [PATCH 7/8] Adjust ERC2771Forwarder gas to avoid GasFloorMoreThanGasLimit --- test/metatx/ERC2771Forwarder.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/metatx/ERC2771Forwarder.test.js b/test/metatx/ERC2771Forwarder.test.js index bf6cfd10c4b..07682c18747 100644 --- a/test/metatx/ERC2771Forwarder.test.js +++ b/test/metatx/ERC2771Forwarder.test.js @@ -174,7 +174,7 @@ describe('ERC2771Forwarder', function () { // Because the relayer call consumes gas until the `CALL` opcode, the gas left after failing // the subcall won't enough to finish the top level call (after testing), so we add a // moderated buffer. - const gasLimit = estimate + 2_000n; + const gasLimit = estimate + 10_000n; // The subcall out of gas should be caught by the contract and then bubbled up consuming // the available gas with an `invalid` opcode. From b4f0f48f329bf9332597a1a3ed63cfe9a7f0990c Mon Sep 17 00:00:00 2001 From: ernestognw Date: Sat, 3 May 2025 01:20:06 -0600 Subject: [PATCH 8/8] Add docs --- contracts/utils/README.adoc | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/contracts/utils/README.adoc b/contracts/utils/README.adoc index 74b26b236cc..4f6531d828a 100644 --- a/contracts/utils/README.adoc +++ b/contracts/utils/README.adoc @@ -46,7 +46,10 @@ Miscellaneous contracts and libraries containing utility functions you can use t * {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. * {Blockhash}: A library for accessing historical block hashes beyond the standard 256 block limit utilizing EIP-2935's historical blockhash functionality. - + * {AbstractSigner}: Abstract contract for internal signature validation in smart contracts. + * {ERC7739}: An abstract contract to validate signatures following the rehashing scheme from `ERC7739Utils`. + * {ERC7739Utils}: Utilities library that implements a defensive rehashing mechanism to prevent replayability of smart contract signatures based on ERC-7739. + [NOTE] ==== Because Solidity does not support generic types, {EnumerableMap} and {EnumerableSet} are specialized to a limited number of key-value types. @@ -78,6 +81,14 @@ Because Solidity does not support generic types, {EnumerableMap} and {Enumerable {{MerkleProof}} +{{ERC7739}} + +{{ERC7739Utils}} + +=== Abstract Signers + +{{AbstractSigner}} + == Security {{ReentrancyGuard}}