Skip to content

Commit

Permalink
merge bitcoin#19425: Get rid of more redundant chain methods
Browse files Browse the repository at this point in the history
  • Loading branch information
@kwvg
kwvg committed Mar 27, 2023
1 parent c62682b commit 5b22a96
Show file tree
Hide file tree
Showing 12 changed files with 108 additions and 163 deletions.
6 changes: 2 additions & 4 deletions src/bench/wallet_balance.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,13 @@ static void WalletBalance(benchmark::Bench& bench, const bool set_dirty, const b
RegTestingSetup test_setup;
const auto& ADDRESS_WATCHONLY = ADDRESS_B58T_UNSPENDABLE;

NodeContext node;
std::unique_ptr<interfaces::Chain> chain = interfaces::MakeChain(node);
CWallet wallet{chain.get(), "", CreateMockWalletDatabase()};
CWallet wallet{test_setup.m_node.chain.get(), "", CreateMockWalletDatabase()};
{
wallet.SetupLegacyScriptPubKeyMan();
bool first_run;
if (wallet.LoadWallet(first_run) != DBErrors::LOAD_OK) assert(false);
}
auto handler = chain->handleNotifications({ &wallet, [](CWallet*) {} });
auto handler = test_setup.m_node.chain->handleNotifications({ &wallet, [](CWallet*) {} });

const std::optional<std::string> address_mine{add_mine ? std::optional<std::string>{getnewaddress(wallet)} : std::nullopt};
if (add_watchonly) importaddress(wallet, ADDRESS_WATCHONLY);
Expand Down
29 changes: 9 additions & 20 deletions src/interfaces/chain.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ class FoundBlock
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; }
//! Return whether block is in the active (most-work) chain.
FoundBlock& inActiveChain(bool& in_active_chain) { m_in_active_chain = &in_active_chain; return *this; }
//! Return next block in the active chain if current block is in the active chain.
FoundBlock& nextBlock(const FoundBlock& next_block) { m_next_block = &next_block; 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; }
Expand All @@ -64,6 +68,8 @@ class FoundBlock
int64_t* m_time = nullptr;
int64_t* m_max_time = nullptr;
int64_t* m_mtp_time = nullptr;
bool* m_in_active_chain = nullptr;
const FoundBlock* m_next_block = nullptr;
CBlock* m_data = nullptr;
};

