From a128bdc9e15dec5cd9aed1e4922c938edf31eb9a Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Thu, 2 Feb 2017 15:30:03 -0500 Subject: [PATCH 1/2] [wallet] Construct CWalletTx objects in CommitTransaction Construct CWalletTx objects in CWallet::CommitTransaction, instead of having callers do it. This ensures CWalletTx objects are constructed in a uniform way and all fields are set. This also makes it possible to avoid confusing and wasteful CWalletTx copies in https://github.com/bitcoin/bitcoin/pull/9381 There is no change in behavior. --- src/qt/walletmodel.cpp | 16 ++++++------ src/qt/walletmodeltransaction.cpp | 14 +++------- src/qt/walletmodeltransaction.h | 5 ++-- src/wallet/feebumper.cpp | 17 +++++------- src/wallet/rpcwallet.cpp | 43 +++++++++++++++---------------- src/wallet/test/wallet_tests.cpp | 10 +++---- src/wallet/wallet.cpp | 33 ++++++++++++++---------- src/wallet/wallet.h | 4 +-- 8 files changed, 68 insertions(+), 74 deletions(-) diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index e7d9d276d..39ef20c83 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -275,9 +275,9 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact int nChangePosRet = -1; std::string strFailReason; - CWalletTx *newTx = transaction.getTransaction(); + CTransactionRef& newTx = transaction.getTransaction(); CReserveKey *keyChange = transaction.getPossibleKeyChange(); - bool fCreated = wallet->CreateTransaction(vecSend, *newTx, *keyChange, nFeeRequired, nChangePosRet, strFailReason, coinControl); + bool fCreated = wallet->CreateTransaction(vecSend, newTx, *keyChange, nFeeRequired, nChangePosRet, strFailReason, coinControl); transaction.setTransactionFee(nFeeRequired); if (fSubtractFeeFromAmount && fCreated) transaction.reassignAmounts(nChangePosRet); @@ -309,8 +309,8 @@ WalletModel::SendCoinsReturn WalletModel::sendCoins(WalletModelTransaction &tran { LOCK2(cs_main, wallet->cs_wallet); - CWalletTx *newTx = transaction.getTransaction(); + std::vector> vOrderForm; for (const SendCoinsRecipient &rcp : transaction.getRecipients()) { if (rcp.paymentRequest.IsInitialized()) @@ -321,22 +321,22 @@ WalletModel::SendCoinsReturn WalletModel::sendCoins(WalletModelTransaction &tran } // Store PaymentRequests in wtx.vOrderForm in wallet. - std::string key("PaymentRequest"); std::string value; rcp.paymentRequest.SerializeToString(&value); - newTx->vOrderForm.push_back(make_pair(key, value)); + vOrderForm.emplace_back("PaymentRequest", std::move(value)); } else if (!rcp.message.isEmpty()) // Message from normal bitcoin:URI (bitcoin:123...?message=example) - newTx->vOrderForm.push_back(make_pair("Message", rcp.message.toStdString())); + vOrderForm.emplace_back("Message", rcp.message.toStdString()); } + CTransactionRef& newTx = transaction.getTransaction(); CReserveKey *keyChange = transaction.getPossibleKeyChange(); CValidationState state; - if(!wallet->CommitTransaction(*newTx, *keyChange, g_connman.get(), state)) + if (!wallet->CommitTransaction(newTx, {} /* mapValue */, std::move(vOrderForm), {} /* fromAccount */, *keyChange, g_connman.get(), state)) return SendCoinsReturn(TransactionCommitFailed, QString::fromStdString(state.GetRejectReason())); CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION); - ssTx << *newTx->tx; + ssTx << newTx; transaction_array.append(&(ssTx[0]), ssTx.size()); } diff --git a/src/qt/walletmodeltransaction.cpp b/src/qt/walletmodeltransaction.cpp index 4b2bef269..4df8a5687 100644 --- a/src/qt/walletmodeltransaction.cpp +++ b/src/qt/walletmodeltransaction.cpp @@ -12,12 +12,6 @@ WalletModelTransaction::WalletModelTransaction(const QList & walletTransaction(0), fee(0) { - walletTransaction = new CWalletTx(); -} - -WalletModelTransaction::~WalletModelTransaction() -{ - delete walletTransaction; } QList WalletModelTransaction::getRecipients() const @@ -25,14 +19,14 @@ QList WalletModelTransaction::getRecipients() const return recipients; } -CWalletTx *WalletModelTransaction::getTransaction() const +CTransactionRef& WalletModelTransaction::getTransaction() { return walletTransaction; } unsigned int WalletModelTransaction::getTransactionSize() { - return (!walletTransaction ? 0 : ::GetVirtualTransactionSize(*walletTransaction->tx)); + return (!walletTransaction ? 0 : ::GetVirtualTransactionSize(*walletTransaction)); } CAmount WalletModelTransaction::getTransactionFee() const @@ -62,7 +56,7 @@ void WalletModelTransaction::reassignAmounts(int nChangePosRet) if (out.amount() <= 0) continue; if (i == nChangePosRet) i++; - subtotal += walletTransaction->tx->vout[i].nValue; + subtotal += walletTransaction->vout[i].nValue; i++; } rcp.amount = subtotal; @@ -71,7 +65,7 @@ void WalletModelTransaction::reassignAmounts(int nChangePosRet) { if (i == nChangePosRet) i++; - rcp.amount = walletTransaction->tx->vout[i].nValue; + rcp.amount = walletTransaction->vout[i].nValue; i++; } } diff --git a/src/qt/walletmodeltransaction.h b/src/qt/walletmodeltransaction.h index cd531dba4..931e960d1 100644 --- a/src/qt/walletmodeltransaction.h +++ b/src/qt/walletmodeltransaction.h @@ -20,11 +20,10 @@ class WalletModelTransaction { public: explicit WalletModelTransaction(const QList &recipients); - ~WalletModelTransaction(); QList getRecipients() const; - CWalletTx *getTransaction() const; + CTransactionRef& getTransaction(); unsigned int getTransactionSize(); void setTransactionFee(const CAmount& newFee); @@ -39,7 +38,7 @@ public: private: QList recipients; - CWalletTx *walletTransaction; + CTransactionRef walletTransaction; std::unique_ptr keyChange; CAmount fee; }; diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index 9cae660c6..0c6d782e3 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -262,23 +262,20 @@ Result CommitTransaction(CWallet* wallet, const uint256& txid, CMutableTransacti return result; } - CWalletTx wtxBumped(wallet, MakeTransactionRef(std::move(mtx))); // commit/broadcast the tx + CTransactionRef tx = MakeTransactionRef(std::move(mtx)); + mapValue_t mapValue = oldWtx.mapValue; + mapValue["replaces_txid"] = oldWtx.GetHash().ToString(); + CReserveKey reservekey(wallet); - wtxBumped.mapValue = oldWtx.mapValue; - wtxBumped.mapValue["replaces_txid"] = oldWtx.GetHash().ToString(); - wtxBumped.vOrderForm = oldWtx.vOrderForm; - wtxBumped.strFromAccount = oldWtx.strFromAccount; - wtxBumped.fTimeReceivedIsTxTime = true; - wtxBumped.fFromMe = true; CValidationState state; - if (!wallet->CommitTransaction(wtxBumped, reservekey, g_connman.get(), state)) { + if (!wallet->CommitTransaction(tx, std::move(mapValue), oldWtx.vOrderForm, oldWtx.strFromAccount, reservekey, g_connman.get(), 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; } - bumped_txid = wtxBumped.GetHash(); + bumped_txid = tx->GetHash(); if (state.IsInvalid()) { // This can happen if the mempool rejected the transaction. Report // what happened in the "errors" response. @@ -286,7 +283,7 @@ Result CommitTransaction(CWallet* wallet, const uint256& txid, CMutableTransacti } // mark the original tx as bumped - if (!wallet->MarkReplaced(oldWtx.GetHash(), wtxBumped.GetHash())) { + if (!wallet->MarkReplaced(oldWtx.GetHash(), bumped_txid)) { // TODO: see if JSON-RPC has a standard way of returning a response // along with an exception. It would be good to return information about // wtxBumped to the caller even if marking the original transaction diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 457abec1b..8810b90e1 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -404,7 +404,7 @@ UniValue getaddressesbyaccount(const JSONRPCRequest& request) return ret; } -static void SendMoney(CWallet * const pwallet, const CTxDestination &address, CAmount nValue, bool fSubtractFeeFromAmount, CWalletTx& wtxNew, const CCoinControl& coin_control) +static CTransactionRef SendMoney(CWallet * const pwallet, const CTxDestination &address, CAmount nValue, bool fSubtractFeeFromAmount, const CCoinControl& coin_control, mapValue_t mapValue, std::string fromAccount) { CAmount curBalance = pwallet->GetBalance(); @@ -430,16 +430,18 @@ static void SendMoney(CWallet * const pwallet, const CTxDestination &address, CA int nChangePosRet = -1; CRecipient recipient = {scriptPubKey, nValue, fSubtractFeeFromAmount}; vecSend.push_back(recipient); - if (!pwallet->CreateTransaction(vecSend, wtxNew, reservekey, nFeeRequired, nChangePosRet, strError, coin_control)) { + CTransactionRef tx; + if (!pwallet->CreateTransaction(vecSend, tx, reservekey, 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(wtxNew, reservekey, g_connman.get(), state)) { + if (!pwallet->CommitTransaction(tx, std::move(mapValue), {} /* orderForm */, std::move(fromAccount), reservekey, g_connman.get(), state)) { strError = strprintf("Error: The transaction was rejected! Reason given: %s", FormatStateMessage(state)); throw JSONRPCError(RPC_WALLET_ERROR, strError); } + return tx; } UniValue sendtoaddress(const JSONRPCRequest& request) @@ -498,11 +500,11 @@ UniValue sendtoaddress(const JSONRPCRequest& request) throw JSONRPCError(RPC_TYPE_ERROR, "Invalid amount for send"); // Wallet comments - CWalletTx wtx; + mapValue_t mapValue; if (!request.params[2].isNull() && !request.params[2].get_str().empty()) - wtx.mapValue["comment"] = request.params[2].get_str(); + mapValue["comment"] = request.params[2].get_str(); if (!request.params[3].isNull() && !request.params[3].get_str().empty()) - wtx.mapValue["to"] = request.params[3].get_str(); + mapValue["to"] = request.params[3].get_str(); bool fSubtractFeeFromAmount = false; if (!request.params[4].isNull()) { @@ -527,9 +529,8 @@ UniValue sendtoaddress(const JSONRPCRequest& request) EnsureWalletIsUnlocked(pwallet); - SendMoney(pwallet, dest, nAmount, fSubtractFeeFromAmount, wtx, coin_control); - - return wtx.GetHash().GetHex(); + CTransactionRef tx = SendMoney(pwallet, dest, nAmount, fSubtractFeeFromAmount, coin_control, std::move(mapValue), {} /* fromAccount */); + return tx->GetHash().GetHex(); } UniValue listaddressgroupings(const JSONRPCRequest& request) @@ -995,12 +996,11 @@ UniValue sendfrom(const JSONRPCRequest& request) if (!request.params[3].isNull()) nMinDepth = request.params[3].get_int(); - CWalletTx wtx; - wtx.strFromAccount = strAccount; + mapValue_t mapValue; if (!request.params[4].isNull() && !request.params[4].get_str().empty()) - wtx.mapValue["comment"] = request.params[4].get_str(); + mapValue["comment"] = request.params[4].get_str(); if (!request.params[5].isNull() && !request.params[5].get_str().empty()) - wtx.mapValue["to"] = request.params[5].get_str(); + mapValue["to"] = request.params[5].get_str(); EnsureWalletIsUnlocked(pwallet); @@ -1010,9 +1010,8 @@ UniValue sendfrom(const JSONRPCRequest& request) throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, "Account has insufficient funds"); CCoinControl no_coin_control; // This is a deprecated API - SendMoney(pwallet, dest, nAmount, false, wtx, no_coin_control); - - return wtx.GetHash().GetHex(); + CTransactionRef tx = SendMoney(pwallet, dest, nAmount, false, no_coin_control, std::move(mapValue), std::move(strAccount)); + return tx->GetHash().GetHex(); } @@ -1083,10 +1082,9 @@ UniValue sendmany(const JSONRPCRequest& request) if (!request.params[2].isNull()) nMinDepth = request.params[2].get_int(); - CWalletTx wtx; - wtx.strFromAccount = strAccount; + mapValue_t mapValue; if (!request.params[3].isNull() && !request.params[3].get_str().empty()) - wtx.mapValue["comment"] = request.params[3].get_str(); + mapValue["comment"] = request.params[3].get_str(); UniValue subtractFeeFromAmount(UniValue::VARR); if (!request.params[4].isNull()) @@ -1152,16 +1150,17 @@ UniValue sendmany(const JSONRPCRequest& request) CAmount nFeeRequired = 0; int nChangePosRet = -1; std::string strFailReason; - bool fCreated = pwallet->CreateTransaction(vecSend, wtx, keyChange, nFeeRequired, nChangePosRet, strFailReason, coin_control); + CTransactionRef tx; + bool fCreated = pwallet->CreateTransaction(vecSend, tx, keyChange, nFeeRequired, nChangePosRet, strFailReason, coin_control); if (!fCreated) throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, strFailReason); CValidationState state; - if (!pwallet->CommitTransaction(wtx, keyChange, g_connman.get(), state)) { + if (!pwallet->CommitTransaction(tx, std::move(mapValue), {} /* orderForm */, std::move(strAccount), keyChange, g_connman.get(), state)) { strFailReason = strprintf("Transaction commit failed:: %s", FormatStateMessage(state)); throw JSONRPCError(RPC_WALLET_ERROR, strFailReason); } - return wtx.GetHash().GetHex(); + return tx->GetHash().GetHex(); } UniValue addmultisigaddress(const JSONRPCRequest& request) diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 41348b50a..37b542844 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -624,23 +624,23 @@ public: CWalletTx& AddTx(CRecipient recipient) { - CWalletTx wtx; + CTransactionRef tx; CReserveKey reservekey(wallet.get()); CAmount fee; int changePos = -1; std::string error; CCoinControl dummy; - BOOST_CHECK(wallet->CreateTransaction({recipient}, wtx, reservekey, fee, changePos, error, dummy)); + BOOST_CHECK(wallet->CreateTransaction({recipient}, tx, reservekey, fee, changePos, error, dummy)); CValidationState state; - BOOST_CHECK(wallet->CommitTransaction(wtx, reservekey, nullptr, state)); + BOOST_CHECK(wallet->CommitTransaction(tx, {}, {}, {}, reservekey, nullptr, state)); CMutableTransaction blocktx; { LOCK(wallet->cs_wallet); - blocktx = CMutableTransaction(*wallet->mapWallet.at(wtx.GetHash()).tx); + blocktx = CMutableTransaction(*wallet->mapWallet.at(tx->GetHash()).tx); } CreateAndProcessBlock({CMutableTransaction(blocktx)}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); LOCK(wallet->cs_wallet); - auto it = wallet->mapWallet.find(wtx.GetHash()); + auto it = wallet->mapWallet.find(tx->GetHash()); BOOST_CHECK(it != wallet->mapWallet.end()); it->second.SetMerkleBranch(chainActive.Tip(), 1); return it->second; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 0c468878e..8e47b21d6 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2631,13 +2631,13 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC LOCK2(cs_main, cs_wallet); CReserveKey reservekey(this); - CWalletTx wtx; - if (!CreateTransaction(vecSend, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, coinControl, false)) { + CTransactionRef tx_new; + if (!CreateTransaction(vecSend, tx_new, reservekey, nFeeRet, nChangePosInOut, strFailReason, coinControl, false)) { return false; } if (nChangePosInOut != -1) { - tx.vout.insert(tx.vout.begin() + nChangePosInOut, wtx.tx->vout[nChangePosInOut]); + 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. reservekey.KeepKey(); @@ -2646,11 +2646,11 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC // Copy output sizes from new transaction; they may have had the fee // subtracted from them. for (unsigned int idx = 0; idx < tx.vout.size(); idx++) { - tx.vout[idx].nValue = wtx.tx->vout[idx].nValue; + tx.vout[idx].nValue = tx_new->vout[idx].nValue; } // Add new txins while keeping original txin scriptSig/order. - for (const CTxIn& txin : wtx.tx->vin) { + for (const CTxIn& txin : tx_new->vin) { if (!coinControl.IsSelected(txin.prevout)) { tx.vin.push_back(txin); @@ -2691,7 +2691,7 @@ OutputType CWallet::TransactionChangeType(OutputType change_type, const std::vec return g_address_type; } -bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletTx& wtxNew, CReserveKey& reservekey, CAmount& nFeeRet, +bool CWallet::CreateTransaction(const std::vector& vecSend, CTransactionRef& tx, CReserveKey& reservekey, CAmount& nFeeRet, int& nChangePosInOut, std::string& strFailReason, const CCoinControl& coin_control, bool sign) { CAmount nValue = 0; @@ -2715,8 +2715,6 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletT return false; } - wtxNew.fTimeReceivedIsTxTime = true; - wtxNew.BindWallet(this); CMutableTransaction txNew; // Discourage fee sniping. @@ -2804,7 +2802,6 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletT nChangePosInOut = nChangePosRequest; txNew.vin.clear(); txNew.vout.clear(); - wtxNew.fFromMe = true; bool fFirst = true; CAmount nValueToSelect = nValue; @@ -3019,11 +3016,11 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletT } } - // Embed the constructed transaction data in wtxNew. - wtxNew.SetTx(MakeTransactionRef(std::move(txNew))); + // Return the constructed transaction data. + tx = MakeTransactionRef(std::move(txNew)); // Limit size - if (GetTransactionWeight(*wtxNew.tx) >= MAX_STANDARD_TX_WEIGHT) + if (GetTransactionWeight(*tx) >= MAX_STANDARD_TX_WEIGHT) { strFailReason = _("Transaction too large"); return false; @@ -3033,7 +3030,7 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletT if (gArgs.GetBoolArg("-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS)) { // Lastly, ensure this tx will pass the mempool's chain limits LockPoints lp; - CTxMemPoolEntry entry(wtxNew.tx, 0, 0, 0, false, 0, lp); + CTxMemPoolEntry entry(tx, 0, 0, 0, false, 0, lp); CTxMemPool::setEntries setAncestors; size_t nLimitAncestors = gArgs.GetArg("-limitancestorcount", DEFAULT_ANCESTOR_LIMIT); size_t nLimitAncestorSize = gArgs.GetArg("-limitancestorsize", DEFAULT_ANCESTOR_SIZE_LIMIT)*1000; @@ -3060,10 +3057,18 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletT /** * Call after CreateTransaction unless you want to abort */ -bool CWallet::CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey, CConnman* connman, CValidationState& state) +bool CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector> orderForm, std::string fromAccount, CReserveKey& reservekey, CConnman* connman, CValidationState& state) { { LOCK2(cs_main, cs_wallet); + + CWalletTx wtxNew(this, std::move(tx)); + wtxNew.mapValue = std::move(mapValue); + wtxNew.vOrderForm = std::move(orderForm); + wtxNew.strFromAccount = std::move(fromAccount); + wtxNew.fTimeReceivedIsTxTime = true; + wtxNew.fFromMe = true; + LogPrintf("CommitTransaction:\n%s", wtxNew.tx->ToString()); { // Take key pair from key pool so it won't be used again diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 3e2d1794d..cdb0def2e 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -974,9 +974,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(const std::vector& vecSend, CWalletTx& wtxNew, CReserveKey& reservekey, CAmount& nFeeRet, int& nChangePosInOut, + bool CreateTransaction(const std::vector& vecSend, CTransactionRef& tx, CReserveKey& reservekey, CAmount& nFeeRet, int& nChangePosInOut, std::string& strFailReason, const CCoinControl& coin_control, bool sign = true); - bool CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey, CConnman* connman, CValidationState& state); + bool CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector> orderForm, std::string fromAccount, CReserveKey& reservekey, CConnman* connman, CValidationState& state); void ListAccountCreditDebit(const std::string& strAccount, std::list& entries); bool AddAccountingEntry(const CAccountingEntry&); From b4bc32a451720167000f59dd73ab07990f9c6b92 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Thu, 19 Jan 2017 16:08:03 -0500 Subject: [PATCH 2/2] [wallet] Get rid of CWalletTx default constructor No change in behavior in the normal case. But buggy mapWallet lookups with invalid txids will now throw exceptions instead of inserting dummy entries into the map, and potentially causing segfaults and other failures. This also makes it a compiler error to use the mapWallet[hash] syntax which could create dummy entries. --- src/wallet/test/accounting_tests.cpp | 8 ++++---- src/wallet/wallet.cpp | 10 +++++----- src/wallet/wallet.h | 5 ----- src/wallet/walletdb.cpp | 6 +++--- 4 files changed, 12 insertions(+), 17 deletions(-) diff --git a/src/wallet/test/accounting_tests.cpp b/src/wallet/test/accounting_tests.cpp index 7b20bd7b0..aae328d81 100644 --- a/src/wallet/test/accounting_tests.cpp +++ b/src/wallet/test/accounting_tests.cpp @@ -29,7 +29,7 @@ GetResults(CWallet& wallet, std::map& results) BOOST_AUTO_TEST_CASE(acc_orderupgrade) { std::vector vpwtx; - CWalletTx wtx; + CWalletTx wtx(nullptr /* pwallet */, MakeTransactionRef()); CAccountingEntry ae; std::map results; @@ -44,7 +44,7 @@ BOOST_AUTO_TEST_CASE(acc_orderupgrade) wtx.mapValue["comment"] = "z"; m_wallet.AddToWallet(wtx); - vpwtx.push_back(&m_wallet.mapWallet[wtx.GetHash()]); + vpwtx.push_back(&m_wallet.mapWallet.at(wtx.GetHash())); vpwtx[0]->nTimeReceived = (unsigned int)1333333335; vpwtx[0]->nOrderPos = -1; @@ -86,7 +86,7 @@ BOOST_AUTO_TEST_CASE(acc_orderupgrade) wtx.SetTx(MakeTransactionRef(std::move(tx))); } m_wallet.AddToWallet(wtx); - vpwtx.push_back(&m_wallet.mapWallet[wtx.GetHash()]); + vpwtx.push_back(&m_wallet.mapWallet.at(wtx.GetHash())); vpwtx[1]->nTimeReceived = (unsigned int)1333333336; wtx.mapValue["comment"] = "x"; @@ -96,7 +96,7 @@ BOOST_AUTO_TEST_CASE(acc_orderupgrade) wtx.SetTx(MakeTransactionRef(std::move(tx))); } m_wallet.AddToWallet(wtx); - vpwtx.push_back(&m_wallet.mapWallet[wtx.GetHash()]); + vpwtx.push_back(&m_wallet.mapWallet.at(wtx.GetHash())); vpwtx[2]->nTimeReceived = (unsigned int)1333333329; vpwtx[2]->nOrderPos = -1; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 8e47b21d6..5ab2f5717 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -531,7 +531,7 @@ void CWallet::SyncMetaData(std::pair ran int nMinOrderPos = std::numeric_limits::max(); const CWalletTx* copyFrom = nullptr; for (TxSpends::iterator it = range.first; it != range.second; ++it) { - const CWalletTx* wtx = &mapWallet[it->second]; + const CWalletTx* wtx = &mapWallet.at(it->second); if (wtx->nOrderPos < nMinOrderPos) { nMinOrderPos = wtx->nOrderPos;; copyFrom = wtx; @@ -544,7 +544,7 @@ void CWallet::SyncMetaData(std::pair ran for (TxSpends::iterator it = range.first; it != range.second; ++it) { const uint256& hash = it->second; - CWalletTx* copyTo = &mapWallet[hash]; + CWalletTx* copyTo = &mapWallet.at(hash); if (copyFrom == copyTo) continue; assert(copyFrom && "Oldest wallet transaction in range assumed to have been found."); if (!copyFrom->IsEquivalentTo(*copyTo)) continue; @@ -3081,7 +3081,7 @@ bool CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::ve // Notify that old coins are spent for (const CTxIn& txin : wtxNew.tx->vin) { - CWalletTx &coin = mapWallet[txin.prevout.hash]; + CWalletTx &coin = mapWallet.at(txin.prevout.hash); coin.BindWallet(this); NotifyTransactionChanged(this, coin.GetHash(), CT_UPDATED); } @@ -3092,7 +3092,7 @@ bool CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::ve // Get the inserted-CWalletTx from mapWallet so that the // fInMempool flag is cached properly - CWalletTx& wtx = mapWallet[wtxNew.GetHash()]; + CWalletTx& wtx = mapWallet.at(wtxNew.GetHash()); if (fBroadcastTransactions) { @@ -3548,7 +3548,7 @@ std::set< std::set > CWallet::GetAddressGroupings() CTxDestination address; if(!IsMine(txin)) /* If this input isn't mine, ignore it */ continue; - if(!ExtractDestination(mapWallet[txin.prevout.hash].tx->vout[txin.prevout.n].scriptPubKey, address)) + if(!ExtractDestination(mapWallet.at(txin.prevout.hash).tx->vout[txin.prevout.n].scriptPubKey, address)) continue; grouping.insert(address); any_mine = true; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index cdb0def2e..f68d40f09 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -348,11 +348,6 @@ public: mutable CAmount nAvailableWatchCreditCached; mutable CAmount nChangeCached; - CWalletTx() - { - Init(nullptr); - } - CWalletTx(const CWallet* pwalletIn, CTransactionRef arg) : CMerkleTx(std::move(arg)) { Init(pwalletIn); diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 0b0880a2b..7f5f3b84b 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -265,7 +265,7 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, { uint256 hash; ssKey >> hash; - CWalletTx wtx; + CWalletTx wtx(nullptr /* pwallet */, MakeTransactionRef()); ssValue >> wtx; CValidationState state; if (!(CheckTransaction(*wtx.tx, state) && (wtx.GetHash() == hash) && state.IsValid())) @@ -603,7 +603,7 @@ DBErrors CWalletDB::LoadWallet(CWallet* pwallet) pwallet->UpdateTimeFirstKey(1); for (uint256 hash : wss.vWalletUpgrade) - WriteTx(pwallet->mapWallet[hash]); + WriteTx(pwallet->mapWallet.at(hash)); // Rewrite encrypted wallets of versions 0.4.0 and 0.5.0rc: if (wss.fIsEncrypted && (wss.nFileVersion == 40000 || wss.nFileVersion == 50000)) @@ -664,7 +664,7 @@ DBErrors CWalletDB::FindWalletTx(std::vector& vTxHash, std::vector> hash; - CWalletTx wtx; + CWalletTx wtx(nullptr /* pwallet */, MakeTransactionRef()); ssValue >> wtx; vTxHash.push_back(hash);