Skip to content

Commit

Permalink
feat: permission hooks installation via installValidation
Browse files Browse the repository at this point in the history
  • Loading branch information
howydev committed Jun 17, 2024
1 parent 2c89d7c commit de53ae8
Show file tree
Hide file tree
Showing 5 changed files with 107 additions and 24 deletions.
68 changes: 55 additions & 13 deletions src/account/PluginManager2.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {IPlugin} from "../interfaces/IPlugin.sol";
import {FunctionReference} from "../interfaces/IPluginManager.sol";
import {FunctionReferenceLib} from "../helpers/FunctionReferenceLib.sol";
import {AccountStorage, getAccountStorage, toSetValue, toFunctionReference} from "./AccountStorage.sol";
import {ExecutionHook} from "../interfaces/IAccountLoupe.sol";

// Temporary additional functions for a user-controlled install flow for validation functions.
abstract contract PluginManager2 {
Expand All @@ -16,13 +17,15 @@ abstract contract PluginManager2 {
error ValidationAlreadySet(bytes4 selector, FunctionReference validationFunction);
error PreValidationAlreadySet(FunctionReference validationFunction, FunctionReference preValidationFunction);
error ValidationNotSet(bytes4 selector, FunctionReference validationFunction);
error PermissionAlreadySet(FunctionReference validationFunction, ExecutionHook hook);

function _installValidation(
FunctionReference validationFunction,
bool isDefault,
bytes4[] memory selectors,
bytes calldata installData,
bytes memory preValidationHooks
bytes memory preValidationHooks,
bytes memory permissionHooks
)
// TODO: flag for signature validation
internal
Expand Down Expand Up @@ -51,6 +54,26 @@ abstract contract PluginManager2 {
}
}

if (permissionHooks.length > 0) {
(ExecutionHook[] memory permissionFunctions, bytes[] memory initDatas) =
abi.decode(permissionHooks, (ExecutionHook[], bytes[]));

for (uint256 i = 0; i < permissionFunctions.length; ++i) {
ExecutionHook memory permissionFunction = permissionFunctions[i];

if (
!_storage.validationData[validationFunction].permissionHooks.add(toSetValue(permissionFunction))
) {
revert PermissionAlreadySet(validationFunction, permissionFunction);
}

if (initDatas[i].length > 0) {
(address executionPlugin,) = FunctionReferenceLib.unpack(permissionFunction.hookFunction);
IPlugin(executionPlugin).onInstall(initDatas[i]);
}
}
}

if (isDefault) {
if (_storage.validationData[validationFunction].isDefault) {
revert DefaultValidationAlreadySet(validationFunction);
Expand All @@ -75,24 +98,43 @@ abstract contract PluginManager2 {
FunctionReference validationFunction,
bytes4[] calldata selectors,
bytes calldata uninstallData,
bytes calldata preValidationHookUninstallData
bytes calldata preValidationHookUninstallData,
bytes calldata permissionHookUninstallData
) internal {
AccountStorage storage _storage = getAccountStorage();

_storage.validationData[validationFunction].isDefault = false;
_storage.validationData[validationFunction].isSignatureValidation = false;

bytes[] memory preValidationHookUninstallDatas = abi.decode(preValidationHookUninstallData, (bytes[]));

// Clear pre validation hooks
EnumerableSet.Bytes32Set storage preValidationHooks =
_storage.validationData[validationFunction].preValidationHooks;
while (preValidationHooks.length() > 0) {
FunctionReference preValidationFunction = toFunctionReference(preValidationHooks.at(0));
preValidationHooks.remove(toSetValue(preValidationFunction));
(address preValidationPlugin,) = FunctionReferenceLib.unpack(preValidationFunction);
if (preValidationHookUninstallDatas[0].length > 0) {
IPlugin(preValidationPlugin).onUninstall(preValidationHookUninstallDatas[0]);
{
bytes[] memory preValidationHookUninstallDatas = abi.decode(preValidationHookUninstallData, (bytes[]));

// Clear pre validation hooks
EnumerableSet.Bytes32Set storage preValidationHooks =
_storage.validationData[validationFunction].preValidationHooks;
while (preValidationHooks.length() > 0) {
FunctionReference preValidationFunction = toFunctionReference(preValidationHooks.at(0));
preValidationHooks.remove(toSetValue(preValidationFunction));
(address preValidationPlugin,) = FunctionReferenceLib.unpack(preValidationFunction);
if (preValidationHookUninstallDatas[0].length > 0) {
IPlugin(preValidationPlugin).onUninstall(preValidationHookUninstallDatas[0]);
}
}
}

{
bytes[] memory permissionHookUninstallDatas = abi.decode(permissionHookUninstallData, (bytes[]));

// Clear permission hooks
EnumerableSet.Bytes32Set storage permissionHooks =
_storage.validationData[validationFunction].permissionHooks;
while (permissionHooks.length() > 0) {
FunctionReference permissionHook = toFunctionReference(permissionHooks.at(0));
permissionHooks.remove(toSetValue(permissionHook));
(address permissionHookPlugin,) = FunctionReferenceLib.unpack(permissionHook);
if (permissionHookUninstallDatas[0].length > 0) {
IPlugin(permissionHookPlugin).onUninstall(permissionHookUninstallDatas[0]);
}
}
}

Expand Down
31 changes: 25 additions & 6 deletions src/account/UpgradeableModularAccount.sol
Original file line number Diff line number Diff line change
Expand Up @@ -291,8 +291,11 @@ contract UpgradeableModularAccount is

_doRuntimeValidation(runtimeValidationFunction, data, authorization[22:]);

// If runtime validation passes, execute the call
// If runtime validation passes, do runtime permission checks
PostExecToRun[] memory postPermissionHooks =
_doPreHooks(getAccountStorage().validationData[runtimeValidationFunction].permissionHooks, data, false);

// Execute the call
(bool success, bytes memory returnData) = address(this).call(data);

if (!success) {
Expand All @@ -301,6 +304,8 @@ contract UpgradeableModularAccount is
}
}

_doCachedPostExecHooks(postPermissionHooks);

return returnData;
}

Expand Down Expand Up @@ -342,7 +347,7 @@ contract UpgradeableModularAccount is
external
initializer
{
_installValidation(validationFunction, true, new bytes4[](0), installData, bytes(""));
_installValidation(validationFunction, true, new bytes4[](0), installData, bytes(""), bytes(""));
emit ModularAccountInitialized(_ENTRY_POINT);
}

Expand All @@ -353,9 +358,12 @@ contract UpgradeableModularAccount is
bool isDefault,
bytes4[] memory selectors,
bytes calldata installData,
bytes calldata preValidationHooks
bytes calldata preValidationHooks,
bytes calldata permissionHooks
) external wrapNativeFunction {
_installValidation(validationFunction, isDefault, selectors, installData, preValidationHooks);
_installValidation(
validationFunction, isDefault, selectors, installData, preValidationHooks, permissionHooks
);
}

/// @inheritdoc IPluginManager
Expand All @@ -364,9 +372,16 @@ contract UpgradeableModularAccount is
FunctionReference validationFunction,
bytes4[] calldata selectors,
bytes calldata uninstallData,
bytes calldata preValidationHookUninstallData
bytes calldata preValidationHookUninstallData,
bytes calldata permissionHookUninstallData
) external wrapNativeFunction {
_uninstallValidation(validationFunction, selectors, uninstallData, preValidationHookUninstallData);
_uninstallValidation(
validationFunction,
selectors,
uninstallData,
preValidationHookUninstallData,
permissionHookUninstallData
);
}

/// @notice ERC165 introspection
Expand Down Expand Up @@ -434,6 +449,9 @@ contract UpgradeableModularAccount is
revert UnrecognizedFunction(bytes4(userOp.callData));
}
bytes4 selector = bytes4(userOp.callData);
if (selector == this.executeUserOp.selector) {
selector = bytes4(userOp.callData[4:8]);
}

// Revert if the provided `authorization` less than 21 bytes long, rather than right-padding.
FunctionReference userOpValidationFunction = FunctionReference.wrap(bytes21(userOp.signature[:21]));
Expand Down Expand Up @@ -604,6 +622,7 @@ contract UpgradeableModularAccount is
try IExecutionHook(plugin).preExecutionHook(functionId, data) returns (bytes memory returnData) {
preExecHookReturnData = returnData;
} catch (bytes memory revertReason) {
// TODO: same issue with EP0.6 - we can't do bytes4 error codes in plugins
revert PreExecHookReverted(plugin, functionId, revertReason);
}
}
Expand Down
11 changes: 9 additions & 2 deletions src/interfaces/IPluginManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,15 @@ interface IPluginManager {
/// @param isDefault Whether the validation function applies for all selectors in the default pool.
/// @param selectors The selectors to install the validation function for.
/// @param installData Optional data to be decoded and used by the plugin to setup initial plugin state.
/// @param preValidationHooks Optional pre-validation hooks to install for the validation function.
/// @param permissionHooks Optional permission hooks to install for the validation function.
function installValidation(
FunctionReference validationFunction,
bool isDefault,
bytes4[] memory selectors,
bytes calldata installData,
bytes calldata preValidationHooks
bytes calldata preValidationHooks,
bytes calldata permissionHooks
) external;

/// @notice Uninstall a validation function from a set of execution selectors.
Expand All @@ -46,11 +49,15 @@ interface IPluginManager {
/// @param selectors The selectors to uninstall the validation function for.
/// @param uninstallData Optional data to be decoded and used by the plugin to clear plugin data for the
/// account.
/// @param preValidationHookUninstallData Optional data to be decoded and used by the plugin to clear account
/// data
/// @param permissionHookUninstallData Optional data to be decoded and used by the plugin to clear account data
function uninstallValidation(
FunctionReference validationFunction,
bytes4[] calldata selectors,
bytes calldata uninstallData,
bytes calldata preValidationHookUninstallData
bytes calldata preValidationHookUninstallData,
bytes calldata permissionHookUninstallData
) external;

/// @notice Uninstall a plugin from the modular account.
Expand Down
7 changes: 6 additions & 1 deletion test/account/AccountLoupe.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,12 @@ contract AccountLoupeTest is AccountTestBase {
bytes[] memory installDatas = new bytes[](2);
vm.prank(address(entryPoint));
account1.installValidation(
ownerValidation, true, new bytes4[](0), bytes(""), abi.encode(preValidationHooks, installDatas)
ownerValidation,
true,
new bytes4[](0),
bytes(""),
abi.encode(preValidationHooks, installDatas),
bytes("")
);
}

Expand Down
14 changes: 12 additions & 2 deletions test/account/ValidationIntersection.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,12 @@ contract ValidationIntersectionTest is AccountTestBase {
});
bytes[] memory installDatas = new bytes[](1);
account1.installValidation(
oneHookValidation, true, new bytes4[](0), bytes(""), abi.encode(preValidationHooks, installDatas)
oneHookValidation,
true,
new bytes4[](0),
bytes(""),
abi.encode(preValidationHooks, installDatas),
bytes("")
);
account1.installPlugin({
plugin: address(twoHookPlugin),
Expand All @@ -87,7 +92,12 @@ contract ValidationIntersectionTest is AccountTestBase {
});
installDatas = new bytes[](2);
account1.installValidation(
twoHookValidation, true, new bytes4[](0), bytes(""), abi.encode(preValidationHooks, installDatas)
twoHookValidation,
true,
new bytes4[](0),
bytes(""),
abi.encode(preValidationHooks, installDatas),
bytes("")
);
vm.stopPrank();
}
Expand Down

0 comments on commit de53ae8

Please sign in to comment.