Expand All @@ -88,9 +94,9 @@ class FoundBlock
//! wallet cache it, fee estimation being driven by node mempool, wallet
//! should be the consumer.
//!
//! * The `guessVerificationProgress`, `getBlockHeight`, `getBlockHash`, etc
//! methods can go away if rescan logic is moved on the node side, and wallet
//! only register rescan request.
//! * `guessVerificationProgress` and similar methods can go away if rescan
//! logic moves out of the wallet, and the wallet just requests scans from the
//! node (https://github.com/bitcoin/bitcoin/issues/11756)
class Chain
{
public:
Expand All @@ -101,25 +107,13 @@ class Chain
//! any blocks)
virtual std::optional<int> getHeight() = 0;

//! Get block height above genesis block. Returns 0 for genesis block,
//! 1 for following block, and so on. Returns nullopt for a block not
//! included in the current chain.
virtual std::optional<int> getBlockHeight(const uint256& hash) = 0;

//! Get block hash. Height must be valid or this function will abort.
virtual uint256 getBlockHash(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;

//! Return height of the first block in the chain with timestamp equal
//! or greater than the given time and height equal or greater than the
//! given height, or nullopt if there is no block with a high enough
//! timestamp and height. Also return the block hash as an optional output parameter
//! (to avoid the cost of a second lookup in case this information is needed.)
virtual std::optional<int> findFirstBlockWithTimeAndHeight(int64_t time, int height, uint256* hash) = 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.
Expand Down Expand Up @@ -149,11 +143,6 @@ class Chain
//! 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;
Expand Down
72 changes: 30 additions & 42 deletions src/node/interfaces.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -469,14 +469,16 @@ class NodeImpl : public Node
CoreContext m_context_ref{std::nullopt};
};

bool FillBlock(const CBlockIndex* index, const FoundBlock& block, UniqueLock<RecursiveMutex>& lock)
bool FillBlock(const CBlockIndex* index, const FoundBlock& block, UniqueLock<RecursiveMutex>& lock, const CChain& active)
{
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_in_active_chain) *block.m_in_active_chain = active[index->nHeight] == index;
if (block.m_next_block) FillBlock(active[index->nHeight] == index ? active[index->nHeight + 1] : nullptr, *block.m_next_block, lock, active);
if (block.m_data) {
REVERSE_LOCK(lock);
if (!ReadBlockFromDisk(*block.m_data, index, Params().GetConsensus())) block.m_data->SetNull();
Expand Down Expand Up @@ -522,7 +524,6 @@ class NotificationsProxy : public CValidationInterface
std::shared_ptr<Chain::Notifications> m_notifications;
};


class NotificationsHandlerImpl : public Handler
{
public:
Expand Down Expand Up @@ -588,49 +589,34 @@ class ChainImpl : public Chain
std::optional<int> getHeight() override
{
LOCK(::cs_main);
int height = ::ChainActive().Height();
const CChain& active = Assert(m_node.chainman)->ActiveChain();
int height = active.Height();
if (height >= 0) {
return height;
}
return std::nullopt;
}
std::optional<int> getBlockHeight(const uint256& hash) override
{
LOCK(::cs_main);
CBlockIndex* block = g_chainman.m_blockman.LookupBlockIndex(hash);
if (block && ::ChainActive().Contains(block)) {
return block->nHeight;
}
return std::nullopt;
}
uint256 getBlockHash(int height) override
{
LOCK(::cs_main);
CBlockIndex* block = ::ChainActive()[height];
const CChain& active = Assert(m_node.chainman)->ActiveChain();
CBlockIndex* block = active[height];
assert(block != nullptr);
return block->GetBlockHash();
}
bool haveBlockOnDisk(int height) override
{
LOCK(cs_main);
CBlockIndex* block = ::ChainActive()[height];
const CChain& active = Assert(m_node.chainman)->ActiveChain();
CBlockIndex* block = active[height];
return block && ((block->nStatus & BLOCK_HAVE_DATA) != 0) && block->nTx > 0;
}
std::optional<int> findFirstBlockWithTimeAndHeight(int64_t time, int height, uint256* hash) override
{
LOCK(cs_main);
CBlockIndex* block = ::ChainActive().FindEarliestAtLeast(time, height);
if (block) {
if (hash) *hash = block->GetBlockHash();
return block->nHeight;
}
return std::nullopt;
}
std::optional<int> findFork(const uint256& hash, std::optional<int>* height) override
{
LOCK(cs_main);
const CChain& active = Assert(m_node.chainman)->ActiveChain();
const CBlockIndex* block = g_chainman.m_blockman.LookupBlockIndex(hash);
const CBlockIndex* fork = block ? ::ChainActive().FindFork(block) : nullptr;
const CBlockIndex* fork = block ? active.FindFork(block) : nullptr;
if (height) {
if (block) {
*height = block->nHeight;
Expand All @@ -646,63 +632,64 @@ class ChainImpl : public Chain
CBlockLocator getTipLocator() override
{
LOCK(cs_main);
return ::ChainActive().GetLocator();
const CChain& active = Assert(m_node.chainman)->ActiveChain();
return active.GetLocator();
}
std::optional<int> findLocatorFork(const CBlockLocator& locator) override
{
LOCK(cs_main);
if (CBlockIndex* fork = g_chainman.m_blockman.FindForkInGlobalIndex(::ChainActive(), locator)) {
const CChain& active = Assert(m_node.chainman)->ActiveChain();
if (CBlockIndex* fork = g_chainman.m_blockman.FindForkInGlobalIndex(active, locator)) {
return fork->nHeight;
}
return std::nullopt;
}
bool checkFinalTx(const CTransaction& tx) override
{
LOCK(cs_main);
return CheckFinalTx(::ChainActive().Tip(), tx);
const CChain& active = Assert(m_node.chainman)->ActiveChain();
return CheckFinalTx(active.Tip(), tx);
}
bool findBlock(const uint256& hash, const FoundBlock& block) override
{
WAIT_LOCK(cs_main, lock);
return FillBlock(g_chainman.m_blockman.LookupBlockIndex(hash), block, lock);
const CChain& active = Assert(m_node.chainman)->ActiveChain();
return FillBlock(g_chainman.m_blockman.LookupBlockIndex(hash), block, lock, active);
}
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);
const CChain& active = Assert(m_node.chainman)->ActiveChain();
return FillBlock(active.FindEarliestAtLeast(min_time, min_height), block, lock, active);
}
bool findAncestorByHeight(const uint256& block_hash, int ancestor_height, const FoundBlock& ancestor_out) override
{
WAIT_LOCK(cs_main, lock);
const CChain& active = Assert(m_node.chainman)->ActiveChain();
if (const CBlockIndex* block = g_chainman.m_blockman.LookupBlockIndex(block_hash)) {
if (const CBlockIndex* ancestor = block->GetAncestor(ancestor_height)) {
return FillBlock(ancestor, ancestor_out, lock);
return FillBlock(ancestor, ancestor_out, lock, active);
}
}
return FillBlock(nullptr, ancestor_out, lock);
return FillBlock(nullptr, ancestor_out, lock, active);
}
bool findAncestorByHash(const uint256& block_hash, const uint256& ancestor_hash, const FoundBlock& ancestor_out) override
{
WAIT_LOCK(cs_main, lock);
const CChain& active = Assert(m_node.chainman)->ActiveChain();
const CBlockIndex* block = g_chainman.m_blockman.LookupBlockIndex(block_hash);
const CBlockIndex* ancestor = g_chainman.m_blockman.LookupBlockIndex(ancestor_hash);
if (block && ancestor && block->GetAncestor(ancestor->nHeight) != ancestor) ancestor = nullptr;
return FillBlock(ancestor, ancestor_out, lock);
return FillBlock(ancestor, ancestor_out, lock, active);
}
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 CChain& active = Assert(m_node.chainman)->ActiveChain();
const CBlockIndex* block1 = g_chainman.m_blockman.LookupBlockIndex(block_hash1);
const CBlockIndex* block2 = g_chainman.m_blockman.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);
return FillBlock(ancestor, ancestor_out, lock, active) & FillBlock(block1, block1_out, lock, active) & FillBlock(block2, block2_out, lock, active);
}
void findCoins(std::map<COutPoint, Coin>& coins) override { return FindCoins(m_node, coins); }
double guessVerificationProgress(const uint256& block_hash) override
Expand Down Expand Up @@ -814,7 +801,8 @@ class ChainImpl : public Chain
{
if (!old_tip.IsNull()) {
LOCK(::cs_main);
if (old_tip == ::ChainActive().Tip()->GetBlockHash()) return;
const CChain& active = Assert(m_node.chainman)->ActiveChain();
if (old_tip == active.Tip()->GetBlockHash()) return;
}
SyncWithValidationInterfaceQueue();
}
Expand Down
51 changes: 24 additions & 27 deletions src/test/interfaces_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ BOOST_FIXTURE_TEST_SUITE(interfaces_tests, TestChain100Setup)

BOOST_AUTO_TEST_CASE(findBlock)
{
auto chain = interfaces::MakeChain(m_node);
auto& active = ChainActive();
auto& chain = m_node.chain;
const CChain& active = Assert(m_node.chainman)->ActiveChain();

uint256 hash;
BOOST_CHECK(chain->findBlock(active[10]->GetBlockHash(), FoundBlock().hash(hash)));
Expand All @@ -44,13 +44,25 @@ BOOST_AUTO_TEST_CASE(findBlock)
BOOST_CHECK(chain->findBlock(active[60]->GetBlockHash(), FoundBlock().mtpTime(mtp_time)));
BOOST_CHECK_EQUAL(mtp_time, active[60]->GetMedianTimePast());

bool cur_active{false}, next_active{false};
uint256 next_hash;
BOOST_CHECK_EQUAL(active.Height(), 100);
BOOST_CHECK(chain->findBlock(active[99]->GetBlockHash(), FoundBlock().inActiveChain(cur_active).nextBlock(FoundBlock().inActiveChain(next_active).hash(next_hash))));
BOOST_CHECK(cur_active);
BOOST_CHECK(next_active);
BOOST_CHECK_EQUAL(next_hash, active[100]->GetBlockHash());
cur_active = next_active = false;
BOOST_CHECK(chain->findBlock(active[100]->GetBlockHash(), FoundBlock().inActiveChain(cur_active).nextBlock(FoundBlock().inActiveChain(next_active))));
BOOST_CHECK(cur_active);
BOOST_CHECK(!next_active);

BOOST_CHECK(!chain->findBlock({}, FoundBlock()));
}

BOOST_AUTO_TEST_CASE(findFirstBlockWithTimeAndHeight)
{
auto chain = interfaces::MakeChain(m_node);
auto& active = ChainActive();
auto& chain = m_node.chain;
const CChain& active = Assert(m_node.chainman)->ActiveChain();
uint256 hash;
int height;
BOOST_CHECK(chain->findFirstBlockWithTimeAndHeight(/* min_time= */ 0, /* min_height= */ 5, FoundBlock().hash(hash).height(height)));
Expand All @@ -59,25 +71,10 @@ BOOST_AUTO_TEST_CASE(findFirstBlockWithTimeAndHeight)
BOOST_CHECK(!chain->findFirstBlockWithTimeAndHeight(/* min_time= */ active.Tip()->GetBlockTimeMax() + 1, /* min_height= */ 0));
}

BOOST_AUTO_TEST_CASE(findNextBlock)
{
auto chain = interfaces::MakeChain(m_node);
auto& active = ChainActive();
bool reorg;
uint256 hash;
BOOST_CHECK(chain->findNextBlock(active[20]->GetBlockHash(), 20, FoundBlock().hash(hash), &reorg));
BOOST_CHECK_EQUAL(hash, active[21]->GetBlockHash());
BOOST_CHECK_EQUAL(reorg, false);
BOOST_CHECK(!chain->findNextBlock(uint256(), 20, {}, &reorg));
BOOST_CHECK_EQUAL(reorg, true);
BOOST_CHECK(!chain->findNextBlock(active.Tip()->GetBlockHash(), active.Height(), {}, &reorg));
BOOST_CHECK_EQUAL(reorg, false);
}

BOOST_AUTO_TEST_CASE(findAncestorByHeight)
{
auto chain = interfaces::MakeChain(m_node);
auto& active = ChainActive();
auto& chain = m_node.chain;
const CChain& active = Assert(m_node.chainman)->ActiveChain();
uint256 hash;
BOOST_CHECK(chain->findAncestorByHeight(active[20]->GetBlockHash(), 10, FoundBlock().hash(hash)));
BOOST_CHECK_EQUAL(hash, active[10]->GetBlockHash());
Expand All @@ -86,8 +83,8 @@ BOOST_AUTO_TEST_CASE(findAncestorByHeight)

BOOST_AUTO_TEST_CASE(findAncestorByHash)
{
auto chain = interfaces::MakeChain(m_node);
auto& active = ChainActive();
auto& chain = m_node.chain;
const CChain& active = Assert(m_node.chainman)->ActiveChain();
int height = -1;
BOOST_CHECK(chain->findAncestorByHash(active[20]->GetBlockHash(), active[10]->GetBlockHash(), FoundBlock().height(height)));
BOOST_CHECK_EQUAL(height, 10);
Expand All @@ -96,8 +93,8 @@ BOOST_AUTO_TEST_CASE(findAncestorByHash)

BOOST_AUTO_TEST_CASE(findCommonAncestor)
{
auto chain = interfaces::MakeChain(m_node);
auto& active = ChainActive();
auto& chain = m_node.chain;
const CChain& active = Assert(m_node.chainman)->ActiveChain();
auto* orig_tip = active.Tip();
for (int i = 0; i < 10; ++i) {
CValidationState state;
Expand All @@ -120,8 +117,8 @@ BOOST_AUTO_TEST_CASE(findCommonAncestor)

BOOST_AUTO_TEST_CASE(hasBlocks)
{
auto chain = interfaces::MakeChain(m_node);
auto& active = ChainActive();
auto& chain = m_node.chain;
const CChain& active = Assert(m_node.chainman)->ActiveChain();

// Test ranges
BOOST_CHECK(chain->hasBlocks(active.Tip()->GetBlockHash(), 10, 90));
Expand Down
7 changes: 1 addition & 6 deletions src/wallet/test/coinjoin_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,10 +129,7 @@ class CTransactionBuilderTestSetup : public TestChain100Setup
CTransactionBuilderTestSetup()
{
CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey()));
node.fee_estimator = std::make_unique<CBlockPolicyEstimator>();
node.mempool = std::make_unique<CTxMemPool>(node.fee_estimator.get());
chain = interfaces::MakeChain(node);
wallet = std::make_unique<CWallet>(chain.get(), "", CreateMockWalletDatabase());
wallet = std::make_unique<CWallet>(m_node.chain.get(), "", CreateMockWalletDatabase());
wallet->SetupLegacyScriptPubKeyMan();
bool firstRun;
wallet->LoadWallet(firstRun);
Expand All @@ -153,8 +150,6 @@ class CTransactionBuilderTestSetup : public TestChain100Setup
RemoveWallet(wallet, std::nullopt);
}

NodeContext node;
std::shared_ptr<interfaces::Chain> chain;
std::shared_ptr<CWallet> wallet;

CWalletTx& AddTxToChain(uint256 nTxHash)
Expand Down
2 changes: 1 addition & 1 deletion src/wallet/test/init_test_fixture.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

InitWalletDirTestingSetup::InitWalletDirTestingSetup(const std::string& chainName) : BasicTestingSetup(chainName)
{
m_wallet_client = MakeWalletClient(*m_chain, *Assert(m_node.args));
m_wallet_client = MakeWalletClient(*m_node.chain, *Assert(m_node.args));

std::string sep;
sep += fs::path::preferred_separator;
Expand Down
Loading

0 comments on commit 5b22a96

Please sign in to comment.