Skip to content

Commit 759e494

Browse files
ryanofskyryanofsky
committed
wallet: Avoid use of Chain::Lock in rescanblockchain
This is a step toward removing the Chain::Lock class and reducing cs_main locking. This change only affects behavior in the case where wallet last block processed falls behind the chain tip. The rescanblockchain error height error checking will just be stricter in this case and only accept values up to the last processed height
1 parent 6067b74 commit 759e494

File tree

6 files changed

+55
-40
lines changed

6 files changed

+55
-40
lines changed

‎src/interfaces/chain.cpp‎

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -88,20 +88,6 @@ class LockImpl : public Chain::Lock, public UniqueLock<RecursiveMutex>
8888
}
8989
return nullopt;
9090
}
91-
Optional<int> findPruned(int start_height, Optional<int> stop_height) override
92-
{
93-
LockAssertion lock(::cs_main);
94-
if (::fPruneMode) {
95-
CBlockIndex* block = stop_height ? ::ChainActive()[*stop_height] : ::ChainActive().Tip();
96-
while (block && block->nHeight >= start_height) {
97-
if ((block->nStatus & BLOCK_HAVE_DATA) == 0) {
98-
return block->nHeight;
99-
}
100-
block = block->pprev;
101-
}
102-
}
103-
return nullopt;
104-
}
10591
Optional<int> findFork(const uint256& hash, Optional<int>* height) override
10692
{
10793
LockAssertion lock(::cs_main);
@@ -309,6 +295,17 @@ class ChainImpl : public Chain
309295
LOCK(cs_main);
310296
return GuessVerificationProgress(Params().TxData(), LookupBlockIndex(block_hash));
311297
}
298+
bool hasBlocks(const uint256& block_hash, Optional<int> min_height, Optional<int> max_height) override
299+
{
300+
LOCK(::cs_main);
301+
CBlockIndex* block = LookupBlockIndex(block_hash);
302+
if (!block) return false;
303+
if (block && max_height) block = block->GetAncestor(*max_height);
304+
for (; block && (!min_height || block->nHeight >= *min_height); block = block->pprev) {
305+
if (!(block->nStatus & BLOCK_HAVE_DATA)) return false;
306+
}
307+
return true;
308+
}
312309
RBFTransactionState isRBFOptIn(const CTransaction& tx) override
313310
{
314311
LOCK(::mempool.cs);

‎src/interfaces/chain.h‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,6 @@ class Chain
9393
//! (to avoid the cost of a second lookup in case this information is needed.)
9494
virtual Optional<int> findFirstBlockWithTimeAndHeight(int64_t time, int height, uint256* hash) = 0;
9595

96-
//! Return height of last block in the specified range which is pruned, or
97-
//! nullopt if no block in the range is pruned. Range is inclusive.
98-
virtual Optional<int> findPruned(int start_height = 0, Optional<int> stop_height = nullopt) = 0;
99-
10096
//! Return height of the specified block if it is on the chain, otherwise
10197
//! return the height of the highest block on chain that's an ancestor
10298
//! of the specified block, or nullopt if there is no common ancestor.
@@ -166,6 +162,9 @@ class Chain
166162
//! the specified block hash are verified.
167163
virtual double guessVerificationProgress(const uint256& block_hash) = 0;
168164

165+
//! Return true if data is available for the specified blocks.
166+
virtual bool hasBlocks(const uint256& block_hash, Optional<int> min_height, Optional<int> max_height) = 0;
167+
169168
//! Check if transaction is RBF opt in.
170169
virtual RBFTransactionState isRBFOptIn(const CTransaction& tx) = 0;
171170

‎src/test/interfaces_tests.cpp‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,4 +72,29 @@ BOOST_AUTO_TEST_CASE(findCommonAncestor)
7272
BOOST_CHECK_EQUAL(fork_hash, active[fork_height]->GetBlockHash());
7373
}
7474

