test: add feebumper coverage for combined bump fee failure #34924

pull MkDev11 wants to merge 2 commits into bitcoin:master from MkDev11:fix/issue-34902-feebumper-coverage changing 1 files +71 −0

MkDev11 commented at 9:08 AM on March 26, 2026: none
Closes #34902

Summary
- Add unit test coverage for the feebumper path where calculateCombinedBumpFee() returns no value.
- Build a 501-transaction unconfirmed dependency cluster and spend cluster tips in a wallet tx.
- Assert CreateRateBumpTransaction() returns WALLET_ERROR with the expected error string.
Test Plan
- build/bin/test_bitcoin --run_test=feebumper_tests --catch_system_error=no --log_level=message
- build/bin/test_bitcoin --run_test=feebumper_tests/feerate_check_handles_combined_bump_fee_unavailable --catch_system_error=no --log_level=message
DrahtBot added the label Tests on Mar 26, 2026
DrahtBot commented at 9:09 AM on March 26, 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. A summary of reviews will appear here.
DrahtBot added the label CI failed on Mar 30, 2026
DrahtBot removed the label CI failed on Apr 3, 2026
sedited commented at 11:36 AM on April 20, 2026: contributor

You will need to squash your last commit into the first one, if it fixes a compilation error.
test: cover feebumper combined bump fee failure 886eed1215
test: tighten feebumper cluster setup assertions f4ec053d63
MkDev11 force-pushed on Apr 20, 2026

in src/wallet/test/feebumper_tests.cpp:122 in 886eed1215

 117 | +    CAmount old_fee{0};
 118 | +    CAmount new_fee{0};
 119 | +    CMutableTransaction bumped_tx;
 120 | +    const Result result = CreateRateBumpTransaction(*wallet, original_txid, coin_control, errors, old_fee, new_fee, bumped_tx, /*require_mine=*/true, /*outputs=*/{}, /*original_change_index=*/std::nullopt);
 121 | +
 122 | +    BOOST_CHECK_EQUAL(static_cast<int>(result), static_cast<int>(Result::WALLET_ERROR));

brunoerg commented at 4:17 PM on April 22, 2026:

886eed1215b06b07d213bf4fadbbf0f4f4320a62: I think you don't need to do this static_cast, you can simply BOOST_CHECK(result == Result::WALLET_ERROR).

in src/wallet/test/feebumper_tests.cpp:82 in 886eed1215

  77 | +    TestMemPoolEntryHelper entry;
  78 | +    {
  79 | +        LOCK2(cs_main, pool.cs);
  80 | +        for (int cluster = 0; cluster < 10; ++cluster) {
  81 | +            CTransactionRef last_tx = m_coinbase_txns.at(cluster);
  82 | +            const int chain_length = cluster == 0 ? 51 : 50; // 501 total txs across all clusters.

brunoerg commented at 4:19 PM on April 22, 2026:

886eed1215b06b07d213bf4fadbbf0f4f4320a62: The comment 501 total tx across all clusters explains what but not why. My suggestion is mentioning the limit from GatherClusters .

in src/wallet/test/feebumper_tests.cpp:102 in f4ec053d63

  97 | +
  98 | +    {
  99 | +        LOCK(wallet->cs_wallet);
 100 | +        for (const auto& tip_tx : tip_txs) {
 101 | +            wallet->AddToWallet(tip_tx, TxStateInactive{}, [](CWalletTx& wtx, bool /*new_tx*/) {
 102 | +                wtx.m_state = TxStateInactive{};

brunoerg commented at 4:23 PM on April 22, 2026:

886eed1215b06b07d213bf4fadbbf0f4f4320a62: I think it can be simplified to wallet->AddToWallet(tip_tx, TxStateInactive{});.

Contributors

Labels

Linked (view graph)

#34902 Add test coverage for bump fee calculation failure in feebumper

test: add feebumper coverage for combined bump fee failure #34924

Summary

Test Plan

Reviews