[wallet] Add regression test for vValue sort order #7293

pull MarcoFalke wants to merge 2 commits into bitcoin:master from MarcoFalke:Mf1601-wallet-vValue changing 1 files +19 −6

MarcoFalke commented at 11:44 PM on January 4, 2016: member

vValue is sorted from high to low. This adds a regression test such that pulls like #7183 would fail travis.
[wallet] Add regression test for vValue sort order fa3c7e644f

MarcoFalke commented at 11:45 PM on January 4, 2016: member

Note: #7183 was changed in the meantime, the original diff might come in handy to test this pull:

diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index d23d54e..3d1f58c 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -1605,7 +1605,7 @@ static void ApproximateBestSubset(vector<pair<CAmount, pair<const CWalletTx*,uns
         bool fReachedTarget = false;
         for (int nPass = 0; nPass < 2 && !fReachedTarget; nPass++)
         {
-            for (unsigned int i = 0; i < vValue.size(); i++)
+            for (unsigned int i = 0; i < vValue.size() && !fReachedTarget; i++)
             {
                 //The solver here uses a randomized algorithm,
                 //the randomness serves no real security purpose but is just
@@ -1625,8 +1625,6 @@ static void ApproximateBestSubset(vector<pair<CAmount, pair<const CWalletTx*,uns
                             nBest = nTotal;
                             vfBest = vfIncluded;
                         }
-                        nTotal -= vValue[i].first;
-                        vfIncluded[i] = false;
                     }
                 }
             }

MarcoFalke cross-referenced this on Jan 4, 2016 from issue Improved readability of ApproximateBestSubset by murchandamus
jonasschnelli added the label Tests on Jan 5, 2016
laanwj commented at 11:01 AM on January 5, 2016: member

utACK
MarcoFalke force-pushed on Jan 5, 2016
murchandamus commented at 3:27 PM on January 5, 2016: contributor

ACK: Fails for the referenced original PR #7183.
[trivial] Merge test cases and replace CENT with COIN faf538bfdb
laanwj merged this on Jan 7, 2016
laanwj closed this on Jan 7, 2016
laanwj referenced this in commit 5541560938 on Jan 7, 2016
laanwj referenced this in commit ff9b610026 on Jan 7, 2016
laanwj commented at 8:26 AM on January 7, 2016: member

Cherry-picked to 0.12 as ff9b610
MarcoFalke deleted the branch on Jan 7, 2016
bitcoin locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#7183 Improved readability of ApproximateBestSubset