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 <[email protected]>

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" },

src/wallet/rpcaddressbook.cpp

@@ -14,6 +14,9 @@ using AddressBookIter = std::map<CTxDestination, CAddressBookData>::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<bool(const AddressBookIter&, const AddressBookIter&)> 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)));
 

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"

src/wallet/wallet.cpp

@@ -3522,11 +3522,13 @@ DBErrors CWallet::ZapWalletTx(std::vector<CWalletTx>& 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<CTxDestination, CAddressBookData>::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));
 }
 

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<std::string, std::string> StringMap;
     StringMap destdata;

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;

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);
 

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)