From c6e8e11fb030ef406752761530421a9e2f0f5d4f Mon Sep 17 00:00:00 2001 From: stickies-v Date: Tue, 27 Sep 2022 19:24:01 +0100 Subject: wallet: fix capitalization in docstring --- src/wallet/wallet.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 5889de2e36..5c9a0133d0 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1957,7 +1957,7 @@ void CWallet::ResubmitWalletTransactions(bool relay, bool force) // Only rebroadcast unconfirmed txs if (!wtx.isUnconfirmed()) continue; - // attempt to rebroadcast all txes more than 5 minutes older than + // Attempt to rebroadcast all txes more than 5 minutes older than // the last block, or all txs if forcing. if (!force && wtx.nTimeReceived > m_best_block_time - 5 * 60) continue; to_submit.insert(&wtx); -- cgit v1.2.3 From 01f3534632d18c772901fb6ce22f6394eae96799 Mon Sep 17 00:00:00 2001 From: stickies-v Date: Tue, 27 Sep 2022 19:21:51 +0100 Subject: refactor: remove unused locks for ResubmitWalletTransactions ReacceptWalletTransactions is replaced by ResubmitWalletTransactions which already handles acquiring the necessary locks internally. --- src/wallet/rpc/backup.cpp | 20 ++++---------------- src/wallet/wallet.cpp | 4 +--- 2 files changed, 5 insertions(+), 19 deletions(-) diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index 6cc3a71e19..88245a78ee 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -293,10 +293,7 @@ RPCHelpMan importaddress() if (fRescan) { RescanWallet(*pwallet, reserver); - { - LOCK(pwallet->cs_wallet); - pwallet->ResubmitWalletTransactions(/*relay=*/false, /*force=*/true); - } + pwallet->ResubmitWalletTransactions(/*relay=*/false, /*force=*/true); } return UniValue::VNULL; @@ -474,10 +471,7 @@ RPCHelpMan importpubkey() if (fRescan) { RescanWallet(*pwallet, reserver); - { - LOCK(pwallet->cs_wallet); - pwallet->ResubmitWalletTransactions(/*relay=*/false, /*force=*/true); - } + pwallet->ResubmitWalletTransactions(/*relay=*/false, /*force=*/true); } return UniValue::VNULL; @@ -1406,10 +1400,7 @@ RPCHelpMan importmulti() } if (fRescan && fRunScan && requests.size()) { int64_t scannedTime = pwallet->RescanFromTime(nLowestTimestamp, reserver, true /* update */); - { - LOCK(pwallet->cs_wallet); - pwallet->ResubmitWalletTransactions(/*relay=*/false, /*force=*/true); - } + pwallet->ResubmitWalletTransactions(/*relay=*/false, /*force=*/true); if (pwallet->IsAbortingRescan()) { throw JSONRPCError(RPC_MISC_ERROR, "Rescan aborted by user."); @@ -1700,10 +1691,7 @@ RPCHelpMan importdescriptors() // Rescan the blockchain using the lowest timestamp if (rescan) { int64_t scanned_time = pwallet->RescanFromTime(lowest_timestamp, reserver, true /* update */); - { - LOCK(pwallet->cs_wallet); - pwallet->ResubmitWalletTransactions(/*relay=*/false, /*force=*/true); - } + pwallet->ResubmitWalletTransactions(/*relay=*/false, /*force=*/true); if (pwallet->IsAbortingRescan()) { throw JSONRPCError(RPC_MISC_ERROR, "Rescan aborted by user."); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 5c9a0133d0..6b54fc19ea 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3196,14 +3196,12 @@ bool CWallet::UpgradeWallet(int version, bilingual_str& error) void CWallet::postInitProcess() { - LOCK(cs_wallet); - // Add wallet transactions that aren't already in a block to mempool // Do this here as mempool requires genesis block to be loaded ResubmitWalletTransactions(/*relay=*/false, /*force=*/true); // Update wallet transactions with current mempool transactions. - chain().requestMempoolTransactions(*this); + WITH_LOCK(cs_wallet, chain().requestMempoolTransactions(*this)); } bool CWallet::BackupWallet(const std::string& strDest) const -- cgit v1.2.3 From 7fbde8af5c06694eecd4ce601109bd826a54bd6f Mon Sep 17 00:00:00 2001 From: stickies-v Date: Fri, 30 Sep 2022 11:03:53 +0100 Subject: refactor: carve out tx resend timer logic into ShouldResend Moves the logic of whether or not transactions should actually be resent out of the function that's resending them. This reduces responsibilities of ResubmitWalletTransactions and allows carving out the updating of m_next_resend in a future commit. --- src/wallet/wallet.cpp | 26 ++++++++++++++++++-------- src/wallet/wallet.h | 2 ++ 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 6b54fc19ea..6ef3c9b66a 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1903,6 +1903,23 @@ std::set CWallet::GetTxConflicts(const CWalletTx& wtx) const return result; } +bool CWallet::ShouldResend() const +{ + // Don't attempt to resubmit if the wallet is configured to not broadcast + if (!fBroadcastTransactions) return false; + + // During reindex, importing and IBD, old wallet transactions become + // unconfirmed. Don't resend them as that would spam other nodes. + // We only allow forcing mempool submission when not relaying to avoid this spam. + if (!chain().isReadyToBroadcast()) return false; + + // Do this infrequently and randomly to avoid giving away + // that these are our transactions. + if (GetTime() < m_next_resend) return false; + + return true; +} + // Resubmit transactions from the wallet to the mempool, optionally asking the // mempool to relay them. On startup, we will do this for all unconfirmed // transactions but will not ask the mempool to relay them. We do this on startup @@ -1934,14 +1951,6 @@ void CWallet::ResubmitWalletTransactions(bool relay, bool force) // even if forcing. if (!fBroadcastTransactions) return; - // During reindex, importing and IBD, old wallet transactions become - // unconfirmed. Don't resend them as that would spam other nodes. - // We only allow forcing mempool submission when not relaying to avoid this spam. - if (!force && relay && !chain().isReadyToBroadcast()) return; - - // Do this infrequently and randomly to avoid giving away - // that these are our transactions. - if (!force && GetTime() < m_next_resend) return; // resend 12-36 hours from now, ~1 day on average. m_next_resend = GetTime() + (12 * 60 * 60) + GetRand(24 * 60 * 60); @@ -1979,6 +1988,7 @@ void CWallet::ResubmitWalletTransactions(bool relay, bool force) void MaybeResendWalletTxs(WalletContext& context) { for (const std::shared_ptr& pwallet : GetWallets(context)) { + if (!pwallet->ShouldResend()) continue; pwallet->ResubmitWalletTransactions(/*relay=*/true, /*force=*/false); } } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 6a1b76505c..9ee077b060 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -537,6 +537,8 @@ public: }; ScanResult ScanForWalletTransactions(const uint256& start_block, int start_height, std::optional max_height, const WalletRescanReserver& reserver, bool fUpdate, const bool save_progress); void transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason, uint64_t mempool_sequence) override; + /** Return true if all conditions for periodically resending transactions are met. */ + bool ShouldResend() const; void ResubmitWalletTransactions(bool relay, bool force); OutputType TransactionChangeType(const std::optional& change_type, const std::vector& vecSend) const; -- cgit v1.2.3 From 9245f456705b285e2d9afcc01a6155e1b3f92fad Mon Sep 17 00:00:00 2001 From: stickies-v Date: Fri, 30 Sep 2022 11:11:31 +0100 Subject: wallet: only update m_next_resend when actually resending We only want to relay our resubmitted transactions once every 12-36h. By separating the timer update logic out of ResubmitWalletTransactions and into MaybeResendWalletTxs we avoid non-relay calls (previously in the separate ReacceptWalletTransactions function) from resetting that timer. --- src/wallet/wallet.cpp | 10 +++++----- src/wallet/wallet.h | 6 +++++- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 6ef3c9b66a..2a4e6df54b 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include