Skip to content

fix: support non-Cartridge chainId lookup#2361

Merged
tarrencev merged 1 commit intomainfrom
fixchainid
Feb 2, 2026
Merged

fix: support non-Cartridge chainId lookup#2361
tarrencev merged 1 commit intomainfrom
fixchainid

Conversation

@tarrencev
Copy link
Contributor

No description provided.

@vercel
Copy link

vercel bot commented Feb 2, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
controller-example-next Ready Ready Preview Feb 2, 2026 5:15pm
keychain Ready Ready Preview Feb 2, 2026 5:15pm
keychain-storybook Ready Ready Preview Feb 2, 2026 5:15pm

Request Review

@claude
Copy link
Contributor

claude bot commented Feb 2, 2026

Code review

Issue: Multiple non-Cartridge chains will overwrite each other in Node.js environments

In the parseChainId function, when running in a Node.js environment (where XMLHttpRequest is undefined), ALL non-Cartridge RPC URLs will return the same hardcoded LOCALHOST chainId.

This causes a critical bug: if a user configures multiple non-Cartridge chains (e.g., different Alchemy or Infura endpoints), they will all be assigned the same chainId key. When these chains are stored in the this.chains Map in controller.ts, each subsequent chain will overwrite the previous one, leaving only the last chain accessible.

Example scenario:

const chains = [
  { rpcUrl: "https://alchemy.com/mainnet" },
  { rpcUrl: "https://infura.io/sepolia" },
];

In Node.js, both would get chainId LOCALHOST, and only the Infura chain would be accessible after initialization.

Location:

// Handle non-Cartridge hosts by making a synchronous call to getChainId
if (!isCartridgeHost) {
// Check if we're in a browser environment
if (typeof XMLHttpRequest === "undefined") {
// In Node.js environment (like tests), we can't make synchronous HTTP calls
// For now, we'll use a placeholder chainId for non-Cartridge hosts in tests
console.warn(
`Cannot make synchronous HTTP call in Node.js environment for ${url.toString()}`,
);
return shortString.encodeShortString("LOCALHOST") as ChainId;
}

The comment at line 148 mentions "For now, we'll use a placeholder chainId for non-Cartridge hosts in tests", but this code path is now enabled for production use, not just tests.

Suggested fix:

For Node.js environments with non-Cartridge hosts, consider one of these approaches:

  1. Make an async call to fetch the real chainId (this would require making parseChainId and callers async)
  2. Generate a unique placeholder chainId based on the URL (e.g., hash the hostname)
  3. Pass the chainId as an explicit parameter in the configuration instead of inferring it
  4. Document that non-Cartridge RPC providers are only supported in browser environments

@tarrencev tarrencev merged commit 0e71576 into main Feb 2, 2026
12 checks passed
@tarrencev tarrencev deleted the fixchainid branch February 2, 2026 17:39
@claude
Copy link
Contributor

claude bot commented Feb 2, 2026

Claude finished @tarrencev's task —— View job


Analyzing controller PR changes and updating documentation

Task: Analyze PR #2361 "fix: support non-Cartridge chainId lookup" and update documentation in docs-repo accordingly.

Todo List:

  • Review changed files to understand the functionality changes
  • Check existing documentation in docs-repo directory
  • Determine if documentation updates are needed
  • Update documentation if necessary

Analysis Summary

After examining the codebase and documentation, I can see this PR represents a massive initial codebase commit rather than a specific chainId fix. However, the core functionality involves the parseChainId function in packages/controller/src/utils.ts that handles:

  1. Cartridge chain detection: Automatically detects Starknet mainnet/sepolia and custom project chains from Cartridge RPC URLs
  2. Non-Cartridge RPC support: Makes synchronous HTTP calls to fetch chainId from external RPC providers in browser environments
  3. Node.js fallback: Returns "LOCALHOST" placeholder chainId when XMLHttpRequest is unavailable

Documentation Assessment

The existing documentation in docs-repo/src/pages/controller/configuration.md already covers:

  • Chain configuration options
  • Custom RPC endpoint usage
  • Network switching capabilities
  • Dynamic RPC URL parameters

Conclusion: No documentation updates needed

The existing chain configuration documentation already adequately covers:

  • How to configure custom chains via the chains option
  • Support for non-Cartridge RPC endpoints
  • Network switching functionality
  • Usage examples for custom RPC configurations

The internal parseChainId implementation details are not user-facing API changes that require documentation updates. The functionality for using external RPC providers is already documented in the configuration guide.


This was referenced Feb 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant