Skip to content

[DRAFT] Composable validation where same validation with different installData can be repetitively installed #87

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

Closed
fangting-alchemy wants to merge 4 commits into from
Closed

[DRAFT] Composable validation where same validation with different installData can be repetitively installed #87

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3768d91
Use validationId as validation identifier in account and allow user t…
fangting-alchemy Jun 27, 2024
52318de
comment out validation installation from install plugin path
fangting-alchemy Jun 27, 2024
e504cfb
add missed field and fix validation oninstall data format
fangting-alchemy Jun 27, 2024
6a320ec
add ecdsa validation tests and fix other tests
fangting-alchemy Jun 27, 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
4 changes: 2 additions & 2 deletions src/account/AccountLoupe.sol
View file
Open in desktop
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(FunctionReference validationFunction)
function getPreValidationHooks(bytes32 validationId)
external
view
override
returns (FunctionReference[] memory preValidationHooks)
{
preValidationHooks =
toFunctionReferenceArray(getAccountStorage().validationData[validationFunction].preValidationHooks);
toFunctionReferenceArray(getAccountStorage().validationData[validationId].preValidationHooks);
}

/// @inheritdoc IAccountLoupe
Expand Down
5 changes: 3 additions & 2 deletions src/account/AccountStorage.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,11 @@ struct SelectorData {
}

struct ValidationData {
FunctionReference validationFunction;
// 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;
bool isSignatureValidationAllowed;
// The pre validation hooks for this function selector.
EnumerableSet.Bytes32Set preValidationHooks;
}
Expand All @@ -51,7 +52,7 @@ struct AccountStorage {
mapping(address => PluginData) pluginData;
// Execution functions and their associated functions
mapping(bytes4 => SelectorData) selectorData;
mapping(FunctionReference validationFunction => ValidationData) validationData;
mapping(bytes32 validationId => ValidationData) validationData;
mapping(address caller => mapping(bytes4 selector => bool)) callPermitted;
// For ERC165 introspection
mapping(bytes4 => uint256) supportedIfaces;
Expand Down
68 changes: 44 additions & 24 deletions src/account/PluginManager2.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,28 +6,54 @@ 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, toFunctionReference} from "./AccountStorage.sol";
import {
AccountStorage,
getAccountStorage,
toSetValue,
toFunctionReference,
ValidationData
} from "./AccountStorage.sol";

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

event ValidationUpdated(bytes32 validationId);
event ValidationUninstalled(bytes32 validationId);

error DefaultValidationAlreadySet(FunctionReference validationFunction);
error PreValidationAlreadySet(FunctionReference validationFunction, FunctionReference preValidationFunction);
// TODO to be renamed once PR https://github.com/erc6900/reference-implementation/pull/85/files merged
error ValidationAlreadySetNew(bytes32 validationId);
error ValidationAlreadySet(bytes4 selector, FunctionReference validationFunction);
error ValidationNotSet(bytes4 selector, FunctionReference validationFunction);

