Remove CreateTransaction while loop and some related variables

Remove the CreateTransaction while loop. Removes variables that were
only needed because of that loop. Also renames a few variables and
moves their declarations to where they are used.

Some subtractFeeFromOutputs handling is moved to after coin selection
in order to reduce their amounts once the fee is known.

If subtracting the fee reduces the change to dust, we will also now
remove the change output
pull/826/head
Andrew Chow 5 years ago
parent 6f0d5189af
commit 9d3bd74ab4

@ -2804,7 +2804,6 @@ bool CWallet::CreateTransactionInternal(
CAmount nValue = 0; CAmount nValue = 0;
const OutputType change_type = TransactionChangeType(coin_control.m_change_type ? *coin_control.m_change_type : m_default_change_type, vecSend); const OutputType change_type = TransactionChangeType(coin_control.m_change_type ? *coin_control.m_change_type : m_default_change_type, vecSend);
ReserveDestination reservedest(this, change_type); ReserveDestination reservedest(this, change_type);
int nChangePosRequest = nChangePosInOut;
unsigned int nSubtractFeeFromAmount = 0; unsigned int nSubtractFeeFromAmount = 0;
for (const auto& recipient : vecSend) for (const auto& recipient : vecSend)
{ {
@ -2909,151 +2908,139 @@ bool CWallet::CreateTransactionInternal(
coin_selection_params.m_change_fee = coin_selection_params.m_effective_feerate.GetFee(coin_selection_params.change_output_size); coin_selection_params.m_change_fee = coin_selection_params.m_effective_feerate.GetFee(coin_selection_params.change_output_size);
coin_selection_params.m_cost_of_change = coin_selection_params.m_discard_feerate.GetFee(coin_selection_params.change_spend_size) + coin_selection_params.m_change_fee; coin_selection_params.m_cost_of_change = coin_selection_params.m_discard_feerate.GetFee(coin_selection_params.change_spend_size) + coin_selection_params.m_change_fee;
nFeeRet = 0;
CAmount nValueIn = 0;
coin_selection_params.m_subtract_fee_outputs = nSubtractFeeFromAmount != 0; // If we are doing subtract fee from recipient, don't use effective values coin_selection_params.m_subtract_fee_outputs = nSubtractFeeFromAmount != 0; // If we are doing subtract fee from recipient, don't use effective values
// Start with no fee and loop until there is enough fee
while (true) // vouts to the payees
if (!coin_selection_params.m_subtract_fee_outputs) {
coin_selection_params.tx_noinputs_size = 11; // Static vsize overhead + outputs vsize. 4 nVersion, 4 nLocktime, 1 input count, 1 output count, 1 witness overhead (dummy, flag, stack size)
}
for (const auto& recipient : vecSend)
{ {
nChangePosInOut = nChangePosRequest; CTxOut txout(recipient.nAmount, recipient.scriptPubKey);
txNew.vin.clear();
txNew.vout.clear();
// vouts to the payees // Include the fee cost for outputs.
if (!coin_selection_params.m_subtract_fee_outputs) { if (!coin_selection_params.m_subtract_fee_outputs) {
coin_selection_params.tx_noinputs_size = 11; // Static vsize overhead + outputs vsize. 4 nVersion, 4 nLocktime, 1 input count, 1 output count, 1 witness overhead (dummy, flag, stack size) coin_selection_params.tx_noinputs_size += ::GetSerializeSize(txout, PROTOCOL_VERSION);
} }
for (const auto& recipient : vecSend)
if (IsDust(txout, chain().relayDustFee()))
{ {
CTxOut txout(recipient.nAmount, recipient.scriptPubKey); error = _("Transaction amount too small");
return false;
}
txNew.vout.push_back(txout);
}
// Include the fee cost for outputs. // Include the fees for things that aren't inputs, excluding the change output
if (!coin_selection_params.m_subtract_fee_outputs) { const CAmount not_input_fees = coin_selection_params.m_effective_feerate.GetFee(coin_selection_params.tx_noinputs_size);
coin_selection_params.tx_noinputs_size += ::GetSerializeSize(txout, PROTOCOL_VERSION); CAmount nValueToSelect = nValue + not_input_fees;
}
if (IsDust(txout, chain().relayDustFee())) // Choose coins to use
{ CAmount inputs_sum = 0;
error = _("Transaction amount too small"); setCoins.clear();
return false; if (!SelectCoins(vAvailableCoins, /* nTargetValue */ nValueToSelect, setCoins, inputs_sum, coin_control, coin_selection_params))
} {
txNew.vout.push_back(txout); error = _("Insufficient funds");
} return false;
}
// Include the fees for things that aren't inputs, excluding the change output // Always make a change output
const CAmount not_input_fees = coin_selection_params.m_effective_feerate.GetFee(coin_selection_params.tx_noinputs_size); // We will reduce the fee from this change output later, and remove the output if it is too small.
CAmount nValueToSelect = nValue + not_input_fees; const CAmount change_and_fee = inputs_sum - nValue;
assert(change_and_fee >= 0);
CTxOut newTxOut(change_and_fee, scriptChange);
// Choose coins to use if (nChangePosInOut == -1)
nValueIn = 0; {
setCoins.clear(); // Insert change txn at random position:
if (!SelectCoins(vAvailableCoins, /* nTargetValue */ nValueToSelect, setCoins, nValueIn, coin_control, coin_selection_params)) nChangePosInOut = GetRandInt(txNew.vout.size()+1);
{ }
error = _("Insufficient funds"); else if ((unsigned int)nChangePosInOut > txNew.vout.size())
return false; {
} error = _("Change index out of range");
return false;
}
// Always make a change output assert(nChangePosInOut != -1);
// We will reduce the fee from this change output later, and remove the output if it is too small. auto change_position = txNew.vout.insert(txNew.vout.begin() + nChangePosInOut, newTxOut);
const CAmount change_and_fee = nValueIn - nValue;
assert(change_and_fee >= 0);
CTxOut newTxOut(change_and_fee, scriptChange);
if (nChangePosInOut == -1) // Dummy fill vin for maximum size estimation
{ //
// Insert change txn at random position: for (const auto& coin : setCoins) {
nChangePosInOut = GetRandInt(txNew.vout.size()+1); txNew.vin.push_back(CTxIn(coin.outpoint,CScript()));
} }
else if ((unsigned int)nChangePosInOut > txNew.vout.size())
{
error = _("Change index out of range");
return false;
}
assert(nChangePosInOut != -1); // Calculate the transaction fee
auto change_position = txNew.vout.insert(txNew.vout.begin() + nChangePosInOut, newTxOut); tx_sizes = CalculateMaximumSignedTxSize(CTransaction(txNew), this, coin_control.fAllowWatchOnly);
nBytes = tx_sizes.first;
if (nBytes < 0) {
error = _("Signing transaction failed");
return false;
}
nFeeRet = coin_selection_params.m_effective_feerate.GetFee(nBytes);
// Dummy fill vin for maximum size estimation // Subtract fee from the change output if not subtrating it from recipient outputs
// CAmount fee_needed = nFeeRet;
for (const auto& coin : setCoins) { if (nSubtractFeeFromAmount == 0) {
txNew.vin.push_back(CTxIn(coin.outpoint,CScript())); change_position->nValue -= fee_needed;
} }
// Calculate the transaction fee // We want to drop the change to fees if:
// 1. The change output would be dust
// 2. The change is within the (almost) exact match window, i.e. it is less than or equal to the cost of the change output (cost_of_change)
CAmount change_amount = change_position->nValue;
if (IsDust(*change_position, coin_selection_params.m_discard_feerate) || change_amount <= coin_selection_params.m_cost_of_change)
{
nChangePosInOut = -1;
change_amount = 0;
txNew.vout.erase(change_position);
// Because we have dropped this change, the tx size and required fee will be different, so let's recalculate those
tx_sizes = CalculateMaximumSignedTxSize(CTransaction(txNew), this, coin_control.fAllowWatchOnly); tx_sizes = CalculateMaximumSignedTxSize(CTransaction(txNew), this, coin_control.fAllowWatchOnly);
nBytes = tx_sizes.first; nBytes = tx_sizes.first;
if (nBytes < 0) { fee_needed = coin_selection_params.m_effective_feerate.GetFee(nBytes);
error = _("Signing transaction failed"); }
return false;
}
nFeeRet = coin_selection_params.m_effective_feerate.GetFee(nBytes);
// Subtract fee from the change output if not subtrating it from recipient outputs // Update nFeeRet in case fee_needed changed due to dropping the change output
CAmount fee_needed = nFeeRet; if (fee_needed <= change_and_fee - change_amount) {
if (nSubtractFeeFromAmount == 0) { nFeeRet = change_and_fee - change_amount;
change_position->nValue -= fee_needed; }
}
// We want to drop the change to fees if: // Reduce output values for subtractFeeFromAmount
// 1. The change output would be dust if (nSubtractFeeFromAmount != 0) {
// 2. The change is within the (almost) exact match window, i.e. it is less than or equal to the cost of the change output (cost_of_change) CAmount to_reduce = fee_needed + change_amount - change_and_fee;
CAmount change_amount = change_position->nValue; int i = 0;
if (IsDust(*change_position, coin_selection_params.m_discard_feerate) || change_amount <= coin_selection_params.m_cost_of_change) bool fFirst = true;
for (const auto& recipient : vecSend)
{ {
nChangePosInOut = -1; if (i == nChangePosInOut) {
change_amount = 0; ++i;
txNew.vout.erase(change_position); }
CTxOut& txout = txNew.vout[i];
// Because we have dropped this change, the tx size and required fee will be different, so let's recalculate those
tx_sizes = CalculateMaximumSignedTxSize(CTransaction(txNew), this, coin_control.fAllowWatchOnly);
nBytes = tx_sizes.first;
fee_needed = coin_selection_params.m_effective_feerate.GetFee(nBytes);
}
// If the fee is covered, there's no need to loop or subtract from recipients
if (fee_needed <= change_and_fee - change_amount) {
nFeeRet = change_and_fee - change_amount;
break;
}
// Reduce output values for subtractFeeFromAmount if (recipient.fSubtractFeeFromAmount)
if (nSubtractFeeFromAmount != 0) {
CAmount to_reduce = fee_needed + change_amount - change_and_fee;
int i = 0;
bool fFirst = true;
for (const auto& recipient : vecSend)
{ {
if (i == nChangePosInOut) { txout.nValue -= to_reduce / nSubtractFeeFromAmount; // Subtract fee equally from each selected recipient
++i;
}
CTxOut& txout = txNew.vout[i];
if (recipient.fSubtractFeeFromAmount) if (fFirst) // first receiver pays the remainder not divisible by output count
{ {
txout.nValue -= to_reduce / nSubtractFeeFromAmount; // Subtract fee equally from each selected recipient fFirst = false;
txout.nValue -= to_reduce % nSubtractFeeFromAmount;
if (fFirst) // first receiver pays the remainder not divisible by output count }
{
fFirst = false;
txout.nValue -= to_reduce % nSubtractFeeFromAmount;
}
// Error if this output is reduced to be below dust // Error if this output is reduced to be below dust
if (IsDust(txout, chain().relayDustFee())) { if (IsDust(txout, chain().relayDustFee())) {
if (txout.nValue < 0) { if (txout.nValue < 0) {
error = _("The transaction amount is too small to pay the fee"); error = _("The transaction amount is too small to pay the fee");
} else { } else {
error = _("The transaction amount is too small to send after the fee has been deducted"); error = _("The transaction amount is too small to send after the fee has been deducted");
}
return false;
} }
return false;
} }
++i;
} }
nFeeRet = fee_needed; ++i;
break; // The fee has been deducted from the recipients, nothing left to do here
} }
nFeeRet = fee_needed;
} }
// Give up if change keypool ran out and change is required // Give up if change keypool ran out and change is required

Loading…
Cancel
Save