Merge #9680: Unify CWalletTx construction

b4bc32a451 [wallet] Get rid of CWalletTx default constructor (Russell Yanofsky)
a128bdc9e1 [wallet] Construct CWalletTx objects in CommitTransaction (Russell Yanofsky)

Pull request description:

  Two commits:

  - `Construct CWalletTx objects in CommitTransaction` moves a bunch of CWalletTx initialization into CWallet::CommitTransaction to dedup some code and avoid future inconsistencies in how wallet transactions are created.
  - `Get rid of CWalletTx default constructor` does what is described and eliminates the possibility of empty transaction entries being inadvertently created by mapWallet[hash] accesses.

  Both of these changes were originally part of #9381

Tree-SHA512: af3841c4f0539e0662d81b33c5369fc70aa06ddde1c59cb00fb21c9e4c7d9ff47f1edc5040cb463af1333838802c56b3ef875b939e2b804ee45b8e0294a4371c

3 files changed, 14 insertions, 21 deletions

diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp
index e7d9d276d7..39ef20c835 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<std::pair<std::string, std::string>> 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 4b2bef2690..4df8a5687e 100644
--- a/src/qt/walletmodeltransaction.cpp
+++ b/src/qt/walletmodeltransaction.cpp
@@ -12,12 +12,6 @@ WalletModelTransaction::WalletModelTransaction(const QList<SendCoinsRecipient> &
     walletTransaction(0),
     fee(0)
 {
-    walletTransaction = new CWalletTx();
-}
-
-WalletModelTransaction::~WalletModelTransaction()
-{
-    delete walletTransaction;
 }
 
 QList<SendCoinsRecipient> WalletModelTransaction::getRecipients() const
@@ -25,14 +19,14 @@ QList<SendCoinsRecipient> 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 cd531dba4b..931e960d18 100644
--- a/src/qt/walletmodeltransaction.h
+++ b/src/qt/walletmodeltransaction.h
@@ -20,11 +20,10 @@ class WalletModelTransaction
 {
 public:
     explicit WalletModelTransaction(const QList<SendCoinsRecipient> &recipients);
-    ~WalletModelTransaction();
 
     QList<SendCoinsRecipient> getRecipients() const;
 
-    CWalletTx *getTransaction() const;
+    CTransactionRef& getTransaction();
     unsigned int getTransactionSize();
 
     void setTransactionFee(const CAmount& newFee);
@@ -39,7 +38,7 @@ public:
 
 private:
     QList<SendCoinsRecipient> recipients;
-    CWalletTx *walletTransaction;
+    CTransactionRef walletTransaction;
     std::unique_ptr<CReserveKey> keyChange;
     CAmount fee;
 };