LBRYCommunity/lbrycrd
1
0
Fork 0
Code Issues 29 Pull requests 1 Projects Releases 32 Packages Wiki Activity

Merge #11145: Fix rounding bug in calculation of minimum change

Browse source 
6af49dd Output a bit more information for fee calculation report. (Alex Morcos)
a54c7b9 Fix rounding errors in calculation of minimum change size (Alex Morcos)

Pull request description:

  Thanks to @juscamarena for reporting this.

  Please backport to 0.15.

  There was a potential rounding error where the fee for the change added to the fee for the original tx could be less than the fee for the tx including change.

  This is fixed in the first commit.  The second commit adds one more snippet of information in the fee calculation report.  I actually realized that there is more information that would be nice to report, but we can add that post 0.15.

  An open question is whether we should be returning failure if the test in line 2885 is hit or just resetting pick_new_inputs and continuing.  Originally I made it a failure to avoid any possible infinite loops.  But the case hit here is an example of where that logic possibly backfired.

Tree-SHA512: efe049781acc1f6a8ad429a689359ac6f7b7c44cdfc9578a866dff4a2f6596e8de474a89d25c704f31ef4f8c89af770e98b75ef06c25419d5a6dfc87247bf274
This commit is contained in:
Wladimir J. van der Laan 2017-08-28 10:05:20 +02:00
commit 9c833f471c
No known key found for this signature in database
GPG key ID: 1E4AED62986CD25D
1 changed files with 13 additions and 10 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

17
src/wallet/wallet.cpp
View file

@ -2612,6 +2612,7 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT
assert(txNew.nLockTime <= (unsigned int)chainActive.Height()); assert(txNew.nLockTime <= (unsigned int)chainActive.Height());
assert(txNew.nLockTime < LOCKTIME_THRESHOLD); assert(txNew.nLockTime < LOCKTIME_THRESHOLD);
FeeCalculation feeCalc; FeeCalculation feeCalc;
CAmount nFeeNeeded;
unsigned int nBytes; unsigned int nBytes;
{ {
std::set<CInputCoin> setCoins; std::set<CInputCoin> setCoins;
@ -2773,7 +2774,7 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT
vin.scriptWitness.SetNull(); vin.scriptWitness.SetNull();
} }
CAmount nFeeNeeded = GetMinimumFee(nBytes, coin_control, ::mempool, ::feeEstimator, &feeCalc); nFeeNeeded = GetMinimumFee(nBytes, coin_control, ::mempool, ::feeEstimator, &feeCalc);
// If we made it here and we aren't even able to meet the relay fee on the next pass, give up // If we made it here and we aren't even able to meet the relay fee on the next pass, give up
// because we must be at the maximum allowed fee. // because we must be at the maximum allowed fee.
@ -2794,14 +2795,16 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT
// new inputs. We now know we only need the smaller fee // new inputs. We now know we only need the smaller fee
// (because of reduced tx size) and so we should add a // (because of reduced tx size) and so we should add a
// change output. Only try this once. // change output. Only try this once.
CAmount fee_needed_for_change = GetMinimumFee(change_prototype_size, coin_control, ::mempool, ::feeEstimator, nullptr); if (nChangePosInOut == -1 && nSubtractFeeFromAmount == 0 && pick_new_inputs) {
unsigned int tx_size_with_change = nBytes + change_prototype_size + 2; // Add 2 as a buffer in case increasing # of outputs changes compact size
CAmount fee_needed_with_change = GetMinimumFee(tx_size_with_change, coin_control, ::mempool, ::feeEstimator, nullptr);
CAmount minimum_value_for_change = GetDustThreshold(change_prototype_txout, discard_rate); CAmount minimum_value_for_change = GetDustThreshold(change_prototype_txout, discard_rate);
CAmount max_excess_fee = fee_needed_for_change + minimum_value_for_change; if (nFeeRet >= fee_needed_with_change + minimum_value_for_change) {
if (nFeeRet > nFeeNeeded + max_excess_fee && nChangePosInOut == -1 && nSubtractFeeFromAmount == 0 && pick_new_inputs) {
pick_new_inputs = false; pick_new_inputs = false;
nFeeRet = nFeeNeeded + fee_needed_for_change; nFeeRet = fee_needed_with_change;
continue; continue;
} }
}
// If we have change output already, just increase it // If we have change output already, just increase it
if (nFeeRet > nFeeNeeded && nChangePosInOut != -1 && nSubtractFeeFromAmount == 0) { if (nFeeRet > nFeeNeeded && nChangePosInOut != -1 && nSubtractFeeFromAmount == 0) {
@ -2895,8 +2898,8 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT
} }
} }
LogPrintf("Fee Calculation: Fee:%d Bytes:%u Tgt:%d (requested %d) Reason:\"%s\" Decay %.5f: Estimation: (%g - %g) %.2f%% %.1f/(%.1f %d mem %.1f out) Fail: (%g - %g) %.2f%% %.1f/(%.1f %d mem %.1f out)\n", LogPrintf("Fee Calculation: Fee:%d Bytes:%u Needed:%d Tgt:%d (requested %d) Reason:\"%s\" Decay %.5f: Estimation: (%g - %g) %.2f%% %.1f/(%.1f %d mem %.1f out) Fail: (%g - %g) %.2f%% %.1f/(%.1f %d mem %.1f out)\n",
nFeeRet, nBytes, feeCalc.returnedTarget, feeCalc.desiredTarget, StringForFeeReason(feeCalc.reason), feeCalc.est.decay, nFeeRet, nBytes, nFeeNeeded, feeCalc.returnedTarget, feeCalc.desiredTarget, StringForFeeReason(feeCalc.reason), feeCalc.est.decay,
feeCalc.est.pass.start, feeCalc.est.pass.end, feeCalc.est.pass.start, feeCalc.est.pass.end,
100 * feeCalc.est.pass.withinTarget / (feeCalc.est.pass.totalConfirmed + feeCalc.est.pass.inMempool + feeCalc.est.pass.leftMempool), 100 * feeCalc.est.pass.withinTarget / (feeCalc.est.pass.totalConfirmed + feeCalc.est.pass.inMempool + feeCalc.est.pass.leftMempool),
feeCalc.est.pass.withinTarget, feeCalc.est.pass.totalConfirmed, feeCalc.est.pass.inMempool, feeCalc.est.pass.leftMempool, feeCalc.est.pass.withinTarget, feeCalc.est.pass.totalConfirmed, feeCalc.est.pass.inMempool, feeCalc.est.pass.leftMempool,