Skip to content

Commit 6965f13

Browse files
ryanofskyryanofsky
committed
refactor: Replace uses ChainActive() in interfaces/chain.cpp
Suggested bitcoin/bitcoin#19425 (comment)
1 parent 3fbbb9a commit 6965f13

File tree

4 files changed

+49
-56
lines changed

4 files changed

+49
-56
lines changed

‎src/bench/wallet_balance.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,15 +24,13 @@ static void WalletBalance(benchmark::Bench& bench, const bool set_dirty, const b
2424

2525
const auto& ADDRESS_WATCHONLY = ADDRESS_BCRT1_UNSPENDABLE;
2626

27-
NodeContext node;
28-
std::unique_ptr<interfaces::Chain> chain = interfaces::MakeChain(node);
29-
CWallet wallet{chain.get(), "", CreateMockWalletDatabase()};
27+
CWallet wallet{test_setup.m_node.chain.get(), "", CreateMockWalletDatabase()};
3028
{
3129
wallet.SetupLegacyScriptPubKeyMan();
3230
bool first_run;
3331
if (wallet.LoadWallet(first_run) != DBErrors::LOAD_OK) assert(false);
3432
}
35-
auto handler = chain->handleNotifications({&wallet, [](CWallet*) {}});
33+
auto handler = test_setup.m_node.chain->handleNotifications({&wallet, [](CWallet*) {}});
3634

3735
const Optional<std::string> address_mine{add_mine ? Optional<std::string>{getnewaddress(wallet)} : nullopt};
3836
if (add_watchonly) importaddress(wallet, ADDRESS_WATCHONLY);

‎src/node/interfaces.cpp

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -303,16 +303,16 @@ class NodeImpl : public Node
303303
util::Ref m_context_ref;
304304
};
305305

306-
bool FillBlock(const CBlockIndex* index, const FoundBlock& block, UniqueLock<RecursiveMutex>& lock)
306+
bool FillBlock(const CBlockIndex* index, const FoundBlock& block, UniqueLock<RecursiveMutex>& lock, const CChain& active)
307307
{
308308
if (!index) return false;
309309
if (block.m_hash) *block.m_hash = index->GetBlockHash();
310310
if (block.m_height) *block.m_height = index->nHeight;
311311
if (block.m_time) *block.m_time = index->GetBlockTime();
312312
if (block.m_max_time) *block.m_max_time = index->GetBlockTimeMax();
313313
if (block.m_mtp_time) *block.m_mtp_time = index->GetMedianTimePast();
314-
if (block.m_in_active_chain) *block.m_in_active_chain = ChainActive()[index->nHeight] == index;
315-
if (block.m_next_block) FillBlock(ChainActive()[index->nHeight] == index ? ChainActive()[index->nHeight + 1] : nullptr, *block.m_next_block, lock);
314+
if (block.m_in_active_chain) *block.m_in_active_chain = active[index->nHeight] == index;
315+
if (block.m_next_block) FillBlock(active[index->nHeight] == index ? active[index->nHeight + 1] : nullptr, *block.m_next_block, lock, active);
316316
if (block.m_data) {
317317
REVERSE_LOCK(lock);
318318
if (!ReadBlockFromDisk(*block.m_data, index, Params().GetConsensus())) block.m_data->SetNull();
@@ -415,7 +415,8 @@ class ChainImpl : public Chain
415415
Optional<int> getHeight() override
416416
{
417417
LOCK(::cs_main);
418-
int height = ::ChainActive().Height();
418+
const CChain& active = Assert(m_node.chainman)->ActiveChain();
419+
int height = active.Height();
419420
if (height >= 0) {
420421
return height;
421422
}
@@ -424,20 +425,23 @@ class ChainImpl : public Chain
424425
uint256 getBlockHash(int height) override
425426
{
426427
LOCK(::cs_main);
427-
CBlockIndex* block = ::ChainActive()[height];
428+
const CChain& active = Assert(m_node.chainman)->ActiveChain();
429+
CBlockIndex* block = active[height];
428430
assert(block);
429431
return block->GetBlockHash();
430432
}
431433
bool haveBlockOnDisk(int height) override
432434
{
433435
LOCK(cs_main);
434-
CBlockIndex* block = ::ChainActive()[height];
436+
const CChain& active = Assert(m_node.chainman)->ActiveChain();
437+
CBlockIndex* block = active[height];
435438
return block && ((block->nStatus & BLOCK_HAVE_DATA) != 0) && block->nTx > 0;
436439
}
437440
CBlockLocator getTipLocator() override
438441
{
439442
LOCK(cs_main);
440-
return ::ChainActive().GetLocator();
443+
const CChain& active = Assert(m_node.chainman)->ActiveChain();
444+
return active.GetLocator();
441445
}
442446
bool checkFinalTx(const CTransaction& tx) override
443447
{
@@ -447,48 +451,54 @@ class ChainImpl : public Chain
447451
Optional<int> findLocatorFork(const CBlockLocator& locator) override
448452
{
449453
LOCK(cs_main);
450-
if (CBlockIndex* fork = FindForkInGlobalIndex(::ChainActive(), locator)) {
454+
const CChain& active = Assert(m_node.chainman)->ActiveChain();
455+
if (CBlockIndex* fork = FindForkInGlobalIndex(active, locator)) {
451456
return fork->nHeight;
452457
}
453458
return nullopt;
454459
}
455460
bool findBlock(const uint256& hash, const FoundBlock& block) override
456461
{
457462
WAIT_LOCK(cs_main, lock);
458-
return FillBlock(LookupBlockIndex(hash), block, lock);
463+
const CChain& active = Assert(m_node.chainman)->ActiveChain();
464+
return FillBlock(LookupBlockIndex(hash), block, lock, active);
459465
}
460466
bool findFirstBlockWithTimeAndHeight(int64_t min_time, int min_height, const FoundBlock& block) override
461467
{
462468
WAIT_LOCK(cs_main, lock);
463-
return FillBlock(ChainActive().FindEarliestAtLeast(min_time, min_height), block, lock);
469+
const CChain& active = Assert(m_node.chainman)->ActiveChain();
470+
return FillBlock(active.FindEarliestAtLeast(min_time, min_height), block, lock, active);
464471
}
465472
bool findAncestorByHeight(const uint256& block_hash, int ancestor_height, const FoundBlock& ancestor_out) override
466473
{
467474
WAIT_LOCK(cs_main, lock);
475+
const CChain& active = Assert(m_node.chainman)->ActiveChain();
468476
if (const CBlockIndex* block = LookupBlockIndex(block_hash)) {
469477
if (const CBlockIndex* ancestor = block->GetAncestor(ancestor_height)) {
470-
return FillBlock(ancestor, ancestor_out, lock);
478+
return FillBlock(ancestor, ancestor_out, lock, active);
471479
}
472480
}
473-
return FillBlock(nullptr, ancestor_out, lock);
481+
return FillBlock(nullptr, ancestor_out, lock, active);
474482
}
475483
bool findAncestorByHash(const uint256& block_hash, const uint256& ancestor_hash, const FoundBlock& ancestor_out) override
476484
{
477485
WAIT_LOCK(cs_main, lock);
486+
const CChain& active = Assert(m_node.chainman)->ActiveChain();
478487
const CBlockIndex* block = LookupBlockIndex(block_hash);
479488
const CBlockIndex* ancestor = LookupBlockIndex(ancestor_hash);
480489
if (block && ancestor && block->GetAncestor(ancestor->nHeight) != ancestor) ancestor = nullptr;
481-
return FillBlock(ancestor, ancestor_out, lock);
490+
return FillBlock(ancestor, ancestor_out, lock, active);
482491
}
483492
bool findCommonAncestor(const uint256& block_hash1, const uint256& block_hash2, const FoundBlock& ancestor_out, const FoundBlock& block1_out, const FoundBlock& block2_out) override
484493
{
485494
WAIT_LOCK(cs_main, lock);
495+
const CChain& active = Assert(m_node.chainman)->ActiveChain();
486496
const CBlockIndex* block1 = LookupBlockIndex(block_hash1);
487497
const CBlockIndex* block2 = LookupBlockIndex(block_hash2);
488498
const CBlockIndex* ancestor = block1 && block2 ? LastCommonAncestor(block1, block2) : nullptr;
489499
// Using & instead of && below to avoid short circuiting and leaving
490500
// output uninitialized.
491-
return FillBlock(ancestor, ancestor_out, lock) & FillBlock(block1, block1_out, lock) & FillBlock(block2, block2_out, lock);
501+
return FillBlock(ancestor, ancestor_out, lock, active) & FillBlock(block1, block1_out, lock, active) & FillBlock(block2, block2_out, lock, active);
492502
}
493503
void findCoins(std::map<COutPoint, Coin>& coins) override { return FindCoins(m_node, coins); }
494504
double guessVerificationProgress(const uint256& block_hash) override
@@ -608,7 +618,8 @@ class ChainImpl : public Chain
608618
{
609619
if (!old_tip.IsNull()) {
610620
LOCK(::cs_main);
611-
if (old_tip == ::ChainActive().Tip()->GetBlockHash()) return;
621+
const CChain& active = Assert(m_node.chainman)->ActiveChain();
622+
if (old_tip == active.Tip()->GetBlockHash()) return;
612623
}
613624
SyncWithValidationInterfaceQueue();
614625
}

‎src/test/interfaces_tests.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ BOOST_FIXTURE_TEST_SUITE(interfaces_tests, TestChain100Setup)
1818
BOOST_AUTO_TEST_CASE(findBlock)
1919
{
2020
auto chain = interfaces::MakeChain(m_node);
21-
auto& active = ChainActive();
21+
const CChain& active = Assert(m_node.chainman)->ActiveChain();
2222

2323
uint256 hash;
2424
BOOST_CHECK(chain->findBlock(active[10]->GetBlockHash(), FoundBlock().hash(hash)));
@@ -62,7 +62,7 @@ BOOST_AUTO_TEST_CASE(findBlock)
6262
BOOST_AUTO_TEST_CASE(findFirstBlockWithTimeAndHeight)
6363
{
6464
auto chain = interfaces::MakeChain(m_node);
65-
auto& active = ChainActive();
65+
const CChain& active = Assert(m_node.chainman)->ActiveChain();
6666
uint256 hash;
6767
int height;
6868
BOOST_CHECK(chain->findFirstBlockWithTimeAndHeight(/* min_time= */ 0, /* min_height= */ 5, FoundBlock().hash(hash).height(height)));
@@ -74,7 +74,7 @@ BOOST_AUTO_TEST_CASE(findFirstBlockWithTimeAndHeight)
7474
BOOST_AUTO_TEST_CASE(findAncestorByHeight)
7575
{
7676
auto chain = interfaces::MakeChain(m_node);
77-
auto& active = ChainActive();
77+
const CChain& active = Assert(m_node.chainman)->ActiveChain();
7878
uint256 hash;
7979
BOOST_CHECK(chain->findAncestorByHeight(active[20]->GetBlockHash(), 10, FoundBlock().hash(hash)));
8080
BOOST_CHECK_EQUAL(hash, active[10]->GetBlockHash());
@@ -84,7 +84,7 @@ BOOST_AUTO_TEST_CASE(findAncestorByHeight)
8484
BOOST_AUTO_TEST_CASE(findAncestorByHash)
8585
{
8686
auto chain = interfaces::MakeChain(m_node);
87-
auto& active = ChainActive();
87+
const CChain& active = Assert(m_node.chainman)->ActiveChain();
8888
int height = -1;
8989
BOOST_CHECK(chain->findAncestorByHash(active[20]->GetBlockHash(), active[10]->GetBlockHash(), FoundBlock().height(height)));
9090
BOOST_CHECK_EQUAL(height, 10);
@@ -94,7 +94,7 @@ BOOST_AUTO_TEST_CASE(findAncestorByHash)
9494
BOOST_AUTO_TEST_CASE(findCommonAncestor)
9595
{
9696
auto chain = interfaces::MakeChain(m_node);
97-
auto& active = ChainActive();
97+
const CChain& active = Assert(m_node.chainman)->ActiveChain();
9898
auto* orig_tip = active.Tip();
9999
for (int i = 0; i < 10; ++i) {
100100
BlockValidationState state;
@@ -124,7 +124,7 @@ BOOST_AUTO_TEST_CASE(findCommonAncestor)
124124
BOOST_AUTO_TEST_CASE(hasBlocks)
125125
{
126126
auto chain = interfaces::MakeChain(m_node);
127-
auto& active = ChainActive();
127+
const CChain& active = Assert(m_node.chainman)->ActiveChain();
128128

129129
// Test ranges
130130
BOOST_CHECK(chain->hasBlocks(active.Tip()->GetBlockHash(), 10, 90));

‎src/wallet/test/wallet_tests.cpp

Lines changed: 15 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -83,12 +83,9 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup)
8383
CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey()));
8484
CBlockIndex* newTip = ::ChainActive().Tip();
8585

86-
NodeContext node;
87-
auto chain = interfaces::MakeChain(node);
88-
8986
// Verify ScanForWalletTransactions fails to read an unknown start block.
9087
{
91-
CWallet wallet(chain.get(), "", CreateDummyWalletDatabase());
88+
CWallet wallet(m_node.chain.get(), "", CreateDummyWalletDatabase());
9289
{
9390
LOCK(wallet.cs_wallet);
9491
wallet.SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash());
@@ -107,7 +104,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup)
107104
// Verify ScanForWalletTransactions picks up transactions in both the old
108105
// and new block files.
109106
{
110-
CWallet wallet(chain.get(), "", CreateDummyWalletDatabase());
107+
CWallet wallet(m_node.chain.get(), "", CreateDummyWalletDatabase());
111108
{
112109
LOCK(wallet.cs_wallet);
113110
wallet.SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash());
@@ -133,7 +130,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup)
133130
// Verify ScanForWalletTransactions only picks transactions in the new block
134131
// file.
135132
{
136-
CWallet wallet(chain.get(), "", CreateDummyWalletDatabase());
133+
CWallet wallet(m_node.chain.get(), "", CreateDummyWalletDatabase());
137134
{
138135
LOCK(wallet.cs_wallet);
139136
wallet.SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash());
@@ -158,7 +155,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup)
158155

159156
// Verify ScanForWalletTransactions scans no blocks.
160157
{
161-
CWallet wallet(chain.get(), "", CreateDummyWalletDatabase());
158+
CWallet wallet(m_node.chain.get(), "", CreateDummyWalletDatabase());
162159
{
163160
LOCK(wallet.cs_wallet);
164161
wallet.SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash());
@@ -183,9 +180,6 @@ BOOST_FIXTURE_TEST_CASE(importmulti_rescan, TestChain100Setup)
183180
CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey()));
184181
CBlockIndex* newTip = ::ChainActive().Tip();
185182

186-
NodeContext node;
187-
auto chain = interfaces::MakeChain(node);
188-
189183
// Prune the older block file.
190184
{
191185
LOCK(cs_main);
@@ -197,7 +191,7 @@ BOOST_FIXTURE_TEST_CASE(importmulti_rescan, TestChain100Setup)
197191
// before the missing block, and success for a key whose creation time is
198192
// after.
199193
{
200-
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(chain.get(), "", CreateDummyWalletDatabase());
194+
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(m_node.chain.get(), "", CreateDummyWalletDatabase());
201195
wallet->SetupLegacyScriptPubKeyMan();
202196
WITH_LOCK(wallet->cs_wallet, wallet->SetLastBlockProcessed(newTip->nHeight, newTip->GetBlockHash()));
203197
AddWallet(wallet);
@@ -255,14 +249,11 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup)
255249
SetMockTime(KEY_TIME);
256250
m_coinbase_txns.emplace_back(CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())).vtx[0]);
257251

258-
NodeContext node;
259-
auto chain = interfaces::MakeChain(node);
260-
261252
std::string backup_file = (GetDataDir() / "wallet.backup").string();
262253

263254
// Import key into wallet and call dumpwallet to create backup file.
264255
{
265-
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(chain.get(), "", CreateDummyWalletDatabase());
256+
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(m_node.chain.get(), "", CreateDummyWalletDatabase());
266257
{
267258
auto spk_man = wallet->GetOrCreateLegacyScriptPubKeyMan();
268259
LOCK2(wallet->cs_wallet, spk_man->cs_KeyStore);
@@ -284,7 +275,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup)
284275
// Call importwallet RPC and verify all blocks with timestamps >= BLOCK_TIME
285276
// were scanned, and no prior blocks were scanned.
286277
{
287-
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(chain.get(), "", CreateDummyWalletDatabase());
278+
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(m_node.chain.get(), "", CreateDummyWalletDatabase());
288279
LOCK(wallet->cs_wallet);
289280
wallet->SetupLegacyScriptPubKeyMan();
290281

@@ -317,10 +308,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup)
317308
// debit functions.
318309
BOOST_FIXTURE_TEST_CASE(coin_mark_dirty_immature_credit, TestChain100Setup)
319310
{
320-
NodeContext node;
321-
auto chain = interfaces::MakeChain(node);
322-
323-
CWallet wallet(chain.get(), "", CreateDummyWalletDatabase());
311+
CWallet wallet(m_node.chain.get(), "", CreateDummyWalletDatabase());
324312
auto spk_man = wallet.GetOrCreateLegacyScriptPubKeyMan();
325313
CWalletTx wtx(&wallet, m_coinbase_txns.back());
326314

@@ -495,7 +483,7 @@ class ListCoinsTestingSetup : public TestChain100Setup
495483
ListCoinsTestingSetup()
496484
{
497485
CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey()));
498-
wallet = MakeUnique<CWallet>(m_chain.get(), "", CreateMockWalletDatabase());
486+
wallet = MakeUnique<CWallet>(m_node.chain.get(), "", CreateMockWalletDatabase());
499487
{
500488
LOCK2(wallet->cs_wallet, ::cs_main);
501489
wallet->SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash());
@@ -545,7 +533,6 @@ class ListCoinsTestingSetup : public TestChain100Setup
545533
return it->second;
546534
}
547535

548-
std::unique_ptr<interfaces::Chain> m_chain = interfaces::MakeChain(m_node);
549536
std::unique_ptr<CWallet> wallet;
550537
};
551538

@@ -612,9 +599,7 @@ BOOST_FIXTURE_TEST_CASE(ListCoins, ListCoinsTestingSetup)
612599

613600
BOOST_FIXTURE_TEST_CASE(wallet_disableprivkeys, TestChain100Setup)
614601
{
615-
NodeContext node;
616-
auto chain = interfaces::MakeChain(node);
617-
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(chain.get(), "", CreateDummyWalletDatabase());
602+
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(m_node.chain.get(), "", CreateDummyWalletDatabase());
618603
wallet->SetupLegacyScriptPubKeyMan();
619604
wallet->SetMinVersion(FEATURE_LATEST);
620605
wallet->SetWalletFlag(WALLET_FLAG_DISABLE_PRIVATE_KEYS);
@@ -709,8 +694,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_descriptor_test, BasicTestingSetup)
709694
BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup)
710695
{
711696
// Create new wallet with known key and unload it.
712-
auto chain = interfaces::MakeChain(m_node);
713-
auto wallet = TestLoadWallet(*chain);
697+
auto wallet = TestLoadWallet(*m_node.chain);
714698
CKey key;
715699
key.MakeNewKey(true);
716700
AddKey(*wallet, key);
@@ -745,12 +729,12 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup)
745729
auto block_tx = TestSimpleSpend(*m_coinbase_txns[0], 0, coinbaseKey, GetScriptForRawPubKey(key.GetPubKey()));
746730
m_coinbase_txns.push_back(CreateAndProcessBlock({block_tx}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())).vtx[0]);
747731
auto mempool_tx = TestSimpleSpend(*m_coinbase_txns[1], 0, coinbaseKey, GetScriptForRawPubKey(key.GetPubKey()));
748-
BOOST_CHECK(chain->broadcastTransaction(MakeTransactionRef(mempool_tx), DEFAULT_TRANSACTION_MAXFEE, false, error));
732+
BOOST_CHECK(m_node.chain->broadcastTransaction(MakeTransactionRef(mempool_tx), DEFAULT_TRANSACTION_MAXFEE, false, error));
749733

750734

751735
// Reload wallet and make sure new transactions are detected despite events
752736
// being blocked
753-
wallet = TestLoadWallet(*chain);
737+
wallet = TestLoadWallet(*m_node.chain);
754738
BOOST_CHECK(rescan_completed);
755739
BOOST_CHECK_EQUAL(addtx_count, 2);
756740
{
@@ -783,12 +767,12 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup)
783767
block_tx = TestSimpleSpend(*m_coinbase_txns[2], 0, coinbaseKey, GetScriptForRawPubKey(key.GetPubKey()));
784768
m_coinbase_txns.push_back(CreateAndProcessBlock({block_tx}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())).vtx[0]);
785769
mempool_tx = TestSimpleSpend(*m_coinbase_txns[3], 0, coinbaseKey, GetScriptForRawPubKey(key.GetPubKey()));
786-
BOOST_CHECK(chain->broadcastTransaction(MakeTransactionRef(mempool_tx), DEFAULT_TRANSACTION_MAXFEE, false, error));
770+
BOOST_CHECK(m_node.chain->broadcastTransaction(MakeTransactionRef(mempool_tx), DEFAULT_TRANSACTION_MAXFEE, false, error));
787771
LEAVE_CRITICAL_SECTION(wallet->wallet()->cs_wallet);
788772
SyncWithValidationInterfaceQueue();
789773
ENTER_CRITICAL_SECTION(wallet->wallet()->cs_wallet);
790774
});
791-
wallet = TestLoadWallet(*chain);
775+
wallet = TestLoadWallet(*m_node.chain);
792776
BOOST_CHECK_EQUAL(addtx_count, 4);
793777
{
794778
LOCK(wallet->cs_wallet);

0 commit comments

Comments
 (0)