feat: add getSettings endpoint, MeshJS wallet integration#145
Open
feat: add getSettings endpoint, MeshJS wallet integration#145
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds a Cloudflare Pages Env type and vmClient response helpers; implements new serverless endpoints for claim (validate, submit, submitTransaction, status) and getSettings; centralizes VM SDK init and CORS/JSON responses; introduces client API utilities (apiClient), React Query setup, query hooks and components for claim/profile/rewards, Mesh wallet integration (ConnectWallet, useWalletSync, AppProviders), a claim flow hook, profile signing helpers, and an API tester page. It also removes legacy token pricing, Blaze wallet connector, several rewards/profile modules and UI navigation/menu components, and updates package/dependency and Vite polyfill configuration. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…se refactor - Add /api/getSettings Pages Function with KV caching (1hr TTL) - Migrate wallet integration to MeshJS + TanStack Query - Add claim flow API stubs (validate, submit, submitTransaction, status) - Add API tester page for development - DRY up functions/ layer with shared helpers (jsonResponse, errorResponse, optionsResponse) - Remove unused code (navigation-menu components, dead imports, debug logs) - Simplify getRewards.ts with extracted mergeAmounts/toClaimableTokens helpers - Clean up profileData.ts, sanitizeAddress.ts, tokenPrices.ts to use shared utils
JaeBrian
force-pushed
the
issue-131/getSettings-refactor
branch
from
f034002 to
868cbd7
Compare
Deploying tosidrop-web with
|
| Latest commit: |
2ec7ad9
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://5753cf2d.tosidrop-web.pages.dev |
| Branch Preview URL: | https://issue-131-getsettings-refact.tosidrop-web.pages.dev |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
functions/services/tokenPricing.ts (1)
23-27:⚠️ Potential issue | 🟡 MinorCaching empty result on missing endpoint may cause unexpected behavior.
When
endpointis undefined, an empty object is cached for the fullCACHE_DURATION. If the endpoint becomes available later (e.g., via configuration update), callers will still receive empty results until the cache expires. Consider either not caching when endpoint is missing, or returning without updating the cache.Proposed fix
if (!endpoint) { - cachedPrices = {}; - cacheTimestamp = now; - return cachedPrices; + return cachedPrices ?? {}; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@functions/services/tokenPricing.ts` around lines 23 - 27, The current logic caches an empty result when endpoint is falsy (setting cachedPrices and cacheTimestamp), causing stale empty responses; change the behavior in the function that checks endpoint so that if endpoint is undefined you return early without updating cachedPrices or cacheTimestamp (i.e., do not set the cache or touch CACHE_DURATION state) so callers can pick up a later-provided endpoint; refer to the endpoint check and the cachedPrices/cacheTimestamp variables to implement this change.
🧹 Nitpick comments (21)
src/api/client.ts (1)
13-14: Thrown object is not anErrorinstance.The thrown
ApiErroris a plain object, not anErrorsubclass. This meanserror instanceof Errorchecks will fail, and stack traces won't be captured. While React Query handles this, error boundaries and logging utilities often expect properErrorinstances.♻️ Suggested approach: Create an ApiError class
// In src/types/api.ts or a new file export class ApiError extends Error { status: number; constructor(message: string, status: number) { super(message); this.name = 'ApiError'; this.status = status; } }Then throw it as:
- const error: ApiError = { message, status: res.status }; - throw error; + throw new ApiError(message, res.status);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/client.ts` around lines 13 - 14, Replace the plain object being thrown with a real Error subclass: create an ApiError class (e.g., export class ApiError extends Error { status: number; constructor(message: string, status: number) { super(message); this.name = 'ApiError'; this.status = status; } }) in your types or errors module, import that into src/api/client.ts, and change the throw site (where currently `const error: ApiError = { message, status: res.status }; throw error;`) to `throw new ApiError(message, res.status)` so stack traces and instanceof checks work correctly.functions/api/getSettings.ts (1)
10-10: Consider making VM_URL configurable via environment.The URL
vmprev.adaseal.euappears to be a preview/staging endpoint. Hardcoding this makes it difficult to switch between environments (dev/staging/prod).♻️ Proposed fix using environment variable
interface Env { VITE_VM_API_KEY: string; VM_WEB_PROFILES: KVNamespace; + VM_URL?: string; } -const VM_URL = 'https://vmprev.adaseal.eu'; const CACHE_KEY = 'settings_cache'; const CACHE_TTL = 3600; export const onRequestGet: PagesFunction<Env> = async (context) => { const { env } = context; + const vmUrl = env.VM_URL || 'https://vmprev.adaseal.eu'; // ... use vmUrl instead of VM_URL🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@functions/api/getSettings.ts` at line 10, The hardcoded constant VM_URL in getSettings.ts should be made configurable via environment variables; replace the literal 'https://vmprev.adaseal.eu' with a lookup that reads process.env.VM_URL (or process.env.VM_URL_DEFAULT fallback) and ensure the code that uses VM_URL (symbol: VM_URL) validates presence or provides a sensible default for different environments (dev/staging/prod) and update any documentation or env sample to include VM_URL.functions/services/vmClient.ts (2)
10-17: Review wildcard CORS origin for security implications.
Access-Control-Allow-Origin: '*'allows any origin to access the API. While convenient for development, this may expose your API to cross-origin abuse in production. Consider restricting to specific allowed origins.♻️ Consider parameterizing allowed origins
-export function corsHeaders(): Record<string, string> { +export function corsHeaders(origin?: string): Record<string, string> { + const allowedOrigin = origin || '*'; // Restrict in production return { 'Content-Type': 'application/json', - 'Access-Control-Allow-Origin': '*', + 'Access-Control-Allow-Origin': allowedOrigin, 'Access-Control-Allow-Methods': 'GET, POST, OPTIONS', 'Access-Control-Allow-Headers': 'Content-Type', }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@functions/services/vmClient.ts` around lines 10 - 17, The current corsHeaders() function returns a wildcard Access-Control-Allow-Origin which is insecure for production; update corsHeaders to support a configurable allowlist (e.g., from an environment variable or a passed-in parameter) and set Access-Control-Allow-Origin dynamically by checking the incoming request Origin against that allowlist (return the specific origin when allowed, or omit/return a safe default when not). Locate and modify corsHeaders() in vmClient.ts to accept an optional origins parameter or read process.env.ALLOWED_ORIGINS, perform origin validation, and emit the specific origin value instead of '*' (also ensure existing headers like Access-Control-Allow-Methods and -Headers remain intact).
27-29: Remove Content-Type header from OPTIONS response.A
204 No Contentresponse should not includeContent-Type: application/jsonsince there is no body. This is technically incorrect per HTTP semantics, though most clients ignore it.♻️ Proposed fix
export function optionsResponse(): Response { - return new Response(null, { status: 204, headers: corsHeaders() }); + const headers = corsHeaders(); + delete headers['Content-Type']; + return new Response(null, { status: 204, headers }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@functions/services/vmClient.ts` around lines 27 - 29, The OPTIONS handler optionsResponse currently returns a 204 with headers that include Content-Type; remove Content-Type for a No Content response by adjusting optionsResponse (or the headers it uses) so the Response's headers do not contain "Content-Type" — e.g., obtain the headers from corsHeaders(), clone into a Headers object and delete the "Content-Type" key (handle both "Content-Type" and lower-case "content-type" if present) before returning new Response(null, { status: 204, headers }). Ensure the unique function name optionsResponse and the helper corsHeaders() are the locations you change.src/layouts/components/PrimaryNavigation.tsx (1)
12-12: Consider restricting API Tester visibility to development environments.The "API Tester" link is currently visible in production navigation. If this is intended only for development/debugging purposes, consider conditionally rendering it based on the environment.
♻️ Proposed conditional rendering
const NAV_LINKS = [ { name: 'Claim', href: '/' }, { name: 'History', href: '/history' }, { name: 'Preferences', href: '/preferences' }, - { name: 'API Tester', href: '/api-tester' }, -]; + ...(import.meta.env.DEV ? [{ name: 'API Tester', href: '/api-tester' }] : []), +];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/layouts/components/PrimaryNavigation.tsx` at line 12, The "API Tester" nav entry is always rendered; restrict it to non-production by conditionally including or filtering out the item in the PrimaryNavigation component: locate the nav data or JSX that contains the object { name: 'API Tester', href: '/api-tester' } inside PrimaryNavigation and only add/render that entry when a development flag is true (e.g., process.env.NODE_ENV !== 'production' or a dedicated NEXT_PUBLIC_*/isDev flag); update the logic that builds the navigation array or the map/render loop to skip the 'API Tester' item when in production.functions/api/sanitizeAddress.ts (1)
17-19: Redundant dynamic import of vm-sdk.The
vm-sdkmodule is imported twice: once insideinitVmSdk(line 17) and again on line 18. Consider refactoring to avoid the redundant import.♻️ Proposed fix to consolidate imports
try { - await initVmSdk(env); - const { getSanitizedAddress } = await import('vm-sdk'); + const vmSdk = await import('vm-sdk'); + vmSdk.setApiToken(env.VITE_VM_API_KEY); + const response = await vmSdk.getSanitizedAddress(address); - const response = await getSanitizedAddress(address); return jsonResponse({ address: response.address });Alternatively, modify
initVmSdkto return the SDK module:// In vmClient.ts export async function initVmSdk(env: Env) { const vmSdk = await import('vm-sdk'); vmSdk.setApiToken(env.VITE_VM_API_KEY); return vmSdk; } // In sanitizeAddress.ts const { getSanitizedAddress } = await initVmSdk(env);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@functions/api/sanitizeAddress.ts` around lines 17 - 19, The file redundantly imports 'vm-sdk' after calling initVmSdk; update initVmSdk to return the initialized SDK module (export async function initVmSdk(env: Env) { const vmSdk = await import('vm-sdk'); vmSdk.setApiToken(env.VITE_VM_API_KEY); return vmSdk; }) and then in sanitizeAddress.ts call const { getSanitizedAddress } = await initVmSdk(env) instead of doing a second dynamic import; remove the extra import of 'vm-sdk' and adjust initVmSdk's callers to use the returned module.src/pages/ApiTesterPage.tsx (1)
37-44: Minor: Display endpoint differs from actual request URL.The
endpointproperty shows the rawstakeAddress(line 38) while theactionencodes it (line 43). For special characters, the displayed URL won't match the actual request. Consider encoding in both places for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/ApiTesterPage.tsx` around lines 37 - 44, The displayed endpoint string for the "Get Rewards" entry uses the raw stakeAddress but the action uses encodeURIComponent, causing a mismatch; update the endpoint property to use an encoded stakeAddress as well (mirror the encoding used in the action) so the displayed URL matches the actual request—look for the object with name 'Get Rewards' and fields endpoint, action, fetchApi, and apiClient.get referencing stakeAddress and apply encodeURIComponent(stakeAddress ?? '') to the endpoint construction.src/pages/PreferencesPage.tsx (1)
6-8: Consider handling loading and error states fromuseProfile.The hook likely returns
isLoadinganderrorproperties from React Query. Showing a loading indicator or error message would improve user experience when profile data is being fetched or fails to load.💡 Example enhancement
export default function PreferencesPage() { const { stakeAddress } = useWalletStore(); - const { data: profile } = useProfile(stakeAddress); + const { data: profile, isLoading, error } = useProfile(stakeAddress); return ( <div className="space-y-8"> {/* ... header ... */} + {isLoading && <p className="text-gray-400">Loading profile...</p>} + {error && <p className="text-red-400">Failed to load profile</p>} + {profile?.value?.name && (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/PreferencesPage.tsx` around lines 6 - 8, PreferencesPage currently calls useProfile(stakeAddress) but doesn't handle the hook's loading or error states; update PreferencesPage to read isLoading and error from useProfile (e.g., const { data: profile, isLoading, error } = useProfile(stakeAddress)) and add early UI handling: render a loading indicator while isLoading is true and render an error message (or fallback UI) when error is present before attempting to use profile; ensure stakeAddress is guarded (e.g., null-check) so useProfile isn't used with an invalid address.functions/api/claim/submitTransaction.ts (1)
10-15: Runtime type validation relies on field checks.
request.json<T>()only provides compile-time typing. The field validation on line 13 provides basic runtime protection, but consider checking field types (e.g.,typeof body.signedTx === 'string') to guard against malformed payloads where fields exist but have wrong types.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@functions/api/claim/submitTransaction.ts` around lines 10 - 15, The current handler uses request.json<T>() and only checks presence of body.signedTx and body.airdropHash; add runtime type checks to ensure both fields are strings (e.g., verify typeof body.signedTx === 'string' and typeof body.airdropHash === 'string') before proceeding, and return errorResponse('signedTx and airdropHash must be strings', 400) if the checks fail; update the validation around the body variable in submitTransaction (where request.json is called and errorResponse is used) to perform these typeof checks.functions/api/profileData.ts (1)
23-25: Consider validatingbody.valuestructure.The code validates
walletIdbut doesn't check thatbody.valueexists or thatbody.value.nameis a non-empty string before storing. This could lead to storing incomplete or malformed profile data.🛡️ Proposed validation
if (!body.walletId) { return errorResponse('Missing walletId', 400); } + + if (!body.value?.name || typeof body.value.name !== 'string') { + return errorResponse('Missing or invalid value.name', 400); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@functions/api/profileData.ts` around lines 23 - 25, The handler currently only checks body.walletId but may accept missing or malformed profile content; add validation to ensure body.value exists and that body.value.name is a non-empty string before proceeding (return errorResponse('Missing or invalid profile name', 400) on failure). Locate the request handler in functions/api/profileData.ts and update the logic around the existing walletId check to validate body.value and body.value.name, rejecting invalid input and only storing when those checks pass.functions/api/claim/status.ts (1)
7-24: Consider adding CORS/OPTIONS support for consistency.Other claim endpoints like
submitTransaction.tsexportonRequestOptionsfor CORS preflight handling. If this endpoint will be called from browser clients, it may need the same treatment.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@functions/api/claim/status.ts` around lines 7 - 24, Add a CORS preflight handler similar to other claim endpoints by exporting an onRequestOptions alongside onRequestGet; implement onRequestOptions to return a 204/200 response with the same CORS headers used elsewhere (e.g., Access-Control-Allow-Origin, Access-Control-Allow-Methods, Access-Control-Allow-Headers) so browser clients can call this endpoint, and ensure the headers align with the existing errorResponse usage and any initVmSdk expectations.functions/api/getRewards.ts (1)
38-39: Potential floating-point precision loss with large token amounts.Division by
Math.pow(10, decimals)can introduce floating-point precision errors for tokens with high decimals (e.g., 18) or large raw amounts. Consider keeping amounts as strings or using a decimal library for financial precision.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@functions/api/getRewards.ts` around lines 38 - 39, The amount calculation using rawAmount / Math.pow(10, decimals) in the getRewards response can lose precision for large integers or high decimals (e.g., 18); change the implementation that constructs the response (the object with properties decimals and amount) to avoid floating-point division: either convert rawAmount to a BigInt and perform integer math then format the decimal string, or use a decimal library (e.g., decimal.js or Big.js) to divide by 10^decimals and return a string representation; keep the field name amount but return a precise string (or BigInt-based object) instead of a floating-point number so callers receive exact token values.src/features/profile/components/ProfileForm.tsx (1)
57-59: Success banner persists indefinitely after save.React Query's
isSuccessremainstrueuntil the next mutation call. The success banner will stay visible until the user attempts another save or leaves the page. Consider resetting the mutation state after showing feedback.Proposed fix - reset mutation state after delay
+import { useEffect } from 'react'; + useEffect(() => { + if (saveProfile.isSuccess) { + const timer = setTimeout(() => saveProfile.reset(), 3000); + return () => clearTimeout(timer); + } + }, [saveProfile.isSuccess]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/profile/components/ProfileForm.tsx` around lines 57 - 59, The success banner in ProfileForm uses the React Query mutation saveProfile.isSuccess which stays true indefinitely; add a side-effect in the ProfileForm component that watches saveProfile.isSuccess and, when true, sets a short timeout (e.g., 2–3s) to call saveProfile.reset() so the banner is cleared automatically, and ensure you clear the timeout on unmount or when isSuccess changes to avoid leaks.src/index.tsx (1)
5-5: Consider wrapping withReact.StrictModefor development benefits.
StrictModehelps identify potential problems by double-invoking certain lifecycle methods and effects in development. This catches issues with impure renders, missing cleanup, and deprecated APIs.Proposed change
+import { StrictMode } from 'react'; import { createRoot } from 'react-dom/client'; import './index.css'; import App from './App'; -createRoot(document.getElementById('root')!).render(<App />); +createRoot(document.getElementById('root')!).render( + <StrictMode> + <App /> + </StrictMode> +);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/index.tsx` at line 5, Wrap the root render in React.StrictMode to enable development-only checks: locate the createRoot(...) call that renders App (the createRoot(document.getElementById('root')!).render(...) invocation) and change it so the rendered element is <React.StrictMode><App /></React.StrictMode>, ensuring you import React if not already imported and keeping this only for development builds if desired.functions/services/tokenPricing.ts (2)
39-39: Unsafe type assertion on JSON response.The response is cast to
Record<string, PriceFeedEntry>without validation. If the API returns malformed data, this could lead to runtime errors when accessinginfo.priceADAorinfo.price. Consider adding basic shape validation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@functions/services/tokenPricing.ts` at line 39, The JSON response is unsafely asserted to Record<string, PriceFeedEntry> as `payload`; add runtime validation instead: replace the direct cast with parsing into an unknown and run a small validator (e.g., an `isPriceFeedEntry`/`isPriceFeedRecord` helper) that checks each entry is an object and has numeric `priceADA` and `price` properties before treating it as a PriceFeedEntry, and handle invalid shapes (throw or skip) so downstream code that reads `info.priceADA`/`info.price` is safe. Ensure the new validator is referenced where `payload` is used and named clearly (e.g., isPriceFeedEntry, priceFeedRecord) to make locating the fix easy.
9-11: Module-level caching is unreliable in serverless environments.Cloudflare Workers isolates are ephemeral and may not share state. This in-memory cache will be reset on cold starts and won't be consistent across isolates. Consider using Cloudflare KV or a similar persistent store for shared caching if consistency matters, or document that this is a per-isolate optimization only.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@functions/services/tokenPricing.ts` around lines 9 - 11, The module-level in-memory cache (CACHE_DURATION, cachedPrices, cacheTimestamp) is unreliable in serverless Cloudflare Workers; replace it with a persistent store (Cloudflare KV or Durable Object) or explicitly document it as a per-isolate optimization. To fix, update tokenPricing.ts to read/write cached token data and timestamp to a KV namespace (use a stable key like "tokenPrices"), set KV entry TTL or store the expiration alongside the value, and change your functions that currently read cachedPrices/cacheTimestamp to fall back to KV fetch before making the external request; alternatively, add a clear comment near CACHE_DURATION/cachedPrices/cacheTimestamp that this is per-isolate only and does not provide cross-instance consistency. Ensure you update any helper functions that reference cachedPrices or cacheTimestamp to operate against the KV API (get/put) and handle KV errors gracefully.src/pages/ClaimPage.tsx (1)
77-79:showprop on RewardsEmptyState may be redundant.The component is already conditionally rendered. If
showsimply controls visibility, consider removing the prop and relying solely on the parent conditional.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/ClaimPage.tsx` around lines 77 - 79, The RewardsEmptyState component is being conditionally rendered by the parent using connected, isLoading, hasRewards, and error, so the JSX prop show is likely redundant; remove the `show` prop from the usage in ClaimPage and update the RewardsEmptyState component (its props interface/type and any internal conditional that checks `show`) to stop expecting or relying on a `show` boolean—if the component needs to remain controllable externally keep a more descriptive prop name, otherwise delete the prop and any related branches inside the RewardsEmptyState component and its tests.src/features/claim/hooks/useClaimFlow.ts (1)
43-44: Consider user feedback during wallet signing.The
signingstate is set before callingwallet.signTx, but if the user cancels the signing prompt, the error is caught generically. Consider providing a more specific error message for user cancellation (e.g., "Transaction signing was cancelled").🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/claim/hooks/useClaimFlow.ts` around lines 43 - 44, The current flow sets setState({ step: 'signing', unsignedTx: submitResult.unsignedTx }) and then calls wallet.signTx(submitResult.unsignedTx, false) but treats all signTx failures the same; update the try/catch around wallet.signTx in useClaimFlow (the block using setState and signTx) to detect a user-cancelled signing error (check error.message or a provider-specific code like "USER_REJECTED" / "ACTION_REJECTED") and setState to an error state with a clear message such as "Transaction signing was cancelled"; for other errors preserve the existing generic error handling and rethrow or log as before.src/App.tsx (2)
18-18: Consider restricting API tester route to development builds.The
/api-testerroute is exposed unconditionally. If this is a debugging tool, consider gating it behind a development environment check to prevent exposure in production.💡 Example conditional route
+import { isDev } from '@/utils/env'; // or use import.meta.env.DEV <Route path="/preferences" element={<PreferencesPage />} /> -<Route path="/api-tester" element={<ApiTesterPage />} /> +{isDev && <Route path="/api-tester" element={<ApiTesterPage />} />}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/App.tsx` at line 18, The /api-tester route is currently always registered via the Route with element ApiTesterPage; restrict it to development builds by wrapping that Route in a runtime check (e.g., process.env.NODE_ENV === 'development' or an isDev flag) so the Route for ApiTesterPage is only rendered when in dev; locate the Route declaration containing ApiTesterPage and conditionally include it (or return null) when the environment is not development.
2-7: Lazy loading removed — bundle size tradeoff.Switching from lazy-loaded pages to direct imports increases the initial bundle size. This is acceptable for small apps but may impact load times as the app grows. Consider re-introducing
React.lazyif page count increases.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/App.tsx` around lines 2 - 7, The PR replaced lazy-loaded page components with direct imports (ClaimPage, HistoryPage, PreferencesPage, ApiTesterPage) which increases initial bundle size; revert to code-splitting by wrapping those page components with React.lazy and Suspense where they are rendered (or create lazy wrappers named like LazyClaimPage, LazyHistoryPage, LazyPreferencesPage, LazyApiTesterPage) and ensure the top-level rendering inside MainLayout/AppProviders uses a fallback UI via React.Suspense so pages are loaded on demand.src/features/wallet/hooks/useWalletSync.ts (1)
38-40: Consider propagating sync errors to the wallet store.When wallet sync fails, the error is only logged to the console. The user has no indication that their wallet state may be stale or incomplete. Consider setting an error state in the wallet store or triggering a user-visible notification.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/wallet/hooks/useWalletSync.ts` around lines 38 - 40, The catch block in useWalletSync.ts only logs errors; update it to propagate the error into the wallet store so UI can react (e.g., set an error flag/message and/or mark state as stale). In the catch for the async sync in useWalletSync (or the syncWallet function), call the wallet store's error setter or dispatcher (e.g., walletStore.setError / walletStore.setSyncError / dispatch({type: 'SYNC_ERROR', payload: error})) and optionally set a stale flag (e.g., walletStore.setStale(true)) or trigger the existing user notification utility (e.g., notifyError) so the user sees the failure. Ensure you pass the caught error object or its message, and keep the console.error for dev logs if desired.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@functions/api/claim/validate.ts`:
- Around line 17-23: The handler currently calls initVmSdk(env) but never
performs validation and always returns valid: true with a predictable
airdropHash; either implement real validation using the initialized SDK (call
the VM SDK validation method(s) with the request input, set valid based on SDK
response, include transactionCount from actual checks, and compute a
non-predictable airdropHash) or mark this as an explicit placeholder by
returning a 501/Not Implemented (matching submit.ts) and/or adding a TODO;
update the response builder (jsonResponse) accordingly and remove the
Date.now()-based airdropHash if implementing real behavior.
In `@functions/api/getRewards.ts`:
- Around line 81-85: The code reads the URL param as walletId but uses
stakeAddress in validation and likely in downstream calls; rename the local
variable stakeAddress to walletId (or vice versa) so the parameter name is
consistent, update the errorResponse message to reference "walletId is required"
instead of "stakeAddress is required", and ensure the subsequent call to
getRewards uses the same identifier (e.g., getRewards(walletId, env)); adjust
any other uses of stakeAddress/error text to match the chosen name.
In `@functions/api/getSettings.ts`:
- Around line 39-43: The catch block in getSettings exposes internal error
details to clients by returning the raw exception message; instead, keep
detailed error logging (console.error or processLogger) but change the
errorResponse call to return a generic message (e.g., "Failed to fetch
settings") or a harmless, non-sensitive string, removing use of
error.message/JSON.stringify(error); locate the catch in getSettings and update
the errorResponse invocation so only a sanitized message is returned while
preserving the detailed error in server logs.
In `@functions/api/profileData.ts`:
- Around line 12-14: The Content-Type check uses strict equality against
'application/json' and will reject valid headers like 'application/json;
charset=utf-8'; update the check around request.headers.get('Content-Type') to
safely handle null and match prefixes (e.g., get the header into a variable like
contentType and verify contentType && contentType.startsWith('application/json')
or contentType.includes('application/json')) before calling
errorResponse('Request body must be JSON', 415).
In `@functions/api/sanitizeAddress.ts`:
- Around line 21-25: In the catch block of sanitizeAddress (the catch handling
that currently logs with console.error and returns errorResponse(`Failed to
sanitize address: ${message}`)), stop returning the internal error.message to
clients; keep logging the full error locally (console.error or processLogger)
but change the returned errorResponse to a generic message like "Failed to
sanitize address" (use sanitizeAddress function and errorResponse helper), so
internal error details are not exposed to the client while still preserving full
internal logs for debugging.
In `@src/components/common/ConnectWallet.tsx`:
- Around line 19-22: The handleConnect function calls connect(walletName)
without catching errors; wrap the await connect(walletName) call in try/catch
inside handleConnect, call setOpen(false) only on success, and in the catch
block log the error (e.g., console.error or processLogger) and surface a
user-facing error (e.g., show a toast/notification or set an error state) so
users get feedback when connection is rejected or fails; refer to handleConnect,
connect, and setOpen when making the changes.
In `@src/features/claim/api/claim.queries.ts`:
- Around line 31-42: Remove the unused useClaimStatus hook: delete the entire
function export function useClaimStatus(hash: string | null) { ... } from
claim.queries.ts (including its queryKey/queryFn/refetchInterval logic) so the
module no longer exports or defines useClaimStatus; ensure no other files import
useClaimStatus (if any, remove those imports) and run a build/test to confirm
there are no remaining references.
In `@src/features/claim/hooks/useClaimFlow.ts`:
- Around line 9-16: ClaimFlowStep includes a 'polling' state that is never used
by useClaimFlow; either remove 'polling' from the ClaimFlowStep type and from
STEP_LABELS in ClaimButton.tsx and the isWorking check, or implement polling
after transaction submission: in useClaimFlow (where validateMutation,
submitMutation, submitTxMutation are used) add a transition to step 'polling'
after submitTxMutation succeeds and start a confirmation loop (using queryClient
or submitTxMutation response) that checks transaction status until confirmed
then set step to 'completed' or on timeout/error set 'error'; update STEP_LABELS
and any isWorking logic to reflect the chosen approach so labels and button
state align with actual flow.
- Line 59: The dependency array for startClaim in useClaimFlow currently
includes unstable mutation objects (validateMutation, submitMutation,
submitTxMutation) causing unnecessary recreations; instead, destructure and use
the stable properties you call (e.g., validateMutation.mutateAsync,
submitMutation.mutateAsync, submitTxMutation.mutateAsync) and include those
specific functions in the dependency array (or omit the mutation objects
entirely if you don't rely on callback identity), keeping stakeAddress, wallet
and queryClient as before; update references in startClaim to use the
destructured names so the callback only depends on stable values.
In `@src/features/profile/components/ProfileForm.tsx`:
- Around line 14-17: handleSubmit currently returns silently when wallet,
stakeAddress, or changeAddress are missing; add visible validation feedback
instead. Update handleSubmit to set a local error state (e.g., profileError or
submitError) when any of wallet, stakeAddress, or changeAddress is falsy and
return, and ensure the component renders that error state alongside existing
banners; reference the handleSubmit function and the wallet, stakeAddress,
changeAddress variables to locate where to set the state and where to display
the message. Ensure the error message is user-friendly (e.g., "Missing wallet or
address information") and cleared when inputs change or on successful submit.
- Around line 35-37: The catch in ProfileForm.tsx only console.error's signing
failures (inside the try around signProfileUpdateMessage) so users get no
feedback; add a local state (e.g., signingError via useState) and set it when
the catch fires, then surface that state to the UI (an inline error message or
toast) and ensure any existing saveProfile.isError handling remains for mutation
errors; update the catch to setSigningError(error) and clear it on successful
sign or retry, and reference the signProfileUpdateMessage call and saveProfile
mutation handling when making the change.
In `@src/types/claim.ts`:
- Around line 34-39: ClaimStatus.hash is ambiguous; rename it to the concrete
field the API uses (e.g., airdropHash or txHash) and update all usages of
ClaimStatus.hash in the polling and status endpoint code (look for references to
ClaimStatus and the polling flow) so the model matches the backend contract;
likewise narrow ClaimSubmitResponse.transactionType from string to the explicit
literal union the backend will return (e.g., 'airdrop' | 'claim' | 'refund' or
the agreed set) and update any type guards/parsing that consume transactionType
to use those literals.
---
Outside diff comments:
In `@functions/services/tokenPricing.ts`:
- Around line 23-27: The current logic caches an empty result when endpoint is
falsy (setting cachedPrices and cacheTimestamp), causing stale empty responses;
change the behavior in the function that checks endpoint so that if endpoint is
undefined you return early without updating cachedPrices or cacheTimestamp
(i.e., do not set the cache or touch CACHE_DURATION state) so callers can pick
up a later-provided endpoint; refer to the endpoint check and the
cachedPrices/cacheTimestamp variables to implement this change.
---
Nitpick comments:
In `@functions/api/claim/status.ts`:
- Around line 7-24: Add a CORS preflight handler similar to other claim
endpoints by exporting an onRequestOptions alongside onRequestGet; implement
onRequestOptions to return a 204/200 response with the same CORS headers used
elsewhere (e.g., Access-Control-Allow-Origin, Access-Control-Allow-Methods,
Access-Control-Allow-Headers) so browser clients can call this endpoint, and
ensure the headers align with the existing errorResponse usage and any initVmSdk
expectations.
In `@functions/api/claim/submitTransaction.ts`:
- Around line 10-15: The current handler uses request.json<T>() and only checks
presence of body.signedTx and body.airdropHash; add runtime type checks to
ensure both fields are strings (e.g., verify typeof body.signedTx === 'string'
and typeof body.airdropHash === 'string') before proceeding, and return
errorResponse('signedTx and airdropHash must be strings', 400) if the checks
fail; update the validation around the body variable in submitTransaction (where
request.json is called and errorResponse is used) to perform these typeof
checks.
In `@functions/api/getRewards.ts`:
- Around line 38-39: The amount calculation using rawAmount / Math.pow(10,
decimals) in the getRewards response can lose precision for large integers or
high decimals (e.g., 18); change the implementation that constructs the response
(the object with properties decimals and amount) to avoid floating-point
division: either convert rawAmount to a BigInt and perform integer math then
format the decimal string, or use a decimal library (e.g., decimal.js or Big.js)
to divide by 10^decimals and return a string representation; keep the field name
amount but return a precise string (or BigInt-based object) instead of a
floating-point number so callers receive exact token values.
In `@functions/api/getSettings.ts`:
- Line 10: The hardcoded constant VM_URL in getSettings.ts should be made
configurable via environment variables; replace the literal
'https://vmprev.adaseal.eu' with a lookup that reads process.env.VM_URL (or
process.env.VM_URL_DEFAULT fallback) and ensure the code that uses VM_URL
(symbol: VM_URL) validates presence or provides a sensible default for different
environments (dev/staging/prod) and update any documentation or env sample to
include VM_URL.
In `@functions/api/profileData.ts`:
- Around line 23-25: The handler currently only checks body.walletId but may
accept missing or malformed profile content; add validation to ensure body.value
exists and that body.value.name is a non-empty string before proceeding (return
errorResponse('Missing or invalid profile name', 400) on failure). Locate the
request handler in functions/api/profileData.ts and update the logic around the
existing walletId check to validate body.value and body.value.name, rejecting
invalid input and only storing when those checks pass.
In `@functions/api/sanitizeAddress.ts`:
- Around line 17-19: The file redundantly imports 'vm-sdk' after calling
initVmSdk; update initVmSdk to return the initialized SDK module (export async
function initVmSdk(env: Env) { const vmSdk = await import('vm-sdk');
vmSdk.setApiToken(env.VITE_VM_API_KEY); return vmSdk; }) and then in
sanitizeAddress.ts call const { getSanitizedAddress } = await initVmSdk(env)
instead of doing a second dynamic import; remove the extra import of 'vm-sdk'
and adjust initVmSdk's callers to use the returned module.
In `@functions/services/tokenPricing.ts`:
- Line 39: The JSON response is unsafely asserted to Record<string,
PriceFeedEntry> as `payload`; add runtime validation instead: replace the direct
cast with parsing into an unknown and run a small validator (e.g., an
`isPriceFeedEntry`/`isPriceFeedRecord` helper) that checks each entry is an
object and has numeric `priceADA` and `price` properties before treating it as a
PriceFeedEntry, and handle invalid shapes (throw or skip) so downstream code
that reads `info.priceADA`/`info.price` is safe. Ensure the new validator is
referenced where `payload` is used and named clearly (e.g., isPriceFeedEntry,
priceFeedRecord) to make locating the fix easy.
- Around line 9-11: The module-level in-memory cache (CACHE_DURATION,
cachedPrices, cacheTimestamp) is unreliable in serverless Cloudflare Workers;
replace it with a persistent store (Cloudflare KV or Durable Object) or
explicitly document it as a per-isolate optimization. To fix, update
tokenPricing.ts to read/write cached token data and timestamp to a KV namespace
(use a stable key like "tokenPrices"), set KV entry TTL or store the expiration
alongside the value, and change your functions that currently read
cachedPrices/cacheTimestamp to fall back to KV fetch before making the external
request; alternatively, add a clear comment near
CACHE_DURATION/cachedPrices/cacheTimestamp that this is per-isolate only and
does not provide cross-instance consistency. Ensure you update any helper
functions that reference cachedPrices or cacheTimestamp to operate against the
KV API (get/put) and handle KV errors gracefully.
In `@functions/services/vmClient.ts`:
- Around line 10-17: The current corsHeaders() function returns a wildcard
Access-Control-Allow-Origin which is insecure for production; update corsHeaders
to support a configurable allowlist (e.g., from an environment variable or a
passed-in parameter) and set Access-Control-Allow-Origin dynamically by checking
the incoming request Origin against that allowlist (return the specific origin
when allowed, or omit/return a safe default when not). Locate and modify
corsHeaders() in vmClient.ts to accept an optional origins parameter or read
process.env.ALLOWED_ORIGINS, perform origin validation, and emit the specific
origin value instead of '*' (also ensure existing headers like
Access-Control-Allow-Methods and -Headers remain intact).
- Around line 27-29: The OPTIONS handler optionsResponse currently returns a 204
with headers that include Content-Type; remove Content-Type for a No Content
response by adjusting optionsResponse (or the headers it uses) so the Response's
headers do not contain "Content-Type" — e.g., obtain the headers from
corsHeaders(), clone into a Headers object and delete the "Content-Type" key
(handle both "Content-Type" and lower-case "content-type" if present) before
returning new Response(null, { status: 204, headers }). Ensure the unique
function name optionsResponse and the helper corsHeaders() are the locations you
change.
In `@src/api/client.ts`:
- Around line 13-14: Replace the plain object being thrown with a real Error
subclass: create an ApiError class (e.g., export class ApiError extends Error {
status: number; constructor(message: string, status: number) { super(message);
this.name = 'ApiError'; this.status = status; } }) in your types or errors
module, import that into src/api/client.ts, and change the throw site (where
currently `const error: ApiError = { message, status: res.status }; throw
error;`) to `throw new ApiError(message, res.status)` so stack traces and
instanceof checks work correctly.
In `@src/App.tsx`:
- Line 18: The /api-tester route is currently always registered via the Route
with element ApiTesterPage; restrict it to development builds by wrapping that
Route in a runtime check (e.g., process.env.NODE_ENV === 'development' or an
isDev flag) so the Route for ApiTesterPage is only rendered when in dev; locate
the Route declaration containing ApiTesterPage and conditionally include it (or
return null) when the environment is not development.
- Around line 2-7: The PR replaced lazy-loaded page components with direct
imports (ClaimPage, HistoryPage, PreferencesPage, ApiTesterPage) which increases
initial bundle size; revert to code-splitting by wrapping those page components
with React.lazy and Suspense where they are rendered (or create lazy wrappers
named like LazyClaimPage, LazyHistoryPage, LazyPreferencesPage,
LazyApiTesterPage) and ensure the top-level rendering inside
MainLayout/AppProviders uses a fallback UI via React.Suspense so pages are
loaded on demand.
In `@src/features/claim/hooks/useClaimFlow.ts`:
- Around line 43-44: The current flow sets setState({ step: 'signing',
unsignedTx: submitResult.unsignedTx }) and then calls
wallet.signTx(submitResult.unsignedTx, false) but treats all signTx failures the
same; update the try/catch around wallet.signTx in useClaimFlow (the block using
setState and signTx) to detect a user-cancelled signing error (check
error.message or a provider-specific code like "USER_REJECTED" /
"ACTION_REJECTED") and setState to an error state with a clear message such as
"Transaction signing was cancelled"; for other errors preserve the existing
generic error handling and rethrow or log as before.
In `@src/features/profile/components/ProfileForm.tsx`:
- Around line 57-59: The success banner in ProfileForm uses the React Query
mutation saveProfile.isSuccess which stays true indefinitely; add a side-effect
in the ProfileForm component that watches saveProfile.isSuccess and, when true,
sets a short timeout (e.g., 2–3s) to call saveProfile.reset() so the banner is
cleared automatically, and ensure you clear the timeout on unmount or when
isSuccess changes to avoid leaks.
In `@src/features/wallet/hooks/useWalletSync.ts`:
- Around line 38-40: The catch block in useWalletSync.ts only logs errors;
update it to propagate the error into the wallet store so UI can react (e.g.,
set an error flag/message and/or mark state as stale). In the catch for the
async sync in useWalletSync (or the syncWallet function), call the wallet
store's error setter or dispatcher (e.g., walletStore.setError /
walletStore.setSyncError / dispatch({type: 'SYNC_ERROR', payload: error})) and
optionally set a stale flag (e.g., walletStore.setStale(true)) or trigger the
existing user notification utility (e.g., notifyError) so the user sees the
failure. Ensure you pass the caught error object or its message, and keep the
console.error for dev logs if desired.
In `@src/index.tsx`:
- Line 5: Wrap the root render in React.StrictMode to enable development-only
checks: locate the createRoot(...) call that renders App (the
createRoot(document.getElementById('root')!).render(...) invocation) and change
it so the rendered element is <React.StrictMode><App /></React.StrictMode>,
ensuring you import React if not already imported and keeping this only for
development builds if desired.
In `@src/layouts/components/PrimaryNavigation.tsx`:
- Line 12: The "API Tester" nav entry is always rendered; restrict it to
non-production by conditionally including or filtering out the item in the
PrimaryNavigation component: locate the nav data or JSX that contains the object
{ name: 'API Tester', href: '/api-tester' } inside PrimaryNavigation and only
add/render that entry when a development flag is true (e.g.,
process.env.NODE_ENV !== 'production' or a dedicated NEXT_PUBLIC_*/isDev flag);
update the logic that builds the navigation array or the map/render loop to skip
the 'API Tester' item when in production.
In `@src/pages/ApiTesterPage.tsx`:
- Around line 37-44: The displayed endpoint string for the "Get Rewards" entry
uses the raw stakeAddress but the action uses encodeURIComponent, causing a
mismatch; update the endpoint property to use an encoded stakeAddress as well
(mirror the encoding used in the action) so the displayed URL matches the actual
request—look for the object with name 'Get Rewards' and fields endpoint, action,
fetchApi, and apiClient.get referencing stakeAddress and apply
encodeURIComponent(stakeAddress ?? '') to the endpoint construction.
In `@src/pages/ClaimPage.tsx`:
- Around line 77-79: The RewardsEmptyState component is being conditionally
rendered by the parent using connected, isLoading, hasRewards, and error, so the
JSX prop show is likely redundant; remove the `show` prop from the usage in
ClaimPage and update the RewardsEmptyState component (its props interface/type
and any internal conditional that checks `show`) to stop expecting or relying on
a `show` boolean—if the component needs to remain controllable externally keep a
more descriptive prop name, otherwise delete the prop and any related branches
inside the RewardsEmptyState component and its tests.
In `@src/pages/PreferencesPage.tsx`:
- Around line 6-8: PreferencesPage currently calls useProfile(stakeAddress) but
doesn't handle the hook's loading or error states; update PreferencesPage to
read isLoading and error from useProfile (e.g., const { data: profile,
isLoading, error } = useProfile(stakeAddress)) and add early UI handling: render
a loading indicator while isLoading is true and render an error message (or
fallback UI) when error is present before attempting to use profile; ensure
stakeAddress is guarded (e.g., null-check) so useProfile isn't used with an
invalid address.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8248c8ea-5751-49f0-af81-30582f704fa5
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (54)
README.mdfunctions/api/claim/status.tsfunctions/api/claim/submit.tsfunctions/api/claim/submitTransaction.tsfunctions/api/claim/validate.tsfunctions/api/getRewards.tsfunctions/api/getSettings.tsfunctions/api/profileData.tsfunctions/api/sanitizeAddress.tsfunctions/api/tokenPrices.tsfunctions/services/tokenPricing.tsfunctions/services/vmClient.tspackage.jsonsrc/App.tsxsrc/api/client.tssrc/api/getTokens.tssrc/api/profileApi.tssrc/api/queryClient.tssrc/app/providers.tsxsrc/components/common/ConnectWallet.tsxsrc/components/common/SectionCard.tsxsrc/components/ui/navigation-menu-styles.tssrc/components/ui/navigation-menu.tsxsrc/components/wallet/CardanoWalletConnector.tsxsrc/features/claim/api/claim.queries.tssrc/features/claim/components/ClaimButton.tsxsrc/features/claim/components/ClaimStatus.tsxsrc/features/claim/hooks/useClaimFlow.tssrc/features/preferences/components/ProfileForm.tsxsrc/features/profile/api/profile.queries.tssrc/features/profile/components/ProfileForm.tsxsrc/features/rewards/api/getRewards.tssrc/features/rewards/api/rewards.queries.tssrc/features/rewards/components/RewardCard.tsxsrc/features/rewards/components/RewardsEmptyState.tsxsrc/features/rewards/components/WalletAddressForm.tsxsrc/features/rewards/hooks/useRewards.tssrc/features/wallet/hooks/useWalletSync.tssrc/index.csssrc/index.tsxsrc/layouts/MainLayout.tsxsrc/layouts/components/PrimaryNavigation.tsxsrc/pages/ApiTesterPage.tsxsrc/pages/ClaimPage.tsxsrc/pages/HistoryPage.tsxsrc/pages/PreferencesPage.tsxsrc/shared/rewards/index.tssrc/store/wallet-state.tssrc/types/api.tssrc/types/claim.tssrc/types/profile.tssrc/types/wallet.tssrc/utils/profile-helpers.tsvite.config.ts
💤 Files with no reviewable changes (13)
- src/api/profileApi.ts
- src/api/getTokens.ts
- src/features/rewards/components/WalletAddressForm.tsx
- src/features/rewards/components/RewardCard.tsx
- src/features/preferences/components/ProfileForm.tsx
- src/pages/HistoryPage.tsx
- src/components/wallet/CardanoWalletConnector.tsx
- src/components/ui/navigation-menu.tsx
- src/features/rewards/hooks/useRewards.ts
- src/shared/rewards/index.ts
- README.md
- src/components/ui/navigation-menu-styles.ts
- src/features/rewards/api/getRewards.ts
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (4)
functions/api/getSettings.ts (1)
39-42:⚠️ Potential issue | 🟡 MinorDon't echo backend exception text to clients.
Line 42 still includes the raw caught error message in the API response. Keep the full detail in logs, but return a generic failure message so VM/KV internals don't leak over the wire.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@functions/api/getSettings.ts` around lines 39 - 42, The catch block in getSettings currently returns the raw exception message to the client; keep full details in logs but return a generic message instead. In the catch for getSettings, keep console.error('getSettings error:', error) and the message variable for logging, but change the errorResponse call to a non-sensitive string like errorResponse('Failed to fetch settings') (remove interpolation of the caught error/message). Ensure you update the return in the catch inside getSettings and leave logging unchanged.src/features/profile/components/ProfileForm.tsx (1)
14-17:⚠️ Potential issue | 🟡 MinorSurface wallet-state and signing failures in the UI.
Line 71 only disables on
!connected, so users can submit beforestakeAddress/changeAddressare hydrated and hit the silent return at Line 17. And Lines 35-37 only log signing errors, so the form appears to do nothing. Add local error state and render it alongside the mutation banner.Also applies to: 35-37, 69-72
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/profile/components/ProfileForm.tsx` around lines 14 - 17, The form currently silently returns in handleSubmit when wallet/stakeAddress/changeAddress are missing and only logs signing errors; add a local error state (e.g., profileError) in the ProfileForm component, update the submit button disabled logic to check !wallet || !stakeAddress || !changeAddress || !connected, set profileError with a user-friendly message when handleSubmit early-returns (mention wallet/stakeAddress/changeAddress) and when the signing routine (the function around lines 35-37 that calls the signer) throws, and render profileError alongside the existing mutation banner so users see wallet/state and signing failures in the UI.functions/api/profileData.ts (1)
12-13:⚠️ Potential issue | 🟡 MinorContent-Type check is still too strict.
application/json; charset=utf-8is a valid JSON request, but Line 12 rejects it. Match the media type more flexibly before returning 415.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@functions/api/profileData.ts` around lines 12 - 13, The Content-Type check currently compares request.headers.get('Content-Type') for an exact match and rejects valid values like "application/json; charset=utf-8"; update the check to parse or match the media type more flexibly (e.g., read request.headers.get('content-type') and test that it startsWith('application/json') or split on ';' and compare the base type) before calling errorResponse('Request body must be JSON', 415) so requests with parameters are accepted.functions/api/claim/validate.ts (1)
17-23:⚠️ Potential issue | 🟠 MajorThis endpoint still doesn't validate claims.
After
initVmSdk, Lines 19-23 always returnvalid: true,transactionCount: 1, and aDate.now()hash. That makes the result independent of the request body and can green-light invalid claims. Either call the VM validation API here or return 501 until it's wired up.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@functions/api/claim/validate.ts` around lines 17 - 23, The handler in validate.ts currently returns a hardcoded success after initVmSdk, which ignores the request; replace the stubbed response with a real VM validation call (or return 501 if not implemented). Specifically, after initVmSdk(env) call into the VM validation API (e.g., a method like vm.validateClaim or validateClaimWithVm) passing the parsed request body, map the VM response to the jsonResponse fields (valid, transactionCount, airdropHash) and handle VM errors by logging and returning an appropriate error status; if the VM validation function does not yet exist, return jsonResponse with a 501 status and a clear message instead of the current hardcoded valid:true response. Ensure you reference initVmSdk and jsonResponse and replace the hardcoded block that sets valid/transactionCount/airdropHash.
🧹 Nitpick comments (3)
functions/api/getRewards.ts (1)
64-71: Token refresh could log cache miss for observability.When a token is missing and requires a refresh, this is a notable event that could indicate stale cache or new tokens. Consider adding a log statement for observability.
📊 Suggested observability improvement
for (const assetId of allAssetIds) { if (!tokens[assetId]) { + console.log(`Token cache miss for ${assetId}, refreshing tokens`); tokens = (await getTokensFromVM()) as unknown as Record<string, TokenInfo> | null; if (!tokens) return []; break; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@functions/api/getRewards.ts` around lines 64 - 71, When looping over allAssetIds and discovering a missing token in tokens, add a logging call to record the cache miss prior to calling getTokensFromVM() — include the assetId and that you're refreshing tokens (use the same context/logger used in this file). After calling getTokensFromVM() (and before returning on null), log the outcome of the refresh (success with count of tokens or failure) so getTokensFromVM and TokenInfo refresh events are observable.src/features/profile/api/profile.queries.ts (1)
11-13: Consider using theApiErrortype for consistent error handling.The error type check is loose. Since
apiClientthrowsApiError(from@/types/api), you can use the same type for a cleaner check:+import type { ApiError } from '@/types/api'; + // in queryFn: } catch (e: unknown) { - if (e && typeof e === 'object' && 'status' in e && e.status === 404) return null; + if ((e as ApiError)?.status === 404) return null; throw e; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/profile/api/profile.queries.ts` around lines 11 - 13, The catch block handling unknown errors should use the ApiError type from "@/types/api" for a precise check: import ApiError and in the catch(e: unknown) replace the loose object/status check with a typed guard such as checking e instanceof ApiError (or casting to ApiError and checking e.status === 404) to return null on 404 and rethrow otherwise; update the block around the function that calls apiClient to reference ApiError for consistent error handling.src/api/client.ts (1)
3-37: Consider extracting shared error-handling logic.The error extraction logic (lines 5-14 and 25-34) is duplicated. A helper would reduce duplication:
♻️ Optional refactor
+async function extractErrorMessage(res: Response): Promise<string> { + try { + const body = (await res.json()) as Record<string, string>; + return body.error || body.message || res.statusText; + } catch { + return res.statusText; + } +} + async function apiGet<T>(url: string): Promise<T> { const res = await fetch(url); if (!res.ok) { - let message: string; - try { - const body = (await res.json()) as Record<string, string>; - message = body.error || body.message || res.statusText; - } catch { - message = res.statusText; - } + const message = await extractErrorMessage(res); const error: ApiError = { message, status: res.status }; throw error; } return res.json() as Promise<T>; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/client.ts` around lines 3 - 37, Extract the duplicated error extraction into a shared async helper (e.g., parseApiError or getErrorFromResponse) that accepts the Response and returns an ApiError { message, status }; have this helper attempt res.json(), pick json.error || json.message || res.statusText and fall back to res.statusText on parse failure, then throw the result from both apiGet and apiPost by replacing their duplicated try/catch blocks with a call to that helper; keep function names apiGet and apiPost and the ApiError shape unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@functions/api/claim/status.ts`:
- Around line 19-24: The catch block in the status handler currently logs the
full error but returns the raw error.message to clients; keep the console.error
(or existing logger) call as-is to record details, but change the errorResponse
call in the catch inside the status function to return a sanitized, generic
message (e.g., "Status check failed" or "Internal server error") without
interpolating error.message or other internal details; ensure you only expose
generic text from the catch while preserving the detailed error in logs via
console.error or the existing logger.
In `@functions/api/claim/validate.ts`:
- Line 11: The call to request.json<{ stakeAddress: string; assets: string[]
}>() is invalid TypeScript because Request.json() isn't generic; change the
parsing to use a non-generic call and assert or annotate the result type instead
(e.g., call request.json() and assign to body with a type assertion/annotation
for { stakeAddress: string; assets: string[] } so variables like body,
stakeAddress and assets keep their typed shapes).
In `@functions/api/getRewards.ts`:
- Line 56: The current early return "if (!rewardsResponse || !tokens) return []"
in getRewards (rewardsResponse and tokens) silently masks API/SDK failures;
change it to throw an error when the VM SDK call returned null/undefined (e.g.,
if rewardsResponse === null || tokens == null) so callers can distinguish an API
failure from a legitimate empty-rewards result, and only return an empty array
when tokens exists but rewardsResponse indicates no rewards (e.g.,
rewardsResponse.length === 0). Follow the pattern used in sanitizeAddress.ts:
validate the VM SDK response and throw a descriptive Error (include function
name getRewards and relevant context) for absent responses, otherwise proceed
and return [] for an actual empty rewards set.
In `@functions/api/getSettings.ts`:
- Around line 7-12: The settings cache shares KV keys with user profiles
(VM_WEB_PROFILES using CACHE_KEY 'settings_cache'), allowing clients from
profileData (walletId) to collide or read it; fix by namespacing settings keys
(e.g., change CACHE_KEY to a unique prefix like 'settings:cache' or incorporate
a constant prefix in getSettings' storeKey generation) or move settings storage
to a dedicated KV namespace; update all usages in this file (references to
CACHE_KEY and any get/put calls around VM_WEB_PROFILES) and ensure profileData's
walletId reads/writes cannot use that new prefix.
In `@functions/api/profileData.ts`:
- Around line 16-29: The handler in profileData.ts accepts body from
request.json() and writes to env.VM_WEB_PROFILES.put without authenticating, so
implement signature verification before the KV.put: extract body.signature and
body.message (matching what ProfileForm.tsx sends), validate that the signature
corresponds to body.walletId (recover the signer from the signed message and
compare to walletId), and only call env.VM_WEB_PROFILES.put and return
jsonResponse when the signature check passes; otherwise return
errorResponse('Unauthorized' or 'Invalid signature', 401). Ensure you perform
this check before the existing env.VM_WEB_PROFILES.put call and keep existing
errorResponse/jsonResponse usage.
In `@src/features/claim/components/ClaimStatus.tsx`:
- Around line 9-41: ClaimStatusDisplay currently renders nothing for in-progress
steps ('validating', 'signing', 'submitting', 'polling'); update
ClaimStatusDisplay to show a loading/progress UI for those steps (e.g., reuse
FeedbackBanner with tone="info" or a spinner + descriptive message) so users get
feedback while a claim is being processed; map each state.step ('validating',
'signing', 'submitting', 'polling') to an appropriate message (like "Validating
claim...", "Waiting for wallet signature...", "Submitting transaction...",
"Waiting for confirmation...") and ensure the UI disables or hides the Done
button until state.step === 'completed'; locate the logic in the
ClaimStatusDisplay function and add the in-progress branch before the final
return null.
In `@src/pages/ApiTesterPage.tsx`:
- Around line 41-44: The tester actions call APIs with a null/empty walletId and
post empty assets; update the action handlers (the fetchApi calls that use
apiClient.get(`/api/getRewards?walletId=${encodeURIComponent(stakeAddress!)}`)
and any apiClient.post calls sending { assets: [] }) to first guard on a real
stakeAddress (not just connected) and early-return or disable the action when
stakeAddress is falsy, and for the claim/validate calls provide a non-empty
assets payload (use a small fixture array or derive a valid asset from existing
state) so functions/api/claim/validate.ts does not reject the request; locate
these changes around the fetchApi usages and the apiClient.post that constructs
the { assets } body and add the stakeAddress checks and fixture asset(s).
In `@src/pages/ClaimPage.tsx`:
- Around line 13-15: The rewards query is being invoked and the UI driven by
`connected` before `stakeAddress` is truthy, causing premature fetch/error/empty
states; compute a readiness flag like `const ready = connected &&
!!stakeAddress` and gate the `useRewards` call and any connected-only UI on that
flag (either call `useRewards(ready ? stakeAddress : undefined)` or pass an `{
enabled: ready }` option into `useRewards`), and update uses of
`connected`/reward state in this component (e.g. around `useRewards`,
`stakeAddress`, `connected`, and `claimFlow`) to rely on `ready` instead of
`connected` alone.
In `@src/store/wallet-state.ts`:
- Around line 19-20: setWalletState currently merges partial updates and can
leave stakeAddress/changeAddress populated when connected becomes false; update
the setWalletState implementation so when partial.connected === false (or when
connected transitions from true to false) it also clears stakeAddress and
changeAddress (or assigns them the same empty values used in initialState)
instead of preserving prior values; reference the setWalletState and resetWallet
functions and the stakeAddress/changeAddress and connected fields to locate and
implement this conditional clear.
In `@src/utils/profile-helpers.ts`:
- Around line 21-25: The helper now returns result.key from wallet.signData but
that key is never propagated; either remove returning key or wire it through the
profile save flow: if you choose to propagate, add a key field to the profile
type in src/types/profile.ts, update the ProfileForm submit handler in
src/features/profile/components/ProfileForm.tsx to include the key when calling
the save API, and update functions/api/profileData.ts to accept and persist the
key; alternatively, if you choose to drop it, remove result.key from the object
returned by the sign function in src/utils/profile-helpers.ts so the helper and
types remain consistent.
---
Duplicate comments:
In `@functions/api/claim/validate.ts`:
- Around line 17-23: The handler in validate.ts currently returns a hardcoded
success after initVmSdk, which ignores the request; replace the stubbed response
with a real VM validation call (or return 501 if not implemented). Specifically,
after initVmSdk(env) call into the VM validation API (e.g., a method like
vm.validateClaim or validateClaimWithVm) passing the parsed request body, map
the VM response to the jsonResponse fields (valid, transactionCount,
airdropHash) and handle VM errors by logging and returning an appropriate error
status; if the VM validation function does not yet exist, return jsonResponse
with a 501 status and a clear message instead of the current hardcoded
valid:true response. Ensure you reference initVmSdk and jsonResponse and replace
the hardcoded block that sets valid/transactionCount/airdropHash.
In `@functions/api/getSettings.ts`:
- Around line 39-42: The catch block in getSettings currently returns the raw
exception message to the client; keep full details in logs but return a generic
message instead. In the catch for getSettings, keep console.error('getSettings
error:', error) and the message variable for logging, but change the
errorResponse call to a non-sensitive string like errorResponse('Failed to fetch
settings') (remove interpolation of the caught error/message). Ensure you update
the return in the catch inside getSettings and leave logging unchanged.
In `@functions/api/profileData.ts`:
- Around line 12-13: The Content-Type check currently compares
request.headers.get('Content-Type') for an exact match and rejects valid values
like "application/json; charset=utf-8"; update the check to parse or match the
media type more flexibly (e.g., read request.headers.get('content-type') and
test that it startsWith('application/json') or split on ';' and compare the base
type) before calling errorResponse('Request body must be JSON', 415) so requests
with parameters are accepted.
In `@src/features/profile/components/ProfileForm.tsx`:
- Around line 14-17: The form currently silently returns in handleSubmit when
wallet/stakeAddress/changeAddress are missing and only logs signing errors; add
a local error state (e.g., profileError) in the ProfileForm component, update
the submit button disabled logic to check !wallet || !stakeAddress ||
!changeAddress || !connected, set profileError with a user-friendly message when
handleSubmit early-returns (mention wallet/stakeAddress/changeAddress) and when
the signing routine (the function around lines 35-37 that calls the signer)
throws, and render profileError alongside the existing mutation banner so users
see wallet/state and signing failures in the UI.
---
Nitpick comments:
In `@functions/api/getRewards.ts`:
- Around line 64-71: When looping over allAssetIds and discovering a missing
token in tokens, add a logging call to record the cache miss prior to calling
getTokensFromVM() — include the assetId and that you're refreshing tokens (use
the same context/logger used in this file). After calling getTokensFromVM() (and
before returning on null), log the outcome of the refresh (success with count of
tokens or failure) so getTokensFromVM and TokenInfo refresh events are
observable.
In `@src/api/client.ts`:
- Around line 3-37: Extract the duplicated error extraction into a shared async
helper (e.g., parseApiError or getErrorFromResponse) that accepts the Response
and returns an ApiError { message, status }; have this helper attempt
res.json(), pick json.error || json.message || res.statusText and fall back to
res.statusText on parse failure, then throw the result from both apiGet and
apiPost by replacing their duplicated try/catch blocks with a call to that
helper; keep function names apiGet and apiPost and the ApiError shape unchanged.
In `@src/features/profile/api/profile.queries.ts`:
- Around line 11-13: The catch block handling unknown errors should use the
ApiError type from "@/types/api" for a precise check: import ApiError and in the
catch(e: unknown) replace the loose object/status check with a typed guard such
as checking e instanceof ApiError (or casting to ApiError and checking e.status
=== 404) to return null on 404 and rethrow otherwise; update the block around
the function that calls apiClient to reference ApiError for consistent error
handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a818eaa4-df87-4f57-92b9-6edff79c6dd0
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (53)
README.mdfunctions/api/claim/status.tsfunctions/api/claim/submit.tsfunctions/api/claim/submitTransaction.tsfunctions/api/claim/validate.tsfunctions/api/getRewards.tsfunctions/api/getSettings.tsfunctions/api/profileData.tsfunctions/api/sanitizeAddress.tsfunctions/services/tokenPricing.tsfunctions/services/vmClient.tspackage.jsonsrc/App.tsxsrc/api/client.tssrc/api/getTokens.tssrc/api/profileApi.tssrc/api/queryClient.tssrc/app/providers.tsxsrc/components/common/ConnectWallet.tsxsrc/components/common/SectionCard.tsxsrc/components/ui/navigation-menu-styles.tssrc/components/ui/navigation-menu.tsxsrc/components/wallet/CardanoWalletConnector.tsxsrc/features/claim/api/claim.queries.tssrc/features/claim/components/ClaimButton.tsxsrc/features/claim/components/ClaimStatus.tsxsrc/features/claim/hooks/useClaimFlow.tssrc/features/preferences/components/ProfileForm.tsxsrc/features/profile/api/profile.queries.tssrc/features/profile/components/ProfileForm.tsxsrc/features/rewards/api/getRewards.tssrc/features/rewards/api/rewards.queries.tssrc/features/rewards/components/RewardCard.tsxsrc/features/rewards/components/RewardsEmptyState.tsxsrc/features/rewards/components/WalletAddressForm.tsxsrc/features/rewards/hooks/useRewards.tssrc/features/wallet/hooks/useWalletSync.tssrc/index.csssrc/index.tsxsrc/layouts/MainLayout.tsxsrc/layouts/components/PrimaryNavigation.tsxsrc/pages/ApiTesterPage.tsxsrc/pages/ClaimPage.tsxsrc/pages/HistoryPage.tsxsrc/pages/PreferencesPage.tsxsrc/shared/rewards/index.tssrc/store/wallet-state.tssrc/types/api.tssrc/types/claim.tssrc/types/profile.tssrc/types/wallet.tssrc/utils/profile-helpers.tsvite.config.ts
💤 Files with no reviewable changes (14)
- src/components/wallet/CardanoWalletConnector.tsx
- src/pages/HistoryPage.tsx
- README.md
- src/components/ui/navigation-menu-styles.ts
- src/shared/rewards/index.ts
- src/api/getTokens.ts
- src/features/rewards/components/WalletAddressForm.tsx
- functions/services/tokenPricing.ts
- src/features/rewards/api/getRewards.ts
- src/features/rewards/components/RewardCard.tsx
- src/features/preferences/components/ProfileForm.tsx
- src/components/ui/navigation-menu.tsx
- src/api/profileApi.ts
- src/features/rewards/hooks/useRewards.ts
🚧 Files skipped from review as they are similar to previous changes (19)
- src/features/rewards/components/RewardsEmptyState.tsx
- src/types/profile.ts
- src/components/common/ConnectWallet.tsx
- package.json
- src/features/claim/components/ClaimButton.tsx
- src/features/wallet/hooks/useWalletSync.ts
- src/types/api.ts
- functions/api/claim/submitTransaction.ts
- src/features/claim/hooks/useClaimFlow.ts
- functions/services/vmClient.ts
- src/index.tsx
- functions/api/claim/submit.ts
- src/index.css
- vite.config.ts
- src/app/providers.tsx
- src/layouts/components/PrimaryNavigation.tsx
- src/types/claim.ts
- src/pages/PreferencesPage.tsx
- src/layouts/MainLayout.tsx
Signed-off-by: JaeBrian <brian@vantasoft.com>
Signed-off-by: JaeBrian <brian@vantasoft.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
functions/api/profileData.ts (1)
11-24:⚠️ Potential issue | 🔴 CriticalVerify the signed payload before writing profiles.
src/features/profile/components/ProfileForm.tsxLines 22-35 andsrc/types/profile.tsLines 8-16 still sendsignature,key, andmessage, but this handler narrows the body to{ walletId, value }and writes straight to KV. That leaves profile updates unauthenticated, so anyone who knows awalletIdcan overwrite that profile unless you validate the signature againstbody.walletIdbeforeVM_WEB_PROFILES.put().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@functions/api/profileData.ts` around lines 11 - 24, The handler writes profile data to KV without authenticating the requester: before calling env.VM_WEB_PROFILES.put in functions/api/profileData.ts, extract and validate the signed payload fields (signature, key, message) that ProfileForm.tsx sends, verify the signature (e.g., with your existing or a new verifySignature utility) against body.walletId (and/or the message containing the walletId), and return an errorResponse if verification fails; only proceed to env.VM_WEB_PROFILES.put when the signature successfully verifies. Ensure you reference the incoming body variable and perform verification prior to the env.VM_WEB_PROFILES.put call.functions/api/getRewards.ts (1)
52-69:⚠️ Potential issue | 🟠 MajorTreat null VM responses as failures, not "no rewards".
Both null-response branches return
[], so VM/token fetch failures get rendered as a legitimate empty state. Throw here and letonRequestGet()map that back to an error response.Suggested change
- if (!rewardsResponse || !tokens) { - console.warn('getRewards: SDK returned null for', !rewardsResponse ? 'rewards' : 'tokens', { stakeAddress }); - return []; - } + if (!rewardsResponse) { + throw new Error(`getRewards: VM returned null rewards for ${stakeAddress}`); + } + if (!tokens) { + throw new Error('getRewards: VM returned null token metadata'); + } @@ - if (!tokens) { - console.warn('getRewards: token re-fetch returned null', { stakeAddress }); - return []; - } + if (!tokens) { + throw new Error('getRewards: token re-fetch returned null'); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@functions/api/getRewards.ts` around lines 52 - 69, The branches in getRewards that treat null SDK/VM responses as empty results should throw an error instead of returning [] so upstream onRequestGet() can translate it to an error response; specifically, replace the early returns when rewardsResponse or tokens is null (the initial if (!rewardsResponse || !tokens) block and the later token re-fetch branch after calling getTokensFromVM()) with thrown errors (include context like stakeAddress) so failures are propagated to onRequestGet() for proper error handling.src/pages/ClaimPage.tsx (1)
13-17:⚠️ Potential issue | 🟠 MajorDrive the rewards query from
walletReady.The page uses
walletReadyfor rendering, butuseRewards()still runs on any truthystakeAddress. Sincesrc/features/rewards/api/rewards.queries.tsLines 1-20 enables the query on!!stakeAddress, this can still fetch during wallet sync and surface empty/error states before the UI considers the wallet ready.Suggested change
export default function ClaimPage() { const { stakeAddress, connected } = useWalletStore(); - const { data: rewards, isLoading, error, refetch } = useRewards(stakeAddress); - const claimFlow = useClaimFlow(); - const walletReady = connected && !!stakeAddress; + const { data: rewards, isLoading, error, refetch } = useRewards( + walletReady ? stakeAddress : null + ); + const claimFlow = useClaimFlow(); const hasRewards = rewards && rewards.length > 0;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/ClaimPage.tsx` around lines 13 - 17, The rewards query is still enabled by truthy stakeAddress and can run during wallet sync; update the ClaimPage call to drive useRewards from walletReady instead of stakeAddress alone so it only runs when connected && !!stakeAddress. Concretely, change the invocation of useRewards in ClaimPage (where useRewards(stakeAddress) is called) to only pass the stakeAddress when walletReady is true or use the query's enabled option driven by walletReady (e.g., useRewards(stakeAddress, { enabled: walletReady })) so the query does not fire until walletReady is true.
🧹 Nitpick comments (4)
src/types/profile.ts (1)
1-16: Consider extracting sharedvaluetype to reduce duplication.
ProfileData.valueandSaveProfileRequest.valuehave identical shapes. Extracting a common type improves maintainability if the profile structure evolves.♻️ Proposed refactor
+export interface ProfileValue { + name: string; +} + export interface ProfileData { walletId: string; - value: { - name: string; - }; + value: ProfileValue; } export interface SaveProfileRequest { walletId: string; - value: { - name: string; - }; + value: ProfileValue; signature: string; key: string; message: string; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/types/profile.ts` around lines 1 - 16, Both ProfileData.value and SaveProfileRequest.value share the same shape; extract a shared type (e.g., ProfileValue) and replace the inline value definitions with that type in the ProfileData and SaveProfileRequest interfaces to remove duplication and centralize future changes; update any imports/usages referencing ProfileData.value or SaveProfileRequest.value accordingly so they use the new ProfileValue type.src/features/claim/hooks/useClaimFlow.ts (1)
43-58: Consider distinguishing wallet signature rejection from other errors.If the user rejects the wallet signature prompt (line 44), the generic error handler will show a technical error message. A user-friendly message for signature rejection would improve UX.
♻️ Proposed improvement
setState({ step: 'signing', unsignedTx: submitResult.unsignedTx }); - const signedTx = await wallet.signTx(submitResult.unsignedTx, false); + let signedTx: string; + try { + signedTx = await wallet.signTx(submitResult.unsignedTx, false); + } catch (signError) { + setState({ step: 'error', message: 'Transaction signing was cancelled' }); + return; + } setState({ step: 'submitting' });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/claim/hooks/useClaimFlow.ts` around lines 43 - 58, The catch block in useClaimFlow currently treats all errors the same; detect a user-rejected signature from the wallet (the call to wallet.signTx with submitResult.unsignedTx) and set a friendly message instead of the raw technical error. Update the catch to inspect the error (e.g., error.name, error.code, or error.message matching /reject|cancel|user rejected/i or known wallet codes like 4001) and call setState({ step: 'error', message: 'Signature rejected by user' }) for that case, otherwise keep the existing behavior of extracting error.message and calling setState({ step: 'error', message }). Ensure this logic is applied in the function handling submitResult → wallet.signTx → submitTxAsync flow so signature rejections are distinguished before invalidating queries or marking completed.src/features/claim/components/ClaimStatus.tsx (1)
30-38: Consider adding a retry/reset option for error state.The error state shows the failure message but doesn't provide an action to recover. Users must navigate away or refresh. Consider adding a "Try Again" button similar to the "Done" button in the completed state.
♻️ Proposed addition
if (state.step === 'error') { return ( - <FeedbackBanner - tone="error" - title="Claim failed" - message={state.message} - /> + <div className="space-y-2"> + <FeedbackBanner + tone="error" + title="Claim failed" + message={state.message} + /> + <button + onClick={onReset} + className="text-sm text-gray-400 underline hover:text-white" + > + Try again + </button> + </div> ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/claim/components/ClaimStatus.tsx` around lines 30 - 38, The error rendering for state.step === 'error' should offer a recovery action: update the ClaimStatus component to render the FeedbackBanner (or wrap it) with a "Try Again" button that triggers the same reset flow used by the completed state's "Done" action; add or reuse a handler prop (e.g., onReset, onRetry, or the existing resetClaim function) and wire the button's onClick to that handler so users can retry without leaving the page. Ensure the button label matches the completed state ("Done" / "Try Again") and that the handler name (onReset/onRetry/resetClaim) is referenced so reviewers can locate the change.src/pages/ApiTesterPage.tsx (1)
35-77: Consider gating this tester behind a non-production flag.As wired in
src/App.tsxLines 1-24 andsrc/layouts/components/PrimaryNavigation.tsxLines 8-13, this page is publicly reachable and linked from primary navigation. If this is meant as a QA/debug tool, I'd keep it behind a dev flag, env check, or admin-only route so production users can't browse raw API responses and fire these endpoints directly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/ApiTesterPage.tsx` around lines 35 - 77, This tester page (ApiTesterPage.tsx) is publicly reachable — gate it behind a non-production or admin-only flag: add a runtime check (e.g., process.env.NODE_ENV !== 'production' or a new ENABLE_API_TESTER env/feature flag) at the top of the ApiTesterPage component and return a 404/redirect/permission-denied UI unless the flag/admin check passes, and also conditionally hide the navigation link in PrimaryNavigation (so the page is not linked from the UI); keep the existing apis array, fetchApi usage, and apiClient calls unchanged but ensure stakeAddress access still respects the gating.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@functions/api/getRewards.ts`:
- Around line 26-40: The current toClaimableTokens() silently drops entries
whose assetId is missing from the tokens map; instead, keep all
Object.entries(amounts) and for any assetId not present in tokens return a
fallback ClaimableToken (e.g., decimals = 0, logo = '', ticker = assetId or '',
and retain amount calculation using fallback decimals) and preserve
premium/native logic (use isNativeToken(assetId) when premium). Update the
mapping in toClaimableTokens() (referencing amounts, tokens, isNativeToken,
premium) to stop filtering by tokens[assetId] and produce a predictable fallback
token object so the claim UI can still include the asset.
---
Duplicate comments:
In `@functions/api/getRewards.ts`:
- Around line 52-69: The branches in getRewards that treat null SDK/VM responses
as empty results should throw an error instead of returning [] so upstream
onRequestGet() can translate it to an error response; specifically, replace the
early returns when rewardsResponse or tokens is null (the initial if
(!rewardsResponse || !tokens) block and the later token re-fetch branch after
calling getTokensFromVM()) with thrown errors (include context like
stakeAddress) so failures are propagated to onRequestGet() for proper error
handling.
In `@functions/api/profileData.ts`:
- Around line 11-24: The handler writes profile data to KV without
authenticating the requester: before calling env.VM_WEB_PROFILES.put in
functions/api/profileData.ts, extract and validate the signed payload fields
(signature, key, message) that ProfileForm.tsx sends, verify the signature
(e.g., with your existing or a new verifySignature utility) against
body.walletId (and/or the message containing the walletId), and return an
errorResponse if verification fails; only proceed to env.VM_WEB_PROFILES.put
when the signature successfully verifies. Ensure you reference the incoming body
variable and perform verification prior to the env.VM_WEB_PROFILES.put call.
In `@src/pages/ClaimPage.tsx`:
- Around line 13-17: The rewards query is still enabled by truthy stakeAddress
and can run during wallet sync; update the ClaimPage call to drive useRewards
from walletReady instead of stakeAddress alone so it only runs when connected &&
!!stakeAddress. Concretely, change the invocation of useRewards in ClaimPage
(where useRewards(stakeAddress) is called) to only pass the stakeAddress when
walletReady is true or use the query's enabled option driven by walletReady
(e.g., useRewards(stakeAddress, { enabled: walletReady })) so the query does not
fire until walletReady is true.
---
Nitpick comments:
In `@src/features/claim/components/ClaimStatus.tsx`:
- Around line 30-38: The error rendering for state.step === 'error' should offer
a recovery action: update the ClaimStatus component to render the FeedbackBanner
(or wrap it) with a "Try Again" button that triggers the same reset flow used by
the completed state's "Done" action; add or reuse a handler prop (e.g., onReset,
onRetry, or the existing resetClaim function) and wire the button's onClick to
that handler so users can retry without leaving the page. Ensure the button
label matches the completed state ("Done" / "Try Again") and that the handler
name (onReset/onRetry/resetClaim) is referenced so reviewers can locate the
change.
In `@src/features/claim/hooks/useClaimFlow.ts`:
- Around line 43-58: The catch block in useClaimFlow currently treats all errors
the same; detect a user-rejected signature from the wallet (the call to
wallet.signTx with submitResult.unsignedTx) and set a friendly message instead
of the raw technical error. Update the catch to inspect the error (e.g.,
error.name, error.code, or error.message matching /reject|cancel|user rejected/i
or known wallet codes like 4001) and call setState({ step: 'error', message:
'Signature rejected by user' }) for that case, otherwise keep the existing
behavior of extracting error.message and calling setState({ step: 'error',
message }). Ensure this logic is applied in the function handling submitResult →
wallet.signTx → submitTxAsync flow so signature rejections are distinguished
before invalidating queries or marking completed.
In `@src/pages/ApiTesterPage.tsx`:
- Around line 35-77: This tester page (ApiTesterPage.tsx) is publicly reachable
— gate it behind a non-production or admin-only flag: add a runtime check (e.g.,
process.env.NODE_ENV !== 'production' or a new ENABLE_API_TESTER env/feature
flag) at the top of the ApiTesterPage component and return a
404/redirect/permission-denied UI unless the flag/admin check passes, and also
conditionally hide the navigation link in PrimaryNavigation (so the page is not
linked from the UI); keep the existing apis array, fetchApi usage, and apiClient
calls unchanged but ensure stakeAddress access still respects the gating.
In `@src/types/profile.ts`:
- Around line 1-16: Both ProfileData.value and SaveProfileRequest.value share
the same shape; extract a shared type (e.g., ProfileValue) and replace the
inline value definitions with that type in the ProfileData and
SaveProfileRequest interfaces to remove duplication and centralize future
changes; update any imports/usages referencing ProfileData.value or
SaveProfileRequest.value accordingly so they use the new ProfileValue type.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b0cb09ea-adea-49e5-928a-f66d3b3f56db
📒 Files selected for processing (22)
functions/api/claim/status.tsfunctions/api/claim/submit.tsfunctions/api/claim/submitTransaction.tsfunctions/api/claim/validate.tsfunctions/api/getRewards.tsfunctions/api/getSettings.tsfunctions/api/profileData.tsfunctions/api/sanitizeAddress.tsfunctions/services/vmClient.tsfunctions/types/env.tssrc/api/client.tssrc/api/profileApi.tssrc/features/claim/components/ClaimStatus.tsxsrc/features/claim/hooks/useClaimFlow.tssrc/features/profile/components/ProfileForm.tsxsrc/features/rewards/api/rewards.queries.tssrc/features/rewards/components/RewardsEmptyState.tsxsrc/pages/ApiTesterPage.tsxsrc/pages/ClaimPage.tsxsrc/store/wallet-state.tssrc/types/api.tssrc/types/profile.ts
💤 Files with no reviewable changes (1)
- src/api/profileApi.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- src/api/client.ts
- functions/api/claim/status.ts
- functions/api/getSettings.ts
- src/features/rewards/api/rewards.queries.ts
- functions/services/vmClient.ts
- src/features/profile/components/ProfileForm.tsx
- functions/api/claim/submitTransaction.ts
- functions/api/claim/submit.ts
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
2 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.
Summary by CodeRabbit
New Features
Refactoring
User-facing Changes