sentry: treat clang-tidy warnings as errors (#1732)

silkworm/sentry/CMakeLists.txt

@@ -23,6 +23,10 @@ find_package(Microsoft.GSL REQUIRED)
 find_package(OpenSSL REQUIRED)
 find_package(Snappy REQUIRED)
 
+if(SILKWORM_CLANG_TIDY)
+  set(CMAKE_CXX_CLANG_TIDY "${CLANG_TIDY};-warnings-as-errors=*")
+endif()
+
 # sentry common
 add_subdirectory(common)
 

silkworm/sentry/common/random.hpp

@@ -55,7 +55,7 @@ std::list<T*> random_list_items(std::list<T>& l, size_t max_count) {
 
         BackInsertPtrIterator& operator*() { return *this; }
         BackInsertPtrIterator& operator++() { return *this; }
-        BackInsertPtrIterator operator++(int) { return *this; }
+        BackInsertPtrIterator operator++(int) { return *this; }  // NOLINT(cert-dcl21-cpp)
 
       private:
         std::list<T*>* container_;

silkworm/sentry/discovery/common/node_address.cpp

@@ -63,7 +63,7 @@ void encode(Bytes& to, const NodeAddress& address) {
 //! RLP decode
 DecodingResult decode(ByteView& from, NodeAddress& to, rlp::Leftover mode) noexcept {
     Bytes ip_bytes;
-    uint16_t port;
+    uint16_t port{0};
     auto result = rlp::decode(from, mode, ip_bytes, port, to.port_rlpx);
     if (!result) {
         return result;

silkworm/sentry/discovery/disc_v4/discovery.cpp

@@ -46,7 +46,7 @@ class DiscoveryImpl : private MessageHandler {
     DiscoveryImpl(
         const boost::asio::any_io_executor& executor,
         uint16_t server_port,
-        std::function<EccKeyPair()> node_key,
+        std::function<EccKeyPair()> node_key,  // NOLINT(performance-unnecessary-value-param)
         std::function<EnodeUrl()> node_url,
         std::function<discovery::enr::EnrRecord()> node_record,
         node_db::NodeDb& node_db)
@@ -110,7 +110,7 @@ class DiscoveryImpl : private MessageHandler {
 
     Task<void> on_enr_request(enr::EnrRequestMessage message, EccPublicKey sender_public_key, boost::asio::ip::udp::endpoint sender_endpoint, Bytes packet_hash) override {
         return enr::EnrRequestHandler::handle(
-            std::move(message),
+            message,
             std::move(sender_public_key),
             std::move(sender_endpoint),
             std::move(packet_hash),
@@ -157,7 +157,7 @@ class DiscoveryImpl : private MessageHandler {
             for (auto& node_id : node_ids) {
                 // grab the semaphore and block once we reach kPingChecksTasksMax
                 co_await ping_checks_semaphore_.send(node_id.serialized());
-                ping_checks_tasks_.spawn(executor, [this, node_id = std::move(node_id)]() -> Task<void> {
+                ping_checks_tasks_.spawn(executor, [this, node_id = std::move(node_id)]() mutable -> Task<void> {
                     // when a ping check is going to finish, unblock the semaphore
                     [[maybe_unused]] auto _ = gsl::finally([this] {
                         auto finished_task_id = this->ping_checks_semaphore_.try_receive();

silkworm/sentry/discovery/disc_v4/enr/enr_response_message_test.cpp

@@ -47,8 +47,8 @@ TEST_CASE("EnrResponseMessage.rlp_decode") {
     CHECK(eth1_fork_id_data_rlp_header->payload_length == 10);
 
     // now eth1_fork_id_data is just RLP([hash_u32, next_u64])
-    uint32_t eth1_fork_id_hash;
-    uint64_t eth1_fork_id_next;
+    uint32_t eth1_fork_id_hash{0};
+    uint64_t eth1_fork_id_next{0};
     auto eth1_fork_id_decode_result = rlp::decode(eth1_fork_id_data, rlp::Leftover::kProhibit, eth1_fork_id_hash, eth1_fork_id_next);
     CHECK(eth1_fork_id_decode_result.has_value());
     if (!eth1_fork_id_decode_result) {

silkworm/sentry/discovery/disc_v4/enr/fetch_enr_record.cpp

@@ -56,7 +56,7 @@ Task<std::optional<discovery::enr::EnrRecord>> fetch_enr_record(
     };
 
     try {
-        co_await message_sender.send_enr_request(std::move(request_message), endpoint);
+        co_await message_sender.send_enr_request(request_message, endpoint);
     } catch (const boost::system::system_error& ex) {
         if (ex.code() == boost::system::errc::operation_canceled)
             throw;

silkworm/sentry/discovery/disc_v4/find/find_neighbors.cpp

@@ -53,7 +53,7 @@ Task<size_t> find_neighbors(
 
     auto executor = co_await boost::asio::this_coro::executor;
     concurrency::Channel<std::map<EccPublicKey, NodeAddress>> neighbors_channel{executor, 2};
-    auto on_neighbors_handler = [&](NeighborsMessage message, EccPublicKey sender_node_id) {
+    auto on_neighbors_handler = [&](NeighborsMessage message, const EccPublicKey& sender_node_id) {
         if ((sender_node_id == node_id) && !is_expired_message_expiration(message.expiration)) {
             neighbors_channel.try_send(std::move(message.node_addresses));
         }

silkworm/sentry/discovery/disc_v4/find/find_node_message.cpp

@@ -35,7 +35,7 @@ Bytes FindNodeMessage::rlp_encode() const {
 
 FindNodeMessage FindNodeMessage::rlp_decode(ByteView data) {
     Bytes target_public_key_data;
-    uint64_t expiration_ts;
+    uint64_t expiration_ts{0};
 
     auto result = rlp::decode(
         data,

silkworm/sentry/discovery/disc_v4/find/lookup.cpp

@@ -45,7 +45,7 @@ Task<size_t> lookup(
         /* max_lookup_time = */ now - 10min,
         /* limit = */ 3,
     };
-    auto node_ids = co_await db.take_lookup_candidates(std::move(query), now);
+    auto node_ids = co_await db.take_lookup_candidates(query, now);
 
     size_t total_neighbors = 0;
     auto group_task = concurrency::generate_parallel_group_task(node_ids.size(), [&](size_t index) -> Task<void> {

silkworm/sentry/discovery/disc_v4/find/neighbors_message.cpp

@@ -49,7 +49,7 @@ void encode(Bytes& to, const NeighborsNodeInfo& info) {
 //! RLP decode NeighborsNodeInfo
 DecodingResult decode(ByteView& from, NeighborsNodeInfo& to, rlp::Leftover mode) noexcept {
     Bytes ip_bytes;
-    uint16_t port;
+    uint16_t port{0};
     Bytes public_key_data;
     auto result = rlp::decode(from, mode, ip_bytes, port, to.address.port_rlpx, public_key_data);
     if (!result) {
@@ -88,7 +88,7 @@ Bytes NeighborsMessage::rlp_encode() const {
 
 NeighborsMessage NeighborsMessage::rlp_decode(ByteView data) {
     std::vector<NeighborsNodeInfo> node_infos;
-    uint64_t expiration_ts;
+    uint64_t expiration_ts{0};
 
     auto result = rlp::decode(
         data,

silkworm/sentry/discovery/disc_v4/ping/ping_check.cpp

@@ -79,7 +79,7 @@ Task<PingCheckResult> ping_check(
         local_enr_seq_num,
         message_sender,
         on_pong_signal,
-        std::move(last_pong_time),
+        last_pong_time,
         ping_fails_count.value_or(0));
 
     co_return result;
@@ -119,7 +119,7 @@ Task<PingCheckResult> ping_check(
     auto executor = co_await boost::asio::this_coro::executor;
     concurrency::EventNotifier pong_received_notifier{executor};
     std::optional<uint64_t> enr_seq_num;
-    auto on_pong_handler = [&](PongMessage message, EccPublicKey sender_node_id) {
+    auto on_pong_handler = [&](const PongMessage& message, const EccPublicKey& sender_node_id) {
         if ((sender_node_id == node_id) && !is_expired_message_expiration(message.expiration)) {
             enr_seq_num = message.enr_seq_num;
             pong_received_notifier.notify();
@@ -164,10 +164,10 @@ Task<PingCheckResult> ping_check(
 
     co_return PingCheckResult{
         std::move(node_id),
-        std::move(pong_time),
+        pong_time,
         ping_fails_count,
-        std::move(next_ping_time1),
-        std::move(enr_seq_num),
+        next_ping_time1,
+        enr_seq_num,
     };
 }
 

silkworm/sentry/discovery/disc_v4/ping/ping_message.cpp

@@ -39,10 +39,10 @@ Bytes PingMessage::rlp_encode() const {
 }
 
 PingMessage PingMessage::rlp_decode(ByteView data) {
-    unsigned int disc_version;
+    unsigned int disc_version{0};
     NodeAddress sender_address;
     NodeAddress recipient_address;
-    uint64_t expiration_ts;
+    uint64_t expiration_ts{0};
     std::optional<uint64_t> enr_seq_num_opt;
 
     auto result = rlp::decode(
@@ -56,7 +56,7 @@ PingMessage PingMessage::rlp_decode(ByteView data) {
         throw DecodingException(result.error(), "Failed to decode PingMessage RLP");
     }
 
-    uint64_t enr_seq_num;
+    uint64_t enr_seq_num{0};
     if (rlp::decode(data, enr_seq_num)) {
         enr_seq_num_opt = enr_seq_num;
     }
@@ -66,7 +66,7 @@ PingMessage PingMessage::rlp_decode(ByteView data) {
         sender_address.port_rlpx,
         std::move(recipient_address.endpoint),
         time_point_from_unix_timestamp(expiration_ts),
-        std::move(enr_seq_num_opt),
+        enr_seq_num_opt,
     };
 }
 

silkworm/sentry/discovery/disc_v4/ping/pong_message.cpp

@@ -39,7 +39,7 @@ Bytes PongMessage::rlp_encode() const {
 PongMessage PongMessage::rlp_decode(ByteView data) {
     NodeAddress recipient_address;
     Bytes ping_hash;
-    uint64_t expiration_ts;
+    uint64_t expiration_ts{0};
     std::optional<uint64_t> enr_seq_num_opt;
 
     auto result = rlp::decode(
@@ -52,7 +52,7 @@ PongMessage PongMessage::rlp_decode(ByteView data) {
         throw DecodingException(result.error(), "Failed to decode PingMessage RLP");
     }
 
-    uint64_t enr_seq_num;
+    uint64_t enr_seq_num{0};
     if (rlp::decode(data, enr_seq_num)) {
         enr_seq_num_opt = enr_seq_num;
     }
@@ -61,7 +61,7 @@ PongMessage PongMessage::rlp_decode(ByteView data) {
         std::move(recipient_address.endpoint),
         std::move(ping_hash),
         time_point_from_unix_timestamp(expiration_ts),
-        std::move(enr_seq_num_opt),
+        enr_seq_num_opt,
     };
 }
 

silkworm/sentry/discovery/disc_v4/server.cpp

@@ -221,7 +221,7 @@ Task<void> Server::send_neighbors(find::NeighborsMessage message, ip::udp::endpo
 }
 
 Task<void> Server::send_enr_request(enr::EnrRequestMessage message, ip::udp::endpoint recipient) {
-    return p_impl_->send_message(std::move(message), std::move(recipient));
+    return p_impl_->send_message(message, std::move(recipient));
 }
 
 Task<void> Server::send_enr_response(enr::EnrResponseMessage message, ip::udp::endpoint recipient) {

silkworm/sentry/discovery/discovery.cpp

@@ -38,7 +38,7 @@ class DiscoveryImpl {
         concurrency::ExecutorPool& executor_pool,
         std::vector<EnodeUrl> peer_urls,
         bool with_dynamic_discovery,
-        const std::filesystem::path& data_dir_path,
+        std::filesystem::path data_dir_path,
         uint64_t network_id,
         std::function<EccKeyPair()> node_key,
         std::function<EnodeUrl()> node_url,
@@ -77,21 +77,21 @@ DiscoveryImpl::DiscoveryImpl(
     concurrency::ExecutorPool& executor_pool,
     std::vector<EnodeUrl> peer_urls,
     bool with_dynamic_discovery,
-    const std::filesystem::path& data_dir_path,
+    std::filesystem::path data_dir_path,
     uint64_t network_id,
-    std::function<EccKeyPair()> node_key,
+    std::function<EccKeyPair()> node_key,  // NOLINT(performance-unnecessary-value-param)
     std::function<EnodeUrl()> node_url,
     std::function<enr::EnrRecord()> node_record,
     std::vector<EnodeUrl> bootnodes,
     uint16_t disc_v4_port)
     : peer_urls_(std::move(peer_urls)),
       with_dynamic_discovery_(with_dynamic_discovery),
-      data_dir_path_(data_dir_path),
+      data_dir_path_(std::move(data_dir_path)),
       node_id_([node_key] { return node_key().public_key(); }),
       network_id_(network_id),
       node_db_(executor_pool.any_executor()),
       bootnodes_(std::move(bootnodes)),
-      disc_v4_discovery_(executor_pool.any_executor(), disc_v4_port, node_key, node_url, node_record, node_db_.interface()) {
+      disc_v4_discovery_(executor_pool.any_executor(), disc_v4_port, node_key, std::move(node_url), std::move(node_record), node_db_.interface()) {
 }
 
 Task<void> DiscoveryImpl::run() {
@@ -139,8 +139,10 @@ Task<std::vector<Discovery::PeerCandidate>> DiscoveryImpl::request_peer_candidat
     using namespace std::chrono_literals;
 
     std::vector<node_db::NodeId> exclude_ids;
-    for (auto& url : exclude_urls)
+    exclude_ids.reserve(exclude_urls.size());
+    for (auto& url : exclude_urls) {
         exclude_ids.push_back(url.public_key());
+    }
 
     auto now = std::chrono::system_clock::now();
     node_db::NodeDb::FindPeerCandidatesQuery query{

silkworm/sentry/discovery/enr/enr_codec.cpp

@@ -83,17 +83,17 @@ static std::optional<NodeAddress> try_decode_node_address(
     const char* ip_key,
     const char* port_disc_key,
     const char* port_rlpx_key) {
-    if ((entries_data.count(ip_key) == 0) || (entries_data.count(port_disc_key) == 0))
+    if (!entries_data.contains(ip_key) || !entries_data.contains(port_disc_key))
         return std::nullopt;
 
     auto ip = ip_address_from_bytes(decode_rlp_bytes(entries_data.at(ip_key), ip_key));
     if (!ip)
         throw std::runtime_error("EnrCodec: invalid IP address");
 
-    uint16_t port_disc = decode_rlp_num_value<uint16_t>(entries_data.at(port_disc_key), port_disc_key);
+    auto port_disc = decode_rlp_num_value<uint16_t>(entries_data.at(port_disc_key), port_disc_key);
 
     uint16_t port_rlpx = 0;
-    if (entries_data.count(port_rlpx_key) > 0) {
+    if (entries_data.contains(port_rlpx_key)) {
         port_rlpx = decode_rlp_num_value<uint16_t>(entries_data.at(port_rlpx_key), port_rlpx_key);
     }
 
@@ -140,7 +140,7 @@ EnrRecord EnrCodec::decode(ByteView data) {
         items.pop_back();
 
     auto& seq_num_data = items[1];
-    uint64_t seq_num = decode_rlp_num_value<uint64_t>(seq_num_data, "seq_num");
+    auto seq_num = decode_rlp_num_value<uint64_t>(seq_num_data, "seq_num");
 
     std::map<std::string, const rlp::RlpByteView> entries_data;
     for (size_t i = 2; i < items.size(); i += 2) {
@@ -149,12 +149,12 @@ EnrRecord EnrCodec::decode(ByteView data) {
         entries_data.emplace(key, items[i + 1]);
     }
 
-    if (entries_data.count("id") == 0)
+    if (!entries_data.contains("id"))
         throw std::runtime_error("EnrCodec: missing required 'id' key");
     if (decode_rlp_bytes(entries_data.at("id"), "id") != Bytes{'v', '4'})
         throw std::runtime_error("EnrCodec: unsupported ID scheme");
 
-    if (entries_data.count("secp256k1") == 0)
+    if (!entries_data.contains("secp256k1"))
         throw std::runtime_error("EnrCodec: missing required 'secp256k1' key");
     auto public_key = EccPublicKey::deserialize_std(decode_rlp_bytes(entries_data.at("secp256k1"), "secp256k1"));
 
@@ -220,7 +220,7 @@ Bytes EnrCodec::encode(const EnrRecord& record, const EccKeyPair& key_pair) {
 
     // set signature
     Bytes signature = sign(items, key_pair.private_key());
-    items[0] = encode_rlp_bytes(std::move(signature));
+    items[0] = encode_rlp_bytes(signature);
 
     Bytes data;
     rlp::encode(data, items);

silkworm/sentry/discovery/node_db/node_db_sqlite.cpp

@@ -493,7 +493,7 @@ class NodeDbSqliteImpl : public NodeDb {
 
     Task<std::vector<NodeId>> take_lookup_candidates(FindLookupCandidatesQuery query, Time time) override {
         SQLite::Transaction transaction{*db_};
-        auto candidates = co_await find_lookup_candidates(std::move(query));
+        auto candidates = co_await find_lookup_candidates(query);
         co_await mark_taken_lookup_candidates(candidates, time);
         transaction.commit();
         co_return candidates;