Reintroduce mixing hard cap for all but the largest denom (#3489)

* Split -privatesenddenoms option into two to be able to specify initial/batched denom "checkpoint" and the hard cap

* Introduce MAX_PRIVATESEND_DENOM_OUTPUTS = 500 constant

Also make sure to never exceed it while looping

* Make sure to never exceed nPrivateSendDenomsBatched while creating batches

* Tweak debug logging

* Use auto for nDenomValue

* Fix detection of so called "final" denom

Now that we don't skip denoms anymore conditions are slightly different. Without this fix the "final" denom is created in a separate tx even when there is enough room in the current one.

* batched -> goal

* Tweak json and a warning

* Drop excessive checks for MAX_PRIVATESEND_DENOM_OUTPUTS in the "goal" part of the algo

Rename it to PRIVATESEND_DENOM_OUTPUTS_THRESHOLD to beter match the meaning and tweak comments

* fix log

src/privatesend/privatesend-client.cpp

@@ -1528,22 +1528,17 @@ bool CPrivateSendClientSession::CreateDenominated(CAmount nBalanceToDenominate,
         mapDenomCount.insert(std::pair<CAmount, int>(nDenomValue, GetWallets()[0]->CountInputsWithAmount(nDenomValue)));
     }
 
-    // NOTE: We do not allow txes larger than 100kB, so we have to limit the number of outputs here.
-    // We still want to create a lot of outputs though.
-    // Knowing that each CTxOut is ~35b big, 400 outputs should take 400 x ~35b = ~17.5kb.
-    // More than 500 outputs starts to make qt quite laggy.
-    // Additionally to need all 500 outputs (assuming a max per denom of 50) you'd need to be trying to
-    // create denominations for over 3000 dash!
-
     // Will generate outputs for the createdenoms up to privatesendmaxdenoms per denom
 
     // This works in the way creating PS denoms has traditionally worked, assuming enough funds,
     // it will start with the smallest denom then create 11 of those, then go up to the next biggest denom create 11
     // and repeat. Previously, once the largest denom was reached, as many would be created were created as possible and
     // then any remaining was put into a change address and denominations were created in the same manner a block later.
-    // Now, in this system, so long as we don't reach 500 outputs the process repeats in the same transaction,
+    // Now, in this system, so long as we don't reach PRIVATESEND_DENOM_OUTPUTS_THRESHOLD outputs the process repeats in
-    // creating up to privatesenddenoms per denomination in a single transaction.
+    // the same transaction, creating up to nPrivateSendDenomsHardCap per denomination in a single transaction.
-    while (nValueLeft >= CPrivateSend::GetSmallestDenomination() && nOutputsTotal <= 500) {
+
+    while (nValueLeft >= CPrivateSend::GetSmallestDenomination() && nOutputsTotal < PRIVATESEND_DENOM_OUTPUTS_THRESHOLD) {
+
         for (auto it = vecStandardDenoms.rbegin(); it != vecStandardDenoms.rend(); ++it) {
             CAmount nDenomValue = *it;
             auto currentDenomIt = mapDenomCount.find(nDenomValue);
@@ -1556,13 +1551,18 @@ bool CPrivateSendClientSession::CreateDenominated(CAmount nBalanceToDenominate,
                                && nValueLeft >= nDenomValue
                                && nBalanceToDenominate > 0
                                && nBalanceToDenominate < nDenomValue);
+                if (fFinal) {
                     fAddFinal = false; // add final denom only once, only the smalest possible one
+                    LogPrint(BCLog::PRIVATESEND,
+                             "CPrivateSendClientSession::CreateDenominated -- 1 - FINAL - nDenomValue: %f, nValueLeft: %f, nBalanceToDenominate: %f\n",
+                             (float) nDenomValue / COIN, (float) nValueLeft / COIN, (float) nBalanceToDenominate / COIN);
+                }
 
                 return fRegular || fFinal;
             };
 
-            // add each output up to 11 times or until it can't be added again or until we reach nPrivateSendDenoms
+            // add each output up to 11 times or until it can't be added again or until we reach nPrivateSendDenomsGoal
-            while (needMoreOutputs() && nOutputs <= 10 && currentDenomIt->second <= privateSendClient.nPrivateSendDenoms) {
+            while (needMoreOutputs() && nOutputs <= 10 && currentDenomIt->second < privateSendClient.nPrivateSendDenomsGoal) {
                 CScript scriptDenom = keyHolderStorageDenom.AddKey(GetWallets()[0]);
 
                 vecSend.push_back((CRecipient) {scriptDenom, nDenomValue, false});
@@ -1573,8 +1573,8 @@ bool CPrivateSendClientSession::CreateDenominated(CAmount nBalanceToDenominate,
                 nValueLeft -= nDenomValue;
                 nBalanceToDenominate -= nDenomValue;
                 LogPrint(BCLog::PRIVATESEND,
-                         "CPrivateSendClientSession::CreateDenominated -- 1 - totalOutputs: %d, nOutputsTotal: %d, nOutputs: %d, nValueLeft: %f\n",
+                         "CPrivateSendClientSession::CreateDenominated -- 1 - nDenomValue: %f, totalOutputs: %d, nOutputsTotal: %d, nOutputs: %d, nValueLeft: %f, nBalanceToDenominate: %f\n",
-                         nOutputsTotal + nOutputs, nOutputsTotal, nOutputs, (float) nValueLeft / COIN);
+                         (float) nDenomValue / COIN, nOutputsTotal + nOutputs, nOutputsTotal, nOutputs, (float) nValueLeft / COIN, (float) nBalanceToDenominate / COIN);
             }
 
             nOutputsTotal += nOutputs;
@@ -1582,24 +1582,32 @@ bool CPrivateSendClientSession::CreateDenominated(CAmount nBalanceToDenominate,
         }
 
         bool finished = true;
-        for (auto it : mapDenomCount) {
+        for (const auto it : mapDenomCount) {
-            // Check if this specific denom could use another loop, check that there aren't nPrivateSendDenoms of this
+            // Check if this specific denom could use another loop, check that there aren't nPrivateSendDenomsGoal of this
             // denom and that our nValueLeft/nBalanceToDenominate is enough to create one of these denoms, if so, loop again.
-            if (it.second <= privateSendClient.nPrivateSendDenoms && nValueLeft >= it.first && nBalanceToDenominate >= it.first) {
+            if (it.second < privateSendClient.nPrivateSendDenomsGoal && nValueLeft >= it.first && nBalanceToDenominate > 0) {
                 finished = false;
+                LogPrint(BCLog::PRIVATESEND,
+                        "CPrivateSendClientSession::CreateDenominated -- 1 - NOT finished - nDenomValue: %f, count: %d, nValueLeft: %f, nBalanceToDenominate: %f\n",
+                        (float) it.first / COIN, it.second, (float) nValueLeft / COIN, (float) nBalanceToDenominate / COIN);
                 break;
             }
+            LogPrint(BCLog::PRIVATESEND,
+                    "CPrivateSendClientSession::CreateDenominated -- 1 - FINSHED - nDenomValue: %f, count: %d, nValueLeft: %f, nBalanceToDenominate: %f\n",
+                    (float) it.first / COIN, it.second, (float) nValueLeft / COIN, (float) nBalanceToDenominate / COIN);
         }
 
         if (finished) break;
     }
 
-    // Now that nPrivateSendDenoms worth of each denom have been created, do something with the remainder.
+    // Now that nPrivateSendDenomsGoal worth of each denom have been created or the max number of denoms given the value of the input, do something with the remainder.
-    while (nValueLeft >= CPrivateSend::GetSmallestDenomination() && nBalanceToDenominate >= CPrivateSend::GetSmallestDenomination()
+    if (nValueLeft >= CPrivateSend::GetSmallestDenomination() && nBalanceToDenominate >= CPrivateSend::GetSmallestDenomination()
-           && nOutputsTotal <= 500) {
+           && nOutputsTotal < PRIVATESEND_DENOM_OUTPUTS_THRESHOLD) {
+
+        CAmount nLargestDenomValue = vecStandardDenoms.front();
 
         // Go big to small
-        for (long nDenomValue : vecStandardDenoms) {
+        for (auto nDenomValue : vecStandardDenoms) {
             int nOutputs = 0;
 
             // Number of denoms we can create given our denom and the amount of funds we have left
@@ -1607,26 +1615,37 @@ bool CPrivateSendClientSession::CreateDenominated(CAmount nBalanceToDenominate,
             int denomsToCreateBal = nBalanceToDenominate / nDenomValue;
             // Use the smaller value
             int denomsToCreate = denomsToCreateValue > denomsToCreateBal ? denomsToCreateBal : denomsToCreateValue;
+            auto it = mapDenomCount.find(nDenomValue);
             for (int i = 0; i < denomsToCreate; i++) {
-                CScript scriptDenom = keyHolderStorageDenom.AddKey(GetWallets()[0]);
+                // Never go above the cap unless it's the largest denom
+                if (nDenomValue != nLargestDenomValue && it->second >= privateSendClient.nPrivateSendDenomsHardCap) break;
 
+                CScript scriptDenom = keyHolderStorageDenom.AddKey(GetWallets()[0]);
                 vecSend.push_back((CRecipient) {scriptDenom, nDenomValue, false});
 
                 // increment outputs and subtract denomination amount
                 nOutputs++;
-                mapDenomCount.find(nDenomValue)->second++;
+                it->second++;
                 nValueLeft -= nDenomValue;
                 nBalanceToDenominate -= nDenomValue;
                 LogPrint(BCLog::PRIVATESEND,
-                         "CPrivateSendClientSession::CreateDenominated -- 1 - totalOutputs: %d, nOutputsTotal: %d, nOutputs: %d, nValueLeft: %f\n",
+                         "CPrivateSendClientSession::CreateDenominated -- 2 - nDenomValue: %f, totalOutputs: %d, nOutputsTotal: %d, nOutputs: %d, nValueLeft: %f, nBalanceToDenominate: %f\n",
-                         nOutputsTotal + nOutputs, nOutputsTotal, nOutputs, (float) nValueLeft / COIN);
+                         (float) nDenomValue / COIN, nOutputsTotal + nOutputs, nOutputsTotal, nOutputs, (float) nValueLeft / COIN, (float) nBalanceToDenominate / COIN);
-
+                if (nOutputs + nOutputsTotal >= PRIVATESEND_DENOM_OUTPUTS_THRESHOLD) break;
             }
             nOutputsTotal += nOutputs;
+            if (nOutputsTotal >= PRIVATESEND_DENOM_OUTPUTS_THRESHOLD) break;
         }
     }
 
-    LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::CreateDenominated -- 2 - nOutputsTotal: %d, nValueLeft: %f\n", nOutputsTotal, (float)nValueLeft / COIN);
+    LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::CreateDenominated -- 3 - nOutputsTotal: %d, nValueLeft: %f, nBalanceToDenominate: %f\n",
+            nOutputsTotal, (float)nValueLeft / COIN, (float)nBalanceToDenominate / COIN);
+
+    for (const auto it : mapDenomCount) {
+        LogPrint(BCLog::PRIVATESEND,
+                "CPrivateSendClientSession::CreateDenominated -- 3 - DONE - nDenomValue: %f, count: %d\n",
+                (float) it.first / COIN, it.second);
+    }
 
     // No reasons to create mixing collaterals if we can't create denoms to mix
     if (nOutputsTotal == 0) return false;
@@ -1742,7 +1761,8 @@ void CPrivateSendClientManager::GetJsonInfo(UniValue& obj) const
     obj.push_back(Pair("max_sessions",  nPrivateSendSessions));
     obj.push_back(Pair("max_rounds",    nPrivateSendRounds));
     obj.push_back(Pair("max_amount",    nPrivateSendAmount));
-    obj.push_back(Pair("max_denoms",    nPrivateSendDenoms));
+    obj.push_back(Pair("denoms_goal",   nPrivateSendDenomsGoal));
+    obj.push_back(Pair("denoms_hardcap", nPrivateSendDenomsHardCap));
     obj.push_back(Pair("queue_size",    GetQueueSize()));
 
     UniValue arrSessions(UniValue::VARR);

src/privatesend/privatesend-client.h

@@ -19,19 +19,32 @@ class UniValue;
 static const int MIN_PRIVATESEND_SESSIONS = 1;
 static const int MIN_PRIVATESEND_ROUNDS = 2;
 static const int MIN_PRIVATESEND_AMOUNT = 2;
-static const int MIN_PRIVATESEND_DENOMS = 10;
+static const int MIN_PRIVATESEND_DENOMS_GOAL = 10;
+static const int MIN_PRIVATESEND_DENOMS_HARDCAP = 10;
 static const int MAX_PRIVATESEND_SESSIONS = 10;
 static const int MAX_PRIVATESEND_ROUNDS = 16;
-static const int MAX_PRIVATESEND_DENOMS = 100000;
+static const int MAX_PRIVATESEND_DENOMS_GOAL = 100000;
+static const int MAX_PRIVATESEND_DENOMS_HARDCAP = 100000;
 static const int MAX_PRIVATESEND_AMOUNT = MAX_MONEY / COIN;
 static const int DEFAULT_PRIVATESEND_SESSIONS = 4;
 static const int DEFAULT_PRIVATESEND_ROUNDS = 4;
 static const int DEFAULT_PRIVATESEND_AMOUNT = 1000;
-static const int DEFAULT_PRIVATESEND_DENOMS = 50;
+static const int DEFAULT_PRIVATESEND_DENOMS_GOAL = 50;
+static const int DEFAULT_PRIVATESEND_DENOMS_HARDCAP = 300;
 
 static const bool DEFAULT_PRIVATESEND_AUTOSTART = false;
 static const bool DEFAULT_PRIVATESEND_MULTISESSION = false;
 
+// How many new denom outputs to create before we consider the "goal" loop in CreateDenominated
+// a final one and start creating an actual tx. Same limit applies for the "hard cap" part of the algo.
+// NOTE: We do not allow txes larger than 100kB, so we have to limit the number of outputs here.
+// We still want to create a lot of outputs though.
+// Knowing that each CTxOut is ~35b big, 400 outputs should take 400 x ~35b = ~17.5kb.
+// More than 500 outputs starts to make qt quite laggy.
+// Additionally to need all 500 outputs (assuming a max per denom of 50) you'd need to be trying to
+// create denominations for over 3000 dash!
+static const int PRIVATESEND_DENOM_OUTPUTS_THRESHOLD = 500;
+
 // Warn user if mixing in gui or try to create backup if mixing in daemon mode
 // when we have only this many keys left
 static const int PRIVATESEND_KEYS_THRESHOLD_WARNING = 100;
@@ -192,7 +205,8 @@ public:
     int nPrivateSendSessions;
     int nPrivateSendRounds;
     int nPrivateSendAmount;
-    int nPrivateSendDenoms;
+    int nPrivateSendDenomsGoal;
+    int nPrivateSendDenomsHardCap;
     bool fEnablePrivateSend;
     bool fPrivateSendRunning;
     bool fPrivateSendMultiSession;
@@ -209,7 +223,8 @@ public:
         nCachedBlockHeight(0),
         nPrivateSendRounds(DEFAULT_PRIVATESEND_ROUNDS),
         nPrivateSendAmount(DEFAULT_PRIVATESEND_AMOUNT),
-        nPrivateSendDenoms(DEFAULT_PRIVATESEND_DENOMS),
+        nPrivateSendDenomsGoal(DEFAULT_PRIVATESEND_DENOMS_GOAL),
+        nPrivateSendDenomsHardCap(DEFAULT_PRIVATESEND_DENOMS_HARDCAP),
         fEnablePrivateSend(false),
         fPrivateSendRunning(false),
         fPrivateSendMultiSession(DEFAULT_PRIVATESEND_MULTISESSION),

src/wallet/init.cpp

@@ -63,7 +63,8 @@ std::string WalletInit::GetHelpString(bool showDebug)
     strUsage += HelpMessageOpt("-privatesendsessions=<n>", strprintf(_("Use N separate masternodes in parallel to mix funds (%u-%u, default: %u)"), MIN_PRIVATESEND_SESSIONS, MAX_PRIVATESEND_SESSIONS, DEFAULT_PRIVATESEND_SESSIONS));
     strUsage += HelpMessageOpt("-privatesendrounds=<n>", strprintf(_("Use N separate masternodes for each denominated input to mix funds (%u-%u, default: %u)"), MIN_PRIVATESEND_ROUNDS, MAX_PRIVATESEND_ROUNDS, DEFAULT_PRIVATESEND_ROUNDS));
     strUsage += HelpMessageOpt("-privatesendamount=<n>", strprintf(_("Target PrivateSend balance (%u-%u, default: %u)"), MIN_PRIVATESEND_AMOUNT, MAX_PRIVATESEND_AMOUNT, DEFAULT_PRIVATESEND_AMOUNT));
-    strUsage += HelpMessageOpt("-privatesenddenoms=<n>", strprintf(_("Create up to N inputs of each denominated amount (%u-%u, default: %u)"), MIN_PRIVATESEND_DENOMS, MAX_PRIVATESEND_DENOMS, DEFAULT_PRIVATESEND_DENOMS));
+    strUsage += HelpMessageOpt("-privatesenddenomsgoal=<n>", strprintf(_("Try to create at least N inputs of each denominated amount (%u-%u, default: %u)"), MIN_PRIVATESEND_DENOMS_GOAL, MAX_PRIVATESEND_DENOMS_GOAL, DEFAULT_PRIVATESEND_DENOMS_GOAL));
+    strUsage += HelpMessageOpt("-privatesenddenomshardcap=<n>", strprintf(_("Create up to N inputs of each denominated amount (%u-%u, default: %u)"), MIN_PRIVATESEND_DENOMS_HARDCAP, MAX_PRIVATESEND_DENOMS_HARDCAP, DEFAULT_PRIVATESEND_DENOMS_HARDCAP));
 
     if (showDebug)
     {
@@ -220,6 +221,19 @@ bool WalletInit::ParameterInteraction()
         LogPrintf("%s: parameter interaction: can't use -hdseed and -mnemonic/-mnemonicpassphrase together, will prefer -seed\n", __func__);
     }
 
+    if (gArgs.IsArgSet("-privatesenddenoms")) {
+        int nDenomsDeprecated = gArgs.GetArg("-privatesenddenoms", DEFAULT_PRIVATESEND_DENOMS_HARDCAP);
+        InitWarning("Warning: -privatesenddenoms is deprecated, please use -privatesenddenomshardcap or -privatesenddenomsgoal.\n");
+        if (gArgs.SoftSetArg("-privatesenddenomshardcap", itostr(nDenomsDeprecated))) {
+            LogPrintf("%s: parameter interaction: -privatesenddenoms=%d -> setting -privatesenddenomshardcap=%d\n", __func__, nDenomsDeprecated, nDenomsDeprecated);
+        }
+        gArgs.ForceRemoveArg("-privatesenddenoms");
+    }
+
+    if (gArgs.GetArg("-privatesenddenomshardcap", DEFAULT_PRIVATESEND_DENOMS_HARDCAP) < gArgs.GetArg("-privatesenddenomsgoal", DEFAULT_PRIVATESEND_DENOMS_GOAL)) {
+        return InitError("-privatesenddenomshardcap can't be lower than -privatesenddenomsgoal");
+    }
+
     return true;
 }
 
@@ -361,14 +375,16 @@ void WalletInit::InitPrivateSendSettings()
     privateSendClient.nPrivateSendSessions = std::min(std::max((int)gArgs.GetArg("-privatesendsessions", DEFAULT_PRIVATESEND_SESSIONS), MIN_PRIVATESEND_SESSIONS), MAX_PRIVATESEND_SESSIONS);
     privateSendClient.nPrivateSendRounds = std::min(std::max((int)gArgs.GetArg("-privatesendrounds", DEFAULT_PRIVATESEND_ROUNDS), MIN_PRIVATESEND_ROUNDS), MAX_PRIVATESEND_ROUNDS);
     privateSendClient.nPrivateSendAmount = std::min(std::max((int)gArgs.GetArg("-privatesendamount", DEFAULT_PRIVATESEND_AMOUNT), MIN_PRIVATESEND_AMOUNT), MAX_PRIVATESEND_AMOUNT);
-    privateSendClient.nPrivateSendDenoms = std::min(std::max((int)gArgs.GetArg("-privatesenddenoms", DEFAULT_PRIVATESEND_DENOMS), MIN_PRIVATESEND_DENOMS), MAX_PRIVATESEND_DENOMS);
+    privateSendClient.nPrivateSendDenomsGoal = std::min(std::max((int)gArgs.GetArg("-privatesenddenomsgoal", DEFAULT_PRIVATESEND_DENOMS_GOAL), MIN_PRIVATESEND_DENOMS_GOAL), MAX_PRIVATESEND_DENOMS_GOAL);
+    privateSendClient.nPrivateSendDenomsHardCap = std::min(std::max((int)gArgs.GetArg("-privatesenddenomshardcap", DEFAULT_PRIVATESEND_DENOMS_HARDCAP), MIN_PRIVATESEND_DENOMS_HARDCAP), MAX_PRIVATESEND_DENOMS_HARDCAP);
 
     if (privateSendClient.fEnablePrivateSend) {
         LogPrintf("PrivateSend: autostart=%d, multisession=%d, "
-                  "sessions=%d, rounds=%d, amount=%d, denoms=%d\n",
+                  "sessions=%d, rounds=%d, amount=%d, denoms_goal=%d, denoms_hardcap=%d\n",
                   privateSendClient.fPrivateSendRunning, privateSendClient.fPrivateSendMultiSession,
                   privateSendClient.nPrivateSendSessions, privateSendClient.nPrivateSendRounds,
-                  privateSendClient.nPrivateSendAmount, privateSendClient.nPrivateSendDenoms);
+                  privateSendClient.nPrivateSendAmount,
+                  privateSendClient.nPrivateSendDenomsGoal, privateSendClient.nPrivateSendDenomsHardCap);
     }
 
 }