wallet: reject duplicate inputs in sendall #36344

Bruce039 commented at 9:57 AM on September 26, 2026: contributor

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.

DrahtBot added the label Wallet on Sep 26, 2026

DrahtBot commented at 9:57 AM on September 26, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

External sites

Code coverage & benchmarks: https://corecheck.dev/bitcoin/bitcoin/pulls/36344
Ralph review <img src=https://review.fish.foo/assets/bitcoin/bitcoin/36344.png alt="Ralph review status"> https://git.fish.foo/bitcoin/bitcoin/pulls/36344#author-ralph

Reviews

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

Type	Reviewers
ACK	l0rinc, maflcko, molnard
Stale ACK	achow101

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#35302 <sub><img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35302.svg"></sub> (Silent Payments: Sending (take 2) by Eunovo)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

LLM Linter (✨ experimental)

Possible places where comparison-specific test macros should replace generic comparisons:

test/functional/wallet_sendall.py: assert_equal(sum(c["txid"] == utxo["txid"] and c["vout"] == utxo["vout"] for c in self.wallet.listunspent()), 1) → Prefer assert_equal(...) instead of the bare assert.

Bruce039 force-pushed on Sep 26, 2026

DrahtBot added the label CI failed on Sep 26, 2026

DrahtBot removed the label CI failed on Sep 26, 2026

achow101 commented at 11:21 PM on September 29, 2026: member

ACK c46a0b34fd06c874191cf880ba7082790b96c194

in test/functional/wallet_sendall.py:257 in c46a0b34fd

 252 | +        utxo = self.wallet.listunspent()[0]
 253 | +
 254 | +        assert_raises_rpc_error(-8,
 255 | +                "Invalid parameter, duplicated input: {}:{}".format(utxo["txid"], utxo["vout"]),
 256 | +                self.wallet.sendall, recipients=[self.remainder_target], inputs=[utxo, utxo])
 257 | +        assert_equal(self.wallet.getbalances()["mine"]["untrusted_pending"], 0)

l0rinc commented at 6:06 AM on October 18, 2026:

The previous line correctly fails without the fix, but this assertion would pass before and after because the invalid transaction never enters the mempool - we should assert the affected wallet state instead.

See my suggested alternative in https://github.com/l0rinc/bitcoin/pull/308/commits which demonstrates the full state transition by first characterizing the old behavior and then updating the expectations with the fix.

Bruce039 commented at 10:50 AM on October 27, 2026:

Thanks, you're right that the pending-balance check passed either way. 1beefce checks the wallet state instead: txcount and the nonmempool balance stay the same and the outpoint is still listed in listunspent. Without the fix the first two change (txcount +1, nonmempool -17) and the outpoint disappears.

l0rinc changes_requested

l0rinc commented at 6:08 AM on October 18, 2026: contributor

Concept ACK

The RPC-error assertion catches the missing rejection, but we should cover the reported wallet-state correction also - see inline suggestion.

Bruce039 force-pushed on Oct 27, 2026

DrahtBot added the label CI failed on Oct 27, 2026

DrahtBot removed the label CI failed on Oct 27, 2026

molnard commented at 2:08 PM on October 30, 2026: none

tACK 1beefce3519348662d833734cadf01a23d60d83e

Ran wallet_sendall.py on win. It passes with the fix and fails with No exception raised when the duplicate input check is removed.

DrahtBot requested review from l0rinc on Oct 30, 2026

DrahtBot requested review from achow101 on Oct 30, 2026

in src/wallet/rpc/spend.cpp:1472 in 1beefce351

