From 595a0e17c41254515014bdd183d14986e95f37a4 Mon Sep 17 00:00:00 2001
From: xdustinface <xdustinfacex@gmail.com>
Date: Fri, 7 Aug 2020 04:11:58 +0200
Subject: [PATCH 01/45] wallet: Add m_dust_feerate to CCoinControl / Use it in
 CreateTransaction

---
 src/wallet/coincontrol.h | 2 ++
 src/wallet/wallet.cpp    | 5 ++---
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/wallet/coincontrol.h b/src/wallet/coincontrol.h
index 4dc3daafec81..2778eba014f1 100644
--- a/src/wallet/coincontrol.h
+++ b/src/wallet/coincontrol.h
@@ -39,6 +39,8 @@ class CCoinControl
     bool fOverrideFeeRate;
     //! Override the default payTxFee if set
     boost::optional<CFeeRate> m_feerate;
+    //! Override the dust feerate estimation with m_dust_feerate in CreateTransaction if set
+    boost::optional<CFeeRate> m_dust_feerate;
     //! Override the default confirmation target if set
     boost::optional<unsigned int> m_confirm_target;
     //! Fee estimation mode to control arguments to estimateSmartFee
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 357c148186e7..7ffc0b468e34 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -3734,6 +3734,7 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT
     assert(txNew.nLockTime <= (unsigned int)chainActive.Height());
     assert(txNew.nLockTime < LOCKTIME_THRESHOLD);
     FeeCalculation feeCalc;
+    CFeeRate discard_rate = coin_control.m_dust_feerate ? *coin_control.m_dust_feerate : GetDiscardRate(::feeEstimator);
     CAmount nFeeNeeded;
     unsigned int nBytes;
     {
@@ -3776,7 +3777,6 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT
             CTxOut change_prototype_txout(0, scriptChange);
             size_t change_prototype_size = GetSerializeSize(change_prototype_txout, SER_DISK, 0);
 
-            CFeeRate discard_rate = GetDiscardRate(::feeEstimator);
             nFeeRet = 0;
             bool pick_new_inputs = true;
             CAmount nValueIn = 0;
@@ -3809,8 +3809,7 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT
                         }
                     }
 
