Skip to content

Commit e8bc0bc

Browse files
jimmychu0807jimmychu0807
authored
feat: added tests for native token transfer (#14)
* to actually execute the tx * Added test to test token transfer
1 parent fec2771 commit e8bc0bc

File tree

3 files changed

+165
-36
lines changed

3 files changed

+165
-36
lines changed

‎src/SemaphoreMSAValidator.sol

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,7 @@ pragma solidity >=0.8.23 <=0.8.29;
44
import { ERC7579ValidatorBase } from "modulekit/Modules.sol";
55
import { VALIDATION_SUCCESS } from "modulekit/accounts/common/interfaces/IERC7579Module.sol";
66
import { PackedUserOperation } from "modulekit/external/ERC4337.sol";
7-
import { LibSort } from "solady/utils/LibSort.sol";
8-
import { LibBytes } from "solady/utils/LibBytes.sol";
7+
import { LibSort, LibBytes } from "solady/Milady.sol";
98

109
import { ISemaphore, ISemaphoreGroups } from "./utils/Semaphore.sol";
1110
import { ValidatorLibBytes } from "./utils/ValidatorLibBytes.sol";
@@ -48,6 +47,9 @@ contract SemaphoreMSAValidator is ERC7579ValidatorBase {
4847
error InvalidSemaphoreProof(bytes reason);
4948
error NonAllowedSelector(address account, bytes4 funcSel);
5049
error NonValidatorCallBanned(address targetAddr, address selfAddr);
50+
error InitiateTxWithNullAddress(address account);
51+
error InitiateTxWithNullCallDataAndNullValue(address account, address targetAddr);
52+
error ExecuteTxFailure(address account, address targetAddr, uint256 value, bytes callData);
5153

5254
// Events
5355
event ModuleInitialized(address indexed account);
@@ -196,11 +198,11 @@ contract SemaphoreMSAValidator is ERC7579ValidatorBase {
196198
emit RemovedMember(account, rmOwner);
197199
}
198200

199-
function getNextSeqNum(address account) external returns (uint256) {
201+
function getNextSeqNum(address account) external view returns (uint256) {
200202
return acctSeqNum[account];
201203
}
202204

203-
function getGroupId(address account) external returns (bool, uint256) {
205+
function getGroupId(address account) external view returns (bool, uint256) {
204206
uint256 groupId = groupMapping[account];
205207
if (thresholds[account] == 0) return (false, 0);
206208
return (true, groupId);
@@ -221,6 +223,14 @@ contract SemaphoreMSAValidator is ERC7579ValidatorBase {
221223
address account = msg.sender;
222224
uint256 groupId = groupMapping[account];
223225

226+
// Check:
227+
// 1. targetAddr cannot be 0
228+
// 2. if txCallData is blank, then msg.value must be > 0, else revert
229+
if (targetAddr == address(0)) revert InitiateTxWithNullAddress(account);
230+
if (LibBytes.cmp(txCallData, "") == 0 && msg.value == 0) {
231+
revert InitiateTxWithNullCallDataAndNullValue(account, targetAddr);
232+
}
233+
224234
// By this point, txParams should be validated.
225235
// combine the txParams with the account nonce and compute its hash
226236
uint256 seq = acctSeqNum[account];
@@ -273,22 +283,29 @@ contract SemaphoreMSAValidator is ERC7579ValidatorBase {
273283
if (execute && cdc.count >= thresholds[account]) executeTx(txHash);
274284
}
275285

276-
function executeTx(bytes32 txHash) public moduleInstalled {
286+
function executeTx(bytes32 txHash) public moduleInstalled returns (bytes memory) {
277287
// retrieve the group ID
278288
address account = msg.sender;
279-
uint256 groupId = groupMapping[account];
280289
uint8 threshold = thresholds[account];
281-
ExtCallCount storage cdc = acctTxCount[account][txHash];
290+
ExtCallCount storage ecc = acctTxCount[account][txHash];
282291

283-
if (cdc.count == 0) revert TxHashNotFound(account, txHash);
284-
if (cdc.count < threshold) revert ThresholdNotReach(account, threshold, cdc.count);
292+
// console.log("executeTx");
293+
if (ecc.count == 0) revert TxHashNotFound(account, txHash);
294+
if (ecc.count < threshold) revert ThresholdNotReach(account, threshold, ecc.count);
285295

286-
//TODO: make the actual contract call here
296+
// console.log("executeTx - check pass");
297+
// REVIEW: Is there a better way to make external contract call given the target address,
298+
// value, and call data.
299+
address payable targetAddr = payable(ecc.targetAddr);
300+
(bool success, bytes memory returnData) = targetAddr.call{ value: ecc.value }(ecc.callData);
301+
if (!success) revert ExecuteTxFailure(account, targetAddr, ecc.value, ecc.callData);
287302

288303
emit ExecutedTx(account, txHash);
289304

290305
// Clean up the storage
291306
delete acctTxCount[account][txHash];
307+
308+
return returnData;
292309
}
293310

294311
/**
@@ -339,7 +356,7 @@ contract SemaphoreMSAValidator is ERC7579ValidatorBase {
339356
// For callData, the first 120 bytes are reserved by ERC-7579 use. Then 32 bytes of value,
340357
// then the remaining as the callData passed in getExecOps
341358
bytes memory valAndCallData = userOp.callData[120:];
342-
(uint256 val) = abi.decode(LibBytes.slice(valAndCallData, 0, 32), (uint256));
359+
// (uint256 val) = abi.decode(LibBytes.slice(valAndCallData, 0, 32), (uint256));
343360
bytes4 funcSel = bytes4(LibBytes.slice(valAndCallData, 32, 36));
344361

345362
// console.log("val: %s", val);

‎test/SemaphoreMSAValidator.t.sol

Lines changed: 132 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// SPDX-License-Identifier: MIT
22
pragma solidity ^0.8.23;
33

4-
// forge
4+
// forge-std
55
import { Test } from "forge-std/Test.sol";
66
// import { console } from "forge-std/console.sol";
77

@@ -155,7 +155,7 @@ contract SemaphoreValidatorUnitTest is RhinestoneModuleKit, Test {
155155
assertEq(semaphoreValidator.memberCount(smartAcct.account), 0);
156156
assertEq(semaphoreValidator.isInitialized(smartAcct.account), false);
157157

158-
(bool bExist, uint256 groupId) = semaphoreValidator.getGroupId(smartAcct.account);
158+
(bool bExist,) = semaphoreValidator.getGroupId(smartAcct.account);
159159
assertEq(bExist, false);
160160
}
161161

@@ -250,47 +250,155 @@ contract SemaphoreValidatorUnitTest is RhinestoneModuleKit, Test {
250250
userOpData.execUserOps();
251251
}
252252

253-
function test_initiateTokensTransferMemberValid() public setupSmartAcctOneMember {
253+
function test_initiateTokensTransferMemberValid()
254+
public
255+
setupSmartAcctOneMember
256+
returns (bytes32 txHash)
257+
{
254258
User storage member = $users[0];
255259

256260
uint256 value = 1 ether;
257261
address targetAddr = $users[1].addr;
258262
uint256 seq = semaphoreValidator.getNextSeqNum(smartAcct.account);
263+
txHash = keccak256(abi.encodePacked(seq, targetAddr, value, ""));
264+
265+
{
266+
// Using scope to limit the number of local variables, work around the `stack too deep`
267+
// error.
268+
// Generate the semaphore proof
269+
(bool bExist, uint256 groupId) = semaphoreValidator.getGroupId(smartAcct.account);
270+
assert(bExist);
271+
uint256[] memory members = new uint256[](1);
272+
members[0] = member.identity.commitment();
273+
ISemaphore.SemaphoreProof memory smProof =
274+
member.identity.generateSempahoreProof(groupId, members, txHash);
275+
276+
// Composing the UserOpData
277+
UserOpData memory userOpData = smartAcct.getExecOps({
278+
target: address(semaphoreValidator),
279+
value: value,
280+
callData: abi.encodeCall(
281+
SemaphoreMSAValidator.initiateTx, (targetAddr, "", smProof, false)
282+
),
283+
txValidator: address(semaphoreValidator)
284+
});
285+
userOpData.userOp.signature = member.identity.signHash(userOpData.userOpHash);
286+
287+
// Expecting `InitiatedTx` event to be emitted
288+
vm.expectEmit(true, true, true, true, address(semaphoreValidator));
289+
emit SemaphoreMSAValidator.InitiatedTx(smartAcct.account, seq, txHash);
290+
userOpData.execUserOps();
291+
}
259292

260-
bytes32 txHash = keccak256(abi.encodePacked(seq, targetAddr, value, ""));
293+
// Test the states are changed accordingly
294+
assertEq(semaphoreValidator.acctSeqNum(smartAcct.account), seq + 1);
261295

262-
// Generate the semaphore proof
263-
(bool bExist, uint256 groupId) = semaphoreValidator.getGroupId(smartAcct.account);
264-
assert(bExist);
265-
uint256[] memory members = new uint256[](1);
266-
members[0] = member.identity.commitment();
267-
ISemaphore.SemaphoreProof memory smProof =
268-
member.identity.generateSempahoreProof(groupId, members, txHash);
296+
(address eccTargetAddr, bytes memory eccCallData, uint256 eccValue, uint8 eccCount) =
297+
semaphoreValidator.acctTxCount(smartAcct.account, txHash);
298+
299+
assertEq(eccTargetAddr, targetAddr);
300+
assertEq(eccValue, value);
301+
assertEq(eccCallData, "");
302+
assertEq(eccCount, 1);
303+
}
304+
305+
function test_initiateTokensTransferMemberValidAndExecuteInvalidTxHash() public {
306+
bytes32 forgedHash = test_initiateTokensTransferMemberValid();
307+
// Changed the last 2 bytes to 0xffff
308+
forgedHash = forgedHash | bytes32(uint256(65_535));
309+
310+
User storage member = $users[0];
311+
312+
// Now execute the token transfer.
313+
// Composing the UserOpData.
314+
UserOpData memory userOpData = smartAcct.getExecOps({
315+
target: address(semaphoreValidator),
316+
value: 0,
317+
callData: abi.encodeCall(SemaphoreMSAValidator.executeTx, (forgedHash)),
318+
txValidator: address(semaphoreValidator)
319+
});
320+
userOpData.userOp.signature = member.identity.signHash(userOpData.userOpHash);
321+
322+
smartAcct.expect4337Revert(
323+
abi.encodeWithSelector(
324+
SemaphoreMSAValidator.TxHashNotFound.selector, smartAcct.account, forgedHash
325+
)
326+
);
327+
userOpData.execUserOps();
328+
}
329+
330+
function test_ExecuteTxFailure() public pure {
331+
revert("to be implemented");
332+
}
269333

270-
// Composing the UserOpData
334+
function test_initiateTokensTransferMemberValidAndExecuteValid() public {
335+
bytes32 txHash = test_initiateTokensTransferMemberValid();
336+
337+
User storage member = $users[0];
338+
address targetAddr = $users[1].addr;
339+
uint256 value = 1 ether;
340+
uint256 beforeBalance = targetAddr.balance;
341+
342+
// Now execute the token transfer.
343+
// Composing the UserOpData.
271344
UserOpData memory userOpData = smartAcct.getExecOps({
272345
target: address(semaphoreValidator),
273-
value: value,
274-
callData: abi.encodeCall(SemaphoreMSAValidator.initiateTx, (targetAddr, "", smProof, false)),
346+
value: 0,
347+
callData: abi.encodeCall(SemaphoreMSAValidator.executeTx, (txHash)),
275348
txValidator: address(semaphoreValidator)
276349
});
277350
userOpData.userOp.signature = member.identity.signHash(userOpData.userOpHash);
278351

279-
// Expecting `InitiatedTx` event to be emitted
352+
// Test event emission
280353
vm.expectEmit(true, true, true, true, address(semaphoreValidator));
281-
emit SemaphoreMSAValidator.InitiatedTx(smartAcct.account, seq, txHash);
354+
emit SemaphoreMSAValidator.ExecutedTx(smartAcct.account, txHash);
282355
userOpData.execUserOps();
283356

284-
// Test the states are changed accordingly
285-
assertEq(semaphoreValidator.acctSeqNum(smartAcct.account), seq + 1);
357+
uint256 afterBalance = targetAddr.balance;
358+
assertEq(afterBalance - beforeBalance, value);
359+
}
286360

287-
(address eccTargetAddr, bytes memory eccCallData, uint256 eccValue, uint8 eccCount) =
288-
semaphoreValidator.acctTxCount(smartAcct.account, txHash);
361+
function test_initiateTokensTransferMemberAndExecuteValid() public setupSmartAcctOneMember {
362+
User storage member = $users[0];
363+
uint256 value = 1 ether;
364+
address targetAddr = $users[1].addr;
365+
uint256 beforeBalance = targetAddr.balance;
366+
uint256 seq = semaphoreValidator.getNextSeqNum(smartAcct.account);
367+
bytes32 txHash = keccak256(abi.encodePacked(seq, targetAddr, value, ""));
289368

290-
assertEq(eccTargetAddr, targetAddr);
291-
assertEq(eccValue, value);
292-
assertEq(eccCallData, "");
293-
assertEq(eccCount, 1);
369+
{
370+
// Using scope to limit the number of local variables, work around the `stack too deep`
371+
// error.
372+
// Generate the semaphore proof
373+
(bool bExist, uint256 groupId) = semaphoreValidator.getGroupId(smartAcct.account);
374+
assert(bExist);
375+
uint256[] memory members = new uint256[](1);
376+
members[0] = member.identity.commitment();
377+
ISemaphore.SemaphoreProof memory smProof =
378+
member.identity.generateSempahoreProof(groupId, members, txHash);
379+
380+
// Composing the UserOpData
381+
UserOpData memory userOpData = smartAcct.getExecOps({
382+
target: address(semaphoreValidator),
383+
value: value,
384+
callData: abi.encodeCall(
385+
SemaphoreMSAValidator.initiateTx, (targetAddr, "", smProof, true)
386+
),
387+
txValidator: address(semaphoreValidator)
388+
});
389+
userOpData.userOp.signature = member.identity.signHash(userOpData.userOpHash);
390+
391+
// Expecting `InitiatedTx` event to be emitted
392+
vm.expectEmit(true, true, true, true, address(semaphoreValidator));
393+
emit SemaphoreMSAValidator.InitiatedTx(smartAcct.account, seq, txHash);
394+
vm.expectEmit(true, true, true, true, address(semaphoreValidator));
395+
emit SemaphoreMSAValidator.ExecutedTx(smartAcct.account, txHash);
396+
userOpData.execUserOps();
397+
}
398+
399+
// Confirm user balance has changed
400+
uint256 afterBalance = targetAddr.balance;
401+
assertEq(afterBalance - beforeBalance, value);
294402
}
295403

296404
function test_initiateTxOneMemberNonValidatorCall()

‎test/utils/TestUtils.sol

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,11 @@ library IdentityLib {
124124
});
125125
}
126126

127-
function _uint256ArrToString(uint256[] memory arr) internal returns (string memory retStr) {
127+
function _uint256ArrToString(uint256[] memory arr)
128+
internal
129+
pure
130+
returns (string memory retStr)
131+
{
128132
for (uint256 i = 0; i < arr.length; i++) {
129133
if (i == arr.length - 1) {
130134
retStr = string.concat(retStr, LibString.toString(arr[i]));

0 commit comments

Comments
 (0)