Skip to content

Commit

Permalink
wallet: Avoid use of Chain::Lock in CWallet::GetKeyBirthTimes
Browse files Browse the repository at this point in the history
Summary:
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, where it will treat the last block processed as the
current tip.

This is part [12/12] of Core [[bitcoin/bitcoin#17954 | PR17954]] : bitcoin/bitcoin@4897340

Depends on D7874

Test Plan:
  ninja all check-all

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D7875
  • Loading branch information
ryanofsky authored and deadalnix committed Oct 11, 2020
1 parent 08a6658 commit 94adb91
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 23 deletions.
6 changes: 0 additions & 6 deletions src/interfaces/chain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,6 @@ namespace {
assert(block != nullptr);
return block->GetBlockHash();
}
int64_t getBlockTime(int height) override {
LockAssertion lock(::cs_main);
CBlockIndex *block = ::ChainActive()[height];
assert(block != nullptr);
return block->GetBlockTime();
}
bool haveBlockOnDisk(int height) override {
LockAssertion lock(::cs_main);
CBlockIndex *block = ::ChainActive()[height];
Expand Down
3 changes: 0 additions & 3 deletions src/interfaces/chain.h
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,6 @@ class Chain {
//! Get block hash. Height must be valid or this function will abort.
virtual BlockHash getBlockHash(int height) = 0;

//! Get block time. Height must be valid or this function will abort.
virtual int64_t getBlockTime(int height) = 0;

//! Check that the block is available on disk (i.e. has not been
//! pruned), and contains transactions.
virtual bool haveBlockOnDisk(int height) = 0;
Expand Down
32 changes: 18 additions & 14 deletions src/wallet/wallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3838,15 +3838,18 @@ void CWallet::GetKeyBirthTimes(interfaces::Chain::Lock &locked_chain,
}
}

// Map in which we'll infer heights of other keys
const Optional<int> tip_height = locked_chain.getHeight();
// map in which we'll infer heights of other keys
std::map<CKeyID, const CWalletTx::Confirmation *> mapKeyFirstBlock;
CWalletTx::Confirmation max_confirm;
// the tip can be reorganized; use a 144-block safety margin
const int max_height =
tip_height && *tip_height > 144 ? *tip_height - 144 : 0;
std::map<CKeyID, int> mapKeyFirstBlock;
max_confirm.block_height =
GetLastBlockHeight() > 144 ? GetLastBlockHeight() - 144 : 0;
CHECK_NONFATAL(chain().findAncestorByHeight(
GetLastBlockHash(), max_confirm.block_height,
FoundBlock().hash(max_confirm.hashBlock)));
for (const CKeyID &keyid : spk_man->GetKeys()) {
if (mapKeyBirth.count(keyid) == 0) {
mapKeyFirstBlock[keyid] = max_height;
mapKeyFirstBlock[keyid] = &max_confirm;
}
}

Expand All @@ -3859,19 +3862,18 @@ void CWallet::GetKeyBirthTimes(interfaces::Chain::Lock &locked_chain,
for (const auto &entry : mapWallet) {
// iterate over all wallet transactions...
const CWalletTx &wtx = entry.second;
if (Optional<int> height =
locked_chain.getBlockHeight(wtx.m_confirm.hashBlock)) {
if (wtx.m_confirm.status == CWalletTx::CONFIRMED) {
// ... which are already in a block
for (const CTxOut &txout : wtx.tx->vout) {
// Iterate over all their outputs...
for (const auto &keyid :
GetAffectedKeys(txout.scriptPubKey, *spk_man)) {
// ... and all their affected keys.
std::map<CKeyID, int>::iterator rit =
mapKeyFirstBlock.find(keyid);
auto rit = mapKeyFirstBlock.find(keyid);
if (rit != mapKeyFirstBlock.end() &&
*height < rit->second) {
rit->second = *height;
wtx.m_confirm.block_height <
rit->second->block_height) {
rit->second = &wtx.m_confirm;
}
}
}
Expand All @@ -3880,9 +3882,11 @@ void CWallet::GetKeyBirthTimes(interfaces::Chain::Lock &locked_chain,

// Extract block timestamps for those keys.
for (const auto &entry : mapKeyFirstBlock) {
int64_t block_time;
CHECK_NONFATAL(chain().findBlock(entry.second->hashBlock,
FoundBlock().time(block_time)));
// block times can be 2h off
mapKeyBirth[entry.first] =
locked_chain.getBlockTime(entry.second) - TIMESTAMP_WINDOW;
mapKeyBirth[entry.first] = block_time - TIMESTAMP_WINDOW;
}
}

Expand Down

0 comments on commit 94adb91

Please sign in to comment.