1468 | @@ -1472,7 +1469,11 @@ RPCMethod sendall()
1469 |              } else if (options.exists("inputs") && (options.exists("minconf") || options.exists("maxconf"))) {
1470 |                  throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot combine minconf or maxconf with specific inputs.");
1471 |              } else if (options.exists("inputs")) {
1472 | +                std::set<COutPoint> seen_inputs;

l0rinc commented at 7:23 PM on October 30, 2026:

Nit: I’d prefer std::unordered_set<COutPoint, SaltedOutpointHasher> with reserve(rawTx.vin.size()) here. We don’t need ordering, and for the expected happy path this gives expected linear work, avoids rehashing, and would probably be faster and potentially use less memory for large sweeps. A tree could be cheaper when an early duplicate lets it reject before allocating for the whole list, but I’d optimize for valid requests rather than that uncommon error case. We already use this unordered set for input checks in IsConsistentPackage.

Bruce039 commented at 10:40 AM on November 3, 2026:

Done in 654a92a, switched to std::unordered_set<COutPoint, SaltedOutpointHasher> with reserve(rawTx.vin.size()). Thanks!

l0rinc approved

l0rinc commented at 7:26 PM on October 30, 2026: contributor

I like 1 beef, ACK 1beefce3519348662d833734cadf01a23d60d83e

Left a nit, also fine with it as-is.

<details> <summary>Characterization used to verify the state transition</summary>

diff --git a/test/functional/wallet_sendall.py b/test/functional/wallet_sendall.py
--- a/test/functional/wallet_sendall.py
+++ b/test/functional/wallet_sendall.py
@@ -266,12 +266,13 @@
       txcount_before = self.wallet.getwalletinfo()["txcount"]
       nonmempool_before = self.wallet.getbalances()["mine"]["nonmempool"]

-        duplicate_txid = self.wallet.sendall(recipients=[self.remainder_target], inputs=[utxo, utxo])["txid"]
-        assert_equal(self.wallet.getwalletinfo()["txcount"], txcount_before + 1)  # TODO: Duplicate inputs must not add a wallet transaction
-        assert_equal(self.wallet.getbalances()["mine"]["nonmempool"], nonmempool_before - utxo["amount"])  # TODO: Duplicate inputs must not record a spend
+        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()), 0)  # TODO: Duplicate inputs must leave the outpoint available
-        self.wallet.abandontransaction(duplicate_txid)
+                         for c in self.wallet.listunspent()), 1) [@cleanup](/bitcoin-bitcoin/contributor/cleanup/)
   def sendall_fails_on_no_address(self):

</details>

wallet: reject duplicate inputs in sendall

sendall with the inputs option adds up the value of each listed input
without checking for duplicates. When the same UTXO is given twice, the
input value is counted twice, and the wallet builds, signs and commits
a transaction that spends one outpoint twice and pays out twice its
value. The RPC reports success, but the transaction is consensus-invalid
(bad-txns-inputs-duplicate), and the coin disappears from listunspent
until the transaction is abandoned.

Reject duplicated inputs with an error, like createrawtransaction does
for duplicated addresses.

654a92a9d3

Bruce039 force-pushed on Nov 3, 2026

l0rinc commented at 10:54 AM on November 3, 2026: contributor

ACK 654a92a9d3e76f3ff4ec4975f4fbcfba1a6091be

The previous hash was funnier, but here the implementation is better - hard to choose...

DrahtBot requested review from molnard on Nov 3, 2026

maflcko commented at 11:31 AM on November 3, 2026: member

review ACK 654a92a9d3e76f3ff4ec4975f4fbcfba1a6091be 🚪

<details><summary>Show signature</summary>

Signature:

untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 654a92a9d3e76f3ff4ec4975f4fbcfba1a6091be 🚪
foLNWW0oHoY7qDcggAvkXHd0AYXCsMo0seyY66FIRx7wpKfKP9rTjDboQpY2nqW292gI6ZYURLimIbd2GgWKBQ==

</details>

molnard commented at 4:50 PM on November 3, 2026: none

tACK 654a92a9d3e76f3ff4ec4975f4fbcfba1a6091be

Ran wallet_sendall.py on win.

fanquake merged this on Nov 3, 2026

fanquake closed this on Nov 3, 2026