Make CoinControl a required argument to CreateTransaction

This commit is contained in:
Alex Morcos 2017-06-28 16:41:55 -04:00
parent 8fdd23a224
commit ecd81dfa3c
7 changed files with 27 additions and 28 deletions

View File

@ -281,7 +281,7 @@ void SendCoinsDialog::on_sendButton_clicked()
ctrl.signalRbf = ui->optInRBF->isChecked(); ctrl.signalRbf = ui->optInRBF->isChecked();
prepareStatus = model->prepareTransaction(currentTransaction, &ctrl); prepareStatus = model->prepareTransaction(currentTransaction, ctrl);
// process prepareStatus and on error generate message shown to user // process prepareStatus and on error generate message shown to user
processSendCoinsReturn(prepareStatus, processSendCoinsReturn(prepareStatus,

View File

@ -191,7 +191,7 @@ bool WalletModel::validateAddress(const QString &address)
return addressParsed.IsValid(); return addressParsed.IsValid();
} }
WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransaction &transaction, const CCoinControl *coinControl) WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransaction &transaction, const CCoinControl& coinControl)
{ {
CAmount total = 0; CAmount total = 0;
bool fSubtractFeeFromAmount = false; bool fSubtractFeeFromAmount = false;
@ -258,7 +258,7 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact
return DuplicateAddress; return DuplicateAddress;
} }
CAmount nBalance = getBalance(coinControl); CAmount nBalance = getBalance(&coinControl);
if(total > nBalance) if(total > nBalance)
{ {

View File

@ -154,7 +154,7 @@ public:
}; };
// prepare transaction for getting txfee before sending coins // prepare transaction for getting txfee before sending coins
SendCoinsReturn prepareTransaction(WalletModelTransaction &transaction, const CCoinControl *coinControl = NULL); SendCoinsReturn prepareTransaction(WalletModelTransaction &transaction, const CCoinControl& coinControl);
// Send coins to a list of recipients // Send coins to a list of recipients
SendCoinsReturn sendCoins(WalletModelTransaction &transaction); SendCoinsReturn sendCoins(WalletModelTransaction &transaction);

View File

@ -356,7 +356,7 @@ UniValue getaddressesbyaccount(const JSONRPCRequest& request)
return ret; return ret;
} }
static void SendMoney(CWallet * const pwallet, const CTxDestination &address, CAmount nValue, bool fSubtractFeeFromAmount, CWalletTx& wtxNew, CCoinControl *coin_control = nullptr) static void SendMoney(CWallet * const pwallet, const CTxDestination &address, CAmount nValue, bool fSubtractFeeFromAmount, CWalletTx& wtxNew, const CCoinControl& coin_control)
{ {
CAmount curBalance = pwallet->GetBalance(); CAmount curBalance = pwallet->GetBalance();
@ -472,7 +472,7 @@ UniValue sendtoaddress(const JSONRPCRequest& request)
EnsureWalletIsUnlocked(pwallet); EnsureWalletIsUnlocked(pwallet);
SendMoney(pwallet, address.Get(), nAmount, fSubtractFeeFromAmount, wtx, &coin_control); SendMoney(pwallet, address.Get(), nAmount, fSubtractFeeFromAmount, wtx, coin_control);
return wtx.GetHash().GetHex(); return wtx.GetHash().GetHex();
} }
@ -898,7 +898,8 @@ UniValue sendfrom(const JSONRPCRequest& request)
if (nAmount > nBalance) if (nAmount > nBalance)
throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, "Account has insufficient funds"); throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, "Account has insufficient funds");
SendMoney(pwallet, address.Get(), nAmount, false, wtx); CCoinControl no_coin_control; // This is a deprecated API
SendMoney(pwallet, address.Get(), nAmount, false, wtx, no_coin_control);
return wtx.GetHash().GetHex(); return wtx.GetHash().GetHex();
} }
@ -1033,7 +1034,7 @@ UniValue sendmany(const JSONRPCRequest& request)
CAmount nFeeRequired = 0; CAmount nFeeRequired = 0;
int nChangePosRet = -1; int nChangePosRet = -1;
std::string strFailReason; std::string strFailReason;
bool fCreated = pwallet->CreateTransaction(vecSend, wtx, keyChange, nFeeRequired, nChangePosRet, strFailReason, &coin_control); bool fCreated = pwallet->CreateTransaction(vecSend, wtx, keyChange, nFeeRequired, nChangePosRet, strFailReason, coin_control);
if (!fCreated) if (!fCreated)
throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, strFailReason); throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, strFailReason);
CValidationState state; CValidationState state;

