Skip to content

Commit

Permalink
refactor: use bool instead of struct to store permitted calls
Browse files Browse the repository at this point in the history
  • Loading branch information
jaypaik committed Jan 23, 2024
1 parent 510b541 commit d154a96
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 27 deletions.
8 changes: 1 addition & 7 deletions src/account/AccountStorage.sol
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,6 @@ struct PluginData {
uint256 dependentCount;
}

// Represents data associated with a plugin's permission to use `executeFromPlugin`
// to interact with another plugin installed on the account.
struct PermittedCallData {
bool callPermitted;
}

// Represents data associated with a plugin's permission to use `executeFromPluginExternal`
// to interact with contracts and addresses external to the account and its plugins.
struct PermittedExternalCallData {
Expand Down Expand Up @@ -69,7 +63,7 @@ struct AccountStorage {
// Execution functions and their associated functions
mapping(bytes4 => SelectorData) selectorData;
// bytes24 key = address(calling plugin) || bytes4(selector of execution function)
mapping(bytes24 => PermittedCallData) permittedCalls;
mapping(bytes24 => bool) callPermitted;
// key = address(calling plugin) || target address
mapping(IPlugin => mapping(address => PermittedExternalCallData)) permittedExternalCalls;
// For ERC165 introspection
Expand Down
23 changes: 4 additions & 19 deletions src/account/PluginManagerInternals.sol
Original file line number Diff line number Diff line change
Expand Up @@ -136,23 +136,6 @@ abstract contract PluginManagerInternals is IPluginManager {
_removeHooks(_selectorData.executionHooks, preExecHook, postExecHook);
}

function _enableExecFromPlugin(bytes4 selector, address plugin, AccountStorage storage accountStorage)
internal
{
bytes24 key = getPermittedCallKey(plugin, selector);

// If there are duplicates, this will just enable the flag again. This is not a problem, since the boolean
// will be set to false twice during uninstall, which is fine.
accountStorage.permittedCalls[key].callPermitted = true;
}

function _disableExecFromPlugin(bytes4 selector, address plugin, AccountStorage storage accountStorage)
internal
{
bytes24 key = getPermittedCallKey(plugin, selector);
accountStorage.permittedCalls[key].callPermitted = false;
}

function _addHooks(HookGroup storage hooks, FunctionReference preExecHook, FunctionReference postExecHook)
internal
{
Expand Down Expand Up @@ -306,7 +289,9 @@ abstract contract PluginManagerInternals is IPluginManager {
// Add installed plugin and selectors this plugin can call
length = manifest.permittedExecutionSelectors.length;
for (uint256 i = 0; i < length;) {
_enableExecFromPlugin(manifest.permittedExecutionSelectors[i], plugin, _storage);
// If there are duplicates, this will just enable the flag again. This is not a problem, since the
// boolean will be set to false twice during uninstall, which is fine.
_storage.callPermitted[getPermittedCallKey(plugin, manifest.permittedExecutionSelectors[i])] = true;

unchecked {
++i;
Expand Down Expand Up @@ -619,7 +604,7 @@ abstract contract PluginManagerInternals is IPluginManager {

length = manifest.permittedExecutionSelectors.length;
for (uint256 i = 0; i < length;) {
_disableExecFromPlugin(manifest.permittedExecutionSelectors[i], plugin, _storage);
_storage.callPermitted[getPermittedCallKey(plugin, manifest.permittedExecutionSelectors[i])] = false;

unchecked {
++i;
Expand Down
2 changes: 1 addition & 1 deletion src/account/UpgradeableModularAccount.sol
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ contract UpgradeableModularAccount is

AccountStorage storage _storage = getAccountStorage();

if (!_storage.permittedCalls[execFromPluginKey].callPermitted) {
if (!_storage.callPermitted[execFromPluginKey]) {
revert ExecFromPluginNotPermitted(callingPlugin, selector);
}

Expand Down

0 comments on commit d154a96

Please sign in to comment.