Plumb "too-long-mempool-chain" to RPC error for send/sendtoaddress

JeremyRand commented at 1:23 pm on September 30, 2021: contributor

Is your feature request related to a problem? Please describe.

If coin selection fails as part of send or sendtoaddress, the error message returned to the RPC client is Insufficient funds, even if the actual cause is due to too-long-mempool-chain. This is likely to alarm users unnecessarily, as they will believe this means their funds have permanently vanished rather than being temporarily inaccessible.

Describe the solution you’d like

If coin selection fails due to too-long-mempool-chain, the send and sendtoaddress methods should pass that error back to the RPC client, instead of Insufficient funds.

Describe alternatives you’ve considered

It would be even nicer from a UX standpoint if the limit on mempool chain length were removed, but AIUI that’s a DoS vector, so is not going to happen.

Additional context

Apparently this is already done for sendrawtransaction. The discussion in #10015 is relevant (it appears that the PR was withdrawn for reasons unrelated to my request).

JeremyRand added the label Feature on Sep 30, 2021

jamesob commented at 9:21 pm on January 27, 2022: member

The relevant code seems to be here: https://github.com/bitcoin/bitcoin/blob/master/src/wallet/rpc/spend.cpp#L73-L76

glozow commented at 12:01 pm on March 8, 2022: member

Seems to me that “insufficient funds” is the correct error message here if you cannot construct a transaction that the mempool would accept. Similarly, you may get “insufficient funds” if your coins are premature or nonfinal. If a user is interested in a more detailed breakdown of their coins, including whether things are spendable / safe to spend, they might want to use something like listunspent.

JeremyRand commented at 12:45 pm on March 9, 2022: contributor

Seems to me that “insufficient funds” is the correct error message here if you cannot construct a transaction that the mempool would accept. Similarly, you may get “insufficient funds” if your coins are premature or nonfinal. If a user is interested in a more detailed breakdown of their coins, including whether things are spendable / safe to spend, they might want to use something like listunspent. @glozow Why do you think the too-long-mempool-chain error code exists if it shouldn’t be used here?

glozow commented at 1:39 pm on March 9, 2022: member

too-long-mempool-chain corresponds to the reason a transaction is rejected from mempool. It’s helpful, for example, to debug a transaction you submitted through sendrawtransaction or testmempoolaccept. It’s also logged for rejected p2p transactions.

JeremyRand commented at 6:58 am on March 14, 2022: contributor

@glozow Okay, so given your description, I don’t understand why that code wouldn’t be intuitively applicable to send when the method fails because the transaction would be rejected from the mempool. The status quo is confusing to nontechnical users (source: personal experience providing tech support to users who think their funds have been stolen after seeing the “Insufficient funds” error code).

FWIW I agree with @achow101 in #24502 (comment) :

I think “insufficient funds” is too generic of an error message for coin selection failures as there are a ton of different reasons why coin selection might fail. I would like to have more granular error messages

achow101 commented at 10:15 am on March 14, 2022: member

We can have errors that contain more explanation than just a few words. For example, this error could be “Insufficient funds: Too long mempool chain. Creating this transaction would require creating a chain of unconfirmed transaction that will currently be rejected by the mempool.”

JeremyRand commented at 10:55 am on March 14, 2022: contributor

We can have errors that contain more explanation than just a few words. For example, this error could be “Insufficient funds: Too long mempool chain. Creating this transaction would require creating a chain of unconfirmed transaction that will currently be rejected by the mempool.” @achow101 I have no objection to this suggestion.

glozow commented at 10:55 am on March 14, 2022: member

“Insufficient funds: Too long mempool chain. Creating this transaction would require creating a chain of unconfirmed transaction that will currently be rejected by the mempool.”

I might prefer something like “insufficient funds: unconfirmed UTXOs are available, but spending them creates a chain of transactions that will be rejected by the mempool. These funds will become available when the transaction confirms” or something.

In general, I’m not saying I’m against more specific error messages. I’m saying that, since the user was not responsible for creating the transaction in the first place, it seems unhelpful to tell them the transaction was too-long-mempool-chain, as they can’t do anything differently to change this. The information that would be helpful to them is “you have money, but it’s in unconfirmed UTXOs and I can’t create a valid transaction right now.”

glozow added the label Wallet on Aug 4, 2022

achow101 referenced this in commit 3f8591d46b on Jan 3, 2023

sidhujag referenced this in commit cab438784e on Jan 4, 2023

glozow closed this on Mar 23, 2023

sidhujag referenced this in commit c8ff93c4af on Mar 24, 2023

bitcoin locked this on Mar 22, 2024

Plumb “too-long-mempool-chain” to RPC error for send/sendtoaddress #23144