LNS: Fix generic_owner std::map preferring operator bool()

src/cryptonote_basic/tx_extra.h

@@ -87,7 +87,7 @@ constexpr inline extra_field& operator|=(extra_field& a, extra_field b) { return
 constexpr inline extra_field& operator&=(extra_field& a, extra_field b) { return a = a & b; }
 
 enum struct  generic_owner_sig_type : uint8_t { monero, ed25519, _count };
-struct generic_owner
+struct alignas(size_t) generic_owner
 {
   union {
     crypto::ed25519_public_key ed25519;
@@ -103,7 +103,7 @@ struct generic_owner
   char                   padding02_[7];
 
   std::string to_string(cryptonote::network_type nettype) const;
-  operator bool() const { return (type == generic_owner_sig_type::monero) ? wallet.address != cryptonote::null_address : ed25519; }
+  explicit operator bool() const { return (type == generic_owner_sig_type::monero) ? wallet.address != cryptonote::null_address : ed25519; }
   bool operator==(generic_owner const &other) const;
 
   BEGIN_SERIALIZE()
@@ -131,7 +131,7 @@ struct generic_signature
     unsigned char             data[sizeof(crypto::ed25519_signature)];
   };
   static constexpr generic_signature null() { return {}; }
-  operator bool() const { return memcmp(data, null().data, sizeof(data)); }
+  explicit operator bool() const { return memcmp(data, null().data, sizeof(data)); }
   bool operator==(generic_signature const &other) const { return other.type == type && memcmp(data, other.data, sizeof(data)) == 0; }
 
   BEGIN_SERIALIZE()

src/rpc/core_rpc_server.cpp

@@ -76,6 +76,17 @@ namespace
   }
 }
 
+namespace std
+{
+template <>
+struct hash<lns::generic_owner>
+{
+  static_assert(sizeof(lns::generic_owner) >= sizeof(std::size_t) && alignof(lns::generic_owner) >= alignof(std::size_t),
+                "Size and alignment of hash must be at least that of size_t");
+  std::size_t operator()(const lns::generic_owner &v) const { return reinterpret_cast<const std::size_t &>(v); }
+};
+} // namespace std
+
 namespace cryptonote
 {
   //-----------------------------------------------------------------------------------
@@ -3416,7 +3427,7 @@ namespace cryptonote
     if (exceeds_quantity_limit(ctx, error_resp, m_restricted, req.entries.size(), COMMAND_RPC_LNS_OWNERS_TO_NAMES::MAX_REQUEST_ENTRIES))
       return false;
 
-    std::map<lns::generic_owner, size_t> owner_to_request_index;
+    std::map<size_t, size_t> owner_to_request_index;
     std::vector<lns::generic_owner> owners;
 
     owners.reserve(req.entries.size());
@@ -3430,8 +3441,13 @@ namespace cryptonote
         return false;
       }
 
+      // TODO(loki): We now serialize both owner and backup_owner, since if
+      // we specify an owner that is backup owner, we don't show the (other)
+      // owner. For RPC compatibility we keep the request_index around until the
+      // next hard fork (16)
+      size_t hash = std::hash<lns::generic_owner>{}(lns_owner);
       owners.push_back(lns_owner);
-      owner_to_request_index[owners.back()] = request_index;
+      owner_to_request_index[hash] = request_index;
     }
 
     lns::name_system_db const &db = m_core.get_blockchain_storage().name_system_db();
@@ -3441,7 +3457,8 @@ namespace cryptonote
       res.entries.emplace_back();
       COMMAND_RPC_LNS_OWNERS_TO_NAMES::response_entry &entry = res.entries.back();
 
