From 0fb2113a0f1b09e8eeef72dd6d04b04bbc0151a8 Mon Sep 17 00:00:00 2001 From: sm-stack Date: Thu, 9 Nov 2023 12:44:26 +0900 Subject: [PATCH 01/33] Add session key plugin and corresponding tests --- .../session-key/BaseSessionKeyPlugin.sol | 209 ++++++++++ .../session-key/TokenSessionKeyPlugin.sol | 135 +++++++ .../interfaces/ISessionKeyPlugin.sol | 45 +++ .../interfaces/ITokenSessionKeyPlugin.sol | 18 + test/mocks/MockERC20.sol | 12 + test/plugin/SessionKeyPlugin.t.sol | 379 ++++++++++++++++++ 6 files changed, 798 insertions(+) create mode 100644 src/plugins/session-key/BaseSessionKeyPlugin.sol create mode 100644 src/plugins/session-key/TokenSessionKeyPlugin.sol create mode 100644 src/plugins/session-key/interfaces/ISessionKeyPlugin.sol create mode 100644 src/plugins/session-key/interfaces/ITokenSessionKeyPlugin.sol create mode 100644 test/mocks/MockERC20.sol create mode 100644 test/plugin/SessionKeyPlugin.t.sol diff --git a/src/plugins/session-key/BaseSessionKeyPlugin.sol b/src/plugins/session-key/BaseSessionKeyPlugin.sol new file mode 100644 index 00000000..792a506e --- /dev/null +++ b/src/plugins/session-key/BaseSessionKeyPlugin.sol @@ -0,0 +1,209 @@ + +// SPDX-License-Identifier: GPL-3.0 +pragma solidity ^0.8.19; + +import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; +import {UserOperation} from "@eth-infinitism/account-abstraction/interfaces/UserOperation.sol"; +import {UpgradeableModularAccount} from "../../account/UpgradeableModularAccount.sol"; +import { + ManifestFunction, + ManifestAssociatedFunctionType, + ManifestAssociatedFunction, + PluginManifest, + ManifestExecutionFunction, + ManifestExternalCallPermission +} from "../../interfaces/IPlugin.sol"; +import {BasePlugin} from "../BasePlugin.sol"; +import {ISessionKeyPlugin} from "./interfaces/ISessionKeyPlugin.sol"; +import {ISingleOwnerPlugin} from "../owner/ISingleOwnerPlugin.sol"; +import {SingleOwnerPlugin} from "../owner/SingleOwnerPlugin.sol"; + +/// @title Base Session Key Plugin +/// @author Decipher ERC-6900 Team +/// @notice This plugin allows some designated EOA or smart contract to temporarily +/// own a modular account. +/// This base session key plugin acts as a 'parent plugin' for all specific session +/// keys. Using dependency, this plugin can be thought as a proxy contract that stores +/// session key duration information, but with validation functions for session keys. +/// +/// It allows for session key owners to access MSCA both through user operation and +/// runtime, with its own validation functions. + +contract BaseSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { + using ECDSA for bytes32; + + string public constant NAME = "Base Session Key Plugin"; + string public constant VERSION = "1.0.0"; + string public constant AUTHOR = "Decipher ERC-6900 Team"; + + uint256 internal constant _DATE_LENGTH = 6; + + mapping(address => mapping(address => bytes)) internal _sessionDuration; + + // ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ + // ┃ Execution functions ┃ + // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ + + /// @inheritdoc ISessionKeyPlugin + function addTemporaryOwner(address tempOwner, uint48 _after, uint48 _until) external { + if (_until <= _after) { + revert WrongTimeRangeForSession(); + } + bytes memory sessionDuration_ = abi.encodePacked(_after, _until); + _sessionDuration[msg.sender][tempOwner] = sessionDuration_; + emit TemporaryOwnerAdded(msg.sender, tempOwner, _after, _until); + } + + /// @inheritdoc ISessionKeyPlugin + function removeTemporaryOwner(address tempOwner) external { + delete _sessionDuration[msg.sender][tempOwner]; + emit TemporaryOwnerRemoved(msg.sender, tempOwner); + } + + // ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ + // ┃ Plugin view functions ┃ + // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ + + /// @inheritdoc ISessionKeyPlugin + function getSessionDuration(address account, address tempOwner) external view returns (uint48 _after, uint48 _until) { + (_after, _until) = _decode(_sessionDuration[account][tempOwner]); + } + + // ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ + // ┃ Plugin interface functions ┃ + // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ + + /// @inheritdoc BasePlugin + function onInstall(bytes calldata data) external override {} + + /// @inheritdoc BasePlugin + function onUninstall(bytes calldata) external override {} + + /// @inheritdoc BasePlugin + function userOpValidationFunction(uint8 functionId, UserOperation calldata userOp, bytes32 userOpHash) + external + view + override + returns (uint256) + { + (address signer,) = (userOpHash.toEthSignedMessageHash()).tryRecover(userOp.signature); + if (functionId == uint8(FunctionId.USER_OP_VALIDATION_TEMPORARY_OWNER)) { + bytes memory duration = _sessionDuration[userOp.sender][signer]; + if (duration.length != 0) { + (uint48 _after, uint48 _until) = _decode(duration); + // first parameter of _packValidationData is sigFailed, which should be false + return _packValidationData(false, _until, _after); + } + } + revert NotImplemented(); + } + + /// @inheritdoc BasePlugin + function runtimeValidationFunction(uint8 functionId, address sender, uint256, bytes calldata) + external + view + override + { + if (functionId == uint8(FunctionId.RUNTIME_VALIDATION_TEMPORARY_OWNER)) { + bytes memory duration = _sessionDuration[msg.sender][sender]; + if (duration.length != 0) { + (uint48 _after, uint48 _until) = _decode(duration); + if (block.timestamp < _after || block.timestamp > _until) { + revert WrongTimeRangeForSession(); + } + return; + } + } + revert NotImplemented(); + } + + /// @inheritdoc BasePlugin + function pluginManifest() external pure override returns (PluginManifest memory) { + PluginManifest memory manifest; + + manifest.name = NAME; + manifest.version = VERSION; + manifest.author = AUTHOR; + + string[] memory ownerPermissions = new string[](1); + ownerPermissions[0] = "Allow Temporary Ownership"; + + manifest.executionFunctions = new ManifestExecutionFunction[](3); + manifest.executionFunctions[0] = + ManifestExecutionFunction(this.addTemporaryOwner.selector, ownerPermissions); + manifest.executionFunctions[1] = + ManifestExecutionFunction(this.removeTemporaryOwner.selector, ownerPermissions); + manifest.executionFunctions[2] = + ManifestExecutionFunction(this.getSessionDuration.selector, new string[](0)); + + ManifestFunction memory ownerUserOpValidationFunction = ManifestFunction({ + functionType: ManifestAssociatedFunctionType.DEPENDENCY, + functionId: 0, // Unused. + dependencyIndex: 0 // Used as first index. + }); + manifest.userOpValidationFunctions = new ManifestAssociatedFunction[](2); + manifest.userOpValidationFunctions[0] = ManifestAssociatedFunction({ + executionSelector: this.addTemporaryOwner.selector, + associatedFunction: ownerUserOpValidationFunction + }); + manifest.userOpValidationFunctions[1] = ManifestAssociatedFunction({ + executionSelector: this.removeTemporaryOwner.selector, + associatedFunction: ownerUserOpValidationFunction + }); + + ManifestFunction memory ownerOrSelfRuntimeValidationFunction = ManifestFunction({ + functionType: ManifestAssociatedFunctionType.DEPENDENCY, + functionId: 0, // Unused. + dependencyIndex: 1 + }); + ManifestFunction memory alwaysAllowFunction = ManifestFunction({ + functionType: ManifestAssociatedFunctionType.RUNTIME_VALIDATION_ALWAYS_ALLOW, + functionId: 0, // Unused. + dependencyIndex: 0 // Unused. + }); + + manifest.runtimeValidationFunctions = new ManifestAssociatedFunction[](3); + manifest.runtimeValidationFunctions[0] = ManifestAssociatedFunction({ + executionSelector: this.addTemporaryOwner.selector, + associatedFunction: ownerOrSelfRuntimeValidationFunction + }); + manifest.runtimeValidationFunctions[1] = ManifestAssociatedFunction({ + executionSelector: this.removeTemporaryOwner.selector, + associatedFunction: ownerOrSelfRuntimeValidationFunction + }); + manifest.runtimeValidationFunctions[2] = ManifestAssociatedFunction({ + executionSelector: this.getSessionDuration.selector, + associatedFunction: alwaysAllowFunction + }); + + manifest.dependencyInterfaceIds = new bytes4[](2); + manifest.dependencyInterfaceIds[0] = type(ISingleOwnerPlugin).interfaceId; + manifest.dependencyInterfaceIds[1] = type(ISingleOwnerPlugin).interfaceId; + + return manifest; + } + + // ┏━━━━━━━━━━━━━━━┓ + // ┃ EIP-165 ┃ + // ┗━━━━━━━━━━━━━━━┛ + + /// @inheritdoc BasePlugin + function supportsInterface(bytes4 interfaceId) public view override returns (bool) { + return interfaceId == type(ISessionKeyPlugin).interfaceId || super.supportsInterface(interfaceId); + } + + // ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ + // ┃ Internal / Private functions ┃ + // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ + + function _decode(bytes memory _data) internal pure returns (uint48 _after, uint48 _until) { + assembly { + _after := mload(add(_data, _DATE_LENGTH)) + _until := mload(add(_data, mul(_DATE_LENGTH, 2))) + } + } + + function _packValidationData(bool sigFailed, uint48 validUntil, uint48 validAfter) internal pure returns (uint256) { + return (sigFailed ? 1 : 0) | (uint256(validUntil) << 160) | (uint256(validAfter) << (160 + 48)); + } +} diff --git a/src/plugins/session-key/TokenSessionKeyPlugin.sol b/src/plugins/session-key/TokenSessionKeyPlugin.sol new file mode 100644 index 00000000..94472545 --- /dev/null +++ b/src/plugins/session-key/TokenSessionKeyPlugin.sol @@ -0,0 +1,135 @@ + + +// SPDX-License-Identifier: GPL-3.0 +pragma solidity ^0.8.19; + +import { + ManifestFunction, + ManifestAssociatedFunctionType, + ManifestAssociatedFunction, + PluginManifest, + ManifestExecutionFunction, + ManifestExternalCallPermission +} from "../../interfaces/IPlugin.sol"; +import {BasePlugin} from "../BasePlugin.sol"; +import {BaseSessionKeyPlugin} from "./BaseSessionKeyPlugin.sol"; +import {ITokenSessionKeyPlugin} from "./interfaces/ITokenSessionKeyPlugin.sol"; +import {ISessionKeyPlugin} from "./interfaces/ISessionKeyPlugin.sol"; +import {IPluginExecutor} from "../../interfaces/IPluginExecutor.sol"; + +/// @title Token Session Key Plugin +/// @author Decipher ERC-6900 Team +/// @notice This plugin allows an EOA or smart contract to own a modular account. +/// It also supports [ERC-1271](https://eips.ethereum.org/EIPS/eip-1271) signature +/// validation for both validating the signature on user operations and in +/// exposing its own `isValidSignature` method. This only works when the owner of +/// modular account also support ERC-1271. +/// +/// ERC-4337's bundler validation rules limit the types of contracts that can be +/// used as owners to validate user operation signatures. For example, the +/// contract's `isValidSignature` function may not use any forbidden opcodes +/// such as `TIMESTAMP` or `NUMBER`, and the contract may not be an ERC-1967 +/// proxy as it accesses a constant implementation slot not associated with +/// the account, violating storage access rules. This also means that the +/// owner of a modular account may not be another modular account if you want to +/// send user operations through a bundler. +contract TokenSessionKeyPlugin is BasePlugin, ITokenSessionKeyPlugin { + + string public constant NAME = "Token Session Key Plugin"; + string public constant VERSION = "1.0.0"; + string public constant AUTHOR = "Decipher ERC-6900 Team"; + + address public constant TARGET_ERC20_CONTRACT = 0xdeaDDeADDEaDdeaDdEAddEADDEAdDeadDEADDEaD; + bytes4 public constant TRANSFERFROM_SELECTOR = bytes4(keccak256(bytes("transferFrom(address,address,uint256)"))); + bytes4 public constant APPROVE_SELECTOR = bytes4(keccak256(bytes("approve(address,uint256)"))); + + // ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ + // ┃ Execution functions ┃ + // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ + + /// @inheritdoc ITokenSessionKeyPlugin + function routeCallToExecuteFromPluginExternal( + address target, + bytes memory data + ) external returns (bytes memory returnData) { + returnData = IPluginExecutor(msg.sender).executeFromPluginExternal(target, 0, data); + } + + // ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ + // ┃ Plugin interface functions ┃ + // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ + + /// @inheritdoc BasePlugin + function onInstall(bytes calldata data) external override {} + + /// @inheritdoc BasePlugin + function onUninstall(bytes calldata) external override {} + + /// @inheritdoc BasePlugin + function pluginManifest() external pure override returns (PluginManifest memory) { + PluginManifest memory manifest; + + manifest.name = NAME; + manifest.version = VERSION; + manifest.author = AUTHOR; + + string[] memory ownerPermissions = new string[](1); + ownerPermissions[0] = "Allow Token Operation By Session Key"; + + manifest.executionFunctions = new ManifestExecutionFunction[](1); + manifest.executionFunctions[0] = ManifestExecutionFunction(this.routeCallToExecuteFromPluginExternal.selector, ownerPermissions); + + ManifestFunction memory tempOwnerUserOpValidationFunction = ManifestFunction({ + functionType: ManifestAssociatedFunctionType.DEPENDENCY, + functionId: 0, // Unused + dependencyIndex: 0 // Used as first index + }); + ManifestFunction memory tempOwnerRuntimeValidationFunction = ManifestFunction({ + functionType: ManifestAssociatedFunctionType.DEPENDENCY, + functionId: 0, // Unused + dependencyIndex: 1 // Used as first index + }); + + manifest.userOpValidationFunctions = new ManifestAssociatedFunction[](1); + manifest.userOpValidationFunctions[0] = ManifestAssociatedFunction({ + executionSelector: this.routeCallToExecuteFromPluginExternal.selector, + associatedFunction: tempOwnerUserOpValidationFunction + }); + + manifest.runtimeValidationFunctions = new ManifestAssociatedFunction[](1); + manifest.runtimeValidationFunctions[0] = ManifestAssociatedFunction({ + executionSelector: this.routeCallToExecuteFromPluginExternal.selector, + associatedFunction: tempOwnerRuntimeValidationFunction + }); + + manifest.dependencyInterfaceIds = new bytes4[](2); + for (uint256 i = 0; i < manifest.dependencyInterfaceIds.length;) { + manifest.dependencyInterfaceIds[i] = type(ISessionKeyPlugin).interfaceId; + unchecked { + i++; + } + } + + bytes4[] memory permittedExecutionSelectors = new bytes4[](2); + permittedExecutionSelectors[0] = TRANSFERFROM_SELECTOR; + permittedExecutionSelectors[1] = APPROVE_SELECTOR; + + manifest.permittedExternalCalls = new ManifestExternalCallPermission[](1); + manifest.permittedExternalCalls[0] = ManifestExternalCallPermission({ + externalAddress: TARGET_ERC20_CONTRACT, + permitAnySelector: false, + selectors: permittedExecutionSelectors + }); + + return manifest; + } + + // ┏━━━━━━━━━━━━━━━┓ + // ┃ EIP-165 ┃ + // ┗━━━━━━━━━━━━━━━┛ + + /// @inheritdoc BasePlugin + function supportsInterface(bytes4 interfaceId) public view override returns (bool) { + return interfaceId == type(ITokenSessionKeyPlugin).interfaceId || super.supportsInterface(interfaceId); + } +} diff --git a/src/plugins/session-key/interfaces/ISessionKeyPlugin.sol b/src/plugins/session-key/interfaces/ISessionKeyPlugin.sol new file mode 100644 index 00000000..29dbc92e --- /dev/null +++ b/src/plugins/session-key/interfaces/ISessionKeyPlugin.sol @@ -0,0 +1,45 @@ +// SPDX-License-Identifier: GPL-3.0 +pragma solidity ^0.8.19; + +import {UserOperation} from "@eth-infinitism/account-abstraction/interfaces/UserOperation.sol"; + +interface ISessionKeyPlugin { + enum FunctionId { + RUNTIME_VALIDATION_TEMPORARY_OWNER, + USER_OP_VALIDATION_TEMPORARY_OWNER + } + + /// @notice This event is emitted when a temporary owner is added to the account. + /// @param account The account whose ownership changed. + /// @param owner The address of the temporary owner. + /// @param _after The time after which the owner is valid. + /// @param _until The time until which the owner is valid. + event TemporaryOwnerAdded(address indexed account, address indexed owner, uint48 _after, uint48 _until); + event TemporaryOwnerRemoved(address indexed account, address indexed owner); + + error NotAuthorized(); + error WrongTimeRangeForSession(); + + /// @notice Add a temporary owner to the account. + /// @dev This function is installed on the account as part of plugin installation, and should + /// only be called from an account. + /// @param tempOwner The address of the temporary owner. + /// @param _after The time after which the owner is valid. + /// @param _until The time until which the owner is valid. + function addTemporaryOwner( + address tempOwner, + uint48 _after, + uint48 _until + ) external; + + /// @notice Remove a temporary owner from the account. + /// @dev This function is installed on the account as part of plugin installation, and should + /// only be called from an account. + /// @param tempOwner The address of the temporary owner. + function removeTemporaryOwner(address tempOwner) external; + + /// @notice Get Session data for a given account and temporary owner. + /// @param account The account to get session data for. + /// @param tempOwner The address of the temporary owner. + function getSessionDuration(address account, address tempOwner) external view returns (uint48 _after, uint48 _until); +} diff --git a/src/plugins/session-key/interfaces/ITokenSessionKeyPlugin.sol b/src/plugins/session-key/interfaces/ITokenSessionKeyPlugin.sol new file mode 100644 index 00000000..e76c82ce --- /dev/null +++ b/src/plugins/session-key/interfaces/ITokenSessionKeyPlugin.sol @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: GPL-3.0 +pragma solidity ^0.8.19; + +import {UserOperation} from "@eth-infinitism/account-abstraction/interfaces/UserOperation.sol"; + +interface ITokenSessionKeyPlugin { + error NotAuthorized(); + + /// @notice Route call to executeFromPluginExternal at the MSCA. + /// @dev This function will call with value = 0, since sending ether + /// for ERC20 contract is not a normal case. + /// @param target The target address to execute the call on. + /// @param data The call data to execute. + function routeCallToExecuteFromPluginExternal( + address target, + bytes memory data + ) external returns (bytes memory returnData); +} \ No newline at end of file diff --git a/test/mocks/MockERC20.sol b/test/mocks/MockERC20.sol new file mode 100644 index 00000000..d87a9234 --- /dev/null +++ b/test/mocks/MockERC20.sol @@ -0,0 +1,12 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.19; + +import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; + +contract MockERC20 is ERC20 { + constructor(string memory name, string memory symbol) ERC20(name, symbol) {} + + function mint(address account, uint256 amount) external { + _mint(account, amount); + } +} diff --git a/test/plugin/SessionKeyPlugin.t.sol b/test/plugin/SessionKeyPlugin.t.sol new file mode 100644 index 00000000..a64367eb --- /dev/null +++ b/test/plugin/SessionKeyPlugin.t.sol @@ -0,0 +1,379 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.19; + +import {Test} from "forge-std/Test.sol"; + +import {EntryPoint} from "@eth-infinitism/account-abstraction/core/EntryPoint.sol"; +import {UserOperation} from "@eth-infinitism/account-abstraction/interfaces/UserOperation.sol"; +import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; + +import {BasePlugin} from "../../src/plugins/BasePlugin.sol"; +import {SingleOwnerPlugin} from "../../src/plugins/owner/SingleOwnerPlugin.sol"; +import {ISingleOwnerPlugin} from "../../src/plugins/owner/ISingleOwnerPlugin.sol"; +import {BaseSessionKeyPlugin} from "../../src/plugins/session-key/BaseSessionKeyPlugin.sol"; +import {ISessionKeyPlugin} from "../../src/plugins/session-key/interfaces/ISessionKeyPlugin.sol"; +import {TokenSessionKeyPlugin} from "../../src/plugins/session-key/TokenSessionKeyPlugin.sol"; + +import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; +import {MSCAFactoryFixture} from "../mocks/MSCAFactoryFixture.sol"; +import {FunctionReference, FunctionReferenceLib} from "../../src/libraries/FunctionReferenceLib.sol"; +import {IPluginManager} from "../../src/interfaces/IPluginManager.sol"; +import {MockERC20} from "../mocks/MockERC20.sol"; + +contract SessionKeyPluginTest is Test { + using ECDSA for bytes32; + using FunctionReferenceLib for address; + + SingleOwnerPlugin public ownerPlugin; + BaseSessionKeyPlugin public baseSessionKeyPlugin; + TokenSessionKeyPlugin public tokenSessionKeyPlugin; + EntryPoint public entryPoint; + MSCAFactoryFixture public factory; + UpgradeableModularAccount public account; + + MockERC20 public mockERC20impl; + MockERC20 public mockERC20; + address public mockEmptyERC20Addr; + + address public owner; + uint256 public ownerKey; + + address public maliciousOwner; + uint256 public maliciousOwnerKey; + + address public tempOwner; + uint256 public tempOwnerKey; + + address payable public beneficiary; + + uint256 public constant CALL_GAS_LIMIT = 150000; + uint256 public constant VERIFICATION_GAS_LIMIT = 3600000; + + // Event declarations (needed for vm.expectEmit) + event UserOperationRevertReason( + bytes32 indexed userOpHash, + address indexed sender, + uint256 nonce, + bytes revertReason + ); + event TemporaryOwnerAdded(address indexed account, address indexed owner, uint48 _after, uint48 _until); + event TemporaryOwnerRemoved(address indexed account, address indexed owner); + + function setUp() public { + ownerPlugin = new SingleOwnerPlugin(); + baseSessionKeyPlugin = new BaseSessionKeyPlugin(); + tokenSessionKeyPlugin = new TokenSessionKeyPlugin(); + + entryPoint = new EntryPoint(); + factory = new MSCAFactoryFixture(entryPoint, ownerPlugin); + mockERC20impl = new MockERC20("Mock", "MCK"); + + // Etching MockERC20 code into hardcoded address at TokenSessionKeyPlugin + mockEmptyERC20Addr = tokenSessionKeyPlugin.TARGET_ERC20_CONTRACT(); + bytes memory code = address(mockERC20impl).code; + vm.etch(mockEmptyERC20Addr, code); + mockERC20 = MockERC20(mockEmptyERC20Addr); + + (owner, ownerKey) = makeAddrAndKey("owner"); + (maliciousOwner, maliciousOwnerKey) = makeAddrAndKey("maliciousOwner"); + (tempOwner, tempOwnerKey) = makeAddrAndKey("tempOwner"); + + beneficiary = payable(makeAddr("beneficiary")); + vm.deal(beneficiary, 1 wei); + vm.deal(owner, 10 ether); + + // Here, SingleOwnerPlugin already installed in factory + account = factory.createAccount(owner, 0); + + // Mine Mock ERC20 Tokens to account + mockERC20.mint(address(account), 1 ether); + // Fund the account with some ether + vm.deal(address(account), 1 ether); + + vm.startPrank(owner); + FunctionReference[] memory baseSessionDependency = new FunctionReference[](2); + baseSessionDependency[0] = address(ownerPlugin).pack( + uint8(ISingleOwnerPlugin.FunctionId.USER_OP_VALIDATION_OWNER) + ); + baseSessionDependency[1] = address(ownerPlugin).pack( + uint8(ISingleOwnerPlugin.FunctionId.RUNTIME_VALIDATION_OWNER_OR_SELF) + ); + + bytes32 baseSessionKeyManifestHash = keccak256(abi.encode(baseSessionKeyPlugin.pluginManifest())); + + account.installPlugin({ + plugin: address(baseSessionKeyPlugin), + manifestHash: baseSessionKeyManifestHash, + pluginInitData: "", + dependencies: baseSessionDependency, + injectedHooks: new IPluginManager.InjectedHook[](0) + }); + + FunctionReference[] memory tokenSessionDependency = new FunctionReference[](2); + tokenSessionDependency[0] = address(baseSessionKeyPlugin).pack( + uint8(ISessionKeyPlugin.FunctionId.USER_OP_VALIDATION_TEMPORARY_OWNER) + ); + tokenSessionDependency[1] = address(baseSessionKeyPlugin).pack( + uint8(ISessionKeyPlugin.FunctionId.RUNTIME_VALIDATION_TEMPORARY_OWNER) + ); + bytes32 tokenSessionKeyManifestHash = + keccak256(abi.encode(tokenSessionKeyPlugin.pluginManifest())); + account.installPlugin({ + plugin: address(tokenSessionKeyPlugin), + manifestHash: tokenSessionKeyManifestHash, + pluginInitData: "", + dependencies: tokenSessionDependency, + injectedHooks: new IPluginManager.InjectedHook[](0) + }); + vm.stopPrank(); + + vm.startPrank(address(account)); + vm.expectEmit(true, true, true, true); + emit TemporaryOwnerAdded(address(account), tempOwner, 0, type(uint48).max); + baseSessionKeyPlugin.addTemporaryOwner(tempOwner, 0, type(uint48).max); + + (uint48 _after, uint48 _until) = + baseSessionKeyPlugin.getSessionDuration(address(account), tempOwner); + + assertEq(_after, 0); + assertEq(_until, type(uint48).max); + } + + function test_sessionKey_userOp() public { + bytes[] memory callData = new bytes[](2); + // Since mint function at MockERC20 is not increasing allowance, we should do it manually for testing + callData[0] = _getApproveCalldata(address(account), 1 ether); + callData[1] = _getTransferFromCalldata(address(account), beneficiary, 1 ether); + + UserOperation[] memory userOps = new UserOperation[](2); + + for (uint256 i; i < callData.length; i++) { + (, UserOperation memory userOp) = _constructUserOp(address(mockERC20), callData[i], i); + userOps[i] = userOp; + } + entryPoint.handleOps(userOps, beneficiary); + + assertEq(mockERC20.balanceOf(address(account)), 0); + assertEq(mockERC20.balanceOf(beneficiary), 1 ether); + } + + function test_sessionKey_runtime() public { + vm.prank(address(account)); + mockERC20.approve(address(account), 1 ether); + + bytes memory callData = _getTransferFromCalldata(address(account), beneficiary, 1 ether); + + vm.prank(address(tempOwner)); + TokenSessionKeyPlugin(address(account)).routeCallToExecuteFromPluginExternal( + address(mockERC20), + callData + ); + + assertEq(mockERC20.balanceOf(address(account)), 0); + assertEq(mockERC20.balanceOf(beneficiary), 1 ether); + } + + function test_sessionKey_removeTempOwner() public { + vm.startPrank(address(account)); + + vm.expectEmit(true, true, true, true); + emit TemporaryOwnerRemoved(address(account), tempOwner); + baseSessionKeyPlugin.removeTemporaryOwner(tempOwner); + + vm.stopPrank(); + + (uint48 _after, uint48 _until) = + baseSessionKeyPlugin.getSessionDuration(address(account), tempOwner); + assertEq(_after, 0); + assertEq(_until, 0); + + // Check if tempOwner can still send user operations + vm.startPrank(address(tempOwner)); + + bytes memory revertReason = abi.encodeWithSelector( + BasePlugin.NotImplemented.selector + ); + bytes memory callData = _getTransferFromCalldata(address(account), beneficiary, 1 ether); + + vm.expectRevert(abi.encodeWithSelector( + UpgradeableModularAccount.RuntimeValidationFunctionReverted.selector, + address(baseSessionKeyPlugin), + ISessionKeyPlugin.FunctionId.RUNTIME_VALIDATION_TEMPORARY_OWNER, + revertReason + )); + TokenSessionKeyPlugin(address(account)).routeCallToExecuteFromPluginExternal( + address(mockERC20), + callData + ); + } + + function test_sessionKey_invalidContractFails() public { + address wrongERC20Contract = makeAddr("wrongERC20Contract"); + bytes memory callData = _getApproveCalldata(address(account), 1 ether); + (bytes32 userOpHash, UserOperation memory userOp) = _constructUserOp(wrongERC20Contract, callData); + + UserOperation[] memory userOps = new UserOperation[](1); + userOps[0] = userOp; + + bytes memory revertReason = abi.encodeWithSelector( + UpgradeableModularAccount.ExecFromPluginExternalNotPermitted.selector, + address(tokenSessionKeyPlugin), + address(wrongERC20Contract), + 0, + callData + ); + vm.expectEmit(true, true, true, true); + emit UserOperationRevertReason(userOpHash, address(account), 0, revertReason); + + entryPoint.handleOps(userOps, beneficiary); + } + + function test_sessionKey_invalidMethodFails() public { + bytes4 wrongSelector = 0x394697a3; // bytes4(keccak256(bytes("safeApprove(address,uint256)"))) + bytes memory callData = abi.encodeWithSelector( + wrongSelector, + address(account), + 1 ether + ); + + (bytes32 userOpHash, UserOperation memory userOp) = _constructUserOp(address(mockERC20), callData); + + UserOperation[] memory userOps = new UserOperation[](1); + + userOps[0] = userOp; + + bytes memory revertReason = abi.encodeWithSelector( + UpgradeableModularAccount.ExecFromPluginExternalNotPermitted.selector, + address(tokenSessionKeyPlugin), + address(mockERC20), + 0, + callData + ); + vm.expectEmit(true, true, true, true); + emit UserOperationRevertReason(userOpHash, address(account), 0, revertReason); + + entryPoint.handleOps(userOps, beneficiary); + } + + function test_sessionKey_unregisteredTempOwnerFails() public { + vm.prank(address(maliciousOwner)); + bytes memory callData = _getApproveCalldata(address(account), 1 ether); + bytes memory revertReason = abi.encodeWithSelector( + BasePlugin.NotImplemented.selector + ); + + vm.expectRevert(abi.encodeWithSelector( + UpgradeableModularAccount.RuntimeValidationFunctionReverted.selector, + address(baseSessionKeyPlugin), + ISessionKeyPlugin.FunctionId.RUNTIME_VALIDATION_TEMPORARY_OWNER, + revertReason + )); + TokenSessionKeyPlugin(address(account)).routeCallToExecuteFromPluginExternal( + address(mockERC20), + callData + ); + } + + function test_sessionKey_invalidSessionDurationFails() public { + vm.prank(address(account)); + baseSessionKeyPlugin.addTemporaryOwner(tempOwner, 0, 2); + // Move block.timestamp to 12345 + vm.warp(12345); + + vm.startPrank(address(tempOwner)); + bytes memory callData = _getApproveCalldata(address(account), 1 ether); + + bytes memory revertReason = abi.encodeWithSelector( + ISessionKeyPlugin.WrongTimeRangeForSession.selector + ); + + vm.expectRevert(abi.encodeWithSelector( + UpgradeableModularAccount.RuntimeValidationFunctionReverted.selector, + address(baseSessionKeyPlugin), + ISessionKeyPlugin.FunctionId.RUNTIME_VALIDATION_TEMPORARY_OWNER, + revertReason + )); + TokenSessionKeyPlugin(address(account)).routeCallToExecuteFromPluginExternal( + address(mockERC20), + callData + ); + } + + // Internal Function + + function _getApproveCalldata(address spender, uint256 amount) internal pure returns (bytes memory) { + return abi.encodeWithSelector( + bytes4(keccak256(bytes("approve(address,uint256)"))), + spender, + amount + ); + } + + function _getTransferFromCalldata(address from, address to, uint256 amount) internal pure returns (bytes memory) { + return abi.encodeWithSelector( + bytes4(keccak256(bytes("transferFrom(address,address,uint256)"))), + from, + to, + amount + ); + } + + function _constructUserOp(address targetContract, bytes memory callData) internal view + returns (bytes32, UserOperation memory) { + bytes memory userOpCallData = abi.encodeCall( + TokenSessionKeyPlugin.routeCallToExecuteFromPluginExternal, + (targetContract, callData) + ); + + UserOperation memory userOp = UserOperation({ + sender: address(account), + nonce: 0, + initCode: "", + callData: userOpCallData, + callGasLimit: CALL_GAS_LIMIT, + verificationGasLimit: VERIFICATION_GAS_LIMIT, + preVerificationGas: 0, + maxFeePerGas: 2, + maxPriorityFeePerGas: 1, + paymasterAndData: "", + signature: "" + }); + + // Generate signature + bytes32 userOpHash = entryPoint.getUserOpHash(userOp); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(tempOwnerKey, userOpHash.toEthSignedMessageHash()); + userOp.signature = abi.encodePacked(r, s, v); + + return (userOpHash, userOp); + } + + function _constructUserOp(address targetContract, bytes memory callData, uint256 nonce) + internal view returns (bytes32, UserOperation memory) { + bytes memory userOpCallData = abi.encodeCall( + TokenSessionKeyPlugin.routeCallToExecuteFromPluginExternal, + (targetContract, callData) + ); + + UserOperation memory userOp = UserOperation({ + sender: address(account), + nonce: nonce, + initCode: "", + callData: userOpCallData, + callGasLimit: CALL_GAS_LIMIT, + verificationGasLimit: VERIFICATION_GAS_LIMIT, + preVerificationGas: 0, + maxFeePerGas: 2, + maxPriorityFeePerGas: 1, + paymasterAndData: "", + signature: "" + }); + + // Generate signature + bytes32 userOpHash = entryPoint.getUserOpHash(userOp); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(tempOwnerKey, userOpHash.toEthSignedMessageHash()); + userOp.signature = abi.encodePacked(r, s, v); + + return (userOpHash, userOp); + } +} + From e4b429c94bab4bb9148b6b6c73b35086f8e57dac Mon Sep 17 00:00:00 2001 From: sm-stack Date: Thu, 9 Nov 2023 15:21:31 +0900 Subject: [PATCH 02/33] Fix name of execution function --- .../session-key/TokenSessionKeyPlugin.sol | 19 ++++++++----------- .../interfaces/ITokenSessionKeyPlugin.sol | 10 +++++----- 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/src/plugins/session-key/TokenSessionKeyPlugin.sol b/src/plugins/session-key/TokenSessionKeyPlugin.sol index 94472545..f4a2c5fe 100644 --- a/src/plugins/session-key/TokenSessionKeyPlugin.sol +++ b/src/plugins/session-key/TokenSessionKeyPlugin.sol @@ -41,17 +41,15 @@ contract TokenSessionKeyPlugin is BasePlugin, ITokenSessionKeyPlugin { address public constant TARGET_ERC20_CONTRACT = 0xdeaDDeADDEaDdeaDdEAddEADDEAdDeadDEADDEaD; bytes4 public constant TRANSFERFROM_SELECTOR = bytes4(keccak256(bytes("transferFrom(address,address,uint256)"))); - bytes4 public constant APPROVE_SELECTOR = bytes4(keccak256(bytes("approve(address,uint256)"))); // ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ // ┃ Execution functions ┃ // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ /// @inheritdoc ITokenSessionKeyPlugin - function routeCallToExecuteFromPluginExternal( - address target, - bytes memory data - ) external returns (bytes memory returnData) { + function transferFromSessionKey(address target, address from, address to, uint256 amount) external + returns (bytes memory returnData) { + bytes memory data = abi.encodeWithSelector(TRANSFERFROM_SELECTOR, from, to, amount); returnData = IPluginExecutor(msg.sender).executeFromPluginExternal(target, 0, data); } @@ -77,7 +75,7 @@ contract TokenSessionKeyPlugin is BasePlugin, ITokenSessionKeyPlugin { ownerPermissions[0] = "Allow Token Operation By Session Key"; manifest.executionFunctions = new ManifestExecutionFunction[](1); - manifest.executionFunctions[0] = ManifestExecutionFunction(this.routeCallToExecuteFromPluginExternal.selector, ownerPermissions); + manifest.executionFunctions[0] = ManifestExecutionFunction(this.transferFromSessionKey.selector, ownerPermissions); ManifestFunction memory tempOwnerUserOpValidationFunction = ManifestFunction({ functionType: ManifestAssociatedFunctionType.DEPENDENCY, @@ -87,18 +85,18 @@ contract TokenSessionKeyPlugin is BasePlugin, ITokenSessionKeyPlugin { ManifestFunction memory tempOwnerRuntimeValidationFunction = ManifestFunction({ functionType: ManifestAssociatedFunctionType.DEPENDENCY, functionId: 0, // Unused - dependencyIndex: 1 // Used as first index + dependencyIndex: 1 // Used as second index }); manifest.userOpValidationFunctions = new ManifestAssociatedFunction[](1); manifest.userOpValidationFunctions[0] = ManifestAssociatedFunction({ - executionSelector: this.routeCallToExecuteFromPluginExternal.selector, + executionSelector: this.transferFromSessionKey.selector, associatedFunction: tempOwnerUserOpValidationFunction }); manifest.runtimeValidationFunctions = new ManifestAssociatedFunction[](1); manifest.runtimeValidationFunctions[0] = ManifestAssociatedFunction({ - executionSelector: this.routeCallToExecuteFromPluginExternal.selector, + executionSelector: this.transferFromSessionKey.selector, associatedFunction: tempOwnerRuntimeValidationFunction }); @@ -110,9 +108,8 @@ contract TokenSessionKeyPlugin is BasePlugin, ITokenSessionKeyPlugin { } } - bytes4[] memory permittedExecutionSelectors = new bytes4[](2); + bytes4[] memory permittedExecutionSelectors = new bytes4[](1); permittedExecutionSelectors[0] = TRANSFERFROM_SELECTOR; - permittedExecutionSelectors[1] = APPROVE_SELECTOR; manifest.permittedExternalCalls = new ManifestExternalCallPermission[](1); manifest.permittedExternalCalls[0] = ManifestExternalCallPermission({ diff --git a/src/plugins/session-key/interfaces/ITokenSessionKeyPlugin.sol b/src/plugins/session-key/interfaces/ITokenSessionKeyPlugin.sol index e76c82ce..90b9f099 100644 --- a/src/plugins/session-key/interfaces/ITokenSessionKeyPlugin.sol +++ b/src/plugins/session-key/interfaces/ITokenSessionKeyPlugin.sol @@ -10,9 +10,9 @@ interface ITokenSessionKeyPlugin { /// @dev This function will call with value = 0, since sending ether /// for ERC20 contract is not a normal case. /// @param target The target address to execute the call on. - /// @param data The call data to execute. - function routeCallToExecuteFromPluginExternal( - address target, - bytes memory data - ) external returns (bytes memory returnData); + /// @param from The address to transfer tokens from. + /// @param to The address to transfer tokens to. + /// @param amount The amount of tokens to transfer. + function transferFromSessionKey(address target, address from, address to, uint256 amount) external + returns (bytes memory returnData); } \ No newline at end of file From f12a0cec2676ae15e69ae5326b45da7b742ecb7e Mon Sep 17 00:00:00 2001 From: sm-stack Date: Thu, 9 Nov 2023 15:23:00 +0900 Subject: [PATCH 03/33] Refactor the logic of updating session key to include selector --- .../session-key/BaseSessionKeyPlugin.sol | 34 ++-- .../interfaces/ISessionKeyPlugin.sol | 29 ++-- test/plugin/SessionKeyPlugin.t.sol | 161 +++++------------- 3 files changed, 78 insertions(+), 146 deletions(-) diff --git a/src/plugins/session-key/BaseSessionKeyPlugin.sol b/src/plugins/session-key/BaseSessionKeyPlugin.sol index 792a506e..b6e41912 100644 --- a/src/plugins/session-key/BaseSessionKeyPlugin.sol +++ b/src/plugins/session-key/BaseSessionKeyPlugin.sol @@ -36,28 +36,26 @@ contract BaseSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { string public constant VERSION = "1.0.0"; string public constant AUTHOR = "Decipher ERC-6900 Team"; - uint256 internal constant _DATE_LENGTH = 6; - - mapping(address => mapping(address => bytes)) internal _sessionDuration; + mapping(address => mapping(address => mapping(bytes4 => bytes))) internal _sessionInfo; // ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ // ┃ Execution functions ┃ // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ /// @inheritdoc ISessionKeyPlugin - function addTemporaryOwner(address tempOwner, uint48 _after, uint48 _until) external { + function addTemporaryOwner(address tempOwner, bytes4 allowedSelector, uint48 _after, uint48 _until) external { if (_until <= _after) { revert WrongTimeRangeForSession(); } - bytes memory sessionDuration_ = abi.encodePacked(_after, _until); - _sessionDuration[msg.sender][tempOwner] = sessionDuration_; - emit TemporaryOwnerAdded(msg.sender, tempOwner, _after, _until); + bytes memory sessionInfo = abi.encodePacked(_after, _until); + _sessionInfo[msg.sender][tempOwner][allowedSelector] = sessionInfo; + emit TemporaryOwnerAdded(msg.sender, tempOwner, allowedSelector, _after, _until); } /// @inheritdoc ISessionKeyPlugin - function removeTemporaryOwner(address tempOwner) external { - delete _sessionDuration[msg.sender][tempOwner]; - emit TemporaryOwnerRemoved(msg.sender, tempOwner); + function removeTemporaryOwner(address tempOwner, bytes4 allowedSelector) external { + delete _sessionInfo[msg.sender][tempOwner][allowedSelector]; + emit TemporaryOwnerRemoved(msg.sender, tempOwner, allowedSelector); } // ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ @@ -65,8 +63,8 @@ contract BaseSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ /// @inheritdoc ISessionKeyPlugin - function getSessionDuration(address account, address tempOwner) external view returns (uint48 _after, uint48 _until) { - (_after, _until) = _decode(_sessionDuration[account][tempOwner]); + function getSessionDuration(address account, address tempOwner, bytes4 allowedSelector) external view returns (uint48 _after, uint48 _until) { + (_after, _until) = _decode(_sessionInfo[account][tempOwner][allowedSelector]); } // ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ @@ -88,7 +86,8 @@ contract BaseSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { { (address signer,) = (userOpHash.toEthSignedMessageHash()).tryRecover(userOp.signature); if (functionId == uint8(FunctionId.USER_OP_VALIDATION_TEMPORARY_OWNER)) { - bytes memory duration = _sessionDuration[userOp.sender][signer]; + bytes4 selector = bytes4(userOp.callData[0:4]); + bytes memory duration = _sessionInfo[userOp.sender][signer][selector]; if (duration.length != 0) { (uint48 _after, uint48 _until) = _decode(duration); // first parameter of _packValidationData is sigFailed, which should be false @@ -99,13 +98,14 @@ contract BaseSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { } /// @inheritdoc BasePlugin - function runtimeValidationFunction(uint8 functionId, address sender, uint256, bytes calldata) + function runtimeValidationFunction(uint8 functionId, address sender, uint256, bytes calldata data) external view override { if (functionId == uint8(FunctionId.RUNTIME_VALIDATION_TEMPORARY_OWNER)) { - bytes memory duration = _sessionDuration[msg.sender][sender]; + bytes4 selector = bytes4(data[0:4]); + bytes memory duration = _sessionInfo[msg.sender][sender][selector]; if (duration.length != 0) { (uint48 _after, uint48 _until) = _decode(duration); if (block.timestamp < _after || block.timestamp > _until) { @@ -198,8 +198,8 @@ contract BaseSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { function _decode(bytes memory _data) internal pure returns (uint48 _after, uint48 _until) { assembly { - _after := mload(add(_data, _DATE_LENGTH)) - _until := mload(add(_data, mul(_DATE_LENGTH, 2))) + _after := mload(add(_data, 0x06)) + _until := mload(add(_data, 0x0C)) } } diff --git a/src/plugins/session-key/interfaces/ISessionKeyPlugin.sol b/src/plugins/session-key/interfaces/ISessionKeyPlugin.sol index 29dbc92e..b2e670af 100644 --- a/src/plugins/session-key/interfaces/ISessionKeyPlugin.sol +++ b/src/plugins/session-key/interfaces/ISessionKeyPlugin.sol @@ -10,12 +10,19 @@ interface ISessionKeyPlugin { } /// @notice This event is emitted when a temporary owner is added to the account. - /// @param account The account whose ownership changed. - /// @param owner The address of the temporary owner. + /// @param account The account whose temporary owner is updated. + /// @param tempOwner The address of the temporary owner. + /// @param selector The selector of the function that the temporary owner is allowed to call. /// @param _after The time after which the owner is valid. /// @param _until The time until which the owner is valid. - event TemporaryOwnerAdded(address indexed account, address indexed owner, uint48 _after, uint48 _until); - event TemporaryOwnerRemoved(address indexed account, address indexed owner); + event TemporaryOwnerAdded(address indexed account, address indexed tempOwner, bytes4 selector, uint48 _after, uint48 _until); + + /// @notice This event is emitted when a temporary owner is removed from the account. + /// @param account The account whose temporary owner is updated. + /// @param tempOwner The address of the temporary owner. + /// @param selector The selector of the function that the temporary owner is allowed to call. + + event TemporaryOwnerRemoved(address indexed account, address indexed tempOwner, bytes4 selector); error NotAuthorized(); error WrongTimeRangeForSession(); @@ -24,22 +31,22 @@ interface ISessionKeyPlugin { /// @dev This function is installed on the account as part of plugin installation, and should /// only be called from an account. /// @param tempOwner The address of the temporary owner. + /// @param allowedSelector The selector of the function that the temporary owner is allowed to call. /// @param _after The time after which the owner is valid. /// @param _until The time until which the owner is valid. - function addTemporaryOwner( - address tempOwner, - uint48 _after, - uint48 _until - ) external; + function addTemporaryOwner(address tempOwner, bytes4 allowedSelector, uint48 _after, uint48 _until) external; /// @notice Remove a temporary owner from the account. /// @dev This function is installed on the account as part of plugin installation, and should /// only be called from an account. /// @param tempOwner The address of the temporary owner. - function removeTemporaryOwner(address tempOwner) external; + /// @param allowedSelector The selector of the function that the temporary owner is allowed to call. + function removeTemporaryOwner(address tempOwner, bytes4 allowedSelector) external; /// @notice Get Session data for a given account and temporary owner. /// @param account The account to get session data for. /// @param tempOwner The address of the temporary owner. - function getSessionDuration(address account, address tempOwner) external view returns (uint48 _after, uint48 _until); + /// @param allowedSelector The selector of the function that the temporary owner is allowed to call. + function getSessionDuration(address account, address tempOwner, bytes4 allowedSelector) external view + returns (uint48 _after, uint48 _until); } diff --git a/test/plugin/SessionKeyPlugin.t.sol b/test/plugin/SessionKeyPlugin.t.sol index a64367eb..99ae7b46 100644 --- a/test/plugin/SessionKeyPlugin.t.sol +++ b/test/plugin/SessionKeyPlugin.t.sol @@ -13,6 +13,7 @@ import {ISingleOwnerPlugin} from "../../src/plugins/owner/ISingleOwnerPlugin.sol import {BaseSessionKeyPlugin} from "../../src/plugins/session-key/BaseSessionKeyPlugin.sol"; import {ISessionKeyPlugin} from "../../src/plugins/session-key/interfaces/ISessionKeyPlugin.sol"; import {TokenSessionKeyPlugin} from "../../src/plugins/session-key/TokenSessionKeyPlugin.sol"; +import {ITokenSessionKeyPlugin} from "../../src/plugins/session-key/interfaces/ITokenSessionKeyPlugin.sol"; import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; import {MSCAFactoryFixture} from "../mocks/MSCAFactoryFixture.sol"; @@ -49,6 +50,8 @@ contract SessionKeyPluginTest is Test { uint256 public constant CALL_GAS_LIMIT = 150000; uint256 public constant VERIFICATION_GAS_LIMIT = 3600000; + bytes4 public constant TRANSFERFROM_SESSIONKEY_SELECTOR = ITokenSessionKeyPlugin.transferFromSessionKey.selector; + // Event declarations (needed for vm.expectEmit) event UserOperationRevertReason( bytes32 indexed userOpHash, @@ -56,8 +59,8 @@ contract SessionKeyPluginTest is Test { uint256 nonce, bytes revertReason ); - event TemporaryOwnerAdded(address indexed account, address indexed owner, uint48 _after, uint48 _until); - event TemporaryOwnerRemoved(address indexed account, address indexed owner); + event TemporaryOwnerAdded(address indexed account, address indexed tempOwner, bytes4 allowedSelector, uint48 _after, uint48 _until); + event TemporaryOwnerRemoved(address indexed account, address indexed tempOwner, bytes4 allowedSelector); function setUp() public { ownerPlugin = new SingleOwnerPlugin(); @@ -128,29 +131,25 @@ contract SessionKeyPluginTest is Test { vm.stopPrank(); vm.startPrank(address(account)); + mockERC20.approve(address(account), 1 ether); + vm.expectEmit(true, true, true, true); - emit TemporaryOwnerAdded(address(account), tempOwner, 0, type(uint48).max); - baseSessionKeyPlugin.addTemporaryOwner(tempOwner, 0, type(uint48).max); + emit TemporaryOwnerAdded(address(account), tempOwner, TRANSFERFROM_SESSIONKEY_SELECTOR, 0, 2); + baseSessionKeyPlugin.addTemporaryOwner(tempOwner, TRANSFERFROM_SESSIONKEY_SELECTOR, 0, 2); (uint48 _after, uint48 _until) = - baseSessionKeyPlugin.getSessionDuration(address(account), tempOwner); + baseSessionKeyPlugin.getSessionDuration(address(account), tempOwner, TRANSFERFROM_SESSIONKEY_SELECTOR); assertEq(_after, 0); - assertEq(_until, type(uint48).max); + assertEq(_until, 2); } function test_sessionKey_userOp() public { - bytes[] memory callData = new bytes[](2); - // Since mint function at MockERC20 is not increasing allowance, we should do it manually for testing - callData[0] = _getApproveCalldata(address(account), 1 ether); - callData[1] = _getTransferFromCalldata(address(account), beneficiary, 1 ether); - - UserOperation[] memory userOps = new UserOperation[](2); + UserOperation[] memory userOps = new UserOperation[](1); - for (uint256 i; i < callData.length; i++) { - (, UserOperation memory userOp) = _constructUserOp(address(mockERC20), callData[i], i); - userOps[i] = userOp; - } + (, UserOperation memory userOp) = _constructUserOp(address(mockERC20), address(account), beneficiary, 1 ether); + userOps[0] = userOp; + entryPoint.handleOps(userOps, beneficiary); assertEq(mockERC20.balanceOf(address(account)), 0); @@ -158,15 +157,9 @@ contract SessionKeyPluginTest is Test { } function test_sessionKey_runtime() public { - vm.prank(address(account)); - mockERC20.approve(address(account), 1 ether); - - bytes memory callData = _getTransferFromCalldata(address(account), beneficiary, 1 ether); - vm.prank(address(tempOwner)); - TokenSessionKeyPlugin(address(account)).routeCallToExecuteFromPluginExternal( - address(mockERC20), - callData + TokenSessionKeyPlugin(address(account)).transferFromSessionKey( + address(mockERC20), address(account), beneficiary, 1 ether ); assertEq(mockERC20.balanceOf(address(account)), 0); @@ -177,13 +170,13 @@ contract SessionKeyPluginTest is Test { vm.startPrank(address(account)); vm.expectEmit(true, true, true, true); - emit TemporaryOwnerRemoved(address(account), tempOwner); - baseSessionKeyPlugin.removeTemporaryOwner(tempOwner); + emit TemporaryOwnerRemoved(address(account), tempOwner, TRANSFERFROM_SESSIONKEY_SELECTOR); + baseSessionKeyPlugin.removeTemporaryOwner(tempOwner, TRANSFERFROM_SESSIONKEY_SELECTOR); vm.stopPrank(); (uint48 _after, uint48 _until) = - baseSessionKeyPlugin.getSessionDuration(address(account), tempOwner); + baseSessionKeyPlugin.getSessionDuration(address(account), tempOwner, TRANSFERFROM_SESSIONKEY_SELECTOR); assertEq(_after, 0); assertEq(_until, 0); @@ -193,61 +186,37 @@ contract SessionKeyPluginTest is Test { bytes memory revertReason = abi.encodeWithSelector( BasePlugin.NotImplemented.selector ); - bytes memory callData = _getTransferFromCalldata(address(account), beneficiary, 1 ether); - vm.expectRevert(abi.encodeWithSelector( UpgradeableModularAccount.RuntimeValidationFunctionReverted.selector, address(baseSessionKeyPlugin), ISessionKeyPlugin.FunctionId.RUNTIME_VALIDATION_TEMPORARY_OWNER, revertReason )); - TokenSessionKeyPlugin(address(account)).routeCallToExecuteFromPluginExternal( - address(mockERC20), - callData + TokenSessionKeyPlugin(address(account)).transferFromSessionKey( + address(mockERC20), address(account), beneficiary, 1 ether ); } function test_sessionKey_invalidContractFails() public { address wrongERC20Contract = makeAddr("wrongERC20Contract"); - bytes memory callData = _getApproveCalldata(address(account), 1 ether); - (bytes32 userOpHash, UserOperation memory userOp) = _constructUserOp(wrongERC20Contract, callData); + (bytes32 userOpHash, UserOperation memory userOp) = + _constructUserOp(address(wrongERC20Contract), address(account), beneficiary, 1 ether); UserOperation[] memory userOps = new UserOperation[](1); userOps[0] = userOp; - - bytes memory revertReason = abi.encodeWithSelector( - UpgradeableModularAccount.ExecFromPluginExternalNotPermitted.selector, - address(tokenSessionKeyPlugin), - address(wrongERC20Contract), - 0, - callData - ); - vm.expectEmit(true, true, true, true); - emit UserOperationRevertReason(userOpHash, address(account), 0, revertReason); - - entryPoint.handleOps(userOps, beneficiary); - } - function test_sessionKey_invalidMethodFails() public { - bytes4 wrongSelector = 0x394697a3; // bytes4(keccak256(bytes("safeApprove(address,uint256)"))) - bytes memory callData = abi.encodeWithSelector( - wrongSelector, + bytes memory revertCallData = abi.encodeWithSelector( + tokenSessionKeyPlugin.TRANSFERFROM_SELECTOR(), address(account), + beneficiary, 1 ether ); - - (bytes32 userOpHash, UserOperation memory userOp) = _constructUserOp(address(mockERC20), callData); - - UserOperation[] memory userOps = new UserOperation[](1); - - userOps[0] = userOp; - bytes memory revertReason = abi.encodeWithSelector( UpgradeableModularAccount.ExecFromPluginExternalNotPermitted.selector, address(tokenSessionKeyPlugin), - address(mockERC20), + address(wrongERC20Contract), 0, - callData + revertCallData ); vm.expectEmit(true, true, true, true); emit UserOperationRevertReason(userOpHash, address(account), 0, revertReason); @@ -257,7 +226,6 @@ contract SessionKeyPluginTest is Test { function test_sessionKey_unregisteredTempOwnerFails() public { vm.prank(address(maliciousOwner)); - bytes memory callData = _getApproveCalldata(address(account), 1 ether); bytes memory revertReason = abi.encodeWithSelector( BasePlugin.NotImplemented.selector ); @@ -268,20 +236,16 @@ contract SessionKeyPluginTest is Test { ISessionKeyPlugin.FunctionId.RUNTIME_VALIDATION_TEMPORARY_OWNER, revertReason )); - TokenSessionKeyPlugin(address(account)).routeCallToExecuteFromPluginExternal( - address(mockERC20), - callData + TokenSessionKeyPlugin(address(account)).transferFromSessionKey( + address(mockERC20), address(account), beneficiary, 1 ether ); } function test_sessionKey_invalidSessionDurationFails() public { - vm.prank(address(account)); - baseSessionKeyPlugin.addTemporaryOwner(tempOwner, 0, 2); // Move block.timestamp to 12345 vm.warp(12345); vm.startPrank(address(tempOwner)); - bytes memory callData = _getApproveCalldata(address(account), 1 ether); bytes memory revertReason = abi.encodeWithSelector( ISessionKeyPlugin.WrongTimeRangeForSession.selector @@ -293,36 +257,26 @@ contract SessionKeyPluginTest is Test { ISessionKeyPlugin.FunctionId.RUNTIME_VALIDATION_TEMPORARY_OWNER, revertReason )); - TokenSessionKeyPlugin(address(account)).routeCallToExecuteFromPluginExternal( - address(mockERC20), - callData - ); + TokenSessionKeyPlugin(address(account)).transferFromSessionKey( + address(mockERC20), address(account), beneficiary, 1 ether + ); } // Internal Function - function _getApproveCalldata(address spender, uint256 amount) internal pure returns (bytes memory) { - return abi.encodeWithSelector( - bytes4(keccak256(bytes("approve(address,uint256)"))), - spender, - amount - ); - } - - function _getTransferFromCalldata(address from, address to, uint256 amount) internal pure returns (bytes memory) { - return abi.encodeWithSelector( - bytes4(keccak256(bytes("transferFrom(address,address,uint256)"))), - from, - to, - amount + function _getTransferFromCalldata(address targetContract, address from, address to, uint256 amount) internal pure + returns (bytes memory) { + return abi.encodeCall( + TokenSessionKeyPlugin.transferFromSessionKey, + (targetContract, from, to, amount) ); } - function _constructUserOp(address targetContract, bytes memory callData) internal view + function _constructUserOp(address targetContract, address from, address to, uint256 amount) internal view returns (bytes32, UserOperation memory) { - bytes memory userOpCallData = abi.encodeCall( - TokenSessionKeyPlugin.routeCallToExecuteFromPluginExternal, - (targetContract, callData) + bytes memory userOpCallData = abi.encodeCall( + TokenSessionKeyPlugin.transferFromSessionKey, + (targetContract, from, to, amount) ); UserOperation memory userOp = UserOperation({ @@ -346,34 +300,5 @@ contract SessionKeyPluginTest is Test { return (userOpHash, userOp); } - - function _constructUserOp(address targetContract, bytes memory callData, uint256 nonce) - internal view returns (bytes32, UserOperation memory) { - bytes memory userOpCallData = abi.encodeCall( - TokenSessionKeyPlugin.routeCallToExecuteFromPluginExternal, - (targetContract, callData) - ); - - UserOperation memory userOp = UserOperation({ - sender: address(account), - nonce: nonce, - initCode: "", - callData: userOpCallData, - callGasLimit: CALL_GAS_LIMIT, - verificationGasLimit: VERIFICATION_GAS_LIMIT, - preVerificationGas: 0, - maxFeePerGas: 2, - maxPriorityFeePerGas: 1, - paymasterAndData: "", - signature: "" - }); - - // Generate signature - bytes32 userOpHash = entryPoint.getUserOpHash(userOp); - (uint8 v, bytes32 r, bytes32 s) = vm.sign(tempOwnerKey, userOpHash.toEthSignedMessageHash()); - userOp.signature = abi.encodePacked(r, s, v); - - return (userOpHash, userOp); - } } From c7fd76fafd61f5718eb7d28bbe5acb11262d2e06 Mon Sep 17 00:00:00 2001 From: sm-stack Date: Thu, 9 Nov 2023 20:09:08 +0900 Subject: [PATCH 04/33] Fix lint --- test/plugin/SessionKeyPlugin.t.sol | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/plugin/SessionKeyPlugin.t.sol b/test/plugin/SessionKeyPlugin.t.sol index 99ae7b46..8e386e70 100644 --- a/test/plugin/SessionKeyPlugin.t.sol +++ b/test/plugin/SessionKeyPlugin.t.sol @@ -59,7 +59,13 @@ contract SessionKeyPluginTest is Test { uint256 nonce, bytes revertReason ); - event TemporaryOwnerAdded(address indexed account, address indexed tempOwner, bytes4 allowedSelector, uint48 _after, uint48 _until); + event TemporaryOwnerAdded( + address indexed account, + address indexed tempOwner, + bytes4 allowedSelector, + uint48 _after, + uint48 _until + ); event TemporaryOwnerRemoved(address indexed account, address indexed tempOwner, bytes4 allowedSelector); function setUp() public { From f77df4cf9712dc432bcb0ed96df3fc9cd4ea4d52 Mon Sep 17 00:00:00 2001 From: sm-stack Date: Thu, 16 Nov 2023 11:09:12 +0900 Subject: [PATCH 05/33] Remove unused internal functions --- test/plugin/SessionKeyPlugin.t.sol | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/test/plugin/SessionKeyPlugin.t.sol b/test/plugin/SessionKeyPlugin.t.sol index 8e386e70..f8393b5e 100644 --- a/test/plugin/SessionKeyPlugin.t.sol +++ b/test/plugin/SessionKeyPlugin.t.sol @@ -40,7 +40,6 @@ contract SessionKeyPluginTest is Test { uint256 public ownerKey; address public maliciousOwner; - uint256 public maliciousOwnerKey; address public tempOwner; uint256 public tempOwnerKey; @@ -84,7 +83,7 @@ contract SessionKeyPluginTest is Test { mockERC20 = MockERC20(mockEmptyERC20Addr); (owner, ownerKey) = makeAddrAndKey("owner"); - (maliciousOwner, maliciousOwnerKey) = makeAddrAndKey("maliciousOwner"); + (maliciousOwner, ) = makeAddrAndKey("maliciousOwner"); (tempOwner, tempOwnerKey) = makeAddrAndKey("tempOwner"); beneficiary = payable(makeAddr("beneficiary")); @@ -94,7 +93,7 @@ contract SessionKeyPluginTest is Test { // Here, SingleOwnerPlugin already installed in factory account = factory.createAccount(owner, 0); - // Mine Mock ERC20 Tokens to account + // Mint Mock ERC20 Tokens to account mockERC20.mint(address(account), 1 ether); // Fund the account with some ether vm.deal(address(account), 1 ether); @@ -269,15 +268,6 @@ contract SessionKeyPluginTest is Test { } // Internal Function - - function _getTransferFromCalldata(address targetContract, address from, address to, uint256 amount) internal pure - returns (bytes memory) { - return abi.encodeCall( - TokenSessionKeyPlugin.transferFromSessionKey, - (targetContract, from, to, amount) - ); - } - function _constructUserOp(address targetContract, address from, address to, uint256 amount) internal view returns (bytes32, UserOperation memory) { bytes memory userOpCallData = abi.encodeCall( From 57cf21dd4e0a63a55c2559fe1adf8bbc389e617a Mon Sep 17 00:00:00 2001 From: sm-stack Date: Thu, 16 Nov 2023 11:09:26 +0900 Subject: [PATCH 06/33] Update comments --- .../session-key/BaseSessionKeyPlugin.sol | 9 ++++--- .../session-key/TokenSessionKeyPlugin.sol | 25 +++++++++---------- .../interfaces/ISessionKeyPlugin.sol | 1 - .../interfaces/ITokenSessionKeyPlugin.sol | 2 +- 4 files changed, 18 insertions(+), 19 deletions(-) diff --git a/src/plugins/session-key/BaseSessionKeyPlugin.sol b/src/plugins/session-key/BaseSessionKeyPlugin.sol index b6e41912..d47ab4fe 100644 --- a/src/plugins/session-key/BaseSessionKeyPlugin.sol +++ b/src/plugins/session-key/BaseSessionKeyPlugin.sol @@ -1,4 +1,3 @@ - // SPDX-License-Identifier: GPL-3.0 pragma solidity ^0.8.19; @@ -23,11 +22,13 @@ import {SingleOwnerPlugin} from "../owner/SingleOwnerPlugin.sol"; /// @notice This plugin allows some designated EOA or smart contract to temporarily /// own a modular account. /// This base session key plugin acts as a 'parent plugin' for all specific session -/// keys. Using dependency, this plugin can be thought as a proxy contract that stores -/// session key duration information, but with validation functions for session keys. -/// +/// keys. Using dependency, this plugin can be thought as a parent contract that stores +/// session key duration information, and validation functions for session keys. All +/// logics for session keys will be implemented in child plugins. /// It allows for session key owners to access MSCA both through user operation and /// runtime, with its own validation functions. +/// Also, it has a dependency on SingleOwnerPlugin, to make sure that only the owner of +/// the MSCA can add or remove session keys. contract BaseSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { using ECDSA for bytes32; diff --git a/src/plugins/session-key/TokenSessionKeyPlugin.sol b/src/plugins/session-key/TokenSessionKeyPlugin.sol index f4a2c5fe..30bae99b 100644 --- a/src/plugins/session-key/TokenSessionKeyPlugin.sol +++ b/src/plugins/session-key/TokenSessionKeyPlugin.sol @@ -19,26 +19,25 @@ import {IPluginExecutor} from "../../interfaces/IPluginExecutor.sol"; /// @title Token Session Key Plugin /// @author Decipher ERC-6900 Team -/// @notice This plugin allows an EOA or smart contract to own a modular account. -/// It also supports [ERC-1271](https://eips.ethereum.org/EIPS/eip-1271) signature -/// validation for both validating the signature on user operations and in -/// exposing its own `isValidSignature` method. This only works when the owner of -/// modular account also support ERC-1271. +/// @notice This plugin acts as a 'child plugin' for BaseSessionKeyPlugin. +/// It implements the logic for session keys that are allowed to call ERC20 +/// transferFrom function. It allows for session key owners to access MSCA +/// with `transferFromSessionKey` function, which calls `executeFromPluginExternal` +/// function in PluginExecutor contract. /// -/// ERC-4337's bundler validation rules limit the types of contracts that can be -/// used as owners to validate user operation signatures. For example, the -/// contract's `isValidSignature` function may not use any forbidden opcodes -/// such as `TIMESTAMP` or `NUMBER`, and the contract may not be an ERC-1967 -/// proxy as it accesses a constant implementation slot not associated with -/// the account, violating storage access rules. This also means that the -/// owner of a modular account may not be another modular account if you want to -/// send user operations through a bundler. +/// The target ERC20 contract and the selector for transferFrom function are hardcoded +/// in this plugin, since the pluginManifest function requires the information of +/// permitted external calls not to be changed in the future. For other child session +/// key plugins, there can be a set of permitted external calls according to the +/// specific needs. + contract TokenSessionKeyPlugin is BasePlugin, ITokenSessionKeyPlugin { string public constant NAME = "Token Session Key Plugin"; string public constant VERSION = "1.0.0"; string public constant AUTHOR = "Decipher ERC-6900 Team"; + // Mock address of target ERC20 contract address public constant TARGET_ERC20_CONTRACT = 0xdeaDDeADDEaDdeaDdEAddEADDEAdDeadDEADDEaD; bytes4 public constant TRANSFERFROM_SELECTOR = bytes4(keccak256(bytes("transferFrom(address,address,uint256)"))); diff --git a/src/plugins/session-key/interfaces/ISessionKeyPlugin.sol b/src/plugins/session-key/interfaces/ISessionKeyPlugin.sol index b2e670af..db0f7bc9 100644 --- a/src/plugins/session-key/interfaces/ISessionKeyPlugin.sol +++ b/src/plugins/session-key/interfaces/ISessionKeyPlugin.sol @@ -21,7 +21,6 @@ interface ISessionKeyPlugin { /// @param account The account whose temporary owner is updated. /// @param tempOwner The address of the temporary owner. /// @param selector The selector of the function that the temporary owner is allowed to call. - event TemporaryOwnerRemoved(address indexed account, address indexed tempOwner, bytes4 selector); error NotAuthorized(); diff --git a/src/plugins/session-key/interfaces/ITokenSessionKeyPlugin.sol b/src/plugins/session-key/interfaces/ITokenSessionKeyPlugin.sol index 90b9f099..d395bb86 100644 --- a/src/plugins/session-key/interfaces/ITokenSessionKeyPlugin.sol +++ b/src/plugins/session-key/interfaces/ITokenSessionKeyPlugin.sol @@ -8,7 +8,7 @@ interface ITokenSessionKeyPlugin { /// @notice Route call to executeFromPluginExternal at the MSCA. /// @dev This function will call with value = 0, since sending ether - /// for ERC20 contract is not a normal case. + /// to ERC20 contract is not a normal case. /// @param target The target address to execute the call on. /// @param from The address to transfer tokens from. /// @param to The address to transfer tokens to. From 3b63b0d7d77b852dcb6e684af05dedeaefa94c8d Mon Sep 17 00:00:00 2001 From: sm-stack Date: Thu, 16 Nov 2023 11:11:49 +0900 Subject: [PATCH 07/33] Fix prank overriding issue --- test/plugin/SessionKeyPlugin.t.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/test/plugin/SessionKeyPlugin.t.sol b/test/plugin/SessionKeyPlugin.t.sol index f8393b5e..a6f3c25b 100644 --- a/test/plugin/SessionKeyPlugin.t.sol +++ b/test/plugin/SessionKeyPlugin.t.sol @@ -147,6 +147,7 @@ contract SessionKeyPluginTest is Test { assertEq(_after, 0); assertEq(_until, 2); + vm.stopPrank(); } function test_sessionKey_userOp() public { From e097c2109bcf2035648e3a306c046408fd4760c0 Mon Sep 17 00:00:00 2001 From: sm-stack Date: Fri, 17 Nov 2023 09:11:21 +0900 Subject: [PATCH 08/33] Fix lint --- src/plugins/session-key/TokenSessionKeyPlugin.sol | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/plugins/session-key/TokenSessionKeyPlugin.sol b/src/plugins/session-key/TokenSessionKeyPlugin.sol index 30bae99b..d395a31c 100644 --- a/src/plugins/session-key/TokenSessionKeyPlugin.sol +++ b/src/plugins/session-key/TokenSessionKeyPlugin.sol @@ -1,5 +1,3 @@ - - // SPDX-License-Identifier: GPL-3.0 pragma solidity ^0.8.19; From d3ad5a78460effa56bc64c3635e253ad45566377 Mon Sep 17 00:00:00 2001 From: sm-stack Date: Tue, 21 Nov 2023 15:16:08 +0900 Subject: [PATCH 09/33] Fix lint --- .../session-key/BaseSessionKeyPlugin.sol | 18 ++- .../session-key/TokenSessionKeyPlugin.sol | 23 ++-- .../interfaces/ISessionKeyPlugin.sol | 12 +- .../interfaces/ITokenSessionKeyPlugin.sol | 9 +- test/plugin/SessionKeyPlugin.t.sol | 128 ++++++++---------- 5 files changed, 98 insertions(+), 92 deletions(-) diff --git a/src/plugins/session-key/BaseSessionKeyPlugin.sol b/src/plugins/session-key/BaseSessionKeyPlugin.sol index d47ab4fe..54c02afb 100644 --- a/src/plugins/session-key/BaseSessionKeyPlugin.sol +++ b/src/plugins/session-key/BaseSessionKeyPlugin.sol @@ -23,9 +23,9 @@ import {SingleOwnerPlugin} from "../owner/SingleOwnerPlugin.sol"; /// own a modular account. /// This base session key plugin acts as a 'parent plugin' for all specific session /// keys. Using dependency, this plugin can be thought as a parent contract that stores -/// session key duration information, and validation functions for session keys. All +/// session key duration information, and validation functions for session keys. All /// logics for session keys will be implemented in child plugins. -/// It allows for session key owners to access MSCA both through user operation and +/// It allows for session key owners to access MSCA both through user operation and /// runtime, with its own validation functions. /// Also, it has a dependency on SingleOwnerPlugin, to make sure that only the owner of /// the MSCA can add or remove session keys. @@ -64,7 +64,11 @@ contract BaseSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ /// @inheritdoc ISessionKeyPlugin - function getSessionDuration(address account, address tempOwner, bytes4 allowedSelector) external view returns (uint48 _after, uint48 _until) { + function getSessionDuration(address account, address tempOwner, bytes4 allowedSelector) + external + view + returns (uint48 _after, uint48 _until) + { (_after, _until) = _decode(_sessionInfo[account][tempOwner][allowedSelector]); } @@ -151,7 +155,7 @@ contract BaseSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { executionSelector: this.removeTemporaryOwner.selector, associatedFunction: ownerUserOpValidationFunction }); - + ManifestFunction memory ownerOrSelfRuntimeValidationFunction = ManifestFunction({ functionType: ManifestAssociatedFunctionType.DEPENDENCY, functionId: 0, // Unused. @@ -204,7 +208,11 @@ contract BaseSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { } } - function _packValidationData(bool sigFailed, uint48 validUntil, uint48 validAfter) internal pure returns (uint256) { + function _packValidationData(bool sigFailed, uint48 validUntil, uint48 validAfter) + internal + pure + returns (uint256) + { return (sigFailed ? 1 : 0) | (uint256(validUntil) << 160) | (uint256(validAfter) << (160 + 48)); } } diff --git a/src/plugins/session-key/TokenSessionKeyPlugin.sol b/src/plugins/session-key/TokenSessionKeyPlugin.sol index d395a31c..c646bdb5 100644 --- a/src/plugins/session-key/TokenSessionKeyPlugin.sol +++ b/src/plugins/session-key/TokenSessionKeyPlugin.sol @@ -17,35 +17,37 @@ import {IPluginExecutor} from "../../interfaces/IPluginExecutor.sol"; /// @title Token Session Key Plugin /// @author Decipher ERC-6900 Team -/// @notice This plugin acts as a 'child plugin' for BaseSessionKeyPlugin. +/// @notice This plugin acts as a 'child plugin' for BaseSessionKeyPlugin. /// It implements the logic for session keys that are allowed to call ERC20 /// transferFrom function. It allows for session key owners to access MSCA /// with `transferFromSessionKey` function, which calls `executeFromPluginExternal` /// function in PluginExecutor contract. /// /// The target ERC20 contract and the selector for transferFrom function are hardcoded -/// in this plugin, since the pluginManifest function requires the information of -/// permitted external calls not to be changed in the future. For other child session -/// key plugins, there can be a set of permitted external calls according to the +/// in this plugin, since the pluginManifest function requires the information of +/// permitted external calls not to be changed in the future. For other child session +/// key plugins, there can be a set of permitted external calls according to the /// specific needs. contract TokenSessionKeyPlugin is BasePlugin, ITokenSessionKeyPlugin { - string public constant NAME = "Token Session Key Plugin"; string public constant VERSION = "1.0.0"; string public constant AUTHOR = "Decipher ERC-6900 Team"; // Mock address of target ERC20 contract address public constant TARGET_ERC20_CONTRACT = 0xdeaDDeADDEaDdeaDdEAddEADDEAdDeadDEADDEaD; - bytes4 public constant TRANSFERFROM_SELECTOR = bytes4(keccak256(bytes("transferFrom(address,address,uint256)"))); + bytes4 public constant TRANSFERFROM_SELECTOR = + bytes4(keccak256(bytes("transferFrom(address,address,uint256)"))); // ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ // ┃ Execution functions ┃ // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ /// @inheritdoc ITokenSessionKeyPlugin - function transferFromSessionKey(address target, address from, address to, uint256 amount) external - returns (bytes memory returnData) { + function transferFromSessionKey(address target, address from, address to, uint256 amount) + external + returns (bytes memory returnData) + { bytes memory data = abi.encodeWithSelector(TRANSFERFROM_SELECTOR, from, to, amount); returnData = IPluginExecutor(msg.sender).executeFromPluginExternal(target, 0, data); } @@ -53,7 +55,7 @@ contract TokenSessionKeyPlugin is BasePlugin, ITokenSessionKeyPlugin { // ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ // ┃ Plugin interface functions ┃ // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ - + /// @inheritdoc BasePlugin function onInstall(bytes calldata data) external override {} @@ -72,7 +74,8 @@ contract TokenSessionKeyPlugin is BasePlugin, ITokenSessionKeyPlugin { ownerPermissions[0] = "Allow Token Operation By Session Key"; manifest.executionFunctions = new ManifestExecutionFunction[](1); - manifest.executionFunctions[0] = ManifestExecutionFunction(this.transferFromSessionKey.selector, ownerPermissions); + manifest.executionFunctions[0] = + ManifestExecutionFunction(this.transferFromSessionKey.selector, ownerPermissions); ManifestFunction memory tempOwnerUserOpValidationFunction = ManifestFunction({ functionType: ManifestAssociatedFunctionType.DEPENDENCY, diff --git a/src/plugins/session-key/interfaces/ISessionKeyPlugin.sol b/src/plugins/session-key/interfaces/ISessionKeyPlugin.sol index db0f7bc9..2e219340 100644 --- a/src/plugins/session-key/interfaces/ISessionKeyPlugin.sol +++ b/src/plugins/session-key/interfaces/ISessionKeyPlugin.sol @@ -8,14 +8,16 @@ interface ISessionKeyPlugin { RUNTIME_VALIDATION_TEMPORARY_OWNER, USER_OP_VALIDATION_TEMPORARY_OWNER } - + /// @notice This event is emitted when a temporary owner is added to the account. /// @param account The account whose temporary owner is updated. /// @param tempOwner The address of the temporary owner. /// @param selector The selector of the function that the temporary owner is allowed to call. /// @param _after The time after which the owner is valid. /// @param _until The time until which the owner is valid. - event TemporaryOwnerAdded(address indexed account, address indexed tempOwner, bytes4 selector, uint48 _after, uint48 _until); + event TemporaryOwnerAdded( + address indexed account, address indexed tempOwner, bytes4 selector, uint48 _after, uint48 _until + ); /// @notice This event is emitted when a temporary owner is removed from the account. /// @param account The account whose temporary owner is updated. @@ -46,6 +48,8 @@ interface ISessionKeyPlugin { /// @param account The account to get session data for. /// @param tempOwner The address of the temporary owner. /// @param allowedSelector The selector of the function that the temporary owner is allowed to call. - function getSessionDuration(address account, address tempOwner, bytes4 allowedSelector) external view - returns (uint48 _after, uint48 _until); + function getSessionDuration(address account, address tempOwner, bytes4 allowedSelector) + external + view + returns (uint48 _after, uint48 _until); } diff --git a/src/plugins/session-key/interfaces/ITokenSessionKeyPlugin.sol b/src/plugins/session-key/interfaces/ITokenSessionKeyPlugin.sol index d395bb86..65e64113 100644 --- a/src/plugins/session-key/interfaces/ITokenSessionKeyPlugin.sol +++ b/src/plugins/session-key/interfaces/ITokenSessionKeyPlugin.sol @@ -7,12 +7,13 @@ interface ITokenSessionKeyPlugin { error NotAuthorized(); /// @notice Route call to executeFromPluginExternal at the MSCA. - /// @dev This function will call with value = 0, since sending ether + /// @dev This function will call with value = 0, since sending ether /// to ERC20 contract is not a normal case. /// @param target The target address to execute the call on. /// @param from The address to transfer tokens from. /// @param to The address to transfer tokens to. /// @param amount The amount of tokens to transfer. - function transferFromSessionKey(address target, address from, address to, uint256 amount) external - returns (bytes memory returnData); -} \ No newline at end of file + function transferFromSessionKey(address target, address from, address to, uint256 amount) + external + returns (bytes memory returnData); +} diff --git a/test/plugin/SessionKeyPlugin.t.sol b/test/plugin/SessionKeyPlugin.t.sol index a6f3c25b..077e31db 100644 --- a/test/plugin/SessionKeyPlugin.t.sol +++ b/test/plugin/SessionKeyPlugin.t.sol @@ -49,21 +49,15 @@ contract SessionKeyPluginTest is Test { uint256 public constant CALL_GAS_LIMIT = 150000; uint256 public constant VERIFICATION_GAS_LIMIT = 3600000; - bytes4 public constant TRANSFERFROM_SESSIONKEY_SELECTOR = ITokenSessionKeyPlugin.transferFromSessionKey.selector; + bytes4 public constant TRANSFERFROM_SESSIONKEY_SELECTOR = + ITokenSessionKeyPlugin.transferFromSessionKey.selector; // Event declarations (needed for vm.expectEmit) event UserOperationRevertReason( - bytes32 indexed userOpHash, - address indexed sender, - uint256 nonce, - bytes revertReason + bytes32 indexed userOpHash, address indexed sender, uint256 nonce, bytes revertReason ); event TemporaryOwnerAdded( - address indexed account, - address indexed tempOwner, - bytes4 allowedSelector, - uint48 _after, - uint48 _until + address indexed account, address indexed tempOwner, bytes4 allowedSelector, uint48 _after, uint48 _until ); event TemporaryOwnerRemoved(address indexed account, address indexed tempOwner, bytes4 allowedSelector); @@ -83,7 +77,7 @@ contract SessionKeyPluginTest is Test { mockERC20 = MockERC20(mockEmptyERC20Addr); (owner, ownerKey) = makeAddrAndKey("owner"); - (maliciousOwner, ) = makeAddrAndKey("maliciousOwner"); + (maliciousOwner,) = makeAddrAndKey("maliciousOwner"); (tempOwner, tempOwnerKey) = makeAddrAndKey("tempOwner"); beneficiary = payable(makeAddr("beneficiary")); @@ -100,12 +94,10 @@ contract SessionKeyPluginTest is Test { vm.startPrank(owner); FunctionReference[] memory baseSessionDependency = new FunctionReference[](2); - baseSessionDependency[0] = address(ownerPlugin).pack( - uint8(ISingleOwnerPlugin.FunctionId.USER_OP_VALIDATION_OWNER) - ); - baseSessionDependency[1] = address(ownerPlugin).pack( - uint8(ISingleOwnerPlugin.FunctionId.RUNTIME_VALIDATION_OWNER_OR_SELF) - ); + baseSessionDependency[0] = + address(ownerPlugin).pack(uint8(ISingleOwnerPlugin.FunctionId.USER_OP_VALIDATION_OWNER)); + baseSessionDependency[1] = + address(ownerPlugin).pack(uint8(ISingleOwnerPlugin.FunctionId.RUNTIME_VALIDATION_OWNER_OR_SELF)); bytes32 baseSessionKeyManifestHash = keccak256(abi.encode(baseSessionKeyPlugin.pluginManifest())); @@ -124,8 +116,8 @@ contract SessionKeyPluginTest is Test { tokenSessionDependency[1] = address(baseSessionKeyPlugin).pack( uint8(ISessionKeyPlugin.FunctionId.RUNTIME_VALIDATION_TEMPORARY_OWNER) ); - bytes32 tokenSessionKeyManifestHash = - keccak256(abi.encode(tokenSessionKeyPlugin.pluginManifest())); + bytes32 tokenSessionKeyManifestHash = keccak256(abi.encode(tokenSessionKeyPlugin.pluginManifest())); + account.installPlugin({ plugin: address(tokenSessionKeyPlugin), manifestHash: tokenSessionKeyManifestHash, @@ -141,10 +133,10 @@ contract SessionKeyPluginTest is Test { vm.expectEmit(true, true, true, true); emit TemporaryOwnerAdded(address(account), tempOwner, TRANSFERFROM_SESSIONKEY_SELECTOR, 0, 2); baseSessionKeyPlugin.addTemporaryOwner(tempOwner, TRANSFERFROM_SESSIONKEY_SELECTOR, 0, 2); - - (uint48 _after, uint48 _until) = + + (uint48 _after, uint48 _until) = baseSessionKeyPlugin.getSessionDuration(address(account), tempOwner, TRANSFERFROM_SESSIONKEY_SELECTOR); - + assertEq(_after, 0); assertEq(_until, 2); vm.stopPrank(); @@ -153,11 +145,12 @@ contract SessionKeyPluginTest is Test { function test_sessionKey_userOp() public { UserOperation[] memory userOps = new UserOperation[](1); - (, UserOperation memory userOp) = _constructUserOp(address(mockERC20), address(account), beneficiary, 1 ether); + (, UserOperation memory userOp) = + _constructUserOp(address(mockERC20), address(account), beneficiary, 1 ether); userOps[0] = userOp; - + entryPoint.handleOps(userOps, beneficiary); - + assertEq(mockERC20.balanceOf(address(account)), 0); assertEq(mockERC20.balanceOf(beneficiary), 1 ether); } @@ -174,14 +167,14 @@ contract SessionKeyPluginTest is Test { function test_sessionKey_removeTempOwner() public { vm.startPrank(address(account)); - + vm.expectEmit(true, true, true, true); emit TemporaryOwnerRemoved(address(account), tempOwner, TRANSFERFROM_SESSIONKEY_SELECTOR); baseSessionKeyPlugin.removeTemporaryOwner(tempOwner, TRANSFERFROM_SESSIONKEY_SELECTOR); - + vm.stopPrank(); - - (uint48 _after, uint48 _until) = + + (uint48 _after, uint48 _until) = baseSessionKeyPlugin.getSessionDuration(address(account), tempOwner, TRANSFERFROM_SESSIONKEY_SELECTOR); assertEq(_after, 0); assertEq(_until, 0); @@ -189,15 +182,15 @@ contract SessionKeyPluginTest is Test { // Check if tempOwner can still send user operations vm.startPrank(address(tempOwner)); - bytes memory revertReason = abi.encodeWithSelector( - BasePlugin.NotImplemented.selector + bytes memory revertReason = abi.encodeWithSelector(BasePlugin.NotImplemented.selector); + vm.expectRevert( + abi.encodeWithSelector( + UpgradeableModularAccount.RuntimeValidationFunctionReverted.selector, + address(baseSessionKeyPlugin), + ISessionKeyPlugin.FunctionId.RUNTIME_VALIDATION_TEMPORARY_OWNER, + revertReason + ) ); - vm.expectRevert(abi.encodeWithSelector( - UpgradeableModularAccount.RuntimeValidationFunctionReverted.selector, - address(baseSessionKeyPlugin), - ISessionKeyPlugin.FunctionId.RUNTIME_VALIDATION_TEMPORARY_OWNER, - revertReason - )); TokenSessionKeyPlugin(address(account)).transferFromSessionKey( address(mockERC20), address(account), beneficiary, 1 ether ); @@ -207,15 +200,12 @@ contract SessionKeyPluginTest is Test { address wrongERC20Contract = makeAddr("wrongERC20Contract"); (bytes32 userOpHash, UserOperation memory userOp) = _constructUserOp(address(wrongERC20Contract), address(account), beneficiary, 1 ether); - + UserOperation[] memory userOps = new UserOperation[](1); userOps[0] = userOp; bytes memory revertCallData = abi.encodeWithSelector( - tokenSessionKeyPlugin.TRANSFERFROM_SELECTOR(), - address(account), - beneficiary, - 1 ether + tokenSessionKeyPlugin.TRANSFERFROM_SELECTOR(), address(account), beneficiary, 1 ether ); bytes memory revertReason = abi.encodeWithSelector( UpgradeableModularAccount.ExecFromPluginExternalNotPermitted.selector, @@ -226,22 +216,22 @@ contract SessionKeyPluginTest is Test { ); vm.expectEmit(true, true, true, true); emit UserOperationRevertReason(userOpHash, address(account), 0, revertReason); - + entryPoint.handleOps(userOps, beneficiary); } function test_sessionKey_unregisteredTempOwnerFails() public { vm.prank(address(maliciousOwner)); - bytes memory revertReason = abi.encodeWithSelector( - BasePlugin.NotImplemented.selector + bytes memory revertReason = abi.encodeWithSelector(BasePlugin.NotImplemented.selector); + + vm.expectRevert( + abi.encodeWithSelector( + UpgradeableModularAccount.RuntimeValidationFunctionReverted.selector, + address(baseSessionKeyPlugin), + ISessionKeyPlugin.FunctionId.RUNTIME_VALIDATION_TEMPORARY_OWNER, + revertReason + ) ); - - vm.expectRevert(abi.encodeWithSelector( - UpgradeableModularAccount.RuntimeValidationFunctionReverted.selector, - address(baseSessionKeyPlugin), - ISessionKeyPlugin.FunctionId.RUNTIME_VALIDATION_TEMPORARY_OWNER, - revertReason - )); TokenSessionKeyPlugin(address(account)).transferFromSessionKey( address(mockERC20), address(account), beneficiary, 1 ether ); @@ -253,28 +243,29 @@ contract SessionKeyPluginTest is Test { vm.startPrank(address(tempOwner)); - bytes memory revertReason = abi.encodeWithSelector( - ISessionKeyPlugin.WrongTimeRangeForSession.selector - ); + bytes memory revertReason = abi.encodeWithSelector(ISessionKeyPlugin.WrongTimeRangeForSession.selector); - vm.expectRevert(abi.encodeWithSelector( - UpgradeableModularAccount.RuntimeValidationFunctionReverted.selector, - address(baseSessionKeyPlugin), - ISessionKeyPlugin.FunctionId.RUNTIME_VALIDATION_TEMPORARY_OWNER, - revertReason - )); + vm.expectRevert( + abi.encodeWithSelector( + UpgradeableModularAccount.RuntimeValidationFunctionReverted.selector, + address(baseSessionKeyPlugin), + ISessionKeyPlugin.FunctionId.RUNTIME_VALIDATION_TEMPORARY_OWNER, + revertReason + ) + ); TokenSessionKeyPlugin(address(account)).transferFromSessionKey( address(mockERC20), address(account), beneficiary, 1 ether - ); + ); } // Internal Function - function _constructUserOp(address targetContract, address from, address to, uint256 amount) internal view - returns (bytes32, UserOperation memory) { - bytes memory userOpCallData = abi.encodeCall( - TokenSessionKeyPlugin.transferFromSessionKey, - (targetContract, from, to, amount) - ); + function _constructUserOp(address targetContract, address from, address to, uint256 amount) + internal + view + returns (bytes32, UserOperation memory) + { + bytes memory userOpCallData = + abi.encodeCall(TokenSessionKeyPlugin.transferFromSessionKey, (targetContract, from, to, amount)); UserOperation memory userOp = UserOperation({ sender: address(account), @@ -298,4 +289,3 @@ contract SessionKeyPluginTest is Test { return (userOpHash, userOp); } } - From f2ad9a6b849746330c68ea92e60a200d367f9def Mon Sep 17 00:00:00 2001 From: sm-stack Date: Sun, 3 Dec 2023 00:45:26 +0900 Subject: [PATCH 10/33] Fix plugin codes to be compatible with the spec update --- .../session-key/BaseSessionKeyPlugin.sol | 46 ++++++++++++------- .../session-key/TokenSessionKeyPlugin.sol | 35 +++++++++----- 2 files changed, 54 insertions(+), 27 deletions(-) diff --git a/src/plugins/session-key/BaseSessionKeyPlugin.sol b/src/plugins/session-key/BaseSessionKeyPlugin.sol index 54c02afb..66dbd89b 100644 --- a/src/plugins/session-key/BaseSessionKeyPlugin.sol +++ b/src/plugins/session-key/BaseSessionKeyPlugin.sol @@ -9,8 +9,8 @@ import { ManifestAssociatedFunctionType, ManifestAssociatedFunction, PluginManifest, - ManifestExecutionFunction, - ManifestExternalCallPermission + PluginMetadata, + SelectorPermission } from "../../interfaces/IPlugin.sol"; import {BasePlugin} from "../BasePlugin.sol"; import {ISessionKeyPlugin} from "./interfaces/ISessionKeyPlugin.sol"; @@ -126,20 +126,10 @@ contract BaseSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { function pluginManifest() external pure override returns (PluginManifest memory) { PluginManifest memory manifest; - manifest.name = NAME; - manifest.version = VERSION; - manifest.author = AUTHOR; - - string[] memory ownerPermissions = new string[](1); - ownerPermissions[0] = "Allow Temporary Ownership"; - - manifest.executionFunctions = new ManifestExecutionFunction[](3); - manifest.executionFunctions[0] = - ManifestExecutionFunction(this.addTemporaryOwner.selector, ownerPermissions); - manifest.executionFunctions[1] = - ManifestExecutionFunction(this.removeTemporaryOwner.selector, ownerPermissions); - manifest.executionFunctions[2] = - ManifestExecutionFunction(this.getSessionDuration.selector, new string[](0)); + manifest.executionFunctions = new bytes4[](3); + manifest.executionFunctions[0] = this.addTemporaryOwner.selector; + manifest.executionFunctions[1] = this.removeTemporaryOwner.selector; + manifest.executionFunctions[2] = this.getSessionDuration.selector; ManifestFunction memory ownerUserOpValidationFunction = ManifestFunction({ functionType: ManifestAssociatedFunctionType.DEPENDENCY, @@ -188,6 +178,30 @@ contract BaseSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { return manifest; } + /// @inheritdoc BasePlugin + function pluginMetadata() external pure virtual override returns (PluginMetadata memory) { + PluginMetadata memory metadata; + metadata.name = NAME; + metadata.version = VERSION; + metadata.author = AUTHOR; + + // Permission strings + string memory modifySessionKeyPermission = "Modify Session Key"; + + // Permission descriptions + metadata.permissionDescriptors = new SelectorPermission[](2); + metadata.permissionDescriptors[0] = SelectorPermission({ + functionSelector: this.addTemporaryOwner.selector, + permissionDescription: modifySessionKeyPermission + }); + metadata.permissionDescriptors[1] = SelectorPermission({ + functionSelector: this.removeTemporaryOwner.selector, + permissionDescription: modifySessionKeyPermission + }); + + return metadata; + } + // ┏━━━━━━━━━━━━━━━┓ // ┃ EIP-165 ┃ // ┗━━━━━━━━━━━━━━━┛ diff --git a/src/plugins/session-key/TokenSessionKeyPlugin.sol b/src/plugins/session-key/TokenSessionKeyPlugin.sol index c646bdb5..085316c4 100644 --- a/src/plugins/session-key/TokenSessionKeyPlugin.sol +++ b/src/plugins/session-key/TokenSessionKeyPlugin.sol @@ -6,7 +6,8 @@ import { ManifestAssociatedFunctionType, ManifestAssociatedFunction, PluginManifest, - ManifestExecutionFunction, + PluginMetadata, + SelectorPermission, ManifestExternalCallPermission } from "../../interfaces/IPlugin.sol"; import {BasePlugin} from "../BasePlugin.sol"; @@ -66,16 +67,8 @@ contract TokenSessionKeyPlugin is BasePlugin, ITokenSessionKeyPlugin { function pluginManifest() external pure override returns (PluginManifest memory) { PluginManifest memory manifest; - manifest.name = NAME; - manifest.version = VERSION; - manifest.author = AUTHOR; - - string[] memory ownerPermissions = new string[](1); - ownerPermissions[0] = "Allow Token Operation By Session Key"; - - manifest.executionFunctions = new ManifestExecutionFunction[](1); - manifest.executionFunctions[0] = - ManifestExecutionFunction(this.transferFromSessionKey.selector, ownerPermissions); + manifest.executionFunctions = new bytes4[](1); + manifest.executionFunctions[0] = this.transferFromSessionKey.selector; ManifestFunction memory tempOwnerUserOpValidationFunction = ManifestFunction({ functionType: ManifestAssociatedFunctionType.DEPENDENCY, @@ -121,6 +114,26 @@ contract TokenSessionKeyPlugin is BasePlugin, ITokenSessionKeyPlugin { return manifest; } + /// @inheritdoc BasePlugin + function pluginMetadata() external pure virtual override returns (PluginMetadata memory) { + PluginMetadata memory metadata; + metadata.name = NAME; + metadata.version = VERSION; + metadata.author = AUTHOR; + + // Permission strings + string memory sessionKeyTransferPermission = "Allow Token Transfer Operation By Session Key"; + + // Permission descriptions + metadata.permissionDescriptors = new SelectorPermission[](1); + metadata.permissionDescriptors[0] = SelectorPermission({ + functionSelector: this.transferFromSessionKey.selector, + permissionDescription: sessionKeyTransferPermission + }); + + return metadata; + } + // ┏━━━━━━━━━━━━━━━┓ // ┃ EIP-165 ┃ // ┗━━━━━━━━━━━━━━━┛ From f61b08596dafb6a42ec406638f1c1ca41a96dfb7 Mon Sep 17 00:00:00 2001 From: sm-stack Date: Wed, 20 Dec 2023 00:10:33 +0900 Subject: [PATCH 11/33] Fix nits --- src/plugins/session-key/BaseSessionKeyPlugin.sol | 7 +++---- src/plugins/session-key/TokenSessionKeyPlugin.sol | 1 - 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/plugins/session-key/BaseSessionKeyPlugin.sol b/src/plugins/session-key/BaseSessionKeyPlugin.sol index 66dbd89b..9582cf89 100644 --- a/src/plugins/session-key/BaseSessionKeyPlugin.sol +++ b/src/plugins/session-key/BaseSessionKeyPlugin.sol @@ -29,7 +29,6 @@ import {SingleOwnerPlugin} from "../owner/SingleOwnerPlugin.sol"; /// runtime, with its own validation functions. /// Also, it has a dependency on SingleOwnerPlugin, to make sure that only the owner of /// the MSCA can add or remove session keys. - contract BaseSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { using ECDSA for bytes32; @@ -89,8 +88,8 @@ contract BaseSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { override returns (uint256) { - (address signer,) = (userOpHash.toEthSignedMessageHash()).tryRecover(userOp.signature); if (functionId == uint8(FunctionId.USER_OP_VALIDATION_TEMPORARY_OWNER)) { + (address signer,) = (userOpHash.toEthSignedMessageHash()).tryRecover(userOp.signature); bytes4 selector = bytes4(userOp.callData[0:4]); bytes memory duration = _sessionInfo[userOp.sender][signer][selector]; if (duration.length != 0) { @@ -99,7 +98,7 @@ contract BaseSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { return _packValidationData(false, _until, _after); } } - revert NotImplemented(); + return _packValidationData(true, 0, 0); } /// @inheritdoc BasePlugin @@ -116,7 +115,7 @@ contract BaseSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { if (block.timestamp < _after || block.timestamp > _until) { revert WrongTimeRangeForSession(); } - return; + revert NotAuthorized(); } } revert NotImplemented(); diff --git a/src/plugins/session-key/TokenSessionKeyPlugin.sol b/src/plugins/session-key/TokenSessionKeyPlugin.sol index 085316c4..640efa6e 100644 --- a/src/plugins/session-key/TokenSessionKeyPlugin.sol +++ b/src/plugins/session-key/TokenSessionKeyPlugin.sol @@ -23,7 +23,6 @@ import {IPluginExecutor} from "../../interfaces/IPluginExecutor.sol"; /// transferFrom function. It allows for session key owners to access MSCA /// with `transferFromSessionKey` function, which calls `executeFromPluginExternal` /// function in PluginExecutor contract. -/// /// The target ERC20 contract and the selector for transferFrom function are hardcoded /// in this plugin, since the pluginManifest function requires the information of /// permitted external calls not to be changed in the future. For other child session From 3e5d7fb4185db70cd62eac2f4fb3d74ff98a5771 Mon Sep 17 00:00:00 2001 From: sm-stack Date: Wed, 20 Dec 2023 00:43:18 +0900 Subject: [PATCH 12/33] Fix functionId --- src/plugins/session-key/BaseSessionKeyPlugin.sol | 2 +- src/plugins/session-key/TokenSessionKeyPlugin.sol | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plugins/session-key/BaseSessionKeyPlugin.sol b/src/plugins/session-key/BaseSessionKeyPlugin.sol index 9582cf89..6d0c30ac 100644 --- a/src/plugins/session-key/BaseSessionKeyPlugin.sol +++ b/src/plugins/session-key/BaseSessionKeyPlugin.sol @@ -115,7 +115,7 @@ contract BaseSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { if (block.timestamp < _after || block.timestamp > _until) { revert WrongTimeRangeForSession(); } - revert NotAuthorized(); + return; } } revert NotImplemented(); diff --git a/src/plugins/session-key/TokenSessionKeyPlugin.sol b/src/plugins/session-key/TokenSessionKeyPlugin.sol index 640efa6e..4bb47891 100644 --- a/src/plugins/session-key/TokenSessionKeyPlugin.sol +++ b/src/plugins/session-key/TokenSessionKeyPlugin.sol @@ -71,7 +71,7 @@ contract TokenSessionKeyPlugin is BasePlugin, ITokenSessionKeyPlugin { ManifestFunction memory tempOwnerUserOpValidationFunction = ManifestFunction({ functionType: ManifestAssociatedFunctionType.DEPENDENCY, - functionId: 0, // Unused + functionId: uint8(ISessionKeyPlugin.FunctionId.RUNTIME_VALIDATION_TEMPORARY_OWNER), dependencyIndex: 0 // Used as first index }); ManifestFunction memory tempOwnerRuntimeValidationFunction = ManifestFunction({ From 65e6d1a8610e59e8bb8f7cf2e9377635f557b2ef Mon Sep 17 00:00:00 2001 From: sm-stack Date: Mon, 25 Dec 2023 16:45:12 +0900 Subject: [PATCH 13/33] Fix session key test corresponding to code changes --- test/plugin/SessionKeyPlugin.t.sol | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/test/plugin/SessionKeyPlugin.t.sol b/test/plugin/SessionKeyPlugin.t.sol index 077e31db..a5f0ef49 100644 --- a/test/plugin/SessionKeyPlugin.t.sol +++ b/test/plugin/SessionKeyPlugin.t.sol @@ -44,6 +44,8 @@ contract SessionKeyPluginTest is Test { address public tempOwner; uint256 public tempOwnerKey; + address public target; + address payable public beneficiary; uint256 public constant CALL_GAS_LIMIT = 150000; @@ -79,6 +81,7 @@ contract SessionKeyPluginTest is Test { (owner, ownerKey) = makeAddrAndKey("owner"); (maliciousOwner,) = makeAddrAndKey("maliciousOwner"); (tempOwner, tempOwnerKey) = makeAddrAndKey("tempOwner"); + target = makeAddr("target"); beneficiary = payable(makeAddr("beneficiary")); vm.deal(beneficiary, 1 wei); @@ -146,23 +149,23 @@ contract SessionKeyPluginTest is Test { UserOperation[] memory userOps = new UserOperation[](1); (, UserOperation memory userOp) = - _constructUserOp(address(mockERC20), address(account), beneficiary, 1 ether); + _constructUserOp(address(mockERC20), address(account), target, 1 ether); userOps[0] = userOp; entryPoint.handleOps(userOps, beneficiary); assertEq(mockERC20.balanceOf(address(account)), 0); - assertEq(mockERC20.balanceOf(beneficiary), 1 ether); + assertEq(mockERC20.balanceOf(target), 1 ether); } function test_sessionKey_runtime() public { vm.prank(address(tempOwner)); TokenSessionKeyPlugin(address(account)).transferFromSessionKey( - address(mockERC20), address(account), beneficiary, 1 ether + address(mockERC20), address(account), target, 1 ether ); assertEq(mockERC20.balanceOf(address(account)), 0); - assertEq(mockERC20.balanceOf(beneficiary), 1 ether); + assertEq(mockERC20.balanceOf(target), 1 ether); } function test_sessionKey_removeTempOwner() public { @@ -182,7 +185,7 @@ contract SessionKeyPluginTest is Test { // Check if tempOwner can still send user operations vm.startPrank(address(tempOwner)); - bytes memory revertReason = abi.encodeWithSelector(BasePlugin.NotImplemented.selector); + bytes memory revertReason = abi.encodeWithSelector(ISessionKeyPlugin.NotAuthorized.selector); vm.expectRevert( abi.encodeWithSelector( UpgradeableModularAccount.RuntimeValidationFunctionReverted.selector, @@ -192,20 +195,20 @@ contract SessionKeyPluginTest is Test { ) ); TokenSessionKeyPlugin(address(account)).transferFromSessionKey( - address(mockERC20), address(account), beneficiary, 1 ether + address(mockERC20), address(account), target, 1 ether ); } function test_sessionKey_invalidContractFails() public { address wrongERC20Contract = makeAddr("wrongERC20Contract"); (bytes32 userOpHash, UserOperation memory userOp) = - _constructUserOp(address(wrongERC20Contract), address(account), beneficiary, 1 ether); + _constructUserOp(address(wrongERC20Contract), address(account), target, 1 ether); UserOperation[] memory userOps = new UserOperation[](1); userOps[0] = userOp; bytes memory revertCallData = abi.encodeWithSelector( - tokenSessionKeyPlugin.TRANSFERFROM_SELECTOR(), address(account), beneficiary, 1 ether + tokenSessionKeyPlugin.TRANSFERFROM_SELECTOR(), address(account), target, 1 ether ); bytes memory revertReason = abi.encodeWithSelector( UpgradeableModularAccount.ExecFromPluginExternalNotPermitted.selector, @@ -222,7 +225,7 @@ contract SessionKeyPluginTest is Test { function test_sessionKey_unregisteredTempOwnerFails() public { vm.prank(address(maliciousOwner)); - bytes memory revertReason = abi.encodeWithSelector(BasePlugin.NotImplemented.selector); + bytes memory revertReason = abi.encodeWithSelector(ISessionKeyPlugin.NotAuthorized.selector); vm.expectRevert( abi.encodeWithSelector( @@ -233,7 +236,7 @@ contract SessionKeyPluginTest is Test { ) ); TokenSessionKeyPlugin(address(account)).transferFromSessionKey( - address(mockERC20), address(account), beneficiary, 1 ether + address(mockERC20), address(account), target, 1 ether ); } @@ -254,7 +257,7 @@ contract SessionKeyPluginTest is Test { ) ); TokenSessionKeyPlugin(address(account)).transferFromSessionKey( - address(mockERC20), address(account), beneficiary, 1 ether + address(mockERC20), address(account), target, 1 ether ); } From f2a74ad7d58e85ac4f2dc1f4c3518ed223df039a Mon Sep 17 00:00:00 2001 From: sm-stack Date: Mon, 25 Dec 2023 16:45:50 +0900 Subject: [PATCH 14/33] Add batch owner operations and additional error handling logics --- pnpm-lock.yaml | 474 ------------------ .../session-key/BaseSessionKeyPlugin.sol | 82 ++- .../session-key/TokenSessionKeyPlugin.sol | 6 +- .../interfaces/ISessionKeyPlugin.sol | 37 ++ .../libraries/PluginStorageLib.sol | 254 ++++++++++ 5 files changed, 366 insertions(+), 487 deletions(-) delete mode 100644 pnpm-lock.yaml create mode 100644 src/plugins/session-key/libraries/PluginStorageLib.sol diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml deleted file mode 100644 index 393efc6e..00000000 --- a/pnpm-lock.yaml +++ /dev/null @@ -1,474 +0,0 @@ -lockfileVersion: '6.0' - -settings: - autoInstallPeers: true - excludeLinksFromLockfile: false - -devDependencies: - pnpm: - specifier: ^8.7.5 - version: 8.7.5 - solhint: - specifier: ^3.6.2 - version: 3.6.2 - -packages: - - /@babel/code-frame@7.22.13: - resolution: {integrity: sha512-XktuhWlJ5g+3TJXc5upd9Ks1HutSArik6jf2eAjYFyIOf4ej3RN+184cZbzDvbPnuTJIUhPKKJE3cIsYTiAT3w==} - engines: {node: '>=6.9.0'} - dependencies: - '@babel/highlight': 7.22.13 - chalk: 2.4.2 - dev: true - - /@babel/helper-validator-identifier@7.22.15: - resolution: {integrity: sha512-4E/F9IIEi8WR94324mbDUMo074YTheJmd7eZF5vITTeYchqAi6sYXRLHUVsmkdmY4QjfKTcB2jB7dVP3NaBElQ==} - engines: {node: '>=6.9.0'} - dev: true - - /@babel/highlight@7.22.13: - resolution: {integrity: sha512-C/BaXcnnvBCmHTpz/VGZ8jgtE2aYlW4hxDhseJAWZb7gqGM/qtCK6iZUb0TyKFf7BOUsBH7Q7fkRsDRhg1XklQ==} - engines: {node: '>=6.9.0'} - dependencies: - '@babel/helper-validator-identifier': 7.22.15 - chalk: 2.4.2 - js-tokens: 4.0.0 - dev: true - - /@solidity-parser/parser@0.16.1: - resolution: {integrity: sha512-PdhRFNhbTtu3x8Axm0uYpqOy/lODYQK+MlYSgqIsq2L8SFYEHJPHNUiOTAJbDGzNjjr1/n9AcIayxafR/fWmYw==} - dependencies: - antlr4ts: 0.5.0-alpha.4 - dev: true - - /ajv@6.12.6: - resolution: {integrity: sha512-j3fVLgvTo527anyYyJOGTYJbG+vnnQYvE0m5mmkc1TK+nxAppkCLMIL0aZ4dblVCNoGShhm+kzE4ZUykBoMg4g==} - dependencies: - fast-deep-equal: 3.1.3 - fast-json-stable-stringify: 2.1.0 - json-schema-traverse: 0.4.1 - uri-js: 4.4.1 - dev: true - - /ajv@8.12.0: - resolution: {integrity: sha512-sRu1kpcO9yLtYxBKvqfTeh9KzZEwO3STyX1HT+4CaDzC6HpTGYhIhPIzj9XuKU7KYDwnaeh5hcOwjy1QuJzBPA==} - dependencies: - fast-deep-equal: 3.1.3 - json-schema-traverse: 1.0.0 - require-from-string: 2.0.2 - uri-js: 4.4.1 - dev: true - - /ansi-regex@5.0.1: - resolution: {integrity: sha512-quJQXlTSUGL2LH9SUXo8VwsY4soanhgo6LNSm84E1LBcE8s3O0wpdiRzyR9z/ZZJMlMWv37qOOb9pdJlMUEKFQ==} - engines: {node: '>=8'} - dev: true - - /ansi-styles@3.2.1: - resolution: {integrity: sha512-VT0ZI6kZRdTh8YyJw3SMbYm/u+NqfsAxEpWO0Pf9sq8/e94WxxOpPKx9FR1FlyCtOVDNOQ+8ntlqFxiRc+r5qA==} - engines: {node: '>=4'} - dependencies: - color-convert: 1.9.3 - dev: true - - /ansi-styles@4.3.0: - resolution: {integrity: sha512-zbB9rCJAT1rbjiVDb2hqKFHNYLxgtk8NURxZ3IZwD3F6NtxbXZQCnnSi1Lkx+IDohdPlFp222wVALIheZJQSEg==} - engines: {node: '>=8'} - dependencies: - color-convert: 2.0.1 - dev: true - - /antlr4@4.13.1: - resolution: {integrity: sha512-kiXTspaRYvnIArgE97z5YVVf/cDVQABr3abFRR6mE7yesLMkgu4ujuyV/sgxafQ8wgve0DJQUJ38Z8tkgA2izA==} - engines: {node: '>=16'} - dev: true - - /antlr4ts@0.5.0-alpha.4: - resolution: {integrity: sha512-WPQDt1B74OfPv/IMS2ekXAKkTZIHl88uMetg6q3OTqgFxZ/dxDXI0EWLyZid/1Pe6hTftyg5N7gel5wNAGxXyQ==} - dev: true - - /argparse@2.0.1: - resolution: {integrity: sha512-8+9WqebbFzpX9OR+Wa6O29asIogeRMzcGtAINdpMHHyAg10f05aSFVBbcEqGf/PXw1EjAZ+q2/bEBg3DvurK3Q==} - dev: true - - /ast-parents@0.0.1: - resolution: {integrity: sha512-XHusKxKz3zoYk1ic8Un640joHbFMhbqneyoZfoKnEGtf2ey9Uh/IdpcQplODdO/kENaMIWsD0nJm4+wX3UNLHA==} - dev: true - - /astral-regex@2.0.0: - resolution: {integrity: sha512-Z7tMw1ytTXt5jqMcOP+OQteU1VuNK9Y02uuJtKQ1Sv69jXQKKg5cibLwGJow8yzZP+eAc18EmLGPal0bp36rvQ==} - engines: {node: '>=8'} - dev: true - - /balanced-match@1.0.2: - resolution: {integrity: sha512-3oSeUO0TMV67hN1AmbXsK4yaqU7tjiHlbxRDZOpH0KW9+CeX4bRAaX0Anxt0tx2MrpRpWwQaPwIlISEJhYU5Pw==} - dev: true - - /brace-expansion@2.0.1: - resolution: {integrity: sha512-XnAIvQ8eM+kC6aULx6wuQiwVsnzsi9d3WxzV3FpWTGA19F621kwdbsAcFKXgKUHZWsy+mY6iL1sHTxWEFCytDA==} - dependencies: - balanced-match: 1.0.2 - dev: true - - /callsites@3.1.0: - resolution: {integrity: sha512-P8BjAsXvZS+VIDUI11hHCQEv74YT67YUi5JJFNWIqL235sBmjX4+qx9Muvls5ivyNENctx46xQLQ3aTuE7ssaQ==} - engines: {node: '>=6'} - dev: true - - /chalk@2.4.2: - resolution: {integrity: sha512-Mti+f9lpJNcwF4tWV8/OrTTtF1gZi+f8FqlyAdouralcFWFQWF2+NgCHShjkCb+IFBLq9buZwE1xckQU4peSuQ==} - engines: {node: '>=4'} - dependencies: - ansi-styles: 3.2.1 - escape-string-regexp: 1.0.5 - supports-color: 5.5.0 - dev: true - - /chalk@4.1.2: - resolution: {integrity: sha512-oKnbhFyRIXpUuez8iBMmyEa4nbj4IOQyuhc/wy9kY7/WVPcwIO9VA668Pu8RkO7+0G76SLROeyw9CpQ061i4mA==} - engines: {node: '>=10'} - dependencies: - ansi-styles: 4.3.0 - supports-color: 7.2.0 - dev: true - - /color-convert@1.9.3: - resolution: {integrity: sha512-QfAUtd+vFdAtFQcC8CCyYt1fYWxSqAiK2cSD6zDB8N3cpsEBAvRxp9zOGg6G/SHHJYAT88/az/IuDGALsNVbGg==} - dependencies: - color-name: 1.1.3 - dev: true - - /color-convert@2.0.1: - resolution: {integrity: sha512-RRECPsj7iu/xb5oKYcsFHSppFNnsj/52OVTRKb4zP5onXwVF3zVmmToNcOfGC+CRDpfK/U584fMg38ZHCaElKQ==} - engines: {node: '>=7.0.0'} - dependencies: - color-name: 1.1.4 - dev: true - - /color-name@1.1.3: - resolution: {integrity: sha512-72fSenhMw2HZMTVHeCA9KCmpEIbzWiQsjN+BHcBbS9vr1mtt+vJjPdksIBNUmKAW8TFUDPJK5SUU3QhE9NEXDw==} - dev: true - - /color-name@1.1.4: - resolution: {integrity: sha512-dOy+3AuW3a2wNbZHIuMZpTcgjGuLU/uBL/ubcZF9OXbDo8ff4O8yVp5Bf0efS8uEoYo5q4Fx7dY9OgQGXgAsQA==} - dev: true - - /commander@10.0.1: - resolution: {integrity: sha512-y4Mg2tXshplEbSGzx7amzPwKKOCGuoSRP/CjEdwwk0FOGlUbq6lKuoyDZTNZkmxHdJtp54hdfY/JUrdL7Xfdug==} - engines: {node: '>=14'} - dev: true - - /cosmiconfig@8.3.6: - resolution: {integrity: sha512-kcZ6+W5QzcJ3P1Mt+83OUv/oHFqZHIx8DuxG6eZ5RGMERoLqp4BuGjhHLYGK+Kf5XVkQvqBSmAy/nGWN3qDgEA==} - engines: {node: '>=14'} - peerDependencies: - typescript: '>=4.9.5' - peerDependenciesMeta: - typescript: - optional: true - dependencies: - import-fresh: 3.3.0 - js-yaml: 4.1.0 - parse-json: 5.2.0 - path-type: 4.0.0 - dev: true - - /emoji-regex@8.0.0: - resolution: {integrity: sha512-MSjYzcWNOA0ewAHpz0MxpYFvwg6yjy1NG3xteoqz644VCo/RPgnr1/GGt+ic3iJTzQ8Eu3TdM14SawnVUmGE6A==} - dev: true - - /error-ex@1.3.2: - resolution: {integrity: sha512-7dFHNmqeFSEt2ZBsCriorKnn3Z2pj+fd9kmI6QoWw4//DL+icEBfc0U7qJCisqrTsKTjw4fNFy2pW9OqStD84g==} - dependencies: - is-arrayish: 0.2.1 - dev: true - - /escape-string-regexp@1.0.5: - resolution: {integrity: sha512-vbRorB5FUQWvla16U8R/qgaFIya2qGzwDrNmCZuYKrbdSUMG6I1ZCGQRefkRVhuOkIGVne7BQ35DSfo1qvJqFg==} - engines: {node: '>=0.8.0'} - dev: true - - /fast-deep-equal@3.1.3: - resolution: {integrity: sha512-f3qQ9oQy9j2AhBe/H9VC91wLmKBCCU/gDOnKNAYG5hswO7BLKj09Hc5HYNz9cGI++xlpDCIgDaitVs03ATR84Q==} - dev: true - - /fast-diff@1.3.0: - resolution: {integrity: sha512-VxPP4NqbUjj6MaAOafWeUn2cXWLcCtljklUtZf0Ind4XQ+QPtmA0b18zZy0jIQx+ExRVCR/ZQpBmik5lXshNsw==} - dev: true - - /fast-json-stable-stringify@2.1.0: - resolution: {integrity: sha512-lhd/wF+Lk98HZoTCtlVraHtfh5XYijIjalXck7saUtuanSDyLMxnHhSXEDJqHxD7msR8D0uCmqlkwjCV8xvwHw==} - dev: true - - /fs.realpath@1.0.0: - resolution: {integrity: sha512-OO0pH2lK6a0hZnAdau5ItzHPI6pUlvI7jMVnxUQRtw4owF2wk8lOSabtGDCTP4Ggrg2MbGnWO9X8K1t4+fGMDw==} - dev: true - - /glob@8.1.0: - resolution: {integrity: sha512-r8hpEjiQEYlF2QU0df3dS+nxxSIreXQS1qRhMJM0Q5NDdR386C7jb7Hwwod8Fgiuex+k0GFjgft18yvxm5XoCQ==} - engines: {node: '>=12'} - dependencies: - fs.realpath: 1.0.0 - inflight: 1.0.6 - inherits: 2.0.4 - minimatch: 5.1.6 - once: 1.4.0 - dev: true - - /has-flag@3.0.0: - resolution: {integrity: sha512-sKJf1+ceQBr4SMkvQnBDNDtf4TXpVhVGateu0t918bl30FnbE2m4vNLX+VWe/dpjlb+HugGYzW7uQXH98HPEYw==} - engines: {node: '>=4'} - dev: true - - /has-flag@4.0.0: - resolution: {integrity: sha512-EykJT/Q1KjTWctppgIAgfSO0tKVuZUjhgMr17kqTumMl6Afv3EISleU7qZUzoXDFTAHTDC4NOoG/ZxU3EvlMPQ==} - engines: {node: '>=8'} - dev: true - - /ignore@5.2.4: - resolution: {integrity: sha512-MAb38BcSbH0eHNBxn7ql2NH/kX33OkB3lZ1BNdh7ENeRChHTYsTvWrMubiIAMNS2llXEEgZ1MUOBtXChP3kaFQ==} - engines: {node: '>= 4'} - dev: true - - /import-fresh@3.3.0: - resolution: {integrity: sha512-veYYhQa+D1QBKznvhUHxb8faxlrwUnxseDAbAp457E0wLNio2bOSKnjYDhMj+YiAq61xrMGhQk9iXVk5FzgQMw==} - engines: {node: '>=6'} - dependencies: - parent-module: 1.0.1 - resolve-from: 4.0.0 - dev: true - - /inflight@1.0.6: - resolution: {integrity: sha512-k92I/b08q4wvFscXCLvqfsHCrjrF7yiXsQuIVvVE7N82W3+aqpzuUdBbfhWcy/FZR3/4IgflMgKLOsvPDrGCJA==} - dependencies: - once: 1.4.0 - wrappy: 1.0.2 - dev: true - - /inherits@2.0.4: - resolution: {integrity: sha512-k/vGaX4/Yla3WzyMCvTQOXYeIHvqOKtnqBduzTHpzpQZzAskKMhZ2K+EnBiSM9zGSoIFeMpXKxa4dYeZIQqewQ==} - dev: true - - /is-arrayish@0.2.1: - resolution: {integrity: sha512-zz06S8t0ozoDXMG+ube26zeCTNXcKIPJZJi8hBrF4idCLms4CG9QtK7qBl1boi5ODzFpjswb5JPmHCbMpjaYzg==} - dev: true - - /is-fullwidth-code-point@3.0.0: - resolution: {integrity: sha512-zymm5+u+sCsSWyD9qNaejV3DFvhCKclKdizYaJUuHA83RLjb7nSuGnddCHGv0hk+KY7BMAlsWeK4Ueg6EV6XQg==} - engines: {node: '>=8'} - dev: true - - /js-tokens@4.0.0: - resolution: {integrity: sha512-RdJUflcE3cUzKiMqQgsCu06FPu9UdIJO0beYbPhHN4k6apgJtifcoCtT9bcxOpYBtpD2kCM6Sbzg4CausW/PKQ==} - dev: true - - /js-yaml@4.1.0: - resolution: {integrity: sha512-wpxZs9NoxZaJESJGIZTyDEaYpl0FKSA+FB9aJiyemKhMwkxQg63h4T1KJgUGHpTqPDNRcmmYLugrRjJlBtWvRA==} - hasBin: true - dependencies: - argparse: 2.0.1 - dev: true - - /json-parse-even-better-errors@2.3.1: - resolution: {integrity: sha512-xyFwyhro/JEof6Ghe2iz2NcXoj2sloNsWr/XsERDK/oiPCfaNhl5ONfp+jQdAZRQQ0IJWNzH9zIZF7li91kh2w==} - dev: true - - /json-schema-traverse@0.4.1: - resolution: {integrity: sha512-xbbCH5dCYU5T8LcEhhuh7HJ88HXuW3qsI3Y0zOZFKfZEHcpWiHU/Jxzk629Brsab/mMiHQti9wMP+845RPe3Vg==} - dev: true - - /json-schema-traverse@1.0.0: - resolution: {integrity: sha512-NM8/P9n3XjXhIZn1lLhkFaACTOURQXjWhV4BA/RnOv8xvgqtqpAX9IO4mRQxSx1Rlo4tqzeqb0sOlruaOy3dug==} - dev: true - - /lines-and-columns@1.2.4: - resolution: {integrity: sha512-7ylylesZQ/PV29jhEDl3Ufjo6ZX7gCqJr5F7PKrqc93v7fzSymt1BpwEU8nAUXs8qzzvqhbjhK5QZg6Mt/HkBg==} - dev: true - - /lodash.truncate@4.4.2: - resolution: {integrity: sha512-jttmRe7bRse52OsWIMDLaXxWqRAmtIUccAQ3garviCqJjafXOfNMO0yMfNpdD6zbGaTU0P5Nz7e7gAT6cKmJRw==} - dev: true - - /lodash@4.17.21: - resolution: {integrity: sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg==} - dev: true - - /lru-cache@6.0.0: - resolution: {integrity: sha512-Jo6dJ04CmSjuznwJSS3pUeWmd/H0ffTlkXXgwZi+eq1UCmqQwCh+eLsYOYCwY991i2Fah4h1BEMCx4qThGbsiA==} - engines: {node: '>=10'} - dependencies: - yallist: 4.0.0 - dev: true - - /minimatch@5.1.6: - resolution: {integrity: sha512-lKwV/1brpG6mBUFHtb7NUmtABCb2WZZmm2wNiOA5hAb8VdCS4B3dtMWyvcoViccwAW/COERjXLt0zP1zXUN26g==} - engines: {node: '>=10'} - dependencies: - brace-expansion: 2.0.1 - dev: true - - /once@1.4.0: - resolution: {integrity: sha512-lNaJgI+2Q5URQBkccEKHTQOPaXdUxnZZElQTZY0MFUAuaEqe1E+Nyvgdz/aIyNi6Z9MzO5dv1H8n58/GELp3+w==} - dependencies: - wrappy: 1.0.2 - dev: true - - /parent-module@1.0.1: - resolution: {integrity: sha512-GQ2EWRpQV8/o+Aw8YqtfZZPfNRWZYkbidE9k5rpl/hC3vtHHBfGm2Ifi6qWV+coDGkrUKZAxE3Lot5kcsRlh+g==} - engines: {node: '>=6'} - dependencies: - callsites: 3.1.0 - dev: true - - /parse-json@5.2.0: - resolution: {integrity: sha512-ayCKvm/phCGxOkYRSCM82iDwct8/EonSEgCSxWxD7ve6jHggsFl4fZVQBPRNgQoKiuV/odhFrGzQXZwbifC8Rg==} - engines: {node: '>=8'} - dependencies: - '@babel/code-frame': 7.22.13 - error-ex: 1.3.2 - json-parse-even-better-errors: 2.3.1 - lines-and-columns: 1.2.4 - dev: true - - /path-type@4.0.0: - resolution: {integrity: sha512-gDKb8aZMDeD/tZWs9P6+q0J9Mwkdl6xMV8TjnGP3qJVJ06bdMgkbBlLU8IdfOsIsFz2BW1rNVT3XuNEl8zPAvw==} - engines: {node: '>=8'} - dev: true - - /pluralize@8.0.0: - resolution: {integrity: sha512-Nc3IT5yHzflTfbjgqWcCPpo7DaKy4FnpB0l/zCAW0Tc7jxAiuqSxHasntB3D7887LSrA93kDJ9IXovxJYxyLCA==} - engines: {node: '>=4'} - dev: true - - /pnpm@8.7.5: - resolution: {integrity: sha512-WI8WZb89Uiq5x2jdz4PcQMG9ovTnXcDCEpoEckPYIT2zD8/+dEhVozPlT7bu3WkBgE0uTARtgyIKAFt+IpW2cQ==} - engines: {node: '>=16.14'} - hasBin: true - dev: true - - /prettier@2.8.8: - resolution: {integrity: sha512-tdN8qQGvNjw4CHbY+XXk0JgCXn9QiF21a55rBe5LJAU+kDyC4WQn4+awm2Xfk2lQMk5fKup9XgzTZtGkjBdP9Q==} - engines: {node: '>=10.13.0'} - hasBin: true - requiresBuild: true - dev: true - optional: true - - /punycode@2.3.0: - resolution: {integrity: sha512-rRV+zQD8tVFys26lAGR9WUuS4iUAngJScM+ZRSKtvl5tKeZ2t5bvdNFdNHBW9FWR4guGHlgmsZ1G7BSm2wTbuA==} - engines: {node: '>=6'} - dev: true - - /require-from-string@2.0.2: - resolution: {integrity: sha512-Xf0nWe6RseziFMu+Ap9biiUbmplq6S9/p+7w7YXP/JBHhrUDDUhwa+vANyubuqfZWTveU//DYVGsDG7RKL/vEw==} - engines: {node: '>=0.10.0'} - dev: true - - /resolve-from@4.0.0: - resolution: {integrity: sha512-pb/MYmXstAkysRFx8piNI1tGFNQIFA3vkE3Gq4EuA1dF6gHp/+vgZqsCGJapvy8N3Q+4o7FwvquPJcnZ7RYy4g==} - engines: {node: '>=4'} - dev: true - - /semver@7.5.4: - resolution: {integrity: sha512-1bCSESV6Pv+i21Hvpxp3Dx+pSD8lIPt8uVjRrxAUt/nbswYc+tK6Y2btiULjd4+fnq15PX+nqQDC7Oft7WkwcA==} - engines: {node: '>=10'} - hasBin: true - dependencies: - lru-cache: 6.0.0 - dev: true - - /slice-ansi@4.0.0: - resolution: {integrity: sha512-qMCMfhY040cVHT43K9BFygqYbUPFZKHOg7K73mtTWJRb8pyP3fzf4Ixd5SzdEJQ6MRUg/WBnOLxghZtKKurENQ==} - engines: {node: '>=10'} - dependencies: - ansi-styles: 4.3.0 - astral-regex: 2.0.0 - is-fullwidth-code-point: 3.0.0 - dev: true - - /solhint@3.6.2: - resolution: {integrity: sha512-85EeLbmkcPwD+3JR7aEMKsVC9YrRSxd4qkXuMzrlf7+z2Eqdfm1wHWq1ffTuo5aDhoZxp2I9yF3QkxZOxOL7aQ==} - hasBin: true - dependencies: - '@solidity-parser/parser': 0.16.1 - ajv: 6.12.6 - antlr4: 4.13.1 - ast-parents: 0.0.1 - chalk: 4.1.2 - commander: 10.0.1 - cosmiconfig: 8.3.6 - fast-diff: 1.3.0 - glob: 8.1.0 - ignore: 5.2.4 - js-yaml: 4.1.0 - lodash: 4.17.21 - pluralize: 8.0.0 - semver: 7.5.4 - strip-ansi: 6.0.1 - table: 6.8.1 - text-table: 0.2.0 - optionalDependencies: - prettier: 2.8.8 - transitivePeerDependencies: - - typescript - dev: true - - /string-width@4.2.3: - resolution: {integrity: sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g==} - engines: {node: '>=8'} - dependencies: - emoji-regex: 8.0.0 - is-fullwidth-code-point: 3.0.0 - strip-ansi: 6.0.1 - dev: true - - /strip-ansi@6.0.1: - resolution: {integrity: sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A==} - engines: {node: '>=8'} - dependencies: - ansi-regex: 5.0.1 - dev: true - - /supports-color@5.5.0: - resolution: {integrity: sha512-QjVjwdXIt408MIiAqCX4oUKsgU2EqAGzs2Ppkm4aQYbjm+ZEWEcW4SfFNTr4uMNZma0ey4f5lgLrkB0aX0QMow==} - engines: {node: '>=4'} - dependencies: - has-flag: 3.0.0 - dev: true - - /supports-color@7.2.0: - resolution: {integrity: sha512-qpCAvRl9stuOHveKsn7HncJRvv501qIacKzQlO/+Lwxc9+0q2wLyv4Dfvt80/DPn2pqOBsJdDiogXGR9+OvwRw==} - engines: {node: '>=8'} - dependencies: - has-flag: 4.0.0 - dev: true - - /table@6.8.1: - resolution: {integrity: sha512-Y4X9zqrCftUhMeH2EptSSERdVKt/nEdijTOacGD/97EKjhQ/Qs8RTlEGABSJNNN8lac9kheH+af7yAkEWlgneA==} - engines: {node: '>=10.0.0'} - dependencies: - ajv: 8.12.0 - lodash.truncate: 4.4.2 - slice-ansi: 4.0.0 - string-width: 4.2.3 - strip-ansi: 6.0.1 - dev: true - - /text-table@0.2.0: - resolution: {integrity: sha512-N+8UisAXDGk8PFXP4HAzVR9nbfmVJ3zYLAWiTIoqC5v5isinhr+r5uaO8+7r3BMfuNIufIsA7RdpVgacC2cSpw==} - dev: true - - /uri-js@4.4.1: - resolution: {integrity: sha512-7rKUyy33Q1yc98pQ1DAmLtwX109F7TIfWlW1Ydo8Wl1ii1SeHieeh0HHfPeL2fMXK6z0s8ecKs9frCuLJvndBg==} - dependencies: - punycode: 2.3.0 - dev: true - - /wrappy@1.0.2: - resolution: {integrity: sha512-l4Sp/DRseor9wL6EvV2+TuQn63dMkPjZ/sp9XkghTEbV9KlPS1xUsZ3u7/IQO4wxtcFB4bgpQPRcR3QCvezPcQ==} - dev: true - - /yallist@4.0.0: - resolution: {integrity: sha512-3wdGidZyq5PB084XLES5TpOSRA3wjXAlIWMhum2kRcv/41Sn2emQ0dycQW4uZXLejwKvg6EsvbdlVL+FYEct7A==} - dev: true diff --git a/src/plugins/session-key/BaseSessionKeyPlugin.sol b/src/plugins/session-key/BaseSessionKeyPlugin.sol index 6d0c30ac..f0b0be21 100644 --- a/src/plugins/session-key/BaseSessionKeyPlugin.sol +++ b/src/plugins/session-key/BaseSessionKeyPlugin.sol @@ -16,6 +16,7 @@ import {BasePlugin} from "../BasePlugin.sol"; import {ISessionKeyPlugin} from "./interfaces/ISessionKeyPlugin.sol"; import {ISingleOwnerPlugin} from "../owner/ISingleOwnerPlugin.sol"; import {SingleOwnerPlugin} from "../owner/SingleOwnerPlugin.sol"; +import {PluginStorageLib} from "./libraries/PluginStorageLib.sol"; /// @title Base Session Key Plugin /// @author Decipher ERC-6900 Team @@ -31,12 +32,13 @@ import {SingleOwnerPlugin} from "../owner/SingleOwnerPlugin.sol"; /// the MSCA can add or remove session keys. contract BaseSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { using ECDSA for bytes32; + using PluginStorageLib for address; string public constant NAME = "Base Session Key Plugin"; string public constant VERSION = "1.0.0"; string public constant AUTHOR = "Decipher ERC-6900 Team"; - mapping(address => mapping(address => mapping(bytes4 => bytes))) internal _sessionInfo; + uint256 internal constant _SIG_VALIDATION_FAILED = 1; // ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ // ┃ Execution functions ┃ @@ -47,17 +49,50 @@ contract BaseSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { if (_until <= _after) { revert WrongTimeRangeForSession(); } + bytes32 key = keccak256(abi.encodePacked(tempOwner, allowedSelector)); bytes memory sessionInfo = abi.encodePacked(_after, _until); - _sessionInfo[msg.sender][tempOwner][allowedSelector] = sessionInfo; + address(msg.sender).writeBytesChecked(key, sessionInfo); emit TemporaryOwnerAdded(msg.sender, tempOwner, allowedSelector, _after, _until); } /// @inheritdoc ISessionKeyPlugin function removeTemporaryOwner(address tempOwner, bytes4 allowedSelector) external { - delete _sessionInfo[msg.sender][tempOwner][allowedSelector]; + bytes32 key = keccak256(abi.encodePacked(tempOwner, allowedSelector)); + bytes memory emptyBytes = new bytes(0); + address(msg.sender).writeBytesChecked(key, emptyBytes); emit TemporaryOwnerRemoved(msg.sender, tempOwner, allowedSelector); } + /// @inheritdoc ISessionKeyPlugin + function addTemporaryOwnerBatch( + address[] calldata tempOwners, + bytes4[] calldata allowedSelectors, + uint48[] calldata _afters, + uint48[] calldata _untils + ) external { + for (uint256 i = 0; i < tempOwners.length; i++) { + if (_untils[i] <= _afters[i]) { + revert WrongTimeRangeForSession(); + } + bytes32 key = keccak256(abi.encodePacked(tempOwners[i], allowedSelectors[i])); + bytes memory sessionInfo = abi.encodePacked(_afters[i], _untils[i]); + address(msg.sender).writeBytesChecked(key, sessionInfo); + } + emit TemporaryOwnersAdded(msg.sender, tempOwners, allowedSelectors, _afters, _untils); + } + + function removeTemporaryOwnerBatch( + address[] calldata tempOwners, + bytes4[] calldata allowedSelectors + ) external { + bytes memory emptyBytes = abi.encodePacked(uint96(1)); + for (uint256 i = 0; i < tempOwners.length; i++) { + bytes32 key = keccak256(abi.encodePacked(tempOwners[i], allowedSelectors[i])); + address(msg.sender).writeBytesChecked(key, emptyBytes); + } + emit TemporaryOwnersRemoved(msg.sender, tempOwners, allowedSelectors); + } + // ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ // ┃ Plugin view functions ┃ // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ @@ -68,7 +103,8 @@ contract BaseSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { view returns (uint48 _after, uint48 _until) { - (_after, _until) = _decode(_sessionInfo[account][tempOwner][allowedSelector]); + bytes memory timeRange = address(account).readBytesChecked(keccak256(abi.encodePacked(tempOwner, allowedSelector))); + (_after, _until) = _decode(timeRange); } // ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ @@ -91,14 +127,17 @@ contract BaseSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { if (functionId == uint8(FunctionId.USER_OP_VALIDATION_TEMPORARY_OWNER)) { (address signer,) = (userOpHash.toEthSignedMessageHash()).tryRecover(userOp.signature); bytes4 selector = bytes4(userOp.callData[0:4]); - bytes memory duration = _sessionInfo[userOp.sender][signer][selector]; + bytes32 key = keccak256(abi.encodePacked(signer, selector)); + bytes memory duration = address(userOp.sender).readBytesChecked(key); if (duration.length != 0) { (uint48 _after, uint48 _until) = _decode(duration); // first parameter of _packValidationData is sigFailed, which should be false return _packValidationData(false, _until, _after); + } else { + return _SIG_VALIDATION_FAILED; } } - return _packValidationData(true, 0, 0); + revert NotImplemented(); } /// @inheritdoc BasePlugin @@ -109,13 +148,16 @@ contract BaseSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { { if (functionId == uint8(FunctionId.RUNTIME_VALIDATION_TEMPORARY_OWNER)) { bytes4 selector = bytes4(data[0:4]); - bytes memory duration = _sessionInfo[msg.sender][sender][selector]; + bytes32 key = keccak256(abi.encodePacked(sender, selector)); + bytes memory duration = address(msg.sender).readBytesChecked(key); if (duration.length != 0) { (uint48 _after, uint48 _until) = _decode(duration); if (block.timestamp < _after || block.timestamp > _until) { revert WrongTimeRangeForSession(); } return; + } else { + revert NotAuthorized(); } } revert NotImplemented(); @@ -125,17 +167,19 @@ contract BaseSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { function pluginManifest() external pure override returns (PluginManifest memory) { PluginManifest memory manifest; - manifest.executionFunctions = new bytes4[](3); + manifest.executionFunctions = new bytes4[](5); manifest.executionFunctions[0] = this.addTemporaryOwner.selector; manifest.executionFunctions[1] = this.removeTemporaryOwner.selector; - manifest.executionFunctions[2] = this.getSessionDuration.selector; + manifest.executionFunctions[2] = this.addTemporaryOwnerBatch.selector; + manifest.executionFunctions[3] = this.removeTemporaryOwnerBatch.selector; + manifest.executionFunctions[4] = this.getSessionDuration.selector; ManifestFunction memory ownerUserOpValidationFunction = ManifestFunction({ functionType: ManifestAssociatedFunctionType.DEPENDENCY, functionId: 0, // Unused. dependencyIndex: 0 // Used as first index. }); - manifest.userOpValidationFunctions = new ManifestAssociatedFunction[](2); + manifest.userOpValidationFunctions = new ManifestAssociatedFunction[](4); manifest.userOpValidationFunctions[0] = ManifestAssociatedFunction({ executionSelector: this.addTemporaryOwner.selector, associatedFunction: ownerUserOpValidationFunction @@ -144,6 +188,14 @@ contract BaseSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { executionSelector: this.removeTemporaryOwner.selector, associatedFunction: ownerUserOpValidationFunction }); + manifest.userOpValidationFunctions[2] = ManifestAssociatedFunction({ + executionSelector: this.addTemporaryOwnerBatch.selector, + associatedFunction: ownerUserOpValidationFunction + }); + manifest.userOpValidationFunctions[3] = ManifestAssociatedFunction({ + executionSelector: this.removeTemporaryOwnerBatch.selector, + associatedFunction: ownerUserOpValidationFunction + }); ManifestFunction memory ownerOrSelfRuntimeValidationFunction = ManifestFunction({ functionType: ManifestAssociatedFunctionType.DEPENDENCY, @@ -156,7 +208,7 @@ contract BaseSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { dependencyIndex: 0 // Unused. }); - manifest.runtimeValidationFunctions = new ManifestAssociatedFunction[](3); + manifest.runtimeValidationFunctions = new ManifestAssociatedFunction[](5); manifest.runtimeValidationFunctions[0] = ManifestAssociatedFunction({ executionSelector: this.addTemporaryOwner.selector, associatedFunction: ownerOrSelfRuntimeValidationFunction @@ -166,6 +218,14 @@ contract BaseSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { associatedFunction: ownerOrSelfRuntimeValidationFunction }); manifest.runtimeValidationFunctions[2] = ManifestAssociatedFunction({ + executionSelector: this.addTemporaryOwnerBatch.selector, + associatedFunction: ownerOrSelfRuntimeValidationFunction + }); + manifest.runtimeValidationFunctions[3] = ManifestAssociatedFunction({ + executionSelector: this.removeTemporaryOwnerBatch.selector, + associatedFunction: ownerOrSelfRuntimeValidationFunction + }); + manifest.runtimeValidationFunctions[4] = ManifestAssociatedFunction({ executionSelector: this.getSessionDuration.selector, associatedFunction: alwaysAllowFunction }); diff --git a/src/plugins/session-key/TokenSessionKeyPlugin.sol b/src/plugins/session-key/TokenSessionKeyPlugin.sol index 4bb47891..3bc6be22 100644 --- a/src/plugins/session-key/TokenSessionKeyPlugin.sol +++ b/src/plugins/session-key/TokenSessionKeyPlugin.sol @@ -57,7 +57,7 @@ contract TokenSessionKeyPlugin is BasePlugin, ITokenSessionKeyPlugin { // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ /// @inheritdoc BasePlugin - function onInstall(bytes calldata data) external override {} + function onInstall(bytes calldata data) external override {} /// @inheritdoc BasePlugin function onUninstall(bytes calldata) external override {} @@ -100,8 +100,10 @@ contract TokenSessionKeyPlugin is BasePlugin, ITokenSessionKeyPlugin { } } - bytes4[] memory permittedExecutionSelectors = new bytes4[](1); + bytes4[] memory permittedExecutionSelectors = new bytes4[](3); permittedExecutionSelectors[0] = TRANSFERFROM_SELECTOR; + permittedExecutionSelectors[1] = ISessionKeyPlugin.addTemporaryOwnerBatch.selector; + permittedExecutionSelectors[2] = ISessionKeyPlugin.removeTemporaryOwnerBatch.selector; manifest.permittedExternalCalls = new ManifestExternalCallPermission[](1); manifest.permittedExternalCalls[0] = ManifestExternalCallPermission({ diff --git a/src/plugins/session-key/interfaces/ISessionKeyPlugin.sol b/src/plugins/session-key/interfaces/ISessionKeyPlugin.sol index 2e219340..baac8251 100644 --- a/src/plugins/session-key/interfaces/ISessionKeyPlugin.sol +++ b/src/plugins/session-key/interfaces/ISessionKeyPlugin.sol @@ -25,6 +25,22 @@ interface ISessionKeyPlugin { /// @param selector The selector of the function that the temporary owner is allowed to call. event TemporaryOwnerRemoved(address indexed account, address indexed tempOwner, bytes4 selector); + /// @notice This event is emitted when temporary owners are added to the account. + /// @param account The account whose temporary owners are updated. + /// @param tempOwners The addresses of the temporary owners. + /// @param selectors The selectors of the functions that the temporary owners are allowed to call. + /// @param _afters The times after which the owners are valid. + /// @param _untils The times until which the owners are valid. + event TemporaryOwnersAdded( + address indexed account, address[] indexed tempOwners, bytes4[] selectors, uint48[] _afters, uint48[] _untils + ); + + /// @notice This event is emitted when temporary owners are removed from the account. + /// @param account The account whose temporary owners are updated. + /// @param tempOwners The addresses of the temporary owners. + /// @param selectors The selectors of the functions that the temporary owners are allowed to call. + event TemporaryOwnersRemoved(address indexed account, address[] indexed tempOwners, bytes4[] selectors); + error NotAuthorized(); error WrongTimeRangeForSession(); @@ -44,6 +60,27 @@ interface ISessionKeyPlugin { /// @param allowedSelector The selector of the function that the temporary owner is allowed to call. function removeTemporaryOwner(address tempOwner, bytes4 allowedSelector) external; + /// @notice Add temporary owners to the account. + /// @dev This function is installed on the account as part of plugin installation, and should + /// only be called from an account. + /// @param tempOwners The addresses of the temporary owners. + /// @param allowedSelectors The selectors of the functions that the temporary owners are allowed to call. + /// @param _afters The times after which the owners are valid. + /// @param _untils The times until which the owners are valid. + function addTemporaryOwnerBatch( + address[] calldata tempOwners, + bytes4[] calldata allowedSelectors, + uint48[] calldata _afters, + uint48[] calldata _untils + ) external; + + /// @notice Remove temporary owners from the account. + /// @dev This function is installed on the account as part of plugin installation, and should + /// only be called from an account. + /// @param tempOwners The addresses of the temporary owners. + /// @param allowedSelectors The selectors of the functions that the temporary owners are allowed to call. + function removeTemporaryOwnerBatch(address[] calldata tempOwners, bytes4[] calldata allowedSelectors) external; + /// @notice Get Session data for a given account and temporary owner. /// @param account The account to get session data for. /// @param tempOwner The address of the temporary owner. diff --git a/src/plugins/session-key/libraries/PluginStorageLib.sol b/src/plugins/session-key/libraries/PluginStorageLib.sol new file mode 100644 index 00000000..e0649f03 --- /dev/null +++ b/src/plugins/session-key/libraries/PluginStorageLib.sol @@ -0,0 +1,254 @@ +/// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.19; + +/// @title Plugin Storage Library for ERC-4337 Address-associated Storage +/// @author Adam Egyed +/// @notice This library treats storage available to associated addresses as one big global mapping of (bytes32 => bytes). +/// +/// THIS IS HIGHLY EXPERIMENTAL AND NOT READY FOR PRODUCTION USE. +/// +/// It is up to the implementer to define the serialization and deserialization of structs, +/// since Solidity itself does not provide ways to encode structs into their storage representations easily. +/// While you can use `abi.encode` and `abi.decode`, this is not recommended because it will result in +/// extraneous data being stored in storage, which will unreasonably increase gas costs. +library PluginStorageLib { + + /// @notice Writes a bytes array to storage using a key and an associated address. + /// @notice This function will write the length of the bytes array to storage. + /// @param addr The address associated with the storage + /// @param key The key used to identify the bytes array + /// @param val The bytes array to write to storage + function writeBytesChecked(address addr, bytes32 key, bytes memory val) internal { + assembly ("memory-safe") { + // compute total length, including the extra word for the length field itself + let len := add(mload(val), 32) + + // reserve 3 words in memory (96 bytes) for hash inputs + let hashInput := mload(0x40) + mstore(0x40, add(hashInput, 96)) + // Hash inputs will always be: + // 1. caller address + // 2. key + // 3. batch index + // So we can set the caller address and key here to reuse, + // but we'll need to set the batch index for each batch + mstore(hashInput, addr) + mstore(add(hashInput, 32), key) + + // Compute the number of batches we need to write, rounded up + let batches := div(add(len, 4095), 4096) + + // Copy the bytes array into storage + for { let batchIndex := 0 } lt(batchIndex, batches) { batchIndex := add(batchIndex, 1) } { + // Hash the batch index with the caller address and key to get a new 128 associated slots + mstore(add(hashInput, 64), batchIndex) + let batchStart := keccak256(hashInput, 96) + + // Write the batch to storage, 128 slots at a time + let end := false + for { let slotIndex := 0 } and(lt(slotIndex, 128), not(end)) { slotIndex := add(slotIndex, 1) } { + // Compute the current slot in storage, and the current offset in memory + let slot := add(batchStart, slotIndex) + let offset := add(mul(slotIndex, 32), mul(batchIndex, 4096)) + + // Is this the last word we need to write? Stop one word before offset = len + end := iszero(sub(len, add(offset, 32))) + + // Copy the word from the bytes array into storage + let dataStart := add(val, offset) + let data := mload(dataStart) + sstore(slot, data) + } + } + } + } + + /// @notice Writes a bytes array to storage using a key and an associated address. + /// @notice This function will write NOT the length of the bytes array to storage, + /// but will use the length of the array to determine how much to write. + /// It is the responsibility of the caller to preserve length information. + /// @param addr The address associated with the storage + /// @param key The key used to identify the bytes array + /// @param val The bytes array to write to storage + function writeBytesUnchecked(address addr, bytes32 key, bytes memory val) internal { + assembly ("memory-safe") { + // compute total length, excluding the length field itself + let len := mload(val) + + // reserve 3 words in memory (96 bytes) for hash inputs + let hashInput := mload(0x40) + mstore(0x40, add(hashInput, 96)) + // Hash inputs will always be: + // 1. caller address + // 2. key + // 3. batch index + // So we can set the caller address and key here to reuse, + // but we'll need to set the batch index for each batch + mstore(hashInput, addr) + mstore(add(hashInput, 32), key) + + // Compute the number of batches we need to write, rounded up + let batches := div(add(len, 4095), 4096) + + // Copy the bytes array into storage + for { let batchIndex := 0 } lt(batchIndex, batches) { batchIndex := add(batchIndex, 1) } { + // Hash the batch index with the caller address and key to get a new 128 associated slots + mstore(add(hashInput, 64), batchIndex) + let batchStart := keccak256(hashInput, 96) + + // Write the batch to storage, 128 slots at a time + let end := false + for { let slotIndex := 0 } and(lt(slotIndex, 128), not(end)) { slotIndex := add(slotIndex, 1) } { + // Compute the current slot in storage, and the current offset in memory + let slot := add(batchStart, slotIndex) + let offset := add(mul(slotIndex, 32), mul(batchIndex, 4096)) + + // Is this the last word we need to write? Stop one word before offset = len + end := iszero(sub(len, add(offset, 32))) + + // Copy the word from the bytes array into storage + let dataStart := add(add(val, 32), offset) + let data := mload(dataStart) + sstore(slot, data) + } + } + } + } + + /// @notice Reads a bytes array from storage using a key and an associated address + /// @param addr The address associated with the storage + /// @param key The key used to identify the bytes array + /// @return ret The bytes array stored in storage + function readBytesChecked(address addr, bytes32 key) internal view returns (bytes memory ret) { + assembly ("memory-safe") { + // reserve 3 words in memory (96 bytes) for hash inputs + let hashInput := mload(0x40) + mstore(0x40, add(hashInput, 96)) + + // Hash inputs will always be: + // 1. caller address + // 2. key + // 3. batch index + // So we can set the caller address and key here to reuse, + // but we'll need to set the batch index for each batch + mstore(hashInput, addr) + mstore(add(hashInput, 32), key) + + // Get the length of the stored bytes array first, then copy everything else over + // Set the batch index to 0 to get the length only + mstore(add(hashInput, 64), 0) + let hash := keccak256(hashInput, 96) + // Include the extra word for the length field itself + let len := add(sload(hash), 32) + + // Allocate memory for the returned bytes array + ret := mload(0x40) + mstore(0x40, add(ret, len)) + + // Copy storage into the bytes array + let batches := div(add(len, 4095), 4096) // num batches rounded up + for { let batchIndex := 0 } lt(batchIndex, batches) { batchIndex := add(batchIndex, 1) } { + // Hash the batch index with the caller address and key to get a new 128 associated slots + mstore(add(hashInput, 64), batchIndex) + let batchStart := keccak256(hashInput, 96) + + // Read the batch from storage, 128 slots at a time + let end := false + for { let slotIndex := 0 } and(lt(slotIndex, 128), not(end)) { slotIndex := add(slotIndex, 1) } { + // Compute the current slot in storage, and the current offset in memory + let slot := add(batchStart, slotIndex) + let offset := add(mul(slotIndex, 32), mul(batchIndex, 4096)) + + // Is this the last word we need to read? Stop one word before offset = len + end := iszero(sub(len, add(offset, 32))) + + // Copy the data from storage to memory + let dataLoc := add(ret, offset) + mstore(dataLoc, sload(slot)) + } + } + } + } + + /// @notice Reads a bytes array from storage using a key and an associated address, of a specified length + /// @param addr The address associated with the storage + /// @param key The key used to identify the bytes array + /// @param len The length of the bytes array to read + /// @return ret The bytes array stored in storage + function readBytesUnchecked(address addr, bytes32 key, uint256 len) internal view returns (bytes memory ret) { + assembly ("memory-safe") { + // reserve 3 words in memory (96 bytes) for hash inputs + let hashInput := mload(0x40) + mstore(0x40, add(hashInput, 96)) + + // Hash inputs will always be: + // 1. caller address + // 2. key + // 3. batch index + // So we can set the caller address and key here to reuse, + // but we'll need to set the batch index for each batch + mstore(hashInput, addr) + mstore(add(hashInput, 32), key) + + // Get the length of the stored bytes array first, then copy everything else over + // Set the batch index to 0 to get the length only + mstore(add(hashInput, 64), 0) + let hash := keccak256(hashInput, 96) + + // Allocate memory for the returned bytes array + ret := mload(0x40) + // Include the extra word for the length field itself. + // The length field is not in storage, but must be set in the returned memory array. + mstore(0x40, add(ret, add(len, 32))) + + // Store the length of the bytes array in memory + mstore(ret, len) + + // Copy storage into the bytes array + let batches := div(add(len, 4095), 4096) // num batches rounded up + for { let batchIndex := 0 } lt(batchIndex, batches) { batchIndex := add(batchIndex, 1) } { + // Hash the batch index with the caller address and key to get a new 128 associated slots + mstore(add(hashInput, 64), batchIndex) + let batchStart := keccak256(hashInput, 96) + + // Read the batch from storage, 128 slots at a time + let end := false + for { let slotIndex := 0 } and(lt(slotIndex, 128), not(end)) { slotIndex := add(slotIndex, 1) } { + // Compute the current slot in storage, and the current offset in memory + let slot := add(batchStart, slotIndex) + let offset := add(mul(slotIndex, 32), mul(batchIndex, 4096)) + + // Is this the last word we need to read? Stop one word before offset = len + end := iszero(sub(len, add(offset, 32))) + + // Copy the data from storage to memory + // data location is the offset plus the length word + let dataLoc := add(add(ret, 32), offset) + mstore(dataLoc, sload(slot)) + } + } + } + } + + /// @notice Efficiently retrieve a full word from a bytes array in memory. + /// The returned value can be casted to any primitive type as needed. + /// @param val The bytes array to read from + /// @param index The index of the word to read + /// @return ret The word at the given index + function wordAt(bytes memory val, uint256 index) internal pure returns (bytes32 ret) { + assembly ("memory-safe") { + ret := mload(add(add(val, 32), mul(index, 32))) + } + } + + /// @notice Efficiently retrieve a single byte from a bytes32 word. + /// The returned value can be casted to any primitive type as needed. + /// @param val The word to read from + /// @param index The index of the byte to read + /// @return ret The byte at the given index + function byteAt(bytes32 val, uint8 index) internal pure returns (bytes1 ret) { + assembly ("memory-safe") { + ret := byte(index, val) + } + } +} \ No newline at end of file From b1367b5870a4b374acd195014c740227dbd101c7 Mon Sep 17 00:00:00 2001 From: sm-stack Date: Mon, 25 Dec 2023 16:47:32 +0900 Subject: [PATCH 15/33] Revert changes for pnpm-lock.yaml --- pnpm-lock.yaml | 474 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 474 insertions(+) create mode 100644 pnpm-lock.yaml diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml new file mode 100644 index 00000000..17766bf3 --- /dev/null +++ b/pnpm-lock.yaml @@ -0,0 +1,474 @@ +lockfileVersion: '6.0' + +settings: + autoInstallPeers: true + excludeLinksFromLockfile: false + +devDependencies: + pnpm: + specifier: ^8.7.5 + version: 8.7.5 + solhint: + specifier: ^3.6.2 + version: 3.6.2 + +packages: + + /@babel/code-frame@7.22.13: + resolution: {integrity: sha512-XktuhWlJ5g+3TJXc5upd9Ks1HutSArik6jf2eAjYFyIOf4ej3RN+184cZbzDvbPnuTJIUhPKKJE3cIsYTiAT3w==} + engines: {node: '>=6.9.0'} + dependencies: + '@babel/highlight': 7.22.13 + chalk: 2.4.2 + dev: true + + /@babel/helper-validator-identifier@7.22.15: + resolution: {integrity: sha512-4E/F9IIEi8WR94324mbDUMo074YTheJmd7eZF5vITTeYchqAi6sYXRLHUVsmkdmY4QjfKTcB2jB7dVP3NaBElQ==} + engines: {node: '>=6.9.0'} + dev: true + + /@babel/highlight@7.22.13: + resolution: {integrity: sha512-C/BaXcnnvBCmHTpz/VGZ8jgtE2aYlW4hxDhseJAWZb7gqGM/qtCK6iZUb0TyKFf7BOUsBH7Q7fkRsDRhg1XklQ==} + engines: {node: '>=6.9.0'} + dependencies: + '@babel/helper-validator-identifier': 7.22.15 + chalk: 2.4.2 + js-tokens: 4.0.0 + dev: true + + /@solidity-parser/parser@0.16.1: + resolution: {integrity: sha512-PdhRFNhbTtu3x8Axm0uYpqOy/lODYQK+MlYSgqIsq2L8SFYEHJPHNUiOTAJbDGzNjjr1/n9AcIayxafR/fWmYw==} + dependencies: + antlr4ts: 0.5.0-alpha.4 + dev: true + + /ajv@6.12.6: + resolution: {integrity: sha512-j3fVLgvTo527anyYyJOGTYJbG+vnnQYvE0m5mmkc1TK+nxAppkCLMIL0aZ4dblVCNoGShhm+kzE4ZUykBoMg4g==} + dependencies: + fast-deep-equal: 3.1.3 + fast-json-stable-stringify: 2.1.0 + json-schema-traverse: 0.4.1 + uri-js: 4.4.1 + dev: true + + /ajv@8.12.0: + resolution: {integrity: sha512-sRu1kpcO9yLtYxBKvqfTeh9KzZEwO3STyX1HT+4CaDzC6HpTGYhIhPIzj9XuKU7KYDwnaeh5hcOwjy1QuJzBPA==} + dependencies: + fast-deep-equal: 3.1.3 + json-schema-traverse: 1.0.0 + require-from-string: 2.0.2 + uri-js: 4.4.1 + dev: true + + /ansi-regex@5.0.1: + resolution: {integrity: sha512-quJQXlTSUGL2LH9SUXo8VwsY4soanhgo6LNSm84E1LBcE8s3O0wpdiRzyR9z/ZZJMlMWv37qOOb9pdJlMUEKFQ==} + engines: {node: '>=8'} + dev: true + + /ansi-styles@3.2.1: + resolution: {integrity: sha512-VT0ZI6kZRdTh8YyJw3SMbYm/u+NqfsAxEpWO0Pf9sq8/e94WxxOpPKx9FR1FlyCtOVDNOQ+8ntlqFxiRc+r5qA==} + engines: {node: '>=4'} + dependencies: + color-convert: 1.9.3 + dev: true + + /ansi-styles@4.3.0: + resolution: {integrity: sha512-zbB9rCJAT1rbjiVDb2hqKFHNYLxgtk8NURxZ3IZwD3F6NtxbXZQCnnSi1Lkx+IDohdPlFp222wVALIheZJQSEg==} + engines: {node: '>=8'} + dependencies: + color-convert: 2.0.1 + dev: true + + /antlr4@4.13.1: + resolution: {integrity: sha512-kiXTspaRYvnIArgE97z5YVVf/cDVQABr3abFRR6mE7yesLMkgu4ujuyV/sgxafQ8wgve0DJQUJ38Z8tkgA2izA==} + engines: {node: '>=16'} + dev: true + + /antlr4ts@0.5.0-alpha.4: + resolution: {integrity: sha512-WPQDt1B74OfPv/IMS2ekXAKkTZIHl88uMetg6q3OTqgFxZ/dxDXI0EWLyZid/1Pe6hTftyg5N7gel5wNAGxXyQ==} + dev: true + + /argparse@2.0.1: + resolution: {integrity: sha512-8+9WqebbFzpX9OR+Wa6O29asIogeRMzcGtAINdpMHHyAg10f05aSFVBbcEqGf/PXw1EjAZ+q2/bEBg3DvurK3Q==} + dev: true + + /ast-parents@0.0.1: + resolution: {integrity: sha512-XHusKxKz3zoYk1ic8Un640joHbFMhbqneyoZfoKnEGtf2ey9Uh/IdpcQplODdO/kENaMIWsD0nJm4+wX3UNLHA==} + dev: true + + /astral-regex@2.0.0: + resolution: {integrity: sha512-Z7tMw1ytTXt5jqMcOP+OQteU1VuNK9Y02uuJtKQ1Sv69jXQKKg5cibLwGJow8yzZP+eAc18EmLGPal0bp36rvQ==} + engines: {node: '>=8'} + dev: true + + /balanced-match@1.0.2: + resolution: {integrity: sha512-3oSeUO0TMV67hN1AmbXsK4yaqU7tjiHlbxRDZOpH0KW9+CeX4bRAaX0Anxt0tx2MrpRpWwQaPwIlISEJhYU5Pw==} + dev: true + + /brace-expansion@2.0.1: + resolution: {integrity: sha512-XnAIvQ8eM+kC6aULx6wuQiwVsnzsi9d3WxzV3FpWTGA19F621kwdbsAcFKXgKUHZWsy+mY6iL1sHTxWEFCytDA==} + dependencies: + balanced-match: 1.0.2 + dev: true + + /callsites@3.1.0: + resolution: {integrity: sha512-P8BjAsXvZS+VIDUI11hHCQEv74YT67YUi5JJFNWIqL235sBmjX4+qx9Muvls5ivyNENctx46xQLQ3aTuE7ssaQ==} + engines: {node: '>=6'} + dev: true + + /chalk@2.4.2: + resolution: {integrity: sha512-Mti+f9lpJNcwF4tWV8/OrTTtF1gZi+f8FqlyAdouralcFWFQWF2+NgCHShjkCb+IFBLq9buZwE1xckQU4peSuQ==} + engines: {node: '>=4'} + dependencies: + ansi-styles: 3.2.1 + escape-string-regexp: 1.0.5 + supports-color: 5.5.0 + dev: true + + /chalk@4.1.2: + resolution: {integrity: sha512-oKnbhFyRIXpUuez8iBMmyEa4nbj4IOQyuhc/wy9kY7/WVPcwIO9VA668Pu8RkO7+0G76SLROeyw9CpQ061i4mA==} + engines: {node: '>=10'} + dependencies: + ansi-styles: 4.3.0 + supports-color: 7.2.0 + dev: true + + /color-convert@1.9.3: + resolution: {integrity: sha512-QfAUtd+vFdAtFQcC8CCyYt1fYWxSqAiK2cSD6zDB8N3cpsEBAvRxp9zOGg6G/SHHJYAT88/az/IuDGALsNVbGg==} + dependencies: + color-name: 1.1.3 + dev: true + + /color-convert@2.0.1: + resolution: {integrity: sha512-RRECPsj7iu/xb5oKYcsFHSppFNnsj/52OVTRKb4zP5onXwVF3zVmmToNcOfGC+CRDpfK/U584fMg38ZHCaElKQ==} + engines: {node: '>=7.0.0'} + dependencies: + color-name: 1.1.4 + dev: true + + /color-name@1.1.3: + resolution: {integrity: sha512-72fSenhMw2HZMTVHeCA9KCmpEIbzWiQsjN+BHcBbS9vr1mtt+vJjPdksIBNUmKAW8TFUDPJK5SUU3QhE9NEXDw==} + dev: true + + /color-name@1.1.4: + resolution: {integrity: sha512-dOy+3AuW3a2wNbZHIuMZpTcgjGuLU/uBL/ubcZF9OXbDo8ff4O8yVp5Bf0efS8uEoYo5q4Fx7dY9OgQGXgAsQA==} + dev: true + + /commander@10.0.1: + resolution: {integrity: sha512-y4Mg2tXshplEbSGzx7amzPwKKOCGuoSRP/CjEdwwk0FOGlUbq6lKuoyDZTNZkmxHdJtp54hdfY/JUrdL7Xfdug==} + engines: {node: '>=14'} + dev: true + + /cosmiconfig@8.3.6: + resolution: {integrity: sha512-kcZ6+W5QzcJ3P1Mt+83OUv/oHFqZHIx8DuxG6eZ5RGMERoLqp4BuGjhHLYGK+Kf5XVkQvqBSmAy/nGWN3qDgEA==} + engines: {node: '>=14'} + peerDependencies: + typescript: '>=4.9.5' + peerDependenciesMeta: + typescript: + optional: true + dependencies: + import-fresh: 3.3.0 + js-yaml: 4.1.0 + parse-json: 5.2.0 + path-type: 4.0.0 + dev: true + + /emoji-regex@8.0.0: + resolution: {integrity: sha512-MSjYzcWNOA0ewAHpz0MxpYFvwg6yjy1NG3xteoqz644VCo/RPgnr1/GGt+ic3iJTzQ8Eu3TdM14SawnVUmGE6A==} + dev: true + + /error-ex@1.3.2: + resolution: {integrity: sha512-7dFHNmqeFSEt2ZBsCriorKnn3Z2pj+fd9kmI6QoWw4//DL+icEBfc0U7qJCisqrTsKTjw4fNFy2pW9OqStD84g==} + dependencies: + is-arrayish: 0.2.1 + dev: true + + /escape-string-regexp@1.0.5: + resolution: {integrity: sha512-vbRorB5FUQWvla16U8R/qgaFIya2qGzwDrNmCZuYKrbdSUMG6I1ZCGQRefkRVhuOkIGVne7BQ35DSfo1qvJqFg==} + engines: {node: '>=0.8.0'} + dev: true + + /fast-deep-equal@3.1.3: + resolution: {integrity: sha512-f3qQ9oQy9j2AhBe/H9VC91wLmKBCCU/gDOnKNAYG5hswO7BLKj09Hc5HYNz9cGI++xlpDCIgDaitVs03ATR84Q==} + dev: true + + /fast-diff@1.3.0: + resolution: {integrity: sha512-VxPP4NqbUjj6MaAOafWeUn2cXWLcCtljklUtZf0Ind4XQ+QPtmA0b18zZy0jIQx+ExRVCR/ZQpBmik5lXshNsw==} + dev: true + + /fast-json-stable-stringify@2.1.0: + resolution: {integrity: sha512-lhd/wF+Lk98HZoTCtlVraHtfh5XYijIjalXck7saUtuanSDyLMxnHhSXEDJqHxD7msR8D0uCmqlkwjCV8xvwHw==} + dev: true + + /fs.realpath@1.0.0: + resolution: {integrity: sha512-OO0pH2lK6a0hZnAdau5ItzHPI6pUlvI7jMVnxUQRtw4owF2wk8lOSabtGDCTP4Ggrg2MbGnWO9X8K1t4+fGMDw==} + dev: true + + /glob@8.1.0: + resolution: {integrity: sha512-r8hpEjiQEYlF2QU0df3dS+nxxSIreXQS1qRhMJM0Q5NDdR386C7jb7Hwwod8Fgiuex+k0GFjgft18yvxm5XoCQ==} + engines: {node: '>=12'} + dependencies: + fs.realpath: 1.0.0 + inflight: 1.0.6 + inherits: 2.0.4 + minimatch: 5.1.6 + once: 1.4.0 + dev: true + + /has-flag@3.0.0: + resolution: {integrity: sha512-sKJf1+ceQBr4SMkvQnBDNDtf4TXpVhVGateu0t918bl30FnbE2m4vNLX+VWe/dpjlb+HugGYzW7uQXH98HPEYw==} + engines: {node: '>=4'} + dev: true + + /has-flag@4.0.0: + resolution: {integrity: sha512-EykJT/Q1KjTWctppgIAgfSO0tKVuZUjhgMr17kqTumMl6Afv3EISleU7qZUzoXDFTAHTDC4NOoG/ZxU3EvlMPQ==} + engines: {node: '>=8'} + dev: true + + /ignore@5.2.4: + resolution: {integrity: sha512-MAb38BcSbH0eHNBxn7ql2NH/kX33OkB3lZ1BNdh7ENeRChHTYsTvWrMubiIAMNS2llXEEgZ1MUOBtXChP3kaFQ==} + engines: {node: '>= 4'} + dev: true + + /import-fresh@3.3.0: + resolution: {integrity: sha512-veYYhQa+D1QBKznvhUHxb8faxlrwUnxseDAbAp457E0wLNio2bOSKnjYDhMj+YiAq61xrMGhQk9iXVk5FzgQMw==} + engines: {node: '>=6'} + dependencies: + parent-module: 1.0.1 + resolve-from: 4.0.0 + dev: true + + /inflight@1.0.6: + resolution: {integrity: sha512-k92I/b08q4wvFscXCLvqfsHCrjrF7yiXsQuIVvVE7N82W3+aqpzuUdBbfhWcy/FZR3/4IgflMgKLOsvPDrGCJA==} + dependencies: + once: 1.4.0 + wrappy: 1.0.2 + dev: true + + /inherits@2.0.4: + resolution: {integrity: sha512-k/vGaX4/Yla3WzyMCvTQOXYeIHvqOKtnqBduzTHpzpQZzAskKMhZ2K+EnBiSM9zGSoIFeMpXKxa4dYeZIQqewQ==} + dev: true + + /is-arrayish@0.2.1: + resolution: {integrity: sha512-zz06S8t0ozoDXMG+ube26zeCTNXcKIPJZJi8hBrF4idCLms4CG9QtK7qBl1boi5ODzFpjswb5JPmHCbMpjaYzg==} + dev: true + + /is-fullwidth-code-point@3.0.0: + resolution: {integrity: sha512-zymm5+u+sCsSWyD9qNaejV3DFvhCKclKdizYaJUuHA83RLjb7nSuGnddCHGv0hk+KY7BMAlsWeK4Ueg6EV6XQg==} + engines: {node: '>=8'} + dev: true + + /js-tokens@4.0.0: + resolution: {integrity: sha512-RdJUflcE3cUzKiMqQgsCu06FPu9UdIJO0beYbPhHN4k6apgJtifcoCtT9bcxOpYBtpD2kCM6Sbzg4CausW/PKQ==} + dev: true + + /js-yaml@4.1.0: + resolution: {integrity: sha512-wpxZs9NoxZaJESJGIZTyDEaYpl0FKSA+FB9aJiyemKhMwkxQg63h4T1KJgUGHpTqPDNRcmmYLugrRjJlBtWvRA==} + hasBin: true + dependencies: + argparse: 2.0.1 + dev: true + + /json-parse-even-better-errors@2.3.1: + resolution: {integrity: sha512-xyFwyhro/JEof6Ghe2iz2NcXoj2sloNsWr/XsERDK/oiPCfaNhl5ONfp+jQdAZRQQ0IJWNzH9zIZF7li91kh2w==} + dev: true + + /json-schema-traverse@0.4.1: + resolution: {integrity: sha512-xbbCH5dCYU5T8LcEhhuh7HJ88HXuW3qsI3Y0zOZFKfZEHcpWiHU/Jxzk629Brsab/mMiHQti9wMP+845RPe3Vg==} + dev: true + + /json-schema-traverse@1.0.0: + resolution: {integrity: sha512-NM8/P9n3XjXhIZn1lLhkFaACTOURQXjWhV4BA/RnOv8xvgqtqpAX9IO4mRQxSx1Rlo4tqzeqb0sOlruaOy3dug==} + dev: true + + /lines-and-columns@1.2.4: + resolution: {integrity: sha512-7ylylesZQ/PV29jhEDl3Ufjo6ZX7gCqJr5F7PKrqc93v7fzSymt1BpwEU8nAUXs8qzzvqhbjhK5QZg6Mt/HkBg==} + dev: true + + /lodash.truncate@4.4.2: + resolution: {integrity: sha512-jttmRe7bRse52OsWIMDLaXxWqRAmtIUccAQ3garviCqJjafXOfNMO0yMfNpdD6zbGaTU0P5Nz7e7gAT6cKmJRw==} + dev: true + + /lodash@4.17.21: + resolution: {integrity: sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg==} + dev: true + + /lru-cache@6.0.0: + resolution: {integrity: sha512-Jo6dJ04CmSjuznwJSS3pUeWmd/H0ffTlkXXgwZi+eq1UCmqQwCh+eLsYOYCwY991i2Fah4h1BEMCx4qThGbsiA==} + engines: {node: '>=10'} + dependencies: + yallist: 4.0.0 + dev: true + + /minimatch@5.1.6: + resolution: {integrity: sha512-lKwV/1brpG6mBUFHtb7NUmtABCb2WZZmm2wNiOA5hAb8VdCS4B3dtMWyvcoViccwAW/COERjXLt0zP1zXUN26g==} + engines: {node: '>=10'} + dependencies: + brace-expansion: 2.0.1 + dev: true + + /once@1.4.0: + resolution: {integrity: sha512-lNaJgI+2Q5URQBkccEKHTQOPaXdUxnZZElQTZY0MFUAuaEqe1E+Nyvgdz/aIyNi6Z9MzO5dv1H8n58/GELp3+w==} + dependencies: + wrappy: 1.0.2 + dev: true + + /parent-module@1.0.1: + resolution: {integrity: sha512-GQ2EWRpQV8/o+Aw8YqtfZZPfNRWZYkbidE9k5rpl/hC3vtHHBfGm2Ifi6qWV+coDGkrUKZAxE3Lot5kcsRlh+g==} + engines: {node: '>=6'} + dependencies: + callsites: 3.1.0 + dev: true + + /parse-json@5.2.0: + resolution: {integrity: sha512-ayCKvm/phCGxOkYRSCM82iDwct8/EonSEgCSxWxD7ve6jHggsFl4fZVQBPRNgQoKiuV/odhFrGzQXZwbifC8Rg==} + engines: {node: '>=8'} + dependencies: + '@babel/code-frame': 7.22.13 + error-ex: 1.3.2 + json-parse-even-better-errors: 2.3.1 + lines-and-columns: 1.2.4 + dev: true + + /path-type@4.0.0: + resolution: {integrity: sha512-gDKb8aZMDeD/tZWs9P6+q0J9Mwkdl6xMV8TjnGP3qJVJ06bdMgkbBlLU8IdfOsIsFz2BW1rNVT3XuNEl8zPAvw==} + engines: {node: '>=8'} + dev: true + + /pluralize@8.0.0: + resolution: {integrity: sha512-Nc3IT5yHzflTfbjgqWcCPpo7DaKy4FnpB0l/zCAW0Tc7jxAiuqSxHasntB3D7887LSrA93kDJ9IXovxJYxyLCA==} + engines: {node: '>=4'} + dev: true + + /pnpm@8.7.5: + resolution: {integrity: sha512-WI8WZb89Uiq5x2jdz4PcQMG9ovTnXcDCEpoEckPYIT2zD8/+dEhVozPlT7bu3WkBgE0uTARtgyIKAFt+IpW2cQ==} + engines: {node: '>=16.14'} + hasBin: true + dev: true + + /prettier@2.8.8: + resolution: {integrity: sha512-tdN8qQGvNjw4CHbY+XXk0JgCXn9QiF21a55rBe5LJAU+kDyC4WQn4+awm2Xfk2lQMk5fKup9XgzTZtGkjBdP9Q==} + engines: {node: '>=10.13.0'} + hasBin: true + requiresBuild: true + dev: true + optional: true + + /punycode@2.3.0: + resolution: {integrity: sha512-rRV+zQD8tVFys26lAGR9WUuS4iUAngJScM+ZRSKtvl5tKeZ2t5bvdNFdNHBW9FWR4guGHlgmsZ1G7BSm2wTbuA==} + engines: {node: '>=6'} + dev: true + + /require-from-string@2.0.2: + resolution: {integrity: sha512-Xf0nWe6RseziFMu+Ap9biiUbmplq6S9/p+7w7YXP/JBHhrUDDUhwa+vANyubuqfZWTveU//DYVGsDG7RKL/vEw==} + engines: {node: '>=0.10.0'} + dev: true + + /resolve-from@4.0.0: + resolution: {integrity: sha512-pb/MYmXstAkysRFx8piNI1tGFNQIFA3vkE3Gq4EuA1dF6gHp/+vgZqsCGJapvy8N3Q+4o7FwvquPJcnZ7RYy4g==} + engines: {node: '>=4'} + dev: true + + /semver@7.5.4: + resolution: {integrity: sha512-1bCSESV6Pv+i21Hvpxp3Dx+pSD8lIPt8uVjRrxAUt/nbswYc+tK6Y2btiULjd4+fnq15PX+nqQDC7Oft7WkwcA==} + engines: {node: '>=10'} + hasBin: true + dependencies: + lru-cache: 6.0.0 + dev: true + + /slice-ansi@4.0.0: + resolution: {integrity: sha512-qMCMfhY040cVHT43K9BFygqYbUPFZKHOg7K73mtTWJRb8pyP3fzf4Ixd5SzdEJQ6MRUg/WBnOLxghZtKKurENQ==} + engines: {node: '>=10'} + dependencies: + ansi-styles: 4.3.0 + astral-regex: 2.0.0 + is-fullwidth-code-point: 3.0.0 + dev: true + + /solhint@3.6.2: + resolution: {integrity: sha512-85EeLbmkcPwD+3JR7aEMKsVC9YrRSxd4qkXuMzrlf7+z2Eqdfm1wHWq1ffTuo5aDhoZxp2I9yF3QkxZOxOL7aQ==} + hasBin: true + dependencies: + '@solidity-parser/parser': 0.16.1 + ajv: 6.12.6 + antlr4: 4.13.1 + ast-parents: 0.0.1 + chalk: 4.1.2 + commander: 10.0.1 + cosmiconfig: 8.3.6 + fast-diff: 1.3.0 + glob: 8.1.0 + ignore: 5.2.4 + js-yaml: 4.1.0 + lodash: 4.17.21 + pluralize: 8.0.0 + semver: 7.5.4 + strip-ansi: 6.0.1 + table: 6.8.1 + text-table: 0.2.0 + optionalDependencies: + prettier: 2.8.8 + transitivePeerDependencies: + - typescript + dev: true + + /string-width@4.2.3: + resolution: {integrity: sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g==} + engines: {node: '>=8'} + dependencies: + emoji-regex: 8.0.0 + is-fullwidth-code-point: 3.0.0 + strip-ansi: 6.0.1 + dev: true + + /strip-ansi@6.0.1: + resolution: {integrity: sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A==} + engines: {node: '>=8'} + dependencies: + ansi-regex: 5.0.1 + dev: true + + /supports-color@5.5.0: + resolution: {integrity: sha512-QjVjwdXIt408MIiAqCX4oUKsgU2EqAGzs2Ppkm4aQYbjm+ZEWEcW4SfFNTr4uMNZma0ey4f5lgLrkB0aX0QMow==} + engines: {node: '>=4'} + dependencies: + has-flag: 3.0.0 + dev: true + + /supports-color@7.2.0: + resolution: {integrity: sha512-qpCAvRl9stuOHveKsn7HncJRvv501qIacKzQlO/+Lwxc9+0q2wLyv4Dfvt80/DPn2pqOBsJdDiogXGR9+OvwRw==} + engines: {node: '>=8'} + dependencies: + has-flag: 4.0.0 + dev: true + + /table@6.8.1: + resolution: {integrity: sha512-Y4X9zqrCftUhMeH2EptSSERdVKt/nEdijTOacGD/97EKjhQ/Qs8RTlEGABSJNNN8lac9kheH+af7yAkEWlgneA==} + engines: {node: '>=10.0.0'} + dependencies: + ajv: 8.12.0 + lodash.truncate: 4.4.2 + slice-ansi: 4.0.0 + string-width: 4.2.3 + strip-ansi: 6.0.1 + dev: true + + /text-table@0.2.0: + resolution: {integrity: sha512-N+8UisAXDGk8PFXP4HAzVR9nbfmVJ3zYLAWiTIoqC5v5isinhr+r5uaO8+7r3BMfuNIufIsA7RdpVgacC2cSpw==} + dev: true + + /uri-js@4.4.1: + resolution: {integrity: sha512-7rKUyy33Q1yc98pQ1DAmLtwX109F7TIfWlW1Ydo8Wl1ii1SeHieeh0HHfPeL2fMXK6z0s8ecKs9frCuLJvndBg==} + dependencies: + punycode: 2.3.0 + dev: true + + /wrappy@1.0.2: + resolution: {integrity: sha512-l4Sp/DRseor9wL6EvV2+TuQn63dMkPjZ/sp9XkghTEbV9KlPS1xUsZ3u7/IQO4wxtcFB4bgpQPRcR3QCvezPcQ==} + dev: true + + /yallist@4.0.0: + resolution: {integrity: sha512-3wdGidZyq5PB084XLES5TpOSRA3wjXAlIWMhum2kRcv/41Sn2emQ0dycQW4uZXLejwKvg6EsvbdlVL+FYEct7A==} + dev: true \ No newline at end of file From c12d26dc05c652b23ebac5cc547392cb0a5f0e1a Mon Sep 17 00:00:00 2001 From: sm-stack Date: Mon, 25 Dec 2023 17:09:57 +0900 Subject: [PATCH 16/33] Remove unused dependencies at test --- test/plugin/SessionKeyPlugin.t.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/test/plugin/SessionKeyPlugin.t.sol b/test/plugin/SessionKeyPlugin.t.sol index a5f0ef49..af00b3ce 100644 --- a/test/plugin/SessionKeyPlugin.t.sol +++ b/test/plugin/SessionKeyPlugin.t.sol @@ -7,7 +7,6 @@ import {EntryPoint} from "@eth-infinitism/account-abstraction/core/EntryPoint.so import {UserOperation} from "@eth-infinitism/account-abstraction/interfaces/UserOperation.sol"; import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; -import {BasePlugin} from "../../src/plugins/BasePlugin.sol"; import {SingleOwnerPlugin} from "../../src/plugins/owner/SingleOwnerPlugin.sol"; import {ISingleOwnerPlugin} from "../../src/plugins/owner/ISingleOwnerPlugin.sol"; import {BaseSessionKeyPlugin} from "../../src/plugins/session-key/BaseSessionKeyPlugin.sol"; From 3cb63a6c5b1d0b566c7ceaa15d72b6b8cd01d652 Mon Sep 17 00:00:00 2001 From: sm-stack Date: Mon, 25 Dec 2023 17:11:46 +0900 Subject: [PATCH 17/33] Fix nits --- pnpm-lock.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 17766bf3..393efc6e 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -471,4 +471,4 @@ packages: /yallist@4.0.0: resolution: {integrity: sha512-3wdGidZyq5PB084XLES5TpOSRA3wjXAlIWMhum2kRcv/41Sn2emQ0dycQW4uZXLejwKvg6EsvbdlVL+FYEct7A==} - dev: true \ No newline at end of file + dev: true From 2252299093e2cdaa81146b6166646ff8196af8c6 Mon Sep 17 00:00:00 2001 From: sm-stack Date: Sat, 30 Dec 2023 03:06:23 +0900 Subject: [PATCH 18/33] Add onInstall and unOninstall function at TokenSessionKey.sol --- .../session-key/BaseSessionKeyPlugin.sol | 12 ++- .../session-key/TokenSessionKeyPlugin.sol | 50 ++++++---- test/plugin/SessionKeyPlugin.t.sol | 94 ++++++++++++++++++- 3 files changed, 132 insertions(+), 24 deletions(-) diff --git a/src/plugins/session-key/BaseSessionKeyPlugin.sol b/src/plugins/session-key/BaseSessionKeyPlugin.sol index f0b0be21..a2eb7c94 100644 --- a/src/plugins/session-key/BaseSessionKeyPlugin.sol +++ b/src/plugins/session-key/BaseSessionKeyPlugin.sol @@ -85,7 +85,7 @@ contract BaseSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { address[] calldata tempOwners, bytes4[] calldata allowedSelectors ) external { - bytes memory emptyBytes = abi.encodePacked(uint96(1)); + bytes memory emptyBytes = new bytes(0); for (uint256 i = 0; i < tempOwners.length; i++) { bytes32 key = keccak256(abi.encodePacked(tempOwners[i], allowedSelectors[i])); address(msg.sender).writeBytesChecked(key, emptyBytes); @@ -248,7 +248,7 @@ contract BaseSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { string memory modifySessionKeyPermission = "Modify Session Key"; // Permission descriptions - metadata.permissionDescriptors = new SelectorPermission[](2); + metadata.permissionDescriptors = new SelectorPermission[](4); metadata.permissionDescriptors[0] = SelectorPermission({ functionSelector: this.addTemporaryOwner.selector, permissionDescription: modifySessionKeyPermission @@ -257,6 +257,14 @@ contract BaseSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { functionSelector: this.removeTemporaryOwner.selector, permissionDescription: modifySessionKeyPermission }); + metadata.permissionDescriptors[2] = SelectorPermission({ + functionSelector: this.addTemporaryOwnerBatch.selector, + permissionDescription: modifySessionKeyPermission + }); + metadata.permissionDescriptors[3] = SelectorPermission({ + functionSelector: this.removeTemporaryOwnerBatch.selector, + permissionDescription: modifySessionKeyPermission + }); return metadata; } diff --git a/src/plugins/session-key/TokenSessionKeyPlugin.sol b/src/plugins/session-key/TokenSessionKeyPlugin.sol index 3bc6be22..48ba1d44 100644 --- a/src/plugins/session-key/TokenSessionKeyPlugin.sol +++ b/src/plugins/session-key/TokenSessionKeyPlugin.sol @@ -57,10 +57,32 @@ contract TokenSessionKeyPlugin is BasePlugin, ITokenSessionKeyPlugin { // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ /// @inheritdoc BasePlugin - function onInstall(bytes calldata data) external override {} + function onInstall(bytes calldata data) external override { + if (data.length != 0) { + // Call to removeTemporaryOwnerBatch function in BaseSessionKeyPlugin + (bool success, bytes memory returnData) = address(msg.sender).call( + abi.encodeWithSelector(IPluginExecutor.executeFromPlugin.selector, data)); + if (!success) { + assembly ("memory-safe") { + revert(add(returnData, 32), mload(returnData)) + } + } + } + } /// @inheritdoc BasePlugin - function onUninstall(bytes calldata) external override {} + function onUninstall(bytes calldata data) external override { + if (data.length != 0) { + // Call to removeTemporaryOwnerBatch function in BaseSessionKeyPlugin + (bool success, bytes memory returnData) = address(msg.sender).call( + abi.encodeWithSelector(IPluginExecutor.executeFromPlugin.selector, data)); + if (!success) { + assembly ("memory-safe") { + revert(add(returnData, 32), mload(returnData)) + } + } + } + } /// @inheritdoc BasePlugin function pluginManifest() external pure override returns (PluginManifest memory) { @@ -96,20 +118,22 @@ contract TokenSessionKeyPlugin is BasePlugin, ITokenSessionKeyPlugin { for (uint256 i = 0; i < manifest.dependencyInterfaceIds.length;) { manifest.dependencyInterfaceIds[i] = type(ISessionKeyPlugin).interfaceId; unchecked { - i++; + ++i; } } - bytes4[] memory permittedExecutionSelectors = new bytes4[](3); - permittedExecutionSelectors[0] = TRANSFERFROM_SELECTOR; - permittedExecutionSelectors[1] = ISessionKeyPlugin.addTemporaryOwnerBatch.selector; - permittedExecutionSelectors[2] = ISessionKeyPlugin.removeTemporaryOwnerBatch.selector; + manifest.permittedExecutionSelectors = new bytes4[](2); + manifest.permittedExecutionSelectors[0] = ISessionKeyPlugin.addTemporaryOwnerBatch.selector; + manifest.permittedExecutionSelectors[1] = ISessionKeyPlugin.removeTemporaryOwnerBatch.selector; + + bytes4[] memory permittedExternalSelectors = new bytes4[](1); + permittedExternalSelectors[0] = TRANSFERFROM_SELECTOR; manifest.permittedExternalCalls = new ManifestExternalCallPermission[](1); manifest.permittedExternalCalls[0] = ManifestExternalCallPermission({ externalAddress: TARGET_ERC20_CONTRACT, permitAnySelector: false, - selectors: permittedExecutionSelectors + selectors: permittedExternalSelectors }); return manifest; @@ -122,16 +146,6 @@ contract TokenSessionKeyPlugin is BasePlugin, ITokenSessionKeyPlugin { metadata.version = VERSION; metadata.author = AUTHOR; - // Permission strings - string memory sessionKeyTransferPermission = "Allow Token Transfer Operation By Session Key"; - - // Permission descriptions - metadata.permissionDescriptors = new SelectorPermission[](1); - metadata.permissionDescriptors[0] = SelectorPermission({ - functionSelector: this.transferFromSessionKey.selector, - permissionDescription: sessionKeyTransferPermission - }); - return metadata; } diff --git a/test/plugin/SessionKeyPlugin.t.sol b/test/plugin/SessionKeyPlugin.t.sol index af00b3ce..7caf451a 100644 --- a/test/plugin/SessionKeyPlugin.t.sol +++ b/test/plugin/SessionKeyPlugin.t.sol @@ -61,6 +61,11 @@ contract SessionKeyPluginTest is Test { address indexed account, address indexed tempOwner, bytes4 allowedSelector, uint48 _after, uint48 _until ); event TemporaryOwnerRemoved(address indexed account, address indexed tempOwner, bytes4 allowedSelector); + event TemporaryOwnersAdded( + address indexed account, address[] indexed tempOwners, bytes4[] allowedSelectors, uint48[] afters, uint48[] untils + ); + event TemporaryOwnersRemoved(address indexed account, address[] indexed tempOwners, bytes4[] allowedSelectors); + event PluginUninstalled(address indexed plugin, bool indexed onUninstallSuccess); function setUp() public { ownerPlugin = new SingleOwnerPlugin(); @@ -120,10 +125,30 @@ contract SessionKeyPluginTest is Test { ); bytes32 tokenSessionKeyManifestHash = keccak256(abi.encode(tokenSessionKeyPlugin.pluginManifest())); + address[] memory tempOwners = new address[](1); + tempOwners[0] = address(tempOwner); + + bytes4[] memory allowedSelectors = new bytes4[](1); + allowedSelectors[0] = TRANSFERFROM_SESSIONKEY_SELECTOR; + + uint48[] memory afters = new uint48[](1); + afters[0] = 0; + + uint48[] memory untils = new uint48[](1); + untils[0] = 2; + + bytes memory data = abi.encodeWithSelector( + ISessionKeyPlugin.addTemporaryOwnerBatch.selector, + tempOwners, + allowedSelectors, + afters, + untils + ); + account.installPlugin({ plugin: address(tokenSessionKeyPlugin), manifestHash: tokenSessionKeyManifestHash, - pluginInitData: "", + pluginInitData: data, dependencies: tokenSessionDependency, injectedHooks: new IPluginManager.InjectedHook[](0) }); @@ -132,9 +157,9 @@ contract SessionKeyPluginTest is Test { vm.startPrank(address(account)); mockERC20.approve(address(account), 1 ether); - vm.expectEmit(true, true, true, true); - emit TemporaryOwnerAdded(address(account), tempOwner, TRANSFERFROM_SESSIONKEY_SELECTOR, 0, 2); - baseSessionKeyPlugin.addTemporaryOwner(tempOwner, TRANSFERFROM_SESSIONKEY_SELECTOR, 0, 2); + // vm.expectEmit(true, true, true, true); + // emit TemporaryOwnerAdded(address(account), tempOwner, TRANSFERFROM_SESSIONKEY_SELECTOR, 0, 2); + // baseSessionKeyPlugin.addTemporaryOwner(tempOwner, TRANSFERFROM_SESSIONKEY_SELECTOR, 0, 2); (uint48 _after, uint48 _until) = baseSessionKeyPlugin.getSessionDuration(address(account), tempOwner, TRANSFERFROM_SESSIONKEY_SELECTOR); @@ -144,6 +169,38 @@ contract SessionKeyPluginTest is Test { vm.stopPrank(); } + function test_sessionKey_batch() public { + address tempOwner2 = makeAddr("tempOwner2"); + address tempOwner3 = makeAddr("tempOwner3"); + + address[] memory tempOwners = new address[](2); + tempOwners[0] = tempOwner2; + tempOwners[1] = tempOwner3; + + bytes4[] memory allowedSelectors = new bytes4[](2); + allowedSelectors[0] = TRANSFERFROM_SESSIONKEY_SELECTOR; + allowedSelectors[1] = TRANSFERFROM_SESSIONKEY_SELECTOR; + + uint48[] memory afters = new uint48[](2); + afters[0] = 0; + afters[1] = 0; + + uint48[] memory untils = new uint48[](2); + untils[0] = 2; + untils[1] = 2; + + vm.startPrank(address(account)); + + vm.expectEmit(true, true, true, true); + emit TemporaryOwnersAdded(address(account), tempOwners, allowedSelectors, afters, untils); + baseSessionKeyPlugin.addTemporaryOwnerBatch(tempOwners, allowedSelectors, afters, untils); + + vm.expectEmit(true, true, true, true); + emit TemporaryOwnersRemoved(address(account), tempOwners, allowedSelectors); + baseSessionKeyPlugin.removeTemporaryOwnerBatch(tempOwners, allowedSelectors); + vm.stopPrank(); + } + function test_sessionKey_userOp() public { UserOperation[] memory userOps = new UserOperation[](1); @@ -260,6 +317,35 @@ contract SessionKeyPluginTest is Test { ); } + function test_sessionKey_uninstallTokenSessionKeyPlugin() public { + address[] memory tempOwners = new address[](1); + tempOwners[0] = address(tempOwner); + + bytes4[] memory allowedSelectors = new bytes4[](1); + allowedSelectors[0] = TRANSFERFROM_SESSIONKEY_SELECTOR; + + bytes memory data = abi.encodeWithSelector( + ISessionKeyPlugin.removeTemporaryOwnerBatch.selector, + tempOwners, + allowedSelectors + ); + + vm.startPrank(owner); + + vm.expectEmit(true, true, true, true); + // The second parameter should be true, but permittedExternalSelectors is disabled + // in the previous step of uninstallation process, resulting in reversion of onUninstall(). + emit PluginUninstalled(address(tokenSessionKeyPlugin), false); + account.uninstallPlugin({ + plugin: address(tokenSessionKeyPlugin), + config: bytes(""), + pluginUninstallData: data, + hookUnapplyData: new bytes[](0) + }); + + vm.stopPrank(); + } + // Internal Function function _constructUserOp(address targetContract, address from, address to, uint256 amount) internal From 4b99521e5d2337b0d6213a1a659045bb97e41bf7 Mon Sep 17 00:00:00 2001 From: sm-stack Date: Tue, 2 Jan 2024 20:06:01 +0900 Subject: [PATCH 19/33] Fix format with forge fmt --- .../session-key/BaseSessionKeyPlugin.sol | 10 ++++----- .../session-key/TokenSessionKeyPlugin.sol | 10 ++++----- .../interfaces/ISessionKeyPlugin.sol | 9 ++++++-- .../libraries/PluginStorageLib.sol | 6 +++--- test/plugin/SessionKeyPlugin.t.sol | 21 ++++++++----------- 5 files changed, 29 insertions(+), 27 deletions(-) diff --git a/src/plugins/session-key/BaseSessionKeyPlugin.sol b/src/plugins/session-key/BaseSessionKeyPlugin.sol index a2eb7c94..c1cd9930 100644 --- a/src/plugins/session-key/BaseSessionKeyPlugin.sol +++ b/src/plugins/session-key/BaseSessionKeyPlugin.sol @@ -81,10 +81,9 @@ contract BaseSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { emit TemporaryOwnersAdded(msg.sender, tempOwners, allowedSelectors, _afters, _untils); } - function removeTemporaryOwnerBatch( - address[] calldata tempOwners, - bytes4[] calldata allowedSelectors - ) external { + function removeTemporaryOwnerBatch(address[] calldata tempOwners, bytes4[] calldata allowedSelectors) + external + { bytes memory emptyBytes = new bytes(0); for (uint256 i = 0; i < tempOwners.length; i++) { bytes32 key = keccak256(abi.encodePacked(tempOwners[i], allowedSelectors[i])); @@ -103,7 +102,8 @@ contract BaseSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { view returns (uint48 _after, uint48 _until) { - bytes memory timeRange = address(account).readBytesChecked(keccak256(abi.encodePacked(tempOwner, allowedSelector))); + bytes memory timeRange = + address(account).readBytesChecked(keccak256(abi.encodePacked(tempOwner, allowedSelector))); (_after, _until) = _decode(timeRange); } diff --git a/src/plugins/session-key/TokenSessionKeyPlugin.sol b/src/plugins/session-key/TokenSessionKeyPlugin.sol index 48ba1d44..8bb06961 100644 --- a/src/plugins/session-key/TokenSessionKeyPlugin.sol +++ b/src/plugins/session-key/TokenSessionKeyPlugin.sol @@ -60,22 +60,22 @@ contract TokenSessionKeyPlugin is BasePlugin, ITokenSessionKeyPlugin { function onInstall(bytes calldata data) external override { if (data.length != 0) { // Call to removeTemporaryOwnerBatch function in BaseSessionKeyPlugin - (bool success, bytes memory returnData) = address(msg.sender).call( - abi.encodeWithSelector(IPluginExecutor.executeFromPlugin.selector, data)); + (bool success, bytes memory returnData) = + address(msg.sender).call(abi.encodeWithSelector(IPluginExecutor.executeFromPlugin.selector, data)); if (!success) { assembly ("memory-safe") { revert(add(returnData, 32), mload(returnData)) } } } - } + } /// @inheritdoc BasePlugin function onUninstall(bytes calldata data) external override { if (data.length != 0) { // Call to removeTemporaryOwnerBatch function in BaseSessionKeyPlugin - (bool success, bytes memory returnData) = address(msg.sender).call( - abi.encodeWithSelector(IPluginExecutor.executeFromPlugin.selector, data)); + (bool success, bytes memory returnData) = + address(msg.sender).call(abi.encodeWithSelector(IPluginExecutor.executeFromPlugin.selector, data)); if (!success) { assembly ("memory-safe") { revert(add(returnData, 32), mload(returnData)) diff --git a/src/plugins/session-key/interfaces/ISessionKeyPlugin.sol b/src/plugins/session-key/interfaces/ISessionKeyPlugin.sol index baac8251..bb660cb2 100644 --- a/src/plugins/session-key/interfaces/ISessionKeyPlugin.sol +++ b/src/plugins/session-key/interfaces/ISessionKeyPlugin.sol @@ -32,7 +32,11 @@ interface ISessionKeyPlugin { /// @param _afters The times after which the owners are valid. /// @param _untils The times until which the owners are valid. event TemporaryOwnersAdded( - address indexed account, address[] indexed tempOwners, bytes4[] selectors, uint48[] _afters, uint48[] _untils + address indexed account, + address[] indexed tempOwners, + bytes4[] selectors, + uint48[] _afters, + uint48[] _untils ); /// @notice This event is emitted when temporary owners are removed from the account. @@ -79,7 +83,8 @@ interface ISessionKeyPlugin { /// only be called from an account. /// @param tempOwners The addresses of the temporary owners. /// @param allowedSelectors The selectors of the functions that the temporary owners are allowed to call. - function removeTemporaryOwnerBatch(address[] calldata tempOwners, bytes4[] calldata allowedSelectors) external; + function removeTemporaryOwnerBatch(address[] calldata tempOwners, bytes4[] calldata allowedSelectors) + external; /// @notice Get Session data for a given account and temporary owner. /// @param account The account to get session data for. diff --git a/src/plugins/session-key/libraries/PluginStorageLib.sol b/src/plugins/session-key/libraries/PluginStorageLib.sol index e0649f03..cae0ba7d 100644 --- a/src/plugins/session-key/libraries/PluginStorageLib.sol +++ b/src/plugins/session-key/libraries/PluginStorageLib.sol @@ -3,7 +3,8 @@ pragma solidity ^0.8.19; /// @title Plugin Storage Library for ERC-4337 Address-associated Storage /// @author Adam Egyed -/// @notice This library treats storage available to associated addresses as one big global mapping of (bytes32 => bytes). +/// @notice This library treats storage available to associated addresses as one big global mapping of (bytes32 => +/// bytes). /// /// THIS IS HIGHLY EXPERIMENTAL AND NOT READY FOR PRODUCTION USE. /// @@ -12,7 +13,6 @@ pragma solidity ^0.8.19; /// While you can use `abi.encode` and `abi.decode`, this is not recommended because it will result in /// extraneous data being stored in storage, which will unreasonably increase gas costs. library PluginStorageLib { - /// @notice Writes a bytes array to storage using a key and an associated address. /// @notice This function will write the length of the bytes array to storage. /// @param addr The address associated with the storage @@ -251,4 +251,4 @@ library PluginStorageLib { ret := byte(index, val) } } -} \ No newline at end of file +} diff --git a/test/plugin/SessionKeyPlugin.t.sol b/test/plugin/SessionKeyPlugin.t.sol index 7caf451a..6b7cb157 100644 --- a/test/plugin/SessionKeyPlugin.t.sol +++ b/test/plugin/SessionKeyPlugin.t.sol @@ -62,7 +62,11 @@ contract SessionKeyPluginTest is Test { ); event TemporaryOwnerRemoved(address indexed account, address indexed tempOwner, bytes4 allowedSelector); event TemporaryOwnersAdded( - address indexed account, address[] indexed tempOwners, bytes4[] allowedSelectors, uint48[] afters, uint48[] untils + address indexed account, + address[] indexed tempOwners, + bytes4[] allowedSelectors, + uint48[] afters, + uint48[] untils ); event TemporaryOwnersRemoved(address indexed account, address[] indexed tempOwners, bytes4[] allowedSelectors); event PluginUninstalled(address indexed plugin, bool indexed onUninstallSuccess); @@ -138,11 +142,7 @@ contract SessionKeyPluginTest is Test { untils[0] = 2; bytes memory data = abi.encodeWithSelector( - ISessionKeyPlugin.addTemporaryOwnerBatch.selector, - tempOwners, - allowedSelectors, - afters, - untils + ISessionKeyPlugin.addTemporaryOwnerBatch.selector, tempOwners, allowedSelectors, afters, untils ); account.installPlugin({ @@ -204,8 +204,7 @@ contract SessionKeyPluginTest is Test { function test_sessionKey_userOp() public { UserOperation[] memory userOps = new UserOperation[](1); - (, UserOperation memory userOp) = - _constructUserOp(address(mockERC20), address(account), target, 1 ether); + (, UserOperation memory userOp) = _constructUserOp(address(mockERC20), address(account), target, 1 ether); userOps[0] = userOp; entryPoint.handleOps(userOps, beneficiary); @@ -325,15 +324,13 @@ contract SessionKeyPluginTest is Test { allowedSelectors[0] = TRANSFERFROM_SESSIONKEY_SELECTOR; bytes memory data = abi.encodeWithSelector( - ISessionKeyPlugin.removeTemporaryOwnerBatch.selector, - tempOwners, - allowedSelectors + ISessionKeyPlugin.removeTemporaryOwnerBatch.selector, tempOwners, allowedSelectors ); vm.startPrank(owner); vm.expectEmit(true, true, true, true); - // The second parameter should be true, but permittedExternalSelectors is disabled + // The second parameter should be true, but permittedExternalSelectors is disabled // in the previous step of uninstallation process, resulting in reversion of onUninstall(). emit PluginUninstalled(address(tokenSessionKeyPlugin), false); account.uninstallPlugin({ From d21659e8dd18ab29a6dbc9a068f48b6b154a621c Mon Sep 17 00:00:00 2001 From: sm-stack Date: Mon, 8 Jan 2024 13:19:34 +0900 Subject: [PATCH 20/33] Add smaples directory --- .../plugins/ModularSessionKeyPlugin.sol} | 121 ++++++++++------- .../plugins}/TokenSessionKeyPlugin.sol | 36 +---- .../plugins}/interfaces/ISessionKeyPlugin.sol | 1 + .../interfaces/ITokenSessionKeyPlugin.sol | 0 .../plugins}/libraries/PluginStorageLib.sol | 0 .../plugins/ModularSessionKeyPlugin.t.sol} | 125 +++++++++--------- 6 files changed, 142 insertions(+), 141 deletions(-) rename src/{plugins/session-key/BaseSessionKeyPlugin.sol => samples/plugins/ModularSessionKeyPlugin.sol} (79%) rename src/{plugins/session-key => samples/plugins}/TokenSessionKeyPlugin.sol (79%) rename src/{plugins/session-key => samples/plugins}/interfaces/ISessionKeyPlugin.sol (99%) rename src/{plugins/session-key => samples/plugins}/interfaces/ITokenSessionKeyPlugin.sol (100%) rename src/{plugins/session-key => samples/plugins}/libraries/PluginStorageLib.sol (100%) rename test/{plugin/SessionKeyPlugin.t.sol => samples/plugins/ModularSessionKeyPlugin.t.sol} (78%) diff --git a/src/plugins/session-key/BaseSessionKeyPlugin.sol b/src/samples/plugins/ModularSessionKeyPlugin.sol similarity index 79% rename from src/plugins/session-key/BaseSessionKeyPlugin.sol rename to src/samples/plugins/ModularSessionKeyPlugin.sol index c1cd9930..c7dda6a0 100644 --- a/src/plugins/session-key/BaseSessionKeyPlugin.sol +++ b/src/samples/plugins/ModularSessionKeyPlugin.sol @@ -12,17 +12,17 @@ import { PluginMetadata, SelectorPermission } from "../../interfaces/IPlugin.sol"; -import {BasePlugin} from "../BasePlugin.sol"; +import {BasePlugin} from "../../plugins/BasePlugin.sol"; import {ISessionKeyPlugin} from "./interfaces/ISessionKeyPlugin.sol"; -import {ISingleOwnerPlugin} from "../owner/ISingleOwnerPlugin.sol"; -import {SingleOwnerPlugin} from "../owner/SingleOwnerPlugin.sol"; +import {ISingleOwnerPlugin} from "../../plugins/owner/ISingleOwnerPlugin.sol"; +import {SingleOwnerPlugin} from "../../plugins/owner/SingleOwnerPlugin.sol"; import {PluginStorageLib} from "./libraries/PluginStorageLib.sol"; -/// @title Base Session Key Plugin +/// @title Modular Session Key Plugin /// @author Decipher ERC-6900 Team /// @notice This plugin allows some designated EOA or smart contract to temporarily /// own a modular account. -/// This base session key plugin acts as a 'parent plugin' for all specific session +/// This modular session key plugin acts as a 'parent plugin' for all specific session /// keys. Using dependency, this plugin can be thought as a parent contract that stores /// session key duration information, and validation functions for session keys. All /// logics for session keys will be implemented in child plugins. @@ -30,11 +30,11 @@ import {PluginStorageLib} from "./libraries/PluginStorageLib.sol"; /// runtime, with its own validation functions. /// Also, it has a dependency on SingleOwnerPlugin, to make sure that only the owner of /// the MSCA can add or remove session keys. -contract BaseSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { +contract ModularSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { using ECDSA for bytes32; using PluginStorageLib for address; - string public constant NAME = "Base Session Key Plugin"; + string public constant NAME = "Modular Session Key Plugin"; string public constant VERSION = "1.0.0"; string public constant AUTHOR = "Decipher ERC-6900 Team"; @@ -46,20 +46,13 @@ contract BaseSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { /// @inheritdoc ISessionKeyPlugin function addTemporaryOwner(address tempOwner, bytes4 allowedSelector, uint48 _after, uint48 _until) external { - if (_until <= _after) { - revert WrongTimeRangeForSession(); - } - bytes32 key = keccak256(abi.encodePacked(tempOwner, allowedSelector)); - bytes memory sessionInfo = abi.encodePacked(_after, _until); - address(msg.sender).writeBytesChecked(key, sessionInfo); + _addTemporaryOwner(msg.sender, tempOwner, allowedSelector, _after, _until); emit TemporaryOwnerAdded(msg.sender, tempOwner, allowedSelector, _after, _until); } /// @inheritdoc ISessionKeyPlugin function removeTemporaryOwner(address tempOwner, bytes4 allowedSelector) external { - bytes32 key = keccak256(abi.encodePacked(tempOwner, allowedSelector)); - bytes memory emptyBytes = new bytes(0); - address(msg.sender).writeBytesChecked(key, emptyBytes); + _removeTemporaryOwner(msg.sender, tempOwner, allowedSelector); emit TemporaryOwnerRemoved(msg.sender, tempOwner, allowedSelector); } @@ -70,13 +63,14 @@ contract BaseSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { uint48[] calldata _afters, uint48[] calldata _untils ) external { + if ( + tempOwners.length != allowedSelectors.length || tempOwners.length != _afters.length + || tempOwners.length != _untils.length + ) { + revert WrongDataLength(); + } for (uint256 i = 0; i < tempOwners.length; i++) { - if (_untils[i] <= _afters[i]) { - revert WrongTimeRangeForSession(); - } - bytes32 key = keccak256(abi.encodePacked(tempOwners[i], allowedSelectors[i])); - bytes memory sessionInfo = abi.encodePacked(_afters[i], _untils[i]); - address(msg.sender).writeBytesChecked(key, sessionInfo); + _addTemporaryOwner(msg.sender, tempOwners[i], allowedSelectors[i], _afters[i], _untils[i]); } emit TemporaryOwnersAdded(msg.sender, tempOwners, allowedSelectors, _afters, _untils); } @@ -84,10 +78,11 @@ contract BaseSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { function removeTemporaryOwnerBatch(address[] calldata tempOwners, bytes4[] calldata allowedSelectors) external { - bytes memory emptyBytes = new bytes(0); + if (tempOwners.length != allowedSelectors.length) { + revert WrongDataLength(); + } for (uint256 i = 0; i < tempOwners.length; i++) { - bytes32 key = keccak256(abi.encodePacked(tempOwners[i], allowedSelectors[i])); - address(msg.sender).writeBytesChecked(key, emptyBytes); + _removeTemporaryOwner(msg.sender, tempOwners[i], allowedSelectors[i]); } emit TemporaryOwnersRemoved(msg.sender, tempOwners, allowedSelectors); } @@ -112,10 +107,39 @@ contract BaseSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ /// @inheritdoc BasePlugin - function onInstall(bytes calldata data) external override {} + function onInstall(bytes calldata data) external override { + if (data.length != 0) { + ( + address[] memory tempOwners, + bytes4[] memory allowedSelectors, + uint48[] memory _afters, + uint48[] memory _untils + ) = abi.decode(data, (address[], bytes4[], uint48[], uint48[])); + if ( + tempOwners.length != allowedSelectors.length || tempOwners.length != _afters.length + || tempOwners.length != _untils.length + ) { + revert WrongDataLength(); + } + for (uint256 i = 0; i < tempOwners.length; i++) { + _addTemporaryOwner(msg.sender, tempOwners[i], allowedSelectors[i], _afters[i], _untils[i]); + } + } + } /// @inheritdoc BasePlugin - function onUninstall(bytes calldata) external override {} + function onUninstall(bytes calldata data) external override { + if (data.length != 0) { + (address[] memory tempOwners, bytes4[] memory allowedSelectors) = + abi.decode(data, (address[], bytes4[])); + if (tempOwners.length != allowedSelectors.length) { + revert WrongDataLength(); + } + for (uint256 i = 0; i < tempOwners.length; i++) { + _removeTemporaryOwner(msg.sender, tempOwners[i], allowedSelectors[i]); + } + } + } /// @inheritdoc BasePlugin function userOpValidationFunction(uint8 functionId, UserOperation calldata userOp, bytes32 userOpHash) @@ -244,28 +268,6 @@ contract BaseSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { metadata.version = VERSION; metadata.author = AUTHOR; - // Permission strings - string memory modifySessionKeyPermission = "Modify Session Key"; - - // Permission descriptions - metadata.permissionDescriptors = new SelectorPermission[](4); - metadata.permissionDescriptors[0] = SelectorPermission({ - functionSelector: this.addTemporaryOwner.selector, - permissionDescription: modifySessionKeyPermission - }); - metadata.permissionDescriptors[1] = SelectorPermission({ - functionSelector: this.removeTemporaryOwner.selector, - permissionDescription: modifySessionKeyPermission - }); - metadata.permissionDescriptors[2] = SelectorPermission({ - functionSelector: this.addTemporaryOwnerBatch.selector, - permissionDescription: modifySessionKeyPermission - }); - metadata.permissionDescriptors[3] = SelectorPermission({ - functionSelector: this.removeTemporaryOwnerBatch.selector, - permissionDescription: modifySessionKeyPermission - }); - return metadata; } @@ -282,6 +284,27 @@ contract BaseSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { // ┃ Internal / Private functions ┃ // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ + function _addTemporaryOwner( + address account, + address tempOwner, + bytes4 allowedSelector, + uint48 _after, + uint48 _until + ) internal { + if (_until <= _after) { + revert WrongTimeRangeForSession(); + } + bytes32 key = keccak256(abi.encodePacked(tempOwner, allowedSelector)); + bytes memory sessionInfo = abi.encodePacked(_after, _until); + address(account).writeBytesChecked(key, sessionInfo); + } + + function _removeTemporaryOwner(address account, address tempOwner, bytes4 allowedSelector) internal { + bytes32 key = keccak256(abi.encodePacked(tempOwner, allowedSelector)); + bytes memory emptyBytes = new bytes(0); + address(account).writeBytesChecked(key, emptyBytes); + } + function _decode(bytes memory _data) internal pure returns (uint48 _after, uint48 _until) { assembly { _after := mload(add(_data, 0x06)) diff --git a/src/plugins/session-key/TokenSessionKeyPlugin.sol b/src/samples/plugins/TokenSessionKeyPlugin.sol similarity index 79% rename from src/plugins/session-key/TokenSessionKeyPlugin.sol rename to src/samples/plugins/TokenSessionKeyPlugin.sol index 8bb06961..ba2d11ee 100644 --- a/src/plugins/session-key/TokenSessionKeyPlugin.sol +++ b/src/samples/plugins/TokenSessionKeyPlugin.sol @@ -10,15 +10,15 @@ import { SelectorPermission, ManifestExternalCallPermission } from "../../interfaces/IPlugin.sol"; -import {BasePlugin} from "../BasePlugin.sol"; -import {BaseSessionKeyPlugin} from "./BaseSessionKeyPlugin.sol"; +import {BasePlugin} from "../../plugins/BasePlugin.sol"; +import {ModularSessionKeyPlugin} from "./ModularSessionKeyPlugin.sol"; import {ITokenSessionKeyPlugin} from "./interfaces/ITokenSessionKeyPlugin.sol"; import {ISessionKeyPlugin} from "./interfaces/ISessionKeyPlugin.sol"; import {IPluginExecutor} from "../../interfaces/IPluginExecutor.sol"; /// @title Token Session Key Plugin /// @author Decipher ERC-6900 Team -/// @notice This plugin acts as a 'child plugin' for BaseSessionKeyPlugin. +/// @notice This plugin acts as a 'child plugin' for ModularSessionKeyPlugin. /// It implements the logic for session keys that are allowed to call ERC20 /// transferFrom function. It allows for session key owners to access MSCA /// with `transferFromSessionKey` function, which calls `executeFromPluginExternal` @@ -57,32 +57,10 @@ contract TokenSessionKeyPlugin is BasePlugin, ITokenSessionKeyPlugin { // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ /// @inheritdoc BasePlugin - function onInstall(bytes calldata data) external override { - if (data.length != 0) { - // Call to removeTemporaryOwnerBatch function in BaseSessionKeyPlugin - (bool success, bytes memory returnData) = - address(msg.sender).call(abi.encodeWithSelector(IPluginExecutor.executeFromPlugin.selector, data)); - if (!success) { - assembly ("memory-safe") { - revert(add(returnData, 32), mload(returnData)) - } - } - } - } + function onInstall(bytes calldata data) external override {} /// @inheritdoc BasePlugin - function onUninstall(bytes calldata data) external override { - if (data.length != 0) { - // Call to removeTemporaryOwnerBatch function in BaseSessionKeyPlugin - (bool success, bytes memory returnData) = - address(msg.sender).call(abi.encodeWithSelector(IPluginExecutor.executeFromPlugin.selector, data)); - if (!success) { - assembly ("memory-safe") { - revert(add(returnData, 32), mload(returnData)) - } - } - } - } + function onUninstall(bytes calldata data) external override {} /// @inheritdoc BasePlugin function pluginManifest() external pure override returns (PluginManifest memory) { @@ -122,10 +100,6 @@ contract TokenSessionKeyPlugin is BasePlugin, ITokenSessionKeyPlugin { } } - manifest.permittedExecutionSelectors = new bytes4[](2); - manifest.permittedExecutionSelectors[0] = ISessionKeyPlugin.addTemporaryOwnerBatch.selector; - manifest.permittedExecutionSelectors[1] = ISessionKeyPlugin.removeTemporaryOwnerBatch.selector; - bytes4[] memory permittedExternalSelectors = new bytes4[](1); permittedExternalSelectors[0] = TRANSFERFROM_SELECTOR; diff --git a/src/plugins/session-key/interfaces/ISessionKeyPlugin.sol b/src/samples/plugins/interfaces/ISessionKeyPlugin.sol similarity index 99% rename from src/plugins/session-key/interfaces/ISessionKeyPlugin.sol rename to src/samples/plugins/interfaces/ISessionKeyPlugin.sol index bb660cb2..a369d318 100644 --- a/src/plugins/session-key/interfaces/ISessionKeyPlugin.sol +++ b/src/samples/plugins/interfaces/ISessionKeyPlugin.sol @@ -47,6 +47,7 @@ interface ISessionKeyPlugin { error NotAuthorized(); error WrongTimeRangeForSession(); + error WrongDataLength(); /// @notice Add a temporary owner to the account. /// @dev This function is installed on the account as part of plugin installation, and should diff --git a/src/plugins/session-key/interfaces/ITokenSessionKeyPlugin.sol b/src/samples/plugins/interfaces/ITokenSessionKeyPlugin.sol similarity index 100% rename from src/plugins/session-key/interfaces/ITokenSessionKeyPlugin.sol rename to src/samples/plugins/interfaces/ITokenSessionKeyPlugin.sol diff --git a/src/plugins/session-key/libraries/PluginStorageLib.sol b/src/samples/plugins/libraries/PluginStorageLib.sol similarity index 100% rename from src/plugins/session-key/libraries/PluginStorageLib.sol rename to src/samples/plugins/libraries/PluginStorageLib.sol diff --git a/test/plugin/SessionKeyPlugin.t.sol b/test/samples/plugins/ModularSessionKeyPlugin.t.sol similarity index 78% rename from test/plugin/SessionKeyPlugin.t.sol rename to test/samples/plugins/ModularSessionKeyPlugin.t.sol index 6b7cb157..10edc483 100644 --- a/test/plugin/SessionKeyPlugin.t.sol +++ b/test/samples/plugins/ModularSessionKeyPlugin.t.sol @@ -7,25 +7,25 @@ import {EntryPoint} from "@eth-infinitism/account-abstraction/core/EntryPoint.so import {UserOperation} from "@eth-infinitism/account-abstraction/interfaces/UserOperation.sol"; import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; -import {SingleOwnerPlugin} from "../../src/plugins/owner/SingleOwnerPlugin.sol"; -import {ISingleOwnerPlugin} from "../../src/plugins/owner/ISingleOwnerPlugin.sol"; -import {BaseSessionKeyPlugin} from "../../src/plugins/session-key/BaseSessionKeyPlugin.sol"; -import {ISessionKeyPlugin} from "../../src/plugins/session-key/interfaces/ISessionKeyPlugin.sol"; -import {TokenSessionKeyPlugin} from "../../src/plugins/session-key/TokenSessionKeyPlugin.sol"; -import {ITokenSessionKeyPlugin} from "../../src/plugins/session-key/interfaces/ITokenSessionKeyPlugin.sol"; - -import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; -import {MSCAFactoryFixture} from "../mocks/MSCAFactoryFixture.sol"; -import {FunctionReference, FunctionReferenceLib} from "../../src/libraries/FunctionReferenceLib.sol"; -import {IPluginManager} from "../../src/interfaces/IPluginManager.sol"; -import {MockERC20} from "../mocks/MockERC20.sol"; - -contract SessionKeyPluginTest is Test { +import {SingleOwnerPlugin} from "../../../src/plugins/owner/SingleOwnerPlugin.sol"; +import {ISingleOwnerPlugin} from "../../../src/plugins/owner/ISingleOwnerPlugin.sol"; +import {ModularSessionKeyPlugin} from "../../../src/samples/plugins/ModularSessionKeyPlugin.sol"; +import {ISessionKeyPlugin} from "../../../src/samples/plugins/interfaces/ISessionKeyPlugin.sol"; +import {TokenSessionKeyPlugin} from "../../../src/samples/plugins/TokenSessionKeyPlugin.sol"; +import {ITokenSessionKeyPlugin} from "../../../src/samples/plugins/interfaces/ITokenSessionKeyPlugin.sol"; + +import {UpgradeableModularAccount} from "../../../src/account/UpgradeableModularAccount.sol"; +import {MSCAFactoryFixture} from "../../mocks/MSCAFactoryFixture.sol"; +import {FunctionReference, FunctionReferenceLib} from "../../../src/libraries/FunctionReferenceLib.sol"; +import {IPluginManager} from "../../../src/interfaces/IPluginManager.sol"; +import {MockERC20} from "../../mocks/MockERC20.sol"; + +contract ModularSessionKeyPluginTest is Test { using ECDSA for bytes32; using FunctionReferenceLib for address; SingleOwnerPlugin public ownerPlugin; - BaseSessionKeyPlugin public baseSessionKeyPlugin; + ModularSessionKeyPlugin public modularSessionKeyPlugin; TokenSessionKeyPlugin public tokenSessionKeyPlugin; EntryPoint public entryPoint; MSCAFactoryFixture public factory; @@ -73,7 +73,7 @@ contract SessionKeyPluginTest is Test { function setUp() public { ownerPlugin = new SingleOwnerPlugin(); - baseSessionKeyPlugin = new BaseSessionKeyPlugin(); + modularSessionKeyPlugin = new ModularSessionKeyPlugin(); tokenSessionKeyPlugin = new TokenSessionKeyPlugin(); entryPoint = new EntryPoint(); @@ -104,30 +104,13 @@ contract SessionKeyPluginTest is Test { vm.deal(address(account), 1 ether); vm.startPrank(owner); - FunctionReference[] memory baseSessionDependency = new FunctionReference[](2); - baseSessionDependency[0] = + FunctionReference[] memory modularSessionDependency = new FunctionReference[](2); + modularSessionDependency[0] = address(ownerPlugin).pack(uint8(ISingleOwnerPlugin.FunctionId.USER_OP_VALIDATION_OWNER)); - baseSessionDependency[1] = + modularSessionDependency[1] = address(ownerPlugin).pack(uint8(ISingleOwnerPlugin.FunctionId.RUNTIME_VALIDATION_OWNER_OR_SELF)); - bytes32 baseSessionKeyManifestHash = keccak256(abi.encode(baseSessionKeyPlugin.pluginManifest())); - - account.installPlugin({ - plugin: address(baseSessionKeyPlugin), - manifestHash: baseSessionKeyManifestHash, - pluginInitData: "", - dependencies: baseSessionDependency, - injectedHooks: new IPluginManager.InjectedHook[](0) - }); - - FunctionReference[] memory tokenSessionDependency = new FunctionReference[](2); - tokenSessionDependency[0] = address(baseSessionKeyPlugin).pack( - uint8(ISessionKeyPlugin.FunctionId.USER_OP_VALIDATION_TEMPORARY_OWNER) - ); - tokenSessionDependency[1] = address(baseSessionKeyPlugin).pack( - uint8(ISessionKeyPlugin.FunctionId.RUNTIME_VALIDATION_TEMPORARY_OWNER) - ); - bytes32 tokenSessionKeyManifestHash = keccak256(abi.encode(tokenSessionKeyPlugin.pluginManifest())); + bytes32 modularSessionKeyManifestHash = keccak256(abi.encode(modularSessionKeyPlugin.pluginManifest())); address[] memory tempOwners = new address[](1); tempOwners[0] = address(tempOwner); @@ -141,14 +124,29 @@ contract SessionKeyPluginTest is Test { uint48[] memory untils = new uint48[](1); untils[0] = 2; - bytes memory data = abi.encodeWithSelector( - ISessionKeyPlugin.addTemporaryOwnerBatch.selector, tempOwners, allowedSelectors, afters, untils + bytes memory data = abi.encode(tempOwners, allowedSelectors, afters, untils); + + account.installPlugin({ + plugin: address(modularSessionKeyPlugin), + manifestHash: modularSessionKeyManifestHash, + pluginInitData: data, + dependencies: modularSessionDependency, + injectedHooks: new IPluginManager.InjectedHook[](0) + }); + + FunctionReference[] memory tokenSessionDependency = new FunctionReference[](2); + tokenSessionDependency[0] = address(modularSessionKeyPlugin).pack( + uint8(ISessionKeyPlugin.FunctionId.USER_OP_VALIDATION_TEMPORARY_OWNER) ); + tokenSessionDependency[1] = address(modularSessionKeyPlugin).pack( + uint8(ISessionKeyPlugin.FunctionId.RUNTIME_VALIDATION_TEMPORARY_OWNER) + ); + bytes32 tokenSessionKeyManifestHash = keccak256(abi.encode(tokenSessionKeyPlugin.pluginManifest())); account.installPlugin({ plugin: address(tokenSessionKeyPlugin), manifestHash: tokenSessionKeyManifestHash, - pluginInitData: data, + pluginInitData: "", dependencies: tokenSessionDependency, injectedHooks: new IPluginManager.InjectedHook[](0) }); @@ -157,12 +155,9 @@ contract SessionKeyPluginTest is Test { vm.startPrank(address(account)); mockERC20.approve(address(account), 1 ether); - // vm.expectEmit(true, true, true, true); - // emit TemporaryOwnerAdded(address(account), tempOwner, TRANSFERFROM_SESSIONKEY_SELECTOR, 0, 2); - // baseSessionKeyPlugin.addTemporaryOwner(tempOwner, TRANSFERFROM_SESSIONKEY_SELECTOR, 0, 2); - - (uint48 _after, uint48 _until) = - baseSessionKeyPlugin.getSessionDuration(address(account), tempOwner, TRANSFERFROM_SESSIONKEY_SELECTOR); + (uint48 _after, uint48 _until) = modularSessionKeyPlugin.getSessionDuration( + address(account), tempOwner, TRANSFERFROM_SESSIONKEY_SELECTOR + ); assertEq(_after, 0); assertEq(_until, 2); @@ -193,11 +188,11 @@ contract SessionKeyPluginTest is Test { vm.expectEmit(true, true, true, true); emit TemporaryOwnersAdded(address(account), tempOwners, allowedSelectors, afters, untils); - baseSessionKeyPlugin.addTemporaryOwnerBatch(tempOwners, allowedSelectors, afters, untils); + modularSessionKeyPlugin.addTemporaryOwnerBatch(tempOwners, allowedSelectors, afters, untils); vm.expectEmit(true, true, true, true); emit TemporaryOwnersRemoved(address(account), tempOwners, allowedSelectors); - baseSessionKeyPlugin.removeTemporaryOwnerBatch(tempOwners, allowedSelectors); + modularSessionKeyPlugin.removeTemporaryOwnerBatch(tempOwners, allowedSelectors); vm.stopPrank(); } @@ -228,12 +223,13 @@ contract SessionKeyPluginTest is Test { vm.expectEmit(true, true, true, true); emit TemporaryOwnerRemoved(address(account), tempOwner, TRANSFERFROM_SESSIONKEY_SELECTOR); - baseSessionKeyPlugin.removeTemporaryOwner(tempOwner, TRANSFERFROM_SESSIONKEY_SELECTOR); + modularSessionKeyPlugin.removeTemporaryOwner(tempOwner, TRANSFERFROM_SESSIONKEY_SELECTOR); vm.stopPrank(); - (uint48 _after, uint48 _until) = - baseSessionKeyPlugin.getSessionDuration(address(account), tempOwner, TRANSFERFROM_SESSIONKEY_SELECTOR); + (uint48 _after, uint48 _until) = modularSessionKeyPlugin.getSessionDuration( + address(account), tempOwner, TRANSFERFROM_SESSIONKEY_SELECTOR + ); assertEq(_after, 0); assertEq(_until, 0); @@ -244,7 +240,7 @@ contract SessionKeyPluginTest is Test { vm.expectRevert( abi.encodeWithSelector( UpgradeableModularAccount.RuntimeValidationFunctionReverted.selector, - address(baseSessionKeyPlugin), + address(modularSessionKeyPlugin), ISessionKeyPlugin.FunctionId.RUNTIME_VALIDATION_TEMPORARY_OWNER, revertReason ) @@ -285,7 +281,7 @@ contract SessionKeyPluginTest is Test { vm.expectRevert( abi.encodeWithSelector( UpgradeableModularAccount.RuntimeValidationFunctionReverted.selector, - address(baseSessionKeyPlugin), + address(modularSessionKeyPlugin), ISessionKeyPlugin.FunctionId.RUNTIME_VALIDATION_TEMPORARY_OWNER, revertReason ) @@ -306,7 +302,7 @@ contract SessionKeyPluginTest is Test { vm.expectRevert( abi.encodeWithSelector( UpgradeableModularAccount.RuntimeValidationFunctionReverted.selector, - address(baseSessionKeyPlugin), + address(modularSessionKeyPlugin), ISessionKeyPlugin.FunctionId.RUNTIME_VALIDATION_TEMPORARY_OWNER, revertReason ) @@ -316,26 +312,33 @@ contract SessionKeyPluginTest is Test { ); } - function test_sessionKey_uninstallTokenSessionKeyPlugin() public { + function test_sessionKey_uninstallModularSessionKeyPlugin() public { address[] memory tempOwners = new address[](1); tempOwners[0] = address(tempOwner); bytes4[] memory allowedSelectors = new bytes4[](1); allowedSelectors[0] = TRANSFERFROM_SESSIONKEY_SELECTOR; - bytes memory data = abi.encodeWithSelector( - ISessionKeyPlugin.removeTemporaryOwnerBatch.selector, tempOwners, allowedSelectors - ); + bytes memory data = + abi.encode(tempOwners, allowedSelectors); vm.startPrank(owner); vm.expectEmit(true, true, true, true); - // The second parameter should be true, but permittedExternalSelectors is disabled - // in the previous step of uninstallation process, resulting in reversion of onUninstall(). - emit PluginUninstalled(address(tokenSessionKeyPlugin), false); + + emit PluginUninstalled(address(tokenSessionKeyPlugin), true); account.uninstallPlugin({ plugin: address(tokenSessionKeyPlugin), config: bytes(""), + pluginUninstallData: "", + hookUnapplyData: new bytes[](0) + }); + + vm.expectEmit(true, true, true, true); + emit PluginUninstalled(address(modularSessionKeyPlugin), true); + account.uninstallPlugin({ + plugin: address(modularSessionKeyPlugin), + config: bytes(""), pluginUninstallData: data, hookUnapplyData: new bytes[](0) }); From 76aa43317c56691ed951b3c50ac325a037f8d722 Mon Sep 17 00:00:00 2001 From: sm-stack Date: Mon, 8 Jan 2024 13:24:29 +0900 Subject: [PATCH 21/33] Fix nits with forge fmt --- test/samples/plugins/ModularSessionKeyPlugin.t.sol | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/samples/plugins/ModularSessionKeyPlugin.t.sol b/test/samples/plugins/ModularSessionKeyPlugin.t.sol index 10edc483..ed2b660e 100644 --- a/test/samples/plugins/ModularSessionKeyPlugin.t.sol +++ b/test/samples/plugins/ModularSessionKeyPlugin.t.sol @@ -319,8 +319,7 @@ contract ModularSessionKeyPluginTest is Test { bytes4[] memory allowedSelectors = new bytes4[](1); allowedSelectors[0] = TRANSFERFROM_SESSIONKEY_SELECTOR; - bytes memory data = - abi.encode(tempOwners, allowedSelectors); + bytes memory data = abi.encode(tempOwners, allowedSelectors); vm.startPrank(owner); From 610533ce696b63d4b815047f7bc00a247176c988 Mon Sep 17 00:00:00 2001 From: sm-stack Date: Thu, 11 Jan 2024 10:06:05 +0900 Subject: [PATCH 22/33] Fix the implementation and test with new PluginStorageLib --- .../plugins/ModularSessionKeyPlugin.sol | 62 +++-- .../plugins/libraries/PluginStorageLib.sol | 254 ------------------ .../plugins/ModularSessionKeyPlugin.t.sol | 13 +- 3 files changed, 49 insertions(+), 280 deletions(-) delete mode 100644 src/samples/plugins/libraries/PluginStorageLib.sol diff --git a/src/samples/plugins/ModularSessionKeyPlugin.sol b/src/samples/plugins/ModularSessionKeyPlugin.sol index c7dda6a0..84fb2e6c 100644 --- a/src/samples/plugins/ModularSessionKeyPlugin.sol +++ b/src/samples/plugins/ModularSessionKeyPlugin.sol @@ -16,7 +16,7 @@ import {BasePlugin} from "../../plugins/BasePlugin.sol"; import {ISessionKeyPlugin} from "./interfaces/ISessionKeyPlugin.sol"; import {ISingleOwnerPlugin} from "../../plugins/owner/ISingleOwnerPlugin.sol"; import {SingleOwnerPlugin} from "../../plugins/owner/SingleOwnerPlugin.sol"; -import {PluginStorageLib} from "./libraries/PluginStorageLib.sol"; +import {PluginStorageLib, StoragePointer} from "../../libraries/PluginStorageLib.sol"; /// @title Modular Session Key Plugin /// @author Decipher ERC-6900 Team @@ -33,6 +33,7 @@ import {PluginStorageLib} from "./libraries/PluginStorageLib.sol"; contract ModularSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { using ECDSA for bytes32; using PluginStorageLib for address; + using PluginStorageLib for bytes; string public constant NAME = "Modular Session Key Plugin"; string public constant VERSION = "1.0.0"; @@ -40,6 +41,11 @@ contract ModularSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { uint256 internal constant _SIG_VALIDATION_FAILED = 1; + struct SessionInfo { + uint48 _after; + uint48 _until; + } + // ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ // ┃ Execution functions ┃ // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ @@ -97,9 +103,11 @@ contract ModularSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { view returns (uint48 _after, uint48 _until) { - bytes memory timeRange = - address(account).readBytesChecked(keccak256(abi.encodePacked(tempOwner, allowedSelector))); - (_after, _until) = _decode(timeRange); + bytes memory key = account.allocateAssociatedStorageKey(0, 1); + StoragePointer ptr = key.associatedStorageLookup(keccak256(abi.encodePacked(tempOwner, allowedSelector))); + SessionInfo storage sessionInfo = _castPtrToStruct(ptr); + _after = sessionInfo._after; + _until = sessionInfo._until; } // ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ @@ -151,11 +159,13 @@ contract ModularSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { if (functionId == uint8(FunctionId.USER_OP_VALIDATION_TEMPORARY_OWNER)) { (address signer,) = (userOpHash.toEthSignedMessageHash()).tryRecover(userOp.signature); bytes4 selector = bytes4(userOp.callData[0:4]); - bytes32 key = keccak256(abi.encodePacked(signer, selector)); - bytes memory duration = address(userOp.sender).readBytesChecked(key); - if (duration.length != 0) { - (uint48 _after, uint48 _until) = _decode(duration); - // first parameter of _packValidationData is sigFailed, which should be false + bytes memory key = userOp.sender.allocateAssociatedStorageKey(0, 1); + StoragePointer ptr = key.associatedStorageLookup(keccak256(abi.encodePacked(signer, selector))); + SessionInfo storage duration = _castPtrToStruct(ptr); + uint48 _after = duration._after; + uint48 _until = duration._until; + + if (_until != 0) { return _packValidationData(false, _until, _after); } else { return _SIG_VALIDATION_FAILED; @@ -172,10 +182,13 @@ contract ModularSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { { if (functionId == uint8(FunctionId.RUNTIME_VALIDATION_TEMPORARY_OWNER)) { bytes4 selector = bytes4(data[0:4]); - bytes32 key = keccak256(abi.encodePacked(sender, selector)); - bytes memory duration = address(msg.sender).readBytesChecked(key); - if (duration.length != 0) { - (uint48 _after, uint48 _until) = _decode(duration); + bytes memory key = address(msg.sender).allocateAssociatedStorageKey(0, 1); + StoragePointer ptr = key.associatedStorageLookup(keccak256(abi.encodePacked(sender, selector))); + SessionInfo storage duration = _castPtrToStruct(ptr); + uint48 _after = duration._after; + uint48 _until = duration._until; + + if (_until != 0) { if (block.timestamp < _after || block.timestamp > _until) { revert WrongTimeRangeForSession(); } @@ -294,21 +307,24 @@ contract ModularSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { if (_until <= _after) { revert WrongTimeRangeForSession(); } - bytes32 key = keccak256(abi.encodePacked(tempOwner, allowedSelector)); - bytes memory sessionInfo = abi.encodePacked(_after, _until); - address(account).writeBytesChecked(key, sessionInfo); + bytes memory key = account.allocateAssociatedStorageKey(0, 1); + StoragePointer ptr = key.associatedStorageLookup(keccak256(abi.encodePacked(tempOwner, allowedSelector))); + SessionInfo storage sessionInfo = _castPtrToStruct(ptr); + sessionInfo._after = _after; + sessionInfo._until = _until; } function _removeTemporaryOwner(address account, address tempOwner, bytes4 allowedSelector) internal { - bytes32 key = keccak256(abi.encodePacked(tempOwner, allowedSelector)); - bytes memory emptyBytes = new bytes(0); - address(account).writeBytesChecked(key, emptyBytes); + bytes memory key = account.allocateAssociatedStorageKey(0, 1); + StoragePointer ptr = key.associatedStorageLookup(keccak256(abi.encodePacked(tempOwner, allowedSelector))); + SessionInfo storage sessionInfo = _castPtrToStruct(ptr); + sessionInfo._after = 0; + sessionInfo._until = 0; } - function _decode(bytes memory _data) internal pure returns (uint48 _after, uint48 _until) { - assembly { - _after := mload(add(_data, 0x06)) - _until := mload(add(_data, 0x0C)) + function _castPtrToStruct(StoragePointer ptr) internal pure returns (SessionInfo storage val) { + assembly ("memory-safe") { + val.slot := ptr } } diff --git a/src/samples/plugins/libraries/PluginStorageLib.sol b/src/samples/plugins/libraries/PluginStorageLib.sol deleted file mode 100644 index cae0ba7d..00000000 --- a/src/samples/plugins/libraries/PluginStorageLib.sol +++ /dev/null @@ -1,254 +0,0 @@ -/// SPDX-License-Identifier: UNLICENSED -pragma solidity ^0.8.19; - -/// @title Plugin Storage Library for ERC-4337 Address-associated Storage -/// @author Adam Egyed -/// @notice This library treats storage available to associated addresses as one big global mapping of (bytes32 => -/// bytes). -/// -/// THIS IS HIGHLY EXPERIMENTAL AND NOT READY FOR PRODUCTION USE. -/// -/// It is up to the implementer to define the serialization and deserialization of structs, -/// since Solidity itself does not provide ways to encode structs into their storage representations easily. -/// While you can use `abi.encode` and `abi.decode`, this is not recommended because it will result in -/// extraneous data being stored in storage, which will unreasonably increase gas costs. -library PluginStorageLib { - /// @notice Writes a bytes array to storage using a key and an associated address. - /// @notice This function will write the length of the bytes array to storage. - /// @param addr The address associated with the storage - /// @param key The key used to identify the bytes array - /// @param val The bytes array to write to storage - function writeBytesChecked(address addr, bytes32 key, bytes memory val) internal { - assembly ("memory-safe") { - // compute total length, including the extra word for the length field itself - let len := add(mload(val), 32) - - // reserve 3 words in memory (96 bytes) for hash inputs - let hashInput := mload(0x40) - mstore(0x40, add(hashInput, 96)) - // Hash inputs will always be: - // 1. caller address - // 2. key - // 3. batch index - // So we can set the caller address and key here to reuse, - // but we'll need to set the batch index for each batch - mstore(hashInput, addr) - mstore(add(hashInput, 32), key) - - // Compute the number of batches we need to write, rounded up - let batches := div(add(len, 4095), 4096) - - // Copy the bytes array into storage - for { let batchIndex := 0 } lt(batchIndex, batches) { batchIndex := add(batchIndex, 1) } { - // Hash the batch index with the caller address and key to get a new 128 associated slots - mstore(add(hashInput, 64), batchIndex) - let batchStart := keccak256(hashInput, 96) - - // Write the batch to storage, 128 slots at a time - let end := false - for { let slotIndex := 0 } and(lt(slotIndex, 128), not(end)) { slotIndex := add(slotIndex, 1) } { - // Compute the current slot in storage, and the current offset in memory - let slot := add(batchStart, slotIndex) - let offset := add(mul(slotIndex, 32), mul(batchIndex, 4096)) - - // Is this the last word we need to write? Stop one word before offset = len - end := iszero(sub(len, add(offset, 32))) - - // Copy the word from the bytes array into storage - let dataStart := add(val, offset) - let data := mload(dataStart) - sstore(slot, data) - } - } - } - } - - /// @notice Writes a bytes array to storage using a key and an associated address. - /// @notice This function will write NOT the length of the bytes array to storage, - /// but will use the length of the array to determine how much to write. - /// It is the responsibility of the caller to preserve length information. - /// @param addr The address associated with the storage - /// @param key The key used to identify the bytes array - /// @param val The bytes array to write to storage - function writeBytesUnchecked(address addr, bytes32 key, bytes memory val) internal { - assembly ("memory-safe") { - // compute total length, excluding the length field itself - let len := mload(val) - - // reserve 3 words in memory (96 bytes) for hash inputs - let hashInput := mload(0x40) - mstore(0x40, add(hashInput, 96)) - // Hash inputs will always be: - // 1. caller address - // 2. key - // 3. batch index - // So we can set the caller address and key here to reuse, - // but we'll need to set the batch index for each batch - mstore(hashInput, addr) - mstore(add(hashInput, 32), key) - - // Compute the number of batches we need to write, rounded up - let batches := div(add(len, 4095), 4096) - - // Copy the bytes array into storage - for { let batchIndex := 0 } lt(batchIndex, batches) { batchIndex := add(batchIndex, 1) } { - // Hash the batch index with the caller address and key to get a new 128 associated slots - mstore(add(hashInput, 64), batchIndex) - let batchStart := keccak256(hashInput, 96) - - // Write the batch to storage, 128 slots at a time - let end := false - for { let slotIndex := 0 } and(lt(slotIndex, 128), not(end)) { slotIndex := add(slotIndex, 1) } { - // Compute the current slot in storage, and the current offset in memory - let slot := add(batchStart, slotIndex) - let offset := add(mul(slotIndex, 32), mul(batchIndex, 4096)) - - // Is this the last word we need to write? Stop one word before offset = len - end := iszero(sub(len, add(offset, 32))) - - // Copy the word from the bytes array into storage - let dataStart := add(add(val, 32), offset) - let data := mload(dataStart) - sstore(slot, data) - } - } - } - } - - /// @notice Reads a bytes array from storage using a key and an associated address - /// @param addr The address associated with the storage - /// @param key The key used to identify the bytes array - /// @return ret The bytes array stored in storage - function readBytesChecked(address addr, bytes32 key) internal view returns (bytes memory ret) { - assembly ("memory-safe") { - // reserve 3 words in memory (96 bytes) for hash inputs - let hashInput := mload(0x40) - mstore(0x40, add(hashInput, 96)) - - // Hash inputs will always be: - // 1. caller address - // 2. key - // 3. batch index - // So we can set the caller address and key here to reuse, - // but we'll need to set the batch index for each batch - mstore(hashInput, addr) - mstore(add(hashInput, 32), key) - - // Get the length of the stored bytes array first, then copy everything else over - // Set the batch index to 0 to get the length only - mstore(add(hashInput, 64), 0) - let hash := keccak256(hashInput, 96) - // Include the extra word for the length field itself - let len := add(sload(hash), 32) - - // Allocate memory for the returned bytes array - ret := mload(0x40) - mstore(0x40, add(ret, len)) - - // Copy storage into the bytes array - let batches := div(add(len, 4095), 4096) // num batches rounded up - for { let batchIndex := 0 } lt(batchIndex, batches) { batchIndex := add(batchIndex, 1) } { - // Hash the batch index with the caller address and key to get a new 128 associated slots - mstore(add(hashInput, 64), batchIndex) - let batchStart := keccak256(hashInput, 96) - - // Read the batch from storage, 128 slots at a time - let end := false - for { let slotIndex := 0 } and(lt(slotIndex, 128), not(end)) { slotIndex := add(slotIndex, 1) } { - // Compute the current slot in storage, and the current offset in memory - let slot := add(batchStart, slotIndex) - let offset := add(mul(slotIndex, 32), mul(batchIndex, 4096)) - - // Is this the last word we need to read? Stop one word before offset = len - end := iszero(sub(len, add(offset, 32))) - - // Copy the data from storage to memory - let dataLoc := add(ret, offset) - mstore(dataLoc, sload(slot)) - } - } - } - } - - /// @notice Reads a bytes array from storage using a key and an associated address, of a specified length - /// @param addr The address associated with the storage - /// @param key The key used to identify the bytes array - /// @param len The length of the bytes array to read - /// @return ret The bytes array stored in storage - function readBytesUnchecked(address addr, bytes32 key, uint256 len) internal view returns (bytes memory ret) { - assembly ("memory-safe") { - // reserve 3 words in memory (96 bytes) for hash inputs - let hashInput := mload(0x40) - mstore(0x40, add(hashInput, 96)) - - // Hash inputs will always be: - // 1. caller address - // 2. key - // 3. batch index - // So we can set the caller address and key here to reuse, - // but we'll need to set the batch index for each batch - mstore(hashInput, addr) - mstore(add(hashInput, 32), key) - - // Get the length of the stored bytes array first, then copy everything else over - // Set the batch index to 0 to get the length only - mstore(add(hashInput, 64), 0) - let hash := keccak256(hashInput, 96) - - // Allocate memory for the returned bytes array - ret := mload(0x40) - // Include the extra word for the length field itself. - // The length field is not in storage, but must be set in the returned memory array. - mstore(0x40, add(ret, add(len, 32))) - - // Store the length of the bytes array in memory - mstore(ret, len) - - // Copy storage into the bytes array - let batches := div(add(len, 4095), 4096) // num batches rounded up - for { let batchIndex := 0 } lt(batchIndex, batches) { batchIndex := add(batchIndex, 1) } { - // Hash the batch index with the caller address and key to get a new 128 associated slots - mstore(add(hashInput, 64), batchIndex) - let batchStart := keccak256(hashInput, 96) - - // Read the batch from storage, 128 slots at a time - let end := false - for { let slotIndex := 0 } and(lt(slotIndex, 128), not(end)) { slotIndex := add(slotIndex, 1) } { - // Compute the current slot in storage, and the current offset in memory - let slot := add(batchStart, slotIndex) - let offset := add(mul(slotIndex, 32), mul(batchIndex, 4096)) - - // Is this the last word we need to read? Stop one word before offset = len - end := iszero(sub(len, add(offset, 32))) - - // Copy the data from storage to memory - // data location is the offset plus the length word - let dataLoc := add(add(ret, 32), offset) - mstore(dataLoc, sload(slot)) - } - } - } - } - - /// @notice Efficiently retrieve a full word from a bytes array in memory. - /// The returned value can be casted to any primitive type as needed. - /// @param val The bytes array to read from - /// @param index The index of the word to read - /// @return ret The word at the given index - function wordAt(bytes memory val, uint256 index) internal pure returns (bytes32 ret) { - assembly ("memory-safe") { - ret := mload(add(add(val, 32), mul(index, 32))) - } - } - - /// @notice Efficiently retrieve a single byte from a bytes32 word. - /// The returned value can be casted to any primitive type as needed. - /// @param val The word to read from - /// @param index The index of the byte to read - /// @return ret The byte at the given index - function byteAt(bytes32 val, uint8 index) internal pure returns (bytes1 ret) { - assembly ("memory-safe") { - ret := byte(index, val) - } - } -} diff --git a/test/samples/plugins/ModularSessionKeyPlugin.t.sol b/test/samples/plugins/ModularSessionKeyPlugin.t.sol index ed2b660e..a41d88bd 100644 --- a/test/samples/plugins/ModularSessionKeyPlugin.t.sol +++ b/test/samples/plugins/ModularSessionKeyPlugin.t.sol @@ -184,16 +184,23 @@ contract ModularSessionKeyPluginTest is Test { untils[0] = 2; untils[1] = 2; - vm.startPrank(address(account)); - vm.expectEmit(true, true, true, true); emit TemporaryOwnersAdded(address(account), tempOwners, allowedSelectors, afters, untils); + vm.prank(address(account)); modularSessionKeyPlugin.addTemporaryOwnerBatch(tempOwners, allowedSelectors, afters, untils); + vm.prank(tempOwner3); + TokenSessionKeyPlugin(address(account)).transferFromSessionKey( + address(mockERC20), address(account), target, 1 ether + ); + + assertEq(mockERC20.balanceOf(address(account)), 0); + assertEq(mockERC20.balanceOf(target), 1 ether); + vm.expectEmit(true, true, true, true); emit TemporaryOwnersRemoved(address(account), tempOwners, allowedSelectors); + vm.prank(address(account)); modularSessionKeyPlugin.removeTemporaryOwnerBatch(tempOwners, allowedSelectors); - vm.stopPrank(); } function test_sessionKey_userOp() public { From c38624c807072720bbb57339c6cae737b36f0be5 Mon Sep 17 00:00:00 2001 From: sm-stack Date: Sun, 21 Jan 2024 10:47:37 +0900 Subject: [PATCH 23/33] Update test codes to be compatible with v0.7 --- test/samples/plugins/ModularSessionKeyPlugin.t.sol | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/test/samples/plugins/ModularSessionKeyPlugin.t.sol b/test/samples/plugins/ModularSessionKeyPlugin.t.sol index a41d88bd..956bc65a 100644 --- a/test/samples/plugins/ModularSessionKeyPlugin.t.sol +++ b/test/samples/plugins/ModularSessionKeyPlugin.t.sol @@ -16,7 +16,7 @@ import {ITokenSessionKeyPlugin} from "../../../src/samples/plugins/interfaces/IT import {UpgradeableModularAccount} from "../../../src/account/UpgradeableModularAccount.sol"; import {MSCAFactoryFixture} from "../../mocks/MSCAFactoryFixture.sol"; -import {FunctionReference, FunctionReferenceLib} from "../../../src/libraries/FunctionReferenceLib.sol"; +import {FunctionReference, FunctionReferenceLib} from "../../../src/helpers/FunctionReferenceLib.sol"; import {IPluginManager} from "../../../src/interfaces/IPluginManager.sol"; import {MockERC20} from "../../mocks/MockERC20.sol"; @@ -130,8 +130,7 @@ contract ModularSessionKeyPluginTest is Test { plugin: address(modularSessionKeyPlugin), manifestHash: modularSessionKeyManifestHash, pluginInitData: data, - dependencies: modularSessionDependency, - injectedHooks: new IPluginManager.InjectedHook[](0) + dependencies: modularSessionDependency }); FunctionReference[] memory tokenSessionDependency = new FunctionReference[](2); @@ -147,8 +146,7 @@ contract ModularSessionKeyPluginTest is Test { plugin: address(tokenSessionKeyPlugin), manifestHash: tokenSessionKeyManifestHash, pluginInitData: "", - dependencies: tokenSessionDependency, - injectedHooks: new IPluginManager.InjectedHook[](0) + dependencies: tokenSessionDependency }); vm.stopPrank(); @@ -336,8 +334,7 @@ contract ModularSessionKeyPluginTest is Test { account.uninstallPlugin({ plugin: address(tokenSessionKeyPlugin), config: bytes(""), - pluginUninstallData: "", - hookUnapplyData: new bytes[](0) + pluginUninstallData: "" }); vm.expectEmit(true, true, true, true); @@ -345,8 +342,7 @@ contract ModularSessionKeyPluginTest is Test { account.uninstallPlugin({ plugin: address(modularSessionKeyPlugin), config: bytes(""), - pluginUninstallData: data, - hookUnapplyData: new bytes[](0) + pluginUninstallData: data }); vm.stopPrank(); From b59391c7d6ad6bcc0253eb6330f0697d6445f513 Mon Sep 17 00:00:00 2001 From: sm-stack Date: Sat, 27 Jan 2024 12:57:58 +0900 Subject: [PATCH 24/33] Update expressions and nits --- .../plugins/ModularSessionKeyPlugin.sol | 140 +++++++++--------- src/samples/plugins/TokenSessionKeyPlugin.sol | 6 +- .../plugins/interfaces/ISessionKeyPlugin.sol | 22 +-- .../plugins/ModularSessionKeyPlugin.t.sol | 49 +++--- 4 files changed, 106 insertions(+), 111 deletions(-) diff --git a/src/samples/plugins/ModularSessionKeyPlugin.sol b/src/samples/plugins/ModularSessionKeyPlugin.sol index 84fb2e6c..dd4576ed 100644 --- a/src/samples/plugins/ModularSessionKeyPlugin.sol +++ b/src/samples/plugins/ModularSessionKeyPlugin.sol @@ -13,7 +13,7 @@ import { SelectorPermission } from "../../interfaces/IPlugin.sol"; import {BasePlugin} from "../../plugins/BasePlugin.sol"; -import {ISessionKeyPlugin} from "./interfaces/ISessionKeyPlugin.sol"; +import {IModularSessionKeyPlugin} from "./interfaces/ISessionKeyPlugin.sol"; import {ISingleOwnerPlugin} from "../../plugins/owner/ISingleOwnerPlugin.sol"; import {SingleOwnerPlugin} from "../../plugins/owner/SingleOwnerPlugin.sol"; import {PluginStorageLib, StoragePointer} from "../../libraries/PluginStorageLib.sol"; @@ -21,7 +21,8 @@ import {PluginStorageLib, StoragePointer} from "../../libraries/PluginStorageLib /// @title Modular Session Key Plugin /// @author Decipher ERC-6900 Team /// @notice This plugin allows some designated EOA or smart contract to temporarily -/// own a modular account. +/// own a modular account. Note that this plugin is ONLY for demonstrating the purpose +/// of the functionalities of ERC-6900, and MUST not be used at the production level. /// This modular session key plugin acts as a 'parent plugin' for all specific session /// keys. Using dependency, this plugin can be thought as a parent contract that stores /// session key duration information, and validation functions for session keys. All @@ -30,7 +31,7 @@ import {PluginStorageLib, StoragePointer} from "../../libraries/PluginStorageLib /// runtime, with its own validation functions. /// Also, it has a dependency on SingleOwnerPlugin, to make sure that only the owner of /// the MSCA can add or remove session keys. -contract ModularSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { +contract ModularSessionKeyPlugin is BasePlugin, IModularSessionKeyPlugin { using ECDSA for bytes32; using PluginStorageLib for address; using PluginStorageLib for bytes; @@ -42,72 +43,70 @@ contract ModularSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { uint256 internal constant _SIG_VALIDATION_FAILED = 1; struct SessionInfo { - uint48 _after; - uint48 _until; + uint48 validAfter; + uint48 validUntil; } // ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ // ┃ Execution functions ┃ // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ - /// @inheritdoc ISessionKeyPlugin - function addTemporaryOwner(address tempOwner, bytes4 allowedSelector, uint48 _after, uint48 _until) external { - _addTemporaryOwner(msg.sender, tempOwner, allowedSelector, _after, _until); - emit TemporaryOwnerAdded(msg.sender, tempOwner, allowedSelector, _after, _until); + /// @inheritdoc IModularSessionKeyPlugin + function addSessionKey(address tempOwner, bytes4 allowedSelector, uint48 validAfter, uint48 validUntil) external { + _addSessionKey(msg.sender, tempOwner, allowedSelector, validAfter, validUntil); + emit SessionKeyAdded(msg.sender, tempOwner, allowedSelector, validAfter, validUntil); } - /// @inheritdoc ISessionKeyPlugin - function removeTemporaryOwner(address tempOwner, bytes4 allowedSelector) external { - _removeTemporaryOwner(msg.sender, tempOwner, allowedSelector); - emit TemporaryOwnerRemoved(msg.sender, tempOwner, allowedSelector); + /// @inheritdoc IModularSessionKeyPlugin + function removeSessionKey(address tempOwner, bytes4 allowedSelector) external { + _removeSessionKey(msg.sender, tempOwner, allowedSelector); + emit SessionKeyRemoved(msg.sender, tempOwner, allowedSelector); } - /// @inheritdoc ISessionKeyPlugin - function addTemporaryOwnerBatch( + /// @inheritdoc IModularSessionKeyPlugin + function addSessionKeyBatch( address[] calldata tempOwners, bytes4[] calldata allowedSelectors, - uint48[] calldata _afters, - uint48[] calldata _untils + uint48[] calldata validAfters, + uint48[] calldata validUntils ) external { if ( - tempOwners.length != allowedSelectors.length || tempOwners.length != _afters.length - || tempOwners.length != _untils.length + tempOwners.length != allowedSelectors.length || tempOwners.length != validAfters.length + || tempOwners.length != validUntils.length ) { revert WrongDataLength(); } for (uint256 i = 0; i < tempOwners.length; i++) { - _addTemporaryOwner(msg.sender, tempOwners[i], allowedSelectors[i], _afters[i], _untils[i]); + _addSessionKey(msg.sender, tempOwners[i], allowedSelectors[i], validAfters[i], validUntils[i]); } - emit TemporaryOwnersAdded(msg.sender, tempOwners, allowedSelectors, _afters, _untils); + emit SessionKeysAdded(msg.sender, tempOwners, allowedSelectors, validAfters, validUntils); } - function removeTemporaryOwnerBatch(address[] calldata tempOwners, bytes4[] calldata allowedSelectors) - external - { + function removeSessionKeyBatch(address[] calldata tempOwners, bytes4[] calldata allowedSelectors) external { if (tempOwners.length != allowedSelectors.length) { revert WrongDataLength(); } for (uint256 i = 0; i < tempOwners.length; i++) { - _removeTemporaryOwner(msg.sender, tempOwners[i], allowedSelectors[i]); + _removeSessionKey(msg.sender, tempOwners[i], allowedSelectors[i]); } - emit TemporaryOwnersRemoved(msg.sender, tempOwners, allowedSelectors); + emit SessionKeysRemoved(msg.sender, tempOwners, allowedSelectors); } // ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ // ┃ Plugin view functions ┃ // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ - /// @inheritdoc ISessionKeyPlugin + /// @inheritdoc IModularSessionKeyPlugin function getSessionDuration(address account, address tempOwner, bytes4 allowedSelector) external view - returns (uint48 _after, uint48 _until) + returns (uint48 _validAfter, uint48 _validUntil) { bytes memory key = account.allocateAssociatedStorageKey(0, 1); StoragePointer ptr = key.associatedStorageLookup(keccak256(abi.encodePacked(tempOwner, allowedSelector))); SessionInfo storage sessionInfo = _castPtrToStruct(ptr); - _after = sessionInfo._after; - _until = sessionInfo._until; + _validAfter = sessionInfo.validAfter; + _validUntil = sessionInfo.validUntil; } // ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ @@ -120,17 +119,17 @@ contract ModularSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { ( address[] memory tempOwners, bytes4[] memory allowedSelectors, - uint48[] memory _afters, - uint48[] memory _untils + uint48[] memory validAfters, + uint48[] memory validUntils ) = abi.decode(data, (address[], bytes4[], uint48[], uint48[])); if ( - tempOwners.length != allowedSelectors.length || tempOwners.length != _afters.length - || tempOwners.length != _untils.length + tempOwners.length != allowedSelectors.length || tempOwners.length != validAfters.length + || tempOwners.length != validUntils.length ) { revert WrongDataLength(); } for (uint256 i = 0; i < tempOwners.length; i++) { - _addTemporaryOwner(msg.sender, tempOwners[i], allowedSelectors[i], _afters[i], _untils[i]); + _addSessionKey(msg.sender, tempOwners[i], allowedSelectors[i], validAfters[i], validUntils[i]); } } } @@ -144,7 +143,7 @@ contract ModularSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { revert WrongDataLength(); } for (uint256 i = 0; i < tempOwners.length; i++) { - _removeTemporaryOwner(msg.sender, tempOwners[i], allowedSelectors[i]); + _removeSessionKey(msg.sender, tempOwners[i], allowedSelectors[i]); } } } @@ -157,19 +156,18 @@ contract ModularSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { returns (uint256) { if (functionId == uint8(FunctionId.USER_OP_VALIDATION_TEMPORARY_OWNER)) { - (address signer,) = (userOpHash.toEthSignedMessageHash()).tryRecover(userOp.signature); + (address signer,) = userOpHash.toEthSignedMessageHash().tryRecover(userOp.signature); bytes4 selector = bytes4(userOp.callData[0:4]); bytes memory key = userOp.sender.allocateAssociatedStorageKey(0, 1); StoragePointer ptr = key.associatedStorageLookup(keccak256(abi.encodePacked(signer, selector))); SessionInfo storage duration = _castPtrToStruct(ptr); - uint48 _after = duration._after; - uint48 _until = duration._until; + uint48 validAfter = duration.validAfter; + uint48 validUntil = duration.validUntil; - if (_until != 0) { - return _packValidationData(false, _until, _after); - } else { - return _SIG_VALIDATION_FAILED; + if (validUntil != 0) { + return _packValidationData(false, validUntil, validAfter); } + return _SIG_VALIDATION_FAILED; } revert NotImplemented(); } @@ -185,11 +183,11 @@ contract ModularSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { bytes memory key = address(msg.sender).allocateAssociatedStorageKey(0, 1); StoragePointer ptr = key.associatedStorageLookup(keccak256(abi.encodePacked(sender, selector))); SessionInfo storage duration = _castPtrToStruct(ptr); - uint48 _after = duration._after; - uint48 _until = duration._until; + uint48 validAfter = duration.validAfter; + uint48 validUntil = duration.validUntil; - if (_until != 0) { - if (block.timestamp < _after || block.timestamp > _until) { + if (validUntil != 0) { + if (block.timestamp < validAfter || block.timestamp > validUntil) { revert WrongTimeRangeForSession(); } return; @@ -205,10 +203,10 @@ contract ModularSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { PluginManifest memory manifest; manifest.executionFunctions = new bytes4[](5); - manifest.executionFunctions[0] = this.addTemporaryOwner.selector; - manifest.executionFunctions[1] = this.removeTemporaryOwner.selector; - manifest.executionFunctions[2] = this.addTemporaryOwnerBatch.selector; - manifest.executionFunctions[3] = this.removeTemporaryOwnerBatch.selector; + manifest.executionFunctions[0] = this.addSessionKey.selector; + manifest.executionFunctions[1] = this.removeSessionKey.selector; + manifest.executionFunctions[2] = this.addSessionKeyBatch.selector; + manifest.executionFunctions[3] = this.removeSessionKeyBatch.selector; manifest.executionFunctions[4] = this.getSessionDuration.selector; ManifestFunction memory ownerUserOpValidationFunction = ManifestFunction({ @@ -218,19 +216,19 @@ contract ModularSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { }); manifest.userOpValidationFunctions = new ManifestAssociatedFunction[](4); manifest.userOpValidationFunctions[0] = ManifestAssociatedFunction({ - executionSelector: this.addTemporaryOwner.selector, + executionSelector: this.addSessionKey.selector, associatedFunction: ownerUserOpValidationFunction }); manifest.userOpValidationFunctions[1] = ManifestAssociatedFunction({ - executionSelector: this.removeTemporaryOwner.selector, + executionSelector: this.removeSessionKey.selector, associatedFunction: ownerUserOpValidationFunction }); manifest.userOpValidationFunctions[2] = ManifestAssociatedFunction({ - executionSelector: this.addTemporaryOwnerBatch.selector, + executionSelector: this.addSessionKeyBatch.selector, associatedFunction: ownerUserOpValidationFunction }); manifest.userOpValidationFunctions[3] = ManifestAssociatedFunction({ - executionSelector: this.removeTemporaryOwnerBatch.selector, + executionSelector: this.removeSessionKeyBatch.selector, associatedFunction: ownerUserOpValidationFunction }); @@ -247,19 +245,19 @@ contract ModularSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { manifest.runtimeValidationFunctions = new ManifestAssociatedFunction[](5); manifest.runtimeValidationFunctions[0] = ManifestAssociatedFunction({ - executionSelector: this.addTemporaryOwner.selector, + executionSelector: this.addSessionKey.selector, associatedFunction: ownerOrSelfRuntimeValidationFunction }); manifest.runtimeValidationFunctions[1] = ManifestAssociatedFunction({ - executionSelector: this.removeTemporaryOwner.selector, + executionSelector: this.removeSessionKey.selector, associatedFunction: ownerOrSelfRuntimeValidationFunction }); manifest.runtimeValidationFunctions[2] = ManifestAssociatedFunction({ - executionSelector: this.addTemporaryOwnerBatch.selector, + executionSelector: this.addSessionKeyBatch.selector, associatedFunction: ownerOrSelfRuntimeValidationFunction }); manifest.runtimeValidationFunctions[3] = ManifestAssociatedFunction({ - executionSelector: this.removeTemporaryOwnerBatch.selector, + executionSelector: this.removeSessionKeyBatch.selector, associatedFunction: ownerOrSelfRuntimeValidationFunction }); manifest.runtimeValidationFunctions[4] = ManifestAssociatedFunction({ @@ -290,36 +288,32 @@ contract ModularSessionKeyPlugin is BasePlugin, ISessionKeyPlugin { /// @inheritdoc BasePlugin function supportsInterface(bytes4 interfaceId) public view override returns (bool) { - return interfaceId == type(ISessionKeyPlugin).interfaceId || super.supportsInterface(interfaceId); + return interfaceId == type(IModularSessionKeyPlugin).interfaceId || super.supportsInterface(interfaceId); } // ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ // ┃ Internal / Private functions ┃ // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ - function _addTemporaryOwner( - address account, - address tempOwner, - bytes4 allowedSelector, - uint48 _after, - uint48 _until - ) internal { - if (_until <= _after) { + function _addSessionKey(address account, address tempOwner, bytes4 allowedSelector, uint48 _validAfter, uint48 _validUntil) + internal + { + if (_validUntil <= _validAfter) { revert WrongTimeRangeForSession(); } bytes memory key = account.allocateAssociatedStorageKey(0, 1); StoragePointer ptr = key.associatedStorageLookup(keccak256(abi.encodePacked(tempOwner, allowedSelector))); SessionInfo storage sessionInfo = _castPtrToStruct(ptr); - sessionInfo._after = _after; - sessionInfo._until = _until; + sessionInfo.validAfter = _validAfter; + sessionInfo.validUntil = _validUntil; } - function _removeTemporaryOwner(address account, address tempOwner, bytes4 allowedSelector) internal { + function _removeSessionKey(address account, address tempOwner, bytes4 allowedSelector) internal { bytes memory key = account.allocateAssociatedStorageKey(0, 1); StoragePointer ptr = key.associatedStorageLookup(keccak256(abi.encodePacked(tempOwner, allowedSelector))); SessionInfo storage sessionInfo = _castPtrToStruct(ptr); - sessionInfo._after = 0; - sessionInfo._until = 0; + sessionInfo.validAfter = 0; + sessionInfo.validUntil = 0; } function _castPtrToStruct(StoragePointer ptr) internal pure returns (SessionInfo storage val) { diff --git a/src/samples/plugins/TokenSessionKeyPlugin.sol b/src/samples/plugins/TokenSessionKeyPlugin.sol index ba2d11ee..53aac1aa 100644 --- a/src/samples/plugins/TokenSessionKeyPlugin.sol +++ b/src/samples/plugins/TokenSessionKeyPlugin.sol @@ -13,7 +13,7 @@ import { import {BasePlugin} from "../../plugins/BasePlugin.sol"; import {ModularSessionKeyPlugin} from "./ModularSessionKeyPlugin.sol"; import {ITokenSessionKeyPlugin} from "./interfaces/ITokenSessionKeyPlugin.sol"; -import {ISessionKeyPlugin} from "./interfaces/ISessionKeyPlugin.sol"; +import {IModularSessionKeyPlugin} from "./interfaces/ISessionKeyPlugin.sol"; import {IPluginExecutor} from "../../interfaces/IPluginExecutor.sol"; /// @title Token Session Key Plugin @@ -71,7 +71,7 @@ contract TokenSessionKeyPlugin is BasePlugin, ITokenSessionKeyPlugin { ManifestFunction memory tempOwnerUserOpValidationFunction = ManifestFunction({ functionType: ManifestAssociatedFunctionType.DEPENDENCY, - functionId: uint8(ISessionKeyPlugin.FunctionId.RUNTIME_VALIDATION_TEMPORARY_OWNER), + functionId: 0, // Unused dependencyIndex: 0 // Used as first index }); ManifestFunction memory tempOwnerRuntimeValidationFunction = ManifestFunction({ @@ -94,7 +94,7 @@ contract TokenSessionKeyPlugin is BasePlugin, ITokenSessionKeyPlugin { manifest.dependencyInterfaceIds = new bytes4[](2); for (uint256 i = 0; i < manifest.dependencyInterfaceIds.length;) { - manifest.dependencyInterfaceIds[i] = type(ISessionKeyPlugin).interfaceId; + manifest.dependencyInterfaceIds[i] = type(IModularSessionKeyPlugin).interfaceId; unchecked { ++i; } diff --git a/src/samples/plugins/interfaces/ISessionKeyPlugin.sol b/src/samples/plugins/interfaces/ISessionKeyPlugin.sol index a369d318..005b7946 100644 --- a/src/samples/plugins/interfaces/ISessionKeyPlugin.sol +++ b/src/samples/plugins/interfaces/ISessionKeyPlugin.sol @@ -3,7 +3,7 @@ pragma solidity ^0.8.19; import {UserOperation} from "@eth-infinitism/account-abstraction/interfaces/UserOperation.sol"; -interface ISessionKeyPlugin { +interface IModularSessionKeyPlugin { enum FunctionId { RUNTIME_VALIDATION_TEMPORARY_OWNER, USER_OP_VALIDATION_TEMPORARY_OWNER @@ -15,7 +15,7 @@ interface ISessionKeyPlugin { /// @param selector The selector of the function that the temporary owner is allowed to call. /// @param _after The time after which the owner is valid. /// @param _until The time until which the owner is valid. - event TemporaryOwnerAdded( + event SessionKeyAdded( address indexed account, address indexed tempOwner, bytes4 selector, uint48 _after, uint48 _until ); @@ -23,7 +23,7 @@ interface ISessionKeyPlugin { /// @param account The account whose temporary owner is updated. /// @param tempOwner The address of the temporary owner. /// @param selector The selector of the function that the temporary owner is allowed to call. - event TemporaryOwnerRemoved(address indexed account, address indexed tempOwner, bytes4 selector); + event SessionKeyRemoved(address indexed account, address indexed tempOwner, bytes4 selector); /// @notice This event is emitted when temporary owners are added to the account. /// @param account The account whose temporary owners are updated. @@ -31,7 +31,7 @@ interface ISessionKeyPlugin { /// @param selectors The selectors of the functions that the temporary owners are allowed to call. /// @param _afters The times after which the owners are valid. /// @param _untils The times until which the owners are valid. - event TemporaryOwnersAdded( + event SessionKeysAdded( address indexed account, address[] indexed tempOwners, bytes4[] selectors, @@ -43,7 +43,7 @@ interface ISessionKeyPlugin { /// @param account The account whose temporary owners are updated. /// @param tempOwners The addresses of the temporary owners. /// @param selectors The selectors of the functions that the temporary owners are allowed to call. - event TemporaryOwnersRemoved(address indexed account, address[] indexed tempOwners, bytes4[] selectors); + event SessionKeysRemoved(address indexed account, address[] indexed tempOwners, bytes4[] selectors); error NotAuthorized(); error WrongTimeRangeForSession(); @@ -51,19 +51,20 @@ interface ISessionKeyPlugin { /// @notice Add a temporary owner to the account. /// @dev This function is installed on the account as part of plugin installation, and should - /// only be called from an account. + /// only be called from an account. The function selector installed by a child session key plugin + /// is passed as a parameter, which enforces its own permissions on the calls it can make. /// @param tempOwner The address of the temporary owner. /// @param allowedSelector The selector of the function that the temporary owner is allowed to call. /// @param _after The time after which the owner is valid. /// @param _until The time until which the owner is valid. - function addTemporaryOwner(address tempOwner, bytes4 allowedSelector, uint48 _after, uint48 _until) external; + function addSessionKey(address tempOwner, bytes4 allowedSelector, uint48 _after, uint48 _until) external; /// @notice Remove a temporary owner from the account. /// @dev This function is installed on the account as part of plugin installation, and should /// only be called from an account. /// @param tempOwner The address of the temporary owner. /// @param allowedSelector The selector of the function that the temporary owner is allowed to call. - function removeTemporaryOwner(address tempOwner, bytes4 allowedSelector) external; + function removeSessionKey(address tempOwner, bytes4 allowedSelector) external; /// @notice Add temporary owners to the account. /// @dev This function is installed on the account as part of plugin installation, and should @@ -72,7 +73,7 @@ interface ISessionKeyPlugin { /// @param allowedSelectors The selectors of the functions that the temporary owners are allowed to call. /// @param _afters The times after which the owners are valid. /// @param _untils The times until which the owners are valid. - function addTemporaryOwnerBatch( + function addSessionKeyBatch( address[] calldata tempOwners, bytes4[] calldata allowedSelectors, uint48[] calldata _afters, @@ -84,8 +85,7 @@ interface ISessionKeyPlugin { /// only be called from an account. /// @param tempOwners The addresses of the temporary owners. /// @param allowedSelectors The selectors of the functions that the temporary owners are allowed to call. - function removeTemporaryOwnerBatch(address[] calldata tempOwners, bytes4[] calldata allowedSelectors) - external; + function removeSessionKeyBatch(address[] calldata tempOwners, bytes4[] calldata allowedSelectors) external; /// @notice Get Session data for a given account and temporary owner. /// @param account The account to get session data for. diff --git a/test/samples/plugins/ModularSessionKeyPlugin.t.sol b/test/samples/plugins/ModularSessionKeyPlugin.t.sol index 956bc65a..7eedb854 100644 --- a/test/samples/plugins/ModularSessionKeyPlugin.t.sol +++ b/test/samples/plugins/ModularSessionKeyPlugin.t.sol @@ -10,7 +10,7 @@ import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; import {SingleOwnerPlugin} from "../../../src/plugins/owner/SingleOwnerPlugin.sol"; import {ISingleOwnerPlugin} from "../../../src/plugins/owner/ISingleOwnerPlugin.sol"; import {ModularSessionKeyPlugin} from "../../../src/samples/plugins/ModularSessionKeyPlugin.sol"; -import {ISessionKeyPlugin} from "../../../src/samples/plugins/interfaces/ISessionKeyPlugin.sol"; +import {IModularSessionKeyPlugin} from "../../../src/samples/plugins/interfaces/ISessionKeyPlugin.sol"; import {TokenSessionKeyPlugin} from "../../../src/samples/plugins/TokenSessionKeyPlugin.sol"; import {ITokenSessionKeyPlugin} from "../../../src/samples/plugins/interfaces/ITokenSessionKeyPlugin.sol"; @@ -22,7 +22,6 @@ import {MockERC20} from "../../mocks/MockERC20.sol"; contract ModularSessionKeyPluginTest is Test { using ECDSA for bytes32; - using FunctionReferenceLib for address; SingleOwnerPlugin public ownerPlugin; ModularSessionKeyPlugin public modularSessionKeyPlugin; @@ -57,18 +56,18 @@ contract ModularSessionKeyPluginTest is Test { event UserOperationRevertReason( bytes32 indexed userOpHash, address indexed sender, uint256 nonce, bytes revertReason ); - event TemporaryOwnerAdded( + event SessionKeyAdded( address indexed account, address indexed tempOwner, bytes4 allowedSelector, uint48 _after, uint48 _until ); - event TemporaryOwnerRemoved(address indexed account, address indexed tempOwner, bytes4 allowedSelector); - event TemporaryOwnersAdded( + event SessionKeyRemoved(address indexed account, address indexed tempOwner, bytes4 allowedSelector); + event SessionKeysAdded( address indexed account, address[] indexed tempOwners, bytes4[] allowedSelectors, uint48[] afters, uint48[] untils ); - event TemporaryOwnersRemoved(address indexed account, address[] indexed tempOwners, bytes4[] allowedSelectors); + event SessionKeysRemoved(address indexed account, address[] indexed tempOwners, bytes4[] allowedSelectors); event PluginUninstalled(address indexed plugin, bool indexed onUninstallSuccess); function setUp() public { @@ -106,9 +105,9 @@ contract ModularSessionKeyPluginTest is Test { vm.startPrank(owner); FunctionReference[] memory modularSessionDependency = new FunctionReference[](2); modularSessionDependency[0] = - address(ownerPlugin).pack(uint8(ISingleOwnerPlugin.FunctionId.USER_OP_VALIDATION_OWNER)); + FunctionReferenceLib.pack(address(ownerPlugin), (uint8(ISingleOwnerPlugin.FunctionId.USER_OP_VALIDATION_OWNER))); modularSessionDependency[1] = - address(ownerPlugin).pack(uint8(ISingleOwnerPlugin.FunctionId.RUNTIME_VALIDATION_OWNER_OR_SELF)); + FunctionReferenceLib.pack(address(ownerPlugin), (uint8(ISingleOwnerPlugin.FunctionId.RUNTIME_VALIDATION_OWNER_OR_SELF))); bytes32 modularSessionKeyManifestHash = keccak256(abi.encode(modularSessionKeyPlugin.pluginManifest())); @@ -134,11 +133,13 @@ contract ModularSessionKeyPluginTest is Test { }); FunctionReference[] memory tokenSessionDependency = new FunctionReference[](2); - tokenSessionDependency[0] = address(modularSessionKeyPlugin).pack( - uint8(ISessionKeyPlugin.FunctionId.USER_OP_VALIDATION_TEMPORARY_OWNER) + tokenSessionDependency[0] = FunctionReferenceLib.pack( + address(modularSessionKeyPlugin), + uint8(IModularSessionKeyPlugin.FunctionId.USER_OP_VALIDATION_TEMPORARY_OWNER) ); - tokenSessionDependency[1] = address(modularSessionKeyPlugin).pack( - uint8(ISessionKeyPlugin.FunctionId.RUNTIME_VALIDATION_TEMPORARY_OWNER) + tokenSessionDependency[1] = FunctionReferenceLib.pack( + address(modularSessionKeyPlugin), + uint8(IModularSessionKeyPlugin.FunctionId.RUNTIME_VALIDATION_TEMPORARY_OWNER) ); bytes32 tokenSessionKeyManifestHash = keccak256(abi.encode(tokenSessionKeyPlugin.pluginManifest())); @@ -183,9 +184,9 @@ contract ModularSessionKeyPluginTest is Test { untils[1] = 2; vm.expectEmit(true, true, true, true); - emit TemporaryOwnersAdded(address(account), tempOwners, allowedSelectors, afters, untils); + emit SessionKeysAdded(address(account), tempOwners, allowedSelectors, afters, untils); vm.prank(address(account)); - modularSessionKeyPlugin.addTemporaryOwnerBatch(tempOwners, allowedSelectors, afters, untils); + modularSessionKeyPlugin.addSessionKeyBatch(tempOwners, allowedSelectors, afters, untils); vm.prank(tempOwner3); TokenSessionKeyPlugin(address(account)).transferFromSessionKey( @@ -196,9 +197,9 @@ contract ModularSessionKeyPluginTest is Test { assertEq(mockERC20.balanceOf(target), 1 ether); vm.expectEmit(true, true, true, true); - emit TemporaryOwnersRemoved(address(account), tempOwners, allowedSelectors); + emit SessionKeysRemoved(address(account), tempOwners, allowedSelectors); vm.prank(address(account)); - modularSessionKeyPlugin.removeTemporaryOwnerBatch(tempOwners, allowedSelectors); + modularSessionKeyPlugin.removeSessionKeyBatch(tempOwners, allowedSelectors); } function test_sessionKey_userOp() public { @@ -227,8 +228,8 @@ contract ModularSessionKeyPluginTest is Test { vm.startPrank(address(account)); vm.expectEmit(true, true, true, true); - emit TemporaryOwnerRemoved(address(account), tempOwner, TRANSFERFROM_SESSIONKEY_SELECTOR); - modularSessionKeyPlugin.removeTemporaryOwner(tempOwner, TRANSFERFROM_SESSIONKEY_SELECTOR); + emit SessionKeyRemoved(address(account), tempOwner, TRANSFERFROM_SESSIONKEY_SELECTOR); + modularSessionKeyPlugin.removeSessionKey(tempOwner, TRANSFERFROM_SESSIONKEY_SELECTOR); vm.stopPrank(); @@ -241,12 +242,12 @@ contract ModularSessionKeyPluginTest is Test { // Check if tempOwner can still send user operations vm.startPrank(address(tempOwner)); - bytes memory revertReason = abi.encodeWithSelector(ISessionKeyPlugin.NotAuthorized.selector); + bytes memory revertReason = abi.encodeWithSelector(IModularSessionKeyPlugin.NotAuthorized.selector); vm.expectRevert( abi.encodeWithSelector( UpgradeableModularAccount.RuntimeValidationFunctionReverted.selector, address(modularSessionKeyPlugin), - ISessionKeyPlugin.FunctionId.RUNTIME_VALIDATION_TEMPORARY_OWNER, + IModularSessionKeyPlugin.FunctionId.RUNTIME_VALIDATION_TEMPORARY_OWNER, revertReason ) ); @@ -281,13 +282,13 @@ contract ModularSessionKeyPluginTest is Test { function test_sessionKey_unregisteredTempOwnerFails() public { vm.prank(address(maliciousOwner)); - bytes memory revertReason = abi.encodeWithSelector(ISessionKeyPlugin.NotAuthorized.selector); + bytes memory revertReason = abi.encodeWithSelector(IModularSessionKeyPlugin.NotAuthorized.selector); vm.expectRevert( abi.encodeWithSelector( UpgradeableModularAccount.RuntimeValidationFunctionReverted.selector, address(modularSessionKeyPlugin), - ISessionKeyPlugin.FunctionId.RUNTIME_VALIDATION_TEMPORARY_OWNER, + IModularSessionKeyPlugin.FunctionId.RUNTIME_VALIDATION_TEMPORARY_OWNER, revertReason ) ); @@ -302,13 +303,13 @@ contract ModularSessionKeyPluginTest is Test { vm.startPrank(address(tempOwner)); - bytes memory revertReason = abi.encodeWithSelector(ISessionKeyPlugin.WrongTimeRangeForSession.selector); + bytes memory revertReason = abi.encodeWithSelector(IModularSessionKeyPlugin.WrongTimeRangeForSession.selector); vm.expectRevert( abi.encodeWithSelector( UpgradeableModularAccount.RuntimeValidationFunctionReverted.selector, address(modularSessionKeyPlugin), - ISessionKeyPlugin.FunctionId.RUNTIME_VALIDATION_TEMPORARY_OWNER, + IModularSessionKeyPlugin.FunctionId.RUNTIME_VALIDATION_TEMPORARY_OWNER, revertReason ) ); From 6a63c8049c6abe65018f0227deb430f21c030582 Mon Sep 17 00:00:00 2001 From: sm-stack Date: Sat, 27 Jan 2024 12:59:32 +0900 Subject: [PATCH 25/33] Apply forge fmt --- src/samples/plugins/ModularSessionKeyPlugin.sol | 14 ++++++++++---- test/samples/plugins/ModularSessionKeyPlugin.t.sol | 13 ++++++++----- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/samples/plugins/ModularSessionKeyPlugin.sol b/src/samples/plugins/ModularSessionKeyPlugin.sol index dd4576ed..c6b98dcf 100644 --- a/src/samples/plugins/ModularSessionKeyPlugin.sol +++ b/src/samples/plugins/ModularSessionKeyPlugin.sol @@ -52,7 +52,9 @@ contract ModularSessionKeyPlugin is BasePlugin, IModularSessionKeyPlugin { // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ /// @inheritdoc IModularSessionKeyPlugin - function addSessionKey(address tempOwner, bytes4 allowedSelector, uint48 validAfter, uint48 validUntil) external { + function addSessionKey(address tempOwner, bytes4 allowedSelector, uint48 validAfter, uint48 validUntil) + external + { _addSessionKey(msg.sender, tempOwner, allowedSelector, validAfter, validUntil); emit SessionKeyAdded(msg.sender, tempOwner, allowedSelector, validAfter, validUntil); } @@ -295,9 +297,13 @@ contract ModularSessionKeyPlugin is BasePlugin, IModularSessionKeyPlugin { // ┃ Internal / Private functions ┃ // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ - function _addSessionKey(address account, address tempOwner, bytes4 allowedSelector, uint48 _validAfter, uint48 _validUntil) - internal - { + function _addSessionKey( + address account, + address tempOwner, + bytes4 allowedSelector, + uint48 _validAfter, + uint48 _validUntil + ) internal { if (_validUntil <= _validAfter) { revert WrongTimeRangeForSession(); } diff --git a/test/samples/plugins/ModularSessionKeyPlugin.t.sol b/test/samples/plugins/ModularSessionKeyPlugin.t.sol index 7eedb854..334f94e7 100644 --- a/test/samples/plugins/ModularSessionKeyPlugin.t.sol +++ b/test/samples/plugins/ModularSessionKeyPlugin.t.sol @@ -104,10 +104,12 @@ contract ModularSessionKeyPluginTest is Test { vm.startPrank(owner); FunctionReference[] memory modularSessionDependency = new FunctionReference[](2); - modularSessionDependency[0] = - FunctionReferenceLib.pack(address(ownerPlugin), (uint8(ISingleOwnerPlugin.FunctionId.USER_OP_VALIDATION_OWNER))); - modularSessionDependency[1] = - FunctionReferenceLib.pack(address(ownerPlugin), (uint8(ISingleOwnerPlugin.FunctionId.RUNTIME_VALIDATION_OWNER_OR_SELF))); + modularSessionDependency[0] = FunctionReferenceLib.pack( + address(ownerPlugin), (uint8(ISingleOwnerPlugin.FunctionId.USER_OP_VALIDATION_OWNER)) + ); + modularSessionDependency[1] = FunctionReferenceLib.pack( + address(ownerPlugin), (uint8(ISingleOwnerPlugin.FunctionId.RUNTIME_VALIDATION_OWNER_OR_SELF)) + ); bytes32 modularSessionKeyManifestHash = keccak256(abi.encode(modularSessionKeyPlugin.pluginManifest())); @@ -303,7 +305,8 @@ contract ModularSessionKeyPluginTest is Test { vm.startPrank(address(tempOwner)); - bytes memory revertReason = abi.encodeWithSelector(IModularSessionKeyPlugin.WrongTimeRangeForSession.selector); + bytes memory revertReason = + abi.encodeWithSelector(IModularSessionKeyPlugin.WrongTimeRangeForSession.selector); vm.expectRevert( abi.encodeWithSelector( From cb031e45921c7cdac45086e091ee2dca1f77c103 Mon Sep 17 00:00:00 2001 From: sm-stack Date: Sat, 27 Jan 2024 13:08:00 +0900 Subject: [PATCH 26/33] Update installPlugin --- test/samples/plugins/ModularSessionKeyPlugin.t.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/samples/plugins/ModularSessionKeyPlugin.t.sol b/test/samples/plugins/ModularSessionKeyPlugin.t.sol index 334f94e7..1e2e208b 100644 --- a/test/samples/plugins/ModularSessionKeyPlugin.t.sol +++ b/test/samples/plugins/ModularSessionKeyPlugin.t.sol @@ -130,7 +130,7 @@ contract ModularSessionKeyPluginTest is Test { account.installPlugin({ plugin: address(modularSessionKeyPlugin), manifestHash: modularSessionKeyManifestHash, - pluginInitData: data, + pluginInstallData: data, dependencies: modularSessionDependency }); @@ -148,7 +148,7 @@ contract ModularSessionKeyPluginTest is Test { account.installPlugin({ plugin: address(tokenSessionKeyPlugin), manifestHash: tokenSessionKeyManifestHash, - pluginInitData: "", + pluginInstallData: "", dependencies: tokenSessionDependency }); vm.stopPrank(); From 5a64a3862c2cf77a0726a16eb9ba158dbc8016e2 Mon Sep 17 00:00:00 2001 From: sm-stack Date: Tue, 13 Feb 2024 09:52:38 +0900 Subject: [PATCH 27/33] Apply suggested changes --- src/samples/plugins/ModularSessionKeyPlugin.sol | 6 ++---- src/samples/plugins/TokenSessionKeyPlugin.sol | 1 - 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/samples/plugins/ModularSessionKeyPlugin.sol b/src/samples/plugins/ModularSessionKeyPlugin.sol index c6b98dcf..7d351a55 100644 --- a/src/samples/plugins/ModularSessionKeyPlugin.sol +++ b/src/samples/plugins/ModularSessionKeyPlugin.sol @@ -193,9 +193,8 @@ contract ModularSessionKeyPlugin is BasePlugin, IModularSessionKeyPlugin { revert WrongTimeRangeForSession(); } return; - } else { - revert NotAuthorized(); } + revert NotAuthorized(); } revert NotImplemented(); } @@ -204,12 +203,11 @@ contract ModularSessionKeyPlugin is BasePlugin, IModularSessionKeyPlugin { function pluginManifest() external pure override returns (PluginManifest memory) { PluginManifest memory manifest; - manifest.executionFunctions = new bytes4[](5); + manifest.executionFunctions = new bytes4[](4); manifest.executionFunctions[0] = this.addSessionKey.selector; manifest.executionFunctions[1] = this.removeSessionKey.selector; manifest.executionFunctions[2] = this.addSessionKeyBatch.selector; manifest.executionFunctions[3] = this.removeSessionKeyBatch.selector; - manifest.executionFunctions[4] = this.getSessionDuration.selector; ManifestFunction memory ownerUserOpValidationFunction = ManifestFunction({ functionType: ManifestAssociatedFunctionType.DEPENDENCY, diff --git a/src/samples/plugins/TokenSessionKeyPlugin.sol b/src/samples/plugins/TokenSessionKeyPlugin.sol index 53aac1aa..062a706c 100644 --- a/src/samples/plugins/TokenSessionKeyPlugin.sol +++ b/src/samples/plugins/TokenSessionKeyPlugin.sol @@ -28,7 +28,6 @@ import {IPluginExecutor} from "../../interfaces/IPluginExecutor.sol"; /// permitted external calls not to be changed in the future. For other child session /// key plugins, there can be a set of permitted external calls according to the /// specific needs. - contract TokenSessionKeyPlugin is BasePlugin, ITokenSessionKeyPlugin { string public constant NAME = "Token Session Key Plugin"; string public constant VERSION = "1.0.0"; From 628888896fba25e4f8e8b8059e9d3477998cefcb Mon Sep 17 00:00:00 2001 From: sm-stack Date: Thu, 15 Feb 2024 22:09:04 +0900 Subject: [PATCH 28/33] Fix onUninstall to be able to run with msg.sender --- .../plugins/ModularSessionKeyPlugin.sol | 137 ++++++++++++------ .../plugins/ModularSessionKeyPlugin.t.sol | 8 +- 2 files changed, 93 insertions(+), 52 deletions(-) diff --git a/src/samples/plugins/ModularSessionKeyPlugin.sol b/src/samples/plugins/ModularSessionKeyPlugin.sol index 7d351a55..26950f2d 100644 --- a/src/samples/plugins/ModularSessionKeyPlugin.sol +++ b/src/samples/plugins/ModularSessionKeyPlugin.sol @@ -2,6 +2,7 @@ pragma solidity ^0.8.19; import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; +import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; import {UserOperation} from "@eth-infinitism/account-abstraction/interfaces/UserOperation.sol"; import {UpgradeableModularAccount} from "../../account/UpgradeableModularAccount.sol"; import { @@ -35,6 +36,7 @@ contract ModularSessionKeyPlugin is BasePlugin, IModularSessionKeyPlugin { using ECDSA for bytes32; using PluginStorageLib for address; using PluginStorageLib for bytes; + using EnumerableSet for EnumerableSet.Bytes32Set; string public constant NAME = "Modular Session Key Plugin"; string public constant VERSION = "1.0.0"; @@ -42,6 +44,8 @@ contract ModularSessionKeyPlugin is BasePlugin, IModularSessionKeyPlugin { uint256 internal constant _SIG_VALIDATION_FAILED = 1; + mapping(address account => EnumerableSet.Bytes32Set) private _sessionKeySet; + struct SessionInfo { uint48 validAfter; uint48 validUntil; @@ -52,46 +56,46 @@ contract ModularSessionKeyPlugin is BasePlugin, IModularSessionKeyPlugin { // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ /// @inheritdoc IModularSessionKeyPlugin - function addSessionKey(address tempOwner, bytes4 allowedSelector, uint48 validAfter, uint48 validUntil) + function addSessionKey(address sessionKey, bytes4 allowedSelector, uint48 validAfter, uint48 validUntil) external { - _addSessionKey(msg.sender, tempOwner, allowedSelector, validAfter, validUntil); - emit SessionKeyAdded(msg.sender, tempOwner, allowedSelector, validAfter, validUntil); + _addSessionKey(msg.sender, sessionKey, allowedSelector, validAfter, validUntil); + emit SessionKeyAdded(msg.sender, sessionKey, allowedSelector, validAfter, validUntil); } /// @inheritdoc IModularSessionKeyPlugin - function removeSessionKey(address tempOwner, bytes4 allowedSelector) external { - _removeSessionKey(msg.sender, tempOwner, allowedSelector); - emit SessionKeyRemoved(msg.sender, tempOwner, allowedSelector); + function removeSessionKey(address sessionKey, bytes4 allowedSelector) external { + _removeSessionKey(msg.sender, sessionKey, allowedSelector); + emit SessionKeyRemoved(msg.sender, sessionKey, allowedSelector); } /// @inheritdoc IModularSessionKeyPlugin function addSessionKeyBatch( - address[] calldata tempOwners, + address[] calldata sessionKeys, bytes4[] calldata allowedSelectors, uint48[] calldata validAfters, uint48[] calldata validUntils ) external { if ( - tempOwners.length != allowedSelectors.length || tempOwners.length != validAfters.length - || tempOwners.length != validUntils.length + sessionKeys.length != allowedSelectors.length || sessionKeys.length != validAfters.length + || sessionKeys.length != validUntils.length ) { revert WrongDataLength(); } - for (uint256 i = 0; i < tempOwners.length; i++) { - _addSessionKey(msg.sender, tempOwners[i], allowedSelectors[i], validAfters[i], validUntils[i]); + for (uint256 i = 0; i < sessionKeys.length; i++) { + _addSessionKey(msg.sender, sessionKeys[i], allowedSelectors[i], validAfters[i], validUntils[i]); } - emit SessionKeysAdded(msg.sender, tempOwners, allowedSelectors, validAfters, validUntils); + emit SessionKeysAdded(msg.sender, sessionKeys, allowedSelectors, validAfters, validUntils); } - function removeSessionKeyBatch(address[] calldata tempOwners, bytes4[] calldata allowedSelectors) external { - if (tempOwners.length != allowedSelectors.length) { + function removeSessionKeyBatch(address[] calldata sessionKeys, bytes4[] calldata allowedSelectors) external { + if (sessionKeys.length != allowedSelectors.length) { revert WrongDataLength(); } - for (uint256 i = 0; i < tempOwners.length; i++) { - _removeSessionKey(msg.sender, tempOwners[i], allowedSelectors[i]); + for (uint256 i = 0; i < sessionKeys.length; i++) { + _removeSessionKey(msg.sender, sessionKeys[i], allowedSelectors[i]); } - emit SessionKeysRemoved(msg.sender, tempOwners, allowedSelectors); + emit SessionKeysRemoved(msg.sender, sessionKeys, allowedSelectors); } // ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ @@ -99,16 +103,31 @@ contract ModularSessionKeyPlugin is BasePlugin, IModularSessionKeyPlugin { // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ /// @inheritdoc IModularSessionKeyPlugin - function getSessionDuration(address account, address tempOwner, bytes4 allowedSelector) + function getSessionDuration(address account, address sessionKey, bytes4 allowedSelector) external view - returns (uint48 _validAfter, uint48 _validUntil) + returns (uint48 validAfter, uint48 validUntil) { bytes memory key = account.allocateAssociatedStorageKey(0, 1); - StoragePointer ptr = key.associatedStorageLookup(keccak256(abi.encodePacked(tempOwner, allowedSelector))); + StoragePointer ptr = key.associatedStorageLookup(keccak256(abi.encodePacked(sessionKey, allowedSelector))); SessionInfo storage sessionInfo = _castPtrToStruct(ptr); - _validAfter = sessionInfo.validAfter; - _validUntil = sessionInfo.validUntil; + validAfter = sessionInfo.validAfter; + validUntil = sessionInfo.validUntil; + } + + /// @inheritdoc IModularSessionKeyPlugin + function getSessionKeysAndSelectors(address account) + external + view + returns (address[] memory sessionKeys, bytes4[] memory selectors) + { + EnumerableSet.Bytes32Set storage sessionKeySet = _sessionKeySet[account]; + uint256 length = sessionKeySet.length(); + sessionKeys = new address[](length); + selectors = new bytes4[](length); + for (uint256 i = 0; i < length; i++) { + (sessionKeys[i], selectors[i]) = _castToAddressAndBytes4(sessionKeySet.at(i)); + } } // ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ @@ -119,33 +138,37 @@ contract ModularSessionKeyPlugin is BasePlugin, IModularSessionKeyPlugin { function onInstall(bytes calldata data) external override { if (data.length != 0) { ( - address[] memory tempOwners, + address[] memory sessionKeys, bytes4[] memory allowedSelectors, uint48[] memory validAfters, uint48[] memory validUntils ) = abi.decode(data, (address[], bytes4[], uint48[], uint48[])); if ( - tempOwners.length != allowedSelectors.length || tempOwners.length != validAfters.length - || tempOwners.length != validUntils.length + sessionKeys.length != allowedSelectors.length || sessionKeys.length != validAfters.length + || sessionKeys.length != validUntils.length ) { revert WrongDataLength(); } - for (uint256 i = 0; i < tempOwners.length; i++) { - _addSessionKey(msg.sender, tempOwners[i], allowedSelectors[i], validAfters[i], validUntils[i]); + for (uint256 i = 0; i < sessionKeys.length;) { + _addSessionKey(msg.sender, sessionKeys[i], allowedSelectors[i], validAfters[i], validUntils[i]); + + unchecked { + ++i; + } } } } /// @inheritdoc BasePlugin - function onUninstall(bytes calldata data) external override { - if (data.length != 0) { - (address[] memory tempOwners, bytes4[] memory allowedSelectors) = - abi.decode(data, (address[], bytes4[])); - if (tempOwners.length != allowedSelectors.length) { - revert WrongDataLength(); - } - for (uint256 i = 0; i < tempOwners.length; i++) { - _removeSessionKey(msg.sender, tempOwners[i], allowedSelectors[i]); + function onUninstall(bytes calldata) external override { + EnumerableSet.Bytes32Set storage sessionKeySet = _sessionKeySet[msg.sender]; + uint256 length = sessionKeySet.length(); + for (uint256 i = 0; i < length;) { + (address sessionKey, bytes4 allowedSelecor) = _castToAddressAndBytes4(sessionKeySet.at(i)); + _removeSessionKey(msg.sender, sessionKey, allowedSelecor); + + unchecked { + ++i; } } } @@ -158,18 +181,19 @@ contract ModularSessionKeyPlugin is BasePlugin, IModularSessionKeyPlugin { returns (uint256) { if (functionId == uint8(FunctionId.USER_OP_VALIDATION_TEMPORARY_OWNER)) { - (address signer,) = userOpHash.toEthSignedMessageHash().tryRecover(userOp.signature); + (address signer, ECDSA.RecoverError err) = + userOpHash.toEthSignedMessageHash().tryRecover(userOp.signature); + if (err != ECDSA.RecoverError.NoError) { + revert InvalidSignature(); + } bytes4 selector = bytes4(userOp.callData[0:4]); - bytes memory key = userOp.sender.allocateAssociatedStorageKey(0, 1); + bytes memory key = msg.sender.allocateAssociatedStorageKey(0, 1); StoragePointer ptr = key.associatedStorageLookup(keccak256(abi.encodePacked(signer, selector))); SessionInfo storage duration = _castPtrToStruct(ptr); uint48 validAfter = duration.validAfter; uint48 validUntil = duration.validUntil; - if (validUntil != 0) { - return _packValidationData(false, validUntil, validAfter); - } - return _SIG_VALIDATION_FAILED; + return _packValidationData(validUntil == 0, validUntil, validAfter); } revert NotImplemented(); } @@ -182,7 +206,7 @@ contract ModularSessionKeyPlugin is BasePlugin, IModularSessionKeyPlugin { { if (functionId == uint8(FunctionId.RUNTIME_VALIDATION_TEMPORARY_OWNER)) { bytes4 selector = bytes4(data[0:4]); - bytes memory key = address(msg.sender).allocateAssociatedStorageKey(0, 1); + bytes memory key = msg.sender.allocateAssociatedStorageKey(0, 1); StoragePointer ptr = key.associatedStorageLookup(keccak256(abi.encodePacked(sender, selector))); SessionInfo storage duration = _castPtrToStruct(ptr); uint48 validAfter = duration.validAfter; @@ -297,7 +321,7 @@ contract ModularSessionKeyPlugin is BasePlugin, IModularSessionKeyPlugin { function _addSessionKey( address account, - address tempOwner, + address sessionKey, bytes4 allowedSelector, uint48 _validAfter, uint48 _validUntil @@ -306,18 +330,24 @@ contract ModularSessionKeyPlugin is BasePlugin, IModularSessionKeyPlugin { revert WrongTimeRangeForSession(); } bytes memory key = account.allocateAssociatedStorageKey(0, 1); - StoragePointer ptr = key.associatedStorageLookup(keccak256(abi.encodePacked(tempOwner, allowedSelector))); + StoragePointer ptr = key.associatedStorageLookup(keccak256(abi.encodePacked(sessionKey, allowedSelector))); SessionInfo storage sessionInfo = _castPtrToStruct(ptr); sessionInfo.validAfter = _validAfter; sessionInfo.validUntil = _validUntil; + + EnumerableSet.Bytes32Set storage sessionKeySet = _sessionKeySet[account]; + sessionKeySet.add(_castToBytes32(sessionKey, allowedSelector)); } - function _removeSessionKey(address account, address tempOwner, bytes4 allowedSelector) internal { + function _removeSessionKey(address account, address sessionKey, bytes4 allowedSelector) internal { bytes memory key = account.allocateAssociatedStorageKey(0, 1); - StoragePointer ptr = key.associatedStorageLookup(keccak256(abi.encodePacked(tempOwner, allowedSelector))); + StoragePointer ptr = key.associatedStorageLookup(keccak256(abi.encodePacked(sessionKey, allowedSelector))); SessionInfo storage sessionInfo = _castPtrToStruct(ptr); sessionInfo.validAfter = 0; sessionInfo.validUntil = 0; + + EnumerableSet.Bytes32Set storage sessionKeySet = _sessionKeySet[account]; + sessionKeySet.remove(_castToBytes32(sessionKey, allowedSelector)); } function _castPtrToStruct(StoragePointer ptr) internal pure returns (SessionInfo storage val) { @@ -326,6 +356,19 @@ contract ModularSessionKeyPlugin is BasePlugin, IModularSessionKeyPlugin { } } + function _castToBytes32(address addr, bytes4 b4) internal pure returns (bytes32 res) { + assembly { + res := or(shl(96, addr), b4) + } + } + + function _castToAddressAndBytes4(bytes32 b32) internal pure returns (address addr, bytes4 b4) { + assembly { + addr := shr(96, b32) + b4 := and(b32, 0xFFFFFFFF) + } + } + function _packValidationData(bool sigFailed, uint48 validUntil, uint48 validAfter) internal pure diff --git a/test/samples/plugins/ModularSessionKeyPlugin.t.sol b/test/samples/plugins/ModularSessionKeyPlugin.t.sol index 1e2e208b..fe241f83 100644 --- a/test/samples/plugins/ModularSessionKeyPlugin.t.sol +++ b/test/samples/plugins/ModularSessionKeyPlugin.t.sol @@ -105,10 +105,10 @@ contract ModularSessionKeyPluginTest is Test { vm.startPrank(owner); FunctionReference[] memory modularSessionDependency = new FunctionReference[](2); modularSessionDependency[0] = FunctionReferenceLib.pack( - address(ownerPlugin), (uint8(ISingleOwnerPlugin.FunctionId.USER_OP_VALIDATION_OWNER)) + address(ownerPlugin), uint8(ISingleOwnerPlugin.FunctionId.USER_OP_VALIDATION_OWNER) ); modularSessionDependency[1] = FunctionReferenceLib.pack( - address(ownerPlugin), (uint8(ISingleOwnerPlugin.FunctionId.RUNTIME_VALIDATION_OWNER_OR_SELF)) + address(ownerPlugin), uint8(ISingleOwnerPlugin.FunctionId.RUNTIME_VALIDATION_OWNER_OR_SELF) ); bytes32 modularSessionKeyManifestHash = keccak256(abi.encode(modularSessionKeyPlugin.pluginManifest())); @@ -328,8 +328,6 @@ contract ModularSessionKeyPluginTest is Test { bytes4[] memory allowedSelectors = new bytes4[](1); allowedSelectors[0] = TRANSFERFROM_SESSIONKEY_SELECTOR; - bytes memory data = abi.encode(tempOwners, allowedSelectors); - vm.startPrank(owner); vm.expectEmit(true, true, true, true); @@ -346,7 +344,7 @@ contract ModularSessionKeyPluginTest is Test { account.uninstallPlugin({ plugin: address(modularSessionKeyPlugin), config: bytes(""), - pluginUninstallData: data + pluginUninstallData: "" }); vm.stopPrank(); From a8c6d36ea3bc35b360503eb426a8ce5d37763cc9 Mon Sep 17 00:00:00 2001 From: sm-stack Date: Thu, 15 Feb 2024 22:09:26 +0900 Subject: [PATCH 29/33] Add getSessionKeysAndSelectors function --- .../plugins/interfaces/ISessionKeyPlugin.sol | 90 ++++++++++--------- 1 file changed, 50 insertions(+), 40 deletions(-) diff --git a/src/samples/plugins/interfaces/ISessionKeyPlugin.sol b/src/samples/plugins/interfaces/ISessionKeyPlugin.sol index 005b7946..ef7831c4 100644 --- a/src/samples/plugins/interfaces/ISessionKeyPlugin.sol +++ b/src/samples/plugins/interfaces/ISessionKeyPlugin.sol @@ -9,90 +9,100 @@ interface IModularSessionKeyPlugin { USER_OP_VALIDATION_TEMPORARY_OWNER } - /// @notice This event is emitted when a temporary owner is added to the account. - /// @param account The account whose temporary owner is updated. - /// @param tempOwner The address of the temporary owner. - /// @param selector The selector of the function that the temporary owner is allowed to call. + /// @notice This event is emitted when a session key is added to the account. + /// @param account The account whose session key is updated. + /// @param sessionKey The address of the session key. + /// @param selector The selector of the function that the session key is allowed to call. /// @param _after The time after which the owner is valid. /// @param _until The time until which the owner is valid. event SessionKeyAdded( - address indexed account, address indexed tempOwner, bytes4 selector, uint48 _after, uint48 _until + address indexed account, address indexed sessionKey, bytes4 selector, uint48 _after, uint48 _until ); - /// @notice This event is emitted when a temporary owner is removed from the account. - /// @param account The account whose temporary owner is updated. - /// @param tempOwner The address of the temporary owner. - /// @param selector The selector of the function that the temporary owner is allowed to call. - event SessionKeyRemoved(address indexed account, address indexed tempOwner, bytes4 selector); + /// @notice This event is emitted when a session key is removed from the account. + /// @param account The account whose session key is updated. + /// @param sessionKey The address of the session key. + /// @param selector The selector of the function that the session key is allowed to call. + event SessionKeyRemoved(address indexed account, address indexed sessionKey, bytes4 selector); - /// @notice This event is emitted when temporary owners are added to the account. - /// @param account The account whose temporary owners are updated. - /// @param tempOwners The addresses of the temporary owners. - /// @param selectors The selectors of the functions that the temporary owners are allowed to call. + /// @notice This event is emitted when session keys are added to the account. + /// @param account The account whose session keys are updated. + /// @param sessionKeys The addresses of the session keys. + /// @param selectors The selectors of the functions that the session keys are allowed to call. /// @param _afters The times after which the owners are valid. /// @param _untils The times until which the owners are valid. event SessionKeysAdded( address indexed account, - address[] indexed tempOwners, + address[] indexed sessionKeys, bytes4[] selectors, uint48[] _afters, uint48[] _untils ); - /// @notice This event is emitted when temporary owners are removed from the account. - /// @param account The account whose temporary owners are updated. - /// @param tempOwners The addresses of the temporary owners. - /// @param selectors The selectors of the functions that the temporary owners are allowed to call. - event SessionKeysRemoved(address indexed account, address[] indexed tempOwners, bytes4[] selectors); + /// @notice This event is emitted when session keys are removed from the account. + /// @param account The account whose session keys are updated. + /// @param sessionKeys The addresses of the session keys. + /// @param selectors The selectors of the functions that the session keys are allowed to call. + event SessionKeysRemoved(address indexed account, address[] indexed sessionKeys, bytes4[] selectors); + error InvalidSignature(); error NotAuthorized(); error WrongTimeRangeForSession(); error WrongDataLength(); - /// @notice Add a temporary owner to the account. + /// @notice Add a session key to the account. /// @dev This function is installed on the account as part of plugin installation, and should /// only be called from an account. The function selector installed by a child session key plugin /// is passed as a parameter, which enforces its own permissions on the calls it can make. - /// @param tempOwner The address of the temporary owner. - /// @param allowedSelector The selector of the function that the temporary owner is allowed to call. + /// @param sessionKey The address of the session key. + /// @param allowedSelector The selector of the function that the session key is allowed to call. /// @param _after The time after which the owner is valid. /// @param _until The time until which the owner is valid. - function addSessionKey(address tempOwner, bytes4 allowedSelector, uint48 _after, uint48 _until) external; + function addSessionKey(address sessionKey, bytes4 allowedSelector, uint48 _after, uint48 _until) external; - /// @notice Remove a temporary owner from the account. + /// @notice Remove a session key from the account. /// @dev This function is installed on the account as part of plugin installation, and should /// only be called from an account. - /// @param tempOwner The address of the temporary owner. - /// @param allowedSelector The selector of the function that the temporary owner is allowed to call. - function removeSessionKey(address tempOwner, bytes4 allowedSelector) external; + /// @param sessionKey The address of the session key. + /// @param allowedSelector The selector of the function that the session key is allowed to call. + function removeSessionKey(address sessionKey, bytes4 allowedSelector) external; - /// @notice Add temporary owners to the account. + /// @notice Add session keys to the account. /// @dev This function is installed on the account as part of plugin installation, and should /// only be called from an account. - /// @param tempOwners The addresses of the temporary owners. - /// @param allowedSelectors The selectors of the functions that the temporary owners are allowed to call. + /// @param sessionKeys The addresses of the session keys. + /// @param allowedSelectors The selectors of the functions that the session keys are allowed to call. /// @param _afters The times after which the owners are valid. /// @param _untils The times until which the owners are valid. function addSessionKeyBatch( - address[] calldata tempOwners, + address[] calldata sessionKeys, bytes4[] calldata allowedSelectors, uint48[] calldata _afters, uint48[] calldata _untils ) external; - /// @notice Remove temporary owners from the account. + /// @notice Remove session keys from the account. /// @dev This function is installed on the account as part of plugin installation, and should /// only be called from an account. - /// @param tempOwners The addresses of the temporary owners. - /// @param allowedSelectors The selectors of the functions that the temporary owners are allowed to call. - function removeSessionKeyBatch(address[] calldata tempOwners, bytes4[] calldata allowedSelectors) external; + /// @param sessionKeys The addresses of the session keys. + /// @param allowedSelectors The selectors of the functions that the session keys are allowed to call. + function removeSessionKeyBatch(address[] calldata sessionKeys, bytes4[] calldata allowedSelectors) external; - /// @notice Get Session data for a given account and temporary owner. + /// @notice Get Session data for a given account and session key. /// @param account The account to get session data for. - /// @param tempOwner The address of the temporary owner. - /// @param allowedSelector The selector of the function that the temporary owner is allowed to call. - function getSessionDuration(address account, address tempOwner, bytes4 allowedSelector) + /// @param sessionKey The address of the session key. + /// @param allowedSelector The selector of the function that the session key is allowed to call. + function getSessionDuration(address account, address sessionKey, bytes4 allowedSelector) external view returns (uint48 _after, uint48 _until); + + /// @notice Get all session keys and selectors for a given account. + /// @param account The account to get session keys and selectors for. + /// @return sessionKeys The addresses of the session keys. + /// @return selectors The selectors of the functions that the session keys are allowed to call. + function getSessionKeysAndSelectors(address account) + external + view + returns (address[] memory sessionKeys, bytes4[] memory selectors); } From f3436aded3889b600361f8ebd229ea856c5c6787 Mon Sep 17 00:00:00 2001 From: sm-stack Date: Fri, 16 Feb 2024 08:30:44 +0900 Subject: [PATCH 30/33] Reduce amount to shift to 32 --- src/samples/plugins/ModularSessionKeyPlugin.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/samples/plugins/ModularSessionKeyPlugin.sol b/src/samples/plugins/ModularSessionKeyPlugin.sol index 26950f2d..e4766397 100644 --- a/src/samples/plugins/ModularSessionKeyPlugin.sol +++ b/src/samples/plugins/ModularSessionKeyPlugin.sol @@ -358,13 +358,13 @@ contract ModularSessionKeyPlugin is BasePlugin, IModularSessionKeyPlugin { function _castToBytes32(address addr, bytes4 b4) internal pure returns (bytes32 res) { assembly { - res := or(shl(96, addr), b4) + res := or(shl(32, addr), b4) } } function _castToAddressAndBytes4(bytes32 b32) internal pure returns (address addr, bytes4 b4) { assembly { - addr := shr(96, b32) + addr := shr(32, b32) b4 := and(b32, 0xFFFFFFFF) } } From 7c833ac42c1eab84b1bd3b4093900141c66b62de Mon Sep 17 00:00:00 2001 From: sm-stack Date: Fri, 16 Feb 2024 08:31:39 +0900 Subject: [PATCH 31/33] Remove indexed field at the array parameter of events --- src/samples/plugins/interfaces/ISessionKeyPlugin.sol | 4 ++-- test/samples/plugins/ModularSessionKeyPlugin.t.sol | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/samples/plugins/interfaces/ISessionKeyPlugin.sol b/src/samples/plugins/interfaces/ISessionKeyPlugin.sol index ef7831c4..7cea5d61 100644 --- a/src/samples/plugins/interfaces/ISessionKeyPlugin.sol +++ b/src/samples/plugins/interfaces/ISessionKeyPlugin.sol @@ -33,7 +33,7 @@ interface IModularSessionKeyPlugin { /// @param _untils The times until which the owners are valid. event SessionKeysAdded( address indexed account, - address[] indexed sessionKeys, + address[] sessionKeys, bytes4[] selectors, uint48[] _afters, uint48[] _untils @@ -43,7 +43,7 @@ interface IModularSessionKeyPlugin { /// @param account The account whose session keys are updated. /// @param sessionKeys The addresses of the session keys. /// @param selectors The selectors of the functions that the session keys are allowed to call. - event SessionKeysRemoved(address indexed account, address[] indexed sessionKeys, bytes4[] selectors); + event SessionKeysRemoved(address indexed account, address[] sessionKeys, bytes4[] selectors); error InvalidSignature(); error NotAuthorized(); diff --git a/test/samples/plugins/ModularSessionKeyPlugin.t.sol b/test/samples/plugins/ModularSessionKeyPlugin.t.sol index fe241f83..96607f9e 100644 --- a/test/samples/plugins/ModularSessionKeyPlugin.t.sol +++ b/test/samples/plugins/ModularSessionKeyPlugin.t.sol @@ -57,17 +57,17 @@ contract ModularSessionKeyPluginTest is Test { bytes32 indexed userOpHash, address indexed sender, uint256 nonce, bytes revertReason ); event SessionKeyAdded( - address indexed account, address indexed tempOwner, bytes4 allowedSelector, uint48 _after, uint48 _until + address indexed account, address indexed sessionKey, bytes4 allowedSelector, uint48 _after, uint48 _until ); - event SessionKeyRemoved(address indexed account, address indexed tempOwner, bytes4 allowedSelector); + event SessionKeyRemoved(address indexed account, address indexed sessionKey, bytes4 allowedSelector); event SessionKeysAdded( address indexed account, - address[] indexed tempOwners, + address[] sessionKeys, bytes4[] allowedSelectors, uint48[] afters, uint48[] untils ); - event SessionKeysRemoved(address indexed account, address[] indexed tempOwners, bytes4[] allowedSelectors); + event SessionKeysRemoved(address indexed account, address[] sessionKeys, bytes4[] allowedSelectors); event PluginUninstalled(address indexed plugin, bool indexed onUninstallSuccess); function setUp() public { From 0f755fb51752c78ed7c561b48c85c7299221ef83 Mon Sep 17 00:00:00 2001 From: sm-stack Date: Fri, 16 Feb 2024 19:49:36 +0900 Subject: [PATCH 32/33] Fix nits --- .../plugins/ModularSessionKeyPlugin.sol | 28 ++++++++++++----- src/samples/plugins/TokenSessionKeyPlugin.sol | 8 ++--- .../plugins/interfaces/ISessionKeyPlugin.sol | 30 +++++++++---------- 3 files changed, 37 insertions(+), 29 deletions(-) diff --git a/src/samples/plugins/ModularSessionKeyPlugin.sol b/src/samples/plugins/ModularSessionKeyPlugin.sol index e4766397..e4a4d02b 100644 --- a/src/samples/plugins/ModularSessionKeyPlugin.sol +++ b/src/samples/plugins/ModularSessionKeyPlugin.sol @@ -82,8 +82,12 @@ contract ModularSessionKeyPlugin is BasePlugin, IModularSessionKeyPlugin { ) { revert WrongDataLength(); } - for (uint256 i = 0; i < sessionKeys.length; i++) { + for (uint256 i = 0; i < sessionKeys.length;) { _addSessionKey(msg.sender, sessionKeys[i], allowedSelectors[i], validAfters[i], validUntils[i]); + + unchecked { + ++i; + } } emit SessionKeysAdded(msg.sender, sessionKeys, allowedSelectors, validAfters, validUntils); } @@ -92,8 +96,12 @@ contract ModularSessionKeyPlugin is BasePlugin, IModularSessionKeyPlugin { if (sessionKeys.length != allowedSelectors.length) { revert WrongDataLength(); } - for (uint256 i = 0; i < sessionKeys.length; i++) { + for (uint256 i = 0; i < sessionKeys.length;) { _removeSessionKey(msg.sender, sessionKeys[i], allowedSelectors[i]); + + unchecked { + ++i; + } } emit SessionKeysRemoved(msg.sender, sessionKeys, allowedSelectors); } @@ -125,8 +133,12 @@ contract ModularSessionKeyPlugin is BasePlugin, IModularSessionKeyPlugin { uint256 length = sessionKeySet.length(); sessionKeys = new address[](length); selectors = new bytes4[](length); - for (uint256 i = 0; i < length; i++) { + for (uint256 i = 0; i < length;) { (sessionKeys[i], selectors[i]) = _castToAddressAndBytes4(sessionKeySet.at(i)); + + unchecked { + ++i; + } } } @@ -323,17 +335,17 @@ contract ModularSessionKeyPlugin is BasePlugin, IModularSessionKeyPlugin { address account, address sessionKey, bytes4 allowedSelector, - uint48 _validAfter, - uint48 _validUntil + uint48 validAfter, + uint48 validUntil ) internal { - if (_validUntil <= _validAfter) { + if (validUntil <= validAfter) { revert WrongTimeRangeForSession(); } bytes memory key = account.allocateAssociatedStorageKey(0, 1); StoragePointer ptr = key.associatedStorageLookup(keccak256(abi.encodePacked(sessionKey, allowedSelector))); SessionInfo storage sessionInfo = _castPtrToStruct(ptr); - sessionInfo.validAfter = _validAfter; - sessionInfo.validUntil = _validUntil; + sessionInfo.validAfter = validAfter; + sessionInfo.validUntil = validUntil; EnumerableSet.Bytes32Set storage sessionKeySet = _sessionKeySet[account]; sessionKeySet.add(_castToBytes32(sessionKey, allowedSelector)); diff --git a/src/samples/plugins/TokenSessionKeyPlugin.sol b/src/samples/plugins/TokenSessionKeyPlugin.sol index 062a706c..9e2e8abd 100644 --- a/src/samples/plugins/TokenSessionKeyPlugin.sol +++ b/src/samples/plugins/TokenSessionKeyPlugin.sol @@ -92,12 +92,8 @@ contract TokenSessionKeyPlugin is BasePlugin, ITokenSessionKeyPlugin { }); manifest.dependencyInterfaceIds = new bytes4[](2); - for (uint256 i = 0; i < manifest.dependencyInterfaceIds.length;) { - manifest.dependencyInterfaceIds[i] = type(IModularSessionKeyPlugin).interfaceId; - unchecked { - ++i; - } - } + manifest.dependencyInterfaceIds[0] = type(IModularSessionKeyPlugin).interfaceId; + manifest.dependencyInterfaceIds[1] = type(IModularSessionKeyPlugin).interfaceId; bytes4[] memory permittedExternalSelectors = new bytes4[](1); permittedExternalSelectors[0] = TRANSFERFROM_SELECTOR; diff --git a/src/samples/plugins/interfaces/ISessionKeyPlugin.sol b/src/samples/plugins/interfaces/ISessionKeyPlugin.sol index 7cea5d61..0068fa46 100644 --- a/src/samples/plugins/interfaces/ISessionKeyPlugin.sol +++ b/src/samples/plugins/interfaces/ISessionKeyPlugin.sol @@ -13,10 +13,10 @@ interface IModularSessionKeyPlugin { /// @param account The account whose session key is updated. /// @param sessionKey The address of the session key. /// @param selector The selector of the function that the session key is allowed to call. - /// @param _after The time after which the owner is valid. - /// @param _until The time until which the owner is valid. + /// @param validAfter The time after which the owner is valid. + /// @param validUntil The time until which the owner is valid. event SessionKeyAdded( - address indexed account, address indexed sessionKey, bytes4 selector, uint48 _after, uint48 _until + address indexed account, address indexed sessionKey, bytes4 selector, uint48 validAfter, uint48 validUntil ); /// @notice This event is emitted when a session key is removed from the account. @@ -29,14 +29,14 @@ interface IModularSessionKeyPlugin { /// @param account The account whose session keys are updated. /// @param sessionKeys The addresses of the session keys. /// @param selectors The selectors of the functions that the session keys are allowed to call. - /// @param _afters The times after which the owners are valid. - /// @param _untils The times until which the owners are valid. + /// @param validAfters The times after which the owners are valid. + /// @param validUntils The times until which the owners are valid. event SessionKeysAdded( address indexed account, address[] sessionKeys, bytes4[] selectors, - uint48[] _afters, - uint48[] _untils + uint48[] validAfters, + uint48[] validUntils ); /// @notice This event is emitted when session keys are removed from the account. @@ -56,9 +56,9 @@ interface IModularSessionKeyPlugin { /// is passed as a parameter, which enforces its own permissions on the calls it can make. /// @param sessionKey The address of the session key. /// @param allowedSelector The selector of the function that the session key is allowed to call. - /// @param _after The time after which the owner is valid. - /// @param _until The time until which the owner is valid. - function addSessionKey(address sessionKey, bytes4 allowedSelector, uint48 _after, uint48 _until) external; + /// @param validAfter The time after which the owner is valid. + /// @param validUntil The time until which the owner is valid. + function addSessionKey(address sessionKey, bytes4 allowedSelector, uint48 validAfter, uint48 validUntil) external; /// @notice Remove a session key from the account. /// @dev This function is installed on the account as part of plugin installation, and should @@ -72,13 +72,13 @@ interface IModularSessionKeyPlugin { /// only be called from an account. /// @param sessionKeys The addresses of the session keys. /// @param allowedSelectors The selectors of the functions that the session keys are allowed to call. - /// @param _afters The times after which the owners are valid. - /// @param _untils The times until which the owners are valid. + /// @param validAfters The times after which the owners are valid. + /// @param validUntils The times until which the owners are valid. function addSessionKeyBatch( address[] calldata sessionKeys, bytes4[] calldata allowedSelectors, - uint48[] calldata _afters, - uint48[] calldata _untils + uint48[] calldata validAfters, + uint48[] calldata validUntils ) external; /// @notice Remove session keys from the account. @@ -95,7 +95,7 @@ interface IModularSessionKeyPlugin { function getSessionDuration(address account, address sessionKey, bytes4 allowedSelector) external view - returns (uint48 _after, uint48 _until); + returns (uint48 validAfter, uint48 validUntil); /// @notice Get all session keys and selectors for a given account. /// @param account The account to get session keys and selectors for. From 874b578e52cfff4669e891fb13b15a93cad016e4 Mon Sep 17 00:00:00 2001 From: sm-stack Date: Fri, 16 Feb 2024 21:53:27 +0900 Subject: [PATCH 33/33] Apply formatting --- src/samples/plugins/interfaces/ISessionKeyPlugin.sol | 3 ++- test/samples/plugins/ModularSessionKeyPlugin.t.sol | 6 +----- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/samples/plugins/interfaces/ISessionKeyPlugin.sol b/src/samples/plugins/interfaces/ISessionKeyPlugin.sol index 0068fa46..88d31f1a 100644 --- a/src/samples/plugins/interfaces/ISessionKeyPlugin.sol +++ b/src/samples/plugins/interfaces/ISessionKeyPlugin.sol @@ -58,7 +58,8 @@ interface IModularSessionKeyPlugin { /// @param allowedSelector The selector of the function that the session key is allowed to call. /// @param validAfter The time after which the owner is valid. /// @param validUntil The time until which the owner is valid. - function addSessionKey(address sessionKey, bytes4 allowedSelector, uint48 validAfter, uint48 validUntil) external; + function addSessionKey(address sessionKey, bytes4 allowedSelector, uint48 validAfter, uint48 validUntil) + external; /// @notice Remove a session key from the account. /// @dev This function is installed on the account as part of plugin installation, and should diff --git a/test/samples/plugins/ModularSessionKeyPlugin.t.sol b/test/samples/plugins/ModularSessionKeyPlugin.t.sol index 96607f9e..bdbcbfff 100644 --- a/test/samples/plugins/ModularSessionKeyPlugin.t.sol +++ b/test/samples/plugins/ModularSessionKeyPlugin.t.sol @@ -61,11 +61,7 @@ contract ModularSessionKeyPluginTest is Test { ); event SessionKeyRemoved(address indexed account, address indexed sessionKey, bytes4 allowedSelector); event SessionKeysAdded( - address indexed account, - address[] sessionKeys, - bytes4[] allowedSelectors, - uint48[] afters, - uint48[] untils + address indexed account, address[] sessionKeys, bytes4[] allowedSelectors, uint48[] afters, uint48[] untils ); event SessionKeysRemoved(address indexed account, address[] sessionKeys, bytes4[] allowedSelectors); event PluginUninstalled(address indexed plugin, bool indexed onUninstallSuccess);