refactor: CHAIN_REGISTRY — one source of truth for every FE chain fact#2401
Merged
Conversation
One chain's facts were spread across seven hand-maintained maps; the drift between them caused the SCROLL rot and the frozen-SDA incident. Every map still exports from its old path, but is now DERIVED from a single registry entry per chain — adding or launching a chain is one edit in one file. Behavior-proven, not just claimed: __tests__/chainRegistry.test.ts asserts each derived map equals the literal it replaced. Two deliberate deltas, both documented in the test: Kaia gains a chainId→Rhino-name mapping (it's a deposit chain; harmless superset), and rollout-flag keying gains display-name aliases (inert superset). One discovery preserved as-is: BASE has never been in the curated withdraw gate — looks like a June-2026 curation oversight, flagged for a product decision rather than smuggled in via refactor. Registry invariants are tested too: no duplicate ids, routable chains must have a Rhino name, deposit chains must be displayable, non-EVM withdraw destinations must carry their synthetic selector record.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
WalkthroughThe PR introduces a centralized chain registry for Rhino metadata, derives existing support and rollout mappings from it, migrates consumers to the new exports, and adds tests for mapping equivalence, registry invariants, and non-EVM withdrawal isolation. ChangesChain registry centralization
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
…ons into the registry Base: missing from the curated withdraw gate since June — an oversight, not a decision (Hugo). Verified live before enabling: ARB→BASE quotes OK for ETH/USDC/USDT + outflow SDA create OK (2026-07-13). Rollout-flagged (chain-rollout-base, created ON). Full clean per review: chainRollout.consts and nonEvmWithdraw.consts existed only to hold single derived constants — their exports now live in chainRegistry.consts itself and the files are gone. No re-export shims remain; rhino.consts/TokenSelector.consts keep their derived chain maps because they co-locate with family-level constants and a dozen consumers, but every value traces to one registry entry.
…raw gates Turns the 'gated only by discipline' caveat into an enforced invariant: NON_EVM_WITHDRAW_CHAINS (solana/tron) must be disjoint from TOKEN_SELECTOR_SUPPORTED_NETWORK_IDS (send/claim/pay gate) and from supportedPeanutChains (URL parse/validation source). Fails at test time if a future change lets a base58-address chain into an EVM-only flow.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/constants/chainRegistry.consts.ts (1)
294-296: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPreserve deep immutability of the exported registry.
readonlycurrently protects only the array; eachChainRegistryEntryremains mutable. A consumer can mutate an entry afterCHAIN_ROLLOUT_FLAGSandNON_EVM_WITHDRAW_CHAINShave been computed, leaving the registry and derived views inconsistent.Proposed fix
-export const CHAIN_REGISTRY: readonly ChainRegistryEntry[] = CHAIN_REGISTRY_LITERAL +export const CHAIN_REGISTRY: ReadonlyArray<Readonly<ChainRegistryEntry>> = CHAIN_REGISTRY_LITERAL🤖 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/constants/chainRegistry.consts.ts` around lines 294 - 296, Update the exported CHAIN_REGISTRY type and its underlying CHAIN_REGISTRY_LITERAL definition to enforce deep immutability, including readonly entry properties and nested values. Ensure consumers cannot mutate any ChainRegistryEntry or nested registry data after CHAIN_ROLLOUT_FLAGS and NON_EVM_WITHDRAW_CHAINS are derived.src/constants/__tests__/chainRegistry.test.ts (1)
154-168: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd an invariant catching id/aliasId collisions.
The uniqueness check only covers
entry.id, butEVM_CHAIN_ID_TO_RHINO_NAMEinrhino.consts.ts(Line 111) flat-maps[c.id, ...(c.aliasIds ?? [])]into one keyed object. If analiasIdon one entry ever collides with another entry's canonicalid(or another entry's alias),Object.fromEntriessilently keeps only the last write — no test would fail. Extending the invariant to assert uniqueness acrossid+ allaliasIdscombined would catch this class of regression before it reaches the Rhino name resolver.♻️ Suggested invariant addition
it('registry invariants', () => { const ids = CHAIN_REGISTRY.map((c) => c.id) expect(new Set(ids).size).toBe(ids.length) // no duplicate ids + const allKeys = CHAIN_REGISTRY.flatMap((c) => [c.id, ...(c.aliasIds ?? [])]) + expect(new Set(allKeys).size).toBe(allKeys.length) // no id/alias collisions for (const entry of CHAIN_REGISTRY) {🤖 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/constants/__tests__/chainRegistry.test.ts` around lines 154 - 168, Extend the registry invariants test around CHAIN_REGISTRY to collect each entry’s canonical id together with all values in aliasIds, then assert the combined list has no duplicates. Preserve the existing canonical-id uniqueness check and ensure collisions between ids and aliases, or between aliases, fail the test.
🤖 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.
Nitpick comments:
In `@src/constants/__tests__/chainRegistry.test.ts`:
- Around line 154-168: Extend the registry invariants test around CHAIN_REGISTRY
to collect each entry’s canonical id together with all values in aliasIds, then
assert the combined list has no duplicates. Preserve the existing canonical-id
uniqueness check and ensure collisions between ids and aliases, or between
aliases, fail the test.
In `@src/constants/chainRegistry.consts.ts`:
- Around line 294-296: Update the exported CHAIN_REGISTRY type and its
underlying CHAIN_REGISTRY_LITERAL definition to enforce deep immutability,
including readonly entry properties and nested values. Ensure consumers cannot
mutate any ChainRegistryEntry or nested registry data after CHAIN_ROLLOUT_FLAGS
and NON_EVM_WITHDRAW_CHAINS are derived.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 018f10ce-28c9-4794-a55b-e81e9c1ec96e
📒 Files selected for processing (12)
src/components/Global/TokenSelector/TokenSelector.consts.tssrc/constants/__tests__/chainRegistry.test.tssrc/constants/__tests__/nonEvmLeak.test.tssrc/constants/chainRegistry.consts.tssrc/constants/chainRollout.consts.tssrc/constants/nonEvmWithdraw.consts.tssrc/constants/rhino.consts.tssrc/context/tokenSelector.context.tsxsrc/features/payments/shared/hooks/useCrossChainTransfer.tssrc/hooks/useChainRollout.tssrc/lib/validation/addressFamily.tssrc/utils/featureFlag.utils.ts
💤 Files with no reviewable changes (2)
- src/constants/chainRollout.consts.ts
- src/constants/nonEvmWithdraw.consts.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The elegance follow-up from the final adversarial review (into #2398 per Hugo): one chain's facts were spread across seven hand-maintained maps (
CHAIN_LOGOS,SUPPORTED_EVM_CHAINS,EVM_CHAIN_ID_TO_RHINO_NAME,RHINO_WITHDRAW_SUPPORTED_TOKENS_BY_CHAIN,EVM_DEPOSIT_TOKEN_EXCEPTIONS,CHAIN_ROLLOUT_FLAGS,NON_EVM_WITHDRAW_CHAINS) — the drift between them caused the SCROLL rot and the frozen-SDA incident. All seven still export from their old paths (zero consumer churn), but each is now derived from a singleCHAIN_REGISTRYentry per chain.Adding/launching a chain is now one edit in one file — id, Rhino name, family, deposit/withdraw token sets, logo, rollout flag, all in one entry with the verification requirements documented at the top.
Behavior-proven
src/constants/__tests__/chainRegistry.test.tsasserts every derived map equals the literal it replaced, plus registry invariants (no duplicate ids; routable ⇒ Rhino name; deposit ⇒ displayable; non-EVM withdraw ⇒ synthetic record). Two deliberate, documented supersets: Kaia gains a chainId→name mapping (deposit chain; inert), rollout flags gain display-name alias keys (inert).🔍 Discovery for a product decision (NOT changed here)
BASE has never been in the curated withdraw gate — users cannot withdraw to Base, apparently since the June-2026 curation. Rhino fully supports it. Enabling = one
withdraw:line on the registry entry (+ verification + optional rollout flag). Deliberately left as-is so the refactor stays behavior-neutral.Out of scope
chain-details.json / token-details.json (generic EVM metadata: explorers, full token lists) and BE
CHAINS_CONFIG(already a single map).QA
typecheck 0 · 1,831 tests green (7 new equality/invariant tests) ·
next build✅