Skip to content
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.

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

refactor: use bool instead of struct to store permitted calls #30

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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