mirrors/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2024-11-19 18:09:47 +01:00
Code Issues Projects Releases Wiki Activity

Merge bitcoin/bitcoin#25526: wallet: avoid double keypool TopUp() call on descriptor wallets

Browse Source 
bfb9b94ebe wallet: remove duplicate descriptor type check in GetNewDestination (furszy)
76b982a4a5 wallet: remove unused `nAccountingEntryNumber` field (furszy)
599ff5adfc wallet: avoid double TopUp() calls on descriptor wallets (furszy)

Pull request description:

  Found it while was digging over a `getnewaddress` timeout on the functional test suite.

  ### Context:

  We are calling `TopUp()` twice in the following flows for descriptor wallets:

  A) `CWallet::GetNewDestination`:
     1) Calls spk_man->TopUp()
     2) Calls spk_man->GetNewDestination() --> which, after the basic script checks, calls TopUp() again.

  B) `CWallet::GetReservedDestination`:
     1) Calls spk_man->TopUp()
     2) Calls spk_man->GetReservedDestination() --> which calls to GetNewDestination (which calls to TopUp again).

  ### Changes:

  Move `TopUp()` responsibility from the wallet class to each scriptpubkeyman.
  So each spkm can decide to call it or not after perform the basic checks
  for the new destination request.

  Aside from that, remove the unused `nAccountingEntryNumber` wallet field. And a duplicated descriptor type check in `GetNewDestination`

ACKs for top commit:
  aureleoules:
    re-ACK bfb9b94ebe.
  achow101:
    ACK bfb9b94ebe
  theStack:
    Code-review ACK bfb9b94ebe

Tree-SHA512: 3ab73f37729e50d6c6a4434f676855bc1fb404619d63c03e5b06ce61c292c09c59d64cb1aa3bd9277b06f26988956991d62c90f9d835884f41ed500b43a12058
This commit is contained in:
Andrew Chow 2022-10-13 11:23:12 -04:00
commit 1dec90d95b
No known key found for this signature in database
GPG Key ID: 17565732E08E5E41
3 changed files with 10 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
src/wallet/scriptpubkeyman.cpp
View File

@ -28,6 +28,9 @@ util::Result<CTxDestination> LegacyScriptPubKeyMan::GetNewDestination(const Outp
}
assert(type != OutputType::BECH32M);
// Fill-up keypool if needed
TopUp();
LOCK(cs_KeyStore);
// Generate a new key that is added to wallet
@ -304,6 +307,9 @@ util::Result<CTxDestination> LegacyScriptPubKeyMan::GetReservedDestination(const
return util::Error{_("Error: Keypool ran out, please call keypoolrefill first")};
}
// Fill-up keypool if needed
TopUp();
if (!ReserveKeyFromKeyPool(index, keypool, internal)) {
return util::Error{_("Error: Keypool ran out, please call keypoolrefill first")};
}
@ -1983,7 +1989,7 @@ util::Result<CTxDestination> DescriptorScriptPubKeyMan::GetNewDestination(const
std::optional<OutputType> desc_addr_type = m_wallet_descriptor.descriptor->GetOutputType();
assert(desc_addr_type);
if (type != *desc_addr_type) {
throw std::runtime_error(std::string(__func__) + ": Types are inconsistent");
throw std::runtime_error(std::string(__func__) + ": Types are inconsistent. Stored type does not match type of newly generated address");
}
TopUp();
@ -2001,11 +2007,8 @@ util::Result<CTxDestination> DescriptorScriptPubKeyMan::GetNewDestination(const
}
CTxDestination dest;
std::optional<OutputType> out_script_type = m_wallet_descriptor.descriptor->GetOutputType();
if (out_script_type && out_script_type == type) {
ExtractDestination(scripts_temp[0], dest);
} else {
throw std::runtime_error(std::string(__func__) + ": Types are inconsistent. Stored type does not match type of newly generated address");
if (!ExtractDestination(scripts_temp[0], dest)) {
return util::Error{_("Error: Cannot extract destination from the generated scriptpubkey")}; // shouldn't happen
}
m_wallet_descriptor.next_index++;
WalletBatch(m_storage.GetDatabase()).WriteDescriptor(GetID(), m_wallet_descriptor);

6
src/wallet/wallet.cpp
View File

@ -2386,7 +2386,6 @@ util::Result<CTxDestination> CWallet::GetNewDestination(const OutputType type, c
return util::Error{strprintf(_("Error: No %s addresses available."), FormatOutputType(type))};
}
spk_man->TopUp();
auto op_dest = spk_man->GetNewDestination(type);
if (op_dest) {
SetAddressBook(*op_dest, label, "receive");
@ -2480,10 +2479,7 @@ util::Result<CTxDestination> ReserveDestination::GetReservedDestination(bool int
return util::Error{strprintf(_("Error: No %s addresses available."), FormatOutputType(type))};
}
if (nIndex == -1)
{
m_spk_man->TopUp();
if (nIndex == -1) {
CKeyPool keypool;
auto op_address = m_spk_man->GetReservedDestination(type, internal, nIndex, keypool);
if (!op_address) return op_address;

1
src/wallet/wallet.h
View File

@ -401,7 +401,6 @@ public:
TxItems wtxOrdered;
int64_t nOrderPosNext GUARDED_BY(cs_wallet) = 0;
uint64_t nAccountingEntryNumber = 0;
std::map<CTxDestination, CAddressBookData> m_address_book GUARDED_BY(cs_wallet);
const CAddressBookData* FindAddressBookEntry(const CTxDestination&, bool allow_change = false) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);