Skip to content

Commit c1694ce

Browse files
committed
wallet: Avoid use of Chain::Lock in importprunedfunds
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, in which case the "Block not found in chain" error will be stricter and not allow importing data from a blocks between the wallet last processed tip and the current node tip.
1 parent ade5f87 commit c1694ce

File tree

4 files changed

+30
-5
lines changed

4 files changed

+30
-5
lines changed

src/interfaces/chain.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,14 @@ class ChainImpl : public Chain
267267
WAIT_LOCK(cs_main, lock);
268268
return FillBlock(LookupBlockIndex(hash), block, lock);
269269
}
270+
bool findAncestorByHash(const uint256& block_hash, const uint256& ancestor_hash, const FoundBlock& ancestor_out) override
271+
{
272+
WAIT_LOCK(cs_main, lock);
273+
const CBlockIndex* block = LookupBlockIndex(block_hash);
274+
const CBlockIndex* ancestor = LookupBlockIndex(ancestor_hash);
275+
if (block && ancestor && block->GetAncestor(ancestor->nHeight) != ancestor) ancestor = nullptr;
276+
return FillBlock(ancestor, ancestor_out, lock);
277+
}
270278
void findCoins(std::map<COutPoint, Coin>& coins) override { return FindCoins(m_node, coins); }
271279
double guessVerificationProgress(const uint256& block_hash) override
272280
{

src/interfaces/chain.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,12 @@ class Chain
150150
//! or contents.
151151
virtual bool findBlock(const uint256& hash, const FoundBlock& block={}) = 0;
152152

153+
//! Return whether block descends from a specified ancestor, and
154+
//! optionally return ancestor information.
155+
virtual bool findAncestorByHash(const uint256& block_hash,
156+
const uint256& ancestor_hash,
157+
const FoundBlock& ancestor_out={}) = 0;
158+
153159
//! Look up unspent output information. Returns coins in the mempool and in
154160
//! the current chain UTXO set. Iterates through all the keys in the map and
155161
//! populates the values.

src/test/interfaces_tests.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,4 +44,14 @@ BOOST_AUTO_TEST_CASE(findBlock)
4444
BOOST_CHECK(!chain->findBlock({}, FoundBlock()));
4545
}
4646

47+
BOOST_AUTO_TEST_CASE(findAncestorByHash)
48+
{
49+
auto chain = interfaces::MakeChain(m_node);
50+
auto& active = ChainActive();
51+
int height = -1;
52+
BOOST_CHECK(chain->findAncestorByHash(active[20]->GetBlockHash(), active[10]->GetBlockHash(), FoundBlock().height(height)));
53+
BOOST_CHECK_EQUAL(height, 10);
54+
BOOST_CHECK(!chain->findAncestorByHash(active[10]->GetBlockHash(), active[20]->GetBlockHash()));
55+
}
56+
4757
BOOST_AUTO_TEST_SUITE_END()

src/wallet/rpcdump.cpp

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828

2929

3030

31+
using interfaces::FoundBlock;
32+
3133
std::string static EncodeDumpString(const std::string &str) {
3234
std::stringstream ret;
3335
for (const unsigned char c : str) {
@@ -359,8 +361,9 @@ UniValue importprunedfunds(const JSONRPCRequest& request)
359361
}
360362

361363
auto locked_chain = pwallet->chain().lock();
362-
Optional<int> height = locked_chain->getBlockHeight(merkleBlock.header.GetHash());
363-
if (height == nullopt) {
364+
LOCK(pwallet->cs_wallet);
365+
int height;
366+
if (!pwallet->chain().findAncestorByHash(pwallet->GetLastBlockHash(), merkleBlock.header.GetHash(), FoundBlock().height(height))) {
364367
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found in chain");
365368
}
366369

@@ -371,11 +374,9 @@ UniValue importprunedfunds(const JSONRPCRequest& request)
371374

372375
unsigned int txnIndex = vIndex[it - vMatch.begin()];
373376

374-
CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, *height, merkleBlock.header.GetHash(), txnIndex);
377+
CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, height, merkleBlock.header.GetHash(), txnIndex);
375378
wtx.m_confirm = confirm;
376379

377-
LOCK(pwallet->cs_wallet);
378-
379380
if (pwallet->IsMine(*wtx.tx)) {
380381
pwallet->AddToWallet(wtx, false);
381382
return NullUniValue;

0 commit comments

Comments
 (0)