feat: withdraw and liquidity adapter#276
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughRefactored vault deposit/withdraw to use the centralized modal system; added a VaultWithdrawModal and renamed VaultDepositModal; extended modal registry and modal store types; added market-specific withdrawal support (withdrawFromMarket) and an encoder for market params; updated UI to open modals and refresh on success. Changes
Sequence DiagramsequenceDiagram
participant User
participant Card as TotalSupplyCard (UI)
participant ModalSystem as Modal Registry / useModal
participant Modal as Vault Deposit/Withdraw Modal
participant Hook as useVaultV2
participant Contract as Vault / Multicall
rect rgb(240,248,255)
User->>Card: Click Deposit / Withdraw
Card->>ModalSystem: openModal(vaultDeposit / vaultWithdraw, payload)
ModalSystem->>Modal: mount with payload
end
User->>Modal: enter amount & confirm
Modal->>Hook: deposit(...) / withdrawFromMarket(market, amount)
Hook->>Hook: build steps (optional allocator setup, encodeMarketParams)
Hook->>Contract: submit multicall tx
Contract-->>Hook: tx confirmed
Hook-->>Modal: onSuccess()
Modal-->>ModalSystem: close modal
ModalSystem-->>Card: invoke onSuccess callback
Card->>Card: refetch vault data
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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
added
feature request
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/hooks/useVaultV2.ts (1)
220-234: Variable shadowing:setAllocatorTxandsubmitSetAllocatorTxare redeclared in Step 6.2.Both variables are declared again at lines 237 and 243. This works due to block scoping but hurts readability. Consider renaming to
setOwnerAllocatorTx/submitSetOwnerAllocatorTxfor clarity.Suggested rename
- // Step 6.1 Set user as allocator (for withdrawal / setting Withdrawal Data) - const setAllocatorTx = encodeFunctionData({ + // Step 6.1 Set user as allocator (for withdrawal / setting Withdrawal Data) + const setOwnerAllocatorTx = encodeFunctionData({ abi: vaultv2Abi, functionName: 'setIsAllocator', args: [account, true], }); - const submitSetAllocatorTx = encodeFunctionData({ + const submitSetOwnerAllocatorTx = encodeFunctionData({ abi: vaultv2Abi, functionName: 'submit', - args: [setAllocatorTx], + args: [setOwnerAllocatorTx], }); - txs.push(submitSetAllocatorTx, setAllocatorTx); + txs.push(submitSetOwnerAllocatorTx, setOwnerAllocatorTx);src/features/autovault/components/vault-detail/total-supply-card.tsx (1)
96-108:assetAddressis passed but unused byVaultWithdrawModal.The modal component doesn't destructure or use
assetAddress(see line 31-39 in vault-withdraw-modal.tsx). Either remove it from the props here or add usage in the modal.src/modals/vault/vault-deposit-modal.tsx (1)
24-36:onOpenChangeis declared in props but never used.The prop exists in
VaultDepositModalProps(line 24) but isn't destructured or utilized. The Modal'sonOpenChangeat line 76 uses an inline handler instead. Remove from props or wire it up.Option 1: Remove unused prop
type VaultDepositModalProps = { vaultAddress: Address; vaultName: string; assetAddress: Address; assetSymbol: string; assetDecimals: number; chainId: SupportedNetworks; onSuccess?: () => void; onClose: () => void; - onOpenChange: (open: boolean) => void; };src/modals/vault/vault-withdraw-modal.tsx (2)
19-39: Unused props:assetAddressandonOpenChange.Both are declared in the type but not destructured. Either remove them or add usage:
assetAddresscould be used for the header icon (like deposit modal does withTokenIcon).onOpenChangeappears to be dead code.Use assetAddress for token icon in header
export function VaultWithdrawModal({ vaultAddress, vaultName, + assetAddress, assetSymbol, assetDecimals, chainId, onSuccess, onClose, }: VaultWithdrawModalProps): JSX.Element {Then in the header:
<ModalHeader title={`Withdraw ${assetSymbol}`} description={`Withdraw from ${vaultName}`} - mainIcon={<FiArrowUpRight className="h-5 w-5" />} + mainIcon={ + <TokenIcon + address={assetAddress} + chainId={chainId} + symbol={assetSymbol} + width={24} + height={24} + /> + } onClose={onClose} />
92-105: Consider adding try-catch for explicit error handling.
withdrawFromMarketthrows on non-rejection errors. WhileuseVaultV2shows toasts internally, wrapping in try-catch would allow local error state or logging if needed.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/features/autovault/components/vault-detail/total-supply-card.tsxsrc/hooks/useVaultV2.tssrc/modals/registry.tsxsrc/modals/vault/vault-deposit-modal.tsxsrc/modals/vault/vault-deposit-process-modal.tsxsrc/modals/vault/vault-withdraw-modal.tsxsrc/stores/useModalStore.tssrc/utils/morpho.ts
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2024-11-25T09:39:42.148Z
Learnt from: antoncoding
Repo: antoncoding/monarch PR: 87
File: app/home/HomePage.tsx:17-39
Timestamp: 2024-11-25T09:39:42.148Z
Learning: In `app/home/HomePage.tsx`, the `useEffect` hook depends on `[showCustomized]` because changing `showCustomized` triggers updates to the yield and risk terms.
Applied to files:
src/features/autovault/components/vault-detail/total-supply-card.tsx
📚 Learning: 2024-10-23T16:17:02.841Z
Learnt from: antoncoding
Repo: antoncoding/monarch PR: 77
File: src/graphql/queries.ts:168-193
Timestamp: 2024-10-23T16:17:02.841Z
Learning: In `src/graphql/queries.ts`, handling only `MarketTransferTransactionData` is intentional at this time.
Applied to files:
src/hooks/useVaultV2.ts
📚 Learning: 2025-12-09T10:06:39.848Z
Learnt from: antoncoding
Repo: antoncoding/monarch PR: 231
File: src/hooks/useDeployMorphoMarketV1Adapter.ts:3-3
Timestamp: 2025-12-09T10:06:39.848Z
Learning: In Wagmi v3, useConnection is the correct hook to obtain the connected wallet address, chainId, and connection status (isConnected). This replaces the useAccount hook from Wagmi v2. In your code under src/hooks, use: const { address, chainId, isConnected } = useConnection() from 'wagmi' to access the connected wallet information.
Applied to files:
src/hooks/useVaultV2.ts
🧬 Code graph analysis (4)
src/utils/morpho.ts (1)
src/utils/types.ts (1)
Market(293-345)
src/features/autovault/components/vault-detail/total-supply-card.tsx (2)
src/hooks/useModal.ts (1)
useModal(25-90)src/components/shared/token-icon.tsx (1)
TokenIcon(23-95)
src/hooks/useVaultV2.ts (3)
src/abis/vaultv2.ts (1)
vaultv2Abi(3-3)src/utils/types.ts (1)
Market(293-345)src/utils/morpho.ts (1)
encodeMarketParams(247-266)
src/modals/registry.tsx (2)
src/modals/vault/vault-deposit-modal.tsx (1)
VaultDepositModal(27-148)src/modals/vault/vault-withdraw-modal.tsx (1)
VaultWithdrawModal(31-217)
🔇 Additional comments (9)
src/utils/morpho.ts (1)
247-266: LGTM!The encoding logic is correct and matches the tuple structure used elsewhere in the file (e.g.,
getMarketCapId). The field mappings align with theMarkettype definition.src/modals/registry.tsx (1)
43-45: LGTM!Registry entries follow the established pattern. Lazy loading is correctly configured for both vault modals.
Also applies to: 64-65
src/hooks/useVaultV2.ts (2)
539-571: LGTM!Clean simplification. The
withdrawfunction now consistently derives the sender from the hook'saccount, which is the expected behavior for basic withdrawals.
573-645: LGTM!The
withdrawFromMarketflow correctly sequences:
- Optional allocator setup (submit + execute)
- Set liquidity adapter with encoded market params
- Execute withdraw
All wrapped in a multicall for atomicity. Error handling is consistent with other methods in the hook.
src/features/autovault/components/vault-detail/total-supply-card.tsx (1)
78-94: LGTM!Modal-driven approach is cleaner than local state. Callbacks are properly memoized with correct dependencies. The refetch-on-success pattern ensures data consistency.
Also applies to: 113-140
src/stores/useModalStore.ts (1)
55-75: LGTM!Props are correctly typed. Both vault operations share the same shape, which is fine for now. If more vault modals are added, consider extracting a shared
VaultModalBasePropstype.src/modals/vault/vault-deposit-modal.tsx (1)
149-150: LGTM!Default export enables the lazy import pattern used in the registry.
src/modals/vault/vault-withdraw-modal.tsx (2)
63-82: LGTM!Allocator check and max withdrawable calculation are correct. The min(allocation, liquidity) logic prevents over-withdrawal.
109-216: LGTM!UI handles loading states, empty states, and market selection flow well. The disabled logic on the button is comprehensive.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/features/autovault/components/vault-detail/settings/AgentsTab.tsx (1)
131-134: Type assertion doesn't prevent undefined.The pattern
ownerAddress={owner as Address}appears in multiple places. Sinceownercan be undefined (fromvaultData?.owner), the type assertion doesn't provide runtime safety. WhileAgentListItemcorrectly handles optionalownerAddress, the assertion misleads readers about type guarantees.🔎 Consider removing the type assertion
The prop is already typed as optional in
AgentListItem, so you can pass it directly:- ownerAddress={owner as Address} + ownerAddress={owner}This makes the optionality explicit at the call site.
Also applies to: 165-168, 212-215
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/components/layout/notification-banner.tsxsrc/features/autovault/components/vault-detail/settings/AgentListItem.tsxsrc/features/autovault/components/vault-detail/settings/AgentsTab.tsxsrc/features/markets/components/table/markets-table-actions.tsxsrc/hooks/useVaultV2.tssrc/modals/vault/vault-withdraw-modal.tsxsrc/utils/morpho.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/utils/morpho.ts
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2024-10-12T09:23:16.495Z
Learnt from: antoncoding
Repo: antoncoding/monarch PR: 63
File: app/markets/components/MarketRowDetail.tsx:49-52
Timestamp: 2024-10-12T09:23:16.495Z
Learning: When rendering oracle feeds in `ExpandedMarketDetail` (`app/markets/components/MarketRowDetail.tsx`), prefer explicit rendering over iterating keys when dealing with a small number of feeds.
Applied to files:
src/features/markets/components/table/markets-table-actions.tsx
📚 Learning: 2024-10-23T16:17:02.841Z
Learnt from: antoncoding
Repo: antoncoding/monarch PR: 77
File: src/graphql/queries.ts:168-193
Timestamp: 2024-10-23T16:17:02.841Z
Learning: In `src/graphql/queries.ts`, handling only `MarketTransferTransactionData` is intentional at this time.
Applied to files:
src/hooks/useVaultV2.ts
📚 Learning: 2025-12-09T10:06:39.848Z
Learnt from: antoncoding
Repo: antoncoding/monarch PR: 231
File: src/hooks/useDeployMorphoMarketV1Adapter.ts:3-3
Timestamp: 2025-12-09T10:06:39.848Z
Learning: In Wagmi v3, useConnection is the correct hook to obtain the connected wallet address, chainId, and connection status (isConnected). This replaces the useAccount hook from Wagmi v2. In your code under src/hooks, use: const { address, chainId, isConnected } = useConnection() from 'wagmi' to access the connected wallet information.
Applied to files:
src/hooks/useVaultV2.ts
🧬 Code graph analysis (3)
src/features/autovault/components/vault-detail/settings/AgentListItem.tsx (3)
src/utils/monarch-agent.ts (1)
findAgent(19-21)src/components/Avatar/Avatar.tsx (1)
Avatar(12-43)src/components/shared/agent-icon.tsx (1)
AgentIcon(14-93)
src/features/autovault/components/vault-detail/settings/AgentsTab.tsx (1)
src/features/autovault/components/vault-detail/settings/AgentListItem.tsx (1)
AgentListItem(12-38)
src/hooks/useVaultV2.ts (2)
src/utils/types.ts (1)
Market(293-345)src/utils/morpho.ts (1)
encodeMarketParams(247-271)
🔇 Additional comments (10)
src/components/layout/notification-banner.tsx (1)
35-38: LGTM!Formatting-only changes. No functional impact.
src/features/markets/components/table/markets-table-actions.tsx (2)
4-4: LGTM: Import formatting.Consistent quote style with the rest of the file.
69-73: LGTM: Multiline ternary.Reformatting improves readability with no logic change.
src/features/autovault/components/vault-detail/settings/AgentListItem.tsx (1)
1-38: LGTM!The conditional rendering logic correctly handles the optional
ownerAddressprop and distinguishes between owner and agent displays.src/hooks/useVaultV2.ts (3)
220-233: Verify: Owner automatically becomes allocator on initialization.Step 6.1 now grants allocator permissions to the vault owner during initialization. This enables the withdrawal flow but changes the default permissions model for all new vaults.
Confirm this is the intended behavior for all vault deployments.
539-571: Approve withdraw signature simplification.The function correctly simplifies the API by using the hook's internal
accountfor the contract call, while maintaining the proper three-argument pattern for the vault's withdraw function.
578-639: The submit + direct call pattern is intentional and confirmed across the codebase.The
withdrawFromMarketfunction uses the submit+execute pattern within a single multicall. This identical pattern appears incompleteInitializationandsetAllocator, confirming it's a deliberate approach. ThesetIsAllocatorfunction has no timelock—it executes immediately—so the direct call (line 597) takes effect within the same transaction, allowing the allocator permission to be available for subsequent calls likesetLiquidityAdapterAndData.src/modals/vault/vault-withdraw-modal.tsx (3)
75-82: LGTM - defensive null handling.The optional chaining on
market.state?.liquidityAssetsprovides safe fallback even though theMarkettype definesstateas required. If liquidity data is missing,maxWithdrawablebecomes0n, preventing invalid withdrawals.
92-99: Allocator setup logic handles legacy vaults.The check for
needsAllocatorSetup(line 96) correctly handles vaults initialized before the owner-as-allocator change, while avoiding redundant setup for new vaults.
120-202: Well-structured progressive disclosure.The UI flow properly gates data display behind loading states and progressively reveals the amount input only after market selection. The button disable logic prevents invalid submissions.
1 participant
Summary by CodeRabbit
New Features
UI Improvements
✏️ Tip: You can customize this high-level summary in your review settings.