wallet, rpc, gui: Allow users to manually rebroadcast a wallet transaction #36481

    //! Return whether transaction can be broadcast.

    //! Rebroadcast a transaction.

What is the & in Re&broadcast? It is also in A&bandon.

    if (!transactionView || !transactionView->selectionModel())
        return;

also, the return should be either on the same line as if or there should be { and }.

    // Rebroadcast the wallet transaction over the walletModel

If the wallet broadcast is disabled (-walletbroadcast=0), then the "is not eligible to be rebroadcast" will be misleading because it implies that there is something specific about this transaction that makes it not eligible.

Consider extending TransactionCanBeBroadcast() to return the reason why the transaction can not be broadcast:

<details> <summary>[patch] TransactionCanBeBroadcast() with a reason</summary>

diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 29460e3ba2..dcd571b684 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -1832,51 +1832,64 @@ util::Expected<void, WalletError> CWallet::RebroadcastSingleTransaction(const Tx
 {
     LOCK(cs_wallet);
     auto it = mapWallet.find(txid);
     if (it == mapWallet.end()) {
         return util::Unexpected{WalletError{WalletErrorCode::InvalidParameter, strprintf(_("%s not in wallet"), txid.ToString())}};
     }
-    if (!TransactionCanBeBroadcast(it->second)) {
-        return util::Unexpected{WalletError{WalletErrorCode::InvalidParameter, strprintf(_("%s is not eligible to be rebroadcast"), txid.ToString())}};
-    }
     std::string err;
+    if (!TransactionCanBeBroadcast(it->second, err)) {
+        return util::Unexpected{WalletError{WalletErrorCode::InvalidParameter, strprintf(_("can not rebroadcast %s: %s"), txid.ToString(), err)}};
+    }
     if (!SubmitTxMemoryPoolAndRelay(it->second, err, node::TxBroadcast::MEMPOOL_AND_BROADCAST_TO_ALL)) {
         return util::Unexpected{WalletError{WalletErrorCode::MiscError, Untranslated(err)}};
     }
     return {};
 }
 
-bool CWallet::TransactionCanBeBroadcast(const Txid& txid) const
+bool CWallet::TransactionCanBeBroadcast(const Txid& txid, std::string& reason) const
 {
     LOCK(cs_wallet);
     const CWalletTx* wtx = GetWalletTx(txid);
-    return wtx && TransactionCanBeBroadcast(*wtx);
+    return wtx && TransactionCanBeBroadcast(*wtx, reason);
 }
 
-bool CWallet::TransactionCanBeBroadcast(const CWalletTx& wtx) const
+bool CWallet::TransactionCanBeBroadcast(const CWalletTx& wtx, std::string& reason) const
 {
     AssertLockHeld(cs_wallet);
     // Can't relay if wallet is not broadcasting
-    if (!GetBroadcastTransactions()) return false;
+    if (!GetBroadcastTransactions()) {
+        reason = "the wallet is configured to not broadcast transactions";
+        return false;
+    }
     // Don't relay abandoned transactions
-    if (wtx.isAbandoned()) return false;
+    if (wtx.isAbandoned()) {
+        reason = "the transaction is abandoned";
+        return false;
+    }
     // Don't try to submit coinbase transactions. These would fail anyway but would
     // cause log spam.
-    if (wtx.IsCoinBase()) return false;
+    if (wtx.IsCoinBase()) {
+        reason = "the transaction is a coinbase transaction";
+        return false;
+    }
     // Don't try to submit conflicted or confirmed transactions.
-    if (GetTxDepthInMainChain(wtx) != 0) return false;
+    if (GetTxDepthInMainChain(wtx) != 0) {
+        reason = "the transaction is already confirmed or is conflicted";
+        return false;
+    }
     return true;
 }
 
 bool CWallet::SubmitTxMemoryPoolAndRelay(CWalletTx& wtx,
                                          std::string& err_string,
                                          node::TxBroadcast broadcast_method) const
 {
     AssertLockHeld(cs_wallet);
 
-    if (!TransactionCanBeBroadcast(wtx)) return false;
+    std::string reason;
+    if (!TransactionCanBeBroadcast(wtx, reason)) return false;
 
     const char* what{""};
     switch (broadcast_method) {
     case node::TxBroadcast::MEMPOOL_AND_BROADCAST_TO_ALL:
         what = "to mempool and for broadcast to peers";
         break;
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index 8d7c72f4d7..0b64d62571 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -443,13 +443,13 @@ private:
     // Update last block processed in memory only
     void SetLastBlockProcessedInMem(int block_height, uint256 block_hash) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
 
     //! Update mempool conflicts for TRUC sibling transactions
     void UpdateTrucSiblingConflicts(const CWalletTx& parent_wtx, const Txid& child_txid, bool add_conflict) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
 
-    bool TransactionCanBeBroadcast(const CWalletTx& wtx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
+    bool TransactionCanBeBroadcast(const CWalletTx& wtx, std::string& reason) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
 
 public:
     /**
      * Main wallet lock.
      * This lock protects all the fields added by CWallet.
      */
@@ -635,13 +635,13 @@ public:
     void transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason) override;
     /** Set the next time this wallet should resend transactions to 12-36 hours from now, ~1 day on average. */
     void SetNextResend() { m_next_resend = GetDefaultNextResend(); }
     /** Return true if all conditions for periodically resending transactions are met. */
     bool ShouldResend() const;
     void ResubmitWalletTransactions(node::TxBroadcast broadcast_method, bool force);
-    bool TransactionCanBeBroadcast(const Txid& txid) const;
+    bool TransactionCanBeBroadcast(const Txid& txid, std::string& reason) const;
     util::Expected<void, WalletError> RebroadcastSingleTransaction(const Txid& txid);
 
     OutputType TransactionChangeType(const std::optional<OutputType>& change_type, const std::vector<CRecipient>& vecSend) const;
 
     /** Fetch the inputs and sign with SIGHASH_ALL. */
     bool SignTransaction(CMutableTransaction& tx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp
index 78139f635d..1759680b02 100644
--- a/src/wallet/interfaces.cpp
+++ b/src/wallet/interfaces.cpp
@@ -283,13 +283,17 @@ public:
     bool transactionCanBeAbandoned(const Txid& txid) override { return m_wallet->TransactionCanBeAbandoned(txid); }
     bool abandonTransaction(const Txid& txid) override
     {
         LOCK(m_wallet->cs_wallet);
         return m_wallet->AbandonTransaction(txid);
     }
-    bool transactionCanBeBroadcast(const Txid& txid) override { return m_wallet->TransactionCanBeBroadcast(txid); }
+    bool transactionCanBeBroadcast(const Txid& txid) override
+    {
+        std::string reason;
+        return m_wallet->TransactionCanBeBroadcast(txid, reason);
+    }
     util::Expected<void, WalletError> rebroadcastTransaction(const Txid& txid) override
     {
         LOCK(m_wallet->cs_wallet);
         return m_wallet->RebroadcastSingleTransaction(txid);
     }
     bool transactionCanBeBumped(const Txid& txid) override

</details>

Should we not throw here? UniValue::VNULL is for "If rebroadcast is successfully initiated".

-            if (!wallet) return UniValue::VNULL;
+            if (!wallet) {
+                throw JSONRPCError(RPC_WALLET_ERROR, "no wallet");
+            }

I think it would be useful to add the sentence Use the listtransactions RPC to see the wallet transactions here.