Guard against network id mismatches - #1405
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses stake address rendering and query safety by enforcing that the network id provided to cardano-cli matches the network id reported by the connected node’s genesis parameters, and by rejecting stake addresses whose embedded network tag doesn’t match the CLI’s network selection. This prevents scenarios where a mainnet node is queried while the CLI is effectively configured as testnet (or vice-versa), which could previously lead to incorrect bech32 prefixes in rendered output.
Changes:
- Add a local-state-query wrapper that checks the CLI network id against the node’s genesis network id and fails fast on mismatch.
- Make
query stake-address-infofail when the input stake address network tag does not match the CLI network id (instead of silently discarding it). - Add a regression test for the new
stake-address-infonetwork-id mismatch behavior, plus a changelog fragment.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| cardano-cli/test/cardano-cli-test/Test/Cli/Run/Query.hs | Adds a test asserting query stake-address-info fails on stake-address/network-id mismatch before socket usage. |
| cardano-cli/src/Cardano/CLI/Type/Error/StakeAddressNetworkIdMismatchError.hs | Introduces a dedicated error type and message for stake address vs CLI network-id mismatches. |
| cardano-cli/src/Cardano/CLI/Type/Error/NodeNetworkIdMismatchError.hs | Introduces a dedicated error type and message for CLI network-id vs node genesis network-id mismatches. |
| cardano-cli/src/Cardano/CLI/LocalStateQuery.hs | Adds executeLocalStateQueryExprWithNetworkIdCheck to enforce node network-id validation on local state queries. |
| cardano-cli/src/Cardano/CLI/EraBased/Transaction/Run.hs | Switches relevant local-state queries to the new network-id-checking wrapper. |
| cardano-cli/src/Cardano/CLI/EraBased/Query/Run.hs | Switches query commands to the new wrapper and adds stake-address network-tag validation for stake-address-info. |
| cardano-cli/cardano-cli.cabal | Registers the new modules in the library stanza. |
| .changes/20260804_011228_cardano-cli_pablo.lamela_check_network_id.yml | Adds release notes for the network-id validation and stake-address-info behavior change. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| -> Consensus.Target ChainPoint | ||
| -> LocalStateQueryExpr BlockInMode ChainPoint QueryInMode () IO a | ||
| -> IO (Either AcquiringFailure a) | ||
| executeLocalStateQueryExprWithNetworkIdCheck connectInfo target f = |
There was a problem hiding this comment.
This feels excessive. I think it may be more readable to just put explicit checkNodeNetwork in those few places where it's needed rather than creating a specialised query execution wrapper.
There was a problem hiding this comment.
I think it is something we must do for every local state query, in the same way the handshake checks the magic number, so having the wrapper makes it more likely that new queries will also do this. And having the wrapper is less code than adding it everywhere, cause there are more than more than 20 places
Every command that opens a local state query connection now checks, on that same connection, that the network id the CLI was given (via `--mainnet`, `--testnet-magic` or `CARDANO_NODE_NETWORK_ID`) matches the network id in the node's genesis, and fails with a dedicated `NodeNetworkIdMismatchError` otherwise.
The header of a stake address encodes whether it is a mainnet or a testnet address. `query stake-address-info` now fails with a dedicated `StakeAddressNetworkIdMismatchError` when that tag does not match the network id the CLI was given, instead of silently dropping the address's network tag and querying just the credential.
9ad73fa to
d20bf2c
Compare
Flatten queryNodeNetworkId into a linear MaybeT do-block using extra's eitherToMaybe, replacing the nested case-of staircase. Collapse the changelog fragment's description to one line, since a plain YAML scalar folds line breaks to spaces regardless of where the source wraps
Context
This PR aims to address #1402. I could find an explanation for why would the CLI would use the testnet bech32 prefix for the response to the
cardano-cli query stake-address-infocommand when using the--mainnetflag.But I can see this happening if
--testnet 764824073is specified when it is actually mainnet, or ifCARDANO_NODE_NETWORK_ID=764824073as env variable instead ofCARDANO_NODE_NETWORK_ID=mainnet. And it shouldn't. Also the mainnet/testnet from the address input is discarded.This PR ensures the node now checks that the network id given to the CLI (
--mainnet,--testnet-magic, orCARDANO_NODE_NETWORK_ID) matches the network id in the node's genesis, so that for exampleCARDANO_NODE_NETWORK_ID=764824073against a mainnet node no longer renders addresses with a testnet prefix.Additionally,
query stake-address-infonow fails when the given stake address does not match the network id, instead of silently dropping the address's network tag.How to trust this PR
Changes are small. The
stake-address-infoquery change has a test. And the other change can be tested against a node, but making a test for it incardano-cliis tricky.Checklist
.changes/