wallet: don't double discard output groups with avoidpartialspends #36284

fjahr commented at 8:19 AM on September 17, 2026: contributor

With -avoidpartialspends or avoid_reuse, GroupOutputs puts every positive-value output into both the mixed and the positive-only map and runs push_output_groups on each, so a group rejected by the eligibility filters lands in discarded_groups twice. AutomaticCoinSelection then subtracts it twice and could fail with an insufficient funds error even when there would be enough confirmed coins to cover the payment.

Not a problem in a default wallet but it can happen with sendtoaddress from an avoid_reuse wallet, or with -avoidpartialspends=1.

The simplest possible fix is to record discards only from the mixed map pass, which contains every output anyway. Also adds a test that reproduces the issue.

wallet: don't double count discarded output groups with avoidpartialspends 20307f0ac0

DrahtBot added the label Wallet on Sep 17, 2026

DrahtBot commented at 8:19 AM on September 17, 2026: 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/36284.

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
ACK	pablomartin4btc, vicjuma, polespinasa, achow101

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

pablomartin4btc commented at 1:36 AM on September 18, 2026: member

tACK 20307f0ac0e17c6e478e2e11318dc7a9b18fc5d1

The test fails without the fix.

JSONRPCException: Unconfirmed UTXOs are available, but spending them creates a chain of transactions that will be rejected by the mempool (-6)

Verified that the bug is not reachable for a default wallet (created with defaults - no avoid_reuse -, and node started without -avoidpartialspends).

Fix doesn't touch actual coin selection (filtered_groups), only the early-exit error path, so no consensus/coin-selection-outcome risk.

Nice to have: no direct unit test for the GroupOutputs discarded-groups overload (the functional test covers it end-to-end but indirectly/slowly).

Also, it's worth release notes as sendtoaddress/send calls that previously failed spuriously under avoid_reuse or -avoidpartialspends=1 will now succeed; perhaps something like:

Wallet
------

