#35511 RFC: consensus: Make `CAmount` a class

full analysis

https://github.com/bitcoin/bitcoin/pull/35511 · hodlinator · +1486/-1258 in 184 files, 12 commits · labels: none · draft

Goal

  • Prevent accidental monetary bugs like invalid conversions or nonsensical math for developers
  • Catch uninitialized coin values and invalid operations at compile time across the codebase

Converts CAmount from a typedef of int64_t into a dedicated class in src/consensus/amount.h, requiring explicit construction, banning implicit conversions from booleans or floating-point numbers, forbidding operations like multiplying two amounts, and introducing a * sats unit multiplication syntax across over 180 files.

Problem: Using a raw integer typedef for monetary amounts permits hazardous code patterns such as implicit conversions from bool, uninitialized amount variables, and meaningless operations like multiplying amounts together.

Category: Utilities (logging, arguments, libraries) (#41 of 66)

P3 · type safety

  • P3 because it eliminates an entire class of monetary calculation bugs across all subsystems
  • Provides broad architectural safety but does not address an active incident and is safely deferrable

Encapsulating CAmount into a strong class prevents class-of-error bugs like assigning from booleans or multiplying amounts across all subsystems. It provides clear architectural value, though it does not resolve an active incident and is safely deferrable.

Membership: Changes the core monetary primitive and unit representation used across the entire node codebase.

Factors: security/stability 1, bug 0, performance 0, user value 0, leverage 2

Category: Validation (#37 of 48)

P3 · type safety

  • P3 because it hardens monetary type definitions used throughout consensus checks
  • Alters no consensus rules or validity semantics and is not urgent for consensus correctness

Hardens consensus type definitions in src/consensus/amount.h by making constructors explicit and eliminating hazardous implicit conversions. However, it alters no consensus rules or validity semantics, and the underlying problem is not urgent for consensus correctness.

Membership: Changes consensus/amount.h and the fundamental representation of coin amounts used throughout validation and consensus checks.

Factors: security/stability 1, bug 0, performance 0, user value 0, leverage 1

Reviewability: Ready

  • Ready for review, the branch builds cleanly and passes CI

The branch builds cleanly and passes CI; author recently rebased and adopted reviewer feedback regarding unit syntax and scripted diffs.

Author status: active

Open concerns:

  • Monolithic diff size touching 184 files in one PR makes review and backporting onerous; multiple reviewers advocate introducing the class first and migrating call sites incrementally.
  • Whether amounts should enforce value-range invariants (e.g., [0, 21M BTC]) or remain unbounded, especially within consensus code where invalid blocks must be representable without throwing.

Resolved concerns:

  • Dropped the proposed _sats user-defined literal in favor of standard * sats unit multiplication syntax to match C++ quantity proposals.
  • Fixed an IPC capnp serialization bug where CAmount fields were not written back into messages.
  • Ensured serialization and amount compression remain wire-compatible with existing formats.

Agreement: Disputed

  • Broad concept support for stronger monetary type safety
  • Concept approval without stated reasons (sedited)
  • Prefers incremental migration over taking the entire codebase transition at once (ryanofsky)
  • Changes requested: monolithic diff is too large and needs invariant clarification (l0rinc)

Broad concept support, but l0rinc requested changes over merging a monolithic 184-file refactor in one step.

Reviewers strongly favor the concept of strong typing for CAmount, but there is active resistance from l0rinc and reservations from ryanofsky regarding taking the entire codebase transition in a single PR rather than via incremental staging.

  • sedited and ryanofsky gave Concept ACKs.
  • purpleKarrot argued against UDLs in favor of multiplication syntax, which the author adopted.
  • l0rinc filed a Changes Requested review expressing discomfort with a single massive step and requesting clarification on serialization and range invariants.

Review verdicts (DrahtBot): 0

Dependencies

Enables:

  • Future strongly typed bounded amounts (e.g., [0, MAX_MONEY])

Files

1770 lines under test/bench/ci.

  • src/wallet/test/coinselector_tests.cpp +134/-109
  • src/test/feefrac_tests.cpp +79/-78
  • src/consensus/amount.h +128/-2
  • src/test/rpc_tests.cpp +51/-51
  • src/test/transaction_tests.cpp +51/-51
  • src/test/script_tests.cpp +50/-50
  • src/test/amount_tests.cpp +42/-42
  • src/test/miner_tests.cpp +39/-39
  • src/test/rbf_tests.cpp +34/-34
  • src/test/mempool_tests.cpp +32/-32
  • src/wallet/coinselection.cpp +25/-25
  • src/wallet/test/fuzz/coinselection.cpp +24/-24
  • src/rpc/blockchain.cpp +22/-22
  • src/wallet/spend.cpp +22/-22
  • src/wallet/test/coinselection_tests.cpp +22/-21
  • src/qt/coincontroldialog.cpp +20/-20
  • src/policy/fees/block_policy_estimator.cpp +19/-20
  • src/test/miniminer_tests.cpp +19/-19
  • src/txmempool.cpp +22/-16
  • src/test/script_p2sh_tests.cpp +18/-18
  • src/bench/coin_selection.cpp +18/-17
  • src/test/txpackage_tests.cpp +17/-17
  • src/test/fuzz/package_eval.cpp +15/-15
  • src/test/util_tests.cpp +15/-15
  • src/util/feefrac.h +15/-14
  • src/wallet/receive.cpp +16/-12
  • src/test/txgraph_tests.cpp +13/-13
  • src/test/coins_tests.cpp +11/-11
  • src/test/fuzz/feefrac.cpp +11/-11
  • src/test/blockfilter_tests.cpp +10/-10
  • src/node/mempool_persist.cpp +8/-11
  • src/test/compress_tests.cpp +11/-8
  • src/qt/bitcoinamountfield.cpp +9/-9
  • src/validation.cpp +9/-9
  • src/interfaces/wallet.h +8/-8
  • src/ipc/capnp/common-types.h +16/-0
  • src/qt/test/uritests.cpp +8/-8
  • src/test/fuzz/util.cpp +13/-3
  • src/wallet/test/group_outputs_tests.cpp +8/-8
  • src/bench/mempool_eviction.cpp +7/-7
  • src/test/blockpolicyestimator_tests.cpp +7/-7
  • src/test/cluster_linearize_tests.cpp +7/-7
  • src/test/feerounder_tests.cpp +7/-7
  • src/test/fuzz/integer.cpp +7/-7
  • src/test/fuzz/txorphan.cpp +7/-7
  • src/txgraph.cpp +7/-7
  • src/util/moneystr.cpp +7/-6
  • src/test/mempool_fee_estimator_tests.cpp +6/-6
  • src/test/psbt_tests.cpp +6/-6
  • src/txmempool.h +6/-6
  • src/wallet/rpc/spend.cpp +6/-6
  • src/compressor.h +7/-4
  • src/net_processing.cpp +6/-5
  • src/primitives/transaction.h +4/-7
  • src/qt/bitcoinamountfield.h +10/-1
  • src/core_io.cpp +5/-5
  • src/index/coinstatsindex.cpp +5/-5
  • src/node/miner.cpp +5/-5
  • src/node/txorphanage.cpp +5/-5
  • src/qt/transactiondesc.cpp +5/-5
  • src/rpc/mempool.cpp +5/-5
  • src/rpc/mining.cpp +5/-5
  • src/wallet/coinselection.h +5/-5
  • src/wallet/rpc/coins.cpp +5/-5
  • src/wallet/test/fuzz/fees.cpp +5/-5
  • src/wallet/test/spend_tests.cpp +5/-5
  • src/wallet/wallet.h +5/-5
  • src/rpc/rawtransaction.cpp +5/-4
  • src/policy/policy.h +4/-4
  • src/qt/overviewpage.cpp +4/-4
  • src/qt/walletmodel.cpp +4/-4
  • src/test/fuzz/txgraph.cpp +4/-4
  • src/test/multisig_tests.cpp +4/-4
  • src/test/txdownload_tests.cpp +4/-4
  • src/test/txvalidation_tests.cpp +4/-4
  • src/test/txvalidationcache_tests.cpp +4/-4
  • src/test/validation_tests.cpp +4/-4
  • src/wallet/feebumper.cpp +4/-4
  • src/wallet/fees.cpp +4/-4
  • src/wallet/wallet.cpp +4/-4
  • src/bitcoin-tx.cpp +4/-3
  • src/kernel/bitcoinkernel.cpp +4/-3
  • src/coins.cpp +3/-3
  • src/ipc/test/ipc_tests.cpp +5/-1
  • src/node/mini_miner.cpp +3/-3
  • src/qt/bitcoinunits.cpp +3/-3
  • src/qt/sendcoinsdialog.cpp +3/-3
  • src/qt/test/wallettests.cpp +3/-3
  • src/qt/transactionrecord.cpp +3/-3
  • src/qt/transactiontablemodel.cpp +3/-3
  • src/test/fuzz/cluster_linearize.cpp +4/-2
  • src/test/fuzz/feeratediagram.cpp +3/-3
  • src/test/fuzz/mini_miner.cpp +3/-3
  • src/test/fuzz/tx_pool.cpp +3/-3
  • src/test/sighash_tests.cpp +3/-3
  • src/test/util/setup_common.cpp +3/-3
  • src/wallet/rpc/addresses.cpp +4/-2
  • src/wallet/rpc/transactions.cpp +3/-3
  • src/bench/txgraph.cpp +3/-2
  • src/consensus/tx_check.cpp +3/-2
  • src/primitives/transaction.cpp +2/-3
  • src/rpc/util.cpp +3/-2
  • src/signet.cpp +3/-2
  • src/bench/mempool_ephemeral_spends.cpp +2/-2
  • src/bench/rpc_mempool.cpp +2/-2
  • src/bench/wallet_balance.cpp +3/-1
  • src/qt/guiutil.cpp +2/-2
  • src/qt/psbtoperationsdialog.cpp +2/-2
  • src/qt/transactionrecord.h +2/-2
  • src/qt/transactionview.cpp +2/-2
  • src/rpc/fees.cpp +2/-2
  • src/test/coinsviewoverlay_tests.cpp +2/-2
  • src/test/fuzz/fee_rate.cpp +2/-2
  • src/test/fuzz/rbf.cpp +2/-2
  • src/test/fuzz/util.h +3/-1
  • src/test/sigopcount_tests.cpp +2/-2
  • src/test/util/cluster_linearize.h +2/-2
  • src/test/validation_block_tests.cpp +2/-2
  • src/wallet/test/wallet_tests.cpp +2/-2
  • src/wallet/transaction.h +2/-2
  • src/bench/block_assemble.cpp +2/-1
  • src/bench/cluster_linearize.cpp +2/-1
  • src/bench/duplicate_inputs.cpp +2/-1
  • src/bench/sign_transaction.cpp +2/-1
  • src/bench/verify_script.cpp +2/-1
  • src/consensus/tx_verify.cpp +2/-1
  • src/kernel/coinstats.cpp +2/-1
  • src/policy/feerate.h +1/-2
  • src/qt/bitcoin.cpp +1/-2
  • src/util/fees.h +2/-1
  • src/bench/mempool_stress.cpp +1/-1
  • src/bench/wallet_create_tx.cpp +1/-1
  • src/consensus/tx_verify.h +1/-1
  • src/core_io.h +1/-1
  • src/kernel/mempool_entry.h +1/-1
  • src/net_processing.h +1/-1
  • src/node/interfaces.cpp +1/-1
  • src/node/miner.h +1/-1
  • src/node/mini_miner.h +1/-1
  • src/node/mining_types.h +1/-1
  • src/node/psbt.cpp +1/-1
  • src/node/transaction.cpp +1/-1
  • src/policy/ephemeral_policy.cpp +1/-1
  • src/policy/feerate.cpp +1/-1
  • src/policy/policy.cpp +1/-1
  • src/psbt.h +1/-1
  • src/qt/bitcoingui.cpp +1/-1
  • src/qt/receivecoinsdialog.cpp +1/-1
  • src/qt/receiverequestdialog.cpp +1/-1
  • src/qt/recentrequeststablemodel.cpp +1/-1
  • src/qt/sendcoinsentry.cpp +1/-1
  • src/qt/sendcoinsrecipient.h +1/-1
  • src/qt/transactionfilterproxy.cpp +1/-1
  • src/qt/walletmodeltransaction.cpp +1/-1
  • src/qt/walletview.cpp +1/-1
  • src/script/interpreter.cpp +1/-1
  • src/script/sigcache.h +1/-1
  • src/script/sign.h +1/-1
  • src/test/blockencodings_tests.cpp +1/-1
  • src/test/descriptor_tests.cpp +1/-1
  • src/test/fuzz/cmpctblock.cpp +1/-1
  • src/test/fuzz/coins_view.cpp +1/-1
  • src/test/fuzz/coinscache_sim.cpp +1/-1
  • src/test/fuzz/deserialize.cpp +1/-1
  • src/test/fuzz/p2p_headers_presync.cpp +1/-1
  • src/test/fuzz/script_flags.cpp +1/-1
  • src/test/fuzz/util/mempool.cpp +1/-1
  • src/test/testnet4_miner_tests.cpp +1/-1
  • src/test/txospenderindex_tests.cpp +1/-1
  • src/test/util/random.h +1/-1
  • src/test/util/transaction_utils.cpp +1/-1
  • src/test/util/transaction_utils.h +1/-1
  • src/test/util/txmempool.cpp +1/-1
  • src/test/validation_chainstatemanager_tests.cpp +1/-1
  • src/txgraph.h +1/-1
  • src/util/feefrac.cpp +1/-1
  • src/wallet/interfaces.cpp +1/-1
  • src/wallet/receive.h +1/-1
  • src/wallet/rpc/wallet.cpp +1/-1
  • src/wallet/test/fuzz/spend.cpp +1/-1
  • src/wallet/types.h +1/-1
  • src/bench/disconnected_transactions.cpp +1/-0
  • src/ipc/test/ipc_test.capnp +1/-0
  • src/ipc/test/ipc_test.h +1/-0

Card

This PR turns the CAmount type alias for int64_t into an explicit class with restricted operators and explicit constructors across more than 180 files. It eliminates bugs stemming from implicit conversions from booleans or floats, uninitialized variables, and invalid operations such as multiplying amounts together. Reviewers strongly support the concept of a strong type for money and converged on a multiplication syntax (* sats), but landing this as an all-in-one refactor faces an open blocking objection from l0rinc advocating an incremental rollout. The PR is an RFC and is fully reviewable as a prototype, though it will likely need to be split into smaller staging steps before merge.

Data

dossier JSON · extract JSON · model openrouter/google/gemini-3.8-flash, generated 2026-09-17T15:53, confidence high, input hash d5041fea107d4f7c