#36087 util: Add and use AssertUnreachable

full analysis

https://github.com/bitcoin/bitcoin/pull/36087 · maflcko · +790/-546 in 312 files, 7 commits · labels: Utils/log/libs, Needs rebase, CI failed · draft

Goal

  • Provide a dedicated unreachable assertion to resolve assertion ambiguity and silence compiler warnings
  • Prevent undefined behavior across the codebase from future std::unreachable usage

Adds an `AssertUnreachable()` macro to `util/check.h` that calls the noreturn `assertion_fail` helper, replacing existing `assert(false)` and `Assert(false)` calls throughout the codebase. Adds linters in the test suite to forbid falsy assertions and disallow `std::unreachable`.

Problem: Assertions on falsy literals like `assert(false)` cause ambiguity about `NDEBUG` semantics, while `Assert(false)` hides `[[noreturn]]` behind a call, triggering GCC return-type warnings. Additionally, future adoption of C++23 `std::unreachable` would invite undefined behavior if reached.

Category: Test infrastructure (#32 of 45)

P4 · test coverage

  • P4 because the added linters only enforce the new macro and block std::unreachable
  • Minor internal test checks with no broader testing leverage

The added linters simply enforce adherence to the newly introduced utility macro and prevent future usage of `std::unreachable`.

Membership: Adds two new linter checks (`assert_falsy` and `std_unreachable`) to the Rust lint runner.

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

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

P3 · cleanup

  • P3 because it improves code cleanliness and silences GCC return-type warnings
  • Directly benefits developers by guarding against undefined behavior from std::unreachable

The PR improves code cleanliness and compiler compatibility by exposing noreturn semantics directly, silencing GCC return-type warnings and preemptively guarding against undefined behavior from `std::unreachable`.

Membership: Introduces `AssertUnreachable` in `util/check.h` and updates global assertion include rules.

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

Reviewability: Stale: Needs rebase

  • Needs rebase and CI is failing
  • Author is reworking to fix consensus library decoupling before taking out of draft

The branch has merge conflicts with master, CI is failing, and the author acknowledged that consensus library decoupling must still be resolved before taking the PR out of draft.

Author status: addressing review

Open concerns:

  • Consensus library linkage: `AssertUnreachable` in `src/script/interpreter.cpp` calls `assertion_fail` in `util/check.cpp`, causing build breakage in standalone `libbitcoin_consensus` builds.

Resolved concerns:

  • Linter regex was tightened to catch variable whitespace in falsy assertions.
  • Commit ordering adjusted so the lint runner is not broken midway through the stack.

Agreement: Mild

  • No reviewers have formally approved the concept yet
  • Unaddressed objection: breaks standalone consensus library build (jeanpablojp)
  • Author agreed to fix the consensus library dependency before readying the PR (maflcko)

Mild: consensus library linkage breakage reported by jeanpablojp; author agreed to fix but has not yet implemented it.

No reviewer has formally concept ACKed the change yet. jeanpablojp noted that linking `util/check` from `src/script/interpreter.cpp` breaks standalone `libbitcoin_consensus`, which the author accepted but has not yet resolved in code.

  • jeanpablojp pointed out that `AssertUnreachable` in `src/script/interpreter.cpp` introduces an unwanted dependency from `libbitcoin_consensus` into `util`
  • maflcko replied: 'Thx, will fix this some time before taking out of draft.'

Review verdicts (DrahtBot): 0

Files

657 lines under test/bench/ci.

  • test/lint/test_runner/src/lint_cpp.rs +58/-2
  • src/qt/sendcoinsdialog.cpp +13/-13
  • src/qt/walletmodel.cpp +13/-13
  • src/qt/transactiontablemodel.cpp +10/-10
  • src/qt/guiutil.cpp +9/-10
  • src/qt/addresstablemodel.cpp +9/-9
  • src/wallet/test/wallet_tests.cpp +9/-9
  • src/netaddress.cpp +9/-8
  • src/qt/test/wallettests.cpp +9/-8
  • src/qt/bitcoinunits.cpp +8/-8
  • src/qt/test/addressbooktests.cpp +8/-8
  • src/cluster_linearize.h +8/-7
  • src/test/miniscript_tests.cpp +8/-7
  • src/test/transaction_tests.cpp +7/-8
  • src/kernel/bitcoinkernel.cpp +7/-7
  • src/qt/walletcontroller.cpp +7/-7
  • src/script/miniscript.h +7/-7
  • test/lint/test_runner/src/main.rs +12/-2
  • src/ipc/capnp/common-types.h +7/-6
  • src/ipc/test/ipc_tests.cpp +7/-6
  • src/qt/peertablemodel.cpp +6/-6
  • src/test/fuzz/feeratediagram.cpp +5/-7
  • src/qt/utilitydialog.cpp +5/-6
  • src/test/fuzz/versionbits.cpp +6/-5
  • src/blockfilter.cpp +6/-4
  • src/test/util_check_tests.cpp +6/-4
  • src/wallet/scriptpubkeyman.cpp +5/-5
  • src/ipc/test/fuzz/ipc.cpp +5/-4
  • src/qt/bantablemodel.cpp +5/-4
  • src/test/blockmanager_tests.cpp +5/-4
  • src/wallet/wallet.cpp +4/-5
  • src/policy/fees/block_policy_estimator.cpp +4/-4
  • src/qt/optionsmodel.cpp +4/-4
  • src/qt/receivecoinsdialog.cpp +4/-4
  • src/qt/walletframe.cpp +4/-4
  • src/test/fuzz/miniscript.cpp +5/-3
  • src/test/txrequest_tests.cpp +4/-4
  • src/test/util_tests.cpp +4/-4
  • src/test/validation_tests.cpp +4/-4
  • src/txgraph.cpp +4/-4
  • src/wallet/external_signer_scriptpubkeyman.cpp +5/-3
  • src/ipc/capnp/protocol.cpp +4/-3
  • src/node/connection_types.cpp +4/-3
  • src/test/crypto_tests.cpp +4/-3
  • src/test/fuzz/base_encode_decode.cpp +3/-4
  • src/test/fuzz/merkle.cpp +4/-3
  • src/test/logging_tests.cpp +4/-3
  • src/test/miniminer_tests.cpp +4/-3
  • src/test/util/net.cpp +4/-3
  • src/common/messages.cpp +3/-3
  • src/memusage.h +3/-3
  • src/net.h +3/-3
  • src/outputtype.cpp +3/-3
  • src/policy/feerate.cpp +4/-2
  • src/qt/bitcoin.h +3/-3
  • src/qt/walletview.cpp +3/-3
  • src/script/interpreter.cpp +3/-3
  • src/test/blockencodings_tests.cpp +3/-3
  • src/test/blockfilter_tests.cpp +3/-3
  • src/test/fuzz/feefrac.cpp +4/-2
  • src/test/fuzz/muhash.cpp +4/-2
  • src/test/fuzz/txgraph.cpp +4/-2
  • src/test/fuzz/txrequest.cpp +4/-2
  • src/test/key_io_tests.cpp +3/-3
  • src/test/private_broadcast_tests.cpp +4/-2
  • src/test/validation_chainstatemanager_tests.cpp +3/-3
  • src/crypto/chacha20.cpp +3/-2
  • src/kernel/mempool_removal_reason.cpp +3/-2
  • src/netbase.cpp +3/-2
  • src/psbt.h +3/-2
  • src/qt/rpcconsole.cpp +3/-2
  • src/qt/splashscreen.cpp +3/-2
  • src/qt/transactionrecord.cpp +3/-2
  • src/test/compress_tests.cpp +3/-2
  • src/test/fuzz/addrman.cpp +3/-2
  • src/test/fuzz/bech32.cpp +3/-2
  • src/test/fuzz/block.cpp +3/-2
  • src/test/fuzz/key.cpp +3/-2
  • src/test/fuzz/key_io.cpp +3/-2
  • src/test/fuzz/locale.cpp +3/-2
  • src/test/fuzz/script.cpp +3/-2
  • src/test/fuzz/string.cpp +3/-2
  • src/test/fuzz/threadpool.cpp +3/-2
  • src/test/fuzz/transaction.cpp +3/-2
  • src/test/fuzz/util/mempool.cpp +3/-2
  • src/test/headers_sync_chainwork_tests.cpp +3/-2
  • src/test/mempool_tests.cpp +3/-2
  • src/test/node_init_tests.cpp +3/-2
  • src/test/orphanage_tests.cpp +3/-2
  • src/test/pcp_tests.cpp +3/-2
  • src/test/rbf_tests.cpp +3/-2
  • src/test/sigopcount_tests.cpp +3/-2
  • src/test/txdownload_tests.cpp +3/-2
  • src/wallet/export.cpp +3/-2
  • src/wallet/feebumper.h +3/-2
  • src/wallet/test/ismine_tests.cpp +3/-2
  • doc/developer-notes.md +2/-2
  • src/addresstype.cpp +2/-2
  • src/bench/verify_script.cpp +2/-2
  • src/chainparams.cpp +2/-2
  • src/chainparamsbase.cpp +2/-2
  • src/node/minisketchwrapper.cpp +2/-2
  • src/node/psbt.cpp +3/-1
  • src/node/transaction.cpp +3/-1
  • src/qt/askpassphrasedialog.cpp +2/-2
  • src/qt/bitcoinamountfield.cpp +2/-2
  • src/script/miniscript.cpp +2/-2
  • src/script/solver.cpp +2/-2
  • src/test/fuzz/asmap.cpp +3/-1
  • src/test/fuzz/asmap_direct.cpp +2/-2
  • src/test/fuzz/bitdeque.cpp +3/-1
  • src/test/fuzz/bitset.cpp +3/-1
  • src/test/fuzz/cluster_linearize.cpp +3/-1
  • src/test/fuzz/fees.cpp +3/-1
  • src/test/fuzz/headerssync.cpp +3/-1
  • src/test/fuzz/net.cpp +3/-1
  • src/test/fuzz/private_broadcast.cpp +3/-1
  • src/test/fuzz/tx_pool.cpp +2/-2
  • src/test/fuzz/txdownloadman.cpp +3/-1
  • src/test/fuzz/util/net.cpp +2/-2
  • src/test/fuzz/vecdeque.cpp +3/-1
  • src/test/util/chainstate.h +2/-2
  • src/test/util/net.h +2/-2
  • src/test/util/transaction_utils.cpp +3/-1
  • src/util/fees.cpp +2/-2
  • src/wallet/db.cpp +3/-1
  • src/wallet/test/coinselector_tests.cpp +3/-1
  • src/wallet/test/fuzz/coincontrol.cpp +3/-1
  • src/wallet/test/fuzz/coinselection.cpp +3/-1
  • src/wallet/test/fuzz/spend.cpp +3/-1
  • src/wallet/test/util.cpp +2/-2
  • src/wallet/wallet.h +2/-2
  • src/base58.cpp +1/-2
  • src/bech32.cpp +2/-1
  • src/bitcoin-cli.cpp +2/-1
  • src/bitcoin-util.cpp +2/-1
  • src/crypto/hex_base.cpp +2/-1
  • src/crypto/sha256.cpp +2/-1
  • src/crypto/sha3.cpp +2/-1
  • src/netaddress.h +2/-1
  • src/node/txdownloadman_impl.cpp +2/-1
  • src/policy/fees/estimator_man.cpp +2/-1
  • src/prevector.h +2/-1
  • src/qt/bitcoin.cpp +2/-1
  • src/qt/test/apptests.cpp +2/-1
  • src/test/fuzz/banman.cpp +2/-1
  • src/test/fuzz/difference_formatter.cpp +2/-1
  • src/test/fuzz/flatfile.cpp +2/-1
  • src/test/fuzz/net_permissions.cpp +2/-1
  • src/test/fuzz/netaddress.cpp +2/-1
  • src/test/fuzz/util/wallet.h +2/-1
  • src/test/httpserver_tests.cpp +2/-1
  • src/test/interfaces_tests.cpp +2/-1
  • src/test/scriptnum10.h +2/-1
  • src/test/txvalidation_tests.cpp +2/-1
  • src/util/check.h +3/-0
  • src/wallet/external_signer_scriptpubkeyman.h +2/-1
  • src/wallet/test/wallet_test_fixture.cpp +2/-1
  • src/wallet/walletutil.cpp +2/-1
  • src/zmq/zmqabstractnotifier.cpp +2/-1
  • src/addrman.cpp +1/-1
  • src/arith_uint256.cpp +1/-1
  • src/bench/sign_transaction.cpp +1/-1
  • src/bip324.cpp +1/-1
  • src/blockencodings.cpp +2/-0
  • src/chain.h +1/-1
  • src/common/signmessage.cpp +1/-1
  • src/crypto/chacha20poly1305.cpp +1/-1
  • src/crypto/hkdf_sha256_32.cpp +1/-1
  • src/crypto/poly1305.h +1/-1
  • src/crypto/siphash.cpp +1/-1
  • src/dbwrapper.cpp +1/-1
  • src/deploymentinfo.h +1/-1
  • src/httpserver.cpp +1/-1
  • src/index/blockfilterindex.cpp +1/-1
  • src/index/txindex.cpp +1/-1
  • src/init.cpp +1/-1
  • src/kernel/chainparams.cpp +1/-1
  • src/kernel/coinstats.cpp +1/-1
  • src/kernel/disconnected_transactions.cpp +1/-1
  • src/key.cpp +1/-1
  • src/key_io.cpp +1/-1
  • src/logging.cpp +1/-1
  • src/mapport.cpp +1/-1
  • src/net_processing.cpp +1/-1
  • src/node/chainstate.cpp +1/-1
  • src/node/eviction.cpp +2/-0
  • src/node/txorphanage.cpp +1/-1
  • src/node/utxo_snapshot.cpp +1/-1
  • src/primitives/transaction.cpp +1/-1
  • src/psbt.cpp +1/-1
  • src/pubkey.cpp +1/-1
  • src/qt/peertablesortproxy.cpp +1/-1
  • src/rest.cpp +1/-1
  • src/rpc/server.cpp +1/-1
  • src/scheduler.cpp +1/-1
  • src/script/descriptor.cpp +1/-1
  • src/script/script.h +1/-1
  • src/script/sign.cpp +1/-1
  • src/sync.h +1/-1
  • src/test/fuzz/bip324.cpp +2/-0
  • src/test/fuzz/block_header.cpp +1/-1
  • src/test/fuzz/bloom_filter.cpp +1/-1
  • src/test/fuzz/coins_view.cpp +1/-1
  • src/test/fuzz/coinscache_sim.cpp +1/-1
  • src/test/fuzz/crypto_aes256.cpp +1/-1
  • src/test/fuzz/crypto_aes256cbc.cpp +1/-1
  • src/test/fuzz/crypto_common.cpp +1/-1
  • src/test/fuzz/cuckoocache.cpp +2/-0
  • src/test/fuzz/decode_tx.cpp +1/-1
  • src/test/fuzz/float.cpp +1/-1
  • src/test/fuzz/golomb_rice.cpp +1/-1
  • src/test/fuzz/hex.cpp +1/-1
  • src/test/fuzz/http_request.cpp +1/-1
  • src/test/fuzz/message.cpp +1/-1
  • src/test/fuzz/node_eviction.cpp +1/-1
  • src/test/fuzz/p2p_transport_serialization.cpp +1/-1
  • src/test/fuzz/parse_hd_keypath.cpp +1/-1
  • src/test/fuzz/parse_iso8601.cpp +1/-1
  • src/test/fuzz/parse_numbers.cpp +1/-1
  • src/test/fuzz/prevector.cpp +2/-0
  • src/test/fuzz/rolling_bloom_filter.cpp +1/-1
  • src/test/fuzz/rpc.cpp +1/-1
  • src/test/fuzz/script_flags.cpp +1/-1
  • src/test/fuzz/script_sign.cpp +1/-1
  • src/test/fuzz/scriptnum_ops.cpp +1/-1
  • src/test/random_tests.cpp +1/-1
  • src/test/sock_tests.cpp +1/-1
  • src/txdb.cpp +1/-1
  • src/txrequest.cpp +1/-1
  • src/uint256.h +1/-1
  • src/util/asmap.cpp +1/-1
  • src/util/chaintype.cpp +1/-1
  • src/wallet/coinselection.cpp +1/-1
  • src/wallet/spend.cpp +1/-1
  • src/wallet/test/group_outputs_tests.cpp +1/-1
  • src/bitcoin-chainstate.cpp +0/-1
  • src/coins.cpp +1/-0
  • src/coins.h +0/-1
  • src/compressor.cpp +1/-0
  • src/deploymentinfo.cpp +1/-0
  • src/deploymentstatus.h +1/-0
  • src/hash.h +1/-0
  • src/merkleblock.cpp +1/-0
  • src/net.cpp +1/-0
  • src/net_permissions.h +1/-0
  • src/netgroup.cpp +1/-0
  • src/netgroup.h +1/-0
  • src/node/coin.cpp +1/-0
  • src/policy/packages.cpp +0/-1
  • src/policy/policy.cpp +1/-0
  • src/protocol.cpp +1/-0
  • src/protocol.h +1/-0
  • src/qt/bitcoingui.cpp +1/-0
  • src/qt/guiutil.h +0/-1
  • src/qt/optionsmodel.h +0/-1
  • src/random.cpp +1/-0
  • src/random.h +0/-1
  • src/script/script.cpp +1/-0
  • src/span.h +0/-1
  • src/streams.h +0/-1
  • src/support/allocators/pool.h +0/-1
  • src/test/blockchain_tests.cpp +1/-0
  • src/test/btcsignals_tests.cpp +1/-0
  • src/test/chainstate_write_tests.cpp +1/-0
  • src/test/checkqueue_tests.cpp +1/-0
  • src/test/cuckoocache_tests.cpp +1/-0
  • src/test/flatfile_tests.cpp +1/-0
  • src/test/fuzz/addition_overflow.cpp +1/-0
  • src/test/fuzz/block_index.cpp +1/-0
  • src/test/fuzz/block_index_tree.cpp +1/-0
  • src/test/fuzz/connect_block.cpp +1/-0
  • src/test/fuzz/connman.cpp +1/-0
  • src/test/fuzz/crypto_chacha20.cpp +1/-0
  • src/test/fuzz/crypto_chacha20poly1305.cpp +1/-0
  • src/test/fuzz/crypto_diff_fuzz_chacha20.cpp +1/-0
  • src/test/fuzz/crypto_poly1305.cpp +1/-0
  • src/test/fuzz/dbwrapper.cpp +0/-1
  • src/test/fuzz/descriptor_parse.cpp +1/-0
  • src/test/fuzz/deserialize.cpp +1/-0
  • src/test/fuzz/fee_rate.cpp +1/-0
  • src/test/fuzz/integer.cpp +0/-1
  • src/test/fuzz/kitchen_sink.cpp +1/-0
  • src/test/fuzz/multiplication_overflow.cpp +1/-0
  • src/test/fuzz/netbase_dns_lookup.cpp +1/-0
  • src/test/fuzz/p2p_headers_presync.cpp +1/-0
  • src/test/fuzz/p2p_private_broadcast.cpp +1/-0
  • src/test/fuzz/poolresource.cpp +1/-0
  • src/test/fuzz/primitives_transaction.cpp +1/-0
  • src/test/fuzz/script_descriptor_cache.cpp +1/-0
  • src/test/fuzz/secp256k1_ec_seckey_import_export_der.cpp +1/-0
  • src/test/fuzz/span.cpp +0/-1
  • src/test/fuzz/tx_in.cpp +0/-1
  • src/test/fuzz/tx_out.cpp +1/-0
  • src/test/fuzz/util.h +1/-0
  • src/test/mempool_fee_estimator_tests.cpp +1/-0
  • src/test/net_tests.cpp +1/-0
  • src/test/scheduler_tests.cpp +1/-0
  • src/test/streams_tests.cpp +1/-0
  • src/test/threadpool_tests.cpp +1/-0
  • src/test/util/cluster_linearize.h +1/-0
  • src/test/util/poolresourcetester.h +0/-1
  • src/torcontrol.cpp +0/-1
  • src/txmempool.h +1/-0
  • src/util/obfuscation.h +1/-0
  • src/util/result.h +1/-0
  • src/validation.cpp +0/-1
  • src/wallet/coincontrol.cpp +1/-0
  • src/wallet/test/fuzz/wallet_bdb_parser.cpp +1/-0
  • src/wallet/test/psbt_wallet_tests.cpp +1/-0
  • src/wallet/test/spend_tests.cpp +1/-0
  • src/wallet/transaction.cpp +1/-0

Card

This PR introduces `AssertUnreachable` in `util/check.h` and replaces falsy assertion patterns like `assert(false)` across hundreds of files in the project, accompanied by two new linters. It aims to eliminate GCC return-type warnings caused by hidden noreturn attributes and prevent future use of `std::unreachable` which introduces undefined behavior. Review is currently stale as the PR has merge conflicts, failing CI, and an unresolved linkage issue where consensus code inadvertently pulled in `util/check`. It has no dependencies and is waiting for author updates.

Data

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