refactor: [rpc] Remove confusing and brittle integral casts (take 2) #34524

1. 
   maflcko commented at 12:26 pm on February 6, 2026: member

   Second take for cases which I seem to have missed in commit 94ddc2dced5736612e358a3b80f2bc718fbd8161.

   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:

   • static_cast<bool>(fCoinBase), because bool{fCoinBase} only works on modern compilers.

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

2. DrahtBot added the label Refactoring on Feb 6, 2026
3. 
   DrahtBot commented at 12:26 pm on February 6, 2026: contributor

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

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	Sjors, sedited

   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:

   • #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. refactor: [rpc] Remove confusing and brittle integral casts (take 2) fa6801366d
5. 
   maflcko commented at 12:44 pm on February 6, 2026: member

   Looks like converting a single bit to a bool is considered narrowing on older compilers. Let me restore the static_cast for them …
6. maflcko force-pushed on Feb 6, 2026
7. DrahtBot added the label CI failed on Feb 6, 2026
8. 
   DrahtBot commented at 12:45 pm on February 6, 2026: contributor

   🚧 At least one of the CI tasks failed. Task macOS-cross to x86_64: https://github.com/bitcoin/bitcoin/actions/runs/21750533712/job/62747075125 LLM reason (✨ experimental): Compilation failed due to a narrowing conversion error in core_io.cpp (bool{prev_coin.fCoinBase} triggers -Wc++11-narrowing).

   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.

9. DrahtBot removed the label CI failed on Feb 6, 2026
10. 
    Sjors commented at 7:12 pm on February 6, 2026: member

    ACK fa6801366d76a34f51a7d60be7d3710ed8e722db

    The casts that are removed seen unambiguous. If the compiler is happy, I’m happy :-)

11. sedited approved
12. 
    sedited commented at 1:14 pm on February 9, 2026: contributor

    ACK fa6801366d76a34f51a7d60be7d3710ed8e722db
13. fanquake merged this on Feb 9, 2026
14. fanquake closed this on Feb 9, 2026

    ---

15. maflcko deleted the branch on Feb 9, 2026

Contributors
maflcko DrahtBot Sjors sedited

Labels
Refactoring