diff options
-rw-r--r-- | src/interfaces/wallet.cpp | 4 | ||||
-rw-r--r-- | src/wallet/rpcdump.cpp | 2 | ||||
-rw-r--r-- | src/wallet/rpcwallet.cpp | 20 | ||||
-rw-r--r-- | src/wallet/wallet.cpp | 7 | ||||
-rw-r--r-- | src/wallet/wallet.h | 4 | ||||
-rwxr-xr-x | test/functional/wallet_avoidreuse.py | 25 |
6 files changed, 45 insertions, 17 deletions
diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp index d55b92a5ff..abce09ca4a 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -156,7 +156,7 @@ public: return false; } if (name) { - *name = it->second.name; + *name = it->second.GetLabel(); } if (is_mine) { *is_mine = m_wallet->IsMine(dest); @@ -172,7 +172,7 @@ public: std::vector<WalletAddress> result; for (const auto& item : m_wallet->m_address_book) { if (item.second.IsChange()) continue; - result.emplace_back(item.first, m_wallet->IsMine(item.first), item.second.name, item.second.purpose); + result.emplace_back(item.first, m_wallet->IsMine(item.first), item.second.GetLabel(), item.second.purpose); } return result; } diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index e1d8f51c4a..ea54027c48 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -66,7 +66,7 @@ static bool GetWalletAddressesForKey(LegacyScriptPubKeyMan* spk_man, const CWall strAddr += ","; } strAddr += EncodeDestination(dest); - strLabel = EncodeDumpString(address_book_entry->name); + strLabel = EncodeDumpString(address_book_entry->GetLabel()); fLabelFound = true; } } diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 336a6cff72..769a343365 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -501,7 +501,7 @@ static UniValue listaddressgroupings(const JSONRPCRequest& request) { const auto* address_book_entry = pwallet->FindAddressBookEntry(address); if (address_book_entry) { - addressInfo.push_back(address_book_entry->name); + addressInfo.push_back(address_book_entry->GetLabel()); } } jsonGrouping.push_back(addressInfo); @@ -1109,7 +1109,7 @@ static UniValue ListReceived(interfaces::Chain::Lock& locked_chain, const CWalle { 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; @@ -1311,7 +1311,7 @@ static void ListTransactions(interfaces::Chain::Lock& locked_chain, const CWalle entry.pushKV("amount", ValueFromAmount(-s.amount)); const auto* address_book_entry = pwallet->FindAddressBookEntry(s.destination); if (address_book_entry) { - entry.pushKV("label", address_book_entry->name); + entry.pushKV("label", address_book_entry->GetLabel()); } entry.pushKV("vout", s.vout); entry.pushKV("fee", ValueFromAmount(-nFee)); @@ -1329,7 +1329,7 @@ static void ListTransactions(interfaces::Chain::Lock& locked_chain, const CWalle std::string label; const auto* address_book_entry = pwallet->FindAddressBookEntry(r.destination); if (address_book_entry) { - label = address_book_entry->name; + label = address_book_entry->GetLabel(); } if (filter_label && label != *filter_label) { continue; @@ -2963,7 +2963,7 @@ static UniValue listunspent(const JSONRPCRequest& request) const auto* address_book_entry = pwallet->FindAddressBookEntry(address); if (address_book_entry) { - entry.pushKV("label", address_book_entry->name); + entry.pushKV("label", address_book_entry->GetLabel()); } std::unique_ptr<SigningProvider> provider = pwallet->GetSolvingProvider(scriptPubKey); @@ -3710,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; @@ -3822,7 +3822,7 @@ UniValue getaddressinfo(const JSONRPCRequest& request) // value of the name key/value pair in the labels array below. const auto* address_book_entry = pwallet->FindAddressBookEntry(dest); if (pwallet->chain().rpcEnableDeprecated("label") && address_book_entry) { - ret.pushKV("label", address_book_entry->name); + ret.pushKV("label", address_book_entry->GetLabel()); } ret.pushKV("ischange", pwallet->IsChange(scriptPubKey)); @@ -3851,7 +3851,7 @@ UniValue getaddressinfo(const JSONRPCRequest& request) if (pwallet->chain().rpcEnableDeprecated("labelspurpose")) { labels.push_back(AddressBookDataToJSON(*address_book_entry, true)); } else { - labels.push_back(address_book_entry->name); + labels.push_back(address_book_entry->GetLabel()); } } ret.pushKV("labels", std::move(labels)); @@ -3897,7 +3897,7 @@ static UniValue getaddressesbylabel(const JSONRPCRequest& request) std::set<std::string> addresses; for (const std::pair<const CTxDestination, CAddressBookData>& item : pwallet->m_address_book) { if (item.second.IsChange()) continue; - if (item.second.name == label) { + if (item.second.GetLabel() == label) { std::string address = EncodeDestination(item.first); // CWallet::m_address_book is not expected to contain duplicate // address strings, but build a separate set as a precaution just in @@ -3963,7 +3963,7 @@ static UniValue listlabels(const JSONRPCRequest& request) 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()); } } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 9ee3bbd038..45f5542cad 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3221,7 +3221,10 @@ 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?). - assert(!IsMine(address)); + 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); @@ -3472,7 +3475,7 @@ std::set<CTxDestination> CWallet::GetLabelAddresses(const std::string& label) co { 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); } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 7e770a40f2..6c54c72e76 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -185,15 +185,15 @@ private: bool m_change{true}; std::string m_label; public: - const std::string& name; std::string purpose; - CAddressBookData() : name(m_label), purpose("unknown") {} + 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; diff --git a/test/functional/wallet_avoidreuse.py b/test/functional/wallet_avoidreuse.py index 8e2dc03ac2..2ce8d459c6 100755 --- a/test/functional/wallet_avoidreuse.py +++ b/test/functional/wallet_avoidreuse.py @@ -83,6 +83,7 @@ class AvoidReuseTest(BitcoinTestFramework): self.nodes[0].generate(110) self.sync_all() + self.test_change_remains_change(self.nodes[1]) reset_balance(self.nodes[1], self.nodes[0].getnewaddress()) self.test_fund_send_fund_senddirty() reset_balance(self.nodes[1], self.nodes[0].getnewaddress()) @@ -137,6 +138,30 @@ class AvoidReuseTest(BitcoinTestFramework): # Unload temp wallet self.nodes[1].unloadwallet(tempwallet) + def test_change_remains_change(self, node): + self.log.info("Test that change doesn't turn into non-change when spent") + + reset_balance(node, node.getnewaddress()) + addr = node.getnewaddress() + txid = node.sendtoaddress(addr, 1) + out = node.listunspent(minconf=0, query_options={'minimumAmount': 2}) + assert_equal(len(out), 1) + assert_equal(out[0]['txid'], txid) + changeaddr = out[0]['address'] + + # Make sure it's starting out as change as expected + assert node.getaddressinfo(changeaddr)['ischange'] + for logical_tx in node.listtransactions(): + assert logical_tx.get('address') != changeaddr + + # Spend it + reset_balance(node, node.getnewaddress()) + + # It should still be change + assert node.getaddressinfo(changeaddr)['ischange'] + for logical_tx in node.listtransactions(): + assert logical_tx.get('address') != changeaddr + def test_fund_send_fund_senddirty(self): ''' Test the same as test_fund_send_fund_send, except send the 10 BTC with |