Only reserve key for scriptChange once in CreateTransaction

This does not affect behavior but allows us to have access to an output to
scriptChange even if we currently do not have change in the transaction.
This commit is contained in:
Alex Morcos 2017-06-30 12:53:29 -04:00
parent ca4c545cc7
commit 253cd7ec4f

View file

@ -2569,6 +2569,38 @@ 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());
}
nFeeRet = 0;
// Start with no fee and loop until there is enough fee
while (true)
@ -2627,37 +2659,6 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT
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 +2667,6 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT
{
nChangePosInOut = -1;
nFeeRet += nChange;
reservekey.ReturnKey();
}
else
{
@ -2685,7 +2685,6 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT
txNew.vout.insert(position, newTxOut);
}
} else {
reservekey.ReturnKey();
nChangePosInOut = -1;
}
@ -2777,6 +2776,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);