Skip to content

Conversation

@KEM-CONSOLATION
Copy link
Contributor

@KEM-CONSOLATION KEM-CONSOLATION commented Sep 8, 2025

Description

This PR completes the deployment and integration of the Noblocks mini app (“Noblocks Mini App”) on Farcaster.

-Configured and deployed the Noblocks project on Vercel.
-Created and set up packaster.json with correct frame metadata (name, icons, images, categories, etc.).
-Wrapped the app with MiniKit for Farcaster mini app compatibility.
-Configured domain association (accountAssociation) for app verification on Farcaster.
-Added redirect for /.well-known/farcaster.json to the hosted manifest URL.
-Verified the manifest and confirmed the app is now visible under “My MiniApps” on the Farcaster Developer portal.
-Ensured the app opens in-app on Farcaster instead of in an external browser.

image

Testing

Log into Farcaster and Search for "Noblocks", it opens up the Noblocks Miniapp.

  • This change adds test coverage for new/changed/fixed functionality

Checklist

  • I have added documentation and tests for new/changed functionality in this PR
  • All active GitHub checks for tests, formatting, and security are passing
  • The correct base branch is being used, if not main

Reference

Closes #307 #182

By submitting a PR, I agree to Paycrest's Contributor Code of Conduct and Contribution Guide.

Summary by CodeRabbit

Release Notes

  • New Features

    • Added Farcaster mini-app support with native wallet integration
    • Enabled dynamic Open Graph image generation for improved social sharing
    • Integrated Base App support for enhanced multi-wallet compatibility
    • Added OnchainKit integration for improved web3 experience
    • Implemented Farcaster manifest configuration for better app discovery
  • Improvements

    • Enhanced wallet detection in mini-app environments with improved fallback logic
    • Strengthened security headers and frame policies
    • Optimized wallet provider initialization with better error handling

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 8, 2025

Walkthrough

This PR introduces comprehensive Farcaster Mini App integration alongside enhanced Base App support. It adds Farcaster manifest files, SDK initialization logic, a new BaseAppContext for environment detection, wallet priority handling across multiple components, expanded configuration for app metadata, and new OnchainKit/Wagmi dependencies for blockchain interactions.

Changes