View File

@ -13,6 +13,7 @@
#include "rpc/server.h" #include "rpc/server.h"
#include "test/test_bitcoin.h" #include "test/test_bitcoin.h"
#include "validation.h" #include "validation.h"
#include "wallet/coincontrol.h"
#include "wallet/test/wallet_test_fixture.h" #include "wallet/test/wallet_test_fixture.h"
#include <boost/test/unit_test.hpp> #include <boost/test/unit_test.hpp>
@ -617,7 +618,8 @@ public:
CAmount fee; CAmount fee;
int changePos = -1; int changePos = -1;
std::string error; std::string error;
BOOST_CHECK(wallet->CreateTransaction({recipient}, wtx, reservekey, fee, changePos, error)); CCoinControl dummy;
BOOST_CHECK(wallet->CreateTransaction({recipient}, wtx, reservekey, fee, changePos, error, dummy));
CValidationState state; CValidationState state;
BOOST_CHECK(wallet->CommitTransaction(wtx, reservekey, nullptr, state)); BOOST_CHECK(wallet->CommitTransaction(wtx, reservekey, nullptr, state));
auto it = wallet->mapWallet.find(wtx.GetHash()); auto it = wallet->mapWallet.find(wtx.GetHash());

View File

@ -2469,9 +2469,9 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC
CReserveKey reservekey(this); CReserveKey reservekey(this);
CWalletTx wtx; CWalletTx wtx;
if (!CreateTransaction(vecSend, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, false)) if (!CreateTransaction(vecSend, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, coinControl, false)) {
return false; return false;
}
if (nChangePosInOut != -1) if (nChangePosInOut != -1)
tx.vout.insert(tx.vout.begin() + nChangePosInOut, wtx.tx->vout[nChangePosInOut]); tx.vout.insert(tx.vout.begin() + nChangePosInOut, wtx.tx->vout[nChangePosInOut]);
@ -2502,7 +2502,7 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC
} }
bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletTx& wtxNew, CReserveKey& reservekey, CAmount& nFeeRet, bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletTx& wtxNew, CReserveKey& reservekey, CAmount& nFeeRet,
int& nChangePosInOut, std::string& strFailReason, const CCoinControl* coinControl, bool sign) int& nChangePosInOut, std::string& strFailReason, const CCoinControl& coin_control, bool sign)
{ {
CAmount nValue = 0; CAmount nValue = 0;
int nChangePosRequest = nChangePosInOut; int nChangePosRequest = nChangePosInOut;
@ -2567,7 +2567,7 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT
LOCK2(cs_main, cs_wallet); LOCK2(cs_main, cs_wallet);
{ {
std::vector<COutput> vAvailableCoins; std::vector<COutput> vAvailableCoins;
AvailableCoins(vAvailableCoins, true, coinControl); AvailableCoins(vAvailableCoins, true, &coin_control);
// Create change script that will be used if we need change // Create change script that will be used if we need change
// TODO: pass in scriptChange instead of reservekey so // TODO: pass in scriptChange instead of reservekey so
@ -2575,12 +2575,9 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT
CScript scriptChange; CScript scriptChange;
// coin control: send change to custom address // coin control: send change to custom address
if (coinControl && !boost::get<CNoDestination>(&coinControl->destChange)) if (!boost::get<CNoDestination>(&coin_control.destChange)) {
scriptChange = GetScriptForDestination(coinControl->destChange); scriptChange = GetScriptForDestination(coin_control.destChange);
} else { // no coin control: send change to newly generated address
// 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. // 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 // 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. // backup is restored, if the backup doesn't have the new private key for the change.
@ -2654,7 +2651,7 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT
if (pick_new_inputs) { if (pick_new_inputs) {
nValueIn = 0; nValueIn = 0;
setCoins.clear(); setCoins.clear();
if (!SelectCoins(vAvailableCoins, nValueToSelect, setCoins, nValueIn, coinControl)) if (!SelectCoins(vAvailableCoins, nValueToSelect, setCoins, nValueIn, &coin_control))
{ {
strFailReason = _("Insufficient funds"); strFailReason = _("Insufficient funds");
return false; return false;
@ -2705,8 +2702,7 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT
// to avoid conflicting with other possible uses of nSequence, // to avoid conflicting with other possible uses of nSequence,
// and in the spirit of "smallest possible change from prior // and in the spirit of "smallest possible change from prior
// behavior." // behavior."
bool rbf = coinControl ? coinControl->signalRbf : fWalletRbf; const uint32_t nSequence = coin_control.signalRbf ? MAX_BIP125_RBF_SEQUENCE : (std::numeric_limits<unsigned int>::max() - 1);
const uint32_t nSequence = rbf ? MAX_BIP125_RBF_SEQUENCE : (std::numeric_limits<unsigned int>::max() - 1);
for (const auto& coin : setCoins) for (const auto& coin : setCoins)
txNew.vin.push_back(CTxIn(coin.outpoint,CScript(), txNew.vin.push_back(CTxIn(coin.outpoint,CScript(),
nSequence)); nSequence));
@ -2727,15 +2723,15 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT
// Allow to override the default confirmation target over the CoinControl instance // Allow to override the default confirmation target over the CoinControl instance
int currentConfirmationTarget = nTxConfirmTarget; int currentConfirmationTarget = nTxConfirmTarget;
if (coinControl && coinControl->nConfirmTarget > 0) if (coin_control.nConfirmTarget > 0)
currentConfirmationTarget = coinControl->nConfirmTarget; currentConfirmationTarget = coin_control.nConfirmTarget;
// Allow to override the default fee estimate mode over the CoinControl instance // Allow to override the default fee estimate mode over the CoinControl instance
bool conservative_estimate = CalculateEstimateType(coinControl ? coinControl->m_fee_mode : FeeEstimateMode::UNSET, rbf); bool conservative_estimate = CalculateEstimateType(coin_control.m_fee_mode, coin_control.signalRbf);
CAmount nFeeNeeded = GetMinimumFee(nBytes, currentConfirmationTarget, ::mempool, ::feeEstimator, &feeCalc, false /* ignoreGlobalPayTxFee */, conservative_estimate); CAmount nFeeNeeded = GetMinimumFee(nBytes, currentConfirmationTarget, ::mempool, ::feeEstimator, &feeCalc, false /* ignoreGlobalPayTxFee */, conservative_estimate);
if (coinControl && coinControl->fOverrideFeeRate) if (coin_control.fOverrideFeeRate)
nFeeNeeded = coinControl->nFeeRate.GetFee(nBytes); nFeeNeeded = coin_control.nFeeRate.GetFee(nBytes);
// 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.

View File

@ -949,7 +949,7 @@ public:
* @note passing nChangePosInOut as -1 will result in setting a random position * @note passing nChangePosInOut as -1 will result in setting a random position
*/ */
bool CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletTx& wtxNew, CReserveKey& reservekey, CAmount& nFeeRet, int& nChangePosInOut, bool CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletTx& wtxNew, CReserveKey& reservekey, CAmount& nFeeRet, int& nChangePosInOut,
std::string& strFailReason, const CCoinControl *coinControl = NULL, bool sign = true); std::string& strFailReason, const CCoinControl& coin_control, bool sign = true);
bool CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey, CConnman* connman, CValidationState& state); bool CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey, CConnman* connman, CValidationState& state);
void ListAccountCreditDebit(const std::string& strAccount, std::list<CAccountingEntry>& entries); void ListAccountCreditDebit(const std::string& strAccount, std::list<CAccountingEntry>& entries);