function _installValidation(
bytes32 validationIdToUpdate,
FunctionReference validationFunction,
bool isDefault,
bool isSignatureValidationAllowed,
bytes4[] memory selectors,
bytes calldata installData,
bytes memory preValidationHooks
)
// TODO: flag for signature validation
internal
{
) internal returns (bytes32 validationId) {
AccountStorage storage _storage = getAccountStorage();
validationId = validationIdToUpdate;

if (validationId == bytes32(0)) {
validationId = keccak256(abi.encode(validationFunction, isDefault, installData));

if (FunctionReferenceLib.notEmpty(_storage.validationData[validationId].validationFunction)) {
revert ValidationAlreadySetNew(validationId);
}
}

// TODO: all fields can be updated on the validation except selectors and hooks are addition only, to
// update, require uninstall and reinstall of validation

_storage.validationData[validationId].validationFunction = validationFunction;
_storage.validationData[validationId].isDefault = isDefault;
_storage.validationData[validationId].isSignatureValidationAllowed = isSignatureValidationAllowed;

if (preValidationHooks.length > 0) {
(FunctionReference[] memory preValidationFunctions, bytes[] memory initDatas) =
Expand All @@ -37,9 +63,7 @@ abstract contract PluginManager2 {
FunctionReference preValidationFunction = preValidationFunctions[i];

if (
!_storage.validationData[validationFunction].preValidationHooks.add(
toSetValue(preValidationFunction)
)
!_storage.validationData[validationId].preValidationHooks.add(toSetValue(preValidationFunction))
) {
revert PreValidationAlreadySet(validationFunction, preValidationFunction);
}
Expand All @@ -51,13 +75,6 @@ abstract contract PluginManager2 {
}
}

if (isDefault) {
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))) {
Expand All @@ -67,26 +84,27 @@ abstract contract PluginManager2 {

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

function _uninstallValidation(
FunctionReference validationFunction,
bytes32 validationId,
bytes4[] calldata selectors,
bytes calldata uninstallData,
bytes calldata preValidationHookUninstallData
) internal {
AccountStorage storage _storage = getAccountStorage();
ValidationData storage validationData = _storage.validationData[validationId];

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

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

// Clear pre validation hooks
EnumerableSet.Bytes32Set storage preValidationHooks =
_storage.validationData[validationFunction].preValidationHooks;
EnumerableSet.Bytes32Set storage preValidationHooks = validationData.preValidationHooks;
while (preValidationHooks.length() > 0) {
FunctionReference preValidationFunction = toFunctionReference(preValidationHooks.at(0));
preValidationHooks.remove(toSetValue(preValidationFunction));
Expand All @@ -101,14 +119,16 @@ abstract contract PluginManager2 {
// 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, validationFunction);
if (!_storage.selectorData[selector].validations.remove(toSetValue(validationData.validationFunction)))
{
revert ValidationNotSet(selector, validationData.validationFunction);
}
}

if (uninstallData.length > 0) {
(address plugin,) = FunctionReferenceLib.unpack(validationFunction);
(address plugin,) = FunctionReferenceLib.unpack(validationData.validationFunction);
IPlugin(plugin).onUninstall(uninstallData);
}
emit ValidationUninstalled(validationId);
}
}
56 changes: 28 additions & 28 deletions src/account/PluginManagerInternals.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,20 +220,20 @@ abstract contract PluginManagerInternals is IPluginManager {
_storage.callPermitted[plugin][manifest.permittedExecutionSelectors[i]] = true;
}

length = manifest.validationFunctions.length;
for (uint256 i = 0; i < length; ++i) {
ManifestAssociatedFunction memory mv = manifest.validationFunctions[i];
_addValidationFunction(
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.validationData[signatureValidationFunction].isSignatureValidation = true;
}
// length = manifest.validationFunctions.length;
// for (uint256 i = 0; i < length; ++i) {
// ManifestAssociatedFunction memory mv = manifest.validationFunctions[i];
// _addValidationFunction(
// 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.validationData[signatureValidationFunction].isSignatureValidation = true;
// }

length = manifest.executionHooks.length;
for (uint256 i = 0; i < length; ++i) {
Expand Down Expand Up @@ -298,20 +298,20 @@ abstract contract PluginManagerInternals is IPluginManager {
_removeExecHooks(mh.executionSelector, hookFunction, mh.isPreHook, mh.isPostHook);
}

length = manifest.signatureValidationFunctions.length;
for (uint256 i = 0; i < length; ++i) {
FunctionReference signatureValidationFunction =
FunctionReferenceLib.pack(plugin, manifest.signatureValidationFunctions[i]);
_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)
);
}
// length = manifest.signatureValidationFunctions.length;
// for (uint256 i = 0; i < length; ++i) {
// FunctionReference signatureValidationFunction =
// FunctionReferenceLib.pack(plugin, manifest.signatureValidationFunctions[i]);
// _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)
// );
// }

length = manifest.permittedExecutionSelectors.length;
for (uint256 i = 0; i < length; ++i) {
Expand Down
Loading