diff --git a/.changeset/modern-taxes-exist.md b/.changeset/modern-taxes-exist.md new file mode 100644 index 00000000000..05e17bc57d7 --- /dev/null +++ b/.changeset/modern-taxes-exist.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': patch +--- + +`AccountERC7579`: Fallback to native validation if module validation fails diff --git a/contracts/account/extensions/draft-AccountERC7579.sol b/contracts/account/extensions/draft-AccountERC7579.sol index 1e86b89a0bc..393da92898d 100644 --- a/contracts/account/extensions/draft-AccountERC7579.sol +++ b/contracts/account/extensions/draft-AccountERC7579.sol @@ -198,7 +198,7 @@ abstract contract AccountERC7579 is Account, IERC1271, IERC7579Execution, IERC75 } catch {} } } - return bytes4(0xffffffff); + return _rawSignatureValidation(hash, signature) ? IERC1271.isValidSignature.selector : bytes4(0xffffffff); } /** diff --git a/contracts/mocks/account/AccountMock.sol b/contracts/mocks/account/AccountMock.sol index ae526c8e817..66797304fef 100644 --- a/contracts/mocks/account/AccountMock.sol +++ b/contracts/mocks/account/AccountMock.sol @@ -128,6 +128,19 @@ abstract contract AccountEIP7702WithModulesMock is } } +abstract contract AccountERC7579NativeValidationMock is AccountERC7579, SignerECDSA { + constructor(address signerAddr) SignerECDSA(signerAddr) { + // no validator module + } + + function _rawSignatureValidation( + bytes32 hash, + bytes calldata signature + ) internal view virtual override(AccountERC7579, SignerECDSA) returns (bool) { + return super._rawSignatureValidation(hash, signature); + } +} + abstract contract AccountERC7579Mock is AccountERC7579 { constructor(address validator, bytes memory initData) { _installModule(MODULE_TYPE_VALIDATOR, validator, initData); diff --git a/test/account/extensions/AccountERC7579.test.js b/test/account/extensions/AccountERC7579.test.js index 83f0224fa9e..9c038898d61 100644 --- a/test/account/extensions/AccountERC7579.test.js +++ b/test/account/extensions/AccountERC7579.test.js @@ -9,24 +9,27 @@ const { shouldBehaveLikeAccountCore } = require('../Account.behavior'); const { shouldBehaveLikeAccountERC7579 } = require('./AccountERC7579.behavior'); const { shouldBehaveLikeERC1271 } = require('../../utils/cryptography/ERC1271.behavior'); -async function fixture() { +const fixtureWithValidator = () => fixture(true); // parameters not supported by `loadFixture` +const fixtureWithoutValidator = () => fixture(false); + +async function fixture(withValidator) { // EOAs and environment const [other] = await ethers.getSigners(); const target = await ethers.deployContract('CallReceiverMock'); const anotherTarget = await ethers.deployContract('CallReceiverMock'); // ERC-7579 validator - const validator = await ethers.deployContract('$ERC7579ValidatorMock'); + const validator = withValidator ? await ethers.deployContract('$ERC7579ValidatorMock') : null; // ERC-4337 signer const signer = ethers.Wallet.createRandom(); // ERC-4337 account const helper = new ERC4337Helper(); - const mock = await helper.newAccount('$AccountERC7579Mock', [ - validator, - ethers.solidityPacked(['address'], [signer.address]), - ]); + const mock = await helper.newAccount( + validator ? '$AccountERC7579Mock' : '$AccountERC7579NativeValidationMock', + validator ? [validator, ethers.solidityPacked(['address'], [signer.address])] : [signer.address], + ); // ERC-4337 Entrypoint domain const entrypointDomain = await getDomain(predeploy.entrypoint.v09); @@ -34,27 +37,30 @@ async function fixture() { return { helper, validator, mock, entrypointDomain, signer, target, anotherTarget, other }; } -describe('AccountERC7579', function () { - beforeEach(async function () { - Object.assign(this, await loadFixture(fixture)); - - this.signer.signMessage = message => - ethers.Wallet.prototype.signMessage - .bind(this.signer)(message) - .then(sign => ethers.concat([this.validator.target, sign])); - this.signer.signTypedData = (domain, types, values) => - ethers.Wallet.prototype.signTypedData - .bind(this.signer)(domain, types, values) - .then(sign => ethers.concat([this.validator.target, sign])); - this.signUserOp = userOp => - ethers.Wallet.prototype.signTypedData - .bind(this.signer)(this.entrypointDomain, { PackedUserOperation }, userOp.packed) - .then(signature => Object.assign(userOp, { signature })); - - this.userOp = { nonce: ethers.zeroPadBytes(ethers.hexlify(this.validator.target), 32) }; - }); - - shouldBehaveLikeAccountCore(); - shouldBehaveLikeAccountERC7579(); - shouldBehaveLikeERC1271(); -}); +[true, false].forEach(withValidator => + describe(`AccountERC7579 ${withValidator ? '' : 'native signer validation fallback'}`, function () { + beforeEach(async function () { + Object.assign(this, await loadFixture(withValidator ? fixtureWithValidator : fixtureWithoutValidator)); + this.signer.signMessage = message => + ethers.Wallet.prototype.signMessage + .bind(this.signer)(message) + .then(sign => (withValidator ? ethers.concat([this.validator.target, sign]) : sign)); + this.signer.signTypedData = (domain, types, values) => + ethers.Wallet.prototype.signTypedData + .bind(this.signer)(domain, types, values) + .then(sign => (withValidator ? ethers.concat([this.validator.target, sign]) : sign)); + this.signUserOp = userOp => + ethers.Wallet.prototype.signTypedData + .bind(this.signer)(this.entrypointDomain, { PackedUserOperation }, userOp.packed) + .then(signature => Object.assign(userOp, { signature })); + + this.userOp = withValidator ? { nonce: ethers.zeroPadBytes(ethers.hexlify(this.validator.target), 32) } : {}; + }); + + if (withValidator) { + shouldBehaveLikeAccountCore(); + shouldBehaveLikeAccountERC7579(); + } + shouldBehaveLikeERC1271(); + }), +);