Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: rename isPublic #172

Merged
merged 1 commit into from
Aug 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading