Skip to content

Commit

Permalink
feat: [v0.8-develop] associate pre-validation hooks with validation f…
Browse files Browse the repository at this point in the history 
…unctions (#64)
  • Loading branch information
@adamegyed
adamegyed committed Jul 26, 2024
1 parent 45b1045 commit 462285b
Show file tree
Hide file tree
Showing 16 changed files with 159 additions and 432 deletions.
4 changes: 2 additions & 2 deletions src/account/AccountLoupe.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,14 @@ abstract contract AccountLoupe is IAccountLoupe {
}

/// @inheritdoc IAccountLoupe
function getPreValidationHooks(bytes4 selector)
function getPreValidationHooks(FunctionReference validationFunction)
external
view
override
returns (FunctionReference[] memory preValidationHooks)
{
preValidationHooks =
toFunctionReferenceArray(getAccountStorage().selectorData[selector].preValidationHooks);
toFunctionReferenceArray(getAccountStorage().validationData[validationFunction].preValidationHooks);
}

/// @inheritdoc IAccountLoupe
Expand Down
27 changes: 11 additions & 16 deletions src/account/AccountStorage.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,17 +41,19 @@ struct SelectorData {
bool isPublic;
// Whether or not a default validation function may be used to validate this function.
bool allowDefaultValidation;
// How many times a `PRE_HOOK_ALWAYS_DENY` has been added for this function.
// Since that is the only type of hook that may overlap, we can use this to track the number of times it has
// been applied, and whether or not the deny should apply. The size `uint48` was chosen somewhat arbitrarily,
// but it packs alongside `plugin` while still leaving some other space in the slot for future packing.
uint48 denyExecutionCount;
// User operation validation and runtime validation share a function reference.
// The execution hooks for this function selector.
EnumerableSet.Bytes32Set executionHooks;
// Which validation functions are associated with this function selector.
EnumerableSet.Bytes32Set validations;
}

struct ValidationData {
// Whether or not this validation can be used as a default validation function.
bool isDefault;
// Whether or not this validation is a signature validator.
bool isSignatureValidation;
// The pre validation hooks for this function selector.
EnumerableSet.Bytes32Set preValidationHooks;
// The execution hooks for this function selector.
EnumerableSet.Bytes32Set executionHooks;
}

struct AccountStorage {
Expand All @@ -63,21 +65,14 @@ struct AccountStorage {
mapping(address => PluginData) pluginData;
// Execution functions and their associated functions
mapping(bytes4 => SelectorData) selectorData;
mapping(FunctionReference validationFunction => ValidationData) validationData;
mapping(address caller => mapping(bytes4 selector => bool)) callPermitted;
// key = address(calling plugin) || target address
mapping(IPlugin => mapping(address => PermittedExternalCallData)) permittedExternalCalls;
// For ERC165 introspection
mapping(bytes4 => uint256) supportedIfaces;
// Installed plugins capable of signature validation.
EnumerableSet.Bytes32Set signatureValidations;
// Todo: merge this with other validation storage?
EnumerableSet.Bytes32Set defaultValidations;
}

// TODO: Change how pre-validation hooks work to allow association with validation, rather than selector.
// Pre signature validation hooks
// mapping(FunctionReference => EnumerableSet.Bytes32Set) preSignatureValidationHooks;

function getAccountStorage() pure returns (AccountStorage storage _storage) {
assembly ("memory-safe") {
_storage.slot := _ACCOUNT_STORAGE_SLOT
Expand Down
92 changes: 69 additions & 23 deletions src/account/PluginManager2.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,63 +6,109 @@ import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet
import {IPlugin} from "../interfaces/IPlugin.sol";
import {FunctionReference} from "../interfaces/IPluginManager.sol";
import {FunctionReferenceLib} from "../helpers/FunctionReferenceLib.sol";
import {AccountStorage, getAccountStorage, toSetValue} from "./AccountStorage.sol";
import {AccountStorage, getAccountStorage, toSetValue, toFunctionReference} from "./AccountStorage.sol";

// Temporary additional functions for a user-controlled install flow for validation functions.
abstract contract PluginManager2 {
using EnumerableSet for EnumerableSet.Bytes32Set;

error DefaultValidationAlreadySet(address plugin, uint8 functionId);
error ValidationAlreadySet(bytes4 selector, address plugin, uint8 functionId);
error ValidationNotSet(bytes4 selector, address plugin, uint8 functionId);
error DefaultValidationAlreadySet(FunctionReference validationFunction);
error PreValidationAlreadySet(FunctionReference validationFunction, FunctionReference preValidationFunction);
error ValidationAlreadySet(bytes4 selector, FunctionReference validationFunction);
error ValidationNotSet(bytes4 selector, FunctionReference validationFunction);

function _installValidation(
address plugin,
uint8 functionId,
FunctionReference validationFunction,
bool isDefault,
bytes4[] memory selectors,
bytes calldata installData
) internal {
FunctionReference validationFunction = FunctionReferenceLib.pack(plugin, functionId);

bytes calldata installData,
bytes memory preValidationHooks
)
// TODO: flag for signature validation
internal
{
AccountStorage storage _storage = getAccountStorage();

if (preValidationHooks.length > 0) {
(FunctionReference[] memory preValidationFunctions, bytes[] memory initDatas) =
abi.decode(preValidationHooks, (FunctionReference[], bytes[]));

for (uint256 i = 0; i < preValidationFunctions.length; ++i) {
FunctionReference preValidationFunction = preValidationFunctions[i];

if (
!_storage.validationData[validationFunction].preValidationHooks.add(
toSetValue(preValidationFunction)
)
) {
revert PreValidationAlreadySet(validationFunction, preValidationFunction);
}

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

if (isDefault) {
if (!_storage.defaultValidations.add(toSetValue(validationFunction))) {
revert DefaultValidationAlreadySet(plugin, functionId);
if (_storage.validationData[validationFunction].isDefault) {
revert DefaultValidationAlreadySet(validationFunction);
}
_storage.validationData[validationFunction].isDefault = true;
}

for (uint256 i = 0; i < selectors.length; ++i) {
bytes4 selector = selectors[i];
if (!_storage.selectorData[selector].validations.add(toSetValue(validationFunction))) {
revert ValidationAlreadySet(selector, plugin, functionId);
revert ValidationAlreadySet(selector, validationFunction);
}
}

IPlugin(plugin).onInstall(installData);
if (installData.length > 0) {
(address plugin,) = FunctionReferenceLib.unpack(validationFunction);
IPlugin(plugin).onInstall(installData);
}
}

function _uninstallValidation(
address plugin,
uint8 functionId,
FunctionReference validationFunction,
bytes4[] calldata selectors,
bytes calldata uninstallData
bytes calldata uninstallData,
bytes calldata preValidationHookUninstallData
) internal {
FunctionReference validationFunction = FunctionReferenceLib.pack(plugin, functionId);

AccountStorage storage _storage = getAccountStorage();

// Ignore return value - remove if present, do nothing otherwise.
_storage.defaultValidations.remove(toSetValue(validationFunction));
_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]);
}
}

// Because this function also calls `onUninstall`, and removes the default flag from validation, we must
// assume these selectors passed in to be exhaustive.
// TODO: consider enforcing this from user-supplied install config.
for (uint256 i = 0; i < selectors.length; ++i) {
bytes4 selector = selectors[i];
if (!_storage.selectorData[selector].validations.remove(toSetValue(validationFunction))) {
revert ValidationNotSet(selector, plugin, functionId);
revert ValidationNotSet(selector, validationFunction);
}
}

IPlugin(plugin).onUninstall(uninstallData);
if (uninstallData.length > 0) {
(address plugin,) = FunctionReferenceLib.unpack(validationFunction);
IPlugin(plugin).onUninstall(uninstallData);
}
}
}
87 changes: 6 additions & 81 deletions src/account/PluginManagerInternals.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ abstract contract PluginManagerInternals is IPluginManager {
SelectorData storage _selectorData = getAccountStorage().selectorData[selector];

// 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
// non-depdency validation functions. Then, if a either plugin is uninstalled, it would cause a partial
// uninstall of the other.
if (!_selectorData.validations.add(toSetValue(validationFunction))) {
revert ValidationFunctionAlreadySet(selector, validationFunction);
Expand Down Expand Up @@ -157,33 +157,6 @@ abstract contract PluginManagerInternals is IPluginManager {
);
}

function _addPreValidationHook(bytes4 selector, FunctionReference preValidationHook)
internal
notNullFunction(preValidationHook)
{
SelectorData storage _selectorData = getAccountStorage().selectorData[selector];
if (preValidationHook.eq(FunctionReferenceLib._PRE_HOOK_ALWAYS_DENY)) {
// Increment `denyExecutionCount`, because this pre validation hook may be applied multiple times.
_selectorData.denyExecutionCount += 1;
return;
}
_selectorData.preValidationHooks.add(toSetValue(preValidationHook));
}

function _removePreValidationHook(bytes4 selector, FunctionReference preValidationHook)
internal
notNullFunction(preValidationHook)
{
SelectorData storage _selectorData = getAccountStorage().selectorData[selector];
if (preValidationHook.eq(FunctionReferenceLib._PRE_HOOK_ALWAYS_DENY)) {
// Decrement `denyExecutionCount`, because this pre exec hook may be applied multiple times.
_selectorData.denyExecutionCount -= 1;
return;
}
// May ignore return value, as the manifest hash is validated to ensure that the hook exists.
_selectorData.preValidationHooks.remove(toSetValue(preValidationHook));
}

function _installPlugin(
address plugin,
bytes32 manifestHash,
Expand Down Expand Up @@ -288,35 +261,15 @@ abstract contract PluginManagerInternals is IPluginManager {
for (uint256 i = 0; i < length; ++i) {
ManifestAssociatedFunction memory mv = manifest.validationFunctions[i];
_addValidationFunction(
mv.executionSelector,
_resolveManifestFunction(
mv.associatedFunction, plugin, dependencies, ManifestAssociatedFunctionType.NONE
)
mv.executionSelector, _resolveManifestFunction(mv.associatedFunction, plugin, dependencies)
);
}

length = manifest.signatureValidationFunctions.length;
for (uint256 i = 0; i < length; ++i) {
FunctionReference signatureValidationFunction =
FunctionReferenceLib.pack(plugin, manifest.signatureValidationFunctions[i]);
_storage.signatureValidations.add(toSetValue(signatureValidationFunction));
}

// Hooks are not allowed to be provided as dependencies, so we use an empty array for resolving them.
FunctionReference[] memory emptyDependencies;

length = manifest.preValidationHooks.length;
for (uint256 i = 0; i < length; ++i) {
ManifestAssociatedFunction memory mh = manifest.preValidationHooks[i];
_addPreValidationHook(
mh.executionSelector,
_resolveManifestFunction(
mh.associatedFunction,
plugin,
emptyDependencies,
ManifestAssociatedFunctionType.PRE_HOOK_ALWAYS_DENY
)
);
_storage.validationData[signatureValidationFunction].isSignatureValidation = true;
}

length = manifest.executionHooks.length;
Expand Down Expand Up @@ -375,45 +328,25 @@ abstract contract PluginManagerInternals is IPluginManager {

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

// Hooks are not allowed to be provided as dependencies, so we use an empty array for resolving them.
FunctionReference[] memory emptyDependencies;

length = manifest.executionHooks.length;
for (uint256 i = 0; i < length; ++i) {
ManifestExecutionHook memory mh = manifest.executionHooks[i];
FunctionReference hookFunction = FunctionReferenceLib.pack(plugin, mh.functionId);
_removeExecHooks(mh.executionSelector, hookFunction, mh.isPreHook, mh.isPostHook);
}

length = manifest.preValidationHooks.length;
for (uint256 i = 0; i < length; ++i) {
ManifestAssociatedFunction memory mh = manifest.preValidationHooks[i];
_removePreValidationHook(
mh.executionSelector,
_resolveManifestFunction(
mh.associatedFunction,
plugin,
emptyDependencies,
ManifestAssociatedFunctionType.PRE_HOOK_ALWAYS_DENY
)
);
}

length = manifest.signatureValidationFunctions.length;
for (uint256 i = 0; i < length; ++i) {
FunctionReference signatureValidationFunction =
FunctionReferenceLib.pack(plugin, manifest.signatureValidationFunctions[i]);
_storage.signatureValidations.remove(toSetValue(signatureValidationFunction));
_storage.validationData[signatureValidationFunction].isSignatureValidation = false;
}

length = manifest.validationFunctions.length;
for (uint256 i = 0; i < length; ++i) {
ManifestAssociatedFunction memory mv = manifest.validationFunctions[i];
_removeValidationFunction(
mv.executionSelector,
_resolveManifestFunction(
mv.associatedFunction, plugin, dependencies, ManifestAssociatedFunctionType.NONE
)
mv.executionSelector, _resolveManifestFunction(mv.associatedFunction, plugin, dependencies)
);
}

Expand Down Expand Up @@ -486,9 +419,7 @@ abstract contract PluginManagerInternals is IPluginManager {
function _resolveManifestFunction(
ManifestFunction memory manifestFunction,
address plugin,
FunctionReference[] memory dependencies,
// Indicates which magic value, if any, is permissible for the function to resolve.
ManifestAssociatedFunctionType allowedMagicValue
FunctionReference[] memory dependencies
) internal pure returns (FunctionReference) {
if (manifestFunction.functionType == ManifestAssociatedFunctionType.SELF) {
return FunctionReferenceLib.pack(plugin, manifestFunction.functionId);
Expand All @@ -497,12 +428,6 @@ abstract contract PluginManagerInternals is IPluginManager {
revert InvalidPluginManifest();
}
return dependencies[manifestFunction.dependencyIndex];
} else if (manifestFunction.functionType == ManifestAssociatedFunctionType.PRE_HOOK_ALWAYS_DENY) {
if (allowedMagicValue == ManifestAssociatedFunctionType.PRE_HOOK_ALWAYS_DENY) {
return FunctionReferenceLib._PRE_HOOK_ALWAYS_DENY;
} else {
revert InvalidPluginManifest();
}
}
return FunctionReferenceLib._EMPTY_FUNCTION_REFERENCE; // Empty checks are done elsewhere
}
Expand Down
Loading

0 comments on commit 462285b

Please sign in to comment.