-                    if (IsDust(txout, ::dustRelayFee))
-                    {
+                    if (IsDust(txout, discard_rate)) {
                         if (recipient.fSubtractFeeFromAmount && nFeeRet > 0)
                         {
                             if (txout.nValue < 0)

From 75196c6b6d7b45a80c5d13a77daf133aa22def86 Mon Sep 17 00:00:00 2001
From: xdustinface <xdustinfacex@gmail.com>
Date: Thu, 6 Aug 2020 04:54:22 +0200
Subject: [PATCH 02/45] privatesend: Introduce
 CTransactionBuilder/CTransactionBuilderOutput

Builder classes for transactions from type

Inputs: Defined by CompactTallyItem
Outputs: Simple outputs with lose reserve keys

This takes fully takes care of fee calculations and makes sure calculations are the same like those happening when actually create the transaction with CreateTransaction.
---
 src/privatesend/privatesend-util.cpp | 226 +++++++++++++++++++++++++++
 src/privatesend/privatesend-util.h   |  96 ++++++++++++
 2 files changed, 322 insertions(+)

diff --git a/src/privatesend/privatesend-util.cpp b/src/privatesend/privatesend-util.cpp
index 161fd7b4a490..1acadbe6725d 100644
--- a/src/privatesend/privatesend-util.cpp
+++ b/src/privatesend/privatesend-util.cpp
@@ -2,7 +2,13 @@
 // Distributed under the MIT/X11 software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
+#include <consensus/validation.h>
+#include <policy/fees.h>
+#include <policy/policy.h>
 #include <privatesend/privatesend-util.h>
+#include <script/sign.h>
+#include <validation.h>
+#include <wallet/fees.h>
 
 CKeyHolder::CKeyHolder(CWallet* pwallet) :
     reserveKey(pwallet)
@@ -70,3 +76,223 @@ void CKeyHolderStorage::ReturnAll()
         LogPrintf("CKeyHolderStorage::%s -- %lld keys returned\n", __func__, tmp.size());
     }
 }
+
+CTransactionBuilderOutput::CTransactionBuilderOutput(CTransactionBuilder* pTxBuilderIn, CWallet* pwalletIn, CAmount nAmountIn) :
+    pTxBuilder(pTxBuilderIn),
+    key(pwalletIn),
+    nAmount(nAmountIn)
+{
+    assert(pTxBuilder);
+    CPubKey pubKey;
+    key.GetReservedKey(pubKey, false);
+    script = ::GetScriptForDestination(pubKey.GetID());
+}
+
+bool CTransactionBuilderOutput::UpdateAmount(const CAmount nNewAmount)
+{
+    LOCK(pTxBuilder->cs_outputs);
+
+    const CAmount nAmountDiff = nNewAmount - nAmount;
+
+    if (nNewAmount > 0 && nAmountDiff <= pTxBuilder->GetAmountLeft()) {
+        if (nAmountDiff != 0) {
+            nAmount = nNewAmount;
+        }
+        return true;
+    }
+    return false;
+}
+
+CTransactionBuilder::CTransactionBuilder(CWallet* pwalletIn, const CompactTallyItem& tallyItemIn) :
+    pwallet(pwalletIn),
+    dummyReserveKey(pwalletIn),
+    tallyItem(tallyItemIn)
+{
+    // Generate d feerate which will be used to consider if the remainder is dust and will go into fees or not
+    coinControl.m_dust_feerate = ::GetDiscardRate(::feeEstimator);
+    // Generate a feerate which will be used by calculations of this class and also by CWallet::CreateTransaction
+    coinControl.m_feerate = ::feeEstimator.estimateSmartFee(::nTxConfirmTarget, nullptr, true);
+    // Fee will always go back to origin
+    coinControl.destChange = tallyItemIn.txdest;
+    // Only allow tallyItems inputs for tx creation
+    coinControl.fAllowOtherInputs = false;
+    // Select all tallyItem outputs in the coinControl so that CreateTransaction knows what to use
+    for (const auto& outpoint : tallyItem.vecOutPoints) {
+        coinControl.Select(outpoint);
+    }
+    // Create dummy tx to calculate the exact required fees upfront for accurate amount and fee calculations
+    CMutableTransaction dummyTx;
+    // Get a comparable dummy scriptPubKey
+    CTransactionBuilderOutput dummyOutput(this, pwallet, 0);
+    CScript dummyScript = dummyOutput.GetScript();
+    dummyOutput.ReturnKey();
+    // And create dummy signatures for all inputs
+    SignatureData dummySignature;
+    ProduceSignature(DummySignatureCreator(pwallet), dummyScript, dummySignature);
+    for (auto out : tallyItem.vecOutPoints) {
+        dummyTx.vin.push_back(CTxIn(out, dummySignature.scriptSig));
+    }
+    // Calculate required bytes for the dummy tx with tallyItem's inputs only
+    nBytesBase = ::GetSerializeSize(dummyTx, SER_NETWORK, PROTOCOL_VERSION);
+    // Add one output and use the size difference to calculate the output size
+    dummyTx.vout.push_back(CTxOut(0, dummyScript));
+    nBytesOutput = ::GetSerializeSize(dummyTx, SER_NETWORK, PROTOCOL_VERSION) - nBytesBase;
+    // Just to make sure..
+    Clear();
+}
+
+CTransactionBuilder::~CTransactionBuilder()
+{
+    Clear();
+}
+
+void CTransactionBuilder::Clear()
+{
+    std::vector<std::unique_ptr<CTransactionBuilderOutput>> vecOutputsTmp;
+    {
+        // Don't hold cs_outputs while clearing the outputs which might indirectly call lock cs_wallet
+        LOCK(cs_outputs);
+        std::swap(vecOutputs, vecOutputsTmp);
+        vecOutputs.clear();
+    }
+
+    for (auto& key : vecOutputsTmp) {
+        if (fKeepKeys) {
+            key->KeepKey();
+        } else {
+            key->ReturnKey();
+        }
+    }
+    // Always return this key just to make sure..
+    dummyReserveKey.ReturnKey();
+}
+
+bool CTransactionBuilder::TryAddOutput(CAmount nAmount) const
+{
+    return GetAmountLeft(tallyItem.nAmount, GetAmountUsed() + nAmount, GetFee(GetBytesTotal() + nBytesOutput, coinControl.m_feerate.get())) >= 0;
+}
+
+bool CTransactionBuilder::TryAddOutputs(const std::vector<CAmount>& vecAmounts) const
+{
+    CAmount nAmountSum{0};
+    for (auto nAmount : vecAmounts) {
+        nAmountSum += nAmount;
+    }
+    return GetAmountLeft(tallyItem.nAmount, GetAmountUsed() + nAmountSum, GetFee(GetBytesTotal() + nBytesOutput * vecAmounts.size(), coinControl.m_feerate.get())) >= 0;
+}
+
+CTransactionBuilderOutput* CTransactionBuilder::AddOutput(CAmount nAmount)
+{
+    LOCK(cs_outputs);
+    if (TryAddOutput(nAmount)) {
+        vecOutputs.push_back(std::make_unique<CTransactionBuilderOutput>(this, pwallet, nAmount));
+        return vecOutputs.back().get();
+    }
+    return nullptr;
+}
+
+CAmount CTransactionBuilder::GetAmountLeft(const CAmount nAmount, const CAmount nAmountUsed, const CAmount nFee)
+{
+    return nAmount - nAmountUsed - nFee;
+}
+
+CAmount CTransactionBuilder::GetAmountUsed() const
+{
+    CAmount nAmountUsed{0};
+    for (const auto& out : vecOutputs) {
+        nAmountUsed += out->GetAmount();
+    }
+    return nAmountUsed;
+}
+
+CAmount CTransactionBuilder::GetFee(int nBytes, const CFeeRate& feeRate)
+{
+    CAmount nFeeCalc = feeRate.GetFee(nBytes);
+    CAmount nRequiredFee = GetRequiredFee(nBytes);
+    if (nRequiredFee > nFeeCalc) {
+        nFeeCalc = nRequiredFee;
+    }
+    if (nFeeCalc > ::maxTxFee) {
+        nFeeCalc = ::maxTxFee;
+    }
+    return nFeeCalc;
+}
+
+CAmount CTransactionBuilder::GetFee() const
+{
+    return GetFee(GetBytesTotal(), coinControl.m_feerate.get());
+}
+
+bool CTransactionBuilder::IsDust(CAmount nAmount) const
+{
+    return ::IsDust(CTxOut(nAmount, ::GetScriptForDestination(tallyItem.txdest)), coinControl.m_dust_feerate.get());
+}
+
+bool CTransactionBuilder::Commit(std::string& strResult)
+{
+    CWalletTx wtx;
+    CAmount nFeeRet = 0;
+    int nChangePosRet = -1;
+
+    // Transfor the outputs to the format CWallet::CreateTransaction requires
+    std::vector<CRecipient> vecSend;
+    {
+        LOCK(cs_outputs);
+        vecSend.reserve(vecOutputs.size());
+        for (const auto& out : vecOutputs) {
+            vecSend.push_back((CRecipient){out->GetScript(), out->GetAmount(), false});
+        }
+    }
+
+    if (!pwallet->CreateTransaction(vecSend, wtx, dummyReserveKey, nFeeRet, nChangePosRet, strResult, coinControl)) {
+        return false;
+    }
+
+    CAmount nAmountLeft = GetAmountLeft();
+    CAmount nFeeAdditional = nAmountLeft && IsDust(nAmountLeft) ? nAmountLeft : 0;
+    int nBytesAdditional = !IsDust(nAmountLeft) ? nBytesOutput : 0;
+    CAmount nFeeCalc = GetFee(GetBytesTotal() + nBytesAdditional, coinControl.m_feerate.get()) + nFeeAdditional;
+
+    // If there is a either remainder which is considered to be dust (will be added to fee in this case) or no amount left there should be no change output, return if there is a change output.
+    if (nChangePosRet != -1 && IsDust(nAmountLeft)) {
+        strResult = strprintf("Unexpected change output %s at position %d", wtx.tx->vout[nChangePosRet].ToString(), nChangePosRet);
+        return false;
+    }
+
+    // If there is a remainder which is not considered to be dust it should end up in a change output, return if not.
+    if (!IsDust(nAmountLeft) && nChangePosRet == -1) {
+        strResult = strprintf("Change output missing: %d", GetAmountLeft());
+        return false;
+    }
+
+    // If the calculated fee does not match the fee returned by CreateTransaction aka if this check fails something is messed!
+    if (nFeeRet != nFeeCalc) {
+        strResult = strprintf("Fee validation failed -> nFeeRet: %d, nFeeCalc: %d, nFeeAdditional: %d, nBytesAdditional: %d, %s", nFeeRet, nFeeCalc, nFeeAdditional, nBytesAdditional, ToString());
+        return false;
+    }
+
+    CValidationState state;
+    if (!pwallet->CommitTransaction(wtx, dummyReserveKey, g_connman.get(), state)) {
+        strResult = state.GetRejectReason();
+        return false;
+    }
+
+    fKeepKeys = true;
+
+    strResult = wtx.GetHash().ToString();
+
+    return true;
+}
+
+std::string CTransactionBuilder::ToString()
+{
+    return strprintf("CTransactionBuilder(Amount left: %d, Bytes base: %d, Bytes output %d, Bytes total: %d, Amount used: %d, Outputs: %d, Fee rate: %d, Fee: %d)",
+        GetAmountLeft(),
+        nBytesBase,
+        nBytesOutput,
+        GetBytesTotal(),
+        GetAmountUsed(),
+        CountOutputs(),
+        coinControl.m_feerate->GetFeePerK(),
+        GetFee());
+}
diff --git a/src/privatesend/privatesend-util.h b/src/privatesend/privatesend-util.h
index 0fffba21fd23..145d1ea21df8 100644
--- a/src/privatesend/privatesend-util.h
+++ b/src/privatesend/privatesend-util.h
@@ -7,6 +7,8 @@
 
 #include <wallet/wallet.h>
 
+class CTransactionBuilder;
+
 class CKeyHolder
 {
 private:
@@ -34,4 +36,98 @@ class CKeyHolderStorage
     void KeepAll();
     void ReturnAll();
 };
+
+class CTransactionBuilderOutput
+{
+    // Used for amount updates
+    CTransactionBuilder* pTxBuilder{nullptr};
+    // Reserve key where the amount of this output will end up
+    CReserveKey key;
+    // Amount this output will receive
+    CAmount nAmount{0};
+    // ScriptPubKey of this output
+    CScript script;
+
+public:
+    CTransactionBuilderOutput(CTransactionBuilder* pTxBuilderIn, CWallet* pwalletIn, CAmount nAmountIn);
+    CTransactionBuilderOutput(CTransactionBuilderOutput&&) = delete;
+    CTransactionBuilderOutput& operator=(CTransactionBuilderOutput&&) = delete;
+    // Get the scriptPubKey of this output
+    CScript GetScript() { return script; }
+    // Get the amount of this output
+    CAmount GetAmount() { return nAmount; }
+    // Try update the amount of this output. Returns true if it was successful and false if not (e.g. insufficient amount left).
+    bool UpdateAmount(const CAmount nAmount);
+    // Tell the wallet to remove the key used by this output from the keypool
+    void KeepKey() { key.KeepKey(); }
+    // Tell the wallet to return the key used by this output to the keypool
+    void ReturnKey() { key.ReturnKey(); }
+};
+
+class CTransactionBuilder
+{
+    // Wallet the transaction will be build for
+    CWallet* pwallet{nullptr};
+    // See CTransactionBuilder() for initialization
+    CCoinControl coinControl;
+    // Dummy since we anyway use tallyItem's destination as change destination in coincontrol.
+    // Its a member just to make sure ReturnKey can be called in destructor just in case it gets generated/kept
+    // somewhere in CWallet code.
+    CReserveKey dummyReserveKey;
+    // Fee rate used to evaluate if an amount should be considered as dust or not
+    CFeeRate dustFeeRate;
+    // Contains all utxos available to generate this transactions. They are all from the same address.
+    CompactTallyItem tallyItem;
+    // Contains the number of bytes required for a transaction with only the inputs of tallyItems, no outputs
+    int nBytesBase{0};
+    // Contains the number of bytes required to add one output
+    int nBytesOutput{0};
+    // Call KeepKey for all keys in desctructor if fKeepKeys is true, call ReturnKey for all key if its false.
+    bool fKeepKeys{false};
+    // Protect vecOutputs
+    mutable CCriticalSection cs_outputs;
+    // Contains all outputs already added to the transaction
+    std::vector<std::unique_ptr<CTransactionBuilderOutput>> vecOutputs;
+    // Needed by CTransactionBuilderOutput::UpdateAmount to lock cs_outputs
+    friend class CTransactionBuilderOutput;
+
+public:
+    CTransactionBuilder(CWallet* pwalletIn, const CompactTallyItem& tallyItemIn);
+    ~CTransactionBuilder();
+    // Clear the output vector and keep/return the included keys depending on the value of fKeepKeys
+    void Clear();
+    // Try to add a single output with the amount nAmount. Returns true if its possble and false if not.
+    bool TryAddOutput(CAmount nAmount) const;
+    // Try to add multiple outputs as vector of amounts. Returns true if its possible to add all of them and false if not.
+    bool TryAddOutputs(const std::vector<CAmount>& vecAmounts) const;
+    // Add an output with the amount nAmount. Returns a pointer to the output if it could be added and nullptr if not due to insufficient amomunt left.
+    CTransactionBuilderOutput* AddOutput(CAmount nAmount = 0);
+    // Get a reference to the coinControl
+    const CCoinControl& GetCoinControl() const { return coinControl; }
+    // Get amount we had available when we started
+    CAmount GetAmountInitial() const { return tallyItem.nAmount; }
+    // Helper to calculate static remainders for output trying
+    static CAmount GetAmountLeft(const CAmount nAmount, const CAmount nAmountUsed, const CAmount nFee);
+    // Get the amount currently left to add more outputs. Does respect fees.
+    CAmount GetAmountLeft() const { return GetAmountInitial() - GetAmountUsed() - GetFee(GetBytesTotal(), coinControl.m_feerate.get()); }
+    // Get the amount currently used by added outputs. Does not include fees.
+    CAmount GetAmountUsed() const;
+    // Helper to calculate static fees for output tryingd
+    static CAmount GetFee(int nBytes, const CFeeRate& feeRate);
+    // Get fees based on the number of total used byes and the feerate set in CoinControl
+    CAmount GetFee() const;
+    // Get the total number of bytes used already by this transaction
+    int GetBytesTotal() const { return nBytesBase + vecOutputs.size() * nBytesOutput; }
+    // Get the number of bytes added by a single output
+    int GetBytesOutput() const { return nBytesOutput; }
+    // Check if an amounts should be considered as dust
+    bool IsDust(CAmount nAmount) const;
+    // Get the total number of added outputs
+    int CountOutputs() const { return vecOutputs.size(); }
+    // Create and Commit the transaction to the wallet
+    bool Commit(std::string& strResult);
+    // Convert to a string
+    std::string ToString();
+};
+
 #endif // BITCOIN_PRIVATESEND_PRIVATESEND_UTIL_H

From 6a0dfa3b14480c8d5382cc25ac37be93db4f736d Mon Sep 17 00:00:00 2001
From: xdustinface <xdustinfacex@gmail.com>
Date: Wed, 5 Aug 2020 03:24:30 +0200
Subject: [PATCH 03/45] privatesend: Improve amount/fee calculation in
 CreateDenominated

---
 src/privatesend/privatesend-client.cpp | 180 ++++++++++---------------
 1 file changed, 74 insertions(+), 106 deletions(-)

diff --git a/src/privatesend/privatesend-client.cpp b/src/privatesend/privatesend-client.cpp
index efc8a073d4dc..6b068431ea66 100644
--- a/src/privatesend/privatesend-client.cpp
+++ b/src/privatesend/privatesend-client.cpp
@@ -1557,32 +1557,24 @@ bool CPrivateSendClientSession::CreateDenominated(CAmount nBalanceToDenominate,
 
     if (!CPrivateSendClientOptions::IsEnabled() || !mixingWallet) return false;
 
-    std::vector<CRecipient> vecSend;
-    CKeyHolderStorage keyHolderStorageDenom;
-
-    CCoinControl coinControl;
-    // Every input will require at least this much fees in duffs
-    const CAmount nInputFee = GetMinimumFee(148, coinControl, ::mempool, ::feeEstimator, nullptr /* feeCalc */);
-    // Every output will require at least this much fees in duffs
-    const CAmount nOutputFee = GetMinimumFee(34, coinControl, ::mempool, ::feeEstimator, nullptr /* feeCalc */);
+    // denominated input is always a single one, so we can check its amount directly and return early
+    if (tallyItem.vecOutPoints.size() == 1 && CPrivateSend::IsDenominatedAmount(tallyItem.nAmount)) {
+        return false;
+    }
 
-    CAmount nValueLeft = tallyItem.nAmount;
-    // Leave some room for fees, assuming we are going to spend all the outpoints
-    nValueLeft -= tallyItem.vecOutPoints.size() * nInputFee;
+    CTransactionBuilder txBuilder(mixingWallet, tallyItem);
 
-    LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::CreateDenominated -- 0 - %s nValueLeft: %f\n", EncodeDestination(tallyItem.txdest), (float)nValueLeft / COIN);
+    LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::%s -- Start %s\n", __func__, txBuilder.ToString());
 
     // ****** Add an output for mixing collaterals ************ /
 
-    if (fCreateMixingCollaterals) {
-        CScript scriptCollateral = keyHolderStorageDenom.AddKey(mixingWallet);
-        vecSend.push_back((CRecipient){scriptCollateral, CPrivateSend::GetMaxCollateralAmount(), false});
-        nValueLeft -= CPrivateSend::GetMaxCollateralAmount() + nOutputFee;
+    if (fCreateMixingCollaterals && !txBuilder.AddOutput(CPrivateSend::GetMaxCollateralAmount())) {
+        LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::%s -- Failed to add collateral output\n", __func__);
+        return false;
     }
 
     // ****** Add outputs for denoms ************ /
 
-    int nOutputsTotal = 0;
     bool fAddFinal = true;
     std::vector<CAmount> vecStandardDenoms = CPrivateSend::GetStandardDenominations();
 
@@ -1600,8 +1592,7 @@ bool CPrivateSendClientSession::CreateDenominated(CAmount nBalanceToDenominate,
     // Now, in this system, so long as we don't reach PRIVATESEND_DENOM_OUTPUTS_THRESHOLD outputs the process repeats in
     // the same transaction, creating up to nPrivateSendDenomsHardCap per denomination in a single transaction.
 
-    while (nValueLeft >= CPrivateSend::GetSmallestDenomination() && nOutputsTotal < PRIVATESEND_DENOM_OUTPUTS_THRESHOLD) {
-
+    while (txBuilder.TryAddOutput(CPrivateSend::GetSmallestDenomination()) && txBuilder.CountOutputs() < PRIVATESEND_DENOM_OUTPUTS_THRESHOLD) {
         for (auto it = vecStandardDenoms.rbegin(); it != vecStandardDenoms.rend(); ++it) {
             CAmount nDenomValue = *it;
             auto currentDenomIt = mapDenomCount.find(nDenomValue);
@@ -1609,72 +1600,76 @@ bool CPrivateSendClientSession::CreateDenominated(CAmount nBalanceToDenominate,
             int nOutputs = 0;
 
             auto needMoreOutputs = [&]() {
-                bool fRegular = ((nValueLeft >= nDenomValue + nOutputFee) && nBalanceToDenominate >= nDenomValue);
-                bool fFinal = (fAddFinal
-                               && nValueLeft >= nDenomValue + nOutputFee
-                               && nBalanceToDenominate > 0
-                               && nBalanceToDenominate < nDenomValue);
-                if (fFinal) {
-                    fAddFinal = false; // add final denom only once, only the smalest possible one
-                    LogPrint(BCLog::PRIVATESEND, /* Continued */
-                             "CPrivateSendClientSession::CreateDenominated -- 1 - FINAL - nDenomValue: %f, nValueLeft: %f, nBalanceToDenominate: %f\n",
-                             (float) nDenomValue / COIN, (float) nValueLeft / COIN, (float) nBalanceToDenominate / COIN);
+                if (txBuilder.TryAddOutput(nDenomValue)) {
+                    if (fAddFinal && nBalanceToDenominate > 0 && nBalanceToDenominate < nDenomValue) {
+                        fAddFinal = false; // add final denom only once, only the smalest possible one
+                        LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::%s -- 1 - FINAL - nDenomValue: %f, nBalanceToDenominate: %f, nOutputs: %d, %s\n",
+                                                     __func__, (float) nDenomValue / COIN, (float) nBalanceToDenominate / COIN, nOutputs, txBuilder.ToString());
+                        return true;
+                    } else if (nBalanceToDenominate >= nDenomValue) {
+                        return true;
+                    }
                 }
-
-                return fRegular || fFinal;
+                return false;
             };
 
             // 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 < CPrivateSendClientOptions::GetDenomsGoal()) {
-                CScript scriptDenom = keyHolderStorageDenom.AddKey(mixingWallet);
-
-                vecSend.push_back((CRecipient) {scriptDenom, nDenomValue, false});
-
-                // increment outputs and subtract denomination amount
-                nOutputs++;
-                currentDenomIt->second++;
-                nValueLeft -= nDenomValue + nOutputFee;
-                nBalanceToDenominate -= nDenomValue;
-                LogPrint(BCLog::PRIVATESEND, /* Continued */
-                         "CPrivateSendClientSession::CreateDenominated -- 1 - nDenomValue: %f, totalOutputs: %d, nOutputsTotal: %d, nOutputs: %d, nValueLeft: %f, nBalanceToDenominate: %f\n",
-                         (float) nDenomValue / COIN, nOutputsTotal + nOutputs, nOutputsTotal, nOutputs, (float) nValueLeft / COIN, (float) nBalanceToDenominate / COIN);
+                // Add output and subtract denomination amount
+                if (txBuilder.AddOutput(nDenomValue)) {
+                    ++nOutputs;
+                    ++currentDenomIt->second;
+                    nBalanceToDenominate -= nDenomValue;
+                    LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::%s -- 1 - nDenomValue: %f, nBalanceToDenominate: %f, nOutputs: %d, %s\n",
+                                                 __func__, (float) nDenomValue / COIN, (float) nBalanceToDenominate / COIN, nOutputs, txBuilder.ToString());
+                } else {
+                    LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::%s -- 1 - Error: AddOutput failed for nDenomValue: %f, nBalanceToDenominate: %f, nOutputs: %d, %s\n",
+                                                 __func__, (float) nDenomValue / COIN, (float) nBalanceToDenominate / COIN, nOutputs, txBuilder.ToString());
+                    return false;
+                }
+
             }
 
-            nOutputsTotal += nOutputs;
-            if (nValueLeft == 0 || nBalanceToDenominate <= 0) break;
+            if (txBuilder.GetAmountLeft() == 0 || nBalanceToDenominate <= 0) break;
         }
 
         bool finished = true;
         for (const auto it : mapDenomCount) {
             // 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 < CPrivateSendClientOptions::GetDenomsGoal() && (nValueLeft >= it.first + nOutputFee) && nBalanceToDenominate > 0) {
+            if (it.second < CPrivateSendClientOptions::GetDenomsGoal() && txBuilder.TryAddOutput(it.first) && nBalanceToDenominate >= it.first) {
                 finished = false;
-                LogPrint(BCLog::PRIVATESEND, /* Continued */
-                        "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);
+                LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::%s -- 1 - NOT finished - nDenomValue: %f, count: %d, nBalanceToDenominate: %f, %s\n",
+                                             __func__, (float) it.first / COIN, it.second, (float) nBalanceToDenominate / COIN, txBuilder.ToString());
                 break;
             }
-            LogPrint(BCLog::PRIVATESEND, /* Continued */
-                    "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);
+            LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::%s -- 1 - FINSHED - nDenomValue: %f, count: %d, nBalanceToDenominate: %f, %s\n",
+                                         __func__, (float) it.first / COIN, it.second, (float) nBalanceToDenominate / COIN, txBuilder.ToString());
         }
 
         if (finished) break;
     }
 
     // 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.
-    if ((nValueLeft >= CPrivateSend::GetSmallestDenomination() + nOutputFee) && nBalanceToDenominate >= CPrivateSend::GetSmallestDenomination()
-           && nOutputsTotal < PRIVATESEND_DENOM_OUTPUTS_THRESHOLD) {
-
+    if (txBuilder.TryAddOutput(CPrivateSend::GetSmallestDenomination()) && nBalanceToDenominate >= CPrivateSend::GetSmallestDenomination() && txBuilder.CountOutputs() < PRIVATESEND_DENOM_OUTPUTS_THRESHOLD) {
         CAmount nLargestDenomValue = vecStandardDenoms.front();
 
+        LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::%s -- 2 - Process remainder: %s\n", __func__, txBuilder.ToString());
+
+        auto countPossibleOutputs = [&](CAmount nAmount) -> int {
+            std::vector<CAmount> vecOutputs;
+            while (txBuilder.TryAddOutputs(vecOutputs)) {
+                vecOutputs.push_back(nAmount);
+            }
+            return vecOutputs.size();
+        };
+
         // Go big to small
         for (auto nDenomValue : vecStandardDenoms) {
             int nOutputs = 0;
 
             // Number of denoms we can create given our denom and the amount of funds we have left
-            int denomsToCreateValue = nValueLeft / (nDenomValue + nOutputFee);
+            int denomsToCreateValue = countPossibleOutputs(nDenomValue);
             // Prefer overshooting the targed balance by larger denoms (hence `+1`) instead of a more
             // accurate approximation by many smaller denoms. This is ok because when we get here we
             // should have nPrivateSendDenomsGoal of each smaller denom already. Also, without `+1`
@@ -1685,78 +1680,51 @@ bool CPrivateSendClientSession::CreateDenominated(CAmount nBalanceToDenominate,
             int denomsToCreateBal = (nBalanceToDenominate / nDenomValue) + 1;
             // Use the smaller value
             int denomsToCreate = denomsToCreateValue > denomsToCreateBal ? denomsToCreateBal : denomsToCreateValue;
+            LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::%s -- 2 - nBalanceToDenominate: %f, nDenomValue: %f, denomsToCreateValue: %d, denomsToCreateBal: %d\n",
+                                         __func__, (float) nBalanceToDenominate / COIN, (float) nDenomValue / COIN, denomsToCreateValue, denomsToCreateBal);
             auto it = mapDenomCount.find(nDenomValue);
             for (int i = 0; i < denomsToCreate; i++) {
                 // Never go above the cap unless it's the largest denom
                 if (nDenomValue != nLargestDenomValue && it->second >= CPrivateSendClientOptions::GetDenomsHardCap()) break;
 
-                CScript scriptDenom = keyHolderStorageDenom.AddKey(mixingWallet);
-                vecSend.push_back((CRecipient) {scriptDenom, nDenomValue, false});
-
-                // increment outputs and subtract denomination amount
-                nOutputs++;
-                it->second++;
-                nValueLeft -= nDenomValue + nOutputFee;
-                nBalanceToDenominate -= nDenomValue;
-                LogPrint(BCLog::PRIVATESEND, /* Continued */
-                         "CPrivateSendClientSession::CreateDenominated -- 2 - nDenomValue: %f, totalOutputs: %d, nOutputsTotal: %d, nOutputs: %d, nValueLeft: %f, nBalanceToDenominate: %f\n",
-                         (float) nDenomValue / COIN, nOutputsTotal + nOutputs, nOutputsTotal, nOutputs, (float) nValueLeft / COIN, (float) nBalanceToDenominate / COIN);
-                if (nOutputs + nOutputsTotal >= PRIVATESEND_DENOM_OUTPUTS_THRESHOLD) break;
+                // Increment helpers, add output and subtract denomination amount
+                if (txBuilder.AddOutput(nDenomValue)) {
+                    nOutputs++;
+                    it->second++;
+                    nBalanceToDenominate -= nDenomValue;
+                } else {
+                    LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::%s -- 2 - Error: AddOutput failed: denomsToCreate: %d/%d, %s\n", __func__, i, denomsToCreate, txBuilder.ToString());
+                    break;
+                }
+                LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::%s -- 2 - nDenomValue: %f, nBalanceToDenominate: %f, nOutputs: %d, %s\n",
+                                             __func__, (float) nDenomValue / COIN, (float) nBalanceToDenominate / COIN, nOutputs, txBuilder.ToString());
+                if (txBuilder.CountOutputs() >= PRIVATESEND_DENOM_OUTPUTS_THRESHOLD) break;
             }
-            nOutputsTotal += nOutputs;
-            if (nOutputsTotal >= PRIVATESEND_DENOM_OUTPUTS_THRESHOLD) break;
+            if (txBuilder.CountOutputs() >= PRIVATESEND_DENOM_OUTPUTS_THRESHOLD) break;
         }
     }
 
-    LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::CreateDenominated -- 3 - nOutputsTotal: %d, nValueLeft: %f, nBalanceToDenominate: %f\n",
-            nOutputsTotal, (float)nValueLeft / COIN, (float)nBalanceToDenominate / COIN);
+    LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::%s -- 3 - nBalanceToDenominate: %f, %s\n", __func__, (float) nBalanceToDenominate / COIN, txBuilder.ToString());
 
     for (const auto it : mapDenomCount) {
-        LogPrint(BCLog::PRIVATESEND, /* Continued */
-                "CPrivateSendClientSession::CreateDenominated -- 3 - DONE - nDenomValue: %f, count: %d\n",
-                (float) it.first / COIN, it.second);
+        LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::%s -- 3 - DONE - nDenomValue: %f, count: %d\n", __func__, (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;
-
-    // if we have anything left over, it will be automatically send back as change - there is no need to send it manually
-
-    coinControl.fAllowOtherInputs = false;
-    coinControl.fAllowWatchOnly = false;
-    coinControl.nCoinType = CoinType::ONLY_NONDENOMINATED;
-    // send change to the same address so that we were able create more denoms out of it later
-    coinControl.destChange = tallyItem.txdest;
-    for (const auto& outpoint : tallyItem.vecOutPoints) {
-        coinControl.Select(outpoint);
-    }
-
-    CWalletTx wtx;
-    CAmount nFeeRet = 0;
-    int nChangePosRet = -1;
-    std::string strFail;
-    // make our change address
-    CReserveKey reservekeyChange(mixingWallet);
-
-    bool fSuccess = mixingWallet->CreateTransaction(vecSend, wtx, reservekeyChange,
-        nFeeRet, nChangePosRet, strFail, coinControl);
-    if (!fSuccess) {
-        LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::CreateDenominated -- Error: %s\n", strFail);
-        keyHolderStorageDenom.ReturnAll();
+    if ((fCreateMixingCollaterals && txBuilder.CountOutputs() == 1) || txBuilder.CountOutputs() == 0) {
         return false;
     }
 
-    keyHolderStorageDenom.KeepAll();
-
-    CValidationState state;
-    if (!mixingWallet->CommitTransaction(wtx, reservekeyChange, &connman, state)) {
-        LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::CreateDenominated -- CommitTransaction failed! Reason given: %s\n", state.GetRejectReason());
+    std::string strResult;
+    if (!txBuilder.Commit(strResult)) {
+        LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::%s -- Commit failed: %s\n", __func__, strResult);
         return false;
     }
 
     // use the same nCachedLastSuccessBlock as for DS mixing to prevent race
     privateSendClientManagers.at(mixingWallet->GetName())->UpdatedSuccessBlock();
-    LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::CreateDenominated -- txid=%s\n", wtx.GetHash().GetHex());
+
+    LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::%s -- txid: %s\n", __func__, strResult);
 
     return true;
 }

From 5705bf70a53f5a0d13c8ec3cbd74289925d14b97 Mon Sep 17 00:00:00 2001
From: xdustinface <xdustinfacex@gmail.com>
Date: Wed, 5 Aug 2020 03:40:22 +0200
Subject: [PATCH 04/45] privatesend: Improve amount/fee calculation in
 MakeCollateralAmounts

---
 src/privatesend/privatesend-client.cpp | 120 ++++++++++++-------------
 1 file changed, 56 insertions(+), 64 deletions(-)

diff --git a/src/privatesend/privatesend-client.cpp b/src/privatesend/privatesend-client.cpp
index 6b068431ea66..f24ec3c48c49 100644
--- a/src/privatesend/privatesend-client.cpp
+++ b/src/privatesend/privatesend-client.cpp
@@ -1424,8 +1424,8 @@ bool CPrivateSendClientSession::MakeCollateralAmounts(const CompactTallyItem& ta
 
     if (!CPrivateSendClientOptions::IsEnabled() || !mixingWallet) return false;
 
-    // Skip way too tiny amounts
-    if (tallyItem.nAmount < CPrivateSend::GetCollateralAmount()) {
+    // Denominated input is always a single one, so we can check its amount directly and return early
+    if (!fTryDenominated && tallyItem.vecOutPoints.size() == 1 && CPrivateSend::IsDenominatedAmount(tallyItem.nAmount)) {
         return false;
     }
 
@@ -1434,83 +1434,75 @@ bool CPrivateSendClientSession::MakeCollateralAmounts(const CompactTallyItem& ta
         return false;
     }
 
-    // denominated input is always a single one, so we can check its amount directly and return early
-    if (!fTryDenominated && tallyItem.vecOutPoints.size() == 1 && CPrivateSend::IsDenominatedAmount(tallyItem.nAmount)) {
+    CTransactionBuilder txBuilder(mixingWallet, tallyItem);
+
+    LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::%s -- Start %s\n", __func__, txBuilder.ToString());
+
+    // Skip way too tiny amounts. Smallest we want is minimum collateral amount in a one output tx
+    if (!txBuilder.TryAddOutput(CPrivateSend::GetCollateralAmount())) {
         return false;
     }
 
-    CWalletTx wtx;
-    CAmount nFeeRet = 0;
-    int nChangePosRet = -1;
-    std::string strFail;
-    std::vector<CRecipient> vecSend;
+    auto getAdjustedAmount = [&](CAmount nAmount) -> CAmount {
+        // If amount is denominated remove one duff, this will to into fees!
+        return CPrivateSend::IsDenominatedAmount(nAmount) ? nAmount - 1 : nAmount;
+    };
 
-    // make our collateral address
-    CReserveKey reservekeyCollateral(mixingWallet);
-    // make our change address
-    CReserveKey reservekeyChange(mixingWallet);
+    int nCase{0}; // Just for debug logs
+    if (txBuilder.TryAddOutputs({CPrivateSend::GetMaxCollateralAmount(), CPrivateSend::GetCollateralAmount()})) {
+        nCase = 1;
+        // <case1>, see TransactionRecord::decomposeTransaction
+        // Out1 == CPrivateSend::GetMaxCollateralAmount()
+        // Out2 >= CPrivateSend::GetCollateralAmount()
 
-    CScript scriptCollateral;
-    CPubKey vchPubKey;
-    assert(reservekeyCollateral.GetReservedKey(vchPubKey, false)); // should never fail, as we just unlocked
-    scriptCollateral = GetScriptForDestination(vchPubKey.GetID());
+        txBuilder.AddOutput(CPrivateSend::GetMaxCollateralAmount());
+        // Note, here we first add a zero amount output to get the remainder after all fees and then assign it
+        CTransactionBuilderOutput* out = txBuilder.AddOutput();
+        out->UpdateAmount(getAdjustedAmount(txBuilder.GetAmountLeft()));
 
-    CAmount nCollateralAmount{0};
-    if (tallyItem.nAmount > CPrivateSend::GetMaxCollateralAmount() + CPrivateSend::GetCollateralAmount()*2) {
-        // Change output will be large enough to be valid as a collateral or a source input for another run
-        nCollateralAmount = CPrivateSend::GetMaxCollateralAmount();
-    } else {
-        // Change output might be too small for another collateral if we try to create the largest collateral
-        // here, create a slightly smaller one instead
-        nCollateralAmount = CPrivateSend::GetMaxCollateralAmount() - CPrivateSend::GetCollateralAmount();
-    }
-    vecSend.push_back((CRecipient){scriptCollateral, nCollateralAmount, false});
-
-    // try to use non-denominated and not mn-like funds first, select them explicitly
-    CCoinControl coinControl;
-    coinControl.fAllowOtherInputs = false;
-    coinControl.fAllowWatchOnly = false;
-    coinControl.nCoinType = CoinType::ONLY_NONDENOMINATED;
-    // send change to the same address so that we were able create more denoms out of it later
-    coinControl.destChange = tallyItem.txdest;
-    for (const auto& outpoint : tallyItem.vecOutPoints) {
-        coinControl.Select(outpoint);
-    }
-
-    bool fSuccess = mixingWallet->CreateTransaction(vecSend, wtx, reservekeyChange,
-        nFeeRet, nChangePosRet, strFail, coinControl);
-    if (!fSuccess) {
-        LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::MakeCollateralAmounts -- ONLY_NONDENOMINATED: %s\n", strFail);
-        // If we failed then most likely there are not enough funds on this address.
-        if (fTryDenominated) {
-            // Try to also use denominated coins (we can't mix denominated without collaterals anyway).
-            coinControl.nCoinType = CoinType::ALL_COINS;
-            if (!mixingWallet->CreateTransaction(vecSend, wtx, reservekeyChange,
-                    nFeeRet, nChangePosRet, strFail, coinControl)) {
-                LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::MakeCollateralAmounts -- ALL_COINS Error: %s\n", strFail);
-                reservekeyCollateral.ReturnKey();
-                return false;
-            }
-        } else {
-            // Nothing else we can do.
-            reservekeyCollateral.ReturnKey();
-            return false;
-        }
+    } else if (txBuilder.TryAddOutputs({CPrivateSend::GetCollateralAmount(), CPrivateSend::GetCollateralAmount()})) {
+        nCase = 2;
+        // <case2>, see TransactionRecord::decomposeTransaction
+        // Out1 CPrivateSend::IsCollateralAmount()
+        // Out2 CPrivateSend::IsCollateralAmount()
+
+        // First add two outputs to get the available value after all fees
+        CTransactionBuilderOutput* out1 = txBuilder.AddOutput();
+        CTransactionBuilderOutput* out2 = txBuilder.AddOutput();
+
+        // Create two equal outputs from the available value. This adds one duff to the fee if txBuilder.GetAmountLeft() is odd.
+        CAmount nAmountOutputs = getAdjustedAmount(txBuilder.GetAmountLeft() / 2);
+
+        assert(CPrivateSend::IsCollateralAmount(nAmountOutputs));
+
+        out1->UpdateAmount(nAmountOutputs);
+        out2->UpdateAmount(nAmountOutputs);
+
+    } else { // still at least possible to add one CPrivateSend::GetCollateralAmount() output
+        nCase = 3;
+        // <case3>, see TransactionRecord::decomposeTransaction
+        // Out1 CPrivateSend::IsCollateralAmount()
+        // Out2 Skipped
+        CTransactionBuilderOutput* out = txBuilder.AddOutput();
+        out->UpdateAmount(getAdjustedAmount(txBuilder.GetAmountLeft()));
+
+        assert(CPrivateSend::IsCollateralAmount(out->GetAmount()));
     }
 
-    reservekeyCollateral.KeepKey();
+    LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::%s -- Done with case %d: %s\n", __func__, nCase, txBuilder.ToString());
 
-    LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::MakeCollateralAmounts -- txid=%s\n", wtx.GetHash().GetHex());
+    assert(txBuilder.IsDust(txBuilder.GetAmountLeft()));
 
-    // use the same nCachedLastSuccessBlock as for DS mixing to prevent race
-    CValidationState state;
-    if (!mixingWallet->CommitTransaction(wtx, reservekeyChange, &connman, state)) {
-        LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::MakeCollateralAmounts -- CommitTransaction failed! Reason given: %s\n", state.GetRejectReason());
+    std::string strResult;
+    if (!txBuilder.Commit(strResult)) {
+        LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::%s -- Commit failed: %s\n", __func__, strResult);
         return false;
     }
 
     privateSendClientManagers.at(mixingWallet->GetName())->UpdatedSuccessBlock();
 
+    LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::%s -- txid: %s\n", __func__, strResult);
+
     return true;
 }
 

From c3afce77e3a9bca4638792fd8b3c1609f7d8325c Mon Sep 17 00:00:00 2001
From: xdustinface <xdustinfacex@gmail.com>
Date: Thu, 6 Aug 2020 04:51:48 +0200
Subject: [PATCH 05/45] qt: Fix decomposeTransaction's MakeCollateralAmounts
 detection

Align it with the three cases in CPrivateSendClientSession::MakeCollateralAmounts
---
 src/qt/transactionrecord.cpp | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/qt/transactionrecord.cpp b/src/qt/transactionrecord.cpp
index afcf45f297f3..635e85c1ce00 100644
--- a/src/qt/transactionrecord.cpp
+++ b/src/qt/transactionrecord.cpp
@@ -150,14 +150,14 @@ QList<TransactionRecord> TransactionRecord::decomposeTransaction(const CWallet *
                     sub.type = TransactionRecord::PrivateSendCollateralPayment;
                 } else {
                     bool fMakeCollateral{false};
-                    if (wtx.tx->vout.size() == 2) {
-                        CAmount nMaxCollateralAmount = CPrivateSend::GetMaxCollateralAmount();
-                        CAmount nPreMaxCollateralAmount = nMaxCollateralAmount  - CPrivateSend::GetCollateralAmount();
-                        fMakeCollateral =
-                            wtx.tx->vout[0].nValue == nMaxCollateralAmount ||
-                            wtx.tx->vout[1].nValue == nMaxCollateralAmount ||
-                            (wtx.tx->vout[0].nValue == nPreMaxCollateralAmount && CPrivateSend::IsCollateralAmount(wtx.tx->vout[1].nValue)) ||
-                            (wtx.tx->vout[1].nValue == nPreMaxCollateralAmount && CPrivateSend::IsCollateralAmount(wtx.tx->vout[0].nValue));
+                    if (wtx.tx->vout.size() == 1) {
+                        fMakeCollateral = !CPrivateSend::IsDenominatedAmount(wtx.tx->vout[0].nValue) && CPrivateSend::IsCollateralAmount(wtx.tx->vout[0].nValue); // <case3>, see CPrivateSendClientSession::MakeCollateralAmounts;
+                    } else if (wtx.tx->vout.size() == 2) {
+                        CAmount nAmount0 = wtx.tx->vout[0].nValue;
+                        CAmount nAmount1 = wtx.tx->vout[1].nValue;
+                        fMakeCollateral = (nAmount0 == CPrivateSend::GetMaxCollateralAmount() && !CPrivateSend::IsDenominatedAmount(nAmount1) && nAmount1 >= CPrivateSend::GetCollateralAmount()) ||
+                                          (nAmount1 == CPrivateSend::GetMaxCollateralAmount() && !CPrivateSend::IsDenominatedAmount(nAmount0) && nAmount0 >= CPrivateSend::GetCollateralAmount()) || // <case1>, see CPrivateSendClientSession::MakeCollateralAmounts
+                                          (nAmount0 == nAmount1 && !CPrivateSend::IsDenominatedAmount(nAmount0) && CPrivateSend::IsCollateralAmount(nAmount0)); // <case2>, see CPrivateSendClientSession::MakeCollateralAmounts
                     }
                     if (fMakeCollateral) {
                         sub.type = TransactionRecord::PrivateSendMakeCollaterals;

From fdf36e635bad625df4b6e07983e136ad5e6b7587 Mon Sep 17 00:00:00 2001
From: UdjinM6 <UdjinM6@users.noreply.github.com>
Date: Sat, 8 Aug 2020 15:28:25 +0300
Subject: [PATCH 06/45] Refactor GetFee

The fee rate is always coinControl.m_feerate, also it's not used outside so should be a private method
---
 src/privatesend/privatesend-util.cpp | 17 ++++++-----------
 src/privatesend/privatesend-util.h   | 10 +++++-----
 2 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/src/privatesend/privatesend-util.cpp b/src/privatesend/privatesend-util.cpp
index 1acadbe6725d..79d679e9b7ee 100644
--- a/src/privatesend/privatesend-util.cpp
+++ b/src/privatesend/privatesend-util.cpp
@@ -169,7 +169,7 @@ void CTransactionBuilder::Clear()
 
 bool CTransactionBuilder::TryAddOutput(CAmount nAmount) const
 {
-    return GetAmountLeft(tallyItem.nAmount, GetAmountUsed() + nAmount, GetFee(GetBytesTotal() + nBytesOutput, coinControl.m_feerate.get())) >= 0;
+    return GetAmountLeft(tallyItem.nAmount, GetAmountUsed() + nAmount, GetFee(GetBytesTotal() + nBytesOutput)) >= 0;
 }
 
 bool CTransactionBuilder::TryAddOutputs(const std::vector<CAmount>& vecAmounts) const
@@ -178,7 +178,7 @@ bool CTransactionBuilder::TryAddOutputs(const std::vector<CAmount>& vecAmounts)
     for (auto nAmount : vecAmounts) {
         nAmountSum += nAmount;
     }
-    return GetAmountLeft(tallyItem.nAmount, GetAmountUsed() + nAmountSum, GetFee(GetBytesTotal() + nBytesOutput * vecAmounts.size(), coinControl.m_feerate.get())) >= 0;
+    return GetAmountLeft(tallyItem.nAmount, GetAmountUsed() + nAmountSum, GetFee(GetBytesTotal() + nBytesOutput * vecAmounts.size())) >= 0;
 }
 
 CTransactionBuilderOutput* CTransactionBuilder::AddOutput(CAmount nAmount)
@@ -205,9 +205,9 @@ CAmount CTransactionBuilder::GetAmountUsed() const
     return nAmountUsed;
 }
 
-CAmount CTransactionBuilder::GetFee(int nBytes, const CFeeRate& feeRate)
+CAmount CTransactionBuilder::GetFee(int nBytes) const
 {
-    CAmount nFeeCalc = feeRate.GetFee(nBytes);
+    CAmount nFeeCalc = coinControl.m_feerate->GetFee(nBytes);
     CAmount nRequiredFee = GetRequiredFee(nBytes);
     if (nRequiredFee > nFeeCalc) {
         nFeeCalc = nRequiredFee;
@@ -218,11 +218,6 @@ CAmount CTransactionBuilder::GetFee(int nBytes, const CFeeRate& feeRate)
     return nFeeCalc;
 }
 
-CAmount CTransactionBuilder::GetFee() const
-{
-    return GetFee(GetBytesTotal(), coinControl.m_feerate.get());
-}
-
 bool CTransactionBuilder::IsDust(CAmount nAmount) const
 {
     return ::IsDust(CTxOut(nAmount, ::GetScriptForDestination(tallyItem.txdest)), coinControl.m_dust_feerate.get());
@@ -251,7 +246,7 @@ bool CTransactionBuilder::Commit(std::string& strResult)
     CAmount nAmountLeft = GetAmountLeft();
     CAmount nFeeAdditional = nAmountLeft && IsDust(nAmountLeft) ? nAmountLeft : 0;
     int nBytesAdditional = !IsDust(nAmountLeft) ? nBytesOutput : 0;
-    CAmount nFeeCalc = GetFee(GetBytesTotal() + nBytesAdditional, coinControl.m_feerate.get()) + nFeeAdditional;
+    CAmount nFeeCalc = GetFee(GetBytesTotal() + nBytesAdditional) + nFeeAdditional;
 
     // If there is a either remainder which is considered to be dust (will be added to fee in this case) or no amount left there should be no change output, return if there is a change output.
     if (nChangePosRet != -1 && IsDust(nAmountLeft)) {
@@ -294,5 +289,5 @@ std::string CTransactionBuilder::ToString()
         GetAmountUsed(),
         CountOutputs(),
         coinControl.m_feerate->GetFeePerK(),
-        GetFee());
+        GetFee(GetBytesTotal()));
 }
diff --git a/src/privatesend/privatesend-util.h b/src/privatesend/privatesend-util.h
index 145d1ea21df8..d9e84a17b51e 100644
--- a/src/privatesend/privatesend-util.h
+++ b/src/privatesend/privatesend-util.h
@@ -109,13 +109,9 @@ class CTransactionBuilder
     // Helper to calculate static remainders for output trying
     static CAmount GetAmountLeft(const CAmount nAmount, const CAmount nAmountUsed, const CAmount nFee);
     // Get the amount currently left to add more outputs. Does respect fees.
-    CAmount GetAmountLeft() const { return GetAmountInitial() - GetAmountUsed() - GetFee(GetBytesTotal(), coinControl.m_feerate.get()); }
+    CAmount GetAmountLeft() const { return GetAmountInitial() - GetAmountUsed() - GetFee(GetBytesTotal()); }
     // Get the amount currently used by added outputs. Does not include fees.
     CAmount GetAmountUsed() const;
-    // Helper to calculate static fees for output tryingd
-    static CAmount GetFee(int nBytes, const CFeeRate& feeRate);
-    // Get fees based on the number of total used byes and the feerate set in CoinControl
-    CAmount GetFee() const;
     // Get the total number of bytes used already by this transaction
     int GetBytesTotal() const { return nBytesBase + vecOutputs.size() * nBytesOutput; }
     // Get the number of bytes added by a single output
@@ -128,6 +124,10 @@ class CTransactionBuilder
     bool Commit(std::string& strResult);
     // Convert to a string
     std::string ToString();
+
+private:
+    // Get fees based on the number of bytes and the feerate set in CoinControl
+    CAmount GetFee(int nBytes) const;
 };
 
 #endif // BITCOIN_PRIVATESEND_PRIVATESEND_UTIL_H

From 580712b9e49c2296f13eb4bcf315042056749284 Mon Sep 17 00:00:00 2001
From: UdjinM6 <UdjinM6@users.noreply.github.com>
Date: Sat, 8 Aug 2020 15:42:46 +0300
Subject: [PATCH 07/45] Simplify nBytesOutput calculations

---
 src/privatesend/privatesend-util.cpp | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/privatesend/privatesend-util.cpp b/src/privatesend/privatesend-util.cpp
index 79d679e9b7ee..80992d4df02f 100644
--- a/src/privatesend/privatesend-util.cpp
+++ b/src/privatesend/privatesend-util.cpp
@@ -134,9 +134,8 @@ CTransactionBuilder::CTransactionBuilder(CWallet* pwalletIn, const CompactTallyI
     }
     // Calculate required bytes for the dummy tx with tallyItem's inputs only
     nBytesBase = ::GetSerializeSize(dummyTx, SER_NETWORK, PROTOCOL_VERSION);
-    // Add one output and use the size difference to calculate the output size
-    dummyTx.vout.push_back(CTxOut(0, dummyScript));
-    nBytesOutput = ::GetSerializeSize(dummyTx, SER_NETWORK, PROTOCOL_VERSION) - nBytesBase;
+    // Calculate the output size
+    nBytesOutput = ::GetSerializeSize(CTxOut(0, dummyScript), SER_NETWORK, PROTOCOL_VERSION);
     // Just to make sure..
     Clear();
 }

From 97645f25968bb705eda4320ba55fe80d6cdb6650 Mon Sep 17 00:00:00 2001
From: UdjinM6 <UdjinM6@users.noreply.github.com>
Date: Sat, 8 Aug 2020 15:43:13 +0300
Subject: [PATCH 08/45] Drop unused GetBytesOutput()

---
 src/privatesend/privatesend-util.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/privatesend/privatesend-util.h b/src/privatesend/privatesend-util.h
index d9e84a17b51e..98bd9a883914 100644
--- a/src/privatesend/privatesend-util.h
+++ b/src/privatesend/privatesend-util.h
@@ -114,8 +114,6 @@ class CTransactionBuilder
     CAmount GetAmountUsed() const;
     // Get the total number of bytes used already by this transaction
     int GetBytesTotal() const { return nBytesBase + vecOutputs.size() * nBytesOutput; }
-    // Get the number of bytes added by a single output
-    int GetBytesOutput() const { return nBytesOutput; }
     // Check if an amounts should be considered as dust
     bool IsDust(CAmount nAmount) const;
     // Get the total number of added outputs

From 03bda06ff5aa083f55af6a6e4100c2db24145771 Mon Sep 17 00:00:00 2001
From: UdjinM6 <UdjinM6@users.noreply.github.com>
Date: Sat, 8 Aug 2020 16:19:27 +0300
Subject: [PATCH 09/45] Make Clear(), GetBytesTotal() and GetAmountUsed()
 private

---
 src/privatesend/privatesend-util.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/privatesend/privatesend-util.h b/src/privatesend/privatesend-util.h
index 98bd9a883914..bd6823b7dc10 100644
--- a/src/privatesend/privatesend-util.h
+++ b/src/privatesend/privatesend-util.h
@@ -94,8 +94,6 @@ class CTransactionBuilder
 public:
     CTransactionBuilder(CWallet* pwalletIn, const CompactTallyItem& tallyItemIn);
     ~CTransactionBuilder();
-    // Clear the output vector and keep/return the included keys depending on the value of fKeepKeys
-    void Clear();
     // Try to add a single output with the amount nAmount. Returns true if its possble and false if not.
     bool TryAddOutput(CAmount nAmount) const;
     // Try to add multiple outputs as vector of amounts. Returns true if its possible to add all of them and false if not.
@@ -110,10 +108,6 @@ class CTransactionBuilder
     static CAmount GetAmountLeft(const CAmount nAmount, const CAmount nAmountUsed, const CAmount nFee);
     // Get the amount currently left to add more outputs. Does respect fees.
     CAmount GetAmountLeft() const { return GetAmountInitial() - GetAmountUsed() - GetFee(GetBytesTotal()); }
-    // Get the amount currently used by added outputs. Does not include fees.
-    CAmount GetAmountUsed() const;
-    // Get the total number of bytes used already by this transaction
-    int GetBytesTotal() const { return nBytesBase + vecOutputs.size() * nBytesOutput; }
     // Check if an amounts should be considered as dust
     bool IsDust(CAmount nAmount) const;
     // Get the total number of added outputs
@@ -124,6 +118,12 @@ class CTransactionBuilder
     std::string ToString();
 
 private:
+    // Clear the output vector and keep/return the included keys depending on the value of fKeepKeys
+    void Clear();
+    // Get the total number of bytes used already by this transaction
+    int GetBytesTotal() const { return nBytesBase + vecOutputs.size() * nBytesOutput; }
+    // Get the amount currently used by added outputs. Does not include fees.
+    CAmount GetAmountUsed() const;
     // Get fees based on the number of bytes and the feerate set in CoinControl
     CAmount GetFee(int nBytes) const;
 };

From f1402b19b3d3257fd739206a62e3e91867ae87d9 Mon Sep 17 00:00:00 2001
From: UdjinM6 <UdjinM6@users.noreply.github.com>
Date: Sat, 8 Aug 2020 17:21:42 +0300
Subject: [PATCH 10/45] Drop unused GetCoinControl()

---
 src/privatesend/privatesend-util.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/privatesend/privatesend-util.h b/src/privatesend/privatesend-util.h
index bd6823b7dc10..4c3ffc5f5268 100644
--- a/src/privatesend/privatesend-util.h
+++ b/src/privatesend/privatesend-util.h
@@ -100,8 +100,6 @@ class CTransactionBuilder
     bool TryAddOutputs(const std::vector<CAmount>& vecAmounts) const;
     // Add an output with the amount nAmount. Returns a pointer to the output if it could be added and nullptr if not due to insufficient amomunt left.
     CTransactionBuilderOutput* AddOutput(CAmount nAmount = 0);
-    // Get a reference to the coinControl
-    const CCoinControl& GetCoinControl() const { return coinControl; }
     // Get amount we had available when we started
     CAmount GetAmountInitial() const { return tallyItem.nAmount; }
     // Helper to calculate static remainders for output trying

From c99a357b17ae56b882a2769bcbeb6b467ebb049d Mon Sep 17 00:00:00 2001
From: UdjinM6 <UdjinM6@users.noreply.github.com>
Date: Sat, 8 Aug 2020 17:22:12 +0300
Subject: [PATCH 11/45] Make ToString() const

---
 src/privatesend/privatesend-util.cpp | 2 +-
 src/privatesend/privatesend-util.h   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/privatesend/privatesend-util.cpp b/src/privatesend/privatesend-util.cpp
index 80992d4df02f..c1f9f70e079f 100644
--- a/src/privatesend/privatesend-util.cpp
+++ b/src/privatesend/privatesend-util.cpp
@@ -278,7 +278,7 @@ bool CTransactionBuilder::Commit(std::string& strResult)
     return true;
 }
 
-std::string CTransactionBuilder::ToString()
+std::string CTransactionBuilder::ToString() const
 {
     return strprintf("CTransactionBuilder(Amount left: %d, Bytes base: %d, Bytes output %d, Bytes total: %d, Amount used: %d, Outputs: %d, Fee rate: %d, Fee: %d)",
         GetAmountLeft(),
diff --git a/src/privatesend/privatesend-util.h b/src/privatesend/privatesend-util.h
index 4c3ffc5f5268..c88e6b516cf1 100644
--- a/src/privatesend/privatesend-util.h
+++ b/src/privatesend/privatesend-util.h
@@ -113,7 +113,7 @@ class CTransactionBuilder
     // Create and Commit the transaction to the wallet
     bool Commit(std::string& strResult);
     // Convert to a string
-    std::string ToString();
+    std::string ToString() const;
 
 private:
     // Clear the output vector and keep/return the included keys depending on the value of fKeepKeys

From 9e4d1019405510e9a7d0477bb1462bc96ca4c29e Mon Sep 17 00:00:00 2001
From: UdjinM6 <UdjinM6@users.noreply.github.com>
Date: Sun, 9 Aug 2020 00:11:54 +0300
Subject: [PATCH 12/45] Refactor `CTransactionBuilder::Commit()`

---
 src/privatesend/privatesend-util.cpp | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/src/privatesend/privatesend-util.cpp b/src/privatesend/privatesend-util.cpp
index c1f9f70e079f..88b858716ca5 100644
--- a/src/privatesend/privatesend-util.cpp
+++ b/src/privatesend/privatesend-util.cpp
@@ -243,23 +243,30 @@ bool CTransactionBuilder::Commit(std::string& strResult)
     }
 
     CAmount nAmountLeft = GetAmountLeft();
-    CAmount nFeeAdditional = nAmountLeft && IsDust(nAmountLeft) ? nAmountLeft : 0;
-    int nBytesAdditional = !IsDust(nAmountLeft) ? nBytesOutput : 0;
-    CAmount nFeeCalc = GetFee(GetBytesTotal() + nBytesAdditional) + nFeeAdditional;
-
+    bool fDust = IsDust(nAmountLeft);
     // If there is a either remainder which is considered to be dust (will be added to fee in this case) or no amount left there should be no change output, return if there is a change output.
-    if (nChangePosRet != -1 && IsDust(nAmountLeft)) {
+    if (nChangePosRet != -1 && fDust) {
         strResult = strprintf("Unexpected change output %s at position %d", wtx.tx->vout[nChangePosRet].ToString(), nChangePosRet);
         return false;
     }
 
     // If there is a remainder which is not considered to be dust it should end up in a change output, return if not.
-    if (!IsDust(nAmountLeft) && nChangePosRet == -1) {
-        strResult = strprintf("Change output missing: %d", GetAmountLeft());
+    if (nChangePosRet == -1 && !fDust) {
+        strResult = strprintf("Change output missing: %d", nAmountLeft);
         return false;
     }
 
+    CAmount nFeeAdditional{0};
+    int nBytesAdditional{0};
+
+    if (fDust) {
+        nFeeAdditional = nAmountLeft;
+    } else {
+        nBytesAdditional = nBytesOutput;
+    }
+
     // If the calculated fee does not match the fee returned by CreateTransaction aka if this check fails something is messed!
+    CAmount nFeeCalc = GetFee(GetBytesTotal() + nBytesAdditional) + nFeeAdditional;
     if (nFeeRet != nFeeCalc) {
         strResult = strprintf("Fee validation failed -> nFeeRet: %d, nFeeCalc: %d, nFeeAdditional: %d, nBytesAdditional: %d, %s", nFeeRet, nFeeCalc, nFeeAdditional, nBytesAdditional, ToString());
         return false;

From 2c05c39fd78585dbebd0cbb73e36a1e5b342760a Mon Sep 17 00:00:00 2001
From: UdjinM6 <UdjinM6@users.noreply.github.com>
Date: Sun, 9 Aug 2020 02:11:09 +0300
Subject: [PATCH 13/45] Reorder cases in decomposeTransaction

---
 src/qt/transactionrecord.cpp | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/src/qt/transactionrecord.cpp b/src/qt/transactionrecord.cpp
index 635e85c1ce00..c7f000a1da19 100644
--- a/src/qt/transactionrecord.cpp
+++ b/src/qt/transactionrecord.cpp
@@ -150,14 +150,17 @@ QList<TransactionRecord> TransactionRecord::decomposeTransaction(const CWallet *
                     sub.type = TransactionRecord::PrivateSendCollateralPayment;
                 } else {
                     bool fMakeCollateral{false};
-                    if (wtx.tx->vout.size() == 1) {
-                        fMakeCollateral = !CPrivateSend::IsDenominatedAmount(wtx.tx->vout[0].nValue) && CPrivateSend::IsCollateralAmount(wtx.tx->vout[0].nValue); // <case3>, see CPrivateSendClientSession::MakeCollateralAmounts;
-                    } else if (wtx.tx->vout.size() == 2) {
+                    if (wtx.tx->vout.size() == 2) {
                         CAmount nAmount0 = wtx.tx->vout[0].nValue;
                         CAmount nAmount1 = wtx.tx->vout[1].nValue;
+                        // <case1>, see CPrivateSendClientSession::MakeCollateralAmounts
                         fMakeCollateral = (nAmount0 == CPrivateSend::GetMaxCollateralAmount() && !CPrivateSend::IsDenominatedAmount(nAmount1) && nAmount1 >= CPrivateSend::GetCollateralAmount()) ||
-                                          (nAmount1 == CPrivateSend::GetMaxCollateralAmount() && !CPrivateSend::IsDenominatedAmount(nAmount0) && nAmount0 >= CPrivateSend::GetCollateralAmount()) || // <case1>, see CPrivateSendClientSession::MakeCollateralAmounts
-                                          (nAmount0 == nAmount1 && !CPrivateSend::IsDenominatedAmount(nAmount0) && CPrivateSend::IsCollateralAmount(nAmount0)); // <case2>, see CPrivateSendClientSession::MakeCollateralAmounts
+                                          (nAmount1 == CPrivateSend::GetMaxCollateralAmount() && !CPrivateSend::IsDenominatedAmount(nAmount0) && nAmount0 >= CPrivateSend::GetCollateralAmount()) ||
+                        // <case2>, see CPrivateSendClientSession::MakeCollateralAmounts
+                                          (nAmount0 == nAmount1 && !CPrivateSend::IsDenominatedAmount(nAmount0) && CPrivateSend::IsCollateralAmount(nAmount0));
+                    } else if (wtx.tx->vout.size() == 1) {
+                        // <case3>, see CPrivateSendClientSession::MakeCollateralAmounts
+                        fMakeCollateral = !CPrivateSend::IsDenominatedAmount(wtx.tx->vout[0].nValue) && CPrivateSend::IsCollateralAmount(wtx.tx->vout[0].nValue);
                     }
                     if (fMakeCollateral) {
                         sub.type = TransactionRecord::PrivateSendMakeCollaterals;

From ff6f4defa939e5d1c3b168d0f3f009473f04407f Mon Sep 17 00:00:00 2001
From: UdjinM6 <UdjinM6@users.noreply.github.com>
Date: Sun, 9 Aug 2020 04:15:25 +0300
Subject: [PATCH 14/45] Fix "finished" conditions in CreateDenominated

---
 src/privatesend/privatesend-client.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/privatesend/privatesend-client.cpp b/src/privatesend/privatesend-client.cpp
index f24ec3c48c49..6951a8f4f975 100644
--- a/src/privatesend/privatesend-client.cpp
+++ b/src/privatesend/privatesend-client.cpp
@@ -1629,7 +1629,7 @@ bool CPrivateSendClientSession::CreateDenominated(CAmount nBalanceToDenominate,
         for (const auto it : mapDenomCount) {
             // 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 < CPrivateSendClientOptions::GetDenomsGoal() && txBuilder.TryAddOutput(it.first) && nBalanceToDenominate >= it.first) {
+            if (it.second < CPrivateSendClientOptions::GetDenomsGoal() && txBuilder.TryAddOutput(it.first) && nBalanceToDenominate > 0) {
                 finished = false;
                 LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::%s -- 1 - NOT finished - nDenomValue: %f, count: %d, nBalanceToDenominate: %f, %s\n",
                                              __func__, (float) it.first / COIN, it.second, (float) nBalanceToDenominate / COIN, txBuilder.ToString());

From 24d282630d9e21931fa2f0dea2f474ef22c1ac51 Mon Sep 17 00:00:00 2001
From: UdjinM6 <UdjinM6@users.noreply.github.com>
Date: Sun, 9 Aug 2020 04:16:14 +0300
Subject: [PATCH 15/45] Fix typo

---
 src/privatesend/privatesend-client.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/privatesend/privatesend-client.cpp b/src/privatesend/privatesend-client.cpp
index 6951a8f4f975..214e403d7fca 100644
--- a/src/privatesend/privatesend-client.cpp
+++ b/src/privatesend/privatesend-client.cpp
@@ -1444,7 +1444,7 @@ bool CPrivateSendClientSession::MakeCollateralAmounts(const CompactTallyItem& ta
     }
 
     auto getAdjustedAmount = [&](CAmount nAmount) -> CAmount {
-        // If amount is denominated remove one duff, this will to into fees!
+        // If amount is denominated remove one duff, this will go into fees!
         return CPrivateSend::IsDenominatedAmount(nAmount) ? nAmount - 1 : nAmount;
     };
 

From 3c473060623feef34be10101cdb398bce94b2537 Mon Sep 17 00:00:00 2001
From: xdustinface <xdustinfacex@gmail.com>
Date: Mon, 10 Aug 2020 13:26:31 +0200
Subject: [PATCH 16/45] wallet|privatesend: Refactor CCoinControl's
 m_dust_feerate -> m_discard_feerate

---
 src/privatesend/privatesend-util.cpp | 4 ++--
 src/wallet/coincontrol.h             | 4 ++--
 src/wallet/wallet.cpp                | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/privatesend/privatesend-util.cpp b/src/privatesend/privatesend-util.cpp
index 88b858716ca5..a733c0125ec0 100644
--- a/src/privatesend/privatesend-util.cpp
+++ b/src/privatesend/privatesend-util.cpp
@@ -109,7 +109,7 @@ CTransactionBuilder::CTransactionBuilder(CWallet* pwalletIn, const CompactTallyI
     tallyItem(tallyItemIn)
 {
     // Generate d feerate which will be used to consider if the remainder is dust and will go into fees or not
-    coinControl.m_dust_feerate = ::GetDiscardRate(::feeEstimator);
+    coinControl.m_discard_feerate = ::GetDiscardRate(::feeEstimator);
     // Generate a feerate which will be used by calculations of this class and also by CWallet::CreateTransaction
     coinControl.m_feerate = ::feeEstimator.estimateSmartFee(::nTxConfirmTarget, nullptr, true);
     // Fee will always go back to origin
@@ -219,7 +219,7 @@ CAmount CTransactionBuilder::GetFee(int nBytes) const
 
 bool CTransactionBuilder::IsDust(CAmount nAmount) const
 {
-    return ::IsDust(CTxOut(nAmount, ::GetScriptForDestination(tallyItem.txdest)), coinControl.m_dust_feerate.get());
+    return ::IsDust(CTxOut(nAmount, ::GetScriptForDestination(tallyItem.txdest)), coinControl.m_discard_feerate.get());
 }
 
 bool CTransactionBuilder::Commit(std::string& strResult)
diff --git a/src/wallet/coincontrol.h b/src/wallet/coincontrol.h
index 2778eba014f1..dea49186d3e3 100644
--- a/src/wallet/coincontrol.h
+++ b/src/wallet/coincontrol.h
@@ -39,8 +39,8 @@ class CCoinControl
     bool fOverrideFeeRate;
     //! Override the default payTxFee if set
     boost::optional<CFeeRate> m_feerate;
-    //! Override the dust feerate estimation with m_dust_feerate in CreateTransaction if set
-    boost::optional<CFeeRate> m_dust_feerate;
+    //! Override the discard feerate estimation with m_discard_feerate in CreateTransaction if set
+    boost::optional<CFeeRate> m_discard_feerate;
     //! Override the default confirmation target if set
     boost::optional<unsigned int> m_confirm_target;
     //! Fee estimation mode to control arguments to estimateSmartFee
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 7ffc0b468e34..c3d061509408 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -3734,7 +3734,7 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT
     assert(txNew.nLockTime <= (unsigned int)chainActive.Height());
     assert(txNew.nLockTime < LOCKTIME_THRESHOLD);
     FeeCalculation feeCalc;
-    CFeeRate discard_rate = coin_control.m_dust_feerate ? *coin_control.m_dust_feerate : GetDiscardRate(::feeEstimator);
+    CFeeRate discard_rate = coin_control.m_discard_feerate ? *coin_control.m_discard_feerate : GetDiscardRate(::feeEstimator);
     CAmount nFeeNeeded;
     unsigned int nBytes;
     {
@@ -3809,7 +3809,7 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT
                         }
                     }
 
-                    if (IsDust(txout, discard_rate)) {
+                    if (IsDust(txout, ::dustRelayFee)) {
                         if (recipient.fSubtractFeeFromAmount && nFeeRet > 0)
                         {
                             if (txout.nValue < 0)

From 8e765411a8a303334fb56bafe96cd9f0b636cb1b Mon Sep 17 00:00:00 2001
From: xdustinface <xdustinfacex@gmail.com>
Date: Tue, 11 Aug 2020 00:14:29 +0200
Subject: [PATCH 17/45] privatesend: Drop unused member
 CTransactionBuilder::dustFeeRate

---
 src/privatesend/privatesend-util.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/privatesend/privatesend-util.h b/src/privatesend/privatesend-util.h
index c88e6b516cf1..fc17ba935cdb 100644
--- a/src/privatesend/privatesend-util.h
+++ b/src/privatesend/privatesend-util.h
@@ -74,8 +74,6 @@ class CTransactionBuilder
     // Its a member just to make sure ReturnKey can be called in destructor just in case it gets generated/kept
     // somewhere in CWallet code.
     CReserveKey dummyReserveKey;
-    // Fee rate used to evaluate if an amount should be considered as dust or not
-    CFeeRate dustFeeRate;
     // Contains all utxos available to generate this transactions. They are all from the same address.
     CompactTallyItem tallyItem;
     // Contains the number of bytes required for a transaction with only the inputs of tallyItems, no outputs

From 068a0f9b4aa07a54e277866b8187beb4499ddd6b Mon Sep 17 00:00:00 2001
From: xdustinface <xdustinfacex@gmail.com>
Date: Tue, 11 Aug 2020 02:03:53 +0200
Subject: [PATCH 18/45] privatesend: Improve CTransactionBuilder's readability

---
 src/privatesend/privatesend-util.cpp | 25 +++++++++++++------------
 src/privatesend/privatesend-util.h   | 14 ++++++++------
 2 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/src/privatesend/privatesend-util.cpp b/src/privatesend/privatesend-util.cpp
index a733c0125ec0..c905510779ce 100644
--- a/src/privatesend/privatesend-util.cpp
+++ b/src/privatesend/privatesend-util.cpp
@@ -166,33 +166,34 @@ void CTransactionBuilder::Clear()
     dummyReserveKey.ReturnKey();
 }
 
-bool CTransactionBuilder::TryAddOutput(CAmount nAmount) const
+bool CTransactionBuilder::TryAddOutput(CAmount nAmountOutput) const
 {
-    return GetAmountLeft(tallyItem.nAmount, GetAmountUsed() + nAmount, GetFee(GetBytesTotal() + nBytesOutput)) >= 0;
+    return GetAmountLeft(GetAmountInitial(), GetAmountUsed() + nAmountOutput, GetFee(GetBytesTotal() + nBytesOutput)) >= 0;
 }
 
-bool CTransactionBuilder::TryAddOutputs(const std::vector<CAmount>& vecAmounts) const
+bool CTransactionBuilder::TryAddOutputs(const std::vector<CAmount>& vecOutputAmounts) const
 {
-    CAmount nAmountSum{0};
-    for (auto nAmount : vecAmounts) {
-        nAmountSum += nAmount;
+    CAmount nAmountAdditional{0};
+    int nBytesAdditional = nBytesOutput * vecOutputAmounts.size();
+    for (const auto nAmountOutput : vecOutputAmounts) {
+        nAmountAdditional += nAmountOutput;
     }
-    return GetAmountLeft(tallyItem.nAmount, GetAmountUsed() + nAmountSum, GetFee(GetBytesTotal() + nBytesOutput * vecAmounts.size())) >= 0;
+    return GetAmountLeft(GetAmountInitial(), GetAmountUsed() + nAmountAdditional, GetFee(GetBytesTotal() + nBytesAdditional)) >= 0;
 }
 
-CTransactionBuilderOutput* CTransactionBuilder::AddOutput(CAmount nAmount)
+CTransactionBuilderOutput* CTransactionBuilder::AddOutput(CAmount nAmountOutput)
 {
     LOCK(cs_outputs);
-    if (TryAddOutput(nAmount)) {
-        vecOutputs.push_back(std::make_unique<CTransactionBuilderOutput>(this, pwallet, nAmount));
+    if (TryAddOutput(nAmountOutput)) {
+        vecOutputs.push_back(std::make_unique<CTransactionBuilderOutput>(this, pwallet, nAmountOutput));
         return vecOutputs.back().get();
     }
     return nullptr;
 }
 
-CAmount CTransactionBuilder::GetAmountLeft(const CAmount nAmount, const CAmount nAmountUsed, const CAmount nFee)
+CAmount CTransactionBuilder::GetAmountLeft(const CAmount nAmountInitial, const CAmount nAmountUsed, const CAmount nFee)
 {
-    return nAmount - nAmountUsed - nFee;
+    return nAmountInitial - nAmountUsed - nFee;
 }
 
 CAmount CTransactionBuilder::GetAmountUsed() const
diff --git a/src/privatesend/privatesend-util.h b/src/privatesend/privatesend-util.h
index fc17ba935cdb..d348a5b62020 100644
--- a/src/privatesend/privatesend-util.h
+++ b/src/privatesend/privatesend-util.h
@@ -93,15 +93,15 @@ class CTransactionBuilder
     CTransactionBuilder(CWallet* pwalletIn, const CompactTallyItem& tallyItemIn);
     ~CTransactionBuilder();
     // Try to add a single output with the amount nAmount. Returns true if its possble and false if not.
-    bool TryAddOutput(CAmount nAmount) const;
+    bool TryAddOutput(CAmount nAmountOutput) const;
     // Try to add multiple outputs as vector of amounts. Returns true if its possible to add all of them and false if not.
-    bool TryAddOutputs(const std::vector<CAmount>& vecAmounts) const;
+    bool TryAddOutputs(const std::vector<CAmount>& vecOutputAmounts) const;
     // Add an output with the amount nAmount. Returns a pointer to the output if it could be added and nullptr if not due to insufficient amomunt left.
-    CTransactionBuilderOutput* AddOutput(CAmount nAmount = 0);
+    CTransactionBuilderOutput* AddOutput(CAmount nAmountOutput = 0);
     // Get amount we had available when we started
     CAmount GetAmountInitial() const { return tallyItem.nAmount; }
-    // Helper to calculate static remainders for output trying
-    static CAmount GetAmountLeft(const CAmount nAmount, const CAmount nAmountUsed, const CAmount nFee);
+    // Helper to calculate static amount left by simply subtracting an used amount and a fee from a provided initial amount.
+    static CAmount GetAmountLeft(const CAmount nAmountInitial, const CAmount nAmountUsed, const CAmount nFee);
     // Get the amount currently left to add more outputs. Does respect fees.
     CAmount GetAmountLeft() const { return GetAmountInitial() - GetAmountUsed() - GetFee(GetBytesTotal()); }
     // Check if an amounts should be considered as dust
@@ -120,7 +120,9 @@ class CTransactionBuilder
     int GetBytesTotal() const { return nBytesBase + vecOutputs.size() * nBytesOutput; }
     // Get the amount currently used by added outputs. Does not include fees.
     CAmount GetAmountUsed() const;
-    // Get fees based on the number of bytes and the feerate set in CoinControl
+    // Get fees based on the number of bytes and the feerate set in CoinControl.
+    // NOTE: To get the total transaction fee this should only be called once with the total number of bytes for the transaction to avoid
+    // calling CFeeRate::GetFee multiple times with subtotals as this may add rounding errors with each further call.
     CAmount GetFee(int nBytes) const;
 };
 

From 85ce8a0863e75e4d3d51a0eef2bde1e50ec41b32 Mon Sep 17 00:00:00 2001
From: xdustinface <xdustinfacex@gmail.com>
Date: Tue, 11 Aug 2020 03:20:48 +0200
Subject: [PATCH 19/45] privatesend: Make the static
 CTransactionBuilder::GetAmountLeft private

---
 src/privatesend/privatesend-util.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/privatesend/privatesend-util.h b/src/privatesend/privatesend-util.h
index d348a5b62020..96bf95eda826 100644
--- a/src/privatesend/privatesend-util.h
+++ b/src/privatesend/privatesend-util.h
@@ -100,8 +100,6 @@ class CTransactionBuilder
     CTransactionBuilderOutput* AddOutput(CAmount nAmountOutput = 0);
     // Get amount we had available when we started
     CAmount GetAmountInitial() const { return tallyItem.nAmount; }
-    // Helper to calculate static amount left by simply subtracting an used amount and a fee from a provided initial amount.
-    static CAmount GetAmountLeft(const CAmount nAmountInitial, const CAmount nAmountUsed, const CAmount nFee);
     // Get the amount currently left to add more outputs. Does respect fees.
     CAmount GetAmountLeft() const { return GetAmountInitial() - GetAmountUsed() - GetFee(GetBytesTotal()); }
     // Check if an amounts should be considered as dust
@@ -118,6 +116,8 @@ class CTransactionBuilder
     void Clear();
     // Get the total number of bytes used already by this transaction
     int GetBytesTotal() const { return nBytesBase + vecOutputs.size() * nBytesOutput; }
+    // Helper to calculate static amount left by simply subtracting an used amount and a fee from a provided initial amount.
+    static CAmount GetAmountLeft(const CAmount nAmountInitial, const CAmount nAmountUsed, const CAmount nFee);
     // Get the amount currently used by added outputs. Does not include fees.
     CAmount GetAmountUsed() const;
     // Get fees based on the number of bytes and the feerate set in CoinControl.

From 8b040156e3766cc8ac6bb7174f2e27144c01fa30 Mon Sep 17 00:00:00 2001
From: xdustinface <xdustinfacex@gmail.com>
Date: Tue, 11 Aug 2020 13:32:12 +0200
Subject: [PATCH 20/45] wallet: Recover previous code style

---
 src/wallet/wallet.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index c3d061509408..07cc571e4540 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -3809,7 +3809,8 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT
                         }
                     }
 
-                    if (IsDust(txout, ::dustRelayFee)) {
+                    if (IsDust(txout, ::dustRelayFee))
+                    {
                         if (recipient.fSubtractFeeFromAmount && nFeeRet > 0)
                         {
                             if (txout.nValue < 0)

From 1be40dc8e6d89203b363010b24a4096dc5e77698 Mon Sep 17 00:00:00 2001
From: dustinface <35775977+xdustinface@users.noreply.github.com>
Date: Tue, 11 Aug 2020 13:32:49 +0200
Subject: [PATCH 21/45] Update src/privatesend/privatesend-util.cpp

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
---
 src/privatesend/privatesend-util.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/privatesend/privatesend-util.cpp b/src/privatesend/privatesend-util.cpp
index c905510779ce..b51138435ada 100644
--- a/src/privatesend/privatesend-util.cpp
+++ b/src/privatesend/privatesend-util.cpp
@@ -108,7 +108,7 @@ CTransactionBuilder::CTransactionBuilder(CWallet* pwalletIn, const CompactTallyI
     dummyReserveKey(pwalletIn),
     tallyItem(tallyItemIn)
 {
-    // Generate d feerate which will be used to consider if the remainder is dust and will go into fees or not
+    // Generate a feerate which will be used to consider if the remainder is dust and will go into fees or not
     coinControl.m_discard_feerate = ::GetDiscardRate(::feeEstimator);
     // Generate a feerate which will be used by calculations of this class and also by CWallet::CreateTransaction
     coinControl.m_feerate = ::feeEstimator.estimateSmartFee(::nTxConfirmTarget, nullptr, true);

From 8ecd6c9ab1404ecc561f8edb0bbd28b76df124d0 Mon Sep 17 00:00:00 2001
From: UdjinM6 <UdjinM6@users.noreply.github.com>
Date: Thu, 13 Aug 2020 15:24:50 +0300
Subject: [PATCH 22/45] Tweak CTransactionBuilder to respect potential compact
 size diffs

---
 src/privatesend/privatesend-util.cpp | 14 ++++++++++++--
 src/privatesend/privatesend-util.h   |  2 +-
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/src/privatesend/privatesend-util.cpp b/src/privatesend/privatesend-util.cpp
index b51138435ada..b8b320c99d9e 100644
--- a/src/privatesend/privatesend-util.cpp
+++ b/src/privatesend/privatesend-util.cpp
@@ -191,6 +191,14 @@ CTransactionBuilderOutput* CTransactionBuilder::AddOutput(CAmount nAmountOutput)
     return nullptr;
 }
 
+unsigned int CTransactionBuilder::GetBytesTotal() const
+{
+    unsigned int nBytesAll = nBytesBase + vecOutputs.size() * nBytesOutput;
+    // Adding outputs can change the serialized size of the vout size
+    unsigned int nBytesCompactSizeDiff = GetSizeOfCompactSize(vecOutputs.size()) - GetSizeOfCompactSize(0);
+    return nBytesAll + nBytesCompactSizeDiff;
+}
+
 CAmount CTransactionBuilder::GetAmountLeft(const CAmount nAmountInitial, const CAmount nAmountUsed, const CAmount nFee)
 {
     return nAmountInitial - nAmountUsed - nFee;
@@ -258,12 +266,14 @@ bool CTransactionBuilder::Commit(std::string& strResult)
     }
 
     CAmount nFeeAdditional{0};
-    int nBytesAdditional{0};
+    unsigned int nBytesAdditional{0};
 
     if (fDust) {
         nFeeAdditional = nAmountLeft;
     } else {
-        nBytesAdditional = nBytesOutput;
+        // Adding another output can change the serialized size of the vout size
+        unsigned int nBytesCompactSizeDiff = GetSizeOfCompactSize(vecOutputs.size() + 1) - GetSizeOfCompactSize(vecOutputs.size());
+        nBytesAdditional = nBytesOutput + nBytesCompactSizeDiff;
     }
 
     // If the calculated fee does not match the fee returned by CreateTransaction aka if this check fails something is messed!
diff --git a/src/privatesend/privatesend-util.h b/src/privatesend/privatesend-util.h
index 96bf95eda826..6b6e073b8d14 100644
--- a/src/privatesend/privatesend-util.h
+++ b/src/privatesend/privatesend-util.h
@@ -115,7 +115,7 @@ class CTransactionBuilder
     // Clear the output vector and keep/return the included keys depending on the value of fKeepKeys
     void Clear();
     // Get the total number of bytes used already by this transaction
-    int GetBytesTotal() const { return nBytesBase + vecOutputs.size() * nBytesOutput; }
+    unsigned int GetBytesTotal() const;
     // Helper to calculate static amount left by simply subtracting an used amount and a fee from a provided initial amount.
     static CAmount GetAmountLeft(const CAmount nAmountInitial, const CAmount nAmountUsed, const CAmount nFee);
     // Get the amount currently used by added outputs. Does not include fees.

From 70b4a39fe884923bfa25fe17720f4f7b62ca1bca Mon Sep 17 00:00:00 2001
From: UdjinM6 <UdjinM6@users.noreply.github.com>
Date: Thu, 13 Aug 2020 16:46:41 +0300
Subject: [PATCH 23/45] Tweak GetFee param type

---
 src/privatesend/privatesend-util.cpp | 2 +-
 src/privatesend/privatesend-util.h   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/privatesend/privatesend-util.cpp b/src/privatesend/privatesend-util.cpp
index b8b320c99d9e..060afb559c17 100644
--- a/src/privatesend/privatesend-util.cpp
+++ b/src/privatesend/privatesend-util.cpp
@@ -213,7 +213,7 @@ CAmount CTransactionBuilder::GetAmountUsed() const
     return nAmountUsed;
 }
 
-CAmount CTransactionBuilder::GetFee(int nBytes) const
+CAmount CTransactionBuilder::GetFee(unsigned int nBytes) const
 {
     CAmount nFeeCalc = coinControl.m_feerate->GetFee(nBytes);
     CAmount nRequiredFee = GetRequiredFee(nBytes);
diff --git a/src/privatesend/privatesend-util.h b/src/privatesend/privatesend-util.h
index 6b6e073b8d14..d6b8f3bad628 100644
--- a/src/privatesend/privatesend-util.h
+++ b/src/privatesend/privatesend-util.h
@@ -123,7 +123,7 @@ class CTransactionBuilder
     // Get fees based on the number of bytes and the feerate set in CoinControl.
     // NOTE: To get the total transaction fee this should only be called once with the total number of bytes for the transaction to avoid
     // calling CFeeRate::GetFee multiple times with subtotals as this may add rounding errors with each further call.
-    CAmount GetFee(int nBytes) const;
+    CAmount GetFee(unsigned int nBytes) const;
 };
 
 #endif // BITCOIN_PRIVATESEND_PRIVATESEND_UTIL_H

From 91b988c89227cc899dd7be0d61a3d5afea63a387 Mon Sep 17 00:00:00 2001
From: UdjinM6 <UdjinM6@users.noreply.github.com>
Date: Thu, 13 Aug 2020 16:47:25 +0300
Subject: [PATCH 24/45] Trivial log/comments tweaks

---
 src/privatesend/privatesend-client.cpp | 2 +-
 src/privatesend/privatesend-util.cpp   | 6 ++++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/privatesend/privatesend-client.cpp b/src/privatesend/privatesend-client.cpp
index 214e403d7fca..9d703f62d987 100644
--- a/src/privatesend/privatesend-client.cpp
+++ b/src/privatesend/privatesend-client.cpp
@@ -1685,7 +1685,7 @@ bool CPrivateSendClientSession::CreateDenominated(CAmount nBalanceToDenominate,
                     it->second++;
                     nBalanceToDenominate -= nDenomValue;
                 } else {
-                    LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::%s -- 2 - Error: AddOutput failed: denomsToCreate: %d/%d, %s\n", __func__, i, denomsToCreate, txBuilder.ToString());
+                    LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::%s -- 2 - Error: AddOutput failed at %d/%d, %s\n", __func__, i + 1, denomsToCreate, txBuilder.ToString());
                     break;
                 }
                 LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::%s -- 2 - nDenomValue: %f, nBalanceToDenominate: %f, nOutputs: %d, %s\n",
diff --git a/src/privatesend/privatesend-util.cpp b/src/privatesend/privatesend-util.cpp
index 060afb559c17..fa96fb3712a1 100644
--- a/src/privatesend/privatesend-util.cpp
+++ b/src/privatesend/privatesend-util.cpp
@@ -112,7 +112,7 @@ CTransactionBuilder::CTransactionBuilder(CWallet* pwalletIn, const CompactTallyI
     coinControl.m_discard_feerate = ::GetDiscardRate(::feeEstimator);
     // Generate a feerate which will be used by calculations of this class and also by CWallet::CreateTransaction
     coinControl.m_feerate = ::feeEstimator.estimateSmartFee(::nTxConfirmTarget, nullptr, true);
-    // Fee will always go back to origin
+    // Change always goes back to origin
     coinControl.destChange = tallyItemIn.txdest;
     // Only allow tallyItems inputs for tx creation
     coinControl.fAllowOtherInputs = false;
@@ -298,7 +298,8 @@ bool CTransactionBuilder::Commit(std::string& strResult)
 
 std::string CTransactionBuilder::ToString() const
 {
-    return strprintf("CTransactionBuilder(Amount left: %d, Bytes base: %d, Bytes output %d, Bytes total: %d, Amount used: %d, Outputs: %d, Fee rate: %d, Fee: %d)",
+    return strprintf("CTransactionBuilder(Amount initial: %d, Amount left: %d, Bytes base: %d, Bytes output: %d, Bytes total: %d, Amount used: %d, Outputs: %d, Fee rate: %d, Discard fee rate: %d, Fee: %d)",
+        GetAmountInitial(),
         GetAmountLeft(),
         nBytesBase,
         nBytesOutput,
@@ -306,5 +307,6 @@ std::string CTransactionBuilder::ToString() const
         GetAmountUsed(),
         CountOutputs(),
         coinControl.m_feerate->GetFeePerK(),
+        coinControl.m_discard_feerate->GetFeePerK(),
         GetFee(GetBytesTotal()));
 }

From 55c6a8cf4613bf42c1f9105e3aa084a20ca1acd2 Mon Sep 17 00:00:00 2001
From: xdustinface <xdustinfacex@gmail.com>
Date: Fri, 14 Aug 2020 14:46:17 +0200
Subject: [PATCH 25/45] privatesend: Fix countPossibleOutputs

- Fix off by one issue
- Respect max outputs threshold
---
 src/privatesend/privatesend-client.cpp | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/privatesend/privatesend-client.cpp b/src/privatesend/privatesend-client.cpp
index 9d703f62d987..6bc0fb7c3a4b 100644
--- a/src/privatesend/privatesend-client.cpp
+++ b/src/privatesend/privatesend-client.cpp
@@ -1650,10 +1650,17 @@ bool CPrivateSendClientSession::CreateDenominated(CAmount nBalanceToDenominate,
 
         auto countPossibleOutputs = [&](CAmount nAmount) -> int {
             std::vector<CAmount> vecOutputs;
-            while (txBuilder.TryAddOutputs(vecOutputs)) {
+            while(true) {
+                // Create an potential output
                 vecOutputs.push_back(nAmount);
+                if (!txBuilder.TryAddOutputs(vecOutputs) || txBuilder.CountOutputs() + vecOutputs.size() > PRIVATESEND_DENOM_OUTPUTS_THRESHOLD) {
+                    // If its not possible to add it due to insufficient amount left or total number of outputs exceeds
+                    // PRIVATESEND_DENOM_OUTPUTS_THRESHOLD drop the output again and stop trying.
+                    vecOutputs.pop_back();
+                    break;
+                }
             }
-            return vecOutputs.size();
+            return static_cast<int>(vecOutputs.size());
         };
 
         // Go big to small

From 295965e49a389e5f2250309cb1ede6a6537f22b9 Mon Sep 17 00:00:00 2001
From: xdustinface <xdustinfacex@gmail.com>
Date: Fri, 14 Aug 2020 20:22:56 +0200
Subject: [PATCH 26/45] privatesend: Use GetSizeOfCompactSizeDiff in
 CTransactionBuilder

---
 src/privatesend/privatesend-util.cpp | 31 ++++++++++++++++++++--------
 src/privatesend/privatesend-util.h   |  2 ++
 2 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/src/privatesend/privatesend-util.cpp b/src/privatesend/privatesend-util.cpp
index fa96fb3712a1..cfec20c97aa1 100644
--- a/src/privatesend/privatesend-util.cpp
+++ b/src/privatesend/privatesend-util.cpp
@@ -10,6 +10,12 @@
 #include <validation.h>
 #include <wallet/fees.h>
 
+inline unsigned int GetSizeOfCompactSizeDiff(uint64_t nSizePrev, uint64_t nSizeNew)
+{
+    assert(nSizePrev <= nSizeNew);
+    return ::GetSizeOfCompactSize(nSizeNew) - ::GetSizeOfCompactSize(nSizePrev);
+}
+
 CKeyHolder::CKeyHolder(CWallet* pwallet) :
     reserveKey(pwallet)
 {
@@ -168,7 +174,9 @@ void CTransactionBuilder::Clear()
 
 bool CTransactionBuilder::TryAddOutput(CAmount nAmountOutput) const
 {
-    return GetAmountLeft(GetAmountInitial(), GetAmountUsed() + nAmountOutput, GetFee(GetBytesTotal() + nBytesOutput)) >= 0;
+    // Adding another output can change the serialized size of the vout size hence + GetSizeOfCompactSizeDiff()
+    unsigned int nBytes = GetBytesTotal() + nBytesOutput + GetSizeOfCompactSizeDiff(1);
+    return GetAmountLeft(GetAmountInitial(), GetAmountUsed() + nAmountOutput, GetFee(nBytes)) >= 0;
 }
 
 bool CTransactionBuilder::TryAddOutputs(const std::vector<CAmount>& vecOutputAmounts) const
@@ -178,7 +186,9 @@ bool CTransactionBuilder::TryAddOutputs(const std::vector<CAmount>& vecOutputAmo
     for (const auto nAmountOutput : vecOutputAmounts) {
         nAmountAdditional += nAmountOutput;
     }
-    return GetAmountLeft(GetAmountInitial(), GetAmountUsed() + nAmountAdditional, GetFee(GetBytesTotal() + nBytesAdditional)) >= 0;
+    // Adding other outputs can change the serialized size of the vout size hence + GetSizeOfCompactSizeDiff()
+    unsigned int nBytes = GetBytesTotal() + nBytesAdditional + GetSizeOfCompactSizeDiff(vecOutputAmounts.size());
+    return GetAmountLeft(GetAmountInitial(), GetAmountUsed() + nAmountAdditional, GetFee(nBytes)) >= 0;
 }
 
 CTransactionBuilderOutput* CTransactionBuilder::AddOutput(CAmount nAmountOutput)
@@ -193,10 +203,8 @@ CTransactionBuilderOutput* CTransactionBuilder::AddOutput(CAmount nAmountOutput)
 
 unsigned int CTransactionBuilder::GetBytesTotal() const
 {
-    unsigned int nBytesAll = nBytesBase + vecOutputs.size() * nBytesOutput;
-    // Adding outputs can change the serialized size of the vout size
-    unsigned int nBytesCompactSizeDiff = GetSizeOfCompactSize(vecOutputs.size()) - GetSizeOfCompactSize(0);
-    return nBytesAll + nBytesCompactSizeDiff;
+    // Adding other outputs can change the serialized size of the vout size hence + GetSizeOfCompactSizeDiff()
+   return nBytesBase + vecOutputs.size() * nBytesOutput + ::GetSizeOfCompactSizeDiff(0, vecOutputs.size());
 }
 
 CAmount CTransactionBuilder::GetAmountLeft(const CAmount nAmountInitial, const CAmount nAmountUsed, const CAmount nFee)
@@ -226,6 +234,12 @@ CAmount CTransactionBuilder::GetFee(unsigned int nBytes) const
     return nFeeCalc;
 }
 
+int CTransactionBuilder::GetSizeOfCompactSizeDiff(size_t nAdd) const
+{
+    size_t nSize = vecOutputs.size();
+    return ::GetSizeOfCompactSizeDiff(nSize, nSize + nAdd);
+}
+
 bool CTransactionBuilder::IsDust(CAmount nAmount) const
 {
     return ::IsDust(CTxOut(nAmount, ::GetScriptForDestination(tallyItem.txdest)), coinControl.m_discard_feerate.get());
@@ -271,9 +285,8 @@ bool CTransactionBuilder::Commit(std::string& strResult)
     if (fDust) {
         nFeeAdditional = nAmountLeft;
     } else {
-        // Adding another output can change the serialized size of the vout size
-        unsigned int nBytesCompactSizeDiff = GetSizeOfCompactSize(vecOutputs.size() + 1) - GetSizeOfCompactSize(vecOutputs.size());
-        nBytesAdditional = nBytesOutput + nBytesCompactSizeDiff;
+        // Add a change output and GetSizeOfCompactSizeDiff(1) as another output can changes the serialized size of the vout size in CTransaction
+        nBytesAdditional = nBytesOutput + GetSizeOfCompactSizeDiff(1);
     }
 
     // If the calculated fee does not match the fee returned by CreateTransaction aka if this check fails something is messed!
diff --git a/src/privatesend/privatesend-util.h b/src/privatesend/privatesend-util.h
index d6b8f3bad628..38764c8a970e 100644
--- a/src/privatesend/privatesend-util.h
+++ b/src/privatesend/privatesend-util.h
@@ -124,6 +124,8 @@ class CTransactionBuilder
     // NOTE: To get the total transaction fee this should only be called once with the total number of bytes for the transaction to avoid
     // calling CFeeRate::GetFee multiple times with subtotals as this may add rounding errors with each further call.
     CAmount GetFee(unsigned int nBytes) const;
+    // Helper to get GetSizeOfCompactSizeDiff(vecOutputs.size(), vecOutputs.size() + nAdd)
+    int GetSizeOfCompactSizeDiff(size_t nAdd) const;
 };
 
 #endif // BITCOIN_PRIVATESEND_PRIVATESEND_UTIL_H

From 5009c1f24a3e599b708ca454aeb1bb52e23cf5d2 Mon Sep 17 00:00:00 2001
From: dustinface <35775977+xdustinface@users.noreply.github.com>
Date: Wed, 19 Aug 2020 13:42:10 +0200
Subject: [PATCH 27/45] Apply suggestions from code review

Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>
---
 src/privatesend/privatesend-util.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/privatesend/privatesend-util.cpp b/src/privatesend/privatesend-util.cpp
index cfec20c97aa1..8853c1d60eba 100644
--- a/src/privatesend/privatesend-util.cpp
+++ b/src/privatesend/privatesend-util.cpp
@@ -204,7 +204,7 @@ CTransactionBuilderOutput* CTransactionBuilder::AddOutput(CAmount nAmountOutput)
 unsigned int CTransactionBuilder::GetBytesTotal() const
 {
     // Adding other outputs can change the serialized size of the vout size hence + GetSizeOfCompactSizeDiff()
-   return nBytesBase + vecOutputs.size() * nBytesOutput + ::GetSizeOfCompactSizeDiff(0, vecOutputs.size());
+    return nBytesBase + vecOutputs.size() * nBytesOutput + ::GetSizeOfCompactSizeDiff(0, vecOutputs.size());
 }
 
 CAmount CTransactionBuilder::GetAmountLeft(const CAmount nAmountInitial, const CAmount nAmountUsed, const CAmount nFee)
@@ -289,7 +289,7 @@ bool CTransactionBuilder::Commit(std::string& strResult)
         nBytesAdditional = nBytesOutput + GetSizeOfCompactSizeDiff(1);
     }
 
-    // If the calculated fee does not match the fee returned by CreateTransaction aka if this check fails something is messed!
+    // If the calculated fee does not match the fee returned by CreateTransaction aka if this check fails something is wrong!
     CAmount nFeeCalc = GetFee(GetBytesTotal() + nBytesAdditional) + nFeeAdditional;
     if (nFeeRet != nFeeCalc) {
         strResult = strprintf("Fee validation failed -> nFeeRet: %d, nFeeCalc: %d, nFeeAdditional: %d, nBytesAdditional: %d, %s", nFeeRet, nFeeCalc, nFeeAdditional, nBytesAdditional, ToString());

From 375522e3d717a1e594f135f18149142eb116e5fc Mon Sep 17 00:00:00 2001
From: xdustinface <xdustinfacex@gmail.com>
Date: Tue, 18 Aug 2020 14:05:24 +0200
Subject: [PATCH 28/45] privatesend: Rename TryAdd to CouldAdd in
 CTransactionBuilder

---
 src/privatesend/privatesend-client.cpp | 16 ++++++++--------
 src/privatesend/privatesend-util.cpp   |  6 +++---
 src/privatesend/privatesend-util.h     |  8 ++++----
 3 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/src/privatesend/privatesend-client.cpp b/src/privatesend/privatesend-client.cpp
index 6bc0fb7c3a4b..ef4b16012c8f 100644
--- a/src/privatesend/privatesend-client.cpp
+++ b/src/privatesend/privatesend-client.cpp
@@ -1439,7 +1439,7 @@ bool CPrivateSendClientSession::MakeCollateralAmounts(const CompactTallyItem& ta
     LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::%s -- Start %s\n", __func__, txBuilder.ToString());
 
     // Skip way too tiny amounts. Smallest we want is minimum collateral amount in a one output tx
-    if (!txBuilder.TryAddOutput(CPrivateSend::GetCollateralAmount())) {
+    if (!txBuilder.CouldAddOutput(CPrivateSend::GetCollateralAmount())) {
         return false;
     }
 
@@ -1449,7 +1449,7 @@ bool CPrivateSendClientSession::MakeCollateralAmounts(const CompactTallyItem& ta
     };
 
     int nCase{0}; // Just for debug logs
-    if (txBuilder.TryAddOutputs({CPrivateSend::GetMaxCollateralAmount(), CPrivateSend::GetCollateralAmount()})) {
+    if (txBuilder.CouldAddOutputs({CPrivateSend::GetMaxCollateralAmount(), CPrivateSend::GetCollateralAmount()})) {
         nCase = 1;
         // <case1>, see TransactionRecord::decomposeTransaction
         // Out1 == CPrivateSend::GetMaxCollateralAmount()
@@ -1460,7 +1460,7 @@ bool CPrivateSendClientSession::MakeCollateralAmounts(const CompactTallyItem& ta
         CTransactionBuilderOutput* out = txBuilder.AddOutput();
         out->UpdateAmount(getAdjustedAmount(txBuilder.GetAmountLeft()));
 
-    } else if (txBuilder.TryAddOutputs({CPrivateSend::GetCollateralAmount(), CPrivateSend::GetCollateralAmount()})) {
+    } else if (txBuilder.CouldAddOutputs({CPrivateSend::GetCollateralAmount(), CPrivateSend::GetCollateralAmount()})) {
         nCase = 2;
         // <case2>, see TransactionRecord::decomposeTransaction
         // Out1 CPrivateSend::IsCollateralAmount()
@@ -1584,7 +1584,7 @@ bool CPrivateSendClientSession::CreateDenominated(CAmount nBalanceToDenominate,
     // Now, in this system, so long as we don't reach PRIVATESEND_DENOM_OUTPUTS_THRESHOLD outputs the process repeats in
     // the same transaction, creating up to nPrivateSendDenomsHardCap per denomination in a single transaction.
 
-    while (txBuilder.TryAddOutput(CPrivateSend::GetSmallestDenomination()) && txBuilder.CountOutputs() < PRIVATESEND_DENOM_OUTPUTS_THRESHOLD) {
+    while (txBuilder.CouldAddOutput(CPrivateSend::GetSmallestDenomination()) && txBuilder.CountOutputs() < PRIVATESEND_DENOM_OUTPUTS_THRESHOLD) {
         for (auto it = vecStandardDenoms.rbegin(); it != vecStandardDenoms.rend(); ++it) {
             CAmount nDenomValue = *it;
             auto currentDenomIt = mapDenomCount.find(nDenomValue);
@@ -1592,7 +1592,7 @@ bool CPrivateSendClientSession::CreateDenominated(CAmount nBalanceToDenominate,
             int nOutputs = 0;
 
             auto needMoreOutputs = [&]() {
-                if (txBuilder.TryAddOutput(nDenomValue)) {
+                if (txBuilder.CouldAddOutput(nDenomValue)) {
                     if (fAddFinal && nBalanceToDenominate > 0 && nBalanceToDenominate < nDenomValue) {
                         fAddFinal = false; // add final denom only once, only the smalest possible one
                         LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::%s -- 1 - FINAL - nDenomValue: %f, nBalanceToDenominate: %f, nOutputs: %d, %s\n",
@@ -1629,7 +1629,7 @@ bool CPrivateSendClientSession::CreateDenominated(CAmount nBalanceToDenominate,
         for (const auto it : mapDenomCount) {
             // 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 < CPrivateSendClientOptions::GetDenomsGoal() && txBuilder.TryAddOutput(it.first) && nBalanceToDenominate > 0) {
+            if (it.second < CPrivateSendClientOptions::GetDenomsGoal() && txBuilder.CouldAddOutput(it.first) && nBalanceToDenominate > 0) {
                 finished = false;
                 LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::%s -- 1 - NOT finished - nDenomValue: %f, count: %d, nBalanceToDenominate: %f, %s\n",
                                              __func__, (float) it.first / COIN, it.second, (float) nBalanceToDenominate / COIN, txBuilder.ToString());
@@ -1643,7 +1643,7 @@ bool CPrivateSendClientSession::CreateDenominated(CAmount nBalanceToDenominate,
     }
 
     // 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.
-    if (txBuilder.TryAddOutput(CPrivateSend::GetSmallestDenomination()) && nBalanceToDenominate >= CPrivateSend::GetSmallestDenomination() && txBuilder.CountOutputs() < PRIVATESEND_DENOM_OUTPUTS_THRESHOLD) {
+    if (txBuilder.CouldAddOutput(CPrivateSend::GetSmallestDenomination()) && nBalanceToDenominate >= CPrivateSend::GetSmallestDenomination() && txBuilder.CountOutputs() < PRIVATESEND_DENOM_OUTPUTS_THRESHOLD) {
         CAmount nLargestDenomValue = vecStandardDenoms.front();
 
         LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::%s -- 2 - Process remainder: %s\n", __func__, txBuilder.ToString());
@@ -1653,7 +1653,7 @@ bool CPrivateSendClientSession::CreateDenominated(CAmount nBalanceToDenominate,
             while(true) {
                 // Create an potential output
                 vecOutputs.push_back(nAmount);
-                if (!txBuilder.TryAddOutputs(vecOutputs) || txBuilder.CountOutputs() + vecOutputs.size() > PRIVATESEND_DENOM_OUTPUTS_THRESHOLD) {
+                if (!txBuilder.CouldAddOutputs(vecOutputs) || txBuilder.CountOutputs() + vecOutputs.size() > PRIVATESEND_DENOM_OUTPUTS_THRESHOLD) {
                     // If its not possible to add it due to insufficient amount left or total number of outputs exceeds
                     // PRIVATESEND_DENOM_OUTPUTS_THRESHOLD drop the output again and stop trying.
                     vecOutputs.pop_back();
diff --git a/src/privatesend/privatesend-util.cpp b/src/privatesend/privatesend-util.cpp
index 8853c1d60eba..37c70856e09f 100644
--- a/src/privatesend/privatesend-util.cpp
+++ b/src/privatesend/privatesend-util.cpp
@@ -172,14 +172,14 @@ void CTransactionBuilder::Clear()
     dummyReserveKey.ReturnKey();
 }
 
-bool CTransactionBuilder::TryAddOutput(CAmount nAmountOutput) const
+bool CTransactionBuilder::CouldAddOutput(CAmount nAmountOutput) const
 {
     // Adding another output can change the serialized size of the vout size hence + GetSizeOfCompactSizeDiff()
     unsigned int nBytes = GetBytesTotal() + nBytesOutput + GetSizeOfCompactSizeDiff(1);
     return GetAmountLeft(GetAmountInitial(), GetAmountUsed() + nAmountOutput, GetFee(nBytes)) >= 0;
 }
 
-bool CTransactionBuilder::TryAddOutputs(const std::vector<CAmount>& vecOutputAmounts) const
+bool CTransactionBuilder::CouldAddOutputs(const std::vector<CAmount>& vecOutputAmounts) const
 {
     CAmount nAmountAdditional{0};
     int nBytesAdditional = nBytesOutput * vecOutputAmounts.size();
@@ -194,7 +194,7 @@ bool CTransactionBuilder::TryAddOutputs(const std::vector<CAmount>& vecOutputAmo
 CTransactionBuilderOutput* CTransactionBuilder::AddOutput(CAmount nAmountOutput)
 {
     LOCK(cs_outputs);
-    if (TryAddOutput(nAmountOutput)) {
+    if (CouldAddOutput(nAmountOutput)) {
         vecOutputs.push_back(std::make_unique<CTransactionBuilderOutput>(this, pwallet, nAmountOutput));
         return vecOutputs.back().get();
     }
diff --git a/src/privatesend/privatesend-util.h b/src/privatesend/privatesend-util.h
index 38764c8a970e..683a4b178391 100644
--- a/src/privatesend/privatesend-util.h
+++ b/src/privatesend/privatesend-util.h
@@ -92,10 +92,10 @@ class CTransactionBuilder
 public:
     CTransactionBuilder(CWallet* pwalletIn, const CompactTallyItem& tallyItemIn);
     ~CTransactionBuilder();
-    // Try to add a single output with the amount nAmount. Returns true if its possble and false if not.
-    bool TryAddOutput(CAmount nAmountOutput) const;
-    // Try to add multiple outputs as vector of amounts. Returns true if its possible to add all of them and false if not.
-    bool TryAddOutputs(const std::vector<CAmount>& vecOutputAmounts) const;
+    // Check it would be possible to add a single output with the amount nAmount. Returns true if its possible and false if not.
+    bool CouldAddOutput(CAmount nAmountOutput) const;
+    // Check if its possible to add multiple outputs as vector of amounts. Returns true if its possible to add all of them and false if not.
+    bool CouldAddOutputs(const std::vector<CAmount>& vecOutputAmounts) const;
     // Add an output with the amount nAmount. Returns a pointer to the output if it could be added and nullptr if not due to insufficient amomunt left.
     CTransactionBuilderOutput* AddOutput(CAmount nAmountOutput = 0);
     // Get amount we had available when we started

From 1b9eb931b8bfcdc68b636a7362a0eb679006ae73 Mon Sep 17 00:00:00 2001
From: xdustinface <xdustinfacex@gmail.com>
Date: Tue, 18 Aug 2020 16:23:05 +0200
Subject: [PATCH 29/45] wallet: Reset m_discard_feerate in
 CCoinControl::SetNull

---
 src/wallet/coincontrol.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/wallet/coincontrol.h b/src/wallet/coincontrol.h
index dea49186d3e3..67f6c5db653a 100644
--- a/src/wallet/coincontrol.h
+++ b/src/wallet/coincontrol.h
@@ -61,6 +61,7 @@ class CCoinControl
         fAllowWatchOnly = false;
         setSelected.clear();
         m_feerate.reset();
+        m_discard_feerate.reset();
         fOverrideFeeRate = false;
         m_confirm_target.reset();
         m_fee_mode = FeeEstimateMode::UNSET;

From 74afa537cfa63491b04826f91b8a0bca3983933a Mon Sep 17 00:00:00 2001
From: UdjinM6 <UdjinM6@users.noreply.github.com>
Date: Fri, 14 Aug 2020 13:41:36 +0300
Subject: [PATCH 30/45] Respect `-paytxfee` and `settxfee`

---
 src/privatesend/privatesend-util.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/privatesend/privatesend-util.cpp b/src/privatesend/privatesend-util.cpp
index 37c70856e09f..78434236ff22 100644
--- a/src/privatesend/privatesend-util.cpp
+++ b/src/privatesend/privatesend-util.cpp
@@ -117,7 +117,7 @@ CTransactionBuilder::CTransactionBuilder(CWallet* pwalletIn, const CompactTallyI
     // Generate a feerate which will be used to consider if the remainder is dust and will go into fees or not
     coinControl.m_discard_feerate = ::GetDiscardRate(::feeEstimator);
     // Generate a feerate which will be used by calculations of this class and also by CWallet::CreateTransaction
-    coinControl.m_feerate = ::feeEstimator.estimateSmartFee(::nTxConfirmTarget, nullptr, true);
+    coinControl.m_feerate = std::max(::feeEstimator.estimateSmartFee(::nTxConfirmTarget, nullptr, true), payTxFee);
     // Change always goes back to origin
     coinControl.destChange = tallyItemIn.txdest;
     // Only allow tallyItems inputs for tx creation

From 1183f29d058ddbda4dfe6a379368b69858cca1f3 Mon Sep 17 00:00:00 2001
From: xdustinface <xdustinfacex@gmail.com>
Date: Mon, 24 Aug 2020 11:17:53 +0200
Subject: [PATCH 31/45] privatesend: Check for denominated amount only where
 really required

---
 src/privatesend/privatesend-client.cpp | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/src/privatesend/privatesend-client.cpp b/src/privatesend/privatesend-client.cpp
index ef4b16012c8f..7d16ab4e454b 100644
--- a/src/privatesend/privatesend-client.cpp
+++ b/src/privatesend/privatesend-client.cpp
@@ -1443,11 +1443,6 @@ bool CPrivateSendClientSession::MakeCollateralAmounts(const CompactTallyItem& ta
         return false;
     }
 
-    auto getAdjustedAmount = [&](CAmount nAmount) -> CAmount {
-        // If amount is denominated remove one duff, this will go into fees!
-        return CPrivateSend::IsDenominatedAmount(nAmount) ? nAmount - 1 : nAmount;
-    };
-
     int nCase{0}; // Just for debug logs
     if (txBuilder.CouldAddOutputs({CPrivateSend::GetMaxCollateralAmount(), CPrivateSend::GetCollateralAmount()})) {
         nCase = 1;
@@ -1458,7 +1453,9 @@ bool CPrivateSendClientSession::MakeCollateralAmounts(const CompactTallyItem& ta
         txBuilder.AddOutput(CPrivateSend::GetMaxCollateralAmount());
         // Note, here we first add a zero amount output to get the remainder after all fees and then assign it
         CTransactionBuilderOutput* out = txBuilder.AddOutput();
-        out->UpdateAmount(getAdjustedAmount(txBuilder.GetAmountLeft()));
+        CAmount nAmountLeft = txBuilder.GetAmountLeft();
+        // If remainder is denominated add one duff to the fee
+        out->UpdateAmount(CPrivateSend::IsDenominatedAmount(nAmountLeft) ? nAmountLeft - 1 : nAmountLeft);
 
     } else if (txBuilder.CouldAddOutputs({CPrivateSend::GetCollateralAmount(), CPrivateSend::GetCollateralAmount()})) {
         nCase = 2;
@@ -1471,7 +1468,7 @@ bool CPrivateSendClientSession::MakeCollateralAmounts(const CompactTallyItem& ta
         CTransactionBuilderOutput* out2 = txBuilder.AddOutput();
 
         // Create two equal outputs from the available value. This adds one duff to the fee if txBuilder.GetAmountLeft() is odd.
-        CAmount nAmountOutputs = getAdjustedAmount(txBuilder.GetAmountLeft() / 2);
+        CAmount nAmountOutputs = txBuilder.GetAmountLeft() / 2;
 
         assert(CPrivateSend::IsCollateralAmount(nAmountOutputs));
 
@@ -1484,7 +1481,7 @@ bool CPrivateSendClientSession::MakeCollateralAmounts(const CompactTallyItem& ta
         // Out1 CPrivateSend::IsCollateralAmount()
         // Out2 Skipped
         CTransactionBuilderOutput* out = txBuilder.AddOutput();
-        out->UpdateAmount(getAdjustedAmount(txBuilder.GetAmountLeft()));
+        out->UpdateAmount(txBuilder.GetAmountLeft());
 
         assert(CPrivateSend::IsCollateralAmount(out->GetAmount()));
     }

From f98740cb4fa16afa519e9c4aec408f702d14d724 Mon Sep 17 00:00:00 2001
From: xdustinface <xdustinfacex@gmail.com>
Date: Mon, 24 Aug 2020 13:11:01 +0200
Subject: [PATCH 32/45] qt: Remove obsolete IsCollateralAmount() checks

---
 src/qt/transactionrecord.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/qt/transactionrecord.cpp b/src/qt/transactionrecord.cpp
index c7f000a1da19..3ec0a36452c8 100644
--- a/src/qt/transactionrecord.cpp
+++ b/src/qt/transactionrecord.cpp
@@ -157,10 +157,10 @@ QList<TransactionRecord> TransactionRecord::decomposeTransaction(const CWallet *
                         fMakeCollateral = (nAmount0 == CPrivateSend::GetMaxCollateralAmount() && !CPrivateSend::IsDenominatedAmount(nAmount1) && nAmount1 >= CPrivateSend::GetCollateralAmount()) ||
                                           (nAmount1 == CPrivateSend::GetMaxCollateralAmount() && !CPrivateSend::IsDenominatedAmount(nAmount0) && nAmount0 >= CPrivateSend::GetCollateralAmount()) ||
                         // <case2>, see CPrivateSendClientSession::MakeCollateralAmounts
-                                          (nAmount0 == nAmount1 && !CPrivateSend::IsDenominatedAmount(nAmount0) && CPrivateSend::IsCollateralAmount(nAmount0));
+                                          (nAmount0 == nAmount1 && CPrivateSend::IsCollateralAmount(nAmount0));
                     } else if (wtx.tx->vout.size() == 1) {
                         // <case3>, see CPrivateSendClientSession::MakeCollateralAmounts
-                        fMakeCollateral = !CPrivateSend::IsDenominatedAmount(wtx.tx->vout[0].nValue) && CPrivateSend::IsCollateralAmount(wtx.tx->vout[0].nValue);
+                        fMakeCollateral = CPrivateSend::IsCollateralAmount(wtx.tx->vout[0].nValue);
                     }
                     if (fMakeCollateral) {
                         sub.type = TransactionRecord::PrivateSendMakeCollaterals;

From f730f9719d0d0ef76f14e1c8b01067fe7762aa22 Mon Sep 17 00:00:00 2001
From: xdustinface <xdustinfacex@gmail.com>
Date: Tue, 25 Aug 2020 23:59:40 +0200
Subject: [PATCH 33/45] privatesend: Don't accept negative amounts in
 CTransactionBuilder

---
 src/privatesend/privatesend-util.cpp | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/privatesend/privatesend-util.cpp b/src/privatesend/privatesend-util.cpp
index 78434236ff22..85c22252ce95 100644
--- a/src/privatesend/privatesend-util.cpp
+++ b/src/privatesend/privatesend-util.cpp
@@ -174,6 +174,9 @@ void CTransactionBuilder::Clear()
 
 bool CTransactionBuilder::CouldAddOutput(CAmount nAmountOutput) const
 {
+    if (nAmountOutput < 0) {
+        return false;
+    }
     // Adding another output can change the serialized size of the vout size hence + GetSizeOfCompactSizeDiff()
     unsigned int nBytes = GetBytesTotal() + nBytesOutput + GetSizeOfCompactSizeDiff(1);
     return GetAmountLeft(GetAmountInitial(), GetAmountUsed() + nAmountOutput, GetFee(nBytes)) >= 0;
@@ -184,6 +187,9 @@ bool CTransactionBuilder::CouldAddOutputs(const std::vector<CAmount>& vecOutputA
     CAmount nAmountAdditional{0};
     int nBytesAdditional = nBytesOutput * vecOutputAmounts.size();
     for (const auto nAmountOutput : vecOutputAmounts) {
+        if (nAmountOutput < 0) {
+            return false;
+        }
         nAmountAdditional += nAmountOutput;
     }
     // Adding other outputs can change the serialized size of the vout size hence + GetSizeOfCompactSizeDiff()

From 0e6218caec7684176387635fe596be46438c26da Mon Sep 17 00:00:00 2001
From: xdustinface <xdustinfacex@gmail.com>
Date: Wed, 26 Aug 2020 04:56:57 +0200
Subject: [PATCH 34/45] privatesend: Remove unused CConnman parameter

---
 src/privatesend/privatesend-client.cpp | 18 +++++++++---------
 src/privatesend/privatesend-client.h   |  8 ++++----
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/src/privatesend/privatesend-client.cpp b/src/privatesend/privatesend-client.cpp
index 7d16ab4e454b..bc70dae54439 100644
--- a/src/privatesend/privatesend-client.cpp
+++ b/src/privatesend/privatesend-client.cpp
@@ -888,12 +888,12 @@ bool CPrivateSendClientSession::DoAutomaticDenominating(CConnman& connman, bool
         // there are funds to denominate and denominated balance does not exceed
         // max amount to mix yet.
         if (nBalanceAnonimizableNonDenom >= nValueMin + CPrivateSend::GetCollateralAmount() && nBalanceToDenominate > 0) {
-            CreateDenominated(nBalanceToDenominate, connman);
+            CreateDenominated(nBalanceToDenominate);
         }
 
         //check if we have the collateral sized inputs
         if (!mixingWallet->HasCollateralInputs()) {
-            return !mixingWallet->HasCollateralInputs(false) && MakeCollateralAmounts(connman);
+            return !mixingWallet->HasCollateralInputs(false) && MakeCollateralAmounts();
         }
 
         if (nSessionID) {
@@ -1376,7 +1376,7 @@ bool CPrivateSendClientSession::PrepareDenominate(int nMinRounds, int nMaxRounds
 }
 
 // Create collaterals by looping through inputs grouped by addresses
-bool CPrivateSendClientSession::MakeCollateralAmounts(CConnman& connman)
+bool CPrivateSendClientSession::MakeCollateralAmounts()
 {
     if (!CPrivateSendClientOptions::IsEnabled() || !mixingWallet) return false;
 
@@ -1400,13 +1400,13 @@ bool CPrivateSendClientSession::MakeCollateralAmounts(CConnman& connman)
 
     // First try to use only non-denominated funds
     for (const auto& item : vecTally) {
-        if (!MakeCollateralAmounts(item, false, connman)) continue;
+        if (!MakeCollateralAmounts(item, false)) continue;
         return true;
     }
 
     // There should be at least some denominated funds we should be able to break in pieces to continue mixing
     for (const auto& item : vecTally) {
-        if (!MakeCollateralAmounts(item, true, connman)) continue;
+        if (!MakeCollateralAmounts(item, true)) continue;
         return true;
     }
 
@@ -1416,7 +1416,7 @@ bool CPrivateSendClientSession::MakeCollateralAmounts(CConnman& connman)
 }
 
 // Split up large inputs or create fee sized inputs
-bool CPrivateSendClientSession::MakeCollateralAmounts(const CompactTallyItem& tallyItem, bool fTryDenominated, CConnman& connman)
+bool CPrivateSendClientSession::MakeCollateralAmounts(const CompactTallyItem& tallyItem, bool fTryDenominated)
 {
     AssertLockHeld(cs_main);
     AssertLockHeld(mempool.cs);
@@ -1504,7 +1504,7 @@ bool CPrivateSendClientSession::MakeCollateralAmounts(const CompactTallyItem& ta
 }
 
 // Create denominations by looping through inputs grouped by addresses
-bool CPrivateSendClientSession::CreateDenominated(CAmount nBalanceToDenominate, CConnman& connman)
+bool CPrivateSendClientSession::CreateDenominated(CAmount nBalanceToDenominate)
 {
     if (!CPrivateSendClientOptions::IsEnabled() || !mixingWallet) return false;
 
@@ -1529,7 +1529,7 @@ bool CPrivateSendClientSession::CreateDenominated(CAmount nBalanceToDenominate,
     bool fCreateMixingCollaterals = !mixingWallet->HasCollateralInputs();
 
     for (const auto& item : vecTally) {
-        if (!CreateDenominated(nBalanceToDenominate, item, fCreateMixingCollaterals, connman)) continue;
+        if (!CreateDenominated(nBalanceToDenominate, item, fCreateMixingCollaterals)) continue;
         return true;
     }
 
@@ -1538,7 +1538,7 @@ bool CPrivateSendClientSession::CreateDenominated(CAmount nBalanceToDenominate,
 }
 
 // Create denominations
-bool CPrivateSendClientSession::CreateDenominated(CAmount nBalanceToDenominate, const CompactTallyItem& tallyItem, bool fCreateMixingCollaterals, CConnman& connman)
+bool CPrivateSendClientSession::CreateDenominated(CAmount nBalanceToDenominate, const CompactTallyItem& tallyItem, bool fCreateMixingCollaterals)
 {
     AssertLockHeld(cs_main);
     AssertLockHeld(mempool.cs);
diff --git a/src/privatesend/privatesend-client.h b/src/privatesend/privatesend-client.h
index 3dc7a9b65d75..4268f2d42f0a 100644
--- a/src/privatesend/privatesend-client.h
+++ b/src/privatesend/privatesend-client.h
@@ -120,12 +120,12 @@ class CPrivateSendClientSession : public CPrivateSendBaseSession
     CWallet* mixingWallet;
 
     /// Create denominations
-    bool CreateDenominated(CAmount nBalanceToDenominate, CConnman& connman);
-    bool CreateDenominated(CAmount nBalanceToDenominate, const CompactTallyItem& tallyItem, bool fCreateMixingCollaterals, CConnman& connman);
+    bool CreateDenominated(CAmount nBalanceToDenominate);
+    bool CreateDenominated(CAmount nBalanceToDenominate, const CompactTallyItem& tallyItem, bool fCreateMixingCollaterals);
 
     /// Split up large inputs or make fee sized inputs
-    bool MakeCollateralAmounts(CConnman& connman);
-    bool MakeCollateralAmounts(const CompactTallyItem& tallyItem, bool fTryDenominated, CConnman& connman);
+    bool MakeCollateralAmounts();
+    bool MakeCollateralAmounts(const CompactTallyItem& tallyItem, bool fTryDenominated);
 
     bool JoinExistingQueue(CAmount nBalanceNeedsAnonymized, CConnman& connman);
     bool StartNewQueue(CAmount nBalanceNeedsAnonymized, CConnman& connman);

From 2543a4c142f78775a6376e137db78fd5f51291c0 Mon Sep 17 00:00:00 2001
From: pasta <pasta@dashboost.org>
Date: Sat, 29 Aug 2020 12:40:57 -0400
Subject: [PATCH 35/45] use emplace_back instead of push_back

Signed-off-by: pasta <pasta@dashboost.org>
---
 src/privatesend/privatesend-util.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/privatesend/privatesend-util.cpp b/src/privatesend/privatesend-util.cpp
index 85c22252ce95..cdd3ee111786 100644
--- a/src/privatesend/privatesend-util.cpp
+++ b/src/privatesend/privatesend-util.cpp
@@ -136,7 +136,7 @@ CTransactionBuilder::CTransactionBuilder(CWallet* pwalletIn, const CompactTallyI
     SignatureData dummySignature;
     ProduceSignature(DummySignatureCreator(pwallet), dummyScript, dummySignature);
     for (auto out : tallyItem.vecOutPoints) {
-        dummyTx.vin.push_back(CTxIn(out, dummySignature.scriptSig));
+        dummyTx.vin.emplace_back(out, dummySignature.scriptSig);
     }
     // Calculate required bytes for the dummy tx with tallyItem's inputs only
     nBytesBase = ::GetSerializeSize(dummyTx, SER_NETWORK, PROTOCOL_VERSION);

From bcdcb1311b077688fdd42e6b22bcf0e969ce1065 Mon Sep 17 00:00:00 2001
From: pasta <pasta@dashboost.org>
Date: Sat, 29 Aug 2020 12:57:47 -0400
Subject: [PATCH 36/45] fix typos

Signed-off-by: pasta <pasta@dashboost.org>
---
 src/privatesend/privatesend-client.cpp | 2 +-
 src/privatesend/privatesend-util.cpp   | 2 +-
 src/privatesend/privatesend-util.h     | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/privatesend/privatesend-client.cpp b/src/privatesend/privatesend-client.cpp
index bc70dae54439..c4d9430f8b1c 100644
--- a/src/privatesend/privatesend-client.cpp
+++ b/src/privatesend/privatesend-client.cpp
@@ -1647,7 +1647,7 @@ bool CPrivateSendClientSession::CreateDenominated(CAmount nBalanceToDenominate,
 
         auto countPossibleOutputs = [&](CAmount nAmount) -> int {
             std::vector<CAmount> vecOutputs;
-            while(true) {
+            while (true) {
                 // Create an potential output
                 vecOutputs.push_back(nAmount);
                 if (!txBuilder.CouldAddOutputs(vecOutputs) || txBuilder.CountOutputs() + vecOutputs.size() > PRIVATESEND_DENOM_OUTPUTS_THRESHOLD) {
diff --git a/src/privatesend/privatesend-util.cpp b/src/privatesend/privatesend-util.cpp
index cdd3ee111786..8213cc6f728d 100644
--- a/src/privatesend/privatesend-util.cpp
+++ b/src/privatesend/privatesend-util.cpp
@@ -257,7 +257,7 @@ bool CTransactionBuilder::Commit(std::string& strResult)
     CAmount nFeeRet = 0;
     int nChangePosRet = -1;
 
-    // Transfor the outputs to the format CWallet::CreateTransaction requires
+    // Transform the outputs to the format CWallet::CreateTransaction requires
     std::vector<CRecipient> vecSend;
     {
         LOCK(cs_outputs);
diff --git a/src/privatesend/privatesend-util.h b/src/privatesend/privatesend-util.h
index 683a4b178391..7ec2c9ce889e 100644
--- a/src/privatesend/privatesend-util.h
+++ b/src/privatesend/privatesend-util.h
@@ -80,7 +80,7 @@ class CTransactionBuilder
     int nBytesBase{0};
     // Contains the number of bytes required to add one output
     int nBytesOutput{0};
-    // Call KeepKey for all keys in desctructor if fKeepKeys is true, call ReturnKey for all key if its false.
+    // Call KeepKey for all keys in destructor if fKeepKeys is true, call ReturnKey for all key if its false.
     bool fKeepKeys{false};
     // Protect vecOutputs
     mutable CCriticalSection cs_outputs;
@@ -96,7 +96,7 @@ class CTransactionBuilder
     bool CouldAddOutput(CAmount nAmountOutput) const;
     // Check if its possible to add multiple outputs as vector of amounts. Returns true if its possible to add all of them and false if not.
     bool CouldAddOutputs(const std::vector<CAmount>& vecOutputAmounts) const;
-    // Add an output with the amount nAmount. Returns a pointer to the output if it could be added and nullptr if not due to insufficient amomunt left.
+    // Add an output with the amount nAmount. Returns a pointer to the output if it could be added and nullptr if not due to insufficient amount left.
     CTransactionBuilderOutput* AddOutput(CAmount nAmountOutput = 0);
     // Get amount we had available when we started
     CAmount GetAmountInitial() const { return tallyItem.nAmount; }

From 87b7431f8eb791bf76290d9cc564a90568a96f35 Mon Sep 17 00:00:00 2001
From: pasta <pasta@dashboost.org>
Date: Sat, 29 Aug 2020 12:56:13 -0400
Subject: [PATCH 37/45] make GetAmount const

Signed-off-by: pasta <pasta@dashboost.org>
---
 src/privatesend/privatesend-util.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/privatesend/privatesend-util.h b/src/privatesend/privatesend-util.h
index 7ec2c9ce889e..02708f2bb913 100644
--- a/src/privatesend/privatesend-util.h
+++ b/src/privatesend/privatesend-util.h
@@ -55,7 +55,7 @@ class CTransactionBuilderOutput
     // Get the scriptPubKey of this output
     CScript GetScript() { return script; }
     // Get the amount of this output
-    CAmount GetAmount() { return nAmount; }
+    CAmount GetAmount() const { return nAmount; }
     // Try update the amount of this output. Returns true if it was successful and false if not (e.g. insufficient amount left).
     bool UpdateAmount(const CAmount nAmount);
     // Tell the wallet to remove the key used by this output from the keypool

From aacb0562138026b75468af14e73d73566a9ae218 Mon Sep 17 00:00:00 2001
From: xdustinface <xdustinfacex@gmail.com>
Date: Sat, 29 Aug 2020 20:34:40 +0200
Subject: [PATCH 38/45] privatesend: Explicit capture __func__ in
 needMoreOutputs lambda

---
 src/privatesend/privatesend-client.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/privatesend/privatesend-client.cpp b/src/privatesend/privatesend-client.cpp
index c4d9430f8b1c..34160286ad3a 100644
--- a/src/privatesend/privatesend-client.cpp
+++ b/src/privatesend/privatesend-client.cpp
@@ -1588,12 +1588,13 @@ bool CPrivateSendClientSession::CreateDenominated(CAmount nBalanceToDenominate,
 
             int nOutputs = 0;
 
+            const auto& strFunc = __func__;
             auto needMoreOutputs = [&]() {
                 if (txBuilder.CouldAddOutput(nDenomValue)) {
                     if (fAddFinal && nBalanceToDenominate > 0 && nBalanceToDenominate < nDenomValue) {
                         fAddFinal = false; // add final denom only once, only the smalest possible one
                         LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::%s -- 1 - FINAL - nDenomValue: %f, nBalanceToDenominate: %f, nOutputs: %d, %s\n",
-                                                     __func__, (float) nDenomValue / COIN, (float) nBalanceToDenominate / COIN, nOutputs, txBuilder.ToString());
+                                                     strFunc, (float) nDenomValue / COIN, (float) nBalanceToDenominate / COIN, nOutputs, txBuilder.ToString());
                         return true;
                     } else if (nBalanceToDenominate >= nDenomValue) {
                         return true;

From 7814ff3bfd393757c4c014b92d9f90e9da3deda0 Mon Sep 17 00:00:00 2001
From: xdustinface <xdustinfacex@gmail.com>
Date: Sat, 29 Aug 2020 20:39:19 +0200
Subject: [PATCH 39/45] privatesend: Update CTransactionBuilder::UpdateAmount

---
 src/privatesend/privatesend-util.cpp | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/src/privatesend/privatesend-util.cpp b/src/privatesend/privatesend-util.cpp
index 8213cc6f728d..4d149ccbdbe3 100644
--- a/src/privatesend/privatesend-util.cpp
+++ b/src/privatesend/privatesend-util.cpp
@@ -97,16 +97,11 @@ CTransactionBuilderOutput::CTransactionBuilderOutput(CTransactionBuilder* pTxBui
 bool CTransactionBuilderOutput::UpdateAmount(const CAmount nNewAmount)
 {
     LOCK(pTxBuilder->cs_outputs);
-
-    const CAmount nAmountDiff = nNewAmount - nAmount;
-
-    if (nNewAmount > 0 && nAmountDiff <= pTxBuilder->GetAmountLeft()) {
-        if (nAmountDiff != 0) {
-            nAmount = nNewAmount;
-        }
-        return true;
+    if (nNewAmount <= 0 || nNewAmount - nAmount > pTxBuilder->GetAmountLeft()) {
+        return false;
     }
-    return false;
+    nAmount = nNewAmount;
+    return true;
 }
 
 CTransactionBuilder::CTransactionBuilder(CWallet* pwalletIn, const CompactTallyItem& tallyItemIn) :

From 531a91730bcddb9d44a0f920bf45ea9081d603c4 Mon Sep 17 00:00:00 2001
From: pasta <pasta@dashboost.org>
Date: Sat, 29 Aug 2020 12:56:44 -0400
Subject: [PATCH 40/45] remove const on parameter in declaration

Signed-off-by: pasta <pasta@dashboost.org>
---
 src/privatesend/privatesend-util.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/privatesend/privatesend-util.h b/src/privatesend/privatesend-util.h
index 02708f2bb913..b5af4d53630c 100644
--- a/src/privatesend/privatesend-util.h
+++ b/src/privatesend/privatesend-util.h
@@ -57,7 +57,7 @@ class CTransactionBuilderOutput
     // Get the amount of this output
     CAmount GetAmount() const { return nAmount; }
     // Try update the amount of this output. Returns true if it was successful and false if not (e.g. insufficient amount left).
-    bool UpdateAmount(const CAmount nAmount);
+    bool UpdateAmount(CAmount nAmount);
     // Tell the wallet to remove the key used by this output from the keypool
     void KeepKey() { key.KeepKey(); }
     // Tell the wallet to return the key used by this output to the keypool
@@ -117,7 +117,7 @@ class CTransactionBuilder
     // Get the total number of bytes used already by this transaction
     unsigned int GetBytesTotal() const;
     // Helper to calculate static amount left by simply subtracting an used amount and a fee from a provided initial amount.
-    static CAmount GetAmountLeft(const CAmount nAmountInitial, const CAmount nAmountUsed, const CAmount nFee);
+    static CAmount GetAmountLeft(CAmount nAmountInitial, CAmount nAmountUsed, CAmount nFee);
     // Get the amount currently used by added outputs. Does not include fees.
     CAmount GetAmountUsed() const;
     // Get fees based on the number of bytes and the feerate set in CoinControl.

From a7d81d92f7a4f7b9b2931a650c6cad1dd4340cd6 Mon Sep 17 00:00:00 2001
From: pasta <pasta@dashboost.org>
Date: Sat, 29 Aug 2020 12:48:50 -0400
Subject: [PATCH 41/45] handle unsigned int -> int conversions a bit better

Signed-off-by: pasta <pasta@dashboost.org>
---
 src/privatesend/privatesend-util.cpp | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/privatesend/privatesend-util.cpp b/src/privatesend/privatesend-util.cpp
index 4d149ccbdbe3..503ce8243dd0 100644
--- a/src/privatesend/privatesend-util.cpp
+++ b/src/privatesend/privatesend-util.cpp
@@ -180,7 +180,8 @@ bool CTransactionBuilder::CouldAddOutput(CAmount nAmountOutput) const
 bool CTransactionBuilder::CouldAddOutputs(const std::vector<CAmount>& vecOutputAmounts) const
 {
     CAmount nAmountAdditional{0};
-    int nBytesAdditional = nBytesOutput * vecOutputAmounts.size();
+    assert(vecOutputAmounts.size() < INT_MAX);
+    int nBytesAdditional = nBytesOutput * (int)vecOutputAmounts.size();
     for (const auto nAmountOutput : vecOutputAmounts) {
         if (nAmountOutput < 0) {
             return false;
@@ -238,7 +239,9 @@ CAmount CTransactionBuilder::GetFee(unsigned int nBytes) const
 int CTransactionBuilder::GetSizeOfCompactSizeDiff(size_t nAdd) const
 {
     size_t nSize = vecOutputs.size();
-    return ::GetSizeOfCompactSizeDiff(nSize, nSize + nAdd);
+    unsigned int ret = ::GetSizeOfCompactSizeDiff(nSize, nSize + nAdd);
+    assert(ret <= INT_MAX);
+    return (int)ret;
 }
 
 bool CTransactionBuilder::IsDust(CAmount nAmount) const

From f7406127eaf3ec6505a39a42777496d7dc1ddce5 Mon Sep 17 00:00:00 2001
From: pasta <pasta@dashboost.org>
Date: Sat, 29 Aug 2020 12:50:16 -0400
Subject: [PATCH 42/45] explicitly cast to int from unsigned int.

estimateSmartFee handles it if negative

Signed-off-by: pasta <pasta@dashboost.org>
---
 src/privatesend/privatesend-util.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/privatesend/privatesend-util.cpp b/src/privatesend/privatesend-util.cpp
index 503ce8243dd0..fbfe55ac4d3e 100644
--- a/src/privatesend/privatesend-util.cpp
+++ b/src/privatesend/privatesend-util.cpp
@@ -112,7 +112,7 @@ CTransactionBuilder::CTransactionBuilder(CWallet* pwalletIn, const CompactTallyI
     // Generate a feerate which will be used to consider if the remainder is dust and will go into fees or not
     coinControl.m_discard_feerate = ::GetDiscardRate(::feeEstimator);
     // Generate a feerate which will be used by calculations of this class and also by CWallet::CreateTransaction
-    coinControl.m_feerate = std::max(::feeEstimator.estimateSmartFee(::nTxConfirmTarget, nullptr, true), payTxFee);
+    coinControl.m_feerate = std::max(::feeEstimator.estimateSmartFee((int)::nTxConfirmTarget, nullptr, true), payTxFee);
     // Change always goes back to origin
     coinControl.destChange = tallyItemIn.txdest;
     // Only allow tallyItems inputs for tx creation

From 9a7b36ac5ac6b2c92dce77ea7c047201e388fb88 Mon Sep 17 00:00:00 2001
From: dustinface <35775977+xdustinface@users.noreply.github.com>
Date: Sun, 30 Aug 2020 17:14:07 +0200
Subject: [PATCH 43/45] Make CTransactionBuilderOutput::GetScript const

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
---
 src/privatesend/privatesend-util.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/privatesend/privatesend-util.h b/src/privatesend/privatesend-util.h
index b5af4d53630c..114bf14b0c04 100644
--- a/src/privatesend/privatesend-util.h
+++ b/src/privatesend/privatesend-util.h
@@ -53,7 +53,7 @@ class CTransactionBuilderOutput
     CTransactionBuilderOutput(CTransactionBuilderOutput&&) = delete;
     CTransactionBuilderOutput& operator=(CTransactionBuilderOutput&&) = delete;
     // Get the scriptPubKey of this output
-    CScript GetScript() { return script; }
+    CScript GetScript() const { return script; }
     // Get the amount of this output
     CAmount GetAmount() const { return nAmount; }
     // Try update the amount of this output. Returns true if it was successful and false if not (e.g. insufficient amount left).

From c30df0dc49958d08725da76ed50e8e345675d8db Mon Sep 17 00:00:00 2001
From: xdustinface <xdustinfacex@gmail.com>
Date: Tue, 1 Sep 2020 01:32:43 +0200
Subject: [PATCH 44/45] privatesend: Update comments to follow doxygen

---
 src/privatesend/privatesend-util.h | 76 +++++++++++++++---------------
 1 file changed, 38 insertions(+), 38 deletions(-)

diff --git a/src/privatesend/privatesend-util.h b/src/privatesend/privatesend-util.h
index 114bf14b0c04..58f54eedcc20 100644
--- a/src/privatesend/privatesend-util.h
+++ b/src/privatesend/privatesend-util.h
@@ -39,92 +39,92 @@ class CKeyHolderStorage
 
 class CTransactionBuilderOutput
 {
-    // Used for amount updates
+    /// Used for amount updates
     CTransactionBuilder* pTxBuilder{nullptr};
-    // Reserve key where the amount of this output will end up
+    /// Reserve key where the amount of this output will end up
     CReserveKey key;
-    // Amount this output will receive
+    /// Amount this output will receive
     CAmount nAmount{0};
-    // ScriptPubKey of this output
+    /// ScriptPubKey of this output
     CScript script;
 
 public:
     CTransactionBuilderOutput(CTransactionBuilder* pTxBuilderIn, CWallet* pwalletIn, CAmount nAmountIn);
     CTransactionBuilderOutput(CTransactionBuilderOutput&&) = delete;
     CTransactionBuilderOutput& operator=(CTransactionBuilderOutput&&) = delete;
-    // Get the scriptPubKey of this output
+    /// Get the scriptPubKey of this output
     CScript GetScript() const { return script; }
-    // Get the amount of this output
+    /// Get the amount of this output
     CAmount GetAmount() const { return nAmount; }
-    // Try update the amount of this output. Returns true if it was successful and false if not (e.g. insufficient amount left).
+    /// Try update the amount of this output. Returns true if it was successful and false if not (e.g. insufficient amount left).
     bool UpdateAmount(CAmount nAmount);
-    // Tell the wallet to remove the key used by this output from the keypool
+    /// Tell the wallet to remove the key used by this output from the keypool
     void KeepKey() { key.KeepKey(); }
-    // Tell the wallet to return the key used by this output to the keypool
+    /// Tell the wallet to return the key used by this output to the keypool
     void ReturnKey() { key.ReturnKey(); }
 };
 
 class CTransactionBuilder
 {
-    // Wallet the transaction will be build for
+    /// Wallet the transaction will be build for
     CWallet* pwallet{nullptr};
-    // See CTransactionBuilder() for initialization
+    /// See CTransactionBuilder() for initialization
     CCoinControl coinControl;
-    // Dummy since we anyway use tallyItem's destination as change destination in coincontrol.
-    // Its a member just to make sure ReturnKey can be called in destructor just in case it gets generated/kept
-    // somewhere in CWallet code.
+    /// Dummy since we anyway use tallyItem's destination as change destination in coincontrol.
+    /// Its a member just to make sure ReturnKey can be called in destructor just in case it gets generated/kept
+    /// somewhere in CWallet code.
     CReserveKey dummyReserveKey;
-    // Contains all utxos available to generate this transactions. They are all from the same address.
+    /// Contains all utxos available to generate this transactions. They are all from the same address.
     CompactTallyItem tallyItem;
-    // Contains the number of bytes required for a transaction with only the inputs of tallyItems, no outputs
+    /// Contains the number of bytes required for a transaction with only the inputs of tallyItems, no outputs
     int nBytesBase{0};
-    // Contains the number of bytes required to add one output
+    /// Contains the number of bytes required to add one output
     int nBytesOutput{0};
-    // Call KeepKey for all keys in destructor if fKeepKeys is true, call ReturnKey for all key if its false.
+    /// Call KeepKey for all keys in destructor if fKeepKeys is true, call ReturnKey for all key if its false.
     bool fKeepKeys{false};
-    // Protect vecOutputs
+    /// Protect vecOutputs
     mutable CCriticalSection cs_outputs;
-    // Contains all outputs already added to the transaction
+    /// Contains all outputs already added to the transaction
     std::vector<std::unique_ptr<CTransactionBuilderOutput>> vecOutputs;
-    // Needed by CTransactionBuilderOutput::UpdateAmount to lock cs_outputs
+    /// Needed by CTransactionBuilderOutput::UpdateAmount to lock cs_outputs
     friend class CTransactionBuilderOutput;
 
 public:
     CTransactionBuilder(CWallet* pwalletIn, const CompactTallyItem& tallyItemIn);
     ~CTransactionBuilder();
-    // Check it would be possible to add a single output with the amount nAmount. Returns true if its possible and false if not.
+    /// Check it would be possible to add a single output with the amount nAmount. Returns true if its possible and false if not.
     bool CouldAddOutput(CAmount nAmountOutput) const;
-    // Check if its possible to add multiple outputs as vector of amounts. Returns true if its possible to add all of them and false if not.
+    /// Check if its possible to add multiple outputs as vector of amounts. Returns true if its possible to add all of them and false if not.
     bool CouldAddOutputs(const std::vector<CAmount>& vecOutputAmounts) const;
-    // Add an output with the amount nAmount. Returns a pointer to the output if it could be added and nullptr if not due to insufficient amount left.
+    /// Add an output with the amount nAmount. Returns a pointer to the output if it could be added and nullptr if not due to insufficient amount left.
     CTransactionBuilderOutput* AddOutput(CAmount nAmountOutput = 0);
-    // Get amount we had available when we started
+    /// Get amount we had available when we started
     CAmount GetAmountInitial() const { return tallyItem.nAmount; }
-    // Get the amount currently left to add more outputs. Does respect fees.
+    /// Get the amount currently left to add more outputs. Does respect fees.
     CAmount GetAmountLeft() const { return GetAmountInitial() - GetAmountUsed() - GetFee(GetBytesTotal()); }
-    // Check if an amounts should be considered as dust
+    /// Check if an amounts should be considered as dust
     bool IsDust(CAmount nAmount) const;
-    // Get the total number of added outputs
+    /// Get the total number of added outputs
     int CountOutputs() const { return vecOutputs.size(); }
-    // Create and Commit the transaction to the wallet
+    /// Create and Commit the transaction to the wallet
     bool Commit(std::string& strResult);
-    // Convert to a string
+    /// Convert to a string
     std::string ToString() const;
 
 private:
-    // Clear the output vector and keep/return the included keys depending on the value of fKeepKeys
+    /// Clear the output vector and keep/return the included keys depending on the value of fKeepKeys
     void Clear();
-    // Get the total number of bytes used already by this transaction
+    /// Get the total number of bytes used already by this transaction
     unsigned int GetBytesTotal() const;
-    // Helper to calculate static amount left by simply subtracting an used amount and a fee from a provided initial amount.
+    /// Helper to calculate static amount left by simply subtracting an used amount and a fee from a provided initial amount.
     static CAmount GetAmountLeft(CAmount nAmountInitial, CAmount nAmountUsed, CAmount nFee);
-    // Get the amount currently used by added outputs. Does not include fees.
+    /// Get the amount currently used by added outputs. Does not include fees.
     CAmount GetAmountUsed() const;
-    // Get fees based on the number of bytes and the feerate set in CoinControl.
-    // NOTE: To get the total transaction fee this should only be called once with the total number of bytes for the transaction to avoid
-    // calling CFeeRate::GetFee multiple times with subtotals as this may add rounding errors with each further call.
+    /// Get fees based on the number of bytes and the feerate set in CoinControl.
+    /// NOTE: To get the total transaction fee this should only be called once with the total number of bytes for the transaction to avoid
+    /// calling CFeeRate::GetFee multiple times with subtotals as this may add rounding errors with each further call.
     CAmount GetFee(unsigned int nBytes) const;
-    // Helper to get GetSizeOfCompactSizeDiff(vecOutputs.size(), vecOutputs.size() + nAdd)
+    /// Helper to get GetSizeOfCompactSizeDiff(vecOutputs.size(), vecOutputs.size() + nAdd)
     int GetSizeOfCompactSizeDiff(size_t nAdd) const;
 };
 

From fb2c6d0ecb571043ae64957d240aef687b50a387 Mon Sep 17 00:00:00 2001
From: xdustinface <xdustinfacex@gmail.com>
Date: Tue, 1 Sep 2020 01:57:11 +0200
Subject: [PATCH 45/45] privatesend: Add class descriptions

---
 src/privatesend/privatesend-util.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/src/privatesend/privatesend-util.h b/src/privatesend/privatesend-util.h
index 58f54eedcc20..17defb72adc7 100644
--- a/src/privatesend/privatesend-util.h
+++ b/src/privatesend/privatesend-util.h
@@ -37,6 +37,10 @@ class CKeyHolderStorage
     void ReturnAll();
 };
 
+/**
+ * @brief Used by CTransactionBuilder to represent its transaction outputs.
+ * It creates a CReserveKey for the given CWallet as destination.
+ */
 class CTransactionBuilderOutput
 {
     /// Used for amount updates
@@ -64,6 +68,11 @@ class CTransactionBuilderOutput
     void ReturnKey() { key.ReturnKey(); }
 };
 
+/**
+ * @brief Enables simple transaction generation for a given CWallet object. The resulting
+ * transaction's inputs are defined by the given CompactTallyItem. The outputs are
+ * defined by CTransactionBuilderOutput.
+ */
 class CTransactionBuilder
 {
     /// Wallet the transaction will be build for