XYConcentrate as a standalone instruction + some math improvements

[BoogerWooger]

Change Summary

XYConcentrate now includes XYCSwap inside and work as a stadalone instruction (makin SwapVM programs much simpler, removing many instructions order related issues

[claude]

Claude finished @BoogerWooger's task —— View job

---

Pull Request Review: XYConcentrate as a standalone instruction

Review Tasks:

• Read repository guidelines (CLAUDE.md) - No CLAUDE.md found
• Review XYCConcentrate.sol changes
• Review XYCSwap.sol changes
• Analyze test modifications
• Check gas impact
• Provide comprehensive feedback

Summary

This PR refactors XYCConcentrate to include the swap logic directly, making it a terminal instruction that performs both liquidity concentration and swapping in a single step. This eliminates the need for separate XYCSwap calls and simplifies the SwapVM program structure.

✅ Code Quality and Best Practices

Positive changes:

• Simplified architecture: Merging concentrate and swap into a single instruction reduces complexity and potential ordering issues
• Better encapsulation: Virtual reserves are now local to the instruction, preventing side effects on ctx.swap.balanceIn/Out
• Improved math precision: Introduction of ONE_HI (1e27) for higher precision in internal calculations
• Gas efficient: The combined instruction eliminates redundant calculations and context passing

Code organization:

• Clear separation between the library (XYCConcentrateArgsBuilder) and the instruction contract
• Well-documented functions with clear parameter descriptions
• Consistent error handling patterns

🔍 Potential Issues

1. Precision loss in computeBalances:

   bLt = Math.mulDiv(targetL, (sqrtPmax - sqrtPspot) * ONE, sqrtPspot * sqrtPmax)

   The multiplication (sqrtPmax - sqrtPspot) * ONE could overflow for large values. Consider reordering: Math.mulDiv(targetL * ONE, sqrtPmax - sqrtPspot, sqrtPspot * sqrtPmax)

2. Edge case handling: The code handles boundary conditions well (when spot price is outside the range), but test coverage should verify these edge cases thoroughly.

⚡ Performance Considerations

Gas improvements observed:

• ConcentrateGrowLiquidity operations show consistent gas usage (~48k for quotes, ~150k for swaps)
• Removing the separate XYCSwap call eliminates one instruction execution overhead
• The direct implementation avoids updating context balances unnecessarily

Optimization opportunities:

• Consider caching repeated calculations like Math.mulDiv(L, ONE, sqrtPriceMax) when used multiple times
• The _computeL function performs expensive sqrt operations - ensure this is only called once per swap

🔒 Security Considerations

1. Overflow protection: Good use of Math.mulDiv throughout to prevent overflow/underflow
2. Input validation: Proper validation of price bounds (sqrtPriceMin < sqrtPriceMax)
3. Reentrancy: Function is marked pure, eliminating reentrancy concerns
4. Error handling: Clear error messages with parameter values for debugging

Potential concern:

• The ConcentrateRecomputeDetected error could be more descriptive about why recomputation was detected

🧪 Test Coverage

Good coverage for:

• Gas benchmarks for all swap scenarios
• Integration with various fee mechanisms
• Invariant testing for concentrated liquidity

Suggestions for additional tests:

1. Edge cases when spot price equals min/max bounds
2. Extreme liquidity values (very large/small L)
3. Price impact validation for large swaps
4. Precision loss scenarios with different decimal tokens

📝 Additional Recommendations

1. Documentation: Add natspec comments explaining the mathematical relationship between real and virtual reserves
2. Constants: Consider making precision constants configurable or explaining the choice of 1e27 for ONE_HI
3. Error messages: Include expected vs actual values in error messages for easier debugging
4. Benchmarking: Compare gas costs before/after this change in the PR description

Conclusion

This is a well-executed refactoring that simplifies the SwapVM architecture while maintaining functionality. The code quality is high, with proper attention to precision and edge cases. The main benefits are reduced complexity for users and elimination of instruction ordering issues. With the minor improvements suggested above, this PR is ready for merging.

[claude]

Claude finished @BoogerWooger's task —— View job

---

Pull Request Review: XYConcentrate as a standalone instruction

Review Tasks:

• Read repository guidelines (CLAUDE.md) - No CLAUDE.md found
• Review XYCConcentrate.sol changes
• Review XYCSwap.sol changes
• Analyze test modifications
• Check gas impact
• Provide comprehensive feedback

Summary

This PR refactors XYCConcentrate to include the swap logic directly, making it a terminal instruction that performs both liquidity concentration and swapping in a single step. This eliminates the need for separate XYCSwap calls and simplifies the SwapVM program structure.

✅ Code Quality and Best Practices

Positive changes:

• Simplified architecture: Merging concentrate and swap into a single instruction reduces complexity and potential ordering issues
• Better encapsulation: Virtual reserves are now local to the instruction, preventing side effects on ctx.swap.balanceIn/Out
• Improved math precision: Introduction of ONE_HI (1e27) for higher precision in internal calculations
• Gas efficient: The combined instruction eliminates redundant calculations and context passing

Code organization:

• Clear separation between the library (XYCConcentrateArgsBuilder) and the instruction contract
• Well-documented functions with clear parameter descriptions
• Consistent error handling patterns

🔍 Potential Issues

1. Precision loss in computeBalances:

   bLt = Math.mulDiv(targetL, (sqrtPmax - sqrtPspot) * ONE, sqrtPspot * sqrtPmax)

   The multiplication (sqrtPmax - sqrtPspot) * ONE could overflow for large values. Consider reordering: Math.mulDiv(targetL * ONE, sqrtPmax - sqrtPspot, sqrtPspot * sqrtPmax)

2. Edge case handling: The code handles boundary conditions well (when spot price is outside the range), but test coverage should verify these edge cases thoroughly.

⚡ Performance Considerations

Gas improvements observed:

• ConcentrateGrowLiquidity operations show consistent gas usage (~48k for quotes, ~150k for swaps)
• Removing the separate XYCSwap call eliminates one instruction execution overhead
• The direct implementation avoids updating context balances unnecessarily

Optimization opportunities:

• Consider caching repeated calculations like Math.mulDiv(L, ONE, sqrtPriceMax) when used multiple times
• The _computeL function performs expensive sqrt operations - ensure this is only called once per swap

🔒 Security Considerations

1. Overflow protection: Good use of Math.mulDiv throughout to prevent overflow/underflow
2. Input validation: Proper validation of price bounds (sqrtPriceMin < sqrtPriceMax)
3. Reentrancy: Function is marked pure, eliminating reentrancy concerns
4. Error handling: Clear error messages with parameter values for debugging

Potential concern:

• The ConcentrateRecomputeDetected error could be more descriptive about why recomputation was detected

🧪 Test Coverage

Good coverage for:

• Gas benchmarks for all swap scenarios
• Integration with various fee mechanisms
• Invariant testing for concentrated liquidity

Suggestions for additional tests:

1. Edge cases when spot price equals min/max bounds
2. Extreme liquidity values (very large/small L)
3. Price impact validation for large swaps
4. Precision loss scenarios with different decimal tokens

📝 Additional Recommendations

1. Documentation: Add natspec comments explaining the mathematical relationship between real and virtual reserves
2. Constants: Consider making precision constants configurable or explaining the choice of 1e27 for ONE_HI
3. Error messages: Include expected vs actual values in error messages for easier debugging
4. Benchmarking: Compare gas costs before/after this change in the PR description

Conclusion

This is a well-executed refactoring that simplifies the SwapVM architecture while maintaining functionality. The code quality is high, with proper attention to precision and edge cases. The main benefits are reduced complexity for users and elimination of instruction ordering issues. With the minor improvements suggested above, this PR is ready for merging.

[ifelsedeveloper]

ConcentrateXYCFeesInvariants add comment to
// Additivity tolerance: 1 wei covers protocol fee floor-division subadditivity
// (floor(outABr) can exceed floor(outAr)+floor(outB*r) by 1, flipping the invariant)
uint256 internal additivityTolerance = 1;