refactor: [rpc] Remove confusing and brittle integral casts #34113

1. 
   maflcko commented at 10:46 am on December 19, 2025: member

   When constructing an UniValue from integral values, historically (long ago), in some cases casts where needed. With the current UniValue constructor, only very few are actually needed.

   Keeping the unused casts around is:

   • confusing, because code readers do not understand why they are needed
   • brittle, because some may copy them into new places, where they will lead to hard-to-find logic bugs, such as the ones fixed in pull #34112

   So fix all issues by removing them, except for a few cases, where casting was required:

   • ret.pushKV("coinbase", static_cast<bool>(coin->fCoinBase));, or
   • static_cast<std::underlying_type_t<decltype(info.nServices)>>(info.nServices).

   This hardening refactor does not fix any bugs and does not change any behavior.

2. DrahtBot added the label Refactoring on Dec 19, 2025
3. 
   DrahtBot commented at 10:46 am on December 19, 2025: 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/34113.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	sedited, rkrux

   If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #33741 (rpc: Optionally print feerates in sat/vb by polespinasa)
   • #32800 (rpc: Distinguish between vsize and sigop adjusted mempool vsize by musaHaruna)
   • #31449 (coins,refactor: Reduce getblockstats RPC UTXO overhead estimation by l0rinc)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

4. maflcko force-pushed on Dec 19, 2025
5. DrahtBot added the label CI failed on Dec 19, 2025
6. 
   DrahtBot commented at 10:58 am on December 19, 2025: contributor

   🚧 At least one of the CI tasks failed. Task No wallet: https://github.com/bitcoin/bitcoin/actions/runs/20367678494/job/58526539222 LLM reason (✨ experimental): Build failure due to a type mismatch: passing ServiceFlags where UniValue is expected, causing JSON-RPC encoding errors.

   Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

   • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

   • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

   • An intermittent issue.

   Leave a comment here, if you need help tracking down a confusing failure.

7. DrahtBot removed the label CI failed on Dec 19, 2025
8. refactor: [rpc] Remove confusing and brittle integral casts fa66e2d07a
9. maflcko force-pushed on Dec 19, 2025
10. sedited approved
11. 
    sedited commented at 8:35 pm on December 19, 2025: contributor

    ACK fa66e2d07a4b87d62382a54acf5fab6af77be24e
12. rkrux approved
13. 
    rkrux commented at 11:53 am on December 20, 2025: contributor

    ACK fa66e2d07a4b87d62382a54acf5fab6af77be24e

    Coming from previous PR #34112, in agreement with the cleanup here.

    I checked for the return types of the functions whose values were being cast and the implementation of the UniValue constructor.

Contributors
maflcko DrahtBot sedited rkrux

Labels
Refactoring