Merge #16557: [wallet] restore coinbase and confirmed/conflicted checks in SubmitMemoryPoolAndRelay()

c8b53c3beafa289dcdbd8c2ee9f957bdeca79cbc [wallet] Restore confirmed/conflicted tx check in SubmitMemoryPoolAndRelay() (John Newbery)
214c4ecb9ab306627a08abb35cf82c73f10ec627 [wallet] restore coinbase check in SubmitMemoryPoolAndRelay() (John Newbery)

Pull request description:

  These checks don't change mempool acceptance/relay behaviour, but reduce log spam.

ACKs for top commit:
  MarcoFalke:
    ACK c8b53c3beafa289dcdbd8c2ee9f957bdeca79cbc (non-doc changes are mostly a git revert 8753f5652b4710e66b50ce87788bf6f33619b75a)
  ariard:
    utACK c8b53c3

Tree-SHA512: f928573ad68d2f70ac69a84b57f352d255dccd1942097cc664f130fcbdcdd7364bc52c43b9157e65ebbaaebbe93586c6e8386f24361b27478e0a23a445677672

2 files changed, 13 insertions, 7 deletions

diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index b3269083ec..03acf23508 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -2150,16 +2150,21 @@ void CWallet::ReacceptWalletTransactions(interfaces::Chain::Lock& locked_chain)
     for (const std::pair<const int64_t, CWalletTx*>& item : mapSorted) {
         CWalletTx& wtx = *(item.second);
         std::string unused_err_string;
-        wtx.SubmitMemoryPoolAndRelay(unused_err_string, false);
+        wtx.SubmitMemoryPoolAndRelay(unused_err_string, false, locked_chain);
     }
 }
 
-bool CWalletTx::SubmitMemoryPoolAndRelay(std::string& err_string, bool relay)
+bool CWalletTx::SubmitMemoryPoolAndRelay(std::string& err_string, bool relay, interfaces::Chain::Lock& locked_chain)
 {
     // Can't relay if wallet is not broadcasting
     if (!pwallet->GetBroadcastTransactions()) return false;
     // Don't relay abandoned transactions
     if (isAbandoned()) return false;
+    // Don't try to submit coinbase transactions. These would fail anyway but would
+    // cause log spam.
+    if (IsCoinBase()) return false;
+    // Don't try to submit conflicted or confirmed transactions.
+    if (GetDepthInMainChain(locked_chain) != 0) return false;
 
     // Submit transaction to mempool for relay
     pwallet->WalletLogPrintf("Submitting wtx %s to mempool for relay\n", GetHash().ToString());
@@ -2374,11 +2379,12 @@ void CWallet::ResendWalletTransactions()
         // Relay transactions
         for (std::pair<const uint256, CWalletTx>& item : mapWallet) {
             CWalletTx& wtx = item.second;
-            // only rebroadcast unconfirmed txes older than 5 minutes before the
-            // last block was found
+            // Attempt to rebroadcast all txes more than 5 minutes older than
+            // the last block. SubmitMemoryPoolAndRelay() will not rebroadcast
+            // any confirmed or conflicting txs.
             if (wtx.nTimeReceived > m_best_block_time - 5 * 60) continue;
             std::string unused_err_string;
-            if (wtx.SubmitMemoryPoolAndRelay(unused_err_string, true)) ++submitted_tx_count;
+            if (wtx.SubmitMemoryPoolAndRelay(unused_err_string, true, *locked_chain)) ++submitted_tx_count;
         }
     } // locked_chain and cs_wallet
 
@@ -3323,7 +3329,7 @@ bool CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::ve
         if (fBroadcastTransactions)
         {
             std::string err_string;
-            if (!wtx.SubmitMemoryPoolAndRelay(err_string, true)) {
+            if (!wtx.SubmitMemoryPoolAndRelay(err_string, true, *locked_chain)) {
                 WalletLogPrintf("CommitTransaction(): Transaction cannot be broadcast immediately, %s\n", err_string);
                 // TODO: if we expect the failure to be long term or permanent, instead delete wtx from the wallet and return failure.
             }
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index 3a45c1ccc5..cf388ad827 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -580,7 +580,7 @@ public:
     int64_t GetTxTime() const;
 
     // Pass this transaction to node for mempool insertion and relay to peers if flag set to true
-    bool SubmitMemoryPoolAndRelay(std::string& err_string, bool relay);
+    bool SubmitMemoryPoolAndRelay(std::string& err_string, bool relay, interfaces::Chain::Lock& locked_chain);
 
     // TODO: Remove "NO_THREAD_SAFETY_ANALYSIS" and replace it with the correct
     // annotation "EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)". The annotation