Simplify ERC7579 Executors validation pattern

contracts/account/modules/ERC7579DelayedExecutor.sol

@@ -32,6 +32,12 @@ import {ERC7579Executor} from "./ERC7579Executor.sol";
  * after a transition period defined by the current delay or {minSetback}, whichever
  * is longer.
  *
+ * ==== Authorization
+ *
+ * Authorization for scheduling and canceling operations is controlled through the {_validateSchedule}
+ * and {_validateCancel} functions. These functions can be overridden to implement custom
+ * authorization logic, such as requiring specific signers or roles.
+ *
  * TIP: Use {_scheduleAt} to schedule operations at a specific points in time. This is
  * useful to pre-schedule operations for non-deployed accounts (e.g. subscriptions).
  */
@@ -94,12 +100,6 @@ abstract contract ERC7579DelayedExecutor is ERC7579Executor {
         bytes32 allowedStates
     );
 
-    /// @dev The operation is not authorized to be canceled.
-    error ERC7579ExecutorUnauthorizedCancellation();
-
-    /// @dev The operation is not authorized to be scheduled.
-    error ERC7579ExecutorUnauthorizedSchedule();
-
     /// @dev The module is not installed on the account.
     error ERC7579ExecutorModuleNotInstalled();
 
@@ -232,20 +232,18 @@ abstract contract ERC7579DelayedExecutor is ERC7579Executor {
      */
     function schedule(address account, bytes32 salt, bytes32 mode, bytes calldata data) public virtual {
         require(_config[account].installed, ERC7579ExecutorModuleNotInstalled());
-        bool allowed = _validateSchedule(account, salt, mode, data);
+        _validateSchedule(account, salt, mode, data);
         (uint32 executableAfter, , ) = getDelay(account);
         _scheduleAt(account, salt, mode, data, Time.timestamp(), executableAfter);
-        require(allowed, ERC7579ExecutorUnauthorizedSchedule());
     }
 
     /**
      * @dev Cancels a previously scheduled operation. Can only be called by the account that
      * scheduled the operation. See {_cancel}.
      */
     function cancel(address account, bytes32 salt, bytes32 mode, bytes calldata data) public virtual {
-        bool allowed = _validateCancel(account, salt, mode, data);
+        _validateCancel(account, salt, mode, data);
         _cancel(account, mode, data, salt); // Prioritize errors thrown in _cancel
-        require(allowed, ERC7579ExecutorUnauthorizedCancellation());
     }
 
     /**
@@ -267,32 +265,30 @@ abstract contract ERC7579DelayedExecutor is ERC7579Executor {
         _setExpiration(msg.sender, 0);
     }
 
-    /// @inheritdoc ERC7579Executor
+    /**
+     * @dev Returns `data` as the execution calldata. See {ERC7579Executor-_execute}.
+     *
+     * NOTE: This function relies on the operation state validation in {_execute} for
+     * authorization. Extensions of this module should override this function to implement
+     * additional validation logic if needed.
+     */
     function _validateExecution(
         address /* account */,
         bytes32 /* salt */,
         bytes32 /* mode */,
         bytes calldata data
-    ) internal view virtual override returns (bool valid, bytes calldata executionCalldata) {
-        return (true, data); // Anyone can execute, the state validation of the operation is enough
+    ) internal virtual override returns (bytes calldata) {
+        return data;
     }
 
     /**
-     * @dev Whether the caller is authorized to cancel operations.
-     * By default, this checks if the caller is the account itself. Derived contracts can
-     * override this to implement custom authorization logic.
+     * @dev Validates whether an operation can be canceled.
      *
      * Example extension:
      *
      * ```solidity
-     *  function _validateCancel(
-     *     address account,
-     *     bytes32 mode,
-     *     bytes calldata data,
-     *     bytes32 salt
-     *  ) internal view override returns (bool) {
-     *    bool isAuthorized = ...; // custom logic to check authorization
-     *    return isAuthorized || super._validateCancel(account, mode, data, salt);
+     *  function _validateCancel(address account, bytes32 salt, bytes32 mode, bytes calldata data) internal override {
+     *    // e.g. require(msg.sender == account);
      *  }
      *```
      */
@@ -301,26 +297,16 @@ abstract contract ERC7579DelayedExecutor is ERC7579Executor {
         bytes32 /* salt */,
         bytes32 /* mode */,
         bytes calldata /* data */
-    ) internal view virtual returns (bool) {
-        return account == msg.sender;
-    }
+    ) internal virtual;
 
     /**
-     * @dev Whether the caller is authorized to schedule operations.
-     * By default, this checks if the caller is the account itself. Derived contracts can
-     * override this to implement custom authorization logic.
+     * @dev Validates whether an operation can be scheduled.
      *
      * Example extension:
      *
      * ```solidity
-     *  function _validateSchedule(
-     *     address account,
-     *     bytes32 mode,
-     *     bytes calldata data,
-     *     bytes32 salt
-     *  ) internal view override returns (bool) {
-     *    bool isAuthorized = ...; // custom logic to check authorization
-     *    return isAuthorized || super._validateSchedule(account, mode, data, salt);
+     *  function _validateSchedule(address account, bytes32 salt, bytes32 mode, bytes calldata data) internal override {
+     *    // e.g. require(msg.sender == account);
      *  }
      *```
      */
