From 049a21e32684eaf94620ca446c0fdddabdaaea01 Mon Sep 17 00:00:00 2001 From: Jimmy Chu <898091+jimmychu0807@users.noreply.github.com> Date: Wed, 8 Jan 2025 11:39:34 +0800 Subject: [PATCH] feat: updated modulekit and added more test (#12) * Using new modulekit * updated * updated --- package.json | 2 +- pnpm-lock.yaml | 81 +++++++++++---------- src/SemaphoreMSAValidator.sol | 119 ++++++++++++++----------------- test/SemaphoreMSAValidator.t.sol | 111 ++++++++++++++++++++-------- test/utils/SimpleContract.sol | 15 ++++ test/utils/TestUtils.sol | 11 +++ 6 files changed, 206 insertions(+), 133 deletions(-) create mode 100644 test/utils/SimpleContract.sol diff --git a/package.json b/package.json index a10469c..2157011 100644 --- a/package.json +++ b/package.json @@ -16,7 +16,7 @@ "url": "https://github.com/jimmychu0807/semaphore-msa-validator/issues" }, "devDependencies": { - "@rhinestone/modulekit": "~0.5.1", + "@rhinestone/modulekit": "~0.5.4", "@semaphore-protocol/contracts": "github:jimmychu0807/semaphore#identity-cli&path:/packages/contracts/contracts", "@semaphore-protocol/core": "github:jimmychu0807/semaphore#identity-cli&path:/packages/core", "@semaphore-protocol/identity": "github:jimmychu0807/semaphore#identity-cli&path:/packages/identity", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index be7433c..000d7bb 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -9,8 +9,8 @@ importers: .: devDependencies: '@rhinestone/modulekit': - specifier: ~0.5.1 - version: 0.5.1(ethers@5.7.2)(hardhat@2.22.17(typescript@4.9.5))(lodash@4.17.21)(typechain@5.2.0(typescript@4.9.5))(typescript@4.9.5) + specifier: ~0.5.4 + version: 0.5.4(ethers@5.7.2)(hardhat@2.22.17(typescript@4.9.5))(lodash@4.17.21)(typechain@5.2.0(typescript@4.9.5))(typescript@4.9.5) '@semaphore-protocol/contracts': specifier: github:jimmychu0807/semaphore#identity-cli&path:/packages/contracts/contracts version: https://codeload.github.com/jimmychu0807/semaphore/tar.gz/27c5443c73c3cc752f4c7d9b1b5b76ffcef8d595#path:/packages/contracts/contracts @@ -326,8 +326,8 @@ packages: resolution: {integrity: sha512-V1JJ1WTRUqHHrOSh597hURcMqVKVGL/ea3kv0gSnEdsEZ0/+VyPghM1lMNGc00z7CIQorSvbKpuJkxvuHbvdbg==} engines: {node: '>= 16'} - '@noble/hashes@1.6.1': - resolution: {integrity: sha512-pq5D8h10hHBjyqX+cfBm0i8JUXJ0UhczFc4r74zbuT9XgewFo2E3J1cOaGtdZynILNmQ685YWGzGE1Zv6io50w==} + '@noble/hashes@1.7.0': + resolution: {integrity: sha512-HXydb0DgzTpDPwbVeDGCG1gIu7X6+AuU6Zl6av/E/KG8LMsvPntvq+w17CHRpKBmN6Ybdrt1eP3k4cj8DJa78w==} engines: {node: ^14.21.3 || >=16} '@noble/secp256k1@1.7.1': @@ -476,8 +476,8 @@ packages: '@rhinestone/erc4337-validation@0.0.4': resolution: {integrity: sha512-9GPvOvmM9j5ZZRCFeujPacUyByRnrGL22/5177hRzXh5mLq/A22EyvVIVNcsWMvNiLcHAV4dkkKpXaljxNOT9A==} - '@rhinestone/modulekit@0.5.1': - resolution: {integrity: sha512-IN6esJl+s8IVNaKJ30FVqhSQvAEmcp2NbmYZ2cWvH0pf1f322YkD+jeXr9gNcgEGqItKoxCPe1QJLNk/dZwC1g==} + '@rhinestone/modulekit@0.5.4': + resolution: {integrity: sha512-0sS+5Dg7tP9MYmYixd1jslytcsO+4eDZemX7u3b/aKhe+/1N6Z7Bs68bqu2rpsq07FYn9IPGhNlmfkR1Oc1EXA==} '@rhinestone/sentinellist@https://codeload.github.com/rhinestonewtf/sentinellist/tar.gz/e722c5cc68c570d535bc3c9f85b3ce90cdc38807': resolution: {tarball: https://codeload.github.com/rhinestonewtf/sentinellist/tar.gz/e722c5cc68c570d535bc3c9f85b3ce90cdc38807} @@ -606,11 +606,11 @@ packages: '@types/ms@0.7.34': resolution: {integrity: sha512-nG96G3Wp6acyAgJqGasjODb+acrI7KltPiRxzHPXnP3NgI28bpQDRv53olbqGXbfcgF5aiiHmO3xpwEpS5Ld9g==} - '@types/node@20.17.11': - resolution: {integrity: sha512-Ept5glCK35R8yeyIeYlRIZtX6SLRyqMhOFTgj5SOkMpLTdw3SEHI9fHx60xaUZ+V1aJxQJODE+7/j5ocZydYTg==} + '@types/node@20.17.12': + resolution: {integrity: sha512-vo/wmBgMIiEA23A/knMfn/cf37VnuF52nZh5ZoW0GWt4e4sxNquibrMRJ7UQsA06+MBx9r/H1jsI9grYjQCQlw==} - '@types/node@22.10.3': - resolution: {integrity: sha512-DifAyw4BkrufCILvD3ucnuN8eydUfc/C1GlyrnI+LK6543w5/L3VeVgf05o3B4fqSXP1dKYLOZsKfutpxPzZrw==} + '@types/node@22.10.5': + resolution: {integrity: sha512-F8Q+SeGimwOo86fiovQh8qiXfFEh2/ocYv7tU5pJ3EXMSSxk1Joj5wefpFK2fHTf/N6HKGSxIDBT9f3gCxXPkQ==} '@types/node@22.7.5': resolution: {integrity: sha512-jML7s2NAzMWc//QSJ1a3prpk78cOPchGvXJsC3C6R6PSMoooztvRVQEz89gmBTBY1SPMaqo5teB4uNHPdetShQ==} @@ -1196,8 +1196,8 @@ packages: fast-diff@1.3.0: resolution: {integrity: sha512-VxPP4NqbUjj6MaAOafWeUn2cXWLcCtljklUtZf0Ind4XQ+QPtmA0b18zZy0jIQx+ExRVCR/ZQpBmik5lXshNsw==} - fast-glob@3.3.2: - resolution: {integrity: sha512-oX2ruAFQwf/Orj8m737Y5adxDQO0LAB7/S5MnxCdTNDd4p6BsyIVsv9JQsATbTSq8KHRpLwIHbVlUNatxd+1Ow==} + fast-glob@3.3.3: + resolution: {integrity: sha512-7MptL8U0cqcFdzIzwOTHoilX9x5BrNqye7Z/LuC7kCMRio1EMSyqRK3BEAUD7sXRq4iT4AzTVuZdhgQ2TCvYLg==} engines: {node: '>=8.6.0'} fast-json-stable-stringify@2.1.0: @@ -1311,8 +1311,12 @@ packages: resolution: {integrity: sha512-DyFP3BM/3YHTQOCUL/w0OZHR0lpKeGrxotcHWcqNEdnltqFwXVfhEBQ94eIo34AfQpo0rGki4cyIiftY06h2Fg==} engines: {node: 6.* || 8.* || >= 10.*} - get-intrinsic@1.2.6: - resolution: {integrity: sha512-qxsEs+9A+u85HhllWJJFicJfPDhRmjzoYdl64aMWW9yRIJmSyxdn8IEkuIM530/7T+lv0TIHd8L6Q/ra0tEoeA==} + get-intrinsic@1.2.7: + resolution: {integrity: sha512-VW6Pxhsrk0KAOqs3WEd0klDiF/+V7gQOpAvY1jVU/LHmaD/kQO4523aiJuikX/QAKYiW6x8Jh+RJej1almdtCA==} + engines: {node: '>= 0.4'} + + get-proto@1.0.1: + resolution: {integrity: sha512-sTSfBjoXBp89JvIKIefqw7U2CCebsc74kiY6awiGogKtoSGbgjYE/G/+l9sF3MWFPNc9IcoOC4ODfKHfxFmp0g==} engines: {node: '>= 0.4'} get-stream@6.0.1: @@ -1783,8 +1787,8 @@ packages: resolution: {integrity: sha512-kDCGIbxkDSXE3euJZZXzc6to7fCrKHNI/hSRQnRuQ+BWjFNzZwiFF8fj/6o2t2G9/jTj8PSIYTfCLelLZEeRpA==} engines: {node: '>= 0.4'} - obliterator@2.0.4: - resolution: {integrity: sha512-lgHwxlxV1qIg1Eap7LgIeoBWIMFibOjbrYPIPJZcI1mmGAI2m3lNYpK12Y+GBdPQ0U1hRwSord7GIaawz962qQ==} + obliterator@2.0.5: + resolution: {integrity: sha512-42CPE9AhahZRsMNslczq0ctAEtqk8Eka26QofnqC346BZdHDySk3LWka23LI7ULIw11NmltpiLagIq8gBozxTw==} once@1.4.0: resolution: {integrity: sha512-lNaJgI+2Q5URQBkccEKHTQOPaXdUxnZZElQTZY0MFUAuaEqe1E+Nyvgdz/aIyNi6Z9MzO5dv1H8n58/GELp3+w==} @@ -2831,7 +2835,7 @@ snapshots: '@noble/hashes@1.4.0': {} - '@noble/hashes@1.6.1': {} + '@noble/hashes@1.7.0': {} '@noble/secp256k1@1.7.1': {} @@ -2980,7 +2984,7 @@ snapshots: - typechain - utf-8-validate - '@rhinestone/modulekit@0.5.1(ethers@5.7.2)(hardhat@2.22.17(typescript@4.9.5))(lodash@4.17.21)(typechain@5.2.0(typescript@4.9.5))(typescript@4.9.5)': + '@rhinestone/modulekit@0.5.4(ethers@5.7.2)(hardhat@2.22.17(typescript@4.9.5))(lodash@4.17.21)(typechain@5.2.0(typescript@4.9.5))(typescript@4.9.5)': dependencies: '@ERC4337/account-abstraction': accountabstraction@https://codeload.github.com/kopy-kat/account-abstraction/tar.gz/c5887153fbfe3ed09b2637cac39873f96d676f38(ethers@5.7.2)(hardhat@2.22.17(typescript@4.9.5))(lodash@4.17.21)(typechain@5.2.0(typescript@4.9.5)) '@ERC4337/account-abstraction-v0.6': accountabstraction@https://codeload.github.com/eth-infinitism/account-abstraction/tar.gz/7174d6d845618dbd11cee68eefa715f5263690b6(ethers@5.7.2)(hardhat@2.22.17(typescript@4.9.5))(lodash@4.17.21)(typechain@5.2.0(typescript@4.9.5)) @@ -3163,11 +3167,11 @@ snapshots: '@types/bn.js@4.11.6': dependencies: - '@types/node': 22.10.3 + '@types/node': 22.10.5 '@types/bn.js@5.1.6': dependencies: - '@types/node': 22.10.3 + '@types/node': 22.10.5 '@types/debug@4.1.12': dependencies: @@ -3176,7 +3180,7 @@ snapshots: '@types/glob@7.2.0': dependencies: '@types/minimatch': 5.1.2 - '@types/node': 22.10.3 + '@types/node': 22.10.5 '@types/http-cache-semantics@4.0.4': {} @@ -3188,11 +3192,11 @@ snapshots: '@types/ms@0.7.34': {} - '@types/node@20.17.11': + '@types/node@20.17.12': dependencies: undici-types: 6.19.8 - '@types/node@22.10.3': + '@types/node@22.10.5': dependencies: undici-types: 6.20.0 @@ -3202,7 +3206,7 @@ snapshots: '@types/pbkdf2@3.1.2': dependencies: - '@types/node': 22.10.3 + '@types/node': 22.10.5 '@types/prettier@2.7.3': {} @@ -3210,7 +3214,7 @@ snapshots: '@types/secp256k1@4.0.6': dependencies: - '@types/node': 22.10.3 + '@types/node': 22.10.5 '@zk-kit/artifacts@1.8.0': {} @@ -3510,7 +3514,7 @@ snapshots: call-bound@1.0.3: dependencies: call-bind-apply-helpers: 1.0.1 - get-intrinsic: 1.2.6 + get-intrinsic: 1.2.7 callsites@3.1.0: {} @@ -3800,7 +3804,7 @@ snapshots: ethereum-bloom-filters@1.2.0: dependencies: - '@noble/hashes': 1.6.1 + '@noble/hashes': 1.7.0 ethereum-cryptography@0.1.3: dependencies: @@ -3936,7 +3940,7 @@ snapshots: fast-diff@1.3.0: {} - fast-glob@3.3.2: + fast-glob@3.3.3: dependencies: '@nodelib/fs.stat': 2.0.5 '@nodelib/fs.walk': 1.2.8 @@ -4047,19 +4051,24 @@ snapshots: get-caller-file@2.0.5: {} - get-intrinsic@1.2.6: + get-intrinsic@1.2.7: dependencies: call-bind-apply-helpers: 1.0.1 - dunder-proto: 1.0.1 es-define-property: 1.0.1 es-errors: 1.3.0 es-object-atoms: 1.0.0 function-bind: 1.1.2 + get-proto: 1.0.1 gopd: 1.2.0 has-symbols: 1.1.0 hasown: 2.0.2 math-intrinsics: 1.1.0 + get-proto@1.0.1: + dependencies: + dunder-proto: 1.0.1 + es-object-atoms: 1.0.0 + get-stream@6.0.1: {} get-tsconfig@4.8.1: @@ -4124,7 +4133,7 @@ snapshots: '@types/glob': 7.2.0 array-union: 2.1.0 dir-glob: 3.0.1 - fast-glob: 3.3.2 + fast-glob: 3.3.3 glob: 7.2.3 ignore: 5.3.2 merge2: 1.4.1 @@ -4472,7 +4481,7 @@ snapshots: mcl-wasm@1.8.0: dependencies: - '@types/node': 20.17.11 + '@types/node': 20.17.12 md5.js@1.3.5: dependencies: @@ -4529,7 +4538,7 @@ snapshots: mnemonist@0.38.5: dependencies: - obliterator: 2.0.4 + obliterator: 2.0.5 mocha@10.8.2: dependencies: @@ -4597,7 +4606,7 @@ snapshots: object-inspect@1.13.3: {} - obliterator@2.0.4: {} + obliterator@2.0.5: {} once@1.4.0: dependencies: @@ -4870,14 +4879,14 @@ snapshots: dependencies: call-bound: 1.0.3 es-errors: 1.3.0 - get-intrinsic: 1.2.6 + get-intrinsic: 1.2.7 object-inspect: 1.13.3 side-channel-weakmap@1.0.2: dependencies: call-bound: 1.0.3 es-errors: 1.3.0 - get-intrinsic: 1.2.6 + get-intrinsic: 1.2.7 object-inspect: 1.13.3 side-channel-map: 1.0.1 diff --git a/src/SemaphoreMSAValidator.sol b/src/SemaphoreMSAValidator.sol index 436cac9..5a63779 100644 --- a/src/SemaphoreMSAValidator.sol +++ b/src/SemaphoreMSAValidator.sol @@ -2,10 +2,7 @@ pragma solidity >=0.8.23 <=0.8.29; import { ERC7579ValidatorBase } from "modulekit/Modules.sol"; -import { - VALIDATION_SUCCESS, - VALIDATION_FAILED -} from "modulekit/accounts/common/interfaces/IERC7579Module.sol"; +import { VALIDATION_SUCCESS } from "modulekit/accounts/common/interfaces/IERC7579Module.sol"; import { PackedUserOperation } from "modulekit/external/ERC4337.sol"; import { LibSort } from "solady/utils/LibSort.sol"; import { LibBytes } from "solady/utils/LibBytes.sol"; @@ -24,26 +21,14 @@ contract SemaphoreMSAValidator is ERC7579ValidatorBase { uint8 internal constant CMT_BYTELEN = 32; // Ensure the following match with the 3 function calls. - bytes4[4] internal ALLOWED_SELECTORS = [ - bytes4( - abi.encodeCall( - this.initiateTx, - ("", ISemaphore.SemaphoreProof(0, 0, 0, 0, 0, [uint256(0), 0, 0, 0, 0, 0, 0, 0]), false) - ) - ), - bytes4( - abi.encodeCall( - this.signTx, - ("", ISemaphore.SemaphoreProof(0, 0, 0, 0, 0, [uint256(0), 0, 0, 0, 0, 0, 0, 0]), false) - ) - ), - bytes4(abi.encodeCall(this.executeTx, (""))), - bytes4("") // this is for native token transfer - ]; - - struct CallDataCount { + bytes4[3] internal ALLOWED_SELECTORS = + [this.initiateTx.selector, this.signTx.selector, this.executeTx.selector]; + + struct ExtCallCount { + address targetAddr; bytes callData; - uint8 sigCount; + uint256 value; + uint8 count; } // Errors @@ -57,10 +42,12 @@ contract SemaphoreMSAValidator is ERC7579ValidatorBase { error TxHasBeenInitiated(address account, bytes32 txHash); error TxHashNotFound(address account, bytes32 txHash); error ThresholdNotReach(address account, uint8 threshold, uint8 current); - error TxAndProofDontMatch(address account, bytes32 txHash); error InvalidInstallData(); error InvalidSignatureLen(address account, uint256 len); error InvalidSignature(address account, bytes signature); + error InvalidSemaphoreProof(bytes reason); + error NonAllowedSelector(address account, bytes4 funcSel); + error NonValidatorCallBanned(address targetAddr, address selfAddr); // Events event ModuleInitialized(address indexed account); @@ -68,7 +55,7 @@ contract SemaphoreMSAValidator is ERC7579ValidatorBase { event AddedMember(address indexed, uint256 indexed commitment); event RemovedMember(address indexed, uint256 indexed commitment); event ThresholdSet(address indexed account, uint8 indexed threshold); - event InitiatedTx(address indexed account, bytes32 indexed txHash); + event InitiatedTx(address indexed account, uint256 indexed seq, bytes32 indexed txHash); event SignedTx(address indexed account, bytes32 indexed txHash); event ExecutedTx(address indexed account, bytes32 indexed txHash); @@ -81,7 +68,7 @@ contract SemaphoreMSAValidator is ERC7579ValidatorBase { mapping(address account => uint8 threshold) public thresholds; // smart account -> hash(call(params)) -> valid proof count - mapping(address account => mapping(bytes32 txHash => CallDataCount callDataCount)) public + mapping(address account => mapping(bytes32 txHash => ExtCallCount callDataCount)) public acctTxCount; // keep track of seqNum of txs that require threshold signature @@ -214,14 +201,19 @@ contract SemaphoreMSAValidator is ERC7579ValidatorBase { } function initiateTx( - bytes calldata callData, + address targetAddr, + bytes calldata txCallData, ISemaphore.SemaphoreProof calldata proof, bool execute ) external + payable moduleInstalled returns (bytes32 txHash) { + // console.log("initiateTx targetAddr: %s", targetAddr); + // console.logBytes(txCallData); + // retrieve the group ID address account = msg.sender; uint256 groupId = groupMapping[account]; @@ -229,31 +221,28 @@ contract SemaphoreMSAValidator is ERC7579ValidatorBase { // By this point, txParams should be validated. // combine the txParams with the account nonce and compute its hash uint256 seq = acctSeqNum[account]; - txHash = keccak256(abi.encode(callData, seq)); - - // Check: validate the proof is related to the callData - if (groupId != proof.scope || uint256(txHash) != proof.message) { - revert TxAndProofDontMatch(account, txHash); + txHash = keccak256(abi.encodePacked(seq, targetAddr, txCallData)); + + ExtCallCount storage ecc = acctTxCount[account][txHash]; + if (ecc.count != 0) revert TxHasBeenInitiated(account, txHash); + + // finally, check semaphore proof + try semaphore.validateProof(groupId, proof) { + // By this point, the proof also passed semaphore check. Start writing to the storage + acctSeqNum[account] += 1; + ecc.targetAddr = targetAddr; + ecc.callData = txCallData; + ecc.value = msg.value; + ecc.count = 1; + + emit InitiatedTx(account, seq, txHash); + // execute the transaction if condition allows + if (execute && ecc.count >= thresholds[account]) executeTx(txHash); + } catch Error(string memory reason) { + revert InvalidSemaphoreProof(bytes(reason)); + } catch (bytes memory reason) { + revert InvalidSemaphoreProof(reason); } - - CallDataCount storage cdc = acctTxCount[account][txHash]; - if (cdc.sigCount != 0) revert TxHasBeenInitiated(account, txHash); - - // the callData and proof should have some kind of inherent relationship - semaphore.validateProof(groupId, proof); - - //TODO: how do you handle plain chain native tokens transfer? - - // By this point, the proof also passed semaphore check. - // Start writing to the storage - acctSeqNum[account] += 1; - cdc.callData = callData; - cdc.sigCount = 1; - - emit InitiatedTx(account, txHash); - - // execute the transaction if condition allows - if (execute && cdc.sigCount >= thresholds[account]) executeTx(txHash); } function signTx( @@ -268,19 +257,17 @@ contract SemaphoreMSAValidator is ERC7579ValidatorBase { address account = msg.sender; uint256 groupId = groupMapping[account]; - if (proof.scope != groupId) revert TxAndProofDontMatch(account, txHash); - // Check if the txHash exist - CallDataCount storage cdc = acctTxCount[account][txHash]; - if (cdc.sigCount == 0) revert TxHashNotFound(account, txHash); + ExtCallCount storage cdc = acctTxCount[account][txHash]; + if (cdc.count == 0) revert TxHashNotFound(account, txHash); semaphore.validateProof(groupId, proof); - cdc.sigCount += 1; + cdc.count += 1; emit SignedTx(account, txHash); // execute the transaction if condition allows - if (execute && cdc.sigCount >= thresholds[account]) executeTx(txHash); + if (execute && cdc.count >= thresholds[account]) executeTx(txHash); } function executeTx(bytes32 txHash) public moduleInstalled { @@ -288,10 +275,10 @@ contract SemaphoreMSAValidator is ERC7579ValidatorBase { address account = msg.sender; uint256 groupId = groupMapping[account]; uint8 threshold = thresholds[account]; - CallDataCount storage cdc = acctTxCount[account][txHash]; + ExtCallCount storage cdc = acctTxCount[account][txHash]; - if (cdc.sigCount == 0) revert TxHashNotFound(account, txHash); - if (cdc.sigCount < threshold) revert ThresholdNotReach(account, threshold, cdc.sigCount); + if (cdc.count == 0) revert TxHashNotFound(account, txHash); + if (cdc.count < threshold) revert ThresholdNotReach(account, threshold, cdc.count); //TODO: make the actual contract call here @@ -342,6 +329,10 @@ contract SemaphoreMSAValidator is ERC7579ValidatorBase { if (!groups.hasMember(groupId, cmt)) revert MemberNotExists(account, cmt); + // We don't allow call to other contract. + address targetAddr = address(bytes20(userOp.callData[100:120])); + if (targetAddr != address(this)) revert NonValidatorCallBanned(targetAddr, address(this)); + // For callData, the first 120 bytes are reserved by ERC-7579 use. Then 32 bytes of value, // then the remaining as the callData passed in getExecOps bytes memory valAndCallData = userOp.callData[120:]; @@ -354,11 +345,9 @@ contract SemaphoreMSAValidator is ERC7579ValidatorBase { // Allow only these few types on function calls to pass, and reject all other on-chain // calls. They must be executed via `executeTx()` function. - if (_isAllowedSelector(funcSel)) { - // TODO: How to handle native token transfer? - return VALIDATION_SUCCESS; - } - return VALIDATION_FAILED; + if (_isAllowedSelector(funcSel)) return VALIDATION_SUCCESS; + + revert NonAllowedSelector(account, funcSel); } function isValidSignatureWithSender( diff --git a/test/SemaphoreMSAValidator.t.sol b/test/SemaphoreMSAValidator.t.sol index e7c60ab..699f99f 100644 --- a/test/SemaphoreMSAValidator.t.sol +++ b/test/SemaphoreMSAValidator.t.sol @@ -15,7 +15,6 @@ import { import { IERC7579Module, IERC7579Validator } from "modulekit/Modules.sol"; import { VALIDATION_SUCCESS, - VALIDATION_FAILED, MODULE_TYPE_VALIDATOR } from "modulekit/accounts/common/interfaces/IERC7579Module.sol"; import { PackedUserOperation } from "modulekit/external/ERC4337.sol"; @@ -35,9 +34,11 @@ import { SemaphoreMSAValidator, ERC7579ValidatorBase } from "../src/SemaphoreMSA import { getEmptyUserOperation, getEmptySemaphoreProof, + getTestUserOpCallData, Identity, IdentityLib } from "./utils/TestUtils.sol"; +import { SimpleContract } from "./utils/SimpleContract.sol"; import { LibSort } from "solady/utils/LibSort.sol"; struct User { @@ -53,6 +54,7 @@ contract SemaphoreValidatorUnitTest is RhinestoneModuleKit, Test { AccountInstance internal smartAcct; SemaphoreMSAValidator internal semaphoreValidator; + SimpleContract internal simpleContract; User[] internal $users; function setUp() public virtual { @@ -80,7 +82,7 @@ contract SemaphoreValidatorUnitTest is RhinestoneModuleKit, Test { $users.push(User({ sk: sk, addr: addr, identity: IdentityLib.genIdentity(2) })); } - modifier setupSmartAcctOneUser() { + modifier setupSmartAcctOneMember() { // Create the smart account and install the validator with one admin smartAcct = makeAccountInstance("SemaphoreMSAValidator"); vm.deal(smartAcct.account, 10 ether); @@ -95,7 +97,12 @@ contract SemaphoreValidatorUnitTest is RhinestoneModuleKit, Test { _; } - function test_onInstallWithOneUser() public setupSmartAcctOneUser { + modifier deploySimpleContract() { + simpleContract = new SimpleContract(0); + _; + } + + function test_onInstallWithOneMember() public setupSmartAcctOneMember { assertEq(semaphoreValidator.groupMapping(smartAcct.account), 0); assertEq(semaphoreValidator.thresholds(smartAcct.account), 1); assertEq(semaphoreValidator.memberCount(smartAcct.account), 1); @@ -115,7 +122,7 @@ contract SemaphoreValidatorUnitTest is RhinestoneModuleKit, Test { }); } - function test_duplicateInstall() public setupSmartAcctOneUser { + function test_duplicateInstall() public setupSmartAcctOneMember { // The modifier has already installed the validator in smartAcct uint256[] memory cmts = new uint256[](1); cmts[0] = $users[0].identity.commitment(); @@ -129,15 +136,21 @@ contract SemaphoreValidatorUnitTest is RhinestoneModuleKit, Test { }); } - function test_onUninstall() public setupSmartAcctOneUser { + function test_onUninstall() public setupSmartAcctOneMember { revert("to be implemented"); } - function test_validateUserOpWithNonMember() public setupSmartAcctOneUser { + // Test only validateUserOp() + function test_validateUserOpWithNonMember() public setupSmartAcctOneMember { PackedUserOperation memory userOp = getEmptyUserOperation(); userOp.sender = smartAcct.account; - userOp.callData = - abi.encodeCall(SemaphoreMSAValidator.initiateTx, ("", getEmptySemaphoreProof(), false)); + userOp.callData = getTestUserOpCallData( + 0, + address(semaphoreValidator), + abi.encodeCall( + SemaphoreMSAValidator.initiateTx, (address(0), "", getEmptySemaphoreProof(), false) + ) + ); bytes32 userOpHash = bytes32(keccak256("userOpHash")); @@ -155,11 +168,16 @@ contract SemaphoreValidatorUnitTest is RhinestoneModuleKit, Test { ); } - function test_validateUserOpWithMember() public setupSmartAcctOneUser { + function test_validateUserOpWithMember() public setupSmartAcctOneMember { PackedUserOperation memory userOp = getEmptyUserOperation(); userOp.sender = smartAcct.account; - userOp.callData = - abi.encodeCall(SemaphoreMSAValidator.initiateTx, ("", getEmptySemaphoreProof(), false)); + userOp.callData = getTestUserOpCallData( + 0, + address(semaphoreValidator), + abi.encodeCall( + SemaphoreMSAValidator.initiateTx, (address(0), "", getEmptySemaphoreProof(), false) + ) + ); bytes32 userOpHash = bytes32(keccak256("userOpHash")); @@ -173,12 +191,14 @@ contract SemaphoreValidatorUnitTest is RhinestoneModuleKit, Test { assertEq(validationData, VALIDATION_SUCCESS); } - function test_initiateNativeTransferOneUserInvalidSignature() public setupSmartAcctOneUser { + function test_initiateTokensTransferInvalidSignature() public setupSmartAcctOneMember { User storage recipient = $users[1]; UserOpData memory userOpData = smartAcct.getExecOps({ - target: recipient.addr, - value: 1, - callData: "", + target: address(semaphoreValidator), + value: 1 ether, + callData: abi.encodeCall( + SemaphoreMSAValidator.initiateTx, (recipient.addr, "", getEmptySemaphoreProof(), false) + ), txValidator: address(semaphoreValidator) }); @@ -187,43 +207,72 @@ contract SemaphoreValidatorUnitTest is RhinestoneModuleKit, Test { forgedSgn[forgedSgn.length - 2] = hex"ff"; userOpData.userOp.signature = forgedSgn; - // TODO: checking with Konrad Rhinestone on this - smartAcct.expect4337Revert( /* SemaphoreMSAValidator.InvalidSignature.selector */ ); + smartAcct.expect4337Revert(SemaphoreMSAValidator.InvalidSignature.selector); userOpData.execUserOps(); } - function test_initiateNativeTransferOneUserNonMember() public setupSmartAcctOneUser { + function test_initiateTokensTransferMemberInvalidSemaphoreProof() + public + setupSmartAcctOneMember + { + User storage member = $users[0]; User storage recipient = $users[1]; UserOpData memory userOpData = smartAcct.getExecOps({ - target: recipient.addr, - value: 1, - callData: "", + target: address(semaphoreValidator), + value: 1 ether, + callData: abi.encodeCall( + SemaphoreMSAValidator.initiateTx, (recipient.addr, "", getEmptySemaphoreProof(), false) + ), txValidator: address(semaphoreValidator) }); - userOpData.userOp.signature = recipient.identity.signHash(userOpData.userOpHash); + userOpData.userOp.signature = member.identity.signHash(userOpData.userOpHash); - // TODO: checking with Konrad Rhinestone on this - smartAcct.expect4337Revert( /* SemaphoreMSAValidator.MemberNotExists.selector */ ); + smartAcct.expect4337Revert(SemaphoreMSAValidator.InvalidSemaphoreProof.selector); userOpData.execUserOps(); } - function test_initiateNativeTransferOneUserMember() public setupSmartAcctOneUser { + function test_initiateTxOneMemberNonValidatorCall() + public + setupSmartAcctOneMember + deploySimpleContract + { User storage member = $users[0]; - User storage recipient = $users[1]; + uint256 testVal = 7; UserOpData memory userOpData = smartAcct.getExecOps({ - target: recipient.addr, - value: 1 ether, - callData: "", + target: address(simpleContract), + value: 0, + callData: abi.encodeCall(SimpleContract.setVal, (testVal)), txValidator: address(semaphoreValidator) }); userOpData.userOp.signature = member.identity.signHash(userOpData.userOpHash); + smartAcct.expect4337Revert(SemaphoreMSAValidator.NonValidatorCallBanned.selector); userOpData.execUserOps(); } - function test_initiateTxExecuteOneUserMember() public setupSmartAcctOneUser { - revert("to be implemented"); + function test_initiateTxOneMemberAllowedSelectorInvalidSemaphoreProof() + public + setupSmartAcctOneMember + deploySimpleContract + { + User storage member = $users[0]; + uint256 testVal = 7; + + bytes memory txCallData = abi.encodeCall(SimpleContract.setVal, (testVal)); + UserOpData memory userOpData = smartAcct.getExecOps({ + target: address(semaphoreValidator), + value: 0, + callData: abi.encodeCall( + SemaphoreMSAValidator.initiateTx, + (address(simpleContract), txCallData, getEmptySemaphoreProof(), false) + ), + txValidator: address(semaphoreValidator) + }); + userOpData.userOp.signature = member.identity.signHash(userOpData.userOpHash); + + smartAcct.expect4337Revert(SemaphoreMSAValidator.InvalidSemaphoreProof.selector); + userOpData.execUserOps(); } } diff --git a/test/utils/SimpleContract.sol b/test/utils/SimpleContract.sol new file mode 100644 index 0000000..6dec3b2 --- /dev/null +++ b/test/utils/SimpleContract.sol @@ -0,0 +1,15 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.23; + +contract SimpleContract { + uint256 val; + + constructor(uint256 _val) { + val = _val; + } + + function setVal(uint256 newVal) public returns (uint256) { + val = newVal; + return val; + } +} diff --git a/test/utils/TestUtils.sol b/test/utils/TestUtils.sol index 2e6c541..d8cd1fe 100644 --- a/test/utils/TestUtils.sol +++ b/test/utils/TestUtils.sol @@ -38,6 +38,17 @@ function getEmptySemaphoreProof() pure returns (ISemaphore.SemaphoreProof memory }); } +function getTestUserOpCallData( + uint256 value, + address targetAddr, + bytes memory txCallData +) + pure + returns (bytes memory callData) +{ + callData = bytes.concat(new bytes(100), bytes20(targetAddr), bytes32(value), txCallData); +} + type Identity is bytes32; library IdentityLib {