fix(p2p stats): improve own-IP geolocation and world map accuracy#1138
Conversation
Resolve the user's public endpoint when libp2p only advertises private listen addresses, look up an accurate country flag for "Your IP", snap peer markers to country centroids, and add leeching seeder link plus panel layout tweaks.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds generators and generated geographic assets, extends peer geolocation and IP utilities, surfaces the node endpoint (IP + country flag) in P2P stats, replaces polygon-based map rendering with a precomputed bitmap path, and expands tests covering these flows. ChangesPeer Geolocation and World Map Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/lib/__tests__/peer-geo.test.ts (1)
99-130: 💤 Low valueModule-level cache state leaks between tests.
fetchOwnPublicEndpointandfetchOwnIpCountryCodepopulate module-level caches (cachedOwnPublicEndpoint,ownIpCountryCache) that persist across test runs. If test order changes or tests run in parallel, cached values from one test can affect another, causing flaky behavior.Use
vi.resetModules()with dynamic import in abeforeEachto get a fresh module instance, or export cache-reset helpers for testing.Example fix using resetModules
describe('fetchOwnPublicEndpoint', () => { beforeEach(() => { vi.resetModules(); }); it('caches the fetched public endpoint', async () => { const fetchMock = vi.fn().mockResolvedValue({ ok: true, json: async () => ({ country: 'US', ip: '2001:4860:4860::8888' }), }); vi.stubGlobal('fetch', fetchMock); // Dynamic import to get fresh module with cleared cache const { fetchOwnPublicEndpoint } = await import('../peer-geo'); await expect(fetchOwnPublicEndpoint()).resolves.toEqual({ countryCode: 'us', ip: '2001:4860:4860::8888' }); await expect(fetchOwnPublicEndpoint()).resolves.toEqual({ countryCode: 'us', ip: '2001:4860:4860::8888' }); expect(fetchMock).toHaveBeenCalledTimes(1); vi.unstubAllGlobals(); }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/__tests__/peer-geo.test.ts` around lines 99 - 130, Tests leak module-level cache (cachedOwnPublicEndpoint, ownIpCountryCache) causing flakiness; fix by resetting the module before each test and importing a fresh instance of the functions under test: call vi.resetModules() in a beforeEach and then dynamically import fetchOwnPublicEndpoint and fetchOwnIpCountryCode from '../peer-geo' so each test sees a clean cachedOwnPublicEndpoint and ownIpCountryCache; alternatively add and call exported test-only cache reset helpers from the module to clear cachedOwnPublicEndpoint and ownIpCountryCache between tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@package.json`:
- Around line 55-60: Package.json was modified so before merging, run corepack
yarn install to sync yarn.lock and then run corepack yarn blotter:check to
validate changelog/version rules; ensure you execute these in the repo root and
commit any updated yarn.lock or blotter-fix outputs so the changes to the
"scripts" block (e.g.,
"generate:assets","llms:generate","map:generate","centroids:generate","release:manifest")
are accompanied by an updated lockfile and passing blotter check.
In `@scripts/generate-country-centroids.mjs`:
- Around line 17-21: The CSV source in CSV_URL
(scripts/generate-country-centroids.mjs) is stale and includes retired codes
like "an" while missing modern ISO alpha-2 codes (e.g., "cw", "sx", "bq");
update CSV_URL to point to a maintained centroid source or, if you prefer to
keep the current CSV, add an override map (e.g., COUNTRY_REPLACEMENTS or
centroidOverrides) in generate-country-centroids.mjs that remaps retired codes
to current ones and adds entries for missing modern codes before the code that
parses/writes the module (ensure the override is applied in the parsing function
that consumes CSV_URL and before the module writer function).
---
Nitpick comments:
In `@src/lib/__tests__/peer-geo.test.ts`:
- Around line 99-130: Tests leak module-level cache (cachedOwnPublicEndpoint,
ownIpCountryCache) causing flakiness; fix by resetting the module before each
test and importing a fresh instance of the functions under test: call
vi.resetModules() in a beforeEach and then dynamically import
fetchOwnPublicEndpoint and fetchOwnIpCountryCode from '../peer-geo' so each test
sees a clean cachedOwnPublicEndpoint and ownIpCountryCache; alternatively add
and call exported test-only cache reset helpers from the module to clear
cachedOwnPublicEndpoint and ownIpCountryCache between tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1563dc0f-7c4f-42a9-8031-5c718f4d6538
📒 Files selected for processing (12)
package.jsonscripts/generate-country-centroids.mjsscripts/generate-world-map-dots.mjssrc/components/settings-modal/p2p-stats-settings/__tests__/p2p-stats-settings.test.tsxsrc/components/settings-modal/p2p-stats-settings/__tests__/peer-world-map.test.tsxsrc/components/settings-modal/p2p-stats-settings/p2p-stats-settings.module.csssrc/components/settings-modal/p2p-stats-settings/p2p-stats-settings.tsxsrc/components/settings-modal/p2p-stats-settings/peer-world-map.tsxsrc/data/country-centroids.tssrc/data/world-map-dots.tssrc/lib/__tests__/peer-geo.test.tssrc/lib/peer-geo.ts
| "scripts": { | ||
| "generate:assets": "node scripts/generate-asset-manifest.js", | ||
| "llms:generate": "node scripts/generate-llms-files.mjs", | ||
| "map:generate": "node scripts/generate-world-map-dots.mjs", | ||
| "centroids:generate": "node scripts/generate-country-centroids.mjs", | ||
| "release:manifest": "node scripts/generate-release-manifest.mjs", |
There was a problem hiding this comment.
Run required post-edit package checks before merge.
Because package.json changed, please run and confirm:
corepack yarn install(syncyarn.lock)corepack yarn blotter:check
As per coding guidelines, "Run corepack yarn install to keep yarn.lock in sync when package.json changes." and "{CHANGELOG.md,package.json}: If CHANGELOG.md or package version changes, run yarn blotter:check."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@package.json` around lines 55 - 60, Package.json was modified so before
merging, run corepack yarn install to sync yarn.lock and then run corepack yarn
blotter:check to validate changelog/version rules; ensure you execute these in
the repo root and commit any updated yarn.lock or blotter-fix outputs so the
changes to the "scripts" block (e.g.,
"generate:assets","llms:generate","map:generate","centroids:generate","release:manifest")
are accompanied by an updated lockfile and passing blotter check.
| // Public-domain ISO country centroids (country,latitude,longitude,name). | ||
| // Override with COUNTRY_CENTROIDS_CSV_URL. | ||
| const CSV_URL = | ||
| process.env.COUNTRY_CENTROIDS_CSV_URL ?? 'https://raw.githubusercontent.com/google/dspl/master/samples/google/canonical/countries.csv'; | ||
|
|
There was a problem hiding this comment.
Centroid source is stale for current ISO alpha-2 coverage.
The generated dataset includes retired code an (Netherlands Antilles) and misses modern split codes like cw, sx, and bq, which can cause centroid snapping gaps for valid country lookups. Please switch to a maintained ISO centroid source or add an explicit override map for modern replacements before writing the module.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/generate-country-centroids.mjs` around lines 17 - 21, The CSV source
in CSV_URL (scripts/generate-country-centroids.mjs) is stale and includes
retired codes like "an" while missing modern ISO alpha-2 codes (e.g., "cw",
"sx", "bq"); update CSV_URL to point to a maintained centroid source or, if you
prefer to keep the current CSV, add an override map (e.g., COUNTRY_REPLACEMENTS
or centroidOverrides) in generate-country-centroids.mjs that remaps retired
codes to current ones and adds entries for missing modern codes before the code
that parses/writes the module (ensure the override is applied in the parsing
function that consumes CSV_URL and before the module writer function).
When the P2P stats panel unmounts mid-request, its AbortSignal cancels the in-flight fetchOwnPublicEndpoint / fetchOwnIpCountryCode calls. Those empty results were still cached for 30-60s, so reopening the panel within that window showed "Your IP" as unavailable or without a country flag even though nothing had actually failed. Skip caching when the signal aborted so a later open retries. Addresses Cursor Bugbot finding.
|
Triaged review feedback and pushed one fix (278c330): Fixed
Declined
Verified (process item from CodeRabbit)
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 278c330. Configure here.
| <ConnectedPeersValue row={row} /> | ||
| </td> | ||
| </tr> | ||
| </Fragment> |
There was a problem hiding this comment.
Updated time hidden on failure
Low Severity
The last-updated row is rendered only inside the connected-peers branch, so when a refresh fails and the reducer clears rows, updatedAt is still set but the timestamp never appears in the table.
Reviewed by Cursor Bugbot for commit 278c330. Configure here.
|
Triaging the two findings that landed after the previous comment: Declined — CodeRabbit "Centroid source is stale" (rated Major): no functional impact here. Deferred — Cursor Bugbot "Updated time hidden on failure" (Low): the timestamp row only renders alongside the connected-peers row, so on a failed refresh (rows cleared, error shown) it doesn't appear. This is debatable as a bug — surfacing "Updated HH:MM" next to an error arguably implies the errored view is fresh. It only affects the error path and the normal/success path is correct, so it's a non-blocking follow-up rather than a merge blocker. |
1 participant
Summary
Test plan
yarn testfor p2p-stats-settings, peer-world-map, and peer-geoyarn build,yarn lint,yarn type-checkNote
Medium Risk
Introduces new network lookups (to
api.country.is/ipify) and caching to resolve the local node’s public IP/country, plus significant changes to P2P stats UI/map rendering; mistakes could affect privacy expectations or display correctness.Overview
Improves browser P2P stats to show an accurate “Your IP” (IPv4/IPv6) and matching country flag by deriving a public endpoint from libp2p-observed addresses and falling back to
api.country.is/ipify, with short-lived caching and abort-safe behavior.Upgrades the peer world map backdrop and marker placement by replacing hardcoded continent polygons with a generated Natural Earth raster mask (
WORLD_MAP_DOTS) and snapping peer markers near per-country centroids (COUNTRY_CENTROIDS) with small deterministic jitter.Tweaks the stats panel UX: adds a “want to seed?” link when in leeching mode, reorders the “updated” row placement, and adjusts styling/contrast; expands/updates tests and adds generation scripts (
yarn map:generate,yarn centroids:generate) for the new data files.Reviewed by Cursor Bugbot for commit 278c330. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Improvements