Skip to content

Commit

Permalink
refactor: rename isPublic to skipRuntimeValidation
Browse files Browse the repository at this point in the history
  • Loading branch information
adamegyed committed Aug 30, 2024
1 parent c5c3091 commit f55d9ff
Show file tree
Hide file tree
Showing 15 changed files with 35 additions and 31 deletions.
2 changes: 1 addition & 1 deletion src/account/AccountStorage.sol
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ struct ExecutionData {
// state changing if this flag is set to true.
// Note that even if this is set to true, user op validation will still be required, otherwise anyone could
// drain the account of native tokens by wasting gas.
bool isPublic;
bool skipRuntimeValidation;
// Whether or not a global validation function may be used to validate this function.
bool allowGlobalValidation;
// The execution hooks for this function selector.
Expand Down
2 changes: 1 addition & 1 deletion src/account/ModularAccountView.sol
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ abstract contract ModularAccountView is IModularAccountView {
} else {
ExecutionData storage executionData = getAccountStorage().executionData[selector];
data.module = executionData.module;
data.isPublic = executionData.isPublic;
data.skipRuntimeValidation = executionData.skipRuntimeValidation;
data.allowGlobalValidation = executionData.allowGlobalValidation;

uint256 executionHooksLen = executionData.executionHooks.length();
Expand Down
17 changes: 10 additions & 7 deletions src/account/ModuleManagerInternals.sol
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,12 @@ abstract contract ModuleManagerInternals is IModularAccount {

// Storage update operations

function _setExecutionFunction(bytes4 selector, bool isPublic, bool allowGlobalValidation, address module)
internal
{
function _setExecutionFunction(
bytes4 selector,
bool skipRuntimeValidation,
bool allowGlobalValidation,
address module
) internal {
ExecutionData storage _executionData = getAccountStorage().executionData[selector];

if (_executionData.module != address(0)) {
Expand Down Expand Up @@ -77,15 +80,15 @@ abstract contract ModuleManagerInternals is IModularAccount {
}

_executionData.module = module;
_executionData.isPublic = isPublic;
_executionData.skipRuntimeValidation = skipRuntimeValidation;
_executionData.allowGlobalValidation = allowGlobalValidation;
}

function _removeExecutionFunction(bytes4 selector) internal {
ExecutionData storage _executionData = getAccountStorage().executionData[selector];

_executionData.module = address(0);
_executionData.isPublic = false;
_executionData.skipRuntimeValidation = false;
_executionData.allowGlobalValidation = false;
}

Expand Down Expand Up @@ -125,9 +128,9 @@ abstract contract ModuleManagerInternals is IModularAccount {
uint256 length = manifest.executionFunctions.length;
for (uint256 i = 0; i < length; ++i) {
bytes4 selector = manifest.executionFunctions[i].executionSelector;
bool isPublic = manifest.executionFunctions[i].isPublic;
bool skipRuntimeValidation = manifest.executionFunctions[i].skipRuntimeValidation;
bool allowGlobalValidation = manifest.executionFunctions[i].allowGlobalValidation;
_setExecutionFunction(selector, isPublic, allowGlobalValidation, module);
_setExecutionFunction(selector, skipRuntimeValidation, allowGlobalValidation, module);
}

length = manifest.executionHooks.length;
Expand Down
2 changes: 1 addition & 1 deletion src/account/ReferenceModularAccount.sol
Original file line number Diff line number Diff line change
Expand Up @@ -559,7 +559,7 @@ contract ReferenceModularAccount is
// and the selector isn't public.
if (
msg.sender != address(_ENTRY_POINT) && msg.sender != address(this)
&& !_storage.executionData[msg.sig].isPublic
&& !_storage.executionData[msg.sig].skipRuntimeValidation
) {
ModuleEntity directCallValidationKey =
ModuleEntityLib.pack(msg.sender, DIRECT_CALL_VALIDATION_ENTITYID);
Expand Down
2 changes: 1 addition & 1 deletion src/interfaces/IExecutionModule.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ struct ManifestExecutionFunction {
// The selector to install
bytes4 executionSelector;
// If true, the function won't need runtime validation, and can be called by anyone.
bool isPublic;
bool skipRuntimeValidation;
// If true, the function can be validated by a global validation function.
bool allowGlobalValidation;
}
Expand Down
2 changes: 1 addition & 1 deletion src/interfaces/IModularAccountView.sol
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ struct ExecutionDataView {
// state changing if this flag is set to true.
// Note that even if this is set to true, user op validation will still be required, otherwise anyone could
// drain the account of native tokens by wasting gas.
bool isPublic;
bool skipRuntimeValidation;
// Whether or not a global validation function may be used to validate this function.
bool allowGlobalValidation;
// The execution hooks for this function selector.
Expand Down
6 changes: 3 additions & 3 deletions src/modules/TokenReceiverModule.sol
Original file line number Diff line number Diff line change
Expand Up @@ -59,17 +59,17 @@ contract TokenReceiverModule is BaseModule, IExecutionModule, IERC721Receiver, I
manifest.executionFunctions = new ManifestExecutionFunction[](3);
manifest.executionFunctions[0] = ManifestExecutionFunction({
executionSelector: this.onERC721Received.selector,
isPublic: true,
skipRuntimeValidation: true,
allowGlobalValidation: false
});
manifest.executionFunctions[1] = ManifestExecutionFunction({
executionSelector: this.onERC1155Received.selector,
isPublic: true,
skipRuntimeValidation: true,
allowGlobalValidation: false
});
manifest.executionFunctions[2] = ManifestExecutionFunction({
executionSelector: this.onERC1155BatchReceived.selector,
isPublic: true,
skipRuntimeValidation: true,
allowGlobalValidation: false
});

Expand Down
7 changes: 4 additions & 3 deletions standard/ERCs/erc-6900.md
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ struct ExecutionDataView {
// state changing if this flag is set to true.
// Note that even if this is set to true, user op validation will still be required, otherwise anyone could
// drain the account of native tokens by wasting gas.
bool isPublic;
bool skipRuntimeValidation;
// Whether or not a global validation function may be used to validate this function.
bool allowGlobalValidation;
// The execution hooks for this function selector.
Expand Down Expand Up @@ -422,7 +422,8 @@ Execution module interface. Modules **MAY** implement this interface to provide
struct ManifestExecutionFunction {
// The selector to install
bytes4 executionSelector;
bool isPublic;
// Whether or not the function needs runtime validation, or can be called by anyone.
bool skipRuntimeValidation;
// If true, the function can be validated by a global validation function.
bool allowGlobalValidation;
}
Expand Down Expand Up @@ -570,7 +571,7 @@ After the execution of the target function, the modular account MUST run any pos

The set of hooks run for a given target function MUST be the hooks specified by account state at the start of the execution phase. In other words, if the set of applicable hooks changes during execution, the original set of hooks MUST still run, and only future invocations of the same target function should reflect the changed set of hooks.

Module execution functions where the field `isPublic` is set to true, or native functions without access control, SHOULD omit the runtime validation step, including any runtime validation hooks. Native functions without access control MAY also omit running execution hooks.
Module execution functions where the field `skipRuntimeValidation` is set to true, or native functions without access control, SHOULD omit the runtime validation step, including any runtime validation hooks. Native functions without access control MAY also omit running execution hooks.

### Extension

Expand Down
2 changes: 1 addition & 1 deletion test/account/AccountExecHooks.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ contract AccountExecHooksTest is AccountTestBase {
_m1.executionFunctions.push(
ManifestExecutionFunction({
executionSelector: _EXEC_SELECTOR,
isPublic: true,
skipRuntimeValidation: true,
allowGlobalValidation: false
})
);
Expand Down
4 changes: 2 additions & 2 deletions test/account/ModularAccountView.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ contract ModularAccountViewTest is CustomValidationTestBase {
ExecutionDataView memory data = account1.getExecutionData(selectorsToCheck[i]);
assertEq(data.module, address(account1));
assertTrue(data.allowGlobalValidation);
assertFalse(data.isPublic);
assertFalse(data.skipRuntimeValidation);
}
}

Expand All @@ -63,7 +63,7 @@ contract ModularAccountViewTest is CustomValidationTestBase {
ExecutionDataView memory data = account1.getExecutionData(selectorsToCheck[i]);
assertEq(data.module, expectedModuleAddress[i]);
assertFalse(data.allowGlobalValidation);
assertFalse(data.isPublic);
assertFalse(data.skipRuntimeValidation);

HookConfig[3] memory expectedHooks = [
HookConfigLib.packExecHook(
Expand Down
2 changes: 1 addition & 1 deletion test/account/ReplaceModule.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ contract UpgradeModuleTest is AccountTestBase {
ManifestExecutionFunction[] memory executionFunctions = new ManifestExecutionFunction[](1);
executionFunctions[0] = ManifestExecutionFunction({
executionSelector: TestModule.testFunction.selector,
isPublic: true,
skipRuntimeValidation: true,
allowGlobalValidation: true
});
m.executionFunctions = executionFunctions;
Expand Down
2 changes: 1 addition & 1 deletion test/mocks/modules/ComprehensiveModule.sol
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ contract ComprehensiveModule is
manifest.executionFunctions = new ManifestExecutionFunction[](1);
manifest.executionFunctions[0] = ManifestExecutionFunction({
executionSelector: this.foo.selector,
isPublic: false,
skipRuntimeValidation: false,
allowGlobalValidation: false
});

Expand Down
2 changes: 1 addition & 1 deletion test/mocks/modules/PermittedCallMocks.sol
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ contract PermittedCallerModule is IExecutionModule, BaseModule {
manifest.executionFunctions[1].executionSelector = this.usePermittedCallNotAllowed.selector;

for (uint256 i = 0; i < manifest.executionFunctions.length; i++) {
manifest.executionFunctions[i].isPublic = true;
manifest.executionFunctions[i].skipRuntimeValidation = true;
}

return manifest;
Expand Down
8 changes: 4 additions & 4 deletions test/mocks/modules/ReturnDataModuleMocks.sol
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,12 @@ contract ResultCreatorModule is IExecutionModule, BaseModule {
manifest.executionFunctions = new ManifestExecutionFunction[](2);
manifest.executionFunctions[0] = ManifestExecutionFunction({
executionSelector: this.foo.selector,
isPublic: true,
skipRuntimeValidation: true,
allowGlobalValidation: false
});
manifest.executionFunctions[1] = ManifestExecutionFunction({
executionSelector: this.bar.selector,
isPublic: false,
skipRuntimeValidation: false,
allowGlobalValidation: false
});

Expand Down Expand Up @@ -126,12 +126,12 @@ contract ResultConsumerModule is IExecutionModule, BaseModule, IValidationModule
manifest.executionFunctions = new ManifestExecutionFunction[](2);
manifest.executionFunctions[0] = ManifestExecutionFunction({
executionSelector: this.checkResultFallback.selector,
isPublic: true,
skipRuntimeValidation: true,
allowGlobalValidation: false
});
manifest.executionFunctions[1] = ManifestExecutionFunction({
executionSelector: this.checkResultExecuteWithAuthorization.selector,
isPublic: true,
skipRuntimeValidation: true,
allowGlobalValidation: false
});

Expand Down
6 changes: 3 additions & 3 deletions test/mocks/modules/ValidationModuleMocks.sol
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ contract MockUserOpValidationModule is MockBaseUserOpValidationModule {
manifest.executionFunctions = new ManifestExecutionFunction[](1);
manifest.executionFunctions[0] = ManifestExecutionFunction({
executionSelector: this.foo.selector,
isPublic: false,
skipRuntimeValidation: false,
allowGlobalValidation: false
});

Expand Down Expand Up @@ -149,7 +149,7 @@ contract MockUserOpValidation1HookModule is MockBaseUserOpValidationModule {
manifest.executionFunctions = new ManifestExecutionFunction[](1);
manifest.executionFunctions[0] = ManifestExecutionFunction({
executionSelector: this.bar.selector,
isPublic: false,
skipRuntimeValidation: false,
allowGlobalValidation: false
});

Expand Down Expand Up @@ -184,7 +184,7 @@ contract MockUserOpValidation2HookModule is MockBaseUserOpValidationModule {
manifest.executionFunctions = new ManifestExecutionFunction[](1);
manifest.executionFunctions[0] = ManifestExecutionFunction({
executionSelector: this.baz.selector,
isPublic: false,
skipRuntimeValidation: false,
allowGlobalValidation: false
});

Expand Down

0 comments on commit f55d9ff

Please sign in to comment.