|
|
@@ -2569,7 +2569,43 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT |
|
|
std::vector<COutput> vAvailableCoins;
|
|
|
AvailableCoins(vAvailableCoins, true, coinControl);
|
|
|
|
|
|
+ // Create change script that will be used if we need change
|
|
|
+ // TODO: pass in scriptChange instead of reservekey so
|
|
|
+ // change transaction isn't always pay-to-bitcoin-address
|
|
|
+ CScript scriptChange;
|
|
|
+
|
|
|
+ // coin control: send change to custom address
|
|
|
+ if (coinControl && !boost::get<CNoDestination>(&coinControl->destChange))
|
|
|
+ scriptChange = GetScriptForDestination(coinControl->destChange);
|
|
|
+
|
|
|
+ // no coin control: send change to newly generated address
|
|
|
+ else
|
|
|
+ {
|
|
|
+ // Note: We use a new key here to keep it from being obvious which side is the change.
|
|
|
+ // The drawback is that by not reusing a previous key, the change may be lost if a
|
|
|
+ // backup is restored, if the backup doesn't have the new private key for the change.
|
|
|
+ // If we reused the old key, it would be possible to add code to look for and
|
|
|
+ // rediscover unknown transactions that were written with keys of ours to recover
|
|
|
+ // post-backup change.
|
|
|
+
|
|
|
+ // Reserve a new key pair from key pool
|
|
|
+ CPubKey vchPubKey;
|
|
|
+ bool ret;
|
|
|
+ ret = reservekey.GetReservedKey(vchPubKey, true);
|
|
|
+ if (!ret)
|
|
|
+ {
|
|
|
+ strFailReason = _("Keypool ran out, please call keypoolrefill first");
|
|
|
+ return false;
|
|
|
+ }
|
|
|
+
|
|
|
+ scriptChange = GetScriptForDestination(vchPubKey.GetID());
|
|
|
+ }
|
|
|
+ CTxOut change_prototype_txout(0, scriptChange);
|
|
|
+ size_t change_prototype_size = GetSerializeSize(change_prototype_txout, SER_DISK, 0);
|
|
|
+
|
|
|
nFeeRet = 0;
|
|
|
+ bool pick_new_inputs = true;
|
|
|
+ CAmount nValueIn = 0;
|
|
|
// Start with no fee and loop until there is enough fee
|
|
|
while (true)
|
|
|
{
|
|
|
@@ -2615,49 +2651,21 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT |
|
|
}
|
|
|
|
|
|
// Choose coins to use
|
|
|
- CAmount nValueIn = 0;
|
|
|
- setCoins.clear();
|
|
|
- if (!SelectCoins(vAvailableCoins, nValueToSelect, setCoins, nValueIn, coinControl))
|
|
|
- {
|
|
|
- strFailReason = _("Insufficient funds");
|
|
|
- return false;
|
|
|
+ if (pick_new_inputs) {
|
|
|
+ nValueIn = 0;
|
|
|
+ setCoins.clear();
|
|
|
+ if (!SelectCoins(vAvailableCoins, nValueToSelect, setCoins, nValueIn, coinControl))
|
|
|
+ {
|
|
|
+ strFailReason = _("Insufficient funds");
|
|
|
+ return false;
|
|
|
+ }
|
|
|
}
|
|
|
|
|
|
const CAmount nChange = nValueIn - nValueToSelect;
|
|
|
+
|
|
|
if (nChange > 0)
|
|
|
{
|
|
|
// Fill a vout to ourself
|
|
|
- // TODO: pass in scriptChange instead of reservekey so
|
|
|
- // change transaction isn't always pay-to-bitcoin-address
|
|
|
- CScript scriptChange;
|
|
|
-
|
|
|
- // coin control: send change to custom address
|
|
|
- if (coinControl && !boost::get<CNoDestination>(&coinControl->destChange))
|
|
|
- scriptChange = GetScriptForDestination(coinControl->destChange);
|
|
|
-
|
|
|
- // no coin control: send change to newly generated address
|
|
|
- else
|
|
|
- {
|
|
|
- // Note: We use a new key here to keep it from being obvious which side is the change.
|
|
|
- // The drawback is that by not reusing a previous key, the change may be lost if a
|
|
|
- // backup is restored, if the backup doesn't have the new private key for the change.
|
|
|
- // If we reused the old key, it would be possible to add code to look for and
|
|
|
- // rediscover unknown transactions that were written with keys of ours to recover
|
|
|
- // post-backup change.
|
|
|
-
|
|
|
- // Reserve a new key pair from key pool
|
|
|
- CPubKey vchPubKey;
|
|
|
- bool ret;
|
|
|
- ret = reservekey.GetReservedKey(vchPubKey, true);
|
|
|
- if (!ret)
|
|
|
- {
|
|
|
- strFailReason = _("Keypool ran out, please call keypoolrefill first");
|
|
|
- return false;
|
|
|
- }
|
|
|
-
|
|
|
- scriptChange = GetScriptForDestination(vchPubKey.GetID());
|
|
|
- }
|
|
|
-
|
|
|
CTxOut newTxOut(nChange, scriptChange);
|
|
|
|
|
|
// Never create dust outputs; if we would, just
|
|
|
@@ -2666,7 +2674,6 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT |
|
|
{
|
|
|
nChangePosInOut = -1;
|
|
|
nFeeRet += nChange;
|
|
|
- reservekey.ReturnKey();
|
|
|
}
|
|
|
else
|
|
|
{
|
|
|
@@ -2685,7 +2692,6 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT |
|
|
txNew.vout.insert(position, newTxOut);
|
|
|
}
|
|
|
} else {
|
|
|
- reservekey.ReturnKey();
|
|
|
nChangePosInOut = -1;
|
|
|
}
|
|
|
|
|
|
@@ -2740,16 +2746,30 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT |
|
|
}
|
|
|
|
|
|
if (nFeeRet >= nFeeNeeded) {
|
|
|
- // Reduce fee to only the needed amount if we have change
|
|
|
- // output to increase. This prevents potential overpayment
|
|
|
- // in fees if the coins selected to meet nFeeNeeded result
|
|
|
- // in a transaction that requires less fee than the prior
|
|
|
- // iteration.
|
|
|
+ // Reduce fee to only the needed amount if possible. This
|
|
|
+ // prevents potential overpayment in fees if the coins
|
|
|
+ // selected to meet nFeeNeeded result in a transaction that
|
|
|
+ // requires less fee than the prior iteration.
|
|
|
+
|
|
|
// TODO: The case where nSubtractFeeFromAmount > 0 remains
|
|
|
// to be addressed because it requires returning the fee to
|
|
|
// the payees and not the change output.
|
|
|
- // TODO: The case where there is no change output remains
|
|
|
- // to be addressed so we avoid creating too small an output.
|
|
|
+
|
|
|
+ // If we have no change and a big enough excess fee, then
|
|
|
+ // try to construct transaction again only without picking
|
|
|
+ // new inputs. We now know we only need the smaller fee
|
|
|
+ // (because of reduced tx size) and so we should add a
|
|
|
+ // change output. Only try this once.
|
|
|
+ CAmount fee_needed_for_change = GetMinimumFee(change_prototype_size, currentConfirmationTarget, ::mempool, ::feeEstimator, nullptr, false /* ignoreGlobalPayTxFee */, conservative_estimate);
|
|
|
+ CAmount minimum_value_for_change = GetDustThreshold(change_prototype_txout, ::dustRelayFee);
|
|
|
+ CAmount max_excess_fee = fee_needed_for_change + minimum_value_for_change;
|
|
|
+ if (nFeeRet > nFeeNeeded + max_excess_fee && nChangePosInOut == -1 && nSubtractFeeFromAmount == 0 && pick_new_inputs) {
|
|
|
+ pick_new_inputs = false;
|
|
|
+ nFeeRet = nFeeNeeded + fee_needed_for_change;
|
|
|
+ continue;
|
|
|
+ }
|
|
|
+
|
|
|
+ // If we have change output already, just increase it
|
|
|
if (nFeeRet > nFeeNeeded && nChangePosInOut != -1 && nSubtractFeeFromAmount == 0) {
|
|
|
CAmount extraFeePaid = nFeeRet - nFeeNeeded;
|
|
|
std::vector<CTxOut>::iterator change_position = txNew.vout.begin()+nChangePosInOut;
|
|
|
@@ -2758,6 +2778,12 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT |
|
|
}
|
|
|
break; // Done, enough fee included.
|
|
|
}
|
|
|
+ else if (!pick_new_inputs) {
|
|
|
+ // This shouldn't happen, we should have had enough excess
|
|
|
+ // fee to pay for the new output and still meet nFeeNeeded
|
|
|
+ strFailReason = _("Transaction fee and change calculation failed");
|
|
|
+ return false;
|
|
|
+ }
|
|
|
|
|
|
// Try to reduce change to include necessary fee
|
|
|
if (nChangePosInOut != -1 && nSubtractFeeFromAmount == 0) {
|
|
|
@@ -2777,6 +2803,8 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT |
|
|
}
|
|
|
}
|
|
|
|
|
|
+ if (nChangePosInOut == -1) reservekey.ReturnKey(); // Return any reserved key if we don't have change
|
|
|
+
|
|
|
if (sign)
|
|
|
{
|
|
|
CTransaction txNewConst(txNew);
|
|
|
|
Nit: else on the same line as
}