review fixes

src/account/AccountLoupe.sol

@@ -7,22 +7,14 @@ 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;
     using EnumerableSet for EnumerableSet.AddressSet;
 
     /// @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

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);
         }

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) {