Wallet: Nondescript error message for 502nd unconfirmed transaction #29711

issue tdb3 opened this issue on March 23, 2024
  1. tdb3 commented at 5:58 PM on March 23, 2024: contributor

    Is there an existing issue for this?

    • I have searched the existing issues

    Current behaviour

    When creating a large number of unconfirmed transactions (e.g. 500+) with bitcoin-cli by spending/sending from a wallet, sending/spending the 502nd time results in a nondescript error message. It may be the case that this is expected behavior (e.g. reaching a limit), and the error message could be made more descriptive/informative.

    error code: -1
    error message:
    map::at
    

    This was encountered when creating a test to populate the mempool with a high number of transactions for the v27.0 RC Testing Guide. This behavior was observed both on v26.0 and v27.0 rc1.

    Confirming the transactions in a block allows the user to perform additional spending.

    Expected behaviour

    A more descriptive error message would be provided to the user. In this case, if a limit is being reached, the user could be informed and take appropriate action (e.g. wait until a block confirms transactions, etc.).

    Steps to reproduce

    Start a node on regtest with its own datadir/bitcoin.conf:

    export DATA_DIR=/tmp/tmpdatadir
    mkdir $DATA_DIR
    export BINARY_PATH=/path/to/dir/containing/bitcoind
    alias bitcoind-test="$BINARY_PATH/bitcoind -datadir=$DATA_DIR"
    alias bcli="$BINARY_PATH/bitcoin-cli -datadir=$DATA_DIR"
    echo "regtest=1" > $DATA_DIR/bitcoin.conf
    bitcoind-test -daemon
    

    Create a wallet, fund it with coinbase outputs, and send 501 times:

    bcli createwallet test
    export ADDRESS=$(bcli -rpcwallet=test getnewaddress)
    bcli -rpcwallet=test -generate 701
    for i in $(seq 1 499); do bcli -rpcwallet=test -named send outputs="{\"$ADDRESS\": 1}" fee_rate=10; done
    bcli -rpcwallet=test getmempoolinfo
    bcli -rpcwallet=test -named send outputs="{\"$ADDRESS\": 1}" fee_rate=10
    bcli -rpcwallet=test getmempoolinfo
    bcli -rpcwallet=test -named send outputs="{\"$ADDRESS\": 1}" fee_rate=10
    bcli -rpcwallet=test getmempoolinfo
    

    Induce the error on the 502nd send:

    bcli -rpcwallet=test -named send outputs="{\"$ADDRESS\": 1}" fee_rate=10
    error code: -1
    error message:
    map::at
    

    Confirm transactions, then add a new transaction:

    bcli -rpcwallet=test -generate 1
    bcli -rpcwallet=test -named send outputs="{\"$ADDRESS\": 1}" fee_rate=10
    bcli -rpcwallet=test getmempoolinfo
    

    Relevant log output

    Nothing relevant observed in debug.log at the time of the error received.

    How did you obtain Bitcoin Core

    Pre-built binaries

    What version of Bitcoin Core are you using?

    v26.0.0

    Operating system and version

    Ubuntu 22.04 LTS

    Machine specifications

    amd64

  2. achow101 commented at 6:19 PM on March 23, 2024: member

    It's hitting the 500 tx cluster limit in MiniMiner, which I think we should keep. The error message just needs to be better. The error is happening when the result of CalculateIndividualBumpFees is an empty map, but we try to get things from it, so the fix would be to return a proper error when that map is empty.

  3. maflcko added this to the milestone 27.0 on Mar 24, 2024
  4. maflcko added the label Bug on Mar 24, 2024
  5. furszy commented at 8:31 PM on March 25, 2024: member

    It's hitting the 500 tx cluster limit in MiniMiner, which I think we should keep. The error message just needs to be better. The error is happening when the result of CalculateIndividualBumpFees is an empty map, but we try to get things from it, so the fix would be to return a proper error when that map is empty.

    I'm unsure whether a wallet containing a group of 500 independent unconfirmed transactions should block transaction creation until those transactions are confirmed. Would be good to obtain some usage metrics. Because this shouldn't present a DoS vector, and we could expand the limit to accept more standalone transactions while maintaining the current limit for clusters with more than one element. Thoughts?

  6. ismaelsadeeq commented at 9:22 PM on March 25, 2024: member

    I was able to reproduce same error on master with the steps from @tdb3 53f4607cc8c67366662f49cb312d2e4ff8b6523a

    ......
    
    abubakarismail@Abubakars-MacBook-Pro bitcoin %  ./src/bitcoin-cli -regtest -named send  outputs="{\"$ADDRESS\": 1}" fee_rate=10
    
    error code: -1
    error message:
    map::at:  key not found                                                                                          
    

    A naive approach of simply preventing accessing returned value of calculateIndividualBumpFees when its empty solves the issue.

    <details> <summary>diff</summary>

    diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
    index 5d23ebd35a..12d320861f 100644
    --- a/src/wallet/spend.cpp
    +++ b/src/wallet/spend.cpp
    @@ -296,7 +296,9 @@ util::Result<PreSelectedInputs> FetchSelectedInputs(const CWallet& wallet, const
     
             /* Set some defaults for depth, spendable, solvable, safe, time, and from_me as these don't matter for preset inputs since no selection is being done. */
             COutput output(outpoint, txout, /*depth=*/ 0, input_bytes, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, /*time=*/ 0, /*from_me=*/ false, coin_selection_params.m_effective_feerate);
    -        output.ApplyBumpFee(map_of_bump_fees.at(output.outpoint));
    +        if (!map_of_bump_fees.empty()) {
    +            output.ApplyBumpFee(map_of_bump_fees.at(output.outpoint));
    +        }
             result.Insert(output, coin_selection_params.m_subtract_fee_outputs);
         }
         return result;
    @@ -455,10 +457,11 @@ CoinsResult AvailableCoins(const CWallet& wallet,
     
         if (feerate.has_value()) {
             std::map<COutPoint, CAmount> map_of_bump_fees = wallet.chain().calculateIndividualBumpF
    ees(outpoints, feerate.value());
    -
    -        for (auto& [_, outputs] : result.coins) {
    -            for (auto& output : outputs) {
    -                output.ApplyBumpFee(map_of_bump_fees.at(output.outpoint));
    +        if (!map_of_bump_fees.empty()) {
    +            for (auto& [_, outputs] : result.coins) {
    +                for (auto& output : outputs) {
    +                    output.ApplyBumpFee(map_of_bump_fees.at(output.outpoint));
    +                }
                 }
             }
         }
    

    </details>

    I can create more transactions

    abubakarismail@Abubakars-MacBook-Pro bitcoin % for i in $(seq 1 4); do ./src/bitcoin-cli -regtest -named send  outputs="{\"$ADDRESS\": 1}" fee_rate=10; done 
    {
      "txid": "87c5c6564bf5c810be070c2fd5ee7b9e75cd93a6b0f3d0ea85163c5da088e857",
      "complete": true
    }
    {
      "txid": "fdc207674a7fa25ae16651ec69f5b7eb8f96b09c3b8c050648e6eb32db7e2532",
      "complete": true
    }
    {
      "txid": "375ad1485ad7f02221e9c79a81ca17ecbcaaf781165807ad23ef8198585e6fc9",
      "complete": true
    }
    {
      "txid": "30284d52762e9c2b4255a31fa851b8b745abd371bfa086e2c2f9edb00a707054",
      "complete": true
    }
    

    The error message just needs to be better. The error is happening when the result of CalculateIndividualBumpFees is an empty map

    The empty map could also be because bump fees have already been calculated, mini miner error message should also be more verbose for each case?

  7. furszy commented at 9:25 PM on March 25, 2024: member

    A naive approach of simply preventing accessing returned value of calculateIndividualBumpFees when its empty solves the issue.

    That would fallback to the pre ancestors aware funding status. Which isn't desirable.

  8. ismaelsadeeq commented at 9:42 PM on March 25, 2024: member

    Oh, thanks @furszy! I can update the return type of AvailableCoins to util::Result<CoinsResult> with a more descriptive error message for when the cluster limit is hit.

    <details> <summary>diff</summary>

    diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
    index 5d23ebd35a..646d93bcd7 100644
    --- a/src/wallet/spend.cpp
    +++ b/src/wallet/spend.cpp
    @@ -296,13 +296,16 @@ util::Result<PreSelectedInputs> FetchSelectedInputs(const CWallet& wallet, const
     
             /* Set some defaults for depth, spendable, solvable, safe, time, and from_me as these don't matter for preset inputs since no selection is being done. */
             COutput output(outpoint, txout, /*depth=*/ 0, input_bytes, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, /*time=*/ 0, /*from_me=*/ false, coin_selection_params.m_effective_feerate);
    +        if (map_of_bump_fees.empty()) {
    +            return util::Error{strprintf(_("Too many connected unconfirmed transactions, maximum cluster limit exceeded"))};
    +        }
             output.ApplyBumpFee(map_of_bump_fees.at(output.outpoint));
             result.Insert(output, coin_selection_params.m_subtract_fee_outputs);
         }
         return result;
     }
     
    -CoinsResult AvailableCoins(const CWallet& wallet,
    +util::Result<CoinsResult> AvailableCoins(const CWallet& wallet,
                                const CCoinControl* coinControl,
                                std::optional<CFeeRate> feerate,
                                const CoinFilterParams& params)
    @@ -456,9 +459,15 @@ CoinsResult AvailableCoins(const CWallet& wallet,
         if (feerate.has_value()) {
             std::map<COutPoint, CAmount> map_of_bump_fees = wallet.chain().calculateIndividualBumpF
    ees(outpoints, feerate.value());
     
    -        for (auto& [_, outputs] : result.coins) {
    -            for (auto& output : outputs) {
    -                output.ApplyBumpFee(map_of_bump_fees.at(output.outpoint));
    +        if (map_of_bump_fees.empty()) {
    +            if  (!outpoints.empty()) {
    +                return util::Error{strprintf(_("Too many connected unconfirmed transactions, maximum cluster limit exceeded"))};
    +            }
    +        } else {
    +            for (auto& [_, outputs] : result.coins) {
    +                for (auto& output : outputs) {
    +                    output.ApplyBumpFee(map_of_bump_fees.at(output.outpoint));
    +                }
                 }
             }
         }
    diff --git a/src/wallet/spend.h b/src/wallet/spend.h
    index 62a7b4e4c8..c387cb69bb 100644
    --- a/src/wallet/spend.h
    +++ b/src/wallet/spend.h
    @@ -83,7 +83,7 @@ struct CoinFilterParams {
     /**
      * Populate the CoinsResult struct with vectors of available COutputs, organized by OutputType.
      */
    -CoinsResult AvailableCoins(const CWallet& wallet,
    +util::Result<CoinsResult> AvailableCoins(const CWallet& wallet,
                                const CCoinControl* coinControl = nullptr,
                                std::optional<CFeeRate> feerate = std::nullopt,
                                const CoinFilterParams& params = {}) EXCLUSIVE_LOCKS_REQUIRED(wallet
    .cs_wallet);
    (END)
    

    </details>

    Would it be better to instead modify Mini Miner to return the error message?

  9. furszy commented at 9:55 PM on March 25, 2024: member

    Would it be better to instead modify Mini Miner to return the error message?

    Yes. It will be better to bubble up the error message from MiniMiner. But, before getting into the implementation details, let's first try to finish discussing the issue. I left a comment above (https://github.com/bitcoin/bitcoin/issues/29711#issuecomment-2018862521) that would be nice to follow it up.

  10. tdb3 commented at 12:24 AM on March 26, 2024: contributor

    I'm unsure whether a wallet containing a group of 500 independent unconfirmed transactions should block transaction creation until those transactions are confirmed. Would be good to obtain some usage metrics. Because this shouldn't present a DoS vector, and we could expand the limit to accept more standalone transactions while maintaining the current limit for clusters with more than one element. Thoughts?

    This seems reasonable to me, but I'm unaware of the history of the 500 limit and the rationale for its implementation (or tradeoffs). The existing "steps to reproduce" relies on the wallet's coin selection (so these transactions might not be independent depending on the selection algorithm). I ran another test that explicitly spends only the mature coinbase outputs independently. The error wasn't received. All transactions were allowed, so it appears that in the existing implementation, independent unconfirmed transactions are not affected by the limit?

    bcli createwallet test
    export ADDRESS=$(bcli -rpcwallet=test getnewaddress)
    bcli -rpcwallet=test -generate 701
    bcli -rpcwallet=test listunspent | grep txid | awk '{print $2}' | sed 's/"//g' | sed 's/,//g' | while read -r line; do bcli -rpcwallet=test -named send outputs="{\"$ADDRESS\": 1}" fee_rate=10 inputs=[{\"txid\":\ \"$line\"\,\ \"vout\":\ 0\,\"sequence\":\ \"4294967295\"}]; done
    bcli -rpcwallet=test getmempoolinfo
    {
      "loaded": true,
      "size": 601,
      "bytes": 84738,
      "usage": 714096,
      "total_fee": 0.00847410,
      "maxmempool": 300000000,
      "mempoolminfee": 0.00001000,
      "minrelaytxfee": 0.00001000,
      "incrementalrelayfee": 0.00001000,
      "unbroadcastcount": 601,
      "fullrbf": false
    }
    
    
  11. furszy commented at 2:10 AM on March 26, 2024: member

    I ran another test that explicitly spends only the mature coinbase outputs independently. The error wasn't received. All transactions were allowed, so it appears that in the existing implementation, independent unconfirmed transactions are not affected by the limit?

    It isn't failing because your script skips the unconfirmed outputs. The send() RPC command provides 'minconf=1' by default. This functional test checks the case of independent unconfirmed transactions and triggers the failure consistently:

    diff --git a/test/functional/wallet_spend_unconfirmed.py b/test/functional/wallet_spend_unconfirmed.py
    --- a/test/functional/wallet_spend_unconfirmed.py	(revision 8ee4a2600371062ddda0ae623bc0b934c308d594)
    +++ b/test/functional/wallet_spend_unconfirmed.py	(date 1711418421497)
    @@ -465,6 +465,25 @@
     
             wallet.unloadwallet()
     
    +    def test_independent_unconfirmed_txs_limit(self):
    +        self.nodes[0].createwallet("independent_txs")
    +        wallet = self.nodes[0].get_wallet_rpc("independent_txs")
    +        # Generate spendable independent txs
    +        self.generatetoaddress(self.nodes[0], 700, wallet.getnewaddress())
    +
    +        # Consume 500 txs
    +        inputs = wallet.listunspent()
    +        assert len(inputs) > 500
    +        address = wallet.getnewaddress()
    +        for i in range(501):
    +            res = wallet.send(outputs=[{address: 1}],
    +                              options={"minconf": 1, "fee_rate": 10, "add_inputs": False, "inputs": [inputs[i]]})
    +            assert 'txid' in res
    +
    +        assert len(wallet.listunspent(minconf=0)) > 500
    +        assert_equal(wallet.getmempoolinfo()['size'], 501)
    +        # Trigger failure
    +        assert 'txid' in wallet.send(outputs=[{address: 5}], options={"minconf": 0, "maxconf": 0, "fee_rate": 10})
     
         def run_test(self):
             self.log.info("Starting UnconfirmedInputTest!")
    @@ -504,5 +523,7 @@
     
             self.test_external_input_unconfirmed_low()
     
    +        self.test_independent_unconfirmed_txs_limit()
    +
     if __name__ == '__main__':
         UnconfirmedInputTest().main()
    
  12. murchandamus commented at 3:11 PM on March 26, 2024: member

    We should definitely have a more descriptive error, and I expect that it would also feel like a bug to any user encountering it live. We probably do want to keep the limit, so there might be two avenues to mitigate the impact a bit:

    If we encounter the limit:

    • instead of outright failing, we could fall back to try building a transaction only with confirmed inputs
    • we could fall back to processing UTXOs in smaller batches to stay under the limit, although that ensue a noticeable slow down of transaction building if there are a lot of unconfirmed transactions in the wallet
  13. ismaelsadeeq commented at 3:01 PM on April 9, 2024: member

    Currently, if you have more than 500 independent unconfirmed transactions, the wallet prevent users from creating a transaction with an unconfirmed input to prevent some DOS vector.

    The limit is set by the CTxMempool::GatherClusters method, which was added with the introduction of the mini miner and package-aware funding to prevent the calculation of clusters containing 500 or more unconfirmed transactions in #27021.

    I’ve bisected, and this issue does not occur on ab42b2ebdbf61225e636e4c00068fd29b2790d41 prior to introduction package aware funding.

    It’s unclear to me why 500 is selected and which DOS limit we are preventing here, especially in this case where all 500 transactions are independent and unconfirmed.

    I see from previous comments that the chosen number was arbitrary. (Source: discussion)

    It’s suggested here (from review comment) that if a wallet deliberately wants to calculate the bump fees of maybe more than 500 unconfirmed transactions, it should do so in batches, which I believe is similar to what @murchandamus suggested?

    If the batching is going be performed in mini miner, by batching GatherCluster calls I think it should not have some significant performance issues if the > 500 transactions are all independent unconfirmed transactions. Also I don't think it will be desirable to batch the calculateIndividualBumpFees calss from the wallet each there will be lots of redundant work

    I’ve seen a comment from @furszy on refactoring the mini miner to improve performance, which I don't fully understand. (Source: discussion) Maybe you could clarify your review comment and whether this will benefit solving this issue.

    Alternatively, within GatherCluster, we could check if the transactions ancestors and descendants we processed are 500. This way, we are indeed preventing some expensive work.

    diff --git a/src/txmempool.cpp b/src/txmempool.cpp
    index 226dd9f353..1be14ce843 100644
    --- a/src/txmempool.cpp
    +++ b/src/txmempool.cpp
    @@ -1207,13 +1207,15 @@ std::vector<CTxMemPool::txiter> CTxMemPool::GatherClusters(const std::vector<uin
         for (const auto& it : clustered_txs) {
             visited(it);
         }
    +    size_t calculated_cluster_count{0};
         // i = index of where the list of entries to process starts
         for (size_t i{0}; i < clustered_txs.size(); ++i) {
             // DoS protection: if there are 500 or more entries to process, just quit.
    -        if (clustered_txs.size() > 500) return {};
    +        if (calculated_cluster_count > 500) return {};
             const txiter& tx_iter = clustered_txs.at(i);
             for (const auto& entries : {tx_iter->GetMemPoolParentsConst(), tx_iter->GetMemPoolChildrenConst()}) {
                 for (const CTxMemPoolEntry& entry : entries) {
    +                calculated_cluster_count++;
                     const auto entry_it = mapTx.iterator_to(entry);
                     if (!visited(entry_it)) {
                         clustered_txs.push_back(entry_it);
    (END)
    

    And then when modify mini miner to return more descriptive error to the wallet, i.e

    1. When DOS limit is hit
    2. When bump fees have already been calculated.

    When DOS limit is hit I agree with @murchandamus that we could fall back to try building a transaction only with confirmed inputs.

    Because currently in cases where most of the transactions are independent without any ancestor or descendant, I don't think the 500 limit is preventing an expensive computation because it's not like the whole mempool is going to be in users wallet. Thoughts?

  14. fanquake removed this from the milestone 27.0 on Apr 16, 2024
  15. fanquake added this to the milestone 27.1 on Apr 16, 2024
  16. fanquake removed this from the milestone 27.1 on Jun 19, 2024
  17. fanquake commented at 9:12 AM on June 19, 2024: member

    Not sure what the status of this is, so removed it from any milestone for now.

  18. shuv-amp referenced this in commit cd1c0cba50 on Feb 28, 2026
  19. shuv-amp referenced this in commit 5f94606927 on Feb 28, 2026
  20. shuv-amp referenced this in commit a241bd8fb1 on Mar 25, 2026
  21. shuv-amp referenced this in commit 69b9601da8 on Mar 25, 2026
  22. cprkrn referenced this in commit b5ab51102d on Mar 31, 2026
  23. cprkrn referenced this in commit c6198f4bda on Mar 31, 2026
  24. cprkrn referenced this in commit b5dbe4abdc on Apr 1, 2026
  25. shuv-amp referenced this in commit e0ef938760 on May 1, 2026
  26. shuv-amp referenced this in commit 119c28b0cb on May 2, 2026
  27. shuv-amp referenced this in commit a34afcd436 on Aug 19, 2026
  28. Amperstrand referenced this in commit af66ef14df on Sep 5, 2026
  29. Amperstrand commented at 3:02 PM on September 5, 2026: none

    Reproduced on regtest at current master (4519933391dd, Sep 2026) — the report's exact failure and workaround both hold, and the root cause is traceable end-to-end.

    Reproducer (self-contained, ~1 min, deterministic; exit 3 = reproduced): https://gist.github.com/Amperstrand/8440a5cf1dfa8c922a9148e650b3ea30 Transcript (observed failure at the pinned revision): https://gist.github.com/Amperstrand/3a519e8669919b6a28386c4d5744dfc6

    Observed sequence, matching the report:

    send [#500](/github-metadata-backup-bitcoin-bitcoin/500/) ok; mempool size 500
    send [#501](/github-metadata-backup-bitcoin-bitcoin/501/) ok; mempool size 501
    send [#502](/github-metadata-backup-bitcoin-bitcoin/502/) → error code: -1 / error message: map::at
    -generate 1; retry → succeeds
    

    Root cause chain at 4519933391dd (all sites verified by reading):

    1. src/wallet/spend.cpp:1216CreateTransactionInternal passes m_effective_feerate into AvailableCoins, activating the bump-fee path.
    2. src/wallet/spend.cpp:512-516 (same pattern at spend.cpp:310 for pre-selected inputs) — unguarded map_of_bump_fees.at(output.outpoint) after calculateIndividualBumpFees.
    3. src/node/interfaces.cpp:705-714 — delegates to MiniMiner(...).CalculateBumpFees(...); note the no-mempool branch directly above prefills a 0 entry for every outpoint — that is the contract the wallet relies on.
    4. src/txmempool.cpp:1035-1038GatherClusters: if (ret.size() > 500) return {}; (DoS guard).
    5. src/node/mini_miner.cpp:60-67 — empty gathered cluster ⇒ m_ready_to_calculate = false.
    6. src/node/mini_miner.cpp:309-310CalculateBumpFees returns an empty map when not ready — the only path that violates the every-outpoint-has-an-entry contract.
    7. spend.cpp:516 .at() throws std::out_of_range("map::at"), uncaught by the RPC layer → the nondescript error.

    Why exactly the 502nd send: each unconfirmed self-send leaves one spendable mempool tx; at send #502 the candidate outpoints span 501 mempool entries, so the gather exceeds the 500 guard and the bump-fee map comes back empty. Confirmed UTXOs need no bump fee — hence mining one block restores spending, as reported.

    Two fix candidates, each with a regression test (test/functional/wallet_spend_cluster_limit.py, ~12 s, red on master with exactly map::at at the 502nd send):

    Happy to open a PR for whichever semantics maintainers prefer.


    Disclaimer: the reproduction, root-cause analysis, and the two fix candidates above were produced with agentic tooling (an autonomous reproduction harness driving a disposable build VM), then verified by direct execution: the regression test fails with map::at on pristine master 4519933391dd and passes on both fix branches; every cited file:line was read at that revision.


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-09-09 07:57 UTC