Merge #13805: [wallet] Correctly limit output group size

a13647b8bd [qa] Add test for too-large wallet output groups (Suhas Daftuar)
57ec1c97b2 [wallet] correctly limit output group size (Suhas Daftuar)

Pull request description:

  Also add a test to ensure that output groups are being limited, even if a wallet has many outputs corresponding to the same scriptPubKey (the test fails without the first commit).

Tree-SHA512: 2aaa82005b0910488f5cbf40690d4c5e2f46949e299ef70b4cb6e440713811443d411dcbc6d71b1701fd82423073125e21747787d70830cd021c841afb732d51

2 files changed, 30 insertions, 1 deletions

diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index bb048949ca..218684fdf1 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -4424,7 +4424,10 @@ std::vector<OutputGroup> CWallet::GroupOutputs(const std::vector<COutput>& outpu
             size_t ancestors, descendants;
             mempool.GetTransactionAncestry(output.tx->GetHash(), ancestors, descendants);
             if (!single_coin && ExtractDestination(output.tx->tx->vout[output.i].scriptPubKey, dst)) {
-                if (gmap.count(dst) == 10) {
+                // Limit output groups to no more than 10 entries, to protect
+                // against inadvertently creating a too-large transaction
+                // when using -avoidpartialspends
+                if (gmap[dst].m_outputs.size() >= 10) {
                     groups.push_back(gmap[dst]);
                     gmap.erase(dst);
                 }
diff --git a/test/functional/wallet_groups.py b/test/functional/wallet_groups.py
index 0d27815da0..408e9dfef0 100755
--- a/test/functional/wallet_groups.py
+++ b/test/functional/wallet_groups.py
@@ -5,6 +5,8 @@
 """Test wallet group functionality."""
 
 from test_framework.test_framework import BitcoinTestFramework
+from test_framework.mininode import FromHex, ToHex
+from test_framework.messages import CTransaction
 from test_framework.util import (
     assert_equal,
 )
@@ -63,5 +65,29 @@ class WalletGroupTest(BitcoinTestFramework):
         assert_approx(v[0], 0.2)
         assert_approx(v[1], 1.3, 0.0001)
 
+        # Empty out node2's wallet
+        self.nodes[2].sendtoaddress(address=self.nodes[0].getnewaddress(), amount=self.nodes[2].getbalance(), subtractfeefromamount=True)
+        self.sync_all()
+        self.nodes[0].generate(1)
+
+        # Fill node2's wallet with 10000 outputs corresponding to the same
+        # scriptPubKey
+        for i in range(5):
+            raw_tx = self.nodes[0].createrawtransaction([{"txid":"0"*64, "vout":0}], [{addr2[0]: 0.05}])
+            tx = FromHex(CTransaction(), raw_tx)
+            tx.vin = []
+            tx.vout = [tx.vout[0]] * 2000
+            funded_tx = self.nodes[0].fundrawtransaction(ToHex(tx))
+            signed_tx = self.nodes[0].signrawtransactionwithwallet(funded_tx['hex'])
+            self.nodes[0].sendrawtransaction(signed_tx['hex'])
+            self.nodes[0].generate(1)
+
+        self.sync_all()
+
+        # Check that we can create a transaction that only requires ~100 of our
+        # utxos, without pulling in all outputs and creating a transaction that
+        # is way too big.
+        assert self.nodes[2].sendtoaddress(address=addr2[0], amount=5)
+
 if __name__ == '__main__':
     WalletGroupTest().main ()