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.

Type	Reviewers
ACK	maflcko, sedited, ajtowns

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

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.

ajtowns commented at 4:34 AM on July 18, 2026:

Same argument as for having IsTestChain() in the first place. For downstream projects the mapping isn't straightforward (IsTestChain() on elements checks for MAINNET and LIQUID1 eg). It seems like a useful self-documenting feature to me.

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?

ajtowns commented at 5:04 AM on July 18, 2026:

The deployment rules are exposed via getdeploymentinfo already, and differ from these rules in that they're not hardcoded (bip9-style deployments are runtime dependent, and even height-based ones can be overridden at runtime via -testactivationheight).

The genesis hash is included, so if you need the contents, you can request the genesis block over p2p and verify that what you received is correct, which is perhaps sufficient?

The pow rules are necessary to know if you're trying to write compatible software that does header validation or better; currently the only place that's documented reliably is the source code itself. Reporting the flags we use in our code to make these decisions seems a fairly simple way to improve how this stuff is documented more broadly to me.

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

maflcko commented at 7:52 AM on July 20, 2026: member

re-ACK 7298281ba8dfb58e07121c74e64f07861ec21f5c 📓

<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: re-ACK 7298281ba8dfb58e07121c74e64f07861ec21f5c 📓
kDxjAbmtTN6Cfo8IAszSkD5JOsSt0zrF2QPx4O2zb+T6rXxXkRGkyDgBn106fzyMecE/RmR5RXw/0e7tDoWCBA==

</details>

bitcoin deleted a comment on Jul 21, 2026

sedited approved

sedited commented at 12:42 PM on July 21, 2026: contributor

ACK 7298281ba8dfb58e07121c74e64f07861ec21f5c

sedited requested review from ajtowns on Jul 21, 2026

ajtowns commented at 3:51 AM on July 22, 2026: contributor

Coauthor ACK 7298281ba8dfb58e07121c74e64f07861ec21f5c

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

fanquake merged this on Jul 22, 2026

fanquake closed this on Jul 22, 2026

ekzyis deleted the branch on Jul 22, 2026