Merge #15842: refactor: replace isPotentialtip/waitForNotifications by higher method
422677963a
refactor: replace isPotentialtip/waitForNotifications by higher method (Antoine Riard)edfe9438ca
Add WITH_LOCK macro: run code while locking a mutex (Antoine Riard) Pull request description: In Chain interface, instead of a isPotentialTip and a WaitForNotifications method, both used only once in CWallet::BlockUntilSyncedToCurrentChain, combine them in a higher WaitForNotificationsUpToTip method. Semantic should be unchanged, wallet wait for pending notifications to be processed unless block hash points to the current chain tip or a descendant. ACKs for commit 422677: jnewbery: ACK422677963a
ryanofsky: utACK422677963a
. Only change is adding the cs_wallet lock annotation. Tree-SHA512: 2834ff0218795ef607543fae822e5cce25d759c1a9cfcb1f896a4af03071faed5276fbe0966e0c6ed65dc0e88af161899c5b2ca358a2d24fe70969a550000bf2
This commit is contained in:
commit
0936f35f65
5 changed files with 29 additions and 37 deletions
|
@ -122,12 +122,6 @@ class LockImpl : public Chain::Lock
|
||||||
}
|
}
|
||||||
return nullopt;
|
return nullopt;
|
||||||
}
|
}
|
||||||
bool isPotentialTip(const uint256& hash) override
|
|
||||||
{
|
|
||||||
if (::chainActive.Tip()->GetBlockHash() == hash) return true;
|
|
||||||
CBlockIndex* block = LookupBlockIndex(hash);
|
|
||||||
return block && block->GetAncestor(::chainActive.Height()) == ::chainActive.Tip();
|
|
||||||
}
|
|
||||||
CBlockLocator getTipLocator() override { return ::chainActive.GetLocator(); }
|
CBlockLocator getTipLocator() override { return ::chainActive.GetLocator(); }
|
||||||
Optional<int> findLocatorFork(const CBlockLocator& locator) override
|
Optional<int> findLocatorFork(const CBlockLocator& locator) override
|
||||||
{
|
{
|
||||||
|
@ -343,7 +337,16 @@ public:
|
||||||
{
|
{
|
||||||
return MakeUnique<NotificationsHandlerImpl>(*this, notifications);
|
return MakeUnique<NotificationsHandlerImpl>(*this, notifications);
|
||||||
}
|
}
|
||||||
void waitForNotifications() override { SyncWithValidationInterfaceQueue(); }
|
void waitForNotificationsIfNewBlocksConnected(const uint256& old_tip) override
|
||||||
|
{
|
||||||
|
if (!old_tip.IsNull()) {
|
||||||
|
LOCK(::cs_main);
|
||||||
|
if (old_tip == ::chainActive.Tip()->GetBlockHash()) return;
|
||||||
|
CBlockIndex* block = LookupBlockIndex(old_tip);
|
||||||
|
if (block && block->GetAncestor(::chainActive.Height()) == ::chainActive.Tip()) return;
|
||||||
|
}
|
||||||
|
SyncWithValidationInterfaceQueue();
|
||||||
|
}
|
||||||
std::unique_ptr<Handler> handleRpc(const CRPCCommand& command) override
|
std::unique_ptr<Handler> handleRpc(const CRPCCommand& command) override
|
||||||
{
|
{
|
||||||
return MakeUnique<RpcHandlerImpl>(command);
|
return MakeUnique<RpcHandlerImpl>(command);
|
||||||
|
|
|
@ -43,12 +43,6 @@ class Wallet;
|
||||||
//! asynchronously
|
//! asynchronously
|
||||||
//! (https://github.com/bitcoin/bitcoin/pull/10973#issuecomment-380101269).
|
//! (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
|
//! * The relayTransactions() and submitToMemoryPool() methods could be replaced
|
||||||
//! with a higher-level broadcastTransaction method
|
//! with a higher-level broadcastTransaction method
|
||||||
//! (https://github.com/bitcoin/bitcoin/pull/14978#issuecomment-459373984).
|
//! (https://github.com/bitcoin/bitcoin/pull/14978#issuecomment-459373984).
|
||||||
|
@ -123,11 +117,6 @@ public:
|
||||||
//! information is desired).
|
//! information is desired).
|
||||||
virtual Optional<int> findFork(const uint256& hash, Optional<int>* height) = 0;
|
virtual Optional<int> findFork(const uint256& hash, Optional<int>* height) = 0;
|
||||||
|
|
||||||
//! Return true if block hash points to the current chain tip, or to a
|
|
||||||
//! possible descendant of the current chain tip that isn't currently
|
|
||||||
//! connected.
|
|
||||||
virtual bool isPotentialTip(const uint256& hash) = 0;
|
|
||||||
|
|
||||||
//! Get locator for the current chain tip.
|
//! Get locator for the current chain tip.
|
||||||
virtual CBlockLocator getTipLocator() = 0;
|
virtual CBlockLocator getTipLocator() = 0;
|
||||||
|
|
||||||
|
@ -256,8 +245,10 @@ public:
|
||||||
//! Register handler for notifications.
|
//! Register handler for notifications.
|
||||||
virtual std::unique_ptr<Handler> handleNotifications(Notifications& notifications) = 0;
|
virtual std::unique_ptr<Handler> handleNotifications(Notifications& notifications) = 0;
|
||||||
|
|
||||||
//! Wait for pending notifications to be handled.
|
//! Wait for pending notifications to be processed unless block hash points to the current
|
||||||
virtual void waitForNotifications() = 0;
|
//! chain tip, or to a possible descendant of the current chain tip that isn't currently
|
||||||
|
//! connected.
|
||||||
|
virtual void waitForNotificationsIfNewBlocksConnected(const uint256& old_tip) = 0;
|
||||||
|
|
||||||
//! Register handler for RPC. Command is not copied, so reference
|
//! Register handler for RPC. Command is not copied, so reference
|
||||||
//! needs to remain valid until Handler is disconnected.
|
//! needs to remain valid until Handler is disconnected.
|
||||||
|
|
10
src/sync.h
10
src/sync.h
|
@ -198,6 +198,16 @@ using DebugLock = UniqueLock<typename std::remove_reference<typename std::remove
|
||||||
LeaveCritical(); \
|
LeaveCritical(); \
|
||||||
}
|
}
|
||||||
|
|
||||||
|
//! Run code while locking a mutex.
|
||||||
|
//!
|
||||||
|
//! Examples:
|
||||||
|
//!
|
||||||
|
//! WITH_LOCK(cs, shared_val = shared_val + 1);
|
||||||
|
//!
|
||||||
|
//! int val = WITH_LOCK(cs, return shared_val);
|
||||||
|
//!
|
||||||
|
#define WITH_LOCK(cs, code) [&] { LOCK(cs); code; }()
|
||||||
|
|
||||||
class CSemaphore
|
class CSemaphore
|
||||||
{
|
{
|
||||||
private:
|
private:
|
||||||
|
|
|
@ -1287,24 +1287,12 @@ void CWallet::UpdatedBlockTip()
|
||||||
|
|
||||||
void CWallet::BlockUntilSyncedToCurrentChain() {
|
void CWallet::BlockUntilSyncedToCurrentChain() {
|
||||||
AssertLockNotHeld(cs_wallet);
|
AssertLockNotHeld(cs_wallet);
|
||||||
|
// Skip the queue-draining stuff if we know we're caught up with
|
||||||
{
|
// chainActive.Tip(), otherwise put a callback in the validation interface queue and wait
|
||||||
// Skip the queue-draining stuff if we know we're caught up with
|
|
||||||
// chainActive.Tip()...
|
|
||||||
// We could also take cs_wallet here, and call m_last_block_processed
|
|
||||||
// protected by cs_wallet instead of cs_main, but as long as we need
|
|
||||||
// cs_main here anyway, it's easier to just call it cs_main-protected.
|
|
||||||
auto locked_chain = chain().lock();
|
|
||||||
|
|
||||||
if (!m_last_block_processed.IsNull() && locked_chain->isPotentialTip(m_last_block_processed)) {
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// ...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
|
// for the queue to drain enough to execute it (indicating we are caught up
|
||||||
// at least with the time we entered this function).
|
// at least with the time we entered this function).
|
||||||
chain().waitForNotifications();
|
uint256 last_block_hash = WITH_LOCK(cs_wallet, return m_last_block_processed);
|
||||||
|
chain().waitForNotificationsIfNewBlocksConnected(last_block_hash);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
|
|
@ -693,7 +693,7 @@ private:
|
||||||
* to have seen all transactions in the chain, but is only used to track
|
* to have seen all transactions in the chain, but is only used to track
|
||||||
* live BlockConnected callbacks.
|
* live BlockConnected callbacks.
|
||||||
*/
|
*/
|
||||||
uint256 m_last_block_processed;
|
uint256 m_last_block_processed GUARDED_BY(cs_wallet);
|
||||||
|
|
||||||
public:
|
public:
|
||||||
/*
|
/*
|
||||||
|
|
Loading…
Reference in a new issue