From 6d07291cc08586dbb7b437c3288d1d0d2d645606 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 erc20 token spend limit plugin --- src/plugins/ERC20TokenLimitPlugin.sol | 170 ++++++++++++++++++++++ test/mocks/MockERC20.sol | 12 ++ test/plugin/ERC20TokenLimitPlugin.t.sol | 185 ++++++++++++++++++++++++ 3 files changed, 367 insertions(+) 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/src/plugins/ERC20TokenLimitPlugin.sol b/src/plugins/ERC20TokenLimitPlugin.sol new file mode 100644 index 00000000..d8e7ded0 --- /dev/null +++ b/src/plugins/ERC20TokenLimitPlugin.sol @@ -0,0 +1,170 @@ +// 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 {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.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; + + string public constant NAME = "ERC20 Token Limit Plugin"; + string public constant VERSION = "1.0.0"; + string public constant AUTHOR = "ERC-6900 Authors"; + + struct ERC20SpendLimit { + address token; + uint256[] limits; + } + + mapping(address account => mapping(address => uint256[] limits)) public limits; + mapping(address account => EnumerableSet.AddressSet) internal _tokenList; + + error ExceededNativeTokenLimit(); + error ExceededNumberOfEntities(); + error SelectorNotAllowed(); + + function getTokensForAccount(address account) external view returns (address[] memory tokens) { + tokens = new address[](_tokenList[account].length()); + for (uint256 i = 0; i < _tokenList[account].length(); i++) { + tokens[i] = _tokenList[account].at(i); + } + return tokens; + } + + function updateLimits(uint8 functionId, address token, uint256 newLimit) external { + _tokenList[msg.sender].add(token); + limits[msg.sender][token][functionId] = newLimit; + } + + function _checkAndDecrementLimit(uint8 functionId, bytes4 selector, uint256 spend, address token) internal { + if (selector == IERC20.transfer.selector || selector == IERC20.approve.selector) { + uint256 limit = limits[msg.sender][token][functionId]; + if (spend > limit) { + revert ExceededNativeTokenLimit(); + } + limits[msg.sender][token][functionId] = limit - spend; + } else { + revert SelectorNotAllowed(); + } + } + + /// @inheritdoc IExecutionHook + function preExecutionHook(uint8 functionId, bytes calldata data) external override returns (bytes memory) { + bytes calldata topLevelCallData; + bytes4 topLevelSelector; + + // TODO: plugins should never have to do these gymnastics + topLevelSelector = bytes4(data[52:56]); + if (topLevelSelector == IAccountExecute.executeUserOp.selector) { + topLevelCallData = data[56:]; + topLevelSelector = bytes4(topLevelCallData); + } else { + topLevelCallData = data[52:]; + } + + if (topLevelSelector == IStandardExecutor.execute.selector) { + address token = address(uint160(uint256(bytes32(topLevelCallData[4:36])))); + if (_tokenList[msg.sender].contains(token)) { + bytes calldata executeCalldata; + uint256 offset = uint256(bytes32(topLevelCallData[68:100])); + + assembly { + let relativeOffset := add(add(topLevelCallData.offset, offset), 4) + executeCalldata.offset := add(relativeOffset, 32) + executeCalldata.length := calldataload(relativeOffset) + } + + _checkAndDecrementLimit( + functionId, bytes4(executeCalldata[:4]), uint256(bytes32(executeCalldata[36:68])), token + ); + } + } else if (topLevelSelector == IStandardExecutor.executeBatch.selector) { + Call[] memory calls = abi.decode(topLevelCallData[4:], (Call[])); + for (uint256 i = 0; i < calls.length; i++) { + if (_tokenList[msg.sender].contains(calls[i].target)) { + bytes memory tokenContractCallData = calls[i].data; + bytes4 selector; + uint256 spend; + assembly { + selector := mload(add(tokenContractCallData, 32)) // 0:32 is arr len, 32:36 is selector + spend := mload(add(tokenContractCallData, 68)) // 36:68 is recipient, 68:100 is spend + } + _checkAndDecrementLimit(functionId, selector, spend, calls[i].target); + } + } + } + + return ""; + } + + /// @inheritdoc IExecutionHook + function postExecutionHook(uint8, bytes calldata) external pure override { + revert NotImplemented(); + } + + /// @inheritdoc IPlugin + function onInstall(bytes calldata data) external override { + ERC20SpendLimit[] memory spendLimits = abi.decode(data, (ERC20SpendLimit[])); + + for (uint256 i = 0; i < spendLimits.length; i++) { + _tokenList[msg.sender].add(spendLimits[i].token); + for (uint256 j = 0; j < spendLimits[i].limits.length; j++) { + limits[msg.sender][spendLimits[i].token].push(spendLimits[i].limits[j]); + } + if (limits[msg.sender][spendLimits[i].token].length > type(uint8).max) { + revert ExceededNumberOfEntities(); + } + } + } + + /// @inheritdoc IPlugin + function onUninstall(bytes calldata data) external override { + (address token, uint8 functionId) = abi.decode(data, (address, uint8)); + delete limits[msg.sender][token][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[](1); + metadata.permissionRequest[0] = "erc20-token-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/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..c8dd926f --- /dev/null +++ b/test/plugin/ERC20TokenLimitPlugin.t.sol @@ -0,0 +1,185 @@ +// 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 {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 {MSCAFactoryFixture} from "../mocks/MSCAFactoryFixture.sol"; +import {OptimizedTest} from "../utils/OptimizedTest.sol"; + +contract ERC20TokenLimitPluginTest is OptimizedTest { + EntryPoint public entryPoint = new EntryPoint(); + 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, + requireUOContext: 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(limit); + + vm.prank(address(acct)); + acct.installValidation( + FunctionReferenceLib.pack(address(validationPlugin), 0), + 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: abi.encodePacked(FunctionReferenceLib.pack(address(validationPlugin), 0), uint8(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(address(acct), address(erc20), 0), 10 ether); + acct.executeUserOp(_getPackedUO(_getExecuteWithSpend(5 ether)), bytes32(0)); + assertEq(plugin.limits(address(acct), address(erc20), 0), 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(address(acct), address(erc20), 0), 10 ether); + acct.executeUserOp(_getPackedUO(abi.encodeCall(IStandardExecutor.executeBatch, (calls))), bytes32(0)); + assertEq(plugin.limits(address(acct), address(erc20), 0), 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(address(acct), address(erc20), 0), 10 ether); + acct.executeUserOp(_getPackedUO(abi.encodeCall(IStandardExecutor.executeBatch, (calls))), bytes32(0)); + assertEq(plugin.limits(address(acct), address(erc20), 0), 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(address(acct), address(erc20), 0), 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(address(acct), address(erc20), 0), 10 ether); + } + + function test_runtime_executeLimit() public { + assertEq(plugin.limits(address(acct), address(erc20), 0), 10 ether); + acct.executeWithAuthorization( + _getExecuteWithSpend(5 ether), abi.encodePacked(validationFunction, uint8(1)) + ); + assertEq(plugin.limits(address(acct), address(erc20), 0), 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(address(acct), address(erc20), 0), 10 ether); + acct.executeWithAuthorization( + abi.encodeCall(IStandardExecutor.executeBatch, (calls)), abi.encodePacked(validationFunction, uint8(1)) + ); + assertEq(plugin.limits(address(acct), address(erc20), 0), 10 ether - 6 ether - 100001); + } +} From 3af15230b7e78afad8bf42c78e0c2418ac9619de Mon Sep 17 00:00:00 2001 From: howydev <132113803+howydev@users.noreply.github.com> Date: Thu, 27 Jun 2024 16:05:52 -0400 Subject: [PATCH 02/11] chore: update plugin --- src/plugins/ERC20TokenLimitPlugin.sol | 76 ++++++++++++------------- test/plugin/ERC20TokenLimitPlugin.t.sol | 3 +- 2 files changed, 38 insertions(+), 41 deletions(-) diff --git a/src/plugins/ERC20TokenLimitPlugin.sol b/src/plugins/ERC20TokenLimitPlugin.sol index d8e7ded0..c3ddfed8 100644 --- a/src/plugins/ERC20TokenLimitPlugin.sol +++ b/src/plugins/ERC20TokenLimitPlugin.sol @@ -4,13 +4,13 @@ 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 {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.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 {BasePlugin, IERC165} from "./BasePlugin.sol"; /// @title ERC20 Token Limit Plugin @@ -20,7 +20,7 @@ import {BasePlugin, IERC165} from "./BasePlugin.sol"; /// 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 { +contract ERC20TokenLimitPlugin is BasePlugin, IExecutionHook, IPermissionHook { using UserOperationLib for PackedUserOperation; using EnumerableSet for EnumerableSet.AddressSet; @@ -53,7 +53,13 @@ contract ERC20TokenLimitPlugin is BasePlugin, IExecutionHook { limits[msg.sender][token][functionId] = newLimit; } - function _checkAndDecrementLimit(uint8 functionId, bytes4 selector, uint256 spend, address token) internal { + 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[msg.sender][token][functionId]; if (spend > limit) { @@ -65,48 +71,40 @@ contract ERC20TokenLimitPlugin is BasePlugin, IExecutionHook { } } - /// @inheritdoc IExecutionHook - function preExecutionHook(uint8 functionId, bytes calldata data) external override returns (bytes memory) { - bytes calldata topLevelCallData; - bytes4 topLevelSelector; - - // TODO: plugins should never have to do these gymnastics - topLevelSelector = bytes4(data[52:56]); - if (topLevelSelector == IAccountExecute.executeUserOp.selector) { - topLevelCallData = data[56:]; - topLevelSelector = bytes4(topLevelCallData); - } else { - topLevelCallData = data[52:]; - } + /// @inheritdoc IPermissionHook + function preUserOpExecutionHook(uint8 functionId, PackedUserOperation calldata uo) + external + override + returns (bytes memory) + { + return _checkSelectorAndDecrementLimit(functionId, uo.callData); + } - if (topLevelSelector == IStandardExecutor.execute.selector) { - address token = address(uint160(uint256(bytes32(topLevelCallData[4:36])))); - if (_tokenList[msg.sender].contains(token)) { - bytes calldata executeCalldata; - uint256 offset = uint256(bytes32(topLevelCallData[68:100])); + /// @inheritdoc IExecutionHook + function preExecutionHook(uint8 functionId, address, uint256, bytes calldata data) + external + override + returns (bytes memory) + { + return _checkSelectorAndDecrementLimit(functionId, data); + } - assembly { - let relativeOffset := add(add(topLevelCallData.offset, offset), 4) - executeCalldata.offset := add(relativeOffset, 32) - executeCalldata.length := calldataload(relativeOffset) - } + function _checkSelectorAndDecrementLimit(uint8 functionId, bytes calldata data) + internal + returns (bytes memory) + { + bytes4 selector = bytes4(data[:4]); - _checkAndDecrementLimit( - functionId, bytes4(executeCalldata[:4]), uint256(bytes32(executeCalldata[36:68])), token - ); + if (selector == IStandardExecutor.execute.selector) { + (address token,, bytes memory innerCalldata) = abi.decode(data[4:], (address, uint256, bytes)); + if (_tokenList[msg.sender].contains(token)) { + _decrementLimit(functionId, token, innerCalldata); } - } else if (topLevelSelector == IStandardExecutor.executeBatch.selector) { - Call[] memory calls = abi.decode(topLevelCallData[4:], (Call[])); + } else if (selector == IStandardExecutor.executeBatch.selector) { + Call[] memory calls = abi.decode(data[4:], (Call[])); for (uint256 i = 0; i < calls.length; i++) { if (_tokenList[msg.sender].contains(calls[i].target)) { - bytes memory tokenContractCallData = calls[i].data; - bytes4 selector; - uint256 spend; - assembly { - selector := mload(add(tokenContractCallData, 32)) // 0:32 is arr len, 32:36 is selector - spend := mload(add(tokenContractCallData, 68)) // 36:68 is recipient, 68:100 is spend - } - _checkAndDecrementLimit(functionId, selector, spend, calls[i].target); + _decrementLimit(functionId, calls[i].target, calls[i].data); } } } diff --git a/test/plugin/ERC20TokenLimitPlugin.t.sol b/test/plugin/ERC20TokenLimitPlugin.t.sol index c8dd926f..c96d467c 100644 --- a/test/plugin/ERC20TokenLimitPlugin.t.sol +++ b/test/plugin/ERC20TokenLimitPlugin.t.sol @@ -44,8 +44,7 @@ contract ERC20TokenLimitPluginTest is OptimizedTest { permissionHooks[0] = ExecutionHook({ hookFunction: FunctionReferenceLib.pack(address(plugin), 0), isPreHook: true, - isPostHook: false, - requireUOContext: false + isPostHook: false }); // arr idx 0 => functionId of 0 has that spend From 11f20f61547124e260fdcdcfb410c02643f2eb57 Mon Sep 17 00:00:00 2001 From: howydev <132113803+howydev@users.noreply.github.com> Date: Mon, 8 Jul 2024 13:44:25 -0400 Subject: [PATCH 03/11] fix: update, lint fixes --- src/plugins/ERC20TokenLimitPlugin.sol | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/src/plugins/ERC20TokenLimitPlugin.sol b/src/plugins/ERC20TokenLimitPlugin.sol index c3ddfed8..f6cce94e 100644 --- a/src/plugins/ERC20TokenLimitPlugin.sol +++ b/src/plugins/ERC20TokenLimitPlugin.sol @@ -10,7 +10,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 {BasePlugin, IERC165} from "./BasePlugin.sol"; /// @title ERC20 Token Limit Plugin @@ -20,19 +19,19 @@ import {BasePlugin, IERC165} from "./BasePlugin.sol"; /// 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, IPermissionHook { +contract ERC20TokenLimitPlugin is BasePlugin, IExecutionHook { using UserOperationLib for PackedUserOperation; using EnumerableSet for EnumerableSet.AddressSet; - string public constant NAME = "ERC20 Token Limit Plugin"; - string public constant VERSION = "1.0.0"; - string public constant AUTHOR = "ERC-6900 Authors"; - struct ERC20SpendLimit { address token; uint256[] limits; } + string public constant NAME = "ERC20 Token Limit Plugin"; + string public constant VERSION = "1.0.0"; + string public constant AUTHOR = "ERC-6900 Authors"; + mapping(address account => mapping(address => uint256[] limits)) public limits; mapping(address account => EnumerableSet.AddressSet) internal _tokenList; @@ -71,15 +70,6 @@ contract ERC20TokenLimitPlugin is BasePlugin, IExecutionHook, IPermissionHook { } } - /// @inheritdoc IPermissionHook - function preUserOpExecutionHook(uint8 functionId, PackedUserOperation calldata uo) - external - override - returns (bytes memory) - { - return _checkSelectorAndDecrementLimit(functionId, uo.callData); - } - /// @inheritdoc IExecutionHook function preExecutionHook(uint8 functionId, address, uint256, bytes calldata data) external @@ -93,15 +83,15 @@ contract ERC20TokenLimitPlugin is BasePlugin, IExecutionHook, IPermissionHook { internal returns (bytes memory) { - bytes4 selector = bytes4(data[:4]); + (bytes4 selector, bytes memory callData) = _getSelectorAndCalldata(data); if (selector == IStandardExecutor.execute.selector) { - (address token,, bytes memory innerCalldata) = abi.decode(data[4:], (address, uint256, bytes)); + (address token,, bytes memory innerCalldata) = abi.decode(callData, (address, uint256, bytes)); if (_tokenList[msg.sender].contains(token)) { _decrementLimit(functionId, token, innerCalldata); } } else if (selector == IStandardExecutor.executeBatch.selector) { - Call[] memory calls = abi.decode(data[4:], (Call[])); + Call[] memory calls = abi.decode(callData, (Call[])); for (uint256 i = 0; i < calls.length; i++) { if (_tokenList[msg.sender].contains(calls[i].target)) { _decrementLimit(functionId, calls[i].target, calls[i].data); From 594c13b794c8e4028768411d572f839d3a02e235 Mon Sep 17 00:00:00 2001 From: howy <132113803+howydev@users.noreply.github.com> Date: Mon, 8 Jul 2024 18:25:27 -0400 Subject: [PATCH 04/11] Update src/plugins/ERC20TokenLimitPlugin.sol Co-authored-by: fangting-alchemy <119372438+fangting-alchemy@users.noreply.github.com> --- src/plugins/ERC20TokenLimitPlugin.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/ERC20TokenLimitPlugin.sol b/src/plugins/ERC20TokenLimitPlugin.sol index f6cce94e..64f52644 100644 --- a/src/plugins/ERC20TokenLimitPlugin.sol +++ b/src/plugins/ERC20TokenLimitPlugin.sol @@ -32,7 +32,7 @@ contract ERC20TokenLimitPlugin is BasePlugin, IExecutionHook { string public constant VERSION = "1.0.0"; string public constant AUTHOR = "ERC-6900 Authors"; - mapping(address account => mapping(address => uint256[] limits)) public limits; + mapping(address account => mapping(address token => uint256[] limits)) public limits; mapping(address account => EnumerableSet.AddressSet) internal _tokenList; error ExceededNativeTokenLimit(); From bac59999ca64f5ef0675d10899efd5ed13176473 Mon Sep 17 00:00:00 2001 From: howy <132113803+howydev@users.noreply.github.com> Date: Mon, 8 Jul 2024 18:25:48 -0400 Subject: [PATCH 05/11] Update src/plugins/ERC20TokenLimitPlugin.sol Co-authored-by: fangting-alchemy <119372438+fangting-alchemy@users.noreply.github.com> --- src/plugins/ERC20TokenLimitPlugin.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/ERC20TokenLimitPlugin.sol b/src/plugins/ERC20TokenLimitPlugin.sol index 64f52644..b31702d9 100644 --- a/src/plugins/ERC20TokenLimitPlugin.sol +++ b/src/plugins/ERC20TokenLimitPlugin.sol @@ -35,7 +35,7 @@ contract ERC20TokenLimitPlugin is BasePlugin, IExecutionHook { mapping(address account => mapping(address token => uint256[] limits)) public limits; mapping(address account => EnumerableSet.AddressSet) internal _tokenList; - error ExceededNativeTokenLimit(); + error ExceededTokenLimit(); error ExceededNumberOfEntities(); error SelectorNotAllowed(); From aee342ffb8254e650ba3c34ea29d13a144d9d296 Mon Sep 17 00:00:00 2001 From: howydev <132113803+howydev@users.noreply.github.com> Date: Mon, 8 Jul 2024 18:42:20 -0400 Subject: [PATCH 06/11] fix: rename error --- src/plugins/ERC20TokenLimitPlugin.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/ERC20TokenLimitPlugin.sol b/src/plugins/ERC20TokenLimitPlugin.sol index b31702d9..37fafbeb 100644 --- a/src/plugins/ERC20TokenLimitPlugin.sol +++ b/src/plugins/ERC20TokenLimitPlugin.sol @@ -62,7 +62,7 @@ contract ERC20TokenLimitPlugin is BasePlugin, IExecutionHook { if (selector == IERC20.transfer.selector || selector == IERC20.approve.selector) { uint256 limit = limits[msg.sender][token][functionId]; if (spend > limit) { - revert ExceededNativeTokenLimit(); + revert ExceededTokenLimit(); } limits[msg.sender][token][functionId] = limit - spend; } else { From 63c67b69fc5ac63045b7ea4dc45e53cb3a5f4496 Mon Sep 17 00:00:00 2001 From: howydev <132113803+howydev@users.noreply.github.com> Date: Wed, 10 Jul 2024 17:45:34 -0400 Subject: [PATCH 07/11] fix: pr review --- .gitmodules | 3 ++ lib/modular-account-libs | 1 + remappings.txt | 1 + src/plugins/ERC20TokenLimitPlugin.sol | 71 +++++++++++++-------------- 4 files changed, 38 insertions(+), 38 deletions(-) create mode 160000 lib/modular-account-libs 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 index 37fafbeb..97095145 100644 --- a/src/plugins/ERC20TokenLimitPlugin.sol +++ b/src/plugins/ERC20TokenLimitPlugin.sol @@ -5,6 +5,7 @@ import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interface 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"; @@ -22,34 +23,36 @@ import {BasePlugin, IERC165} from "./BasePlugin.sol"; 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 public constant NAME = "ERC20 Token Limit Plugin"; - string public constant VERSION = "1.0.0"; - string public constant AUTHOR = "ERC-6900 Authors"; + string internal constant _NAME = "ERC20 Token Limit Plugin"; + string internal constant _VERSION = "1.0.0"; + string internal constant _AUTHOR = "ERC-6900 Authors"; - mapping(address account => mapping(address token => uint256[] limits)) public limits; - mapping(address account => EnumerableSet.AddressSet) internal _tokenList; + mapping(uint8 functionId => mapping(address token => mapping(address account => uint256 limit))) public limits; + AssociatedLinkedListSet internal _tokenList; error ExceededTokenLimit(); error ExceededNumberOfEntities(); error SelectorNotAllowed(); function getTokensForAccount(address account) external view returns (address[] memory tokens) { - tokens = new address[](_tokenList[account].length()); - for (uint256 i = 0; i < _tokenList[account].length(); i++) { - tokens[i] = _tokenList[account].at(i); + 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; } function updateLimits(uint8 functionId, address token, uint256 newLimit) external { - _tokenList[msg.sender].add(token); - limits[msg.sender][token][functionId] = newLimit; + _tokenList.tryAdd(msg.sender, SetValue.wrap(bytes30(bytes20(token)))); + limits[functionId][token][msg.sender] = newLimit; } function _decrementLimit(uint8 functionId, address token, bytes memory innerCalldata) internal { @@ -59,12 +62,13 @@ contract ERC20TokenLimitPlugin is BasePlugin, IExecutionHook { 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[msg.sender][token][functionId]; + // transfer.selector = 0xa9059cbb. Using `transfer.selector` causes solhint to throw a warning + if (selector == 0xa9059cbb || selector == IERC20.approve.selector) { + uint256 limit = limits[functionId][token][msg.sender]; if (spend > limit) { revert ExceededTokenLimit(); } - limits[msg.sender][token][functionId] = limit - spend; + limits[functionId][token][msg.sender] = limit - spend; } else { revert SelectorNotAllowed(); } @@ -75,25 +79,18 @@ contract ERC20TokenLimitPlugin is BasePlugin, IExecutionHook { external override returns (bytes memory) - { - return _checkSelectorAndDecrementLimit(functionId, data); - } - - function _checkSelectorAndDecrementLimit(uint8 functionId, bytes calldata data) - internal - 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[msg.sender].contains(token)) { + 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[msg.sender].contains(calls[i].target)) { + if (_tokenList.contains(msg.sender, SetValue.wrap(bytes30(bytes20(calls[i].target))))) { _decrementLimit(functionId, calls[i].target, calls[i].data); } } @@ -109,15 +106,16 @@ contract ERC20TokenLimitPlugin is BasePlugin, IExecutionHook { /// @inheritdoc IPlugin function onInstall(bytes calldata data) external override { - ERC20SpendLimit[] memory spendLimits = abi.decode(data, (ERC20SpendLimit[])); + (uint8 startFunctionId, ERC20SpendLimit[] memory spendLimits) = abi.decode(data, (uint8, ERC20SpendLimit[])); - for (uint256 i = 0; i < spendLimits.length; i++) { - _tokenList[msg.sender].add(spendLimits[i].token); + 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[msg.sender][spendLimits[i].token].push(spendLimits[i].limits[j]); - } - if (limits[msg.sender][spendLimits[i].token].length > type(uint8).max) { - revert ExceededNumberOfEntities(); + limits[i + startFunctionId][spendLimits[i].token][msg.sender] = spendLimits[i].limits[j]; } } } @@ -125,22 +123,19 @@ contract ERC20TokenLimitPlugin is BasePlugin, IExecutionHook { /// @inheritdoc IPlugin function onUninstall(bytes calldata data) external override { (address token, uint8 functionId) = abi.decode(data, (address, uint8)); - delete limits[msg.sender][token][functionId]; + delete limits[functionId][token][msg.sender]; } /// @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[](1); metadata.permissionRequest[0] = "erc20-token-limit"; From db9fc97786fc99cf4477bb683afcf495a6cfbc3d Mon Sep 17 00:00:00 2001 From: howydev <132113803+howydev@users.noreply.github.com> Date: Wed, 10 Jul 2024 17:52:11 -0400 Subject: [PATCH 08/11] fix: test, lint --- src/plugins/ERC20TokenLimitPlugin.sol | 79 +++++++++++++------------ test/plugin/ERC20TokenLimitPlugin.t.sol | 26 ++++---- 2 files changed, 53 insertions(+), 52 deletions(-) diff --git a/src/plugins/ERC20TokenLimitPlugin.sol b/src/plugins/ERC20TokenLimitPlugin.sol index 97095145..203c1184 100644 --- a/src/plugins/ERC20TokenLimitPlugin.sol +++ b/src/plugins/ERC20TokenLimitPlugin.sol @@ -5,7 +5,11 @@ import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interface 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 { + SetValue, + AssociatedLinkedListSet, + AssociatedLinkedListSetLib +} from "@modular-account-libs/libraries/AssociatedLinkedListSetLib.sol"; import {PluginManifest, PluginMetadata} from "../interfaces/IPlugin.sol"; import {IStandardExecutor, Call} from "../interfaces/IStandardExecutor.sol"; @@ -41,39 +45,11 @@ contract ERC20TokenLimitPlugin is BasePlugin, IExecutionHook { error ExceededNumberOfEntities(); error SelectorNotAllowed(); - 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; - } - function updateLimits(uint8 functionId, address token, uint256 newLimit) external { _tokenList.tryAdd(msg.sender, SetValue.wrap(bytes30(bytes20(token)))); limits[functionId][token][msg.sender] = newLimit; } - 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 - } - // transfer.selector = 0xa9059cbb. Using `transfer.selector` causes solhint to throw a warning - if (selector == 0xa9059cbb || selector == IERC20.approve.selector) { - uint256 limit = limits[functionId][token][msg.sender]; - if (spend > limit) { - revert ExceededTokenLimit(); - } - limits[functionId][token][msg.sender] = limit - spend; - } else { - revert SelectorNotAllowed(); - } - } - /// @inheritdoc IExecutionHook function preExecutionHook(uint8 functionId, address, uint256, bytes calldata data) external @@ -99,14 +75,10 @@ contract ERC20TokenLimitPlugin is BasePlugin, IExecutionHook { return ""; } - /// @inheritdoc IExecutionHook - function postExecutionHook(uint8, bytes calldata) external pure override { - revert NotImplemented(); - } - /// @inheritdoc IPlugin function onInstall(bytes calldata data) external override { - (uint8 startFunctionId, ERC20SpendLimit[] memory spendLimits) = abi.decode(data, (uint8, ERC20SpendLimit[])); + (uint8 startFunctionId, ERC20SpendLimit[] memory spendLimits) = + abi.decode(data, (uint8, ERC20SpendLimit[])); if (startFunctionId + spendLimits.length > type(uint8).max) { revert ExceededNumberOfEntities(); @@ -126,6 +98,20 @@ contract ERC20TokenLimitPlugin is BasePlugin, IExecutionHook { 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) {} @@ -142,12 +128,27 @@ contract ERC20TokenLimitPlugin is BasePlugin, IExecutionHook { return metadata; } - // ┏━━━━━━━━━━━━━━━┓ - // ┃ EIP-165 ┃ - // ┗━━━━━━━━━━━━━━━┛ - /// @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 + } + // transfer.selector = 0xa9059cbb. Using `transfer.selector` causes solhint to throw a warning + if (selector == 0xa9059cbb || selector == IERC20.approve.selector) { + uint256 limit = limits[functionId][token][msg.sender]; + if (spend > limit) { + revert ExceededTokenLimit(); + } + limits[functionId][token][msg.sender] = limit - spend; + } else { + revert SelectorNotAllowed(); + } + } } diff --git a/test/plugin/ERC20TokenLimitPlugin.t.sol b/test/plugin/ERC20TokenLimitPlugin.t.sol index c96d467c..698c1b2d 100644 --- a/test/plugin/ERC20TokenLimitPlugin.t.sol +++ b/test/plugin/ERC20TokenLimitPlugin.t.sol @@ -55,7 +55,7 @@ contract ERC20TokenLimitPluginTest is OptimizedTest { limit[0] = ERC20TokenLimitPlugin.ERC20SpendLimit({token: address(erc20), limits: limits}); bytes[] memory permissionInitDatas = new bytes[](1); - permissionInitDatas[0] = abi.encode(limit); + permissionInitDatas[0] = abi.encode(uint8(0), limit); vm.prank(address(acct)); acct.installValidation( @@ -93,9 +93,9 @@ contract ERC20TokenLimitPluginTest is OptimizedTest { function test_userOp_executeLimit() public { vm.startPrank(address(entryPoint)); - assertEq(plugin.limits(address(acct), address(erc20), 0), 10 ether); + assertEq(plugin.limits(0, address(erc20), address(acct)), 10 ether); acct.executeUserOp(_getPackedUO(_getExecuteWithSpend(5 ether)), bytes32(0)); - assertEq(plugin.limits(address(acct), address(erc20), 0), 5 ether); + assertEq(plugin.limits(0, address(erc20), address(acct)), 5 ether); } function test_userOp_executeBatchLimit() public { @@ -111,9 +111,9 @@ contract ERC20TokenLimitPluginTest is OptimizedTest { }); vm.startPrank(address(entryPoint)); - assertEq(plugin.limits(address(acct), address(erc20), 0), 10 ether); + assertEq(plugin.limits(0, address(erc20), address(acct)), 10 ether); acct.executeUserOp(_getPackedUO(abi.encodeCall(IStandardExecutor.executeBatch, (calls))), bytes32(0)); - assertEq(plugin.limits(address(acct), address(erc20), 0), 10 ether - 6 ether - 100001); + assertEq(plugin.limits(0, address(erc20), address(acct)), 10 ether - 6 ether - 100001); } function test_userOp_executeBatch_approveAndTransferLimit() public { @@ -129,9 +129,9 @@ contract ERC20TokenLimitPluginTest is OptimizedTest { }); vm.startPrank(address(entryPoint)); - assertEq(plugin.limits(address(acct), address(erc20), 0), 10 ether); + assertEq(plugin.limits(0, address(erc20), address(acct)), 10 ether); acct.executeUserOp(_getPackedUO(abi.encodeCall(IStandardExecutor.executeBatch, (calls))), bytes32(0)); - assertEq(plugin.limits(address(acct), address(erc20), 0), 10 ether - 6 ether - 100001); + assertEq(plugin.limits(0, address(erc20), address(acct)), 10 ether - 6 ether - 100001); } function test_userOp_executeBatch_approveAndTransferLimit_fail() public { @@ -147,20 +147,20 @@ contract ERC20TokenLimitPluginTest is OptimizedTest { }); vm.startPrank(address(entryPoint)); - assertEq(plugin.limits(address(acct), address(erc20), 0), 10 ether); + 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(address(acct), address(erc20), 0), 10 ether); + assertEq(plugin.limits(0, address(erc20), address(acct)), 10 ether); } function test_runtime_executeLimit() public { - assertEq(plugin.limits(address(acct), address(erc20), 0), 10 ether); + assertEq(plugin.limits(0, address(erc20), address(acct)), 10 ether); acct.executeWithAuthorization( _getExecuteWithSpend(5 ether), abi.encodePacked(validationFunction, uint8(1)) ); - assertEq(plugin.limits(address(acct), address(erc20), 0), 5 ether); + assertEq(plugin.limits(0, address(erc20), address(acct)), 5 ether); } function test_runtime_executeBatchLimit() public { @@ -175,10 +175,10 @@ contract ERC20TokenLimitPluginTest is OptimizedTest { data: abi.encodeCall(IERC20.approve, (recipient, 5 ether + 100000)) }); - assertEq(plugin.limits(address(acct), address(erc20), 0), 10 ether); + assertEq(plugin.limits(0, address(erc20), address(acct)), 10 ether); acct.executeWithAuthorization( abi.encodeCall(IStandardExecutor.executeBatch, (calls)), abi.encodePacked(validationFunction, uint8(1)) ); - assertEq(plugin.limits(address(acct), address(erc20), 0), 10 ether - 6 ether - 100001); + assertEq(plugin.limits(0, address(erc20), address(acct)), 10 ether - 6 ether - 100001); } } From 61ff2d622186b4b17b38afa7ac73b34d94a79173 Mon Sep 17 00:00:00 2001 From: howydev <132113803+howydev@users.noreply.github.com> Date: Wed, 10 Jul 2024 19:57:24 -0400 Subject: [PATCH 09/11] fix: test --- test/plugin/ERC20TokenLimitPlugin.t.sol | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/plugin/ERC20TokenLimitPlugin.t.sol b/test/plugin/ERC20TokenLimitPlugin.t.sol index 698c1b2d..9b5e4ae8 100644 --- a/test/plugin/ERC20TokenLimitPlugin.t.sol +++ b/test/plugin/ERC20TokenLimitPlugin.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 {MockERC20} from "../mocks/MockERC20.sol"; import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; @@ -16,10 +15,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 ERC20TokenLimitPluginTest is OptimizedTest { - EntryPoint public entryPoint = new EntryPoint(); +contract ERC20TokenLimitPluginTest is AccountTestBase { address public recipient = address(1); MockERC20 public erc20; address payable public bundler = payable(address(2)); @@ -80,7 +78,7 @@ contract ERC20TokenLimitPluginTest is OptimizedTest { preVerificationGas: 200000, gasFees: bytes32(uint256(uint128(0))), paymasterAndData: "", - signature: abi.encodePacked(FunctionReferenceLib.pack(address(validationPlugin), 0), uint8(1)) + signature: _encodeSignature(FunctionReferenceLib.pack(address(validationPlugin), 0), 1, "") }); } @@ -158,7 +156,8 @@ contract ERC20TokenLimitPluginTest is OptimizedTest { function test_runtime_executeLimit() public { assertEq(plugin.limits(0, address(erc20), address(acct)), 10 ether); acct.executeWithAuthorization( - _getExecuteWithSpend(5 ether), abi.encodePacked(validationFunction, uint8(1)) + _getExecuteWithSpend(5 ether), + _encodeSignature(FunctionReferenceLib.pack(address(validationPlugin), 0), 1, "") ); assertEq(plugin.limits(0, address(erc20), address(acct)), 5 ether); } @@ -177,7 +176,8 @@ contract ERC20TokenLimitPluginTest is OptimizedTest { assertEq(plugin.limits(0, address(erc20), address(acct)), 10 ether); acct.executeWithAuthorization( - abi.encodeCall(IStandardExecutor.executeBatch, (calls)), abi.encodePacked(validationFunction, uint8(1)) + 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); } From ed0b5471834fd70a0146a0a3aaf2867c8528ea93 Mon Sep 17 00:00:00 2001 From: howydev <132113803+howydev@users.noreply.github.com> Date: Mon, 15 Jul 2024 23:24:10 -0400 Subject: [PATCH 10/11] fix: tests --- test/plugin/ERC20TokenLimitPlugin.t.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/plugin/ERC20TokenLimitPlugin.t.sol b/test/plugin/ERC20TokenLimitPlugin.t.sol index 9b5e4ae8..96a18c20 100644 --- a/test/plugin/ERC20TokenLimitPlugin.t.sol +++ b/test/plugin/ERC20TokenLimitPlugin.t.sol @@ -13,6 +13,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"; @@ -57,8 +58,7 @@ contract ERC20TokenLimitPluginTest 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), new bytes(0), From fa6762ed2fc8bd2061a897a2f849ddfb4a9715be Mon Sep 17 00:00:00 2001 From: howydev <132113803+howydev@users.noreply.github.com> Date: Tue, 16 Jul 2024 15:57:39 -0700 Subject: [PATCH 11/11] fix: improve lint --- src/plugins/ERC20TokenLimitPlugin.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plugins/ERC20TokenLimitPlugin.sol b/src/plugins/ERC20TokenLimitPlugin.sol index 203c1184..1df5bcfd 100644 --- a/src/plugins/ERC20TokenLimitPlugin.sol +++ b/src/plugins/ERC20TokenLimitPlugin.sol @@ -140,12 +140,12 @@ contract ERC20TokenLimitPlugin is BasePlugin, IExecutionHook { 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 } - // transfer.selector = 0xa9059cbb. Using `transfer.selector` causes solhint to throw a warning - if (selector == 0xa9059cbb || selector == IERC20.approve.selector) { + 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();