From 701212aac2a8b69f09ae9b86436c9b004da08c62 Mon Sep 17 00:00:00 2001 From: livingrockrises <90545960+livingrockrises@users.noreply.github.com> Date: Mon, 14 Apr 2025 21:25:14 +0400 Subject: [PATCH 1/7] fix: add missing storage bases for erc7779 --- foundry.toml | 2 +- src/MSAAdvanced.sol | 4 ++++ src/core/ModuleManager.sol | 2 ++ src/core/PreValidationHookManager.sol | 1 + 4 files changed, 8 insertions(+), 1 deletion(-) diff --git a/foundry.toml b/foundry.toml index ff7f6c4..6c48258 100644 --- a/foundry.toml +++ b/foundry.toml @@ -1,5 +1,5 @@ [profile.default] -evm_version = "cancun" +evm_version = "prague" src = "src" out = "out" libs = ["node_modules"] diff --git a/src/MSAAdvanced.sol b/src/MSAAdvanced.sol index c9db2cb..d20a90d 100644 --- a/src/MSAAdvanced.sol +++ b/src/MSAAdvanced.sol @@ -419,6 +419,7 @@ contract MSAAdvanced is if (isERC7702) { _addStorageBase(MODULEMANAGER_STORAGE_LOCATION); _addStorageBase(HOOKMANAGER_STORAGE_LOCATION); + _addStorageBase(PREVALIDATION_HOOKMANAGER_STORAGE_LOCATION); } // bootstrap the account @@ -442,6 +443,9 @@ contract MSAAdvanced is _tryUninstallValidators(); _tryUninstallExecutors(); _tryUninstallHook(_getHook()); + // Review + _tryUninstallPreValidationHook(_getPreValidationHook(MODULE_TYPE_PREVALIDATION_HOOK_ERC1271), MODULE_TYPE_PREVALIDATION_HOOK_ERC1271); + _tryUninstallPreValidationHook(_getPreValidationHook(MODULE_TYPE_PREVALIDATION_HOOK_ERC4337), MODULE_TYPE_PREVALIDATION_HOOK_ERC4337); _initModuleManager(); } } diff --git a/src/core/ModuleManager.sol b/src/core/ModuleManager.sol index 24ffd64..0a9a89f 100644 --- a/src/core/ModuleManager.sol +++ b/src/core/ModuleManager.sol @@ -95,6 +95,7 @@ abstract contract ModuleManager is AccountBase, Receiver { IValidator(validator).onUninstall(disableModuleData); } + // Review: This can be removed. /* function _tryUninstallValidators(bytes[] calldata data) internal { SentinelListLib.SentinelList storage $valdiators = $moduleManager().$valdiators; @@ -168,6 +169,7 @@ abstract contract ModuleManager is AccountBase, Receiver { IExecutor(executor).onUninstall(disableModuleData); } + // Review: This can be removed. /* function _tryUninstallExecutors(bytes[] calldata data) internal { SentinelListLib.SentinelList storage $executors = $moduleManager().$executors; diff --git a/src/core/PreValidationHookManager.sol b/src/core/PreValidationHookManager.sol index f672d82..991d813 100644 --- a/src/core/PreValidationHookManager.sol +++ b/src/core/PreValidationHookManager.sol @@ -92,6 +92,7 @@ abstract contract PreValidationHookManager { } function _tryUninstallPreValidationHook(address hook, uint256 hookType) internal virtual { + if (hook == address(0)) return; PreValidationHookManagerStorage storage $ = _getStorage(); if (hookType == MODULE_TYPE_PREVALIDATION_HOOK_ERC1271) { try $.hook1271.onUninstall("") { } From 430393db1ac9b2f2cd916152f9f0646394df1bb3 Mon Sep 17 00:00:00 2001 From: livingrockrises <90545960+livingrockrises@users.noreply.github.com> Date: Mon, 14 Apr 2025 21:28:41 +0400 Subject: [PATCH 2/7] chore: lint --- src/MSAAdvanced.sol | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/MSAAdvanced.sol b/src/MSAAdvanced.sol index d20a90d..cc12b64 100644 --- a/src/MSAAdvanced.sol +++ b/src/MSAAdvanced.sol @@ -444,8 +444,14 @@ contract MSAAdvanced is _tryUninstallExecutors(); _tryUninstallHook(_getHook()); // Review - _tryUninstallPreValidationHook(_getPreValidationHook(MODULE_TYPE_PREVALIDATION_HOOK_ERC1271), MODULE_TYPE_PREVALIDATION_HOOK_ERC1271); - _tryUninstallPreValidationHook(_getPreValidationHook(MODULE_TYPE_PREVALIDATION_HOOK_ERC4337), MODULE_TYPE_PREVALIDATION_HOOK_ERC4337); + _tryUninstallPreValidationHook( + _getPreValidationHook(MODULE_TYPE_PREVALIDATION_HOOK_ERC1271), + MODULE_TYPE_PREVALIDATION_HOOK_ERC1271 + ); + _tryUninstallPreValidationHook( + _getPreValidationHook(MODULE_TYPE_PREVALIDATION_HOOK_ERC4337), + MODULE_TYPE_PREVALIDATION_HOOK_ERC4337 + ); _initModuleManager(); } } From 40318b945d4f91fd61580bcf2c58e6039386e541 Mon Sep 17 00:00:00 2001 From: livingrockrises <90545960+livingrockrises@users.noreply.github.com> Date: Tue, 15 Apr 2025 17:26:19 +0400 Subject: [PATCH 3/7] chore: refactor --- src/MSAAdvanced.sol | 1 - src/core/ModuleManager.sol | 44 -------------------------------------- 2 files changed, 45 deletions(-) diff --git a/src/MSAAdvanced.sol b/src/MSAAdvanced.sol index cc12b64..b3e8146 100644 --- a/src/MSAAdvanced.sol +++ b/src/MSAAdvanced.sol @@ -443,7 +443,6 @@ contract MSAAdvanced is _tryUninstallValidators(); _tryUninstallExecutors(); _tryUninstallHook(_getHook()); - // Review _tryUninstallPreValidationHook( _getPreValidationHook(MODULE_TYPE_PREVALIDATION_HOOK_ERC1271), MODULE_TYPE_PREVALIDATION_HOOK_ERC1271 diff --git a/src/core/ModuleManager.sol b/src/core/ModuleManager.sol index 0a9a89f..16e1274 100644 --- a/src/core/ModuleManager.sol +++ b/src/core/ModuleManager.sol @@ -95,28 +95,6 @@ abstract contract ModuleManager is AccountBase, Receiver { IValidator(validator).onUninstall(disableModuleData); } - // Review: This can be removed. - /* - function _tryUninstallValidators(bytes[] calldata data) internal { - SentinelListLib.SentinelList storage $valdiators = $moduleManager().$valdiators; - uint256 length = data.length; - uint256 index; - address validator = $valdiators.getNext(SENTINEL); - while (validator != SENTINEL) { - bytes memory uninstallData; - if (index < length) { - uninstallData = data[index]; - } - try IValidator(validator).onUninstall(uninstallData) {} catch { - emit ValidatorUninstallFailed(validator, uninstallData); - } - validator = $valdiators.getNext(validator); - index++; - } - $valdiators.popAll(); - } - */ - function _tryUninstallValidators() internal { SentinelListLib.SentinelList storage $valdiators = $moduleManager().$valdiators; address validator = $valdiators.getNext(SENTINEL); @@ -169,28 +147,6 @@ abstract contract ModuleManager is AccountBase, Receiver { IExecutor(executor).onUninstall(disableModuleData); } - // Review: This can be removed. - /* - function _tryUninstallExecutors(bytes[] calldata data) internal { - SentinelListLib.SentinelList storage $executors = $moduleManager().$executors; - uint256 length = data.length; - uint256 index; - address executor = $executors.getNext(SENTINEL); - while (executor != SENTINEL) { - bytes memory uninstallData; - if (index < length) { - uninstallData = data[index]; - } - try IExecutor(executor).onUninstall(uninstallData) {} catch { - emit ExecutorUninstallFailed(executor, uninstallData); - } - executor = $executors.getNext(executor); - index++; - } - $executors.popAll(); - } - */ - function _tryUninstallExecutors() internal { SentinelListLib.SentinelList storage $executors = $moduleManager().$executors; address executor = $executors.getNext(SENTINEL); From a6f3ec9120ee2bc96da20f01677f364765588a14 Mon Sep 17 00:00:00 2001 From: livingrockrises <90545960+livingrockrises@users.noreply.github.com> Date: Wed, 7 May 2025 14:17:13 +0400 Subject: [PATCH 4/7] fix: avoid adding duplicate storage bases --- src/core/ERC7779Adapter.sol | 6 ++++++ src/core/ModuleManager.sol | 40 +++++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/src/core/ERC7779Adapter.sol b/src/core/ERC7779Adapter.sol index e35fdec..2de7014 100644 --- a/src/core/ERC7779Adapter.sol +++ b/src/core/ERC7779Adapter.sol @@ -42,6 +42,12 @@ abstract contract ERC7779Adapter is IERC7779 { assembly { $.slot := ERC7779_STORAGE_BASE } + // Check if the storageBase already exists to avoid duplication + for (uint256 i = 0; i < $.storageBases.length; i++) { + if ($.storageBases[i] == storageBase) { + return; // Exit if the storageBase is already present + } + } $.storageBases.push(storageBase); } diff --git a/src/core/ModuleManager.sol b/src/core/ModuleManager.sol index 16e1274..dcafd88 100644 --- a/src/core/ModuleManager.sol +++ b/src/core/ModuleManager.sol @@ -108,6 +108,26 @@ abstract contract ModuleManager is AccountBase, Receiver { $valdiators.popAll(); } + function _tryUninstallValidators(bytes[] calldata data) internal { + SentinelListLib.SentinelList storage $valdiators = $moduleManager().$valdiators; + uint256 length = data.length; + uint256 index; + address validator = $valdiators.getNext(SENTINEL); + while (validator != SENTINEL) { + bytes memory uninstallData; + if (index < length) { + uninstallData = data[index]; + } + try IValidator(validator).onUninstall(uninstallData) { } + catch { + emit ValidatorUninstallFailed(validator, uninstallData); + } + validator = $valdiators.getNext(validator); + index++; + } + $valdiators.popAll(); + } + function _isValidatorInstalled(address validator) internal view virtual returns (bool) { SentinelListLib.SentinelList storage $valdiators = $moduleManager().$valdiators; return $valdiators.contains(validator); @@ -160,6 +180,26 @@ abstract contract ModuleManager is AccountBase, Receiver { $executors.popAll(); } + function _tryUninstallExecutors(bytes[] calldata data) internal { + SentinelListLib.SentinelList storage $executors = $moduleManager().$executors; + uint256 length = data.length; + uint256 index; + address executor = $executors.getNext(SENTINEL); + while (executor != SENTINEL) { + bytes memory uninstallData; + if (index < length) { + uninstallData = data[index]; + } + try IExecutor(executor).onUninstall(uninstallData) { } + catch { + emit ExecutorUninstallFailed(executor, uninstallData); + } + executor = $executors.getNext(executor); + index++; + } + $executors.popAll(); + } + function _isExecutorInstalled(address executor) internal view virtual returns (bool) { SentinelListLib.SentinelList storage $executors = $moduleManager().$executors; return $executors.contains(executor); From f2223af5ba49c5b94fe68d705da17952382d7210 Mon Sep 17 00:00:00 2001 From: livingrockrises <90545960+livingrockrises@users.noreply.github.com> Date: Wed, 7 May 2025 17:25:33 +0400 Subject: [PATCH 5/7] feat: onRedelegation to have context(data) passed --- src/MSAAdvanced.sol | 12 +++++++++--- src/core/ERC7779Adapter.sol | 8 +++++--- src/core/ModuleManager.sol | 8 ++++---- src/interfaces/IERC7779.sol | 3 ++- test/advanced/EIP7702.t.sol | 2 +- test/mocks/MockERC7779.sol | 2 +- 6 files changed, 22 insertions(+), 13 deletions(-) diff --git a/src/MSAAdvanced.sol b/src/MSAAdvanced.sol index b3e8146..96db8d9 100644 --- a/src/MSAAdvanced.sol +++ b/src/MSAAdvanced.sol @@ -439,9 +439,15 @@ contract MSAAdvanced is if (!success) revert(); } - function _onRedelegation() internal override { - _tryUninstallValidators(); - _tryUninstallExecutors(); + function _onRedelegation(bytes calldata context) internal override { + // Decode the context data which should contain arrays of uninstall data for validators and executors + (bytes[] memory validatorUninstallData, bytes[] memory executorUninstallData) = abi.decode(context, (bytes[], bytes[])); + + // Call the uninstall functions with the decoded data + _tryUninstallValidators(validatorUninstallData); + _tryUninstallExecutors(executorUninstallData); + + // Continue with other uninstallations _tryUninstallHook(_getHook()); _tryUninstallPreValidationHook( _getPreValidationHook(MODULE_TYPE_PREVALIDATION_HOOK_ERC1271), diff --git a/src/core/ERC7779Adapter.sol b/src/core/ERC7779Adapter.sol index 2de7014..7f2ed50 100644 --- a/src/core/ERC7779Adapter.sol +++ b/src/core/ERC7779Adapter.sol @@ -60,14 +60,16 @@ abstract contract ERC7779Adapter is IERC7779 { * It should uninitialize storages if needed and execute wallet-specific logic to prepare for redelegation. * msg.sender should be the owner of the account. + * @param context - The context of the redelegation. Additional data required to uninitialize storages. */ - function onRedelegation() external returns (bool) { + function onRedelegation(bytes calldata context) external returns (bool) { require(msg.sender == address(this), NonAuthorizedOnRedelegationCaller()); - _onRedelegation(); + _onRedelegation(context); return true; } /// @dev This function is called before redelegation. /// @dev Account should override this function to implement the specific logic. - function _onRedelegation() internal virtual; + /// @param context The context is the additional data required to uninitialize storages. + function _onRedelegation(bytes calldata context) internal virtual; } diff --git a/src/core/ModuleManager.sol b/src/core/ModuleManager.sol index dcafd88..4ff31dc 100644 --- a/src/core/ModuleManager.sol +++ b/src/core/ModuleManager.sol @@ -95,7 +95,7 @@ abstract contract ModuleManager is AccountBase, Receiver { IValidator(validator).onUninstall(disableModuleData); } - function _tryUninstallValidators() internal { + function _tryUninstallValidators() internal virtual { SentinelListLib.SentinelList storage $valdiators = $moduleManager().$valdiators; address validator = $valdiators.getNext(SENTINEL); while (validator != SENTINEL) { @@ -108,7 +108,7 @@ abstract contract ModuleManager is AccountBase, Receiver { $valdiators.popAll(); } - function _tryUninstallValidators(bytes[] calldata data) internal { + function _tryUninstallValidators(bytes[] memory data) internal virtual { SentinelListLib.SentinelList storage $valdiators = $moduleManager().$valdiators; uint256 length = data.length; uint256 index; @@ -167,7 +167,7 @@ abstract contract ModuleManager is AccountBase, Receiver { IExecutor(executor).onUninstall(disableModuleData); } - function _tryUninstallExecutors() internal { + function _tryUninstallExecutors() internal virtual { SentinelListLib.SentinelList storage $executors = $moduleManager().$executors; address executor = $executors.getNext(SENTINEL); while (executor != SENTINEL) { @@ -180,7 +180,7 @@ abstract contract ModuleManager is AccountBase, Receiver { $executors.popAll(); } - function _tryUninstallExecutors(bytes[] calldata data) internal { + function _tryUninstallExecutors(bytes[] memory data) internal virtual { SentinelListLib.SentinelList storage $executors = $moduleManager().$executors; uint256 length = data.length; uint256 index; diff --git a/src/interfaces/IERC7779.sol b/src/interfaces/IERC7779.sol index 74dc068..06a76c5 100644 --- a/src/interfaces/IERC7779.sol +++ b/src/interfaces/IERC7779.sol @@ -27,6 +27,7 @@ interface IERC7779 { * It should uninitialize storages if needed and execute wallet-specific logic to prepare for redelegation. * msg.sender should be the owner of the account. + * @param context - The context of the redelegation. Additional data required to uninitialize storages. */ - function onRedelegation() external returns (bool); + function onRedelegation(bytes calldata context) external returns (bool); } diff --git a/test/advanced/EIP7702.t.sol b/test/advanced/EIP7702.t.sol index e3b5433..ca262a7 100644 --- a/test/advanced/EIP7702.t.sol +++ b/test/advanced/EIP7702.t.sol @@ -308,7 +308,7 @@ contract EIP7702 is TestBaseUtilAdvanced { assertTrue(IMSA(account).isModuleInstalled(MODULE_TYPE_HOOK, address(hook), "")); // storage is cleared vm.prank(address(account)); - IMSA(account).onRedelegation(); + IMSA(account).onRedelegation(bytes("")); assertFalse( IMSA(account).isModuleInstalled(MODULE_TYPE_VALIDATOR, address(defaultValidator), "") ); diff --git a/test/mocks/MockERC7779.sol b/test/mocks/MockERC7779.sol index fc89ca6..c4c7b3a 100644 --- a/test/mocks/MockERC7779.sol +++ b/test/mocks/MockERC7779.sol @@ -8,7 +8,7 @@ contract MockERC7779 is ERC7779Adapter { _addStorageBase(storageBase); } - function _onRedelegation() internal override { + function _onRedelegation(bytes calldata context) internal override { // do nothing } } From c179af70dfef5bae08cddb56fd863c717dcaa6ed Mon Sep 17 00:00:00 2001 From: livingrockrises <90545960+livingrockrises@users.noreply.github.com> Date: Wed, 7 May 2025 20:18:49 +0400 Subject: [PATCH 6/7] chore: lint --- src/MSAAdvanced.sol | 10 ++++++---- src/core/ERC7779Adapter.sol | 3 ++- src/interfaces/IERC7779.sol | 3 ++- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/MSAAdvanced.sol b/src/MSAAdvanced.sol index 96db8d9..344459a 100644 --- a/src/MSAAdvanced.sol +++ b/src/MSAAdvanced.sol @@ -440,13 +440,15 @@ contract MSAAdvanced is } function _onRedelegation(bytes calldata context) internal override { - // Decode the context data which should contain arrays of uninstall data for validators and executors - (bytes[] memory validatorUninstallData, bytes[] memory executorUninstallData) = abi.decode(context, (bytes[], bytes[])); - + // Decode the context data which should contain arrays of uninstall data for validators and + // executors + (bytes[] memory validatorUninstallData, bytes[] memory executorUninstallData) = + abi.decode(context, (bytes[], bytes[])); + // Call the uninstall functions with the decoded data _tryUninstallValidators(validatorUninstallData); _tryUninstallExecutors(executorUninstallData); - + // Continue with other uninstallations _tryUninstallHook(_getHook()); _tryUninstallPreValidationHook( diff --git a/src/core/ERC7779Adapter.sol b/src/core/ERC7779Adapter.sol index 7f2ed50..ab971f9 100644 --- a/src/core/ERC7779Adapter.sol +++ b/src/core/ERC7779Adapter.sol @@ -60,7 +60,8 @@ abstract contract ERC7779Adapter is IERC7779 { * It should uninitialize storages if needed and execute wallet-specific logic to prepare for redelegation. * msg.sender should be the owner of the account. - * @param context - The context of the redelegation. Additional data required to uninitialize storages. + * @param context - The context of the redelegation. Additional data required to uninitialize + storages. */ function onRedelegation(bytes calldata context) external returns (bool) { require(msg.sender == address(this), NonAuthorizedOnRedelegationCaller()); diff --git a/src/interfaces/IERC7779.sol b/src/interfaces/IERC7779.sol index 06a76c5..0e57f6f 100644 --- a/src/interfaces/IERC7779.sol +++ b/src/interfaces/IERC7779.sol @@ -27,7 +27,8 @@ interface IERC7779 { * It should uninitialize storages if needed and execute wallet-specific logic to prepare for redelegation. * msg.sender should be the owner of the account. - * @param context - The context of the redelegation. Additional data required to uninitialize storages. + * @param context - The context of the redelegation. Additional data required to uninitialize + storages. */ function onRedelegation(bytes calldata context) external returns (bool); } From 9ccedd162ebf91959e25e7b5cfd6d9bafca2c17f Mon Sep 17 00:00:00 2001 From: livingrockrises <90545960+livingrockrises@users.noreply.github.com> Date: Thu, 8 May 2025 00:33:23 +0400 Subject: [PATCH 7/7] feat: allow to clear fallback handlers --- src/MSAAdvanced.sol | 12 ++++++++---- src/core/ModuleManager.sol | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/src/MSAAdvanced.sol b/src/MSAAdvanced.sol index 344459a..714322c 100644 --- a/src/MSAAdvanced.sol +++ b/src/MSAAdvanced.sol @@ -442,12 +442,15 @@ contract MSAAdvanced is function _onRedelegation(bytes calldata context) internal override { // Decode the context data which should contain arrays of uninstall data for validators and // executors - (bytes[] memory validatorUninstallData, bytes[] memory executorUninstallData) = - abi.decode(context, (bytes[], bytes[])); + // (bytes[] memory validatorUninstallData, bytes[] memory executorUninstallData) = + // abi.decode(context, (bytes[], bytes[])); // Call the uninstall functions with the decoded data - _tryUninstallValidators(validatorUninstallData); - _tryUninstallExecutors(executorUninstallData); + // _tryUninstallValidators(validatorUninstallData); + // _tryUninstallExecutors(executorUninstallData); + + _tryUninstallValidators(); + _tryUninstallExecutors(); // Continue with other uninstallations _tryUninstallHook(_getHook()); @@ -459,6 +462,7 @@ contract MSAAdvanced is _getPreValidationHook(MODULE_TYPE_PREVALIDATION_HOOK_ERC4337), MODULE_TYPE_PREVALIDATION_HOOK_ERC4337 ); + _tryUninstallFallbacks(); _initModuleManager(); } } diff --git a/src/core/ModuleManager.sol b/src/core/ModuleManager.sol index 4ff31dc..029ba6c 100644 --- a/src/core/ModuleManager.sol +++ b/src/core/ModuleManager.sol @@ -46,6 +46,8 @@ abstract contract ModuleManager is AccountBase, Receiver { // single fallback handler for all fallbacks // account vendors may implement this differently. This is just a reference implementation mapping(bytes4 selector => FallbackHandler fallbackHandler) $fallbacks; + ///< List of fallback selectors, initialized upon contract deployment. + bytes4[] fallbackSelectors; } function $moduleManager() internal pure virtual returns (ModuleManagerStorage storage $ims) { @@ -235,6 +237,8 @@ abstract contract ModuleManager is AccountBase, Receiver { revert("Function selector already used"); } $moduleManager().$fallbacks[selector] = FallbackHandler(handler, calltype); + // Add the selector to the maintained list of fallback selectors + $moduleManager().fallbackSelectors.push(selector); IFallback(handler).onInstall(initData); } @@ -260,9 +264,37 @@ abstract contract ModuleManager is AccountBase, Receiver { $moduleManager().$fallbacks[selector] = FallbackHandler(address(0), CallType.wrap(0x00)); + // Remove selector from fallbackSelectors via swap-and-pop + uint256 len = $moduleManager().fallbackSelectors.length; + for (uint256 i = 0; i < len; i++) { + if ($moduleManager().fallbackSelectors[i] == selector) { + $moduleManager().fallbackSelectors[i] = $moduleManager().fallbackSelectors[len - 1]; + $moduleManager().fallbackSelectors.pop(); + break; + } + } + IFallback(handler).onUninstall(_deInitData); } + // Review: if uninstalling selectors also need some data. + function _tryUninstallFallbacks() internal { + uint256 len = $moduleManager().fallbackSelectors.length; + + for (uint256 i = 0; i < len; i++) { + bytes4 selector = $moduleManager().fallbackSelectors[i]; + FallbackHandler memory fallbackHandler = $moduleManager().$fallbacks[selector]; + + if (address(fallbackHandler.handler) == address(0)) continue; + + IFallback(fallbackHandler.handler).onUninstall(abi.encodePacked(selector)); + + $moduleManager().$fallbacks[selector] = FallbackHandler(address(0), CallType.wrap(0x00)); + } + + delete $moduleManager().fallbackSelectors; + } + function _isFallbackHandlerInstalled(bytes4 functionSig) internal view virtual returns (bool) { FallbackHandler storage $fallback = $moduleManager().$fallbacks[functionSig]; return $fallback.handler != address(0);