diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5531e1bc..19baefc6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -161,7 +161,7 @@ jobs: uses: crytic/slither-action@v0.4.0 id: slither with: - slither-version: "0.10.0" + slither-version: "0.10.1" node-version: "22" fail-on: "none" slither-args: '--solc-args="--evm-version cancun" --exclude "assembly|solc-version|low-level-calls|naming-convention|controlled-delegatecall|write-after-write|divide-before-multiply|incorrect-shift" --exclude-informational --exclude-low --filter-paths "contracts/mock|node_modules" --checklist --markdown-root ${{ github.server_url }}/${{ github.repository }}/blob/${{ github.sha }}/contracts/' diff --git a/CHANGELOG.md b/CHANGELOG.md index faa1dd20..8d1cc373 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ## [Unreleased] +## [1.0.0] - 2024-10-14 +Code Freeze post audit remediations. + ## [1.0.0-beta.1] - 2024-09-30 ## [1.0.0-beta] - 2024-06-04 diff --git a/audits/report-cantinacode-biconomy-0708-final.pdf b/audits/report-cantinacode-biconomy-0708-final.pdf new file mode 100644 index 00000000..efaf1c93 Binary files /dev/null and b/audits/report-cantinacode-biconomy-0708-final.pdf differ diff --git a/contracts/Nexus.sol b/contracts/Nexus.sol index df41b654..c6e83e75 100644 --- a/contracts/Nexus.sol +++ b/contracts/Nexus.sol @@ -343,6 +343,6 @@ contract Nexus is INexus, BaseAccount, ExecutionHelper, ModuleManager, UUPSUpgra /// @dev EIP712 domain name and version. function _domainNameAndVersion() internal pure override returns (string memory name, string memory version) { name = "Nexus"; - version = "1.0.0-beta.1"; + version = "1.0.0"; } } diff --git a/contracts/base/BaseAccount.sol b/contracts/base/BaseAccount.sol index b93828f8..1f15e989 100644 --- a/contracts/base/BaseAccount.sol +++ b/contracts/base/BaseAccount.sol @@ -25,7 +25,7 @@ import { IBaseAccount } from "../interfaces/base/IBaseAccount.sol"; /// Special thanks to the Solady team for foundational contributions: https://github.com/Vectorized/solady contract BaseAccount is IBaseAccount { /// @notice Identifier for this implementation on the network - string internal constant _ACCOUNT_IMPLEMENTATION_ID = "biconomy.nexus.1.0.0-beta.1"; + string internal constant _ACCOUNT_IMPLEMENTATION_ID = "biconomy.nexus.1.0.0"; /// @notice The canonical address for the ERC4337 EntryPoint contract, version 0.7. /// This address is consistent across all supported networks. diff --git a/contracts/base/ExecutionHelper.sol b/contracts/base/ExecutionHelper.sol index 74cc605e..5158b16b 100644 --- a/contracts/base/ExecutionHelper.sol +++ b/contracts/base/ExecutionHelper.sol @@ -31,7 +31,7 @@ contract ExecutionHelper is IExecutionHelperEventsAndErrors { using ExecLib for bytes; /// @notice Executes a call to a target address with specified value and data. - /// @dev calls to an EOA should be counted as successful. + /// @notice calls to an EOA should be counted as successful. /// @param target The address to execute the call on. /// @param value The amount of wei to send with the call. /// @param callData The calldata to send. @@ -53,9 +53,25 @@ contract ExecutionHelper is IExecutionHelperEventsAndErrors { } } + /// @notice Executes a call to a target address with specified value and data. + /// Same as _execute but without return data for gas optimization. + function _executeNoReturndata(address target, uint256 value, bytes calldata callData) internal virtual { + /// @solidity memory-safe-assembly + assembly { + let result := mload(0x40) + calldatacopy(result, callData.offset, callData.length) + if iszero(call(gas(), target, value, result, callData.length, codesize(), 0x00)) { + // Bubble up the revert if the call reverts. + returndatacopy(result, 0x00, returndatasize()) + revert(result, returndatasize()) + } + mstore(0x40, add(result, callData.length)) //allocate memory + } + } + /// @notice Tries to execute a call and captures if it was successful or not. /// @dev Similar to _execute but returns a success boolean and catches reverts instead of propagating them. - /// @dev calls to an EOA should be counted as successful. + /// @notice calls to an EOA should be counted as successful. /// @param target The address to execute the call on. /// @param value The amount of wei to send with the call. /// @param callData The calldata to send. @@ -87,6 +103,16 @@ contract ExecutionHelper is IExecutionHelperEventsAndErrors { } } + /// @notice Executes a batch of calls without returning the result. + /// @param executions An array of Execution structs each containing target, value, and calldata. + function _executeBatchNoReturndata(Execution[] calldata executions) internal { + Execution calldata exec; + for (uint256 i; i < executions.length; i++) { + exec = executions[i]; + _executeNoReturndata(exec.target, exec.value, exec.callData); + } + } + /// @notice Tries to execute a batch of calls and emits an event for each unsuccessful call. /// @param executions An array of Execution structs. /// @return result An array of bytes returned from each executed call, with unsuccessful calls marked by events. @@ -122,6 +148,22 @@ contract ExecutionHelper is IExecutionHelperEventsAndErrors { } } + /// @dev Execute a delegatecall with `delegate` on this account. + /// Same as _executeDelegatecall but without return data for gas optimization. + function _executeDelegatecallNoReturndata(address delegate, bytes calldata callData) internal { + /// @solidity memory-safe-assembly + assembly { + let result := mload(0x40) + calldatacopy(result, callData.offset, callData.length) + if iszero(delegatecall(gas(), delegate, result, callData.length, codesize(), 0x00)) { + // Bubble up the revert if the call reverts. + returndatacopy(result, 0x00, returndatasize()) + revert(result, returndatasize()) + } + mstore(0x40, add(result, callData.length)) //allocate memory + } + } + /// @dev Execute a delegatecall with `delegate` on this account and catch reverts. /// @return success True if the delegatecall was successful, false otherwise. /// @return result The bytes returned from the delegatecall, which contains the returned data from the delegate contract. @@ -144,7 +186,7 @@ contract ExecutionHelper is IExecutionHelperEventsAndErrors { /// @param execType The execution type, which can be DEFAULT (revert on failure) or TRY (return on failure). function _handleSingleExecution(bytes calldata executionCalldata, ExecType execType) internal { (address target, uint256 value, bytes calldata callData) = executionCalldata.decodeSingle(); - if (execType == EXECTYPE_DEFAULT) _execute(target, value, callData); + if (execType == EXECTYPE_DEFAULT) _executeNoReturndata(target, value, callData); else if (execType == EXECTYPE_TRY) { (bool success, bytes memory result) = _tryExecute(target, value, callData); if (!success) emit TryExecuteUnsuccessful(callData, result); @@ -156,7 +198,7 @@ contract ExecutionHelper is IExecutionHelperEventsAndErrors { /// @param execType The execution type, which can be DEFAULT (revert on failure) or TRY (return on failure). function _handleBatchExecution(bytes calldata executionCalldata, ExecType execType) internal { Execution[] calldata executions = executionCalldata.decodeBatch(); - if (execType == EXECTYPE_DEFAULT) _executeBatch(executions); + if (execType == EXECTYPE_DEFAULT) _executeBatchNoReturndata(executions); else if (execType == EXECTYPE_TRY) _tryExecuteBatch(executions); else revert UnsupportedExecType(execType); } @@ -166,7 +208,7 @@ contract ExecutionHelper is IExecutionHelperEventsAndErrors { /// @param execType The execution type, which can be DEFAULT (revert on failure) or TRY (return on failure). function _handleDelegateCallExecution(bytes calldata executionCalldata, ExecType execType) internal { (address delegate, bytes calldata callData) = executionCalldata.decodeDelegateCall(); - if (execType == EXECTYPE_DEFAULT) _executeDelegatecall(delegate, callData); + if (execType == EXECTYPE_DEFAULT) _executeDelegatecallNoReturndata(delegate, callData); else if (execType == EXECTYPE_TRY) { (bool success, bytes memory result) = _tryExecuteDelegatecall(delegate, callData); if (!success) emit TryDelegateCallUnsuccessful(callData, result); diff --git a/contracts/base/ModuleManager.sol b/contracts/base/ModuleManager.sol index 1ed672df..a59ca9b3 100644 --- a/contracts/base/ModuleManager.sol +++ b/contracts/base/ModuleManager.sol @@ -179,7 +179,7 @@ abstract contract ModuleManager is Storage, EIP712, IModuleManagerEventsAndError // Perform the removal first validators.pop(prev, validator); - // Sentinel pointing to itself means the list is empty, so check this after removal + // Sentinel pointing to itself / zero means the list is empty / uninitialized, so check this after removal // Below error is very specific to uninstalling validators. require(_hasValidators(), CanNotRemoveLastValidator()); validator.excessivelySafeCall(gasleft(), 0, 0, abi.encodeWithSelector(IModule.onUninstall.selector, disableModuleData)); diff --git a/contracts/modules/validators/K1Validator.sol b/contracts/modules/validators/K1Validator.sol index c393f67d..2d5dbf76 100644 --- a/contracts/modules/validators/K1Validator.sol +++ b/contracts/modules/validators/K1Validator.sol @@ -12,13 +12,12 @@ pragma solidity ^0.8.27; // Nexus: A suite of contracts for Modular Smart Accounts compliant with ERC-7579 and ERC-4337, developed by Biconomy. // Learn more at https://biconomy.io. To report security issues, please contact us at: security@biconomy.io -import { SignatureCheckerLib } from "solady/utils/SignatureCheckerLib.sol"; +import { ECDSA } from "solady/utils/ECDSA.sol"; import { PackedUserOperation } from "account-abstraction/interfaces/PackedUserOperation.sol"; import { ERC7739Validator } from "../../base/ERC7739Validator.sol"; import { IValidator } from "../../interfaces/modules/IValidator.sol"; import { EnumerableSet } from "../../lib/EnumerableSet4337.sol"; import { MODULE_TYPE_VALIDATOR, VALIDATION_SUCCESS, VALIDATION_FAILED } from "../../types/Constants.sol"; -import { MessageHashUtils } from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol"; /// @title Nexus - K1Validator (ECDSA) /// @notice Validator module for smart accounts, verifying user operation signatures @@ -33,7 +32,7 @@ import { MessageHashUtils } from "@openzeppelin/contracts/utils/cryptography/Mes /// @author @zeroknots | Rhinestone.wtf | zeroknots.eth /// Special thanks to the Solady team for foundational contributions: https://github.com/Vectorized/solady contract K1Validator is IValidator, ERC7739Validator { - using SignatureCheckerLib for address; + using ECDSA for bytes32; using EnumerableSet for EnumerableSet.AddressSet; /*////////////////////////////////////////////////////////////////////////// @@ -57,6 +56,9 @@ contract K1Validator is IValidator, ERC7739Validator { /// @notice Error to indicate that the new owner cannot be a contract address error NewOwnerIsContract(); + /// @notice Error to indicate that the owner cannot be the zero address + error OwnerCannotBeZeroAddress(); + /// @notice Error to indicate that the data length is invalid error InvalidDataLength(); @@ -73,6 +75,7 @@ contract K1Validator is IValidator, ERC7739Validator { require(data.length != 0, NoOwnerProvided()); require(!_isInitialized(msg.sender), ModuleAlreadyInitialized()); address newOwner = address(bytes20(data[:20])); + require(newOwner != address(0), OwnerCannotBeZeroAddress()); require(!_isContract(newOwner), NewOwnerIsContract()); smartAccountOwners[msg.sender] = newOwner; if (data.length > 20) { @@ -185,7 +188,7 @@ contract K1Validator is IValidator, ERC7739Validator { /// @notice Returns the version of the module /// @return The version of the module function version() external pure returns (string memory) { - return "1.0.0-beta.1"; + return "1.0.0"; } /// @notice Checks if the module is of the specified type @@ -199,6 +202,15 @@ contract K1Validator is IValidator, ERC7739Validator { INTERNAL //////////////////////////////////////////////////////////////////////////*/ + /// @notice Recovers the signer from a signature + /// @param hash The hash of the data to validate + /// @param signature The signature data + /// @return The recovered signer address + /// @notice tryRecover returns address(0) on invalid signature + function _recoverSigner(bytes32 hash, bytes calldata signature) internal view returns (address) { + return hash.tryRecover(signature); + } + /// @dev Returns whether the `hash` and `signature` are valid. /// Obtains the authorized signer's credentials and calls some /// module's specific internal function to validate the signature @@ -236,12 +248,10 @@ contract K1Validator is IValidator, ERC7739Validator { return false; } - if (SignatureCheckerLib.isValidSignatureNowCalldata(owner, hash, signature)) { - return true; - } - if (SignatureCheckerLib.isValidSignatureNowCalldata(owner, MessageHashUtils.toEthSignedMessageHash(hash), signature)) { - return true; - } + // verify signer + // owner can not be zero address in this contract + if (_recoverSigner(hash, signature) == owner) return true; + if (_recoverSigner(hash.toEthSignedMessageHash(), signature) == owner) return true; return false; } diff --git a/contracts/utils/NexusBootstrap.sol b/contracts/utils/NexusBootstrap.sol index 7d3c28ac..3cebd3b2 100644 --- a/contracts/utils/NexusBootstrap.sol +++ b/contracts/utils/NexusBootstrap.sol @@ -160,6 +160,6 @@ contract NexusBootstrap is ModuleManager { /// @dev EIP712 domain name and version. function _domainNameAndVersion() internal pure override returns (string memory name, string memory version) { name = "NexusBootstrap"; - version = "1.0.0-beta.1"; + version = "1.0.0"; } } diff --git a/package.json b/package.json index 6faf4dd7..d8298c72 100644 --- a/package.json +++ b/package.json @@ -1,13 +1,13 @@ { "name": "nexus", "description": "Nexus - ERC7579 Modular Smart Account", - "version": "1.0.0-beta.1", + "version": "1.0.0", "author": { "name": "Biconomy", "url": "https://github.com/bcnmy" }, "dependencies": { - "@openzeppelin": "https://github.com/OpenZeppelin/openzeppelin-contracts/", + "@openzeppelin": "https://github.com/OpenZeppelin/openzeppelin-contracts", "dotenv": "^16.4.5", "solarray": "github:sablier-labs/solarray", "viem": "2.7.13" diff --git a/scripts/foundry/Deploy.s.sol b/scripts/foundry/Deploy.s.sol index c5316bba..9388d437 100644 --- a/scripts/foundry/Deploy.s.sol +++ b/scripts/foundry/Deploy.s.sol @@ -5,15 +5,33 @@ pragma solidity >=0.8.0 <0.9.0; import { Nexus } from "../../contracts/Nexus.sol"; import { BaseScript } from "./Base.s.sol"; +import { K1ValidatorFactory } from "../../contracts/factory/K1ValidatorFactory.sol"; +import { K1Validator } from "../../contracts/modules/validators/K1Validator.sol"; +import { BootstrapLib } from "../../contracts/lib/BootstrapLib.sol"; +import { NexusBootstrap } from "../../contracts/utils/NexusBootstrap.sol"; +import { MockRegistry } from "../../contracts/mocks/MockRegistry.sol"; +import { HelperConfig } from "./HelperConfig.s.sol"; -/// @dev See the Solidity Scripting tutorial: https://book.getfoundry.sh/tutorials/solidity-scripting contract Deploy is BaseScript { - address private constant _ENTRYPOINT = 0x0000000071727De22E5E9d8BAf0edAc6f37da032; - function run() public broadcast returns (Nexus smartAccount) { - smartAccount = new Nexus(_ENTRYPOINT); - } + K1ValidatorFactory private k1ValidatorFactory; + K1Validator private k1Validator; + NexusBootstrap private bootstrapper; + MockRegistry private registry; + HelperConfig private helperConfig; - function test() public { - // This is a test function to exclude this script from the coverage report. + function run() public broadcast returns (Nexus smartAccount) { + helperConfig = new HelperConfig(); + require(address(helperConfig.ENTRYPOINT()) != address(0), "ENTRYPOINT is not set"); + smartAccount = new Nexus(address(helperConfig.ENTRYPOINT())); + k1Validator = new K1Validator(); + bootstrapper = new NexusBootstrap(); + registry = new MockRegistry(); + k1ValidatorFactory = new K1ValidatorFactory( + address(smartAccount), + msg.sender, + address(k1Validator), + bootstrapper, + registry + ); } } diff --git a/scripts/foundry/HelperConfig.s.sol b/scripts/foundry/HelperConfig.s.sol new file mode 100644 index 00000000..7ed3d999 --- /dev/null +++ b/scripts/foundry/HelperConfig.s.sol @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity >=0.8.0 <0.9.0; +pragma solidity >=0.8.0 <0.9.0; + +import { EntryPoint } from "account-abstraction/core/EntryPoint.sol"; +import { IEntryPoint } from "account-abstraction/interfaces/IEntryPoint.sol"; + +import {Script} from "forge-std/Script.sol"; + +contract HelperConfig is Script { + IEntryPoint public ENTRYPOINT; + address private constant MAINNET_ENTRYPOINT_ADDRESS = 0x0000000071727De22E5E9d8BAf0edAc6f37da032; + + constructor() { + if (block.chainid == 31337) { + setupAnvilConfig(); + } else { + ENTRYPOINT = IEntryPoint(MAINNET_ENTRYPOINT_ADDRESS); + } + } + + function setupAnvilConfig() public { + if(address(ENTRYPOINT) != address(0)){ + return; + } + ENTRYPOINT = new EntryPoint(); + vm.etch(address(MAINNET_ENTRYPOINT_ADDRESS), address(ENTRYPOINT).code); + ENTRYPOINT = IEntryPoint(MAINNET_ENTRYPOINT_ADDRESS); + } + +} \ No newline at end of file diff --git a/test/foundry/fork/arbitrum/ArbitrumSmartAccountUpgradeTest.t.sol b/test/foundry/fork/arbitrum/ArbitrumSmartAccountUpgradeTest.t.sol index 7725d1ca..06b9584f 100644 --- a/test/foundry/fork/arbitrum/ArbitrumSmartAccountUpgradeTest.t.sol +++ b/test/foundry/fork/arbitrum/ArbitrumSmartAccountUpgradeTest.t.sol @@ -45,7 +45,7 @@ contract ArbitrumSmartAccountUpgradeTest is NexusTest_Base, ArbitrumSettings { /// @notice Validates the account ID after the upgrade process. function test_AccountIdValidationAfterUpgrade() public { test_UpgradeV2ToV3AndInitialize(); - string memory expectedAccountId = "biconomy.nexus.1.0.0-beta.1"; + string memory expectedAccountId = "biconomy.nexus.1.0.0"; string memory actualAccountId = IAccountConfig(payable(address(smartAccountV2))).accountId(); assertEq(actualAccountId, expectedAccountId, "Account ID does not match after upgrade."); } diff --git a/test/foundry/unit/concrete/accountconfig/TestAccountConfig_AccountId.t.sol b/test/foundry/unit/concrete/accountconfig/TestAccountConfig_AccountId.t.sol index f59811f9..c8e13797 100644 --- a/test/foundry/unit/concrete/accountconfig/TestAccountConfig_AccountId.t.sol +++ b/test/foundry/unit/concrete/accountconfig/TestAccountConfig_AccountId.t.sol @@ -19,7 +19,7 @@ contract TestAccountConfig_AccountId is Test { /// @notice Tests if the account ID returns the expected value function test_WhenCheckingTheAccountID() external givenTheAccountConfiguration { - string memory expected = "biconomy.nexus.1.0.0-beta.1"; + string memory expected = "biconomy.nexus.1.0.0"; assertEq(accountConfig.accountId(), expected, "AccountConfig should return the expected account ID."); } } diff --git a/test/foundry/unit/concrete/factory/TestAccountFactory_Deployments.t.sol b/test/foundry/unit/concrete/factory/TestAccountFactory_Deployments.t.sol index 9270cd12..6b97f1e4 100644 --- a/test/foundry/unit/concrete/factory/TestAccountFactory_Deployments.t.sol +++ b/test/foundry/unit/concrete/factory/TestAccountFactory_Deployments.t.sol @@ -72,7 +72,7 @@ contract TestAccountFactory_Deployments is NexusTest_Base { userOps[0] = buildUserOpWithInitAndCalldata(user, initCode, "", address(VALIDATOR_MODULE)); ENTRYPOINT.depositTo{ value: 1 ether }(address(accountAddress)); ENTRYPOINT.handleOps(userOps, payable(user.addr)); - assertEq(IAccountConfig(accountAddress).accountId(), "biconomy.nexus.1.0.0-beta.1", "Not deployed properly"); + assertEq(IAccountConfig(accountAddress).accountId(), "biconomy.nexus.1.0.0", "Not deployed properly"); } /// @notice Tests that deploying an account fails if it already exists. diff --git a/test/foundry/unit/concrete/factory/TestNexusAccountFactory_Deployments.t.sol b/test/foundry/unit/concrete/factory/TestNexusAccountFactory_Deployments.t.sol index 73d0db7a..ce4146d9 100644 --- a/test/foundry/unit/concrete/factory/TestNexusAccountFactory_Deployments.t.sol +++ b/test/foundry/unit/concrete/factory/TestNexusAccountFactory_Deployments.t.sol @@ -83,7 +83,7 @@ contract TestNexusAccountFactory_Deployments is NexusTest_Base { userOps[0] = buildUserOpWithInitAndCalldata(user, initCode, "", address(VALIDATOR_MODULE)); ENTRYPOINT.depositTo{ value: 1 ether }(address(accountAddress)); ENTRYPOINT.handleOps(userOps, payable(user.addr)); - assertEq(IAccountConfig(accountAddress).accountId(), "biconomy.nexus.1.0.0-beta.1", "Not deployed properly"); + assertEq(IAccountConfig(accountAddress).accountId(), "biconomy.nexus.1.0.0", "Not deployed properly"); } /// @notice Tests that deploying an account fails if it already exists. diff --git a/test/foundry/unit/concrete/gas/TestGas_ExecutionHelper.t.sol b/test/foundry/unit/concrete/gas/TestGas_ExecutionHelper.t.sol index 366280d0..a4309694 100644 --- a/test/foundry/unit/concrete/gas/TestGas_ExecutionHelper.t.sol +++ b/test/foundry/unit/concrete/gas/TestGas_ExecutionHelper.t.sol @@ -26,6 +26,7 @@ contract TestGas_ExecutionHelper is TestAccountExecution_Base { 0 ); ENTRYPOINT.handleOps(userOpsInstall, payable(address(BOB.addr))); + assertTrue(BOB_ACCOUNT.isModuleInstalled(MODULE_TYPE_EXECUTOR, address(mockExecutor), ""), "MockExecutor should be installed"); } // Execute Tests @@ -83,11 +84,11 @@ contract TestGas_ExecutionHelper is TestAccountExecution_Base { // ExecuteFromExecutor Tests function test_Gas_ExecuteFromExecutor_Single() public { - prank(address(mockExecutor)); - + vm.startPrank(address(mockExecutor)); uint256 initialGas = gasleft(); BOB_ACCOUNT.executeFromExecutor(ModeLib.encodeSimpleSingle(), ExecLib.encodeSingle(address(0), 0, "")); uint256 gasUsed = initialGas - gasleft(); + vm.stopPrank(); console.log("Gas used for single empty execution from executor: ", gasUsed); } diff --git a/test/foundry/unit/concrete/gas/TestGas_NexusAccountFactory.t.sol b/test/foundry/unit/concrete/gas/TestGas_NexusAccountFactory.t.sol index 8bc22fd4..628adbd5 100644 --- a/test/foundry/unit/concrete/gas/TestGas_NexusAccountFactory.t.sol +++ b/test/foundry/unit/concrete/gas/TestGas_NexusAccountFactory.t.sol @@ -71,7 +71,7 @@ contract TestGas_NexusAccountFactory is TestModuleManagement_Base { /// @notice Validates the creation of a new account. /// @param _account The new account address. function assertValidCreation(Nexus _account) internal { - string memory expected = "biconomy.nexus.1.0.0-beta.1"; + string memory expected = "biconomy.nexus.1.0.0"; assertEq(_account.accountId(), expected, "AccountConfig should return the expected account ID."); assertTrue( _account.isModuleInstalled(MODULE_TYPE_VALIDATOR, address(VALIDATOR_MODULE), ""), "Account should have the validation module installed" diff --git a/test/foundry/unit/concrete/modules/TestK1Validator.t.sol b/test/foundry/unit/concrete/modules/TestK1Validator.t.sol index 062e16a3..f99ba7a5 100644 --- a/test/foundry/unit/concrete/modules/TestK1Validator.t.sol +++ b/test/foundry/unit/concrete/modules/TestK1Validator.t.sol @@ -102,8 +102,6 @@ contract TestK1Validator is NexusTest_Base { /// @notice Tests the validateUserOp function with a valid signature function test_ValidateUserOp_toEthSignedMessageHash_Success() public { - prank(address(BOB_ACCOUNT)); - userOp.signature = signature; uint256 validationResult = validator.validateUserOp(userOp, userOpHash); @@ -113,8 +111,6 @@ contract TestK1Validator is NexusTest_Base { /// @notice Tests the validateUserOp function with an invalid signature function test_ValidateUserOp_Failure() public { - prank(address(BOB_ACCOUNT)); - userOp.signature = abi.encodePacked(signMessage(BOB, keccak256(abi.encodePacked("invalid")))); uint256 validationResult = validator.validateUserOp(userOp, userOpHash); @@ -124,8 +120,6 @@ contract TestK1Validator is NexusTest_Base { /// @notice Tests the isValidSignatureWithSender function with a valid signature function test_IsValidSignatureWithSender_Success() public { - startPrank(address(BOB_ACCOUNT)); - bytes32 originalHash = keccak256(abi.encodePacked("valid message")); (uint8 v, bytes32 r, bytes32 s) = vm.sign(BOB.privateKey, toERC1271HashPersonalSign(originalHash)); bytes memory signedMessage = abi.encodePacked(r, s, v); @@ -133,15 +127,11 @@ contract TestK1Validator is NexusTest_Base { bytes4 result = BOB_ACCOUNT.isValidSignature(originalHash, completeSignature); - stopPrank(); - assertEq(result, ERC1271_MAGICVALUE, "Signature should be valid"); } /// @notice Tests the validateUserOp function with a valid signature function test_ValidateUserOp_Success() public { - startPrank(address(BOB_ACCOUNT)); - bytes32 originalHash = keccak256(abi.encodePacked("123")); (uint8 v, bytes32 r, bytes32 s) = vm.sign(BOB.privateKey, originalHash); @@ -150,15 +140,12 @@ contract TestK1Validator is NexusTest_Base { uint256 res = validator.validateUserOp(userOp, originalHash); - stopPrank(); - assertEq(res, VALIDATION_SUCCESS, "Signature should be valid"); } /// @notice Tests the isValidSignatureWithSender function with an invalid signature function test_IsValidSignatureWithSender_Failure() public { prank(address(BOB_ACCOUNT)); - vm.expectRevert(); //it should revert as last try to check if it's an RPC call which reverts if called on-chain validator.isValidSignatureWithSender(address(BOB_ACCOUNT), userOpHash, abi.encodePacked(signMessage(BOB, keccak256(abi.encodePacked("invalid"))))); } @@ -200,7 +187,7 @@ contract TestK1Validator is NexusTest_Base { function test_Version() public { string memory contractVersion = validator.version(); - assertEq(contractVersion, "1.0.0-beta.1", "Contract version should be '1.0.0-beta.1'"); + assertEq(contractVersion, "1.0.0", "Contract version should be '1.0.0'"); } /// @notice Tests the isModuleType function to return the correct module type @@ -232,8 +219,6 @@ contract TestK1Validator is NexusTest_Base { /// @notice Tests that a valid signature with a valid 's' value is accepted function test_ValidateUserOp_ValidSignature() public { - startPrank(address(BOB_ACCOUNT)); - bytes32 originalHash = keccak256(abi.encodePacked("valid message")); (uint8 v, bytes32 r, bytes32 s) = vm.sign(BOB.privateKey, originalHash); @@ -244,15 +229,11 @@ contract TestK1Validator is NexusTest_Base { uint256 res = validator.validateUserOp(userOp, originalHash); - stopPrank(); - assertEq(res, VALIDATION_SUCCESS, "Valid signature should be accepted"); } /// @notice Tests that a signature with an invalid 's' value is rejected function test_ValidateUserOp_InvalidSValue() public { - startPrank(address(BOB_ACCOUNT)); - bytes32 originalHash = keccak256(abi.encodePacked("invalid message")); (uint8 v, bytes32 r, bytes32 s) = vm.sign(BOB.privateKey, originalHash); @@ -265,15 +246,12 @@ contract TestK1Validator is NexusTest_Base { uint256 res = validator.validateUserOp(userOp, originalHash); - stopPrank(); - assertEq(res, VALIDATION_FAILED, "Signature with invalid 's' value should be rejected"); } /// @notice Tests that a valid signature with a valid 's' value is accepted for isValidSignatureWithSender function test_IsValidSignatureWithSender_ValidSignature() public { startPrank(address(BOB_ACCOUNT)); - // Generate a valid message hash bytes32 originalHash = keccak256(abi.encodePacked("valid message")); @@ -288,17 +266,13 @@ contract TestK1Validator is NexusTest_Base { // Call isValidSignatureWithSender on the validator contract with the correct parameters bytes4 result = validator.isValidSignatureWithSender(address(BOB_ACCOUNT), originalHash, signedMessage); - stopPrank(); - // Ensure the result is the expected ERC1271_MAGICVALUE assertEq(result, ERC1271_MAGICVALUE, "Valid signature should be accepted"); } /// @notice Tests that a signature with an invalid 's' value is rejected for isValidSignatureWithSender function test_IsValidSignatureWithSender_InvalidSValue() public { - startPrank(address(BOB_ACCOUNT)); - bytes32 originalHash = keccak256(abi.encodePacked("invalid message")); (uint8 v, bytes32 r, bytes32 s) = vm.sign(BOB.privateKey, originalHash); @@ -312,15 +286,11 @@ contract TestK1Validator is NexusTest_Base { bytes4 result = BOB_ACCOUNT.isValidSignature(originalHash, completeSignature); - stopPrank(); - assertEq(result, ERC1271_INVALID, "Signature with invalid 's' value should be rejected"); } function test_IsValidSignatureWithSender_SafeCaller_Success() public { assertEq(mockSafe1271Caller.balanceOf(address(BOB_ACCOUNT)), 0); - - vm.startPrank(address(BOB_ACCOUNT)); // alternative way of setting mockSafe1271Caller as safe sender in k1 validator // commented out as it was already set at setup @@ -348,8 +318,6 @@ contract TestK1Validator is NexusTest_Base { uint256 res = mockSafe1271Caller.validateUserOp(userOp, mockUserOpHash); - stopPrank(); - assertEq(res, VALIDATION_SUCCESS, "Signature should be valid"); assertEq(mockSafe1271Caller.balanceOf(address(BOB_ACCOUNT)), 1); } @@ -362,7 +330,7 @@ contract TestK1Validator is NexusTest_Base { abi.encode( keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"), keccak256("Nexus"), - keccak256("1.0.0-beta.1"), + keccak256("1.0.0"), block.chainid, address(BOB_ACCOUNT) ) diff --git a/test/foundry/utils/TestHelper.t.sol b/test/foundry/utils/TestHelper.t.sol index 694a7e95..5419eb08 100644 --- a/test/foundry/utils/TestHelper.t.sol +++ b/test/foundry/utils/TestHelper.t.sol @@ -25,6 +25,7 @@ import { NexusAccountFactory } from "../../../contracts/factory/NexusAccountFact import { BootstrapLib } from "../../../contracts/lib/BootstrapLib.sol"; import { MODE_VALIDATION } from "../../../contracts/types/Constants.sol"; import { MockRegistry } from "../../../contracts/mocks/MockRegistry.sol"; +import { HelperConfig } from "../../../scripts/foundry/HelperConfig.s.sol"; contract TestHelper is CheatCodes, EventsAndErrors { // ----------------------------------------- @@ -103,9 +104,8 @@ contract TestHelper is CheatCodes, EventsAndErrors { } function deployTestContracts() internal { - ENTRYPOINT = new EntryPoint(); - vm.etch(address(0x0000000071727De22E5E9d8BAf0edAc6f37da032), address(ENTRYPOINT).code); - ENTRYPOINT = IEntryPoint(0x0000000071727De22E5E9d8BAf0edAc6f37da032); + HelperConfig helperConfig = new HelperConfig(); + ENTRYPOINT = helperConfig.ENTRYPOINT(); ACCOUNT_IMPLEMENTATION = new Nexus(address(ENTRYPOINT)); FACTORY = new NexusAccountFactory(address(ACCOUNT_IMPLEMENTATION), address(FACTORY_OWNER.addr)); META_FACTORY = new BiconomyMetaFactory(address(FACTORY_OWNER.addr)); diff --git a/test/hardhat/smart-account/Nexus.Basics.specs.ts b/test/hardhat/smart-account/Nexus.Basics.specs.ts index fa36699f..3735ea07 100644 --- a/test/hardhat/smart-account/Nexus.Basics.specs.ts +++ b/test/hardhat/smart-account/Nexus.Basics.specs.ts @@ -134,7 +134,7 @@ describe("Nexus Basic Specs", function () { describe("Smart Account Basics", function () { it("Should correctly return the Nexus's ID", async function () { expect(await smartAccount.accountId()).to.equal( - "biconomy.nexus.1.0.0-beta.1", + "biconomy.nexus.1.0.0", ); }); diff --git a/test/hardhat/smart-account/Nexus.Factory.specs.ts b/test/hardhat/smart-account/Nexus.Factory.specs.ts index a7f79fd5..6774fab2 100644 --- a/test/hardhat/smart-account/Nexus.Factory.specs.ts +++ b/test/hardhat/smart-account/Nexus.Factory.specs.ts @@ -4,10 +4,8 @@ import { AddressLike, Signer, ZeroAddress, - ZeroHash, keccak256, solidityPacked, - zeroPadBytes, } from "ethers"; import { loadFixture } from "@nomicfoundation/hardhat-network-helpers"; import { @@ -28,7 +26,7 @@ import { deployContractsAndSAFixture, deployContractsFixture, } from "../utils/deployment"; -import { encodeData, to18 } from "../utils/encoding"; +import { to18 } from "../utils/encoding"; import { MODE_VALIDATION, buildPackedUserOp, @@ -36,7 +34,6 @@ import { numberTo3Bytes, } from "../utils/operationHelpers"; import { BootstrapConfigStruct } from "../../../typechain-types/contracts/lib/BootstrapLib"; -import { SignerWithAddress } from "@nomiclabs/hardhat-ethers/signers"; describe("Nexus Factory Tests", function () { let factory: K1ValidatorFactory; @@ -470,7 +467,7 @@ describe("Nexus Factory Tests", function () { let smartAccount: Nexus; let entryPoint: EntryPoint; let factory: RegistryFactory; - let bootstrap: Bootstrap; + let bootstrap: NexusBootstrap; let validatorModule: MockValidator; let executorModule: MockExecutor; let BootstrapLib: BootstrapLib; diff --git a/test/hardhat/smart-account/Nexus.Module.K1Validator.specs.ts b/test/hardhat/smart-account/Nexus.Module.K1Validator.specs.ts index 7ab3b059..993238e2 100644 --- a/test/hardhat/smart-account/Nexus.Module.K1Validator.specs.ts +++ b/test/hardhat/smart-account/Nexus.Module.K1Validator.specs.ts @@ -25,6 +25,7 @@ describe("K1Validator module tests", () => { let k1ModuleAddress: AddressLike; let mockExecutor: MockExecutor; let accountOwner: Signer; + let aliceAccountOwner: Signer; let entryPoint: EntryPoint; let bundler: Signer; let counter: Counter; @@ -35,6 +36,7 @@ describe("K1Validator module tests", () => { ecdsaValidator: k1Validator, mockExecutor, accountOwner, + aliceAccountOwner, entryPoint, mockValidator, counter, @@ -43,6 +45,7 @@ describe("K1Validator module tests", () => { k1ModuleAddress = await k1Validator.getAddress(); mockExecutor = mockExecutor; accountOwner = accountOwner; + aliceAccountOwner = aliceAccountOwner; entryPoint = entryPoint; bundler = ethers.Wallet.createRandom(); @@ -76,7 +79,7 @@ describe("K1Validator module tests", () => { it("should get module version", async () => { const version = await k1Validator.version(); - expect(version).to.equal("1.0.0-beta.1"); + expect(version).to.equal("1.0.0"); }); it("should check module type", async () => { @@ -264,22 +267,28 @@ describe("K1Validator module tests", () => { userOp.sender, ethers.zeroPadBytes(validatorModuleAddress.toString(), 24), ); - userOp.nonce = nonce; const userOpHash = await entryPoint.getUserOpHash(userOp); + const connectedSigner = await ethers.provider.getSigner( + await accountOwner.getAddress(), + ); + const signerProvider = connectedSigner.provider; + + // Review: the signer + const eth_sign = await signerProvider.send("eth_sign", [ + await accountOwner.getAddress(), + userOpHash, + ]); + console.log("eth_sign", eth_sign); + + userOp.signature = eth_sign; + const isValid = await k1Validator.validateUserOp(userOp, userOpHash); // 0 - valid, 1 - invalid - expect(isValid).to.equal(1); - }); - - it("Should check signature using isValidSignatureWithSender", async () => { - const message = "Some Message"; - // const isValid = await k1Validator.isValidSignatureWithSender(await deployedNexus.getAddress(), , ); - // 0x1626ba7e - valid - // 0xffffffff - invalid + expect(isValid).to.equal(0); }); }); }); diff --git a/yarn.lock b/yarn.lock index c63e6327..69a234fd 100644 --- a/yarn.lock +++ b/yarn.lock @@ -752,9 +752,9 @@ resolved "https://registry.yarnpkg.com/@openzeppelin/contracts/-/contracts-5.0.2.tgz#b1d03075e49290d06570b2fd42154d76c2a5d210" integrity sha512-ytPc6eLGcHHnapAZ9S+5qsdomhjo6QBHTDRRBFfTxXIpsicMhVPouPgmUPebZZZGX7vt9USA+Z+0M0dSVtSUEA== -"@openzeppelin@https://github.com/OpenZeppelin/openzeppelin-contracts/": +"@openzeppelin@https://github.com/OpenZeppelin/openzeppelin-contracts": version "5.0.2" - resolved "https://github.com/OpenZeppelin/openzeppelin-contracts/#cceac54953ccda8a9a028d0b9bf4378605fdf67e" + resolved "https://github.com/OpenZeppelin/openzeppelin-contracts#49cd64565aafa5b8f6863bf60a30ef015861614c" "@pnpm/config.env-replace@^1.1.0": version "1.1.0"