-      auto it = owner_to_request_index.find(record.owner);
+      size_t hash = std::hash<lns::generic_owner>{}(record.owner);
+      auto it     = owner_to_request_index.find(hash);
       if (it == owner_to_request_index.end())
       {
         error_resp.code    = CORE_RPC_ERROR_CODE_INTERNAL_ERROR;
@@ -3452,6 +3469,7 @@ namespace cryptonote
       entry.request_index   = it->second;
       entry.type            = static_cast<uint16_t>(record.type);
       entry.name_hash       = std::move(record.name_hash);
+      if (record.owner) entry.owner = record.owner.to_string(nettype());
       if (record.backup_owner) entry.backup_owner = record.backup_owner.to_string(nettype());
       entry.encrypted_value = epee::to_hex::string(record.encrypted_value.to_span());
       entry.register_height = record.register_height;

src/rpc/core_rpc_server_commands_defs.h

@@ -94,7 +94,7 @@ constexpr char const CORE_RPC_STATUS_TX_LONG_POLL_MAX_CONNECTIONS[] = "Daemon ma
 // advance which version they will stop working with
 // Don't go over 32767 for any of these
 #define CORE_RPC_VERSION_MAJOR 3
-#define CORE_RPC_VERSION_MINOR 2
+#define CORE_RPC_VERSION_MINOR 3
 #define MAKE_CORE_RPC_VERSION(major,minor) (((major)<<16)|(minor))
 #define CORE_RPC_VERSION MAKE_CORE_RPC_VERSION(CORE_RPC_VERSION_MAJOR, CORE_RPC_VERSION_MINOR)
 
@@ -3538,9 +3538,10 @@ constexpr char const CORE_RPC_STATUS_TX_LONG_POLL_MAX_CONNECTIONS[] = "Daemon ma
 
     struct response_entry
     {
-      uint64_t    request_index;   // The index in request's `entries` array that was resolved via Loki Name Service.
+      uint64_t    request_index;   // (Deprecated) The index in request's `entries` array that was resolved via Loki Name Service.
       uint16_t    type;            // The category the Loki Name Service entry belongs to, currently only Session whose value is 0.
       std::string name_hash;       // The hash of the name that the owner purchased via Loki Name Service in base64
+      std::string owner;           // The backup public key specified by the owner that purchased the Loki Name Service entry.
       std::string backup_owner;    // The backup public key specified by the owner that purchased the Loki Name Service entry.
       std::string encrypted_value; // The encrypted value that the name maps to. This value is encrypted using the name (not the hash) as the secret.
       uint64_t    register_height; // The height that this Loki Name Service entry was purchased on the Blockchain.
@@ -3550,6 +3551,7 @@ constexpr char const CORE_RPC_STATUS_TX_LONG_POLL_MAX_CONNECTIONS[] = "Daemon ma
         KV_SERIALIZE(request_index)
         KV_SERIALIZE(type)
         KV_SERIALIZE(name_hash)
+        KV_SERIALIZE(owner)
         KV_SERIALIZE(backup_owner)
         KV_SERIALIZE(encrypted_value)
         KV_SERIALIZE(register_height)

tests/core_tests/loki_tests.cpp

@@ -1076,8 +1076,8 @@ static bool verify_lns_mapping_record(char const *perr_context,
   CHECK_EQ(record.register_height, register_height);
   CHECK_EQ(record.txid,            txid);
   CHECK_EQ(record.prev_txid,       prev_txid);
-  CHECK_EQ(record.owner,           owner);
-  CHECK_EQ(record.backup_owner,    backup_owner);
+  CHECK_TEST_CONDITION_MSG(record.owner == owner, record.owner.to_string(cryptonote::FAKECHAIN) << " == "<< owner.to_string(cryptonote::FAKECHAIN));
+  CHECK_TEST_CONDITION_MSG(record.backup_owner == backup_owner, record.backup_owner.to_string(cryptonote::FAKECHAIN) << " == "<< backup_owner.to_string(cryptonote::FAKECHAIN));
   return true;
 }
 
@@ -1158,7 +1158,9 @@ bool loki_name_system_expiration::generate(std::vector<test_event_entry> &events
         lns::owner_record owner = lns_db.get_owner_by_key(miner_key.owner);
         CHECK_EQ(owner.loaded, true);
         CHECK_EQ(owner.id, 1);
-        CHECK_EQ(miner_key.owner, owner.address);
+        CHECK_TEST_CONDITION_MSG(miner_key.owner == owner.address,
+                                 miner_key.owner.to_string(cryptonote::FAKECHAIN)
+                                     << " == " << owner.address.to_string(cryptonote::FAKECHAIN));
 
         lns::mapping_record record = lns_db.get_mapping(mapping_type, name_hash);
         CHECK_TEST_CONDITION(verify_lns_mapping_record(perr_context, record, lns::mapping_type::lokinet_1year, name, miner_key.lokinet_value, height_of_lns_entry, tx_hash, crypto::null_hash, miner_key.owner, {} /*backup_owner*/));
@@ -1177,7 +1179,9 @@ bool loki_name_system_expiration::generate(std::vector<test_event_entry> &events
         lns::owner_record owner = lns_db.get_owner_by_key(miner_key.owner);
         CHECK_EQ(owner.loaded, true);
         CHECK_EQ(owner.id, 1);
-        CHECK_EQ(miner_key.owner, owner.address);
+        CHECK_TEST_CONDITION_MSG(miner_key.owner == owner.address,
+                                 miner_key.owner.to_string(cryptonote::FAKECHAIN)
+                                     << " == " << owner.address.to_string(cryptonote::FAKECHAIN));
 
         lns::mapping_record record = lns_db.get_mapping(mapping_type, name_hash);
         CHECK_TEST_CONDITION(verify_lns_mapping_record(perr_context, record, lns::mapping_type::lokinet_1year, name, miner_key.lokinet_value, height_of_lns_entry, tx_hash, crypto::null_hash, miner_key.owner, {} /*backup_owner*/));
@@ -1451,7 +1455,9 @@ bool loki_name_system_handles_duplicate_in_lns_db::generate(std::vector<test_eve
     lns::owner_record owner = lns_db.get_owner_by_key(miner_key.owner);
     CHECK_EQ(owner.loaded, true);
     CHECK_EQ(owner.id, 1);
-    CHECK_EQ(miner_key.owner, owner.address);
+    CHECK_TEST_CONDITION_MSG(miner_key.owner == owner.address,
+                             miner_key.owner.to_string(cryptonote::FAKECHAIN)
+                                 << " == " << owner.address.to_string(cryptonote::FAKECHAIN));
 
     std::string session_name_hash = lns::name_to_base64_hash(session_name);
     lns::mapping_record record1 = lns_db.get_mapping(lns::mapping_type::session, session_name_hash);
@@ -1881,7 +1887,9 @@ bool loki_name_system_name_renewal::generate(std::vector<test_event_entry> &even
     lns::owner_record owner = lns_db.get_owner_by_key(miner_key.owner);
     CHECK_EQ(owner.loaded, true);
     CHECK_EQ(owner.id, 1);
-    CHECK_EQ(miner_key.owner, owner.address);
+    CHECK_TEST_CONDITION_MSG(miner_key.owner == owner.address,
+                             miner_key.owner.to_string(cryptonote::FAKECHAIN)
+                                 << " == " << owner.address.to_string(cryptonote::FAKECHAIN));
 
     std::string name_hash = lns::name_to_base64_hash(name);
     lns::mapping_record record = lns_db.get_mapping(lns::mapping_type::lokinet_1year, name_hash);
@@ -1906,7 +1914,9 @@ bool loki_name_system_name_renewal::generate(std::vector<test_event_entry> &even
     lns::owner_record owner = lns_db.get_owner_by_key(miner_key.owner);
     CHECK_EQ(owner.loaded, true);
     CHECK_EQ(owner.id, 1);
-    CHECK_EQ(miner_key.owner, owner.address);
+    CHECK_TEST_CONDITION_MSG(miner_key.owner == owner.address,
+                             miner_key.owner.to_string(cryptonote::FAKECHAIN)
+                                 << " == " << owner.address.to_string(cryptonote::FAKECHAIN));
 
     std::string name_hash = lns::name_to_base64_hash(name);
     lns::mapping_record record = lns_db.get_mapping(lns::mapping_type::lokinet_1year, name_hash);
@@ -2029,7 +2039,9 @@ bool loki_name_system_update_mapping_after_expiry_fails::generate(std::vector<te
       lns::owner_record owner = lns_db.get_owner_by_key(miner_key.owner);
       CHECK_EQ(owner.loaded, true);
       CHECK_EQ(owner.id, 1);
-      CHECK_EQ(miner_key.owner, owner.address);
+      CHECK_TEST_CONDITION_MSG(miner_key.owner == owner.address,
+                               miner_key.owner.to_string(cryptonote::FAKECHAIN)
+                                   << " == " << owner.address.to_string(cryptonote::FAKECHAIN));
 
       std::string name_hash        = lns::name_to_base64_hash(name);
       lns::mapping_record record = lns_db.get_mapping(lns::mapping_type::lokinet_1year, name_hash);