Feat starknet on noblocks#342
Conversation
|
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 end-to-end Starknet support: new Starknet React context, hooks, utils and lib for Privy raw-signing and paymaster flows, server API routes for Starknet wallet/order/transfer, UI and balance wiring, network mock, env vars and new dependency. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant Backend
participant Privy
participant StarknetRPC as Starknet RPC
participant Paymaster
User->>App: UI requests create-wallet
App->>Backend: POST /api/starknet/create-wallet (Bearer JWT)
Backend->>Privy: verify JWT, find/create wallet
alt wallet exists
Backend-->>App: {walletId, address, publicKey}
else
Backend->>Privy: createWallet -> maybe fetch publicKey
Backend-->>App: {walletId, address, publicKey}
end
App->>App: StarknetContext.persist -> localStorage
sequenceDiagram
participant User
participant App
participant Backend
participant Privy
participant Paymaster
participant StarknetRPC as Starknet RPC
User->>App: Initiate Starknet transfer/create-order
App->>Backend: POST /api/starknet/transfer or /create-order (JWT + params)
Backend->>Privy: Validate JWT, get wallet & auth key
Backend->>StarknetRPC: check deployment / estimate fees
Backend->>Paymaster: setupPaymaster (API key/mode/gasToken)
Backend->>Backend: buildReadyAccount (Privy rawSign signer)
alt account not deployed
Backend->>Paymaster: deployReadyAccount (estimate + execute)
else
Backend->>Paymaster: execute paymaster transaction (estimate + execute)
end
Backend->>StarknetRPC: wait for confirmation -> parse events (orderId)
Backend-->>App: { transactionHash, orderId, status }
App-->>User: update UI status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 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)
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: 12
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/components/MobileDropdown.tsx (1)
102-104: Missing Starknet balance handling - inconsistent with WalletDetails.tsx.The
activeBalancederivation doesn't account for Starknet, unlikeWalletDetails.tsx(lines 79-83) which includesallBalances.starknetWalletfor Starknet networks. This could show incorrect balances when on Starknet.Suggested fix for consistency
const activeBalance = isInjectedWallet ? allBalances.injectedWallet - : allBalances.smartWallet; + : selectedNetwork.chain.name === "Starknet" + ? allBalances.starknetWallet + : allBalances.smartWallet;app/components/TransferForm.tsx (1)
190-192: MissingstarknetWalletBalancein useEffect dependency array.The effect uses
starknetWalletBalance(line 170) but it's not in the dependency array. This can cause stale data when Starknet balance updates don't trigger a re-fetch oftransferNetworkBalance.🐛 Proposed fix
}, [transferNetwork, user?.linkedAccounts]); + }, [transferNetwork, user?.linkedAccounts, starknetWalletBalance]);
🤖 Fix all issues with AI agents
In @app/api/starknet/create-wallet/route.ts:
- Around line 47-71: The existence check for a Starknet wallet (privy.getUser
and existingStarknetWallet logic) swallows errors and allows creation to
proceed; change the catch in the try/catch that wraps privy.getUser so that on
error you stop the flow and return an error response (e.g., NextResponse.json
with success: false and a 500 status) instead of continuing to create a new
wallet, ensuring the route does not create duplicates when the check fails; keep
the error log (console.error) but immediately return after logging to abort
further processing.
In @app/api/starknet/get-public-key/route.ts:
- Around line 57-61: The response currently returns the full wallet object
alongside publicKey in the NextResponse.json call; remove the wallet field to
avoid leaking sensitive internal data and return only the minimal fields (e.g.,
success and publicKey). Locate the return statement that constructs the JSON
(the block returning { success: true, publicKey: publicKey, wallet: wallet })
and modify it to omit wallet (or replace it with a safe, explicit field such as
walletAddress if you must return a derived public identifier), ensuring only
publicKey (and success) are sent back.
- Around line 49-54: The error response in the get-public-key route currently
returns the full wallet object; remove the wallet property from the
NextResponse.json error response so sensitive wallet details are not exposed.
Locate the return that calls NextResponse.json({ error: "...", wallet: wallet },
{ status: 404 }) in the route handler and change it to only return the error
message (and status 404); if you need the wallet for diagnostics keep it for
internal logging but do not include it in the JSON response.
- Around line 32-55: The route currently fetches a wallet by walletId but never
verifies ownership against the authenticated user (authUserId), so add an
authorization check after obtaining the wallet: compare wallet.userId (or
wallet.ownerId/public_owner field if present) to authUserId retrieved earlier
and if they do not match return a 403 JSON response; ensure you also handle the
case where getWallet returns null/undefined by returning 404 before the
ownership check. Use the existing symbols wallet, authUserId, and
privy.walletApi.getWallet to locate where to add the check and the early error
responses.
In @app/context/StarknetContext.tsx:
- Around line 208-234: refreshWalletState calls getAccessToken() but doesn't
handle a null token; update refreshWalletState to check the returned token
before using it (e.g., if token is null/undefined, return early or skip the
fetch and optionally clear auth-dependent state), and ensure the Authorization
header is only set when token is present (or omit header entirely when token is
null) — adjust the logic inside refreshWalletState (referencing getAccessToken,
refreshWalletState, and the fetch to /api/starknet/wallet-state) accordingly.
- Around line 135-206: The createWallet function must guard against a missing
access token and ensure its declared return type matches actual returns: after
calling getAccessToken() (in createWallet), check if token is null/undefined and
setError/throw a descriptive Error before using it in Authorization headers; and
for the wallet id return value either change the function signature to
Promise<string | null> or enforce a non-null newWalletId by throwing an Error if
newWalletId is null (so callers always receive a string). Update setError/throw
messages accordingly and adjust any call sites or types that rely on
createWallet returning a non-null string.
- Around line 45-74: The effect incorrectly assumes a linked Privy wallet is
deployed and uses loose typing; update the useEffect handling of
user.linkedAccounts to (1) stop setting deployed=true and remove deployed from
saveToLocalStorage here—deployment must be determined via an on‑chain check or
by calling deployReadyAccount from app/lib/starknet.ts instead, and (2) replace
the (account: any) callback with a proper type or a type guard matching the
linked account shape (e.g., an interface with type, chainType/chain_type, id,
address, publicKey/public_key) so the find and property accesses are type-safe
(apply the same typing pattern used in app/api/starknet/create-wallet/route.ts).
In @app/hooks/useSmartWalletTransfer.ts:
- Around line 137-192: In useSmartWalletTransfer, add validations for
Starknet-specific fields before making the API call: ensure
starknetWallet.address is present and non-null (validate
starknetWallet.address), ensure process.env.NEXT_PUBLIC_STARKNET_READY_CLASSHASH
(classHash) is defined and non-empty, and verify starknetWallet.deployed ===
true (reject/throw if not deployed); if any check fails, set an appropriate
error via setError, call trackEvent("Transfer failed", ...) with the failure
reason, and throw an Error to stop execution before calling getAccessToken or
fetch.
In @app/lib/authorization.ts:
- Around line 5-34: In getUserAuthorizationKey, remove the incorrect string cast
for res.expiresAt and parse it as a number (e.g., Number(res.expiresAt)) when
computing expiresAt; avoid using the shared "unknown" fallback for cacheKey —
require userId or derive a per-request key (e.g., hash userJwt) so entries for
anonymous requests don’t collide; and replace the unbounded userSignerCache Map
with a bounded cache (implement a simple LRU or size-limited eviction around
userSignerCache) to prevent unbounded memory growth when storing entries from
generateUserSigner.
In @app/mocks.ts:
- Around line 4-26: The StarknetMainnet mock incorrectly constructs the chain ID
and leaves the RPC URL without a fallback: replace the id generation in the
starknetMainnet object (id property) with the correct Starknet Mainnet chain ID
hex string "0x534e5f4d41494e" (do not use toHex or BigInt on the literal), and
update rpcUrls.default.http and rpcUrls.public.http to use
process.env.NEXT_PUBLIC_STARKNET_RPC_URL with a sensible fallback (e.g., a known
public Starknet mainnet RPC URL) so the value is never undefined at runtime.
In @app/pages/TransactionForm.tsx:
- Around line 122-126: The balance selection in TransactionStatus.tsx is
inverted and missing the Starknet network check; update the logic that computes
activeBalance (or similarly named variable) to match TransactionForm.tsx: if
isInjectedWallet use injectedWalletBalance, else if selectedNetwork.chain.name
=== "Starknet" use starknetWalletBalance, else use smartWalletBalance; locate
the balance selection in TransactionStatus.tsx (around where activeBalance or
balance is derived, lines ~348-350) and replace the inverted condition with this
three-way ternary/if sequence so injected wallets are prioritized and Starknet
uses starknetWalletBalance.
In @app/pages/TransactionPreview.tsx:
- Around line 198-277: The Starknet branch inside the TransactionPreview flow
(the if checking selectedNetwork.chain.name === "Starknet" && !isInjectedWallet)
completes the order creation but does not return, so execution falls through
into the injected/smart wallet paths and duplicates work; fix it by adding an
early return immediately after the Starknet success sequence (for example right
after setIsOrderCreated(true) and trackEvent(...)) so the function exits once
the Starknet order is created and prevents calling the injected wallet/smart
wallet logic and getOrderId() again.
🟡 Minor comments (9)
app/hooks/useCNGNRate.ts-44-51 (1)
44-51: Update comment to reflect actual behavior.The comment states "(testnet, not supported by aggregator)" but the code skips rate fetching for all Starknet networks (both testnet and mainnet). According to the PR objectives, Starknet Mainnet was also added. The comment should clarify that this applies to all Starknet networks, not just testnet.
📝 Suggested comment update
- // Skip rate fetching for Starknet networks (testnet, not supported by aggregator) + // Skip rate fetching for Starknet networks (not yet supported by aggregator) if (network.toLowerCase().includes("starknet")) {app/hooks/useCNGNRate.ts-116-120 (1)
116-120: Update comment and consider more robust network detection.Same as the hook above—the comment mentions "testnet" but applies to all Starknet networks. Additionally, using
.includes("starknet")for network detection could match unintended network names (e.g., "not-starknet-compatible" would also match).📝 Suggested improvements
Fix the comment:
- // Skip rate fetching for Starknet networks (testnet, not supported by aggregator) + // Skip rate fetching for Starknet networks (not yet supported by aggregator) if (network.toLowerCase().includes("starknet")) {Optional: More robust network detection (if network naming conventions are well-defined):
- if (network.toLowerCase().includes("starknet")) { + const isStarknetNetwork = network.toLowerCase().startsWith("starknet") || + network.toLowerCase() === "starknet sepolia" || + network.toLowerCase() === "starknet mainnet"; + if (isStarknetNetwork) {Or consider using a helper function for network type detection if this pattern is repeated across the codebase.
app/context/TokensContext.tsx-75-76 (1)
75-76: Misleading comment about Starknet handling.The comment states "Skip Starknet since we already handled it above", but there is no Starknet-specific handling visible in the preceding code (lines 34-74).
Update the comment to accurately reflect the reason for skipping Starknet, such as "Skip Starknet as its tokens are managed separately" or provide the actual location where Starknet tokens are handled.
📝 Proposed fix for the comment
// Merge fallback tokens for any networks missing from API response Object.keys(FALLBACK_TOKENS).forEach((networkName) => { - // Skip Starknet since we already handled it above + // Skip Starknet as it uses a separate token management flow if (networkName === "Starknet") return;app/api/starknet/create-wallet/route.ts-32-42 (1)
32-42: Remove unused request body parsing and redundant validation.Line 33 parses the request body but never uses it. Lines 37-42 perform a redundant check since
userIdis assigned fromauthUserId, which was already validated at line 25.🧹 Proposed cleanup
-// Get request body -const body = await request.json(); - const userId = authUserId; -if (!userId) { - return NextResponse.json( - { error: "No user ID available" }, - { status: 400 }, - ); -} - // First, check if user already has a Starknet walletapp/hooks/useWalletAddress.ts-22-29 (1)
22-29: Unhandled exception risk fromnormalizeStarknetAddress.According to
app/utils.ts(lines 746-774),normalizeStarknetAddressthrows errors for invalid addresses (e.g., missing0xprefix, invalid hex characters, address too long). IfstarknetAddressfrom context is malformed, this hook will throw an unhandled exception.Consider wrapping in a try-catch for graceful degradation:
Suggested defensive handling
// If on Starknet, return Starknet wallet address if (selectedNetwork?.chain?.name === "Starknet") { if (!starknetAddress) { return undefined; } - const address = normalizeStarknetAddress(starknetAddress); - return address ?? starknetAddress; + try { + return normalizeStarknetAddress(starknetAddress); + } catch { + // Fallback to raw address if normalization fails + return starknetAddress; + } }app/api/starknet/transfer/route.ts-128-130 (1)
128-130: BigInt conversion can throw for invalid input.
BigInt(amount)will throw aSyntaxErrorifamountis not a valid integer string (e.g., contains decimals, non-numeric characters, or is empty). Consider validating the amount format before conversion.Suggested validation
+ // Validate amount is a valid integer string + if (!/^\d+$/.test(String(amount))) { + return NextResponse.json( + { error: "Invalid amount format: must be a non-negative integer string" }, + { status: 400 }, + ); + } + // Convert amount to u256 format const amountU256 = cairo.uint256(BigInt(amount));app/utils.ts-564-565 (1)
564-565: Potential precision loss for large token balances.Converting
BigInttoNumberviaNumber(balanceInWei)can lose precision for values exceedingNumber.MAX_SAFE_INTEGER(2^53 - 1). While unlikely for typical balances, very large holdings could show incorrect values.Consider using string-based division or a BigInt-safe decimal library for precision-critical calculations:
// Alternative approach using string manipulation for precision const balanceStr = balanceInWei.toString(); const decimals = token.decimals; const intPart = balanceStr.slice(0, -decimals) || '0'; const decPart = balanceStr.slice(-decimals).padStart(decimals, '0'); const balance = parseFloat(`${intPart}.${decPart}`);app/pages/TransactionPreview.tsx-272-276 (1)
272-276: Inconsistent property name in trackEvent.Line 275 uses
result.transaction_hashbut the response from the API (line 276 in route.ts) returnstransactionHash. This will result inundefinedbeing tracked.🐛 Proposed fix
trackEvent("Swap started", { "Entry point": "Transaction preview", "Wallet type": "Starknet embedded wallet", - "Transaction hash": result.transaction_hash, + "Transaction hash": result.transactionHash, });app/lib/starknet.ts-285-289 (1)
285-289: Empty string fallback for RPC URL can cause cryptic errors.If
NEXT_PUBLIC_STARKNET_RPC_URLis not set, the empty string will be used, leading to confusing network errors rather than a clear configuration error.🐛 Proposed fix
export function getRpcProvider() { const rpcUrl = process.env.NEXT_PUBLIC_STARKNET_RPC_URL || ""; + if (!rpcUrl) { + throw new Error("NEXT_PUBLIC_STARKNET_RPC_URL environment variable is required"); + } return new RpcProvider({ nodeUrl: rpcUrl }); }
🧹 Nitpick comments (13)
app/api/starknet/create-wallet/route.ts (1)
73-89: Consider adding wallet creation validation.The route returns the wallet immediately after creation without verifying the operation succeeded or the wallet is in a usable state. Additionally, the multiple property fallbacks (
public_key || publicKey,chainType || chain_type) suggest API response inconsistency.Consider:
- Adding explicit validation that required wallet properties are present before returning
- Investigating the Privy API response schema to use proper TypeScript types instead of
as any- Documenting why property fallbacks are necessary (API versioning, inconsistent responses, etc.)
app/api/starknet/transfer/route.ts (2)
198-214: Transaction confirmation could block indefinitely.
account.waitForTransaction()without a timeout could hang indefinitely if the transaction is stuck or the network is slow. The current catch block only logs and continues, but this doesn't prevent the request from timing out at the infrastructure level.Consider adding a timeout or documenting the expected behavior.
Suggested timeout handling
// Wait for transaction confirmation try { - const txReceipt = await account.waitForTransaction( - result.transaction_hash, - ); + const txReceipt = await Promise.race([ + account.waitForTransaction(result.transaction_hash), + new Promise((_, reject) => + setTimeout(() => reject(new Error("Transaction confirmation timeout")), 60000) + ), + ]); if (!txReceipt.isSuccess()) {
210-214: Useconsole.warnfor warning messages.Line 211 uses
console.logfor a warning message. Consider usingconsole.warnfor consistency with the message severity.Minor fix
} catch (error) { - console.log( + console.warn( "[API] Warning: Could not confirm transaction, but it may still succeed", ); }app/lib/authorization.ts (2)
36-49: Consider explicit error handling over silent empty string.Returning an empty string (Line 48) when signature generation fails may lead to silent failures downstream. Consider returning
nullor throwing an error to make failures explicit and easier to debug.♻️ Proposed alternative
export function buildAuthorizationSignature({ input, authorizationKey, }: { input: WalletApiRequestSignatureInput; authorizationKey: string; }): string { + if (!input || !authorizationKey) { + throw new Error("Missing required parameters for authorization signature"); + } + const signature = generateAuthorizationSignature({ input, authorizationPrivateKey: authorizationKey, }); - return signature ?? ""; + if (!signature) { + throw new Error("Failed to generate authorization signature"); + } + return signature; }
1-49: Consider adding JSDoc documentation.Since this file exports a public API (
getUserAuthorizationKeyandbuildAuthorizationSignature), adding JSDoc comments would improve developer experience and document expected behavior, parameters, and return values.app/context/StarknetContext.tsx (1)
76-103: Consider more robust localStorage handling.
Boolean parsing (Line 95): The check
storedDeployed === "true"is correct but could be more explicit. Consider usingJSON.parseor explicitly checking for"false"as well.Missing error handling: localStorage access can fail in private browsing or when storage is full. Consider wrapping in try/catch to prevent crashes.
♻️ Proposed improvements
useEffect(() => { + try { if (authenticated && user?.id) { const storedWalletId = localStorage.getItem( `${STORAGE_PREFIX}walletId_${user.id}`, ); // ... other getItem calls ... const storedDeployed = localStorage.getItem( `${STORAGE_PREFIX}deployed_${user.id}`, ); if (storedWalletId) setWalletId(storedWalletId); if (storedAddress) setAddress(storedAddress); if (storedPublicKey) setPublicKey(storedPublicKey); - if (storedDeployed === "true") setDeployed(true); + setDeployed(storedDeployed === "true"); } else if (!user) { setWalletId(null); setAddress(null); setPublicKey(null); setDeployed(false); setError(null); } + } catch (error) { + console.error("Failed to load Starknet wallet state from localStorage:", error); + } }, [authenticated, user]);app/hooks/useSmartWalletTransfer.ts (1)
238-262: Consider more robust error type detection.The error type detection (Lines 256-260) relies on string matching (
includes("429"),includes("HTTP")), which could be fragile if error message formats change. Consider using error codes or more structured error properties if available from the underlying libraries.app/api/starknet/create-order/route.ts (4)
51-52: Inconsistent variable naming convention.
WalletAddressuses PascalCase while other destructured variables use camelCase. This should bewalletAddressfor consistency.♻️ Suggested fix
- address: WalletAddress, + address: walletAddress, } = body; const provider = getRpcProvider(); try { - await provider.getClassHashAt(WalletAddress); + await provider.getClassHashAt(walletAddress); isDeployed = true;Also update line 242:
- address: starknetAddress, + address: walletAddress,
114-114: Potential confusion betweenpublicKeyfrom body andwalletPublicKeyfrom wallet lookup.The code extracts
publicKeyfrom the request body (line 40) but then fetcheswalletPublicKeyfromgetStarknetWallet(line 114) and uses only the latter. Consider clarifying intent or removing the unusedpublicKeyfrom destructuring if it's not needed.
256-262: Extract magic hex string to a named constant.The event key
0x3427759bfd3b607...should be a named constant for maintainability and clarity about what event it represents.♻️ Suggested fix
+const ORDER_CREATED_EVENT_KEY = "0x3427759bfd3b941f14e687e129519da3c9b0046c5b9aaa290bb1dede63753b3"; + // In the event processing: - if ( - Object.values(event.keys).includes( - "0x3427759bfd3b941f14e687e129519da3c9b0046c5b9aaa290bb1dede63753b3", - ) - ) { + if (Object.values(event.keys).includes(ORDER_CREATED_EVENT_KEY)) {
247-248: Hardcodedwaitvariable is unused as a configurable option.The
waitvariable is hardcoded totrueand never configurable. Either remove it and inline the condition, or make it a request parameter if waiting should be optional.♻️ Suggested fix
- const wait = true; - if (wait) { + // Wait for transaction confirmation and extract orderId + { try { const txReceipt = await account.waitForTransaction(app/utils.ts (1)
482-514: Consider caching CoinGecko price responses.
fetchTokenPricemakes an API call for each token on every balance fetch. CoinGecko has rate limits (10-50 calls/minute for free tier). Consider:
- Caching prices with a short TTL (e.g., 30-60 seconds)
- Batching multiple token prices into a single API call
♻️ Example caching approach
// Price cache with 60-second TTL const priceCache: Map<string, { price: number; timestamp: number }> = new Map(); const PRICE_CACHE_TTL = 60 * 1000; async function fetchTokenPrice(tokenSymbol: string): Promise<number | null> { const cached = priceCache.get(tokenSymbol); if (cached && Date.now() - cached.timestamp < PRICE_CACHE_TTL) { return cached.price; } // ... existing fetch logic ... if (price !== null) { priceCache.set(tokenSymbol, { price, timestamp: Date.now() }); } return price; }app/lib/starknet.ts (1)
181-189: Fragile signature extraction with multiple fallback paths.The code checks multiple nested paths for the signature (
data?.signature,data?.result?.signature, etc.). This suggests uncertainty about the API response format. Consider documenting the expected response structure or adding explicit validation.// Document expected response format from Privy // Expected: { signature: "0x..." } or { result: { signature: "0x..." } }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlpublic/logos/eth-logo.svgis excluded by!**/*.svgpublic/logos/strk-logo.svgis excluded by!**/*.svg
📒 Files selected for processing (30)
.env.exampleapp/api/starknet/create-order/route.tsapp/api/starknet/create-wallet/route.tsapp/api/starknet/get-public-key/route.tsapp/api/starknet/transfer/route.tsapp/api/track-logout/route.tsapp/components/MainPageContent.tsxapp/components/MobileDropdown.tsxapp/components/Navbar.tsxapp/components/NetworkSelectionModal.tsxapp/components/NetworksDropdown.tsxapp/components/SettingsDropdown.tsxapp/components/TransferForm.tsxapp/components/WalletDetails.tsxapp/context/BalanceContext.tsxapp/context/StarknetContext.tsxapp/context/TokensContext.tsxapp/context/index.tsapp/hooks/useCNGNRate.tsapp/hooks/useSmartWalletTransfer.tsapp/hooks/useWalletAddress.tsapp/lib/authorization.tsapp/lib/privy.tsapp/lib/starknet.tsapp/mocks.tsapp/pages/TransactionForm.tsxapp/pages/TransactionPreview.tsxapp/providers.tsxapp/utils.tspackage.json
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-10-10T16:44:32.125Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 244
File: app/components/CopyAddressWarningModal.tsx:48-52
Timestamp: 2025-10-10T16:44:32.125Z
Learning: In the CopyAddressWarningModal component (app/components/CopyAddressWarningModal.tsx), selectedNetwork from useNetwork() is always defined and does not require null safety checks when accessing its properties like selectedNetwork.chain.name.
Applied to files:
app/hooks/useWalletAddress.tsapp/components/NetworksDropdown.tsxapp/hooks/useSmartWalletTransfer.tsapp/components/Navbar.tsxapp/components/MobileDropdown.tsxapp/components/NetworkSelectionModal.tsxapp/components/WalletDetails.tsxapp/components/TransferForm.tsxapp/context/BalanceContext.tsxapp/components/MainPageContent.tsxapp/utils.tsapp/pages/TransactionPreview.tsxapp/components/SettingsDropdown.tsx
📚 Learning: 2025-07-23T07:30:23.720Z
Learnt from: jeremy0x
Repo: paycrest/noblocks PR: 177
File: app/utils.ts:179-196
Timestamp: 2025-07-23T07:30:23.720Z
Learning: The `normalizeNetworkName` function in app/utils.ts has been updated to be dynamic and scalable, converting any network identifier to sentence case with hyphens replaced by spaces and proper handling of acronyms like "BNB".
Applied to files:
app/context/TokensContext.tsxapp/components/NetworkSelectionModal.tsxapp/utils.ts
📚 Learning: 2025-11-06T07:37:39.036Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 231
File: app/components/recipient/RecipientDetailsForm.tsx:539-552
Timestamp: 2025-11-06T07:37:39.036Z
Learning: In RecipientDetailsForm (app/components/recipient/RecipientDetailsForm.tsx), when isRecipientNameEditable is false (verification succeeded), the recipient safety alert should display when: !isRecipientNameEditable && recipientName && !recipientNameError. The !errors.recipientName check is unnecessary because in non-editable mode the recipient name is displayed as read-only text (not an input field), so form validation errors cannot occur.
Applied to files:
app/components/TransferForm.tsx
📚 Learning: 2025-11-06T07:37:39.036Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 231
File: app/components/recipient/RecipientDetailsForm.tsx:539-552
Timestamp: 2025-11-06T07:37:39.036Z
Learning: In RecipientDetailsForm (app/components/recipient/RecipientDetailsForm.tsx), when isRecipientNameEditable is true (verification failed/returned "Ok"), the recipient safety alert should display when: isRecipientNameEditable && recipientName && !errors.recipientName && !recipientNameError. The !isFetchingRecipientName check is redundant because recipientName is cleared at fetch start and only populated after fetching completes or when the user manually enters it.
Applied to files:
app/components/TransferForm.tsx
📚 Learning: 2025-12-05T02:22:28.063Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 312
File: app/page.tsx:7-11
Timestamp: 2025-12-05T02:22:28.063Z
Learning: In Next.js 15 with React 19, components using useSearchParams() cannot be statically prerendered. When app/layout.tsx has `export const dynamic = "force-static"`, MainPageContent must be dynamically imported with ssr: false to avoid "Element type is invalid" errors during build. The dynamic import pattern `dynamic(() => import("./components/MainPageContent").then(mod => ({ default: mod.MainPageContent })), { ssr: false })` is the correct solution for this scenario in the noblocks repository.
Applied to files:
app/components/MainPageContent.tsx
📚 Learning: 2025-07-23T07:28:30.491Z
Learnt from: jeremy0x
Repo: paycrest/noblocks PR: 177
File: app/utils.ts:179-196
Timestamp: 2025-07-23T07:28:30.491Z
Learning: The `normalizeNetworkName` function should be dynamic and scalable, converting any network identifier to sentence case with hyphens replaced by spaces, rather than using a hardcoded mapping.
Applied to files:
app/utils.ts
🧬 Code graph analysis (20)
app/api/starknet/get-public-key/route.ts (4)
app/api/starknet/create-wallet/route.ts (1)
POST(10-99)app/lib/jwt.ts (1)
verifyJWT(11-46)app/lib/config.ts (1)
DEFAULT_PRIVY_CONFIG(29-36)app/lib/privy.ts (1)
getPrivyClient(9-24)
app/hooks/useWalletAddress.ts (3)
app/context/index.ts (3)
useInjectedWallet(6-6)useStarknet(15-15)useNetwork(2-2)app/context/StarknetContext.tsx (1)
useStarknet(301-307)app/utils.ts (1)
normalizeStarknetAddress(747-775)
app/api/starknet/create-order/route.ts (1)
app/lib/starknet.ts (5)
getRpcProvider(285-289)getStarknetWallet(89-112)setupPaymaster(294-342)buildReadyAccount(195-280)deployReadyAccount(348-440)
app/api/starknet/create-wallet/route.ts (3)
app/lib/jwt.ts (1)
verifyJWT(11-46)app/lib/config.ts (1)
DEFAULT_PRIVY_CONFIG(29-36)app/lib/privy.ts (1)
getPrivyClient(9-24)
app/components/NetworksDropdown.tsx (1)
app/context/StarknetContext.tsx (1)
useStarknet(301-307)
app/pages/TransactionForm.tsx (2)
app/context/BalanceContext.tsx (1)
useBalance(262-268)app/context/index.ts (1)
useBalance(3-3)
app/context/StarknetContext.tsx (1)
app/context/index.ts (2)
StarknetProvider(15-15)useStarknet(15-15)
app/hooks/useSmartWalletTransfer.ts (1)
app/hooks/analytics/analytics-utils.ts (1)
trackEvent(231-231)
app/components/Navbar.tsx (1)
app/hooks/useWalletAddress.ts (1)
useWalletAddress(11-34)
app/components/MobileDropdown.tsx (5)
app/context/StarknetContext.tsx (1)
useStarknet(301-307)app/context/index.ts (2)
useStarknet(15-15)useInjectedWallet(6-6)app/context/InjectedWalletContext.tsx (1)
useInjectedWallet(114-114)app/hooks/useWalletAddress.ts (1)
useWalletAddress(11-34)app/components/index.ts (1)
CopyAddressWarningModal(11-11)
app/lib/authorization.ts (1)
app/lib/privy.ts (1)
getPrivyClient(9-24)
app/components/NetworkSelectionModal.tsx (2)
app/context/StarknetContext.tsx (1)
useStarknet(301-307)app/context/index.ts (1)
useStarknet(15-15)
app/components/WalletDetails.tsx (2)
app/hooks/useWalletAddress.ts (1)
useWalletAddress(11-34)app/utils.ts (1)
formatCurrency(70-83)
app/components/TransferForm.tsx (4)
app/context/BalanceContext.tsx (1)
useBalance(262-268)app/context/TokensContext.tsx (1)
useTokens(115-121)app/context/StarknetContext.tsx (1)
useStarknet(301-307)app/utils.ts (1)
fetchBalanceForNetwork(697-714)
app/components/MainPageContent.tsx (1)
app/hooks/useWalletAddress.ts (1)
useWalletAddress(11-34)
app/utils.ts (1)
app/types.ts (2)
Token(209-215)Network(262-270)
app/pages/TransactionPreview.tsx (4)
app/context/StarknetContext.tsx (1)
useStarknet(301-307)app/context/BalanceContext.tsx (1)
useBalance(262-268)app/utils.ts (1)
getGatewayContractAddress(791-805)app/hooks/analytics/analytics-utils.ts (1)
trackEvent(231-231)
app/providers.tsx (2)
app/context/StarknetContext.tsx (1)
StarknetProvider(34-299)app/context/index.ts (1)
StarknetProvider(15-15)
app/components/SettingsDropdown.tsx (2)
app/context/InjectedWalletContext.tsx (1)
useInjectedWallet(114-114)app/hooks/useWalletAddress.ts (1)
useWalletAddress(11-34)
app/lib/starknet.ts (2)
app/lib/privy.ts (1)
getPrivyClient(9-24)app/lib/authorization.ts (2)
getUserAuthorizationKey(12-34)buildAuthorizationSignature(36-49)
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 130-130: [UnorderedKey] The STARKNET_PAYMASTER_MODE key should go before the STARKNET_PAYMASTER_URL key
(UnorderedKey)
[warning] 131-131: [UnorderedKey] The STARKNET_PAYMASTER_API_KEY key should go before the STARKNET_PAYMASTER_MODE key
(UnorderedKey)
🔇 Additional comments (58)
app/api/track-logout/route.ts (1)
34-34: The original regex validation is correct. Starknet addresses follow a canonical format of0xfollowed by exactly 64 hexadecimal characters (32 bytes, zero-padded). Official Starknet tooling enforces and returns this standardized form across the ecosystem, so the regex requiring precisely 64 characters is appropriate.Likely an incorrect or invalid review comment.
app/context/index.ts (1)
15-15: LGTM!The export pattern is consistent with other context providers in this file.
app/mocks.ts (1)
100-103: LGTM!The addition of Starknet to the networks array follows the established pattern, assuming the chain definition is corrected.
.env.example (1)
117-135: LGTM! Starknet configuration is well-structured.The new Starknet environment variables are logically grouped and well-documented. The static analysis warnings about key ordering (lines 130-131) can be safely ignored—logical grouping by functionality (paymaster URL, mode, API key) is more valuable than strict alphabetical ordering in this context.
package.json (1)
38-38: Both package versions have been verified as existing and stable with no known security advisories:
- @privy-io/server-auth@1.32.2: Exists in npm registry, published 4 months ago, well-maintained with 280 versions and multiple active maintainers. Latest stable is 1.32.5.
- starknet@9.2.1: Exists in npm registry, published 4 weeks ago, well-maintained with 289 versions. This is the current latest stable version.
No security vulnerabilities detected in the npm registry for either package.
app/components/NetworkSelectionModal.tsx (3)
11-11: LGTM!The import of
useStarknetenables access to Starknet wallet creation functionality needed for network switching.
27-27: LGTM!Correctly retrieves the
ensureWalletExistsfunction from the Starknet context for wallet creation during network switches.
64-64: LGTM!Correctly passes the wallet creation function to enable Starknet wallet provisioning during network switches. The inline comment clearly documents the intent.
app/components/Navbar.tsx (3)
30-30: LGTM!The import of
useWalletAddresscentralizes wallet address logic, supporting multiple wallet types (injected, Starknet, EVM smart wallet) in a single hook.
41-41: LGTM!The hook call correctly retrieves the appropriate wallet address based on the current context (injected wallet, Starknet, or EVM smart wallet).
45-48: Excellent refactor!The simplified
activeWalletconstruction is much cleaner and more maintainable. By delegating address resolution touseWalletAddress, this code now handles Starknet, injected wallets, and EVM smart wallets consistently across the application.app/components/NetworksDropdown.tsx (3)
16-16: LGTM!Correctly imports
useStarknetalongside other context hooks, maintaining consistent import organization.
30-30: LGTM!Correctly retrieves the wallet creation function from Starknet context, consistent with the pattern used in other components.
60-60: LGTM!Correctly passes the wallet creation function to the network switch handler, enabling automatic Starknet wallet provisioning when needed. The implementation is consistent across all network selection components.
app/api/starknet/create-wallet/route.ts (2)
12-30: LGTM!Authentication logic correctly validates the JWT token and extracts the user ID following the established pattern.
90-98: LGTM!The error handling appropriately catches exceptions, logs them for debugging, and returns a proper 500 response to the client.
app/components/MainPageContent.tsx (3)
22-26: Clean import reorganization for Starknet integration.The import consolidation and addition of
useWalletAddressaligns with the PR's goal to centralize wallet address derivation across injected, Starknet, and EVM wallets.Also applies to: 44-44
64-65: Good use of the centralized wallet address hook.Using
useWalletAddress()here simplifies thePageLayoutcomponent and ensures consistent wallet address resolution across all network types (injected, Starknet, EVM).
146-168: Formatting-only changes to stateProps.These are whitespace/formatting adjustments with no functional impact.
app/hooks/useWalletAddress.ts (1)
1-34: Well-structured centralized wallet address hook.The hook provides a clean single source of truth for wallet addresses across all network types (injected, Starknet, EVM). The priority order (injected → Starknet → EVM smart wallet) is logical and aligns with the PR objectives.
app/lib/privy.ts (2)
7-24: Good singleton pattern implementation.The lazy initialization with environment variable validation and error handling for the authorization key setup is well-structured. The singleton avoids repeated PrivyClient instantiation.
16-22: No action needed. ThewalletApi.updateAuthorizationKey()method is synchronous according to Privy's documentation—it does not return a Promise and should not be awaited. The current implementation is correct.app/providers.tsx (1)
92-106: Correct provider hierarchy for Starknet integration.Placing
StarknetProviderafterInjectedWalletProviderand before downstream providers (TokensProvider, BalanceProvider, etc.) is the right ordering. This ensures:
- StarknetProvider has access to NetworkProvider and InjectedWalletProvider context
- Inner providers can consume Starknet wallet state for balance fetching and transactions
app/api/starknet/transfer/route.ts (2)
13-34: Well-structured JWT authentication flow.The authorization header validation, JWT verification, and user ID extraction follow good security practices with clear error responses for each failure case.
36-47: Wallet ownership validation is delegated to Privy, not explicitly checked locally.The route calls
getStarknetWallet(walletId)without verifying the wallet belongs toauthUserId. However, wallet ownership IS implicitly validated downstream:rawSign()passes theuserJwtto Privy'sgenerateUserSigner(), which only generates a valid authorization key if the JWT matches the wallet owner. A guessedwalletIdwould succeed atgetStarknetWallet()but fail at the signing step when Privy rejects the unauthorized user.While this is secure in practice, adding explicit ownership verification in the route handler (e.g., checking if the wallet appears in the user's wallet list before processing) would be more defensive and clearer to reviewers.
app/components/WalletDetails.tsx (4)
60-60: Good centralization of wallet address.Using
useWalletAddress()and derivingactiveWalletfrom it simplifies the component and ensures consistent address resolution across network types.Also applies to: 76-76
81-83: Correct balance source selection for Starknet.The ternary properly routes to
starknetWalletbalances when on Starknet, falling back tosmartWalletfor EVM networks.
233-241: Clarify the Starknet balance display intent.Both branches display a
$prefix, but the comment suggests Starknet shows "token amount directly" while mainnet shows "USD value." If the Starknet branch should show raw token amounts (not USD), the$prefix may confuse users.If the intent is to show USD value for both (just using different formatting), the comment should be updated for clarity.
376-385: Good USD conversion display for STRK and ETH.Using
balancesUsdfor Starknet-native tokens ensures accurate USD-equivalent display based on current prices.app/components/MobileDropdown.tsx (2)
45-45: Good wallet address centralization and Starknet integration.Using
useWalletAddress()forwalletAddressandsmartWallet, plus extractingensureWalletExistsfromuseStarknet, aligns with the PR's goal of unified wallet handling.Also applies to: 52-52, 54-61
129-129: Enables Starknet wallet auto-creation on network switch.Passing
ensureWalletExiststohandleNetworkSwitchallows automatic Starknet wallet creation when users switch to the Starknet network, providing a seamless onboarding experience.app/components/SettingsDropdown.tsx (4)
26-26: LGTM! Centralized wallet address sourcing.The new
useWalletAddresshook cleanly centralizes wallet address resolution across injected wallets, Starknet, and EVM smart wallets, improving maintainability.Also applies to: 33-34
127-131: LGTM! Proper Starknet state cleanup on logout.The localStorage cleanup correctly removes all Starknet wallet state keys on logout, maintaining consistency with the StarknetContext's storage pattern.
46-51: LGTM! Safe walletAddress usage.All usages of
walletAddressproperly handle the potentially undefined value with null coalescing operators.
136-140: The logout idempotency concern is not an issue. Privy'slogout()function is idempotent and safe to call multiple times—calling it when no session is active simply becomes a no-op. The catch block pattern here is actually sound defensive programming that ensures logout completes regardless of whether the error originated fromlogout()itself or fromdisconnectWallet(). No changes needed.Likely an incorrect or invalid review comment.
app/pages/TransactionForm.tsx (1)
67-72: LGTM! Properly destructures Starknet balance.The expanded destructuring correctly retrieves
starknetWalletBalancefrom theuseBalancehook, aligning with the BalanceContext updates to support Starknet.app/context/StarknetContext.tsx (2)
12-43: LGTM! Well-structured state management.The interfaces and state setup are clean and properly typed. The
STORAGE_PREFIXconstant ensures consistent localStorage key naming.
274-307: LGTM! Standard React context pattern.The provider and hook follow React best practices with proper error handling for usage outside the provider.
app/hooks/useSmartWalletTransfer.ts (4)
19-32: LGTM! Backward-compatible interface extension.The optional
starknetWalletparameter maintains backward compatibility while enabling Starknet transfer support.
107-114: LGTM! Proper chain switching for EVM-only.Correctly skips chain switching for Starknet, which operates differently from EVM networks.
Based on learnings,
selectedNetworkis always defined and does not require null safety checks.
194-209: LGTM! EVM transfer logic preserved.The existing EVM transfer logic is correctly preserved within an
elseblock, maintaining backward compatibility.
272-272: LGTM! Dependency array updated correctly.The addition of
starknetWalletto the dependency array ensures thetransfercallback updates when Starknet wallet state changes.app/context/BalanceContext.tsx (3)
80-104: Starknet balance fetching logic looks correct.The early return pattern cleanly separates Starknet from EVM balance fetching. The error handling appropriately resets balance state on failure.
169-192: MissingstarknetWalletBalancein useEffect dependency array.The
starknetWalletBalanceis used insidefetchBalancewhen setting the transfer network balance (Line 170), but it's not listed in the dependency array. This could cause stale closure issues where the effect captures an outdatedstarknetWalletBalancevalue.However, reviewing the context, this useEffect is in
TransferForm.tsx, notBalanceContext.tsx. Let me re-examine.
228-236: Logout/cleanup effect correctly resets all balance states.This effect ensures all balance states (including Starknet) are cleared when the user logs out or disconnects, preventing stale data from persisting.
app/api/starknet/create-order/route.ts (1)
202-206: Fee margin calculation is correct.The
withMargin15function correctly implements ceiling division for 1.5x multiplier:(bi * 3 + 1) / 2ensures proper rounding up.app/components/TransferForm.tsx (3)
169-178: Starknet balance path correctly uses context value.When the transfer network is Starknet, the code correctly uses the pre-fetched
starknetWalletBalancefrom context rather than making a new fetch call. This is efficient since the balance is already maintained byBalanceContext.
366-374: Address validation correctly supports both EVM and Starknet formats.The regex
/^0x[a-fA-F0-9]{40}$|^0x[a-fA-F0-9]{64}$/properly validates:
- EVM addresses:
0x+ 40 hex chars (42 total)- Starknet addresses:
0x+ 64 hex chars (66 total)The length validation and error message are also clear and helpful.
120-126: Starknet wallet data correctly passed to transfer hook.The
starknetWalletobject includes all necessary fields (walletId,publicKey,address,deployed) for the transfer hook to handle Starknet transactions.app/pages/TransactionPreview.tsx (2)
137-139: Balance selection correctly prioritizes Starknet when on Starknet network.The ternary chain properly checks injected wallet first, then Starknet network, then falls back to smart wallet balance.
477-478: Guard condition for save transaction data is appropriate.The check
!embeddedWallet?.address && !starknetAddresscorrectly prevents saving when neither wallet type has an address, while still allowing the save to proceed if either exists.app/utils.ts (3)
747-775: Well-implemented Starknet address normalization.The function correctly:
- Validates the
0xprefix- Checks for valid hex characters
- Enforces max length of 64 hex chars
- Left-pads shorter addresses with zeros
The documentation with examples is helpful.
973-981: Starknet wallet creation failure doesn't block network switch.The
try-catcharoundensureWalletExists()allows the network switch to proceed even if wallet creation fails. This is appropriate UX - users should be able to view the Starknet network even if their wallet isn't created yet.
153-154: Starknet explorer link uses Voyager.Voyager is the standard Starknet block explorer, this is correct.
app/lib/starknet.ts (4)
242-243: Signature splitting assumes exact 128-character hex body.The code assumes the signature (after removing
0x) is exactly 128 hex characters (64 bytes = r + s components). If Privy returns a different format, this will silently produce incorrect signature components.Consider adding validation:
const body = sig.slice(2); if (body.length !== 128) { throw new Error(`Unexpected signature length: ${body.length}, expected 128`); } return [`0x${body.slice(0, 64)}`, `0x${body.slice(64)}`];
222-277: Custom signer implementation correctly overrides required methods.The anonymous class extending
RawSignerproperly implementssignMessage,signTransaction, andsignRawusing Privy's raw signing API. The typed data hashing for paymaster signatures is correctly implemented.
294-342: Paymaster setup has appropriate validation and error handling.The function:
- Validates required configuration
- Enforces API key requirement for sponsored mode
- Checks paymaster availability before returning
- Resolves gas token for non-sponsored mode
This is a robust implementation.
417-421: Fee estimation only runs when there are calls to execute.The condition
!isSponsored && calls.length > 0correctly skips fee estimation for sponsored transactions and deploy-only operations.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@app/utils.ts`:
- Around line 381-398: Update the Starknet token entries for USDC and USDT in
the Starknet array: locate the objects with name "USD Coin" (symbol "USDC") and
"Tether USD" (symbol "USDT") and replace their address fields with the correct
mainnet contract addresses: set USDC address to
0x033068f6539f8e6e6b131e6b2b814e6c34a5224bc66947c47dab9dfee93b35fb and set USDT
address to 0x68f5c6a61780768455de69077e07e89787839bf816decfbf92b645209c0fb8,
keeping the other properties (decimals, imageUrl) unchanged.
🧹 Nitpick comments (2)
app/mocks.ts (1)
14-20: Consider adding a fallback for the RPC URL.If
NEXT_PUBLIC_STARKNET_RPC_URLis not set, thehttparrays will contain[undefined], which could cause runtime errors when the RPC client attempts to use this chain configuration. Consider adding a fallback public RPC endpoint.🔧 Optional: Add fallback RPC URL
rpcUrls: { default: { - http: [process.env.NEXT_PUBLIC_STARKNET_RPC_URL], + http: [process.env.NEXT_PUBLIC_STARKNET_RPC_URL || "https://starknet-mainnet.public.blastapi.io"], }, public: { - http: [process.env.NEXT_PUBLIC_STARKNET_RPC_URL], + http: [process.env.NEXT_PUBLIC_STARKNET_RPC_URL || "https://starknet-mainnet.public.blastapi.io"], }, },app/utils.ts (1)
567-604: Consider caching token prices to avoid rate limiting.The
fetchTokenPricefunction is called for each token in parallel (line 590), which could trigger CoinGecko's rate limits (typically 10-50 requests/minute on free tier) when fetching balances for multiple tokens. Consider caching prices or batching the CoinGecko request.💡 Optional: Batch price fetching
CoinGecko supports fetching multiple token prices in a single request:
// Example: Batch fetch prices const ids = tokens.map(t => coinGeckoIds[t.symbol.toUpperCase()]).filter(Boolean).join(','); const response = await fetch( `https://api.coingecko.com/api/v3/simple/price?ids=${ids}&vs_currencies=usd` );This would reduce API calls from N to 1.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@app/pages/TransactionPreview.tsx`:
- Around line 272-276: In TransactionPreview update the tracking call to use the
camelCased property: replace the incorrect result.transaction_hash with
result.transactionHash in the trackEvent call (the code around trackEvent("Swap
started", { "Entry point": "Transaction preview", "Wallet type": "Starknet
embedded wallet", "Transaction hash": ... }) so the event uses
result.transactionHash that matches the API transformation performed in
create-order/route.ts).
- Around line 488-489: In TransactionPreview.tsx inside the saveTransactionData
flow, set the transaction.walletAddress to use starknetAddress as a fallback for
embeddedWallet?.address (e.g., assign embeddedWallet?.address || starknetAddress
|| ""), so Starknet-only users don’t get an empty walletAddress; update the
TransactionCreateInput construction where walletAddress is assigned and ensure
any validation or early-return logic uses this combined fallback before calling
the API.
♻️ Duplicate comments (2)
app/context/StarknetContext.tsx (2)
207-219: Add null check for access token.Similar to
createWallet, therefreshWalletStatefunction usesgetAccessToken()without checking for null before constructing the Authorization header.🐛 Proposed fix
const refreshWalletState = async () => { if (!authenticated || !user?.id) return; try { const token = await getAccessToken(); + if (!token) { + console.error("Failed to get access token for refreshing wallet state"); + return; + } + const response = await fetch( `/api/starknet/wallet-state?userId=${user.id}`,
144-154: Add null check for access token before use.
getAccessToken()can returnnull, but the token is used directly in the Authorization header without validation. This could cause requests to fail with invalid auth headers.🐛 Proposed fix
const token = await getAccessToken(); + if (!token) { + throw new Error("Failed to obtain access token"); + } // Step 1: Create the wallet const response = await fetch("/api/starknet/create-wallet", {
🧹 Nitpick comments (2)
app/api/starknet/create-wallet/route.ts (1)
32-42: Remove unused code and redundant check.
- Line 33:
bodyis parsed but never used in the function.- Lines 35-42: The second
userIdcheck is redundant sinceauthUserIdwas already validated on line 25, and it's directly assigned on line 35.♻️ Proposed fix
- // Get request body - const body = await request.json(); - - const userId = authUserId; - - if (!userId) { - return NextResponse.json( - { error: "No user ID available" }, - { status: 400 }, - ); - } + const userId = authUserId;app/context/StarknetContext.tsx (1)
48-56: Consider adding type safety for linked accounts.Using
(account: any)bypasses TypeScript's type checking. Consider defining an interface for the expected account structure to improve maintainability and catch potential property access errors at compile time.♻️ Suggested type definition
interface LinkedAccount { type: string; chainType?: string; chain_type?: string; id?: string; address?: string; publicKey?: string; public_key?: string; } // Then use it: const starknetWallet = linkedAccounts.find( (account: LinkedAccount) => account.type === "wallet" && (account.chainType === "starknet" || account.chain_type === "starknet"), );
|
@coderabbitai recheck your previous reviews and close all that have been resolved |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain
🏁 Script executed: Length of output: 7309 🏁 Script executed: Length of output: 4724 🏁 Script executed: Length of output: 4832 🏁 Script executed: Length of output: 1780 🏁 Script executed: Length of output: 455 🏁 Script executed: Length of output: 9897 🏁 Script executed: Length of output: 2009
✅ Resolved Issues (6 comments can be closed):
⏳ Still Outstanding (4 issues):
ℹ️ Acknowledged (user confirmed working as intended):
Unfortunately, GitHub's REST API doesn't provide a direct way to programmatically resolve review comment threads. You'll need to manually resolve the 6 fixed issues through the GitHub UI. Would you like me to create a follow-up issue for the 4 outstanding items, or would you prefer to address them in this PR? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainI'll check all my previous review comments on this PR and resolve the ones that have been addressed. 🏁 Script executed: Length of output: 0 🏁 Script executed: Length of output: 64534 I'll check the current state of the code for all my previous review comments and resolve the ones that have been addressed. 🏁 Script executed: Length of output: 8330 Perfect! I can see that many of my previous review comments have been addressed. Let me now resolve the ones that are confirmed fixed. 🏁 Script executed: Length of output: 728 Let me try the correct approach to resolve these review threads. 🏁 Script executed: Length of output: 761 Perfect! Now I can see the unresolved threads. Let me resolve the ones that have been fixed and get the thread IDs for the others. 🏁 Script executed: Length of output: 1390 Excellent! I've successfully resolved the review comments. Let me provide you with a complete summary:
✅ Review Comments SummaryI've reviewed all my previous comments on PR 🎉 Resolved (10 comments)Already resolved before:
Just resolved:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@app/utils.ts`:
- Around line 558-604: Validate NEXT_PUBLIC_STARKNET_RPC_URL before creating
RpcProvider (throw or return early with a clear error) and pass the validated
URL into new RpcProvider; normalize the account address with
normalizeStarknetAddress when building calldata for provider.callContract
(replace the raw address variable used inside the tokens.map callback); avoid
precision loss by doing token amount math in BigInt (compute balanceInWei / (10n
** BigInt(token.decimals)) or format the integer result to a decimal string)
instead of Number(balanceInWei) before converting to human-readable amounts; and
stop silently swallowing errors in the tokens.map catch—log the caught error
with context (e.g., token.symbol and address) using console.error or the
existing logger so failures in RpcProvider.callContract or fetchTokenPrice are
visible (references: RpcProvider, tokens.map async callback,
normalizeStarknetAddress, fetchTokenPrice, balances and balancesUsd).
🧹 Nitpick comments (2)
app/utils.ts (2)
506-538: Consider caching CoinGecko responses and short-circuiting for stablecoins.A few observations:
Rate limiting: CoinGecko's free tier has rate limits. If
fetchStarknetBalanceis called frequently with multiple tokens, you may hit these limits.Stablecoin optimization: For USDC/USDT, the price is approximately $1. Consider returning
1.0directly to reduce API calls and latency:+ // Stablecoins are pegged to $1, skip API call + if (tokenSymbol.toUpperCase() === 'USDC' || tokenSymbol.toUpperCase() === 'USDT') { + return 1.0; + } + const coinId = coinGeckoIds[tokenSymbol.toUpperCase()];
- Caching: Consider caching prices with a short TTL (e.g., 60 seconds) to reduce redundant calls.
1006-1014: Consider notifying users when Starknet wallet creation fails.Currently, if
ensureWalletExists()fails, the error is logged but the network switch proceeds silently. This may lead to confusion when users try to transact on Starknet without a wallet.Consider showing a non-blocking notification so users are aware:
} catch (error) { console.error("Failed to ensure Starknet wallet exists:", error); - // Continue with network switch even if wallet creation fails + // Notify user but continue with network switch + toast.warning("Could not create Starknet wallet. You can try again later."); }Alternatively, if wallet creation is critical for Starknet functionality, consider preventing the network switch and calling
onError.
5ran6
force-pushed
the
main
branch
2 times, most recently
from
cac70dc to
447a769
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/pages/TransactionPreview.tsx (1)
184-191:⚠️ Potential issue | 🟠 MajorUse the Starknet account as the refund address.
prepareCreateOrderParams()still sourcesrefundAddressfromactiveWallet?.address. On the Starknet path, that is the EVM injected/smart-wallet address, while the actual order is created forstarknetAddress. A failed or cancelled Starknet order can therefore refund to the wrong account or an empty string.♻️ Suggested fix
senderFeeRecipient: getAddress(senderFeeRecipientAddress), senderFee: senderFeeInTokenUnits, - refundAddress: activeWallet?.address as `0x${string}`, + refundAddress: + selectedNetwork.chain.name === "Starknet" + ? (starknetAddress! as `0x${string}`) + : (activeWallet?.address as `0x${string}`), messageHash: encryptedRecipient,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/pages/TransactionPreview.tsx` around lines 184 - 191, prepareCreateOrderParams currently sets refundAddress from activeWallet?.address which is wrong for Starknet flows; change the refundAddress assignment to prefer the Starknet account (starknetAddress) when present/when on the Starknet path and fall back to activeWallet?.address otherwise. Update the params object in prepareCreateOrderParams to set refundAddress: (starknetAddress ?? (activeWallet?.address as `0x${string}`)) (or equivalent conditional logic that checks whether starknetAddress is non-empty/using Starknet) so failed/cancelled Starknet orders refund to the correct account.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/lib/privy.ts`:
- Around line 11-13: Tests are failing because getPrivyClient() in
app/lib/privy.ts expects process.env.PRIVY_APP_SECRET (appSecret) and throws
when missing; update the test environment setup (e.g., jest.setup.js) to define
PRIVY_APP_SECRET alongside NEXT_PUBLIC_PRIVY_APP_ID so appSecret and appId are
both present during tests and the throw on lines referencing appId/appSecret no
longer occurs.
In `@app/utils.ts`:
- Around line 479-483: The current loop in the merge fallback logic skips
applying FALLBACK_TOKENS for "Starknet" due to the early guard `if (networkName
=== "Starknet") return;`, causing an empty tokens["Starknet"] when the API omits
it; remove that special-case guard (or replace it with a check that ensures
FALLBACK_TOKENS[networkName] exists) so that the loop always assigns
`tokens[networkName] = FALLBACK_TOKENS[networkName]` when `!tokens[networkName]
|| tokens[networkName].length === 0`, thereby restoring the Starknet fallback;
locate this in the function that iterates Object.keys(FALLBACK_TOKENS) and
touches `tokens`, `FALLBACK_TOKENS`, and `networkName`.
- Around line 789-807: The function that normalizes Starknet addresses currently
pads an empty "0x" into an all-zero address; after extracting hexPart
(trimmedAddress?.slice(2)) add a check to reject an empty hexPart (e.g., if
hexPart.length === 0) and throw a clear Error like "Empty Starknet address"
before any padding or further validation; keep the existing hex character and
max-length checks and only perform padStart(64, "0") when hexPart is non-empty
so normalizeStarknetAddress/hexPart/paddedHex cannot convert a missing address
into a valid-looking all-zero address.
- Around line 1006-1013: The current Starknet branch swallows
ensureWalletExists() errors and continues the network switch; change this so
failures abort the switch by rethrowing or propagating the error instead of just
console.error. In the block where you check if (network.chain.name ===
"Starknet" && ensureWalletExists) and call await ensureWalletExists(), remove
the silent catch or replace it so the catch rethrows the caught error (or throws
a new Error with context) to surface the failure to the caller and stop the
network switch flow.
---
Outside diff comments:
In `@app/pages/TransactionPreview.tsx`:
- Around line 184-191: prepareCreateOrderParams currently sets refundAddress
from activeWallet?.address which is wrong for Starknet flows; change the
refundAddress assignment to prefer the Starknet account (starknetAddress) when
present/when on the Starknet path and fall back to activeWallet?.address
otherwise. Update the params object in prepareCreateOrderParams to set
refundAddress: (starknetAddress ?? (activeWallet?.address as `0x${string}`)) (or
equivalent conditional logic that checks whether starknetAddress is
non-empty/using Starknet) so failed/cancelled Starknet orders refund to the
correct account.
🪄 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: 71b4f282-1618-4ea1-aabb-cf270f2ce025
📒 Files selected for processing (7)
app/api/starknet/create-wallet/route.tsapp/lib/privy.tsapp/lib/starknet.tsapp/lib/starknetRpc.tsapp/pages/TransactionPreview.tsxapp/utils.tsjest.setup.js
✅ Files skipped from review due to trivial changes (1)
- jest.setup.js
🚧 Files skipped from review as they are similar to previous changes (2)
- app/api/starknet/create-wallet/route.ts
- app/lib/starknet.ts
| const appId = process.env.NEXT_PUBLIC_PRIVY_APP_ID | ||
| const appSecret = process.env.PRIVY_APP_SECRET | ||
| if (!appId || !appSecret) throw new Error('Missing NEXT_PUBLIC_PRIVY_APP_ID or PRIVY_APP_SECRET') |
There was a problem hiding this comment.
Add PRIVY_APP_SECRET to test env setup to avoid runtime throws.
getPrivyClient() now hard-fails on Line 13 when PRIVY_APP_SECRET is missing. Current jest.setup.js only sets NEXT_PUBLIC_PRIVY_APP_ID, so tests hitting this path can fail before assertions.
Suggested patch (outside this file)
// jest.setup.js
process.env.SUPABASE_URL = 'https://test.supabase.co'
process.env.SUPABASE_SERVICE_ROLE_KEY = 'test-service-role-key'
process.env.NEXT_PUBLIC_PRIVY_APP_ID = 'test-privy-app-id'
+process.env.PRIVY_APP_SECRET = 'test-privy-app-secret'
process.env.INTERNAL_API_KEY = 'test-internal-api-key'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/lib/privy.ts` around lines 11 - 13, Tests are failing because
getPrivyClient() in app/lib/privy.ts expects process.env.PRIVY_APP_SECRET
(appSecret) and throws when missing; update the test environment setup (e.g.,
jest.setup.js) to define PRIVY_APP_SECRET alongside NEXT_PUBLIC_PRIVY_APP_ID so
appSecret and appId are both present during tests and the throw on lines
referencing appId/appSecret no longer occurs.
| // Merge fallback tokens for any networks missing from API response | ||
| Object.keys(FALLBACK_TOKENS).forEach((networkName) => { | ||
| if (networkName === "Starknet") return; | ||
| if (!tokens[networkName] || tokens[networkName].length === 0) { | ||
| tokens[networkName] = FALLBACK_TOKENS[networkName]; |
There was a problem hiding this comment.
Don't bypass the Starknet fallback on partial token responses.
If the aggregator call succeeds but omits Starknet, this early return leaves tokens["Starknet"] empty and the app gets [] for Starknet despite FALLBACK_TOKENS.Starknet being defined above. That breaks Starknet token selection and balance flows whenever backend rollout lags behind the frontend.
♻️ Suggested fix
Object.keys(FALLBACK_TOKENS).forEach((networkName) => {
- if (networkName === "Starknet") return;
if (!tokens[networkName] || tokens[networkName].length === 0) {
tokens[networkName] = FALLBACK_TOKENS[networkName];
}
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/utils.ts` around lines 479 - 483, The current loop in the merge fallback
logic skips applying FALLBACK_TOKENS for "Starknet" due to the early guard `if
(networkName === "Starknet") return;`, causing an empty tokens["Starknet"] when
the API omits it; remove that special-case guard (or replace it with a check
that ensures FALLBACK_TOKENS[networkName] exists) so that the loop always
assigns `tokens[networkName] = FALLBACK_TOKENS[networkName]` when
`!tokens[networkName] || tokens[networkName].length === 0`, thereby restoring
the Starknet fallback; locate this in the function that iterates
Object.keys(FALLBACK_TOKENS) and touches `tokens`, `FALLBACK_TOKENS`, and
`networkName`.
| // Extract hex part (without 0x prefix) | ||
| const hexPart = trimmedAddress?.slice(2); | ||
|
|
||
| // Validate hex characters | ||
| if (!/^[a-fA-F0-9]*$/.test(hexPart)) { | ||
| throw new Error("Invalid hex characters in Starknet address"); | ||
| } | ||
|
|
||
| // Validate length (must not exceed 64 hex chars) | ||
| if (hexPart.length > 64) { | ||
| throw new Error( | ||
| "Starknet address too long (max 64 hex characters after 0x)", | ||
| ); | ||
| } | ||
|
|
||
| // Pad with zeros after 0x to make it 66 chars total | ||
| const paddedHex = hexPart.padStart(64, "0"); | ||
|
|
||
| return `0x${paddedHex}`; |
There was a problem hiding this comment.
Reject an empty 0x address before padding.
normalizeStarknetAddress("0x") currently becomes the all-zero address. That turns a missing value into a valid-looking account string and can leak into balance or transaction calls.
♻️ Suggested fix
// Extract hex part (without 0x prefix)
const hexPart = trimmedAddress?.slice(2);
+
+ if (!hexPart) {
+ throw new Error("Starknet address cannot be empty");
+ }
// Validate hex characters
if (!/^[a-fA-F0-9]*$/.test(hexPart)) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/utils.ts` around lines 789 - 807, The function that normalizes Starknet
addresses currently pads an empty "0x" into an all-zero address; after
extracting hexPart (trimmedAddress?.slice(2)) add a check to reject an empty
hexPart (e.g., if hexPart.length === 0) and throw a clear Error like "Empty
Starknet address" before any padding or further validation; keep the existing
hex character and max-length checks and only perform padStart(64, "0") when
hexPart is non-empty so normalizeStarknetAddress/hexPart/paddedHex cannot
convert a missing address into a valid-looking all-zero address.
| // If switching to Starknet, ensure wallet exists first | ||
| if (network.chain.name === "Starknet" && ensureWalletExists) { | ||
| try { | ||
| await ensureWalletExists(); | ||
| } catch (error) { | ||
| console.error("Failed to ensure Starknet wallet exists:", error); | ||
| // Continue with network switch even if wallet creation fails | ||
| } |
There was a problem hiding this comment.
Fail the network switch when Starknet wallet creation fails.
ensureWalletExists() is the prerequisite for a usable Starknet session. Swallowing that error and continuing leaves the user on Starknet with no wallet, after which later flows can only fail. Stop the switch and surface the error instead.
♻️ Suggested fix
if (network.chain.name === "Starknet" && ensureWalletExists) {
try {
await ensureWalletExists();
} catch (error) {
console.error("Failed to ensure Starknet wallet exists:", error);
- // Continue with network switch even if wallet creation fails
+ onError(error as Error);
+ return;
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/utils.ts` around lines 1006 - 1013, The current Starknet branch swallows
ensureWalletExists() errors and continues the network switch; change this so
failures abort the switch by rethrowing or propagating the error instead of just
console.error. In the block where you check if (network.chain.name ===
"Starknet" && ensureWalletExists) and call await ensureWalletExists(), remove
the silent catch or replace it so the catch rethrows the caught error (or throws
a new Error with context) to surface the failure to the caller and stop the
network switch flow.
…knet Mainnet ID definition
…in Starknet routes
…PC provider initialization
Made-with: Cursor
onahprosper
force-pushed
the
feat-starknet-on-noblocks
branch
from
346b88d to
6c631f2
Compare
2 participants
Description
This pull request introduces comprehensive Starknet Layer 2 support to the application, enabling wallet creation, public key retrieval, token transfers, and order creation on Starknet via new API routes. It also adds associated environment configuration for Starknet and refactors wallet address handling in the frontend to improve maintainability and reliability. Additionally, a minor fix expands wallet address validation to support 64-character hexadecimal addresses.
The most important changes are:
Starknet Integration:
Added new API endpoints for Starknet operations:
app/api/starknet/create-wallet/route.ts: Allows users to create a Starknet wallet via Privy, including checks for existing wallets.app/api/starknet/get-public-key/route.ts: Fetches the public key for a Starknet wallet from Privy.app/api/starknet/transfer/route.ts: Handles token transfers on Starknet, with support for paymaster-sponsored and default gas modes, including account deployment if necessary.app/api/starknet/create-order/route.ts: Creates an order on Starknet, handling token approvals, order creation, and paymaster configuration.Added Starknet configuration variables to
.env.example, including RPC URL, account class hash, paymaster settings, and gas token address.Frontend Refactor:
MainPageContent.tsxto use the newuseWalletAddresshook, improving code clarity and maintainability. [1] [2]Validation Improvements:
track-logout/route.tsto support both 40- and 64-character hexadecimal addresses.References
closes #321
Testing
see loom here Loom
Checklist
mainBy submitting a PR, I agree to Paycrest's Contributor Code of Conduct and Contribution Guide.
Summary by CodeRabbit
New Features
Improvements