75+
BOOST_AUTO_TEST_CASE(hasBlocks)
76+
{
77+
auto chain = interfaces::MakeChain(m_node);
78+
auto& active = ChainActive();
79+
BOOST_CHECK(chain->hasBlocks(active.Tip()->GetBlockHash(), 10, 90));
80+
BOOST_CHECK(chain->hasBlocks(active.Tip()->GetBlockHash(), 10, {}));
81+
BOOST_CHECK(chain->hasBlocks(active.Tip()->GetBlockHash(), {}, 90));
82+
BOOST_CHECK(chain->hasBlocks(active.Tip()->GetBlockHash(), {}, {}));
83+
active[5]->nStatus &= ~BLOCK_HAVE_DATA;
84+
BOOST_CHECK(chain->hasBlocks(active.Tip()->GetBlockHash(), 10, 90));
85+
BOOST_CHECK(chain->hasBlocks(active.Tip()->GetBlockHash(), 10, {}));
86+
BOOST_CHECK(!chain->hasBlocks(active.Tip()->GetBlockHash(), {}, 90));
87+
BOOST_CHECK(!chain->hasBlocks(active.Tip()->GetBlockHash(), {}, {}));
88+
active[95]->nStatus &= ~BLOCK_HAVE_DATA;
89+
BOOST_CHECK(chain->hasBlocks(active.Tip()->GetBlockHash(), 10, 90));
90+
BOOST_CHECK(!chain->hasBlocks(active.Tip()->GetBlockHash(), 10, {}));
91+
BOOST_CHECK(!chain->hasBlocks(active.Tip()->GetBlockHash(), {}, 90));
92+
BOOST_CHECK(!chain->hasBlocks(active.Tip()->GetBlockHash(), {}, {}));
93+
active[50]->nStatus &= ~BLOCK_HAVE_DATA;
94+
BOOST_CHECK(!chain->hasBlocks(active.Tip()->GetBlockHash(), 10, 90));
95+
BOOST_CHECK(!chain->hasBlocks(active.Tip()->GetBlockHash(), 10, {}));
96+
BOOST_CHECK(!chain->hasBlocks(active.Tip()->GetBlockHash(), {}, 90));
97+
BOOST_CHECK(!chain->hasBlocks(active.Tip()->GetBlockHash(), {}, {}));
98+
}
99+
75100
BOOST_AUTO_TEST_SUITE_END()

‎src/wallet/rpcwallet.cpp‎

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3545,22 +3545,23 @@ UniValue rescanblockchain(const JSONRPCRequest& request)
35453545
}
35463546

