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.

Sign up for GitHub

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

Fix: Events and AccountLoupe revamp for data availability #122

Merged
merged 5 commits into from
Aug 1, 2024
Merged

Fix: Events and AccountLoupe revamp for data availability #122

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c8775f1
refactor: rename selectorData to executionData
fangting-alchemy Jul 31, 2024
9a4b74b
fix: revamp account loup for data availability
fangting-alchemy Jul 31, 2024
2fadf6c
chore: clean up ExecutionHook usage
fangting-alchemy Jul 31, 2024
e1a0783
chore: add events emision for validation install and uninstall
fangting-alchemy Jul 31, 2024
a118f31
fix loupe data type and uninstall success value
fangting-alchemy Aug 1, 2024
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
92 changes: 30 additions & 62 deletions src/account/AccountLoupe.sol
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading