highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-11-11 22:50:59 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge #17954: wallet: Remove calls to Chain::Lock methods

Browse Source 
48973402d8 wallet: Avoid use of Chain::Lock in CWallet::GetKeyBirthTimes (Russell Yanofsky)
e958ff9ab5 wallet: Avoid use of Chain::Lock in CWallet::CreateTransaction (Russell Yanofsky)
c0d07dc4cb wallet: Avoid use of Chain::Lock in CWallet::ScanForWalletTransactions (Russell Yanofsky)
1be8ff280c wallet: Avoid use of Chain::Lock in rescanblockchain (Russell Yanofsky)
3cb85ac594 wallet refactor: Avoid use of Chain::Lock in CWallet::RescanFromTime (Russell Yanofsky)
f7ba881bc6 wallet: Avoid use of Chain::Lock in listsinceblock (Russell Yanofsky)
bc96a9bfc6 wallet: Avoid use of Chain::Lock in importmulti (Russell Yanofsky)
25a9fcf9e5 wallet: Avoid use of Chain::Lock in importwallet and dumpwallet (Russell Yanofsky)
c1694ce6bb wallet: Avoid use of Chain::Lock in importprunedfunds (Russell Yanofsky)
ade5f87971 wallet refactor: Avoid use of Chain::Lock in qt wallettests (Russell Yanofsky)
f6da44ccce wallet: Avoid use of Chain::Lock in tryGetTxStatus and tryGetBalances (Russell Yanofsky)
bf30cd4922 refactor: Add interfaces::FoundBlock class to selectively return block data (Russell Yanofsky)

Pull request description:

  This is a set of changes updating wallet code to make fewer calls to `Chain::Lock` methods, so the `Chain::Lock` class will be easier to remove in #16426 with fewer code changes and small changes to behavior.

ACKs for top commit:
  MarcoFalke:
    re-ACK 48973402d8, only change is fixing bug 📀
  fjahr:
    re-ACK 48973402d8, reviewed rebase and changes since last review, built and ran tests locally
  ariard:
    Coce Review ACK 4897340, only changes are one suggested by last review on more accurate variable naming, human-readable output, args comments in `findCommonAncestor`

Tree-SHA512: cfd2f559f976b6faaa032794c40c9659191d5597b013abcb6c7968d36b2abb2b14d4e596f8ed8b9a077e96522365261299a241a939b3111eaf729ba0c3ef519b
This commit is contained in:
MarcoFalke
2020-04-14 07:18:02 -04:00
parent 6110ae8326 48973402d8
commit 4702cadca9
12 changed files with 409 additions and 199 deletions
Download Patch File Download Diff File Expand all files Collapse all files

118
src/interfaces/chain.cpp
View File

@@ -38,6 +38,21 @@
namespace interfaces {
namespace {
bool FillBlock(const CBlockIndex* index, const FoundBlock& block, UniqueLock<RecursiveMutex>& lock)
{
if (!index) return false;
if (block.m_hash) *block.m_hash = index->GetBlockHash();
if (block.m_height) *block.m_height = index->nHeight;
if (block.m_time) *block.m_time = index->GetBlockTime();
if (block.m_max_time) *block.m_max_time = index->GetBlockTimeMax();
if (block.m_mtp_time) *block.m_mtp_time = index->GetMedianTimePast();
if (block.m_data) {
REVERSE_LOCK(lock);
if (!ReadBlockFromDisk(*block.m_data, index, Params().GetConsensus())) block.m_data->SetNull();
}
return true;
}
class LockImpl : public Chain::Lock, public UniqueLock<RecursiveMutex>
{
Optional<int> getHeight() override
@@ -65,20 +80,6 @@ class LockImpl : public Chain::Lock, public UniqueLock<RecursiveMutex>
assert(block != nullptr);
return block->GetBlockHash();
}
int64_t getBlockTime(int height) override
{
LockAssertion lock(::cs_main);
CBlockIndex* block = ::ChainActive()[height];
assert(block != nullptr);
return block->GetBlockTime();
}
int64_t getBlockMedianTimePast(int height) override
{
LockAssertion lock(::cs_main);
CBlockIndex* block = ::ChainActive()[height];
assert(block != nullptr);
return block->GetMedianTimePast();
}
bool haveBlockOnDisk(int height) override
{
LockAssertion lock(::cs_main);
@@ -95,20 +96,6 @@ class LockImpl : public Chain::Lock, public UniqueLock<RecursiveMutex>
}
return nullopt;
}
Optional<int> findPruned(int start_height, Optional<int> stop_height) override
{
LockAssertion lock(::cs_main);
if (::fPruneMode) {
CBlockIndex* block = stop_height ? ::ChainActive()[*stop_height] : ::ChainActive().Tip();
while (block && block->nHeight >= start_height) {
if ((block->nStatus & BLOCK_HAVE_DATA) == 0) {
return block->nHeight;
}
block = block->pprev;
}
}
return nullopt;
}
Optional<int> findFork(const uint256& hash, Optional<int>* height) override
{
LockAssertion lock(::cs_main);
@@ -247,26 +234,48 @@ public:
std::unique_ptr<Chain::Lock> result = std::move(lock); // Temporary to avoid CWG 1579
return result;
}
bool findBlock(const uint256& hash, CBlock* block, int64_t* time, int64_t* time_max) override
bool findBlock(const uint256& hash, const FoundBlock& block) override
{
CBlockIndex* index;
{
LOCK(cs_main);
index = LookupBlockIndex(hash);
if (!index) {
return false;
}
if (time) {
*time = index->GetBlockTime();
}
if (time_max) {
*time_max = index->GetBlockTimeMax();
WAIT_LOCK(cs_main, lock);
return FillBlock(LookupBlockIndex(hash), block, lock);
}
bool findFirstBlockWithTimeAndHeight(int64_t min_time, int min_height, const FoundBlock& block) override
{
WAIT_LOCK(cs_main, lock);
return FillBlock(ChainActive().FindEarliestAtLeast(min_time, min_height), block, lock);
}
bool findNextBlock(const uint256& block_hash, int block_height, const FoundBlock& next, bool* reorg) override {
WAIT_LOCK(cs_main, lock);
CBlockIndex* block = ChainActive()[block_height];
if (block && block->GetBlockHash() != block_hash) block = nullptr;
if (reorg) *reorg = !block;
return FillBlock(block ? ChainActive()[block_height + 1] : nullptr, next, lock);
}
bool findAncestorByHeight(const uint256& block_hash, int ancestor_height, const FoundBlock& ancestor_out) override
{
WAIT_LOCK(cs_main, lock);
if (const CBlockIndex* block = LookupBlockIndex(block_hash)) {
if (const CBlockIndex* ancestor = block->GetAncestor(ancestor_height)) {
return FillBlock(ancestor, ancestor_out, lock);
}
}
if (block && !ReadBlockFromDisk(*block, index, Params().GetConsensus())) {
block->SetNull();
}
return true;
return FillBlock(nullptr, ancestor_out, lock);
}
bool findAncestorByHash(const uint256& block_hash, const uint256& ancestor_hash, const FoundBlock& ancestor_out) override
{
WAIT_LOCK(cs_main, lock);
const CBlockIndex* block = LookupBlockIndex(block_hash);
const CBlockIndex* ancestor = LookupBlockIndex(ancestor_hash);
if (block && ancestor && block->GetAncestor(ancestor->nHeight) != ancestor) ancestor = nullptr;
return FillBlock(ancestor, ancestor_out, lock);
}
bool findCommonAncestor(const uint256& block_hash1, const uint256& block_hash2, const FoundBlock& ancestor_out, const FoundBlock& block1_out, const FoundBlock& block2_out) override
{
WAIT_LOCK(cs_main, lock);
const CBlockIndex* block1 = LookupBlockIndex(block_hash1);
const CBlockIndex* block2 = LookupBlockIndex(block_hash2);
const CBlockIndex* ancestor = block1 && block2 ? LastCommonAncestor(block1, block2) : nullptr;
return FillBlock(ancestor, ancestor_out, lock) & FillBlock(block1, block1_out, lock) & FillBlock(block2, block2_out, lock);
}
void findCoins(std::map<COutPoint, Coin>& coins) override { return FindCoins(m_node, coins); }
double guessVerificationProgress(const uint256& block_hash) override
@@ -274,6 +283,25 @@ public:
LOCK(cs_main);
return GuessVerificationProgress(Params().TxData(), LookupBlockIndex(block_hash));
}
bool hasBlocks(const uint256& block_hash, int min_height, Optional<int> max_height) override
{
// hasBlocks returns true if all ancestors of block_hash in specified
// range have block data (are not pruned), false if any ancestors in
// specified range are missing data.
//
// For simplicity and robustness, min_height and max_height are only
// used to limit the range, and passing min_height that's too low or
// max_height that's too high will not crash or change the result.
LOCK(::cs_main);
if (CBlockIndex* block = LookupBlockIndex(block_hash)) {
if (max_height && block->nHeight >= *max_height) block = block->GetAncestor(*max_height);
for (; block->nStatus & BLOCK_HAVE_DATA; block = block->pprev) {
// Check pprev to not segfault if min_height is too low
if (block->nHeight <= min_height || !block->pprev) return true;
}
}
return false;
}
RBFTransactionState isRBFOptIn(const CTransaction& tx) override
{
LOCK(::mempool.cs);

75
src/interfaces/chain.h
View File

@@ -30,6 +30,27 @@ namespace interfaces {
class Handler;
class Wallet;
//! Helper for findBlock to selectively return pieces of block data.
class FoundBlock
{
public:
FoundBlock& hash(uint256& hash) { m_hash = &hash; return *this; }
FoundBlock& height(int& height) { m_height = &height; return *this; }
FoundBlock& time(int64_t& time) { m_time = &time; return *this; }
FoundBlock& maxTime(int64_t& max_time) { m_max_time = &max_time; return *this; }
FoundBlock& mtpTime(int64_t& mtp_time) { m_mtp_time = &mtp_time; return *this; }
//! Read block data from disk. If the block exists but doesn't have data
//! (for example due to pruning), the CBlock variable will be set to null.
FoundBlock& data(CBlock& data) { m_data = &data; return *this; }
uint256* m_hash = nullptr;
int* m_height = nullptr;
int64_t* m_time = nullptr;
int64_t* m_max_time = nullptr;
int64_t* m_mtp_time = nullptr;
CBlock* m_data = nullptr;
};
//! Interface giving clients (wallet processes, maybe other analysis tools in
//! the future) ability to access to the chain state, receive notifications,
//! estimate fees, and submit transactions.
@@ -79,13 +100,6 @@ public:
//! Get block hash. Height must be valid or this function will abort.
virtual uint256 getBlockHash(int height) = 0;
//! Get block time. Height must be valid or this function will abort.
virtual int64_t getBlockTime(int height) = 0;
//! Get block median time past. Height must be valid or this function
//! will abort.
virtual int64_t getBlockMedianTimePast(int height) = 0;
//! Check that the block is available on disk (i.e. has not been
//! pruned), and contains transactions.
virtual bool haveBlockOnDisk(int height) = 0;
@@ -97,10 +111,6 @@ public:
//! (to avoid the cost of a second lookup in case this information is needed.)
virtual Optional<int> findFirstBlockWithTimeAndHeight(int64_t time, int height, uint256* hash) = 0;
//! Return height of last block in the specified range which is pruned, or
//! nullopt if no block in the range is pruned. Range is inclusive.
virtual Optional<int> findPruned(int start_height = 0, Optional<int> stop_height = nullopt) = 0;
//! Return height of the specified block if it is on the chain, otherwise
//! return the height of the highest block on chain that's an ancestor
//! of the specified block, or nullopt if there is no common ancestor.
@@ -127,14 +137,36 @@ public:
//! Return whether node has the block and optionally return block metadata
//! or contents.
//!
//! If a block pointer is provided to retrieve the block contents, and the
//! block exists but doesn't have data (for example due to pruning), the
//! block will be empty and all fields set to null.
virtual bool findBlock(const uint256& hash,
CBlock* block = nullptr,
int64_t* time = nullptr,
int64_t* max_time = nullptr) = 0;
virtual bool findBlock(const uint256& hash, const FoundBlock& block={}) = 0;
//! Find first block in the chain with timestamp >= the given time
//! and height >= than the given height, return false if there is no block
//! with a high enough timestamp and height. Optionally return block
//! information.
virtual bool findFirstBlockWithTimeAndHeight(int64_t min_time, int min_height, const FoundBlock& block={}) = 0;
//! Find next block if block is part of current chain. Also flag if
//! there was a reorg and the specified block hash is no longer in the
//! current chain, and optionally return block information.
virtual bool findNextBlock(const uint256& block_hash, int block_height, const FoundBlock& next={}, bool* reorg=nullptr) = 0;
//! Find ancestor of block at specified height and optionally return
//! ancestor information.
virtual bool findAncestorByHeight(const uint256& block_hash, int ancestor_height, const FoundBlock& ancestor_out={}) = 0;
//! Return whether block descends from a specified ancestor, and
//! optionally return ancestor information.
virtual bool findAncestorByHash(const uint256& block_hash,
const uint256& ancestor_hash,
const FoundBlock& ancestor_out={}) = 0;
//! Find most recent common ancestor between two blocks and optionally
//! return block information.
virtual bool findCommonAncestor(const uint256& block_hash1,
const uint256& block_hash2,
const FoundBlock& ancestor_out={},
const FoundBlock& block1_out={},
const FoundBlock& block2_out={}) = 0;
//! Look up unspent output information. Returns coins in the mempool and in
//! the current chain UTXO set. Iterates through all the keys in the map and
@@ -145,6 +177,11 @@ public:
//! the specified block hash are verified.
virtual double guessVerificationProgress(const uint256& block_hash) = 0;
//! Return true if data is available for all blocks in the specified range
//! of blocks. This checks all blocks that are ancestors of block_hash in
//! the height range from min_height to max_height, inclusive.
virtual bool hasBlocks(const uint256& block_hash, int min_height = 0, Optional<int> max_height = {}) = 0;
//! Check if transaction is RBF opt in.
virtual RBFTransactionState isRBFOptIn(const CTransaction& tx) = 0;

17
src/interfaces/wallet.cpp
View File

@@ -14,6 +14,7 @@
#include <sync.h>
#include <ui_interface.h>
#include <uint256.h>
#include <util/check.h>
#include <util/system.h>
#include <wallet/feebumper.h>
#include <wallet/fees.h>
@@ -62,7 +63,7 @@ WalletTx MakeWalletTx(CWallet& wallet, const CWalletTx& wtx)
WalletTxStatus MakeWalletTxStatus(interfaces::Chain::Lock& locked_chain, const CWalletTx& wtx)
{
WalletTxStatus result;
result.block_height = locked_chain.getBlockHeight(wtx.m_confirm.hashBlock).get_value_or(std::numeric_limits<int>::max());
result.block_height = wtx.m_confirm.block_height > 0 ? wtx.m_confirm.block_height : std::numeric_limits<int>::max();
result.blocks_to_maturity = wtx.GetBlocksToMaturity();
result.depth_in_main_chain = wtx.GetDepthInMainChain();
result.time_received = wtx.nTimeReceived;
@@ -318,13 +319,9 @@ public:
if (mi == m_wallet->mapWallet.end()) {
return false;
}
if (Optional<int> height = locked_chain->getHeight()) {
num_blocks = *height;
block_time = locked_chain->getBlockTime(*height);
} else {
num_blocks = -1;
block_time = -1;
}
num_blocks = m_wallet->GetLastBlockHeight();
block_time = -1;
CHECK_NONFATAL(m_wallet->chain().findBlock(m_wallet->GetLastBlockHash(), FoundBlock().time(block_time)));
tx_status = MakeWalletTxStatus(*locked_chain, mi->second);
return true;
}
@@ -373,12 +370,12 @@ public:
{
auto locked_chain = m_wallet->chain().lock(true /* try_lock */);
if (!locked_chain) return false;
num_blocks = locked_chain->getHeight().get_value_or(-1);
if (!force && num_blocks == cached_num_blocks) return false;
TRY_LOCK(m_wallet->cs_wallet, locked_wallet);
if (!locked_wallet) {
return false;
}
num_blocks = m_wallet->GetLastBlockHeight();
if (!force && num_blocks == cached_num_blocks) return false;
balances = getBalances();
return true;
}