35473547
int start_height = 0;
3548-
uint256 start_block, stop_block;
3548+
Optional<int> stop_height;
3549+
uint256 start_block;
35493550
{
35503551
auto locked_chain = pwallet->chain().lock();
3551-
Optional<int> tip_height = locked_chain->getHeight();
3552+
LOCK(pwallet->cs_wallet);
3553+
int tip_height = pwallet->GetLastBlockHeight();
35523554

35533555
if (!request.params[0].isNull()) {
35543556
start_height = request.params[0].get_int();
3555-
if (start_height < 0 || !tip_height || start_height > *tip_height) {
3557+
if (start_height < 0 || tip_height < 0 || start_height > tip_height) {
35563558
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid start_height");
35573559
}
35583560
}
35593561

3560-
Optional<int> stop_height;
35613562
if (!request.params[1].isNull()) {
35623563
stop_height = request.params[1].get_int();
3563-
if (*stop_height < 0 || !tip_height || *stop_height > *tip_height) {
3564+
if (*stop_height < 0 || tip_height < 0 || *stop_height > tip_height) {
35643565
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid stop_height");
35653566
}
35663567
else if (*stop_height < start_height) {
@@ -3569,25 +3570,17 @@ UniValue rescanblockchain(const JSONRPCRequest& request)
35693570
}
35703571

35713572
// We can't rescan beyond non-pruned blocks, stop and throw an error
3572-
if (locked_chain->findPruned(start_height, stop_height)) {
3573+
if (!pwallet->chain().hasBlocks(pwallet->GetLastBlockHash(), start_height, stop_height)) {
35733574
throw JSONRPCError(RPC_MISC_ERROR, "Can't rescan beyond pruned data. Use RPC call getblockchaininfo to determine your pruned height.");
35743575
}
35753576

3576-
if (tip_height) {
3577-
start_block = locked_chain->getBlockHash(start_height);
3578-
// If called with a stop_height, set the stop_height here to
3579-
// trigger a rescan to that height.
3580-
// If called without a stop height, leave stop_height as null here
3581-
// so rescan continues to the tip (even if the tip advances during
3582-
// rescan).
3583-
if (stop_height) {
3584-
stop_block = locked_chain->getBlockHash(*stop_height);
3585-
}
3577+
if (tip_height >= 0) {
3578+
start_block = pwallet->chain().findAncestorByHeight(pwallet->GetLastBlockHash(), start_height);
35863579
}
35873580
}
35883581

35893582
CWallet::ScanResult result =
3590-
pwallet->ScanForWalletTransactions(start_block, stop_block, reserver, true /* fUpdate */);
3583+
pwallet->ScanForWalletTransactions(start_block, stop_height, reserver, true /* fUpdate */);
35913584
switch (result.status) {
35923585
case CWallet::ScanResult::SUCCESS:
35933586
break;

‎src/wallet/wallet.cpp‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1623,7 +1623,7 @@ int64_t CWallet::RescanFromTime(int64_t startTime, const WalletRescanReserver& r
16231623
* the main chain after to the addition of any new keys you want to detect
16241624
* transactions for.
16251625
*/
1626-
CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_block, const uint256& stop_block, const WalletRescanReserver& reserver, bool fUpdate)
1626+
CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_block, Optional<int> max_height, const WalletRescanReserver& reserver, bool fUpdate)
16271627
{
16281628
int64_t nNow = GetTime();
16291629
int64_t start_time = GetTimeMillis();
@@ -1649,7 +1649,8 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc
16491649
}
16501650
block_height = locked_chain->getBlockHeight(block_hash);
16511651
progress_begin = chain().guessVerificationProgress(block_hash);
1652-
progress_end = chain().guessVerificationProgress(stop_block.IsNull() ? tip_hash : stop_block);
1652+
uint256 stop_block = max_height ? chain().findAncestorByHeight(tip_hash, *max_height) : tip_hash;
1653+
progress_end = chain().guessVerificationProgress(stop_block);
16531654
}
16541655
double progress_current = progress_begin;
16551656
while (block_height && !fAbortRescan && !chain().shutdownRequested()) {
@@ -1687,7 +1688,7 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc
16871688
result.last_failed_block = block_hash;
16881689
result.status = ScanResult::FAILURE;
16891690
}
1690-
if (block_hash == stop_block) {
1691+
if (max_height && *block_height >= *max_height) {
16911692
break;
16921693
}
16931694
{
@@ -1706,7 +1707,7 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc
17061707
// handle updated tip hash
17071708
const uint256 prev_tip_hash = tip_hash;
17081709
tip_hash = locked_chain->getBlockHash(*tip_height);
1709-
if (stop_block.IsNull() && prev_tip_hash != tip_hash) {
1710+
if (!max_height && prev_tip_hash != tip_hash) {
17101711
// in case the tip has changed, update progress max
17111712
progress_end = chain().guessVerificationProgress(tip_hash);
17121713
}

‎src/wallet/wallet.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -894,7 +894,7 @@ class CWallet final : public WalletStorage, private interfaces::Chain::Notificat
894894
//! USER_ABORT.
895895
uint256 last_failed_block;
896896
};
897-
ScanResult ScanForWalletTransactions(const uint256& first_block, const uint256& last_block, const WalletRescanReserver& reserver, bool fUpdate);
897+
ScanResult ScanForWalletTransactions(const uint256& first_block, Optional<int> max_height, const WalletRescanReserver& reserver, bool fUpdate);
898898
void TransactionRemovedFromMempool(const CTransactionRef &ptx) override;
899899
void ReacceptWalletTransactions() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
900900
void ResendWalletTransactions();

0 commit comments

Comments
 (0)