Skip to content

Commit

Permalink
wallet: Move internal to be per key when importing
Browse files Browse the repository at this point in the history 
Instead of applying internal-ness to all keys being imported at the same
time, apply it on a per key basis. So each key that is imported will
carry with it whether it is for the change keypool.
  • Loading branch information
@achow101
achow101 committed Oct 11, 2023
1 parent 5043f8e commit baa1523
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 14 deletions.
2 changes: 1 addition & 1 deletion src/qt/test/wallettests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ std::shared_ptr<CWallet> SetupLegacyWatchOnlyWallet(interfaces::Node& node, Test
wallet->SetupLegacyScriptPubKeyMan();
// Add watched key
CPubKey pubKey = test.coinbaseKey.GetPubKey();
bool import_keys = wallet->ImportPubKeys({pubKey.GetID()}, {{pubKey.GetID(), pubKey}} , /*key_origins=*/{}, /*add_keypool=*/false, /*internal=*/false, /*timestamp=*/1);
bool import_keys = wallet->ImportPubKeys({{pubKey.GetID(), false}}, {{pubKey.GetID(), pubKey}} , /*key_origins=*/{}, /*add_keypool=*/false, /*timestamp=*/1);
assert(import_keys);
wallet->SetLastBlockProcessed(105, WITH_LOCK(node.context()->chainman->GetMutex(), return node.context()->chainman->ActiveChain().Tip()->GetBlockHash()));
}
Expand Down
16 changes: 9 additions & 7 deletions src/wallet/rpc/backup.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,7 @@ RPCHelpMan importpubkey()

pwallet->ImportScriptPubKeys(strLabel, script_pub_keys, /*have_solving_data=*/true, /*apply_label=*/true, /*timestamp=*/1);