@@ -329,9 +315,7 @@ abstract contract ERC7579DelayedExecutor is ERC7579Executor {
         bytes32 /* salt */,
         bytes32 /* mode */,
         bytes calldata /* data */
-    ) internal view virtual returns (bool) {
-        return account == msg.sender;
-    }
+    ) internal virtual;
 
     /**
      * @dev Internal implementation for setting an account's delay. See {getDelay}.

contracts/account/modules/ERC7579Executor.sol

@@ -25,9 +25,6 @@ abstract contract ERC7579Executor is IERC7579Module {
         bytes executionCalldata
     );
 
-    /// @dev Thrown when the execution is invalid. See {_validateExecution} for details.
-    error ERC7579InvalidExecution();
-
     /// @inheritdoc IERC7579Module
     function isModuleType(uint256 moduleTypeId) public pure virtual returns (bool) {
         return moduleTypeId == MODULE_TYPE_EXECUTOR;
@@ -44,36 +41,38 @@ abstract contract ERC7579Executor is IERC7579Module {
         bytes32 mode,
         bytes calldata data
     ) public virtual returns (bytes[] memory returnData) {
-        (bool allowed, bytes calldata executionCalldata) = _validateExecution(account, salt, mode, data);
+        bytes calldata executionCalldata = _validateExecution(account, salt, mode, data);
         returnData = _execute(account, mode, salt, executionCalldata); // Prioritize errors thrown in _execute
-        require(allowed, ERC7579InvalidExecution());
         return returnData;
     }
 
     /**
-     * @dev Check if the caller is authorized to execute operations.
-     * Derived contracts can implement this with custom authorization logic.
+     * @dev Validates whether the execution can proceed. This function is called before executing
+     * the operation and returns the execution calldata to be used.
      *
      * Example extension:
      *
      * ```solidity
-     *  function _validateExecution(
-     *     address account,
-     *     bytes32 salt,
-     *     bytes32 mode,
-     *     bytes calldata data
-     *  ) internal view override returns (bool valid, bytes calldata executionCalldata) {
-     *    /// ...
-     *    return isAuthorized; // custom logic to check authorization
+     *  function _validateExecution(address account, bytes32 salt, bytes32 mode, bytes calldata data)
+     *      internal
+     *      override
+     *      returns (bytes calldata)
+     *  {
+     *      // custom logic
+     *      return data;
      *  }
      *```
+     *
+     * TIP: Pack extra data in the `data` arguments (e.g. a signature) to be used in the
+     * validation process. Calldata can be sliced to extract it and return only the
+     * execution calldata.
      */
     function _validateExecution(
         address account,
         bytes32 salt,
         bytes32 mode,
         bytes calldata data
-    ) internal view virtual returns (bool valid, bytes calldata executionCalldata);
+    ) internal virtual returns (bytes calldata);
 
     /**
      * @dev Internal version of {execute}. Emits {ERC7579ExecutorOperationExecuted} event.

contracts/mocks/account/modules/ERC7579ExecutorMocks.sol

@@ -0,0 +1,31 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.27;
+
+import {ERC7579Executor} from "../../../account/modules/ERC7579Executor.sol";
+import {ERC7579DelayedExecutor} from "../../../account/modules/ERC7579DelayedExecutor.sol";
+
+abstract contract ERC7579ExecutorMock is ERC7579Executor {
+    function onInstall(bytes calldata data) external {}
+
+    function onUninstall(bytes calldata data) external {}
+
+    function _validateExecution(
+        address,
+        bytes32,
+        bytes32,
+        bytes calldata data
+    ) internal pure override returns (bytes calldata) {
+        return data;
+    }
+}
+
+abstract contract ERC7579DelayedExecutorMock is ERC7579DelayedExecutor {
+    function _validateSchedule(address account, bytes32, bytes32, bytes calldata) internal view override {
+        require(msg.sender == account);
+    }
+
+    function _validateCancel(address account, bytes32, bytes32, bytes calldata) internal view override {
+        require(msg.sender == account);
+    }
+}

contracts/mocks/account/modules/ERC7579MultisigMocks.sol

@@ -20,18 +20,12 @@ abstract contract ERC7579MultisigExecutorMock is EIP712, ERC7579Executor, ERC757
         bytes32 salt,
         bytes32 mode,
         bytes calldata data
-    ) internal view override returns (bool valid, bytes calldata executionCalldata) {
+    ) internal view override returns (bytes calldata) {
         uint16 executionCalldataLength = uint16(uint256(bytes32(data[0:2]))); // First 2 bytes are the length
-        bytes calldata actualExecutionCalldata = data[2:2 + executionCalldataLength]; // Next bytes are the calldata
-        bytes calldata signature = data[2 + executionCalldataLength:]; // Remaining bytes are the signature
-        return (
-            _validateMultisignature(
-                account,
-                _getExecuteTypeHash(account, salt, mode, actualExecutionCalldata),
-                signature
-            ),
-            actualExecutionCalldata
-        );
+        bytes calldata executionCalldata = data[2:2 + executionCalldataLength]; // Next bytes are the calldata
+        bytes32 typeHash = _getExecuteTypeHash(account, salt, mode, executionCalldata);
+        require(_validateMultisignature(account, typeHash, data[2 + executionCalldataLength:])); // Remaining bytes are the signature
+        return executionCalldata;
     }
 
     function _getExecuteTypeHash(
@@ -54,18 +48,12 @@ abstract contract ERC7579MultisigWeightedExecutorMock is EIP712, ERC7579Executor
         bytes32 salt,
         bytes32 mode,
         bytes calldata data
-    ) internal view override returns (bool valid, bytes calldata executionCalldata) {
+    ) internal view override returns (bytes calldata) {
         uint16 executionCalldataLength = uint16(uint256(bytes32(data[0:2]))); // First 2 bytes are the length
-        bytes calldata actualExecutionCalldata = data[2:2 + executionCalldataLength]; // Next bytes are the calldata
-        bytes calldata signature = data[2 + executionCalldataLength:]; // Remaining bytes are the signature
-        return (
-            _validateMultisignature(
-                account,
-                _getExecuteTypeHash(account, salt, mode, actualExecutionCalldata),
-                signature
-            ),
-            actualExecutionCalldata
-        );
+        bytes calldata executionCalldata = data[2:2 + executionCalldataLength]; // Next bytes are the calldata
+        bytes32 typeHash = _getExecuteTypeHash(account, salt, mode, executionCalldata);
+        require(_validateMultisignature(account, typeHash, data[2 + executionCalldataLength:])); // Remaining bytes are the signature
+        return executionCalldata;
     }
 
     function _getExecuteTypeHash(
@@ -88,18 +76,12 @@ abstract contract ERC7579MultisigConfirmationExecutorMock is ERC7579Executor, ER
         bytes32 salt,
         bytes32 mode,
         bytes calldata data
-    ) internal view override returns (bool valid, bytes calldata executionCalldata) {
+    ) internal view override returns (bytes calldata) {
         uint16 executionCalldataLength = uint16(uint256(bytes32(data[0:2]))); // First 2 bytes are the length
-        bytes calldata actualExecutionCalldata = data[2:2 + executionCalldataLength]; // Next bytes are the calldata
-        bytes calldata signature = data[2 + executionCalldataLength:]; // Remaining bytes are the signature
-        return (
-            _validateMultisignature(
-                account,
-                _getExecuteTypeHash(account, salt, mode, actualExecutionCalldata),
-                signature
-            ),
-            actualExecutionCalldata
-        );
+        bytes calldata executionCalldata = data[2:2 + executionCalldataLength]; // Next bytes are the calldata
+        bytes32 typeHash = _getExecuteTypeHash(account, salt, mode, executionCalldata);
+        require(_validateMultisignature(account, typeHash, data[2 + executionCalldataLength:])); // Remaining bytes are the signature
+        return executionCalldata;
     }
 
     function _getExecuteTypeHash(

test/account/modules/ERC7579DelayedExecutor.test.js

@@ -18,7 +18,7 @@ async function fixture() {
   const [other] = await ethers.getSigners();
 
   // Deploy ERC-7579 validator module
-  const mock = await ethers.deployContract('$ERC7579DelayedExecutor');
+  const mock = await ethers.deployContract('$ERC7579DelayedExecutorMock');
   const target = await ethers.deployContract('CallReceiverMockExtended');
 
   // ERC-4337 env
@@ -216,12 +216,6 @@ describe('ERC7579DelayedExecutor', function () {
       ).to.eventually.deep.equal([now, now + this.delay, now + this.delay + this.expiration]);
     });
 
-    it('reverts with ERC7579ExecutorUnauthorizedSchedule if called by other account', async function () {
-      await expect(
-        this.mock.schedule(this.mockAccount.address, salt, this.mode, this.calldata),
-      ).to.be.revertedWithCustomError(this.mock, 'ERC7579ExecutorUnauthorizedSchedule');
-    });
-
     it('reverts with ERC7579ExecutorModuleNotInstalled if the module is not installed', async function () {
       await expect(
         this.mock.schedule(this.other.address, salt, this.mode, this.calldata),
@@ -301,11 +295,5 @@ describe('ERC7579DelayedExecutor', function () {
         this.mockFromAccount.cancel(this.mockAccount.address, salt, this.mode, this.calldata),
       ).to.be.revertedWithCustomError(this.mock, 'ERC7579ExecutorUnexpectedOperationState'); // Can't cancel twice
     });
-
-    it('reverts with ERC7579ExecutorUnauthorizedCancellation if called by other account', async function () {
-      await expect(
-        this.mock.cancel(this.mockAccount.address, salt, this.mode, this.calldata),
-      ).to.be.revertedWithCustomError(this.mock, 'ERC7579ExecutorUnauthorizedCancellation');
-    });
   });
 });

test/account/modules/ERC7579Executor.test.js

@@ -15,7 +15,7 @@ const { shouldBehaveLikeERC7579Module } = require('./ERC7579Module.behavior');
 
 async function fixture() {
   // Deploy ERC-7579 validator module
-  const mock = await ethers.deployContract('$ERC7579MultisigExecutorMock', ['MultisigExecutor', '1']);
+  const mock = await ethers.deployContract('$ERC7579ExecutorMock');
   const target = await ethers.deployContract('CallReceiverMockExtended');
 
   // ERC-4337 env