aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRussell Yanofsky <russ@yanofsky.org>2017-07-30 16:00:56 -0400
committerRussell Yanofsky <russ@yanofsky.org>2019-03-05 10:20:00 -0400
commit91868e6288abf9d133620b585bc6de793a11e0e3 (patch)
tree19fd82b1cfacf752ea4802b6ae4c0dfda9a94c2e
parentd8a62db8bf68896397f175e1d7d52b7be5021985 (diff)
Remove use CValidationInterface in wallet code
This commit does not change behavior.
-rw-r--r--src/interfaces/chain.cpp56
-rw-r--r--src/interfaces/chain.h26
-rw-r--r--src/wallet/test/wallet_test_fixture.cpp7
-rw-r--r--src/wallet/test/wallet_test_fixture.h1
-rw-r--r--src/wallet/wallet.cpp28
-rw-r--r--src/wallet/wallet.h14
6 files changed, 107 insertions, 25 deletions
diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp
index 2eecea28d0..8ce718cc50 100644
--- a/src/interfaces/chain.cpp
+++ b/src/interfaces/chain.cpp
@@ -6,6 +6,7 @@
#include <chain.h>
#include <chainparams.h>
+#include <interfaces/handler.h>
#include <interfaces/wallet.h>
#include <net.h>
#include <policy/fees.h>
@@ -22,6 +23,7 @@
#include <uint256.h>
#include <util/system.h>
#include <validation.h>
+#include <validationinterface.h>
#include <memory>
#include <utility>
@@ -161,6 +163,55 @@ class LockingStateImpl : public LockImpl, public UniqueLock<CCriticalSection>
using UniqueLock::UniqueLock;
};
+class NotificationsHandlerImpl : public Handler, CValidationInterface
+{
+public:
+ explicit NotificationsHandlerImpl(Chain& chain, Chain::Notifications& notifications)
+ : m_chain(chain), m_notifications(&notifications)
+ {
+ RegisterValidationInterface(this);
+ }
+ ~NotificationsHandlerImpl() override { disconnect(); }
+ void disconnect() override
+ {
+ if (m_notifications) {
+ m_notifications = nullptr;
+ UnregisterValidationInterface(this);
+ }
+ }
+ void TransactionAddedToMempool(const CTransactionRef& tx) override
+ {
+ m_notifications->TransactionAddedToMempool(tx);
+ }
+ void TransactionRemovedFromMempool(const CTransactionRef& tx) override
+ {
+ m_notifications->TransactionRemovedFromMempool(tx);
+ }
+ void BlockConnected(const std::shared_ptr<const CBlock>& block,
+ const CBlockIndex* index,
+ const std::vector<CTransactionRef>& tx_conflicted) override
+ {
+ m_notifications->BlockConnected(*block, tx_conflicted);
+ }
+ void BlockDisconnected(const std::shared_ptr<const CBlock>& block) override
+ {
+ m_notifications->BlockDisconnected(*block);
+ }
+ void ChainStateFlushed(const CBlockLocator& locator) override { m_notifications->ChainStateFlushed(locator); }
+ void ResendWalletTransactions(int64_t best_block_time, 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, best_block_time);
+ }
+ Chain& m_chain;
+ Chain::Notifications* m_notifications;
+};
+
class ChainImpl : public Chain
{
public:
@@ -254,6 +305,11 @@ public:
void initWarning(const std::string& message) override { InitWarning(message); }
void initError(const std::string& message) override { InitError(message); }
void loadWallet(std::unique_ptr<Wallet> wallet) override { ::uiInterface.LoadWallet(wallet); }
+ std::unique_ptr<Handler> handleNotifications(Notifications& notifications) override
+ {
+ return MakeUnique<NotificationsHandlerImpl>(*this, notifications);
+ }
+ void waitForNotifications() override { SyncWithValidationInterfaceQueue(); }
};
} // namespace
diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h
index 037e8e9ff5..72862617b3 100644
--- a/src/interfaces/chain.h
+++ b/src/interfaces/chain.h
@@ -25,6 +25,7 @@ struct FeeCalculation;
namespace interfaces {
+class Handler;
class Wallet;
//! Interface giving clients (wallet processes, maybe other analysis tools in
@@ -40,6 +41,12 @@ class Wallet;
//! asynchronously
//! (https://github.com/bitcoin/bitcoin/pull/10973#issuecomment-380101269).
//!
+//! * The isPotentialTip() and waitForNotifications() methods are too low-level
+//! and should be replaced with a higher level
+//! waitForNotificationsUpTo(block_hash) method that the wallet can call
+//! instead
+//! (https://github.com/bitcoin/bitcoin/pull/10973#discussion_r266995234).
+//!
//! * The relayTransactions() and submitToMemoryPool() methods could be replaced
//! with a higher-level broadcastTransaction method
//! (https://github.com/bitcoin/bitcoin/pull/14978#issuecomment-459373984).
@@ -217,6 +224,25 @@ public:
//! Send wallet load notification to the GUI.
virtual void loadWallet(std::unique_ptr<Wallet> wallet) = 0;
+
+ //! Chain notifications.
+ class Notifications
+ {
+ public:
+ virtual ~Notifications() {}
+ virtual void TransactionAddedToMempool(const CTransactionRef& tx) {}
+ virtual void TransactionRemovedFromMempool(const CTransactionRef& ptx) {}
+ virtual void BlockConnected(const CBlock& block, const std::vector<CTransactionRef>& tx_conflicted) {}
+ virtual void BlockDisconnected(const CBlock& block) {}
+ virtual void ChainStateFlushed(const CBlockLocator& locator) {}
+ virtual void ResendWalletTransactions(Lock& locked_chain, int64_t best_block_time) {}
+ };
+
+ //! Register handler for notifications.
+ virtual std::unique_ptr<Handler> handleNotifications(Notifications& notifications) = 0;
+
+ //! Wait for pending notifications to be handled.
+ virtual void waitForNotifications() = 0;
};
//! Interface to let node manage chain clients (wallets, or maybe tools for
diff --git a/src/wallet/test/wallet_test_fixture.cpp b/src/wallet/test/wallet_test_fixture.cpp
index a5fb1db86c..6a9922b3cd 100644
--- a/src/wallet/test/wallet_test_fixture.cpp
+++ b/src/wallet/test/wallet_test_fixture.cpp
@@ -13,12 +13,7 @@ WalletTestingSetup::WalletTestingSetup(const std::string& chainName):
{
bool fFirstRun;
m_wallet.LoadWallet(fFirstRun);
- RegisterValidationInterface(&m_wallet);
+ m_wallet.m_chain_notifications_handler = m_chain->handleNotifications(m_wallet);
RegisterWalletRPCCommands(tableRPC);
}
-
-WalletTestingSetup::~WalletTestingSetup()
-{
- UnregisterValidationInterface(&m_wallet);
-}
diff --git a/src/wallet/test/wallet_test_fixture.h b/src/wallet/test/wallet_test_fixture.h
index e6fe8c9473..edc06a8351 100644
--- a/src/wallet/test/wallet_test_fixture.h
+++ b/src/wallet/test/wallet_test_fixture.h
@@ -17,7 +17,6 @@
*/
struct WalletTestingSetup: public TestingSetup {
explicit WalletTestingSetup(const std::string& chainName = CBaseChainParams::MAIN);
- ~WalletTestingSetup();
std::unique_ptr<interfaces::Chain> m_chain = interfaces::MakeChain();
CWallet m_wallet;
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 02f3a265d9..f6acec2d3e 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -96,7 +96,7 @@ static void ReleaseWallet(CWallet* wallet)
wallet->WalletLogPrintf("Releasing wallet\n");
wallet->BlockUntilSyncedToCurrentChain();
wallet->Flush();
- UnregisterValidationInterface(wallet);
+ wallet->m_chain_notifications_handler.reset();
delete wallet;
// Wallet is now released, notify UnloadWallet, if any.
{
@@ -1243,7 +1243,8 @@ void CWallet::TransactionRemovedFromMempool(const CTransactionRef &ptx) {
}
}
-void CWallet::BlockConnected(const std::shared_ptr<const CBlock>& pblock, const CBlockIndex *pindex, const std::vector<CTransactionRef>& vtxConflicted) {
+void CWallet::BlockConnected(const CBlock& block, const std::vector<CTransactionRef>& vtxConflicted) {
+ const uint256& block_hash = block.GetHash();
auto locked_chain = chain().lock();
LOCK(cs_wallet);
// TODO: Temporarily ensure that mempool removals are notified before
@@ -1258,19 +1259,19 @@ void CWallet::BlockConnected(const std::shared_ptr<const CBlock>& pblock, const
SyncTransaction(ptx, {} /* block hash */, 0 /* position in block */);
TransactionRemovedFromMempool(ptx);
}
- for (size_t i = 0; i < pblock->vtx.size(); i++) {
- SyncTransaction(pblock->vtx[i], pindex->GetBlockHash(), i);
- TransactionRemovedFromMempool(pblock->vtx[i]);
+ for (size_t i = 0; i < block.vtx.size(); i++) {
+ SyncTransaction(block.vtx[i], block_hash, i);
+ TransactionRemovedFromMempool(block.vtx[i]);
}
- m_last_block_processed = pindex->GetBlockHash();
+ m_last_block_processed = block_hash;
}
-void CWallet::BlockDisconnected(const std::shared_ptr<const CBlock>& pblock) {
+void CWallet::BlockDisconnected(const CBlock& block) {
auto locked_chain = chain().lock();
LOCK(cs_wallet);
- for (const CTransactionRef& ptx : pblock->vtx) {
+ for (const CTransactionRef& ptx : block.vtx) {
SyncTransaction(ptx, {} /* block hash */, 0 /* position in block */);
}
}
@@ -1297,7 +1298,7 @@ void CWallet::BlockUntilSyncedToCurrentChain() {
// ...otherwise put a callback in the validation interface queue and wait
// for the queue to drain enough to execute it (indicating we are caught up
// at least with the time we entered this function).
- SyncWithValidationInterfaceQueue();
+ chain().waitForNotifications();
}
@@ -2137,7 +2138,7 @@ std::vector<uint256> CWallet::ResendWalletTransactionsBefore(interfaces::Chain::
return result;
}
-void CWallet::ResendWalletTransactions(int64_t nBestBlockTime, CConnman* connman)
+void CWallet::ResendWalletTransactions(interfaces::Chain::Lock& locked_chain, int64_t nBestBlockTime)
{
// Do this infrequently and randomly to avoid giving away
// that these are our transactions.
@@ -2155,8 +2156,7 @@ void CWallet::ResendWalletTransactions(int64_t nBestBlockTime, CConnman* connman
// Rebroadcast unconfirmed txes older than 5 minutes before the last
// block was found:
- auto locked_chain = chain().assumeLocked(); // Temporary. Removed in upcoming lock cleanup
- std::vector<uint256> relayed = ResendWalletTransactionsBefore(*locked_chain, nBestBlockTime-5*60);
+ std::vector<uint256> relayed = ResendWalletTransactionsBefore(locked_chain, nBestBlockTime-5*60);
if (!relayed.empty())
WalletLogPrintf("%s: rebroadcast %u unconfirmed transactions\n", __func__, relayed.size());
}
@@ -4385,8 +4385,8 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain,
chain.loadWallet(interfaces::MakeWallet(walletInstance));
- // Register with the validation interface. It's ok to do this after rescan since we're still holding cs_main.
- RegisterValidationInterface(walletInstance.get());
+ // Register with the validation interface. It's ok to do this after rescan since we're still holding locked_chain.
+ walletInstance->m_chain_notifications_handler = chain.handleNotifications(*walletInstance);
walletInstance->SetBroadcastTransactions(gArgs.GetBoolArg("-walletbroadcast", DEFAULT_WALLETBROADCAST));
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index 51e3edac34..02f1788ddb 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -8,6 +8,7 @@
#include <amount.h>
#include <interfaces/chain.h>
+#include <interfaces/handler.h>
#include <outputtype.h>
#include <policy/feerate.h>
#include <streams.h>
@@ -637,7 +638,7 @@ class WalletRescanReserver; //forward declarations for ScanForWalletTransactions
* A CWallet is an extension of a keystore, which also maintains a set of transactions and balances,
* and provides the ability to create new transactions.
*/
-class CWallet final : public CCryptoKeyStore, public CValidationInterface
+class CWallet final : public CCryptoKeyStore, private interfaces::Chain::Notifications
{
private:
std::atomic<bool> fAbortRescan{false};
@@ -808,6 +809,9 @@ public:
std::set<COutPoint> setLockedCoins GUARDED_BY(cs_wallet);
+ /** Registered interfaces::Chain::Notifications handler. */
+ std::unique_ptr<interfaces::Handler> m_chain_notifications_handler;
+
/** Interface for accessing chain state. */
interfaces::Chain& chain() const { return m_chain; }
@@ -920,8 +924,8 @@ public:
bool AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose=true);
void LoadToWallet(const CWalletTx& wtxIn) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
void TransactionAddedToMempool(const CTransactionRef& tx) override;
- void BlockConnected(const std::shared_ptr<const CBlock>& pblock, const CBlockIndex *pindex, const std::vector<CTransactionRef>& vtxConflicted) override;
- void BlockDisconnected(const std::shared_ptr<const CBlock>& pblock) override;
+ void BlockConnected(const CBlock& block, const std::vector<CTransactionRef>& vtxConflicted) override;
+ void BlockDisconnected(const CBlock& block) override;
int64_t RescanFromTime(int64_t startTime, const WalletRescanReserver& reserver, bool update);
struct ScanResult {
@@ -942,7 +946,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();
- void ResendWalletTransactions(int64_t nBestBlockTime, CConnman* connman) override EXCLUSIVE_LOCKS_REQUIRED(cs_main);
+ void ResendWalletTransactions(interfaces::Chain::Lock& locked_chain, int64_t nBestBlockTime) override;
// ResendWalletTransactionsBefore may only be called if fBroadcastTransactions!
std::vector<uint256> ResendWalletTransactionsBefore(interfaces::Chain::Lock& locked_chain, int64_t nTime);
CAmount GetBalance(const isminefilter& filter=ISMINE_SPENDABLE, const int min_depth=0) const;
@@ -1220,6 +1224,8 @@ public:
/** Add a KeyOriginInfo to the wallet */
bool AddKeyOrigin(const CPubKey& pubkey, const KeyOriginInfo& info);
+
+ friend struct WalletTestingSetup;
};
/** A key allocated from the key pool. */