Skip to content

Commit 700558d

Browse files
committed
Make schedule and cancel noops when not authorized
1 parent ec0ae9e commit 700558d

File tree

3 files changed

+22
-19
lines changed

3 files changed

+22
-19
lines changed

contracts/account/modules/ERC7579DelayedExecutor.sol

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -91,12 +91,6 @@ abstract contract ERC7579DelayedExecutor is ERC7579Executor {
9191
bytes32 allowedStates
9292
);
9393

94-
/// @dev The operation is not authorized to be canceled.
95-
error ERC7579ExecutorUnauthorizedCancellation();
96-
97-
/// @dev The operation is not authorized to be scheduled.
98-
error ERC7579ExecutorUnauthorizedSchedule();
99-
10094
mapping(address account => ExecutionConfig) private _config;
10195
mapping(bytes32 operationId => Schedule) private _schedules;
10296

@@ -227,19 +221,19 @@ abstract contract ERC7579DelayedExecutor is ERC7579Executor {
227221
* See {_validateSchedule} for authorization checks.
228222
*/
229223
function schedule(address account, bytes32 salt, bytes32 mode, bytes calldata data) public virtual {
230-
bool allowed = _validateSchedule(account, salt, mode, data);
231-
_schedule(account, salt, mode, data); // Prioritize errors thrown in _schedule
232-
require(allowed, ERC7579ExecutorUnauthorizedSchedule());
224+
if (_validateSchedule(account, salt, mode, data)) {
225+
_schedule(account, salt, mode, data);
226+
} // else no-op
233227
}
234228

235229
/**
236230
* @dev Cancels a previously scheduled operation. Can only be called by the account that
237231
* scheduled the operation. See {_cancel}.
238232
*/
239233
function cancel(address account, bytes32 salt, bytes32 mode, bytes calldata data) public virtual {
240-
bool allowed = _validateCancel(account, salt, mode, data);
241-
_cancel(account, mode, data, salt); // Prioritize errors thrown in _cancel
242-
require(allowed, ERC7579ExecutorUnauthorizedCancellation());
234+
if (_validateCancel(account, salt, mode, data)) {
235+
_cancel(account, mode, data, salt);
236+
} // else no-op
243237
}
244238

245239
/**
@@ -325,7 +319,7 @@ abstract contract ERC7579DelayedExecutor is ERC7579Executor {
325319
bytes32 /* mode */,
326320
bytes calldata /* data */
327321
) internal view virtual returns (bool) {
328-
return account == msg.sender;
322+
return _config[account].installed && account == msg.sender;
329323
}
330324

331325
/**

test/account/modules/ERC7579DelayedExecutor.test.js

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ const {
1212
encodeSingle,
1313
} = require('@openzeppelin/contracts/test/helpers/erc7579');
1414
const { shouldBehaveLikeERC7579Module } = require('./ERC7579Module.behavior');
15+
const { OperationState, ProposalState } = require('../../helpers/enums');
1516

1617
async function fixture() {
1718
// Deploy ERC-7579 validator module
@@ -155,10 +156,14 @@ describe('ERC7579DelayedExecutor', function () {
155156
).to.eventually.deep.equal([now, now + this.delay, now + this.delay + this.expiration]);
156157
});
157158

158-
it('reverts with ERC7579ExecutorUnauthorizedSchedule if called by other account', async function () {
159+
it('no-ops if called by other account', async function () {
160+
await this.mock.schedule(this.mockAccount.address, salt, this.mode, this.calldata); // not revert
159161
await expect(
160-
this.mock.schedule(this.mockAccount.address, salt, this.mode, this.calldata),
161-
).to.be.revertedWithCustomError(this.mock, 'ERC7579ExecutorUnauthorizedSchedule');
162+
this.mock.getSchedule(this.mockAccount.address, salt, this.mode, this.calldata),
163+
).to.eventually.deep.equal([0n, 0n, 0n]);
164+
await expect(this.mock.state(this.mockAccount.address, salt, this.mode, this.calldata)).to.eventually.equal(
165+
OperationState.Unknown,
166+
);
162167
});
163168
});
164169

@@ -232,9 +237,12 @@ describe('ERC7579DelayedExecutor', function () {
232237
});
233238

234239
it('reverts with ERC7579ExecutorUnauthorizedCancellation if called by other account', async function () {
235-
await expect(
236-
this.mock.cancel(this.mockAccount.address, salt, this.mode, this.calldata),
237-
).to.be.revertedWithCustomError(this.mock, 'ERC7579ExecutorUnauthorizedCancellation');
240+
const previousState = await this.mock.state(this.mockAccount.address, salt, this.mode, this.calldata);
241+
expect(previousState).to.not.eq(ProposalState.Canceled);
242+
await this.mock.cancel(this.mockAccount.address, salt, this.mode, this.calldata); // not revert
243+
await expect(this.mock.state(this.mockAccount.address, salt, this.mode, this.calldata)).to.eventually.equal(
244+
previousState,
245+
);
238246
});
239247
});
240248
});

test/helpers/enums.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,5 @@ module.exports = {
1111
'EmailProof',
1212
),
1313
Case: enums.EnumTyped('CHECKSUM', 'LOWERCASE', 'UPPERCASE', 'ANY'),
14+
OperationState: enums.Enum('Unknown', 'Scheduled', 'Ready', 'Expired', 'Executed'),
1415
};

0 commit comments

Comments
 (0)