From acbf20ab0eb2544e2dc8892523fcd21ce889caf8 Mon Sep 17 00:00:00 2001 From: howydev <132113803+howydev@users.noreply.github.com> Date: Wed, 12 Jun 2024 16:04:58 -0400 Subject: [PATCH 01/11] feat: add native token spend limit plugin --- src/plugins/NativeTokenLimitPlugin.sol | 152 +++++++++++++++++ test/plugin/NativeTokenLimitPlugin.t.sol | 206 +++++++++++++++++++++++ 2 files changed, 358 insertions(+) create mode 100644 src/plugins/NativeTokenLimitPlugin.sol create mode 100644 test/plugin/NativeTokenLimitPlugin.t.sol diff --git a/src/plugins/NativeTokenLimitPlugin.sol b/src/plugins/NativeTokenLimitPlugin.sol new file mode 100644 index 00000000..5127f18a --- /dev/null +++ b/src/plugins/NativeTokenLimitPlugin.sol @@ -0,0 +1,152 @@ +// SPDX-License-Identifier: GPL-3.0 +pragma solidity ^0.8.25; + +import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interfaces/PackedUserOperation.sol"; +import {UserOperationLib} from "@eth-infinitism/account-abstraction/core/UserOperationLib.sol"; +import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; +import {IAccountExecute} from "@eth-infinitism/account-abstraction/interfaces/IAccountExecute.sol"; + +import {PluginManifest, PluginMetadata} from "../interfaces/IPlugin.sol"; +import {IStandardExecutor, Call} from "../interfaces/IStandardExecutor.sol"; +import {IPlugin} from "../interfaces/IPlugin.sol"; +import {IExecutionHook} from "../interfaces/IExecutionHook.sol"; +import {IValidationHook} from "../interfaces/IValidationHook.sol"; +import {BasePlugin, IERC165} from "./BasePlugin.sol"; + +/// @title Native Token Limit Plugin +/// @author ERC-6900 Authors +/// @notice This plugin supports a single total native token spend limit. +/// It tracks a total spend limit across UserOperation gas limits and native token transfers. +/// If a paymaster is used, UO gas would not cause the limit to decrease. +contract NativeTokenLimitPlugin is BasePlugin, IExecutionHook, IValidationHook { + using UserOperationLib for PackedUserOperation; + using EnumerableSet for EnumerableSet.Bytes32Set; + + string public constant NAME = "Native Token Limit"; + string public constant VERSION = "1.0.0"; + string public constant AUTHOR = "ERC-6900 Authors"; + + mapping(address account => uint256[] limits) public limits; + + error ExceededNativeTokenLimit(); + error ExceededNumberOfEntities(); + + function updateLimits(uint8 functionId, uint256 newLimit) external { + limits[msg.sender][functionId] = newLimit; + } + + /// @inheritdoc IExecutionHook + function preExecutionHook(uint8 functionId, bytes calldata data) external override returns (bytes memory) { + bytes calldata callData; + bytes4 execSelector; + + // TODO: plugins should never have to do these gymnastics + execSelector = bytes4(data[52:56]); + if (execSelector == IAccountExecute.executeUserOp.selector) { + callData = data[56:]; + execSelector = bytes4(callData); + } else { + callData = data[52:]; + } + + uint256 value; + // Get value being sent + if (execSelector == IStandardExecutor.execute.selector) { + value = uint256(bytes32(callData[36:68])); + } else if (execSelector == IStandardExecutor.executeBatch.selector) { + Call[] memory calls = abi.decode(callData[4:], (Call[])); + for (uint256 i = 0; i < calls.length; i++) { + value += calls[i].value; + } + } + + uint256 limit = limits[msg.sender][functionId]; + if (value > limit) { + revert ExceededNativeTokenLimit(); + } + limits[msg.sender][functionId] = limit - value; + + return ""; + } + + /// @inheritdoc IExecutionHook + function postExecutionHook(uint8, bytes calldata) external pure override { + revert NotImplemented(); + } + + // No implementation, no revert + // Runtime spends no account gas, and we check native token spend limits in exec hooks + function preRuntimeValidationHook(uint8 functionId, address, uint256, bytes calldata) external pure override { + // silence warnings + (functionId); + } + + /// @inheritdoc IValidationHook + function preUserOpValidationHook(uint8 functionId, PackedUserOperation calldata userOp, bytes32) + external + returns (uint256) + { + // Decrease limit only if no paymaster is used + if (userOp.paymasterAndData.length == 0) { + uint256 vgl = UserOperationLib.unpackVerificationGasLimit(userOp); + uint256 cgl = UserOperationLib.unpackCallGasLimit(userOp); + uint256 totalGas = userOp.preVerificationGas + vgl + cgl; + uint256 usage = totalGas * UserOperationLib.unpackMaxFeePerGas(userOp); + + uint256 limit = limits[msg.sender][functionId]; + if (usage > limit) { + revert ExceededNativeTokenLimit(); + } + limits[msg.sender][functionId] = limit - usage; + } + return 0; + } + + /// @inheritdoc IPlugin + function onInstall(bytes calldata data) external override { + uint256[] memory spendLimits = abi.decode(data, (uint256[])); + + for (uint256 i = 0; i < spendLimits.length; i++) { + limits[msg.sender].push(spendLimits[i]); + } + + if (limits[msg.sender].length > type(uint8).max) { + revert ExceededNumberOfEntities(); + } + } + + /// @inheritdoc IPlugin + function onUninstall(bytes calldata data) external override { + uint8 functionId = abi.decode(data, (uint8)); + delete limits[msg.sender][functionId]; + } + + /// @inheritdoc IPlugin + function pluginManifest() external pure override returns (PluginManifest memory) { + // silence warnings + PluginManifest memory manifest; + return manifest; + } + + /// @inheritdoc IPlugin + function pluginMetadata() external pure virtual override returns (PluginMetadata memory) { + PluginMetadata memory metadata; + metadata.name = NAME; + metadata.version = VERSION; + metadata.author = AUTHOR; + + metadata.permissionRequest = new string[](2); + metadata.permissionRequest[0] = "native-token-limit"; + metadata.permissionRequest[1] = "gas-limit"; + return metadata; + } + + // ┏━━━━━━━━━━━━━━━┓ + // ┃ EIP-165 ┃ + // ┗━━━━━━━━━━━━━━━┛ + + /// @inheritdoc BasePlugin + function supportsInterface(bytes4 interfaceId) public view override(BasePlugin, IERC165) returns (bool) { + return super.supportsInterface(interfaceId); + } +} diff --git a/test/plugin/NativeTokenLimitPlugin.t.sol b/test/plugin/NativeTokenLimitPlugin.t.sol new file mode 100644 index 00000000..4284d36e --- /dev/null +++ b/test/plugin/NativeTokenLimitPlugin.t.sol @@ -0,0 +1,206 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.19; + +import {EntryPoint} from "@eth-infinitism/account-abstraction/core/EntryPoint.sol"; +import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interfaces/PackedUserOperation.sol"; + +import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; +import {FunctionReference} from "../../src/helpers/FunctionReferenceLib.sol"; +import {NativeTokenLimitPlugin} from "../../src/plugins/NativeTokenLimitPlugin.sol"; +import {MockPlugin} from "../mocks/MockPlugin.sol"; +import {ExecutionHook} from "../../src/interfaces/IAccountLoupe.sol"; +import {FunctionReferenceLib} from "../../src/helpers/FunctionReferenceLib.sol"; +import {IStandardExecutor, Call} from "../../src/interfaces/IStandardExecutor.sol"; +import {PluginManifest} from "../../src/interfaces/IPlugin.sol"; + +import {MSCAFactoryFixture} from "../mocks/MSCAFactoryFixture.sol"; +import {OptimizedTest} from "../utils/OptimizedTest.sol"; + +contract NativeTokenLimitPluginTest is OptimizedTest { + EntryPoint public entryPoint = new EntryPoint(); + address public recipient = address(1); + address payable public bundler = payable(address(2)); + PluginManifest internal _m; + MockPlugin public validationPlugin = new MockPlugin(_m); + FunctionReference public validationFunction; + + UpgradeableModularAccount public acct; + NativeTokenLimitPlugin public plugin = new NativeTokenLimitPlugin(); + uint256 public spendLimit = 10 ether; + + function setUp() public { + // Set up a validator with hooks from the gas spend limit plugin attached + + MSCAFactoryFixture factory = new MSCAFactoryFixture(entryPoint, _deploySingleOwnerPlugin()); + + acct = factory.createAccount(address(this), 0); + + vm.deal(address(acct), 10 ether); + + FunctionReference[] memory preValidationHooks = new FunctionReference[](1); + preValidationHooks[0] = FunctionReferenceLib.pack(address(plugin), 0); + + ExecutionHook[] memory permissionHooks = new ExecutionHook[](1); + permissionHooks[0] = ExecutionHook({ + hookFunction: FunctionReferenceLib.pack(address(plugin), 0), + isPreHook: true, + isPostHook: false, + requireUOContext: false + }); + + uint256[] memory spendLimits = new uint256[](1); + spendLimits[0] = spendLimit; + + bytes[] memory preValHooksInitDatas = new bytes[](1); + preValHooksInitDatas[0] = ""; + + bytes[] memory permissionInitDatas = new bytes[](1); + permissionInitDatas[0] = abi.encode(spendLimits); + + vm.prank(address(acct)); + acct.installValidation( + FunctionReferenceLib.pack(address(validationPlugin), 0), + true, + new bytes4[](0), + new bytes(0), + abi.encode(preValidationHooks, preValHooksInitDatas), + abi.encode(permissionHooks, permissionInitDatas) + ); + + validationFunction = FunctionReferenceLib.pack(address(validationPlugin), 0); + } + + function _getExecuteWithValue(uint256 value) internal view returns (bytes memory) { + return abi.encodeCall(UpgradeableModularAccount.execute, (recipient, value, "")); + } + + function _getPackedUO(uint256 gas1, uint256 gas2, uint256 gas3, uint256 gasPrice, bytes memory callData) + internal + view + returns (PackedUserOperation memory uo) + { + uo = PackedUserOperation({ + sender: address(acct), + nonce: 0, + initCode: "", + callData: abi.encodePacked(UpgradeableModularAccount.executeUserOp.selector, callData), + accountGasLimits: bytes32(bytes16(uint128(gas1))) | bytes32(uint256(gas2)), + preVerificationGas: gas3, + gasFees: bytes32(uint256(uint128(gasPrice))), + paymasterAndData: "", + signature: abi.encodePacked(FunctionReferenceLib.pack(address(validationPlugin), 0), uint8(1)) + }); + } + + function test_userOp_gasLimit() public { + vm.startPrank(address(entryPoint)); + + // uses 10e - 200000 of gas + assertEq(plugin.limits(address(acct), 0), 10 ether); + uint256 result = acct.validateUserOp( + _getPackedUO(100000, 100000, 10 ether - 400000, 1, _getExecuteWithValue(0)), bytes32(0), 0 + ); + assertEq(plugin.limits(address(acct), 0), 200000); + + uint256 expected = uint256(type(uint48).max) << 160; + assertEq(result, expected); + + // uses 200k + 1 wei of gas + vm.expectRevert(NativeTokenLimitPlugin.ExceededNativeTokenLimit.selector); + result = acct.validateUserOp(_getPackedUO(100000, 100000, 1, 1, _getExecuteWithValue(0)), bytes32(0), 0); + } + + function test_userOp_executeLimit() public { + vm.startPrank(address(entryPoint)); + + // uses 5e of native tokens + assertEq(plugin.limits(address(acct), 0), 10 ether); + acct.executeUserOp(_getPackedUO(0, 0, 0, 0, _getExecuteWithValue(5 ether)), bytes32(0)); + assertEq(plugin.limits(address(acct), 0), 5 ether); + + // uses 5e + 1wei of native tokens + vm.expectRevert( + abi.encodePacked( + UpgradeableModularAccount.PreExecHookReverted.selector, + abi.encode( + address(plugin), + uint8(0), + abi.encodePacked(NativeTokenLimitPlugin.ExceededNativeTokenLimit.selector) + ) + ) + ); + acct.executeUserOp(_getPackedUO(0, 0, 0, 0, _getExecuteWithValue(5 ether + 1)), bytes32(0)); + } + + function test_userOp_executeBatchLimit() public { + Call[] memory calls = new Call[](3); + calls[0] = Call({target: recipient, value: 1, data: ""}); + calls[1] = Call({target: recipient, value: 1 ether, data: ""}); + calls[2] = Call({target: recipient, value: 5 ether + 100000, data: ""}); + + vm.startPrank(address(entryPoint)); + assertEq(plugin.limits(address(acct), 0), 10 ether); + acct.executeUserOp( + _getPackedUO(0, 0, 0, 0, abi.encodeCall(IStandardExecutor.executeBatch, (calls))), bytes32(0) + ); + assertEq(plugin.limits(address(acct), 0), 10 ether - 6 ether - 100001); + assertEq(recipient.balance, 6 ether + 100001); + } + + function test_userOp_combinedExecLimit_success() public { + assertEq(plugin.limits(address(acct), 0), 10 ether); + PackedUserOperation[] memory uos = new PackedUserOperation[](1); + uos[0] = _getPackedUO(100000, 100000, 100000, 1, _getExecuteWithValue(5 ether)); + entryPoint.handleOps(uos, bundler); + + assertEq(plugin.limits(address(acct), 0), 5 ether - 300000); + assertEq(recipient.balance, 5 ether); + } + + function test_userOp_combinedExecBatchLimit_success() public { + Call[] memory calls = new Call[](3); + calls[0] = Call({target: recipient, value: 1, data: ""}); + calls[1] = Call({target: recipient, value: 1 ether, data: ""}); + calls[2] = Call({target: recipient, value: 5 ether + 100000, data: ""}); + + vm.startPrank(address(entryPoint)); + assertEq(plugin.limits(address(acct), 0), 10 ether); + PackedUserOperation[] memory uos = new PackedUserOperation[](1); + uos[0] = _getPackedUO(200000, 200000, 200000, 1, abi.encodeCall(IStandardExecutor.executeBatch, (calls))); + entryPoint.handleOps(uos, bundler); + + assertEq(plugin.limits(address(acct), 0), 10 ether - 6 ether - 700001); + assertEq(recipient.balance, 6 ether + 100001); + } + + function test_userOp_combinedExecLimit_failExec() public { + assertEq(plugin.limits(address(acct), 0), 10 ether); + PackedUserOperation[] memory uos = new PackedUserOperation[](1); + uos[0] = _getPackedUO(100000, 100000, 100000, 1, _getExecuteWithValue(10 ether)); + entryPoint.handleOps(uos, bundler); + + assertEq(plugin.limits(address(acct), 0), 10 ether - 300000); + assertEq(recipient.balance, 0); + } + + function test_runtime_executeLimit() public { + assertEq(plugin.limits(address(acct), 0), 10 ether); + acct.executeWithAuthorization( + _getExecuteWithValue(5 ether), abi.encodePacked(validationFunction, uint8(1)) + ); + assertEq(plugin.limits(address(acct), 0), 5 ether); + } + + function test_runtime_executeBatchLimit() public { + Call[] memory calls = new Call[](3); + calls[0] = Call({target: recipient, value: 1, data: ""}); + calls[1] = Call({target: recipient, value: 1 ether, data: ""}); + calls[2] = Call({target: recipient, value: 5 ether + 100000, data: ""}); + + assertEq(plugin.limits(address(acct), 0), 10 ether); + acct.executeWithAuthorization( + abi.encodeCall(IStandardExecutor.executeBatch, (calls)), abi.encodePacked(validationFunction, uint8(1)) + ); + assertEq(plugin.limits(address(acct), 0), 4 ether - 100001); + } +} From 8d445685cc18c768dc6d67d22ee39d7620674f18 Mon Sep 17 00:00:00 2001 From: howydev <132113803+howydev@users.noreply.github.com> Date: Thu, 27 Jun 2024 15:23:25 -0400 Subject: [PATCH 02/11] chore: update --- src/plugins/NativeTokenLimitPlugin.sol | 37 ++++++++++++++---------- test/plugin/NativeTokenLimitPlugin.t.sol | 3 +- 2 files changed, 22 insertions(+), 18 deletions(-) diff --git a/src/plugins/NativeTokenLimitPlugin.sol b/src/plugins/NativeTokenLimitPlugin.sol index 5127f18a..19a192d3 100644 --- a/src/plugins/NativeTokenLimitPlugin.sol +++ b/src/plugins/NativeTokenLimitPlugin.sol @@ -4,12 +4,12 @@ pragma solidity ^0.8.25; import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interfaces/PackedUserOperation.sol"; import {UserOperationLib} from "@eth-infinitism/account-abstraction/core/UserOperationLib.sol"; import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; -import {IAccountExecute} from "@eth-infinitism/account-abstraction/interfaces/IAccountExecute.sol"; import {PluginManifest, PluginMetadata} from "../interfaces/IPlugin.sol"; import {IStandardExecutor, Call} from "../interfaces/IStandardExecutor.sol"; import {IPlugin} from "../interfaces/IPlugin.sol"; import {IExecutionHook} from "../interfaces/IExecutionHook.sol"; +import {IPermissionHook} from "../interfaces/IPermissionHook.sol"; import {IValidationHook} from "../interfaces/IValidationHook.sol"; import {BasePlugin, IERC165} from "./BasePlugin.sol"; @@ -18,7 +18,7 @@ import {BasePlugin, IERC165} from "./BasePlugin.sol"; /// @notice This plugin supports a single total native token spend limit. /// It tracks a total spend limit across UserOperation gas limits and native token transfers. /// If a paymaster is used, UO gas would not cause the limit to decrease. -contract NativeTokenLimitPlugin is BasePlugin, IExecutionHook, IValidationHook { +contract NativeTokenLimitPlugin is BasePlugin, IExecutionHook, IPermissionHook, IValidationHook { using UserOperationLib for PackedUserOperation; using EnumerableSet for EnumerableSet.Bytes32Set; @@ -36,25 +36,30 @@ contract NativeTokenLimitPlugin is BasePlugin, IExecutionHook, IValidationHook { } /// @inheritdoc IExecutionHook - function preExecutionHook(uint8 functionId, bytes calldata data) external override returns (bytes memory) { - bytes calldata callData; - bytes4 execSelector; - - // TODO: plugins should never have to do these gymnastics - execSelector = bytes4(data[52:56]); - if (execSelector == IAccountExecute.executeUserOp.selector) { - callData = data[56:]; - execSelector = bytes4(callData); - } else { - callData = data[52:]; - } + function preExecutionHook(uint8 functionId, address, uint256, bytes calldata data) + external + override + returns (bytes memory) + { + return _checkAndDecrementLimit(functionId, data); + } + + function preUserOpExecutionHook(uint8 functionId, PackedUserOperation calldata uo) + external + override + returns (bytes memory) + { + return _checkAndDecrementLimit(functionId, uo.callData); + } + function _checkAndDecrementLimit(uint8 functionId, bytes calldata data) internal returns (bytes memory) { + bytes4 execSelector = bytes4(data); uint256 value; // Get value being sent if (execSelector == IStandardExecutor.execute.selector) { - value = uint256(bytes32(callData[36:68])); + value = uint256(bytes32(data[36:68])); } else if (execSelector == IStandardExecutor.executeBatch.selector) { - Call[] memory calls = abi.decode(callData[4:], (Call[])); + Call[] memory calls = abi.decode(data[4:], (Call[])); for (uint256 i = 0; i < calls.length; i++) { value += calls[i].value; } diff --git a/test/plugin/NativeTokenLimitPlugin.t.sol b/test/plugin/NativeTokenLimitPlugin.t.sol index 4284d36e..81b8f7ea 100644 --- a/test/plugin/NativeTokenLimitPlugin.t.sol +++ b/test/plugin/NativeTokenLimitPlugin.t.sol @@ -44,8 +44,7 @@ contract NativeTokenLimitPluginTest is OptimizedTest { permissionHooks[0] = ExecutionHook({ hookFunction: FunctionReferenceLib.pack(address(plugin), 0), isPreHook: true, - isPostHook: false, - requireUOContext: false + isPostHook: false }); uint256[] memory spendLimits = new uint256[](1); From 5f3d43c7ef8542c6a65689f74f0bf4d9f1f3cc23 Mon Sep 17 00:00:00 2001 From: howydev <132113803+howydev@users.noreply.github.com> Date: Thu, 27 Jun 2024 15:42:51 -0400 Subject: [PATCH 03/11] fix: tests --- test/plugin/NativeTokenLimitPlugin.t.sol | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/plugin/NativeTokenLimitPlugin.t.sol b/test/plugin/NativeTokenLimitPlugin.t.sol index 81b8f7ea..07a554d2 100644 --- a/test/plugin/NativeTokenLimitPlugin.t.sol +++ b/test/plugin/NativeTokenLimitPlugin.t.sol @@ -149,10 +149,10 @@ contract NativeTokenLimitPluginTest is OptimizedTest { function test_userOp_combinedExecLimit_success() public { assertEq(plugin.limits(address(acct), 0), 10 ether); PackedUserOperation[] memory uos = new PackedUserOperation[](1); - uos[0] = _getPackedUO(100000, 100000, 100000, 1, _getExecuteWithValue(5 ether)); + uos[0] = _getPackedUO(200000, 200000, 200000, 1, _getExecuteWithValue(5 ether)); entryPoint.handleOps(uos, bundler); - assertEq(plugin.limits(address(acct), 0), 5 ether - 300000); + assertEq(plugin.limits(address(acct), 0), 5 ether - 600000); assertEq(recipient.balance, 5 ether); } @@ -175,10 +175,10 @@ contract NativeTokenLimitPluginTest is OptimizedTest { function test_userOp_combinedExecLimit_failExec() public { assertEq(plugin.limits(address(acct), 0), 10 ether); PackedUserOperation[] memory uos = new PackedUserOperation[](1); - uos[0] = _getPackedUO(100000, 100000, 100000, 1, _getExecuteWithValue(10 ether)); + uos[0] = _getPackedUO(200000, 200000, 200000, 1, _getExecuteWithValue(10 ether)); entryPoint.handleOps(uos, bundler); - assertEq(plugin.limits(address(acct), 0), 10 ether - 300000); + assertEq(plugin.limits(address(acct), 0), 10 ether - 600000); assertEq(recipient.balance, 0); } From 632209a7453e3e3ef14ff4b409672962974eac0e Mon Sep 17 00:00:00 2001 From: howydev <132113803+howydev@users.noreply.github.com> Date: Fri, 28 Jun 2024 16:50:04 -0400 Subject: [PATCH 04/11] chore: remove permission hooks, add new internal helper function for devX, fix lint --- src/plugins/BasePlugin.sol | 24 +++++++ src/plugins/NativeTokenLimitPlugin.sol | 99 ++++++++++++-------------- 2 files changed, 70 insertions(+), 53 deletions(-) diff --git a/src/plugins/BasePlugin.sol b/src/plugins/BasePlugin.sol index 3b0fd521..18345b14 100644 --- a/src/plugins/BasePlugin.sol +++ b/src/plugins/BasePlugin.sol @@ -2,6 +2,8 @@ pragma solidity ^0.8.25; import {ERC165, IERC165} from "@openzeppelin/contracts/utils/introspection/ERC165.sol"; +import {IAccountExecute} from "@eth-infinitism/account-abstraction/interfaces/IAccountExecute.sol"; +import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interfaces/PackedUserOperation.sol"; import {IPlugin} from "../interfaces/IPlugin.sol"; @@ -27,4 +29,26 @@ abstract contract BasePlugin is ERC165, IPlugin { function supportsInterface(bytes4 interfaceId) public view virtual override(ERC165, IERC165) returns (bool) { return interfaceId == type(IPlugin).interfaceId || super.supportsInterface(interfaceId); } + + function _getSelectorAndCalldata(bytes calldata data) internal view returns (bytes4, bytes memory) { + if (bytes4(data[:4]) == IAccountExecute.executeUserOp.selector) { + (PackedUserOperation memory uo,) = abi.decode(data[4:], (PackedUserOperation, bytes32)); + bytes4 selector; + bytes memory callData = uo.callData; + // Bytes arr representation: [bytes32(len), bytes4(executeUserOp.selector), bytes4(actualSelector), + // bytes(actualCallData)] + // 1. Copy actualSelector into a new var + // 2. Shorten bytes arry by 8 by: store length - 8 into the new pointer location + // 3. Move the callData pointer by 8 + assembly { + selector := mload(add(callData, 36)) + + let len := mload(callData) + mstore(add(callData, 8), sub(len, 8)) + callData := add(callData, 8) + } + return (selector, callData); + } + return (bytes4(data[:4]), data[4:]); + } } diff --git a/src/plugins/NativeTokenLimitPlugin.sol b/src/plugins/NativeTokenLimitPlugin.sol index 19a192d3..5fd806d6 100644 --- a/src/plugins/NativeTokenLimitPlugin.sol +++ b/src/plugins/NativeTokenLimitPlugin.sol @@ -9,7 +9,6 @@ import {PluginManifest, PluginMetadata} from "../interfaces/IPlugin.sol"; import {IStandardExecutor, Call} from "../interfaces/IStandardExecutor.sol"; import {IPlugin} from "../interfaces/IPlugin.sol"; import {IExecutionHook} from "../interfaces/IExecutionHook.sol"; -import {IPermissionHook} from "../interfaces/IPermissionHook.sol"; import {IValidationHook} from "../interfaces/IValidationHook.sol"; import {BasePlugin, IERC165} from "./BasePlugin.sol"; @@ -18,7 +17,8 @@ import {BasePlugin, IERC165} from "./BasePlugin.sol"; /// @notice This plugin supports a single total native token spend limit. /// It tracks a total spend limit across UserOperation gas limits and native token transfers. /// If a paymaster is used, UO gas would not cause the limit to decrease. -contract NativeTokenLimitPlugin is BasePlugin, IExecutionHook, IPermissionHook, IValidationHook { + +contract NativeTokenLimitPlugin is BasePlugin, IExecutionHook, IValidationHook { using UserOperationLib for PackedUserOperation; using EnumerableSet for EnumerableSet.Bytes32Set; @@ -35,57 +35,6 @@ contract NativeTokenLimitPlugin is BasePlugin, IExecutionHook, IPermissionHook, limits[msg.sender][functionId] = newLimit; } - /// @inheritdoc IExecutionHook - function preExecutionHook(uint8 functionId, address, uint256, bytes calldata data) - external - override - returns (bytes memory) - { - return _checkAndDecrementLimit(functionId, data); - } - - function preUserOpExecutionHook(uint8 functionId, PackedUserOperation calldata uo) - external - override - returns (bytes memory) - { - return _checkAndDecrementLimit(functionId, uo.callData); - } - - function _checkAndDecrementLimit(uint8 functionId, bytes calldata data) internal returns (bytes memory) { - bytes4 execSelector = bytes4(data); - uint256 value; - // Get value being sent - if (execSelector == IStandardExecutor.execute.selector) { - value = uint256(bytes32(data[36:68])); - } else if (execSelector == IStandardExecutor.executeBatch.selector) { - Call[] memory calls = abi.decode(data[4:], (Call[])); - for (uint256 i = 0; i < calls.length; i++) { - value += calls[i].value; - } - } - - uint256 limit = limits[msg.sender][functionId]; - if (value > limit) { - revert ExceededNativeTokenLimit(); - } - limits[msg.sender][functionId] = limit - value; - - return ""; - } - - /// @inheritdoc IExecutionHook - function postExecutionHook(uint8, bytes calldata) external pure override { - revert NotImplemented(); - } - - // No implementation, no revert - // Runtime spends no account gas, and we check native token spend limits in exec hooks - function preRuntimeValidationHook(uint8 functionId, address, uint256, bytes calldata) external pure override { - // silence warnings - (functionId); - } - /// @inheritdoc IValidationHook function preUserOpValidationHook(uint8 functionId, PackedUserOperation calldata userOp, bytes32) external @@ -107,6 +56,15 @@ contract NativeTokenLimitPlugin is BasePlugin, IExecutionHook, IPermissionHook, return 0; } + /// @inheritdoc IExecutionHook + function preExecutionHook(uint8 functionId, address, uint256, bytes calldata data) + external + override + returns (bytes memory) + { + return _checkAndDecrementLimit(functionId, data); + } + /// @inheritdoc IPlugin function onInstall(bytes calldata data) external override { uint256[] memory spendLimits = abi.decode(data, (uint256[])); @@ -126,6 +84,18 @@ contract NativeTokenLimitPlugin is BasePlugin, IExecutionHook, IPermissionHook, delete limits[msg.sender][functionId]; } + /// @inheritdoc IExecutionHook + function postExecutionHook(uint8, bytes calldata) external pure override { + revert NotImplemented(); + } + + // No implementation, no revert + // Runtime spends no account gas, and we check native token spend limits in exec hooks + function preRuntimeValidationHook(uint8 functionId, address, uint256, bytes calldata) external pure override { + // silence warnings + (functionId); + } + /// @inheritdoc IPlugin function pluginManifest() external pure override returns (PluginManifest memory) { // silence warnings @@ -154,4 +124,27 @@ contract NativeTokenLimitPlugin is BasePlugin, IExecutionHook, IPermissionHook, function supportsInterface(bytes4 interfaceId) public view override(BasePlugin, IERC165) returns (bool) { return super.supportsInterface(interfaceId); } + + function _checkAndDecrementLimit(uint8 functionId, bytes calldata data) internal returns (bytes memory) { + (bytes4 selector, bytes memory callData) = _getSelectorAndCalldata(data); + + uint256 value; + // Get value being sent + if (selector == IStandardExecutor.execute.selector) { + (, value) = abi.decode(callData, (address, uint256)); + } else if (selector == IStandardExecutor.executeBatch.selector) { + Call[] memory calls = abi.decode(callData, (Call[])); + for (uint256 i = 0; i < calls.length; i++) { + value += calls[i].value; + } + } + + uint256 limit = limits[msg.sender][functionId]; + if (value > limit) { + revert ExceededNativeTokenLimit(); + } + limits[msg.sender][functionId] = limit - value; + + return ""; + } } From b1a2a32ed7397834b3e2d476c82ede07bf360b78 Mon Sep 17 00:00:00 2001 From: howydev <132113803+howydev@users.noreply.github.com> Date: Mon, 8 Jul 2024 13:34:38 -0400 Subject: [PATCH 05/11] fix: move to associated storage --- src/plugins/NativeTokenLimitPlugin.sol | 27 ++++++++++--------- test/plugin/NativeTokenLimitPlugin.t.sol | 34 ++++++++++++------------ 2 files changed, 32 insertions(+), 29 deletions(-) diff --git a/src/plugins/NativeTokenLimitPlugin.sol b/src/plugins/NativeTokenLimitPlugin.sol index 5fd806d6..984b373b 100644 --- a/src/plugins/NativeTokenLimitPlugin.sol +++ b/src/plugins/NativeTokenLimitPlugin.sol @@ -26,13 +26,13 @@ contract NativeTokenLimitPlugin is BasePlugin, IExecutionHook, IValidationHook { string public constant VERSION = "1.0.0"; string public constant AUTHOR = "ERC-6900 Authors"; - mapping(address account => uint256[] limits) public limits; + mapping(uint256 funcIds => mapping(address account => uint256 limit)) public limits; error ExceededNativeTokenLimit(); error ExceededNumberOfEntities(); function updateLimits(uint8 functionId, uint256 newLimit) external { - limits[msg.sender][functionId] = newLimit; + limits[functionId][msg.sender] = newLimit; } /// @inheritdoc IValidationHook @@ -47,11 +47,11 @@ contract NativeTokenLimitPlugin is BasePlugin, IExecutionHook, IValidationHook { uint256 totalGas = userOp.preVerificationGas + vgl + cgl; uint256 usage = totalGas * UserOperationLib.unpackMaxFeePerGas(userOp); - uint256 limit = limits[msg.sender][functionId]; + uint256 limit = limits[functionId][msg.sender]; if (usage > limit) { revert ExceededNativeTokenLimit(); } - limits[msg.sender][functionId] = limit - usage; + limits[functionId][msg.sender] = limit - usage; } return 0; } @@ -67,21 +67,24 @@ contract NativeTokenLimitPlugin is BasePlugin, IExecutionHook, IValidationHook { /// @inheritdoc IPlugin function onInstall(bytes calldata data) external override { - uint256[] memory spendLimits = abi.decode(data, (uint256[])); + (uint8 startFunctionId, uint256[] memory spendLimits) = abi.decode(data, (uint8, uint256[])); - for (uint256 i = 0; i < spendLimits.length; i++) { - limits[msg.sender].push(spendLimits[i]); + if (startFunctionId + spendLimits.length > type(uint8).max) { + revert ExceededNumberOfEntities(); } - if (limits[msg.sender].length > type(uint8).max) { - revert ExceededNumberOfEntities(); + for (uint256 i = 0; i < spendLimits.length; i++) { + limits[i + startFunctionId][msg.sender] = spendLimits[i]; } } /// @inheritdoc IPlugin function onUninstall(bytes calldata data) external override { + // This is the highest functionId that's being used by the account uint8 functionId = abi.decode(data, (uint8)); - delete limits[msg.sender][functionId]; + for (uint256 i = 0; i < functionId; i++) { + delete limits[i][msg.sender]; + } } /// @inheritdoc IExecutionHook @@ -139,11 +142,11 @@ contract NativeTokenLimitPlugin is BasePlugin, IExecutionHook, IValidationHook { } } - uint256 limit = limits[msg.sender][functionId]; + uint256 limit = limits[functionId][msg.sender]; if (value > limit) { revert ExceededNativeTokenLimit(); } - limits[msg.sender][functionId] = limit - value; + limits[functionId][msg.sender] = limit - value; return ""; } diff --git a/test/plugin/NativeTokenLimitPlugin.t.sol b/test/plugin/NativeTokenLimitPlugin.t.sol index 07a554d2..b8072238 100644 --- a/test/plugin/NativeTokenLimitPlugin.t.sol +++ b/test/plugin/NativeTokenLimitPlugin.t.sol @@ -54,7 +54,7 @@ contract NativeTokenLimitPluginTest is OptimizedTest { preValHooksInitDatas[0] = ""; bytes[] memory permissionInitDatas = new bytes[](1); - permissionInitDatas[0] = abi.encode(spendLimits); + permissionInitDatas[0] = abi.encode(0, spendLimits); vm.prank(address(acct)); acct.installValidation( @@ -95,11 +95,11 @@ contract NativeTokenLimitPluginTest is OptimizedTest { vm.startPrank(address(entryPoint)); // uses 10e - 200000 of gas - assertEq(plugin.limits(address(acct), 0), 10 ether); + assertEq(plugin.limits(0, address(acct)), 10 ether); uint256 result = acct.validateUserOp( _getPackedUO(100000, 100000, 10 ether - 400000, 1, _getExecuteWithValue(0)), bytes32(0), 0 ); - assertEq(plugin.limits(address(acct), 0), 200000); + assertEq(plugin.limits(0, address(acct)), 200000); uint256 expected = uint256(type(uint48).max) << 160; assertEq(result, expected); @@ -113,9 +113,9 @@ contract NativeTokenLimitPluginTest is OptimizedTest { vm.startPrank(address(entryPoint)); // uses 5e of native tokens - assertEq(plugin.limits(address(acct), 0), 10 ether); + assertEq(plugin.limits(0, address(acct)), 10 ether); acct.executeUserOp(_getPackedUO(0, 0, 0, 0, _getExecuteWithValue(5 ether)), bytes32(0)); - assertEq(plugin.limits(address(acct), 0), 5 ether); + assertEq(plugin.limits(0, address(acct)), 5 ether); // uses 5e + 1wei of native tokens vm.expectRevert( @@ -138,21 +138,21 @@ contract NativeTokenLimitPluginTest is OptimizedTest { calls[2] = Call({target: recipient, value: 5 ether + 100000, data: ""}); vm.startPrank(address(entryPoint)); - assertEq(plugin.limits(address(acct), 0), 10 ether); + assertEq(plugin.limits(0, address(acct)), 10 ether); acct.executeUserOp( _getPackedUO(0, 0, 0, 0, abi.encodeCall(IStandardExecutor.executeBatch, (calls))), bytes32(0) ); - assertEq(plugin.limits(address(acct), 0), 10 ether - 6 ether - 100001); + assertEq(plugin.limits(0, address(acct)), 10 ether - 6 ether - 100001); assertEq(recipient.balance, 6 ether + 100001); } function test_userOp_combinedExecLimit_success() public { - assertEq(plugin.limits(address(acct), 0), 10 ether); + assertEq(plugin.limits(0, address(acct)), 10 ether); PackedUserOperation[] memory uos = new PackedUserOperation[](1); uos[0] = _getPackedUO(200000, 200000, 200000, 1, _getExecuteWithValue(5 ether)); entryPoint.handleOps(uos, bundler); - assertEq(plugin.limits(address(acct), 0), 5 ether - 600000); + assertEq(plugin.limits(0, address(acct)), 5 ether - 600000); assertEq(recipient.balance, 5 ether); } @@ -163,31 +163,31 @@ contract NativeTokenLimitPluginTest is OptimizedTest { calls[2] = Call({target: recipient, value: 5 ether + 100000, data: ""}); vm.startPrank(address(entryPoint)); - assertEq(plugin.limits(address(acct), 0), 10 ether); + assertEq(plugin.limits(0, address(acct)), 10 ether); PackedUserOperation[] memory uos = new PackedUserOperation[](1); uos[0] = _getPackedUO(200000, 200000, 200000, 1, abi.encodeCall(IStandardExecutor.executeBatch, (calls))); entryPoint.handleOps(uos, bundler); - assertEq(plugin.limits(address(acct), 0), 10 ether - 6 ether - 700001); + assertEq(plugin.limits(0, address(acct)), 10 ether - 6 ether - 700001); assertEq(recipient.balance, 6 ether + 100001); } function test_userOp_combinedExecLimit_failExec() public { - assertEq(plugin.limits(address(acct), 0), 10 ether); + assertEq(plugin.limits(0, address(acct)), 10 ether); PackedUserOperation[] memory uos = new PackedUserOperation[](1); uos[0] = _getPackedUO(200000, 200000, 200000, 1, _getExecuteWithValue(10 ether)); entryPoint.handleOps(uos, bundler); - assertEq(plugin.limits(address(acct), 0), 10 ether - 600000); + assertEq(plugin.limits(0, address(acct)), 10 ether - 600000); assertEq(recipient.balance, 0); } function test_runtime_executeLimit() public { - assertEq(plugin.limits(address(acct), 0), 10 ether); + assertEq(plugin.limits(0, address(acct)), 10 ether); acct.executeWithAuthorization( _getExecuteWithValue(5 ether), abi.encodePacked(validationFunction, uint8(1)) ); - assertEq(plugin.limits(address(acct), 0), 5 ether); + assertEq(plugin.limits(0, address(acct)), 5 ether); } function test_runtime_executeBatchLimit() public { @@ -196,10 +196,10 @@ contract NativeTokenLimitPluginTest is OptimizedTest { calls[1] = Call({target: recipient, value: 1 ether, data: ""}); calls[2] = Call({target: recipient, value: 5 ether + 100000, data: ""}); - assertEq(plugin.limits(address(acct), 0), 10 ether); + assertEq(plugin.limits(0, address(acct)), 10 ether); acct.executeWithAuthorization( abi.encodeCall(IStandardExecutor.executeBatch, (calls)), abi.encodePacked(validationFunction, uint8(1)) ); - assertEq(plugin.limits(address(acct), 0), 4 ether - 100001); + assertEq(plugin.limits(0, address(acct)), 4 ether - 100001); } } From 655539c83dc80cf9b82526cc99f8fc2c96912814 Mon Sep 17 00:00:00 2001 From: howydev <132113803+howydev@users.noreply.github.com> Date: Mon, 8 Jul 2024 13:38:44 -0400 Subject: [PATCH 06/11] feat: add special paymaster list to still count towards limits --- src/plugins/BasePlugin.sol | 2 +- src/plugins/NativeTokenLimitPlugin.sol | 18 ++++++++++++++---- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/plugins/BasePlugin.sol b/src/plugins/BasePlugin.sol index 18345b14..f5c8523b 100644 --- a/src/plugins/BasePlugin.sol +++ b/src/plugins/BasePlugin.sol @@ -30,7 +30,7 @@ abstract contract BasePlugin is ERC165, IPlugin { return interfaceId == type(IPlugin).interfaceId || super.supportsInterface(interfaceId); } - function _getSelectorAndCalldata(bytes calldata data) internal view returns (bytes4, bytes memory) { + function _getSelectorAndCalldata(bytes calldata data) internal pure returns (bytes4, bytes memory) { if (bytes4(data[:4]) == IAccountExecute.executeUserOp.selector) { (PackedUserOperation memory uo,) = abi.decode(data[4:], (PackedUserOperation, bytes32)); bytes4 selector; diff --git a/src/plugins/NativeTokenLimitPlugin.sol b/src/plugins/NativeTokenLimitPlugin.sol index 984b373b..777484ce 100644 --- a/src/plugins/NativeTokenLimitPlugin.sol +++ b/src/plugins/NativeTokenLimitPlugin.sol @@ -16,8 +16,8 @@ import {BasePlugin, IERC165} from "./BasePlugin.sol"; /// @author ERC-6900 Authors /// @notice This plugin supports a single total native token spend limit. /// It tracks a total spend limit across UserOperation gas limits and native token transfers. -/// If a paymaster is used, UO gas would not cause the limit to decrease. - +/// If a non whitelisted paymaster is used, UO gas would not cause the limit to decrease. +/// If a whitelisted paymaster is used, gas is still counted towards the limit contract NativeTokenLimitPlugin is BasePlugin, IExecutionHook, IValidationHook { using UserOperationLib for PackedUserOperation; using EnumerableSet for EnumerableSet.Bytes32Set; @@ -27,6 +27,9 @@ contract NativeTokenLimitPlugin is BasePlugin, IExecutionHook, IValidationHook { string public constant AUTHOR = "ERC-6900 Authors"; mapping(uint256 funcIds => mapping(address account => uint256 limit)) public limits; + // Accounts should add paymasters that still use the accounts tokens here + // E.g. ERC20 paymasters that pull funds from the account + mapping(address paymaster => mapping(address account => bool allowed)) public specialPaymasters; error ExceededNativeTokenLimit(); error ExceededNumberOfEntities(); @@ -35,13 +38,20 @@ contract NativeTokenLimitPlugin is BasePlugin, IExecutionHook, IValidationHook { limits[functionId][msg.sender] = newLimit; } + function updateSpecialPaymaster(address paymaster, bool allowed) external { + specialPaymasters[paymaster][msg.sender] = allowed; + } + /// @inheritdoc IValidationHook function preUserOpValidationHook(uint8 functionId, PackedUserOperation calldata userOp, bytes32) external returns (uint256) { - // Decrease limit only if no paymaster is used - if (userOp.paymasterAndData.length == 0) { + // Decrease limit only if no paymaster is used, or if its a special paymaster + if ( + userOp.paymasterAndData.length == 0 + || specialPaymasters[address(bytes20(userOp.paymasterAndData[:20]))][msg.sender] + ) { uint256 vgl = UserOperationLib.unpackVerificationGasLimit(userOp); uint256 cgl = UserOperationLib.unpackCallGasLimit(userOp); uint256 totalGas = userOp.preVerificationGas + vgl + cgl; From 246ddd1c44347c10840c3df750eac49aba578f54 Mon Sep 17 00:00:00 2001 From: howydev <132113803+howydev@users.noreply.github.com> Date: Wed, 10 Jul 2024 13:57:13 -0400 Subject: [PATCH 07/11] c --- src/plugins/NativeTokenLimitPlugin.sol | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/plugins/NativeTokenLimitPlugin.sol b/src/plugins/NativeTokenLimitPlugin.sol index 777484ce..cb84a4ee 100644 --- a/src/plugins/NativeTokenLimitPlugin.sol +++ b/src/plugins/NativeTokenLimitPlugin.sol @@ -22,9 +22,9 @@ contract NativeTokenLimitPlugin is BasePlugin, IExecutionHook, IValidationHook { using UserOperationLib for PackedUserOperation; using EnumerableSet for EnumerableSet.Bytes32Set; - string public constant NAME = "Native Token Limit"; - string public constant VERSION = "1.0.0"; - string public constant AUTHOR = "ERC-6900 Authors"; + string internal constant NAME = "Native Token Limit"; + string internal constant VERSION = "1.0.0"; + string internal constant AUTHOR = "ERC-6900 Authors"; mapping(uint256 funcIds => mapping(address account => uint256 limit)) public limits; // Accounts should add paymasters that still use the accounts tokens here @@ -104,10 +104,7 @@ contract NativeTokenLimitPlugin is BasePlugin, IExecutionHook, IValidationHook { // No implementation, no revert // Runtime spends no account gas, and we check native token spend limits in exec hooks - function preRuntimeValidationHook(uint8 functionId, address, uint256, bytes calldata) external pure override { - // silence warnings - (functionId); - } + function preRuntimeValidationHook(uint8, address, uint256, bytes calldata) external pure override {} /// @inheritdoc IPlugin function pluginManifest() external pure override returns (PluginManifest memory) { @@ -135,7 +132,7 @@ contract NativeTokenLimitPlugin is BasePlugin, IExecutionHook, IValidationHook { /// @inheritdoc BasePlugin function supportsInterface(bytes4 interfaceId) public view override(BasePlugin, IERC165) returns (bool) { - return super.supportsInterface(interfaceId); + return interfaceId == type(IExecutionHook).interfaceId || super.supportsInterface(interfaceId); } function _checkAndDecrementLimit(uint8 functionId, bytes calldata data) internal returns (bytes memory) { From 8040550b603a8dc188d519bd8b7236a6b107b616 Mon Sep 17 00:00:00 2001 From: howydev <132113803+howydev@users.noreply.github.com> Date: Wed, 10 Jul 2024 14:00:33 -0400 Subject: [PATCH 08/11] fix: lint --- src/plugins/NativeTokenLimitPlugin.sol | 28 +++++++++++++++----------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/src/plugins/NativeTokenLimitPlugin.sol b/src/plugins/NativeTokenLimitPlugin.sol index cb84a4ee..58d10a9b 100644 --- a/src/plugins/NativeTokenLimitPlugin.sol +++ b/src/plugins/NativeTokenLimitPlugin.sol @@ -22,9 +22,9 @@ contract NativeTokenLimitPlugin is BasePlugin, IExecutionHook, IValidationHook { using UserOperationLib for PackedUserOperation; using EnumerableSet for EnumerableSet.Bytes32Set; - string internal constant NAME = "Native Token Limit"; - string internal constant VERSION = "1.0.0"; - string internal constant AUTHOR = "ERC-6900 Authors"; + string internal constant _NAME = "Native Token Limit"; + string internal constant _VERSION = "1.0.0"; + string internal constant _AUTHOR = "ERC-6900 Authors"; mapping(uint256 funcIds => mapping(address account => uint256 limit)) public limits; // Accounts should add paymasters that still use the accounts tokens here @@ -54,7 +54,13 @@ contract NativeTokenLimitPlugin is BasePlugin, IExecutionHook, IValidationHook { ) { uint256 vgl = UserOperationLib.unpackVerificationGasLimit(userOp); uint256 cgl = UserOperationLib.unpackCallGasLimit(userOp); - uint256 totalGas = userOp.preVerificationGas + vgl + cgl; + uint256 pvgl; + uint256 ppogl; + if (userOp.paymasterAndData.length > 0) { + // Can skip the EP length check here since it would have reverted there if it was invalid + (, pvgl, ppogl) = UserOperationLib.unpackPaymasterStaticFields(userOp.paymasterAndData); + } + uint256 totalGas = userOp.preVerificationGas + vgl + cgl + pvgl + ppogl; uint256 usage = totalGas * UserOperationLib.unpackMaxFeePerGas(userOp); uint256 limit = limits[functionId][msg.sender]; @@ -104,21 +110,19 @@ contract NativeTokenLimitPlugin is BasePlugin, IExecutionHook, IValidationHook { // No implementation, no revert // Runtime spends no account gas, and we check native token spend limits in exec hooks + // solhint-disable-next-line no-empty-blocks function preRuntimeValidationHook(uint8, address, uint256, bytes calldata) external pure override {} /// @inheritdoc IPlugin - function pluginManifest() external pure override returns (PluginManifest memory) { - // silence warnings - PluginManifest memory manifest; - return manifest; - } + // solhint-disable-next-line no-empty-blocks + function pluginManifest() external pure override returns (PluginManifest memory) {} /// @inheritdoc IPlugin function pluginMetadata() external pure virtual override returns (PluginMetadata memory) { PluginMetadata memory metadata; - metadata.name = NAME; - metadata.version = VERSION; - metadata.author = AUTHOR; + metadata.name = _NAME; + metadata.version = _VERSION; + metadata.author = _AUTHOR; metadata.permissionRequest = new string[](2); metadata.permissionRequest[0] = "native-token-limit"; From cb61b7f724c56ac3edc041be1017871535c5b4ae Mon Sep 17 00:00:00 2001 From: howydev <132113803+howydev@users.noreply.github.com> Date: Wed, 10 Jul 2024 19:55:09 -0400 Subject: [PATCH 09/11] fix: lint, test --- src/plugins/NativeTokenLimitPlugin.sol | 7 +++++-- test/plugin/NativeTokenLimitPlugin.t.sol | 14 +++++--------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/plugins/NativeTokenLimitPlugin.sol b/src/plugins/NativeTokenLimitPlugin.sol index 58d10a9b..2b512c22 100644 --- a/src/plugins/NativeTokenLimitPlugin.sol +++ b/src/plugins/NativeTokenLimitPlugin.sol @@ -110,8 +110,11 @@ contract NativeTokenLimitPlugin is BasePlugin, IExecutionHook, IValidationHook { // No implementation, no revert // Runtime spends no account gas, and we check native token spend limits in exec hooks - // solhint-disable-next-line no-empty-blocks - function preRuntimeValidationHook(uint8, address, uint256, bytes calldata) external pure override {} + function preRuntimeValidationHook(uint8, address, uint256, bytes calldata, bytes calldata) + external + pure + override + {} // solhint-disable-line no-empty-blocks /// @inheritdoc IPlugin // solhint-disable-next-line no-empty-blocks diff --git a/test/plugin/NativeTokenLimitPlugin.t.sol b/test/plugin/NativeTokenLimitPlugin.t.sol index b8072238..bfc16b33 100644 --- a/test/plugin/NativeTokenLimitPlugin.t.sol +++ b/test/plugin/NativeTokenLimitPlugin.t.sol @@ -1,7 +1,6 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.19; -import {EntryPoint} from "@eth-infinitism/account-abstraction/core/EntryPoint.sol"; import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interfaces/PackedUserOperation.sol"; import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; @@ -14,10 +13,9 @@ import {IStandardExecutor, Call} from "../../src/interfaces/IStandardExecutor.so import {PluginManifest} from "../../src/interfaces/IPlugin.sol"; import {MSCAFactoryFixture} from "../mocks/MSCAFactoryFixture.sol"; -import {OptimizedTest} from "../utils/OptimizedTest.sol"; +import {AccountTestBase} from "../utils/AccountTestBase.sol"; -contract NativeTokenLimitPluginTest is OptimizedTest { - EntryPoint public entryPoint = new EntryPoint(); +contract NativeTokenLimitPluginTest is AccountTestBase { address public recipient = address(1); address payable public bundler = payable(address(2)); PluginManifest internal _m; @@ -87,7 +85,7 @@ contract NativeTokenLimitPluginTest is OptimizedTest { preVerificationGas: gas3, gasFees: bytes32(uint256(uint128(gasPrice))), paymasterAndData: "", - signature: abi.encodePacked(FunctionReferenceLib.pack(address(validationPlugin), 0), uint8(1)) + signature: _encodeSignature(FunctionReferenceLib.pack(address(validationPlugin), 0), 1, "") }); } @@ -184,9 +182,7 @@ contract NativeTokenLimitPluginTest is OptimizedTest { function test_runtime_executeLimit() public { assertEq(plugin.limits(0, address(acct)), 10 ether); - acct.executeWithAuthorization( - _getExecuteWithValue(5 ether), abi.encodePacked(validationFunction, uint8(1)) - ); + acct.executeWithAuthorization(_getExecuteWithValue(5 ether), _encodeSignature(validationFunction, 1, "")); assertEq(plugin.limits(0, address(acct)), 5 ether); } @@ -198,7 +194,7 @@ contract NativeTokenLimitPluginTest is OptimizedTest { assertEq(plugin.limits(0, address(acct)), 10 ether); acct.executeWithAuthorization( - abi.encodeCall(IStandardExecutor.executeBatch, (calls)), abi.encodePacked(validationFunction, uint8(1)) + abi.encodeCall(IStandardExecutor.executeBatch, (calls)), _encodeSignature(validationFunction, 1, "") ); assertEq(plugin.limits(0, address(acct)), 4 ether - 100001); } From e3b9d2a9f463346bdaf87df610f915197bccc266 Mon Sep 17 00:00:00 2001 From: howydev <132113803+howydev@users.noreply.github.com> Date: Mon, 15 Jul 2024 22:52:26 -0400 Subject: [PATCH 10/11] fix: test --- test/plugin/NativeTokenLimitPlugin.t.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/plugin/NativeTokenLimitPlugin.t.sol b/test/plugin/NativeTokenLimitPlugin.t.sol index bfc16b33..5fbf2a3d 100644 --- a/test/plugin/NativeTokenLimitPlugin.t.sol +++ b/test/plugin/NativeTokenLimitPlugin.t.sol @@ -11,6 +11,7 @@ import {ExecutionHook} from "../../src/interfaces/IAccountLoupe.sol"; import {FunctionReferenceLib} from "../../src/helpers/FunctionReferenceLib.sol"; import {IStandardExecutor, Call} from "../../src/interfaces/IStandardExecutor.sol"; import {PluginManifest} from "../../src/interfaces/IPlugin.sol"; +import {ValidationConfigLib} from "../../src/helpers/ValidationConfigLib.sol"; import {MSCAFactoryFixture} from "../mocks/MSCAFactoryFixture.sol"; import {AccountTestBase} from "../utils/AccountTestBase.sol"; @@ -56,8 +57,7 @@ contract NativeTokenLimitPluginTest is AccountTestBase { vm.prank(address(acct)); acct.installValidation( - FunctionReferenceLib.pack(address(validationPlugin), 0), - true, + ValidationConfigLib.pack(address(validationPlugin), 0, true, true), new bytes4[](0), new bytes(0), abi.encode(preValidationHooks, preValHooksInitDatas), From 3be43b79f0a0607c262aa5a5be613dc5ba991cba Mon Sep 17 00:00:00 2001 From: howy <132113803+howydev@users.noreply.github.com> Date: Tue, 16 Jul 2024 16:59:00 -0700 Subject: [PATCH 11/11] [5/n permissions] feat: add erc20 token spend limit plugin (#80) Co-authored-by: fangting-alchemy <119372438+fangting-alchemy@users.noreply.github.com> --- .gitmodules | 3 + lib/modular-account-libs | 1 + remappings.txt | 1 + src/plugins/ERC20TokenLimitPlugin.sol | 154 ++++++++++++++++++++ test/mocks/MockERC20.sol | 12 ++ test/plugin/ERC20TokenLimitPlugin.t.sol | 184 ++++++++++++++++++++++++ 6 files changed, 355 insertions(+) create mode 160000 lib/modular-account-libs create mode 100644 src/plugins/ERC20TokenLimitPlugin.sol create mode 100644 test/mocks/MockERC20.sol create mode 100644 test/plugin/ERC20TokenLimitPlugin.t.sol diff --git a/.gitmodules b/.gitmodules index 813d955e..05bd137f 100644 --- a/.gitmodules +++ b/.gitmodules @@ -7,3 +7,6 @@ [submodule "lib/forge-std"] path = lib/forge-std url = https://github.com/foundry-rs/forge-std +[submodule "lib/modular-account-libs"] + path = lib/modular-account-libs + url = https://github.com/erc6900/modular-account-libs diff --git a/lib/modular-account-libs b/lib/modular-account-libs new file mode 160000 index 00000000..5d9d0e40 --- /dev/null +++ b/lib/modular-account-libs @@ -0,0 +1 @@ +Subproject commit 5d9d0e403332251045eee2954c2a8b7ea0bae953 diff --git a/remappings.txt b/remappings.txt index 3d0ee0df..bc2ce0be 100644 --- a/remappings.txt +++ b/remappings.txt @@ -2,3 +2,4 @@ ds-test/=lib/forge-std/lib/ds-test/src/ forge-std/=lib/forge-std/src/ @eth-infinitism/account-abstraction/=lib/account-abstraction/contracts/ @openzeppelin/=lib/openzeppelin-contracts/ +@modular-account-libs/=lib/modular-account-libs/src/ \ No newline at end of file diff --git a/src/plugins/ERC20TokenLimitPlugin.sol b/src/plugins/ERC20TokenLimitPlugin.sol new file mode 100644 index 00000000..1df5bcfd --- /dev/null +++ b/src/plugins/ERC20TokenLimitPlugin.sol @@ -0,0 +1,154 @@ +// SPDX-License-Identifier: GPL-3.0 +pragma solidity ^0.8.25; + +import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interfaces/PackedUserOperation.sol"; +import {UserOperationLib} from "@eth-infinitism/account-abstraction/core/UserOperationLib.sol"; +import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; +import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import { + SetValue, + AssociatedLinkedListSet, + AssociatedLinkedListSetLib +} from "@modular-account-libs/libraries/AssociatedLinkedListSetLib.sol"; + +import {PluginManifest, PluginMetadata} from "../interfaces/IPlugin.sol"; +import {IStandardExecutor, Call} from "../interfaces/IStandardExecutor.sol"; +import {IPlugin} from "../interfaces/IPlugin.sol"; +import {IExecutionHook} from "../interfaces/IExecutionHook.sol"; +import {BasePlugin, IERC165} from "./BasePlugin.sol"; + +/// @title ERC20 Token Limit Plugin +/// @author ERC-6900 Authors +/// @notice This plugin supports an ERC20 token spend limit. This should be combined with a contract whitelist +/// plugin to make sure that token transfers not tracked by the plugin don't happen. +/// Note: this plugin is opinionated on what selectors can be called for token contracts to guard against weird +/// edge cases like DAI. You wouldn't be able to use uni v2 pairs directly as the pair contract is also the LP +/// token contract +contract ERC20TokenLimitPlugin is BasePlugin, IExecutionHook { + using UserOperationLib for PackedUserOperation; + using EnumerableSet for EnumerableSet.AddressSet; + using AssociatedLinkedListSetLib for AssociatedLinkedListSet; + + struct ERC20SpendLimit { + address token; + uint256[] limits; + } + + string internal constant _NAME = "ERC20 Token Limit Plugin"; + string internal constant _VERSION = "1.0.0"; + string internal constant _AUTHOR = "ERC-6900 Authors"; + + mapping(uint8 functionId => mapping(address token => mapping(address account => uint256 limit))) public limits; + AssociatedLinkedListSet internal _tokenList; + + error ExceededTokenLimit(); + error ExceededNumberOfEntities(); + error SelectorNotAllowed(); + + function updateLimits(uint8 functionId, address token, uint256 newLimit) external { + _tokenList.tryAdd(msg.sender, SetValue.wrap(bytes30(bytes20(token)))); + limits[functionId][token][msg.sender] = newLimit; + } + + /// @inheritdoc IExecutionHook + function preExecutionHook(uint8 functionId, address, uint256, bytes calldata data) + external + override + returns (bytes memory) + { + (bytes4 selector, bytes memory callData) = _getSelectorAndCalldata(data); + + if (selector == IStandardExecutor.execute.selector) { + (address token,, bytes memory innerCalldata) = abi.decode(callData, (address, uint256, bytes)); + if (_tokenList.contains(msg.sender, SetValue.wrap(bytes30(bytes20(token))))) { + _decrementLimit(functionId, token, innerCalldata); + } + } else if (selector == IStandardExecutor.executeBatch.selector) { + Call[] memory calls = abi.decode(callData, (Call[])); + for (uint256 i = 0; i < calls.length; i++) { + if (_tokenList.contains(msg.sender, SetValue.wrap(bytes30(bytes20(calls[i].target))))) { + _decrementLimit(functionId, calls[i].target, calls[i].data); + } + } + } + + return ""; + } + + /// @inheritdoc IPlugin + function onInstall(bytes calldata data) external override { + (uint8 startFunctionId, ERC20SpendLimit[] memory spendLimits) = + abi.decode(data, (uint8, ERC20SpendLimit[])); + + if (startFunctionId + spendLimits.length > type(uint8).max) { + revert ExceededNumberOfEntities(); + } + + for (uint8 i = 0; i < spendLimits.length; i++) { + _tokenList.tryAdd(msg.sender, SetValue.wrap(bytes30(bytes20(spendLimits[i].token)))); + for (uint256 j = 0; j < spendLimits[i].limits.length; j++) { + limits[i + startFunctionId][spendLimits[i].token][msg.sender] = spendLimits[i].limits[j]; + } + } + } + + /// @inheritdoc IPlugin + function onUninstall(bytes calldata data) external override { + (address token, uint8 functionId) = abi.decode(data, (address, uint8)); + delete limits[functionId][token][msg.sender]; + } + + function getTokensForAccount(address account) external view returns (address[] memory tokens) { + SetValue[] memory set = _tokenList.getAll(account); + tokens = new address[](set.length); + for (uint256 i = 0; i < tokens.length; i++) { + tokens[i] = address(bytes20(bytes32(SetValue.unwrap(set[i])))); + } + return tokens; + } + + /// @inheritdoc IExecutionHook + function postExecutionHook(uint8, bytes calldata) external pure override { + revert NotImplemented(); + } + + /// @inheritdoc IPlugin + // solhint-disable-next-line no-empty-blocks + function pluginManifest() external pure override returns (PluginManifest memory) {} + + /// @inheritdoc IPlugin + function pluginMetadata() external pure virtual override returns (PluginMetadata memory) { + PluginMetadata memory metadata; + metadata.name = _NAME; + metadata.version = _VERSION; + metadata.author = _AUTHOR; + + metadata.permissionRequest = new string[](1); + metadata.permissionRequest[0] = "erc20-token-limit"; + return metadata; + } + + /// @inheritdoc BasePlugin + function supportsInterface(bytes4 interfaceId) public view override(BasePlugin, IERC165) returns (bool) { + return super.supportsInterface(interfaceId); + } + + function _decrementLimit(uint8 functionId, address token, bytes memory innerCalldata) internal { + bytes4 selector; + uint256 spend; + assembly { + selector := mload(add(innerCalldata, 32)) // 0:32 is arr len, 32:36 is selector + spend := mload(add(innerCalldata, 68)) // 36:68 is recipient, 68:100 is spend + } + if (selector == IERC20.transfer.selector || selector == IERC20.approve.selector) { + uint256 limit = limits[functionId][token][msg.sender]; + if (spend > limit) { + revert ExceededTokenLimit(); + } + // solhint-disable-next-line reentrancy + limits[functionId][token][msg.sender] = limit - spend; + } else { + revert SelectorNotAllowed(); + } + } +} diff --git a/test/mocks/MockERC20.sol b/test/mocks/MockERC20.sol new file mode 100644 index 00000000..131e0d1a --- /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() ERC20("MockERC20", "MERC") {} + + function mint(address to, uint256 amount) external { + _mint(to, amount); + } +} diff --git a/test/plugin/ERC20TokenLimitPlugin.t.sol b/test/plugin/ERC20TokenLimitPlugin.t.sol new file mode 100644 index 00000000..96a18c20 --- /dev/null +++ b/test/plugin/ERC20TokenLimitPlugin.t.sol @@ -0,0 +1,184 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.19; + +import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interfaces/PackedUserOperation.sol"; +import {MockERC20} from "../mocks/MockERC20.sol"; +import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; + +import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol"; +import {FunctionReference} from "../../src/helpers/FunctionReferenceLib.sol"; +import {ERC20TokenLimitPlugin} from "../../src/plugins/ERC20TokenLimitPlugin.sol"; +import {MockPlugin} from "../mocks/MockPlugin.sol"; +import {ExecutionHook} from "../../src/interfaces/IAccountLoupe.sol"; +import {FunctionReferenceLib} from "../../src/helpers/FunctionReferenceLib.sol"; +import {IStandardExecutor, Call} from "../../src/interfaces/IStandardExecutor.sol"; +import {PluginManifest} from "../../src/interfaces/IPlugin.sol"; +import {ValidationConfigLib} from "../../src/helpers/ValidationConfigLib.sol"; + +import {MSCAFactoryFixture} from "../mocks/MSCAFactoryFixture.sol"; +import {AccountTestBase} from "../utils/AccountTestBase.sol"; + +contract ERC20TokenLimitPluginTest is AccountTestBase { + address public recipient = address(1); + MockERC20 public erc20; + address payable public bundler = payable(address(2)); + PluginManifest internal _m; + MockPlugin public validationPlugin = new MockPlugin(_m); + FunctionReference public validationFunction; + + UpgradeableModularAccount public acct; + ERC20TokenLimitPlugin public plugin = new ERC20TokenLimitPlugin(); + uint256 public spendLimit = 10 ether; + + function setUp() public { + // Set up a validator with hooks from the erc20 spend limit plugin attached + MSCAFactoryFixture factory = new MSCAFactoryFixture(entryPoint, _deploySingleOwnerPlugin()); + + acct = factory.createAccount(address(this), 0); + + erc20 = new MockERC20(); + erc20.mint(address(acct), 10 ether); + + ExecutionHook[] memory permissionHooks = new ExecutionHook[](1); + permissionHooks[0] = ExecutionHook({ + hookFunction: FunctionReferenceLib.pack(address(plugin), 0), + isPreHook: true, + isPostHook: false + }); + + // arr idx 0 => functionId of 0 has that spend + uint256[] memory limits = new uint256[](1); + limits[0] = spendLimit; + + ERC20TokenLimitPlugin.ERC20SpendLimit[] memory limit = new ERC20TokenLimitPlugin.ERC20SpendLimit[](1); + limit[0] = ERC20TokenLimitPlugin.ERC20SpendLimit({token: address(erc20), limits: limits}); + + bytes[] memory permissionInitDatas = new bytes[](1); + permissionInitDatas[0] = abi.encode(uint8(0), limit); + + vm.prank(address(acct)); + acct.installValidation( + ValidationConfigLib.pack(address(validationPlugin), 0, true, true), + new bytes4[](0), + new bytes(0), + new bytes(0), + abi.encode(permissionHooks, permissionInitDatas) + ); + + validationFunction = FunctionReferenceLib.pack(address(validationPlugin), 0); + } + + function _getPackedUO(bytes memory callData) internal view returns (PackedUserOperation memory uo) { + uo = PackedUserOperation({ + sender: address(acct), + nonce: 0, + initCode: "", + callData: abi.encodePacked(UpgradeableModularAccount.executeUserOp.selector, callData), + accountGasLimits: bytes32(bytes16(uint128(200000))) | bytes32(uint256(200000)), + preVerificationGas: 200000, + gasFees: bytes32(uint256(uint128(0))), + paymasterAndData: "", + signature: _encodeSignature(FunctionReferenceLib.pack(address(validationPlugin), 0), 1, "") + }); + } + + function _getExecuteWithSpend(uint256 value) internal view returns (bytes memory) { + return abi.encodeCall( + UpgradeableModularAccount.execute, + (address(erc20), 0, abi.encodeCall(IERC20.transfer, (recipient, value))) + ); + } + + function test_userOp_executeLimit() public { + vm.startPrank(address(entryPoint)); + assertEq(plugin.limits(0, address(erc20), address(acct)), 10 ether); + acct.executeUserOp(_getPackedUO(_getExecuteWithSpend(5 ether)), bytes32(0)); + assertEq(plugin.limits(0, address(erc20), address(acct)), 5 ether); + } + + function test_userOp_executeBatchLimit() public { + Call[] memory calls = new Call[](3); + calls[0] = + Call({target: address(erc20), value: 0, data: abi.encodeCall(IERC20.transfer, (recipient, 1 wei))}); + calls[1] = + Call({target: address(erc20), value: 0, data: abi.encodeCall(IERC20.transfer, (recipient, 1 ether))}); + calls[2] = Call({ + target: address(erc20), + value: 0, + data: abi.encodeCall(IERC20.transfer, (recipient, 5 ether + 100000)) + }); + + vm.startPrank(address(entryPoint)); + assertEq(plugin.limits(0, address(erc20), address(acct)), 10 ether); + acct.executeUserOp(_getPackedUO(abi.encodeCall(IStandardExecutor.executeBatch, (calls))), bytes32(0)); + assertEq(plugin.limits(0, address(erc20), address(acct)), 10 ether - 6 ether - 100001); + } + + function test_userOp_executeBatch_approveAndTransferLimit() public { + Call[] memory calls = new Call[](3); + calls[0] = + Call({target: address(erc20), value: 0, data: abi.encodeCall(IERC20.approve, (recipient, 1 wei))}); + calls[1] = + Call({target: address(erc20), value: 0, data: abi.encodeCall(IERC20.transfer, (recipient, 1 ether))}); + calls[2] = Call({ + target: address(erc20), + value: 0, + data: abi.encodeCall(IERC20.approve, (recipient, 5 ether + 100000)) + }); + + vm.startPrank(address(entryPoint)); + assertEq(plugin.limits(0, address(erc20), address(acct)), 10 ether); + acct.executeUserOp(_getPackedUO(abi.encodeCall(IStandardExecutor.executeBatch, (calls))), bytes32(0)); + assertEq(plugin.limits(0, address(erc20), address(acct)), 10 ether - 6 ether - 100001); + } + + function test_userOp_executeBatch_approveAndTransferLimit_fail() public { + Call[] memory calls = new Call[](3); + calls[0] = + Call({target: address(erc20), value: 0, data: abi.encodeCall(IERC20.approve, (recipient, 1 wei))}); + calls[1] = + Call({target: address(erc20), value: 0, data: abi.encodeCall(IERC20.transfer, (recipient, 1 ether))}); + calls[2] = Call({ + target: address(erc20), + value: 0, + data: abi.encodeCall(IERC20.approve, (recipient, 9 ether + 100000)) + }); + + vm.startPrank(address(entryPoint)); + assertEq(plugin.limits(0, address(erc20), address(acct)), 10 ether); + PackedUserOperation[] memory uos = new PackedUserOperation[](1); + uos[0] = _getPackedUO(abi.encodeCall(IStandardExecutor.executeBatch, (calls))); + entryPoint.handleOps(uos, bundler); + // no spend consumed + assertEq(plugin.limits(0, address(erc20), address(acct)), 10 ether); + } + + function test_runtime_executeLimit() public { + assertEq(plugin.limits(0, address(erc20), address(acct)), 10 ether); + acct.executeWithAuthorization( + _getExecuteWithSpend(5 ether), + _encodeSignature(FunctionReferenceLib.pack(address(validationPlugin), 0), 1, "") + ); + assertEq(plugin.limits(0, address(erc20), address(acct)), 5 ether); + } + + function test_runtime_executeBatchLimit() public { + Call[] memory calls = new Call[](3); + calls[0] = + Call({target: address(erc20), value: 0, data: abi.encodeCall(IERC20.approve, (recipient, 1 wei))}); + calls[1] = + Call({target: address(erc20), value: 0, data: abi.encodeCall(IERC20.transfer, (recipient, 1 ether))}); + calls[2] = Call({ + target: address(erc20), + value: 0, + data: abi.encodeCall(IERC20.approve, (recipient, 5 ether + 100000)) + }); + + assertEq(plugin.limits(0, address(erc20), address(acct)), 10 ether); + acct.executeWithAuthorization( + abi.encodeCall(IStandardExecutor.executeBatch, (calls)), + _encodeSignature(FunctionReferenceLib.pack(address(validationPlugin), 0), 1, "") + ); + assertEq(plugin.limits(0, address(erc20), address(acct)), 10 ether - 6 ether - 100001); + } +}