Skip to content

Commit a023191

Browse files
ernestognwernestognw
committed
Simplify ERC7579 Executors validation pattern (#158)
1 parent 8bc5b33 commit a023191

File tree

6 files changed

+88
-104
lines changed

6 files changed

+88
-104
lines changed

‎contracts/account/modules/ERC7579DelayedExecutor.sol

Lines changed: 25 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,12 @@ import {ERC7579Executor} from "./ERC7579Executor.sol";
3232
* after a transition period defined by the current delay or {minSetback}, whichever
3333
* is longer.
3434
*
35+
* ==== Authorization
36+
*
37+
* Authorization for scheduling and canceling operations is controlled through the {_validateSchedule}
38+
* and {_validateCancel} functions. These functions can be overridden to implement custom
39+
* authorization logic, such as requiring specific signers or roles.
40+
*
3541
* TIP: Use {_scheduleAt} to schedule operations at a specific points in time. This is
3642
* useful to pre-schedule operations for non-deployed accounts (e.g. subscriptions).
3743
*/
@@ -94,12 +100,6 @@ abstract contract ERC7579DelayedExecutor is ERC7579Executor {
94100
bytes32 allowedStates
95101
);
96102

97-
/// @dev The operation is not authorized to be canceled.
98-
error ERC7579ExecutorUnauthorizedCancellation();
99-
100-
/// @dev The operation is not authorized to be scheduled.
101-
error ERC7579ExecutorUnauthorizedSchedule();
102-
103103
/// @dev The module is not installed on the account.
104104
error ERC7579ExecutorModuleNotInstalled();
105105

@@ -232,20 +232,18 @@ abstract contract ERC7579DelayedExecutor is ERC7579Executor {
232232
*/
233233
function schedule(address account, bytes32 salt, bytes32 mode, bytes calldata data) public virtual {
234234
require(_config[account].installed, ERC7579ExecutorModuleNotInstalled());
235-
bool allowed = _validateSchedule(account, salt, mode, data);
235+
_validateSchedule(account, salt, mode, data);
236236
(uint32 executableAfter, , ) = getDelay(account);
237237
_scheduleAt(account, salt, mode, data, Time.timestamp(), executableAfter);
238-
require(allowed, ERC7579ExecutorUnauthorizedSchedule());
239238
}
240239

241240
/**
242241
* @dev Cancels a previously scheduled operation. Can only be called by the account that
243242
* scheduled the operation. See {_cancel}.
244243
*/
245244
function cancel(address account, bytes32 salt, bytes32 mode, bytes calldata data) public virtual {
246-
bool allowed = _validateCancel(account, salt, mode, data);
245+
_validateCancel(account, salt, mode, data);
247246
_cancel(account, mode, data, salt); // Prioritize errors thrown in _cancel
248-
require(allowed, ERC7579ExecutorUnauthorizedCancellation());
249247
}
250248

251249
/**
@@ -267,32 +265,30 @@ abstract contract ERC7579DelayedExecutor is ERC7579Executor {
267265
_setExpiration(msg.sender, 0);
268266
}
269267

270-
/// @inheritdoc ERC7579Executor
268+
/**
269+
* @dev Returns `data` as the execution calldata. See {ERC7579Executor-_execute}.
270+
*
271+
* NOTE: This function relies on the operation state validation in {_execute} for
272+
* authorization. Extensions of this module should override this function to implement
273+
* additional validation logic if needed.
274+
*/
271275
function _validateExecution(
272276
address /* account */,
273277
bytes32 /* salt */,
274278
bytes32 /* mode */,
275279
bytes calldata data
276-
) internal view virtual override returns (bool valid, bytes calldata executionCalldata) {
277-
return (true, data); // Anyone can execute, the state validation of the operation is enough
280+
) internal virtual override returns (bytes calldata) {
281+
return data;
278282
}
279283

280284
/**
281-
* @dev Whether the caller is authorized to cancel operations.
282-
* By default, this checks if the caller is the account itself. Derived contracts can
283-
* override this to implement custom authorization logic.
285+
* @dev Validates whether an operation can be canceled.
284286
*
285287
* Example extension:
286288
*
287289
* ```solidity
288-
* function _validateCancel(
289-
* address account,
290-
* bytes32 mode,
291-
* bytes calldata data,
292-
* bytes32 salt
293-
* ) internal view override returns (bool) {
294-
* bool isAuthorized = ...; // custom logic to check authorization
295-
* return isAuthorized || super._validateCancel(account, mode, data, salt);
290+
* function _validateCancel(address account, bytes32 salt, bytes32 mode, bytes calldata data) internal override {
291+
* // e.g. require(msg.sender == account);
296292
* }
297293
*```
298294
*/
@@ -301,26 +297,16 @@ abstract contract ERC7579DelayedExecutor is ERC7579Executor {
301297
bytes32 /* salt */,
302298
bytes32 /* mode */,
303299
bytes calldata /* data */
304-
) internal view virtual returns (bool) {
305-
return account == msg.sender;
306-
}
300+
) internal virtual;
307301

308302
/**
309-
* @dev Whether the caller is authorized to schedule operations.
310-
* By default, this checks if the caller is the account itself. Derived contracts can
311-
* override this to implement custom authorization logic.
303+
* @dev Validates whether an operation can be scheduled.
312304
*
313305
* Example extension:
314306
*
315307
* ```solidity
316-
* function _validateSchedule(
317-
* address account,
318-
* bytes32 mode,
319-
* bytes calldata data,
320-
* bytes32 salt
321-
* ) internal view override returns (bool) {
322-
* bool isAuthorized = ...; // custom logic to check authorization
323-
* return isAuthorized || super._validateSchedule(account, mode, data, salt);
308+
* function _validateSchedule(address account, bytes32 salt, bytes32 mode, bytes calldata data) internal override {
309+
* // e.g. require(msg.sender == account);
324310
* }
325311
*```
326312
*/
@@ -329,9 +315,7 @@ abstract contract ERC7579DelayedExecutor is ERC7579Executor {
329315
bytes32 /* salt */,
330316
bytes32 /* mode */,
331317
bytes calldata /* data */
332-
) internal view virtual returns (bool) {
333-
return account == msg.sender;
334-
}
318+
) internal virtual;
335319

