diff --git a/src/account/AccountLoupe.sol b/src/account/AccountLoupe.sol index 0593298a..9e9053fe 100644 --- a/src/account/AccountLoupe.sol +++ b/src/account/AccountLoupe.sol @@ -7,13 +7,7 @@ import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet import {IAccountLoupe, ExecutionHook} from "../interfaces/IAccountLoupe.sol"; import {FunctionReference, IPluginManager} from "../interfaces/IPluginManager.sol"; import {IStandardExecutor} from "../interfaces/IStandardExecutor.sol"; -import { - AccountStorage, - getAccountStorage, - SelectorData, - toFunctionReferenceArray, - toExecutionHook -} from "./AccountStorage.sol"; +import {getAccountStorage, SelectorData, toFunctionReferenceArray, toExecutionHook} from "./AccountStorage.sol"; abstract contract AccountLoupe is IAccountLoupe { using EnumerableSet for EnumerableSet.Bytes32Set; @@ -21,8 +15,6 @@ abstract contract AccountLoupe is IAccountLoupe { /// @inheritdoc IAccountLoupe function getExecutionFunctionHandler(bytes4 selector) external view override returns (address plugin) { - AccountStorage storage _storage = getAccountStorage(); - if ( selector == IStandardExecutor.execute.selector || selector == IStandardExecutor.executeBatch.selector || selector == UUPSUpgradeable.upgradeToAndCall.selector @@ -32,7 +24,7 @@ abstract contract AccountLoupe is IAccountLoupe { return address(this); } - return _storage.selectorData[selector].plugin; + return getAccountStorage().selectorData[selector].plugin; } /// @inheritdoc IAccountLoupe diff --git a/src/account/PluginManagerInternals.sol b/src/account/PluginManagerInternals.sol index 7f8337e4..cfdbef16 100644 --- a/src/account/PluginManagerInternals.sol +++ b/src/account/PluginManagerInternals.sol @@ -86,8 +86,9 @@ abstract contract PluginManagerInternals is IPluginManager { { SelectorData storage _selectorData = getAccountStorage().selectorData[selector]; - // Fail on duplicate definitions - otherwise dependencies could shadow non-depdency - // validation functions, leading to partial uninstalls. + // Fail on duplicate validation functions. Otherwise, dependency validation functions could shadow + // non-depdency validation functions. Then, if a either plugin is uninstall, it would cause a partial + // uninstall of the other. if (!_selectorData.validations.add(toSetValue(validationFunction))) { revert ValidationFunctionAlreadySet(selector, validationFunction); } diff --git a/src/account/UpgradeableModularAccount.sol b/src/account/UpgradeableModularAccount.sol index 84a73cd3..69a11cbd 100644 --- a/src/account/UpgradeableModularAccount.sol +++ b/src/account/UpgradeableModularAccount.sol @@ -268,9 +268,10 @@ contract UpgradeableModularAccount is payable returns (bytes memory) { - bytes4 execSelector = bytes4(data[0:4]); + bytes4 execSelector = bytes4(data[:4]); - FunctionReference runtimeValidationFunction = FunctionReference.wrap(bytes21(authorization[0:21])); + // Revert if the provided `authorization` less than 21 bytes long, rather than right-padding. + FunctionReference runtimeValidationFunction = FunctionReference.wrap(bytes21(authorization[:21])); AccountStorage storage _storage = getAccountStorage(); @@ -395,6 +396,7 @@ contract UpgradeableModularAccount is revert AlwaysDenyRule(); } + // Revert if the provided `authorization` less than 21 bytes long, rather than right-padding. FunctionReference userOpValidationFunction = FunctionReference.wrap(bytes21(userOp.signature[:21])); if (!getAccountStorage().selectorData[selector].validations.contains(toSetValue(userOpValidationFunction))) @@ -463,7 +465,7 @@ contract UpgradeableModularAccount is ) internal { // run all preRuntimeValidation hooks EnumerableSet.Bytes32Set storage preRuntimeValidationHooks = - getAccountStorage().selectorData[bytes4(callData[0:4])].preValidationHooks; + getAccountStorage().selectorData[bytes4(callData[:4])].preValidationHooks; uint256 preRuntimeValidationHooksLength = preRuntimeValidationHooks.length(); for (uint256 i = 0; i < preRuntimeValidationHooksLength; ++i) {