Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
Major refactoring of FluxManager and related contracts to address audit findings, focusing on gas optimization, type safety, and signature verification improvements.
- Changed token amount types from
uint256touint128acrossUniswapV4FluxManagerand test files to match Uniswap's expectations and prevent overflows - Removed performance review functionality from
FluxManager.solincludinglastPerformanceReview,performanceReviewFrequency, and related state variables - Added
token0IsNativeimmutable flag inFluxManager.solfor efficient native token handling and gas optimization - Improved signature verification in
IntentsTeller.solwith EIP-712 standards and separated user intent from execution details - Added new mainnet fork test file
UniswapV4FluxManagerSorellaMainnet.t.solfor comprehensive Sorella-specific functionality testing
11 files reviewed, 4 comments
Edit PR Review Bot Settings | Greptile
| token1.approve(address(boringVault), type(uint256).max); | ||
| vm.stopPrank(); | ||
|
|
||
| // Generate signature for user 0 to deposit 1e10 USDC to user 1 | ||
| // Generate signature for user 0 to deposit 1e10 USDC to user 0 |
There was a problem hiding this comment.
logic: Function comment is incorrect - generating signature for user 0, not user 1
| fluxManager.getRateSafe( | ||
| depositData.rate, | ||
| (depositData.intent.asset == token0 || token0IsNative && depositData.intent.asset == ERC20(nativeWrapper)) | ||
| ) |
There was a problem hiding this comment.
style: Consider caching depositData.intent.asset == token0 || token0IsNative && depositData.intent.asset == ERC20(nativeWrapper) to avoid duplicate computation in deposits/withdrawals
| (uint256 token0Balance, uint256 token1Balance) = manager.totalAssets(price); | ||
| assertApproxEqRel(token1Balance, ethAmount, 0.02e18, "token0Balance should equate to original ethAmount"); | ||
| assertApproxEqRel(token0Balance, usdcAmount, 0.02e18, "token1Balance should equate to original usdcAmount"); |
There was a problem hiding this comment.
syntax: Assertion message tokens are reversed - token1Balance is compared against ethAmount but message says token0Balance
| (uint256 token0Balance, uint256 token1Balance) = manager.totalAssets(price); | |
| assertApproxEqRel(token1Balance, ethAmount, 0.02e18, "token0Balance should equate to original ethAmount"); | |
| assertApproxEqRel(token0Balance, usdcAmount, 0.02e18, "token1Balance should equate to original usdcAmount"); | |
| (uint256 token0Balance, uint256 token1Balance) = manager.totalAssets(price); | |
| assertApproxEqRel(token1Balance, ethAmount, 0.02e18, "token1Balance should equate to original ethAmount"); | |
| assertApproxEqRel(token0Balance, usdcAmount, 0.02e18, "token0Balance should equate to original usdcAmount"); |
| address internal scriptOwner; | ||
| address internal owner; | ||
| address internal payout; | ||
| address internal strategist; |
There was a problem hiding this comment.
logic: These state variables (owner, payout, strategist) are declared but never initialized, which will cause role assignment to fail on line 95
1 participant
Fix audit findings other than G-1, G-2, and L-3. Update tests accordingly.