Skip to content

feat: [v0.8-develop] associate pre-validation hooks with validation functions #64

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

Merged
adamegyed merged 2 commits into from
Jun 20, 2024
Merged

feat: [v0.8-develop] associate pre-validation hooks with validation functions #64

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4d2390b
associate pre-validation hooks with validation functions, remove alwa…
adamegyed Jun 4, 2024
53b13e9
Update comments and naming
adamegyed Jun 20, 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(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
Open in desktop
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;
Copy link
Collaborator

@howydev howydev Jun 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughts on removing this entirely? What I'm thinking of:

  1. We just keep the global validation fns set, no more selector associated validation fns
  2. Permissions framework (pre val hooks attached to validation fns) is responsible for what selectors a val fn can operate on

If I'm not wrong this should remove the need to do an existence check across the global validations set + the selector associated validation set and simplify a lot of the checking logic around validation

Copy link
Collaborator

@howydev howydev Jun 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's some flexibility in the shape of #2 - a simple MA could just do:

struct ValidationData {
  bool isShared
  bool isSignatureValidation
  mapping(bytes4 selector -> bool canValidate) canValidateSelector
  EnumerableSet.Bytes32Set preValidationHooks
}

to support 2 modes of validation functions - those that can be used either globally, or just for a set of selectors

Copy link
Contributor Author

@adamegyed adamegyed Jun 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without selector-associated validations, every validation would need a pre-hook just to do selector associations - it's one of those things that benefits from being builtin, but yes I agree we should be careful in what we do this for.

I actually thought about making the refactor to the format of mapping(bytes4 selector -> bool canValidate) canValidateSelector, but there's one reason we can't do it just yet - the current interface of our loupe functions.

Right now, we have function getValidations(bytes4 selector) external view returns (FunctionReference[] memory), which returns the validations per selector. Even if we made the canValidateSelector enumerable, we wouldn't be able to implement this interface efficiently, we'd need to search every single validation function.

But, if we change the view function to function getSelectors(FunctionReference validation) external view returns (bytes4[] memory), then we could make this refactor.

We should revisit this in the future, and decide which of these functions it actually makes sense to implement.

Copy link
Collaborator

@fangting-alchemy fangting-alchemy Jun 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should capture this conversation somewhere. It is very meaningful, but is not in this PR's scope. @howydev Seems you proposed it. Maybe even capture as a proposal to the issues list.

In this world where there are only global validations, how are native functions' validation handled?
Would those native functions just parts of canValidateSelector on some of the validations? (Note, right now, default/global validators can validate any native functions).

Copy link
Collaborator

@fangting-alchemy fangting-alchemy Jun 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the code below, seems we don't really support default/global validators can validate any native functions natively in code. Users still need to setup by adding the validation functions to each native functions. Did I miss anything?

If the above is the case, then there is no reason to have two set of pools (default, and selector specific), we should combine them.

Copy link
Contributor Author

@adamegyed adamegyed Jun 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I discussed this with @fangting-alchemy in a call, documenting here for others:

Users still need to setup by adding the validation functions to each native functions.

This is not necessary, once a validation function is marked as isDefault, it will apply to any function allowed by default validation in _defaultValidationAllowed, which includes the native functions.

The ability to pass in selectors to installValidation is how to install the validation as a non-default validation function. This allows users to install a validation as both a default validation and as a selector-associated one for multiple selectors at once, but in practice I expect most users to do one or the other (either default, or associated with some selectors).

Copy link
Collaborator

@jaypaik jaypaik Jun 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related thought - have we considered making allowDefaultValidation configurable for native functions too? How would we reliably prevent validating into uninstallPlugin if we want to enforce a time delay? I guess we can just use a pre exec hook instead of a pre validation hook.

Is there ever a case where users might want to opt out of default validation for some subset of native functions? Hmm...

}

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;
Copy link
Collaborator

@howydev howydev Jun 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it possible to add defaultValidations as a bool field here?

Copy link
Contributor Author

