Repository navigation
feat: add best-rate lending router action provider - #2
zaryab2000 wants to merge 4 commits into
Conversation
Cross-protocol lending router that compares live supply/borrow APY across Compound III, Aave v3, Moonwell, and Morpho Blue on Base mainnet (8453), then routes deposits and borrows to the optimal venue. Actions: - compare_lending_rates: Read live APY from all 4 protocols, rank best-first - get_aggregated_position: Cross-protocol portfolio view with health factors - route_supply: Auto-route supply to best-rate venue (all 4 protocols) - route_borrow: Auto-route borrow to cheapest venue (Compound/Aave v1) - rebalance: Advisory rebalance plan when rates drift v1 limitations: borrow routing is Compound/Aave only; Morpho position tracking is minimal; rebalance is advisory. https://claude.ai/code/session_01H9og3kM8YwPCcUCHnDxTxw
|
@claude start review |
|
Claude finished @zaryab2000's task in 2m 45s —— View job Code Review —
|
| Requirement | Status |
|---|---|
Actions return Promise<string> |
✅ |
| Errors returned, not thrown | ✅ |
Zod v4 .describe() on every field, no .strip() |
✅ |
supportsNetwork correct (Base mainnet only) |
✅ |
Factory exported + re-exported from index.ts |
✅ |
Per-provider README.md |
✅ |
| Changeset (patch, past tense) | ✅ |
pnpm test / pnpm build / lint / format passing |
✅ (per PR) |
Summary: 3 of the 6 critical issues (#1 Compound multi-market, #2 Moonwell decimals, #6 Morpho address cast) are correctness bugs that produce wrong outputs or would cause runtime failures for non-USDC assets or future Morpho execution. These should be fixed before merging. Issues #3 (health factor comparability) and #5 (rebalance reads positions, not just rates) are design-level and worth addressing even if they require a bit more work.
…h checks - Replace single COMPOUND_COMET_ADDRESS with COMPOUND_COMET_ADDRESSES map (USDC + WETH Comets) - Fix executeSupply/executeBorrow to look up correct Comet by asset symbol - Add Compound health pre-check in routeBorrow alongside existing Aave check - Rewrite rebalance to read user positions and compare against current protocol rate - Fix RateResult.marketAddress → marketId (string, not Address — supports Morpho bytes32) - Add PositionResult.healthComparable flag; skip non-comparable in findLowestHealth - Fix encodeMoonwellMint signature (remove unused mToken param) - Fix Moonwell position to use per-token decimals and non-comparable health - Fix Morpho position user param type and add healthComparable: false - Fix cbeth/cbbtc mToken addresses in constants - Update tests for multi-Comet mocks and position-aware rebalance https://claude.ai/code/session_01H9og3kM8YwPCcUCHnDxTxw
|
@claude review the fixes |
|
Claude finished @zaryab2000's task in 1m 59s —— View job Fix Review —
|
|
@claude conduct final review and confirm if the PR is accurate now? |
|
Claude finished @zaryab2000's task in 3m 23s —— View job Final Review —
|
| # | Issue | Status |
|---|---|---|
| 1 | Compound always queried USDC Comet | ✅ COMPOUND_COMET_ADDRESSES record + getCometForAsset() |
| 2 | Moonwell decimals hardcoded to 18 | ✅ MOONWELL_UNDERLYING_DECIMALS map used in getMoonwellPosition |
| 3 | Moonwell health factor not comparable | ✅ healthComparable: false, raw USD surplus stored, skipped in findLowestHealth |
| 4 | Unused mToken param in encodeMoonwellMint |
✅ Removed; call site updated |
| 5 | Rebalance read rates only, ignored positions | ✅ Promise.all for rates + positions; activeProtocols filter applied |
| 6 | Morpho marketAddress bytes32 cast as Address |
✅ marketId: string in RateResult, no cast |
| 7 | Compound notes hardcoded "Comet USDC" |
✅ Dynamically uses asset.toUpperCase() |
| 8 | No Compound health pre-check before borrow | ✅ Both Aave and Compound health checked before executing borrow |
Remaining Open Items (Minor, Non-Blocking)
coinbase#9 — Sequential RPC calls in getCompoundPosition (unchanged)
The collateral loop at [adapters/compound.ts:124-164] makes 4–5 sequential readContract calls per collateral asset. For a user with many collateral positions this is slow but functionally correct. multicall batching is the v2 improvement path; not a merge blocker.
coinbase#10 — cbeth mToken address still needs verification [constants.ts:243]
Address is 0x3bf93770f2d4a794c3d9EBEfBAeBAE2a8f09A5E5. The suspicious repeated-byte pattern from the original is gone, but this address has never been cross-checked against Moonwell's official deployment registry. Please verify this address before the upstream PR is submitted. An incorrect mToken address would silently fail at runtime for cbETH suppliers.
coinbase#11 — apy field stores a linear APR, not a compound APY (unchanged) [adapters/compound.ts:68, adapters/moonwell.ts:47]
getCompoundRates and getMoonwellRates compute rate_per_second × seconds_per_year × 100. This is a linear APR annualization, not compound APY. The field is named apy in RateResult. For a v1 router the difference is small (a few basis points), but it may mislead LLM consumers comparing against Aave (which reads the actual WAD-encoded interest rate, also linear). Worth renaming the field apr in v2 or computing true APY ((1 + rate)^n - 1).
coinbase#12 — No test for routeSupply → Moonwell path (still missing) [lendingRouterActionProvider.test.ts]
There is no test with preferProtocol: "moonwell". The MOONWELL_MTOKEN_ADDRESSES lookup and encodeMoonwellMint call site ([lendingRouterActionProvider.ts:527-534]) remain uncovered. Since encodeMoonwellMint was changed during the fix cycle (the dead mToken param was removed), a test verifying this path would confirm the call site is correct. Fix this →
coinbase#13 — getAggregatedPosition explicit user param not tested (still missing) [lendingRouterActionProvider.test.ts:247-282]
All three getAggregatedPosition tests omit user and rely on walletProvider.getAddress(). A test passing user: "0xsome...address" would cover the args.user ?? walletProvider.getAddress() branch ([lendingRouterActionProvider.ts:119]) and confirm address plumbing. Fix this →
New Observation: Aave "aggregate" match in rebalance (design note)
[lendingRouterActionProvider.ts:405-408]
p.supplies.some(
s => s.asset.toLowerCase() === args.asset.toLowerCase() || s.asset === "aggregate",
)Because getAavePosition returns asset: "aggregate" for all positions (it can only read totals via getUserAccountData), any Aave position will match the s.asset === "aggregate" condition regardless of which asset is actually being supplied. A user with WETH supplied to Aave but no USDC would be flagged as having an active USDC Aave position when running rebalance({ asset: "USDC" }). This is a documented v1 limitation (Aave aggregate reads are by design), but it could produce surprising advisory output. Consider noting it in the rebalance action description.
Checklist Compliance — Final
| Requirement | Status |
|---|---|
Actions return Promise<string> |
✅ |
| Errors returned, not thrown | ✅ |
Zod v4 .describe() on every field, no .strip() |
✅ |
supportsNetwork correct (Base mainnet "8453" only) |
✅ |
Factory exported + re-exported from src/action-providers/index.ts |
✅ |
Per-provider README.md |
✅ |
| Changeset: patch, past tense, correct package | ✅ |
pnpm test / pnpm build / lint / format (per PR) |
✅ |
Verdict
The PR is correct and ready for upstream submission with one action item before merging: verify the cbeth mToken address (constants.ts:243) against Moonwell's deployment registry. Items coinbase#12 and coinbase#13 (missing test paths) are recommended additions but are not merge-blocking given the existing 29-test suite and the fact that both paths exercise code already unit-verified at the module level.
- Add test for routeSupply with preferProtocol: "moonwell" covering mToken lookup and encodeMoonwellMint call path - Add test for getAggregatedPosition with explicit user address covering the args.user ?? walletProvider.getAddress() branch https://claude.ai/code/session_01H9og3kM8YwPCcUCHnDxTxw
Summary
lendingRouter— a cross-protocol lending router for Base mainnet (chain ID8453)compare_lending_rates,get_aggregated_position,route_supply,route_borrow,rebalanceWhat's included
lendingRouterActionProvider.ts@CreateActionmethodsschemas.ts.describe()on every fieldconstants.tsadapters/compound.tsadapters/aave.tsadapters/moonwell.tsadapters/morpho.tsutils.tslendingRouterActionProvider.test.tsindex.tsREADME.mdv1 Protocol Coverage
v1 Limitations
chainId === "8453")Test plan
pnpm build— cleanpnpm test— 29/29 passedpnpm run lint— cleanpnpm run format— cleansrc/action-providers/index.tssupportsNetworkreturns true only for Base mainnetPromise<string>; errors returned not thrown.describe()on every field, no.strip()https://claude.ai/code/session_01H9og3kM8YwPCcUCHnDxTxw
Generated by Claude Code