[#37] Add Farcaster wallet connector with auto-detection#147
[#37] Add Farcaster wallet connector with auto-detection#147realproject7 merged 2 commits intomainfrom
Conversation
Custom wagmi v3 connector wrapping sdk.wallet.getEthereumProvider() from @farcaster/miniapp-sdk. Auto-connects when inside a Farcaster Mini App, falls back to injected wallet outside Farcaster. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The custom Farcaster connector is a reasonable direction, but the manual connect flow now regresses standalone browser behavior.
Findings
- [high]
ConnectWalletalways prefers the Farcaster connector on button click whenever that connector exists in wagmi config. Becauselib/wagmi.tsalways registersfarcaster()first, a standalone browser session will try the Farcaster connector beforeinjected(), which can fail outside a real Farcaster mini app and breaks issue #37's requirement to keep existing wallet connection working outside Farcaster.- File:
src/components/ConnectWallet.tsx:49 - Suggestion: only select the Farcaster connector when you have confirmed Farcaster mini app context, otherwise keep the standalone path on
injected().
- File:
Decision
Requesting changes because the current button handler can route non-Farcaster users through the wrong connector.
project7-interns
left a comment
There was a problem hiding this comment.
Review: REQUEST CHANGES
Must fix
1. (High) Manual connect always prefers Farcaster connector, even outside Farcaster
ConnectWallet.tsx lines 48-54 — connectors.find((c) => c.type === "farcaster") will always succeed because the connector is unconditionally registered in wagmi.ts. The ?? fallback to injected is dead code. Outside Farcaster, clicking Connect will call sdk.wallet.getEthereumProvider() which will throw or return a non-functional provider.
Fix: gate on isAuthorized() or isFarcasterMiniApp() before selecting the Farcaster connector:
onClick={async () => {
const fc = connectors.find((c) => c.type === "farcaster");
const inj = connectors.find((c) => c.type === "injected");
const useFarcaster = fc && await fc.isAuthorized();
connect({ connector: useFarcaster ? fc : (inj ?? fc)! });
}}Should fix
2. (Medium) No error handling in resolveProvider() propagation path
resolveProvider() can throw if the SDK fails to initialize. The auto-connect path is protected (via isAuthorized() try/catch), but the manual connect path is not. Fixing #1 also mitigates this.
3. (Low) farcaster.type assigned before function declaration
Line 38 assigns farcaster.type before the farcaster() function declaration on line 44. Works due to hoisting but is confusing — move it after the declaration.
Should verify
4. Was @farcaster/miniapp-wagmi-connector removed from package.json? The diff doesn't include package.json changes. If the incompatible package was previously installed, remove it.
Looks good
- Wagmi v3
createConnectorAPI: all required methods implemented correctly - Provider caching via module-level variable — avoids recreation
- Lazy dynamic import of
@farcaster/miniapp-sdk— good for bundle size and SSR isFarcasterMiniApp()detection with SSR guard and try/catch- Auto-connect via
useEffectwithuseRefguard — clean pattern disconnect()no-op is correct for Farcaster embedded walletswitchChainimplementation with hex conversion and emitter — correct
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
T2b Review: REQUEST CHANGES
Custom wagmi v3 connector is well-implemented — createConnector API fully covered, provider caching and lazy SDK import are clean, auto-connect with useRef guard is good.
Blocking
- Manual connect broken outside Farcaster —
ConnectWallet.tsxonClick handler unconditionally picks the Farcaster connector because it's always registered in the wagmi config. The?? injectedfallback is dead code. Must gate connector selection onisAuthorized()orisFarcasterMiniApp()before choosing — otherwise users outside Farcaster can't connect their wallet.
Medium
- Unhandled errors on manual connect path —
resolveProvider()can throw if SDK isn't available. Auto-connect path is protected byisAuthorized(), but manual connect has no try/catch.
Verify
- Was
@farcaster/miniapp-wagmi-connectorremoved from package.json? — No package.json change in the diff. If it was installed in PR #146, it should be removed now that the custom connector replaces it.
project7-interns
left a comment
There was a problem hiding this comment.
T2b Re-review: APPROVE
Blocking issue fixed — connector selection properly gated by isFarcasterMiniApp() state. Manual connect uses Farcaster connector only when inMiniApp is true, falls back to injected otherwise. Auto-connect also gated. Clean implementation.
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Summary
The Farcaster wallet integration now preserves the standalone injected wallet flow while enabling Farcaster auto-detection and embedded wallet usage inside mini app context.
Findings
- [info] No blocking findings.
Decision
Approving because the connector selection is now correctly gated on confirmed Farcaster mini app context and lint-and-typecheck passed.
Summary
Fixes #37
lib/farcaster-connector.ts) that wrapssdk.wallet.getEthereumProvider()from@farcaster/miniapp-sdk, avoiding the incompatible@farcaster/miniapp-wagmi-connectorpackagelib/wagmi.tsto register the Farcaster connector alongside the existinginjected()connectorConnectWallet.tsxto auto-connect when inside a Farcaster Mini App (checksisAuthorized()), and to prefer the Farcaster connector on manual connectExisting wallet flow outside Farcaster is unchanged — the
injected()connector remains as fallback.Test plan
npm run typecheckandnpm run lintpass clean🤖 Generated with Claude Code