diff --git a/contracts/account/README.adoc b/contracts/account/README.adoc index 0b286551..b78b8a44 100644 --- a/contracts/account/README.adoc +++ b/contracts/account/README.adoc @@ -6,8 +6,10 @@ This directory includes contracts to build accounts for ERC-4337. These include: * {AccountCore}: An ERC-4337 smart account implementation that includes the core logic to process user operations. * {Account}: An extension of `AccountCore` that implements the recommended features for ERC-4337 smart accounts. - * {AccountSignerERC7702}: An account implementation with low-level signature validation performed by an EOA. + * {AccountERC7579}: An extension of `AccountCore` that implements support for ERC-7579 modules. + * {AccountERC7579Hooked}: An extension of `AccountERC7579` with support for a single hook module (type 4). * {ERC7821}: Minimal batch executor implementation contracts. Useful to enable easy batch execution for smart contracts. + * {ERC7579SocialRecoveryExecutor}: A social recovery module for ERC-7579 accounts. == Core @@ -17,10 +19,18 @@ This directory includes contracts to build accounts for ERC-4337. These include: == Extensions -{{SignerERC7702}} +{{AccountERC7579}} -{{AccountERC7759}} - -{{AccountERC7759Hooked}} +{{AccountERC7579Hooked}} {{ERC7821}} + +{{ERC7579Modules}} + +== ERC7579 Modules + +This section includes several ERC-7579 Module implementations for {AccountERC7579}: + + * {ERC7579SocialRecoveryExecutor}: A social recovery module enabling account reconfiguration through a timelocked guardian-based consensus mechanism. + +{{ERC7579SocialRecoveryExecutor}} \ No newline at end of file diff --git a/contracts/account/extensions/ERC7579Modules/ERC7579SocialRecoveryExecutor.sol b/contracts/account/extensions/ERC7579Modules/ERC7579SocialRecoveryExecutor.sol new file mode 100644 index 00000000..ebc56243 --- /dev/null +++ b/contracts/account/extensions/ERC7579Modules/ERC7579SocialRecoveryExecutor.sol @@ -0,0 +1,436 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.24; + +import {IERC7579Module, MODULE_TYPE_EXECUTOR} from "@openzeppelin/contracts/interfaces/draft-IERC7579.sol"; +import {SignatureChecker} from "@openzeppelin/contracts/utils/cryptography/SignatureChecker.sol"; +import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; +import {EIP712} from "@openzeppelin/contracts/utils/cryptography/EIP712.sol"; +import {Address} from "@openzeppelin/contracts/utils/Address.sol"; +import {Nonces} from "@openzeppelin/contracts/utils/Nonces.sol"; + +/** + * @title Social Recovery Executor Module + * + * @dev Social recovery module enabling account reconfiguration through a timelocked + * guardian-based consensus mechanism. Provides M-of-N guardian multi-signature with + * timelock protection, recovery cancellation, and configurable security parameters. + */ +contract ERC7579SocialRecoveryExecutor is IERC7579Module, EIP712, Nonces { + using EnumerableSet for EnumerableSet.AddressSet; + + /// @dev Status of the recovery process used to determine what actions are currently allowed + /// @param NotStarted No recovery process has started + /// @param Started Recovery process has started but timelock hasn't expired + /// @param Ready Recovery process has passed timelock and is ready for execution + enum RecoveryStatus { + NotStarted, + Started, + Ready + } + + /// @dev Recovery configuration for an ERC-7579 Account containing all necessary information for the recovery process + struct RecoveryConfig { + EnumerableSet.AddressSet guardians; + bytes32 pendingExecutionHash; + uint256 recoveryStart; + uint256 threshold; + uint256 timelock; + } + + /// @dev Structure representing a guardian's signature used for verification in recovery operations + /// @param signature The cryptographic signature bytes + /// @param signer The address of the guardian who signed + struct GuardianSignature { + bytes signature; + address signer; + } + + /*////////////////////////////////////////////////////////////////////////// + CONSTANTS & STORAGE + //////////////////////////////////////////////////////////////////////////*/ + + bytes32 private constant START_RECOVERY_TYPEHASH = + keccak256("StartRecovery(address account,bytes executionCalldata,uint256 nonce)"); + + bytes32 private constant CANCEL_RECOVERY_TYPEHASH = keccak256("CancelRecovery(address account,uint256 nonce)"); + + mapping(address account => RecoveryConfig recoveryConfig) private _recoveryConfigs; + + /*////////////////////////////////////////////////////////////////////////// + EVENTS & ERRORS + //////////////////////////////////////////////////////////////////////////*/ + + event ModuleUninstalledReceived(address indexed account, bytes data); + event ModuleInstalledReceived(address indexed account, bytes data); + + event RecoveryCancelled(address indexed account); + event RecoveryExecuted(address indexed account); + event RecoveryStarted(address indexed account); + + event ThresholdChanged(address indexed account, uint256 threshold); + event TimelockChanged(address indexed account, uint256 timelock); + event GuardianRemoved(address indexed account, address guardian); + event GuardianAdded(address indexed account, address guardian); + + error DuplicatedOrUnsortedGuardianSignatures(); + error ExecutionDiffersFromPending(); + error TooManyGuardianSignatures(); + error InvalidGuardianSignature(); + error ThresholdNotMet(); + + error RecoveryAlreadyStarted(); + error RecoveryNotStarted(); + error RecoveryNotReady(); + + error InvalidGuardianLength(); + error InvalidThreshold(); + error InvalidTimelock(); + + error CannotRemoveGuardian(); + error GuardianNotFound(); + error TooManyGuardians(); + error AlreadyGuardian(); + + /*/////////////////////////////////////////////////////////////////////////// + MODIFIERS + //////////////////////////////////////////////////////////////////////////*/ + + /// @dev Reverts if the recovery process has already started. + modifier whenRecoveryIsNotStarted(address account) { + if (getRecoveryStatus(account) != RecoveryStatus.NotStarted) { + revert RecoveryAlreadyStarted(); + } + _; + } + + /// @dev Reverts if the recovery process is not ready to be executed. + modifier whenRecoveryIsReady(address account) { + if (getRecoveryStatus(account) != RecoveryStatus.Ready) { + revert RecoveryNotReady(); + } + _; + } + + /// @dev Reverts if the recovery process is not started or ready. + modifier whenRecoveryIsStartedOrReady(address account) { + if (getRecoveryStatus(account) == RecoveryStatus.NotStarted) { + revert RecoveryNotStarted(); + } + _; + } + + /*////////////////////////////////////////////////////////////////////////// + MODULE SETUP CONFIGURATION + //////////////////////////////////////////////////////////////////////////*/ + + /// @dev Initializes the module with a name and version. + constructor(string memory name, string memory version) EIP712(name, version) {} + + /// @inheritdoc IERC7579Module + function isModuleType(uint256 moduleTypeId) external pure returns (bool) { + return moduleTypeId == MODULE_TYPE_EXECUTOR; + } + + /// @inheritdoc IERC7579Module + function onInstall(bytes memory data) public virtual override { + address account = msg.sender; + + (address[] memory _guardians, uint256 _threshold, uint256 _timelock) = abi.decode( + data, + (address[], uint256, uint256) + ); + + if (_guardians.length == 0) { + revert InvalidGuardianLength(); + } + + for (uint256 i = 0; i < _guardians.length; i++) { + _addGuardian(account, _guardians[i]); + } + + _setThreshold(account, _threshold); + _setTimelock(account, _timelock); + + emit ModuleInstalledReceived(account, data); + } + + /// @inheritdoc IERC7579Module + function onUninstall(bytes calldata data) public virtual override { + address account = msg.sender; + + // clear the guardian EnumerableSet. + _recoveryConfigs[account].guardians.clear(); + + // slither-disable-next-line mapping-deletion + delete _recoveryConfigs[account]; + + emit ModuleUninstalledReceived(account, data); + } + + /*////////////////////////////////////////////////////////////////////////// + MODULE MAIN LOGIC + //////////////////////////////////////////////////////////////////////////*/ + + /// @dev Starts the recovery process for an ERC-7579 Account. Requires threshold number of valid guardian signatures and commits the hash of the approved execution calldata for recovery. + /// @param account The ERC-7579 Account to start recovery for + /// @param guardianSignatures Array of guardian signatures authorizing the recovery. Each signature contains a signature and the signer's address, sorted by signer address in ascending order. + /// @param executionCalldata The calldata to execute during recovery. This defines the account reconfiguration to perform. + /// @custom:security Uses EIP-712 for signature verification and nonces for replay protection + function startRecovery( + address account, + GuardianSignature[] calldata guardianSignatures, + bytes calldata executionCalldata + ) public virtual whenRecoveryIsNotStarted(account) { + bytes32 digest = _getStartRecoveryDigest(account, executionCalldata, _useNonce(account)); + _validateGuardianSignatures(account, guardianSignatures, digest); + + // store and start the recovery process. + _recoveryConfigs[account].pendingExecutionHash = keccak256(executionCalldata); + _recoveryConfigs[account].recoveryStart = block.timestamp; + + emit RecoveryStarted(account); + } + + /// @dev Executes the recovery process after timelock period has passed. Only callable when recovery status is Ready. Validates that execution matches the hash committed during startRecovery and resets recovery state afterward. + /// @param account The account to execute recovery for + /// @param executionCalldata The calldata to execute, must match the pending recovery hash stored during startRecovery + /// @custom:security Validates execution matches the hash committed during startRecovery and resets recovery state afterward + function executeRecovery( + address account, + bytes calldata executionCalldata + ) public virtual whenRecoveryIsReady(account) { + if (keccak256(executionCalldata) != _recoveryConfigs[account].pendingExecutionHash) { + revert ExecutionDiffersFromPending(); + } + + // reset recovery status. + _recoveryConfigs[account].pendingExecutionHash = bytes32(0); + _recoveryConfigs[account].recoveryStart = 0; + + // execute the recovery. + // slither-disable-next-line reentrancy-no-eth + Address.functionCall(account, executionCalldata); + + emit RecoveryExecuted(account); + } + + /// @dev Cancels an ongoing recovery process. Can only be called by the account itself for self-cancellation. + /// @custom:security Direct account control takes precedence over recovery process + function cancelRecovery() public virtual whenRecoveryIsStartedOrReady(msg.sender) { + _cancelRecovery(msg.sender); + } + + /// @dev Allows guardians to cancel a recovery process. Requires threshold signatures from guardians, similar to starting recovery. + /// Uses same signature threshold and verification process as recovery initiation. + /// @param account The account to cancel recovery for + /// @param guardianSignatures Array of guardian signatures authorizing cancellation, sorted by signer address + /// @custom:security Uses same signature threshold and verification process as recovery initiation + function cancelRecovery( + address account, + GuardianSignature[] calldata guardianSignatures + ) public virtual whenRecoveryIsStartedOrReady(account) { + bytes32 digest = _getCancelRecoveryDigest(account, _useNonce(account)); + _validateGuardianSignatures(account, guardianSignatures, digest); + + _cancelRecovery(account); + } + + /*////////////////////////////////////////////////////////////////////////// + MODULE MANAGEMENT + //////////////////////////////////////////////////////////////////////////*/ + + /// @dev Adds a new guardian to the account's recovery configuration. Only callable by the account itself. + /// @param guardian Address of the new guardian + function addGuardian(address guardian) public virtual { + _addGuardian(msg.sender, guardian); + } + + /// @dev Removes a guardian from the account's recovery configuration. Only callable by the account itself. + /// @param guardian Address of the guardian to remove + function removeGuardian(address guardian) public virtual { + _removeGuardian(msg.sender, guardian); + } + + /// @dev Changes the number of required guardian signatures. Only callable by the account itself. + /// @param threshold New threshold value + function updateThreshold(uint256 threshold) public virtual { + _setThreshold(msg.sender, threshold); + } + + /// @dev Changes the timelock duration for recovery. Only callable by the account itself. + /// @param timelock New timelock duration in seconds + function updateTimelock(uint256 timelock) public virtual { + _setTimelock(msg.sender, timelock); + } + + /*////////////////////////////////////////////////////////////////////////// + VIEW FUNCTIONS + //////////////////////////////////////////////////////////////////////////*/ + + /// @dev Gets the current recovery status of an ERC-7579 Account. Determines status based on recoveryStart timestamp and timelock duration. + /// @param account The ERC-7579 Account to get the recovery status for + /// @return Status enum value: NotStarted (0), Started (1), or Ready (2) + function getRecoveryStatus(address account) public view virtual returns (RecoveryStatus) { + uint256 recoveryStart = _recoveryConfigs[account].recoveryStart; + if (recoveryStart == 0) { + return RecoveryStatus.NotStarted; + } + if (block.timestamp < recoveryStart + _recoveryConfigs[account].timelock) { + return RecoveryStatus.Started; + } + return RecoveryStatus.Ready; + } + + /// @dev Checks if an address is a guardian for an ERC-7579 Account. + /// @param account The ERC-7579 Account to check guardians for + /// @param guardian The address to check as a guardian + /// @return true if the address is a guardian, false otherwise + function isGuardian(address account, address guardian) public view virtual returns (bool) { + return _recoveryConfigs[account].guardians.contains(guardian); + } + + /// @dev Gets all guardians for an ERC-7579 Account. + /// @param account The ERC-7579 Account to get guardians for + /// @return An array of all guardians + function getGuardians(address account) public view virtual returns (address[] memory) { + return _recoveryConfigs[account].guardians.values(); + } + + /// @dev Gets the threshold for an ERC-7579 Account. + /// @param account The ERC-7579 Account to get the threshold for + /// @return The threshold value + function getThreshold(address account) public view virtual returns (uint256) { + return _recoveryConfigs[account].threshold; + } + + /// @dev Gets the timelock for an ERC-7579 Account. + /// @param account The ERC-7579 Account to get the timelock for + /// @return The timelock value + function getTimelock(address account) public view virtual returns (uint256) { + return _recoveryConfigs[account].timelock; + } + + /// @dev Gets the maximum number of guardians for an ERC-7579 Account. + /// @return The maximum number of guardians + function getMaxGuardians() public pure virtual returns (uint256) { + return 32; + } + + /*////////////////////////////////////////////////////////////////////////// + INTERNAL FUNCTIONS + //////////////////////////////////////////////////////////////////////////*/ + + /// @dev EIP-712 digest for starting recovery. + /// @param account The ERC-7579 Account to start recovery for + /// @param executionCalldata The calldata to execute during recovery + /// @return The EIP-712 digest for starting recovery + function _getStartRecoveryDigest( + address account, + bytes calldata executionCalldata, + uint256 nonce + ) internal view virtual returns (bytes32) { + bytes32 structHash = keccak256( + abi.encode(START_RECOVERY_TYPEHASH, account, keccak256(executionCalldata), nonce) + ); + return _hashTypedDataV4(structHash); + } + + /// @dev EIP-712 digest for cancelling recovery. + /// @param account The ERC-7579 Account to cancel recovery for + /// @param nonce The nonce of the account + /// @return The EIP-712 digest for cancelling recovery + function _getCancelRecoveryDigest(address account, uint256 nonce) internal view virtual returns (bytes32) { + bytes32 structHash = keccak256(abi.encode(CANCEL_RECOVERY_TYPEHASH, account, nonce)); + return _hashTypedDataV4(structHash); + } + + /// @dev Verifies multiple guardian signatures for a given digest. Ensures signatures are unique, properly sorted, and threshold is met. + /// @param account The account the signatures are for + /// @param guardianSignatures Array of guardian signatures sorted by signer address in ascending order + /// @param digest The EIP-712 typed data digest to verify the signatures against + /// @custom:security Enforces ascending order of signers to prevent duplicates + function _validateGuardianSignatures( + address account, + GuardianSignature[] calldata guardianSignatures, + bytes32 digest + ) internal view virtual { + // bound `for` cycle + if (guardianSignatures.length > getMaxGuardians()) { + revert TooManyGuardianSignatures(); + } + if (guardianSignatures.length < _recoveryConfigs[account].threshold) { + revert ThresholdNotMet(); + } + + address lastSigner = address(0); + for (uint256 i = 0; i < guardianSignatures.length; i++) { + address signer = guardianSignatures[i].signer; + if ( + !isGuardian(account, signer) || + !SignatureChecker.isValidSignatureNow(signer, digest, guardianSignatures[i].signature) + ) { + revert InvalidGuardianSignature(); + } + if (uint160(signer) <= uint160(lastSigner)) { + revert DuplicatedOrUnsortedGuardianSignatures(); + } + lastSigner = signer; + } + } + + /// @dev Cancels an ongoing recovery process. + /// @param account The ERC-7579 Account to cancel recovery for + function _cancelRecovery(address account) internal virtual { + _recoveryConfigs[account].pendingExecutionHash = bytes32(0); + _recoveryConfigs[account].recoveryStart = 0; + emit RecoveryCancelled(account); + } + + /// @dev Adds a new guardian to the account's recovery configuration. + /// @param account The ERC-7579 Account to add the guardian to + /// @param guardian Address of the new guardian + function _addGuardian(address account, address guardian) internal virtual { + if (_recoveryConfigs[account].guardians.length() >= getMaxGuardians()) { + revert TooManyGuardians(); + } + if (!_recoveryConfigs[account].guardians.add(guardian)) { + revert AlreadyGuardian(); + } + emit GuardianAdded(account, guardian); + } + + /// @dev Removes a guardian from the account's recovery configuration. Cannot remove if it would make threshold unreachable. + /// @param account The ERC-7579 Account to remove the guardian from + /// @param guardian Address of the guardian to remove + function _removeGuardian(address account, address guardian) internal virtual { + if (_recoveryConfigs[account].guardians.length() == _recoveryConfigs[account].threshold) { + revert CannotRemoveGuardian(); + } + if (!_recoveryConfigs[account].guardians.remove(guardian)) { + revert GuardianNotFound(); + } + emit GuardianRemoved(account, guardian); + } + + /// @dev Changes the number of required guardian signatures. Cannot be set to zero and cannot be greater than the current number of guardians. + /// @param account The ERC-7579 Account to change the threshold for + /// @param threshold New threshold value + function _setThreshold(address account, uint256 threshold) internal virtual { + if (threshold == 0 || threshold > _recoveryConfigs[account].guardians.length()) { + revert InvalidThreshold(); + } + _recoveryConfigs[account].threshold = threshold; + emit ThresholdChanged(account, threshold); + } + + /// @dev Changes the timelock duration for recovery. Cannot be set to zero. + /// @param account The ERC-7579 Account to change the timelock for + /// @param timelock New timelock duration in seconds + function _setTimelock(address account, uint256 timelock) internal virtual { + if (timelock == 0) { + revert InvalidTimelock(); + } + _recoveryConfigs[account].timelock = timelock; + emit TimelockChanged(account, timelock); + } +} diff --git a/contracts/mocks/account/modules/ERC7579SocialRecoveryExecutorMock.sol b/contracts/mocks/account/modules/ERC7579SocialRecoveryExecutorMock.sol new file mode 100644 index 00000000..5099a0cc --- /dev/null +++ b/contracts/mocks/account/modules/ERC7579SocialRecoveryExecutorMock.sol @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.24; + +import {ERC7579SocialRecoveryExecutor} from "../../../account/extensions/ERC7579Modules/ERC7579SocialRecoveryExecutor.sol"; + +contract ERC7579SocialRecoveryExecutorMock is ERC7579SocialRecoveryExecutor { + constructor(string memory name, string memory version) ERC7579SocialRecoveryExecutor(name, version) {} + + // helper for testing signature validation + function validateGuardianSignatures( + address account, + GuardianSignature[] calldata guardianSignatures, + bytes32 digest + ) public view virtual { + super._validateGuardianSignatures(account, guardianSignatures, digest); + } +} diff --git a/contracts/mocks/account/modules/ERC7579ValidatorMock.sol b/contracts/mocks/account/modules/ERC7579ValidatorMock.sol index 3eb685e0..a6588223 100644 --- a/contracts/mocks/account/modules/ERC7579ValidatorMock.sol +++ b/contracts/mocks/account/modules/ERC7579ValidatorMock.sol @@ -22,6 +22,14 @@ abstract contract ERC7579ValidatorMock is ERC7579ModuleMock(MODULE_TYPE_VALIDATO super.onUninstall(data); } + function updateSigner(address newSigner) public virtual { + _associatedSigners[msg.sender] = newSigner; + } + + function getSigner(address sender) public view virtual returns (address) { + return _associatedSigners[sender]; + } + function validateUserOp( PackedUserOperation calldata userOp, bytes32 userOpHash diff --git a/lib/@openzeppelin-contracts b/lib/@openzeppelin-contracts index 441dc141..930598ed 160000 --- a/lib/@openzeppelin-contracts +++ b/lib/@openzeppelin-contracts @@ -1 +1 @@ -Subproject commit 441dc141ac99622de7e535fa75dfc74af939019c +Subproject commit 930598edfb6241a179ade6ad44ba59ed8b68f7d8 diff --git a/lib/@openzeppelin-contracts-upgradeable b/lib/@openzeppelin-contracts-upgradeable index 266b24b1..8ab1f5ac 160000 --- a/lib/@openzeppelin-contracts-upgradeable +++ b/lib/@openzeppelin-contracts-upgradeable @@ -1 +1 @@ -Subproject commit 266b24b1338f88281040cab1e805f96795d59d3e +Subproject commit 8ab1f5acf958b937531cee87f99ae4c0242f0dee diff --git a/lib/forge-std b/lib/forge-std index 3b20d60d..bf909b22 160000 --- a/lib/forge-std +++ b/lib/forge-std @@ -1 +1 @@ -Subproject commit 3b20d60d14b343ee4f908cb8079495c07f5e8981 +Subproject commit bf909b22fa55e244796dfa920c9639fdffa1c545 diff --git a/test/account/extensions/ERC7579Modules/SocialRecoveryExecutorModule.test.js b/test/account/extensions/ERC7579Modules/SocialRecoveryExecutorModule.test.js new file mode 100644 index 00000000..12339b62 --- /dev/null +++ b/test/account/extensions/ERC7579Modules/SocialRecoveryExecutorModule.test.js @@ -0,0 +1,573 @@ +const { expect } = require('chai'); +const { ethers, entrypoint } = require('hardhat'); +const { loadFixture, time } = require('@nomicfoundation/hardhat-network-helpers'); +const { impersonate } = require('@openzeppelin/contracts/test/helpers/account'); +const { + MODULE_TYPE_EXECUTOR, + encodeMode, + CALL_TYPE_SINGLE, + EXEC_TYPE_DEFAULT, + encodeSingle, +} = require('@openzeppelin/contracts/test/helpers/erc7579'); +const { PackedUserOperation } = require('../../../helpers/eip712-types'); +const { ERC4337Helper } = require('../../../helpers/erc4337'); +const { ERC7579SocialRecoveryExecutorHelper } = require('../../../helpers/erc7579-modules'); +const { SIG_VALIDATION_SUCCESS, SIG_VALIDATION_FAILURE } = require('@openzeppelin/contracts/test/helpers/erc4337'); +const { getDomain } = require('@openzeppelin/contracts/test/helpers/eip712'); + +async function fixture() { + // ERC-7579 validator + const validatorMock = await ethers.deployContract('$ERC7579ValidatorMock'); + + // ERC-4337 signers + const initialSigner = ethers.Wallet.createRandom(); + const newSigner = ethers.Wallet.createRandom(); + + // ERC-4337 account + const erc4337Helper = new ERC4337Helper(); + const env = await erc4337Helper.wait(); + const accountMock = await erc4337Helper.newAccount('$AccountERC7579Mock', [ + 'AccountERC7579', + '1', + validatorMock.target, + initialSigner.address, + ]); + await accountMock.deploy(); + + // domain cannot be fetched using getDomain(mock) before the mock is deployed + const domain = { + name: 'AccountERC7579', + version: '1', + chainId: env.chainId, + verifyingContract: accountMock.address, + }; + + const signUserOpWithSigner = (userOp, signer) => + signer + .signTypedData(domain, { PackedUserOperation }, userOp.packed) + .then(signature => Object.assign(userOp, { signature })); + + const userOp = { + // Use the first 20 bytes from the nonce key (24 bytes) to identify the validator module + nonce: ethers.zeroPadBytes(ethers.hexlify(validatorMock.target), 32), + }; + + // impersonate ERC-4337 Canonical Entrypoint + const accountMockFromEntrypoint = accountMock.connect(await impersonate(entrypoint.target)); + + // ERC-7579 Social Recovery Executor Module + const mock = await ethers.deployContract('$ERC7579SocialRecoveryExecutorMock', [ + 'ERC7579SocialRecoveryExecutorMock', + '0.0.1', + ]); + + // ERC-7579 Social Recovery Executor Module Initial Config + const recoveryConfig = { + guardians: new Array(3).fill(null).map(() => ethers.Wallet.createRandom()), + threshold: 2, + timelock: time.duration.days(1), + }; + + return { + ...env, + validatorMock, + accountMock, + domain, + initialSigner, + newSigner, + signUserOpWithSigner, + userOp, + mock, + recoveryConfig, + accountMockFromEntrypoint, + }; +} + +describe('SocialRecoveryExecutorModule', function () { + beforeEach(async function () { + Object.assign(this, await loadFixture(fixture)); + }); + + it('should not be installed', async function () { + expect(await this.accountMock.isModuleInstalled(MODULE_TYPE_EXECUTOR, this.mock.target, '0x')).to.equal(false); + }); + + describe('with installed module', function () { + beforeEach(async function () { + this.initData = ethers.AbiCoder.defaultAbiCoder().encode( + ['address[]', 'uint256', 'uint256'], + [ + this.recoveryConfig.guardians.map(g => g.address), + this.recoveryConfig.threshold, + this.recoveryConfig.timelock, + ], + ); + await expect(this.accountMockFromEntrypoint.installModule(MODULE_TYPE_EXECUTOR, this.mock.target, this.initData)) + .to.emit(this.accountMock, 'ModuleInstalled') + .withArgs(MODULE_TYPE_EXECUTOR, this.mock.target) + .to.emit(this.mock, 'ModuleInstalledReceived') + .withArgs(this.accountMock.target, this.initData); + }); + + it('should ensure module has been installed', async function () { + expect(await this.accountMock.isModuleInstalled(MODULE_TYPE_EXECUTOR, this.mock.target, '0x')).to.equal(true); + }); + + it('should be able to uninstall module', async function () { + await expect(this.accountMockFromEntrypoint.uninstallModule(MODULE_TYPE_EXECUTOR, this.mock.target, '0x')) + .to.emit(this.accountMock, 'ModuleUninstalled') + .withArgs(MODULE_TYPE_EXECUTOR, this.mock.target) + .to.emit(this.mock, 'ModuleUninstalledReceived') + .withArgs(this.accountMock.target, '0x'); + + expect(await this.accountMock.isModuleInstalled(MODULE_TYPE_EXECUTOR, this.mock.target, '0x')).to.equal(false); + + const guardians = await this.mock.getGuardians(this.accountMock.target); + expect(guardians).to.deep.equal([]); + + const threshold = await this.mock.getThreshold(this.accountMock.target); + expect(threshold).to.equal(0); + + const timelock = await this.mock.getTimelock(this.accountMock.target); + expect(timelock).to.equal(0); + + const nonce = await this.mock.nonces(this.accountMock.target); + expect(nonce).to.equal(0); + }); + + describe('signature validation', function () { + it('should recognize guardians', async function () { + const isGuardian = await this.mock.isGuardian( + this.accountMock.target, + this.recoveryConfig.guardians[0].address, + ); + expect(isGuardian).to.equal(true); + }); + + it('should invalidate signatures from invalid guardians', async function () { + const guardians = this.recoveryConfig.guardians.slice(0, 3); + const invalidGuardian = ethers.Wallet.createRandom(); + const invalidGuardians = [...guardians, invalidGuardian]; + + const message = 'Hello Social Recovery'; + const digest = ethers.hashMessage(message); + + const guardianSignatures = ERC7579SocialRecoveryExecutorHelper.sortGuardianSignatures( + invalidGuardians.map(g => ({ + signer: g.address, + signature: g.signMessage(message), + })), + ); + + await expect( + this.mock.validateGuardianSignatures(this.accountMock.target, guardianSignatures, digest), + ).to.be.revertedWithCustomError(this.mock, 'InvalidGuardianSignature'); + }); + + it('should invalidate unsorted guardian signatures', async function () { + const guardians = this.recoveryConfig.guardians.slice(0, 3); + const reversedGuardians = [...guardians] + .sort((a, b) => (BigInt(a.address) > BigInt(b.address) ? 1 : -1)) + .reverse(); + + const message = 'Hello Social Recovery'; + const digest = ethers.hashMessage(message); + + const guardianSignatures = reversedGuardians.map(g => ({ + signer: g.address, + signature: g.signMessage(message), + })); + + await expect( + this.mock.validateGuardianSignatures(this.accountMock.target, guardianSignatures, digest), + ).to.be.revertedWithCustomError(this.mock, 'DuplicatedOrUnsortedGuardianSignatures'); + }); + + it('should invalidate duplicate guardian signatures', async function () { + const guardian = this.recoveryConfig.guardians[0]; + const identicalGuardians = [guardian, guardian]; + + const message = 'Hello Social Recovery'; + const digest = ethers.hashMessage(message); + + const guardianSignatures = ERC7579SocialRecoveryExecutorHelper.sortGuardianSignatures( + identicalGuardians.map(g => ({ + signer: g.address, + signature: g.signMessage(message), + })), + ); + + await expect( + this.mock.validateGuardianSignatures(this.accountMock.target, guardianSignatures, digest), + ).to.be.revertedWithCustomError(this.mock, 'DuplicatedOrUnsortedGuardianSignatures'); + }); + + it('should fail if threshold is not met', async function () { + const insufficientGuardians = this.recoveryConfig.guardians.slice(0, this.recoveryConfig.threshold - 1); + const message = 'Hello Social Recovery'; + const digest = ethers.hashMessage(message); + + const guardianSignatures = ERC7579SocialRecoveryExecutorHelper.sortGuardianSignatures( + insufficientGuardians.map(g => ({ + signer: g.address, + signature: g.signMessage(message), + })), + ); + + await expect( + this.mock.validateGuardianSignatures(this.accountMock.target, guardianSignatures, digest), + ).to.be.revertedWithCustomError(this.mock, 'ThresholdNotMet'); + }); + + it('should validate valid guardian signatures', async function () { + const guardians = this.recoveryConfig.guardians.slice(0, this.recoveryConfig.threshold); + const message = 'Hello Social Recovery'; + const digest = ethers.hashMessage(message); + + const guardianSignatures = ERC7579SocialRecoveryExecutorHelper.sortGuardianSignatures( + guardians.map(g => ({ + signer: g.address, + signature: g.signMessage(message), + })), + ); + + await expect(this.mock.validateGuardianSignatures(this.accountMock.target, guardianSignatures, digest)).to.not + .be.reverted; + }); + }); + + describe('recovery', function () { + it('status should be not started', async function () { + const status = await this.mock.getRecoveryStatus(this.accountMock.target); + expect(status).to.equal(ERC7579SocialRecoveryExecutorHelper.RecoveryStatus.NotStarted); + }); + + describe('with recovery started', function () { + beforeEach(async function () { + const guardians = this.recoveryConfig.guardians.slice(0, this.recoveryConfig.threshold); + const domain = await getDomain(this.mock); + + const encodedCallToValidatorModule = this.validatorMock.interface.encodeFunctionData('updateSigner', [ + this.newSigner.address, + ]); + const recoveryCallData = encodeSingle(this.validatorMock.target, 0, encodedCallToValidatorModule); + const executionCalldata = this.accountMock.interface.encodeFunctionData('executeFromExecutor', [ + encodeMode(CALL_TYPE_SINGLE, EXEC_TYPE_DEFAULT), + recoveryCallData, + ]); + + const message = { + account: this.accountMock.target, + nonce: await this.mock.nonces(this.accountMock.target), + executionCalldata: executionCalldata, + }; + + const guardianSignatures = ERC7579SocialRecoveryExecutorHelper.sortGuardianSignatures( + guardians.map(g => ({ + signer: g.address, + signature: g.signTypedData(domain, ERC7579SocialRecoveryExecutorHelper.START_RECOVERY_TYPEHASH, message), + })), + ); + + await expect(this.mock.startRecovery(this.accountMock.target, guardianSignatures, executionCalldata)) + .to.emit(this.mock, 'RecoveryStarted') + .withArgs(this.accountMock.target); + }); + + it('status should be started', async function () { + const status = await this.mock.getRecoveryStatus(this.accountMock.target); + expect(status).to.equal(ERC7579SocialRecoveryExecutorHelper.RecoveryStatus.Started); + }); + + it('should not be able to start recovery again', async function () { + await expect(this.mock.startRecovery(this.accountMock.target, [], '0x')).to.be.revertedWithCustomError( + this.mock, + 'RecoveryAlreadyStarted', + ); + }); + + describe('execute recovery', function () { + it('should fail to execute if timelock is not met', async function () { + await expect(this.mock.executeRecovery(this.accountMock.target, '0x')).to.be.revertedWithCustomError( + this.mock, + 'RecoveryNotReady', + ); + }); + + describe('with timelock met', function () { + beforeEach(async function () { + await time.increase(time.duration.days(1)); + }); + + it('should fail if the execution calldata differs from the signed by guardians', async function () { + const differentNewSigner = ethers.Wallet.createRandom(); + + const encodedCallToValidatorModule = this.validatorMock.interface.encodeFunctionData('updateSigner', [ + differentNewSigner.address, + ]); + const recoveryCallData = encodeSingle(this.validatorMock.target, 0, encodedCallToValidatorModule); + const executionCalldata = this.accountMock.interface.encodeFunctionData('executeFromExecutor', [ + encodeMode(CALL_TYPE_SINGLE, EXEC_TYPE_DEFAULT), + recoveryCallData, + ]); + + await expect( + this.mock.executeRecovery(this.accountMock.target, executionCalldata), + ).to.be.revertedWithCustomError(this.mock, 'ExecutionDiffersFromPending'); + }); + + it('new signer should not be able to validate himself on the account yet', async function () { + const operation = await this.accountMock + .createUserOp(this.userOp) + .then(op => this.signUserOpWithSigner(op, this.newSigner)); + + await expect( + this.accountMockFromEntrypoint.validateUserOp.staticCall(operation.packed, operation.hash(), 0), + ).to.eventually.equal(SIG_VALIDATION_FAILURE); + }); + + describe('with recovery executed successfully', function () { + beforeEach(async function () { + const encodedCallToValidatorModule = this.validatorMock.interface.encodeFunctionData('updateSigner', [ + this.newSigner.address, + ]); + const recoveryCallData = encodeSingle(this.validatorMock.target, 0, encodedCallToValidatorModule); + const executionCalldata = this.accountMock.interface.encodeFunctionData('executeFromExecutor', [ + encodeMode(CALL_TYPE_SINGLE, EXEC_TYPE_DEFAULT), + recoveryCallData, + ]); + + await expect(this.mock.executeRecovery(this.accountMock.target, executionCalldata)) + .to.emit(this.mock, 'RecoveryExecuted') + .withArgs(this.accountMock.target); + }); + + it('should change recovery status to NotStarted', async function () { + const status = await this.mock.getRecoveryStatus(this.accountMock.target); + expect(status).to.equal(ERC7579SocialRecoveryExecutorHelper.RecoveryStatus.NotStarted); + }); + + it('should change the account validator module signer', async function () { + const signer = await this.validatorMock.getSigner(this.accountMock.target); + expect(signer).to.equal(this.newSigner.address); + }); + + it('should allow the new signer to get validated on the account', async function () { + const operation = await this.accountMock + .createUserOp(this.userOp) + .then(op => this.signUserOpWithSigner(op, this.newSigner)); + expect( + await this.accountMockFromEntrypoint.validateUserOp.staticCall(operation.packed, operation.hash(), 0), + ).to.eq(SIG_VALIDATION_SUCCESS); + }); + + it('should ensure previous signer is not able to validate on the account any more', async function () { + const operation = await this.accountMock + .createUserOp(this.userOp) + .then(op => this.signUserOpWithSigner(op, this.initialSigner)); + expect( + await this.accountMockFromEntrypoint.validateUserOp.staticCall(operation.packed, operation.hash(), 0), + ).to.eq(SIG_VALIDATION_FAILURE); + }); + }); + }); + }); + + describe('cancel recovery by the guardians', async function () { + describe('with cancelled recovery', async function () { + beforeEach(async function () { + // allow the guardians to cancel the recovery + const guardians = this.recoveryConfig.guardians.slice(0, this.recoveryConfig.threshold); + const domain = await getDomain(this.mock); + + const message = { + account: this.accountMock.target, + nonce: await this.mock.nonces(this.accountMock.target), + }; + + const guardianSignatures = ERC7579SocialRecoveryExecutorHelper.sortGuardianSignatures( + guardians.map(g => ({ + signer: g.address, + signature: g.signTypedData( + domain, + ERC7579SocialRecoveryExecutorHelper.CANCEL_RECOVERY_TYPEHASH, + message, + ), + })), + ); + + await expect(this.mock.cancelRecovery(this.accountMock.target, guardianSignatures)) + .to.emit(this.mock, 'RecoveryCancelled') + .withArgs(this.accountMock.target); + }); + + it('should change recovery status to NotStarted', async function () { + const status = await this.mock.getRecoveryStatus(this.accountMock.target); + expect(status).to.equal(ERC7579SocialRecoveryExecutorHelper.RecoveryStatus.NotStarted); + }); + + it('should not be able to cancel again', async function () { + await expect(this.mock.cancelRecovery(this.accountMock.target, [])).to.be.revertedWithCustomError( + this.mock, + 'RecoveryNotStarted', + ); + }); + }); + }); + + describe('cancel recovery by the Account', async function () { + describe('with cancelled recovery', async function () { + beforeEach(async function () { + await expect( + this.accountMockFromEntrypoint.execute( + encodeMode(CALL_TYPE_SINGLE, EXEC_TYPE_DEFAULT), + encodeSingle(this.mock.target, 0, this.mock.interface.encodeFunctionData('cancelRecovery()')), + ), + ) + .to.emit(this.mock, 'RecoveryCancelled') + .withArgs(this.accountMock.target); + }); + + it('should change recovery status to NotStarted', async function () { + const status = await this.mock.getRecoveryStatus(this.accountMock.target); + expect(status).to.equal(ERC7579SocialRecoveryExecutorHelper.RecoveryStatus.NotStarted); + }); + + it('should resist replay attacks via nonce protection', async function () { + // guardians attempt to reuse initial signatures to startRecovery again + const alreadyUsedNonce = (await this.mock.nonces(this.accountMock.target)) - 1n; + const guardians = this.recoveryConfig.guardians; + const domain = await getDomain(this.mock); + + const encodedCallToValidatorModule = this.validatorMock.interface.encodeFunctionData('updateSigner', [ + this.newSigner.address, + ]); + + const recoveryCallData = encodeSingle(this.validatorMock.target, 0, encodedCallToValidatorModule); + + const executionCalldata = this.accountMock.interface.encodeFunctionData('executeFromExecutor', [ + encodeMode(CALL_TYPE_SINGLE, EXEC_TYPE_DEFAULT), + recoveryCallData, + ]); + + const message = { + account: this.accountMock.target, + nonce: alreadyUsedNonce, + executionCalldata: executionCalldata, + }; + + const guardianSignatures = ERC7579SocialRecoveryExecutorHelper.sortGuardianSignatures( + guardians.map(g => ({ + signer: g.address, + signature: g.signTypedData( + domain, + ERC7579SocialRecoveryExecutorHelper.START_RECOVERY_TYPEHASH, + message, + ), + })), + ); + + await expect( + this.mock.startRecovery(this.accountMock.target, guardianSignatures, executionCalldata), + ).to.be.revertedWithCustomError(this.mock, 'InvalidGuardianSignature'); + }); + }); + }); + }); + + describe('module configuration', function () { + it('should be able to add a guardian', async function () { + const newGuardian = ethers.Wallet.createRandom(); + await expect( + this.accountMockFromEntrypoint.execute( + encodeMode(CALL_TYPE_SINGLE, EXEC_TYPE_DEFAULT), + encodeSingle( + this.mock.target, + 0, + this.mock.interface.encodeFunctionData('addGuardian', [newGuardian.address]), + ), + ), + ) + .to.emit(this.mock, 'GuardianAdded') + .withArgs(this.accountMock.target, newGuardian.address); + + const isGuardian = await this.mock.isGuardian(this.accountMock.target, newGuardian.address); + expect(isGuardian).to.equal(true); + }); + + it('should be able to remove a guardian', async function () { + const guardianToRemove = this.recoveryConfig.guardians[0]; + await expect( + this.accountMockFromEntrypoint.execute( + encodeMode(CALL_TYPE_SINGLE, EXEC_TYPE_DEFAULT), + encodeSingle( + this.mock.target, + 0, + this.mock.interface.encodeFunctionData('removeGuardian', [guardianToRemove.address]), + ), + ), + ) + .to.emit(this.mock, 'GuardianRemoved') + .withArgs(this.accountMock.target, guardianToRemove.address); + + const isGuardian = await this.mock.isGuardian(this.accountMock.target, guardianToRemove.address); + expect(isGuardian).to.equal(false); + }); + + it('should be able to update the threshold', async function () { + const newThreshold = this.recoveryConfig.threshold + 1; + await expect( + this.accountMockFromEntrypoint.execute( + encodeMode(CALL_TYPE_SINGLE, EXEC_TYPE_DEFAULT), + encodeSingle( + this.mock.target, + 0, + this.mock.interface.encodeFunctionData('updateThreshold', [newThreshold]), + ), + ), + ) + .to.emit(this.mock, 'ThresholdChanged') + .withArgs(this.accountMock.target, newThreshold); + + const threshold = await this.mock.getThreshold(this.accountMock.target); + expect(threshold).to.equal(newThreshold); + }); + + describe('updating timelock', function () { + it('should fail if timelock is zero', async function () { + await expect( + this.accountMockFromEntrypoint.execute( + encodeMode(CALL_TYPE_SINGLE, EXEC_TYPE_DEFAULT), + encodeSingle(this.mock.target, 0, this.mock.interface.encodeFunctionData('updateTimelock', [0])), + ), + ).to.be.revertedWithCustomError(this.mock, 'InvalidTimelock'); + }); + + it('should be able to update the timelock', async function () { + const newTimelock = this.recoveryConfig.timelock + 1; + await expect( + this.accountMockFromEntrypoint.execute( + encodeMode(CALL_TYPE_SINGLE, EXEC_TYPE_DEFAULT), + encodeSingle( + this.mock.target, + 0, + this.mock.interface.encodeFunctionData('updateTimelock', [newTimelock]), + ), + ), + ) + .to.emit(this.mock, 'TimelockChanged') + .withArgs(this.accountMock.target, newTimelock); + + const timelock = await this.mock.getTimelock(this.accountMock.target); + expect(timelock).to.equal(newTimelock); + }); + }); + }); + }); + }); + describe('module metadata', function () { + it('should match the correct module type', async function () { + expect(await this.mock.isModuleType(MODULE_TYPE_EXECUTOR)).to.equal(true); + }); + }); +}); diff --git a/test/helpers/erc7579-modules.js b/test/helpers/erc7579-modules.js new file mode 100644 index 00000000..5e596c4a --- /dev/null +++ b/test/helpers/erc7579-modules.js @@ -0,0 +1,23 @@ +const { Enum } = require('@openzeppelin/contracts/test/helpers/enums'); +const { formatType } = require('@openzeppelin/contracts/test/helpers/eip712'); + +class ERC7579SocialRecoveryExecutorHelper { + static RecoveryStatus = Enum('NotStarted', 'Started', 'Ready'); + static START_RECOVERY_TYPEHASH = { + StartRecovery: formatType({ account: 'address', executionCalldata: 'bytes', nonce: 'uint256' }), + }; + static CANCEL_RECOVERY_TYPEHASH = { + CancelRecovery: formatType({ account: 'address', nonce: 'uint256' }), + }; + static sortGuardianSignatures(guardianSignatures) { + return guardianSignatures.sort((a, b) => { + if (BigInt(a.signer) < BigInt(b.signer)) return -1; + if (BigInt(a.signer) > BigInt(b.signer)) return 1; + return 0; + }); + } +} + +module.exports = { + ERC7579SocialRecoveryExecutorHelper, +};