diff options
Diffstat (limited to 'src/wallet')
-rw-r--r-- | src/wallet/rpcdump.cpp | 7 | ||||
-rw-r--r-- | src/wallet/rpcwallet.cpp | 134 | ||||
-rw-r--r-- | src/wallet/wallet.cpp | 52 | ||||
-rw-r--r-- | src/wallet/wallet.h | 19 | ||||
-rw-r--r-- | src/wallet/walletdb.cpp | 6 | ||||
-rw-r--r-- | src/wallet/wallettool.cpp | 2 |
6 files changed, 135 insertions, 85 deletions
diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index e4d0a3fa6d..ea54027c48 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -60,12 +60,13 @@ static bool GetWalletAddressesForKey(LegacyScriptPubKeyMan* spk_man, const CWall CKey key; spk_man->GetKey(keyid, key); for (const auto& dest : GetAllDestinationsForKey(key.GetPubKey())) { - if (pwallet->mapAddressBook.count(dest)) { + const auto* address_book_entry = pwallet->FindAddressBookEntry(dest); + if (address_book_entry) { if (!strAddr.empty()) { strAddr += ","; } strAddr += EncodeDestination(dest); - strLabel = EncodeDumpString(pwallet->mapAddressBook.at(dest).name); + strLabel = EncodeDumpString(address_book_entry->GetLabel()); fLabelFound = true; } } @@ -168,7 +169,7 @@ UniValue importprivkey(const JSONRPCRequest& request) // label all new addresses, and label existing addresses if a // label was passed. for (const auto& dest : GetAllDestinationsForKey(pubkey)) { - if (!request.params[1].isNull() || pwallet->mapAddressBook.count(dest) == 0) { + if (!request.params[1].isNull() || !pwallet->FindAddressBookEntry(dest)) { pwallet->SetAddressBook(dest, strLabel, "receive"); } } diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index d8b3b57b61..61ad2f1198 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -499,8 +499,9 @@ static UniValue listaddressgroupings(const JSONRPCRequest& request) addressInfo.push_back(EncodeDestination(address)); addressInfo.push_back(ValueFromAmount(balances[address])); { - if (pwallet->mapAddressBook.find(address) != pwallet->mapAddressBook.end()) { - addressInfo.push_back(pwallet->mapAddressBook.find(address)->second.name); + const auto* address_book_entry = pwallet->FindAddressBookEntry(address); + if (address_book_entry) { + addressInfo.push_back(address_book_entry->GetLabel()); } } jsonGrouping.push_back(addressInfo); @@ -1092,13 +1093,13 @@ static UniValue ListReceived(interfaces::Chain::Lock& locked_chain, const CWalle UniValue ret(UniValue::VARR); std::map<std::string, tallyitem> label_tally; - // Create mapAddressBook iterator + // Create m_address_book iterator // If we aren't filtering, go from begin() to end() - auto start = pwallet->mapAddressBook.begin(); - auto end = pwallet->mapAddressBook.end(); + auto start = pwallet->m_address_book.begin(); + auto end = pwallet->m_address_book.end(); // If we are filtering, find() the applicable entry if (has_filtered_address) { - start = pwallet->mapAddressBook.find(filtered_address); + start = pwallet->m_address_book.find(filtered_address); if (start != end) { end = std::next(start); } @@ -1106,8 +1107,9 @@ static UniValue ListReceived(interfaces::Chain::Lock& locked_chain, const CWalle for (auto item_it = start; item_it != end; ++item_it) { + if (item_it->second.IsChange()) continue; const CTxDestination& address = item_it->first; - const std::string& label = item_it->second.name; + const std::string& label = item_it->second.GetLabel(); auto it = mapTally.find(address); if (it == mapTally.end() && !fIncludeEmpty) continue; @@ -1307,8 +1309,9 @@ static void ListTransactions(interfaces::Chain::Lock& locked_chain, const CWalle MaybePushAddress(entry, s.destination); entry.pushKV("category", "send"); entry.pushKV("amount", ValueFromAmount(-s.amount)); - if (pwallet->mapAddressBook.count(s.destination)) { - entry.pushKV("label", pwallet->mapAddressBook.at(s.destination).name); + const auto* address_book_entry = pwallet->FindAddressBookEntry(s.destination); + if (address_book_entry) { + entry.pushKV("label", address_book_entry->GetLabel()); } entry.pushKV("vout", s.vout); entry.pushKV("fee", ValueFromAmount(-nFee)); @@ -1324,8 +1327,9 @@ static void ListTransactions(interfaces::Chain::Lock& locked_chain, const CWalle for (const COutputEntry& r : listReceived) { std::string label; - if (pwallet->mapAddressBook.count(r.destination)) { - label = pwallet->mapAddressBook.at(r.destination).name; + const auto* address_book_entry = pwallet->FindAddressBookEntry(r.destination); + if (address_book_entry) { + label = address_book_entry->GetLabel(); } if (filter_label && label != *filter_label) { continue; @@ -1349,7 +1353,7 @@ static void ListTransactions(interfaces::Chain::Lock& locked_chain, const CWalle entry.pushKV("category", "receive"); } entry.pushKV("amount", ValueFromAmount(r.amount)); - if (pwallet->mapAddressBook.count(r.destination)) { + if (address_book_entry) { entry.pushKV("label", label); } entry.pushKV("vout", r.vout); @@ -1912,44 +1916,52 @@ static UniValue walletpassphrase(const JSONRPCRequest& request) }, }.Check(request); - auto locked_chain = pwallet->chain().lock(); - LOCK(pwallet->cs_wallet); - - if (!pwallet->IsCrypted()) { - throw JSONRPCError(RPC_WALLET_WRONG_ENC_STATE, "Error: running with an unencrypted wallet, but walletpassphrase was called."); - } + int64_t nSleepTime; + { + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); - // Note that the walletpassphrase is stored in request.params[0] which is not mlock()ed - SecureString strWalletPass; - strWalletPass.reserve(100); - // TODO: get rid of this .c_str() by implementing SecureString::operator=(std::string) - // Alternately, find a way to make request.params[0] mlock()'d to begin with. - strWalletPass = request.params[0].get_str().c_str(); + if (!pwallet->IsCrypted()) { + throw JSONRPCError(RPC_WALLET_WRONG_ENC_STATE, "Error: running with an unencrypted wallet, but walletpassphrase was called."); + } - // Get the timeout - int64_t nSleepTime = request.params[1].get_int64(); - // Timeout cannot be negative, otherwise it will relock immediately - if (nSleepTime < 0) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Timeout cannot be negative."); - } - // Clamp timeout - constexpr int64_t MAX_SLEEP_TIME = 100000000; // larger values trigger a macos/libevent bug? - if (nSleepTime > MAX_SLEEP_TIME) { - nSleepTime = MAX_SLEEP_TIME; - } + // Note that the walletpassphrase is stored in request.params[0] which is not mlock()ed + SecureString strWalletPass; + strWalletPass.reserve(100); + // TODO: get rid of this .c_str() by implementing SecureString::operator=(std::string) + // Alternately, find a way to make request.params[0] mlock()'d to begin with. + strWalletPass = request.params[0].get_str().c_str(); + + // Get the timeout + nSleepTime = request.params[1].get_int64(); + // Timeout cannot be negative, otherwise it will relock immediately + if (nSleepTime < 0) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Timeout cannot be negative."); + } + // Clamp timeout + constexpr int64_t MAX_SLEEP_TIME = 100000000; // larger values trigger a macos/libevent bug? + if (nSleepTime > MAX_SLEEP_TIME) { + nSleepTime = MAX_SLEEP_TIME; + } - if (strWalletPass.empty()) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "passphrase can not be empty"); - } + if (strWalletPass.empty()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "passphrase can not be empty"); + } - if (!pwallet->Unlock(strWalletPass)) { - throw JSONRPCError(RPC_WALLET_PASSPHRASE_INCORRECT, "Error: The wallet passphrase entered was incorrect."); - } + if (!pwallet->Unlock(strWalletPass)) { + throw JSONRPCError(RPC_WALLET_PASSPHRASE_INCORRECT, "Error: The wallet passphrase entered was incorrect."); + } - pwallet->TopUpKeyPool(); + pwallet->TopUpKeyPool(); - pwallet->nRelockTime = GetTime() + nSleepTime; + pwallet->nRelockTime = GetTime() + nSleepTime; + } + // rpcRunLater must be called without cs_wallet held otherwise a deadlock + // can occur. The deadlock would happen when RPCRunLater removes the + // previous timer (and waits for the callback to finish if already running) + // and the callback locks cs_wallet. + AssertLockNotHeld(wallet->cs_wallet); // Keep a weak pointer to the wallet so that it is possible to unload the // wallet before the following callback is called. If a valid shared pointer // is acquired in the callback then the wallet is still loaded. @@ -2949,9 +2961,9 @@ static UniValue listunspent(const JSONRPCRequest& request) if (fValidAddress) { entry.pushKV("address", EncodeDestination(address)); - auto i = pwallet->mapAddressBook.find(address); - if (i != pwallet->mapAddressBook.end()) { - entry.pushKV("label", i->second.name); + const auto* address_book_entry = pwallet->FindAddressBookEntry(address); + if (address_book_entry) { + entry.pushKV("label", address_book_entry->GetLabel()); } std::unique_ptr<SigningProvider> provider = pwallet->GetSolvingProvider(scriptPubKey); @@ -3698,7 +3710,7 @@ static UniValue AddressBookDataToJSON(const CAddressBookData& data, const bool v { UniValue ret(UniValue::VOBJ); if (verbose) { - ret.pushKV("name", data.name); + ret.pushKV("name", data.GetLabel()); } ret.pushKV("purpose", data.purpose); return ret; @@ -3808,8 +3820,9 @@ UniValue getaddressinfo(const JSONRPCRequest& request) // DEPRECATED: Return label field if existing. Currently only one label can // be associated with an address, so the label should be equivalent to the // value of the name key/value pair in the labels array below. - if ((pwallet->chain().rpcEnableDeprecated("label")) && (pwallet->mapAddressBook.count(dest))) { - ret.pushKV("label", pwallet->mapAddressBook.at(dest).name); + const auto* address_book_entry = pwallet->FindAddressBookEntry(dest); + if (pwallet->chain().rpcEnableDeprecated("label") && address_book_entry) { + ret.pushKV("label", address_book_entry->GetLabel()); } ret.pushKV("ischange", pwallet->IsChange(scriptPubKey)); @@ -3832,14 +3845,13 @@ UniValue getaddressinfo(const JSONRPCRequest& request) // stable if we allow multiple labels to be associated with an address in // the future. UniValue labels(UniValue::VARR); - std::map<CTxDestination, CAddressBookData>::const_iterator mi = pwallet->mapAddressBook.find(dest); - if (mi != pwallet->mapAddressBook.end()) { + if (address_book_entry) { // DEPRECATED: The previous behavior of returning an array containing a // JSON object of `name` and `purpose` key/value pairs is deprecated. if (pwallet->chain().rpcEnableDeprecated("labelspurpose")) { - labels.push_back(AddressBookDataToJSON(mi->second, true)); + labels.push_back(AddressBookDataToJSON(*address_book_entry, true)); } else { - labels.push_back(mi->second.name); + labels.push_back(address_book_entry->GetLabel()); } } ret.pushKV("labels", std::move(labels)); @@ -3883,10 +3895,11 @@ static UniValue getaddressesbylabel(const JSONRPCRequest& request) // Find all addresses that have the given label UniValue ret(UniValue::VOBJ); std::set<std::string> addresses; - for (const std::pair<const CTxDestination, CAddressBookData>& item : pwallet->mapAddressBook) { - if (item.second.name == label) { + for (const std::pair<const CTxDestination, CAddressBookData>& item : pwallet->m_address_book) { + if (item.second.IsChange()) continue; + if (item.second.GetLabel() == label) { std::string address = EncodeDestination(item.first); - // CWallet::mapAddressBook is not expected to contain duplicate + // CWallet::m_address_book is not expected to contain duplicate // address strings, but build a separate set as a precaution just in // case it does. bool unique = addresses.emplace(address).second; @@ -3947,9 +3960,10 @@ static UniValue listlabels(const JSONRPCRequest& request) // Add to a set to sort by label name, then insert into Univalue array std::set<std::string> label_set; - for (const std::pair<const CTxDestination, CAddressBookData>& entry : pwallet->mapAddressBook) { + for (const std::pair<const CTxDestination, CAddressBookData>& entry : pwallet->m_address_book) { + if (entry.second.IsChange()) continue; if (purpose.empty() || entry.second.purpose == purpose) { - label_set.insert(entry.second.name); + label_set.insert(entry.second.GetLabel()); } } @@ -4242,6 +4256,8 @@ UniValue importprunedfunds(const JSONRPCRequest& request); UniValue removeprunedfunds(const JSONRPCRequest& request); UniValue importmulti(const JSONRPCRequest& request); +void RegisterWalletRPCCommands(interfaces::Chain& chain, std::vector<std::unique_ptr<interfaces::Handler>>& handlers) +{ // clang-format off static const CRPCCommand commands[] = { // category name actor (function) argNames @@ -4305,8 +4321,6 @@ static const CRPCCommand commands[] = }; // clang-format on -void RegisterWalletRPCCommands(interfaces::Chain& chain, std::vector<std::unique_ptr<interfaces::Handler>>& handlers) -{ for (unsigned int vcidx = 0; vcidx < ARRAYLEN(commands); vcidx++) handlers.emplace_back(chain.handleRpc(commands[vcidx])); } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index bc9f84a11d..45f5542cad 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1242,8 +1242,9 @@ bool CWallet::IsChange(const CScript& script) const return true; LOCK(cs_wallet); - if (!mapAddressBook.count(address)) + if (!FindAddressBookEntry(address)) { return true; + } } return false; } @@ -3196,11 +3197,11 @@ bool CWallet::SetAddressBookWithDB(WalletBatch& batch, const CTxDestination& add bool fUpdated = false; { LOCK(cs_wallet); - std::map<CTxDestination, CAddressBookData>::iterator mi = mapAddressBook.find(address); - fUpdated = mi != mapAddressBook.end(); - mapAddressBook[address].name = strName; + std::map<CTxDestination, CAddressBookData>::iterator mi = m_address_book.find(address); + fUpdated = (mi != m_address_book.end() && !mi->second.IsChange()); + m_address_book[address].SetLabel(strName); if (!strPurpose.empty()) /* update purpose only if requested */ - mapAddressBook[address].purpose = strPurpose; + m_address_book[address].purpose = strPurpose; } NotifyAddressBookChanged(this, address, strName, IsMine(address) != ISMINE_NO, strPurpose, (fUpdated ? CT_UPDATED : CT_NEW) ); @@ -3217,16 +3218,24 @@ bool CWallet::SetAddressBook(const CTxDestination& address, const std::string& s bool CWallet::DelAddressBook(const CTxDestination& address) { + // If we want to delete receiving addresses, we need to take care that DestData "used" (and possibly newer DestData) gets preserved (and the "deleted" address transformed into a change entry instead of actually being deleted) + // NOTE: This isn't a problem for sending addresses because they never have any DestData yet! + // When adding new DestData, it should be considered here whether to retain or delete it (or move it?). + if (IsMine(address)) { + WalletLogPrintf("%s called with IsMine address, NOT SUPPORTED. Please report this bug! %s\n", __func__, PACKAGE_BUGREPORT); + return false; + } + { LOCK(cs_wallet); // Delete destdata tuples associated with address std::string strAddress = EncodeDestination(address); - for (const std::pair<const std::string, std::string> &item : mapAddressBook[address].destdata) + for (const std::pair<const std::string, std::string> &item : m_address_book[address].destdata) { WalletBatch(*database).EraseDestData(strAddress, item.first); } - mapAddressBook.erase(address); + m_address_book.erase(address); } NotifyAddressBookChanged(this, address, "", IsMine(address) != ISMINE_NO, "", CT_DELETED); @@ -3462,10 +3471,11 @@ std::set<CTxDestination> CWallet::GetLabelAddresses(const std::string& label) co { LOCK(cs_wallet); std::set<CTxDestination> result; - for (const std::pair<const CTxDestination, CAddressBookData>& item : mapAddressBook) + for (const std::pair<const CTxDestination, CAddressBookData>& item : m_address_book) { + if (item.second.IsChange()) continue; const CTxDestination& address = item.first; - const std::string& strName = item.second.name; + const std::string& strName = item.second.GetLabel(); if (strName == label) result.insert(address); } @@ -3666,26 +3676,26 @@ bool CWallet::AddDestData(WalletBatch& batch, const CTxDestination &dest, const if (boost::get<CNoDestination>(&dest)) return false; - mapAddressBook[dest].destdata.insert(std::make_pair(key, value)); + m_address_book[dest].destdata.insert(std::make_pair(key, value)); return batch.WriteDestData(EncodeDestination(dest), key, value); } bool CWallet::EraseDestData(WalletBatch& batch, const CTxDestination &dest, const std::string &key) { - if (!mapAddressBook[dest].destdata.erase(key)) + if (!m_address_book[dest].destdata.erase(key)) return false; return batch.EraseDestData(EncodeDestination(dest), key); } void CWallet::LoadDestData(const CTxDestination &dest, const std::string &key, const std::string &value) { - mapAddressBook[dest].destdata.insert(std::make_pair(key, value)); + m_address_book[dest].destdata.insert(std::make_pair(key, value)); } bool CWallet::GetDestData(const CTxDestination &dest, const std::string &key, std::string *value) const { - std::map<CTxDestination, CAddressBookData>::const_iterator i = mapAddressBook.find(dest); - if(i != mapAddressBook.end()) + std::map<CTxDestination, CAddressBookData>::const_iterator i = m_address_book.find(dest); + if(i != m_address_book.end()) { CAddressBookData::StringMap::const_iterator j = i->second.destdata.find(key); if(j != i->second.destdata.end()) @@ -3701,7 +3711,7 @@ bool CWallet::GetDestData(const CTxDestination &dest, const std::string &key, st std::vector<std::string> CWallet::GetDestValues(const std::string& prefix) const { std::vector<std::string> values; - for (const auto& address : mapAddressBook) { + for (const auto& address : m_address_book) { for (const auto& data : address.second.destdata) { if (!data.first.compare(0, prefix.size(), prefix)) { values.emplace_back(data.second); @@ -4103,12 +4113,22 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain, { walletInstance->WalletLogPrintf("setKeyPool.size() = %u\n", walletInstance->GetKeyPoolSize()); walletInstance->WalletLogPrintf("mapWallet.size() = %u\n", walletInstance->mapWallet.size()); - walletInstance->WalletLogPrintf("mapAddressBook.size() = %u\n", walletInstance->mapAddressBook.size()); + walletInstance->WalletLogPrintf("m_address_book.size() = %u\n", walletInstance->m_address_book.size()); } return walletInstance; } +const CAddressBookData* CWallet::FindAddressBookEntry(const CTxDestination& dest, bool allow_change) const +{ + const auto& address_book_it = m_address_book.find(dest); + if (address_book_it == m_address_book.end()) return nullptr; + if ((!allow_change) && address_book_it->second.IsChange()) { + return nullptr; + } + return &address_book_it->second; +} + void CWallet::postInitProcess() { auto locked_chain = chain().lock(); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index fe59773488..6c54c72e76 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -181,14 +181,23 @@ public: /** Address book data */ class CAddressBookData { +private: + bool m_change{true}; + std::string m_label; public: - std::string name; std::string purpose; CAddressBookData() : purpose("unknown") {} typedef std::map<std::string, std::string> StringMap; StringMap destdata; + + bool IsChange() const { return m_change; } + const std::string& GetLabel() const { return m_label; } + void SetLabel(const std::string& label) { + m_change = false; + m_label = label; + } }; struct CRecipient @@ -775,7 +784,8 @@ public: int64_t nOrderPosNext GUARDED_BY(cs_wallet) = 0; uint64_t nAccountingEntryNumber = 0; - std::map<CTxDestination, CAddressBookData> mapAddressBook GUARDED_BY(cs_wallet); + 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); std::set<COutPoint> setLockedCoins GUARDED_BY(cs_wallet); @@ -842,7 +852,10 @@ public: bool LoadMinVersion(int nVersion) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) { AssertLockHeld(cs_wallet); nWalletVersion = nVersion; nWalletMaxVersion = std::max(nWalletMaxVersion, nVersion); return true; } - //! Adds a destination data tuple to the store, and saves it to disk + /** + * Adds a destination data tuple to the store, and saves it to disk + * When adding new fields, take care to consider how DelAddressBook should handle it! + */ bool AddDestData(WalletBatch& batch, const CTxDestination& dest, const std::string& key, const std::string& value) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); //! Erases a destination data tuple in the store and on disk bool EraseDestData(WalletBatch& batch, const CTxDestination& dest, const std::string& key) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index a1928f45c4..568b21ed00 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -206,11 +206,13 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, if (strType == DBKeys::NAME) { std::string strAddress; ssKey >> strAddress; - ssValue >> pwallet->mapAddressBook[DecodeDestination(strAddress)].name; + std::string label; + ssValue >> label; + pwallet->m_address_book[DecodeDestination(strAddress)].SetLabel(label); } else if (strType == DBKeys::PURPOSE) { std::string strAddress; ssKey >> strAddress; - ssValue >> pwallet->mapAddressBook[DecodeDestination(strAddress)].purpose; + ssValue >> pwallet->m_address_book[DecodeDestination(strAddress)].purpose; } else if (strType == DBKeys::TX) { uint256 hash; ssKey >> hash; diff --git a/src/wallet/wallettool.cpp b/src/wallet/wallettool.cpp index fbfdf9dd6b..8c918f4eb0 100644 --- a/src/wallet/wallettool.cpp +++ b/src/wallet/wallettool.cpp @@ -99,7 +99,7 @@ static void WalletShowInfo(CWallet* wallet_instance) tfm::format(std::cout, "HD (hd seed available): %s\n", wallet_instance->IsHDEnabled() ? "yes" : "no"); tfm::format(std::cout, "Keypool Size: %u\n", wallet_instance->GetKeyPoolSize()); tfm::format(std::cout, "Transactions: %zu\n", wallet_instance->mapWallet.size()); - tfm::format(std::cout, "Address Book: %zu\n", wallet_instance->mapAddressBook.size()); + tfm::format(std::cout, "Address Book: %zu\n", wallet_instance->m_address_book.size()); } bool ExecuteWalletToolFunc(const std::string& command, const std::string& name) |