Skip to content

Commit

Permalink
Fix: Events and AccountLoupe revamp for data availability (#122)
Browse files Browse the repository at this point in the history
  • Loading branch information
@fangting-alchemy @adamegyed
fangting-alchemy authored and adamegyed committed Aug 5, 2024
1 parent 66d1fed commit db1e77a
Show file tree
Hide file tree
Showing 13 changed files with 179 additions and 194 deletions.
92 changes: 30 additions & 62 deletions src/account/AccountLoupe.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,95 +7,63 @@ import {EnumerableMap} from "@openzeppelin/contracts/utils/structs/EnumerableMap
import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";

import {HookConfigLib} from "../helpers/HookConfigLib.sol";
import {ExecutionHook, IAccountLoupe} from "../interfaces/IAccountLoupe.sol";
import {ExecutionDataView, IAccountLoupe, ValidationDataView} from "../interfaces/IAccountLoupe.sol";
import {HookConfig, IModuleManager, ModuleEntity} from "../interfaces/IModuleManager.sol";
import {IStandardExecutor} from "../interfaces/IStandardExecutor.sol";
import {getAccountStorage, toHookConfig, toSelector} from "./AccountStorage.sol";
import {ExecutionData, ValidationData, getAccountStorage} from "./AccountStorage.sol";

abstract contract AccountLoupe is IAccountLoupe {
using EnumerableSet for EnumerableSet.Bytes32Set;
using EnumerableMap for EnumerableMap.AddressToUintMap;
using HookConfigLib for HookConfig;

/// @inheritdoc IAccountLoupe
function getExecutionFunctionHandler(bytes4 selector) external view override returns (address module) {
function getExecutionData(bytes4 selector) external view override returns (ExecutionDataView memory data) {
if (
selector == IStandardExecutor.execute.selector || selector == IStandardExecutor.executeBatch.selector
|| selector == UUPSUpgradeable.upgradeToAndCall.selector
|| selector == IModuleManager.installExecution.selector
|| selector == IModuleManager.uninstallExecution.selector
) {
return address(this);
}

return getAccountStorage().selectorData[selector].module;
}

/// @inheritdoc IAccountLoupe
function getSelectors(ModuleEntity validationFunction) external view returns (bytes4[] memory) {
uint256 length = getAccountStorage().validationData[validationFunction].selectors.length();

bytes4[] memory selectors = new bytes4[](length);
data.module = address(this);
data.allowGlobalValidation = true;
} else {
ExecutionData storage executionData = getAccountStorage().executionData[selector];
data.module = executionData.module;
data.isPublic = executionData.isPublic;
data.allowGlobalValidation = executionData.allowGlobalValidation;

for (uint256 i = 0; i < length; ++i) {
selectors[i] = toSelector(getAccountStorage().validationData[validationFunction].selectors.at(i));
uint256 executionHooksLen = executionData.executionHooks.length();
data.executionHooks = new HookConfig[](executionHooksLen);
for (uint256 i = 0; i < executionHooksLen; ++i) {
data.executionHooks[i] = HookConfig.wrap(bytes26(executionData.executionHooks.at(i)));
}
}

return selectors;
}

/// @inheritdoc IAccountLoupe
function getExecutionHooks(bytes4 selector)
function getValidationData(ModuleEntity validationFunction)
external
view
override
returns (ExecutionHook[] memory execHooks)
returns (ValidationDataView memory data)
{
EnumerableSet.Bytes32Set storage hooks = getAccountStorage().selectorData[selector].executionHooks;
uint256 executionHooksLength = hooks.length();

execHooks = new ExecutionHook[](executionHooksLength);
ValidationData storage validationData = getAccountStorage().validationData[validationFunction];
data.isGlobal = validationData.isGlobal;
data.isSignatureValidation = validationData.isSignatureValidation;
data.preValidationHooks = validationData.preValidationHooks;

for (uint256 i = 0; i < executionHooksLength; ++i) {
bytes32 key = hooks.at(i);
HookConfig hookConfig = toHookConfig(key);
execHooks[i] = ExecutionHook({
hookFunction: hookConfig.moduleEntity(),
isPreHook: hookConfig.hasPreHook(),
isPostHook: hookConfig.hasPostHook()
});
uint256 permissionHooksLen = validationData.permissionHooks.length();
data.permissionHooks = new HookConfig[](permissionHooksLen);
for (uint256 i = 0; i < permissionHooksLen; ++i) {
data.permissionHooks[i] = HookConfig.wrap(bytes26(validationData.permissionHooks.at(i)));
}
}

/// @inheritdoc IAccountLoupe
function getPermissionHooks(ModuleEntity validationFunction)
external
view
override
returns (ExecutionHook[] memory permissionHooks)
{
EnumerableSet.Bytes32Set storage hooks =
getAccountStorage().validationData[validationFunction].permissionHooks;
uint256 executionHooksLength = hooks.length();
permissionHooks = new ExecutionHook[](executionHooksLength);
for (uint256 i = 0; i < executionHooksLength; ++i) {
bytes32 key = hooks.at(i);
HookConfig hookConfig = toHookConfig(key);
permissionHooks[i] = ExecutionHook({
hookFunction: hookConfig.moduleEntity(),
isPreHook: hookConfig.hasPreHook(),
isPostHook: hookConfig.hasPostHook()
});
bytes32[] memory selectors = validationData.selectors.values();
uint256 selectorsLen = selectors.length;
data.selectors = new bytes4[](selectorsLen);
for (uint256 j = 0; j < selectorsLen; ++j) {
data.selectors[j] = bytes4(selectors[j]);
}
}

/// @inheritdoc IAccountLoupe
function getPreValidationHooks(ModuleEntity validationFunction)
external
view
override
returns (ModuleEntity[] memory preValidationHooks)
{
preValidationHooks = getAccountStorage().validationData[validationFunction].preValidationHooks;
}
}
4 changes: 2 additions & 2 deletions src/account/AccountStorage.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {HookConfig, ModuleEntity} from "../interfaces/IModuleManager.sol";
bytes32 constant _ACCOUNT_STORAGE_SLOT = 0x9f09680beaa4e5c9f38841db2460c401499164f368baef687948c315d9073e40;

// Represents data associated with a specifc function selector.
struct SelectorData {
struct ExecutionData {
// The module that implements this execution function.
// If this is a native function, the address must remain address(0).
address module;
Expand Down Expand Up @@ -42,7 +42,7 @@ struct AccountStorage {
uint8 initialized;
bool initializing;
// Execution functions and their associated functions
mapping(bytes4 => SelectorData) selectorData;
mapping(bytes4 selector => ExecutionData) executionData;
mapping(ModuleEntity validationFunction => ValidationData) validationData;
// For ERC165 introspection
mapping(bytes4 => uint256) supportedIfaces;
Expand Down
51 changes: 32 additions & 19 deletions src/account/ModuleManagerInternals.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {IModule} from "../interfaces/IModule.sol";
import {HookConfig, IModuleManager, ModuleEntity, ValidationConfig} from "../interfaces/IModuleManager.sol";
import {
AccountStorage,
SelectorData,
ExecutionData,
ValidationData,
getAccountStorage,
toModuleEntity,
Expand Down Expand Up @@ -46,9 +46,9 @@ abstract contract ModuleManagerInternals is IModuleManager {
function _setExecutionFunction(bytes4 selector, bool isPublic, bool allowGlobalValidation, address module)
internal
{
SelectorData storage _selectorData = getAccountStorage().selectorData[selector];
ExecutionData storage _executionData = getAccountStorage().executionData[selector];

if (_selectorData.module != address(0)) {
if (_executionData.module != address(0)) {
revert ExecutionFunctionAlreadySet(selector);
}

Expand All @@ -72,17 +72,17 @@ abstract contract ModuleManagerInternals is IModuleManager {
revert Erc4337FunctionNotAllowed(selector);
}

_selectorData.module = module;
_selectorData.isPublic = isPublic;
_selectorData.allowGlobalValidation = allowGlobalValidation;
_executionData.module = module;
_executionData.isPublic = isPublic;
_executionData.allowGlobalValidation = allowGlobalValidation;
}

function _removeExecutionFunction(bytes4 selector) internal {
SelectorData storage _selectorData = getAccountStorage().selectorData[selector];
ExecutionData storage _executionData = getAccountStorage().executionData[selector];

_selectorData.module = address(0);
_selectorData.isPublic = false;
_selectorData.allowGlobalValidation = false;
_executionData.module = address(0);
_executionData.isPublic = false;
_executionData.allowGlobalValidation = false;
}

function _addValidationFunction(ValidationConfig validationConfig, bytes4[] memory selectors) internal {
Expand Down Expand Up @@ -152,7 +152,8 @@ abstract contract ModuleManagerInternals is IModuleManager {
length = manifest.executionHooks.length;
for (uint256 i = 0; i < length; ++i) {
ManifestExecutionHook memory mh = manifest.executionHooks[i];
EnumerableSet.Bytes32Set storage execHooks = _storage.selectorData[mh.executionSelector].executionHooks;
EnumerableSet.Bytes32Set storage execHooks =
_storage.executionData[mh.executionSelector].executionHooks;
HookConfig hookConfig = HookConfigLib.packExecHook({
_module: module,
_entityId: mh.entityId,
Expand Down Expand Up @@ -187,7 +188,8 @@ abstract contract ModuleManagerInternals is IModuleManager {
uint256 length = manifest.executionHooks.length;
for (uint256 i = 0; i < length; ++i) {
ManifestExecutionHook memory mh = manifest.executionHooks[i];
EnumerableSet.Bytes32Set storage execHooks = _storage.selectorData[mh.executionSelector].executionHooks;
EnumerableSet.Bytes32Set storage execHooks =
_storage.executionData[mh.executionSelector].executionHooks;
HookConfig hookConfig = HookConfigLib.packExecHook({
_module: module,
_entityId: mh.entityId,
Expand Down Expand Up @@ -225,9 +227,15 @@ abstract contract ModuleManagerInternals is IModuleManager {
}
}

function _onUninstall(address module, bytes calldata data) internal {
function _onUninstall(address module, bytes calldata data) internal returns (bool onUninstallSuccess) {
onUninstallSuccess = true;
if (data.length > 0) {
IModule(module).onUninstall(data);
// Clear the module storage for the account.
// solhint-disable-next-line no-empty-blocks
try IModule(module).onUninstall(data) {}
catch {
onUninstallSuccess = false;
}
}
}

Expand All @@ -239,6 +247,7 @@ abstract contract ModuleManagerInternals is IModuleManager {
) internal {
ValidationData storage _validationData =
getAccountStorage().validationData[validationConfig.moduleEntity()];
ModuleEntity moduleEntity = validationConfig.moduleEntity();

for (uint256 i = 0; i < hooks.length; ++i) {
HookConfig hookConfig = HookConfig.wrap(bytes26(hooks[i][:26]));
Expand All @@ -253,7 +262,7 @@ abstract contract ModuleManagerInternals is IModuleManager {
}
} // Hook is an execution hook
else if (!_validationData.permissionHooks.add(toSetValue(hookConfig))) {
revert PermissionAlreadySet(validationConfig.moduleEntity(), hookConfig);
revert PermissionAlreadySet(moduleEntity, hookConfig);
}

_onInstall(hookConfig.module(), hookData);
Expand All @@ -262,14 +271,15 @@ abstract contract ModuleManagerInternals is IModuleManager {
for (uint256 i = 0; i < selectors.length; ++i) {
bytes4 selector = selectors[i];
if (!_validationData.selectors.add(toSetValue(selector))) {
revert ValidationAlreadySet(selector, validationConfig.moduleEntity());
revert ValidationAlreadySet(selector, moduleEntity);
}
}

_validationData.isGlobal = validationConfig.isGlobal();
_validationData.isSignatureValidation = validationConfig.isSignatureValidation();

_onInstall(validationConfig.module(), installData);
emit ValidationInstalled(moduleEntity);
}

function _uninstallValidation(
Expand All @@ -278,6 +288,7 @@ abstract contract ModuleManagerInternals is IModuleManager {
bytes[] calldata hookUninstallDatas
) internal {
ValidationData storage _validationData = getAccountStorage().validationData[validationFunction];
bool onUninstallSuccess = true;

_removeValidationFunction(validationFunction);

Expand All @@ -296,15 +307,15 @@ abstract contract ModuleManagerInternals is IModuleManager {
for (uint256 i = 0; i < _validationData.preValidationHooks.length; ++i) {
bytes calldata hookData = hookUninstallDatas[hookIndex];
(address hookModule,) = ModuleEntityLib.unpack(_validationData.preValidationHooks[i]);
_onUninstall(hookModule, hookData);
onUninstallSuccess = onUninstallSuccess && _onUninstall(hookModule, hookData);
hookIndex++;
}

for (uint256 i = 0; i < _validationData.permissionHooks.length(); ++i) {
bytes calldata hookData = hookUninstallDatas[hookIndex];
(address hookModule,) =
ModuleEntityLib.unpack(toModuleEntity(_validationData.permissionHooks.at(i)));
_onUninstall(hookModule, hookData);
onUninstallSuccess = onUninstallSuccess && _onUninstall(hookModule, hookData);
hookIndex++;
}
}
Expand All @@ -327,6 +338,8 @@ abstract contract ModuleManagerInternals is IModuleManager {
}

(address module,) = ModuleEntityLib.unpack(validationFunction);
_onUninstall(module, uninstallData);
onUninstallSuccess = onUninstallSuccess && _onUninstall(module, uninstallData);

emit ValidationUninstalled(validationFunction, onUninstallSuccess);
}
}
8 changes: 4 additions & 4 deletions src/account/UpgradeableModularAccount.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ contract UpgradeableModularAccount is
/// @dev We route calls to execution functions based on incoming msg.sig
/// @dev If there's no module associated with this function selector, revert
fallback(bytes calldata) external payable returns (bytes memory) {
address execModule = getAccountStorage().selectorData[msg.sig].module;
address execModule = getAccountStorage().executionData[msg.sig].module;
if (execModule == address(0)) {
revert UnrecognizedFunction(msg.sig);
}
Expand Down Expand Up @@ -604,7 +604,7 @@ contract UpgradeableModularAccount is
// and the selector isn't public.
if (
msg.sender != address(_ENTRY_POINT) && msg.sender != address(this)
&& !_storage.selectorData[msg.sig].isPublic
&& !_storage.executionData[msg.sig].isPublic
) {
ModuleEntity directCallValidationKey =
ModuleEntityLib.pack(msg.sender, DIRECT_CALL_VALIDATION_ENTITYID);
Expand All @@ -629,7 +629,7 @@ contract UpgradeableModularAccount is

// Exec hooks
PostExecToRun[] memory postExecutionHooks =
_doPreHooks(_storage.selectorData[msg.sig].executionHooks, msg.data);
_doPreHooks(_storage.executionData[msg.sig].executionHooks, msg.data);

return (postPermissionHooks, postExecutionHooks);
}
Expand Down Expand Up @@ -697,7 +697,7 @@ contract UpgradeableModularAccount is
return true;
}

return getAccountStorage().selectorData[selector].allowGlobalValidation;
return getAccountStorage().executionData[selector].allowGlobalValidation;
}

function _checkIfValidationAppliesSelector(bytes4 selector, ModuleEntity validationFunction, bool isGlobal)
Expand Down
5 changes: 2 additions & 3 deletions src/helpers/KnownSelectors.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,8 @@ library KnownSelectors {
|| selector == IStandardExecutor.execute.selector || selector == IStandardExecutor.executeBatch.selector
|| selector == IStandardExecutor.executeWithAuthorization.selector
// check against IAccountLoupe methods
|| selector == IAccountLoupe.getExecutionFunctionHandler.selector
|| selector == IAccountLoupe.getSelectors.selector || selector == IAccountLoupe.getExecutionHooks.selector
|| selector == IAccountLoupe.getPreValidationHooks.selector;
|| selector == IAccountLoupe.getExecutionData.selector
|| selector == IAccountLoupe.getValidationData.selector;
}

function isErc4337Function(bytes4 selector) internal pure returns (bool) {
Expand Down
Loading

0 comments on commit db1e77a

Please sign in to comment.