From 6eedbff8ee6b905f6d4237ac7eaaee389410d19d Mon Sep 17 00:00:00 2001 From: Mihai Ciumeica Date: Fri, 3 May 2019 17:18:53 +0200 Subject: [PATCH] Add timestamps for address book entries This commit extends the `CAddressBookData` structure with a new field, `timestamp`, representing the creation or modification time of the address. It also adds a new argument to the `filteraddresses` RPC call, which allows sorting by the timestamp field, either in ascending or descending order. Required for implementing dtr-org/unit-e-desktop#57. Signed-off-by: Mihai Ciumeica --- src/rpc/parameter_conversion.cpp | 4 +- src/wallet/rpcaddressbook.cpp | 67 +++++++++++++++++++++--------- src/wallet/rpcwallet.cpp | 4 ++ src/wallet/wallet.cpp | 6 +++ src/wallet/wallet.h | 3 +- src/wallet/walletdb.cpp | 16 +++++++ src/wallet/walletdb.h | 3 ++ test/functional/rpc_addressbook.py | 8 ++-- 8 files changed, 84 insertions(+), 27 deletions(-) diff --git a/src/rpc/parameter_conversion.cpp b/src/rpc/parameter_conversion.cpp index 2f70f840f8..28b0b4b688 100644 --- a/src/rpc/parameter_conversion.cpp +++ b/src/rpc/parameter_conversion.cpp @@ -173,8 +173,8 @@ static const CRPCConvertParam vRPCConvertParams[] = { "filtertransactions", 0, "options" }, { "filteraddresses", 0, "offset" }, { "filteraddresses", 1, "count" }, - { "filteraddresses", 2, "sort_code" }, - { "filteraddresses", 4, "match_owned" }, + { "filteraddresses", 3, "sort_code" }, + { "filteraddresses", 5, "match_owned" }, { "sendtypeto", 2, "outputs" }, { "sendtypeto", 5, "test_fee" }, diff --git a/src/wallet/rpcaddressbook.cpp b/src/wallet/rpcaddressbook.cpp index 89e185bae1..55df6ddac5 100644 --- a/src/wallet/rpcaddressbook.cpp +++ b/src/wallet/rpcaddressbook.cpp @@ -14,6 +14,9 @@ using AddressBookIter = std::map::iterator; enum class MatchOwned { ALL = 0, ONLY_OWNED = 1, ONLY_NOT_OWNED = 2 }; +// Not a strongly-typed enum since it's used in the sorting comparator +enum SortOrder { SORT_NONE = 0, SORT_ASCENDING = 1, SORT_DESCENDING = -1 }; + static bool CompareCharsI(unsigned char a, unsigned char b) { return std::tolower(a) == std::tolower(b); } @@ -70,16 +73,20 @@ UniValue filteraddresses(const JSONRPCRequest &request) { if (request.fHelp || request.params.size() > 6) { throw std::runtime_error( - "filteraddresses ( offset count sort_code \"search\" match_owned )\n" + "filteraddresses ( offset count \"sort_key\" sort_code \"search\" match_owned )\n" "\nList addresses.\n" "\nArguments:\n" "1. \"offset\": (numeric, optional) number of addresses to skip\n" "2. \"count\": (numeric, optional) number of addresses to be " "displayed\n" - "3. \"sort_code\": (numeric, optional) 0 sort by label ascending,\n" - " 1 sort by label descending, default 0\n" - "4. \"search\": (string, optional) a query to search labels\n" - "5. \"match_owned\": (numeric, optional) 0 off, 1 owned, 2 non-owned,\n" + "3. \"sort_key\": (string, optional) field to sort by, can be one " + "of:\n" + " \"label\"\n" + " \"timestamp\"\n" + "4. \"sort_code\": (numeric, optional) 0 sort ascending,\n" + " 1 sort descending, default 0\n" + "5. \"search\": (string, optional) a query to search labels\n" + "6. \"match_owned\": (numeric, optional) 0 off, 1 owned, 2 non-owned,\n" " default 0\n"); } @@ -104,23 +111,44 @@ UniValue filteraddresses(const JSONRPCRequest &request) { throw JSONRPCError(RPC_INVALID_PARAMETER, "count must be 1 or greater."); } - bool sortAsc = true; - if (request.params.size() > 2) { - int sortCode = request.params[2].get_int(); + SortOrder sort_order = SORT_NONE; + std::string sort_key; + std::function comparator; + + if (request.params.size() > 2 && !request.params[2].isNull()) { + sort_key = request.params[2].get_str(); + sort_order = SORT_ASCENDING; + if (sort_key == "label") { + comparator = [&sort_order](const AddressBookIter &a, const AddressBookIter &b) -> bool { + return a->second.name.compare(b->second.name) * sort_order < 0; + }; + } else if (sort_key == "timestamp") { + comparator = [&sort_order](const AddressBookIter &a, const AddressBookIter &b) -> bool { + return (a->second.timestamp - b->second.timestamp) * sort_order < 0; + }; + } else { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Unknown sort_key."); + } + } + + if (request.params.size() > 3) { + int sortCode = request.params[3].get_int(); if (sortCode != 0 && sortCode != 1) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Unknown sort_code."); } - sortAsc = sortCode == 0; + if (sortCode == 1 && !sort_key.empty()) { + sort_order = SORT_DESCENDING; + } } std::string search; - if (request.params.size() > 3) { - search = request.params[3].get_str(); + if (request.params.size() > 4) { + search = request.params[4].get_str(); } MatchOwned matchOwned = MatchOwned::ALL; - if (request.params.size() > 4) { - int i = request.params[4].get_int(); + if (request.params.size() > 5) { + int i = request.params[5].get_int(); if (i < 0 || i > 2) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Unknown match_owned."); } @@ -158,14 +186,9 @@ UniValue filteraddresses(const JSONRPCRequest &request) { vitMapAddressBook.push_back(it); } - auto comparator = sortAsc - ? [](const AddressBookIter &a, const AddressBookIter &b) -> bool { - return a->second.name.compare(b->second.name) < 0; + if (sort_order != SORT_NONE) { + std::sort(vitMapAddressBook.begin(), vitMapAddressBook.end(), comparator); } - : [](const AddressBookIter &a, const AddressBookIter &b) -> bool { - return b->second.name.compare(a->second.name) < 0; - }; - std::sort(vitMapAddressBook.begin(), vitMapAddressBook.end(), comparator); int numEntries = 0; for (auto vit = vitMapAddressBook.begin() + offset; @@ -176,6 +199,7 @@ UniValue filteraddresses(const JSONRPCRequest &request) { entry.pushKV("address", EncodeDestination(item->first)); entry.pushKV("label", item->second.name); entry.pushKV("owned", UniValue(addressIsMine[item->first])); + entry.pushKV("timestamp", item->second.timestamp); result.push_back(entry); ++numEntries; @@ -316,6 +340,9 @@ static UniValue AddressInfo(CWallet *pwallet, const std::string &address, result.pushKV("label", addressBookIt->second.name); result.pushKV("purpose", addressBookIt->second.purpose); + if (addressBookIt->second.timestamp) { + result.pushKV("timestamp", addressBookIt->second.timestamp); + } result.pushKV("owned", UniValue(!!IsMine(*pwallet, addressBookIt->first))); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 20f115c606..3630d85fff 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -4245,6 +4245,9 @@ static UniValue AddressBookDataToJSON(const CAddressBookData& data, const bool v ret.pushKV("name", data.name); } ret.pushKV("purpose", data.purpose); + if (data.timestamp) { + ret.pushKV("timestamp", data.timestamp); + } return ret; } @@ -4296,6 +4299,7 @@ UniValue getaddressinfo(const JSONRPCRequest& request) " { (json object of label data)\n" " \"name\": \"labelname\" (string) The label\n" " \"purpose\": \"string\" (string) Purpose of address (\"send\" for sending address, \"receive\" for receiving address)\n" + " \"timestamp\": timestamp (number, optional) The creation/modification time of the address if available, in seconds since epoch (Jan 1 1970 GMT)\n" " },...\n" " ]\n" "}\n" diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 44c9d9de68..968eeeea50 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3522,11 +3522,13 @@ DBErrors CWallet::ZapWalletTx(std::vector& vWtx) bool CWallet::SetAddressBook(const CTxDestination& address, const std::string& strName, const std::string& strPurpose) { bool fUpdated = false; + std::time_t timestamp = GetTime(); { LOCK(cs_wallet); // mapAddressBook std::map::iterator mi = mapAddressBook.find(address); fUpdated = mi != mapAddressBook.end(); mapAddressBook[address].name = strName; + mapAddressBook[address].timestamp = timestamp; if (!strPurpose.empty()) { /* update purpose only if requested */ mapAddressBook[address].purpose = strPurpose; } @@ -3536,6 +3538,9 @@ bool CWallet::SetAddressBook(const CTxDestination& address, const std::string& s if (!strPurpose.empty() && !WalletBatch(*database).WritePurpose(EncodeDestination(address), strPurpose)) { return false; } + if (!WalletBatch(*database).WriteTimestamp(EncodeDestination(address), timestamp)) { + return false; + } return WalletBatch(*database).WriteName(EncodeDestination(address), strName); } @@ -3556,6 +3561,7 @@ bool CWallet::DelAddressBook(const CTxDestination& address) NotifyAddressBookChanged(this, address, "", ::IsMine(*this, address) != ISMINE_NO, "", CT_DELETED); WalletBatch(*database).ErasePurpose(EncodeDestination(address)); + WalletBatch(*database).EraseTimestamp(EncodeDestination(address)); return WalletBatch(*database).EraseName(EncodeDestination(address)); } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index df796692da..1cadb8e910 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -179,8 +179,9 @@ class CAddressBookData public: std::string name; std::string purpose; + int64_t timestamp; //!< The Unix time when the entry was last updated - CAddressBookData() : purpose("unknown") {} + CAddressBookData() : purpose("unknown"), timestamp(0) {} typedef std::map StringMap; StringMap destdata; diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 5db46a97c9..ecfd187885 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -47,6 +47,16 @@ bool WalletBatch::ErasePurpose(const std::string& strAddress) return EraseIC(std::make_pair(std::string("purpose"), strAddress)); } +bool WalletBatch::WriteTimestamp(const std::string& address, std::time_t timestamp) +{ + return WriteIC(std::make_pair(std::string("timestamp"), address), timestamp); +} + +bool WalletBatch::EraseTimestamp(const std::string& address) +{ + return EraseIC(std::make_pair(std::string("timestamp"), address)); +} + bool WalletBatch::WriteTx(const CWalletTx& wtx) { return WriteIC(std::make_pair(std::string("tx"), wtx.GetHash()), wtx); @@ -273,6 +283,12 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, ssKey >> strAddress; ssValue >> pwallet->mapAddressBook[DecodeDestination(strAddress)].purpose; } + else if (strType == "timestamp") + { + std::string strAddress; + ssKey >> strAddress; + ssValue >> pwallet->mapAddressBook[DecodeDestination(strAddress)].timestamp; + } else if (strType == "tx") { uint256 hash; diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index 7d4f10afe8..54b1e09150 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -199,6 +199,9 @@ class WalletBatch bool WritePurpose(const std::string& strAddress, const std::string& purpose); bool ErasePurpose(const std::string& strAddress); + bool WriteTimestamp(const std::string& address, std::time_t timestamp); + bool EraseTimestamp(const std::string& address); + bool WriteTx(const CWalletTx& wtx); bool EraseTx(uint256 hash); diff --git a/test/functional/rpc_addressbook.py b/test/functional/rpc_addressbook.py index a63caa5616..be3ec90736 100755 --- a/test/functional/rpc_addressbook.py +++ b/test/functional/rpc_addressbook.py @@ -19,7 +19,7 @@ def run_test (self): node = self.nodes[0] # filteraddresses [offset] [count] [sort_code] [match_str] [match_owned] - resp = node.filteraddresses(0, 100, 0, '', 0) + resp = node.filteraddresses(0, 100, None, 0, '', 0) assert_equal(len(resp), 0) resp = node.addressbookinfo() @@ -42,7 +42,7 @@ def run_test (self): assert_equal(resp['label'], 'label1') assert_equal(resp['purpose'], 'purpose1') - resp = node.filteraddresses(0, 100, 0, '', 2) # list not owned + resp = node.filteraddresses(0, 100, None, 0, '', 2) # list not owned assert_equal(len(resp), 1) assert_equal(resp[0]['address'], not_owned_address) assert_equal(resp[0]['label'], 'label1') @@ -59,7 +59,7 @@ def run_test (self): assert_equal(resp['purpose'], 'purpose2') assert_equal(resp['owned'], False) - resp = node.filteraddresses(0, 100, 0, '', 2) + resp = node.filteraddresses(0, 100, None, 0, '', 2) assert_equal(len(resp), 1) assert_equal(resp[0]['address'], not_owned_address) assert_equal(resp[0]['label'], 'label2') @@ -70,7 +70,7 @@ def run_test (self): -8, 'is recorded in the address book', node.manageaddressbook, 'add', owned_address, 'owned1', 'receive') - resp = node.filteraddresses(0, 100, 0, '', 1) # list only owned + resp = node.filteraddresses(0, 100, None, 0, '', 1) # list only owned assert_equal(len(resp), 1) assert_equal(resp[0]['address'], owned_address) assert_equal(resp[0]['owned'], True)