Skip to content

More accurate fee calculation in CreateDenominated #3588

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
UdjinM6 merged 3 commits into from
Jul 7, 2020
Merged

More accurate fee calculation in CreateDenominated #3588

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b42afa9
More accurate fee calculation in CreateDenominated
UdjinM6 Jun 13, 2020
5269bd9
Apply suggestions from code review
UdjinM6 Jul 6, 2020
85c3a33
Fix `finished` conditions
UdjinM6 Jul 7, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 17 additions & 10 deletions src/privatesend/privatesend-client.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include <utilmoneystr.h>
#include <validation.h>
#include <wallet/coincontrol.h>
#include <wallet/fees.h>

#include <memory>
#include <univalue.h>
Expand Down Expand Up @@ -1504,8 +1505,15 @@ bool CPrivateSendClientSession::CreateDenominated(CAmount nBalanceToDenominate,
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 */);

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);

Expand All @@ -1514,7 +1522,7 @@ bool CPrivateSendClientSession::CreateDenominated(CAmount nBalanceToDenominate,
if (fCreateMixingCollaterals) {
CScript scriptCollateral = keyHolderStorageDenom.AddKey(GetWallets()[0]);
vecSend.push_back((CRecipient){scriptCollateral, CPrivateSend::GetMaxCollateralAmount(), false});
nValueLeft -= CPrivateSend::GetMaxCollateralAmount();
nValueLeft -= CPrivateSend::GetMaxCollateralAmount() + nOutputFee;
}

// ****** Add outputs for denoms ************ /
Expand Down Expand Up @@ -1546,9 +1554,9 @@ bool CPrivateSendClientSession::CreateDenominated(CAmount nBalanceToDenominate,
int nOutputs = 0;

auto needMoreOutputs = [&]() {
bool fRegular = (nValueLeft >= nDenomValue && nBalanceToDenominate >= nDenomValue);
bool fRegular = ((nValueLeft >= nDenomValue + nOutputFee) && nBalanceToDenominate >= nDenomValue);
bool fFinal = (fAddFinal
&& nValueLeft >= nDenomValue
&& nValueLeft >= nDenomValue + nOutputFee
&& nBalanceToDenominate > 0
&& nBalanceToDenominate < nDenomValue);
if (fFinal) {
Expand All @@ -1570,7 +1578,7 @@ bool CPrivateSendClientSession::CreateDenominated(CAmount nBalanceToDenominate,
// increment outputs and subtract denomination amount
nOutputs++;
currentDenomIt->second++;
nValueLeft -= nDenomValue;
nValueLeft -= nDenomValue + nOutputFee;
nBalanceToDenominate -= nDenomValue;
LogPrint(BCLog::PRIVATESEND,
"CPrivateSendClientSession::CreateDenominated -- 1 - nDenomValue: %f, totalOutputs: %d, nOutputsTotal: %d, nOutputs: %d, nValueLeft: %f, nBalanceToDenominate: %f\n",
Expand All @@ -1585,7 +1593,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 < privateSendClient.nPrivateSendDenomsGoal && nValueLeft >= it.first && nBalanceToDenominate > 0) {
if (it.second < privateSendClient.nPrivateSendDenomsGoal && (nValueLeft >= it.first + nOutputFee) && nBalanceToDenominate > 0) {
finished = false;
LogPrint(BCLog::PRIVATESEND,
"CPrivateSendClientSession::CreateDenominated -- 1 - NOT finished - nDenomValue: %f, count: %d, nValueLeft: %f, nBalanceToDenominate: %f\n",
Expand All @@ -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.
if (nValueLeft >= CPrivateSend::GetSmallestDenomination() && nBalanceToDenominate >= CPrivateSend::GetSmallestDenomination()
if ((nValueLeft >= CPrivateSend::GetSmallestDenomination() + nOutputFee) && nBalanceToDenominate >= CPrivateSend::GetSmallestDenomination()
&& nOutputsTotal < PRIVATESEND_DENOM_OUTPUTS_THRESHOLD) {

CAmount nLargestDenomValue = vecStandardDenoms.front();
Expand All @@ -1611,7 +1619,7 @@ bool CPrivateSendClientSession::CreateDenominated(CAmount nBalanceToDenominate,
int nOutputs = 0;

// 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;
// Use the smaller value
int denomsToCreate = denomsToCreateValue > denomsToCreateBal ? denomsToCreateBal : denomsToCreateValue;
Expand All @@ -1626,7 +1634,7 @@ bool CPrivateSendClientSession::CreateDenominated(CAmount nBalanceToDenominate,
// increment outputs and subtract denomination amount
nOutputs++;
it->second++;
nValueLeft -= nDenomValue;
nValueLeft -= nDenomValue + nOutputFee;
nBalanceToDenominate -= nDenomValue;
LogPrint(BCLog::PRIVATESEND,
"CPrivateSendClientSession::CreateDenominated -- 2 - nDenomValue: %f, totalOutputs: %d, nOutputsTotal: %d, nOutputs: %d, nValueLeft: %f, nBalanceToDenominate: %f\n",
Expand All @@ -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

CCoinControl coinControl;
coinControl.fAllowOtherInputs = false;
coinControl.fAllowWatchOnly = false;
coinControl.nCoinType = CoinType::ONLY_NONDENOMINATED;
Expand Down