pwallet->ImportPubKeys({pubKey.GetID()}, {{pubKey.GetID(), pubKey}} , /*key_origins=*/{}, /*add_keypool=*/false, /*internal=*/false, /*timestamp=*/1);
pwallet->ImportPubKeys({{pubKey.GetID(), false}}, {{pubKey.GetID(), pubKey}} , /*key_origins=*/{}, /*add_keypool=*/false, /*timestamp=*/1);
}
if (fRescan)
{
Expand Down Expand Up @@ -922,7 +922,7 @@ static std::string RecurseImportData(const CScript& script, ImportData& import_d
NONFATAL_UNREACHABLE();
}

static UniValue ProcessImportLegacy(ImportData& import_data, std::map<CKeyID, CPubKey>& pubkey_map, std::map<CKeyID, CKey>& privkey_map, std::set<CScript>& script_pub_keys, bool& have_solving_data, const UniValue& data, std::vector<CKeyID>& ordered_pubkeys)
static UniValue ProcessImportLegacy(ImportData& import_data, std::map<CKeyID, CPubKey>& pubkey_map, std::map<CKeyID, CKey>& privkey_map, std::set<CScript>& script_pub_keys, bool& have_solving_data, const UniValue& data, std::vector<std::pair<CKeyID, bool>>& ordered_pubkeys)
{
UniValue warnings(UniValue::VARR);

Expand Down Expand Up @@ -996,7 +996,7 @@ static UniValue ProcessImportLegacy(ImportData& import_data, std::map<CKeyID, CP
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey \"" + str + "\" is not a valid public key");
}
pubkey_map.emplace(pubkey.GetID(), pubkey);
ordered_pubkeys.push_back(pubkey.GetID());
ordered_pubkeys.push_back({pubkey.GetID(), internal});
}
for (size_t i = 0; i < keys.size(); ++i) {
const auto& str = keys[i].get_str();
Expand Down Expand Up @@ -1069,8 +1069,10 @@ static UniValue ProcessImportLegacy(ImportData& import_data, std::map<CKeyID, CP
return warnings;
}

static UniValue ProcessImportDescriptor(ImportData& import_data, std::map<CKeyID, CPubKey>& pubkey_map, std::map<CKeyID, CKey>& privkey_map, std::set<CScript>& script_pub_keys, bool& have_solving_data, const UniValue& data, std::vector<CKeyID>& ordered_pubkeys)
static UniValue ProcessImportDescriptor(ImportData& import_data, std::map<CKeyID, CPubKey>& pubkey_map, std::map<CKeyID, CKey>& privkey_map, std::set<CScript>& script_pub_keys, bool& have_solving_data, const UniValue& data, std::vector<std::pair<CKeyID, bool>>& ordered_pubkeys)
{
const bool internal = data.exists("internal") ? data["internal"].get_bool() : false;

UniValue warnings(UniValue::VARR);

const std::string& descriptor = data["desc"].get_str();
Expand Down Expand Up @@ -1107,7 +1109,7 @@ static UniValue ProcessImportDescriptor(ImportData& import_data, std::map<CKeyID
parsed_desc->Expand(i, keys, scripts_temp, out_keys);
std::copy(scripts_temp.begin(), scripts_temp.end(), std::inserter(script_pub_keys, script_pub_keys.end()));
for (const auto& key_pair : out_keys.pubkeys) {
ordered_pubkeys.push_back(key_pair.first);
ordered_pubkeys.push_back({key_pair.first, internal});
}

for (const auto& x : out_keys.scripts) {
Expand Down Expand Up @@ -1182,7 +1184,7 @@ static UniValue ProcessImport(CWallet& wallet, const UniValue& data, const int64
std::map<CKeyID, CPubKey> pubkey_map;
std::map<CKeyID, CKey> privkey_map;
std::set<CScript> script_pub_keys;
std::vector<CKeyID> ordered_pubkeys;
std::vector<std::pair<CKeyID, bool>> ordered_pubkeys;
bool have_solving_data;

if (data.exists("scriptPubKey") && data.exists("desc")) {
Expand Down Expand Up @@ -1215,7 +1217,7 @@ static UniValue ProcessImport(CWallet& wallet, const UniValue& data, const int64
if (!wallet.ImportPrivKeys(privkey_map, timestamp)) {
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding key to wallet");
}
if (!wallet.ImportPubKeys(ordered_pubkeys, pubkey_map, import_data.key_origins, add_keypool, internal, timestamp)) {
if (!wallet.ImportPubKeys(ordered_pubkeys, pubkey_map, import_data.key_origins, add_keypool, timestamp)) {
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet");
}
if (!wallet.ImportScriptPubKeys(label, script_pub_keys, have_solving_data, !internal, timestamp)) {
Expand Down
4 changes: 2 additions & 2 deletions src/wallet/scriptpubkeyman.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1611,13 +1611,13 @@ bool LegacyScriptPubKeyMan::ImportPrivKeys(const std::map<CKeyID, CKey>& privkey
return true;
}

bool LegacyScriptPubKeyMan::ImportPubKeys(const std::vector<CKeyID>& ordered_pubkeys, const std::map<CKeyID, CPubKey>& pubkey_map, const std::map<CKeyID, std::pair<CPubKey, KeyOriginInfo>>& key_origins, const bool add_keypool, const bool internal, const int64_t timestamp)
bool LegacyScriptPubKeyMan::ImportPubKeys(const std::vector<std::pair<CKeyID, bool>>& ordered_pubkeys, const std::map<CKeyID, CPubKey>& pubkey_map, const std::map<CKeyID, std::pair<CPubKey, KeyOriginInfo>>& key_origins, const bool add_keypool, const int64_t timestamp)
{
WalletBatch batch(m_storage.GetDatabase());
for (const auto& entry : key_origins) {
AddKeyOriginWithDB(batch, entry.second.first, entry.second.second);
}
for (const CKeyID& id : ordered_pubkeys) {
for (const auto& [id, internal] : ordered_pubkeys) {
auto entry = pubkey_map.find(id);
if (entry == pubkey_map.end()) {
continue;
Expand Down
2 changes: 1 addition & 1 deletion src/wallet/scriptpubkeyman.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ class LegacyScriptPubKeyMan : public ScriptPubKeyMan, public FillableSigningProv

bool ImportScripts(const std::set<CScript> scripts, int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(cs_KeyStore);
bool ImportPrivKeys(const std::map<CKeyID, CKey>& privkey_map, const int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(cs_KeyStore);
bool ImportPubKeys(const std::vector<CKeyID>& ordered_pubkeys, const std::map<CKeyID, CPubKey>& pubkey_map, const std::map<CKeyID, std::pair<CPubKey, KeyOriginInfo>>& key_origins, const bool add_keypool, const bool internal, const int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(cs_KeyStore);
bool ImportPubKeys(const std::vector<std::pair<CKeyID, bool>>& ordered_pubkeys, const std::map<CKeyID, CPubKey>& pubkey_map, const std::map<CKeyID, std::pair<CPubKey, KeyOriginInfo>>& key_origins, const bool add_keypool, const int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(cs_KeyStore);
bool ImportScriptPubKeys(const std::set<CScript>& script_pub_keys, const bool have_solving_data, const int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(cs_KeyStore);

/* Returns true if the wallet can generate new keys */
Expand Down
4 changes: 2 additions & 2 deletions src/wallet/wallet.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1730,14 +1730,14 @@ bool CWallet::ImportPrivKeys(const std::map<CKeyID, CKey>& privkey_map, const in
return spk_man->ImportPrivKeys(privkey_map, timestamp);
}

bool CWallet::ImportPubKeys(const std::vector<CKeyID>& ordered_pubkeys, const std::map<CKeyID, CPubKey>& pubkey_map, const std::map<CKeyID, std::pair<CPubKey, KeyOriginInfo>>& key_origins, const bool add_keypool, const bool internal, const int64_t timestamp)
bool CWallet::ImportPubKeys(const std::vector<std::pair<CKeyID, bool>>& ordered_pubkeys, const std::map<CKeyID, CPubKey>& pubkey_map, const std::map<CKeyID, std::pair<CPubKey, KeyOriginInfo>>& key_origins, const bool add_keypool, const int64_t timestamp)
{
auto spk_man = GetLegacyScriptPubKeyMan();
if (!spk_man) {
return false;
}
LOCK(spk_man->cs_KeyStore);
return spk_man->ImportPubKeys(ordered_pubkeys, pubkey_map, key_origins, add_keypool, internal, timestamp);
return spk_man->ImportPubKeys(ordered_pubkeys, pubkey_map, key_origins, add_keypool, timestamp);
}

bool CWallet::ImportScriptPubKeys(const std::string& label, const std::set<CScript>& script_pub_keys, const bool have_solving_data, const bool apply_label, const int64_t timestamp)
Expand Down
2 changes: 1 addition & 1 deletion src/wallet/wallet.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -675,7 +675,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati

bool ImportScripts(const std::set<CScript> scripts, int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
bool ImportPrivKeys(const std::map<CKeyID, CKey>& privkey_map, const int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
bool ImportPubKeys(const std::vector<CKeyID>& ordered_pubkeys, const std::map<CKeyID, CPubKey>& pubkey_map, const std::map<CKeyID, std::pair<CPubKey, KeyOriginInfo>>& key_origins, const bool add_keypool, const bool internal, const int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
bool ImportPubKeys(const std::vector<std::pair<CKeyID, bool>>& ordered_pubkeys, const std::map<CKeyID, CPubKey>& pubkey_map, const std::map<CKeyID, std::pair<CPubKey, KeyOriginInfo>>& key_origins, const bool add_keypool, const int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
bool ImportScriptPubKeys(const std::string& label, const std::set<CScript>& script_pub_keys, const bool have_solving_data, const bool apply_label, const int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);

/** Updates wallet birth time if 'new_birth_time' is below it */
Expand Down

0 comments on commit baa1523

Please sign in to comment.