added invariants testing

[Sunnesoft]

No description provided.

[claude]

Claude finished @Sunnesoft's task —— View job

---

Pull Request Review: Added Invariants Testing

Summary

This PR introduces a comprehensive invariant testing framework for the SwapVM project, adding 26 test files with over 9,000 lines of code. The tests validate core mathematical properties (symmetry, monotonicity, additivity) for various instruction combinations.

Review Feedback

1. Code Quality and Best Practices

Strengths:

• Well-structured test architecture using abstract base contracts (CoreInvariants.t.sol)
• Clear separation of concerns with dedicated test files per instruction
• Comprehensive documentation in each test file explaining the invariants
• Good use of inheritance and composition patterns

Areas for Improvement:

• Large number of TODO comments: The TESTING.md file reveals 69 TODO comments throughout the test suite, indicating incomplete analysis or unresolved behaviors
• Inconsistent test naming: Some tests use underscore separation (test_DecayXYC_FlatFeeIn) while others use camelCase within parts
• Magic numbers: Several tests use hardcoded values without clear explanation (e.g., 0.8e18, 1.25e18 for price ranges)

2. Potential Bugs or Issues

Critical Findings:

1. Source Code Changes with Minimal Context:

   • DutchAuction.sol: Function names changed from _balanceIn1D/_balanceOut1D to _dutchAuctionBalanceIn1D/_dutchAuctionBalanceOut1D
   • These changes affect the public API but lack justification in the PR description

2. Skipped Invariants Without Resolution:

   • Many tests skip core invariants (symmetry, additivity) with TODO comments like "need to research behavior"
   • Example from ConcentrateXYCDecayFeesInvariants.t.sol: Multiple tests skip symmetry checks entirely

3. Incomplete Test Coverage:

   • Several test methods have comments like "why it didn't fail?" indicating unexpected behavior
   • The expected failure cases aren't properly documented or handled

3. Performance Considerations

Observations:

• Gas snapshots show reasonable gas consumption for individual tests (most under 2M gas)
• Some complex tests reach 24M+ gas (e.g., test_ConcentrateGrowLiquidity_SpreadSlowlyGrowsForSomeReason)
• No performance benchmarking for the invariant checking logic itself

Recommendations:

• Consider adding gas optimization for frequently used helper methods
• Profile the most expensive tests to identify optimization opportunities
• Add performance regression tests for critical paths

4. Security Concerns

Identified Issues:

1. Unvalidated Mathematical Operations:

   • The decay calculations use pow operations without overflow checks in some test scenarios
   • Price manipulation tests are incomplete (many TODOs around edge cases)

2. Missing Boundary Condition Tests:

   • No tests for extreme values (max uint256, zero values in critical places)
   • Limited testing of precision loss in mathematical operations

3. State Dependencies Not Fully Tested:

   • TWAP tests acknowledge "time and state dependencies" but skip critical invariants
   • Decay mechanism interactions with fees aren't fully validated

5. Test Coverage

Coverage Analysis:

• Good Coverage: LimitSwap, basic XYC operations, individual fee types
• Partial Coverage: Complex instruction combinations, especially with multiple fee types
• Missing Coverage:
  • Error path testing (most tests focus on happy paths)
  • Edge cases for time-based instructions (TWAP, DutchAuction)
  • Cross-instruction interference scenarios

Specific Gaps:

1. Many invariant tests are disabled with skip flags
2. No tests for instruction ordering edge cases beyond the documented ones
3. Limited negative testing (what should fail doesn't always have tests)

6. Recommendations

1. Address TODO Comments: Create issues to track and resolve the 69 TODO items, prioritizing those affecting core invariants
2. Document API Changes: Explain why DutchAuction.sol functions were renamed
3. Complete Invariant Analysis: Either fix the failing invariants or document why they legitimately don't hold
4. Add Negative Tests: Include tests that verify operations fail when they should
5. Improve Error Messages: Many assertions use generic messages; make them more specific
6. Add Integration Tests: Test real-world scenarios with multiple instructions
7. Security Audit Focus Areas:
   • Time manipulation resistance
   • Precision loss in calculations
   • State-dependent behavior edge cases

Conclusion

While this PR adds valuable testing infrastructure, the high number of TODOs and skipped tests suggests it may be premature to merge. Consider splitting this into smaller PRs: first establishing the framework with working tests, then incrementally adding more complex scenarios as the TODOs are resolved.

The source code changes to DutchAuction.sol should be documented or moved to a separate PR if they're not directly related to testing.