Skip to content

Commit

Permalink
Address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
adamegyed committed Apr 26, 2024
1 parent 619793b commit 3002b9d
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 7 deletions.
1 change: 0 additions & 1 deletion src/account/AccountLoupe.sol
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ abstract contract AccountLoupe is IAccountLoupe {
uint256 preExecHooksLength = selectorData.preHooks.length();
uint256 postOnlyExecHooksLength = selectorData.postOnlyHooks.length();

// Overallocate on length - not all of this may get filled up. We set the correct length later.
execHooks = new ExecutionHooks[](preExecHooksLength + postOnlyExecHooksLength);

for (uint256 i = 0; i < preExecHooksLength; ++i) {
Expand Down
6 changes: 5 additions & 1 deletion src/account/AccountStorage.sol
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ struct SelectorData {
// The plugin that implements this execution function.
// If this is a native function, the address must remain address(0).
address plugin;
// 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.
FunctionReference validation;
Expand Down Expand Up @@ -73,9 +77,9 @@ function getPermittedCallKey(address addr, bytes4 selector) pure returns (bytes2
return bytes24(bytes20(addr)) | (bytes24(selector) >> 160);
}

// Helper function to get all elements of a set into memory.
using EnumerableSet for EnumerableSet.Bytes32Set;

/// @dev Helper function to get all elements of a set into memory.
function toFunctionReferenceArray(EnumerableSet.Bytes32Set storage set)
view
returns (FunctionReference[] memory)
Expand Down
4 changes: 2 additions & 2 deletions src/account/PluginManagerInternals.sol
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ abstract contract PluginManagerInternals is IPluginManager {

if (!preExecHook.isEmpty()) {
if (preExecHook.eq(FunctionReferenceLib._PRE_HOOK_ALWAYS_DENY)) {
// Increment the overlappingDenies counter.
// Increment `denyExecutionCount`, because this pre exec hook may be applied multiple times.
_selectorData.denyExecutionCount += 1;
return;
}
Expand Down Expand Up @@ -133,7 +133,7 @@ abstract contract PluginManagerInternals is IPluginManager {

if (!preExecHook.isEmpty()) {
if (preExecHook.eq(FunctionReferenceLib._PRE_HOOK_ALWAYS_DENY)) {
// Decrement the overlappingDenies counter.
// Decrement `denyExecutionCount`, because this pre exec hook may be applied multiple times.
_selectorData.denyExecutionCount -= 1;
return;
}
Expand Down
6 changes: 3 additions & 3 deletions src/account/UpgradeableModularAccount.sol
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,7 @@ contract UpgradeableModularAccount is

FunctionReference associatedPostExecHook = selectorData.associatedPostHooks[preExecHook];

if (associatedPostExecHook.notEq(FunctionReferenceLib._EMPTY_FUNCTION_REFERENCE)) {
if (associatedPostExecHook.notEmpty()) {
postHooksToRun[i + postOnlyHooksLength].postExecHook = associatedPostExecHook;
}
}
Expand All @@ -485,7 +485,7 @@ contract UpgradeableModularAccount is

// If there is an associated post-exec hook, save the return data.
PostExecToRun memory postExecToRun = postHooksToRun[i + postOnlyHooksLength];
if (postExecToRun.postExecHook.notEq(FunctionReferenceLib._EMPTY_FUNCTION_REFERENCE)) {
if (postExecToRun.postExecHook.notEmpty()) {
postExecToRun.preExecHookReturnData = preExecHookReturnData;
}
}
Expand Down Expand Up @@ -514,7 +514,7 @@ contract UpgradeableModularAccount is

PostExecToRun memory postHookToRun = postHooksToRun[i];

if (postHookToRun.postExecHook.eq(FunctionReferenceLib._EMPTY_FUNCTION_REFERENCE)) {
if (postHookToRun.postExecHook.isEmpty()) {
// This is an empty post hook, from a pre-only hook, so we skip it.
continue;
}
Expand Down
4 changes: 4 additions & 0 deletions src/helpers/FunctionReferenceLib.sol
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ library FunctionReferenceLib {
return FunctionReference.unwrap(fr) == bytes21(0);
}

function notEmpty(FunctionReference fr) internal pure returns (bool) {
return FunctionReference.unwrap(fr) != bytes21(0);
}

function isEmptyOrMagicValue(FunctionReference fr) internal pure returns (bool) {
return FunctionReference.unwrap(fr) <= bytes21(uint168(2));
}
Expand Down

0 comments on commit 3002b9d

Please sign in to comment.