draft: route external Custom chains from the --chain string#359
Open
shahan-khatchadourian-anchorage wants to merge 1 commit into
Open
draft: route external Custom chains from the --chain string#359shahan-khatchadourian-anchorage wants to merge 1 commit into
shahan-khatchadourian-anchorage wants to merge 1 commit into
Conversation
parse_chain mapped any unrecognized chain string to Chain::Unspecified, so a chain contributed by an external ChainPlugin -- which registers its converter under Chain::Custom -- could never be selected: both the plugin lookup in run() and the registry lookup in parse_and_display() resolve the requested chain through parse_chain. Map unrecognized strings to Chain::Custom(string) instead, matching the existing `impl FromStr for Chain` contract. The plugin that registered under that Custom chain is now selected, so an external workspace can add a chain purely by depending on its chain crate -- no edit to upstream chain-string parsing required, which is the stated purpose of this crate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot started reviewing on behalf of
shahan-khatchadourian-anchorage
June 3, 2026 14:40
View session
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates parser_cli_core’s chain-string parsing so external/custom chain plugins can be selected via --chain <string> by mapping unknown chain strings to Chain::Custom(<string>) instead of Chain::Unspecified, aligning behavior with the Chain parsing contract and plugin registration expectations.
Changes:
- Changed
parse_chainto returnChain::Custom(chain_str)for unrecognized chain strings. - Updated the
parse_chainrustdoc to describe the new behavior and rationale. - Added unit tests to verify built-in mappings remain stable and unknown strings map to
Chain::Custom.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+18
to
+20
| /// Built-in chains map to their dedicated variant. Any other string maps to | ||
| /// `Chain::Custom`, so a chain contributed by an external [`ChainPlugin`] is | ||
| /// selected by the plugin that registered under the matching `Chain::Custom`. |
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
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.
Problem
parser_cli_coreexists so external workspaces can compose their ownparser_clibinary with a custom set of chain plugins (see the crate docs).A plugin registers its converter under a
Chain; a chain without a dedicatedenum variant uses
Chain::Custom("name").But
chains::parse_chainmaps any unrecognized string toChain::Unspecified, and both code paths that turn--chain <string>into aChaingo through it:run(plugins.iter().find(|p| p.chain() == chain))output::parse_and_display(convert_transaction(&parse_chain(chain), ...))So a plugin registered under
Chain::Custom("foo")can never be selected:parse_chain("foo")yieldsUnspecified, which matches neither the pluginnor its registry entry. The CLI even lists the chain as supported (that list
is built from the plugins) while rejecting it — a confusing contradiction.
Fix
Map unrecognized chain strings to
Chain::Custom(string)instead ofChain::Unspecified, mirroring the existingimpl FromStr for Chaincontract. Built-in chains are unaffected (matched first). The plugin that
registered under that
Customchain is now selected, so an externalworkspace can add a chain purely by depending on its chain crate — no edit to
upstream chain-string parsing.
Test plan
chains.rs: built-ins still map to their variant; anunknown string (
"near") maps toChain::Custom("near").cargo test -p parser_cli_core— 28 passed.cargo test -p parser_cli— 8 passed (no existing behavior regressed).make -C src fmt+make -C src lint— clean across the workspace.🤖 Generated with Claude Code