Cohort / File(s) Summary
Farcaster Mini App Manifests & Routes
public/farcaster-manifest.json, app/.well-known/farcaster.json/route.ts, vercel.json, app/head.tsx
New Farcaster manifest defining app metadata, window dimensions, icons, splash screens, and account association; API route handler returning dynamic manifest JSON with caching headers; Vercel redirect from /.well-known/farcaster.json to hosted manifest; Head component with Farcaster mini app meta tags and config-based branding.
Farcaster SDK Initialization & Early Ready
app/early-ready.tsx, app/layout.tsx
New client component using Farcaster SDK to signal app readiness with global flag; integrated at layout root before Script to ensure early execution; prevents re-initialization.
Base App & Farcaster Context
app/context/BaseAppContext.tsx, app/context/index.ts
New context provider detecting Base App and Farcaster environments via query params, user agent, and SDK inspection; exposes flags (isBaseApp, isFarcaster), baseAppWallet, readiness status, and SDK reference; includes hydration guards.
Configuration & Environment Variables
app/lib/config.ts, app/types.ts
Expanded config with 25+ new fields for app metadata, Farcaster/Base integration, Supabase URLs, OnchainKit project name; updated serverConfig to include supabaseRoleKey.
Supabase Client Lazy Initialization
app/lib/supabase.ts
Changed from eager to lazy client creation; now only instantiates when both supabaseUrl and supabaseRoleKey are available; replaced direct env var access with config-based values.
Utility Functions
app/lib/manifest-utils.ts
New exported utility filtering object properties, removing nulls/undefined, trimming strings, filtering arrays, preserving booleans.
Wallet Context Integration
app/context/InjectedWalletContext.tsx, app/utils.ts
Enhanced InjectedWalletContext with Farcaster SDK provider detection (fallback to window.ethereum), 100ms initialization delay, improved error handling (4001, "User rejected", general failures); updated shouldUseInjectedWallet detection logic to check Farcaster mini app environment.
OnchainKit Provider
providers/MiniKitProvider.tsx
New context provider wrapping OnchainKit's MiniKitProvider with cdpApiKey from config; includes Base Mainnet chain config (id 8453); skips wrapping in non-production if key missing.
Provider Tree Restructuring
app/providers.tsx, app/layout.tsx
Added BaseAppProvider as outer wrapper around existing ContextProviders; layout now includes Providers wrapper and EarlyReady component.
Transaction Components Wallet Integration
app/pages/TransactionForm.tsx, app/pages/TransactionPreview.tsx, app/pages/TransactionStatus.tsx
Updated all three components to use BaseAppContext for wallet priority (Base App > Farcaster > Injected > Smart); refactored balance selection logic to prioritize injected balance when Base App/Farcaster wallet present; added wallet type tracking for analytics.
AppLayout Client-Side Rendering
app/components/AppLayout.tsx
Added isMiniApp detection via useBaseApp; conditionally hides Navbar, NoticeBanner, PWAInstall, Footer in mini app context; uses mounted state to prevent hydration mismatches.
Blog Pages Mini App Redirect
app/blog/page.tsx, app/blog/[id]/page.tsx
Added searchParams handling; redirects to /?mini=true when mini==="true" query param present; executes before post fetching.
OG Image Generation Route
app/api/og/route.tsx
New Next.js route returning ImageResponse with fixed dimensions (1200x630); renders Noblocks branding, tagline, and website URL with blue gradient background.
UI & Formatting Updates
app/components/HomePage.tsx, app/components/MainPageContent.tsx, app/components/Navbar.tsx, next.config.mjs
Comment clarification in HomePage; indentation adjustments in MainPageContent; className formatting in Navbar; X-Frame-Options header value changed to empty string; Content-Security-Policy header added with frame-ancestors directive.
Dependencies
package.json
Added @farcaster/miniapp-sdk ^0.1.9, @coinbase/onchainkit ^0.38.19, @wagmi/chains ^1.8.0, @wagmi/core ^2.20.3, wagmi ^2.16.9.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant App as App (Next.js)
    participant EarlyReady
    participant FarcasterSDK as Farcaster SDK
    participant BaseAppContext
    participant WalletContext
    participant Provider as Provider Tree

    User->>App: Load app in Farcaster mini app
    App->>EarlyReady: Render at layout root (early)
    EarlyReady->>FarcasterSDK: Call sdk.ready()
    FarcasterSDK-->>EarlyReady: Signal ready or error
    EarlyReady->>EarlyReady: Set __farcasterMiniAppReady flag
    
    App->>BaseAppContext: Initialize provider
    BaseAppContext->>FarcasterSDK: isInMiniApp(), getContext()
    FarcasterSDK-->>BaseAppContext: Return environment info
    BaseAppContext->>BaseAppContext: Detect isBaseApp, isFarcaster, get baseAppWallet
    
    App->>WalletContext: Initialize InjectedWalletContext
    WalletContext->>FarcasterSDK: Attempt sdk.wallet.getEthereumProvider()
    alt Farcaster SDK Provider Available
        FarcasterSDK-->>WalletContext: Return provider
    else Fallback to window.ethereum
        WalletContext->>WalletContext: Use window.ethereum
    end
    WalletContext->>WalletContext: Delay 100ms, request accounts
    
    App->>Provider: Render wrapped components
    Provider->>Provider: Wallet priority: BaseApp > Farcaster > Injected > Smart
    Provider-->>User: App ready with active wallet
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

  • Areas requiring extra attention:
    • BaseAppContext initialization logic (app/context/BaseAppContext.tsx): Complex environment detection via multiple signals (params, userAgent, SDK calls); requires validation of hydration guards and SDK state consistency.
    • InjectedWalletContext provider resolution (app/context/InjectedWalletContext.tsx): Farcaster SDK provider fallback, timing delays (100ms/200ms), error handling for 4001/"User rejected" codes; verify error paths and cleanup.
    • Wallet priority & balance logic across TransactionForm.tsx, TransactionPreview.tsx, TransactionStatus.tsx: Ensure consistent wallet selection (BaseApp > Injected > Smart) and verify balance calculations don't regress for non-mini-app scenarios.
    • Configuration expansion (app/lib/config.ts, app/types.ts): 25+ new fields; verify all environment variables are correctly mapped and have sensible defaults.
    • Farcaster manifest & accountAssociation (public/farcaster-manifest.json, route handler): Verify base64 encoding of account association fields; note potential URL typos (https://https://) in splash_screen URLs.
    • Lazy Supabase client (app/lib/supabase.ts): Verify supabaseAdmin is only accessed when both URL and key are present; check for runtime errors if accessed in non-production without key.

Possibly related issues

Possibly related PRs

Suggested labels

enhancement

Suggested reviewers

  • chibie
  • KEM-CONSOLATION

Poem

🐰 Through Farcaster frames we now leap,
With mini apps in gardens deep,
Wallets dancing—Base, Farcaster, and more,
Configuration keys opened every door,
Early-ready signals in the dawn,
Noblocks hops on, integration's reborn! 🚀

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 26.67% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Description check ❓ Inconclusive The PR description covers the main objective (Farcaster Mini App deployment) and testing instructions, but lacks details on implementation specifics, breaking changes, alternatives considered, and environment details. Add more implementation details: explain key code changes, any breaking changes or API modifications, why specific approaches were chosen, and specify the development environment (Node/browser versions, etc.).
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: deploying Noblocks as a Farcaster Mini App using MiniKit. It directly aligns with the changeset's primary objective.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 14

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (6)
app/api/v1/transactions/[id]/route.ts (1)

22-24: Validate and sanitize request body; avoid writing undefined/null and enforce allowed status values.

Currently txHash/timeSpent/status are unchecked and may upsert nulls/invalid states. Validate and only update defined fields (and trim strings). You can reuse withValidProperties.

Apply this diff:

 import { NextRequest, NextResponse } from "next/server";
 import { supabaseAdmin } from "@/app/lib/supabase";
 import { withRateLimit } from "@/app/lib/rate-limit";
+import { withValidProperties } from "@/app/lib/manifest-utils";

@@
-      const body = await request.json();
-      const { txHash, timeSpent, status } = body;
+      const body = await request.json();
+      const { txHash, timeSpent, status } = body as {
+        txHash?: string;
+        timeSpent?: number;
+        status?: "pending" | "confirmed" | "failed";
+      };
+
+      // Basic validation
+      if (txHash && !/^0x[0-9a-fA-F]{64}$/.test(txHash)) {
+        return NextResponse.json({ success: false, error: "Invalid txHash" }, { status: 400 });
+      }
+      if (typeof timeSpent !== "undefined" && (typeof timeSpent !== "number" || timeSpent < 0)) {
+        return NextResponse.json({ success: false, error: "Invalid timeSpent" }, { status: 400 });
+      }
+      if (typeof status !== "undefined" && !["pending", "confirmed", "failed"].includes(status)) {
+        return NextResponse.json({ success: false, error: "Invalid status" }, { status: 400 });
+      }
+
+      const updates = withValidProperties({
+        tx_hash: txHash?.trim(),
+        time_spent: timeSpent,
+        status,
+        updated_at: new Date().toISOString(),
+      });

@@
-      const { data, error } = await supabaseAdmin
+      const { data, error } = await supabaseAdmin
         .from("transactions")
-        .update({
-          tx_hash: txHash,
-          time_spent: timeSpent,
-          status: status,
-          updated_at: new Date().toISOString(),
-        })
+        .update(updates)
         .eq("id", id)
         .eq("wallet_address", walletAddress)
         .select()
         .single();

Also applies to: 41-53

app/utils.ts (3)

559-607: Farcaster Mini App detection wrongly gated on window.ethereum; tighten heuristics and allow SDK wallet.

In many Farcaster mini-app contexts, a provider may exist via SDK/OnchainKit without populating window.ethereum. Current logic returns false in that case and may disable the injected flow. Also, the generic iframe check can cause false positives.

Refactor to:

  • Detect any viable provider (window.ethereum OR SDK wallet).
  • Remove the broad iframe heuristic.
  • Accept more values for the injected flag.

Apply:

 export function shouldUseInjectedWallet(
   searchParams: URLSearchParams,
 ): boolean {
   const injectedParam = searchParams.get("injected");
-  const hasEthereum = typeof window !== "undefined" && (window as any).ethereum;
+  const ethereumProvider =
+    typeof window !== "undefined" ? (window as any).ethereum : undefined;

   // Detect if running inside Farcaster Mini App environment
   const isLikelyFarcasterMiniApp = (() => {
     if (typeof window === "undefined") return false;
     try {
       const w = window as any;
       const ua = navigator.userAgent || "";
-      const ref = document.referrer || "";
-      const url = window.location.href || "";
+      const ref = document.referrer || "";
+      const url = window.location.href || "";

-      // More aggressive detection for Farcaster Mini App
-      return (
-        Boolean(w.__farcasterMiniAppReady) ||
-        Boolean(w.farcaster) ||
-        Boolean(w.warpcast) ||
-        Boolean(w.farcasterMiniApp) ||
-        /Farcaster|Warpcast/i.test(ua) ||
-        /warpcast\.com/i.test(ref) ||
-        /warpcast\.com/i.test(url) ||
-        /farcaster\.xyz/i.test(url) ||
-        /farcaster\.xyz/i.test(ref) ||
-        // Check if we're in an iframe or embedded context
-        window !== window.top ||
-        // Check for Farcaster-specific query parameters
-        searchParams.has("farcaster") ||
-        searchParams.has("warpcast") ||
-        searchParams.has("miniapp") ||
-        // Check for Farcaster SDK availability
-        (typeof w !== "undefined" && w.sdk && w.sdk.wallet)
-      );
+      // Heuristics focused on Farcaster/Warpcast contexts and SDK presence
+      return (
+        Boolean(w.__farcasterMiniAppReady) ||
+        Boolean(w.farcaster) ||
+        Boolean(w.warpcast) ||
+        Boolean(w.farcasterMiniApp) ||
+        /Farcaster|Warpcast/i.test(ua) ||
+        /(warpcast\.com|farcaster\.xyz)/i.test(ref + url) ||
+        searchParams.has("farcaster") ||
+        searchParams.has("warpcast") ||
+        searchParams.has("miniapp") ||
+        (w?.sdk && w?.sdk?.wallet)
+      );
     } catch {
       return false;
     }
   })();

-  // Treat Farcaster Mini App as injected flow if a provider exists
-  if (isLikelyFarcasterMiniApp && hasEthereum) return true;
+  const hasFarcasterSdkWallet =
+    typeof window !== "undefined" &&
+    !!(window as any)?.sdk &&
+    !!(window as any)?.sdk?.wallet;
+
+  // Treat Farcaster Mini App as injected flow if any provider exists
+  if (isLikelyFarcasterMiniApp && (ethereumProvider || hasFarcasterSdkWallet)) {
+    return true;
+  }
 
-  return Boolean(injectedParam === "true" && hasEthereum);
+  const injectedFlag =
+    (injectedParam || "").toLowerCase() === "true" ||
+    injectedParam === "1" ||
+    (injectedParam || "").toLowerCase() === "yes";
+  return Boolean(injectedFlag && (ethereumProvider || hasFarcasterSdkWallet));
 }

335-378: Ensure getNetworkTokens always returns an array and set cache timestamp at completion.

The function promises Token[] but can return undefined on fallback. Also, lastTokenFetch should reflect completion time.

Apply:

-        lastTokenFetch = now;
+        lastTokenFetch = Date.now();
 ...
-    // Return fallback tokens if API fails
-    return FALLBACK_TOKENS[network];
+    // Return fallback tokens if API fails
+    return FALLBACK_TOKENS[network] || [];

702-754: setSelectedNetwork/onSuccess fire before the switch completes; await the promise.

UI can reflect a switched network even if wallet_switchEthereumChain fails.

Apply:

-    try {
-      toast.promise(
-        (async () => {
+    try {
+      const switchPromise = (async () => {
           try {
             // First try to switch to the network
             await (window.ethereum as any).request({
               method: "wallet_switchEthereumChain",
               params: [{ chainId }],
             });
           } catch (switchError: any) {
             // This error code indicates that the chain has not been added to MetaMask.
             if (switchError.code === 4902) {
               await (window.ethereum as any).request({
                 method: "wallet_addEthereumChain",
                 params: [getAddChainParameters(network)],
               });
             } else {
               throw switchError;
             }
           }
-        })(),
-        {
+      })();
+      await toast.promise(switchPromise, {
           loading: `Switching to ${network.chain.name}...`,
           success: `Successfully switched to ${network.chain.name}`,
           error: (err) => err.message,
-        },
-      );
+        });
+      await switchPromise;
 
       setSelectedNetwork(network);
       onSuccess();
app/types.ts (2)

249-280: Fix typos in Config type and update corresponding usages in config loader

  • In app/types.ts, rename the misspelled keys:
    • appSubstitleappSubtitle
    • appSpashImageappSplashImage
    • farcasterSignatuurefarcasterSignature
  • In app/lib/config.ts (around lines 19–27), update the same three property names to match the type:
    • appSubstitleappSubtitle
    • appSpashImageappSplashImage
    • farcasterSignatuurefarcasterSignature
  • Run
    rg -nP 'appSubstitle|appSpashImage|farcasterSignatuure'
    
    to confirm no remaining references.

249-280: Remove Supabase service_role key from the client bundle

supabaseRoleKey in app/lib/config.ts (process.env.SUPABASE_SERVICE_ROLE_KEY) is exported to the frontend — the service_role key bypasses RLS and grants full database access. Replace with the anon/public key (NEXT_PUBLIC_SUPABASE_ANON_KEY) in client code, and load the service_role key only in server‐only contexts (API routes or server components). Split into PublicConfig (client) and ServerConfig (backend) and update app/lib/config.ts accordingly.

🧹 Nitpick comments (30)
vercel.json (1)

4-6: Extract hosted-manifest ID to an environment variable for redirects
Move this redirect into next.config.js (or handle it in an app route) so the destination comes from process.env and you avoid code changes whenever the manifest ID rotates. Note that in vercel.json, "permanent": false already issues an HTTP 307 redirect, so you don’t need a separate statusCode.

app/api/v1/transactions/[id]/route.ts (1)

55-66: Surface domain errors clearly to clients (optional).

Optionally map Supabase errors (e.g., unique violation on tx_hash) to 409/422 instead of generic 500 for better client UX.

package.json (1)

16-25: Add a type-check script to CI.

Helpful for catching SDK integration regressions.

   "scripts": {
     "dev": "next dev --turbopack",
     "build": "next build",
     "start": "next start",
     "lint": "next lint",
+    "typecheck": "tsc --noEmit",
     "sanity:dev": "sanity dev --studio studio",
app/api/og/route.tsx (2)

3-4: Run on the Edge and add caching headers for the OG image.

Edge runtime and cache headers reduce cold starts and recompute.

 export async function GET() {
-  return new ImageResponse(
+  const res = new ImageResponse(
@@
-    {
-      width: 1200,
-      height: 630,
-    },
+    { width: 1200, height: 630 },
   );
+  res.headers.set("Cache-Control", "public, s-maxage=86400, stale-while-revalidate=604800");
+  return res;
 }
+
+export const runtime = "edge";

20-44: Consider pulling copy from config to avoid divergence.

If you already added appOgTitle/appOgDescription, render them here to keep OG consistent with metadata.

app/early-ready.tsx (2)

12-13: Guard SDK call to avoid throwing outside Farcaster.

Be defensive in non-miniapp contexts and older SDKs.

-      // Notify Farcaster SDK
-      sdk.actions.ready();
+      // Notify Farcaster SDK
+      try {
+        // @ts-expect-error: older SDKs may not type optional chaining
+        sdk?.actions?.ready?.();
+      } catch { /* no-op */ }

23-25: Limit postMessage target if you can infer origin (optional).

Using "*" is fine for readiness pings, but if the SDK exposes the parent origin, prefer that for stricter security.

app/lib/manifest-utils.ts (1)

1-22: Add basic unit tests for edge cases (empty strings, arrays with blanks, false booleans).

This helper is easy to regress; a small test will lock behavior.

public/farcaster-manifest.json (3)

12-18: Parametrize asset URLs (icon/splash) to avoid drift across environments.

Tie these to config so staging/production assets don’t get out of sync with the app.url, and to keep a single source of truth with the hosted manifest endpoint you added.

-  "icon": {
-    "light": "https://noblocks-ialc.vercel.app/icons/android-chrome-192x192.png",
-    "dark": "https://noblocks-ialc.vercel.app/icons/android-chrome-192x192.png"
-  },
+  "icon": {
+    "light": "<from config>",
+    "dark": "<from config>"
+  },
-  "splash_screen": {
-    "light": "https://noblocks-ialc.vercel.app/screenshots/desktop-wide.png",
-    "dark": "https://noblocks-ialc.vercel.app/screenshots/desktop-wide.png"
-  },
+  "splash_screen": {
+    "light": "<from config>",
+    "dark": "<from config>"
+  },

23-24: Confirm versioning strategy.

If this is the first release of the mini app, 10.0.0 looks accidental. Consider aligning manifest version with your app/package versioning.


30-34: Ensure domain association triple matches the deployed domain.

Verify that:

  • payload.domain equals the exact host users load (no www vs apex mismatch),
  • signature matches the header/payload, and
  • header key is the intended signer.

If you’ll rotate these, prefer pulling from env at build time to avoid frequent commits.

Would you like a small server route that emits this manifest from env and validates base64 structure before serving?

next.config.mjs (3)

23-27: Constrain frame-ancestors to the minimal set and consider reporting.

The CSP is correct in principle. Consider:

  • Keeping only the domains truly needed (least privilege).
  • Adding a report-to/report-uri for visibility on violations.
-          value:
-            "frame-ancestors 'self' https://warpcast.com https://*.warpcast.com https://*.farcaster.xyz https://farcaster.xyz https://auth.privy.io https://client.warpcast.com;",
+          value:
+            "frame-ancestors 'self' https://warpcast.com https://*.warpcast.com https://*.farcaster.xyz; report-to=csp-endpoint;",

39-49: Service worker headers look fine; consider text/javascript for broadest compatibility.

application/javascript usually works, but text/javascript is the de-facto for SW scripts.

-          value: "application/javascript; charset=utf-8",
+          value: "text/javascript; charset=utf-8",

29-35: X-XSS-Protection is obsolete.

Modern browsers ignore it. Safe to remove to reduce header noise.

-        {
-          key: "X-XSS-Protection",
-          value: "1; mode=block",
-        },
app/context/InjectedWalletContext.tsx (6)

14-14: Lazy-load the Farcaster SDK to avoid bundling it for non-miniapp users.

Dynamically import the SDK only when you’ve determined you should use the injected wallet. This reduces bundle size and avoids initializing the SDK unnecessarily.

-import { sdk } from "@farcaster/miniapp-sdk";
+          // Inside the shouldUse branch:
+          const { sdk } = await import("@farcaster/miniapp-sdk");

44-62: Provider selection fallback is solid. Minor: add a timeout around SDK provider fetch.

If sdk.wallet.getEthereumProvider() hangs, you’ll stall init. Wrap it with a short timeout and fall back to window.ethereum deterministically.

-            const farcasterProvider = await sdk.wallet.getEthereumProvider();
+            const farcasterProvider = await Promise.race([
+              sdk.wallet.getEthereumProvider(),
+              new Promise<null>((resolve) => setTimeout(() => resolve(null), 800)),
+            ]);

71-77: Avoid arbitrary sleeps; prefer readiness signals.

Instead of a fixed 100ms delay, await an explicit provider-ready event if available (or the timeout-wrapped getEthereumProvider above). Fixed sleeps can be flaky under load.


74-83: Type the provider and pass the client back to context.

Use EIP1193Provider typing and consider exposing the viem client in context for downstream code.

-import { createWalletClient, custom } from "viem";
+import { createWalletClient, custom, type EIP1193Provider } from "viem";
@@
-  const [injectedProvider, setInjectedProvider] = useState<any | null>(null);
+  const [injectedProvider, setInjectedProvider] = useState<EIP1193Provider | null>(null);
+  const [client, setClient] = useState<ReturnType<typeof createWalletClient> | null>(null);
@@
-          client = createWalletClient({
+          client = createWalletClient({
             transport: custom(ethereumProvider as any),
           });
+          setClient(client);

79-90: Listen for provider events to keep state in sync.

Handle accountsChanged, chainChanged, and disconnect, and clean up listeners on unmount.

+          const onAccountsChanged = (accounts: string[]) => {
+            setInjectedAddress(accounts?.[0] ?? null);
+            setInjectedReady(Boolean(accounts?.length));
+          };
+          const onDisconnect = () => {
+            setInjectedReady(false);
+            setInjectedAddress(null);
+          };
+          (ethereumProvider as any).on?.("accountsChanged", onAccountsChanged);
+          (ethereumProvider as any).on?.("disconnect", onDisconnect);
+          // In cleanup (use the cancelled guard block above), also:
+          // (ethereumProvider as any).removeListener?.("accountsChanged", onAccountsChanged);
+          // (ethereumProvider as any).removeListener?.("disconnect", onDisconnect);

94-108: Error handling duplication.

You already handle code 4001; the string “User rejected” branch largely duplicates behavior. Consider collapsing into a single branch keyed by 4001 or by a helper predicate.

app/utils.ts (1)

397-409: Avoid precision/overflow by using formatUnits instead of Number(BigInt).

Apply:

-        const balanceInWei = await client.readContract({
+        const balanceInWei = await client.readContract({
           address: token.address as `0x${string}`,
           abi: erc20Abi,
           functionName: "balanceOf",
           args: [address as `0x${string}`],
         });
-        const balance = Number(balanceInWei) / Math.pow(10, token.decimals);
+        const balance = parseFloat(formatUnits(balanceInWei as bigint, token.decimals));

And update imports:

- import { erc20Abi } from "viem";
+ import { erc20Abi, formatUnits } from "viem";
app/types.ts (1)

249-280: Stronger typing for env-like fields.

nodeEnv and noIndex are strings; consider:

  • nodeEnv: "development" | "production" | "test"
  • noIndex: boolean

Example:

-  nodeEnv: string;
-  noIndex: string;
+  nodeEnv: "development" | "production" | "test";
+  noIndex: boolean;

Gate conversion in app/lib/config.ts when reading process.env.

app/head.tsx (2)

6-11: Guard against empty appUrl and consider adding SEO/OG tags from config.

If appUrl is empty, replace() will throw. Also, you already have OG fields in config; adding them here improves share previews.

Apply:

-  const baseUrl = appUrl.replace(/\/$/, "");
+  const safeUrl = appUrl || "https://noblocks.xyz";
+  const baseUrl = safeUrl.replace(/\/$/, "");

Optionally add:

+      <meta property="og:title" content={config.appOgTitle} />
+      <meta property="og:description" content={config.appOgDescription} />
+      <meta property="og:image" content={config.publicAppOGImage} />
+      <meta name="twitter:card" content="summary_large_image" />

20-24: Remove legacy Frame meta tag
fc:frame:button:1:target is part of the old Frame spec and isn’t required for Mini Apps; Mini Apps use only the fc:miniapp embed meta (containing version, imageUrl, and button.title + button.action) (miniapps.farcaster.xyz, docs.farcaster.xyz)

providers/MiniKitProvider.tsx (2)

7-23: Prefer official chain constants over hand-rolled objects.

Import the Base chain from viem to reduce drift and maintenance.

Apply this diff:

+import { base } from "viem/chains";
 
-const baseChain = {
-  id: 8453,
-  name: "Base Mainnet",
-  network: "base",
-  nativeCurrency: { name: "ETH", symbol: "ETH", decimals: 18 },
-  rpcUrls: {
-    default: { http: ["https://mainnet.base.org"] },
-    public: { http: ["https://mainnet.base.org"] },
-  },
-  blockExplorers: {
-    default: { name: "BaseScan", url: "https://basescan.org" },
-    baseScan: { name: "BaseScan", url: "https://basescan.org" },
-  },
-  contracts: {},
-  testnet: false,
-  blockTime: 12,
-};
+// use viem's canonical Base chain config
-    <MiniKitProvider apiKey={cdpApiKey} chain={baseChain}>
+    <MiniKitProvider apiKey={cdpApiKey} chain={base}>

Also applies to: 38-38


28-35: Strengthen the missing-key path.

Consider surfacing an explicit error in production (e.g., console.error or report to telemetry) so silent degradation doesn’t mask misconfigurations.

   if (!cdpApiKey) {
-    if (nodeEnv !== "production") {
-      console.warn(
-        "MiniKitContextProvider: NEXT_PUBLIC_CDP_API_KEY is not set. Rendering without MiniKitProvider.",
-      );
-    }
+    const msg =
+      "MiniKitContextProvider: NEXT_PUBLIC_CDP_API_KEY is not set. Rendering without MiniKitProvider.";
+    nodeEnv !== "production" ? console.warn(msg) : console.error(msg);
     return <>{children}</>;
   }

If you expect MiniKit features in production, please confirm that NEXT_PUBLIC_CDP_API_KEY is set in Vercel env for the production project.

app/lib/supabase.ts (1)

6-16: Optional: expose a getter that throws with a clear message when misconfigured.

Avoid passing null around; fail fast where used.

export function getSupabaseAdmin(): SupabaseClient {
  if (!supabaseAdmin) {
    throw new Error("Supabase admin client is not configured (supabaseUrl/supabaseRoleKey missing).");
  }
  return supabaseAdmin;
}
app/lib/config.ts (3)

20-20: Give publicUrl a sensible default to avoid empty strings.

Align with appUrl’s behavior.

-  publicUrl: process.env.NEXT_PUBLIC_URL || "",
+  publicUrl: process.env.NEXT_PUBLIC_URL || "https://noblocks.xyz",

45-45: Treat noIndex as a boolean, not a string.

Prevents truthy string bugs in meta/robots logic.

-  noIndex: process.env.NEXT_PUBLIC_NOINDEX || "",
+  noIndex: process.env.NEXT_PUBLIC_NOINDEX === "true",

Ensure the Config type reflects boolean.


14-14: Minor: appUrl/publicUrl duplication.

You’re sourcing both from NEXT_PUBLIC_URL; consider documenting their intended use or consolidating to reduce confusion.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 14c9ee3 and e9453ef.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (18)
  • app/api/og/route.tsx (1 hunks)
  • app/api/v1/transactions/[id]/route.ts (1 hunks)
  • app/context/InjectedWalletContext.tsx (3 hunks)
  • app/early-ready.tsx (1 hunks)
  • app/head.tsx (1 hunks)
  • app/layout.tsx (3 hunks)
  • app/lib/config.ts (1 hunks)
  • app/lib/manifest-utils.ts (1 hunks)
  • app/lib/supabase.ts (1 hunks)
  • app/types.ts (2 hunks)
  • app/utils.ts (2 hunks)
  • next.config.mjs (2 hunks)
  • package.json (3 hunks)
  • providers/MiniKitProvider.tsx (1 hunks)
  • public/.well-known/farcaster.json (1 hunks)
  • public/.well-known/route.ts (1 hunks)
  • public/farcaster-manifest.json (1 hunks)
  • vercel.json (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-23T07:30:23.720Z
Learnt from: jeremy0x
PR: paycrest/noblocks#177
File: app/utils.ts:179-196
Timestamp: 2025-07-23T07:30:23.720Z
Learning: The `normalizeNetworkName` function in app/utils.ts has been updated to be dynamic and scalable, converting any network identifier to sentence case with hyphens replaced by spaces and proper handling of acronyms like "BNB".

Applied to files:

  • app/utils.ts
🧬 Code graph analysis (2)
app/layout.tsx (2)
app/early-ready.tsx (1)
  • EarlyReady (5-34)
providers/MiniKitProvider.tsx (1)
  • MiniKitContextProvider (25-42)
app/lib/supabase.ts (1)
app/lib/sanity-client.ts (1)
  • client (7-10)
🔇 Additional comments (6)
app/api/v1/transactions/[id]/route.ts (2)

9-9: LGTM: fix to sync destructure of params.

This corrects the unnecessary await.


11-21: Normalize wallet addresses consistently at write-time and query-time.

You lower-case the header value; ensure wallet_address is stored lower-cased in DB (or use ilike) to avoid mismatches. Add a unit/integration test covering mixed-case inputs.

package.json (1)

27-28: Verify peerDependency warnings on install
Run pnpm install --reporter=append-only and confirm there are no peer warnings for Next.js 15, React 19, wagmi/@wagmi/core, @wagmi/chains, viem, @coinbase/onchainkit or @farcaster/miniapp-sdk. Applies to lines 27–28, 43–45 and 71–73.

public/.well-known/farcaster.json (1)

30-34: Ensure accountAssociation domain & signature consistency
File: public/.well-known/farcaster.json (lines 30–34)
Decoded payload domain: noblocks-ialc.vercel.app. Confirm this matches your live deployment domain—if you’ve switched domains, update the payload accordingly and regenerate its signature.

app/layout.tsx (2)

17-18: Imports for EarlyReady and MiniKit provider look good.

No issues. These are correctly isolated client components used from a server layout.


247-268: Provider composition and EarlyReady placement LGTM.

Early-ready signal is emitted early in the body, and MiniKit wraps Providers without altering layout order. Looks correct for in-app launch.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

♻️ Duplicate comments (3)
app/context/InjectedWalletContext.tsx (1)

79-90: Guard state updates if unmounted; avoid setState after unmount

Use the existing cancelled flag to gate updates inside the async flow.

-          if (address) {
-            setInjectedProvider(ethereumProvider);
-            setInjectedAddress(address);
-            setInjectedReady(true);
-            console.log("Successfully connected to wallet:", address);
-          } else {
-            console.warn("No address returned from injected wallet.");
-            toast.error(
-              "Couldn't connect to your wallet. Please check your wallet connection.",
-            );
-            setIsInjectedWallet(false);
-          }
+          if (cancelled) return;
+          if (address) {
+            setInjectedProvider(ethereumProvider);
+            setInjectedAddress(address);
+            setInjectedReady(true);
+            console.log("Successfully connected to wallet:", address);
+          } else {
+            console.warn("No address returned from injected wallet.");
+            toast.error("Couldn't connect to your wallet. Please check your wallet connection.");
+            setIsInjectedWallet(false);
+          }
app/lib/config.ts (1)

81-81: Good move: supabaseRoleKey isolated to serverConfig.

This addresses the earlier concern about leaking the service role key via shared config.

app/lib/supabase.ts (1)

1-3: Mark server-only and add typing to avoid accidental client import and any.

Prevents bundling in client and improves type safety.

-import { createClient } from "@supabase/supabase-js";
+import "server-only";
+import { createClient, type SupabaseClient } from "@supabase/supabase-js";
 import config from "./config";
 import { serverConfig } from "./config";
@@
-let client: any = null;
+let client: SupabaseClient | null = null;
@@
-export const supabaseAdmin: any = client;
+export const supabaseAdmin: SupabaseClient | null = client;

Also applies to: 10-10, 20-20

🧹 Nitpick comments (6)
app/context/InjectedWalletContext.tsx (2)

67-69: Simplify client creation; avoid unused mutable var

-          let client;
-          ...
-          client = createWalletClient({
+          const client = createWalletClient({
             transport: custom(ethereumProvider as any),
           });

Also applies to: 47-48


44-66: Optional: type the provider and reduce console noise in production

Type ethereumProvider as EIP1193Provider from viem and gate console logs behind NODE_ENV !== "production".

Want a quick diff to add the type import and a tiny logger?

app/types.ts (2)

250-251: Tighten types for env-driven fields (prevents misuse).

Prefer precise types for better safety.

-  nodeEnv: string;
-  noIndex: string;
+  nodeEnv: "development" | "test" | "production";
+  noIndex: boolean;

Companion change suggested in app/lib/config.ts Line 44 to parse the env into a boolean.


278-279: Align cdpApiKey optionality with actual config usage.

config always supplies a string (possibly empty). Make it required to simplify consumers.

-  cdpApiKey?: string;
+  cdpApiKey: string;
app/lib/config.ts (2)

19-19: Use a consistent fallback for publicUrl (matches appUrl).

Avoid empty string; mirror appUrl’s default.

-  publicUrl: process.env.NEXT_PUBLIC_URL || "",
+  publicUrl: process.env.NEXT_PUBLIC_URL || "https://noblocks.xyz",

44-45: Parse NOINDEX env to boolean (and align type).

If you adopt the boolean type in app/types.ts, parse here.

-  noIndex: process.env.NEXT_PUBLIC_NOINDEX || "",
+  noIndex: /^(1|true|yes)$/i.test(process.env.NEXT_PUBLIC_NOINDEX || ""),
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e9453ef and 659b567.

📒 Files selected for processing (9)
  • app/.well-known/farcaster.json (1 hunks)
  • app/.well-known/route.ts (1 hunks)
  • app/context/InjectedWalletContext.tsx (3 hunks)
  • app/layout.tsx (3 hunks)
  • app/lib/config.ts (2 hunks)
  • app/lib/manifest-utils.ts (1 hunks)
  • app/lib/supabase.ts (1 hunks)
  • app/types.ts (2 hunks)
  • public/farcaster-manifest.json (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • app/lib/manifest-utils.ts
  • public/farcaster-manifest.json
  • app/layout.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
app/lib/supabase.ts (2)
app/lib/config.ts (1)
  • serverConfig (76-82)
app/lib/sanity-client.ts (1)
  • client (7-10)
🔇 Additional comments (3)
app/.well-known/farcaster.json (2)

12-18: Icon vs splash_screen paths inconsistent; likely 404 risk

The icon points to /icons/... while splash_screen points to root /. Ensure both URLs exist and meet recommended sizes.

-    "light": "https://noblocks.xyz/android-chrome-192x192.png",
-    "dark": "https://noblocks.xyz/android-chrome-192x192.png"
+    "light": "https://noblocks.xyz/icons/android-chrome-192x192.png",
+    "dark": "https://noblocks.xyz/icons/android-chrome-192x192.png"

23-26: Manifest schema fields—confirm spec compliance and asset sizes

primaryCategory/tags vs categories have changed across spec versions; also verify required icon/splash dimensions.

If you want, I’ll add a small checker that validates keys and image dimensions via HEAD/Content-Type/Size.

app/context/InjectedWalletContext.tsx (1)

121-128: Nice: timeout + cancellation cleanup implemented

Comment on lines 1 to 10
export async function GET() {
const hostedManifestUrl = process.env.NEXT_PUBLIC_FC_HOSTED_MANIFEST_URL!;
return new Response(null, {
status: 307,
headers: {
Location: hostedManifestUrl,
"Cache-Control": "public, s-maxage=300, stale-while-revalidate=86400",
},
});
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Handle missing/invalid hosted manifest URL; improve caching

Non-null assertion on env can yield an invalid Location at runtime. Add a guard and validate as a URL. Also add max-age for browsers (s-maxage applies to CDNs only).

 export async function GET() {
-  const hostedManifestUrl = process.env.NEXT_PUBLIC_FC_HOSTED_MANIFEST_URL!;
-  return new Response(null, {
+  const hostedManifestUrl = process.env.NEXT_PUBLIC_FC_HOSTED_MANIFEST_URL;
+  if (!hostedManifestUrl) {
+    return new Response("Missing NEXT_PUBLIC_FC_HOSTED_MANIFEST_URL", { status: 500 });
+  }
+  try {
+    new URL(hostedManifestUrl);
+  } catch {
+    return new Response("Invalid NEXT_PUBLIC_FC_HOSTED_MANIFEST_URL", { status: 500 });
+  }
+  return new Response(null, {
     status: 307,
     headers: {
       Location: hostedManifestUrl,
-      "Cache-Control": "public, s-maxage=300, stale-while-revalidate=86400",
+      "Cache-Control": "public, max-age=300, s-maxage=300, stale-while-revalidate=86400",
     },
   });
 }
+
+export async function HEAD() {
+  return GET();
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export async function GET() {
const hostedManifestUrl = process.env.NEXT_PUBLIC_FC_HOSTED_MANIFEST_URL!;
return new Response(null, {
status: 307,
headers: {
Location: hostedManifestUrl,
"Cache-Control": "public, s-maxage=300, stale-while-revalidate=86400",
},
});
}
export async function GET() {
const hostedManifestUrl = process.env.NEXT_PUBLIC_FC_HOSTED_MANIFEST_URL;
if (!hostedManifestUrl) {
return new Response("Missing NEXT_PUBLIC_FC_HOSTED_MANIFEST_URL", { status: 500 });
}
try {
new URL(hostedManifestUrl);
} catch {
return new Response("Invalid NEXT_PUBLIC_FC_HOSTED_MANIFEST_URL", { status: 500 });
}
return new Response(null, {
status: 307,
headers: {
Location: hostedManifestUrl,
"Cache-Control": "public, max-age=300, s-maxage=300, stale-while-revalidate=86400",
},
});
}
export async function HEAD() {
return GET();
}
🤖 Prompt for AI Agents
In app/.well-known/route.ts around lines 1–10, remove the non-null assertion and
add a guard that checks process.env.NEXT_PUBLIC_FC_HOSTED_MANIFEST_URL exists
and is a valid URL (e.g., try new URL(value) and catch errors); if
missing/invalid, return a 400 (or 500) Response with a clear message instead of
performing the redirect; when valid, use the parsed URL.href as the Location
header and update Cache-Control to include a browser-facing max-age as well as
s-maxage and stale-while-revalidate (for example: "public, max-age=60,
s-maxage=300, stale-while-revalidate=86400").

💡 Verification agent

🧩 Analysis chain

Route path doesn’t intercept /.well-known/farcaster.json

This route handles /.well-known, not /.well-known/farcaster.json. If you intend to redirect the JSON path via code (instead of vercel.json), move this file under app/.well-known/farcaster.json/route.ts. Otherwise, delete this route and rely on the Vercel redirect only.

I can script a curl check to confirm the live behavior for both /.well-known and /.well-known/farcaster.json.


🏁 Script executed:

#!/bin/bash
# Locate vercel.json
echo "=== vercel.json path ==="
fd -t f vercel.json || echo "vercel.json not found"

# Show content of vercel.json if exists
if [ -f vercel.json ]; then
  echo "=== vercel.json content ==="
  sed -n '1,200p' vercel.json
fi

# Locate next.config.js or next.config.ts
echo "=== next.config.js or ts ==="
fd -t f next.config.js next.config.ts || echo "next.config.js/ts not found"

# Show redirect config in next.config.js
if [ -f next.config.js ]; then
  echo "=== next.config.js redirects ==="
  grep -R "redirects" -n next.config.js | sed 's/^/   /'
fi

# Show redirect config in next.config.ts
if [ -f next.config.ts ]; then
  echo "=== next.config.ts redirects ==="
  grep -R "redirects" -n next.config.ts | sed 's/^/   /'
fi

# Show app/.well-known/route.ts content
echo "=== app/.well-known/route.ts content ==="
if [ -f app/.well-known/route.ts ]; then
  sed -n '1,200p' app/.well-known/route.ts
else
  echo "app/.well-known/route.ts not found"
fi

Length of output: 781


Route file only matches /.well-known, not /.well-known/farcaster.json
Move app/.well-known/route.ts to app/.well-known/farcaster.json/route.ts if you intend to handle the JSON redirect in code; otherwise delete this route and rely on the existing vercel.json redirect for the .well-known/farcaster.json path.

🤖 Prompt for AI Agents
In app/.well-known/route.ts around lines 1 to 10, the route currently only
matches /.well-known; either move this file to
app/.well-known/farcaster.json/route.ts so it handles requests to
/.well-known/farcaster.json (keeping the same GET handler) or delete this route
file entirely and rely on the existing vercel.json redirect for the
/.well-known/farcaster.json path; after moving, ensure the file path and exports
remain the same and update any imports or references if necessary.

Comment on lines +103 to 116
} else if ((error as any)?.message?.includes("User rejected")) {
toast.error("Wallet connection was cancelled by user.", {
description: "You can try connecting again later.",
});
setIsInjectedWallet(false);
} else {
toast.error(
"Failed to connect to wallet. Please refresh and try again.",
{
description: "If the problem persists, try restarting the app.",
},
);
setIsInjectedWallet(false);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Reset all injected state on any failure path

The “User rejected” and generic error branches leave stale provider/address/ready.

-          } else if ((error as any)?.message?.includes("User rejected")) {
-            toast.error("Wallet connection was cancelled by user.", {
-              description: "You can try connecting again later.",
-            });
-            setIsInjectedWallet(false);
+          } else if ((error as any)?.message?.includes("User rejected")) {
+            toast.error("Wallet connection was cancelled by user.", {
+              description: "You can try connecting again later.",
+            });
+            setIsInjectedWallet(false);
+            setInjectedProvider(null);
+            setInjectedAddress(null);
+            setInjectedReady(false);
           } else {
             toast.error(
               "Failed to connect to wallet. Please refresh and try again.",
               {
                 description: "If the problem persists, try restarting the app.",
               },
             );
-            setIsInjectedWallet(false);
+            setIsInjectedWallet(false);
+            setInjectedProvider(null);
+            setInjectedAddress(null);
+            setInjectedReady(false);
           }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
} else if ((error as any)?.message?.includes("User rejected")) {
toast.error("Wallet connection was cancelled by user.", {
description: "You can try connecting again later.",
});
setIsInjectedWallet(false);
} else {
toast.error(
"Failed to connect to wallet. Please refresh and try again.",
{
description: "If the problem persists, try restarting the app.",
},
);
setIsInjectedWallet(false);
}
} else if ((error as any)?.message?.includes("User rejected")) {
toast.error("Wallet connection was cancelled by user.", {
description: "You can try connecting again later.",
});
setIsInjectedWallet(false);
setInjectedProvider(null);
setInjectedAddress(null);
setInjectedReady(false);
} else {
toast.error(
"Failed to connect to wallet. Please refresh and try again.",
{
description: "If the problem persists, try restarting the app.",
},
);
setIsInjectedWallet(false);
setInjectedProvider(null);
setInjectedAddress(null);
setInjectedReady(false);
}
🤖 Prompt for AI Agents
In app/context/InjectedWalletContext.tsx around lines 103 to 116, the error
branches for "User rejected" and the generic failure currently only
setIsInjectedWallet(false) and leave stale provider/address/ready state; update
both failure branches to fully reset all injected-wallet state (e.g., set
provider to null/undefined, clear connected address, mark ready/connected flags
false, and setIsInjectedWallet(false)) — or call an existing resetInjectedState
helper that does that — so no provider/address/ready values remain after any
failure path.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (4)
next.config.mjs (1)

20-27: Align framing headers with modern CSP usage and keep origins maintainable

You’re correctly relying on Content-Security-Policy’s frame-ancestors to allow embedding from Warpcast/Farcaster and related origins, which matches the mini‑app use case. However, X-Frame-Options: ALLOWALL is non‑standard and effectively redundant when CSP frame-ancestors is present; it can be confusing for future readers.

Consider:

  • Dropping the X-Frame-Options header entirely and letting frame-ancestors be the single source of truth.
  • Optionally factoring the allowed frame origins into a small helper/constant or env-driven list so it’s easier to update if Farcaster adds more domains.

This is non-blocking and mostly about clarity and long‑term maintainability.

app/blog/[id]/page.tsx (1)

28-35: Align params/searchParams types and mini-mode redirect with App Router conventions

In the App Router, params and searchParams are passed in as plain objects, not Promises. Typing them as Promises and awaiting them works at runtime (since await on a non-Promise just returns the value) but fights the canonical Next.js types and makes the API harder to work with.

You can simplify both the types and the control flow here (and in generateMetadata) by treating them as plain objects and making searchParams optional:

-export async function generateMetadata({
-  params,
-}: {
-  params: Promise<{ id: string }>;
-}): Promise<Metadata> {
-  const { id } = await params;
+export async function generateMetadata({
+  params,
+}: {
+  params: { id: string };
+}): Promise<Metadata> {
+  const { id } = params;
   const post = await getPost(id);
   if (!post) return {};
   // ...
 }
 
 export default async function BlogPostDetailPage({
   params,
   searchParams,
 }: {
-  params: Promise<{ id: string }>;
-  searchParams: Promise<{ mini?: string }>;
+  params: { id: string };
+  searchParams?: { mini?: string };
 }) {
-  // Redirect to home page if in mini mode
-  const resolvedSearchParams = await searchParams;
-  if (resolvedSearchParams.mini === "true") {
-    redirect("/?mini=true");
-  }
-
-  const { id } = await params;
+  // Redirect to home page if in mini mode
+  if (searchParams?.mini === "true") {
+    redirect("/?mini=true");
+  }
+
+  const { id } = params;
   const post = await getPost(id);
   if (!post) notFound();
   const recent = await getRecentPosts(post.slug.current);
   return <DetailClient post={post} recent={recent} />;
 }

This keeps the behavior you want (redirecting mini-mode blog detail requests back to /?mini=true) while:

  • Matching Next.js’ canonical App Router types for params/searchParams.
  • Removing unnecessary awaits and an intermediate resolvedSearchParams variable.
  • Making the code safer if searchParams is ever omitted or undefined.

Also, just to confirm product intent: in mini mode, this will always bounce users from an individual blog post back to the home mini view. If you instead want to support reading posts inside mini mode, we’d need a different handling strategy (e.g., adapting the layout instead of redirecting). Happy to help sketch that if needed.

Also applies to: 110-125

app/components/Navbar.tsx (1)

135-144: LGTM! Dropdown toggle properly gated in mini mode.

The button's ARIA attributes and click handler correctly reflect the disabled state in mini mode, and the dropdown arrow is appropriately hidden.

Note: A past review comment suggests additional accessibility improvements (adding disabled and aria-disabled attributes, functional state updates, and proper id on the dropdown element). Consider implementing those enhancements for better accessibility.

Also applies to: 160-172

app/layout.tsx (1)

40-44: Critical: Missing import for config object.

Lines 41 and 44 reference config.publicUrl, but the config object is not imported. This will cause a runtime error.

Add the missing import at the top of the file:

+import { config } from "./config";
 import "./globals.css";
 import React from "react";

Additionally, past review comments have already flagged concerns about:

  1. Using hardcoded preview URLs instead of config-driven values
  2. Potential duplicate Farcaster frame metadata across files

Please address those comments as well.

🧹 Nitpick comments (1)
app/types.ts (1)

250-250: Consider using a union type for nodeEnv for better type safety.

The nodeEnv field is currently typed as string, but it typically has a fixed set of values. Using a union type would provide better type safety and IDE autocomplete support.

Apply this diff to improve type safety:

-  nodeEnv: string;
+  nodeEnv: "development" | "production" | "test";
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cd1e447 and ff5635a.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (11)
  • app/blog/[id]/page.tsx (2 hunks)
  • app/blog/page.tsx (2 hunks)
  • app/components/HomePage.tsx (4 hunks)
  • app/components/MainPageContent.tsx (4 hunks)
  • app/components/Navbar.tsx (8 hunks)
  • app/layout.tsx (3 hunks)
  • app/lib/config.ts (2 hunks)
  • app/types.ts (1 hunks)
  • app/utils.ts (2 hunks)
  • next.config.mjs (3 hunks)
  • package.json (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
  • app/utils.ts
  • app/components/MainPageContent.tsx
  • package.json
  • app/lib/config.ts
  • app/blog/page.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-10T16:44:32.125Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 244
File: app/components/CopyAddressWarningModal.tsx:48-52
Timestamp: 2025-10-10T16:44:32.125Z
Learning: In the CopyAddressWarningModal component (app/components/CopyAddressWarningModal.tsx), selectedNetwork from useNetwork() is always defined and does not require null safety checks when accessing its properties like selectedNetwork.chain.name.

Applied to files:

  • app/components/Navbar.tsx
🧬 Code graph analysis (2)
app/components/HomePage.tsx (1)
app/hooks/useMiniMode.ts (1)
  • useMiniMode (10-16)
app/components/Navbar.tsx (6)
app/hooks/useMiniMode.ts (1)
  • useMiniMode (10-16)
app/utils.ts (3)
  • classNames (23-25)
  • IS_MAIN_PRODUCTION_DOMAIN (730-732)
  • getNetworkImageUrl (863-868)
app/components/WalletDetails.tsx (1)
  • WalletDetails (41-433)
app/components/NetworksDropdown.tsx (1)
  • NetworksDropdown (23-128)
app/components/SettingsDropdown.tsx (1)
  • SettingsDropdown (28-296)
app/components/MobileDropdown.tsx (1)
  • MobileDropdown (26-299)
🪛 Biome (2.1.2)
app/components/Navbar.tsx

[error] 178-178: expected ) but instead found {

Remove {

(parse)

🔇 Additional comments (12)
next.config.mjs (2)

52-52: Whitespace-only change; no behavioral impact

This blank-line change is cosmetic only and has no effect on behavior; fine to keep as-is.


90-90: Whitespace-only change; no behavioral impact

Same here: whitespace-only adjustment, safe and non-functional.

app/types.ts (2)

268-268: > Likely an incorrect or invalid review comment.


265-267: The fields farcasterHeader, farcasterPayload, and farcasterSignature do not exist in the codebase. A comprehensive search found zero occurrences of these fields in app/types.ts or anywhere else in the repository. The actual content at lines 265-267 contains different configuration fields (aggregatorUrl, privyAppId, thirdwebClientId, mixpanelToken, hotjarSiteId, googleVerificationCode). The only Farcaster references in the codebase are UI components for social sharing with Farcaster icons, not authentication configuration fields.

Likely an incorrect or invalid review comment.

app/blog/[id]/page.tsx (1)

3-3: Importing redirect for server-side navigation looks correct

Using redirect from next/navigation is the right primitive for this new mini-mode redirect behavior in an App Router page. No issues here.

app/components/HomePage.tsx (2)

18-18: LGTM! Clean integration of mini mode detection.

The useMiniMode hook is correctly imported and used to determine the current mode.

Also applies to: 53-54


149-441: LGTM! Marketing content appropriately hidden in mini mode.

The conditional rendering correctly hides all additional content (video walkthrough, use cases, rates, FAQs, liquidity provider section, and mobile app download) when in mini mode, aligning with the simplified UI requirements for the Farcaster Mini App integration.

app/layout.tsx (2)

8-18: LGTM! Layout component imports properly organized.

The new imports for the restructured layout (Providers, MainContent, navigation components, and MiniKitContextProvider) are properly organized and follow Next.js conventions.


258-279: LGTM! New layout structure properly implements Farcaster Mini App integration.

The restructured layout correctly introduces:

  • EarlyReady component for Farcaster readiness signaling
  • MiniKitContextProvider wrapper for Farcaster SDK integration
  • Proper nesting of providers and layout components
  • Conditional NoticeBanner rendering

The component composition follows React best practices.

Note: Line 265 also uses config.noticeBannerText, which requires the missing config import flagged in the earlier comment.

app/components/Navbar.tsx (3)

30-30: LGTM! Mini mode hook properly integrated.

The useMiniMode hook is correctly imported and initialized.

Also applies to: 41-41


121-121: LGTM! Dropdown hover behavior correctly disabled in mini mode.

The onMouseEnter handler properly prevents the dropdown from opening when in mini mode.


201-213: LGTM! Wallet controls and navigation correctly hidden in mini mode.

The implementation correctly:

  • Shows the Swap link only on blog routes when not in mini mode
  • Hides the entire dropdown menu in mini mode
  • Hides all wallet controls (WalletDetails, NetworksDropdown, SettingsDropdown, and mobile wallet button) in mini mode

This creates the simplified UI appropriate for the Farcaster Mini App integration.

Also applies to: 215-245, 270-309

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
app/components/Navbar.tsx (1)

175-208: Critical: Duplicate Blog link will render twice.

Lines 175-185 and 198-208 are identical Blog link blocks with the same condition !pathname.startsWith("/blog") && !isMiniMode. This will render two "Blog" links side by side in the navbar.

Remove one of the duplicate blocks:

            {/* Blog Link - Desktop Only - Hidden in mini mode */}
            {!pathname.startsWith("/blog") && !isMiniMode && (
              <div className="hidden items-center sm:flex">
                <Link
                  href="/blog"
                  className={` ${IS_MAIN_PRODUCTION_DOMAIN ? "" : "-mt-[3px]"} text-sm font-medium text-gray-700 transition-colors hover:text-gray-900 dark:text-white/80 dark:hover:text-white`}
                >
                  Blog
                </Link>
              </div>
            )}
            {/* Home Link */}
            {pathname !== "/" && (
              <div className="hidden items-center sm:flex">
                <Link
                  href="/"
                  className={` ${IS_MAIN_PRODUCTION_DOMAIN ? "" : "-mt-[3px]"} text-sm font-medium text-gray-700 transition-colors hover:text-gray-900 dark:text-white/80 dark:hover:text-white`}
                >
                  Home
                </Link>
              </div>
            )}

-            {/* Blog Link - Desktop Only - Hidden in mini mode */}
-            {!pathname.startsWith("/blog") && !isMiniMode && (
-              <div className="hidden items-center sm:flex">
-                <Link
-                  href="/blog"
-                  className={` ${IS_MAIN_PRODUCTION_DOMAIN ? "" : "-mt-[3px]"} text-sm font-medium text-gray-700 transition-colors hover:text-gray-900 dark:text-white/80 dark:hover:text-white`}
-                >
-                  Blog
-                </Link>
-              </div>
-            )}
-

Also note: The Home link (lines 186-196) doesn't check isMiniMode. Verify if this is intentional—if Home should also be hidden in mini mode, add && !isMiniMode to its condition.

♻️ Duplicate comments (3)
app/layout.tsx (1)

51-55: Consider consolidating Farcaster frame meta with app/head.tsx.

Past review flagged potential duplication of fc:frame meta tags between this file and app/head.tsx. Verify that only one location emits these tags to avoid conflicting frame targets.

#!/bin/bash
# Check for fc:frame meta tag locations across the codebase
rg -n 'fc:frame' -g '*.tsx' -g '*.ts' -C2
app/lib/config.ts (1)

51-51: Remove supabaseRoleKey from client config — exposes server secret to browser.

SUPABASE_SERVICE_ROLE_KEY is a privileged server-side credential that should never be bundled into client code. It's already correctly placed in serverConfig at line 126.

app/components/Navbar.tsx (1)

133-144: aria-controls references missing id on dropdown element.

The button sets aria-controls="navbar-dropdown" (line 137) but the dropdown motion.div at line 227 lacks id="navbar-dropdown". Add the id to complete the ARIA relationship:

                  <motion.div
+                   id="navbar-dropdown"
+                   role="menu"
                    initial={{ opacity: 0, y: -10 }}
🧹 Nitpick comments (1)
app/components/Navbar.tsx (1)

245-253: Redundant !isMiniMode check.

This condition is already inside the isDropdownOpen && !isMiniMode block (line 223), so the !isMiniMode check here is unnecessary.

-                   {!pathname.startsWith("/blog") && !isMiniMode && (
+                   {!pathname.startsWith("/blog") && (
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ff5635a and 9095ff4.

📒 Files selected for processing (6)
  • app/components/MainPageContent.tsx (7 hunks)
  • app/components/Navbar.tsx (9 hunks)
  • app/layout.tsx (3 hunks)
  • app/lib/config.ts (2 hunks)
  • app/types.ts (1 hunks)
  • package.json (3 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-10-13T14:37:45.409Z
Learnt from: sundayonah
Repo: paycrest/noblocks PR: 250
File: app/components/AppLayout.tsx:33-46
Timestamp: 2025-10-13T14:37:45.409Z
Learning: In TypeScript/JavaScript/React codebases, JSX whitespace expressions like `{" "}` between elements should be classified as style/nitpick issues, not "Potential issues" or problems that affect functionality. These are purely cosmetic concerns about code cleanliness.

Applied to files:

  • app/components/Navbar.tsx
📚 Learning: 2025-10-10T16:44:32.125Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 244
File: app/components/CopyAddressWarningModal.tsx:48-52
Timestamp: 2025-10-10T16:44:32.125Z
Learning: In the CopyAddressWarningModal component (app/components/CopyAddressWarningModal.tsx), selectedNetwork from useNetwork() is always defined and does not require null safety checks when accessing its properties like selectedNetwork.chain.name.

Applied to files:

  • app/components/Navbar.tsx
📚 Learning: 2025-11-06T07:37:39.036Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 231
File: app/components/recipient/RecipientDetailsForm.tsx:539-552
Timestamp: 2025-11-06T07:37:39.036Z
Learning: In RecipientDetailsForm (app/components/recipient/RecipientDetailsForm.tsx), when isRecipientNameEditable is false (verification succeeded), the recipient safety alert should display when: !isRecipientNameEditable && recipientName && !recipientNameError. The !errors.recipientName check is unnecessary because in non-editable mode the recipient name is displayed as read-only text (not an input field), so form validation errors cannot occur.

Applied to files:

  • app/components/MainPageContent.tsx
📚 Learning: 2025-07-23T07:30:23.720Z
Learnt from: jeremy0x
Repo: paycrest/noblocks PR: 177
File: app/utils.ts:179-196
Timestamp: 2025-07-23T07:30:23.720Z
Learning: The `normalizeNetworkName` function in app/utils.ts has been updated to be dynamic and scalable, converting any network identifier to sentence case with hyphens replaced by spaces and proper handling of acronyms like "BNB".

Applied to files:

  • app/types.ts
🧬 Code graph analysis (3)
app/components/Navbar.tsx (4)
app/hooks/useMiniMode.ts (1)
  • useMiniMode (10-16)
app/utils.ts (2)
  • classNames (23-25)
  • IS_MAIN_PRODUCTION_DOMAIN (730-732)
app/components/WalletDetails.tsx (1)
  • WalletDetails (41-433)
app/components/NetworksDropdown.tsx (1)
  • NetworksDropdown (23-128)
app/layout.tsx (6)
app/early-ready.tsx (1)
  • EarlyReady (5-34)
providers/MiniKitProvider.tsx (1)
  • MiniKitContextProvider (25-42)
app/components/Navbar.tsx (1)
  • Navbar (32-333)
app/components/LayoutWrapper.tsx (1)
  • LayoutWrapper (11-22)
app/components/Footer.tsx (1)
  • Footer (56-222)
app/components/PWAInstallManager.tsx (1)
  • PWAInstall (5-37)
app/components/MainPageContent.tsx (4)
app/utils.ts (1)
  • getBannerPadding (1129-1132)
app/types.ts (1)
  • STEPS (29-33)
app/hooks/useMiniMode.ts (1)
  • useMiniMode (10-16)
app/hooks/useBlockFestReferral.ts (1)
  • useBlockFestReferral (5-28)
🪛 Biome (2.1.2)
app/layout.tsx

[error] 19-19: Shouldn't redeclare 'Providers'. Consider to delete it or rename it.

'Providers' is defined here:

(lint/suspicious/noRedeclare)


[error] 20-20: Shouldn't redeclare 'MainContent'. Consider to delete it or rename it.

'MainContent' is defined here:

(lint/suspicious/noRedeclare)


[error] 22-22: Shouldn't redeclare 'Footer'. Consider to delete it or rename it.

'Footer' is defined here:

(lint/suspicious/noRedeclare)


[error] 23-23: Shouldn't redeclare 'Navbar'. Consider to delete it or rename it.

'Navbar' is defined here:

(lint/suspicious/noRedeclare)


[error] 24-24: Shouldn't redeclare 'LayoutWrapper'. Consider to delete it or rename it.

'LayoutWrapper' is defined here:

(lint/suspicious/noRedeclare)


[error] 25-25: Shouldn't redeclare 'PWAInstall'. Consider to delete it or rename it.

'PWAInstall' is defined here:

(lint/suspicious/noRedeclare)


[error] 26-26: Shouldn't redeclare 'NoticeBanner'. Consider to delete it or rename it.

'NoticeBanner' is defined here:

(lint/suspicious/noRedeclare)


[error] 28-28: Shouldn't redeclare 'EarlyReady'. Consider to delete it or rename it.

'EarlyReady' is defined here:

(lint/suspicious/noRedeclare)


[error] 29-29: Shouldn't redeclare 'MiniKitContextProvider'. Consider to delete it or rename it.

'MiniKitContextProvider' is defined here:

(lint/suspicious/noRedeclare)

app/components/MainPageContent.tsx

[error] 41-41: Shouldn't redeclare 'useMiniMode'. Consider to delete it or rename it.

'useMiniMode' is defined here:

(lint/suspicious/noRedeclare)


[error] 121-121: Shouldn't redeclare 'isMiniMode'. Consider to delete it or rename it.

'isMiniMode' is defined here:

(lint/suspicious/noRedeclare)

app/lib/config.ts

[error] 12-12: This property is later overwritten by an object member with the same name.

Overwritten with this property.

If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.

(lint/suspicious/noDuplicateObjectKeys)


[error] 13-13: This property is later overwritten by an object member with the same name.

Overwritten with this property.

If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.

(lint/suspicious/noDuplicateObjectKeys)


[error] 14-14: This property is later overwritten by an object member with the same name.

Overwritten with this property.

If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.

(lint/suspicious/noDuplicateObjectKeys)


[error] 15-15: This property is later overwritten by an object member with the same name.

Overwritten with this property.

If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.

(lint/suspicious/noDuplicateObjectKeys)


[error] 16-16: This property is later overwritten by an object member with the same name.

Overwritten with this property.

If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.

(lint/suspicious/noDuplicateObjectKeys)


[error] 17-17: This property is later overwritten by an object member with the same name.

Overwritten with this property.

If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.

(lint/suspicious/noDuplicateObjectKeys)


[error] 18-18: This property is later overwritten by an object member with the same name.

Overwritten with this property.

If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.

(lint/suspicious/noDuplicateObjectKeys)


[error] 19-19: This property is later overwritten by an object member with the same name.

Overwritten with this property.

If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.

(lint/suspicious/noDuplicateObjectKeys)


[error] 20-20: This property is later overwritten by an object member with the same name.

Overwritten with this property.

If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.

(lint/suspicious/noDuplicateObjectKeys)


[error] 21-21: This property is later overwritten by an object member with the same name.

Overwritten with this property.

If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.

(lint/suspicious/noDuplicateObjectKeys)


[error] 22-24: This property is later overwritten by an object member with the same name.

Overwritten with this property.

If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.

(lint/suspicious/noDuplicateObjectKeys)


[error] 25-27: This property is later overwritten by an object member with the same name.

Overwritten with this property.

If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.

(lint/suspicious/noDuplicateObjectKeys)


[error] 28-28: This property is later overwritten by an object member with the same name.

Overwritten with this property.

If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.

(lint/suspicious/noDuplicateObjectKeys)


[error] 29-29: This property is later overwritten by an object member with the same name.

Overwritten with this property.

If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.

(lint/suspicious/noDuplicateObjectKeys)


[error] 30-32: This property is later overwritten by an object member with the same name.

Overwritten with this property.

If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.

(lint/suspicious/noDuplicateObjectKeys)


[error] 33-34: This property is later overwritten by an object member with the same name.

Overwritten with this property.

If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.

(lint/suspicious/noDuplicateObjectKeys)


[error] 35-36: This property is later overwritten by an object member with the same name.

Overwritten with this property.

If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.

(lint/suspicious/noDuplicateObjectKeys)


[error] 37-39: This property is later overwritten by an object member with the same name.

Overwritten with this property.

If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.

(lint/suspicious/noDuplicateObjectKeys)


[error] 40-42: This property is later overwritten by an object member with the same name.

Overwritten with this property.

If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.

(lint/suspicious/noDuplicateObjectKeys)


[error] 43-43: This property is later overwritten by an object member with the same name.

Overwritten with this property.

If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.

(lint/suspicious/noDuplicateObjectKeys)


[error] 44-44: This property is later overwritten by an object member with the same name.

Overwritten with this property.

If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.

(lint/suspicious/noDuplicateObjectKeys)

package.json

[error] 74-74: expected , but instead found "tls"

Remove "tls"

(parse)


[error] 73-73: The key wagmi was already declared.

This where a duplicated key was declared again.

If a key is defined multiple times, only the last definition takes effect. Previous definitions are ignored.

(lint/suspicious/noDuplicateObjectKeys)

🔇 Additional comments (4)
app/components/Navbar.tsx (4)

30-41: LGTM!

The useMiniMode hook is correctly imported and used to conditionally adjust the Navbar behavior based on the mini query parameter.


160-172: LGTM!

Correctly hides the dropdown caret in mini mode, preventing user confusion about non-functional UI elements.


210-221: LGTM!

The Swap link correctly appears only on blog pages and is properly hidden in mini mode.


277-316: LGTM!

Cleanly wraps all wallet, network, and settings controls in a single !isMiniMode condition. This appropriately hides these controls in the Farcaster mini app context where the host app manages wallet functionality.

package.json Outdated
Comment on lines 73 to 77
"wagmi": "^2.16.9"
"tls": "^0.0.1",
"viem": "^2.31.0"
"viem": "^2.31.0",
"wagmi": "^2.16.9"
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Fix JSON syntax error and remove duplicate wagmi entry — breaks package installation.

Line 73 is missing a trailing comma, and wagmi is declared twice (lines 73 and 76). This will cause pnpm install to fail.

Apply this diff:

     "thirdweb": "^5.102.6",
-    "wagmi": "^2.16.9"
     "tls": "^0.0.1",
-    "viem": "^2.31.0",
+    "viem": "^2.31.0",
     "wagmi": "^2.16.9"

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 Biome (2.1.2)

[error] 74-74: expected , but instead found "tls"

Remove "tls"

(parse)


[error] 73-73: The key wagmi was already declared.

This where a duplicated key was declared again.

If a key is defined multiple times, only the last definition takes effect. Previous definitions are ignored.

(lint/suspicious/noDuplicateObjectKeys)

🤖 Prompt for AI Agents
In package.json around lines 73 to 77, the dependencies block has a JSON syntax
error (missing trailing comma after the entry on line 73) and a duplicate
"wagmi" entry (lines 73 and 76); fix by removing the duplicate "wagmi"
declaration and ensure proper JSON punctuation by adding the missing comma where
needed so the dependencies object is valid for pnpm install.

…and adding supabaseRoleKey to the Config type. Update package.json to remove duplicate wagmi entry.
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
app/types.ts (1)

249-281: Tighten Config typing and separate public vs server-only fields.

Centralizing config here is good, but a few small tweaks would improve safety and type‑checking:

  • nodeEnv and noIndex are better modeled as narrower types, e.g.:
    • nodeEnv: "development" | "test" | "production";
    • noIndex: boolean; (convert from the env string in app/lib/config.ts once, then use as a boolean everywhere).
  • Sensitive fields like supabaseRoleKey and cdpApiKey are mixed with clearly public fields (publicUrl, OG/meta fields, etc.) in a single Config shape. Consider splitting into something like PublicConfig and ServerConfig (or PrivateConfig) so that server‑only values cannot be accidentally exposed to client code by reusing the wrong config object.

These are non-breaking refactors but will make misuse of config much harder over time.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9095ff4 and 0d1b93b.

📒 Files selected for processing (3)
  • app/lib/config.ts (2 hunks)
  • app/types.ts (1 hunks)
  • package.json (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • app/lib/config.ts
  • package.json
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-23T07:30:23.720Z
Learnt from: jeremy0x
Repo: paycrest/noblocks PR: 177
File: app/utils.ts:179-196
Timestamp: 2025-07-23T07:30:23.720Z
Learning: The `normalizeNetworkName` function in app/utils.ts has been updated to be dynamic and scalable, converting any network identifier to sentence case with hyphens replaced by spaces and proper handling of acronyms like "BNB".

Applied to files:

  • app/types.ts
🔇 Additional comments (1)
app/types.ts (1)

283-291: Network.imageUrl union shape looks good.

The imageUrl union (string or { light; dark }) is clear and flexible for theming; the reformatting here doesn’t introduce new behavior and the type remains sound for current and future callers.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (3)
app/lib/config.ts (1)

48-48: Security: supabaseRoleKey should not be in client-accessible config.

The SUPABASE_SERVICE_ROLE_KEY is a sensitive server-side secret that grants admin access to your Supabase database. Including it in the shared config object risks exposing it in client bundles.

This was flagged in a previous review. Move it exclusively to serverConfig (which you already have at line 97) and remove it from the main config object.

   blockfestEndDate:
     process.env.NEXT_PUBLIC_BLOCKFEST_END_DATE || "2025-10-11T23:59:00+01:00",
-  supabaseRoleKey: process.env.SUPABASE_SERVICE_ROLE_KEY || "",
   baseBuilderOwnerAddress: process.env.BASE_BUILDER_OWNER_ADDRESS || "",

Then import serverConfig.supabaseRoleKey only in server-side code.

app/context/InjectedWalletContext.tsx (2)

88-110: Reset all injected wallet state in every error branch (avoid stale provider/address).

The code === 4001 branch correctly clears isInjectedWallet, injectedProvider, injectedAddress, and injectedReady. However:

  • The "User rejected" message branch only calls setIsInjectedWallet(false).
  • The generic error branch also only calls setIsInjectedWallet(false).

If the user had previously connected successfully (or a prior attempt partially initialized state), these branches will leave a stale provider/address/ready flag in context, which can confuse downstream consumers.

You already have the full reset pattern in the 4001 branch; mirror it in all failure paths, or factor out a small helper:

-          } else if ((error as any)?.message?.includes("User rejected")) {
-            toast.error("Wallet connection was cancelled by user.", {
-              description: "You can try connecting again later.",
-            });
-            setIsInjectedWallet(false);
-          } else {
+          } else if ((error as any)?.message?.includes("User rejected")) {
+            toast.error("Wallet connection was cancelled by user.", {
+              description: "You can try connecting again later.",
+            });
+            setIsInjectedWallet(false);
+            setInjectedProvider(null);
+            setInjectedAddress(null);
+            setInjectedReady(false);
+          } else {
             toast.error(
               "Failed to connect to wallet. Please refresh and try again.",
               {
                 description: "If the problem persists, try restarting the app.",
               },
             );
-            setIsInjectedWallet(false);
+            setIsInjectedWallet(false);
+            setInjectedProvider(null);
+            setInjectedAddress(null);
+            setInjectedReady(false);
           }

Or:

const resetInjectedState = () => {
  setIsInjectedWallet(false);
  setInjectedProvider(null);
  setInjectedAddress(null);
  setInjectedReady(false);
};

and call resetInjectedState() in all error branches.


37-43: Guard async state updates with the cancelled flag to avoid setState after unmount.

You added a cancelled flag and only schedule initInjectedWallet if !cancelled, which protects against the effect firing after unmount before the timeout elapses. However, once initInjectedWallet has started, unmounting the component doesn’t prevent the async flow from continuing and calling setInjectedProvider, setInjectedAddress, setInjectedReady, etc.

To fully avoid setState on an unmounted component:

  • Capture cancelled inside initInjectedWallet (it already closes over it).
  • Check if (cancelled) return; before any state updates.
  • Optionally early‑return after shouldUseInjectedWallet(...) if cancelled.

For example:

    const initInjectedWallet = async () => {
      const shouldUse = shouldUseInjectedWallet(searchParams);

-      setIsInjectedWallet(shouldUse);
+      if (cancelled) return;
+      setIsInjectedWallet(shouldUse);

      // ... after async calls
-      if (address) {
+      if (cancelled) return;
+      if (address) {
         setInjectedProvider(ethereumProvider);
         setInjectedAddress(address);
         setInjectedReady(true);
      }

      // In each error/failure branch:
-      setIsInjectedWallet(false);
-      setInjectedProvider(null);
-      setInjectedAddress(null);
-      setInjectedReady(false);
+      if (!cancelled) {
+        setIsInjectedWallet(false);
+        setInjectedProvider(null);
+        setInjectedAddress(null);
+        setInjectedReady(false);
+      }
    };

    let cancelled = false;
    const timeoutId = setTimeout(() => !cancelled && initInjectedWallet(), 200);
    return () => {
      cancelled = true;
      clearTimeout(timeoutId);
    };

This keeps the async wallet initialization safe even if the provider is slow and the user navigates away mid‑flow.

Also applies to: 65-67, 73-77, 88-97, 114-121

🧹 Nitpick comments (7)
app/context/BaseAppContext.tsx (2)

38-41: Remove unused isMounted state.

isMounted is set to true on Line 41 but never read. This is dead code that should be removed.

Apply this diff:

     const [sdk, setSdk] = useState<any | null>(null);
-    const [isMounted, setIsMounted] = useState(false);

     useEffect(() => {
-        setIsMounted(true);
-
         const initBaseApp = async () => {

45-58: Consolidate duplicate URL parameter parsing.

The code parses the same query parameters twice: once via searchParams hook (Lines 45-48) and again via window.location.search (Lines 55-58). Consider consolidating to reduce duplication and potential divergence.

         const initBaseApp = async () => {
-            // Check for query params for local testing (including Farcaster pattern)
-            const baseAppParam = searchParams?.get("baseApp");
-            const miniParam = searchParams?.get("mini");
-            const miniAppParam = searchParams?.get("miniApp");
-            const shouldUseForTesting = baseAppParam === "true" || miniParam === "true" || miniAppParam === "true";
-
             if (typeof window === "undefined") {
                 return;
             }

             // Check URL query param and pathname (Farcaster pattern: /mini pathname)
             const urlParams = new URLSearchParams(window.location.search);
             const urlPathname = window.location.pathname;
-            const hasBaseAppParam = urlParams.get("baseApp") === "true" || urlParams.get("mini") === "true" || urlParams.get("miniApp") === "true";
+            const shouldUseForTesting = urlParams.get("baseApp") === "true" || urlParams.get("mini") === "true" || urlParams.get("miniApp") === "true";
             const hasMiniPathname = urlPathname.startsWith("/mini"); // Farcaster docs pattern
app/.well-known/farcaster.json/route.ts (2)

1-1: Remove unused import.

NextRequest is imported but never used in this route handler.

-import { NextRequest, NextResponse } from "next/server";
+import { NextResponse } from "next/server";

7-7: Consider adding a type for the manifest object.

Using any loses TypeScript benefits. Define an interface for the Farcaster manifest structure to improve type safety and documentation.

-    const manifest: any = {
+    interface FarcasterManifest {
+        accountAssociation: {
+            header: string;
+            payload: string;
+            signature: string;
+        };
+        baseBuilder: {
+            ownerAddress: string;
+        };
+        miniapp: {
+            version: string;
+            name: string;
+            homeUrl: string;
+            iconUrl: string;
+            imageUrl: string;
+            splashImageUrl: string;
+            splashBackgroundColor: string;
+            subtitle: string;
+            description: string;
+            screenshotUrls: string[];
+            primaryCategory: string;
+            tags: string[];
+            heroImageUrl: string;
+            tagline: string;
+            ogTitle: string;
+            ogDescription: string;
+            ogImageUrl: string;
+            noindex: boolean;
+        };
+    }
+
+    const manifest: FarcasterManifest = {
app/pages/TransactionPreview.tsx (1)

120-133: Wallet objects include type property here but not in TransactionForm.

The baseAppWalletObj and injectedWallet here include a type property, while the equivalent objects in TransactionForm.tsx do not. Consider aligning the structure across both files for consistency and to enable consistent analytics tracking.

app/pages/TransactionStatus.tsx (1)

354-375: Balance and wallet type logic is correct.

The balance priority and wallet type derivation follow the established pattern from other components.

Minor inconsistency: The walletType for injected wallets is "Injected" here (line 364), but "Injected wallet" in TransactionPreview.tsx (line 281). Consider aligning these for consistent analytics data.

         const walletType = isBaseApp
           ? "Base App wallet"
           : isFarcaster
             ? "Farcaster wallet"
             : isInjectedWallet
-              ? "Injected"
+              ? "Injected wallet"
               : "Smart wallet";
app/types.ts (1)

249-275: Consider tightening Config and Network types for stronger compile‑time guarantees.

The new fields on Config and the widened Network.imageUrl union look consistent with the app’s config needs, but you might get more mileage from stricter typing:

  • nodeEnv and noIndex are plain string; consider:
    • nodeEnv: "development" | "production" | "test" (or whatever you actually support).
    • noIndex: boolean (or "true" | "false" if it mirrors env vars directly).
  • Fields like appPrimaryCategory and Farcaster/Base App metadata could be restricted to known literals if they are constrained by external APIs (e.g., Farcaster categories).
  • For Network.imageUrl, the union is fine; just ensure all renderers handle both string and { light, dark } branches (a small helper like getNetworkImageUrl(network, theme) can keep that logic centralized).

These are non‑blocking, but they’ll catch misconfigured deployments earlier in TypeScript instead of at runtime.

Also applies to: 282-282, 287-295

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0d1b93b and 4035478.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (17)
  • app/.well-known/farcaster.json/route.ts (1 hunks)
  • app/components/AppLayout.tsx (3 hunks)
  • app/components/HomePage.tsx (1 hunks)
  • app/components/MainPageContent.tsx (2 hunks)
  • app/components/Navbar.tsx (1 hunks)
  • app/context/BaseAppContext.tsx (1 hunks)
  • app/context/InjectedWalletContext.tsx (3 hunks)
  • app/context/index.ts (1 hunks)
  • app/early-ready.tsx (1 hunks)
  • app/layout.tsx (3 hunks)
  • app/lib/config.ts (2 hunks)
  • app/pages/TransactionForm.tsx (7 hunks)
  • app/pages/TransactionPreview.tsx (6 hunks)
  • app/pages/TransactionStatus.tsx (8 hunks)
  • app/providers.tsx (3 hunks)
  • app/types.ts (1 hunks)
  • next.config.mjs (3 hunks)
✅ Files skipped from review due to trivial changes (1)
  • app/components/Navbar.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
  • app/components/MainPageContent.tsx
  • app/components/HomePage.tsx
  • next.config.mjs
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-12-05T02:22:28.063Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 312
File: app/page.tsx:7-11
Timestamp: 2025-12-05T02:22:28.063Z
Learning: In Next.js 15 with React 19, components using useSearchParams() cannot be statically prerendered. When app/layout.tsx has `export const dynamic = "force-static"`, MainPageContent must be dynamically imported with ssr: false to avoid "Element type is invalid" errors during build. The dynamic import pattern `dynamic(() => import("./components/MainPageContent").then(mod => ({ default: mod.MainPageContent })), { ssr: false })` is the correct solution for this scenario in the noblocks repository.

Applied to files:

  • app/context/BaseAppContext.tsx
  • app/components/AppLayout.tsx
  • app/layout.tsx
📚 Learning: 2025-10-10T16:44:32.125Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 244
File: app/components/CopyAddressWarningModal.tsx:48-52
Timestamp: 2025-10-10T16:44:32.125Z
Learning: In the CopyAddressWarningModal component (app/components/CopyAddressWarningModal.tsx), selectedNetwork from useNetwork() is always defined and does not require null safety checks when accessing its properties like selectedNetwork.chain.name.

Applied to files:

  • app/pages/TransactionPreview.tsx
  • app/pages/TransactionForm.tsx
📚 Learning: 2025-11-06T07:37:39.036Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 231
File: app/components/recipient/RecipientDetailsForm.tsx:539-552
Timestamp: 2025-11-06T07:37:39.036Z
Learning: In RecipientDetailsForm (app/components/recipient/RecipientDetailsForm.tsx), when isRecipientNameEditable is false (verification succeeded), the recipient safety alert should display when: !isRecipientNameEditable && recipientName && !recipientNameError. The !errors.recipientName check is unnecessary because in non-editable mode the recipient name is displayed as read-only text (not an input field), so form validation errors cannot occur.

Applied to files:

  • app/pages/TransactionForm.tsx
  • app/pages/TransactionStatus.tsx
📚 Learning: 2025-11-06T07:37:39.036Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 231
File: app/components/recipient/RecipientDetailsForm.tsx:539-552
Timestamp: 2025-11-06T07:37:39.036Z
Learning: In RecipientDetailsForm (app/components/recipient/RecipientDetailsForm.tsx), when isRecipientNameEditable is true (verification failed/returned "Ok"), the recipient safety alert should display when: isRecipientNameEditable && recipientName && !errors.recipientName && !recipientNameError. The !isFetchingRecipientName check is redundant because recipientName is cleared at fetch start and only populated after fetching completes or when the user manually enters it.

Applied to files:

  • app/pages/TransactionForm.tsx
  • app/context/InjectedWalletContext.tsx
  • app/pages/TransactionStatus.tsx
📚 Learning: 2025-11-06T07:08:54.593Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 231
File: app/components/recipient/RecipientDetailsForm.tsx:271-280
Timestamp: 2025-11-06T07:08:54.593Z
Learning: In the RecipientDetailsForm component (app/components/recipient/RecipientDetailsForm.tsx), Mobile Money institutions always return "Ok" from the fetchAccountName verification endpoint. Therefore, checking for accountName.toLowerCase() === "ok" is sufficient to handle both verification failures and Mobile Money institutions without needing explicit institution type checks in the frontend.

Applied to files:

  • app/pages/TransactionStatus.tsx
📚 Learning: 2025-10-13T14:59:46.723Z
Learnt from: jeremy0x
Repo: paycrest/noblocks PR: 251
File: app/components/blockfest/BlockFestCashbackComponent.tsx:191-191
Timestamp: 2025-10-13T14:59:46.723Z
Learning: In app/components/blockfest/BlockFestCashbackComponent.tsx, the social share URLs intentionally mention "2% cashback" even though the component's cashbackPercentage prop defaults to "1%". The 2% represents the total potential cashback (1% instant + 1% bonus from showing the post at the booth), while the prop represents only the automatic portion.

Applied to files:

  • app/pages/TransactionStatus.tsx
🧬 Code graph analysis (6)
app/early-ready.tsx (1)
public/sw.js (1)
  • url (54-54)
app/context/BaseAppContext.tsx (1)
app/context/index.ts (2)
  • BaseAppProvider (15-15)
  • useBaseApp (15-15)
app/providers.tsx (2)
app/context/BaseAppContext.tsx (1)
  • BaseAppProvider (174-180)
app/context/index.ts (1)
  • BaseAppProvider (15-15)
app/pages/TransactionForm.tsx (5)
app/context/BaseAppContext.tsx (1)
  • useBaseApp (182-182)
app/context/index.ts (1)
  • useBaseApp (15-15)
app/components/index.ts (2)
  • AnimatedComponent (15-15)
  • slideInOut (19-19)
app/components/AnimatedComponents.tsx (2)
  • AnimatedComponent (172-188)
  • slideInOut (28-32)
app/utils.ts (1)
  • formatNumberWithCommas (56-60)
app/pages/TransactionStatus.tsx (4)
app/context/BaseAppContext.tsx (1)
  • useBaseApp (182-182)
app/context/index.ts (1)
  • useBaseApp (15-15)
app/components/index.ts (2)
  • AnimatedComponent (15-15)
  • slideInOut (19-19)
app/components/AnimatedComponents.tsx (2)
  • AnimatedComponent (172-188)
  • slideInOut (28-32)
app/layout.tsx (2)
app/early-ready.tsx (1)
  • EarlyReady (10-60)
app/components/AppLayout.tsx (1)
  • AppLayout (16-63)
🪛 ast-grep (0.40.0)
app/layout.tsx

[warning] 257-257: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html

(react-unsafe-html-injection)

🪛 Biome (2.1.2)
app/early-ready.tsx

[error] 31-31: Unexpected constant condition.

(lint/correctness/noConstantCondition)

app/context/BaseAppContext.tsx

[error] 141-141: This branch can never execute. Its condition is a duplicate or covered by previous conditions in the if-else-if chain.

(lint/suspicious/noDuplicateElseIf)

app/layout.tsx

[error] 258-258: Avoid passing content using the dangerouslySetInnerHTML prop.

Setting content using code can expose users to cross-site scripting (XSS) attacks

(lint/security/noDangerouslySetInnerHtml)

🔇 Additional comments (15)
app/context/index.ts (1)

15-15: LGTM!

The new export follows the established barrel pattern and correctly exposes the BaseAppProvider and useBaseApp from the new context module.

app/layout.tsx (2)

28-41: Hardcoded production URLs in fc:miniapp metadata.

The fc:miniapp payload uses hardcoded https://noblocks.xyz URLs. This is appropriate for the production canonical URL, but consider whether preview/staging deployments need different URLs. If so, derive from a config value.

Confirm that all Farcaster deployments should point to the canonical production URL, or update to use config.publicUrl for environment-specific behavior.


253-262: LGTM!

The placement of EarlyReady at the top of the body ensures SDK readiness signals fire early. Wrapping AppLayout with Providers correctly establishes the context hierarchy. The dangerouslySetInnerHTML for JSON-LD (Line 258) is safe since jsonLd is a static constant with no user input.

app/components/AppLayout.tsx (2)

17-26: LGTM — correct hydration-safe pattern for conditional UI.

The isMounted state with useEffect ensures the mini app check only applies after hydration, preventing server/client mismatch. The full UI renders on the server, then conditionally hides on the client when in a mini app context.


31-43: Appropriate UI simplification for mini app contexts.

Hiding Navbar, NoticeBanner, Footer, and PWAInstall when embedded in Farcaster/Base App provides a cleaner, more integrated experience without redundant navigation or install prompts.

app/providers.tsx (1)

90-108: BaseAppProvider integration looks correct.

The BaseAppProvider is appropriately placed as the outermost wrapper within ContextProviders, ensuring the base app context is available to all nested providers and components.

Minor formatting note: The child elements (lines 91-107) lack indentation under the <BaseAppProvider> wrapper, which slightly hurts readability.

app/.well-known/farcaster.json/route.ts (1)

42-52: Review caching and X-Frame-Options headers for production.

Two concerns:

  1. Cache-Control: The comment says "during testing" but this aggressive no-cache policy will persist in production. Consider using environment-based caching:
const cacheControl = process.env.NODE_ENV === "production" 
  ? "public, max-age=3600" 
  : "no-cache, no-store, must-revalidate, max-age=0";
  1. X-Frame-Options: Setting it to an empty string sends an invalid header rather than removing it. If you need to allow framing, simply omit the header or use ALLOWALL (non-standard). For proper control, use Content-Security-Policy: frame-ancestors instead.
app/pages/TransactionForm.tsx (2)

115-133: Wallet and balance priority logic is well-structured.

The priority chain (Base App → Injected → Smart Wallet) is correctly implemented, and the balance selection appropriately uses injectedWalletBalance for Base App/Farcaster contexts since they use the injected provider.

Minor observation: The wallet objects have slightly inconsistent shapes—baseAppWalletObj and injectedWalletObj are plain objects with only address, while smartWalletObj comes from linkedAccounts and may have additional properties. This works because only activeWallet?.address is accessed, but consider unifying the shape for clarity.


687-690: Formatting change only—no functional impact.

The className template literal restructuring maintains the same conditional logic.

app/pages/TransactionPreview.tsx (2)

200-204: Unified wallet flow for Base App, Farcaster, and Injected wallets looks correct.

The condition (isBaseApp || isFarcaster || isInjectedWallet) && injectedProvider correctly routes all external wallet types through the same code path since they all use the injected provider.


279-282: Analytics tracking correctly distinguishes wallet types.

The ternary chain properly identifies "Base App wallet", "Farcaster wallet", or "Injected wallet" for event tracking.

app/pages/TransactionStatus.tsx (2)

507-514: StatusIndicator className formatting refactored.

The nested ternary in the className is functionally equivalent to the previous version—just reformatted.


859-869: BlockFest cashback component rendering unchanged.

The JSX indentation was adjusted but the rendering logic and props remain the same.

app/lib/config.ts (1)

49-52: The referenced code does not exist in the current codebase.

The fields baseBuilderOwnerAddress, baseAppHeader, baseAppPayload, and baseAppSignature mentioned in the review are not present in app/lib/config.ts. Lines 49-52 of the current file contain Sanity configuration, not Base App configuration. A search of the entire codebase confirms these fields are not defined anywhere.

Likely an incorrect or invalid review comment.

app/context/InjectedWalletContext.tsx (1)

44-60: The review is based on code that does not exist in the file.

The code snippet shown in the review comment (lines 44-60) references Farcaster SDK integration (sdk.isInMiniApp(), sdk.wallet.getEthereumProvider()), a setTimeout mechanism, and a cancelled flag—none of which are present in the actual file. The current implementation is much simpler and only uses window.ethereum directly.

While the underlying concerns about state consistency are valid, they apply to the actual code in a different way:

  • Line 41 sets isInjectedWallet = shouldUse unconditionally, then line 43 gates the initialization on both shouldUse && window.ethereum. If shouldUse is true but window.ethereum is undefined, the state ends up inconsistent (isInjectedWallet=true with null provider/address/ready).
  • Lines 75–79 reset only isInjectedWallet on generic errors, unlike the 4001 error case (lines 71–74), which fully resets provider/address/ready.

Please clarify: Is this review intended for a different branch or version of the code, or is it a suggested refactoring to add Farcaster SDK support?

Likely an incorrect or invalid review comment.

Comment on lines +141 to +145
} else if (shouldUseForTesting) {
// For testing mode, default to Base App
setIsBaseAppEnv(true);
setBaseAppReady(true);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Unreachable code: this branch can never execute.

The condition at Line 141 (shouldUseForTesting) can never be true here. If shouldUseForTesting were true, the if at Line 104 (isInMiniApp || shouldUseForTesting) would have been taken instead. Remove this dead branch.

Apply this diff:

                     }
                 } catch (error) {
                     console.warn("[BaseApp/Farcaster] Failed to get context:", error);
                     // Default to Base App for testing
                     if (shouldUseForTesting) {
                         setIsBaseAppEnv(true);
                     }
                 }
-            } else if (shouldUseForTesting) {
-                // For testing mode, default to Base App
-                setIsBaseAppEnv(true);
-                setBaseAppReady(true);
             }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
} else if (shouldUseForTesting) {
// For testing mode, default to Base App
setIsBaseAppEnv(true);
setBaseAppReady(true);
}
🧰 Tools
🪛 Biome (2.1.2)

[error] 141-141: This branch can never execute. Its condition is a duplicate or covered by previous conditions in the if-else-if chain.

(lint/suspicious/noDuplicateElseIf)

🤖 Prompt for AI Agents
In app/context/BaseAppContext.tsx around lines 141 to 145, the else-if branch
checking shouldUseForTesting is unreachable because the earlier if at line ~104
already handles isInMiniApp || shouldUseForTesting; remove the dead else-if
block (and its setIsBaseAppEnv/setBaseAppReady calls) so there is no redundant
branch, and if any logic from that block is required under different conditions,
move it into the appropriate reachable branch or consolidate it into the earlier
if to preserve intended behavior.

Comment on lines +19 to +31
const isMini =
url.pathname.startsWith("/mini") ||
url.searchParams.get("miniApp") === "true" ||
url.searchParams.get("mini") === "true" ||
url.searchParams.get("baseApp") === "true" ||
!!(window as any).minikit || // Base App MiniKit
window.navigator?.userAgent?.toLowerCase().includes("baseapp") ||
window.navigator?.userAgent?.toLowerCase().includes("farcaster") ||
window.navigator?.userAgent?.toLowerCase().includes("warpcast") ||
window.top !== window.self; // In iframe


if (isMini || true) { // Always call for Base App compatibility
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Remove dead code: isMini is never used due to || true.

The isMini || true condition (Line 31) always evaluates to true, making the entire isMini computation (Lines 19-28) dead code. If the intent is to always call ready() for Base App compatibility, simplify by removing the unused logic.

Apply this diff:

-    // Check for mini app conditions (Farcaster docs pattern + Base App)
-    const url = new URL(window.location.href);
-    const isMini =
-      url.pathname.startsWith("/mini") ||
-      url.searchParams.get("miniApp") === "true" ||
-      url.searchParams.get("mini") === "true" ||
-      url.searchParams.get("baseApp") === "true" ||
-      !!(window as any).minikit || // Base App MiniKit
-      window.navigator?.userAgent?.toLowerCase().includes("baseapp") ||
-      window.navigator?.userAgent?.toLowerCase().includes("farcaster") ||
-      window.navigator?.userAgent?.toLowerCase().includes("warpcast") ||
-      window.top !== window.self; // In iframe
-
-    
-    if (isMini || true) { // Always call for Base App compatibility
+    // Always call ready() for Base App compatibility
+    {
       try {

Alternatively, if the detection logic should be preserved for future conditional behavior, remove the || true:

-    if (isMini || true) { // Always call for Base App compatibility
+    if (isMini) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const isMini =
url.pathname.startsWith("/mini") ||
url.searchParams.get("miniApp") === "true" ||
url.searchParams.get("mini") === "true" ||
url.searchParams.get("baseApp") === "true" ||
!!(window as any).minikit || // Base App MiniKit
window.navigator?.userAgent?.toLowerCase().includes("baseapp") ||
window.navigator?.userAgent?.toLowerCase().includes("farcaster") ||
window.navigator?.userAgent?.toLowerCase().includes("warpcast") ||
window.top !== window.self; // In iframe
if (isMini || true) { // Always call for Base App compatibility
const isMini =
url.pathname.startsWith("/mini") ||
url.searchParams.get("miniApp") === "true" ||
url.searchParams.get("mini") === "true" ||
url.searchParams.get("baseApp") === "true" ||
!!(window as any).minikit || // Base App MiniKit
window.navigator?.userAgent?.toLowerCase().includes("baseapp") ||
window.navigator?.userAgent?.toLowerCase().includes("farcaster") ||
window.navigator?.userAgent?.toLowerCase().includes("warpcast") ||
window.top !== window.self; // In iframe
if (isMini) {
🧰 Tools
🪛 Biome (2.1.2)

[error] 31-31: Unexpected constant condition.

(lint/correctness/noConstantCondition)

🤖 Prompt for AI Agents
In app/early-ready.tsx around lines 19–31, the condition `if (isMini || true)`
makes the `isMini` computation dead code; either remove the unused detection
logic and call ready() unconditionally (delete the `isMini` declaration and the
`if` wrapping so ready() is invoked directly), or preserve the detection logic
by removing the `|| true` so the `if (isMini)` check behaves as intended—pick
one approach and update the code accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deploy Noblocks into Base App Deploy Noblocks as Farcaster Mini App using MiniKit

3 participants