Audi store #2303

kamilsa · 2024-12-04T03:52:54Z

Referenced issues

Description of the Change

Implements AudiStore that persistently stores discovered authority's peer information

Possible Drawbacks

DB-based audi store instead of in-memory

Checklist Before Opening a PR

Before you open a Pull Request (PR), please make sure you've completed the following steps and confirm by answering 'Yes' to each item:

Code is formatted: Have you run your code through clang-format to ensure it adheres to the project's coding standards? [Yes|No]
Code is documented: Have you added comments and documentation to your code according to the guidelines in the project's contributing guidelines? [Yes|No]
Self-review: Have you reviewed your own code to ensure it is free of typos, syntax errors, logical errors, and unresolved TODOs or FIXME without linking to an issue? [Yes|No]
Zombienet Tests: Have you ensured that the zombienet tests are passing? Zombienet is a network simulation and testing tool used in this project. It's important to ensure that these tests pass to maintain the stability and reliability of the project. [Yes|No]

turuslan · 2024-12-04T04:07:49Z

core/authority_discovery/query/authority_peer_info.hpp

+#include "scale/libp2p_types.hpp"
+
+namespace kagome::authority_discovery {
+  struct AuthorityPeerInfo {


Suggested change

struct AuthorityPeerInfo {

struct AuthorityPeerInfo {

SCALE_TIE(3);

turuslan · 2024-12-04T04:08:00Z

core/authority_discovery/query/authority_peer_info.hpp

+  template <class Stream>
+    requires Stream::is_encoder_stream
+  Stream &operator<<(Stream &s, const AuthorityPeerInfo &api) {
+    return s << api.raw << api.time << api.peer;
+  }
+
+  template <class Stream>
+    requires Stream::is_decoder_stream
+  Stream &operator>>(Stream &s, AuthorityPeerInfo &api) {
+    return s >> api.raw >> api.time >> api.peer;
+  }


Suggested change

template <class Stream>

requires Stream::is_encoder_stream

Stream &operator<<(Stream &s, const AuthorityPeerInfo &api) {

return s << api.raw << api.time << api.peer;

}

template <class Stream>

requires Stream::is_decoder_stream

Stream &operator>>(Stream &s, AuthorityPeerInfo &api) {

return s >> api.raw >> api.time >> api.peer;

}

turuslan · 2024-12-04T04:19:14Z

core/storage/rocksdb/rocksdb_spaces.cpp

@@ -22,7 +22,8 @@ namespace kagome::storage {
                                          "trie_value",
                                          "dispute_data",
                                          "beefy_justification",
-                                          "avaliability_storage"};
+                                          "avaliability_storage",
+                                          "audi_peers"};


Suggested change

"audi_peers"};

"audi_peers",

};

turuslan · 2024-12-04T04:24:17Z

core/authority_discovery/query/query_impl.cpp

@@ -281,12 +287,13 @@ namespace kagome::authority_discovery {
                  scale::decode<TimestampScale>(
                      qtils::str2byte(record.creation_time().timestamp())));
      time = *tmp;
-      if (it != auth_to_peer_cache_.end() and time <= it->second.time) {
+      if (it != std::nullopt and time <= it->time->number) {


Suggested change

if (it != std::nullopt and time <= it->time->number) {

if (it and it->time and time <= it->time->number) {

turuslan · 2024-12-04T04:29:16Z

core/authority_discovery/query/query_impl.cpp

+    // remove outdated authorities
+    std::deque<std::pair<primitives::AuthorityDiscoveryId, AuthorityPeerInfo>>
+        to_remove;
+    audi_store_->forEach([&](const primitives::AuthorityDiscoveryId &id,
+                             const AuthorityPeerInfo &info) {
+      if (not has(authorities, id)) {
+        to_remove.emplace_back(id, info);
      }
+    });
+    for (auto &pair : to_remove) {
+      audi_store_->remove(pair.first);
+      validation_protocol_.get()->reserve(pair.second.peer.id, false);


May simplify

audi_store_->retain([&](id, info) { if (has(id)) return true; reserve(info.peer, false); return false; });

turuslan · 2024-12-04T04:36:25Z

core/authority_discovery/query/audi_store_impl.cpp

+    return decoded.value();
+  }
+
+  bool AudiStoreImpl::remove(


Return value is not used.
remove always returns outcome::success, even for non-existing key

Suggested change

bool AudiStoreImpl::remove(

void AudiStoreImpl::remove(

turuslan · 2024-12-04T04:36:49Z

core/authority_discovery/query/audi_store_impl.cpp

+  bool AudiStoreImpl::contains(
+      const primitives::AuthorityDiscoveryId &authority) const {
+    auto res = space_->tryGet(authority);
+    return res.has_value() ? res.value().has_value() : false;


Suggested change

return res.has_value() ? res.value().has_value() : false;

return res.has_value() and res.value().has_value();

turuslan · 2024-12-04T04:39:35Z

core/authority_discovery/query/audi_store_impl.cpp

+  void AudiStoreImpl::forEach(
+      std::function<void(const primitives::AuthorityDiscoveryId &,
+                         const AuthorityPeerInfo &)> f) const {
+    auto cursor = space_->cursor();


Suggested change

auto cursor = space_->cursor();

auto cursor = space_->cursor();

std::ignore = cursor->seekFirst();

Audi store

d35d1f3

turuslan reviewed Dec 4, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Audi store #2303

Audi store #2303

kamilsa commented Dec 4, 2024

turuslan Dec 4, 2024

turuslan Dec 4, 2024

turuslan Dec 4, 2024

turuslan Dec 4, 2024

turuslan Dec 4, 2024

turuslan Dec 4, 2024

turuslan Dec 4, 2024

turuslan Dec 4, 2024

	struct AuthorityPeerInfo {
	struct AuthorityPeerInfo {
	SCALE_TIE(3);

	if (it != std::nullopt and time <= it->time->number) {
	if (it and it->time and time <= it->time->number) {

	return res.has_value() ? res.value().has_value() : false;
	return res.has_value() and res.value().has_value();

	auto cursor = space_->cursor();
	auto cursor = space_->cursor();
	std::ignore = cursor->seekFirst();

Audi store #2303

Are you sure you want to change the base?

Audi store #2303

Conversation

kamilsa commented Dec 4, 2024

Referenced issues

Description of the Change

Possible Drawbacks

Checklist Before Opening a PR

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment