aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--src/interfaces/wallet.cpp7
-rw-r--r--src/wallet/feebumper.cpp9
-rw-r--r--src/wallet/rpcwallet.cpp10
-rw-r--r--src/wallet/test/wallet_tests.cpp5
-rw-r--r--src/wallet/wallet.cpp19
-rw-r--r--src/wallet/wallet.h8
6 files changed, 23 insertions, 35 deletions
diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp
index 8982c70fd2..deb1618ceb 100644
--- a/src/interfaces/wallet.cpp
+++ b/src/interfaces/wallet.cpp
@@ -36,7 +36,7 @@ namespace {
class PendingWalletTxImpl : public PendingWalletTx
{
public:
- explicit PendingWalletTxImpl(CWallet& wallet) : m_wallet(wallet), m_dest(&wallet) {}
+ explicit PendingWalletTxImpl(CWallet& wallet) : m_wallet(wallet) {}
const CTransaction& get() override { return *m_tx; }
@@ -47,7 +47,7 @@ public:
auto locked_chain = m_wallet.chain().lock();
LOCK(m_wallet.cs_wallet);
CValidationState state;
- if (!m_wallet.CommitTransaction(m_tx, std::move(value_map), std::move(order_form), m_dest, state)) {
+ if (!m_wallet.CommitTransaction(m_tx, std::move(value_map), std::move(order_form), state)) {
reject_reason = state.GetRejectReason();
return false;
}
@@ -56,7 +56,6 @@ public:
CTransactionRef m_tx;
CWallet& m_wallet;
- ReserveDestination m_dest;
};
//! Construct wallet tx struct.
@@ -238,7 +237,7 @@ public:
auto locked_chain = m_wallet->chain().lock();
LOCK(m_wallet->cs_wallet);
auto pending = MakeUnique<PendingWalletTxImpl>(*m_wallet);
- if (!m_wallet->CreateTransaction(*locked_chain, recipients, pending->m_tx, pending->m_dest, fee, change_pos,
+ if (!m_wallet->CreateTransaction(*locked_chain, recipients, pending->m_tx, fee, change_pos,
fail_reason, coin_control, sign)) {
return {};
}
diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp
index 0d07ae860c..619197a57a 100644
--- a/src/wallet/feebumper.cpp
+++ b/src/wallet/feebumper.cpp
@@ -272,18 +272,14 @@ Result CreateRateBumpTransaction(CWallet* wallet, const uint256& txid, const CCo
new_coin_control.m_min_depth = 1;
CTransactionRef tx_new = MakeTransactionRef();
- ReserveDestination reservedest(wallet);
CAmount fee_ret;
int change_pos_in_out = -1; // No requested location for change
std::string fail_reason;
- if (!wallet->CreateTransaction(*locked_chain, recipients, tx_new, reservedest, fee_ret, change_pos_in_out, fail_reason, new_coin_control, false)) {
+ if (!wallet->CreateTransaction(*locked_chain, recipients, tx_new, fee_ret, change_pos_in_out, fail_reason, new_coin_control, false)) {
errors.push_back("Unable to create transaction: " + fail_reason);
return Result::WALLET_ERROR;
}
- // If change key hasn't been ReturnKey'ed by this point, we take it out of keypool
- reservedest.KeepDestination();
-
// Write back new fee if successful
new_fee = fee_ret;
@@ -330,9 +326,8 @@ Result CommitTransaction(CWallet* wallet, const uint256& txid, CMutableTransacti
mapValue_t mapValue = oldWtx.mapValue;
mapValue["replaces_txid"] = oldWtx.GetHash().ToString();
- ReserveDestination reservedest(wallet);
CValidationState state;
- if (!wallet->CommitTransaction(tx, std::move(mapValue), oldWtx.vOrderForm, reservedest, state)) {
+ if (!wallet->CommitTransaction(tx, std::move(mapValue), oldWtx.vOrderForm, state)) {
// NOTE: CommitTransaction never returns false, so this should never happen.
errors.push_back(strprintf("The transaction was rejected: %s", FormatStateMessage(state)));
return Result::WALLET_ERROR;
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
index 19becf93c7..ab732dc0d8 100644
--- a/src/wallet/rpcwallet.cpp
+++ b/src/wallet/rpcwallet.cpp
@@ -317,7 +317,6 @@ static CTransactionRef SendMoney(interfaces::Chain::Lock& locked_chain, CWallet
CScript scriptPubKey = GetScriptForDestination(address);
// Create and send the transaction
- ReserveDestination reservedest(pwallet);
CAmount nFeeRequired;
std::string strError;
std::vector<CRecipient> vecSend;
@@ -325,13 +324,13 @@ static CTransactionRef SendMoney(interfaces::Chain::Lock& locked_chain, CWallet
CRecipient recipient = {scriptPubKey, nValue, fSubtractFeeFromAmount};
vecSend.push_back(recipient);
CTransactionRef tx;
- if (!pwallet->CreateTransaction(locked_chain, vecSend, tx, reservedest, nFeeRequired, nChangePosRet, strError, coin_control)) {
+ if (!pwallet->CreateTransaction(locked_chain, vecSend, tx, nFeeRequired, nChangePosRet, strError, coin_control)) {
if (!fSubtractFeeFromAmount && nValue + nFeeRequired > curBalance)
strError = strprintf("Error: This transaction requires a transaction fee of at least %s", FormatMoney(nFeeRequired));
throw JSONRPCError(RPC_WALLET_ERROR, strError);
}
CValidationState state;
- if (!pwallet->CommitTransaction(tx, std::move(mapValue), {} /* orderForm */, reservedest, state)) {
+ if (!pwallet->CommitTransaction(tx, std::move(mapValue), {} /* orderForm */, state)) {
strError = strprintf("Error: The transaction was rejected! Reason given: %s", FormatStateMessage(state));
throw JSONRPCError(RPC_WALLET_ERROR, strError);
}
@@ -915,16 +914,15 @@ static UniValue sendmany(const JSONRPCRequest& request)
std::shuffle(vecSend.begin(), vecSend.end(), FastRandomContext());
// Send
- ReserveDestination changedest(pwallet);
CAmount nFeeRequired = 0;
int nChangePosRet = -1;
std::string strFailReason;
CTransactionRef tx;
- bool fCreated = pwallet->CreateTransaction(*locked_chain, vecSend, tx, changedest, nFeeRequired, nChangePosRet, strFailReason, coin_control);
+ bool fCreated = pwallet->CreateTransaction(*locked_chain, vecSend, tx, nFeeRequired, nChangePosRet, strFailReason, coin_control);
if (!fCreated)
throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, strFailReason);
CValidationState state;
- if (!pwallet->CommitTransaction(tx, std::move(mapValue), {} /* orderForm */, changedest, state)) {
+ if (!pwallet->CommitTransaction(tx, std::move(mapValue), {} /* orderForm */, state)) {
strFailReason = strprintf("Transaction commit failed:: %s", FormatStateMessage(state));
throw JSONRPCError(RPC_WALLET_ERROR, strFailReason);
}
diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp
index a2fa94a2de..8af05dea45 100644
--- a/src/wallet/test/wallet_tests.cpp
+++ b/src/wallet/test/wallet_tests.cpp
@@ -361,17 +361,16 @@ public:
CWalletTx& AddTx(CRecipient recipient)
{
CTransactionRef tx;
- ReserveDestination reservedest(wallet.get());
CAmount fee;
int changePos = -1;
std::string error;
CCoinControl dummy;
{
auto locked_chain = m_chain->lock();
- BOOST_CHECK(wallet->CreateTransaction(*locked_chain, {recipient}, tx, reservedest, fee, changePos, error, dummy));
+ BOOST_CHECK(wallet->CreateTransaction(*locked_chain, {recipient}, tx, fee, changePos, error, dummy));
}
CValidationState state;
- BOOST_CHECK(wallet->CommitTransaction(tx, {}, {}, reservedest, state));
+ BOOST_CHECK(wallet->CommitTransaction(tx, {}, {}, state));
CMutableTransaction blocktx;
{
LOCK(wallet->cs_wallet);
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 07fc8c35d5..452d4f7a6a 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -2779,17 +2779,13 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC
auto locked_chain = chain().lock();
LOCK(cs_wallet);
- ReserveDestination reservedest(this);
CTransactionRef tx_new;
- if (!CreateTransaction(*locked_chain, vecSend, tx_new, reservedest, nFeeRet, nChangePosInOut, strFailReason, coinControl, false)) {
+ if (!CreateTransaction(*locked_chain, vecSend, tx_new, nFeeRet, nChangePosInOut, strFailReason, coinControl, false)) {
return false;
}
if (nChangePosInOut != -1) {
tx.vout.insert(tx.vout.begin() + nChangePosInOut, tx_new->vout[nChangePosInOut]);
- // We don't have the normal Create/Commit cycle, and don't want to risk
- // reusing change, so just remove the key from the keypool here.
- reservedest.KeepDestination();
}
// Copy output sizes from new transaction; they may have had the fee
@@ -2900,10 +2896,11 @@ OutputType CWallet::TransactionChangeType(OutputType change_type, const std::vec
return m_default_address_type;
}
-bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std::vector<CRecipient>& vecSend, CTransactionRef& tx, ReserveDestination& reservedest, CAmount& nFeeRet,
+bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std::vector<CRecipient>& vecSend, CTransactionRef& tx, CAmount& nFeeRet,
int& nChangePosInOut, std::string& strFailReason, const CCoinControl& coin_control, bool sign)
{
CAmount nValue = 0;
+ ReserveDestination reservedest(this);
int nChangePosRequest = nChangePosInOut;
unsigned int nSubtractFeeFromAmount = 0;
for (const auto& recipient : vecSend)
@@ -3183,8 +3180,6 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std
}
}
- if (nChangePosInOut == -1) reservedest.ReturnDestination(); // Return any reserved address if we don't have change
-
// Shuffle selected coins and fill in final vin
txNew.vin.clear();
std::vector<CInputCoin> selected_coins(setCoins.begin(), setCoins.end());
@@ -3247,6 +3242,10 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std
}
}
+ // Before we return success, we assume any change key will be used to prevent
+ // accidental re-use.
+ reservedest.KeepDestination();
+
WalletLogPrintf("Fee Calculation: Fee:%d Bytes:%u Needed:%d Tgt:%d (requested %d) Reason:\"%s\" Decay %.5f: Estimation: (%g - %g) %.2f%% %.1f/(%.1f %d mem %.1f out) Fail: (%g - %g) %.2f%% %.1f/(%.1f %d mem %.1f out)\n",
nFeeRet, nBytes, nFeeNeeded, feeCalc.returnedTarget, feeCalc.desiredTarget, StringForFeeReason(feeCalc.reason), feeCalc.est.decay,
feeCalc.est.pass.start, feeCalc.est.pass.end,
@@ -3261,7 +3260,7 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std
/**
* Call after CreateTransaction unless you want to abort
*/
-bool CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector<std::pair<std::string, std::string>> orderForm, ReserveDestination& reservedest, CValidationState& state)
+bool CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector<std::pair<std::string, std::string>> orderForm, CValidationState& state)
{
{
auto locked_chain = chain().lock();
@@ -3275,8 +3274,6 @@ bool CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::ve
WalletLogPrintf("CommitTransaction:\n%s", wtxNew.tx->ToString()); /* Continued */
{
- // Take key pair from key pool so it won't be used again
- reservedest.KeepDestination();
// Add tx to wallet, because if it has change it's also ours,
// otherwise just for transaction history.
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index 5f226cf5c0..6a7097bf44 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -266,8 +266,8 @@ public:
/** A wrapper to reserve an address from a wallet
*
- * ReserveDestination is used to reserve an address. It is passed around
- * during the CreateTransaction/CommitTransaction procedure.
+ * ReserveDestination is used to reserve an address.
+ * It is currently only used inside of CreateTransaction.
*
* Instantiating a ReserveDestination does not reserve an address. To do so,
* GetReservedDestination() needs to be called on the object. Once an address has been
@@ -1137,9 +1137,9 @@ public:
* selected by SelectCoins(); Also create the change output, when needed
* @note passing nChangePosInOut as -1 will result in setting a random position
*/
- bool CreateTransaction(interfaces::Chain::Lock& locked_chain, const std::vector<CRecipient>& vecSend, CTransactionRef& tx, ReserveDestination& reservedest, CAmount& nFeeRet, int& nChangePosInOut,
+ bool CreateTransaction(interfaces::Chain::Lock& locked_chain, const std::vector<CRecipient>& vecSend, CTransactionRef& tx, CAmount& nFeeRet, int& nChangePosInOut,
std::string& strFailReason, const CCoinControl& coin_control, bool sign = true);
- bool CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector<std::pair<std::string, std::string>> orderForm, ReserveDestination& reservedest, CValidationState& state);
+ bool CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector<std::pair<std::string, std::string>> orderForm, CValidationState& state);
bool DummySignTx(CMutableTransaction &txNew, const std::set<CTxOut> &txouts, bool use_max_sig = false) const
{