Skip to content

feat: add foreign chain tx config checker - #3716

Merged
gilcu3 merged 5 commits into
mainfrom
foreign_chain_config_tester
Jul 1, 2026
Merged

feat: add foreign chain tx config checker#3716
gilcu3 merged 5 commits into
mainfrom
foreign_chain_config_tester

Conversation

@gilcu3

@gilcu3 gilcu3 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Closes #3721

@gilcu3
gilcu3 force-pushed the foreign_chain_config_tester branch 3 times, most recently from baddd27 to 3aecfd2 Compare July 1, 2026 07:42
@gilcu3
gilcu3 force-pushed the foreign_chain_config_tester branch from 3aecfd2 to 926ba6b Compare July 1, 2026 09:26
anodar
anodar previously approved these changes Jul 1, 2026

@anodar anodar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! 🙏

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not for this PR, but we should probably move this, backup CLI, ckd-example-cli (and whatever else CLI we may have) under /crates/tools.

build_http_client(url, auth).map_err(|e| anyhow::anyhow!("failed to build HTTP client: {e}"))
}

fn prepare_aptos(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can probably code from crates/node/src/providers/verify_foreign_tx.rs here (new_aptos_inspector and few more I guess) instead for constructiong auth&client, possibly with some refactoring.

That way we also test production path too.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

given that the mechanism there uses the fanout, it seems it would require a sizable refactor, we better leave it for a follow-up

Comment thread crates/foreign-chain-config-tester/src/config.rs Outdated
@gilcu3
gilcu3 marked this pull request as ready for review July 1, 2026 12:44
@claude

claude Bot commented Jul 1, 2026

Copy link
Copy Markdown

Pull request overview

Adds a new standalone CLI, foreign-chain-config-tester, that probes every configured foreign-chain RPC provider with a fixed golden reference transaction so operators can validate their config off-line. Also extracts the shared auth_config_to_rpc_auth helper into a new foreign-chain-rpc-auth crate consumed by both the node and the tester (no behavioral change on the node path).

Changes:

  • New foreign-chain-config-tester binary crate with per-chain golden checks, config-shape-agnostic loader (dstack, launcher, legacy YAML), and human-readable report.
  • New foreign-chain-rpc-auth crate extracted verbatim from crates/node/src/config.rs and reused from verify_foreign_tx.rs.
  • Workspace registration for both new crates and Cargo.lock update.

Reviewed changes

Per-file summary
File Description
Cargo.toml, Cargo.lock Register new crates in the workspace.
crates/foreign-chain-config-tester/Cargo.toml Binary crate manifest.
crates/foreign-chain-config-tester/README.md Usage docs and sample output.
crates/foreign-chain-config-tester/src/main.rs Clap entrypoint, per-chain dispatch, timeout handling, exit code.
crates/foreign-chain-config-tester/src/checks.rs Per-chain check functions (EVM / BTC / Starknet / Aptos).
crates/foreign-chain-config-tester/src/config.rs Format-agnostic loader for foreign_chains and network detection.
crates/foreign-chain-config-tester/src/golden.rs Per-network golden vectors + hex/felt decoding helpers.
crates/foreign-chain-config-tester/src/report.rs Table renderer and pass/fail/skip aggregation.
crates/foreign-chain-rpc-auth/{Cargo.toml,src/lib.rs} Shared auth_config_to_rpc_auth helper + moved tests.
crates/node/src/config.rs, crates/node/src/providers/verify_foreign_tx.rs, crates/node/Cargo.toml Switch node to depend on the new foreign-chain-rpc-auth crate.

Findings

Non-blocking (nits, follow-ups, suggestions):

  • crates/foreign-chain-config-tester/src/main.rs:135 — Potential token leak in reported error output. For AuthConfig::Path/Query, auth_config_to_rpc_auth writes the resolved token into the URL. If the subsequent build_http_client(url, auth) returns an error whose Display includes the URL, the token is rendered verbatim into stdout via Status::Failed(format!("{e:#}")) and render. This mirrors existing node behavior and isn't unique to this PR, but the tester is more likely to hit malformed URLs (that's its point) and it emits errors to a terminal an operator will paste around. Consider redacting the URL from the outbound status string, or building the client with the un-templated URL and then attaching the secret separately for the request itself.
  • crates/foreign-chain-config-tester/src/main.rs:74-115 — All chains and providers are checked sequentially, so worst-case wall-clock is the sum of every timeout_sec across chains × providers. For a fully populated config that's several minutes. futures::future::join_all (or tokio::task::JoinSet) over the per-provider checks would keep the report deterministic while cutting the wait dramatically.
  • crates/foreign-chain-config-tester/src/main.rs:161-286run_evm, run_bitcoin, run_starknet are nearly identical (parse vector, iterate providers, dispatch check, push result). Extracting a small generic helper that takes the parse closure + a check closure would remove ~60 lines of copy-paste; not required but keeps drift risk low as chains get added.
  • crates/foreign-chain-config-tester/src/config.rs:60-115parse_foreign_chains and detect_network each parse the entire config string a second time via serde_yaml::from_str / toml::from_str. For a launcher config that includes mpc_node_config this can be non-trivial. Parsing once in main and threading the serde_yaml::Value / toml::Value down would remove the duplication and let both APIs share a single format-inference step.
  • crates/foreign-chain-config-tester/src/report.rs:69-77 — If a failure reason contains an embedded newline (jsonrpsee errors sometimes do), the table alignment falls apart. A reason.replace('\n', \" \") (or .lines().next()) before rendering would keep the report one row per provider; the full error can still go to stderr if you want to preserve it.
  • crates/foreign-chain-config-tester/src/golden.rs:80,105 — Bitcoin block hashes are canonically displayed in reverse byte order relative to the hash the node returns internally. Please double-check that the byte order in MAINNET.bitcoin.block_hash matches what BitcoinBlockHash::into::<[u8;32]>() produces (i.e. whether these are natural or explorer-order bytes). A single successful mainnet run against a real provider covers this.
  • crates/foreign-chain-config-tester/src/checks.rs:159-225 — Aptos tests cover the golden-pass and type-tag-mismatch paths; a sequence_number mismatch test would round out coverage since the same ensure! guards it in the code.
  • crates/foreign-chain-config-tester/src/config.rs:40-42 — The mainnet contract-id classifier accepts anything ending in .near (plus the bare v1.signer). Sub-accounts of a testnet workspace can technically end in .near too (e.g. NEAR-implicit accounts are 64-hex without a suffix, but <x>.near outside the top-level system accounts is possible on a fresh test). Not a blocker — the --network override handles it — but worth commenting the assumption.

The three previously raised threads (move under crates/tools, reuse new_aptos_inspector, use ChainId enum) are all valid follow-ups; I've deliberately not restated them.

✅ Approved

@pbeza pbeza left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, shallow review though.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an operator-facing utility to validate foreign-chain RPC provider configuration offline, and factors shared RPC auth/url handling into a dedicated crate so both the node and the tester exercise the same auth resolution logic.

Changes:

  • Introduce a new foreign-chain-config-tester binary crate that loads a config (--config), auto-detects or accepts --network, and checks each configured provider against a per-chain golden vector, reporting pass/fail/skip and exiting non-zero on any failure.
  • Extract auth_config_to_rpc_auth out of mpc-node into a new shared crate foreign-chain-rpc-auth, and update the node’s verify-foreign-tx provider wiring to use it.
  • Add golden vectors, per-chain check implementations, and table-style reporting (with unit tests) for the new tester.

Reviewed changes

Copilot reviewed 13 out of 14 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
crates/node/src/providers/verify_foreign_tx.rs Switches RPC auth conversion to the new shared foreign-chain-rpc-auth crate.
crates/node/src/config.rs Removes the old node-local auth_config_to_rpc_auth helper and its tests.
crates/node/Cargo.toml Adds foreign-chain-rpc-auth dependency to node crate.
crates/foreign-chain-rpc-auth/Cargo.toml New shared crate for converting config auth into inspector auth.
crates/foreign-chain-rpc-auth/src/lib.rs Implements (and tests) the shared auth_config_to_rpc_auth helper.
crates/foreign-chain-config-tester/Cargo.toml New binary crate manifest and dependencies.
crates/foreign-chain-config-tester/src/main.rs CLI entrypoint, config parsing + network detection, per-provider execution, exit code logic, and some unit tests.
crates/foreign-chain-config-tester/src/config.rs Parses foreign_chains subtree from YAML/TOML in multiple supported layouts; network auto-detection.
crates/foreign-chain-config-tester/src/golden.rs Network-specific golden vectors and hex/felt decoding helpers (with tests).
crates/foreign-chain-config-tester/src/checks.rs Per-chain inspector-based verification routines (with tests).
crates/foreign-chain-config-tester/src/report.rs Aggregates results and renders a human-readable aligned report (with tests).
crates/foreign-chain-config-tester/README.md Usage and output documentation, including note about potential secret exposure in URLs.
Cargo.toml Adds the new crates to the workspace and workspace dependency list.
Cargo.lock Locks new crates and dependency graph updates.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread crates/foreign-chain-config-tester/src/config.rs
Comment thread crates/foreign-chain-config-tester/src/main.rs
Comment thread crates/foreign-chain-config-tester/src/main.rs
@gilcu3
gilcu3 added this pull request to the merge queue Jul 1, 2026
Merged via the queue into main with commit cd067dd Jul 1, 2026
16 checks passed
@gilcu3
gilcu3 deleted the foreign_chain_config_tester branch July 1, 2026 14:55
nocktoshi pushed a commit to nocktoshi/mpc that referenced this pull request Jul 9, 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.

Add a foreign-chain RPC config tester binary

4 participants