-
Notifications
You must be signed in to change notification settings - Fork 0
test: add Moloch DAO-style systematic test suite (104 tests) #29
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,134 @@ | ||||||||||||||||||||||||||||||||||
| # IRSB Protocol Test Suite | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| ## Testing Philosophy | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| IRSB follows a **Moloch DAO-inspired testing methodology** combined with Foundry-native patterns: | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| 1. **Trigger every require/revert** - Every revert path in every contract has a dedicated test | ||||||||||||||||||||||||||||||||||
| 2. **Test every modifier** - Each custom modifier has allow/reject test pairs | ||||||||||||||||||||||||||||||||||
| 3. **Verify all state transitions** - Post-condition assertions check ALL affected fields, not just the obvious ones | ||||||||||||||||||||||||||||||||||
| 4. **Test boundary conditions** - Systematic 0, 1, MAX-1, MAX, MAX+1 for every numeric parameter | ||||||||||||||||||||||||||||||||||
| 5. **Fuzz for invariants** - Property-based testing for protocol-wide invariants | ||||||||||||||||||||||||||||||||||
| 6. **Security regressions** - Named tests for every discovered vulnerability (IRSB-SEC-NNN) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| ## Directory Structure | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||||||||||||
| test/ | ||||||||||||||||||||||||||||||||||
| ├── SolverRegistry.t.sol # Core unit tests + security regressions | ||||||||||||||||||||||||||||||||||
| ├── IntentReceiptHub.t.sol # Receipt lifecycle + challenger bonds | ||||||||||||||||||||||||||||||||||
| ├── DisputeModule.t.sol # Evidence, escalation, arbitration | ||||||||||||||||||||||||||||||||||
| ├── EscrowVault.t.sol # Native ETH escrow lifecycle | ||||||||||||||||||||||||||||||||||
| ├── EscrowVaultERC20.t.sol # ERC20 escrow lifecycle | ||||||||||||||||||||||||||||||||||
| ├── WalletDelegate.t.sol # EIP-7702 delegation + execution | ||||||||||||||||||||||||||||||||||
| ├── X402Facilitator.t.sol # x402 payment settlement | ||||||||||||||||||||||||||||||||||
| ├── OptimisticDispute.t.sol # Counter-bond dispute resolution | ||||||||||||||||||||||||||||||||||
| ├── ReceiptV2Extension.t.sol # Dual attestation receipts | ||||||||||||||||||||||||||||||||||
| ├── ERC8004Adapter.t.sol # Validation signal adapter | ||||||||||||||||||||||||||||||||||
| ├── ERC8004Integration.t.sol # End-to-end ERC-8004 flow | ||||||||||||||||||||||||||||||||||
| ├── CredibilityRegistry.t.sol # Credibility tracking | ||||||||||||||||||||||||||||||||||
| ├── AcrossAdapter.t.sol # Across bridge integration | ||||||||||||||||||||||||||||||||||
| │ | ||||||||||||||||||||||||||||||||||
| ├── moloch/ # Moloch DAO-style systematic tests | ||||||||||||||||||||||||||||||||||
| │ ├── RequireAudit.t.sol # Every untested revert path (~43 tests) | ||||||||||||||||||||||||||||||||||
| │ ├── StateTransitions.t.sol # Comprehensive state verification (~10 tests) | ||||||||||||||||||||||||||||||||||
| │ ├── BoundaryTests.t.sol # 0/1/MAX boundary conditions (~27 tests) | ||||||||||||||||||||||||||||||||||
| │ └── ModifierTests.t.sol # Modifier allow/reject pairs (~17 tests) | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+33
to
+36
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The test counts mentioned in the
Please update these counts to be accurate for better documentation clarity.
Suggested change
Comment on lines
+33
to
+36
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Update the Moloch test counts to match the new suite size. 📝 Suggested change-│ ├── RequireAudit.t.sol # Every untested revert path (~43 tests)
-│ ├── StateTransitions.t.sol # Comprehensive state verification (~10 tests)
-│ ├── BoundaryTests.t.sol # 0/1/MAX boundary conditions (~27 tests)
-│ └── ModifierTests.t.sol # Modifier allow/reject pairs (~17 tests)
+│ ├── RequireAudit.t.sol # Every untested revert path (~51 tests)
+│ ├── StateTransitions.t.sol # Comprehensive state verification (~8 tests)
+│ ├── BoundaryTests.t.sol # 0/1/MAX boundary conditions (~28 tests)
+│ └── ModifierTests.t.sol # Modifier allow/reject pairs (~17 tests)📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
| │ | ||||||||||||||||||||||||||||||||||
| ├── enforcers/ # Caveat enforcer tests | ||||||||||||||||||||||||||||||||||
| │ ├── SpendLimitEnforcer.t.sol | ||||||||||||||||||||||||||||||||||
| │ ├── TimeWindowEnforcer.t.sol | ||||||||||||||||||||||||||||||||||
| │ ├── AllowedTargetsEnforcer.t.sol | ||||||||||||||||||||||||||||||||||
| │ ├── AllowedMethodsEnforcer.t.sol | ||||||||||||||||||||||||||||||||||
| │ └── NonceEnforcer.t.sol | ||||||||||||||||||||||||||||||||||
| │ | ||||||||||||||||||||||||||||||||||
| ├── fuzz/ # Fuzz tests (256 runs default, 10k in CI) | ||||||||||||||||||||||||||||||||||
| │ ├── SolverRegistryFuzz.t.sol | ||||||||||||||||||||||||||||||||||
| │ ├── IntentReceiptHubFuzz.t.sol | ||||||||||||||||||||||||||||||||||
| │ ├── EscrowVaultFuzz.t.sol | ||||||||||||||||||||||||||||||||||
| │ ├── ReceiptV2Fuzz.t.sol | ||||||||||||||||||||||||||||||||||
| │ ├── OptimisticDisputeFuzz.t.sol | ||||||||||||||||||||||||||||||||||
| │ ├── SpendLimitEnforcer.fuzz.t.sol | ||||||||||||||||||||||||||||||||||
| │ └── WalletDelegate.fuzz.t.sol | ||||||||||||||||||||||||||||||||||
| │ | ||||||||||||||||||||||||||||||||||
| ├── invariants/ # Invariant tests | ||||||||||||||||||||||||||||||||||
| │ ├── SolverRegistry.invariants.t.sol | ||||||||||||||||||||||||||||||||||
| │ ├── IntentReceiptHub.invariants.t.sol | ||||||||||||||||||||||||||||||||||
| │ └── DisputeModule.invariants.t.sol | ||||||||||||||||||||||||||||||||||
| │ | ||||||||||||||||||||||||||||||||||
| ├── helpers/ # Test utilities | ||||||||||||||||||||||||||||||||||
| │ ├── MockTarget.sol # Simple target for delegation tests | ||||||||||||||||||||||||||||||||||
| │ ├── MockETHRejecter.sol # Rejects ETH (tests transfer failures) | ||||||||||||||||||||||||||||||||||
| │ └── VerificationHelpers.sol # Reusable state-checking assertions | ||||||||||||||||||||||||||||||||||
| │ | ||||||||||||||||||||||||||||||||||
| └── security-exercise/ # Vulnerability demonstration | ||||||||||||||||||||||||||||||||||
| ├── Attacker.sol | ||||||||||||||||||||||||||||||||||
| ├── VulnerableVault.sol | ||||||||||||||||||||||||||||||||||
| ├── SecureVault.sol | ||||||||||||||||||||||||||||||||||
| └── VulnerableVault.t.sol | ||||||||||||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| ## Naming Conventions | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| | Category | Pattern | Example | | ||||||||||||||||||||||||||||||||||
| |----------|---------|---------| | ||||||||||||||||||||||||||||||||||
| | Core unit | `test_[FunctionName]` | `test_RegisterSolver` | | ||||||||||||||||||||||||||||||||||
| | Revert | `test_[FunctionName]_Revert[Reason]` | `test_DepositBond_RevertZeroAmount` | | ||||||||||||||||||||||||||||||||||
| | Require audit | `test_requireFail_[Contract]_[function]_[reason]` | `test_requireFail_SolverRegistry_slash_transferFailed` | | ||||||||||||||||||||||||||||||||||
| | Boundary | `test_boundary_[Contract]_[parameter]_[condition]` | `test_boundary_IntentReceiptHub_batchSize_max` | | ||||||||||||||||||||||||||||||||||
| | State transition | `test_stateTransition_[action]_[assertion]` | `test_stateTransition_depositBond_activationThreshold` | | ||||||||||||||||||||||||||||||||||
| | Modifier | `test_modifier_[name]_[allows\|rejects]_[who]` | `test_modifier_onlyOperator_rejects_nonOperator` | | ||||||||||||||||||||||||||||||||||
| | Security | `test_IRSB_SEC_NNN_[description]` | `test_IRSB_SEC_005_zeroSlashAmountReverts` | | ||||||||||||||||||||||||||||||||||
| | Fuzz | `testFuzz_[Action]([params])` | `testFuzz_DepositAndWithdraw(uint256 amount)` | | ||||||||||||||||||||||||||||||||||
| | Invariant | `invariant_[property]` | `invariant_totalBondedMatchesSum` | | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| ## Running Tests | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| ```bash | ||||||||||||||||||||||||||||||||||
| # All tests | ||||||||||||||||||||||||||||||||||
| forge test | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| # Specific category | ||||||||||||||||||||||||||||||||||
| forge test --match-path "test/moloch/*" # All Moloch-style tests | ||||||||||||||||||||||||||||||||||
| forge test --match-test "test_requireFail" # Require audit only | ||||||||||||||||||||||||||||||||||
| forge test --match-test "test_boundary" # Boundary tests only | ||||||||||||||||||||||||||||||||||
| forge test --match-test "test_stateTransition" # State transitions only | ||||||||||||||||||||||||||||||||||
| forge test --match-test "test_modifier" # Modifier pairs only | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| # Verbose (see revert messages) | ||||||||||||||||||||||||||||||||||
| forge test --match-path "test/moloch/*" -vvv | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| # Gas report | ||||||||||||||||||||||||||||||||||
| forge test --gas-report | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| # Fuzz (CI profile: 10k runs) | ||||||||||||||||||||||||||||||||||
| FOUNDRY_PROFILE=ci forge test --match-path "test/fuzz/*" | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| # Single test file | ||||||||||||||||||||||||||||||||||
| forge test --match-path "test/SolverRegistry.t.sol" | ||||||||||||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| ## Key Parameters | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| | Parameter | Value | Tested Boundaries | | ||||||||||||||||||||||||||||||||||
| |-----------|-------|-------------------| | ||||||||||||||||||||||||||||||||||
| | MINIMUM_BOND | 0.1 ETH | 0, 1 wei, MIN-1, MIN, MIN+1 | | ||||||||||||||||||||||||||||||||||
| | MAX_BATCH_SIZE | 50 | 0, 1, 50, 51 | | ||||||||||||||||||||||||||||||||||
| | CHALLENGE_WINDOW | 1 hour | 14m59s, 15m, 24h, 24h+1s | | ||||||||||||||||||||||||||||||||||
| | MAX_JAILS | 3 | Jail #2 (jailed), Jail #3 (banned) | | ||||||||||||||||||||||||||||||||||
| | WITHDRAWAL_COOLDOWN | 7 days | 7d (fail), 7d+1s (pass) | | ||||||||||||||||||||||||||||||||||
| | ARBITRATION_TIMEOUT | 7 days | Before/after timeout | | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| ## Security Regression Policy | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| Every discovered vulnerability gets a permanent regression test named `test_IRSB_SEC_NNN_*`: | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| | ID | Vulnerability | Test | | ||||||||||||||||||||||||||||||||||
| |----|--------------|------| | ||||||||||||||||||||||||||||||||||
| | IRSB-SEC-001 | Cross-chain/contract replay | `test_IRSB_SEC_001_crossChainReplayPrevented` | | ||||||||||||||||||||||||||||||||||
| | IRSB-SEC-002 | Escalation DoS by third parties | Checked in DisputeModule tests | | ||||||||||||||||||||||||||||||||||
| | IRSB-SEC-003 | Re-challenge after rejected dispute | `test_IRSB_SEC_003_rejectedDisputeCannotBeRechallenged` | | ||||||||||||||||||||||||||||||||||
| | IRSB-SEC-005 | Zero-amount slash silent no-op | `test_IRSB_SEC_005_zeroSlashAmountReverts` | | ||||||||||||||||||||||||||||||||||
| | IRSB-SEC-006 | Same-chain nonce replay | Verified in all receipt posting tests | | ||||||||||||||||||||||||||||||||||
| | IRSB-SEC-009 | Batch post skipping validation | Verified in batch post tests | | ||||||||||||||||||||||||||||||||||
| | IRSB-SEC-010 | Zero-slash rounding in arbitration | Checked in DisputeModule resolve | | ||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| // SPDX-License-Identifier: MIT | ||
| pragma solidity ^0.8.25; | ||
|
|
||
| /// @title MockETHRejecter | ||
| /// @notice Contract that rejects ETH transfers (no receive/fallback) | ||
| /// @dev Used to test all "Transfer failed" revert paths | ||
| contract MockETHRejecter { | ||
| // Intentionally no receive() or fallback() | ||
| // Any ETH sent to this contract will revert | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| // SPDX-License-Identifier: MIT | ||
| pragma solidity ^0.8.25; | ||
|
|
||
| import { Test } from "forge-std/Test.sol"; | ||
| import { SolverRegistry } from "../../src/SolverRegistry.sol"; | ||
| import { IntentReceiptHub } from "../../src/IntentReceiptHub.sol"; | ||
| import { EscrowVault } from "../../src/EscrowVault.sol"; | ||
| import { IEscrowVault } from "../../src/interfaces/IEscrowVault.sol"; | ||
| import { Types } from "../../src/libraries/Types.sol"; | ||
|
|
||
| /// @title VerificationHelpers - Moloch DAO-Style State Verification | ||
| /// @notice Reusable assertions for checking ALL fields after state transitions | ||
| /// @dev Inherit this in test contracts to get comprehensive verification functions | ||
| abstract contract VerificationHelpers is Test { | ||
| /// @notice Verify solver state after a bond deposit | ||
| function verifyPostDeposit( | ||
| SolverRegistry registry, | ||
| bytes32 solverId, | ||
| uint256 expectedBond, | ||
| Types.SolverStatus expectedStatus, | ||
| uint256 expectedTotalBonded | ||
| ) internal view { | ||
| Types.Solver memory solver = registry.getSolver(solverId); | ||
| assertEq(solver.bondBalance, expectedBond, "Bond balance mismatch"); | ||
| assertEq(uint256(solver.status), uint256(expectedStatus), "Status mismatch after deposit"); | ||
| assertEq(registry.totalBonded(), expectedTotalBonded, "Total bonded mismatch"); | ||
| } | ||
|
|
||
| /// @notice Verify solver state after a slash | ||
| function verifyPostSlash( | ||
| SolverRegistry registry, | ||
| bytes32 solverId, | ||
| uint256 expectedBond, | ||
| uint256 expectedLocked, | ||
| uint64 expectedDisputesLost, | ||
| Types.SolverStatus expectedStatus | ||
| ) internal view { | ||
| Types.Solver memory solver = registry.getSolver(solverId); | ||
| assertEq(solver.bondBalance, expectedBond, "Bond balance mismatch after slash"); | ||
| assertEq(solver.lockedBalance, expectedLocked, "Locked balance mismatch after slash"); | ||
| assertEq(solver.score.disputesLost, expectedDisputesLost, "Disputes lost mismatch"); | ||
| assertEq(uint256(solver.status), uint256(expectedStatus), "Status mismatch after slash"); | ||
| } | ||
|
|
||
| /// @notice Verify receipt and dispute state after dispute opening | ||
| function verifyPostDispute( | ||
| IntentReceiptHub hub, | ||
| bytes32 receiptId, | ||
| Types.ReceiptStatus expectedStatus, | ||
| address expectedChallenger, | ||
| uint256 expectedBond | ||
| ) internal view { | ||
| (, Types.ReceiptStatus status) = hub.getReceipt(receiptId); | ||
| assertEq(uint256(status), uint256(expectedStatus), "Receipt status mismatch after dispute"); | ||
|
|
||
| Types.Dispute memory dispute = hub.getDispute(receiptId); | ||
| assertEq(dispute.challenger, expectedChallenger, "Challenger mismatch"); | ||
| assertFalse(dispute.resolved, "Dispute should not be resolved yet"); | ||
| assertEq(hub.getChallengerBond(receiptId), expectedBond, "Challenger bond mismatch"); | ||
| } | ||
|
|
||
| /// @notice Verify receipt state after finalization | ||
| function verifyPostFinalization( | ||
| IntentReceiptHub hub, | ||
| SolverRegistry registry, | ||
| bytes32 receiptId, | ||
| bytes32 solverId, | ||
| Types.ReceiptStatus expectedStatus, | ||
| uint64 expectedTotalFills | ||
| ) internal view { | ||
| (, Types.ReceiptStatus status) = hub.getReceipt(receiptId); | ||
| assertEq(uint256(status), uint256(expectedStatus), "Receipt status mismatch after finalization"); | ||
|
|
||
| Types.IntentScore memory score = registry.getIntentScore(solverId); | ||
| assertEq(score.totalFills, expectedTotalFills, "Total fills mismatch after finalization"); | ||
| } | ||
|
|
||
| /// @notice Verify escrow state | ||
| function verifyEscrowState( | ||
| EscrowVault vault, | ||
| bytes32 escrowId, | ||
| IEscrowVault.EscrowStatus expectedStatus, | ||
| uint256 expectedAmount | ||
| ) internal view { | ||
| IEscrowVault.Escrow memory escrow = vault.getEscrow(escrowId); | ||
| assertEq(uint256(escrow.status), uint256(expectedStatus), "Escrow status mismatch"); | ||
| assertEq(escrow.amount, expectedAmount, "Escrow amount mismatch"); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Specify a language for the directory tree fence.
markdownlint flags this fence because it lacks a language identifier.
📝 Suggested change
📝 Committable suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 16-16: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents