More accurate fee calculation in CreateDenominated (#3588)

* More accurate fee calculation in CreateDenominated

* Apply suggestions from code review

Co-authored-by: dustinface <35775977+xdustinface@users.noreply.github.com>

* Fix `finished` conditions

Co-authored-by: dustinface <35775977+xdustinface@users.noreply.github.com>
This commit is contained in:
UdjinM6 2020-07-07 20:31:33 +03:00 committed by GitHub
parent ab9a8eed37
commit dc66c13863
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

View File

@ -17,6 +17,7 @@
#include <utilmoneystr.h> #include <utilmoneystr.h>
#include <validation.h> #include <validation.h>
#include <wallet/coincontrol.h> #include <wallet/coincontrol.h>
#include <wallet/fees.h>
#include <memory> #include <memory>
#include <univalue.h> #include <univalue.h>
@ -1504,8 +1505,15 @@ bool CPrivateSendClientSession::CreateDenominated(CAmount nBalanceToDenominate,
std::vector<CRecipient> vecSend; std::vector<CRecipient> vecSend;
CKeyHolderStorage keyHolderStorageDenom; 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 */);
CAmount nValueLeft = tallyItem.nAmount; CAmount nValueLeft = tallyItem.nAmount;
nValueLeft -= CPrivateSend::GetCollateralAmount(); // leave some room for fees // Leave some room for fees, assuming we are going to spend all the outpoints
nValueLeft -= tallyItem.vecOutPoints.size() * nInputFee;
LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::CreateDenominated -- 0 - %s nValueLeft: %f\n", EncodeDestination(tallyItem.txdest), (float)nValueLeft / COIN); LogPrint(BCLog::PRIVATESEND, "CPrivateSendClientSession::CreateDenominated -- 0 - %s nValueLeft: %f\n", EncodeDestination(tallyItem.txdest), (float)nValueLeft / COIN);
@ -1514,7 +1522,7 @@ bool CPrivateSendClientSession::CreateDenominated(CAmount nBalanceToDenominate,
if (fCreateMixingCollaterals) { if (fCreateMixingCollaterals) {
CScript scriptCollateral = keyHolderStorageDenom.AddKey(GetWallets()[0]); CScript scriptCollateral = keyHolderStorageDenom.AddKey(GetWallets()[0]);
vecSend.push_back((CRecipient){scriptCollateral, CPrivateSend::GetMaxCollateralAmount(), false}); vecSend.push_back((CRecipient){scriptCollateral, CPrivateSend::GetMaxCollateralAmount(), false});
nValueLeft -= CPrivateSend::GetMaxCollateralAmount(); nValueLeft -= CPrivateSend::GetMaxCollateralAmount() + nOutputFee;
} }
// ****** Add outputs for denoms ************ / // ****** Add outputs for denoms ************ /
@ -1546,9 +1554,9 @@ bool CPrivateSendClientSession::CreateDenominated(CAmount nBalanceToDenominate,
int nOutputs = 0; int nOutputs = 0;
auto needMoreOutputs = [&]() { auto needMoreOutputs = [&]() {
bool fRegular = (nValueLeft >= nDenomValue && nBalanceToDenominate >= nDenomValue); bool fRegular = ((nValueLeft >= nDenomValue + nOutputFee) && nBalanceToDenominate >= nDenomValue);
bool fFinal = (fAddFinal bool fFinal = (fAddFinal
&& nValueLeft >= nDenomValue && nValueLeft >= nDenomValue + nOutputFee
&& nBalanceToDenominate > 0 && nBalanceToDenominate > 0
&& nBalanceToDenominate < nDenomValue); && nBalanceToDenominate < nDenomValue);
if (fFinal) { if (fFinal) {
@ -1570,7 +1578,7 @@ bool CPrivateSendClientSession::CreateDenominated(CAmount nBalanceToDenominate,
// increment outputs and subtract denomination amount // increment outputs and subtract denomination amount
nOutputs++; nOutputs++;
currentDenomIt->second++; currentDenomIt->second++;
nValueLeft -= nDenomValue; nValueLeft -= nDenomValue + nOutputFee;
nBalanceToDenominate -= nDenomValue; nBalanceToDenominate -= nDenomValue;
LogPrint(BCLog::PRIVATESEND, LogPrint(BCLog::PRIVATESEND,
"CPrivateSendClientSession::CreateDenominated -- 1 - nDenomValue: %f, totalOutputs: %d, nOutputsTotal: %d, nOutputs: %d, nValueLeft: %f, nBalanceToDenominate: %f\n", "CPrivateSendClientSession::CreateDenominated -- 1 - nDenomValue: %f, totalOutputs: %d, nOutputsTotal: %d, nOutputs: %d, nValueLeft: %f, nBalanceToDenominate: %f\n",
@ -1585,7 +1593,7 @@ bool CPrivateSendClientSession::CreateDenominated(CAmount nBalanceToDenominate,
for (const auto it : mapDenomCount) { for (const auto it : mapDenomCount) {
// Check if this specific denom could use another loop, check that there aren't nPrivateSendDenomsGoal of this // Check if this specific denom could use another loop, check that there aren't nPrivateSendDenomsGoal of this
// denom and that our nValueLeft/nBalanceToDenominate is enough to create one of these denoms, if so, loop again. // denom and that our nValueLeft/nBalanceToDenominate is enough to create one of these denoms, if so, loop again.
if (it.second < privateSendClient.nPrivateSendDenomsGoal && nValueLeft >= it.first && nBalanceToDenominate > 0) { if (it.second < privateSendClient.nPrivateSendDenomsGoal && (nValueLeft >= it.first + nOutputFee) && nBalanceToDenominate > 0) {
finished = false; finished = false;
LogPrint(BCLog::PRIVATESEND, LogPrint(BCLog::PRIVATESEND,
"CPrivateSendClientSession::CreateDenominated -- 1 - NOT finished - nDenomValue: %f, count: %d, nValueLeft: %f, nBalanceToDenominate: %f\n", "CPrivateSendClientSession::CreateDenominated -- 1 - NOT finished - nDenomValue: %f, count: %d, nValueLeft: %f, nBalanceToDenominate: %f\n",
@ -1601,7 +1609,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. // 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() && nBalanceToDenominate >= CPrivateSend::GetSmallestDenomination() if ((nValueLeft >= CPrivateSend::GetSmallestDenomination() + nOutputFee) && nBalanceToDenominate >= CPrivateSend::GetSmallestDenomination()
&& nOutputsTotal < PRIVATESEND_DENOM_OUTPUTS_THRESHOLD) { && nOutputsTotal < PRIVATESEND_DENOM_OUTPUTS_THRESHOLD) {
CAmount nLargestDenomValue = vecStandardDenoms.front(); CAmount nLargestDenomValue = vecStandardDenoms.front();
@ -1611,7 +1619,7 @@ bool CPrivateSendClientSession::CreateDenominated(CAmount nBalanceToDenominate,
int nOutputs = 0; int nOutputs = 0;
// Number of denoms we can create given our denom and the amount of funds we have left // Number of denoms we can create given our denom and the amount of funds we have left
int denomsToCreateValue = nValueLeft / nDenomValue; int denomsToCreateValue = nValueLeft / (nDenomValue + nOutputFee);
int denomsToCreateBal = nBalanceToDenominate / nDenomValue; int denomsToCreateBal = nBalanceToDenominate / nDenomValue;
// Use the smaller value // Use the smaller value
int denomsToCreate = denomsToCreateValue > denomsToCreateBal ? denomsToCreateBal : denomsToCreateValue; int denomsToCreate = denomsToCreateValue > denomsToCreateBal ? denomsToCreateBal : denomsToCreateValue;
@ -1626,7 +1634,7 @@ bool CPrivateSendClientSession::CreateDenominated(CAmount nBalanceToDenominate,
// increment outputs and subtract denomination amount // increment outputs and subtract denomination amount
nOutputs++; nOutputs++;
it->second++; it->second++;
nValueLeft -= nDenomValue; nValueLeft -= nDenomValue + nOutputFee;
nBalanceToDenominate -= nDenomValue; nBalanceToDenominate -= nDenomValue;
LogPrint(BCLog::PRIVATESEND, LogPrint(BCLog::PRIVATESEND,
"CPrivateSendClientSession::CreateDenominated -- 2 - nDenomValue: %f, totalOutputs: %d, nOutputsTotal: %d, nOutputs: %d, nValueLeft: %f, nBalanceToDenominate: %f\n", "CPrivateSendClientSession::CreateDenominated -- 2 - nDenomValue: %f, totalOutputs: %d, nOutputsTotal: %d, nOutputs: %d, nValueLeft: %f, nBalanceToDenominate: %f\n",
@ -1652,7 +1660,6 @@ bool CPrivateSendClientSession::CreateDenominated(CAmount nBalanceToDenominate,
// if we have anything left over, it will be automatically send back as change - there is no need to send it manually // if we have anything left over, it will be automatically send back as change - there is no need to send it manually
CCoinControl coinControl;
coinControl.fAllowOtherInputs = false; coinControl.fAllowOtherInputs = false;
coinControl.fAllowWatchOnly = false; coinControl.fAllowWatchOnly = false;
coinControl.nCoinType = CoinType::ONLY_NONDENOMINATED; coinControl.nCoinType = CoinType::ONLY_NONDENOMINATED;