Skip to content

Commit

Permalink
Merge #20266: wallet: fix change detection of imported internal descr…
Browse files Browse the repository at this point in the history
…iptors

bd93fc9 Fix change detection of imported internal descriptors (Andrew Chow)

Pull request description:

  Import internal descriptors were having address book entries added which meant they would be detected as non-change. Fix this and add a test for it.

ACKs for top commit:
  laanwj:
    Code review ACK bd93fc9
  meshcollider:
    utACK bd93fc9
  promag:
    Code review ACK bd93fc9.

Tree-SHA512: 8fa9e364be317627ec171eedffdb505976c0e7f1e55bc7e8cfdffa3aeea5db24d231f55166602cd0e97a5ba621acc871de0a765c75d0c65678f83e93c3b657c5
  • Loading branch information
laanwj committed Nov 9, 2020
2 parents f70eb51 + bd93fc9 commit 663fd92
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 4 deletions.
2 changes: 1 addition & 1 deletion src/wallet/rpcdump.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1559,7 +1559,7 @@ static UniValue ProcessDescriptorImport(CWallet * const pwallet, const UniValue&
}

// Add descriptor to the wallet
auto spk_manager = pwallet->AddWalletDescriptor(w_desc, keys, label);
auto spk_manager = pwallet->AddWalletDescriptor(w_desc, keys, label, internal);
if (spk_manager == nullptr) {
throw JSONRPCError(RPC_WALLET_ERROR, strprintf("Could not add descriptor '%s'", descriptor));
}
Expand Down
4 changes: 2 additions & 2 deletions src/wallet/wallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4511,7 +4511,7 @@ DescriptorScriptPubKeyMan* CWallet::GetDescriptorScriptPubKeyMan(const WalletDes
return nullptr;
}

ScriptPubKeyMan* CWallet::AddWalletDescriptor(WalletDescriptor& desc, const FlatSigningProvider& signing_provider, const std::string& label)
ScriptPubKeyMan* CWallet::AddWalletDescriptor(WalletDescriptor& desc, const FlatSigningProvider& signing_provider, const std::string& label, bool internal)
{
if (!IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)) {
WalletLogPrintf("Cannot add WalletDescriptor to a non-descriptor wallet\n");
Expand Down Expand Up @@ -4568,7 +4568,7 @@ ScriptPubKeyMan* CWallet::AddWalletDescriptor(WalletDescriptor& desc, const Flat
}

CTxDestination dest;
if (ExtractDestination(script_pub_keys.at(0), dest)) {
if (!internal && ExtractDestination(script_pub_keys.at(0), dest)) {
SetAddressBook(dest, label, "receive");
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/wallet/wallet.h
Original file line number Diff line number Diff line change
Expand Up @@ -1280,7 +1280,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati
DescriptorScriptPubKeyMan* GetDescriptorScriptPubKeyMan(const WalletDescriptor& desc) const;

//! Add a descriptor to the wallet, return a ScriptPubKeyMan & associated output type
ScriptPubKeyMan* AddWalletDescriptor(WalletDescriptor& desc, const FlatSigningProvider& signing_provider, const std::string& label);
ScriptPubKeyMan* AddWalletDescriptor(WalletDescriptor& desc, const FlatSigningProvider& signing_provider, const std::string& label, bool internal);
};

/**
Expand Down
12 changes: 12 additions & 0 deletions test/functional/wallet_importdescriptors.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
- `test_address()` is called to call getaddressinfo for an address on node1
and test the values returned."""

from test_framework.address import key_to_p2pkh
from test_framework.test_framework import BitcoinTestFramework
from test_framework.descriptors import descsum_create
from test_framework.util import (
Expand Down Expand Up @@ -107,6 +108,17 @@ def run_test(self):
error_code=-8,
error_message="Internal addresses should not have a label")

self.log.info("Internal addresses should be detected as such")
key = get_generate_key()
addr = key_to_p2pkh(key.pubkey)
self.test_importdesc({"desc": descsum_create("pkh(" + key.pubkey + ")"),
"timestamp": "now",
"internal": True},
success=True)
info = w1.getaddressinfo(addr)
assert_equal(info["ismine"], True)
assert_equal(info["ischange"], True)

# # Test importing of a P2SH-P2WPKH descriptor
key = get_generate_key()
self.log.info("Should not import a p2sh-p2wpkh descriptor without checksum")
Expand Down

0 comments on commit 663fd92

Please sign in to comment.