- Fixed a bug where `sendtoaddress`/`send` could fail with a spurious
  "Insufficient funds" error on wallets with `avoid_reuse` enabled, or with
  `-avoidpartialspends=1` set, even when enough eligible funds were actually
  available. A coin group excluded from selection (for example, for exceeding
  the mempool ancestor/descendant limits) was being counted against the
  available balance twice instead of once. (#36284)

test: Check that ineligible output groups are discarded only once 719324977a

fjahr commented at 8:07 AM on September 18, 2026: contributor

Thanks for the review @pablomartin4btc !

Nice to have: no direct unit test for the GroupOutputs discarded-groups overload (the functional test covers it end-to-end but indirectly/slowly).

I added a minimal unit test for this in a separate commit by extending an existing test in group_outputs_tests.cpp that already covered GroupOutputs with a fitting scenario. The overload wasn't declared in the header yet, so that had to be added to make this test possible.

Also, it's worth release notes as sendtoaddress/send calls that previously failed spuriously under avoid_reuse or -avoidpartialspends=1 will now succeed;

Hm, I don't think we typically add release notes for bugfixes. I think it mostly makes sense if the change requires action from the user, like removing some file from their datadir, reindexing etc. but that is not the case here. If users ran into this in the past they may not even have noticed since we didn't see any bug reports on this afaict. So I don't think it rises to the level of being release-note worthy, but if others agree with you that it's valuable I am happy to add it.

pablomartin4btc commented at 4:10 PM on September 18, 2026: member

ACK 719324977a41ab5132699d9c81032f8b2687f599

Added a unit test for the GroupOutputs discarded-groups overload since my previous review.

in src/wallet/spend.cpp:676 in 719324977a

 671 | @@ -672,7 +672,8 @@ FilteredOutputGroups GroupOutputs(const CWallet& wallet,
 672 |                      filtered_groups[filter].Push(group, type, positive_only, /*insert_mixed=*/!positive_only);
 673 |                      accepted = true;
 674 |                  }
 675 | -                if (!accepted) ret_discarded_groups.emplace_back(group);
 676 | +                // The positive-only groups are a subset of the mixed ones, don't record them twice
 677 | +                if (!accepted && !positive_only) ret_discarded_groups.emplace_back(group);

vicjuma commented at 9:04 AM on September 23, 2026:

Used RPC commands to reproduce the issue (leveraged the unit test too)

Steps

MINER_WALLET=$(./bitcoin-cli -named createwallet wallet_name="miner" | jq -r '.name')
./bitcoin-cli -rpcwallet=$MINER_WALLET -generate 101
TEST_WALLET=$(./bitcoin-cli -named createwallet wallet_name="36284" disable_private_keys=false blank=false passphrase="" avoid_reuse=true | jq -r '.name')
TEST_WALLET_ADDR1=$(./bitcoin-cli -rpcwallet=$TEST_WALLET getnewaddress)
./bitcoin-cli -rpcwallet=$MINER_WALLET sendtoaddress $TEST_WALLET_ADDR1 0.3
./bitcoin-cli -rpcwallet=$MINER_WALLET -generate 1
for i in $(seq 1 25); do ADDR_N=$(./bitcoin-cli -rpcwallet=$TEST_WALLET getnewaddress) LAST_TXID=$(./bitcoin-cli -rpcwallet=$TEST_WALLET sendall "["$ADDR_N"]") echo "[$i/25] ADDR_N=$ADDR_N LAST_TXID=$LAST_TXID" done
./bitcoin-cli getmempoolentry $LAST_TXID
TEST_WALLET_ADDR2=$(./bitcoin-cli -rpcwallet=$TEST_WALLET getnewaddress)
MINER_WALLET_TXID=$(./bitcoin-cli -rpcwallet=$MINER_WALLET sendtoaddress $TEST_WALLET_ADDR2 0.7)
MINER_WALLET_ADDR1=$(./bitcoin-cli -rpcwallet=$MINER_WALLET getnewaddress)
./bitcoin-cli generateblock $MINER_WALLET_ADDR1 "["$MINER_WALLET_TXID"]"
MINER_WALLET_ADDR2=$(./bitcoin-cli -rpcwallet=$MINER_WALLET getnewaddress)
./bitcoin-cli -rpcwallet=$TEST_WALLET sendtoaddress $MINER_WALLET_ADDR2 0.65

Finding

From the test, it is seen that there is indeed double UTXO counting that leads to the failed sendtoaddress of 0.65 that should be successful since only 0.3 should be 'discarded' instead of twice the amount

Outputs

Before:

ratedg@0xratedg:~/projects/contributions/bitcoin/build/bin$ ./bitcoin-cli -rpcwallet=$TEST_WALLET sendtoaddress $MINER_WALLET_ADDR2 0.65
error code: -6
error message:
Unconfirmed UTXOs are available, but spending them creates a chain of transactions that will be rejected by the mempool

After:

ratedg@0xratedg:~/projects/contributions/bitcoin/build2/bin$ ./bitcoin-cli getmempoolentry 9dcdf2a114a123273d53399469063defb142d37973bbcbaabe10e959963beb6e
{
  "vsize_adjusted": 110,
  "vsize": 110,
  "vsize_bip141": 110,
  "weight": 437,
  "time": 1790152439,
  "height": 102,
  "descendantcount": 1,
  "descendantsize": 110,
  "ancestorcount": 25,
  "ancestorsize": 2750,
  "wtxid": "a4d3ecebf8e0dc8fa93273664ea61893762a9f59ef4c7821785c01f96ccf6304",
  "chunkweight": 437,
  "fees": {
    "base": 0.00001100,
    "modified": 0.00001100,
    "ancestor": 0.00027500,
    "descendant": 0.00001100,
    "chunk": 0.00001100
  },
  "depends": [
    "19a95372d996bb24cbb934c4314bb2100e34b8dfe55ab8b6a370fdde6007443f"
  ],
  "spentby": [
  ],
  "unbroadcast": true
}
ratedg@0xratedg:~/projects/contributions/bitcoin/build2/bin$ TEST_WALLET_ADDR2=$(./bitcoin-cli -rpcwallet=$TEST_WALLET getnewaddress)
ratedg@0xratedg:~/projects/contributions/bitcoin/build2/bin$ MINER_WALLET_TXID=$(./bitcoin-cli -rpcwallet=$MINER_WALLET sendtoaddress $TEST_WALLET_ADDR2 0.7)
ratedg@0xratedg:~/projects/contributions/bitcoin/build2/bin$ MINER_WALLET_ADDR1=$(./bitcoin-cli -rpcwallet=$MINER_WALLET getnewaddress)
ratedg@0xratedg:~/projects/contributions/bitcoin/build2/bin$ ./bitcoin-cli generateblock $MINER_WALLET_ADDR1 "[\"$MINER_WALLET_TXID\"]"
{
  "hash": "6886bb18d6b98152d21e53259f7548ceb003878d399d5d3b8f3d24c192386d0f"
}
ratedg@0xratedg:~/projects/contributions/bitcoin/build2/bin$ MINER_WALLET_ADDR2=$(./bitcoin-cli -rpcwallet=$MINER_WALLET getnewaddress)
ratedg@0xratedg:~/projects/contributions/bitcoin/build2/bin$ ./bitcoin-cli -rpcwallet=$TEST_WALLET sendtoaddress $MINER_WALLET_ADDR2 0.65
0b37799db7bbc95014fd17cb0d87df8c6496424bebe3443c3aff2c9d97f964e4
ratedg@0xratedg:~/projects/contributions/bitcoin/build2/bin$

in src/wallet/spend.h:126 in 719324977a

 119 | @@ -120,6 +120,15 @@ FilteredOutputGroups GroupOutputs(const CWallet& wallet,
 120 |                            const CoinSelectionParams& coin_sel_params,
 121 |                            const std::vector<SelectionFilter>& filters);
 122 |  
 123 | +/**
 124 | + * Group coins by the provided filters, groups that pass no filter are appended to `ret_discarded_groups`.
 125 | + */
 126 | +FilteredOutputGroups GroupOutputs(const CWallet& wallet,

vicjuma commented at 9:09 AM on September 23, 2026:

Necessary for src/wallet/spend.cpp:930: FilteredOutputGroups filtered_groups = GroupOutputs(wallet, available_coins, coin_selection_params, ordered_filters, discarded_groups);

vicjuma commented at 9:15 AM on September 23, 2026: contributor

ACK 719324977a41ab5132699d9c81032f8b2687f599

polespinasa commented at 9:21 AM on September 24, 2026: member

code reviewed ACK 719324977a41ab5132699d9c81032f8b2687f599

I don't have any comment, I think this is fine as is.

achow101 commented at 9:10 PM on September 24, 2026: member

ACK 719324977a41ab5132699d9c81032f8b2687f599

achow101 merged this on Sep 24, 2026

achow101 closed this on Sep 24, 2026