Skip to content

Commit

Permalink
refactor: Replace uses ChainActive() in interfaces/chain.cpp
Browse files Browse the repository at this point in the history 
Suggested #19425 (comment)
  • Loading branch information
@ryanofsky
ryanofsky committed Sep 23, 2020
1 parent 97e683d commit 9931714
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 53 deletions.
7 changes: 3 additions & 4 deletions src/bench/wallet_balance.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,14 @@ static void WalletBalance(benchmark::Bench& bench, const bool set_dirty, const b

const auto& ADDRESS_WATCHONLY = ADDRESS_BCRT1_UNSPENDABLE;

NodeContext node;
std::unique_ptr<interfaces::Chain> chain = interfaces::MakeChain(node);
CWallet wallet{chain.get(), "", CreateMockWalletDatabase()};
test_setup.m_node.chain = interfaces::MakeChain(test_setup.m_node);
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 Optional<std::string> address_mine{add_mine ? Optional<std::string>{getnewaddress(wallet)} : nullopt};
if (add_watchonly) importaddress(wallet, ADDRESS_WATCHONLY);
Expand Down
41 changes: 26 additions & 15 deletions src/interfaces/chain.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,16 @@
namespace interfaces {
namespace {

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 = ChainActive()[index->nHeight] == index;
if (block.m_next_block) FillBlock(ChainActive()[index->nHeight] == index ? ChainActive()[index->nHeight + 1] : nullptr, *block.m_next_block, lock);
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 @@ -150,7 +150,8 @@ class ChainImpl : public Chain
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;
}
Expand All @@ -159,20 +160,23 @@ class ChainImpl : public Chain
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);
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;
}
CBlockLocator getTipLocator() override
{
LOCK(cs_main);
return ::ChainActive().GetLocator();
const CChain& active = Assert(m_node.chainman)->ActiveChain();
return active.GetLocator();
}
bool checkFinalTx(const CTransaction& tx) override
{
Expand All @@ -182,48 +186,54 @@ class ChainImpl : public Chain
Optional<int> findLocatorFork(const CBlockLocator& locator) override
{
LOCK(cs_main);
if (CBlockIndex* fork = FindForkInGlobalIndex(::ChainActive(), locator)) {
const CChain& active = Assert(m_node.chainman)->ActiveChain();
if (CBlockIndex* fork = FindForkInGlobalIndex(active, locator)) {
return fork->nHeight;
}
return nullopt;
}
bool findBlock(const uint256& hash, const FoundBlock& block) override
{
WAIT_LOCK(cs_main, lock);
return FillBlock(LookupBlockIndex(hash), block, lock);
const CChain& active = Assert(m_node.chainman)->ActiveChain();
return FillBlock(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);
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 = 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 = 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);
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 = LookupBlockIndex(block_hash1);
const CBlockIndex* block2 = LookupBlockIndex(block_hash2);
const CBlockIndex* ancestor = block1 && block2 ? LastCommonAncestor(block1, block2) : nullptr;
// Using & instead of && below to avoid short circuiting and leaving
// output uninitialized.
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 @@ -341,7 +351,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
12 changes: 6 additions & 6 deletions src/test/interfaces_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ BOOST_FIXTURE_TEST_SUITE(interfaces_tests, TestChain100Setup)
BOOST_AUTO_TEST_CASE(findBlock)
{
auto chain = interfaces::MakeChain(m_node);
auto& active = ChainActive();
const CChain& active = Assert(m_node.chainman)->ActiveChain();

uint256 hash;
BOOST_CHECK(chain->findBlock(active[10]->GetBlockHash(), FoundBlock().hash(hash)));
Expand Down Expand Up @@ -62,7 +62,7 @@ BOOST_AUTO_TEST_CASE(findBlock)
BOOST_AUTO_TEST_CASE(findFirstBlockWithTimeAndHeight)
{
auto chain = interfaces::MakeChain(m_node);
auto& active = ChainActive();
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 @@ -74,7 +74,7 @@ BOOST_AUTO_TEST_CASE(findFirstBlockWithTimeAndHeight)
BOOST_AUTO_TEST_CASE(findAncestorByHeight)
{
auto chain = interfaces::MakeChain(m_node);
auto& active = ChainActive();
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 @@ -84,7 +84,7 @@ BOOST_AUTO_TEST_CASE(findAncestorByHeight)
BOOST_AUTO_TEST_CASE(findAncestorByHash)
{
auto chain = interfaces::MakeChain(m_node);
auto& active = ChainActive();
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 @@ -94,7 +94,7 @@ BOOST_AUTO_TEST_CASE(findAncestorByHash)
BOOST_AUTO_TEST_CASE(findCommonAncestor)
{
auto chain = interfaces::MakeChain(m_node);
auto& active = ChainActive();
const CChain& active = Assert(m_node.chainman)->ActiveChain();
auto* orig_tip = active.Tip();
for (int i = 0; i < 10; ++i) {
BlockValidationState state;
Expand Down Expand Up @@ -124,7 +124,7 @@ BOOST_AUTO_TEST_CASE(findCommonAncestor)
BOOST_AUTO_TEST_CASE(hasBlocks)
{
auto chain = interfaces::MakeChain(m_node);
auto& active = ChainActive();
const CChain& active = Assert(m_node.chainman)->ActiveChain();

// Test ranges
BOOST_CHECK(chain->hasBlocks(active.Tip()->GetBlockHash(), 10, 90));
Expand Down
48 changes: 20 additions & 28 deletions src/wallet/test/wallet_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,11 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup)
CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey()));
CBlockIndex* newTip = ::ChainActive().Tip();

NodeContext node;
auto chain = interfaces::MakeChain(node);
m_node.chain = interfaces::MakeChain(m_node);

// Verify ScanForWalletTransactions fails to read an unknown start block.
{
CWallet wallet(chain.get(), "", CreateDummyWalletDatabase());
CWallet wallet(m_node.chain.get(), "", CreateDummyWalletDatabase());
{
LOCK(wallet.cs_wallet);
wallet.SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash());
Expand All @@ -107,7 +106,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup)
// Verify ScanForWalletTransactions picks up transactions in both the old
// and new block files.
{
CWallet wallet(chain.get(), "", CreateDummyWalletDatabase());
CWallet wallet(m_node.chain.get(), "", CreateDummyWalletDatabase());
{
LOCK(wallet.cs_wallet);
wallet.SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash());
Expand All @@ -133,7 +132,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup)
// Verify ScanForWalletTransactions only picks transactions in the new block
// file.
{
CWallet wallet(chain.get(), "", CreateDummyWalletDatabase());
CWallet wallet(m_node.chain.get(), "", CreateDummyWalletDatabase());
{
LOCK(wallet.cs_wallet);
wallet.SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash());
Expand All @@ -158,7 +157,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup)

// Verify ScanForWalletTransactions scans no blocks.
{
CWallet wallet(chain.get(), "", CreateDummyWalletDatabase());
CWallet wallet(m_node.chain.get(), "", CreateDummyWalletDatabase());
{
LOCK(wallet.cs_wallet);
wallet.SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash());
Expand All @@ -183,8 +182,7 @@ BOOST_FIXTURE_TEST_CASE(importmulti_rescan, TestChain100Setup)
CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey()));
CBlockIndex* newTip = ::ChainActive().Tip();

NodeContext node;
auto chain = interfaces::MakeChain(node);
m_node.chain = interfaces::MakeChain(m_node);

// Prune the older block file.
{
Expand All @@ -197,7 +195,7 @@ BOOST_FIXTURE_TEST_CASE(importmulti_rescan, TestChain100Setup)
// before the missing block, and success for a key whose creation time is
// after.
{
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(chain.get(), "", CreateDummyWalletDatabase());
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(m_node.chain.get(), "", CreateDummyWalletDatabase());
wallet->SetupLegacyScriptPubKeyMan();
WITH_LOCK(wallet->cs_wallet, wallet->SetLastBlockProcessed(newTip->nHeight, newTip->GetBlockHash()));
AddWallet(wallet);
Expand Down Expand Up @@ -255,14 +253,13 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup)
SetMockTime(KEY_TIME);
m_coinbase_txns.emplace_back(CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())).vtx[0]);

NodeContext node;
auto chain = interfaces::MakeChain(node);
m_node.chain = interfaces::MakeChain(m_node);

std::string backup_file = (GetDataDir() / "wallet.backup").string();

// Import key into wallet and call dumpwallet to create backup file.
{
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(chain.get(), "", CreateDummyWalletDatabase());
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(m_node.chain.get(), "", CreateDummyWalletDatabase());
{
auto spk_man = wallet->GetOrCreateLegacyScriptPubKeyMan();
LOCK2(wallet->cs_wallet, spk_man->cs_KeyStore);
Expand All @@ -284,7 +281,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup)
// Call importwallet RPC and verify all blocks with timestamps >= BLOCK_TIME
// were scanned, and no prior blocks were scanned.
{
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(chain.get(), "", CreateDummyWalletDatabase());
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(m_node.chain.get(), "", CreateDummyWalletDatabase());
LOCK(wallet->cs_wallet);
wallet->SetupLegacyScriptPubKeyMan();

Expand Down Expand Up @@ -317,10 +314,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup)
// debit functions.
BOOST_FIXTURE_TEST_CASE(coin_mark_dirty_immature_credit, TestChain100Setup)
{
NodeContext node;
auto chain = interfaces::MakeChain(node);

CWallet wallet(chain.get(), "", CreateDummyWalletDatabase());
CWallet wallet(m_node.chain.get(), "", CreateDummyWalletDatabase());
auto spk_man = wallet.GetOrCreateLegacyScriptPubKeyMan();
CWalletTx wtx(&wallet, m_coinbase_txns.back());

Expand Down Expand Up @@ -495,7 +489,8 @@ class ListCoinsTestingSetup : public TestChain100Setup
ListCoinsTestingSetup()
{
CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey()));
wallet = MakeUnique<CWallet>(m_chain.get(), "", CreateMockWalletDatabase());
m_node.chain = interfaces::MakeChain(m_node);
wallet = MakeUnique<CWallet>(m_node.chain.get(), "", CreateMockWalletDatabase());
{
LOCK2(wallet->cs_wallet, ::cs_main);
wallet->SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash());
Expand Down Expand Up @@ -544,7 +539,6 @@ class ListCoinsTestingSetup : public TestChain100Setup
return it->second;
}

std::unique_ptr<interfaces::Chain> m_chain = interfaces::MakeChain(m_node);
std::unique_ptr<CWallet> wallet;
};

Expand Down Expand Up @@ -611,9 +605,7 @@ BOOST_FIXTURE_TEST_CASE(ListCoins, ListCoinsTestingSetup)

BOOST_FIXTURE_TEST_CASE(wallet_disableprivkeys, TestChain100Setup)
{
NodeContext node;
auto chain = interfaces::MakeChain(node);
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(chain.get(), "", CreateDummyWalletDatabase());
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(m_node.chain.get(), "", CreateDummyWalletDatabase());
wallet->SetupLegacyScriptPubKeyMan();
wallet->SetMinVersion(FEATURE_LATEST);
wallet->SetWalletFlag(WALLET_FLAG_DISABLE_PRIVATE_KEYS);
Expand Down Expand Up @@ -708,8 +700,8 @@ BOOST_FIXTURE_TEST_CASE(wallet_descriptor_test, BasicTestingSetup)
BOOST_FIXTURE_TEST_CASE(CreateWalletFromFile, TestChain100Setup)
{
// Create new wallet with known key and unload it.
auto chain = interfaces::MakeChain(m_node);
auto wallet = TestLoadWallet(*chain);
m_node.chain = interfaces::MakeChain(m_node);
auto wallet = TestLoadWallet(*m_node.chain);
CKey key;
key.MakeNewKey(true);
AddKey(*wallet, key);
Expand Down Expand Up @@ -744,12 +736,12 @@ BOOST_FIXTURE_TEST_CASE(CreateWalletFromFile, TestChain100Setup)
auto block_tx = TestSimpleSpend(*m_coinbase_txns[0], 0, coinbaseKey, GetScriptForRawPubKey(key.GetPubKey()));
m_coinbase_txns.push_back(CreateAndProcessBlock({block_tx}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())).vtx[0]);
auto mempool_tx = TestSimpleSpend(*m_coinbase_txns[1], 0, coinbaseKey, GetScriptForRawPubKey(key.GetPubKey()));
BOOST_CHECK(chain->broadcastTransaction(MakeTransactionRef(mempool_tx), DEFAULT_TRANSACTION_MAXFEE, false, error));
BOOST_CHECK(m_node.chain->broadcastTransaction(MakeTransactionRef(mempool_tx), DEFAULT_TRANSACTION_MAXFEE, false, error));


// Reload wallet and make sure new transactions are detected despite events
// being blocked
wallet = TestLoadWallet(*chain);
wallet = TestLoadWallet(*m_node.chain);
BOOST_CHECK(rescan_completed);
BOOST_CHECK_EQUAL(addtx_count, 2);
{
Expand Down Expand Up @@ -782,12 +774,12 @@ BOOST_FIXTURE_TEST_CASE(CreateWalletFromFile, TestChain100Setup)
block_tx = TestSimpleSpend(*m_coinbase_txns[2], 0, coinbaseKey, GetScriptForRawPubKey(key.GetPubKey()));
m_coinbase_txns.push_back(CreateAndProcessBlock({block_tx}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())).vtx[0]);
mempool_tx = TestSimpleSpend(*m_coinbase_txns[3], 0, coinbaseKey, GetScriptForRawPubKey(key.GetPubKey()));
BOOST_CHECK(chain->broadcastTransaction(MakeTransactionRef(mempool_tx), DEFAULT_TRANSACTION_MAXFEE, false, error));
BOOST_CHECK(m_node.chain->broadcastTransaction(MakeTransactionRef(mempool_tx), DEFAULT_TRANSACTION_MAXFEE, false, error));
LEAVE_CRITICAL_SECTION(wallet->wallet()->cs_wallet);
SyncWithValidationInterfaceQueue();
ENTER_CRITICAL_SECTION(wallet->wallet()->cs_wallet);
});
wallet = TestLoadWallet(*chain);
wallet = TestLoadWallet(*m_node.chain);
BOOST_CHECK_EQUAL(addtx_count, 4);
{
LOCK(wallet->cs_wallet);
Expand Down

0 comments on commit 9931714

Please sign in to comment.