Skip to content

Commit 3ab3335

Browse files
ryanofskyryanofsky
committed
Remove use CValidationInterface in wallet code
This commit does not change behavior.
1 parent 64ad91d commit 3ab3335

File tree

5 files changed

+110
-20
lines changed

5 files changed

+110
-20
lines changed

‎src/ipc/interfaces.h‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ struct CBlockLocator;
1717

1818
namespace ipc {
1919

20+
class Handler;
21+
2022
//! Interface for giving wallet processes access to blockchain state.
2123
class Chain
2224
{
@@ -177,6 +179,30 @@ class Chain
177179
//! Send init error.
178180
virtual bool initError(const std::string& message) = 0;
179181

182+
//! Chain notifications.
183+
class Notifications
184+
{
185+
public:
186+
virtual ~Notifications() {}
187+
virtual void TransactionAddedToMempool(const CTransactionRef& tx) {}
188+
virtual void TransactionRemovedFromMempool(const CTransactionRef& ptx) {}
189+
virtual void BlockConnected(const CBlock& block,
190+
const uint256& block_hash,
191+
const std::vector<CTransactionRef>& tx_conflicted)
192+
{
193+
}
194+
virtual void BlockDisconnected(const CBlock& block) {}
195+
virtual void SetBestChain(const CBlockLocator& locator) {}
196+
virtual void Inventory(const uint256& hash) {}
197+
virtual void ResendWalletTransactions(int64_t best_block_time) {}
198+
};
199+
200+
//! Register handler for notifications.
201+
virtual std::unique_ptr<Handler> handleNotifications(Notifications& notifications) = 0;
202+
203+
//! Wait for pending notifications to be handled.
204+
virtual void waitForNotifications() = 0;
205+
180206
//! Interface to let node manage chain clients (wallets, or maybe tools for
181207
//! monitoring and analysis in the future).
182208
class Client
@@ -205,6 +231,16 @@ class Chain
205231
using Clients = std::vector<std::unique_ptr<Client>>;
206232
};
207233

234+
//! Interface for managing a registered handler.
235+
class Handler
236+
{
237+
public:
238+
virtual ~Handler() {}
239+
240+
//! Disconnect the handler.
241+
virtual void disconnect() = 0;
242+
};
243+
208244
//! Protocol IPC interface should use to communicate with implementation.
209245
enum Protocol {
210246
LOCAL, //!< Call functions linked into current executable.

‎src/ipc/local/bitcoind.cpp‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#include <policy/policy.h>
88
#include <ui_interface.h>
99
#include <validation.h>
10+
#include <validationinterface.h>
1011

1112
#if defined(HAVE_CONFIG_H)
1213
#include <config/bitcoin-config.h>
@@ -19,6 +20,8 @@
1920
#define CHECK_WALLET(x) throw std::logic_error("Wallet function called in non-wallet build.")
2021
#endif
2122

23+
#include <future>
24+
2225
namespace ipc {
2326
namespace local {
2427
namespace {
@@ -108,6 +111,48 @@ class LockingStateImpl : public LockedStateImpl, public CCriticalBlock
108111
using CCriticalBlock::CCriticalBlock;
109112
};
110113

114+
class HandlerImpl : public Handler, private CValidationInterface
115+
{
116+
public:
117+
HandlerImpl(Chain::Notifications& notifications) : m_notifications(notifications)
118+
{
119+
RegisterValidationInterface(this);
120+
}
121+
~HandlerImpl() override
122+
{
123+
// Don't call UnregisterValidationInterface here because it would try to
124+
// access virtual methods on this object which can't be accessed during
125+
// destruction. Also UnregisterAllValidationInterfaces is already called
126+
// at this point, so unregistering this object would be redundant.
127+
}
128+
void disconnect() override { UnregisterValidationInterface(this); }
129+
void TransactionAddedToMempool(const CTransactionRef& tx) override
130+
{
131+
m_notifications.TransactionAddedToMempool(tx);
132+
}
133+
void TransactionRemovedFromMempool(const CTransactionRef& tx) override
134+
{
135+
m_notifications.TransactionRemovedFromMempool(tx);
136+
}
137+
void BlockConnected(const std::shared_ptr<const CBlock>& block,
138+
const CBlockIndex* index,
139+
const std::vector<CTransactionRef>& tx_conflicted) override
140+
{
141+
m_notifications.BlockConnected(*block, index->GetBlockHash(), tx_conflicted);
142+
}
143+
void BlockDisconnected(const std::shared_ptr<const CBlock>& block) override
144+
{
145+
m_notifications.BlockDisconnected(*block);
146+
}
147+
void SetBestChain(const CBlockLocator& locator) override { m_notifications.SetBestChain(locator); }
148+
void Inventory(const uint256& hash) override { m_notifications.Inventory(hash); }
149+
void ResendWalletTransactions(int64_t best_block_time, CConnman* connman) override
150+
{
151+
m_notifications.ResendWalletTransactions(best_block_time);
152+
}
153+
Chain::Notifications& m_notifications;
154+
};
155+
111156
class ChainImpl : public Chain
112157
{
113158
public:
@@ -201,6 +246,16 @@ class ChainImpl : public Chain
201246
void initMessage(const std::string& message) override { ::uiInterface.InitMessage(message); }
202247
void initWarning(const std::string& message) override { InitWarning(message); }
203248
bool initError(const std::string& message) override { return InitError(message); }
249+
std::unique_ptr<Handler> handleNotifications(Notifications& notifications) override
250+
{
251+
return MakeUnique<HandlerImpl>(notifications);
252+
}
253+
void waitForNotifications() override
254+
{
255+
std::promise<void> promise;
256+
CallFunctionInValidationInterfaceQueue([&promise] { promise.set_value(); });
257+
promise.get_future().wait();
258+
}
204259
};
205260

206261
} // namespace

‎src/wallet/test/wallet_test_fixture.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,14 @@ WalletTestingSetup::WalletTestingSetup(const std::string& chainName):
2020
std::unique_ptr<CWalletDBWrapper> dbw(new CWalletDBWrapper(&bitdb, "wallet_test.dat"));
2121
pwalletMain = new CWallet(m_ipc_chain.get(), std::move(dbw));
2222
pwalletMain->LoadWallet(fFirstRun);
23-
RegisterValidationInterface(pwalletMain);
23+
pwalletMain->m_ipc_handler = m_ipc_chain->handleNotifications(*pwalletMain);
2424

2525
RegisterWalletRPCCommands(tableRPC);
2626
}
2727

2828
WalletTestingSetup::~WalletTestingSetup()
2929
{
30-
UnregisterValidationInterface(pwalletMain);
30+
pwalletMain->m_ipc_handler->disconnect();
3131
delete pwalletMain;
3232
pwalletMain = nullptr;
3333

‎src/wallet/wallet.cpp‎

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1200,7 +1200,7 @@ void CWallet::TransactionRemovedFromMempool(const CTransactionRef &ptx) {
12001200
}
12011201
}
12021202

1203-
void CWallet::BlockConnected(const std::shared_ptr<const CBlock>& pblock, const CBlockIndex *pindex, const std::vector<CTransactionRef>& vtxConflicted) {
1203+
void CWallet::BlockConnected(const CBlock& block, const uint256& block_hash, const std::vector<CTransactionRef>& vtxConflicted) {
12041204
auto ipc_locked = m_ipc_chain->lockState();
12051205
LOCK(cs_wallet);
12061206
// TODO: Temporarily ensure that mempool removals are notified before
@@ -1215,19 +1215,19 @@ void CWallet::BlockConnected(const std::shared_ptr<const CBlock>& pblock, const
12151215
SyncTransaction(*ipc_locked, ptx, {} /* block hash */, 0 /* position in block */);
12161216
TransactionRemovedFromMempool(ptx);
12171217
}
1218-
for (size_t i = 0; i < pblock->vtx.size(); i++) {
1219-
SyncTransaction(*ipc_locked, pblock->vtx[i], pindex->GetBlockHash(), i);
1220-
TransactionRemovedFromMempool(pblock->vtx[i]);
1218+
for (size_t i = 0; i < block.vtx.size(); i++) {
1219+
SyncTransaction(*ipc_locked, block.vtx[i], block_hash, i);
1220+
TransactionRemovedFromMempool(block.vtx[i]);
12211221
}
12221222

1223-
m_last_block_processed = pindex->GetBlockHash();
1223+
m_last_block_processed = block_hash;
12241224
}
12251225

1226-
void CWallet::BlockDisconnected(const std::shared_ptr<const CBlock>& pblock) {
1226+
void CWallet::BlockDisconnected(const CBlock& block) {
12271227
auto ipc_locked = m_ipc_chain->lockState();
12281228
LOCK(cs_wallet);
12291229

1230-
for (const CTransactionRef& ptx : pblock->vtx) {
1230+
for (const CTransactionRef& ptx : block.vtx) {
12311231
SyncTransaction(*ipc_locked, ptx, {} /* block hash */, 0 /* position in block */);
12321232
}
12331233
}
@@ -1247,11 +1247,7 @@ void CWallet::BlockUntilSyncedToCurrentChain() {
12471247
}
12481248
}
12491249

1250-
std::promise<void> promise;
1251-
CallFunctionInValidationInterfaceQueue([&promise] {
1252-
promise.set_value();
1253-
});
1254-
promise.get_future().wait();
1250+
m_ipc_chain->waitForNotifications();
12551251
}
12561252

12571253

@@ -1940,7 +1936,7 @@ std::vector<uint256> CWallet::ResendWalletTransactionsBefore(ipc::Chain::LockedS
19401936
return result;
19411937
}
19421938

1943-
void CWallet::ResendWalletTransactions(int64_t nBestBlockTime, CConnman* connman)
1939+
void CWallet::ResendWalletTransactions(int64_t nBestBlockTime)
19441940
{
19451941
// Do this infrequently and randomly to avoid giving away
19461942
// that these are our transactions.
@@ -3937,7 +3933,7 @@ CWallet* CWallet::CreateWalletFromFile(ipc::Chain& ipc_chain, const std::string
39373933
walletInstance->m_last_block_processed.SetNull();
39383934
}
39393935

3940-
RegisterValidationInterface(walletInstance);
3936+
walletInstance->m_ipc_handler = ipc_chain.handleNotifications(*walletInstance);
39413937

39423938
const int tip_height = ipc_locked->getHeight();
39433939
if (tip_height >= 0 && tip_height != index_rescan)

‎src/wallet/wallet.h‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -654,7 +654,7 @@ class CAccountingEntry
654654
* A CWallet is an extension of a keystore, which also maintains a set of transactions and balances,
655655
* and provides the ability to create new transactions.
656656
*/
657-
class CWallet final : public CCryptoKeyStore, public CValidationInterface
657+
class CWallet final : public CCryptoKeyStore, private ipc::Chain::Notifications
658658
{
659659
private:
660660
static std::atomic<bool> fFlushScheduled;
@@ -727,6 +727,7 @@ class CWallet final : public CCryptoKeyStore, public CValidationInterface
727727
* IPC interface for accessing blockchain state.
728728
*/
729729
ipc::Chain* m_ipc_chain = nullptr;
730+
std::unique_ptr<ipc::Handler> m_ipc_handler;
730731
std::unique_ptr<CWalletDBWrapper> dbw;
731732

732733
/**
@@ -935,14 +936,14 @@ class CWallet final : public CCryptoKeyStore, public CValidationInterface
935936
bool AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose=true);
936937
bool LoadToWallet(const CWalletTx& wtxIn);
937938
void TransactionAddedToMempool(const CTransactionRef& tx) override;
938-
void BlockConnected(const std::shared_ptr<const CBlock>& pblock, const CBlockIndex *pindex, const std::vector<CTransactionRef>& vtxConflicted) override;
939-
void BlockDisconnected(const std::shared_ptr<const CBlock>& pblock) override;
939+
void BlockConnected(const CBlock& block, const uint256& block_hash, const std::vector<CTransactionRef>& vtxConflicted) override;
940+
void BlockDisconnected(const CBlock& block) override;
940941
bool AddToWalletIfInvolvingMe(ipc::Chain::LockedState& ipc_locked, const CTransactionRef& tx, const uint256& block_hash, int posInBlock, bool fUpdate);
941942
int64_t RescanFromTime(ipc::Chain::LockedState& ipc_locked, int64_t startTime, bool update);
942943
int ScanForWalletTransactions(ipc::Chain::LockedState& ipc_locked, int start_height, bool fUpdate = false);
943944
void TransactionRemovedFromMempool(const CTransactionRef &ptx) override;
944945
void ReacceptWalletTransactions();
945-
void ResendWalletTransactions(int64_t nBestBlockTime, CConnman* connman) override;
946+
void ResendWalletTransactions(int64_t nBestBlockTime) override;
946947
// ResendWalletTransactionsBefore may only be called if fBroadcastTransactions!
947948
std::vector<uint256> ResendWalletTransactionsBefore(ipc::Chain::LockedState& ipc_locked, int64_t nTime);
948949
CAmount GetBalance() const;
@@ -1135,6 +1136,8 @@ class CWallet final : public CCryptoKeyStore, public CValidationInterface
11351136
* deadlock
11361137
*/
11371138
void BlockUntilSyncedToCurrentChain();
1139+
1140+
friend class WalletTestingSetup;
11381141
};
11391142

11401143
CFeeRate GetDiscardRate(const CBlockPolicyEstimator& estimator);

0 commit comments

Comments
 (0)