bitcoin-util: replace netmagic command with getchainparams command #35736

ekzyis commented at 5:38 PM on July 16, 2026: contributor

This is a follow-up to #35610. It replaces the netmagic command with a more versatile getchainparams command, as suggested in #35610 (comment).

DrahtBot commented at 5:38 PM on July 16, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35736.

Reviews

See the guideline and AI policy for information on the review process. A summary of reviews will appear here.

maflcko added this to the milestone 32.0 on Jul 16, 2026

maflcko removed this from the milestone 32.0 on Jul 16, 2026

in src/bitcoin-util.cpp:167 in 64fec7f4ce outdated

 165 | +    const auto& params = Params();
 166 | +    const auto& consensus = params.GetConsensus();
 167 | +
 168 | +    UniValue result{UniValue::VOBJ};
 169 | +    result.pushKV("chain", params.GetChainTypeString());
 170 | +    result.pushKV("test_chain", params.IsTestChain());

maflcko commented at 7:19 AM on July 17, 2026:

Should this be exposed? This is just a variation of the chain name:

src/kernel/chainparams.h:    bool IsTestChain() const { return m_chain_type != ChainType::MAIN; }

Any human or any code should be able to trivially decide what meaning a chain has by just looking at the chain type string.

in src/bitcoin-util.cpp:208 in 64fec7f4ce

 206 | +        UniValue addr{UniValue::VOBJ};
 207 | +        addr.pushKV("bech32_hrp", params.Bech32HRP());
 208 | +        result.pushKV("addresses", addr);
 209 | +    }
 210 | +
 211 | +    strPrint = result.write(2);

maflcko commented at 7:22 AM on July 17, 2026:

nit: Maybe use a named arg /*prettyIndent=*/2 here for clarity?

ekzyis commented at 12:00 PM on July 17, 2026:

done in 7298281ba8

in src/bitcoin-util.cpp:187 in 64fec7f4ce outdated

 185 | +            pow.pushKV("difficulty_retarget_interval", consensus.DifficultyAdjustmentInterval());
 186 | +            std::string mindiff_blocks = (consensus.fPowAllowMinDifficultyBlocks ?
 187 | +                  (consensus.enforce_BIP94 ? "bip94" : "yes") : "no");
 188 | +            pow.pushKV("mindiff_blocks", mindiff_blocks);
 189 | +        }
 190 | +        result.pushKV("pow", pow);

maflcko commented at 7:28 AM on July 17, 2026:

Is there a use case to expose the pow-consensus rules? the deployment-consensus rules are not exposed either, neither is the full raw genesis block hex. Also, any code calling this and wanting to do pow checks on (let's say bip94 rules) will have write code to implement bip94. So that code can just be triggered by if chain_type_string == "testnet4": ...

So I think, any code calling this will either not care about those consensus rules, or will have to implement the exact pow-rules for that chain type string themselves anyway. In both cases there is no use case for this?

maflcko commented at 7:38 AM on July 17, 2026: member

lgtm, but a bunch of stuff can be removed for now, and added when there is a real need to? (New Json dict fields can trivially be added in the future)

lgtm 64fec7f4ce22f48b6251b31825c44b3d028a713f 🗃

<details><summary>Show signature</summary>

Signature:

untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: lgtm 64fec7f4ce22f48b6251b31825c44b3d028a713f  🗃
cRv2K1EmOFS9f0dqGdkKVazKSf4OJ9dP+O6CjifDmQlPyrWtDs5JPtvZbtZl53wNK7/FWRDtQjjP1TBKqRRrDQ==

</details>

ekzyis force-pushed on Jul 17, 2026

ekzyis commented at 12:00 PM on July 17, 2026: contributor

lgtm, but a bunch of stuff can be removed for now, and added when there is a real need to? (New Json dict fields can trivially be added in the future)

I cherry-picked https://github.com/ajtowns/bitcoin/commit/824f8e96bfd94b104c08e3ad582e17a4b4bdbad7. @ajtowns mentioned this in #35610 (comment):

Post-merge query: does it really make sense for this command to be quite so dedicated to a single task? Would it be better for this to be bitcoin-util getchainparams and return a little json object so the output is (a) self-documenting (including "chain: signet" and "signet_challenge: ..." as well, eg), and (b) upgradable so that other constants can also be obtained, eg the genesis block, halving interval, pow limit/target spacing/retarget period, assume utxo params, base58/bech32 prefixes, fixed/dns seeds. Basically the impactful chain/consensus params that aren't configurable and aren't already exposed via things like getdeploymentinfo.

I don't mind removing or keeping test_chain or pow, since my use case in #34566 only needs the network magic.

I think there are good arguments for both sides: the command is called getchainparams, so one might expect it to return what's in CChainParams[^1], or, as @ajtowns mentioned, "the impactful chain/consensus params that aren't configurable and aren't already exposed via things like getdeploymentinfo." But since there's no use case for them yet, as you mentioned, we could also remove them and add them when we know more about how they will be used.

If I had to choose, I would say I am +0 on removing pow and test_chain.

[^1]: which would even be an argument to add more, which I considered, but decided against it for similar reasons

bitcoin-util: replace netmagic command with getchainparams command

Co-Authored-By: ekzyis <ramdip.singhgill@gmail.com>

7298281ba8

ekzyis force-pushed on Jul 17, 2026

DrahtBot added the label CI failed on Jul 17, 2026

DrahtBot removed the label CI failed on Jul 17, 2026