Fix: corrections for aave and compound accounting#11
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes critical accounting bugs in the YieldSeeker fee tracking system for Aave and Compound vault adapters. The changes address four audit findings related to incorrect fee calculations that could lead to inflated fees or denial-of-service scenarios.
Changes:
- Fixed fee calculation formula in FeeTracker to use explicit exchange rates instead of share ratios, preventing fee inflation for rebasing tokens
- Added safety cap to prevent underflow when fees exceed withdrawal amounts in edge cases with large rewards
- Updated CompoundV2 to use current exchange rate instead of stored rate for accurate fee calculations
- Fixed CompoundV3 deposit tracking to record actual shares received instead of input amount
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/FeeTracker.sol | Added vaultTokenToBaseAssetRate parameter and safety cap to fix fee calculation bugs |
| src/adapters/AaveV3Adapter.sol | Added 1e18 rate parameter for rebasing token fee calculation |
| src/adapters/CompoundV2Adapter.sol | Switched to exchangeRateCurrent and added exchange rate parameter for accurate fees |
| src/adapters/CompoundV3Adapter.sol | Fixed deposit to record actual shares received; added 1e18 rate for rebasing tokens |
| src/agentwalletkit/adapters/AWKCompoundV2Adapter.sol | Added exchangeRateCurrent function to interface |
| test/unit/adapters/AaveV3Adapter.t.sol | Added comprehensive tests for rebasing fee fixes and underflow protection |
| test/unit/adapters/CompoundV2Adapter.t.sol | Added tests for exchange rate accuracy in fee calculations |
| test/unit/adapters/CompoundV3Adapter.t.sol | Added tests for deposit amount tracking and rebasing fee calculations |
| test/unit/FeeTracker.t.sol | Added tests for fee cap safety and exchange rate conversions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issues addressed:
[Medium] — Incorrect exchange rate for rebasing protocols
Fixed by adding an explicit vaultTokenToBaseAssetRate parameter to recordAgentVaultAssetWithdraw. Aave V3 and Compound V3 pass
ASSET_EXCHANGE_RATE_PRECISION(1e18, meaning 1:1). Compound V2 passesexchangeRateCurrent(). The oldtotalVaultBalanceBefore / totalSharesimplicit rate is gone.[Medium] — Underflow DoS when fee > assetsReceived
Fixed by adding a safety cap:
if (feeInBaseAsset > assetsReceived) feeInBaseAsset = assetsReceived;before the subtraction.[Low] — Stale exchange rate in Compound V2
Fixed by replacing
exchangeRateStored()withexchangeRateCurrent()inCompoundV2Adapter._withdrawInternal.[Medium] — type(uint256).max recorded as cost basis
Fixed more generally than the auditor suggested: all 4 AWK adapters now measure the actual base asset balance delta (
balanceBefore - balanceAfter) and return it asassetsDeposited. The YieldSeeker adapters record this resolved value as cost basis. This handlestype(uint256).max, amount capping, and any other vault-specific amount resolution — not just the Compound V3 case.