Merge #16415: Get rid of PendingWalletTx class

4d94916f0dda535cb69b538ee4e3fffb5b033c87 Get rid of PendingWalletTx class. (Russell Yanofsky)

Pull request description:

  No reason for this class to exist if it doesn't have any code to run in the destructor. e10e1e8db043e9b7c113e07faf408f337c1b732d from https://github.com/bitcoin/bitcoin/pull/16208 recently removed the destructor code that would return an unused key if the transaction wasn't committed.

  This is just cleanup, there's no change in behavior.

ACKs for top commit:
  ariard:
    utACK 4d94916. Successfully built both `bitcoind` and `bitcoin-qt`. `PendingWalletTx` was only a wrapper to enforce call to `ReturnDestination` if `CommitTransaction` doesn't `KeepDestination` before.
  promag:
    ACK 4d94916f0dda535cb69b538ee4e3fffb5b033c87, refactor looks good to me.
  meshcollider:
    utACK 4d94916f0dda535cb69b538ee4e3fffb5b033c87

Tree-SHA512: f3f93d2f2f5d8f1e7810d609d881c1b1cbbaa8629f483f4293e20b3210292605e947bc4903fde9d2d8736277ca3bd6de182f7eac1e13515d5a327f2ebc130839

2 files changed, 25 insertions, 46 deletions

diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp
index deb1618ceb..077dc1ab4d 100644
--- a/src/interfaces/wallet.cpp
+++ b/src/interfaces/wallet.cpp
@@ -33,31 +33,6 @@
 namespace interfaces {
 namespace {
 
-class PendingWalletTxImpl : public PendingWalletTx
-{
-public:
-    explicit PendingWalletTxImpl(CWallet& wallet) : m_wallet(wallet) {}
-
-    const CTransaction& get() override { return *m_tx; }
-
-    bool commit(WalletValueMap value_map,
-        WalletOrderForm order_form,
-        std::string& reject_reason) override
-    {
-        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), state)) {
-            reject_reason = state.GetRejectReason();
-            return false;
-        }
-        return true;
-    }
-
-    CTransactionRef m_tx;
-    CWallet& m_wallet;
-};
-
 //! Construct wallet tx struct.
 WalletTx MakeWalletTx(interfaces::Chain::Lock& locked_chain, CWallet& wallet, const CWalletTx& wtx)
 {
@@ -227,7 +202,7 @@ public:
         LOCK(m_wallet->cs_wallet);
         return m_wallet->ListLockedCoins(outputs);
     }
-    std::unique_ptr<PendingWalletTx> createTransaction(const std::vector<CRecipient>& recipients,
+    CTransactionRef createTransaction(const std::vector<CRecipient>& recipients,
         const CCoinControl& coin_control,
         bool sign,
         int& change_pos,
@@ -236,12 +211,26 @@ 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, fee, change_pos,
+        CTransactionRef tx;
+        if (!m_wallet->CreateTransaction(*locked_chain, recipients, tx, fee, change_pos,
                 fail_reason, coin_control, sign)) {
             return {};
         }
-        return std::move(pending);
+        return tx;
+    }
+    bool commitTransaction(CTransactionRef tx,
+        WalletValueMap value_map,
+        WalletOrderForm order_form,
+        std::string& reject_reason) override
+    {
+        auto locked_chain = m_wallet->chain().lock();
+        LOCK(m_wallet->cs_wallet);
+        CValidationState state;
+        if (!m_wallet->CommitTransaction(std::move(tx), std::move(value_map), std::move(order_form), state)) {
+            reject_reason = state.GetRejectReason();
+            return false;
+        }
+        return true;
     }
     bool transactionCanBeAbandoned(const uint256& txid) override { return m_wallet->TransactionCanBeAbandoned(txid); }
     bool abandonTransaction(const uint256& txid) override
diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h
index db47dbafaf..89e056b18b 100644
--- a/src/interfaces/wallet.h
+++ b/src/interfaces/wallet.h
@@ -34,7 +34,6 @@ struct CRecipient;
 namespace interfaces {
 
 class Handler;
-class PendingWalletTx;
 struct WalletAddress;
 struct WalletBalances;
 struct WalletTx;
@@ -134,13 +133,19 @@ public:
     virtual void listLockedCoins(std::vector<COutPoint>& outputs) = 0;
 
     //! Create transaction.
-    virtual std::unique_ptr<PendingWalletTx> createTransaction(const std::vector<CRecipient>& recipients,
+    virtual CTransactionRef createTransaction(const std::vector<CRecipient>& recipients,
         const CCoinControl& coin_control,
         bool sign,
         int& change_pos,
         CAmount& fee,
         std::string& fail_reason) = 0;
 
+    //! Commit transaction.
+    virtual bool commitTransaction(CTransactionRef tx,
+        WalletValueMap value_map,
+        WalletOrderForm order_form,
+        std::string& reject_reason) = 0;
+
     //! Return whether transaction can be abandoned.
     virtual bool transactionCanBeAbandoned(const uint256& txid) = 0;
 
@@ -288,21 +293,6 @@ public:
     virtual std::unique_ptr<Handler> handleCanGetAddressesChanged(CanGetAddressesChangedFn fn) = 0;
 };
 
-//! Tracking object returned by CreateTransaction and passed to CommitTransaction.
-class PendingWalletTx
-{
-public:
-    virtual ~PendingWalletTx() {}
-
-    //! Get transaction data.
-    virtual const CTransaction& get() = 0;
-
-    //! Send pending transaction and commit to wallet.
-    virtual bool commit(WalletValueMap value_map,
-        WalletOrderForm order_form,
-        std::string& reject_reason) = 0;
-};
-
 //! Information about one wallet address.
 struct WalletAddress
 {