aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/wallet/wallet.cpp120
1 files changed, 74 insertions, 46 deletions
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index a1b4eb106e..5e9701c71c 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -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);
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-03 07:54:55 +0000