Skip to content

Commit

Permalink
wallet: Avoid use of Chain::Lock in importprunedfunds
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ryanofsky committed Mar 31, 2020
1 parent ade5f87 commit c1694ce
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 5 deletions.
8 changes: 8 additions & 0 deletions src/interfaces/chain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,14 @@ class ChainImpl : public Chain
WAIT_LOCK(cs_main, lock);
return FillBlock(LookupBlockIndex(hash), block, lock);
}
bool findAncestorByHash(const uint256& block_hash, const uint256& ancestor_hash, const FoundBlock& ancestor_out) override
{
WAIT_LOCK(cs_main, lock);
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);
}
void findCoins(std::map<COutPoint, Coin>& coins) override { return FindCoins(m_node, coins); }
double guessVerificationProgress(const uint256& block_hash) override
{
Expand Down
6 changes: 6 additions & 0 deletions src/interfaces/chain.h
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,12 @@ class Chain
//! or contents.
virtual bool findBlock(const uint256& hash, const FoundBlock& block={}) = 0;

//! Return whether block descends from a specified ancestor, and
//! optionally return ancestor information.
virtual bool findAncestorByHash(const uint256& block_hash,
const uint256& ancestor_hash,
const FoundBlock& ancestor_out={}) = 0;

//! Look up unspent output information. Returns coins in the mempool and in
//! the current chain UTXO set. Iterates through all the keys in the map and
//! populates the values.
Expand Down
10 changes: 10 additions & 0 deletions src/test/interfaces_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,14 @@ BOOST_AUTO_TEST_CASE(findBlock)
BOOST_CHECK(!chain->findBlock({}, FoundBlock()));
}

BOOST_AUTO_TEST_CASE(findAncestorByHash)
{
auto chain = interfaces::MakeChain(m_node);
auto& active = ChainActive();
int height = -1;
BOOST_CHECK(chain->findAncestorByHash(active[20]->GetBlockHash(), active[10]->GetBlockHash(), FoundBlock().height(height)));
BOOST_CHECK_EQUAL(height, 10);
BOOST_CHECK(!chain->findAncestorByHash(active[10]->GetBlockHash(), active[20]->GetBlockHash()));
}

BOOST_AUTO_TEST_SUITE_END()
11 changes: 6 additions & 5 deletions src/wallet/rpcdump.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@



using interfaces::FoundBlock;

std::string static EncodeDumpString(const std::string &str) {
std::stringstream ret;
for (const unsigned char c : str) {
Expand Down Expand Up @@ -359,8 +361,9 @@ UniValue importprunedfunds(const JSONRPCRequest& request)
}

auto locked_chain = pwallet->chain().lock();
Optional<int> height = locked_chain->getBlockHeight(merkleBlock.header.GetHash());
if (height == nullopt) {
LOCK(pwallet->cs_wallet);
int height;
if (!pwallet->chain().findAncestorByHash(pwallet->GetLastBlockHash(), merkleBlock.header.GetHash(), FoundBlock().height(height))) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found in chain");
}

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

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

CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, *height, merkleBlock.header.GetHash(), txnIndex);
CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, height, merkleBlock.header.GetHash(), txnIndex);
wtx.m_confirm = confirm;

LOCK(pwallet->cs_wallet);

if (pwallet->IsMine(*wtx.tx)) {
pwallet->AddToWallet(wtx, false);
return NullUniValue;
Expand Down

0 comments on commit c1694ce

Please sign in to comment.