Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughMigrates vault indexing from a localStorage utility to a central Zustand store, updates useVaultIndexing and VaultInitializationModal to the new store API, adds optional allocator images and UI tweaks, and refactors market-cap editing/removal behavior and related tables. Changes
Sequence Diagram(s)mermaid Modal->>Hook: call useVaultIndexing({ vaultAddress, chainId, hasPostInitData, refetch }) Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/features/autovault/components/vault-detail/settings/EditCaps.tsx`:
- Around line 138-163: The closure bug comes from using wasNewCap after calling
setMarketCaps; instead, determine whether the cap is new or existing by reading
the previous marketCaps inside the same updater and update removedMarketIds from
there. In handleRemoveMarketCap (the function), remove wasNewCap and change
setMarketCaps to a single updater that inspects prev.get(key): if info &&
!info.existingCapId, create a new Map, delete the key and return it; otherwise
(existing on-chain cap) return prev and call setRemovedMarketIds(prev => { const
next = new Set(prev); next.add(key); return next; }) from within the branch so
the removed set is updated synchronously based on the prev state you inspected.
In `@src/hooks/useVaultIndexing.tsx`:
- Around line 17-23: The doc comment for useVaultIndexing incorrectly states a
2-minute timeout while the store uses INDEXING_TIMEOUT_MS set to 10 minutes;
update the documentation/comment block above useVaultIndexing to reflect the
actual timeout (10 minutes) or reference the INDEXING_TIMEOUT_MS constant
directly so they stay in sync, ensuring the description of retry behavior and
timeout matches the INDEXING_TIMEOUT_MS value used by the hook.
🧹 Nitpick comments (4)
src/modals/vault/vault-deposit-modal.tsx (1)
114-123: Static "Deposit" label may mislead users during approval flow.The button now always shows "Deposit" even when an approval transaction is required first. Users might expect visual feedback about the current step. The internal logic at lines 60-66 still handles both paths correctly.
If this simplification is intentional for UX reasons, consider adding a subtitle or secondary indicator when approval is pending.
src/features/autovault/components/vault-detail/settings/EditCaps.tsx (1)
299-309: Empty relativeCap string with absoluteCap set results in 0% relative cap.If user clears relativeCap but sets an absoluteCap,
newRelativeCapBigIntstays0n. This might not be intended—typically you'd want at least some relative allocation if absolute is set.Consider whether this edge case needs validation or if the current behavior is acceptable.
src/features/autovault/components/vault-detail/modals/vault-initialization-modal.tsx (2)
296-299: Unsafe type cast.
as anybypasses type checking. The ABI should provide proper typing.Suggested fix
- const adapter = (decoded.args as any).morphoMarketV1Adapter as Address; + const adapter = (decoded.args as { morphoMarketV1Adapter: Address }).morphoMarketV1Adapter;
369-374: Implicit undefined comparison.
marketAdaptercan beundefined. The comparisonundefined !== ZERO_ADDRESSistrue, so this works—but an explicit check is clearer.Optional explicit check
useEffect(() => { - if (marketAdapter !== ZERO_ADDRESS && stepIndex === 0 && deployedAdapter === ZERO_ADDRESS) { + if (marketAdapter && marketAdapter !== ZERO_ADDRESS && stepIndex === 0 && deployedAdapter === ZERO_ADDRESS) { setStepIndex(1); } }, [marketAdapter, stepIndex, deployedAdapter]);
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/features/autovault/components/vault-detail/settings/EditCaps.tsx`:
- Around line 289-304: The code calls parseUnits with info.relativeCap and
info.absoluteCap which can throw InvalidDecimalNumberError for malformed
strings; before calling parseUnits in the block inside EditCaps (where
newRelativeCapBigInt and newAbsoluteCapBigInt are set), validate the input
strings (e.g. ensure info.relativeCap and info.absoluteCap match a
decimal-number regex like /^\d+(\.\d+)?$/) or wrap each parseUnits(...) call in
a try-catch that handles InvalidDecimalNumberError (e.g. set the cap to 0n or
maxUint128 as appropriate and surface a validation error), referencing
parseUnits, info.relativeCap, info.absoluteCap, newRelativeCapBigInt,
newAbsoluteCapBigInt and maxUint128 to locate the changes.
🧹 Nitpick comments (4)
src/features/autovault/components/vault-detail/modals/vault-initialization-modal.tsx (2)
66-66: Nested ternary is fine here, but consider extracting for clarity.The logic is straightforward. If this grows, consider a small helper or early return pattern.
239-239: Type assertion before nullish coalescing is slightly awkward.The cast happens before
??, so you're assertingundefined as Addresswhen array is empty. Works at runtime, but cleaner to move the cast after:♻️ Optional cleanup
-const [selectedAgent, setSelectedAgent] = useState<Address | null>((v2AgentsBase.at(0)?.address as Address) ?? null); +const [selectedAgent, setSelectedAgent] = useState<Address | null>( + (v2AgentsBase.at(0)?.address ?? null) as Address | null +);src/features/autovault/components/vault-detail/settings/EditCaps.tsx (2)
138-157: Nested setState is an anti-pattern.Calling
setRemovedMarketIdsinside thesetMarketCapsupdater can cause unpredictable batching behavior. Move the check outside.Suggested refactor
const handleRemoveMarketCap = useCallback((marketId: string) => { hasUserEditsRef.current = true; const key = marketId.toLowerCase(); + const info = marketCaps.get(key); + + if (!info) return; - setMarketCaps((prev) => { - const info = prev.get(key); - if (!info) return prev; - - if (!info.existingCapId) { - // Newly added cap — remove from state entirely - const next = new Map(prev); - next.delete(key); - return next; - } - - // Existing on-chain cap — mark as removed (will be zeroed on save) - setRemovedMarketIds((prevRemoved) => new Set(prevRemoved).add(key)); - return prev; - }); -}, []); + if (!info.existingCapId) { + // Newly added cap — remove from state entirely + setMarketCaps((prev) => { + const next = new Map(prev); + next.delete(key); + return next; + }); + } else { + // Existing on-chain cap — mark as removed (will be zeroed on save) + setRemovedMarketIds((prev) => new Set(prev).add(key)); + } +}, [marketCaps]);
375-375: MemoizeexistingMarketIds.This creates a new Set on every render, potentially causing unnecessary child re-renders.
Suggested fix
-const existingMarketIds = new Set(Array.from(marketCaps.keys()).filter((key) => !removedMarketIds.has(key))); +const existingMarketIds = useMemo( + () => new Set(Array.from(marketCaps.keys()).filter((key) => !removedMarketIds.has(key))), + [marketCaps, removedMarketIds] +);
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/hooks/useVaultIndexing.tsx`:
- Around line 64-87: The effect in useVaultIndexing watches only isIndexing so
when startIndexing is called again the old interval keeps using the previous
startTime; update the effect to also depend on the vault start time
(indexingVault?.startTime) or a derived startTime variable so the interval is
reset whenever startTime changes, ensuring the timeout calculation uses the
newest startTime; modify the dependency array for the useEffect that contains
refetch/clearInterval to include indexingVault?.startTime (or startTime) and
ensure the intervalId is cleared on cleanup and restart when startTime updates.
1 participant
Summary by CodeRabbit
New Features
UI/UX Updates
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.