feat(starknet): add Earn product via Vesu lending pools#499
Open
starkience wants to merge 10 commits into
Open
feat(starknet): add Earn product via Vesu lending pools#499starkience wants to merge 10 commits into
starkience wants to merge 10 commits into
Conversation
Adds an "Earn" action in the Starknet wallet drawer that lets users supply USDC and USDT to Vesu lending pools through the starkzap SDK: - USDC → Clearstar USDC Reactor pool - USDT → Prime pool Surfaces: - New "Earn" action button in the wallet sidebar (Starknet only). - Earn modal with deposit/withdraw tabs, token selector, live APR / monthly / yearly projection, and a success view mirroring Transfer. - New "Earn activity" tab next to Balances and Transactions, with a per-token "Currently supplied" card and a date-grouped list of past deposits/withdrawals. Clicking a row opens a detail page with a Voyager link, mirroring the Transactions detail flow. Integration: - starkzap SDK (^3.0.0) for Vesu deposit/withdraw call preparation, market metadata, and position reads. - next.config aliases shim out starkzap's optional peer deps so Webpack resolves cleanly without bloating node_modules. - Reuses existing noblocks Privy primitives (getStarknetWallet, rawSign, buildReadyAccount, setupPaymaster, deployReadyAccount). No changes to wallet creation, auth, or signing pipelines. Includes scripts/test-starkzap.ts: a read-only integration smoke test that hits Vesu mainnet (~2s) to verify SDK call shapes and pool wiring. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Address reviewer feedback from @0xLucqs on #1: the deposit/withdraw routes accepted a wallet address from the request body without verifying it matched the one derived by buildReadyAccount. No attack vector (auth gates the route), but a buggy client could send a mismatched address and credit a Vesu position to the wrong owner. Cleanest fix per his suggestion: remove the field. Compute the canonical address server-side via computeReadyAddress(walletPublicKey, classHash) using the publicKey fetched from Privy in getStarknetWallet. That value equals what buildReadyAccount derives, so mismatch is now impossible by construction. Drops a redundant validateAndParseAddress block and the unused starknet/validateAndParseAddress import; also drops the unused deployReadyAccount import in withdraw. Client (useEarnHandler) no longer sends address in deposit/withdraw request bodies. UI readiness gate keeps the address check (it gates the button, not the payload). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a Starknet Earn feature: Starkzap/Vesu wrappers and shim, server routes (position/deposit/withdraw) with paymaster flows, client hook and UI, balance raw-unit plumbing, and a smoke-test script. ChangesStarknet Earn Implementation
Sequence Diagram(s) sequenceDiagram
participant Client
participant Hook as useEarnHandler
participant API as /api/starknet/earn
participant Starkzap
participant StarknetRPC
Client->>Hook: deposit(token, amount) / withdraw(...)
Hook->>API: POST /deposit or /withdraw (Bearer JWT)
API->>Starkzap: prepareDeposit / prepareWithdraw
Starkzap->>StarknetRPC: returns Call[] / estimates
API->>StarknetRPC: execute paymaster tx or deploy ready account
StarknetRPC-->>API: transactionHash / receipt
API-->>Hook: { success, transactionHash }
Estimated code review effort: Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
scripts/test-starkzap.ts (1)
57-79: 💤 Low valueConsider importing constants from app/lib/earn.ts to avoid duplication.
The constants are duplicated with a comment "kept in sync by hand." This creates a maintenance burden and risk of drift. Since this script can use ES imports, consider importing these values directly from
app/lib/earn.ts.However, if the goal is to have a truly independent smoke test that doesn't rely on Next.js module resolution, the current approach is acceptable.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/test-starkzap.ts` around lines 57 - 79, Replace the duplicated token and pool constants with imports from the canonical source in app/lib/earn.ts: remove the local STARKNET_USDC_TOKEN, STARKNET_USDT_TOKEN, VESU_USDC_POOL_ADDRESS, and VESU_USDT_POOL_ADDRESS definitions and import the corresponding exported constants (or names used there) instead; ensure the ES import path resolves in this script (adjust relative path or tsconfig/moduleResolution as needed) so the script uses the single source of truth.app/lib/earn.ts (1)
161-167: ⚡ Quick winConsider more granular error handling for position fetching.
The try-catch block swallows all errors and returns an empty positions array. This conflates "user has no position" with "RPC/network error" or "provider malfunction." While returning a safe default prevents crashes, it may hide real connectivity or API issues from monitoring.
Consider either:
- Logging the error before returning the fallback
- Allowing certain error types (e.g., network failures) to propagate
- Returning a structured result that distinguishes "no position" from "fetch failed"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/lib/earn.ts` around lines 161 - 167, The try-catch around provider.getPositions (the call using provider.getPositions?.(ctx, { user: fromAddress(walletAddress) })) currently swallows all errors; update the catch to log the caught error with context (walletAddress and the provider/getPositions call) using the module logger (e.g., processLogger or logger) and then either rethrow network/critical errors or return a structured result so callers can distinguish "no positions" from "fetch failed" (e.g., return { positions: [], error } or throw for retryable errors). Ensure you modify the handling around the positions variable and provider.getPositions usage so the code both logs details and exposes the failure state instead of silently returning an empty array.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/api/starknet/earn/withdraw/route.ts`:
- Around line 219-231: The code currently swallows errors from
account.waitForTransaction and can still return { success: true } later; update
the withdrawal handler so that if account.waitForTransaction throws or
txReceipt.isSuccess() is false you do NOT return confirmed success.
Specifically, in the try/catch around
account.waitForTransaction(result.transaction_hash) (and the txReceipt check),
either return a non-success JSON response (e.g., { error: "Transaction not
confirmed" } with appropriate 4xx/5xx or 202 pending) or rethrow the error so
the outer handler can return failure; ensure you reference
account.waitForTransaction and txReceipt.isSuccess() when making the change and
do not leave an empty catch that masks confirmation failures.
In `@app/components/EarnActivityDetails.tsx`:
- Around line 183-190: The copy button inside the EarnActivityDetails component
is icon-only and needs an explicit accessible label; add an aria-label (for
example aria-label="Copy pool address") to the <button> that calls
copyToClipboard(pool.address, "Pool address") so screen readers announce the
action; locate the button element in EarnActivityDetails (the onClick handler
invoking copyToClipboard) and add the aria-label attribute with a clear
descriptive phrase.
In `@app/components/EarnActivityPanel.tsx`:
- Around line 201-207: The activity row uses a clickable motion.div which isn’t
keyboard-accessible; replace the element with a semantic interactive element by
using motion.button (from framer-motion) or wrap the content in a <button
type="button"> so the existing onClick prop becomes keyboard-operable, keep the
motion props (initial, animate, exit) and className, and ensure you add
type="button" to avoid form submissions and preserve any focus/hover styles and
aria attributes for accessibility.
In `@app/components/EarnWalletForm.tsx`:
- Around line 79-86: The code converts a display number back to base units
causing precision loss; update the Starknet balance flow so EarnWalletForm reads
native base units instead of reconstructing them: extend
fetchStarknetBalancesUnified to populate balancesInWei (matching the EVM
pattern), update the WalletBalances type in BalanceContext to include
balancesInWei for starknetWallet, and modify EarnWalletForm to use
starknetWallet.balancesInWei[token] (or BigInt("0") fallback) for
walletBalanceBaseUnits instead of parsing walletBalanceUnit.toString(); keep
existing display balances for UI only.
---
Nitpick comments:
In `@app/lib/earn.ts`:
- Around line 161-167: The try-catch around provider.getPositions (the call
using provider.getPositions?.(ctx, { user: fromAddress(walletAddress) }))
currently swallows all errors; update the catch to log the caught error with
context (walletAddress and the provider/getPositions call) using the module
logger (e.g., processLogger or logger) and then either rethrow network/critical
errors or return a structured result so callers can distinguish "no positions"
from "fetch failed" (e.g., return { positions: [], error } or throw for
retryable errors). Ensure you modify the handling around the positions variable
and provider.getPositions usage so the code both logs details and exposes the
failure state instead of silently returning an empty array.
In `@scripts/test-starkzap.ts`:
- Around line 57-79: Replace the duplicated token and pool constants with
imports from the canonical source in app/lib/earn.ts: remove the local
STARKNET_USDC_TOKEN, STARKNET_USDT_TOKEN, VESU_USDC_POOL_ADDRESS, and
VESU_USDT_POOL_ADDRESS definitions and import the corresponding exported
constants (or names used there) instead; ensure the ES import path resolves in
this script (adjust relative path or tsconfig/moduleResolution as needed) so the
script uses the single source of truth.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 47bbaec8-df29-4479-89e6-5786a861a515
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
app/api/starknet/earn/deposit/route.tsapp/api/starknet/earn/position/route.tsapp/api/starknet/earn/withdraw/route.tsapp/components/EarnActivityDetails.tsxapp/components/EarnActivityPanel.tsxapp/components/EarnWalletForm.tsxapp/components/WalletDetails.tsxapp/hooks/useEarnHandler.tsapp/lib/_starkzap-unused-shim.tsapp/lib/earn.tsnext.config.mjspackage.jsonscripts/package.jsonscripts/test-starkzap.ts
- deposit/withdraw routes: log a warning when waitForTransaction soft-fails, matching the existing pattern in app/api/starknet/transfer/route.ts instead of silently swallowing the error - EarnActivityDetails: add aria-label to icon-only copy button - EarnActivityPanel: clickable row uses motion.button (with type="button" and w-full/text-left/bg-transparent so the layout matches the prior motion.div) so the row is keyboard-operable - earn.ts: log getPositions failures via console.error before falling back to the empty positions array, so production debugging can distinguish "user has no position" from "fetch failed" Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
CodeRabbit follow-up on PR paycrest#499: 1. Deposit / withdraw responses now include a `confirmed: boolean` field. When `waitForTransaction` throws (network error, RPC hiccup), the handler still returns `success: true` with the tx hash so the client gets the optimistic UX, but `confirmed: false` so the response is honest about the on-chain state. The client can ignore the field today (preserves current UX) or surface it in a future pass. 2. Starknet wallet balances now carry `balancesInWei: Record<string, bigint>` end-to-end: - `fetchStarknetBalancesUnified` (app/utils.ts) was already computing the per-token bigint internally; it now stores and returns it, mirroring the EVM path. - `fetchStarknetBalance` wrapper exposes the new field. - `WalletBalances` interface (app/context/BalanceContext.tsx) declares `balancesInWei?` as an optional field, so existing consumers are unaffected. - `EarnWalletForm` reads `allBalances.starknetWallet?.balancesInWei?.[token]` directly instead of the lossy `parseAmountToBaseUnits(walletBalanceUnit.toString())` round-trip. The Max button and amount validation now use exact base units, eliminating precision loss at the 6th USDC decimal. The `WalletBalances` change is backward-compatible (optional field with a `BigInt("0")` fallback in the consumer); other balance consumers (Transfer, swap, balance display) are unaffected. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/utils.ts (1)
904-910:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd
balanceWeito Starknet entries for consistency with EVM.Starknet
ChainBalanceEntryobjects don't populate thebalanceWeifield, while EVM entries do (Line 784). This inconsistency could break code that expectsbalanceWeito be present for per-token exact-integer math.🔧 Proposed fix
const entries: ChainBalanceEntry[] = tokens.map((token) => ({ chainName, symbol: token.symbol, address: token.address, decimals: token.decimals, balance: balances[token.symbol] ?? 0, + balanceWei: balancesInWei[token.symbol], }));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/utils.ts` around lines 904 - 910, The Starknet token entries created in the tokens.map (producing ChainBalanceEntry objects) are missing the balanceWei field; update the map that builds entries to include balanceWei (populate it the same way EVM entries do) by pulling the per-token exact-integer wei value (e.g., from the existing Wei-balances map or by converting balance+decimals into the integer wei representation) so every ChainBalanceEntry (created in the tokens.map) includes a properly typed balanceWei value.app/context/BalanceContext.tsx (1)
117-135:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftCritical:
balancesInWeiis lost when applying CNGN conversion for EVM balances.The JSDoc at Line 47 states that
balancesInWeiis "Populated by Starknet and EVM fetchers," but EVM balances in this context losebalancesInWeiwhenbuildWalletBalancesFromRawis called:
fetchWalletBalance(utils.ts:973) returnsbalancesInWei.- Callers extract only
result.balances(e.g., Line 295:const rawBalances = { ...rawResult.balances }).buildWalletBalancesFromRawaccepts onlyrawBalances(human-readable numbers) and doesn't preservebalancesInWei.- Result:
balancesInWeiis discarded.This affects Lines 295, 480, 511, 535, 572, 594, 622. The Starknet path (Line 369-371) is fine because
fetchStarknetBalanceresult is set directly without CNGN conversion.Impact: The Earn feature needs
balancesInWeifor exact contract call amounts. If Earn UI reads fromBalanceContext, it won't findbalancesInWeifor EVM networks, breaking deposit/withdraw flows.🔧 Proposed fix
Update
buildWalletBalancesFromRawto accept and preservebalancesInWei:function buildWalletBalancesFromRaw( rawBalances: Record<string, number>, rate: number | null, + balancesInWei?: Record<string, bigint>, ): WalletBalances { const rawTotal = Object.values(rawBalances).reduce( (s, v) => s + (typeof v === "number" && !isNaN(v) ? v : 0), 0, ); const rawResult = { total: rawTotal, balances: rawBalances }; const correctedTotal = calculateCorrectedTotalBalance(rawResult, rate); const correctedBalances = applyCNGNBalanceConversion(rawBalances, rate); return { total: correctedTotal, balances: correctedBalances, rawBalances: { ...rawBalances }, + balancesInWei, cngnUsdUnknown: hasPositiveCngn(rawBalances) && !(rate != null && rate > 0), }; }Then update all call sites to pass
balancesInWei. Example for Line 295:-const rawBalances = { ...rawResult.balances }; -return { - network, - balances: buildWalletBalancesFromRaw(rawBalances, cngnRateValue), -}; +const rawBalances = { ...rawResult.balances }; +const rawBalancesInWei = rawResult.balancesInWei; +return { + network, + balances: buildWalletBalancesFromRaw(rawBalances, cngnRateValue, rawBalancesInWei), +};Apply similar changes to Lines 480, 511, 535, 572, 594, 622.
Also applies to: 295-299, 480-480, 511-511, 535-535, 572-572, 594-594, 622-622
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/context/BalanceContext.tsx` around lines 117 - 135, buildWalletBalancesFromRaw currently only accepts human-readable rawBalances and drops balancesInWei from EVM fetchers; update buildWalletBalancesFromRaw to accept an optional balancesInWei param and preserve it on the returned WalletBalances object (return balancesInWei: balancesInWei ? { ...balancesInWei } : undefined), while keeping existing behavior for calculateCorrectedTotalBalance and applyCNGNBalanceConversion; then update each call site that uses fetchWalletBalance (e.g., where rawBalances is constructed from rawResult.balances in the callers referenced) to pass rawResult.balancesInWei (or undefined) into buildWalletBalancesFromRaw so EVM flows retain balancesInWei for Earn contract calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@app/context/BalanceContext.tsx`:
- Around line 117-135: buildWalletBalancesFromRaw currently only accepts
human-readable rawBalances and drops balancesInWei from EVM fetchers; update
buildWalletBalancesFromRaw to accept an optional balancesInWei param and
preserve it on the returned WalletBalances object (return balancesInWei:
balancesInWei ? { ...balancesInWei } : undefined), while keeping existing
behavior for calculateCorrectedTotalBalance and applyCNGNBalanceConversion; then
update each call site that uses fetchWalletBalance (e.g., where rawBalances is
constructed from rawResult.balances in the callers referenced) to pass
rawResult.balancesInWei (or undefined) into buildWalletBalancesFromRaw so EVM
flows retain balancesInWei for Earn contract calls.
In `@app/utils.ts`:
- Around line 904-910: The Starknet token entries created in the tokens.map
(producing ChainBalanceEntry objects) are missing the balanceWei field; update
the map that builds entries to include balanceWei (populate it the same way EVM
entries do) by pulling the per-token exact-integer wei value (e.g., from the
existing Wei-balances map or by converting balance+decimals into the integer wei
representation) so every ChainBalanceEntry (created in the tokens.map) includes
a properly typed balanceWei value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7c67a3ca-09f4-4e4c-b4e4-614ca15b20f9
📒 Files selected for processing (5)
app/api/starknet/earn/deposit/route.tsapp/api/starknet/earn/withdraw/route.tsapp/components/EarnWalletForm.tsxapp/context/BalanceContext.tsxapp/utils.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- app/api/starknet/earn/deposit/route.ts
- app/api/starknet/earn/withdraw/route.ts
- app/components/EarnWalletForm.tsx
CodeRabbit follow-up on PR paycrest#499: the previous commit added `balancesInWei?` to the WalletBalances interface and plumbed it through the Starknet path, but EVM balances still discarded it in `buildWalletBalancesFromRaw`. That made the JSDoc claim ("Populated by Starknet and EVM fetchers") misleading and left a latent precision gap for any future caller doing exact-integer math on EVM amounts. Concretely: - `buildWalletBalancesFromRaw` now accepts an optional `balancesInWei` parameter and returns it on the WalletBalances object. The CNGN conversion logic is unchanged; base units bypass the float corridor entirely (correct, since CNGN <-> NGN is a display-only rate). - All 7 call sites in BalanceContext.tsx that build WalletBalances from a `fetchWalletBalance` result now pass `result.balancesInWei` through. - The two patch sites in the `[cngnRate]` useEffect (re-applying the rate after it resolves) preserve `balancesInWei` from the existing WalletBalances object. - `fetchStarknetBalancesUnified` (utils.ts) populates `balanceWei` on each ChainBalanceEntry, matching the EVM entries shape so per-token exact-integer math is available downstream. All changes are backward-compatible: `balancesInWei` is optional on WalletBalances, and existing consumers that read only `balances` (display number) are unaffected. The roadmap to bridge EVM USDC into Vesu via the Earn product can now read `allBalances.<chain>?.balancesInWei?.[token]` on any network without further infrastructure changes. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
onahprosper
requested changes
May 14, 2026
…ails Addresses CodeRabbit's follow-up on PR paycrest#499 (after @onahprosper asked for a recheck). The previous approach (return `success: true, confirmed: false`) relied on a client-side check that does not exist in useEarnHandler, so withdrawals where on-chain confirmation failed were still being recorded as completed activity in the UI. Switching to CodeRabbit's original suggestion of an HTTP 502 response: the failure is now honest at the protocol level, the existing client check (`!res.ok || !data?.success`) treats it correctly without any client change, and the response body still carries `transactionHash` so callers can point the user to the explorer. Drops the now-unused `confirmed` flag from both deposit and withdraw response shapes. Same simplification applied to both routes. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Addresses @onahprosper's review feedback on PR paycrest#499: replaces the hard-coded EARN_TOKENS dropdown with a runtime fetch of Paycrest's supported-token list (via the existing `getNetworkTokens("Starknet")` helper, which already wraps `api.paycrest.io/v2/tokens` with cache + fallback). New `useEarnAvailableTokens()` hook in useEarnHandler.ts: - Fetches Paycrest's Starknet token list on mount. - Intersects with `EARN_TOKENS` (the registry of tokens we have Vesu pool mappings for) so we never expose a token without a pool. - Falls back to the full `EARN_TOKENS` list if Paycrest is unreachable or returns no Starknet tokens. EarnWalletForm.tsx now consumes the hook for both the dropdown items and the `onSelect` handler (the previous `name === "USDT" ? ... : ...` hardcode is replaced with a lookup against the dynamic list). Effect: when Paycrest adds a new Starknet token AND we add a matching Vesu pool mapping to `EARN_TOKEN_CONFIG`, the new token appears in the UI with no further code change. The `EARN_TOKENS` registry remains the source of truth for which tokens our backend can build calls for; it is kept static (and exported) for type-level coverage and for the position-refresh / activity-filter loops that should iterate every supported token regardless of Paycrest's current advertisement. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/api/starknet/earn/deposit/route.ts`:
- Around line 214-264: The fee estimation is being run for undeployed accounts
and its result is unused on the deploy path; only estimate fees for the execute
path. Change the guard to run account.estimatePaymasterTransactionFee (and
compute withMargin15) only when isDeployed && !isSponsored, keep maxFee assigned
there, and ensure maxFee is passed into account.executePaymasterTransaction; do
not call estimatePaymasterTransactionFee or apply withMargin15 before calling
deployReadyAccount — rely on deployReadyAccount's internal deployment-aware fee
handling instead.
In `@app/api/starknet/earn/withdraw/route.ts`:
- Around line 189-193: The helper named withMargin15 is misnamed and duplicated;
replace it with a single shared utility (e.g., rename to inflateFeeBy50Pct or
applySafetyMargin) exported from a common module (e.g., app/lib/starknet) that
implements ceil(1.5x) using the existing formula ((bi * 3 + 1) / 2) on BigInt
inputs, then update all call sites that do maxFee =
withMargin15(est.suggested_max_fee_in_gas_token) (and other occurrences) to
import and call the new function; add a short doc comment "Apply 1.5x (50%)
safety margin, ceil" on the new function and remove the duplicated local
definitions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1463edd5-e082-4bc3-a777-e1b2ef77f07f
📒 Files selected for processing (6)
app/api/starknet/earn/deposit/route.tsapp/api/starknet/earn/withdraw/route.tsapp/components/EarnWalletForm.tsxapp/context/BalanceContext.tsxapp/hooks/useEarnHandler.tsapp/utils.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- app/context/BalanceContext.tsx
- app/utils.ts
- app/components/EarnWalletForm.tsx
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
… fee estimation on isDeployed Addresses two CodeRabbit comments on PR paycrest#499: 1. Fee estimation runs on undeployed accounts without deployment context, and the result is discarded on the deploy path. In `app/api/starknet/earn/deposit/route.ts`, the fee-estimation block now only runs when `isDeployed && !isSponsored`. On the first-deposit path (account not yet deployed), `deployReadyAccount` already builds its own `deploy_and_invoke` paymaster transaction with proper `deploymentData` and performs its own deployment-aware fee estimation internally, so estimating here without `deploymentData` produced an inaccurate fee that was never used. Withdraw is unaffected because it already rejects undeployed accounts upfront. 2. `withMargin15` was misleadingly named and duplicated across 5 sites. The helper actually applies a 1.5x (50%) safety margin via `(bi * 3 + 1) / 2`, not 15%. Extracted into a single shared `applySafetyMargin(v)` export in `app/lib/starknet.ts` with a clarifying doc comment. All 5 call sites updated to import and use it: - `app/lib/starknet.ts:deployReadyAccount` - `app/api/starknet/transfer/route.ts` - `app/api/starknet/create-order/route.ts` - `app/api/starknet/earn/deposit/route.ts` - `app/api/starknet/earn/withdraw/route.ts` The local duplicate copies are removed; the formula is unchanged. This keeps the cross-route fee-margin behaviour consistent and gives the helper an honest name that matches the math. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Author
|
Hi @onahprosper — addressing your four threads from commit
CodeRabbit's two follow-up items ( |
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/api/starknet/transfer/route.ts (1)
226-233:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGate fee estimation by deployment state to avoid false 500s.
At Line 226, estimation runs for all non-sponsored requests, but undeployed accounts are handled later by
deployReadyAccount(Line 254). If estimation fails, the handler returns 500 at Line 243 and never reaches deploy flow. Gate this block withisDeployed.💡 Proposed fix
- if (!isSponsored) { + if (!isSponsored && isDeployed) { try { const est = await account.estimatePaymasterTransactionFee( calls, paymasterDetails, ); maxFee = applySafetyMargin(est.suggested_max_fee_in_gas_token); } catch (error: any) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/api/starknet/transfer/route.ts` around lines 226 - 233, The fee estimation for non-sponsored requests should only run when the account is already deployed; wrap the existing estimation block (the isSponsored check that calls account.estimatePaymasterTransactionFee, sets maxFee via applySafetyMargin on est.suggested_max_fee_in_gas_token, and catches errors) with an additional guard checking isDeployed so that undeployed accounts proceed to deployReadyAccount instead of triggering the catch path and returning a 500; ensure paymasterDetails and calls still flow into the estimation when isDeployed is true and preserve the existing error handling and maxFee assignment.app/api/starknet/create-order/route.ts (1)
280-287:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid pre-deploy estimation failures blocking create-order.
At Line 280, fee estimation runs before the deployment branch at Line 307. For undeployed accounts, estimation errors return 500 (Line 297) and prevent
deployReadyAccountfrom executing. Restrict estimation to deployed accounts.💡 Proposed fix
- if (!isSponsored) { + if (!isSponsored && isDeployed) { try { const est = await account.estimatePaymasterTransactionFee( calls, paymasterDetails, ); maxFee = applySafetyMargin(est.suggested_max_fee_in_gas_token); } catch (error: any) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/api/starknet/create-order/route.ts` around lines 280 - 287, The fee estimation is running for undeployed accounts and can throw, blocking deployReadyAccount; move or guard the call to account.estimatePaymasterTransactionFee (and applySafetyMargin) so it only runs for already-deployed accounts (e.g., check the existing deployment indicator used in this file such as deployReadyAccount/deployTransactionHash or an isDeployed flag) and skip estimation when the account is not deployed or when isSponsored is true, allowing deployReadyAccount to proceed uninterrupted.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@app/api/starknet/create-order/route.ts`:
- Around line 280-287: The fee estimation is running for undeployed accounts and
can throw, blocking deployReadyAccount; move or guard the call to
account.estimatePaymasterTransactionFee (and applySafetyMargin) so it only runs
for already-deployed accounts (e.g., check the existing deployment indicator
used in this file such as deployReadyAccount/deployTransactionHash or an
isDeployed flag) and skip estimation when the account is not deployed or when
isSponsored is true, allowing deployReadyAccount to proceed uninterrupted.
In `@app/api/starknet/transfer/route.ts`:
- Around line 226-233: The fee estimation for non-sponsored requests should only
run when the account is already deployed; wrap the existing estimation block
(the isSponsored check that calls account.estimatePaymasterTransactionFee, sets
maxFee via applySafetyMargin on est.suggested_max_fee_in_gas_token, and catches
errors) with an additional guard checking isDeployed so that undeployed accounts
proceed to deployReadyAccount instead of triggering the catch path and returning
a 500; ensure paymasterDetails and calls still flow into the estimation when
isDeployed is true and preserve the existing error handling and maxFee
assignment.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 06ae40b3-06d0-4264-982b-4af1eeda0f47
📒 Files selected for processing (5)
app/api/starknet/create-order/route.tsapp/api/starknet/earn/deposit/route.tsapp/api/starknet/earn/withdraw/route.tsapp/api/starknet/transfer/route.tsapp/lib/starknet.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- app/api/starknet/earn/withdraw/route.ts
- app/api/starknet/earn/deposit/route.ts
Author
|
Re: CodeRabbit review #pullrequestreview-4290058247 — both findings (
Keeping this PR scoped to Earn. The |
Collaborator
All good. |
Collaborator
All good mate |
onahprosper
approved these changes
May 16, 2026
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
3 participants
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.
Description
Adds an Earn product to the Starknet wallet drawer, letting users supply USDC and USDT to Vesu lending pools through the starkzap SDK.
User-facing
FormDropdown), Max chips, live APR + monthly / yearly projection, success view mirroringTransferForm.TransactionDetails: token + network logos, amount, pool name (linked to Vesu Pro), pool address (copy), date, status, "View in Explorer" link to Voyager.Pool selection
0x01bc5de5…5803)0x0451fe48…c3b5)Server-side
app/api/starknet/earn/{deposit,withdraw,position}/route.ts: auth via the existingverifyJWTpipeline; reusesgetStarknetWallet,rawSign,buildReadyAccount,setupPaymaster, anddeployReadyAccountunchanged.computeReadyAddress(walletPublicKey, classHash)using the public key fetched from Privy. Clients do not supply the address, so a mismatch between the signing account and the Vesu position owner is impossible by construction.Integration approach
starkzap(^3.0.0) handles Vesu market reads and deposit / withdraw call preparation. This integration uses only starkzap's Vesu module, not its Tongo or Hyperlane / Solana bridge features.@fatsolutions/tongo-sdk,@hyperlane-xyz/*,@solana/web3.js),next.config.mjs(and the matching TurbopackresolveAlias) routes them to a small Proxy shim (app/lib/_starkzap-unused-shim.ts) so the bundler resolves cleanly without installing the unused peers. The shim throws on any property access; at runtime this never fires because the Vesu path does not touch Tongo or Solana modules.Live APR
Fetched from
https://api.vesu.xyz/marketsvia starkzap'sVesuLendingProvider.getMarkets()(seeapp/lib/earn.ts:183-198) and polled every 30s while the modal or activity tab is open. No hardcoded values, no fallback rates. We display the grosssupplyApyfield; Vesu's own UI shows post-fee APR, the delta being the protocolfeeRatereturned in the same response.Breaking changes: none.
References
None.
Testing
Automated (read-only, ~2s, hits live Vesu mainnet):
Verifies:
getMarketsresolves both pinned pools;supplyApyis in[0, 0.5]; position reads return well-formed structures;prepareDeposit/prepareWithdrawproduce the correct call shapes (approve target, deposit / withdraw entrypoints, vToken vs underlying); pool-name to address mapping matches the Vesu API. Use this as a regression check on starkzap upgrades.Manual:
pnpm install && pnpm build: confirm the build passes.Environment: Node 22.x, pnpm 10.27, macOS / Linux. TypeScript / Next.js 15.
Checklist
A read-only starkzap smoke test is included at
scripts/test-starkzap.ts. No Jest / unit coverage was added; existing components that the new UI mirrors (TransferForm,TransactionDetails) retain their existing coverage.All active GitHub checks for tests, formatting, and security are passing
The correct base branch is being used, if not
mainBy submitting a PR, I agree to Paycrest's Contributor Code of Conduct and Contribution Guide.
Summary by CodeRabbit
New Features
UI
Client
Chores
Tests