From ab28e31c9563bd2cd1e4a088ffd2479517dc83f2 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Thu, 11 Jul 2019 20:21:05 -0400 Subject: Change ImportScriptPubKeys' internal to apply_label The internal bool was only to indicate whether the given label should be applied as things that are internal should not have a label. To make this clearer, we change internal to apply_label and invert its usage so things that have labels set this to true in order to have their labels applied. --- src/wallet/rpcdump.cpp | 2 +- src/wallet/wallet.cpp | 4 ++-- src/wallet/wallet.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 3112dca9f5..506b7c4bef 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -1283,7 +1283,7 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con if (!pwallet->ImportPubKeys(ordered_pubkeys, pubkey_map, import_data.key_origins, add_keypool, internal, timestamp)) { throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet"); } - if (!pwallet->ImportScriptPubKeys(label, script_pub_keys, have_solving_data, internal, timestamp)) { + if (!pwallet->ImportScriptPubKeys(label, script_pub_keys, have_solving_data, !internal, timestamp)) { throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet"); } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 16366a0c23..2b4819c981 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1717,7 +1717,7 @@ bool CWallet::ImportPubKeys(const std::vector& ordered_pubkeys, const st return true; } -bool CWallet::ImportScriptPubKeys(const std::string& label, const std::set& script_pub_keys, const bool have_solving_data, const bool internal, const int64_t timestamp) +bool CWallet::ImportScriptPubKeys(const std::string& label, const std::set& script_pub_keys, const bool have_solving_data, const bool apply_label, const int64_t timestamp) { WalletBatch batch(*database); for (const CScript& script : script_pub_keys) { @@ -1728,7 +1728,7 @@ bool CWallet::ImportScriptPubKeys(const std::string& label, const std::set scripts) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool ImportPrivKeys(const std::map& privkey_map, const int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool ImportPubKeys(const std::vector& ordered_pubkeys, const std::map& pubkey_map, const std::map>& key_origins, const bool add_keypool, const bool internal, const int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - bool ImportScriptPubKeys(const std::string& label, const std::set& script_pub_keys, const bool have_solving_data, const bool internal, const int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + bool ImportScriptPubKeys(const std::string& label, const std::set& script_pub_keys, const bool have_solving_data, const bool apply_label, const int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); CFeeRate m_pay_tx_fee{DEFAULT_PAY_TX_FEE}; unsigned int m_confirm_target{DEFAULT_TX_CONFIRM_TARGET}; -- cgit v1.2.3 From fae7a5befd0b8746d84a6fde575e5b4ea46cb3c4 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Tue, 16 Jul 2019 18:51:39 -0400 Subject: Log when an import is being skipped because we already have it Behavior Changes: * Those pubkeys being imported with add_keypool set and are already in the wallet will no longer be added to the keypool --- src/wallet/wallet.cpp | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 2b4819c981..45cd2d0e07 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1665,7 +1665,12 @@ bool CWallet::ImportScripts(const std::set scripts) { WalletBatch batch(*database); for (const auto& entry : scripts) { - if (!HaveCScript(CScriptID(entry)) && !AddCScriptWithDB(batch, entry)) { + CScriptID id(entry); + if (HaveCScript(id)) { + WalletLogPrintf("Already have script %s, skipping\n", HexStr(entry)); + continue; + } + if (!AddCScriptWithDB(batch, entry)) { return false; } } @@ -1680,9 +1685,14 @@ bool CWallet::ImportPrivKeys(const std::map& privkey_map, const in CPubKey pubkey = key.GetPubKey(); const CKeyID& id = entry.first; assert(key.VerifyPubKey(pubkey)); + // Skip if we already have the key + if (HaveKey(id)) { + WalletLogPrintf("Already have key with pubkey %s, skipping\n", HexStr(pubkey)); + continue; + } mapKeyMetadata[id].nCreateTime = timestamp; // If the private key is not present in the wallet, insert it. - if (!HaveKey(id) && !AddKeyPubKeyWithDB(batch, key, pubkey)) { + if (!AddKeyPubKeyWithDB(batch, key, pubkey)) { return false; } UpdateTimeFirstKey(timestamp); @@ -1703,7 +1713,12 @@ bool CWallet::ImportPubKeys(const std::vector& ordered_pubkeys, const st } const CPubKey& pubkey = entry->second; CPubKey temp; - if (!GetPubKey(id, temp) && !AddWatchOnlyWithDB(batch, GetScriptForRawPubKey(pubkey), timestamp)) { + if (GetPubKey(id, temp)) { + // Already have pubkey, skipping + WalletLogPrintf("Already have pubkey %s, skipping\n", HexStr(temp)); + continue; + } + if (!AddWatchOnlyWithDB(batch, GetScriptForRawPubKey(pubkey), timestamp)) { return false; } mapKeyMetadata[id].nCreateTime = timestamp; -- cgit v1.2.3 From c6a827424711333f6f66cf5f9d79e0e6884769de Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Thu, 27 Jun 2019 17:53:08 -0400 Subject: Have importprivkey use CWallet's ImportPrivKeys, ImportScripts, and ImportScriptPubKeys Behavior changes: * If we already have the key, it's wpkh script will still be added, although it should already be there --- src/wallet/rpcdump.cpp | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) (limited to 'src') diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 506b7c4bef..7f192e9364 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -187,19 +187,15 @@ UniValue importprivkey(const JSONRPCRequest& request) } } - // Don't throw error in case a key is already there - if (pwallet->HaveKey(vchAddress)) { - return NullUniValue; + // Use timestamp of 1 to scan the whole chain + if (!pwallet->ImportPrivKeys({{vchAddress, key}}, 1)) { + throw JSONRPCError(RPC_WALLET_ERROR, "Error adding key to wallet"); } - // whenever a key is imported, we need to scan the whole chain - pwallet->UpdateTimeFirstKey(1); - pwallet->mapKeyMetadata[vchAddress].nCreateTime = 1; - - if (!pwallet->AddKeyPubKey(key, pubkey)) { - throw JSONRPCError(RPC_WALLET_ERROR, "Error adding key to wallet"); + // Add the wpkh script for this key if possible + if (pubkey.IsCompressed()) { + pwallet->ImportScripts({GetScriptForDestination(WitnessV0KeyHash(vchAddress))}); } - pwallet->LearnAllRelatedScripts(pubkey); } } if (fRescan) { -- cgit v1.2.3 From a00d1e5ec5eb019f8bbeb060a2b09e341d360fe5 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Thu, 27 Jun 2019 18:09:49 -0400 Subject: Have importpubkey use CWallet's ImportScriptPubKeys and ImportPubKeys functions Behavior changes: * If any scripts for the pubkey were already in the wallet, their timestamps will be set to 1 and label updated --- src/wallet/rpcdump.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 7f192e9364..7b2a1a3377 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -538,11 +538,16 @@ UniValue importpubkey(const JSONRPCRequest& request) auto locked_chain = pwallet->chain().lock(); LOCK(pwallet->cs_wallet); + std::set script_pub_keys; for (const auto& dest : GetAllDestinationsForKey(pubKey)) { - ImportAddress(pwallet, dest, strLabel); + script_pub_keys.insert(GetScriptForDestination(dest)); } - ImportScript(pwallet, GetScriptForRawPubKey(pubKey), strLabel, false); - pwallet->LearnAllRelatedScripts(pubKey); + + pwallet->MarkDirty(); + + pwallet->ImportScriptPubKeys(strLabel, script_pub_keys, true /* have_solving_data */, true /* apply_label */, 1 /* timestamp */); + + pwallet->ImportPubKeys({pubKey.GetID()}, {{pubKey.GetID(), pubKey}} , {} /* key_origins */, false /* add_keypool */, false /* internal */, 1 /* timestamp */); } if (fRescan) { -- cgit v1.2.3 From 94bf156f391759420465b2ff8c44f5f150246c7f Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Thu, 27 Jun 2019 18:36:48 -0400 Subject: Have importaddress use ImportScripts and ImportScriptPubKeys Also removes the now unused ImportAddress and ImportScript from rpcdump.cpp Behavior changes: * No errors will be thrown when the script or key already exists in the wallet. * If the key or script is already in the wallet, their labels will be updated. --- src/wallet/rpcdump.cpp | 52 ++++++++++++++------------------------------------ 1 file changed, 14 insertions(+), 38 deletions(-) (limited to 'src') diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 7b2a1a3377..964676e6b1 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -235,42 +235,6 @@ UniValue abortrescan(const JSONRPCRequest& request) return true; } -static void ImportAddress(CWallet*, const CTxDestination& dest, const std::string& strLabel); -static void ImportScript(CWallet* const pwallet, const CScript& script, const std::string& strLabel, bool isRedeemScript) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) -{ - if (!isRedeemScript && ::IsMine(*pwallet, script) == ISMINE_SPENDABLE) { - throw JSONRPCError(RPC_WALLET_ERROR, "The wallet already contains the private key for this address or script"); - } - - pwallet->MarkDirty(); - - if (!pwallet->HaveWatchOnly(script) && !pwallet->AddWatchOnly(script, 0 /* nCreateTime */)) { - throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet"); - } - - if (isRedeemScript) { - const CScriptID id(script); - if (!pwallet->HaveCScript(id) && !pwallet->AddCScript(script)) { - throw JSONRPCError(RPC_WALLET_ERROR, "Error adding p2sh redeemScript to wallet"); - } - ImportAddress(pwallet, ScriptHash(id), strLabel); - } else { - CTxDestination destination; - if (ExtractDestination(script, destination)) { - pwallet->SetAddressBook(destination, strLabel, "receive"); - } - } -} - -static void ImportAddress(CWallet* const pwallet, const CTxDestination& dest, const std::string& strLabel) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) -{ - CScript script = GetScriptForDestination(dest); - ImportScript(pwallet, script, strLabel, false); - // add to address book or update label - if (IsValidDestination(dest)) - pwallet->SetAddressBook(dest, strLabel, "receive"); -} - UniValue importaddress(const JSONRPCRequest& request) { std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); @@ -343,10 +307,22 @@ UniValue importaddress(const JSONRPCRequest& request) if (fP2SH) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Cannot use the p2sh flag with an address - use a script instead"); } - ImportAddress(pwallet, dest, strLabel); + + pwallet->MarkDirty(); + + pwallet->ImportScriptPubKeys(strLabel, {GetScriptForDestination(dest)}, false /* have_solving_data */, true /* apply_label */, 1 /* timestamp */); } else if (IsHex(request.params[0].get_str())) { std::vector data(ParseHex(request.params[0].get_str())); - ImportScript(pwallet, CScript(data.begin(), data.end()), strLabel, fP2SH); + CScript redeem_script(data.begin(), data.end()); + + std::set scripts = {redeem_script}; + pwallet->ImportScripts(scripts); + + if (fP2SH) { + scripts.insert(GetScriptForDestination(ScriptHash(CScriptID(redeem_script)))); + } + + pwallet->ImportScriptPubKeys(strLabel, scripts, false /* have_solving_data */, true /* apply_label */, 1 /* timestamp */); } else { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid Bitcoin address or script"); } -- cgit v1.2.3 From 78941da5baf6244c7c54e86cf8ce3e09ce60c239 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Thu, 27 Jun 2019 21:17:42 -0400 Subject: Optionally allow ImportScripts to set script creation timestamp Behavior changes: * scripts imported in importmulti that are not explicilty scriptPubKeys will have timestamps set for them --- src/wallet/rpcdump.cpp | 6 +++--- src/wallet/wallet.cpp | 10 +++++++++- src/wallet/wallet.h | 2 +- 3 files changed, 13 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 964676e6b1..480832c627 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -194,7 +194,7 @@ UniValue importprivkey(const JSONRPCRequest& request) // Add the wpkh script for this key if possible if (pubkey.IsCompressed()) { - pwallet->ImportScripts({GetScriptForDestination(WitnessV0KeyHash(vchAddress))}); + pwallet->ImportScripts({GetScriptForDestination(WitnessV0KeyHash(vchAddress))}, 0 /* timestamp */); } } } @@ -316,7 +316,7 @@ UniValue importaddress(const JSONRPCRequest& request) CScript redeem_script(data.begin(), data.end()); std::set scripts = {redeem_script}; - pwallet->ImportScripts(scripts); + pwallet->ImportScripts(scripts, 0 /* timestamp */); if (fP2SH) { scripts.insert(GetScriptForDestination(ScriptHash(CScriptID(redeem_script)))); @@ -1251,7 +1251,7 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con // All good, time to import pwallet->MarkDirty(); - if (!pwallet->ImportScripts(import_data.import_scripts)) { + if (!pwallet->ImportScripts(import_data.import_scripts, timestamp)) { throw JSONRPCError(RPC_WALLET_ERROR, "Error adding script to wallet"); } if (!pwallet->ImportPrivKeys(privkey_map, timestamp)) { diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 45cd2d0e07..6f080f4b8f 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1661,7 +1661,7 @@ bool CWallet::DummySignTx(CMutableTransaction &txNew, const std::vector return true; } -bool CWallet::ImportScripts(const std::set scripts) +bool CWallet::ImportScripts(const std::set scripts, int64_t timestamp) { WalletBatch batch(*database); for (const auto& entry : scripts) { @@ -1673,7 +1673,15 @@ bool CWallet::ImportScripts(const std::set scripts) if (!AddCScriptWithDB(batch, entry)) { return false; } + + if (timestamp > 0) { + m_script_metadata[CScriptID(entry)].nCreateTime = timestamp; + } } + if (timestamp > 0) { + UpdateTimeFirstKey(timestamp); + } + return true; } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index b2810246dc..1b6a2c99f4 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1092,7 +1092,7 @@ public: bool DummySignTx(CMutableTransaction &txNew, const std::vector &txouts, bool use_max_sig = false) const; bool DummySignInput(CTxIn &tx_in, const CTxOut &txout, bool use_max_sig = false) const; - bool ImportScripts(const std::set scripts) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + bool ImportScripts(const std::set scripts, int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool ImportPrivKeys(const std::map& privkey_map, const int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool ImportPubKeys(const std::vector& ordered_pubkeys, const std::map& pubkey_map, const std::map>& key_origins, const bool add_keypool, const bool internal, const int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool ImportScriptPubKeys(const std::string& label, const std::set& script_pub_keys, const bool have_solving_data, const bool apply_label, const int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); -- cgit v1.2.3 From 40ad2f6a58228c72c655e3061a19a63640419378 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Thu, 27 Jun 2019 20:42:14 -0400 Subject: Have importwallet use ImportPrivKeys and ImportScripts Behavior changes: * An "Importing ..." line is logged for every key, even ones that are skipped --- src/wallet/rpcdump.cpp | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) (limited to 'src') diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 480832c627..932108744b 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -656,18 +656,18 @@ UniValue importwallet(const JSONRPCRequest& request) CPubKey pubkey = key.GetPubKey(); assert(key.VerifyPubKey(pubkey)); CKeyID keyid = pubkey.GetID(); - if (pwallet->HaveKey(keyid)) { - pwallet->WalletLogPrintf("Skipping import of %s (key already present)\n", EncodeDestination(PKHash(keyid))); - continue; - } + pwallet->WalletLogPrintf("Importing %s...\n", EncodeDestination(PKHash(keyid))); - if (!pwallet->AddKeyPubKey(key, pubkey)) { + + if (!pwallet->ImportPrivKeys({{keyid, key}}, time)) { + pwallet->WalletLogPrintf("Error importing key for %s\n", EncodeDestination(PKHash(keyid))); fGood = false; continue; } - pwallet->mapKeyMetadata[keyid].nCreateTime = time; + if (has_label) pwallet->SetAddressBook(PKHash(keyid), label, "receive"); + nTimeBegin = std::min(nTimeBegin, time); progress++; } @@ -675,24 +675,19 @@ UniValue importwallet(const JSONRPCRequest& request) pwallet->chain().showProgress("", std::max(50, std::min(75, (int)((progress / total) * 100) + 50)), false); const CScript& script = script_pair.first; int64_t time = script_pair.second; - CScriptID id(script); - if (pwallet->HaveCScript(id)) { - pwallet->WalletLogPrintf("Skipping import of %s (script already present)\n", HexStr(script)); - continue; - } - if(!pwallet->AddCScript(script)) { + + if (!pwallet->ImportScripts({script}, time)) { pwallet->WalletLogPrintf("Error importing script %s\n", HexStr(script)); fGood = false; continue; } if (time > 0) { - pwallet->m_script_metadata[id].nCreateTime = time; nTimeBegin = std::min(nTimeBegin, time); } + progress++; } pwallet->chain().showProgress("", 100, false); // hide progress dialog in GUI - pwallet->UpdateTimeFirstKey(nTimeBegin); } pwallet->chain().showProgress("", 100, false); // hide progress dialog in GUI RescanWallet(*pwallet, reserver, nTimeBegin, false /* update */); -- cgit v1.2.3