aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--src/interfaces/chain.cpp11
-rw-r--r--src/interfaces/chain.h4
-rw-r--r--src/net_processing.cpp8
-rw-r--r--src/validationinterface.cpp7
-rw-r--r--src/validationinterface.h3
-rw-r--r--src/wallet/init.cpp3
-rw-r--r--src/wallet/wallet.cpp21
-rw-r--r--src/wallet/wallet.h8
-rwxr-xr-xtest/functional/wallet_resendwallettransactions.py6
9 files changed, 39 insertions, 32 deletions
diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp
index 6c918b73f9..f278e5de95 100644
--- a/src/interfaces/chain.cpp
+++ b/src/interfaces/chain.cpp
@@ -207,16 +207,6 @@ public:
m_notifications->UpdatedBlockTip();
}
void ChainStateFlushed(const CBlockLocator& locator) override { m_notifications->ChainStateFlushed(locator); }
- void ResendWalletTransactions(CConnman*) override
- {
- // `cs_main` is always held when this method is called, so it is safe to
- // call `assumeLocked`. This is awkward, and the `assumeLocked` method
- // should be able to be removed entirely if `ResendWalletTransactions`
- // is replaced by a wallet timer as suggested in
- // https://github.com/bitcoin/bitcoin/issues/15619
- auto locked_chain = m_chain.assumeLocked();
- m_notifications->ResendWalletTransactions(*locked_chain);
- }
Chain& m_chain;
Chain::Notifications* m_notifications;
};
@@ -351,6 +341,7 @@ public:
CAmount maxTxFee() override { return ::maxTxFee; }
bool getPruneMode() override { return ::fPruneMode; }
bool p2pEnabled() override { return g_connman != nullptr; }
+ bool isReadyToBroadcast() override { return !::fImporting && !::fReindex && !IsInitialBlockDownload(); }
bool isInitialBlockDownload() override { return IsInitialBlockDownload(); }
bool shutdownRequested() override { return ShutdownRequested(); }
int64_t getAdjustedTime() override { return GetAdjustedTime(); }
diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h
index 82e2c7eb23..4e0ef49f6c 100644
--- a/src/interfaces/chain.h
+++ b/src/interfaces/chain.h
@@ -223,6 +223,9 @@ public:
//! Check if p2p enabled.
virtual bool p2pEnabled() = 0;
+ //! Check if the node is ready to broadcast transactions.
+ virtual bool isReadyToBroadcast() = 0;
+
//! Check if in IBD.
virtual bool isInitialBlockDownload() = 0;
@@ -258,7 +261,6 @@ public:
virtual void BlockDisconnected(const CBlock& block) {}
virtual void UpdatedBlockTip() {}
virtual void ChainStateFlushed(const CBlockLocator& locator) {}
- virtual void ResendWalletTransactions(Lock& locked_chain) {}
};
//! Register handler for notifications.
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 3fd3068fbd..044fcc90cd 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -3546,14 +3546,6 @@ bool PeerLogicValidation::SendMessages(CNode* pto)
}
}
- // Resend wallet transactions that haven't gotten in a block yet
- // Except during reindex, importing and IBD, when old wallet
- // transactions become unconfirmed and spams other nodes.
- if (!fReindex && !fImporting && !IsInitialBlockDownload())
- {
- GetMainSignals().Broadcast(connman);
- }
-
//
// Try sending block announcements via headers
//
diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp
index abd229d561..5d0ee1d1fc 100644
--- a/src/validationinterface.cpp
+++ b/src/validationinterface.cpp
@@ -25,7 +25,6 @@ struct ValidationInterfaceConnections {
boost::signals2::scoped_connection BlockDisconnected;
boost::signals2::scoped_connection TransactionRemovedFromMempool;
boost::signals2::scoped_connection ChainStateFlushed;
- boost::signals2::scoped_connection Broadcast;
boost::signals2::scoped_connection BlockChecked;
boost::signals2::scoped_connection NewPoWValidBlock;
};
@@ -37,7 +36,6 @@ struct MainSignalsInstance {
boost::signals2::signal<void (const std::shared_ptr<const CBlock> &)> BlockDisconnected;
boost::signals2::signal<void (const CTransactionRef &)> TransactionRemovedFromMempool;
boost::signals2::signal<void (const CBlockLocator &)> ChainStateFlushed;
- boost::signals2::signal<void (CConnman* connman)> Broadcast;
boost::signals2::signal<void (const CBlock&, const CValidationState&)> BlockChecked;
boost::signals2::signal<void (const CBlockIndex *, const std::shared_ptr<const CBlock>&)> NewPoWValidBlock;
@@ -101,7 +99,6 @@ void RegisterValidationInterface(CValidationInterface* pwalletIn) {
conns.BlockDisconnected = g_signals.m_internals->BlockDisconnected.connect(std::bind(&CValidationInterface::BlockDisconnected, pwalletIn, std::placeholders::_1));
conns.TransactionRemovedFromMempool = g_signals.m_internals->TransactionRemovedFromMempool.connect(std::bind(&CValidationInterface::TransactionRemovedFromMempool, pwalletIn, std::placeholders::_1));
conns.ChainStateFlushed = g_signals.m_internals->ChainStateFlushed.connect(std::bind(&CValidationInterface::ChainStateFlushed, pwalletIn, std::placeholders::_1));
- conns.Broadcast = g_signals.m_internals->Broadcast.connect(std::bind(&CValidationInterface::ResendWalletTransactions, pwalletIn, std::placeholders::_1));
conns.BlockChecked = g_signals.m_internals->BlockChecked.connect(std::bind(&CValidationInterface::BlockChecked, pwalletIn, std::placeholders::_1, std::placeholders::_2));
conns.NewPoWValidBlock = g_signals.m_internals->NewPoWValidBlock.connect(std::bind(&CValidationInterface::NewPoWValidBlock, pwalletIn, std::placeholders::_1, std::placeholders::_2));
}
@@ -175,10 +172,6 @@ void CMainSignals::ChainStateFlushed(const CBlockLocator &locator) {
});
}
-void CMainSignals::Broadcast(CConnman* connman) {
- m_internals->Broadcast(connman);
-}
-
void CMainSignals::BlockChecked(const CBlock& block, const CValidationState& state) {
m_internals->BlockChecked(block, state);
}
diff --git a/src/validationinterface.h b/src/validationinterface.h
index 0d5bdd60fb..78f3026a80 100644
--- a/src/validationinterface.h
+++ b/src/validationinterface.h
@@ -134,8 +134,6 @@ protected:
* Called on a background thread.
*/
virtual void ChainStateFlushed(const CBlockLocator &locator) {}
- /** Tells listeners to broadcast their data. */
- virtual void ResendWalletTransactions(CConnman* connman) {}
/**
* Notifies listeners of a block validation result.
* If the provided CValidationState IsValid, the provided block
@@ -184,7 +182,6 @@ public:
void BlockConnected(const std::shared_ptr<const CBlock> &, const CBlockIndex *pindex, const std::shared_ptr<const std::vector<CTransactionRef>> &);
void BlockDisconnected(const std::shared_ptr<const CBlock> &);
void ChainStateFlushed(const CBlockLocator &);
- void Broadcast(CConnman* connman);
void BlockChecked(const CBlock&, const CValidationState&);
void NewPoWValidBlock(const CBlockIndex *, const std::shared_ptr<const CBlock>&);
};
diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp
index 76a7eaa681..a8096dc671 100644
--- a/src/wallet/init.cpp
+++ b/src/wallet/init.cpp
@@ -211,8 +211,9 @@ void StartWallets(CScheduler& scheduler)
pwallet->postInitProcess();
}
- // Run a thread to flush wallet periodically
+ // Schedule periodic wallet flushes and tx rebroadcasts
scheduler.scheduleEvery(MaybeCompactWalletDB, 500);
+ scheduler.scheduleEvery(MaybeResendWalletTxs, 1000);
}
void FlushWallets()
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 243b51a1e6..cf477982ca 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -2114,8 +2114,21 @@ bool CWalletTx::IsEquivalentTo(const CWalletTx& _tx) const
return CTransaction(tx1) == CTransaction(tx2);
}
+// Rebroadcast transactions from the wallet. We do this on a random timer
+// to slightly obfuscate which transactions come from our wallet.
+//
+// Ideally, we'd only resend transactions that we think should have been
+// mined in the most recent block. Any transaction that wasn't in the top
+// blockweight of transactions in the mempool shouldn't have been mined,
+// and so is probably just sitting in the mempool waiting to be confirmed.
+// Rebroadcasting does nothing to speed up confirmation and only damages
+// privacy.
void CWallet::ResendWalletTransactions(interfaces::Chain::Lock& locked_chain)
{
+ // During reindex, importing and IBD, old wallet transactions become
+ // unconfirmed. Don't resend them as that would spam other nodes.
+ if (!chain().isReadyToBroadcast()) return;
+
// Do this infrequently and randomly to avoid giving away
// that these are our transactions.
if (GetTime() < nNextResend || !fBroadcastTransactions) return;
@@ -2149,7 +2162,13 @@ void CWallet::ResendWalletTransactions(interfaces::Chain::Lock& locked_chain)
/** @} */ // end of mapWallet
-
+void MaybeResendWalletTxs()
+{
+ for (const std::shared_ptr<CWallet>& pwallet : GetWallets()) {
+ auto locked_chain = pwallet->chain().lock();
+ pwallet->ResendWalletTransactions(*locked_chain);
+ }
+}
/** @defgroup Actions
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index cef58c6419..a0c5f63cee 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -949,7 +949,7 @@ public:
ScanResult ScanForWalletTransactions(const uint256& first_block, const uint256& last_block, const WalletRescanReserver& reserver, bool fUpdate);
void TransactionRemovedFromMempool(const CTransactionRef &ptx) override;
void ReacceptWalletTransactions(interfaces::Chain::Lock& locked_chain) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
- void ResendWalletTransactions(interfaces::Chain::Lock& locked_chain) override;
+ void ResendWalletTransactions(interfaces::Chain::Lock& locked_chain);
struct Balance {
CAmount m_mine_trusted{0}; //!< Trusted, at depth=GetBalance.min_depth or more
CAmount m_mine_untrusted_pending{0}; //!< Untrusted, but in mempool (pending)
@@ -1232,6 +1232,12 @@ public:
friend struct WalletTestingSetup;
};
+/**
+ * Called periodically by the schedule thread. Prompts individual wallets to resend
+ * their transactions. Actual rebroadcast schedule is managed by the wallets themselves.
+ */
+void MaybeResendWalletTxs();
+
/** A key allocated from the key pool. */
class CReserveKey final : public CReserveScript
{
diff --git a/test/functional/wallet_resendwallettransactions.py b/test/functional/wallet_resendwallettransactions.py
index 8aafa94c2e..ec5e230e5a 100755
--- a/test/functional/wallet_resendwallettransactions.py
+++ b/test/functional/wallet_resendwallettransactions.py
@@ -39,6 +39,12 @@ class ResendWalletTransactionsTest(BitcoinTestFramework):
self.log.info("Create a new transaction and wait until it's broadcast")
txid = int(node.sendtoaddress(node.getnewaddress(), 1), 16)
+ # Wallet rebroadcast is first scheduled 1 sec after startup (see
+ # nNextResend in ResendWalletTransactions()). Sleep for just over a
+ # second to be certain that it has been called before the first
+ # setmocktime call below.
+ time.sleep(1.1)
+
# Can take a few seconds due to transaction trickling
wait_until(lambda: node.p2p.tx_invs_received[txid] >= 1, lock=mininode_lock)