Streamline Farcaster onboarding #1616
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThis PR refactors FID registration to make signing optional, enabling wallet-based FID inference as a primary path before falling back to signer-based registration. It propagates a new Changes
Sequence DiagramsequenceDiagram
participant User as User Action
participant App as LoggedInStateProvider
participant Wallet as Wallet/Identity
participant API as FID Inference API
participant Auth as Authenticator Manager
participant Signer as Farcaster Signer
participant Store as FarcasterStore
participant DB as Database
User->>App: Load App / Register Account
App->>Wallet: Fetch currentIdentity.associatedFids
alt FIDs Available
Wallet-->>App: Return associatedFids array
App->>Store: registerFidForCurrentIdentity(fid, null, null)
Store->>DB: Register FID without signature
DB-->>Store: Success
else No FIDs
Wallet-->>App: No associatedFids
App->>Wallet: Get wallet address
App->>API: inferFidFromWallet(walletAddress)
alt FID Found
API-->>App: Return inferred FID
App->>Store: registerFidForCurrentIdentity(fid, null, null)
Store->>DB: Register FID without signature
else FID Not Found
API-->>App: Not found / Error
App->>Auth: waitForAuthenticator(FARCASTER_AUTHENTICATOR_NAME)
Auth-->>App: Authenticator ready
App->>Signer: getAccountFid()
Signer-->>App: Return FID + signer public key
App->>Signer: Create signature via signMessage
Signer-->>App: Return signature
App->>Store: registerFidForCurrentIdentity(fid, pubKey, signMessage)
Store->>DB: Register FID with signature
DB-->>Store: Success
end
end
App-->>User: Registration complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Testing PR #1616 LocallyPulled the branch and tested the migration + API changes. Setupgit checkout codex/update-user-sign-up-to-infer-fid
supabase db reset # applied all migrations including 20241002120000_allow_null_signer_fields.sqlAPI Tests1. Register FID without signer (new behavior)curl -X POST http://localhost:3000/api/fid-link \
-H "Content-Type: application/json" \
-d '{
"fid": 12345,
"identityPublicKey": "0x1234567890abcdef1234567890abcdef12345678",
"timestamp": "2025-12-09T23:00:00.000Z"
}'Response: {
"result": "success",
"value": {
"fid": 12345,
"signature": null,
"signingPublicKey": null,
"isSigningKeyValid": false
}
}✓ Works - FID registered without signer data 2. Lookup FID by identitycurl "http://localhost:3000/api/fid-link?identityPublicKey=0x1234567890abcdef1234567890abcdef12345678"Response: {
"result": "success",
"value": {
"identity": "0x1234567890abcdef1234567890abcdef12345678",
"fids": [12345]
}
}✓ Works 3. Validation with invalid signerTried posting with invalid signingPublicKey - correctly rejected with 500 (Neynar validation). ✓ Works Database CheckVerified fidRegistrations table via Supabase Studio:
NotesThe type guard in The migration is safe - just drops NOT NULL constraints, doesn't touch existing data. Frontend Issue During LoginTried testing the login flow in the browser but hit an error. Console shows: Also seeing: Looks like there might be an issue with the Farcaster profile component or missing env vars in local setup. Not sure if this is related to the PR changes or just my local config, but couldn't complete the full onboarding flow because of this. ResultBackend/API working as expected. Migration safe to apply to staging. The API changes are solid - FID registration without signer works correctly. Couldn't fully test the frontend flow (FID inference, on-demand signer prompts) due to the error above, but the core functionality this PR introduces at the API/DB level is working. |
…hout signer - Add FID inference from wallet address during onboarding - Allow FID registration without signer keys (signature/signingPublicKey can be null) - Update registerAccounts flow to try inference first, only fallback to signer if needed - Fix /api/farcaster/neynar/users endpoint to support both addresses and fids params - Update currentUserFid to use associatedFids first, works without signer - Add error handling and logging throughout the flow
Skip REQUIRED_AUTHENTICATORS_INSTALLED step when there are no required authenticators to avoid getting stuck in the flow
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/fidgets/farcaster/components/CreateCast.tsx (1)
418-434: Misleading log statement after signer guard.The
signerUndefinedfield in the error log at line 430 will always befalseat this point since the guard clause on lines 419-422 ensuressigneris defined. Consider removing it or updating the log message.if (!draft?.text && !draft?.embeds?.length) { console.error( - "Submission failed: Missing text or embeds, or signer is undefined.", + "Submission failed: Missing text or embeds.", { draftText: draft?.text, draftEmbedsLength: draft?.embeds?.length, - signerUndefined: isUndefined(signer), }, ); return false; }
🧹 Nitpick comments (9)
src/pages/api/farcaster/neynar/users.ts (2)
5-17: LGTM! Query parsing helpers are well-structured.The helper functions correctly handle both array and string query parameter formats and provide sensible fallbacks. The
getFidsfunction appropriately filters out invalid numeric conversions.Optional enhancement: Consider adding validation to ensure addresses are valid Ethereum/Solana format and that fids are positive integers. This would provide earlier failure feedback rather than relying on downstream Neynar API errors.
36-36: Consider extracting viewer_fid parsing for consistency.The
viewer_fidparameter is parsed inline, whileaddressesandfidsuse dedicated helper functions. For consistency and potential reuse, consider creating agetViewerFidhelper.Example helper:
const getViewerFid = (query: NextApiRequest["query"]) => { const raw = query.viewer_fid; if (typeof raw === "string") { const num = Number(raw); return isNaN(num) ? undefined : num; } return undefined; };Then use it:
viewerFid: getViewerFid(req.query),src/constants/requiredAuthenticators.ts (1)
1-1: Add a comment explaining why authenticators are now optional.The change from
["farcaster:nounspace"]to an empty array represents a significant shift in authentication requirements, moving from mandatory to on-demand signer authorization. A brief comment would help future maintainers understand this intentional change and its relationship to the newrequestSignerAuthorizationflow.Consider adding:
+// Authenticators are now installed on-demand via requestSignerAuthorization +// rather than being required upfront during initialization export default [];src/pages/api/fid-link.ts (1)
68-122: Consider validating both signing fields or neither.The current logic uses
!!reqBody.signingPublicKeyto determine if signing info is present (line 68). This means:
- If only
signatureis provided withoutsigningPublicKey, it's silently ignored- If only
signingPublicKeyis provided withoutsignature, validation fails at line 73While not incorrect, this asymmetry could be confusing. Consider either:
- Checking both fields:
hasSigningKeyInfo = !!(reqBody.signingPublicKey && reqBody.signature)- Adding a validation error if partial signing data is provided
- const hasSigningKeyInfo = !!reqBody.signingPublicKey; + const hasSigningKeyInfo = !!(reqBody.signingPublicKey && reqBody.signature); + + // Reject partial signing data + if ((!!reqBody.signingPublicKey) !== (!!reqBody.signature)) { + res.status(400).json({ + result: "error", + error: { + message: "Both signature and signingPublicKey must be provided together or omitted together", + }, + }); + return; + }src/fidgets/ui/profile.tsx (1)
68-110: Consider error handling for authorization failures.The new two-step guard pattern is cleaner:
- Check signer availability and request authorization if needed
- Proceed with follow/unfollow only if user exists
However,
requestSignerAuthorization()is awaited but any errors are not caught. If authorization fails or the user cancels, the function returns silently. While this may be acceptable UX (no action = no error), consider whether user feedback would be helpful.If user feedback is desired:
const toggleFollowing = async () => { if (!signer || viewerFid <= 0) { - await requestSignerAuthorization(); - return; + try { + await requestSignerAuthorization(); + } catch (error) { + // Optional: show toast or error message + console.error("Authorization failed:", error); + } + return; }src/fidgets/farcaster/components/CastRow.tsx (1)
370-424: Consider error handling for authorization failures.The updated logic properly requests signer authorization when needed before allowing like/recast actions. However, similar to the profile component, errors from
requestSignerAuthorization()are not caught. Consider whether user feedback for authorization failures would improve the UX.If error feedback is desired:
// We check if we have the signer before proceeding if (isUndefined(signer) || userFid < 0) { - await requestSignerAuthorization(); + try { + await requestSignerAuthorization(); + } catch (error) { + console.error("Signer authorization failed:", error); + // Optionally show toast notification + } return; }src/common/data/stores/app/accounts/farcasterStore.ts (1)
79-83: Consider reducing log verbosity for production.The detailed logging is helpful for debugging the new optional signing flow. Once the feature is stable, consider using a debug-level logger or removing these logs to reduce noise in production.
src/fidgets/farcaster/index.tsx (1)
129-177: Consider preventing duplicate FID registration attempts.The
syncFidRegistrationeffect runs whenever dependencies change (includingauthenticatorManager.lastUpdatedAt). If the effect fires multiple times before the first registration completes, it could trigger duplicate API calls.Consider adding a guard to track registration state:
+const [isRegistering, setIsRegistering] = useState(false); useEffect(() => { - if (!hasRequestedSigner || !signer || fid < 0) return; + if (!hasRequestedSigner || !signer || fid < 0 || isRegistering) return; const syncFidRegistration = async () => { + setIsRegistering(true); try { // ... existing logic } catch (error) { console.error("Error syncing FID registration with signer:", error); + } finally { + setIsRegistering(false); } }; syncFidRegistration(); -}, [...deps]); +}, [...deps, isRegistering]);Alternatively, if the backend handles duplicate registrations idempotently (e.g., returns existing record), this may be acceptable.
src/common/providers/LoggedInStateProvider.tsx (1)
276-287: Consider adding type annotation formessageHashparameter.The
messageHashparameter lacks a type annotation. Adding one improves code clarity and type safety.- const signForFid = async (messageHash) => { + const signForFid = async (messageHash: Uint8Array) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
src/app/(spaces)/PublicSpace.tsx(3 hunks)src/common/data/stores/app/accounts/farcasterStore.ts(2 hunks)src/common/providers/LoggedInStateProvider.tsx(3 hunks)src/constants/requiredAuthenticators.ts(1 hunks)src/fidgets/farcaster/components/CastRow.tsx(2 hunks)src/fidgets/farcaster/components/CreateCast.tsx(3 hunks)src/fidgets/farcaster/index.tsx(2 hunks)src/fidgets/token/Directory/Directory.tsx(3 hunks)src/fidgets/token/Directory/components/DirectoryCardView.tsx(3 hunks)src/fidgets/token/Directory/components/DirectoryFollowButton.tsx(2 hunks)src/fidgets/token/Directory/components/DirectoryListView.tsx(3 hunks)src/fidgets/ui/profile.tsx(2 hunks)src/pages/api/farcaster/neynar/users.ts(1 hunks)src/pages/api/fid-link.ts(6 hunks)src/supabase/database.d.ts(1 hunks)supabase/migrations/20241002120000_allow_null_signer_fields.sql(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-28T21:31:10.654Z
Learnt from: CR
Repo: Nounspace/nounspace.ts PR: 0
File: docs/DEVELOPMENT/AGENTS.md:0-0
Timestamp: 2025-11-28T21:31:10.654Z
Learning: Ensure authentication state follows the `SetupStep` lifecycle and integrate with the authenticator manager for platform-specific flows
Applied to files:
src/common/providers/LoggedInStateProvider.tsx
🧬 Code graph analysis (8)
src/fidgets/token/Directory/Directory.tsx (1)
src/fidgets/farcaster/index.tsx (1)
useFarcasterSigner(61-186)
src/fidgets/ui/profile.tsx (1)
src/fidgets/farcaster/index.tsx (1)
useFarcasterSigner(61-186)
src/fidgets/farcaster/components/CastRow.tsx (1)
src/fidgets/farcaster/index.tsx (1)
useFarcasterSigner(61-186)
src/fidgets/farcaster/components/CreateCast.tsx (1)
src/fidgets/farcaster/index.tsx (1)
useFarcasterSigner(61-186)
src/common/providers/LoggedInStateProvider.tsx (1)
src/fidgets/farcaster/index.tsx (1)
FARCASTER_AUTHENTICATOR_NAME(12-12)
src/common/data/stores/app/accounts/farcasterStore.ts (4)
src/common/lib/wallets.ts (1)
signMessage(14-22)src/pages/api/fid-link.ts (2)
FidLinkToIdentityRequest(11-17)FidLinkToIdentityResponse(35-42)src/common/lib/signedFiles.ts (1)
hashObject(25-27)src/common/providers/AnalyticsProvider.tsx (1)
analytics(11-33)
src/pages/api/fid-link.ts (2)
src/common/data/api/requestHandler.ts (1)
NounspaceResponse(9-16)src/common/lib/signedFiles.ts (1)
validateSignable(43-52)
src/app/(spaces)/PublicSpace.tsx (1)
src/common/data/stores/app/index.tsx (1)
useAppStore(168-168)
🔇 Additional comments (22)
src/pages/api/farcaster/neynar/users.ts (1)
40-50: LGTM! Error handling is well-structured.The error handling appropriately distinguishes between client errors (400 for missing parameters) and server errors (500 for Neynar API failures). The structured error responses with descriptive messages will help API consumers handle errors gracefully.
src/supabase/database.d.ts (1)
186-217: LGTM! Type definitions correctly reflect nullable signing fields.The updated type definitions properly align with the migration that allows FID registration without signing keys:
- Row types correctly reflect that signing fields can be null
- Insert types make signing fields optional with proper null handling
isSigningKeyValidremains non-nullable in Row (consistent with database default offalse)supabase/migrations/20241002120000_allow_null_signer_fields.sql (1)
1-5: LGTM! Migration safely enables optional signer fields.The migration correctly:
- Drops NOT NULL constraints on signing-related fields to support FID registration without signers
- Sets a sensible default (
false) forisSigningKeyValid- Makes no destructive changes to existing data
Based on the PR comments, this has been tested and verified to work correctly.
src/pages/api/fid-link.ts (3)
19-33: LGTM! Type guard correctly updated for optional signing fields.The refactored type guard properly validates only the required fields (
fid,timestamp,identityPublicKey) and no longer enforcesisSignable, aligning with the new optional signer flow.
44-51: LGTM! Signing key validation function correctly implemented.The function properly validates that a signing key is approved for the given FID via Neynar, with appropriate error handling.
216-216: LGTM! Query correctly returns all FIDs for an identity.Removing the
isSigningKeyValidfilter is correct since FIDs can now be registered without valid signers, and signer authorization happens on-demand.src/fidgets/token/Directory/Directory.tsx (2)
163-164: LGTM! Correctly integrates signer authorization.The component properly obtains
requestSignerAuthorizationfrom theuseFarcasterSignerhook and will forward it to child components.
1056-1081: LGTM! Signer authorization correctly propagated to view components.Both
DirectoryListViewandDirectoryCardViewreceive therequestSignerAuthorizationcallback, enabling on-demand signer authorization in follow/unfollow flows.src/fidgets/token/Directory/components/DirectoryListView.tsx (2)
39-51: LGTM! Prop signature correctly updated.The component properly accepts and destructures the optional
requestSignerAuthorizationcallback for forwarding to child components.
126-132: LGTM! Authorization callback correctly forwarded.The
DirectoryFollowButtonreceives therequestSignerAuthorizationprop, enabling it to trigger signer authorization when needed.src/fidgets/ui/profile.tsx (1)
48-49: LGTM! Signer authorization correctly integrated.The component properly destructures
requestSignerAuthorizationfrom the hook for use in the follow/unfollow flow.src/fidgets/farcaster/components/CastRow.tsx (1)
316-317: LGTM! Signer authorization correctly integrated.The component properly obtains
requestSignerAuthorizationfrom the hook for use in the reaction flow.src/app/(spaces)/PublicSpace.tsx (2)
109-110: LGTM! Clean extraction of identity state.The use of optional chaining with a fallback to an empty array for
associatedFidsis appropriate and prevents undefined errors.
127-153: LGTM! Well-structured FID resolution with clear priority.The three-branch logic correctly implements the PR objective:
- Prefer
associatedFids(wallet-inferred FIDs without signer)- Fall back to authenticator if signed into Farcaster
- Explicitly clear to
nullotherwiseThe dependency array correctly includes
associatedFidsfor proper reactivity.src/fidgets/token/Directory/components/DirectoryFollowButton.tsx (1)
57-62: Verify the intended UX after signer authorization.After
requestSignerAuthorization()completes, the handler returns early without retrying the follow action. This means users must click "Follow" again after authorization.If this is intentional (i.e., authorization is a separate step), the code is correct. If the expectation is that following happens automatically after authorization, consider re-invoking the follow logic after
await requestSignerAuthorization()completes successfully.src/fidgets/token/Directory/components/DirectoryCardView.tsx (1)
40-53: LGTM! Clean prop propagation.The
requestSignerAuthorizationcallback is correctly threaded from props to the childDirectoryFollowButtoncomponent.src/fidgets/farcaster/index.tsx (1)
84-88: LGTM! Simple FID sync from identity.The effect correctly syncs the first associated FID to local state when
currentIdentityFidschanges.src/fidgets/farcaster/components/CreateCast.tsx (2)
170-171: LGTM!The destructuring of
requestSignerAuthorizationfromuseFarcasterSigneraligns with the hook's updated return type and enables on-demand signer authorization flow.
732-739: LGTM!The button text logic correctly shows "Connect Farcaster" when the signer is not available, guiding users to authorize. Minor note: this uses
!signerwhileonSubmitPostusesisUndefined(signer)- both work but consider using consistent checks throughout.src/common/providers/LoggedInStateProvider.tsx (3)
9-9: LGTM!Import of
FARCASTER_AUTHENTICATOR_NAMEfrom the centralized location ensures consistency across the codebase.
188-201: LGTM!The polling mechanism is a reasonable approach for waiting on async authenticator initialization. The 10-second default timeout (10 attempts × 1000ms) is appropriate for this use case.
214-223: LGTM!Good optimization to skip authenticator installation when
requiredAuthenticatorsis empty, directly advancing toAUTHENTICATORS_INITIALIZED.
| const baseRequest: FidLinkToIdentityRequest = { | ||
| fid, | ||
| identityPublicKey: get().account.currentSpaceIdentityPublicKey!, | ||
| timestamp: moment().toISOString(), | ||
| signingPublicKey: signingKey, | ||
| }; | ||
| const signedRequest: FidLinkToIdentityRequest = { | ||
| ...request, | ||
| signature: bytesToHex(await signMessage(hashObject(request))), | ||
| signingPublicKey: signingKey ?? null, | ||
| signature: null, | ||
| }; |
There was a problem hiding this comment.
Consider handling undefined currentSpaceIdentityPublicKey.
Line 68 uses a non-null assertion (!) on currentSpaceIdentityPublicKey. If this value is undefined (e.g., no identity is loaded), the request will contain undefined as the identity key, which could cause a server-side error or unexpected behavior.
Consider adding a guard:
registerFidForCurrentIdentity: async (fid, signingKey, signMessage) => {
console.log("[registerFidForCurrentIdentity] Starting registration:", { fid, hasSigningKey: !!signingKey });
if (signingKey && !signMessage) {
throw new Error("signMessage is required when signingKey is provided");
}
+ const identityPublicKey = get().account.currentSpaceIdentityPublicKey;
+ if (!identityPublicKey) {
+ throw new Error("No identity public key available");
+ }
const baseRequest: FidLinkToIdentityRequest = {
fid,
- identityPublicKey: get().account.currentSpaceIdentityPublicKey!,
+ identityPublicKey,
timestamp: moment().toISOString(),📝 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.
| const baseRequest: FidLinkToIdentityRequest = { | |
| fid, | |
| identityPublicKey: get().account.currentSpaceIdentityPublicKey!, | |
| timestamp: moment().toISOString(), | |
| signingPublicKey: signingKey, | |
| }; | |
| const signedRequest: FidLinkToIdentityRequest = { | |
| ...request, | |
| signature: bytesToHex(await signMessage(hashObject(request))), | |
| signingPublicKey: signingKey ?? null, | |
| signature: null, | |
| }; | |
| const identityPublicKey = get().account.currentSpaceIdentityPublicKey; | |
| if (!identityPublicKey) { | |
| throw new Error("No identity public key available"); | |
| } | |
| const baseRequest: FidLinkToIdentityRequest = { | |
| fid, | |
| identityPublicKey, | |
| timestamp: moment().toISOString(), | |
| signingPublicKey: signingKey ?? null, | |
| signature: null, | |
| }; |
🤖 Prompt for AI Agents
In src/common/data/stores/app/accounts/farcasterStore.ts around lines 66 to 72,
the code uses a non-null assertion on
get().account.currentSpaceIdentityPublicKey which can be undefined; change this
to guard against a missing identity by checking if currentSpaceIdentityPublicKey
is present before building the request and either (a) throw or return a rejected
Promise with a clear error/error code indicating "no current space identity
loaded", or (b) attempt to load/resolve the identity first and then proceed;
ensure you do not insert undefined into the request object and update callers to
handle the thrown/rejected result accordingly.
| const inferFidFromWallet = async (): Promise<number | undefined> => { | ||
| if (!user?.wallet?.address) { | ||
| console.log("[inferFidFromWallet] No wallet address available"); | ||
| return undefined; | ||
| } | ||
| try { | ||
| console.log("[inferFidFromWallet] Fetching FID for wallet:", user.wallet.address); | ||
| const response = await fetch( | ||
| `/api/farcaster/neynar/users?addresses=${user.wallet.address}`, | ||
| ); | ||
| if (!response.ok) { | ||
| console.log("[inferFidFromWallet] API response not OK:", response.status, response.statusText); | ||
| return undefined; | ||
| } | ||
| const data = await response.json(); | ||
| console.log("[inferFidFromWallet] API response data:", data); | ||
| const users = data?.users ?? []; | ||
| if (users.length === 0) { | ||
| console.log("[inferFidFromWallet] No users found in response"); | ||
| return undefined; | ||
| } | ||
|
|
||
| const walletLower = user.wallet.address.toLowerCase(); | ||
| const matchingUser = users.find((u: any) => { | ||
| // Check verified_addresses.primary.eth_address | ||
| if (u.verified_addresses?.primary?.eth_address?.toLowerCase() === walletLower) { | ||
| return true; | ||
| } | ||
| // Check verified_addresses.eth_addresses array | ||
| if (u.verified_addresses?.eth_addresses?.some( | ||
| (addr: string) => addr.toLowerCase() === walletLower | ||
| )) { | ||
| return true; | ||
| } | ||
| // Check verifications array (fallback) | ||
| if (u.verifications?.some( | ||
| (addr: string) => addr.toLowerCase() === walletLower | ||
| )) { | ||
| return true; | ||
| } | ||
| return false; | ||
| }); | ||
|
|
||
| const fid = matchingUser?.fid ?? users[0]?.fid; | ||
| console.log("[inferFidFromWallet] Found FID:", fid, "from matching user:", !!matchingUser); | ||
| return fid; | ||
| } catch (e) { | ||
| console.error("[inferFidFromWallet] Error inferring FID from wallet:", e); | ||
| return undefined; | ||
| } | ||
| }; |
There was a problem hiding this comment.
Potential incorrect FID association with fallback to users[0].
Line 179 falls back to users[0]?.fid when no matching user is found. If the API returns users that don't have the wallet address verified, this could incorrectly associate an unrelated FID with the current identity.
Consider returning undefined when no explicit match is found:
- const fid = matchingUser?.fid ?? users[0]?.fid;
- console.log("[inferFidFromWallet] Found FID:", fid, "from matching user:", !!matchingUser);
- return fid;
+ if (!matchingUser) {
+ console.log("[inferFidFromWallet] No matching user found for wallet");
+ return undefined;
+ }
+ console.log("[inferFidFromWallet] Found FID:", matchingUser.fid);
+ return matchingUser.fid;📝 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.
| const inferFidFromWallet = async (): Promise<number | undefined> => { | |
| if (!user?.wallet?.address) { | |
| console.log("[inferFidFromWallet] No wallet address available"); | |
| return undefined; | |
| } | |
| try { | |
| console.log("[inferFidFromWallet] Fetching FID for wallet:", user.wallet.address); | |
| const response = await fetch( | |
| `/api/farcaster/neynar/users?addresses=${user.wallet.address}`, | |
| ); | |
| if (!response.ok) { | |
| console.log("[inferFidFromWallet] API response not OK:", response.status, response.statusText); | |
| return undefined; | |
| } | |
| const data = await response.json(); | |
| console.log("[inferFidFromWallet] API response data:", data); | |
| const users = data?.users ?? []; | |
| if (users.length === 0) { | |
| console.log("[inferFidFromWallet] No users found in response"); | |
| return undefined; | |
| } | |
| const walletLower = user.wallet.address.toLowerCase(); | |
| const matchingUser = users.find((u: any) => { | |
| // Check verified_addresses.primary.eth_address | |
| if (u.verified_addresses?.primary?.eth_address?.toLowerCase() === walletLower) { | |
| return true; | |
| } | |
| // Check verified_addresses.eth_addresses array | |
| if (u.verified_addresses?.eth_addresses?.some( | |
| (addr: string) => addr.toLowerCase() === walletLower | |
| )) { | |
| return true; | |
| } | |
| // Check verifications array (fallback) | |
| if (u.verifications?.some( | |
| (addr: string) => addr.toLowerCase() === walletLower | |
| )) { | |
| return true; | |
| } | |
| return false; | |
| }); | |
| const fid = matchingUser?.fid ?? users[0]?.fid; | |
| console.log("[inferFidFromWallet] Found FID:", fid, "from matching user:", !!matchingUser); | |
| return fid; | |
| } catch (e) { | |
| console.error("[inferFidFromWallet] Error inferring FID from wallet:", e); | |
| return undefined; | |
| } | |
| }; | |
| const inferFidFromWallet = async (): Promise<number | undefined> => { | |
| if (!user?.wallet?.address) { | |
| console.log("[inferFidFromWallet] No wallet address available"); | |
| return undefined; | |
| } | |
| try { | |
| console.log("[inferFidFromWallet] Fetching FID for wallet:", user.wallet.address); | |
| const response = await fetch( | |
| `/api/farcaster/neynar/users?addresses=${user.wallet.address}`, | |
| ); | |
| if (!response.ok) { | |
| console.log("[inferFidFromWallet] API response not OK:", response.status, response.statusText); | |
| return undefined; | |
| } | |
| const data = await response.json(); | |
| console.log("[inferFidFromWallet] API response data:", data); | |
| const users = data?.users ?? []; | |
| if (users.length === 0) { | |
| console.log("[inferFidFromWallet] No users found in response"); | |
| return undefined; | |
| } | |
| const walletLower = user.wallet.address.toLowerCase(); | |
| const matchingUser = users.find((u: any) => { | |
| // Check verified_addresses.primary.eth_address | |
| if (u.verified_addresses?.primary?.eth_address?.toLowerCase() === walletLower) { | |
| return true; | |
| } | |
| // Check verified_addresses.eth_addresses array | |
| if (u.verified_addresses?.eth_addresses?.some( | |
| (addr: string) => addr.toLowerCase() === walletLower | |
| )) { | |
| return true; | |
| } | |
| // Check verifications array (fallback) | |
| if (u.verifications?.some( | |
| (addr: string) => addr.toLowerCase() === walletLower | |
| )) { | |
| return true; | |
| } | |
| return false; | |
| }); | |
| if (!matchingUser) { | |
| console.log("[inferFidFromWallet] No matching user found for wallet"); | |
| return undefined; | |
| } | |
| console.log("[inferFidFromWallet] Found FID:", matchingUser.fid); | |
| return matchingUser.fid; | |
| } catch (e) { | |
| console.error("[inferFidFromWallet] Error inferring FID from wallet:", e); | |
| return undefined; | |
| } | |
| }; |
🤖 Prompt for AI Agents
In src/common/providers/LoggedInStateProvider.tsx around lines 136 to 186, the
current logic falls back to users[0]?.fid when no matching user is found which
can incorrectly associate an unrelated FID; change the behavior to return
undefined when no explicit wallet match exists by removing the users[0]
fallback, set fid to matchingUser?.fid (or undefined), and update the log to
clearly state when no matching FID was found before returning undefined.
| if (currentIdentity.associatedFids.length === 0) { | ||
| console.log("[registerAccounts] No FID found/inferred, falling back to signer flow..."); | ||
| await authenticatorManager.installAuthenticators([ | ||
| FARCASTER_AUTHENTICATOR_NAME, | ||
| ]); | ||
| authenticatorManager.initializeAuthenticators([ | ||
| FARCASTER_AUTHENTICATOR_NAME, | ||
| ]); | ||
| const signerReady = await waitForAuthenticator(FARCASTER_AUTHENTICATOR_NAME); | ||
| if (signerReady) { | ||
| try { | ||
| const fidResult = (await authenticatorManager.callMethod({ | ||
| requestingFidgetId: "root", | ||
| authenticatorId: FARCASTER_AUTHENTICATOR_NAME, | ||
| methodName: "getAccountFid", | ||
| isLookup: true, | ||
| })) as { value: number }; | ||
| const publicKeyResult = (await authenticatorManager.callMethod({ | ||
| requestingFidgetId: "root", | ||
| authenticatorId: "farcaster:nounspace", | ||
| methodName: "signMessage", | ||
| isLookup: false, | ||
| }, | ||
| messageHash, | ||
| )) as { value: Uint8Array }; | ||
| return signResult.value; | ||
| }; | ||
| await registerFidForCurrentIdentity( | ||
| fidResult.value, | ||
| bytesToHex(publicKeyResult.value), | ||
| signForFid, | ||
| ); | ||
| authenticatorId: FARCASTER_AUTHENTICATOR_NAME, | ||
| methodName: "getSignerPublicKey", | ||
| isLookup: true, | ||
| })) as { value: Uint8Array }; | ||
| const signForFid = async (messageHash) => { | ||
| const signResult = (await authenticatorManager.callMethod( | ||
| { | ||
| requestingFidgetId: "root", | ||
| authenticatorId: FARCASTER_AUTHENTICATOR_NAME, | ||
| methodName: "signMessage", | ||
| isLookup: false, | ||
| }, | ||
| messageHash, | ||
| )) as { value: Uint8Array }; | ||
| return signResult.value; | ||
| }; | ||
| await registerFidForCurrentIdentity( | ||
| fidResult.value, | ||
| bytesToHex(publicKeyResult.value), | ||
| signForFid, | ||
| ); | ||
| } catch (e) { | ||
| console.error("Error registering FID with signer:", e); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| setCurrentStep(SetupStep.ACCOUNTS_REGISTERED); |
There was a problem hiding this comment.
Silent failure when authenticator initialization times out.
If waitForAuthenticator returns false (line 261), the code silently proceeds to setCurrentStep(SetupStep.ACCOUNTS_REGISTERED) at line 300 without registering any FID. This could leave the user in an inconsistent state where they appear registered but have no associated FID.
Consider logging a warning or handling this case explicitly:
const signerReady = await waitForAuthenticator(FARCASTER_AUTHENTICATOR_NAME);
if (signerReady) {
try {
// ... existing signer registration code
} catch (e) {
console.error("Error registering FID with signer:", e);
}
+ } else {
+ console.warn("[registerAccounts] Authenticator initialization timed out, proceeding without FID registration");
}📝 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.
| if (currentIdentity.associatedFids.length === 0) { | |
| console.log("[registerAccounts] No FID found/inferred, falling back to signer flow..."); | |
| await authenticatorManager.installAuthenticators([ | |
| FARCASTER_AUTHENTICATOR_NAME, | |
| ]); | |
| authenticatorManager.initializeAuthenticators([ | |
| FARCASTER_AUTHENTICATOR_NAME, | |
| ]); | |
| const signerReady = await waitForAuthenticator(FARCASTER_AUTHENTICATOR_NAME); | |
| if (signerReady) { | |
| try { | |
| const fidResult = (await authenticatorManager.callMethod({ | |
| requestingFidgetId: "root", | |
| authenticatorId: FARCASTER_AUTHENTICATOR_NAME, | |
| methodName: "getAccountFid", | |
| isLookup: true, | |
| })) as { value: number }; | |
| const publicKeyResult = (await authenticatorManager.callMethod({ | |
| requestingFidgetId: "root", | |
| authenticatorId: "farcaster:nounspace", | |
| methodName: "signMessage", | |
| isLookup: false, | |
| }, | |
| messageHash, | |
| )) as { value: Uint8Array }; | |
| return signResult.value; | |
| }; | |
| await registerFidForCurrentIdentity( | |
| fidResult.value, | |
| bytesToHex(publicKeyResult.value), | |
| signForFid, | |
| ); | |
| authenticatorId: FARCASTER_AUTHENTICATOR_NAME, | |
| methodName: "getSignerPublicKey", | |
| isLookup: true, | |
| })) as { value: Uint8Array }; | |
| const signForFid = async (messageHash) => { | |
| const signResult = (await authenticatorManager.callMethod( | |
| { | |
| requestingFidgetId: "root", | |
| authenticatorId: FARCASTER_AUTHENTICATOR_NAME, | |
| methodName: "signMessage", | |
| isLookup: false, | |
| }, | |
| messageHash, | |
| )) as { value: Uint8Array }; | |
| return signResult.value; | |
| }; | |
| await registerFidForCurrentIdentity( | |
| fidResult.value, | |
| bytesToHex(publicKeyResult.value), | |
| signForFid, | |
| ); | |
| } catch (e) { | |
| console.error("Error registering FID with signer:", e); | |
| } | |
| } | |
| } | |
| } | |
| } | |
| setCurrentStep(SetupStep.ACCOUNTS_REGISTERED); | |
| if (currentIdentity.associatedFids.length === 0) { | |
| console.log("[registerAccounts] No FID found/inferred, falling back to signer flow..."); | |
| await authenticatorManager.installAuthenticators([ | |
| FARCASTER_AUTHENTICATOR_NAME, | |
| ]); | |
| authenticatorManager.initializeAuthenticators([ | |
| FARCASTER_AUTHENTICATOR_NAME, | |
| ]); | |
| const signerReady = await waitForAuthenticator(FARCASTER_AUTHENTICATOR_NAME); | |
| if (signerReady) { | |
| try { | |
| const fidResult = (await authenticatorManager.callMethod({ | |
| requestingFidgetId: "root", | |
| authenticatorId: FARCASTER_AUTHENTICATOR_NAME, | |
| methodName: "getAccountFid", | |
| isLookup: true, | |
| })) as { value: number }; | |
| const publicKeyResult = (await authenticatorManager.callMethod({ | |
| requestingFidgetId: "root", | |
| authenticatorId: FARCASTER_AUTHENTICATOR_NAME, | |
| methodName: "getSignerPublicKey", | |
| isLookup: true, | |
| })) as { value: Uint8Array }; | |
| const signForFid = async (messageHash) => { | |
| const signResult = (await authenticatorManager.callMethod( | |
| { | |
| requestingFidgetId: "root", | |
| authenticatorId: FARCASTER_AUTHENTICATOR_NAME, | |
| methodName: "signMessage", | |
| isLookup: false, | |
| }, | |
| messageHash, | |
| )) as { value: Uint8Array }; | |
| return signResult.value; | |
| }; | |
| await registerFidForCurrentIdentity( | |
| fidResult.value, | |
| bytesToHex(publicKeyResult.value), | |
| signForFid, | |
| ); | |
| } catch (e) { | |
| console.error("Error registering FID with signer:", e); | |
| } | |
| } else { | |
| console.warn("[registerAccounts] Authenticator initialization timed out, proceeding without FID registration"); | |
| } | |
| } | |
| } | |
| } | |
| setCurrentStep(SetupStep.ACCOUNTS_REGISTERED); |
🤖 Prompt for AI Agents
In src/common/providers/LoggedInStateProvider.tsx around lines 253 to 300, when
waitForAuthenticator returns false the flow silently continues and later calls
setCurrentStep(SetupStep.ACCOUNTS_REGISTERED) even though no FID was registered;
update the control flow to handle the timeout case explicitly: if signerReady is
false, log a warning/error, avoid advancing to ACCOUNTS_REGISTERED (either set
an error/failure step or keep the current step), and surface the failure to the
user/telemetry so the UI does not show the account as registered without an FID.
| const requestSignerAuthorization = useCallback(async () => { | ||
| setHasRequestedSigner(true); | ||
| setIsLoadingSigner(true); | ||
| await authenticatorManager.installAuthenticators([authenticatorName]); | ||
| authenticatorManager.initializeAuthenticators([authenticatorName]); | ||
| }, [authenticatorManager, authenticatorName]); |
There was a problem hiding this comment.
Missing await on initializeAuthenticators may cause race conditions.
Line 81 calls initializeAuthenticators without await, meaning the function returns before initialization completes. The subsequent effect (lines 90-127) relies on authenticatorManager.lastUpdatedAt changing to detect when initialization is done, but this could lead to timing issues if the caller expects requestSignerAuthorization to fully complete before continuing.
Consider awaiting the initialization:
const requestSignerAuthorization = useCallback(async () => {
setHasRequestedSigner(true);
setIsLoadingSigner(true);
await authenticatorManager.installAuthenticators([authenticatorName]);
- authenticatorManager.initializeAuthenticators([authenticatorName]);
+ await authenticatorManager.initializeAuthenticators([authenticatorName]);
}, [authenticatorManager, authenticatorName]);📝 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.
| const requestSignerAuthorization = useCallback(async () => { | |
| setHasRequestedSigner(true); | |
| setIsLoadingSigner(true); | |
| await authenticatorManager.installAuthenticators([authenticatorName]); | |
| authenticatorManager.initializeAuthenticators([authenticatorName]); | |
| }, [authenticatorManager, authenticatorName]); | |
| const requestSignerAuthorization = useCallback(async () => { | |
| setHasRequestedSigner(true); | |
| setIsLoadingSigner(true); | |
| await authenticatorManager.installAuthenticators([authenticatorName]); | |
| await authenticatorManager.initializeAuthenticators([authenticatorName]); | |
| }, [authenticatorManager, authenticatorName]); |
🤖 Prompt for AI Agents
In src/fidgets/farcaster/index.tsx around lines 77 to 82,
requestSignerAuthorization calls authenticatorManager.initializeAuthenticators
without awaiting it which can cause race conditions; update the function to
await authenticatorManager.initializeAuthenticators([authenticatorName]) so the
callback only resolves after initialization completes, and propagate errors
(try/catch) if needed to ensure setIsLoadingSigner is cleared consistently.
| const signForFid = async (messageHash: Uint8Array) => { | ||
| const signResult = await authenticatorManager.callMethod( | ||
| { | ||
| requestingFidgetId: fidgetId, | ||
| authenticatorId: FARCASTER_AUTHENTICATOR_NAME, | ||
| methodName: "signMessage", | ||
| isLookup: false, | ||
| }, | ||
| messageHash, | ||
| ); | ||
| return signResult.result === "success" | ||
| ? (signResult.value as Uint8Array) | ||
| : new Uint8Array(); | ||
| }; |
There was a problem hiding this comment.
Silent failure in signForFid could produce invalid signatures.
When signing fails, signForFid returns an empty Uint8Array instead of throwing an error. This empty value would be hex-encoded and sent to the server as an invalid signature, resulting in a confusing server-side error rather than a clear client-side failure.
Consider throwing on failure:
const signForFid = async (messageHash: Uint8Array) => {
const signResult = await authenticatorManager.callMethod(
{
requestingFidgetId: fidgetId,
authenticatorId: FARCASTER_AUTHENTICATOR_NAME,
methodName: "signMessage",
isLookup: false,
},
messageHash,
);
- return signResult.result === "success"
- ? (signResult.value as Uint8Array)
- : new Uint8Array();
+ if (signResult.result !== "success") {
+ throw new Error("Failed to sign message for FID registration");
+ }
+ return signResult.value as Uint8Array;
};📝 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.
| const signForFid = async (messageHash: Uint8Array) => { | |
| const signResult = await authenticatorManager.callMethod( | |
| { | |
| requestingFidgetId: fidgetId, | |
| authenticatorId: FARCASTER_AUTHENTICATOR_NAME, | |
| methodName: "signMessage", | |
| isLookup: false, | |
| }, | |
| messageHash, | |
| ); | |
| return signResult.result === "success" | |
| ? (signResult.value as Uint8Array) | |
| : new Uint8Array(); | |
| }; | |
| const signForFid = async (messageHash: Uint8Array) => { | |
| const signResult = await authenticatorManager.callMethod( | |
| { | |
| requestingFidgetId: fidgetId, | |
| authenticatorId: FARCASTER_AUTHENTICATOR_NAME, | |
| methodName: "signMessage", | |
| isLookup: false, | |
| }, | |
| messageHash, | |
| ); | |
| if (signResult.result !== "success") { | |
| throw new Error("Failed to sign message for FID registration"); | |
| } | |
| return signResult.value as Uint8Array; | |
| }; |
🤖 Prompt for AI Agents
In src/fidgets/farcaster/index.tsx around lines 140 to 153, signForFid currently
returns an empty Uint8Array on failure which produces invalid signatures; change
it to throw a descriptive error when signResult.result !== "success" (include
any error message or code from signResult if available) instead of returning an
empty array so callers can handle/abort the operation; update the function's
contract/typing or callers if necessary to accept that it may throw and ensure
any awaiting code catches and surfaces the error to the user.
| if (addresses.length > 0) { | ||
| const response = await neynar.fetchBulkUsersByEthOrSolAddress({ | ||
| addresses, | ||
| }); | ||
| // Transform the response to match the expected format | ||
| // Neynar returns Record<address, User[]>, we need to flatten it | ||
| const users = Object.values(response).flat(); | ||
| return res.status(200).json({ users }); | ||
| } else if (fids.length > 0) { | ||
| const response = await neynar.fetchBulkUsers({ | ||
| fids, | ||
| viewerFid: req.query.viewer_fid ? Number(req.query.viewer_fid) : undefined, | ||
| }); | ||
| return res.status(200).json(response); |
There was a problem hiding this comment.
Inconsistent response shapes between query paths.
The addresses path returns { users } after flattening (line 32), while the fids path returns the raw Neynar response (line 38). This creates inconsistent API behavior:
- Consumers must handle different response structures based on which query parameter they use
- The comment at lines 29-30 suggests an intent to match a specific format, but only the addresses path performs this transformation
Consider standardizing both paths to return the same shape, either by also flattening the fids response or by returning the raw response for both paths.
Apply this diff to standardize the fids path response:
} else if (fids.length > 0) {
const response = await neynar.fetchBulkUsers({
fids,
viewerFid: req.query.viewer_fid ? Number(req.query.viewer_fid) : undefined,
});
- return res.status(200).json(response);
+ // Match the format returned by the addresses path
+ return res.status(200).json({ users: response.users });
} else {Note: This assumes the Neynar fetchBulkUsers response has a users field. Please verify the actual response shape.
🤖 Prompt for AI Agents
In src/pages/api/farcaster/neynar/users.ts around lines 25 to 38, the addresses
branch returns a flattened object "{ users }" while the fids branch returns the
raw Neynar response causing inconsistent API shapes; change the fids branch to
return the same shape as the addresses branch by extracting/flattening the users
field and returning res.status(200).json({ users }), or if fetchBulkUsers
already returns Record<fid, User[]>, flatten Object.values(response).flat() and
return { users }; verify the actual fetchBulkUsers response shape and apply the
equivalent transformation used in the addresses path so both branches return
identical "{ users }" payloads.
| const response = await neynar.fetchBulkUsersByEthOrSolAddress({ | ||
| addresses, | ||
| }); | ||
| // Transform the response to match the expected format | ||
| // Neynar returns Record<address, User[]>, we need to flatten it | ||
| const users = Object.values(response).flat(); | ||
| return res.status(200).json({ users }); | ||
| } else if (fids.length > 0) { | ||
| const response = await neynar.fetchBulkUsers({ | ||
| fids, | ||
| viewerFid: req.query.viewer_fid ? Number(req.query.viewer_fid) : undefined, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find the Neynar client initialization
find . -type f -name "*.ts" -o -name "*.tsx" | xargs grep -l "neynar" | grep -E "(neynar|client)" | head -20Repository: Nounspace/nounspace.ts
Length of output: 953
🏁 Script executed:
# Look for Neynar client definition
fd -type f -name "neynar.ts"Repository: Nounspace/nounspace.ts
Length of output: 236
🏁 Script executed:
# Search for fetchBulkUsersByEthOrSolAddress and fetchBulkUsers usage/definitions
rg "fetchBulkUsersByEthOrSolAddress|fetchBulkUsers" -A 5 -B 2Repository: Nounspace/nounspace.ts
Length of output: 10041
🏁 Script executed:
# Check the file in question to get full context
cat -n src/pages/api/farcaster/neynar/users.tsRepository: Nounspace/nounspace.ts
Length of output: 2449
🏁 Script executed:
# Look for package.json to check Neynar SDK version
cat package.json | jq '.dependencies | select(. != null) | keys[] | select(contains("neynar"))'Repository: Nounspace/nounspace.ts
Length of output: 86
🏁 Script executed:
# Check other Neynar endpoint responses to see the pattern
cat -n src/pages/api/farcaster/neynar/bulk-address.tsRepository: Nounspace/nounspace.ts
Length of output: 1445
🏁 Script executed:
# Check the search/users.ts to see how it returns fetchBulkUsers response
cat -n src/pages/api/search/users.ts | head -40Repository: Nounspace/nounspace.ts
Length of output: 1581
🏁 Script executed:
# Check if there's a type definition for the expected response
cat -n src/pages/api/farcaster/neynar/users.ts | grep -A 5 -B 5 "response"Repository: Nounspace/nounspace.ts
Length of output: 1118
Ensure consistent response format between address and FID branches.
The code transforms fetchBulkUsersByEthOrSolAddress response to { users } on line 32, but returns the raw fetchBulkUsers response on line 38. Since fetchBulkUsers returns an object with additional fields beyond users, this creates an inconsistent API response. Normalize both branches to return the same shape—either both return { users } or both return the full response object.
🤖 Prompt for AI Agents
In src/pages/api/farcaster/neynar/users.ts around lines 26–37, the address
branch returns { users } while the FID branch returns the raw fetchBulkUsers
response, causing inconsistent API shapes; change the FID branch to normalize to
the same shape by extracting the users array from the fetchBulkUsers response
(e.g., read response.users or flatten as needed) and return
res.status(200).json({ users }) so both branches return the identical { users }
payload.
|
@Jhonattan2121 i'm able to sign up without connecting Farcaster (nice work), but still seeing the issue here where users can't authorize nounspace as a signer after signing up (updating their fidRegistrations row with signature, signingPublicKey, and signingKeyLastValidatedAt, and setting isSigningKeyValid to true). As a reminder, if a user hasn't linked their FID and attempts to interact with Farcaster, they should see the 'Connect Farcaster' modal when they attempt any actions that require a farcaster signature (liking, casting, replying, recasting, follow/unfollow). Also, not sure if this is related, but I noticed that the Portfolio fidget on the default profile space is no longer populated with the Farcaster username. There's a good chance this is a regression related to these changes; can you investigate this too? |
Summary
Testing
Codex Task
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.