From f55d9ffe83c2159e351922ea0258656baf44866e Mon Sep 17 00:00:00 2001 From: adam Date: Fri, 30 Aug 2024 13:05:04 -0400 Subject: [PATCH] refactor: rename isPublic to skipRuntimeValidation --- src/account/AccountStorage.sol | 2 +- src/account/ModularAccountView.sol | 2 +- src/account/ModuleManagerInternals.sol | 17 ++++++++++------- src/account/ReferenceModularAccount.sol | 2 +- src/interfaces/IExecutionModule.sol | 2 +- src/interfaces/IModularAccountView.sol | 2 +- src/modules/TokenReceiverModule.sol | 6 +++--- standard/ERCs/erc-6900.md | 7 ++++--- test/account/AccountExecHooks.t.sol | 2 +- test/account/ModularAccountView.t.sol | 4 ++-- test/account/ReplaceModule.t.sol | 2 +- test/mocks/modules/ComprehensiveModule.sol | 2 +- test/mocks/modules/PermittedCallMocks.sol | 2 +- test/mocks/modules/ReturnDataModuleMocks.sol | 8 ++++---- test/mocks/modules/ValidationModuleMocks.sol | 6 +++--- 15 files changed, 35 insertions(+), 31 deletions(-) diff --git a/src/account/AccountStorage.sol b/src/account/AccountStorage.sol index 79393208..9e9aa00c 100644 --- a/src/account/AccountStorage.sol +++ b/src/account/AccountStorage.sol @@ -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. diff --git a/src/account/ModularAccountView.sol b/src/account/ModularAccountView.sol index 36150fde..ecf801cf 100644 --- a/src/account/ModularAccountView.sol +++ b/src/account/ModularAccountView.sol @@ -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(); diff --git a/src/account/ModuleManagerInternals.sol b/src/account/ModuleManagerInternals.sol index 9f6ee1a2..5b5e53cd 100644 --- a/src/account/ModuleManagerInternals.sol +++ b/src/account/ModuleManagerInternals.sol @@ -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)) { @@ -77,7 +80,7 @@ abstract contract ModuleManagerInternals is IModularAccount { } _executionData.module = module; - _executionData.isPublic = isPublic; + _executionData.skipRuntimeValidation = skipRuntimeValidation; _executionData.allowGlobalValidation = allowGlobalValidation; } @@ -85,7 +88,7 @@ abstract contract ModuleManagerInternals is IModularAccount { ExecutionData storage _executionData = getAccountStorage().executionData[selector]; _executionData.module = address(0); - _executionData.isPublic = false; + _executionData.skipRuntimeValidation = false; _executionData.allowGlobalValidation = false; } @@ -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; diff --git a/src/account/ReferenceModularAccount.sol b/src/account/ReferenceModularAccount.sol index d142e8ef..fdf54555 100644 --- a/src/account/ReferenceModularAccount.sol +++ b/src/account/ReferenceModularAccount.sol @@ -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); diff --git a/src/interfaces/IExecutionModule.sol b/src/interfaces/IExecutionModule.sol index 267daf4d..2c03b8d6 100644 --- a/src/interfaces/IExecutionModule.sol +++ b/src/interfaces/IExecutionModule.sol @@ -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; } diff --git a/src/interfaces/IModularAccountView.sol b/src/interfaces/IModularAccountView.sol index 8b27cd97..6c20a561 100644 --- a/src/interfaces/IModularAccountView.sol +++ b/src/interfaces/IModularAccountView.sol @@ -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. diff --git a/src/modules/TokenReceiverModule.sol b/src/modules/TokenReceiverModule.sol index df02cd1f..eb1377e5 100644 --- a/src/modules/TokenReceiverModule.sol +++ b/src/modules/TokenReceiverModule.sol @@ -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 }); diff --git a/standard/ERCs/erc-6900.md b/standard/ERCs/erc-6900.md index d0720db9..424b7a75 100644 --- a/standard/ERCs/erc-6900.md +++ b/standard/ERCs/erc-6900.md @@ -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. @@ -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; } @@ -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 diff --git a/test/account/AccountExecHooks.t.sol b/test/account/AccountExecHooks.t.sol index bc61a205..bae59d45 100644 --- a/test/account/AccountExecHooks.t.sol +++ b/test/account/AccountExecHooks.t.sol @@ -33,7 +33,7 @@ contract AccountExecHooksTest is AccountTestBase { _m1.executionFunctions.push( ManifestExecutionFunction({ executionSelector: _EXEC_SELECTOR, - isPublic: true, + skipRuntimeValidation: true, allowGlobalValidation: false }) ); diff --git a/test/account/ModularAccountView.t.sol b/test/account/ModularAccountView.t.sol index 9806116b..f28f29bb 100644 --- a/test/account/ModularAccountView.t.sol +++ b/test/account/ModularAccountView.t.sol @@ -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); } } @@ -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( diff --git a/test/account/ReplaceModule.t.sol b/test/account/ReplaceModule.t.sol index 050a9e09..cd600c96 100644 --- a/test/account/ReplaceModule.t.sol +++ b/test/account/ReplaceModule.t.sol @@ -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; diff --git a/test/mocks/modules/ComprehensiveModule.sol b/test/mocks/modules/ComprehensiveModule.sol index cfaead0e..ba89fa78 100644 --- a/test/mocks/modules/ComprehensiveModule.sol +++ b/test/mocks/modules/ComprehensiveModule.sol @@ -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 }); diff --git a/test/mocks/modules/PermittedCallMocks.sol b/test/mocks/modules/PermittedCallMocks.sol index 41931d4f..1340f99f 100644 --- a/test/mocks/modules/PermittedCallMocks.sol +++ b/test/mocks/modules/PermittedCallMocks.sol @@ -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; diff --git a/test/mocks/modules/ReturnDataModuleMocks.sol b/test/mocks/modules/ReturnDataModuleMocks.sol index 1c7ff6a2..7d16dc05 100644 --- a/test/mocks/modules/ReturnDataModuleMocks.sol +++ b/test/mocks/modules/ReturnDataModuleMocks.sol @@ -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 }); @@ -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 }); diff --git a/test/mocks/modules/ValidationModuleMocks.sol b/test/mocks/modules/ValidationModuleMocks.sol index 40e6ad26..4c383744 100644 --- a/test/mocks/modules/ValidationModuleMocks.sol +++ b/test/mocks/modules/ValidationModuleMocks.sol @@ -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 }); @@ -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 }); @@ -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 });