diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -261,9 +261,9 @@ 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, + bool fCreated = wallet->CreateTransaction(vecSend, newTx, *keyChange, nFeeRequired, nChangePosRet, strFailReason, coinControl); transaction.setTransactionFee(nFeeRequired); @@ -299,8 +299,8 @@ { LOCK2(cs_main, wallet->cs_wallet); - CWalletTx *newTx = transaction.getTransaction(); + std::vector> vOrderForm; for (const SendCoinsRecipient &rcp : transaction.getRecipients()) { if (rcp.paymentRequest.IsInitialized()) { // Make sure any payment requests involved are still valid. @@ -310,29 +310,29 @@ } // 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 bitcoincash:URI // (bitcoincash: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.h b/src/qt/walletmodeltransaction.h --- a/src/qt/walletmodeltransaction.h +++ b/src/qt/walletmodeltransaction.h @@ -20,11 +20,10 @@ public: explicit WalletModelTransaction( const QList &recipients); - ~WalletModelTransaction(); QList getRecipients() const; - CWalletTx *getTransaction() const; + CTransactionRef &getTransaction(); unsigned int getTransactionSize(); void setTransactionFee(const Amount newFee); @@ -40,7 +39,7 @@ private: QList recipients; - CWalletTx *walletTransaction; + CTransactionRef walletTransaction; std::unique_ptr keyChange; Amount fee; }; diff --git a/src/qt/walletmodeltransaction.cpp b/src/qt/walletmodeltransaction.cpp --- a/src/qt/walletmodeltransaction.cpp +++ b/src/qt/walletmodeltransaction.cpp @@ -9,19 +9,13 @@ WalletModelTransaction::WalletModelTransaction( const QList &_recipients) - : recipients(_recipients), walletTransaction(0), fee() { - walletTransaction = new CWalletTx(); -} - -WalletModelTransaction::~WalletModelTransaction() { - delete walletTransaction; -} + : recipients(_recipients), walletTransaction(0), fee() {} QList WalletModelTransaction::getRecipients() const { return recipients; } -CWalletTx *WalletModelTransaction::getTransaction() const { +CTransactionRef &WalletModelTransaction::getTransaction() { return walletTransaction; } @@ -55,7 +49,7 @@ i++; } - subtotal += walletTransaction->tx->vout[i].nValue; + subtotal += walletTransaction->vout[i].nValue; i++; } rcp.amount = subtotal; @@ -65,7 +59,7 @@ i++; } - rcp.amount = walletTransaction->tx->vout[i].nValue; + rcp.amount = walletTransaction->vout[i].nValue; i++; } } diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -425,9 +425,10 @@ return ret; } -static void SendMoney(CWallet *const pwallet, const CTxDestination &address, - Amount nValue, bool fSubtractFeeFromAmount, - CWalletTx &wtxNew) { +static CTransactionRef SendMoney(CWallet *const pwallet, + const CTxDestination &address, Amount nValue, + bool fSubtractFeeFromAmount, + mapValue_t mapValue, std::string fromAccount) { Amount curBalance = pwallet->GetBalance(); // Check amount @@ -458,7 +459,8 @@ vecSend.push_back(recipient); CCoinControl coinControl; - if (!pwallet->CreateTransaction(vecSend, wtxNew, reservekey, nFeeRequired, + CTransactionRef tx; + if (!pwallet->CreateTransaction(vecSend, tx, reservekey, nFeeRequired, nChangePosRet, strError, coinControl)) { if (!fSubtractFeeFromAmount && nValue + nFeeRequired > curBalance) { strError = strprintf("Error: This transaction requires a " @@ -468,13 +470,15 @@ 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", state.GetRejectReason()); throw JSONRPCError(RPC_WALLET_ERROR, strError); } + return tx; } static UniValue sendtoaddress(const Config &config, @@ -549,14 +553,14 @@ } // Wallet comments - CWalletTx wtx; + mapValue_t mapValue; if (request.params.size() > 2 && !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.size() > 3 && !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; @@ -566,9 +570,10 @@ EnsureWalletIsUnlocked(pwallet); - SendMoney(pwallet, dest, nAmount, fSubtractFeeFromAmount, wtx); - - return wtx.GetId().GetHex(); + CTransactionRef tx = + SendMoney(pwallet, dest, nAmount, fSubtractFeeFromAmount, + std::move(mapValue), {} /* fromAccount */); + return tx->GetId().GetHex(); } static UniValue listaddressgroupings(const Config &config, @@ -1139,16 +1144,15 @@ nMinDepth = request.params[3].get_int(); } - CWalletTx wtx; - wtx.strFromAccount = label; + mapValue_t mapValue; if (request.params.size() > 4 && !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.size() > 5 && !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); @@ -1161,9 +1165,9 @@ "Account has insufficient funds"); } - SendMoney(pwallet, dest, nAmount, false, wtx); - - return wtx.GetId().GetHex(); + CTransactionRef tx = SendMoney(pwallet, dest, nAmount, false, + std::move(mapValue), std::move(label)); + return tx->GetId().GetHex(); } static UniValue sendmany(const Config &config, const JSONRPCRequest &request) { @@ -1266,11 +1270,10 @@ nMinDepth = request.params[2].get_int(); } - CWalletTx wtx; - wtx.strFromAccount = strAccount; + mapValue_t mapValue; if (request.params.size() > 3 && !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); @@ -1332,21 +1335,24 @@ Amount nFeeRequired = Amount::zero(); int nChangePosRet = -1; std::string strFailReason; + CTransactionRef tx; CCoinControl coinControl; bool fCreated = - pwallet->CreateTransaction(vecSend, wtx, keyChange, nFeeRequired, + pwallet->CreateTransaction(vecSend, tx, keyChange, nFeeRequired, nChangePosRet, strFailReason, coinControl); 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", state.GetRejectReason()); throw JSONRPCError(RPC_WALLET_ERROR, strFailReason); } - return wtx.GetId().GetHex(); + return tx->GetId().GetHex(); } static UniValue addmultisigaddress(const Config &config, diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -725,26 +725,27 @@ } CWalletTx &AddTx(CRecipient recipient) { - CWalletTx wtx; + CTransactionRef tx; CReserveKey reservekey(wallet.get()); Amount fee; int changePos = -1; std::string error; CCoinControl dummy; - BOOST_CHECK(wallet->CreateTransaction({recipient}, wtx, reservekey, fee, + 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.GetId()).tx); + CMutableTransaction(*wallet->mapWallet.at(tx->GetId()).tx); } CreateAndProcessBlock({CMutableTransaction(blocktx)}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); LOCK(wallet->cs_wallet); - auto it = wallet->mapWallet.find(wtx.GetId()); + auto it = wallet->mapWallet.find(tx->GetId()); BOOST_CHECK(it != wallet->mapWallet.end()); it->second.SetMerkleBranch(chainActive.Tip(), 1); return it->second; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1006,12 +1006,15 @@ * position */ bool CreateTransaction(const std::vector &vecSend, - CWalletTx &wtxNew, CReserveKey &reservekey, + CTransactionRef &tx, CReserveKey &reservekey, Amount &nFeeRet, int &nChangePosInOut, std::string &strFailReason, const CCoinControl &coinControl, 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); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2775,25 +2775,25 @@ } CReserveKey reservekey(this); - CWalletTx wtx; - if (!CreateTransaction(vecSend, wtx, reservekey, nFeeRet, nChangePosInOut, + 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_new->vout[nChangePosInOut]); } // Copy output sizes from new transaction; they may have had the fee // subtracted from them. for (size_t 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 (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); @@ -2813,7 +2813,7 @@ } bool CWallet::CreateTransaction(const std::vector &vecSend, - CWalletTx &wtxNew, CReserveKey &reservekey, + CTransactionRef &tx, CReserveKey &reservekey, Amount &nFeeRet, int &nChangePosInOut, std::string &strFailReason, const CCoinControl &coinControl, bool sign) { @@ -2838,8 +2838,6 @@ return false; } - wtxNew.fTimeReceivedIsTxTime = true; - wtxNew.BindWallet(this); CMutableTransaction txNew; // Discourage fee sniping. @@ -2926,7 +2924,6 @@ nChangePosInOut = nChangePosRequest; txNew.vin.clear(); txNew.vout.clear(); - wtxNew.fFromMe = true; bool fFirst = true; Amount nValueToSelect = nValue; @@ -3188,11 +3185,11 @@ } } - // 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 (CTransaction(wtxNew).GetTotalSize() >= MAX_STANDARD_TX_SIZE) { + if (tx->GetTotalSize() >= MAX_STANDARD_TX_SIZE) { strFailReason = _("Transaction too large"); return false; } @@ -3202,8 +3199,8 @@ DEFAULT_WALLET_REJECT_LONG_CHAINS)) { // Lastly, ensure this tx will pass the mempool's chain limits. LockPoints lp; - CTxMemPoolEntry entry(wtxNew.tx, Amount::zero(), 0, 0, 0, - Amount::zero(), false, 0, lp); + CTxMemPoolEntry entry(tx, Amount::zero(), 0, 0, 0, Amount::zero(), + false, 0, lp); CTxMemPool::setEntries setAncestors; size_t nLimitAncestors = gArgs.GetArg("-limitancestorcount", DEFAULT_ANCESTOR_LIMIT); @@ -3231,9 +3228,20 @@ /** * 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.