Merge bitcoin/bitcoin#36344: wallet: reject duplicate inputs in sendall
What the developer wrote
Authored by merge-script
Merge bitcoin/bitcoin#36344: wallet: reject duplicate inputs in sendall
654a92a9d3e76f3ff4ec4975f4fbcfba1a6091be wallet: reject duplicate inputs in sendall (Bruce0X)
Pull request description:
`sendall` with the `inputs` option doesn't check for duplicates. Each listed input is added to `total_input_value` in the loop at spend.cpp:1472, so passing the same UTXO twice counts its value twice.
On regtest, with a wallet that has a single 1 BTC UTXO:
```
sendall '["<addr>"]' '{"inputs": [<utxo>, <utxo>]}'
```
- the RPC returns `complete: true`
- the committed tx has 2 inputs (the same outpoint twice) and one output of 1.99998230 BTC
- it isn't in the mempool, and `testmempoolaccept` rejects it with `bad-txns-inputs-duplicate`
- after that, `listunspent` is empty and `getbalances` shows `nonmempool: -1.0` until the tx is abandoned
`send` and `walletcreatefundedpsbt` aren't affected, because `CCoinControl::Select` removes duplicates.
This rejects a duplicated input with `Invalid parameter, duplicated input: <txid>:<vout>` (RPC_INVALID_PARAMETER). The message matches what `createrawtransaction` says for duplicated addresses. The new `sendall_fails_on_duplicate_input` test fails on master ("No exception raised") and passes with the fix. `wallet_send.py` still passes.
ACKs for top commit:
l0rinc:
ACK 654a92a9d3e76f3ff4ec4975f4fbcfba1a6091be
maflcko:
review ACK 654a92a9d3e76f3ff4ec4975f4fbcfba1a6091be 🚪
molnard:
tACK 654a92a9d3e76f3ff4ec4975f4fbcfba1a6091be
Tree-SHA512: ae8541f9458a12e97d6c516cd2327519d2be8cf0e7501622c7a9d295ecbfd61290d7f874e205cf1e8fc8bcc147edc55e4d773fbe700b6a20db4a7071a9a97b19
What changed, and why it matters
This patch fixes a bug in Bitcoin Core's `sendall` wallet RPC command. Previously, if a user accidentally listed the same coin (UTXO) twice in the `inputs` option, the command would create an invalid transaction that counted the coin's value twice. The transaction would be rejected by the network, but it would temporarily confuse the wallet's balance display. The fix now rejects duplicate inputs immediately with a clear error message.
Apply the patch. It is a low-risk, defensive validation fix with a regression test. No immediate incident response is required because the invalid transactions cannot be mined or relayed.
Security signals we found
Input validation bypass leading to creation of an invalid transaction
Wallet balance accounting temporarily corrupted (negative nonmempool balance)
Denial-of-service-like user impact: funds appear missing until transaction is abandoned
No network-level consensus impact; transactions are rejected by mempool policy
Evidence from the diff
The sendall RPC in src/wallet/rpc/spend.cpp did not deduplicate entries in the user-supplied inputs option. Each input was added to total_input_value, so a duplicated outpoint inflated the apparent input total. The resulting transaction contained duplicate vin entries and was rejected by testmempoolaccept with bad-txns-inputs-duplicate. The patch adds an unordered_set with SaltedOutpointHasher to detect duplicate COutPoint values and throws RPC_INVALID_PARAMETER before any transaction is created or wallet state is affected. A regression test is included.
Changed components
src/wallet/rpc/spend.cppwallet sendall RPCtest/functional/wallet_sendall.pyInspect captured patch +27 / −0
### src/wallet/rpc/spend.cpp
@@ -12,6 +12,7 @@
#include <rpc/rawtransaction_util.h>
#include <rpc/util.h>
#include <script/script.h>
+#include <util/hasher.h>
#include <util/rbf.h>
#include <util/translation.h>
#include <util/vector.h>
@@ -24,6 +25,8 @@
#include <univalue.h>
+#include <unordered_set>
+
using common::FeeModeFromString;
using common::FeeModesDetail;
using common::FeeReasons;
@@ -1472,7 +1475,12 @@ RPCMethod sendall()
} else if (options.exists("inputs") && (options.exists("minconf") || options.exists("maxconf"))) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot combine minconf or maxconf with specific inputs.");
} else if (options.exists("inputs")) {
+ std::unordered_set<COutPoint, SaltedOutpointHasher> seen_inputs;
+ seen_inputs.reserve(rawTx.vin.size());
for (const CTxIn& input : rawTx.vin) {
+ if (!seen_inputs.insert(input.prevout).second) {
+ throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid parameter, duplicated input: %s:%d", input.prevout.hash.ToString(), input.prevout.n));
+ }
if (pwallet->IsSpent(input.prevout)) {
throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Input not available. UTXO (%s:%d) was already spent.", input.prevout.hash.ToString(), input.prevout.n));
}
### test/functional/wallet_sendall.py
@@ -258,6 +258,22 @@ def sendall_fails_on_missing_input(self):
foreign_utxo["vout"]), self.wallet.sendall, recipients=[self.remainder_target],
inputs=[foreign_utxo])
+ @cleanup
+ def sendall_fails_on_duplicate_input(self):
+ self.log.info("Test sendall fails if the same UTXO is given twice in inputs")
+ self.add_utxos([17, 4])
+ utxo = self.wallet.listunspent()[0]
+ txcount_before = self.wallet.getwalletinfo()["txcount"]
+ nonmempool_before = self.wallet.getbalances()["mine"]["nonmempool"]
+
+ assert_raises_rpc_error(-8,
+ "Invalid parameter, duplicated input: {}:{}".format(utxo["txid"], utxo["vout"]),
+ self.wallet.sendall, recipients=[self.remainder_target], inputs=[utxo, utxo])
+ assert_equal(self.wallet.getwalletinfo()["txcount"], txcount_before)
+ assert_equal(self.wallet.getbalances()["mine"]["nonmempool"], nonmempool_before)
+ assert_equal(sum(c["txid"] == utxo["txid"] and c["vout"] == utxo["vout"]
+ for c in self.wallet.listunspent()), 1)
+
@cleanup
def sendall_fails_on_no_address(self):
self.log.info("Test sendall fails because no address is provided")
@@ -566,6 +582,9 @@ def run_test(self):
# Fails for the right reasons on missing or previously spent UTXOs
self.sendall_fails_on_missing_input()
+ # Sendall fails when the same input is given twice
+ self.sendall_fails_on_duplicate_input()
+
# Sendall fails when no address is provided
self.sendall_fails_on_no_address()
Why this scored 37/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.