Bump React Native example to SDK 1.0.0 and fix the mock wallet#41
Conversation
Was pinned at ^0.1.6. 1.0.0 is the first release carrying SDK-owned session_id, the synthesized mobile user agent, and Android install attribution (P-2199 / P-2207). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The wagmi `mock` connector intercepts only a handful of RPC methods and forwards the rest — including eth_sendTransaction and eth_sign — to a real node. The chains resolved to their public RPCs, which hold no keys, so Send Tx failed with `-32601 rpc method is unsupported` (viem retries as wallet_sendTransaction, equally unsupported) and Sign Message with "unknown account". Override rpcUrls on the chain objects rather than on `transports`: the mock connector never reads transports, it takes chain.rpcUrls.default.http[0] directly in getProvider(), so a transport-only override silently leaves the mock wallet on the public endpoint. Hardcoded rather than env-driven — no hook in this app reads chain state, so the only calls reaching these URLs are the two mock-wallet actions, which need a local node regardless. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Dependency limit exceeded — report not shown. This pull request scan exceeded the 10,000-dependency limit applied to this scan, so the results are incomplete and may be inaccurate. To avoid reporting false positives, Socket has not posted a report. Upgrade your plan to raise the dependency limit and get complete reports, or view the partial scan in the dashboard. Socket is always free for open source. If this is a non-commercial open source project, contact us to request a free Team account. |
Documenting `curl ... | bash` hands readers a remote-code-execution habit, and security scanning flags it. Point at the official installation page and let readers choose their install method. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@codex review |
pnpm 11 no longer reads the "pnpm" field from package.json — it only warns that the keys were ignored. Five overrides had been stranded there, so the repo believed it was patching packages it was not: undici wanted ^7.24.1, resolved 6.25.0 @xmldom/xmldom wanted 0.8.13, resolved 0.7.13 alongside 0.8.13 h3 / node-forge / @wagmi/core happened to already satisfy their pins Moved all five into pnpm-workspace.yaml, where pnpm 11 reads overrides, and added two more to clear the criticals failing the audit gate: tar GHSA-23hp-3jrh-7fpw — no patched 6.x line, needs the 7.x major shell-quote GHSA-w7jw-789q-3m8p — same pin the SDK repo uses Both criticals predate this branch; main resolves the identical tar@6.2.1 and its CI has not run since June. 52 vulnerabilities -> 34, 1 critical -> 0. tar 6->7 and undici 6->7 are major bumps, so verified beyond the audit: tsc --noEmit clean and `expo export --platform ios` still produces a working bundle. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 20f48074dc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }); | ||
|
|
||
| export const chains = [ | ||
| withRpcUrl(baseSepolia, "http://localhost:8545"), |
There was a problem hiding this comment.
On an Android Emulator, localhost resolves to the emulator itself, not the development machine where the README starts anvil. Thus both Mock Wallet RPC calls fail with a connection error on the documented Android target even when anvil is running. Use emulator port reversal or a configurable Android host address (and bind anvil appropriately); the OP URL on the next line has the same issue.
Useful? React with 👍 / 👎.
main landed security PRs #36-#40 while this branch was open. Three conflicts: - pnpm-workspace.yaml: kept both sets of overrides. main's shell-quote pin supersedes the equivalent one added here; dropped the duplicate. - package.json: kept this branch's removal of the "pnpm" block. main's PRs were still editing that block, which pnpm 11 does not read — #38 revised undici to 6.27.0 there and it never took effect. - pnpm-lock.yaml: regenerated from main's copy so those PRs' resolutions are the base, then re-applied the overrides. Took main's newer undici intent (6.27.0) over the ^7.24.1 originally stranded in package.json, which also avoids forcing an untested 6 -> 7 major. 0 criticals. SDK stays at 1.0.0; tsc --noEmit clean and expo export still produces a working iOS bundle. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2be5ad02ee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }); | ||
|
|
||
| export const chains = [ | ||
| withRpcUrl(baseSepolia, "http://localhost:8545"), |
There was a problem hiding this comment.
When the app runs on an Android emulator or any physical device, localhost resolves to that emulator/device rather than the development machine where the README tells the user to start anvil. Consequently, both newly “fixed” mock-wallet actions still fail with a connection error in those supported React Native targets; use a configurable host address (and the Android emulator host alias) instead of hardcoding the device loopback address.
Useful? React with 👍 / 👎.
| "dependencies": { | ||
| "@babel/runtime": "^7.28.6", | ||
| "@formo/analytics-react-native": "^0.1.6", | ||
| "@formo/analytics-react-native": "^1.0.0", |
There was a problem hiding this comment.
The updated SDK declares react-native-play-install-referrer as an optional peer in the generated lockfile, but it is absent from this app's dependencies. Optional peers are not installed or linked into the Android build, so this example cannot exercise the Android install-attribution capability cited for the 1.0.0 upgrade; add the peer dependency and rebuild the development client if that feature is intended to be demonstrated.
Useful? React with 👍 / 👎.
P1 — hardcoding localhost only works on the iOS simulator. An Android emulator resolves localhost to itself (the host is 10.0.2.2) and a physical device to the device, so the mock-wallet fix did not actually apply on those targets. Derive the host from Expo's dev-server address instead: that is by definition the machine serving the bundle, which is the machine the README says to run anvil on, so all three targets reach it with nothing to configure. P2 — react-native-play-install-referrer is an optional peer of the SDK, so it was never installed and the example could not exercise the Android install attribution the 1.0.0 bump ships. Added it as a direct dependency; the example already requires a development build, so this costs no Expo Go compatibility. Verified: tsc --noEmit clean, and both `expo export --platform ios` and `--platform android` bundle. The Android bundle now contains PlayInstallReferrer, confirming the native module resolves rather than falling into the SDK's missing-module warning path. Audit gate still reports 0 criticals. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Both findings were correct — fixed in 49d2bf9. P1 — RPC host unreachable on Android emulator / physical device. Right, and it invalidated the fix on exactly the targets the README tells people to use. Rather than a configurable address, const devHost = Constants.expoConfig?.hostUri?.split(":")[0] ?? "localhost";
const localRpc = (port: number) => `http://${devHost}:${port}`;That machine is by definition the one serving the JS bundle, which is the one running anvil — so the emulator, a physical device on the LAN, and the simulator all reach it with nothing to configure. It also avoids hardcoding P2 — install-referrer peer absent. Correct; the example couldn't exercise the Android attribution the 1.0.0 bump ships. Added Verified rather than assumed: the Android bundle now contains One caveat worth stating: real referrer data only comes back for installs that genuinely came from the Play Store. A dev build or sideload reports an organic install, so this makes the code path live but full end-to-end validation still needs a Play Console internal-testing track. Noted in the README. |
|
@codex review |
yosriady
left a comment
yosriady
left a comment
There was a problem hiding this comment.
feel free to merge after the remaining codex review is addressed
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 49d2bf96db
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ...chain.rpcUrls, | ||
| default: { ...chain.rpcUrls.default, http: [url] }, |
There was a problem hiding this comment.
pnpm test now fails before wagmi.test.ts can run: its Jest mock for wagmi/chains provides only id and name (jest.setup.js lines 62–65), so this dereference throws while importing config/wagmi.ts. Add the required rpcUrls.default shape to that mock (or make the helper tolerate it) so the existing configuration tests remain runnable.
Useful? React with 👍 / 👎.
Codex flagged that pnpm test could not run. Two causes, only one of them mine: - Pre-existing: jest.setup.js mocked "@formo/react-native-analytics", a package that has never existed — the real name transposes the words. Every suite died at setup, so the tests have been dead on main too, not just on this branch. No CI job runs them, which is why it went unnoticed. - Mine: withRpcUrl dereferences chain.rpcUrls.default, and the wagmi/chains mock stubbed only id and name. Gave the stubs a realistic rpcUrls shape rather than making the helper tolerate a malformed chain. 13 tests now pass. Added two guards to wagmi.test.ts: one that each chain points at a local node (the mock connector reads rpcUrls directly and ignores transports, so an override in the wrong place is silent), and one that the host is derived from Expo's dev server rather than hardcoded. The second guard needs a non-localhost hostUri to be meaningful — the fallback when hostUri is absent is localhost, so a hardcoded localhost would otherwise satisfy it. Verified by reintroducing the exact bug: the guard fails, and passes again once reverted. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Correct — fixed in 4115648. I hadn't run Worth separating two causes, because only one is mine: Pre-existing. Mine. 13 tests pass now. I also added two guards to
The second needed care to be worth anything: with One thing outside this PR's scope: |
2 participants
Summary
Updates
with-react-nativeto@formo/analytics-react-native@1.0.0, and fixes the Mock Wallet's Send Tx and Sign Message buttons, which could never have worked against the configured RPCs.Two independent concerns, one commit each.
Key changes
package.json/pnpm-lock.yaml— SDK^0.1.6→1.0.0, the first release with SDK-ownedsession_id, the synthesized mobile user agent, and Android install attribution (P-2199 / P-2207).config/wagmi.ts— overriderpcUrlson the chain objects to point at a local anvil node.README.md— new optional install step for anvil, plus a troubleshooting entry.Why the mock wallet was broken
wagmi's
mockconnector intercepts only a handful of RPC methods (eth_chainId,wallet_switchEthereumChain,wallet_sendCalls,personal_sign…) and forwards everything else to a real node.eth_sendTransactionandeth_signare not in that set, so they went to the chains' public RPCs — which hold no keys:-32601 rpc method is unsupported. viem trieseth_sendTransaction, gets method-not-supported, retries aswallet_sendTransaction, and that fails too — which is why the error names a method nobody wrote.unknown account, after the connector rewritespersonal_signtoeth_signand swaps the params.MOCK_ADDRESSis Hardhat account #0, so this config was always intended for a local node — it just never said so.Why the override is on the chain, not
transportsThe mock connector never reads wagmi's
transports. It takes the URL straight offchain.rpcUrls.default.http[0]ingetProvider()(@wagmi/core/src/connectors/mock.ts, and identically in the bundleddist/esm). Overriding only the transport looks correct, typechecks, and silently leaves the mock wallet on the public endpoint.Why hardcoded rather than env-driven
No hook in this app reads chain state — no
useBalance, nouseReadContract. The only calls that reach these URLs are the two mock-wallet actions, which need a local node either way. Without anvil you get a connection error instead of-32601; both mean "start anvil". An env var would only guard a fallback nothing uses, at the cost of an extra setup step and a--clearrequirement.Verification
Against
anvil --chain-id 84532, the two previously failing methods:eth_sendTransaction-32601 rpc method is unsupported0x5ab7b72f6bb65eed…eth_signunknown account0x41bce68533b74ab9…npx tsc --noEmitpasses. The SDK bump was verified by installing1.0.0from npm and bundling withnpx expo export --platform ios— 7.49 MB Hermes bundle, no peer-dependency issues.🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.