Skip to content

Commit

Permalink
chore: remove existing permissions system
Browse files Browse the repository at this point in the history
  • Loading branch information
howydev committed Jun 19, 2024
1 parent b680f75 commit ee0a657
Show file tree
Hide file tree
Showing 8 changed files with 11 additions and 38 deletions.
1 change: 0 additions & 1 deletion src/account/AccountStorage.sol
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ struct AccountStorage {
// Execution functions and their associated functions
mapping(bytes4 => SelectorData) selectorData;
mapping(FunctionReference => ValidationData) validationData;
mapping(address caller => mapping(bytes4 selector => bool)) callPermitted;
// For ERC165 introspection
mapping(bytes4 => uint256) supportedIfaces;
}
Expand Down
13 changes: 0 additions & 13 deletions src/account/PluginManagerInternals.sol
Original file line number Diff line number Diff line change
Expand Up @@ -187,14 +187,6 @@ abstract contract PluginManagerInternals is IPluginManager {
_setExecutionFunction(selector, isPublic, allowSharedValidation, plugin);
}

// Add installed plugin and selectors this plugin can call
length = manifest.permittedExecutionSelectors.length;
for (uint256 i = 0; i < length; ++i) {
// 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[plugin][manifest.permittedExecutionSelectors[i]] = true;
}

length = manifest.validationFunctions.length;
for (uint256 i = 0; i < length; ++i) {
ManifestAssociatedFunction memory mv = manifest.validationFunctions[i];
Expand Down Expand Up @@ -288,11 +280,6 @@ abstract contract PluginManagerInternals is IPluginManager {
);
}

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

length = manifest.executionFunctions.length;
for (uint256 i = 0; i < length; ++i) {
bytes4 selector = manifest.executionFunctions[i].executionSelector;
Expand Down
8 changes: 3 additions & 5 deletions src/account/UpgradeableModularAccount.sol
Original file line number Diff line number Diff line change
Expand Up @@ -546,11 +546,9 @@ contract UpgradeableModularAccount is
AccountStorage storage _storage = getAccountStorage();

if (
msg.sender == address(_ENTRY_POINT) || msg.sender == address(this)
|| _storage.selectorData[msg.sig].isPublic
) return;

if (!_storage.callPermitted[msg.sender][msg.sig]) {
msg.sender != address(_ENTRY_POINT) && msg.sender != address(this)
&& !_storage.selectorData[msg.sig].isPublic
) {
revert ExecFromPluginNotPermitted(msg.sender, msg.sig);
}
}
Expand Down
2 changes: 0 additions & 2 deletions src/interfaces/IPlugin.sol
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,6 @@ struct PluginManifest {
ManifestAssociatedFunction[] validationFunctions;
ManifestExecutionHook[] executionHooks;
uint8[] signatureValidationFunctions;
// Plugin execution functions already installed on the MSCA that this plugin will be able to call.
bytes4[] permittedExecutionSelectors;
// List of ERC-165 interface IDs to add to account to support introspection checks. This MUST NOT include
// IPlugin's interface ID.
bytes4[] interfaceIds;
Expand Down
10 changes: 5 additions & 5 deletions test/account/AccountExecHooks.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ contract AccountExecHooksTest is AccountTestBase {
uint8 internal constant _POST_HOOK_FUNCTION_ID_2 = 2;
uint8 internal constant _BOTH_HOOKS_FUNCTION_ID_3 = 3;

PluginManifest internal m1;
PluginManifest internal m2;
PluginManifest internal _m1;
PluginManifest internal _m2;

event PluginInstalled(address indexed plugin, bytes32 manifestHash, FunctionReference[] dependencies);
event PluginUninstalled(address indexed plugin, bool indexed callbacksSucceeded);
Expand All @@ -35,7 +35,7 @@ contract AccountExecHooksTest is AccountTestBase {
function setUp() public {
_transferOwnershipToTest();

m1.executionFunctions.push(
_m1.executionFunctions.push(
ManifestExecutionFunction({
executionSelector: _EXEC_SELECTOR,
isPublic: true,
Expand Down Expand Up @@ -163,8 +163,8 @@ contract AccountExecHooksTest is AccountTestBase {
}

function _installPlugin1WithHooks(ManifestExecutionHook memory execHooks) internal {
m1.executionHooks.push(execHooks);
mockPlugin1 = new MockPlugin(m1);
_m1.executionHooks.push(execHooks);
mockPlugin1 = new MockPlugin(_m1);
manifestHash1 = keccak256(abi.encode(mockPlugin1.pluginManifest()));

vm.expectEmit(true, true, true, true);
Expand Down
8 changes: 3 additions & 5 deletions test/account/UpgradeableModularAccount.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {IERC1271} from "@openzeppelin/contracts/interfaces/IERC1271.sol";
import {PluginManagerInternals} from "../../src/account/PluginManagerInternals.sol";
import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol";
import {FunctionReference, FunctionReferenceLib} from "../../src/helpers/FunctionReferenceLib.sol";
import {IPlugin, PluginManifest} from "../../src/interfaces/IPlugin.sol";
import {PluginManifest} from "../../src/interfaces/IPlugin.sol";
import {IAccountLoupe} from "../../src/interfaces/IAccountLoupe.sol";
import {IPluginManager} from "../../src/interfaces/IPluginManager.sol";
import {Call} from "../../src/interfaces/IStandardExecutor.sol";
Expand All @@ -37,7 +37,7 @@ contract UpgradeableModularAccountTest is AccountTestBase {

address public ethRecipient;
Counter public counter;
PluginManifest internal manifest;
PluginManifest internal _manifest;

FunctionReference public ownerValidation;

Expand Down Expand Up @@ -262,8 +262,6 @@ contract UpgradeableModularAccountTest is AccountTestBase {
vm.startPrank(address(entryPoint));

PluginManifest memory m;
m.permittedExecutionSelectors = new bytes4[](1);
m.permittedExecutionSelectors[0] = IPlugin.onInstall.selector;

MockPlugin mockPluginWithBadPermittedExec = new MockPlugin(m);
bytes32 manifestHash = keccak256(abi.encode(mockPluginWithBadPermittedExec.pluginManifest()));
Expand Down Expand Up @@ -403,7 +401,7 @@ contract UpgradeableModularAccountTest is AccountTestBase {
function _installPluginWithExecHooks() internal returns (MockPlugin plugin) {
vm.startPrank(address(entryPoint));

plugin = new MockPlugin(manifest);
plugin = new MockPlugin(_manifest);
bytes32 manifestHash = keccak256(abi.encode(plugin.pluginManifest()));

IPluginManager(account1).installPlugin({
Expand Down
4 changes: 0 additions & 4 deletions test/mocks/plugins/PermittedCallMocks.sol
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,6 @@ contract PermittedCallerPlugin is BasePlugin {
manifest.executionFunctions[i].isPublic = true;
}

// Request permission only for "foo", but not "bar", from ResultCreatorPlugin
manifest.permittedExecutionSelectors = new bytes4[](1);
manifest.permittedExecutionSelectors[0] = ResultCreatorPlugin.foo.selector;

return manifest;
}

Expand Down
3 changes: 0 additions & 3 deletions test/mocks/plugins/ReturnDataPluginMocks.sol
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,6 @@ contract ResultConsumerPlugin is BasePlugin, IValidation {
allowSharedValidation: false
});

manifest.permittedExecutionSelectors = new bytes4[](1);
manifest.permittedExecutionSelectors[0] = ResultCreatorPlugin.foo.selector;

return manifest;
}

Expand Down

0 comments on commit ee0a657

Please sign in to comment.