Skip to content

Commit

Permalink
feat: remove permitted call hooks from MSCA (rebase on cache PR) (#76)
Browse files Browse the repository at this point in the history 
Co-authored-by: Jay Paik <[email protected]>
  • Loading branch information
@fangting-alchemy @jaypaik
fangting-alchemy and jaypaik authored Jan 22, 2024
1 parent fead83d commit a37331d
Show file tree
Hide file tree
Showing 37 changed files with 221 additions and 2,360 deletions.
1 change: 0 additions & 1 deletion src/account/AccountExecutor.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ abstract contract AccountExecutor {
/// - Pre Runtime Validation Hook
/// - Runtime Validation
/// - Pre Execution Hook
/// - Pre Permitted Call Hook
/// And if it fails, reverts with the appropriate custom error.
function _executeRuntimePluginFunction(bytes memory buffer, address plugin, bytes4 errorSelector) internal {
if (!_executeRaw(plugin, buffer)) {
Expand Down
16 changes: 2 additions & 14 deletions src/account/AccountLoupe.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,18 +41,6 @@ abstract contract AccountLoupe is IAccountLoupe, AccountStorageV1 {
execHooks = _getHooks(_getAccountStorage().selectorData[selector].executionHooks);
}

/// @inheritdoc IAccountLoupe
function getPermittedCallHooks(address callingPlugin, bytes4 selector)
external
view
returns (ExecutionHooks[] memory execHooks)
{
PermittedCallData storage permittedCallData =
_getAccountStorage().permittedCalls[_getPermittedCallKey(callingPlugin, selector)];

execHooks = _getHooks(permittedCallData.permittedCallHooks);
}

/// @inheritdoc IAccountLoupe
function getPreValidationHooks(bytes4 selector)
external
Expand All @@ -73,8 +61,8 @@ abstract contract AccountLoupe is IAccountLoupe, AccountStorageV1 {
pluginAddresses = CastLib.toAddressArray(_getAccountStorage().plugins.getAll());
}

/// @dev Collects hook data from stored hooks (either execution hooks or permitted call hooks) and prepares it
/// for returning as the `ExecutionHooks` type defined by `IAccountLoupe`.
/// @dev Collects hook data from stored execution hooks and prepares it for returning as the `ExecutionHooks`
/// type defined by `IAccountLoupe`.
function _getHooks(HookGroup storage storedHooks) internal view returns (ExecutionHooks[] memory execHooks) {
FunctionReference[] memory preExecHooks = CastLib.toFunctionReferenceArray(storedHooks.preHooks.getAll());
FunctionReference[] memory postOnlyExecHooks =
Expand Down
211 changes: 7 additions & 204 deletions src/account/PluginManagerInternals.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,7 @@ abstract contract PluginManagerInternals is IPluginManager, AccountStorageV1 {
error NativeFunctionNotAllowed(bytes4 selector);
error NullFunctionReference();
error PluginAlreadyInstalled(address plugin);
error PluginApplyHookCallbackFailed(address providingPlugin, bytes revertReason);
error PluginDependencyViolation(address plugin);
error PluginHookUnapplyCallbackFailed(address providingPlugin, bytes revertReason);
error PluginInstallCallbackFailed(address plugin, bytes revertReason);
error PluginInterfaceNotSupported(address plugin);
error PluginNotInstalled(address plugin);
Expand Down Expand Up @@ -155,55 +153,6 @@ abstract contract PluginManagerInternals is IPluginManager, AccountStorageV1 {
}
}

function _enableExecFromPlugin(bytes4 selector, address plugin, AccountStorage storage storage_) internal {
PermittedCallData storage permittedCallData =
storage_.permittedCalls[_getPermittedCallKey(plugin, selector)];

// If there are duplicates, this will just enable the flag again. This is not a problem, since the boolean
// will be set to false twice during uninstall, which is fine.
permittedCallData.callPermitted = true;
}

function _addPermittedCallHooks(
bytes4 selector,
address plugin,
FunctionReference preExecHook,
FunctionReference postExecHook
) internal {
PermittedCallData storage permittedCallData =
_getAccountStorage().permittedCalls[_getPermittedCallKey(plugin, selector)];

_addHooks(permittedCallData.permittedCallHooks, selector, preExecHook, postExecHook);

if (preExecHook != FunctionReferenceLib._EMPTY_FUNCTION_REFERENCE) {
permittedCallData.hasPrePermittedCallHooks = true;
} else if (postExecHook != FunctionReferenceLib._EMPTY_FUNCTION_REFERENCE) {
// Only set this flag if the pre hook is empty and the post hook is non-empty.
permittedCallData.hasPostOnlyPermittedCallHooks = true;
}
}

function _removePermittedCallHooks(
bytes4 selector,
address plugin,
FunctionReference preExecHook,
FunctionReference postExecHook
) internal {
PermittedCallData storage permittedCallData =
_getAccountStorage().permittedCalls[_getPermittedCallKey(plugin, selector)];

(bool shouldClearHasPreHooks, bool shouldClearHasPostOnlyHooks) =
_removeHooks(permittedCallData.permittedCallHooks, preExecHook, postExecHook);

if (shouldClearHasPreHooks) {
permittedCallData.hasPrePermittedCallHooks = false;
}

if (shouldClearHasPostOnlyHooks) {
permittedCallData.hasPostOnlyPermittedCallHooks = false;
}
}

function _addHooks(
HookGroup storage hooks,
bytes4 selector,
Expand Down Expand Up @@ -326,8 +275,7 @@ abstract contract PluginManagerInternals is IPluginManager, AccountStorageV1 {
address plugin,
bytes32 manifestHash,
bytes memory pluginInitData,
FunctionReference[] memory dependencies,
InjectedHook[] memory injectedHooks
FunctionReference[] memory dependencies
) internal {
AccountStorage storage storage_ = _getAccountStorage();

Expand Down Expand Up @@ -382,7 +330,7 @@ abstract contract PluginManagerInternals is IPluginManager, AccountStorageV1 {
// Add installed plugin and selectors this plugin can call
length = manifest.permittedExecutionSelectors.length;
for (uint256 i = 0; i < length; ++i) {
_enableExecFromPlugin(manifest.permittedExecutionSelectors[i], plugin, storage_);
storage_.callPermitted[_getPermittedCallKey(plugin, manifest.permittedExecutionSelectors[i])] = true;
}

// Add the permitted external calls to the account.
Expand Down Expand Up @@ -410,42 +358,6 @@ abstract contract PluginManagerInternals is IPluginManager, AccountStorageV1 {
}
}

// Add injected hooks
length = injectedHooks.length;
// Manually set injected hooks array length
StoredInjectedHook[] storage injectedHooksArray = storage_.pluginData[plugin].injectedHooks;
assembly ("memory-safe") {
sstore(injectedHooksArray.slot, length)
}
for (uint256 i = 0; i < length; ++i) {
InjectedHook memory hook = injectedHooks[i];

// Check that the dependency is installed. This also blocks self-dependencies.
if (storage_.pluginData[hook.providingPlugin].manifestHash == bytes32(0)) {
revert MissingPluginDependency(hook.providingPlugin);
}

injectedHooksArray[i] = StoredInjectedHook({
providingPlugin: hook.providingPlugin,
selector: hook.selector,
preExecHookFunctionId: hook.injectedHooksInfo.preExecHookFunctionId,
isPostHookUsed: hook.injectedHooksInfo.isPostHookUsed,
postExecHookFunctionId: hook.injectedHooksInfo.postExecHookFunctionId
});

// Increment the dependent count for the plugin providing the hook.
storage_.pluginData[hook.providingPlugin].dependentCount += 1;

_addPermittedCallHooks(
hook.selector,
plugin,
FunctionReferenceLib.pack(hook.providingPlugin, hook.injectedHooksInfo.preExecHookFunctionId),
hook.injectedHooksInfo.isPostHookUsed
? FunctionReferenceLib.pack(hook.providingPlugin, hook.injectedHooksInfo.postExecHookFunctionId)
: FunctionReferenceLib._EMPTY_FUNCTION_REFERENCE
);
}

// Add user operation validation functions
length = manifest.userOpValidationFunctions.length;
for (uint256 i = 0; i < length; ++i) {
Expand Down Expand Up @@ -518,27 +430,6 @@ abstract contract PluginManagerInternals is IPluginManager, AccountStorageV1 {
);
}

// Add pre and post permitted call hooks
length = manifest.permittedCallHooks.length;
for (uint256 i = 0; i < length; ++i) {
_addPermittedCallHooks(
manifest.permittedCallHooks[i].executionSelector,
plugin,
_resolveManifestFunction(
manifest.permittedCallHooks[i].preExecHook,
plugin,
dependencies,
ManifestAssociatedFunctionType.PRE_HOOK_ALWAYS_DENY
),
_resolveManifestFunction(
manifest.permittedCallHooks[i].postExecHook,
plugin,
dependencies,
ManifestAssociatedFunctionType.NONE
)
);
}

// Add new interface ids the plugin enabled for the account
length = manifest.interfaceIds.length;
for (uint256 i = 0; i < length; ++i) {
Expand All @@ -558,38 +449,17 @@ abstract contract PluginManagerInternals is IPluginManager, AccountStorageV1 {
storage_.pluginData[plugin].canSpendNativeToken = true;
}

// Call injected hooks' onHookApply after all setup, this is before calling plugin onInstall
length = injectedHooks.length;
for (uint256 i = 0; i < length; ++i) {
InjectedHook memory hook = injectedHooks[i];

/* solhint-disable no-empty-blocks */
try IPlugin(hook.providingPlugin).onHookApply(
plugin, hook.injectedHooksInfo, injectedHooks[i].hookApplyData
) {} catch (bytes memory revertReason) {
revert PluginApplyHookCallbackFailed(hook.providingPlugin, revertReason);
}
/* solhint-enable no-empty-blocks */

// zero out hookApplyData to reduce log cost
injectedHooks[i].hookApplyData = new bytes(0);
}

// Initialize the plugin storage for the account.
// solhint-disable-next-line no-empty-blocks
try IPlugin(plugin).onInstall(pluginInitData) {}
catch (bytes memory revertReason) {
revert PluginInstallCallbackFailed(plugin, revertReason);
}

emit PluginInstalled(plugin, manifestHash, dependencies, injectedHooks);
emit PluginInstalled(plugin, manifestHash, dependencies);
}

function _uninstallPlugin(
UninstallPluginArgs memory args,
bytes calldata uninstallData,
bytes[] calldata hookUnapplyData
) internal {
function _uninstallPlugin(UninstallPluginArgs memory args, bytes calldata uninstallData) internal {
AccountStorage storage storage_ = _getAccountStorage();

// Check if the plugin exists.
Expand Down Expand Up @@ -625,27 +495,6 @@ abstract contract PluginManagerInternals is IPluginManager, AccountStorageV1 {

// Remove components according to the manifest, in reverse order (by component type) of their installation.

// Remove pre and post permitted call hooks
length = args.manifest.permittedCallHooks.length;
for (uint256 i = 0; i < length; ++i) {
_removePermittedCallHooks(
args.manifest.permittedCallHooks[i].executionSelector,
args.plugin,
_resolveManifestFunction(
args.manifest.permittedCallHooks[i].preExecHook,
args.plugin,
dependencies,
ManifestAssociatedFunctionType.PRE_HOOK_ALWAYS_DENY
),
_resolveManifestFunction(
args.manifest.permittedCallHooks[i].postExecHook,
args.plugin,
dependencies,
ManifestAssociatedFunctionType.NONE
)
);
}

// Remove pre and post execution function hooks
length = args.manifest.executionHooks.length;
for (uint256 i = 0; i < length; ++i) {
Expand Down Expand Up @@ -735,29 +584,11 @@ abstract contract PluginManagerInternals is IPluginManager, AccountStorageV1 {
}
}

// Remove injected hooks
length = pluginData.injectedHooks.length;
for (uint256 i = 0; i < length; ++i) {
StoredInjectedHook memory hook = pluginData.injectedHooks[i];

// Decrement the dependent count for the plugin providing the hook.
storage_.pluginData[hook.providingPlugin].dependentCount -= 1;

_removePermittedCallHooks(
hook.selector,
args.plugin,
FunctionReferenceLib.pack(hook.providingPlugin, hook.preExecHookFunctionId),
hook.isPostHookUsed
? FunctionReferenceLib.pack(hook.providingPlugin, hook.postExecHookFunctionId)
: FunctionReferenceLib._EMPTY_FUNCTION_REFERENCE
);
}

// Remove permitted account execution function call permissions
length = args.manifest.permittedExecutionSelectors.length;
for (uint256 i = 0; i < length; ++i) {
storage_.permittedCalls[_getPermittedCallKey(args.plugin, args.manifest.permittedExecutionSelectors[i])]
.callPermitted = false;
storage_.callPermitted[_getPermittedCallKey(args.plugin, args.manifest.permittedExecutionSelectors[i])]
= false;
}

// Remove installed execution function
Expand All @@ -772,36 +603,8 @@ abstract contract PluginManagerInternals is IPluginManager, AccountStorageV1 {
storage_.supportedInterfaces[args.manifest.interfaceIds[i]] -= 1;
}

// Call onHookUnapply on all injected hooks
bool callbacksSucceeded = true;
length = pluginData.injectedHooks.length;
bool hasUnapplyHookData = hookUnapplyData.length != 0;
if (hasUnapplyHookData && hookUnapplyData.length != length) {
revert ArrayLengthMismatch();
}
for (uint256 i = 0; i < length; ++i) {
StoredInjectedHook memory hook = pluginData.injectedHooks[i];

/* solhint-disable no-empty-blocks */
try IPlugin(hook.providingPlugin).onHookUnapply{gas: args.callbackGasLimit}(
args.plugin,
InjectedHooksInfo({
preExecHookFunctionId: hook.preExecHookFunctionId,
isPostHookUsed: hook.isPostHookUsed,
postExecHookFunctionId: hook.postExecHookFunctionId
}),
hasUnapplyHookData ? hookUnapplyData[i] : bytes("")
) {} catch (bytes memory revertReason) {
if (!args.forceUninstall) {
revert PluginHookUnapplyCallbackFailed(hook.providingPlugin, revertReason);
}
callbacksSucceeded = false;
emit PluginIgnoredHookUnapplyCallbackFailure(args.plugin, hook.providingPlugin);
}
/* solhint-enable no-empty-blocks */
}

// Clear the plugin storage for the account.
bool callbacksSucceeded = true;
// solhint-disable-next-line no-empty-blocks
try IPlugin(args.plugin).onUninstall{gas: args.callbackGasLimit}(uninstallData) {}
catch (bytes memory revertReason) {
Expand Down
Loading

0 comments on commit a37331d

Please sign in to comment.