336320
/**
337321
* @dev Internal implementation for setting an account's delay. See {getDelay}.

‎contracts/account/modules/ERC7579Executor.sol

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,6 @@ abstract contract ERC7579Executor is IERC7579Module {
2525
bytes executionCalldata
2626
);
2727

28-
/// @dev Thrown when the execution is invalid. See {_validateExecution} for details.
29-
error ERC7579InvalidExecution();
30-
3128
/// @inheritdoc IERC7579Module
3229
function isModuleType(uint256 moduleTypeId) public pure virtual returns (bool) {
3330
return moduleTypeId == MODULE_TYPE_EXECUTOR;
@@ -44,36 +41,38 @@ abstract contract ERC7579Executor is IERC7579Module {
4441
bytes32 mode,
4542
bytes calldata data
4643
) public virtual returns (bytes[] memory returnData) {
47-
(bool allowed, bytes calldata executionCalldata) = _validateExecution(account, salt, mode, data);
44+
bytes calldata executionCalldata = _validateExecution(account, salt, mode, data);
4845
returnData = _execute(account, mode, salt, executionCalldata); // Prioritize errors thrown in _execute
49-
require(allowed, ERC7579InvalidExecution());
5046
return returnData;
5147
}
5248

5349
/**
54-
* @dev Check if the caller is authorized to execute operations.
55-
* Derived contracts can implement this with custom authorization logic.
50+
* @dev Validates whether the execution can proceed. This function is called before executing
51+
* the operation and returns the execution calldata to be used.
5652
*
5753
* Example extension:
5854
*
5955
* ```solidity
60-
* function _validateExecution(
61-
* address account,
62-
* bytes32 salt,
63-
* bytes32 mode,
64-
* bytes calldata data
65-
* ) internal view override returns (bool valid, bytes calldata executionCalldata) {
66-
* /// ...
67-
* return isAuthorized; // custom logic to check authorization
56+
* function _validateExecution(address account, bytes32 salt, bytes32 mode, bytes calldata data)
57+
* internal
58+
* override
59+
* returns (bytes calldata)
60+
* {
61+
* // custom logic
62+
* return data;
6863
* }
6964
*```
65+
*
66+
* TIP: Pack extra data in the `data` arguments (e.g. a signature) to be used in the
67+
* validation process. Calldata can be sliced to extract it and return only the
68+
* execution calldata.
7069
*/
7170
function _validateExecution(
7271
address account,
7372
bytes32 salt,
7473
bytes32 mode,
7574
bytes calldata data
76-
) internal view virtual returns (bool valid, bytes calldata executionCalldata);
75+
) internal virtual returns (bytes calldata);
7776

7877
/**
7978
* @dev Internal version of {execute}. Emits {ERC7579ExecutorOperationExecuted} event.

‎contracts/mocks/account/modules/ERC7579ExecutorMocks.sol

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// SPDX-License-Identifier: MIT
2+
3+
pragma solidity ^0.8.27;
4+
5+
import {ERC7579Executor} from "../../../account/modules/ERC7579Executor.sol";
6+
import {ERC7579DelayedExecutor} from "../../../account/modules/ERC7579DelayedExecutor.sol";
7+
8+
abstract contract ERC7579ExecutorMock is ERC7579Executor {
9+
function onInstall(bytes calldata data) external {}
10+
11+
function onUninstall(bytes calldata data) external {}
12+
13+
function _validateExecution(
14+
address,
15+
bytes32,
16+
bytes32,
17+
bytes calldata data
18+
) internal pure override returns (bytes calldata) {
19+
return data;
20+
}
21+
}
22+
23+
abstract contract ERC7579DelayedExecutorMock is ERC7579DelayedExecutor {
24+
function _validateSchedule(address account, bytes32, bytes32, bytes calldata) internal view override {
25+
require(msg.sender == account);
26+
}
27+
28+
function _validateCancel(address account, bytes32, bytes32, bytes calldata) internal view override {
29+
require(msg.sender == account);
30+
}
31+
}

‎contracts/mocks/account/modules/ERC7579MultisigMocks.sol

Lines changed: 15 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -20,18 +20,12 @@ abstract contract ERC7579MultisigExecutorMock is EIP712, ERC7579Executor, ERC757
2020
bytes32 salt,
2121
bytes32 mode,
2222
bytes calldata data
23-
) internal view override returns (bool valid, bytes calldata executionCalldata) {
23+
) internal view override returns (bytes calldata) {
2424
uint16 executionCalldataLength = uint16(uint256(bytes32(data[0:2]))); // First 2 bytes are the length
25-
bytes calldata actualExecutionCalldata = data[2:2 + executionCalldataLength]; // Next bytes are the calldata
26-
bytes calldata signature = data[2 + executionCalldataLength:]; // Remaining bytes are the signature
27-
return (
28-
_validateMultisignature(
29-
account,
30-
_getExecuteTypeHash(account, salt, mode, actualExecutionCalldata),
31-
signature
32-
),
33-
actualExecutionCalldata
34-
);
25+
bytes calldata executionCalldata = data[2:2 + executionCalldataLength]; // Next bytes are the calldata
26+
bytes32 typeHash = _getExecuteTypeHash(account, salt, mode, executionCalldata);
27+
require(_validateMultisignature(account, typeHash, data[2 + executionCalldataLength:])); // Remaining bytes are the signature
28+
return executionCalldata;
3529
}
3630

3731
function _getExecuteTypeHash(
@@ -54,18 +48,12 @@ abstract contract ERC7579MultisigWeightedExecutorMock is EIP712, ERC7579Executor
5448
bytes32 salt,
5549
bytes32 mode,
5650
bytes calldata data
57-
) internal view override returns (bool valid, bytes calldata executionCalldata) {
51+
) internal view override returns (bytes calldata) {
5852
uint16 executionCalldataLength = uint16(uint256(bytes32(data[0:2]))); // First 2 bytes are the length
59-
bytes calldata actualExecutionCalldata = data[2:2 + executionCalldataLength]; // Next bytes are the calldata
60-
bytes calldata signature = data[2 + executionCalldataLength:]; // Remaining bytes are the signature
61-
return (
62-
_validateMultisignature(
63-
account,
64-
_getExecuteTypeHash(account, salt, mode, actualExecutionCalldata),
65-
signature
66-
),
67-
actualExecutionCalldata
68-
);
53+
bytes calldata executionCalldata = data[2:2 + executionCalldataLength]; // Next bytes are the calldata
54+
bytes32 typeHash = _getExecuteTypeHash(account, salt, mode, executionCalldata);
55+
require(_validateMultisignature(account, typeHash, data[2 + executionCalldataLength:])); // Remaining bytes are the signature
56+
return executionCalldata;
6957
}
7058

7159
function _getExecuteTypeHash(
@@ -88,18 +76,12 @@ abstract contract ERC7579MultisigConfirmationExecutorMock is ERC7579Executor, ER
8876
bytes32 salt,
8977
bytes32 mode,
9078
bytes calldata data
91-
) internal view override returns (bool valid, bytes calldata executionCalldata) {
79+
) internal view override returns (bytes calldata) {
9280
uint16 executionCalldataLength = uint16(uint256(bytes32(data[0:2]))); // First 2 bytes are the length
93-
bytes calldata actualExecutionCalldata = data[2:2 + executionCalldataLength]; // Next bytes are the calldata
94-
bytes calldata signature = data[2 + executionCalldataLength:]; // Remaining bytes are the signature
95-
return (
96-
_validateMultisignature(
97-
account,
98-
_getExecuteTypeHash(account, salt, mode, actualExecutionCalldata),
99-
signature
100-
),
101-
actualExecutionCalldata
102-
);
81+
bytes calldata executionCalldata = data[2:2 + executionCalldataLength]; // Next bytes are the calldata
82+
bytes32 typeHash = _getExecuteTypeHash(account, salt, mode, executionCalldata);
83+
require(_validateMultisignature(account, typeHash, data[2 + executionCalldataLength:])); // Remaining bytes are the signature
84+
return executionCalldata;
10385
}
10486

10587
function _getExecuteTypeHash(

‎test/account/modules/ERC7579DelayedExecutor.test.js

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ async function fixture() {
1818
const [other] = await ethers.getSigners();
1919

2020
// Deploy ERC-7579 validator module
21-
const mock = await ethers.deployContract('$ERC7579DelayedExecutor');
21+
const mock = await ethers.deployContract('$ERC7579DelayedExecutorMock');
2222
const target = await ethers.deployContract('CallReceiverMockExtended');
2323

2424
// ERC-4337 env
@@ -216,12 +216,6 @@ describe('ERC7579DelayedExecutor', function () {
216216
).to.eventually.deep.equal([now, now + this.delay, now + this.delay + this.expiration]);
217217
});
218218

219-
it('reverts with ERC7579ExecutorUnauthorizedSchedule if called by other account', async function () {
220-
await expect(
221-
this.mock.schedule(this.mockAccount.address, salt, this.mode, this.calldata),
222-
).to.be.revertedWithCustomError(this.mock, 'ERC7579ExecutorUnauthorizedSchedule');
223-
});
224-
225219
it('reverts with ERC7579ExecutorModuleNotInstalled if the module is not installed', async function () {
226220
await expect(
227221
this.mock.schedule(this.other.address, salt, this.mode, this.calldata),
@@ -301,11 +295,5 @@ describe('ERC7579DelayedExecutor', function () {
301295
this.mockFromAccount.cancel(this.mockAccount.address, salt, this.mode, this.calldata),
302296
).to.be.revertedWithCustomError(this.mock, 'ERC7579ExecutorUnexpectedOperationState'); // Can't cancel twice
303297
});
304-
305-
it('reverts with ERC7579ExecutorUnauthorizedCancellation if called by other account', async function () {
306-
await expect(
307-
this.mock.cancel(this.mockAccount.address, salt, this.mode, this.calldata),
308-
).to.be.revertedWithCustomError(this.mock, 'ERC7579ExecutorUnauthorizedCancellation');
309-
});
310298
});
311299
});

‎test/account/modules/ERC7579Executor.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ const { shouldBehaveLikeERC7579Module } = require('./ERC7579Module.behavior');
1515

1616
async function fixture() {
1717
// Deploy ERC-7579 validator module
18-
const mock = await ethers.deployContract('$ERC7579MultisigExecutorMock', ['MultisigExecutor', '1']);
18+
const mock = await ethers.deployContract('$ERC7579ExecutorMock');
1919
const target = await ethers.deployContract('CallReceiverMockExtended');
2020

2121
// ERC-4337 env

0 commit comments

Comments
 (0)