From bb956a4803d3dea279183567e5b8ebe66a4096d4 Mon Sep 17 00:00:00 2001 From: JaredBorders Date: Tue, 17 Oct 2023 15:57:53 -0400 Subject: [PATCH 01/11] =?UTF-8?q?=E2=9B=B3=EF=B8=8F=20Refactor=20&=20Optim?= =?UTF-8?q?ize=20SMv2=20SVM?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/SMv2SessionValidationModule.sol | 62 +++++++++++------------------ 1 file changed, 23 insertions(+), 39 deletions(-) diff --git a/src/SMv2SessionValidationModule.sol b/src/SMv2SessionValidationModule.sol index f8d6e03..d554174 100644 --- a/src/SMv2SessionValidationModule.sol +++ b/src/SMv2SessionValidationModule.sol @@ -1,15 +1,15 @@ // SPDX-License-Identifier: GPL-3.0-or-later pragma solidity 0.8.18; +import {ECDSA} from "src/openzeppelin/ECDSA.sol"; +import {IAccount} from "src/kwenta/smv2/IAccount.sol"; import { ISessionValidationModule, UserOperation } from "src/biconomy/interfaces/ISessionValidationModule.sol"; -import {ECDSA} from "src/openzeppelin/ECDSA.sol"; /** - * @title Kwenta Smart Margin v2 Session Validation Module for Biconomy Smart Accounts. - * @dev Validates userOps for `Account.execute()` using a session key signature. + * @title Kwenta Smart Margin v2 Session Validation Module for Biconomy Smart Accounts * @author Fil Makarov - * @author JaredBorders (jaredborders@pm.me) */ @@ -33,20 +33,17 @@ contract SMv2SessionValidationModule is ISessionValidationModule { bytes calldata _funcCallData, bytes calldata _sessionKeyData, bytes calldata /*_callSpecificData*/ - ) external virtual override returns (address) { - ( - address sessionKey, - address smv2ProxyAccount, - bytes4 smv2ExecuteSelector - ) = abi.decode(_sessionKeyData, (address, address, bytes4)); + ) external pure override returns (address) { + (address sessionKey, address smv2ProxyAccount) = + abi.decode(_sessionKeyData, (address, address)); /// @dev ensure destinationContract is the SMv2ProxyAccount if (destinationContract != smv2ProxyAccount) { revert InvalidDestinationContract(); } - /// @dev ensure the function selector is the `SmartAccount.execute` selector - if (bytes4(_funcCallData[0:4]) != smv2ExecuteSelector) { + /// @dev ensure the function selector is the `IAccount.execute` selector + if (bytes4(_funcCallData[0:4]) != IAccount.execute.selector) { revert InvalidSMv2Selector(); } @@ -55,13 +52,6 @@ contract SMv2SessionValidationModule is ISessionValidationModule { revert InvalidCallValue(); } - // (IAccount.Command[] memory _commands, bytes[] memory _inputs) = abi.decode( - // _funcCallData[4:], - // (IAccount.Command[], bytes[]) - // ); - - /// @custom:add-param-validation-here-if-needed - return sessionKey; } @@ -89,28 +79,22 @@ contract SMv2SessionValidationModule is ISessionValidationModule { revert InvalidSelector(); } - ( - address sessionKey, - address smv2ProxyAccount, - bytes4 smv2ExecuteSelector - ) = abi.decode(_sessionKeyData, (address, address, bytes4)); + (address sessionKey, address smv2ProxyAccount) = + abi.decode(_sessionKeyData, (address, address)); - { - // we expect _op.callData to be `SmartAccount.execute(to, value, calldata)` calldata - (address destinationContract, uint256 callValue,) = abi.decode( - _op.callData[4:], // skip selector - (address, uint256, bytes) - ); + (address destinationContract, uint256 callValue,) = abi.decode( + _op.callData[4:], // skip selector; already checked + (address, uint256, bytes) + ); - /// @dev ensure destinationContract is the SMv2ProxyAccount - if (destinationContract != smv2ProxyAccount) { - revert InvalidDestinationContract(); - } + /// @dev ensure destinationContract is the SMv2ProxyAccount + if (destinationContract != smv2ProxyAccount) { + revert InvalidDestinationContract(); + } - /// @dev ensure call value is zero - if (callValue != 0) { - revert InvalidCallValue(); - } + /// @dev ensure call value is zero + if (callValue != 0) { + revert InvalidCallValue(); } // working with userOp.callData @@ -124,8 +108,8 @@ contract SMv2SessionValidationModule is ISessionValidationModule { data = _op.callData[4 + offset + 32:4 + offset + 32 + length]; } - /// @dev ensure the function selector is the smv2ExecuteSelector selector - if (bytes4(data[0:4]) != smv2ExecuteSelector) { + /// @dev ensure the function selector is `IAccount.execute` + if (bytes4(data[0:4]) != IAccount.execute.selector) { revert InvalidSMv2Selector(); } From 1514330568a638e70cec8e11e82af911fd29de0d Mon Sep 17 00:00:00 2001 From: JaredBorders Date: Tue, 17 Oct 2023 15:58:22 -0400 Subject: [PATCH 02/11] =?UTF-8?q?=E2=9B=B3=EF=B8=8F=20Add=20Selectors,=20R?= =?UTF-8?q?efactor=20&=20Optimize=20SMv3=20SVM?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/SMv3SessionValidationModule.sol | 111 ++++++++++++---------------- 1 file changed, 47 insertions(+), 64 deletions(-) diff --git a/src/SMv3SessionValidationModule.sol b/src/SMv3SessionValidationModule.sol index 1c228b0..331038f 100644 --- a/src/SMv3SessionValidationModule.sol +++ b/src/SMv3SessionValidationModule.sol @@ -1,20 +1,16 @@ // SPDX-License-Identifier: GPL-3.0-or-later pragma solidity 0.8.18; +import {ECDSA} from "src/openzeppelin/ECDSA.sol"; +import {IEngine} from "src/kwenta/smv3/IEngine.sol"; +import {IERC7412} from "src/kwenta/smv3/IERC7412.sol"; import { ISessionValidationModule, UserOperation } from "src/biconomy/interfaces/ISessionValidationModule.sol"; -import {ECDSA} from "src/openzeppelin/ECDSA.sol"; /** - * @title Kwenta Smart Margin v3 Session Validation Module for Biconomy Smart Accounts. - * @dev Validates userOps for - * `IEngine.modifyCollateral()` - * `IEngine.commitOrder()` - * `IEngine.invalidateUnorderedNonces()` - * `IERC7412.fulfillOracleQuery()` - * using a session key signature. + * @title Kwenta Smart Margin v3 Session Validation Module for Biconomy Smart Accounts * @author Fil Makarov - * @author JaredBorders (jaredborders@pm.me) */ @@ -38,36 +34,34 @@ contract SMv3SessionValidationModule is ISessionValidationModule { bytes calldata _funcCallData, bytes calldata _sessionKeyData, bytes calldata /*_callSpecificData*/ - ) external virtual override returns (address) { - ( - address sessionKey, - address smv3Engine, - bytes4 smv3ModifyCollateralSelector, - bytes4 smv3CommitOrderSelector, - bytes4 smv3InvalidateUnorderedNoncesSelector, - bytes4 smv3FulfillOracleQuerySelector - ) = abi.decode( - _sessionKeyData, (address, address, bytes4, bytes4, bytes4, bytes4) - ); + ) external pure override returns (address) { + (address sessionKey, address smv3Engine) = + abi.decode(_sessionKeyData, (address, address)); /// @dev ensure destinationContract is the smv3Engine if (destinationContract != smv3Engine) { revert InvalidDestinationContract(); } - /// @dev ensure the function selector is the a valid selector + /// @dev ensure the function selector is the a valid IEngine selector bytes4 funcSelector = bytes4(_funcCallData[0:4]); if ( - funcSelector != smv3ModifyCollateralSelector - && funcSelector != smv3CommitOrderSelector - && funcSelector != smv3InvalidateUnorderedNoncesSelector - && funcSelector != smv3FulfillOracleQuerySelector + funcSelector != IEngine.modifyCollateral.selector + && funcSelector != IEngine.commitOrder.selector + && funcSelector != IEngine.invalidateUnorderedNonces.selector + && funcSelector != IERC7412.fulfillOracleQuery.selector + && funcSelector != IEngine.depositEth.selector + && funcSelector != IEngine.withdrawEth.selector ) { revert InvalidSMv3Selector(); } - /// @dev ensure call value is zero - if (callValue != 0) { + /// @dev ensure call value is zero unless calling IEngine.depositEth + if (funcSelector == IEngine.depositEth.selector) { + if (callValue == 0) { + revert InvalidCallValue(); + } + } else if (callValue != 0) { revert InvalidCallValue(); } @@ -98,25 +92,17 @@ contract SMv3SessionValidationModule is ISessionValidationModule { revert InvalidSelector(); } - (address sessionKey, address smv3Engine,,,,) = abi.decode( - _sessionKeyData, (address, address, bytes4, bytes4, bytes4, bytes4) - ); + (address sessionKey, address smv3Engine) = + abi.decode(_sessionKeyData, (address, address)); - { - (address destinationContract, uint256 callValue,) = abi.decode( - _op.callData[4:], // skip selector - (address, uint256, bytes) - ); - - /// @dev ensure destinationContract is the smv3Engine - if (destinationContract != smv3Engine) { - revert InvalidDestinationContract(); - } + (address destinationContract, uint256 callValue,) = abi.decode( + _op.callData[4:], // skip selector; already checked + (address, uint256, bytes) + ); - /// @dev ensure call value is zero - if (callValue != 0) { - revert InvalidCallValue(); - } + /// @dev ensure destinationContract is the smv3Engine + if (destinationContract != smv3Engine) { + revert InvalidDestinationContract(); } // working with userOp.callData @@ -129,29 +115,26 @@ contract SMv3SessionValidationModule is ISessionValidationModule { data = _op.callData[4 + offset + 32:4 + offset + 32 + length]; } - { - ( - , - , - bytes4 smv3ModifyCollateralSelector, - bytes4 smv3CommitOrderSelector, - bytes4 smv3InvalidateUnorderedNoncesSelector, - bytes4 smv3FulfillOracleQuerySelector - ) = abi.decode( - _sessionKeyData, - (address, address, bytes4, bytes4, bytes4, bytes4) - ); + /// @dev ensure the function selector is the a valid IEngine selector + bytes4 funcSelector = bytes4(data[0:4]); + if ( + funcSelector != IEngine.modifyCollateral.selector + && funcSelector != IEngine.commitOrder.selector + && funcSelector != IEngine.invalidateUnorderedNonces.selector + && funcSelector != IERC7412.fulfillOracleQuery.selector + && funcSelector != IEngine.depositEth.selector + && funcSelector != IEngine.withdrawEth.selector + ) { + revert InvalidSMv3Selector(); + } - /// @dev ensure the function selector is the a valid selector - bytes4 funcSelector = bytes4(data[0:4]); - if ( - funcSelector != smv3ModifyCollateralSelector - && funcSelector != smv3CommitOrderSelector - && funcSelector != smv3InvalidateUnorderedNoncesSelector - && funcSelector != smv3FulfillOracleQuerySelector - ) { - revert InvalidSMv3Selector(); + /// @dev ensure call value is zero unless calling IEngine.depositEth + if (funcSelector == IEngine.depositEth.selector) { + if (callValue == 0) { + revert InvalidCallValue(); } + } else if (callValue != 0) { + revert InvalidCallValue(); } /// @dev this method of signature validation is out-of-date From 61c6b517f5b3754df07a31b3515792c774b29364 Mon Sep 17 00:00:00 2001 From: JaredBorders Date: Tue, 17 Oct 2023 15:58:36 -0400 Subject: [PATCH 03/11] =?UTF-8?q?=F0=9F=91=B7=F0=9F=8F=BB=E2=80=8D?= =?UTF-8?q?=E2=99=82=EF=B8=8F=20Update=20interface?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/kwenta/smv3/IEngine.sol | 150 ++++++++++++++++++++++++++++++------ 1 file changed, 127 insertions(+), 23 deletions(-) diff --git a/src/kwenta/smv3/IEngine.sol b/src/kwenta/smv3/IEngine.sol index 1589a05..05c6b44 100644 --- a/src/kwenta/smv3/IEngine.sol +++ b/src/kwenta/smv3/IEngine.sol @@ -40,6 +40,8 @@ interface IEngine { bool requireVerified; // address that can execute the order if requireVerified is false address trustedExecutor; + // max fee denominated in ETH that can be paid to the executor + uint256 maxExecutorFee; // array of extra conditions to be met bytes[] conditions; } @@ -67,15 +69,34 @@ interface IEngine { /// @notice thrown when attempting to verify a condition identified by an invalid selector error InvalidConditionSelector(bytes4 selector); + /// @notice thrown when attempting to debit an account with insufficient balance + error InsufficientEthBalance(); + + /// @notice thrown when attempting to transfer eth fails + error EthTransferFailed(); + /*////////////////////////////////////////////////////////////// EVENTS //////////////////////////////////////////////////////////////*/ /// @notice emitted when the account owner or delegate successfully invalidates an unordered nonce + /// @param accountId the id of the account that was invalidated + /// @param word the word position of the bitmap that was invalidated + /// @param mask the mask used to invalidate the bitmap event UnorderedNonceInvalidation( uint128 indexed accountId, uint256 word, uint256 mask ); + /// @notice emitted when eth is deposited into the engine and credited to an account + /// @param accountId the id of the account that was credited + /// @param amount the amount of eth deposited + event EthDeposit(uint128 indexed accountId, uint256 amount); + + /// @notice emitted when eth is withdrawn from the engine and debited from an account + /// @param accountId the id of the account that was debited + /// @param amount the amount of eth withdrawn + event EthWithdraw(uint128 indexed accountId, uint256 amount); + /*////////////////////////////////////////////////////////////// AUTHENTICATION //////////////////////////////////////////////////////////////*/ @@ -102,6 +123,19 @@ interface IEngine { view returns (bool); + /*////////////////////////////////////////////////////////////// + ETH MANAGEMENT + //////////////////////////////////////////////////////////////*/ + + /// @notice deposit eth into the engine and credit the account identified by the accountId + /// @param _accountId the id of the account to credit + function depositEth(uint128 _accountId) external payable; + + /// @notice withdraw eth from the engine and debit the account identified by the accountId + /// @param _accountId the id of the account to debit + /// @param _amount the amount of eth to withdraw + function withdrawEth(uint128 _accountId, uint256 _amount) external; + /*////////////////////////////////////////////////////////////// NONCE MANAGEMENT //////////////////////////////////////////////////////////////*/ @@ -169,41 +203,111 @@ interface IEngine { CONDITIONAL ORDER MANAGEMENT //////////////////////////////////////////////////////////////*/ - /// In order for a conditional order to be committed and then executed there are a number of requirements that need to be met: + /// Conditional Orders + /// + /// tldr: + /// Conditional Orders (co's) are signed objects that define an async order + /// and the conditions that must be met for the order to be executed. + /// + /// deep dive: + /// co's are composed of 8 main parts: + /// 1. The async order details which are defined in the OrderDetails struct + /// (the order that is being submitted to Synthetix perps v3 market) + /// 2. isReduceOnly flag which indicates if the order can only reduce + /// the position size and is also defined in the OrderDetails struct + /// 3. The signer of the co which must be the account owner or delegate + /// and is included in the ConditionalOrder struct. + /// THIS DATA IS ALWAYS CHECKED ON-CHAIN + /// 4. The nonce of the co which is included in the ConditionalOrder struct. + /// The nonce is specific to the account id and is used to prevent replay attacks. + /// The nonce is not specific to an address, but rather an account id. + /// THIS DATA IS ALWAYS CHECKED ON-CHAIN + /// 5. The requireVerified flag which is included in the ConditionalOrder struct. + /// If requireVerified is true, all conditions defined in the co must be satisfied on-chain. + /// If requireVerified is false, the co can ONLY be executed by the trustedExecutor. + /// Notice that the conditions are not checked on-chain if requireVerified is false but are + /// expected to be checked off-chain by the trustedExecutor. This saves a significant amount gas. + /// 6. The trustedExecutor address which is included in the ConditionalOrder struct. + /// The trustedExecutor is the address that can execute the co if requireVerified is false. + /// If requireVerified is true, the trustedExecutor is ignored/not used. + /// 7. The maxExecutorFee which is included in the ConditionalOrder struct. + /// The maxExecutorFee is the maximum fee that can be imposed by the address that + /// successfully executes the co (trustedExecutor or not). This max fee is denominated in ETH and is + /// enforced on-chain. If the maxExecutorFee is greater than the fee specified + /// by the executor, the co will *not* be executed. + /// 8. The conditions which are included in the ConditionalOrder struct. + /// Conditions are encoded function selectors and parameters that are used to determine + /// if the co can be executed. Conditions are checked on-chain if requireVerified is true. + /// If requireVerified is false, conditions are expected to be checked off-chain by the trustedExecutor. + /// Conditions are stictly limited selectors defined in the Engine contract + /// (ex: isTimestampBeforeSelector, isPriceAboveSelector, etc.) + /// + /// co's are not creaed on-chain. They are composed and signed off-chain. The signature + /// is then passed to the Engine contract along with the co. The Engine contract then + /// verifies the signature along with many other "things" to determine if the co can be executed. /// - /// (1) The account must have sufficient snxUSD collateral to handle the order - /// (2) The account must not have another order committed - /// (3) The order’s set `acceptablePrice` needs to be met both on committing the order and when it gets executed - /// (users should choose a value for this that is likely to execute based on the conditions set) - /// (4) The order can only be executed within Synthetix’s set settlement window - /// (5) There must be a keeper that executes a conditional order + /// Checklist: + /// In *every* case of co execution, the logic of validating the co is: /// - /// @notice There is no guarantee a conditional order will be executed + /// 1. Check if the fee specified by the executor is less than or equal to the maxExecutorFee + /// 2. Check if the account has sufficient ETH credit to pay the fee + /// (see ETH MANAGEMENT for how that can be accomplished) + /// 3. Check if the nonce has been used (see NONCE MANAGEMENT for how that can be accomplished) + /// 4. Check if the signer is the owner or delegate of the account + /// 5. Check if the signature is valid for the given co and signer + /// 6. IF requireVerified is true, check if all conditions are met + /// ELSE IF requireVerified is false, check if the msg.sender is the trustedExecutor + /// + /// All of these checks are carried out via a call to the Engine's canExecute function + /// that returns true or false. If canExecute returns true, the co can be executed. + /// If canExecute returns false, the co cannot be executed. + /// This function is expected to be used off-chain to determine if the co can be executed. + /// It will be called within the Engine's execute function to determine if the co can be executed + /// and if it returns true, the co will be executed. If it returns false, the co will not be executed + /// and the transaction will revert with CannotExecuteOrder(). + /// + /// The Engine contract does not store co's. It only stores the nonceBitmaps for each account. + /// The Engine does hold and account for ETH credit and can modify the ETH credit of an account. + /// + /// ETH Management: + /// With the introduction of co's, the Engine contract now holds ETH credit for accounts. + /// Using collateral to pay for fees is not ideal due to accounting risks associated with + /// orders that are close to max leverage. To mitigate this risk, the Engine contract + /// holds ETH credit for accounts. This ETH credit is used to pay for fees. + /// Furthermore, given the multi-colateral nature of the protocol, the Engine contract + /// does not need to handle scenarios where an account does not have sufficient + /// snxUSD collateral to pay the fee. + /// + /// Finally, the current approach to implementing Account Abstraction via ERC-4337 + /// requires traders deposit ETH to the "protocol" prior to trading. This ETH can be + /// multipurposed to pay for fees. This is the approach taken by the Engine contract. + + /// @custom:docs for more in-depth documentation of conditional order mechanism, + /// please refer to https://github.com/Kwenta/smart-margin-v3/wiki/Conditional-Orders /// @notice execute a conditional order /// @param _co the conditional order /// @param _signature the signature of the conditional order + /// @param _fee the fee paid to executor for the conditional order /// @return retOrder the order committed /// @return fees the fees paid for the order to Synthetix - /// @return conditionalOrderFee the fee paid to executor for the conditional order - function execute(ConditionalOrder calldata _co, bytes calldata _signature) - external - returns ( - IPerpsMarketProxy.Data memory retOrder, - uint256 fees, - uint256 conditionalOrderFee - ); + function execute( + ConditionalOrder calldata _co, + bytes calldata _signature, + uint256 _fee + ) external returns (IPerpsMarketProxy.Data memory retOrder, uint256 fees); - /// @notice checks if the order can be executed based on defined conditions - /// @dev this function does NOT check if the order can be executed based on the account's balance - /// (i.e. does not check if enough USD is available to pay for the order fee nor does it check - /// if enough collateral is available to cover the order) - /// @param _co the conditional order + /// @notice checks if the conditional order can be executed + /// @param _co the conditional order which details the order to be executed and the conditions to be met /// @param _signature the signature of the conditional order - /// @return true if the order can be executed based on defined conditions, false otherwise + /// @param _fee the executor specified fee for the executing the conditional order + /// @dev if the fee is greater than the maxExecutorFee defined in the conditional order, + /// or if the account lacks sufficient ETH credit to pay the fee, canExecute will return false + /// @return true if the order can be executed, false otherwise function canExecute( ConditionalOrder calldata _co, - bytes calldata _signature + bytes calldata _signature, + uint256 _fee ) external view returns (bool); /// @notice verify the conditional order signer is the owner or delegate of the account From 378c605827696e2b1720e2681ac67191ca8a8914 Mon Sep 17 00:00:00 2001 From: JaredBorders Date: Tue, 17 Oct 2023 15:58:55 -0400 Subject: [PATCH 04/11] =?UTF-8?q?=E2=9C=85=20Refactor=20&=20Improve=20Test?= =?UTF-8?q?s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- test/SMv2SessionValidationModule.t.sol | 191 ++++---- test/SMv3SessionValidationModule.t.sol | 592 +++++++++---------------- test/utils/Bootstrap.sol | 3 + 3 files changed, 299 insertions(+), 487 deletions(-) diff --git a/test/SMv2SessionValidationModule.t.sol b/test/SMv2SessionValidationModule.t.sol index a24f08f..b393a7f 100644 --- a/test/SMv2SessionValidationModule.t.sol +++ b/test/SMv2SessionValidationModule.t.sol @@ -14,12 +14,9 @@ import {IAccount} from "src/kwenta/smv2/IAccount.sol"; contract SMv2SessionValidationModuleTest is Bootstrap { address signer; uint256 signerPrivateKey; - address bad_signer; - uint256 bad_signerPrivateKey; address sessionKey; address smv2ProxyAccount; - bytes4 smv2ExecuteSelector; address destinationContract; uint256 callValue; bytes funcCallData; @@ -43,20 +40,16 @@ contract SMv2SessionValidationModuleTest is Bootstrap { // signers signerPrivateKey = 0x12341234; signer = vm.addr(signerPrivateKey); - bad_signerPrivateKey = 0x12341235; - bad_signer = vm.addr(bad_signerPrivateKey); // session key data sessionKey = signer; smv2ProxyAccount = address(0x2); - smv2ExecuteSelector = IAccount.execute.selector; // validateSessionParams params destinationContract = smv2ProxyAccount; callValue = 0; - funcCallData = abi.encode(smv2ExecuteSelector, bytes32("")); - sessionKeyData = - abi.encode(sessionKey, smv2ProxyAccount, smv2ExecuteSelector); + funcCallData = abi.encode(IAccount.execute.selector, bytes32("")); + sessionKeyData = abi.encode(sessionKey, destinationContract); callSpecificData = ""; // validateSessionUserOp params @@ -83,8 +76,10 @@ contract ValidateSessionParams is SMv2SessionValidationModuleTest { assertEq(sessionKey, retSessionKey); } - function test_validateSessionParams_destinationContract_invalid() public { - destinationContract = address(0); + function test_validateSessionParams_destinationContract_invalid( + address invalid_destinationContract + ) public { + vm.assume(invalid_destinationContract != destinationContract); vm.expectRevert( abi.encodeWithSelector( @@ -93,7 +88,7 @@ contract ValidateSessionParams is SMv2SessionValidationModuleTest { ); smv2SessionValidationModule.validateSessionParams( - destinationContract, + invalid_destinationContract, callValue, funcCallData, sessionKeyData, @@ -101,8 +96,10 @@ contract ValidateSessionParams is SMv2SessionValidationModuleTest { ); } - function test_validateSessionParams_callValue_invalid() public { - callValue = 1; + function test_validateSessionParams_callValue_invalid( + uint256 invalid_callValue + ) public { + vm.assume(invalid_callValue != callValue); vm.expectRevert( abi.encodeWithSelector( @@ -112,15 +109,20 @@ contract ValidateSessionParams is SMv2SessionValidationModuleTest { smv2SessionValidationModule.validateSessionParams( destinationContract, - callValue, + invalid_callValue, funcCallData, sessionKeyData, callSpecificData ); } - function test_validateSessionParams_funcCallData_invalid() public { - funcCallData = abi.encode(bytes4(""), bytes4("")); + function test_validateSessionParams_funcCallData_invalid( + bytes4 invalid_selector + ) public { + vm.assume(invalid_selector != IAccount.execute.selector); + + bytes memory invalid_funcCallData = + abi.encode(invalid_selector, bytes32("")); vm.expectRevert( abi.encodeWithSelector( @@ -131,53 +133,40 @@ contract ValidateSessionParams is SMv2SessionValidationModuleTest { smv2SessionValidationModule.validateSessionParams( destinationContract, callValue, - funcCallData, + invalid_funcCallData, sessionKeyData, callSpecificData ); } - function test_validateSessionParams_sessionKeyData_invalid() public { - address invalidSessionKey = address(0); - sessionKeyData = - abi.encode(invalidSessionKey, smv2ProxyAccount, smv2ExecuteSelector); + function test_validateSessionParams_sessionKeyData_invalid( + address invalid_sessionKey, + address invalid_destinationContract + ) public { + vm.assume(invalid_sessionKey != sessionKey); + + bytes memory invalid_sessionKeyData = + abi.encode(invalid_sessionKey, destinationContract); address retSessionKey = smv2SessionValidationModule .validateSessionParams( destinationContract, callValue, funcCallData, - sessionKeyData, + invalid_sessionKeyData, callSpecificData ); assertFalse(retSessionKey == sessionKey); - address invalidSmv2ProxyAccount = address(0); - sessionKeyData = - abi.encode(sessionKey, invalidSmv2ProxyAccount, smv2ExecuteSelector); + vm.assume(invalid_destinationContract != destinationContract); - vm.expectRevert( - abi.encodeWithSelector( - SMv2SessionValidationModule.InvalidDestinationContract.selector - ) - ); - - smv2SessionValidationModule.validateSessionParams( - destinationContract, - callValue, - funcCallData, - sessionKeyData, - callSpecificData - ); - - bytes4 invalidSmv2ExecuteSelector = bytes4(""); - sessionKeyData = - abi.encode(sessionKey, smv2ProxyAccount, invalidSmv2ExecuteSelector); + invalid_sessionKeyData = + abi.encode(sessionKey, invalid_destinationContract); vm.expectRevert( abi.encodeWithSelector( - SMv2SessionValidationModule.InvalidSMv2Selector.selector + SMv2SessionValidationModule.InvalidDestinationContract.selector ) ); @@ -185,7 +174,7 @@ contract ValidateSessionParams is SMv2SessionValidationModuleTest { destinationContract, callValue, funcCallData, - sessionKeyData, + invalid_sessionKeyData, callSpecificData ); } @@ -198,28 +187,18 @@ contract ValidateSessionUserOp is SMv2SessionValidationModuleTest { ); assertTrue(isValid); - - op.callData = abi.encodeWithSelector( - EXECUTE_OPTIMIZED_SELECTOR, - destinationContract, - callValue, - funcCallData - ); - userOpHash = userOpSignature.hashUserOperation(op); - sessionKeySignature = - userOpSignature.getUserOperationSignature(op, signerPrivateKey); - - isValid = smv2SessionValidationModule.validateSessionUserOp( - op, userOpHash, sessionKeyData, sessionKeySignature - ); - - assertTrue(isValid); } - function test_validateSessionUserOp_op_callData_invalid() public { - bytes4 invalidSelector = bytes4(""); + function test_validateSessionUserOp_op_callData_invalid( + bytes4 invalid_selector, + address invalid_destinationContract, + uint256 invalid_callValue + ) public { + vm.assume(invalid_selector != EXECUTE_SELECTOR); + vm.assume(invalid_selector != EXECUTE_OPTIMIZED_SELECTOR); + op.callData = abi.encodeWithSelector( - invalidSelector, smv2ProxyAccount, callValue, funcCallData + invalid_selector, destinationContract, callValue, funcCallData ); vm.expectRevert( @@ -232,9 +211,13 @@ contract ValidateSessionUserOp is SMv2SessionValidationModuleTest { op, userOpHash, sessionKeyData, sessionKeySignature ); - destinationContract = address(0); + vm.assume(invalid_destinationContract != destinationContract); + op.callData = abi.encodeWithSelector( - EXECUTE_SELECTOR, destinationContract, callValue, funcCallData + EXECUTE_SELECTOR, + invalid_destinationContract, + callValue, + funcCallData ); vm.expectRevert( @@ -247,10 +230,13 @@ contract ValidateSessionUserOp is SMv2SessionValidationModuleTest { op, userOpHash, sessionKeyData, sessionKeySignature ); - callValue = 1; - destinationContract = smv2ProxyAccount; + vm.assume(invalid_callValue != callValue); + op.callData = abi.encodeWithSelector( - EXECUTE_SELECTOR, destinationContract, callValue, funcCallData + EXECUTE_SELECTOR, + destinationContract, + invalid_callValue, + funcCallData ); vm.expectRevert( @@ -263,10 +249,16 @@ contract ValidateSessionUserOp is SMv2SessionValidationModuleTest { op, userOpHash, sessionKeyData, sessionKeySignature ); - callValue = 0; - funcCallData = abi.encode(bytes4(""), bytes4("")); + vm.assume(invalid_selector != IAccount.execute.selector); + + bytes memory invalid_funcCallData = + abi.encode(invalid_selector, bytes32("")); + op.callData = abi.encodeWithSelector( - EXECUTE_SELECTOR, destinationContract, callValue, funcCallData + EXECUTE_SELECTOR, + destinationContract, + callValue, + invalid_funcCallData ); vm.expectRevert( @@ -280,29 +272,36 @@ contract ValidateSessionUserOp is SMv2SessionValidationModuleTest { ); } - function test_validateSessionUserOp_userOpHash_invalid() public { - bytes32 invalidUserOpHash = bytes32(""); + function test_validateSessionUserOp_userOpHash_invalid( + bytes32 invalid_userOpHash + ) public { + vm.assume(invalid_userOpHash != userOpHash); + bool isValid = smv2SessionValidationModule.validateSessionUserOp( - op, invalidUserOpHash, sessionKeyData, sessionKeySignature + op, invalid_userOpHash, sessionKeyData, sessionKeySignature ); assertFalse(isValid); } - function test_validateSessionUserOp_sessionKeyData_invalid() public { - address invalidSessionKey = address(0); - sessionKeyData = - abi.encode(invalidSessionKey, smv2ProxyAccount, smv2ExecuteSelector); + function test_validateSessionUserOp_sessionKeyData_invalid( + address invalid_sessionKey, + address invalid_destinationContract + ) public { + vm.assume(invalid_sessionKey != sessionKey); + + bytes memory invalid_sessionKeyData = + abi.encode(invalid_sessionKey, destinationContract); bool isValid = smv2SessionValidationModule.validateSessionUserOp( - op, userOpHash, sessionKeyData, sessionKeySignature + op, userOpHash, invalid_sessionKeyData, sessionKeySignature ); assertFalse(isValid); - address invalidSmv2ProxyAccount = address(0); - sessionKeyData = - abi.encode(sessionKey, invalidSmv2ProxyAccount, smv2ExecuteSelector); + vm.assume(invalid_destinationContract != destinationContract); + + sessionKeyData = abi.encode(sessionKey, invalid_destinationContract); vm.expectRevert( abi.encodeWithSelector( @@ -313,25 +312,21 @@ contract ValidateSessionUserOp is SMv2SessionValidationModuleTest { smv2SessionValidationModule.validateSessionUserOp( op, userOpHash, sessionKeyData, sessionKeySignature ); + } - bytes4 invalidSmv2ExecuteSelector = bytes4(""); - sessionKeyData = - abi.encode(sessionKey, smv2ProxyAccount, invalidSmv2ExecuteSelector); + function test_validateSessionUserOp_sessionKeySignature_invalid( + uint256 invalid_privateKey + ) public { + // restrictions enforced by foundry + vm.assume(invalid_privateKey != 0); + vm.assume(invalid_privateKey < secp256k1_curve_order); - vm.expectRevert( - abi.encodeWithSelector( - SMv2SessionValidationModule.InvalidSMv2Selector.selector - ) - ); - - smv2SessionValidationModule.validateSessionUserOp( - op, userOpHash, sessionKeyData, sessionKeySignature - ); - } + // test specific + vm.assume(invalid_privateKey != signerPrivateKey); - function test_validateSessionUserOp_sessionKeySignature_invalid() public { bytes memory invalidSessionKeySignature = - userOpSignature.getUserOperationSignature(op, bad_signerPrivateKey); + userOpSignature.getUserOperationSignature(op, invalid_privateKey); + bool isValid = smv2SessionValidationModule.validateSessionUserOp( op, userOpHash, sessionKeyData, invalidSessionKeySignature ); diff --git a/test/SMv3SessionValidationModule.t.sol b/test/SMv3SessionValidationModule.t.sol index 643f6fc..d88f04f 100644 --- a/test/SMv3SessionValidationModule.t.sol +++ b/test/SMv3SessionValidationModule.t.sol @@ -15,8 +15,6 @@ import {IERC7412} from "src/kwenta/smv3/IERC7412.sol"; contract SMv3SessionValidationModuleTest is Bootstrap { address signer; uint256 signerPrivateKey; - address bad_signer; - uint256 bad_signerPrivateKey; address sessionKey; address smv3Engine; @@ -39,6 +37,8 @@ contract SMv3SessionValidationModuleTest is Bootstrap { bytes32 userOpHash; bytes data; + bytes4[] public validSelectors; + function setUp() public { initializeOptimismGoerli(); @@ -47,30 +47,18 @@ contract SMv3SessionValidationModuleTest is Bootstrap { // signers signerPrivateKey = 0x12341234; signer = vm.addr(signerPrivateKey); - bad_signerPrivateKey = 0x12341235; - bad_signer = vm.addr(bad_signerPrivateKey); // session key data sessionKey = signer; smv3Engine = address(0x2); - smv3ModifyCollateralSelector = IEngine.modifyCollateral.selector; - smv3CommitOrderSelector = IEngine.commitOrder.selector; - smv3InvalidateUnorderedNoncesSelector = - IEngine.invalidateUnorderedNonces.selector; - smv3FulfillOracleQuerySelector = IERC7412.fulfillOracleQuery.selector; // validateSessionParams params destinationContract = smv3Engine; callValue = 0; - funcCallData = abi.encode(smv3ModifyCollateralSelector, bytes32("")); - sessionKeyData = abi.encode( - sessionKey, - smv3Engine, - smv3ModifyCollateralSelector, - smv3CommitOrderSelector, - smv3InvalidateUnorderedNoncesSelector, - smv3FulfillOracleQuerySelector - ); + /// @notice a valid selector for IEngine + funcCallData = + abi.encode(IEngine.modifyCollateral.selector, bytes32("")); + sessionKeyData = abi.encode(sessionKey, smv3Engine); callSpecificData = ""; // validateSessionUserOp params @@ -80,60 +68,46 @@ contract SMv3SessionValidationModuleTest is Bootstrap { userOpHash = userOpSignature.hashUserOperation(op); sessionKeySignature = userOpSignature.getUserOperationSignature(op, signerPrivateKey); + + // define array of valid selectors + validSelectors.push(IEngine.modifyCollateral.selector); + validSelectors.push(IEngine.commitOrder.selector); + validSelectors.push(IEngine.invalidateUnorderedNonces.selector); + validSelectors.push(IERC7412.fulfillOracleQuery.selector); + validSelectors.push(IEngine.depositEth.selector); + validSelectors.push(IEngine.withdrawEth.selector); } } contract ValidateSessionParams is SMv3SessionValidationModuleTest { function test_validateSessionParams() public { - funcCallData = abi.encode(smv3ModifyCollateralSelector, bytes32("")); - address retSessionKey = smv3SessionValidationModule - .validateSessionParams( - destinationContract, - callValue, - funcCallData, - sessionKeyData, - callSpecificData - ); - - assertEq(sessionKey, retSessionKey); - - funcCallData = abi.encode(smv3CommitOrderSelector, bytes32("")); - retSessionKey = smv3SessionValidationModule.validateSessionParams( - destinationContract, - callValue, - funcCallData, - sessionKeyData, - callSpecificData - ); - - assertEq(sessionKey, retSessionKey); - - funcCallData = - abi.encode(smv3InvalidateUnorderedNoncesSelector, bytes32("")); - retSessionKey = smv3SessionValidationModule.validateSessionParams( - destinationContract, - callValue, - funcCallData, - sessionKeyData, - callSpecificData - ); - - assertEq(sessionKey, retSessionKey); - - funcCallData = abi.encode(smv3FulfillOracleQuerySelector, bytes32("")); - retSessionKey = smv3SessionValidationModule.validateSessionParams( - destinationContract, - callValue, - funcCallData, - sessionKeyData, - callSpecificData - ); - - assertEq(sessionKey, retSessionKey); + for (uint256 i; i < validSelectors.length; i++) { + // ensure each valid selector is accepted + funcCallData = abi.encode(validSelectors[i], bytes32("")); + + if (validSelectors[i] == IEngine.depositEth.selector) { + callValue = 1; // valid for depositEth + } else { + callValue = 0; // invalid for depositEth + } + + address retSessionKey = smv3SessionValidationModule + .validateSessionParams( + destinationContract, + callValue, + funcCallData, + sessionKeyData, + callSpecificData + ); + + assertEq(sessionKey, retSessionKey); + } } - function test_validateSessionParams_destinationContract_invalid() public { - destinationContract = address(0); + function test_validateSessionParams_destinationContract_invalid( + address invalid_destinationContract + ) public { + vm.assume(invalid_destinationContract != destinationContract); vm.expectRevert( abi.encodeWithSelector( @@ -142,7 +116,7 @@ contract ValidateSessionParams is SMv3SessionValidationModuleTest { ); smv3SessionValidationModule.validateSessionParams( - destinationContract, + invalid_destinationContract, callValue, funcCallData, sessionKeyData, @@ -150,75 +124,50 @@ contract ValidateSessionParams is SMv3SessionValidationModuleTest { ); } - function test_validateSessionParams_callValue_invalid() public { - callValue = 1; - - vm.expectRevert( - abi.encodeWithSelector( - SMv3SessionValidationModule.InvalidCallValue.selector - ) - ); - - smv3SessionValidationModule.validateSessionParams( - destinationContract, - callValue, - funcCallData, - sessionKeyData, - callSpecificData - ); + function test_validateSessionParams_callValue_invalid( + uint256 invalid_callValue + ) public { + vm.assume(invalid_callValue != callValue); + + for (uint256 i; i < validSelectors.length; i++) { + // ensure each valid selector is accepted + funcCallData = abi.encode(validSelectors[i], bytes32("")); + + if (validSelectors[i] == IEngine.depositEth.selector) { + callValue = 0; // i.e. invalid for depositEth + } else { + callValue = invalid_callValue; + } + + vm.expectRevert( + abi.encodeWithSelector( + SMv3SessionValidationModule.InvalidCallValue.selector + ) + ); + + smv3SessionValidationModule.validateSessionParams( + destinationContract, + callValue, // invalid + funcCallData, + sessionKeyData, + callSpecificData + ); + } } - function test_validateSessionParams_funcCallData_invalid() public { - funcCallData = abi.encodeWithSelector(bytes4(""), bytes32("")); - - vm.expectRevert( - abi.encodeWithSelector( - SMv3SessionValidationModule.InvalidSMv3Selector.selector - ) + function test_validateSessionParams_funcCallData_invalid( + bytes4 invalid_selector + ) public { + vm.assume(invalid_selector != IEngine.modifyCollateral.selector); + vm.assume(invalid_selector != IEngine.commitOrder.selector); + vm.assume( + invalid_selector != IEngine.invalidateUnorderedNonces.selector ); + vm.assume(invalid_selector != IERC7412.fulfillOracleQuery.selector); + vm.assume(invalid_selector != IEngine.depositEth.selector); + vm.assume(invalid_selector != IEngine.withdrawEth.selector); - smv3SessionValidationModule.validateSessionParams( - destinationContract, - callValue, - funcCallData, - sessionKeyData, - callSpecificData - ); - } - - function test_validateSessionParams_sessionKeyData_invalid() public { - sessionKeyData = abi.encode( - sessionKey, - address(0), - smv3ModifyCollateralSelector, - smv3CommitOrderSelector, - smv3InvalidateUnorderedNoncesSelector, - smv3FulfillOracleQuerySelector - ); - - vm.expectRevert( - abi.encodeWithSelector( - SMv3SessionValidationModule.InvalidDestinationContract.selector - ) - ); - - smv3SessionValidationModule.validateSessionParams( - destinationContract, - callValue, - funcCallData, - sessionKeyData, - callSpecificData - ); - - funcCallData = abi.encode(smv3ModifyCollateralSelector, bytes32("")); - sessionKeyData = abi.encode( - sessionKey, - smv3Engine, - bytes4(""), - smv3CommitOrderSelector, - smv3InvalidateUnorderedNoncesSelector, - smv3FulfillOracleQuerySelector - ); + funcCallData = abi.encodeWithSelector(bytes4(""), invalid_selector); vm.expectRevert( abi.encodeWithSelector( @@ -233,69 +182,36 @@ contract ValidateSessionParams is SMv3SessionValidationModuleTest { sessionKeyData, callSpecificData ); + } - funcCallData = abi.encode(smv3CommitOrderSelector, bytes32("")); - sessionKeyData = abi.encode( - sessionKey, - smv3Engine, - smv3ModifyCollateralSelector, - bytes4(""), - smv3InvalidateUnorderedNoncesSelector, - smv3FulfillOracleQuerySelector - ); + function test_validateSessionParams_sessionKeyData_invalid( + address invalid_sessionKey, + address invalid_destinationContract + ) public { + vm.assume(invalid_sessionKey != sessionKey); - vm.expectRevert( - abi.encodeWithSelector( - SMv3SessionValidationModule.InvalidSMv3Selector.selector - ) - ); + bytes memory invalid_sessionKeyData = + abi.encode(invalid_sessionKey, destinationContract); - smv3SessionValidationModule.validateSessionParams( + address retSessionKey = smv3SessionValidationModule + .validateSessionParams( destinationContract, callValue, funcCallData, - sessionKeyData, + invalid_sessionKeyData, callSpecificData ); - funcCallData = - abi.encode(smv3InvalidateUnorderedNoncesSelector, bytes32("")); - sessionKeyData = abi.encode( - sessionKey, - smv3Engine, - smv3ModifyCollateralSelector, - smv3CommitOrderSelector, - bytes4(""), - smv3FulfillOracleQuerySelector - ); + assertFalse(retSessionKey == sessionKey); - vm.expectRevert( - abi.encodeWithSelector( - SMv3SessionValidationModule.InvalidSMv3Selector.selector - ) - ); - - smv3SessionValidationModule.validateSessionParams( - destinationContract, - callValue, - funcCallData, - sessionKeyData, - callSpecificData - ); + vm.assume(invalid_destinationContract != destinationContract); - funcCallData = abi.encode(smv3FulfillOracleQuerySelector, bytes32("")); - sessionKeyData = abi.encode( - sessionKey, - smv3Engine, - smv3ModifyCollateralSelector, - smv3CommitOrderSelector, - smv3InvalidateUnorderedNoncesSelector, - bytes4("") - ); + invalid_sessionKeyData = + abi.encode(sessionKey, invalid_destinationContract); vm.expectRevert( abi.encodeWithSelector( - SMv3SessionValidationModule.InvalidSMv3Selector.selector + SMv3SessionValidationModule.InvalidDestinationContract.selector ) ); @@ -303,7 +219,7 @@ contract ValidateSessionParams is SMv3SessionValidationModuleTest { destinationContract, callValue, funcCallData, - sessionKeyData, + invalid_sessionKeyData, callSpecificData ); } @@ -311,79 +227,43 @@ contract ValidateSessionParams is SMv3SessionValidationModuleTest { contract ValidateSessionUserOp is SMv3SessionValidationModuleTest { function test_validateSessionUserOp() public { - funcCallData = abi.encode(smv3ModifyCollateralSelector, bytes32("")); - op.callData = abi.encodeWithSelector( - EXECUTE_SELECTOR, destinationContract, callValue, funcCallData - ); - userOpHash = userOpSignature.hashUserOperation(op); - sessionKeySignature = - userOpSignature.getUserOperationSignature(op, signerPrivateKey); - bool ret = smv3SessionValidationModule.validateSessionUserOp( - op, userOpHash, sessionKeyData, sessionKeySignature - ); + for (uint256 i; i < validSelectors.length; i++) { + // ensure each valid selector is accepted + funcCallData = abi.encode(validSelectors[i], bytes32("")); - assertTrue(ret); + if (validSelectors[i] == IEngine.depositEth.selector) { + callValue = 1; // valid for depositEth + } else { + callValue = 0; // invalid for depositEth + } - funcCallData = abi.encode(smv3CommitOrderSelector, bytes32("")); - op.callData = abi.encodeWithSelector( - EXECUTE_SELECTOR, destinationContract, callValue, funcCallData - ); - userOpHash = userOpSignature.hashUserOperation(op); - sessionKeySignature = - userOpSignature.getUserOperationSignature(op, signerPrivateKey); - ret = smv3SessionValidationModule.validateSessionUserOp( - op, userOpHash, sessionKeyData, sessionKeySignature - ); + op.callData = abi.encodeWithSelector( + EXECUTE_SELECTOR, destinationContract, callValue, funcCallData + ); - assertTrue(ret); + userOpHash = userOpSignature.hashUserOperation(op); - funcCallData = - abi.encode(smv3InvalidateUnorderedNoncesSelector, bytes32("")); - op.callData = abi.encodeWithSelector( - EXECUTE_SELECTOR, destinationContract, callValue, funcCallData - ); - userOpHash = userOpSignature.hashUserOperation(op); - sessionKeySignature = - userOpSignature.getUserOperationSignature(op, signerPrivateKey); - ret = smv3SessionValidationModule.validateSessionUserOp( - op, userOpHash, sessionKeyData, sessionKeySignature - ); - - assertTrue(ret); + sessionKeySignature = + userOpSignature.getUserOperationSignature(op, signerPrivateKey); - funcCallData = abi.encode(smv3FulfillOracleQuerySelector, bytes32("")); - op.callData = abi.encodeWithSelector( - EXECUTE_SELECTOR, destinationContract, callValue, funcCallData - ); - userOpHash = userOpSignature.hashUserOperation(op); - sessionKeySignature = - userOpSignature.getUserOperationSignature(op, signerPrivateKey); - ret = smv3SessionValidationModule.validateSessionUserOp( - op, userOpHash, sessionKeyData, sessionKeySignature - ); + bool ret = smv3SessionValidationModule.validateSessionUserOp( + op, userOpHash, sessionKeyData, sessionKeySignature + ); - assertTrue(ret); - - op.callData = abi.encodeWithSelector( - EXECUTE_OPTIMIZED_SELECTOR, - destinationContract, - callValue, - funcCallData - ); - userOpHash = userOpSignature.hashUserOperation(op); - sessionKeySignature = - userOpSignature.getUserOperationSignature(op, signerPrivateKey); - - ret = smv3SessionValidationModule.validateSessionUserOp( - op, userOpHash, sessionKeyData, sessionKeySignature - ); - - assertTrue(ret); + assertTrue(ret); + } } - function test_validateSessionUserOp_op_callData_invalid() public { + function test_validateSessionUserOp_op_callData_invalid( + bytes4 invalid_selector, + address invalid_destinationContract, + uint256 invalid_callValue + ) public { + vm.assume(invalid_selector != EXECUTE_SELECTOR); + vm.assume(invalid_selector != EXECUTE_OPTIMIZED_SELECTOR); + op.callData = abi.encodeWithSelector( - bytes4(""), destinationContract, callValue, funcCallData + invalid_selector, destinationContract, 1, funcCallData ); vm.expectRevert( @@ -396,8 +276,13 @@ contract ValidateSessionUserOp is SMv3SessionValidationModuleTest { op, userOpHash, sessionKeyData, sessionKeySignature ); + vm.assume(invalid_destinationContract != destinationContract); + op.callData = abi.encodeWithSelector( - EXECUTE_SELECTOR, address(0), callValue, funcCallData + EXECUTE_SELECTOR, + invalid_destinationContract, + callValue, + funcCallData ); vm.expectRevert( @@ -410,25 +295,53 @@ contract ValidateSessionUserOp is SMv3SessionValidationModuleTest { op, userOpHash, sessionKeyData, sessionKeySignature ); - op.callData = abi.encodeWithSelector( - EXECUTE_SELECTOR, destinationContract, 1, funcCallData - ); + vm.assume(invalid_callValue != callValue); - vm.expectRevert( - abi.encodeWithSelector( - SMv3SessionValidationModule.InvalidCallValue.selector - ) - ); + for (uint256 i; i < validSelectors.length; i++) { + // ensure each valid selector is accepted + funcCallData = abi.encode(validSelectors[i], bytes32("")); - smv3SessionValidationModule.validateSessionUserOp( - op, userOpHash, sessionKeyData, sessionKeySignature + if (validSelectors[i] == IEngine.depositEth.selector) { + callValue = 0; // i.e. invalid for depositEth + } else { + callValue = invalid_callValue; + } + + op.callData = abi.encodeWithSelector( + EXECUTE_SELECTOR, + destinationContract, + callValue, // invalid + funcCallData + ); + + vm.expectRevert( + abi.encodeWithSelector( + SMv3SessionValidationModule.InvalidCallValue.selector + ) + ); + + smv3SessionValidationModule.validateSessionUserOp( + op, userOpHash, sessionKeyData, sessionKeySignature + ); + } + + vm.assume(invalid_selector != IEngine.modifyCollateral.selector); + vm.assume(invalid_selector != IEngine.commitOrder.selector); + vm.assume( + invalid_selector != IEngine.invalidateUnorderedNonces.selector ); + vm.assume(invalid_selector != IERC7412.fulfillOracleQuery.selector); + vm.assume(invalid_selector != IEngine.depositEth.selector); + vm.assume(invalid_selector != IEngine.withdrawEth.selector); + + bytes memory invalid_funcCallData = + abi.encode(invalid_selector, bytes32("")); op.callData = abi.encodeWithSelector( EXECUTE_SELECTOR, destinationContract, callValue, - abi.encode(bytes4(""), bytes4("")) + invalid_funcCallData ); vm.expectRevert( @@ -442,164 +355,65 @@ contract ValidateSessionUserOp is SMv3SessionValidationModuleTest { ); } - function test_validateSessionUserOp_userOpHash_invalid() public { - bytes32 invalidUserOpHash = bytes32(""); - bool ret = smv3SessionValidationModule.validateSessionUserOp( - op, invalidUserOpHash, sessionKeyData, sessionKeySignature - ); - - assertFalse(ret); - } - - function test_validateSessionUserOp_sessionKeyData_invalid() public { - address invalidSessionKey = address(0); - sessionKeyData = abi.encode( - invalidSessionKey, - smv3Engine, - smv3ModifyCollateralSelector, - smv3CommitOrderSelector, - smv3InvalidateUnorderedNoncesSelector, - smv3FulfillOracleQuerySelector - ); + function test_validateSessionUserOp_userOpHash_invalid( + bytes32 invalid_userOpHash + ) public { + vm.assume(invalid_userOpHash != userOpHash); bool ret = smv3SessionValidationModule.validateSessionUserOp( - op, userOpHash, sessionKeyData, sessionKeySignature + op, invalid_userOpHash, sessionKeyData, sessionKeySignature ); assertFalse(ret); + } - sessionKeyData = abi.encode( - sessionKey, - address(0), - smv3ModifyCollateralSelector, - smv3CommitOrderSelector, - smv3InvalidateUnorderedNoncesSelector, - smv3FulfillOracleQuerySelector - ); - - vm.expectRevert( - abi.encodeWithSelector( - SMv3SessionValidationModule.InvalidDestinationContract.selector - ) - ); - - smv3SessionValidationModule.validateSessionUserOp( - op, userOpHash, sessionKeyData, sessionKeySignature - ); - - funcCallData = abi.encode(smv3ModifyCollateralSelector, bytes32("")); - op.callData = abi.encodeWithSelector( - EXECUTE_SELECTOR, destinationContract, callValue, funcCallData - ); - userOpHash = userOpSignature.hashUserOperation(op); - sessionKeySignature = - userOpSignature.getUserOperationSignature(op, signerPrivateKey); - sessionKeyData = abi.encode( - sessionKey, - smv3Engine, - bytes4(""), - smv3CommitOrderSelector, - smv3InvalidateUnorderedNoncesSelector, - smv3FulfillOracleQuerySelector - ); + function test_validateSessionUserOp_sessionKeyData_invalid( + address invalid_sessionKey, + address invalid_destinationContract + ) public { + vm.assume(invalid_sessionKey != sessionKey); - vm.expectRevert( - abi.encodeWithSelector( - SMv3SessionValidationModule.InvalidSMv3Selector.selector - ) - ); + bytes memory invalid_sessionKeyData = + abi.encode(invalid_sessionKey, smv3Engine); - smv3SessionValidationModule.validateSessionUserOp( - op, userOpHash, sessionKeyData, sessionKeySignature + bool isValid = smv3SessionValidationModule.validateSessionUserOp( + op, userOpHash, invalid_sessionKeyData, sessionKeySignature ); - funcCallData = abi.encode(smv3CommitOrderSelector, bytes32("")); - op.callData = abi.encodeWithSelector( - EXECUTE_SELECTOR, destinationContract, callValue, funcCallData - ); - userOpHash = userOpSignature.hashUserOperation(op); - sessionKeySignature = - userOpSignature.getUserOperationSignature(op, signerPrivateKey); - sessionKeyData = abi.encode( - sessionKey, - smv3Engine, - smv3ModifyCollateralSelector, - bytes4(""), - smv3InvalidateUnorderedNoncesSelector, - smv3FulfillOracleQuerySelector - ); + assertFalse(isValid); - vm.expectRevert( - abi.encodeWithSelector( - SMv3SessionValidationModule.InvalidSMv3Selector.selector - ) - ); + vm.assume(invalid_destinationContract != destinationContract); - smv3SessionValidationModule.validateSessionUserOp( - op, userOpHash, sessionKeyData, sessionKeySignature - ); - - funcCallData = - abi.encode(smv3InvalidateUnorderedNoncesSelector, bytes32("")); - op.callData = abi.encodeWithSelector( - EXECUTE_SELECTOR, destinationContract, callValue, funcCallData - ); - userOpHash = userOpSignature.hashUserOperation(op); - sessionKeySignature = - userOpSignature.getUserOperationSignature(op, signerPrivateKey); - sessionKeyData = abi.encode( - sessionKey, - smv3Engine, - smv3ModifyCollateralSelector, - smv3CommitOrderSelector, - bytes4(""), - smv3FulfillOracleQuerySelector - ); + sessionKeyData = abi.encode(sessionKey, invalid_destinationContract); vm.expectRevert( abi.encodeWithSelector( - SMv3SessionValidationModule.InvalidSMv3Selector.selector + SMv3SessionValidationModule.InvalidDestinationContract.selector ) ); smv3SessionValidationModule.validateSessionUserOp( op, userOpHash, sessionKeyData, sessionKeySignature ); + } - funcCallData = abi.encode(smv3FulfillOracleQuerySelector, bytes32("")); - op.callData = abi.encodeWithSelector( - EXECUTE_SELECTOR, destinationContract, callValue, funcCallData - ); - userOpHash = userOpSignature.hashUserOperation(op); - sessionKeySignature = - userOpSignature.getUserOperationSignature(op, signerPrivateKey); - sessionKeyData = abi.encode( - sessionKey, - smv3Engine, - smv3ModifyCollateralSelector, - smv3CommitOrderSelector, - smv3InvalidateUnorderedNoncesSelector, - bytes4("") - ); + function test_validateSessionUserOp_sessionKeySignature_invalid( + uint256 invalid_privateKey + ) public { + // restrictions enforced by foundry + vm.assume(invalid_privateKey != 0); + vm.assume(invalid_privateKey < secp256k1_curve_order); - vm.expectRevert( - abi.encodeWithSelector( - SMv3SessionValidationModule.InvalidSMv3Selector.selector - ) - ); + // test specific + vm.assume(invalid_privateKey != signerPrivateKey); - smv3SessionValidationModule.validateSessionUserOp( - op, userOpHash, sessionKeyData, sessionKeySignature - ); - } + bytes memory invalidSessionKeySignature = + userOpSignature.getUserOperationSignature(op, invalid_privateKey); - function test_validateSessionUserOp_sessionKeySignature_invalid() public { - sessionKeySignature = - userOpSignature.getUserOperationSignature(op, bad_signerPrivateKey); - bool ret = smv3SessionValidationModule.validateSessionUserOp( - op, userOpHash, sessionKeyData, sessionKeySignature + bool isValid = smv3SessionValidationModule.validateSessionUserOp( + op, userOpHash, sessionKeyData, invalidSessionKeySignature ); - assertFalse(ret); + assertFalse(isValid); } } diff --git a/test/utils/Bootstrap.sol b/test/utils/Bootstrap.sol index 12cb242..3b449e0 100644 --- a/test/utils/Bootstrap.sol +++ b/test/utils/Bootstrap.sol @@ -14,6 +14,9 @@ import {Test} from "lib/forge-std/src/Test.sol"; contract Bootstrap is Test { using console2 for *; + uint256 public secp256k1_curve_order = + 115_792_089_237_316_195_423_570_985_008_687_907_852_837_564_279_074_904_382_605_163_141_518_161_494_337; + SMv2SessionValidationModule public smv2SessionValidationModule; SMv3SessionValidationModule public smv3SessionValidationModule; From 4996246cae47b5eb1ccc3e1482f8127b5d046330 Mon Sep 17 00:00:00 2001 From: JaredBorders Date: Tue, 17 Oct 2023 16:00:04 -0400 Subject: [PATCH 05/11] =?UTF-8?q?=F0=9F=93=B8=20Update=20gas-snapshot/lcov?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .gas-snapshot | 40 +++--- lcov.info | 372 ++++++++++++++++++++++++++------------------------ 2 files changed, 213 insertions(+), 199 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index a6fc51b..f7e5880 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -1,20 +1,20 @@ -ValidateSessionParams:test_validateSessionParams() (gas: 31723) -ValidateSessionParams:test_validateSessionParams() (gas: 67203) -ValidateSessionParams:test_validateSessionParams_callValue_invalid() (gas: 52768) -ValidateSessionParams:test_validateSessionParams_callValue_invalid() (gas: 59733) -ValidateSessionParams:test_validateSessionParams_destinationContract_invalid() (gas: 30614) -ValidateSessionParams:test_validateSessionParams_destinationContract_invalid() (gas: 42281) -ValidateSessionParams:test_validateSessionParams_funcCallData_invalid() (gas: 32032) -ValidateSessionParams:test_validateSessionParams_funcCallData_invalid() (gas: 41891) -ValidateSessionParams:test_validateSessionParams_sessionKeyData_invalid() (gas: 50994) -ValidateSessionParams:test_validateSessionParams_sessionKeyData_invalid() (gas: 93348) -ValidateSessionUserOp:test_validateSessionUserOp() (gas: 137313) -ValidateSessionUserOp:test_validateSessionUserOp() (gas: 284073) -ValidateSessionUserOp:test_validateSessionUserOp_op_callData_invalid() (gas: 140834) -ValidateSessionUserOp:test_validateSessionUserOp_op_callData_invalid() (gas: 144764) -ValidateSessionUserOp:test_validateSessionUserOp_sessionKeyData_invalid() (gas: 106098) -ValidateSessionUserOp:test_validateSessionUserOp_sessionKeyData_invalid() (gas: 279589) -ValidateSessionUserOp:test_validateSessionUserOp_sessionKeySignature_invalid() (gas: 107963) -ValidateSessionUserOp:test_validateSessionUserOp_sessionKeySignature_invalid() (gas: 81852) -ValidateSessionUserOp:test_validateSessionUserOp_userOpHash_invalid() (gas: 71922) -ValidateSessionUserOp:test_validateSessionUserOp_userOpHash_invalid() (gas: 79475) \ No newline at end of file +ValidateSessionParams:test_validateSessionParams() (gas: 29489) +ValidateSessionParams:test_validateSessionParams() (gas: 76317) +ValidateSessionParams:test_validateSessionParams_callValue_invalid(uint256) (runs: 256, μ: 107033, ~: 107033) +ValidateSessionParams:test_validateSessionParams_callValue_invalid(uint256) (runs: 256, μ: 30859, ~: 30859) +ValidateSessionParams:test_validateSessionParams_destinationContract_invalid(address) (runs: 256, μ: 30633, ~: 30633) +ValidateSessionParams:test_validateSessionParams_destinationContract_invalid(address) (runs: 256, μ: 30641, ~: 30641) +ValidateSessionParams:test_validateSessionParams_funcCallData_invalid(bytes4) (runs: 256, μ: 24502, ~: 24502) +ValidateSessionParams:test_validateSessionParams_funcCallData_invalid(bytes4) (runs: 256, μ: 39796, ~: 39928) +ValidateSessionParams:test_validateSessionParams_sessionKeyData_invalid(address,address) (runs: 256, μ: 31515, ~: 31515) +ValidateSessionParams:test_validateSessionParams_sessionKeyData_invalid(address,address) (runs: 256, μ: 31633, ~: 31633) +ValidateSessionUserOp:test_validateSessionUserOp() (gas: 308778) +ValidateSessionUserOp:test_validateSessionUserOp() (gas: 71829) +ValidateSessionUserOp:test_validateSessionUserOp_op_callData_invalid(bytes4,address,uint256) (runs: 256, μ: 130244, ~: 130244) +ValidateSessionUserOp:test_validateSessionUserOp_op_callData_invalid(bytes4,address,uint256) (runs: 256, μ: 282649, ~: 285031) +ValidateSessionUserOp:test_validateSessionUserOp_sessionKeyData_invalid(address,address) (runs: 256, μ: 93063, ~: 93082) +ValidateSessionUserOp:test_validateSessionUserOp_sessionKeyData_invalid(address,address) (runs: 256, μ: 95260, ~: 95260) +ValidateSessionUserOp:test_validateSessionUserOp_sessionKeySignature_invalid(uint256) (runs: 256, μ: 82854, ~: 82854) +ValidateSessionUserOp:test_validateSessionUserOp_sessionKeySignature_invalid(uint256) (runs: 256, μ: 83024, ~: 83024) +ValidateSessionUserOp:test_validateSessionUserOp_userOpHash_invalid(bytes32) (runs: 256, μ: 74764, ~: 74764) +ValidateSessionUserOp:test_validateSessionUserOp_userOpHash_invalid(bytes32) (runs: 256, μ: 74934, ~: 74934) \ No newline at end of file diff --git a/lcov.info b/lcov.info index 722f290..78fdda0 100644 --- a/lcov.info +++ b/lcov.info @@ -1,36 +1,36 @@ TN: SF:script/Deploy.s.sol -FN:62,DeployOptimism.run -FNDA:0,DeployOptimism.run -DA:63,0 -DA:64,0 -DA:66,0 -DA:68,0 FN:77,DeployOptimismGoerli.run FNDA:0,DeployOptimismGoerli.run DA:78,0 DA:79,0 DA:81,0 DA:83,0 -FN:18,Setup.deploySystem -FNDA:0,Setup.deploySystem -DA:19,0 -DA:20,0 -DA:21,0 -DA:22,0 -DA:23,0 FN:34,DeployBase.run FNDA:0,DeployBase.run DA:35,0 DA:36,0 DA:38,0 DA:40,0 +FN:62,DeployOptimism.run +FNDA:0,DeployOptimism.run +DA:63,0 +DA:64,0 +DA:66,0 +DA:68,0 FN:48,DeployBaseGoerli.run FNDA:0,DeployBaseGoerli.run DA:49,0 DA:50,0 DA:52,0 DA:54,0 +FN:18,Setup.deploySystem +FNDA:0,Setup.deploySystem +DA:19,0 +DA:20,0 +DA:21,0 +DA:22,0 +DA:23,0 FNF:5 FNH:0 LF:21 @@ -41,50 +41,50 @@ end_of_record TN: SF:src/SMv2SessionValidationModule.sol FN:30,SMv2SessionValidationModule.validateSessionParams -FNDA:7,SMv2SessionValidationModule.validateSessionParams -DA:37,7 -DA:41,7 -DA:44,7 -BRDA:44,0,0,2 -BRDA:44,0,1,5 -DA:45,2 -DA:49,5 -BRDA:49,1,0,2 -BRDA:49,1,1,3 -DA:50,2 -DA:54,3 -BRDA:54,2,0,1 -BRDA:54,2,1,2 -DA:55,1 -DA:65,2 -FN:78,SMv2SessionValidationModule.validateSessionUserOp -FNDA:11,SMv2SessionValidationModule.validateSessionUserOp -DA:86,11 -DA:87,2 -BRDA:85,3,0,1 -BRDA:85,3,1,10 -DA:89,1 -DA:92,10 -DA:96,10 -DA:100,10 -DA:106,10 -BRDA:106,4,0,2 -BRDA:106,4,1,8 -DA:107,2 -DA:111,8 -BRDA:111,5,0,1 -BRDA:111,5,1,7 -DA:112,1 -DA:118,7 -DA:120,7 -DA:121,7 -DA:122,7 -DA:124,7 -DA:128,7 -BRDA:128,6,0,2 -BRDA:128,6,1,5 -DA:129,2 -DA:134,5 +FNDA:1281,SMv2SessionValidationModule.validateSessionParams +DA:37,1281 +DA:38,1281 +DA:41,1281 +BRDA:41,0,0,512 +BRDA:41,0,1,769 +DA:42,512 +DA:46,769 +BRDA:46,1,0,256 +BRDA:46,1,1,513 +DA:47,256 +DA:51,513 +BRDA:51,2,0,256 +BRDA:51,2,1,257 +DA:52,256 +DA:55,257 +FN:68,SMv2SessionValidationModule.validateSessionUserOp +FNDA:2049,SMv2SessionValidationModule.validateSessionUserOp +DA:76,2049 +DA:77,256 +BRDA:75,3,0,256 +BRDA:75,3,1,1793 +DA:79,256 +DA:82,1793 +DA:83,1793 +DA:85,1793 +DA:91,1793 +BRDA:91,4,0,512 +BRDA:91,4,1,1281 +DA:92,512 +DA:96,1281 +BRDA:96,5,0,256 +BRDA:96,5,1,1025 +DA:97,256 +DA:102,1025 +DA:104,1025 +DA:105,1025 +DA:106,1025 +DA:108,1025 +DA:112,1025 +BRDA:112,6,0,256 +BRDA:112,6,1,769 +DA:113,256 +DA:118,769 FNF:2 FNH:2 LF:27 @@ -94,66 +94,83 @@ BRH:14 end_of_record TN: SF:src/SMv3SessionValidationModule.sol -FN:35,SMv3SessionValidationModule.validateSessionParams -FNDA:12,SMv3SessionValidationModule.validateSessionParams -DA:42,12 -DA:49,12 -DA:54,12 -BRDA:54,0,0,2 -BRDA:54,0,1,10 -DA:55,2 -DA:59,10 -DA:61,10 -DA:62,8 -DA:63,7 -DA:64,6 -BRDA:60,1,0,5 -BRDA:60,1,1,5 -DA:66,5 -DA:70,5 -BRDA:70,2,0,1 -BRDA:70,2,1,4 -DA:71,1 -DA:74,4 -FN:87,SMv3SessionValidationModule.validateSessionUserOp -FNDA:17,SMv3SessionValidationModule.validateSessionUserOp -DA:95,17 -DA:96,2 -BRDA:94,3,0,1 -BRDA:94,3,1,16 -DA:98,1 -DA:101,16 -DA:106,16 -DA:112,16 -BRDA:112,4,0,2 -BRDA:112,4,1,14 -DA:113,2 -DA:117,14 -BRDA:117,5,0,1 -BRDA:117,5,1,13 -DA:118,1 -DA:124,13 -DA:126,13 -DA:127,13 -DA:128,13 -DA:129,13 -DA:133,13 -DA:140,13 -DA:146,13 -DA:148,13 -DA:149,9 -DA:150,8 -DA:151,7 -BRDA:147,6,0,5 -BRDA:147,6,1,8 -DA:153,5 -DA:159,8 +FN:31,SMv3SessionValidationModule.validateSessionParams +FNDA:2566,SMv3SessionValidationModule.validateSessionParams +DA:38,2566 +DA:39,2566 +DA:42,2566 +BRDA:42,0,0,512 +BRDA:42,0,1,2054 +DA:43,512 +DA:47,2054 +DA:49,2054 +DA:50,1541 +DA:51,1284 +DA:52,1027 +DA:53,770 +DA:54,513 +BRDA:48,1,0,256 +BRDA:48,1,1,1798 +DA:56,256 +DA:60,1798 +BRDA:60,2,0,256 +BRDA:60,2,1,1 +DA:61,257 +BRDA:61,3,0,256 +BRDA:61,3,1,1 +DA:62,256 +DA:64,1541 +BRDA:64,4,0,1280 +BRDA:64,4,1,261 +DA:65,1280 +DA:68,262 +FN:81,SMv3SessionValidationModule.validateSessionUserOp +FNDA:3334,SMv3SessionValidationModule.validateSessionUserOp +DA:89,3334 +DA:90,256 +BRDA:88,5,0,256 +BRDA:88,5,1,3078 +DA:92,256 +DA:95,3078 +DA:96,3078 +DA:98,3078 +DA:104,3078 +BRDA:104,6,0,512 +BRDA:104,6,1,2566 +DA:105,512 +DA:110,2566 +DA:112,2566 +DA:113,2566 +DA:114,2566 +DA:115,2566 +DA:119,2566 +DA:121,2566 +DA:122,1541 +DA:123,1284 +DA:124,1027 +DA:125,770 +DA:126,513 +BRDA:120,7,0,256 +BRDA:120,7,1,2310 +DA:128,256 +DA:132,2310 +BRDA:132,8,0,256 +BRDA:132,8,1,1 +DA:133,257 +BRDA:133,9,0,256 +BRDA:133,9,1,1 +DA:134,256 +DA:136,2053 +BRDA:136,10,0,1280 +BRDA:136,10,1,773 +DA:137,1280 +DA:142,774 FNF:2 FNH:2 -LF:36 -LH:36 -BRF:14 -BRH:14 +LF:45 +LH:45 +BRF:22 +BRH:22 end_of_record TN: SF:src/biconomy/interfaces/UserOperation.sol @@ -191,24 +208,24 @@ end_of_record TN: SF:src/openzeppelin/ECDSA.sol FN:56,ECDSA.tryRecover -FNDA:13,ECDSA.tryRecover -DA:61,13 -BRDA:61,0,0,13 +FNDA:1543,ECDSA.tryRecover +DA:61,1543 +BRDA:61,0,0,1543 BRDA:61,0,1,- -DA:62,13 -DA:63,13 -DA:64,13 -DA:69,13 -DA:70,13 -DA:71,13 -DA:73,13 +DA:62,1543 +DA:63,1543 +DA:64,1543 +DA:69,1543 +DA:70,1543 +DA:71,1543 +DA:73,1543 DA:75,0 FN:97,ECDSA.recover -FNDA:13,ECDSA.recover -DA:102,13 -DA:103,13 -DA:104,13 -DA:105,13 +FNDA:1543,ECDSA.recover +DA:102,1543 +DA:103,1543 +DA:104,1543 +DA:105,1543 FN:113,ECDSA.tryRecover FNDA:0,ECDSA.tryRecover DA:119,0 @@ -222,17 +239,17 @@ DA:138,0 DA:139,0 DA:140,0 FN:147,ECDSA.tryRecover -FNDA:13,ECDSA.tryRecover -DA:162,13 +FNDA:1543,ECDSA.tryRecover +DA:162,1543 BRDA:161,1,0,- -BRDA:161,1,1,13 +BRDA:161,1,1,1543 DA:165,0 -DA:169,13 -DA:170,13 +DA:169,1543 +DA:170,1543 BRDA:170,2,0,- -BRDA:170,2,1,13 +BRDA:170,2,1,1543 DA:171,0 -DA:174,13 +DA:174,1543 FN:181,ECDSA.recover FNDA:0,ECDSA.recover DA:186,0 @@ -240,11 +257,11 @@ DA:187,0 DA:188,0 DA:189,0 FN:195,ECDSA._throwError -FNDA:13,ECDSA._throwError -DA:196,13 +FNDA:1543,ECDSA._throwError +DA:196,1543 BRDA:196,3,0,- -BRDA:196,3,1,13 -DA:197,13 +BRDA:196,3,1,1543 +DA:197,1543 DA:198,0 BRDA:198,4,0,- BRDA:198,4,1,- @@ -258,8 +275,8 @@ BRDA:202,6,0,- BRDA:202,6,1,- DA:203,0 FN:212,ECDSA.toEthSignedMessageHash -FNDA:13,ECDSA.toEthSignedMessageHash -DA:219,13 +FNDA:1543,ECDSA.toEthSignedMessageHash +DA:219,1543 FNF:8 FNH:5 LF:40 @@ -269,28 +286,25 @@ BRH:4 end_of_record TN: SF:test/SMv2SessionValidationModule.t.sol -FN:38,SMv2SessionValidationModuleTest.setUp +FN:35,SMv2SessionValidationModuleTest.setUp FNDA:0,SMv2SessionValidationModuleTest.setUp -DA:39,0 +DA:36,0 +DA:38,0 DA:41,0 -DA:44,0 +DA:42,0 DA:45,0 DA:46,0 -DA:47,0 +DA:49,0 DA:50,0 DA:51,0 DA:52,0 -DA:55,0 +DA:53,0 DA:56,0 -DA:57,0 -DA:58,0 +DA:59,0 DA:60,0 -DA:63,0 -DA:66,0 -DA:67,0 FNF:1 FNH:0 -LF:17 +LF:14 LH:0 BRF:0 BRH:0 @@ -303,22 +317,22 @@ DA:43,0 DA:45,0 DA:48,0 DA:49,0 -DA:50,0 -DA:51,0 -DA:54,0 -DA:55,0 +DA:52,0 +DA:53,0 DA:56,0 DA:57,0 -DA:58,0 -DA:60,0 -DA:63,0 -DA:64,0 +DA:59,0 +DA:61,0 +DA:62,0 DA:65,0 -DA:66,0 +DA:68,0 +DA:69,0 +DA:73,0 DA:74,0 +DA:75,0 +DA:76,0 DA:77,0 -DA:80,0 -DA:81,0 +DA:78,0 FNF:1 FNH:0 LF:20 @@ -328,18 +342,18 @@ BRH:0 end_of_record TN: SF:test/utils/Bootstrap.sol -FN:20,Bootstrap.initializeOptimismGoerli +FN:23,Bootstrap.initializeOptimismGoerli FNDA:0,Bootstrap.initializeOptimismGoerli -DA:21,0 -DA:22,0 +DA:24,0 DA:25,0 -DA:27,0 -DA:29,0 -FN:37,BootstrapOptimismGoerli.init +DA:28,0 +DA:30,0 +DA:32,0 +FN:40,BootstrapOptimismGoerli.init FNDA:0,BootstrapOptimismGoerli.init -DA:38,0 DA:41,0 -DA:43,0 +DA:44,0 +DA:46,0 FNF:2 FNH:0 LF:8 @@ -350,13 +364,13 @@ end_of_record TN: SF:test/utils/UserOperationSignature.sol FN:18,UserOperationSignature.getUserOperationSignature -FNDA:12,UserOperationSignature.getUserOperationSignature -DA:22,12 -DA:24,12 -DA:26,12 +FNDA:518,UserOperationSignature.getUserOperationSignature +DA:22,518 +DA:24,518 +DA:26,518 FN:29,UserOperationSignature.hashUserOperation -FNDA:10,UserOperationSignature.hashUserOperation -DA:34,22 +FNDA:6,UserOperationSignature.hashUserOperation +DA:34,524 FNF:2 FNH:2 LF:4 From 6549bfc3c57f8078be3d87cdbca4fa323daecc6e Mon Sep 17 00:00:00 2001 From: JaredBorders Date: Tue, 17 Oct 2023 16:18:21 -0400 Subject: [PATCH 06/11] forge install: scw-contracts v1.0.3 --- .gitmodules | 3 +++ lib/scw-contracts | 1 + 2 files changed, 4 insertions(+) create mode 160000 lib/scw-contracts diff --git a/.gitmodules b/.gitmodules index 888d42d..f26d09f 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,3 +1,6 @@ [submodule "lib/forge-std"] path = lib/forge-std url = https://github.com/foundry-rs/forge-std +[submodule "lib/scw-contracts"] + path = lib/scw-contracts + url = https://github.com/bcnmy/scw-contracts diff --git a/lib/scw-contracts b/lib/scw-contracts new file mode 160000 index 0000000..52a84cc --- /dev/null +++ b/lib/scw-contracts @@ -0,0 +1 @@ +Subproject commit 52a84cc64e1aa103539370c01f5724cdafcf71ff From c2eccd0c5680a4d7c0743ae8df27da1945ad6dc2 Mon Sep 17 00:00:00 2001 From: JaredBorders Date: Tue, 17 Oct 2023 16:47:25 -0400 Subject: [PATCH 07/11] =?UTF-8?q?=F0=9F=97=91=EF=B8=8F=20Remove=20import?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .gitmodules | 3 --- lib/scw-contracts | 1 - 2 files changed, 4 deletions(-) delete mode 160000 lib/scw-contracts diff --git a/.gitmodules b/.gitmodules index f26d09f..888d42d 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,6 +1,3 @@ [submodule "lib/forge-std"] path = lib/forge-std url = https://github.com/foundry-rs/forge-std -[submodule "lib/scw-contracts"] - path = lib/scw-contracts - url = https://github.com/bcnmy/scw-contracts diff --git a/lib/scw-contracts b/lib/scw-contracts deleted file mode 160000 index 52a84cc..0000000 --- a/lib/scw-contracts +++ /dev/null @@ -1 +0,0 @@ -Subproject commit 52a84cc64e1aa103539370c01f5724cdafcf71ff From b9a5ca43c60be43cf37d1acaa782af525c4e7af6 Mon Sep 17 00:00:00 2001 From: JaredBorders Date: Tue, 17 Oct 2023 21:26:12 -0400 Subject: [PATCH 08/11] =?UTF-8?q?=F0=9F=9A=80=20Deploy=20Modules?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- deployments/Base.json | 3 ++- deployments/BaseGoerli.json | 3 ++- deployments/Optimism.json | 3 ++- deployments/OptimismGoerli.json | 4 ++-- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/deployments/Base.json b/deployments/Base.json index dcfb5af..dc249d4 100644 --- a/deployments/Base.json +++ b/deployments/Base.json @@ -1,3 +1,4 @@ { - "SMv2SessionValidationModule": "" + "SMv2SessionValidationModule": "0xaD873e5E79df7F7a7fFE637EdaCcD5A3640B4a49", + "SMv3SessionValidationModule": "0x3e52b5f840eafD79394c6359E93Bf3FfdAE89ee4" } \ No newline at end of file diff --git a/deployments/BaseGoerli.json b/deployments/BaseGoerli.json index dcfb5af..b2c2811 100644 --- a/deployments/BaseGoerli.json +++ b/deployments/BaseGoerli.json @@ -1,3 +1,4 @@ { - "SMv2SessionValidationModule": "" + "SMv2SessionValidationModule": "0xf88D3220a7AB9AC75aE18A6c158557a8CcaBa691", + "SMv3SessionValidationModule": "0xa5Aac6b5De821E631C7Ad01f978e32e80a8461c7" } \ No newline at end of file diff --git a/deployments/Optimism.json b/deployments/Optimism.json index dcfb5af..1572689 100644 --- a/deployments/Optimism.json +++ b/deployments/Optimism.json @@ -1,3 +1,4 @@ { - "SMv2SessionValidationModule": "" + "SMv2SessionValidationModule": "0xfc026f2230C55DC8BDE3bD9bE8941fbDCA6B39C2", + "SMv3SessionValidationModule": "0xEfc2aAE2eBABcD242b210AACc6A121078714bF26" } \ No newline at end of file diff --git a/deployments/OptimismGoerli.json b/deployments/OptimismGoerli.json index 1bd53cb..a422c19 100644 --- a/deployments/OptimismGoerli.json +++ b/deployments/OptimismGoerli.json @@ -1,4 +1,4 @@ { - "SMv2SessionValidationModule": "0x38Fec4DccA924e6dF408083bd18fb82aff9a838D", - "SMv3SessionValidationModule": "0x7b1d4aF0b8FA4D9860aa3690569220fAa04DeD44" + "SMv2SessionValidationModule": "0x88d5A3EaCeeFB1Ca054a0EaDb788F6BE77a579fC", + "SMv3SessionValidationModule": "0xf82191044a40a92a4d1A2Cad5F6Bc8e527Fc520A" } \ No newline at end of file From 740f28340285500382d262ea397660bf19682bf0 Mon Sep 17 00:00:00 2001 From: JaredBorders Date: Wed, 18 Oct 2023 18:30:13 -0400 Subject: [PATCH 09/11] =?UTF-8?q?=F0=9F=97=91=EF=B8=8F=20Remove=20callValu?= =?UTF-8?q?e=20check?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/SMv2SessionValidationModule.sol | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/SMv2SessionValidationModule.sol b/src/SMv2SessionValidationModule.sol index d554174..9ca9c2c 100644 --- a/src/SMv2SessionValidationModule.sol +++ b/src/SMv2SessionValidationModule.sol @@ -23,13 +23,12 @@ contract SMv2SessionValidationModule is ISessionValidationModule { * @dev validates that the call (destinationContract, callValue, funcCallData) * complies with the Session Key permissions represented by sessionKeyData * @param destinationContract address of the contract to be called - * @param callValue value to be sent with the call * @param _funcCallData the data for the call. is parsed inside the SVM * @param _sessionKeyData SessionKey data, that describes sessionKey permissions */ function validateSessionParams( address destinationContract, - uint256 callValue, + uint256, /*callValue*/ bytes calldata _funcCallData, bytes calldata _sessionKeyData, bytes calldata /*_callSpecificData*/ @@ -47,11 +46,6 @@ contract SMv2SessionValidationModule is ISessionValidationModule { revert InvalidSMv2Selector(); } - /// @dev ensure call value is zero - if (callValue != 0) { - revert InvalidCallValue(); - } - return sessionKey; } From ff0c89a281a7a81028a0907ff3fb4ea05e1af1d8 Mon Sep 17 00:00:00 2001 From: JaredBorders Date: Wed, 18 Oct 2023 18:30:25 -0400 Subject: [PATCH 10/11] =?UTF-8?q?=F0=9F=97=91=EF=B8=8F=20Remove=20test?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- test/SMv2SessionValidationModule.t.sol | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/test/SMv2SessionValidationModule.t.sol b/test/SMv2SessionValidationModule.t.sol index b393a7f..863dc66 100644 --- a/test/SMv2SessionValidationModule.t.sol +++ b/test/SMv2SessionValidationModule.t.sol @@ -96,26 +96,6 @@ contract ValidateSessionParams is SMv2SessionValidationModuleTest { ); } - function test_validateSessionParams_callValue_invalid( - uint256 invalid_callValue - ) public { - vm.assume(invalid_callValue != callValue); - - vm.expectRevert( - abi.encodeWithSelector( - SMv2SessionValidationModule.InvalidCallValue.selector - ) - ); - - smv2SessionValidationModule.validateSessionParams( - destinationContract, - invalid_callValue, - funcCallData, - sessionKeyData, - callSpecificData - ); - } - function test_validateSessionParams_funcCallData_invalid( bytes4 invalid_selector ) public { From 65f06ddbb055fff42bc886543dd95bdb07149ac0 Mon Sep 17 00:00:00 2001 From: JaredBorders Date: Wed, 18 Oct 2023 18:31:34 -0400 Subject: [PATCH 11/11] =?UTF-8?q?=F0=9F=93=B8=20Update=20gas-snapshot/lcov?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .gas-snapshot | 11 +++-- lcov.info | 110 ++++++++++++++++++++++++-------------------------- 2 files changed, 58 insertions(+), 63 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index f7e5880..ff5f8ed 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -1,19 +1,18 @@ -ValidateSessionParams:test_validateSessionParams() (gas: 29489) +ValidateSessionParams:test_validateSessionParams() (gas: 29469) ValidateSessionParams:test_validateSessionParams() (gas: 76317) ValidateSessionParams:test_validateSessionParams_callValue_invalid(uint256) (runs: 256, μ: 107033, ~: 107033) -ValidateSessionParams:test_validateSessionParams_callValue_invalid(uint256) (runs: 256, μ: 30859, ~: 30859) ValidateSessionParams:test_validateSessionParams_destinationContract_invalid(address) (runs: 256, μ: 30633, ~: 30633) ValidateSessionParams:test_validateSessionParams_destinationContract_invalid(address) (runs: 256, μ: 30641, ~: 30641) ValidateSessionParams:test_validateSessionParams_funcCallData_invalid(bytes4) (runs: 256, μ: 24502, ~: 24502) -ValidateSessionParams:test_validateSessionParams_funcCallData_invalid(bytes4) (runs: 256, μ: 39796, ~: 39928) -ValidateSessionParams:test_validateSessionParams_sessionKeyData_invalid(address,address) (runs: 256, μ: 31515, ~: 31515) +ValidateSessionParams:test_validateSessionParams_funcCallData_invalid(bytes4) (runs: 256, μ: 39815, ~: 39928) +ValidateSessionParams:test_validateSessionParams_sessionKeyData_invalid(address,address) (runs: 256, μ: 31495, ~: 31495) ValidateSessionParams:test_validateSessionParams_sessionKeyData_invalid(address,address) (runs: 256, μ: 31633, ~: 31633) ValidateSessionUserOp:test_validateSessionUserOp() (gas: 308778) ValidateSessionUserOp:test_validateSessionUserOp() (gas: 71829) ValidateSessionUserOp:test_validateSessionUserOp_op_callData_invalid(bytes4,address,uint256) (runs: 256, μ: 130244, ~: 130244) -ValidateSessionUserOp:test_validateSessionUserOp_op_callData_invalid(bytes4,address,uint256) (runs: 256, μ: 282649, ~: 285031) +ValidateSessionUserOp:test_validateSessionUserOp_op_callData_invalid(bytes4,address,uint256) (runs: 256, μ: 282952, ~: 282231) ValidateSessionUserOp:test_validateSessionUserOp_sessionKeyData_invalid(address,address) (runs: 256, μ: 93063, ~: 93082) -ValidateSessionUserOp:test_validateSessionUserOp_sessionKeyData_invalid(address,address) (runs: 256, μ: 95260, ~: 95260) +ValidateSessionUserOp:test_validateSessionUserOp_sessionKeyData_invalid(address,address) (runs: 256, μ: 95222, ~: 95260) ValidateSessionUserOp:test_validateSessionUserOp_sessionKeySignature_invalid(uint256) (runs: 256, μ: 82854, ~: 82854) ValidateSessionUserOp:test_validateSessionUserOp_sessionKeySignature_invalid(uint256) (runs: 256, μ: 83024, ~: 83024) ValidateSessionUserOp:test_validateSessionUserOp_userOpHash_invalid(bytes32) (runs: 256, μ: 74764, ~: 74764) diff --git a/lcov.info b/lcov.info index 78fdda0..d47961c 100644 --- a/lcov.info +++ b/lcov.info @@ -1,17 +1,5 @@ TN: SF:script/Deploy.s.sol -FN:77,DeployOptimismGoerli.run -FNDA:0,DeployOptimismGoerli.run -DA:78,0 -DA:79,0 -DA:81,0 -DA:83,0 -FN:34,DeployBase.run -FNDA:0,DeployBase.run -DA:35,0 -DA:36,0 -DA:38,0 -DA:40,0 FN:62,DeployOptimism.run FNDA:0,DeployOptimism.run DA:63,0 @@ -24,6 +12,12 @@ DA:49,0 DA:50,0 DA:52,0 DA:54,0 +FN:34,DeployBase.run +FNDA:0,DeployBase.run +DA:35,0 +DA:36,0 +DA:38,0 +DA:40,0 FN:18,Setup.deploySystem FNDA:0,Setup.deploySystem DA:19,0 @@ -31,6 +25,12 @@ DA:20,0 DA:21,0 DA:22,0 DA:23,0 +FN:77,DeployOptimismGoerli.run +FNDA:0,DeployOptimismGoerli.run +DA:78,0 +DA:79,0 +DA:81,0 +DA:83,0 FNF:5 FNH:0 LF:21 @@ -40,57 +40,53 @@ BRH:0 end_of_record TN: SF:src/SMv2SessionValidationModule.sol -FN:30,SMv2SessionValidationModule.validateSessionParams -FNDA:1281,SMv2SessionValidationModule.validateSessionParams -DA:37,1281 -DA:38,1281 -DA:41,1281 -BRDA:41,0,0,512 -BRDA:41,0,1,769 -DA:42,512 -DA:46,769 -BRDA:46,1,0,256 -BRDA:46,1,1,513 -DA:47,256 -DA:51,513 -BRDA:51,2,0,256 -BRDA:51,2,1,257 -DA:52,256 -DA:55,257 -FN:68,SMv2SessionValidationModule.validateSessionUserOp +FN:29,SMv2SessionValidationModule.validateSessionParams +FNDA:1025,SMv2SessionValidationModule.validateSessionParams +DA:36,1025 +DA:37,1025 +DA:40,1025 +BRDA:40,0,0,512 +BRDA:40,0,1,513 +DA:41,512 +DA:45,513 +BRDA:45,1,0,256 +BRDA:45,1,1,257 +DA:46,256 +DA:49,257 +FN:62,SMv2SessionValidationModule.validateSessionUserOp FNDA:2049,SMv2SessionValidationModule.validateSessionUserOp -DA:76,2049 -DA:77,256 -BRDA:75,3,0,256 -BRDA:75,3,1,1793 -DA:79,256 -DA:82,1793 -DA:83,1793 +DA:70,2049 +DA:71,256 +BRDA:69,2,0,256 +BRDA:69,2,1,1793 +DA:73,256 +DA:76,1793 +DA:77,1793 +DA:79,1793 DA:85,1793 -DA:91,1793 -BRDA:91,4,0,512 -BRDA:91,4,1,1281 -DA:92,512 -DA:96,1281 -BRDA:96,5,0,256 -BRDA:96,5,1,1025 -DA:97,256 +BRDA:85,3,0,512 +BRDA:85,3,1,1281 +DA:86,512 +DA:90,1281 +BRDA:90,4,0,256 +BRDA:90,4,1,1025 +DA:91,256 +DA:96,1025 +DA:98,1025 +DA:99,1025 +DA:100,1025 DA:102,1025 -DA:104,1025 -DA:105,1025 DA:106,1025 -DA:108,1025 -DA:112,1025 -BRDA:112,6,0,256 -BRDA:112,6,1,769 -DA:113,256 -DA:118,769 +BRDA:106,5,0,256 +BRDA:106,5,1,769 +DA:107,256 +DA:112,769 FNF:2 FNH:2 -LF:27 -LH:27 -BRF:14 -BRH:14 +LF:25 +LH:25 +BRF:12 +BRH:12 end_of_record TN: SF:src/SMv3SessionValidationModule.sol