Merge 539023ab41 into merged_master (Bitcoin PR bitcoin/bitcoin#22492)

This commit is contained in:
Andrew Poelstra 2021-08-03 16:35:52 +00:00
commit 71045e76ea
3 changed files with 53 additions and 35 deletions

View file

@ -743,7 +743,7 @@ RPCHelpMan dumpwallet()
// the user could have gotten from another RPC command prior to now
wallet.BlockUntilSyncedToCurrentChain();
LOCK2(wallet.cs_wallet, spk_man.cs_KeyStore);
LOCK(wallet.cs_wallet);
EnsureWalletIsUnlocked(wallet);
@ -765,9 +765,16 @@ RPCHelpMan dumpwallet()
throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot open wallet dump file");
std::map<CKeyID, int64_t> mapKeyBirth;
const std::map<CKeyID, int64_t>& mapKeyPool = spk_man.GetAllReserveKeys();
wallet.GetKeyBirthTimes(mapKeyBirth);
int64_t block_time = 0;
CHECK_NONFATAL(wallet.chain().findBlock(wallet.GetLastBlockHash(), FoundBlock().time(block_time)));
// Note: To avoid a lock order issue, access to cs_main must be locked before cs_KeyStore.
// So we do the two things in this function that lock cs_main first: GetKeyBirthTimes, and findBlock.
LOCK(spk_man.cs_KeyStore);
const std::map<CKeyID, int64_t>& mapKeyPool = spk_man.GetAllReserveKeys();
std::set<CScriptID> scripts = spk_man.GetCScripts();
// sort time/key pairs
@ -782,8 +789,6 @@ RPCHelpMan dumpwallet()
file << strprintf("# Wallet dump created by Bitcoin %s\n", CLIENT_BUILD);
file << strprintf("# * Created on %s\n", FormatISO8601DateTime(GetTime()));
file << strprintf("# * Best block at time of backup was %i (%s),\n", wallet.GetLastBlockHeight(), wallet.GetLastBlockHash().ToString());
int64_t block_time = 0;
CHECK_NONFATAL(wallet.chain().findBlock(wallet.GetLastBlockHash(), FoundBlock().time(block_time)));
file << strprintf("# mined on %s\n", FormatISO8601DateTime(block_time));
file << "\n";

View file

@ -2489,44 +2489,48 @@ void CWallet::GetKeyBirthTimes(std::map<CKeyID, int64_t>& mapKeyBirth) const {
AssertLockHeld(cs_wallet);
mapKeyBirth.clear();
LegacyScriptPubKeyMan* spk_man = GetLegacyScriptPubKeyMan();
assert(spk_man != nullptr);
LOCK(spk_man->cs_KeyStore);
// get birth times for keys with metadata
for (const auto& entry : spk_man->mapKeyMetadata) {
if (entry.second.nCreateTime) {
mapKeyBirth[entry.first] = entry.second.nCreateTime;
}
}
// map in which we'll infer heights of other keys
std::map<CKeyID, const CWalletTx::Confirmation*> mapKeyFirstBlock;
CWalletTx::Confirmation max_confirm;
max_confirm.block_height = GetLastBlockHeight() > 144 ? GetLastBlockHeight() - 144 : 0; // the tip can be reorganized; use a 144-block safety margin
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_confirm;
}
// if there are no such keys, we're done
if (mapKeyFirstBlock.empty())
return;
{
LegacyScriptPubKeyMan* spk_man = GetLegacyScriptPubKeyMan();
assert(spk_man != nullptr);
LOCK(spk_man->cs_KeyStore);
// find first block that affects those keys, if there are any left
for (const auto& entry : mapWallet) {
// iterate over all wallet transactions...
const CWalletTx &wtx = entry.second;
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
auto rit = mapKeyFirstBlock.find(keyid);
if (rit != mapKeyFirstBlock.end() && wtx.m_confirm.block_height < rit->second->block_height) {
rit->second = &wtx.m_confirm;
// get birth times for keys with metadata
for (const auto& entry : spk_man->mapKeyMetadata) {
if (entry.second.nCreateTime) {
mapKeyBirth[entry.first] = entry.second.nCreateTime;
}
}
// Prepare to infer birth heights for keys without metadata
for (const CKeyID &keyid : spk_man->GetKeys()) {
if (mapKeyBirth.count(keyid) == 0)
mapKeyFirstBlock[keyid] = &max_confirm;
}
// if there are no such keys, we're done
if (mapKeyFirstBlock.empty())
return;
// find first block that affects those keys, if there are any left
for (const auto& entry : mapWallet) {
// iterate over all wallet transactions...
const CWalletTx &wtx = entry.second;
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
auto rit = mapKeyFirstBlock.find(keyid);
if (rit != mapKeyFirstBlock.end() && wtx.m_confirm.block_height < rit->second->block_height) {
rit->second = &wtx.m_confirm;
}
}
}
}

View file

@ -209,6 +209,15 @@ class WalletDumpTest(BitcoinTestFramework):
with self.nodes[0].assert_debug_log(['Flushing wallet.dat'], timeout=20):
self.nodes[0].getnewaddress()
# Make sure that dumpwallet doesn't have a lock order issue when there is an unconfirmed tx and it is reloaded
# See https://github.com/bitcoin/bitcoin/issues/22489
self.nodes[0].createwallet("w3")
w3 = self.nodes[0].get_wallet_rpc("w3")
w3.importprivkey(privkey=self.nodes[0].get_deterministic_priv_key().key, label="coinbase_import")
w3.sendtoaddress(w3.getnewaddress(), 10)
w3.unloadwallet()
self.nodes[0].loadwallet("w3")
w3.dumpwallet(os.path.join(self.nodes[0].datadir, "w3.dump"))
if __name__ == '__main__':
WalletDumpTest().main()