@adamegyed adamegyed Jun 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep that's what the isShared flag is doing, it's whether or not it counts as a "default" validation. I think the word "default" implies that there's only one ("the default"), but in reality there can be many validations in the shared "default" pool, so I was kinda trying to avoid using that term. But I totally see how this is confusing, can change it or add a comment if it would help.

Copy link
Collaborator

@howydev howydev Jun 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be helpful yeah! How about global validators? Global validators are able to perform validation across all native selectors + installed execution selectors that opt in to global validation

Copy link
Contributor Author

@adamegyed adamegyed Jun 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the same thing, right?

Copy link
Collaborator

@howydev howydev Jun 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup, just a name suggestion, my contribution it to the list of names for the naming meeting :p

Copy link
Collaborator

@jaypaik jaypaik Jun 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can punt, but I do like the idea of using "global" here. I think it's fine that "global validation functions" don't necessarily apply to all selectors, just like how global variables can be overriden.

Copy link
Contributor

@huaweigu huaweigu Jun 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can punt, but I do like the idea of using "global" here. I think it's fine that "global validation functions" don't necessarily apply to all selectors, just like how global variables can be overriden.

yeah that's the behavior I expect as well. We often adhere to default vs overridden in our codebase, but the same idea.

// 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
Open in desktop
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]);
Copy link
Collaborator

@jaypaik jaypaik Jun 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So dependencies no longer need to be installed ahead of time? Do we expect the plugin to be able to handle multiple onInstalls with different data?

Side note: It's interesting how we're going back towards the updatePlugins pattern that we had before. 😅

Copy link
Contributor Author

@adamegyed adamegyed Jun 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This workflow kinda doesn't touch dependencies at all, since it's not reading from the manifest 😅 .

As for expectations around onInstall, this function currently allows the caller to decide whether or not to send it multiple times - if no data is provided, the onInstall call is skipped. Not sure if that should be the final design or not.

Copy link
Collaborator

@jaypaik jaypaik Jun 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re: dependencies - I meant in the sense that we had previously operated under the assumption that plugins can only use what is available from plugins installed already. This sort of breaks that and allows us to pull in arbitrary hooks from non-installed plugins.

Copy link
Collaborator

@jaypaik jaypaik Jun 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not saying the previous approach is right though. I've also been thinking about what it would look like to get rid of the idea of "installing" plugins, and just "using" them instead.

Copy link
Contributor Author

@adamegyed adamegyed Jun 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see what you mean. In a sense, this function is doing the same for the validation function too - the process of "installing" just becomes setting the storage in the account to allow using it, and optionally sending the onInstall callback with some data.

It does create a discrepancy with how the account currently treats a few things related to plugin install state:

  • The implementing plugins for both the validation function and pre-validation hooks won't appear in the list of installed plugins from the loupe function getInstalledPlugins().
  • The validation functions can't be used as dependencies, because the implementing plugin isn't held on that list.
  • The manifest hash is not stored (nor is the manifest even checked)

These all seem like things we should follow up on. It may not be useful for us to even have a getInstalledPlugins function anymore, if they can be installed multiple times in different circumstances. And the extent of manifest contents and checking needs an overhaul in user-supplied install configs.

Copy link
Collaborator

@jaypaik jaypaik Jun 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😅

I think all worth following up on for sure. We're no longer getting value out of the integrity of the manifest. Happy to have us revisit in a future PR. We're deep into this approach already.

}
}
}

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))) {
Copy link
Collaborator

@fangting-alchemy fangting-alchemy Jun 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember native functions can be validated by any default validation.
If the selector is a native function, and the validation function is default, can we skip this step?

Copy link
Contributor Author

@adamegyed adamegyed Jun 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this step can be skipped in that case. For example, the new initialization method passes an empty bytes4[] selectors to this function:

reference-implementation/src/account/UpgradeableModularAccount.sol

Line 337 in 4d2390b

_installValidation(validationFunction, true, new bytes4[](0), installData, bytes(""));

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
Open in desktop
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