Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sentry: treat clang-tidy warnings as errors #1732

Merged
merged 10 commits into from
Jan 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions silkworm/sentry/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion silkworm/sentry/common/random.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
Expand Down
2 changes: 1 addition & 1 deletion silkworm/sentry/discovery/common/node_address.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 3 additions & 3 deletions silkworm/sentry/discovery/disc_v4/discovery.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion silkworm/sentry/discovery/disc_v4/enr/fetch_enr_record.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion silkworm/sentry/discovery/disc_v4/find/find_neighbors.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion silkworm/sentry/discovery/disc_v4/find/lookup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
4 changes: 2 additions & 2 deletions silkworm/sentry/discovery/disc_v4/find/neighbors_message.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
Expand Down
10 changes: 5 additions & 5 deletions silkworm/sentry/discovery/disc_v4/ping/ping_check.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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,
};
}

Expand Down
8 changes: 4 additions & 4 deletions silkworm/sentry/discovery/disc_v4/ping/ping_message.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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;
}
Expand All @@ -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,
};
}

Expand Down
6 changes: 3 additions & 3 deletions silkworm/sentry/discovery/disc_v4/ping/pong_message.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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;
}
Expand All @@ -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,
};
}

Expand Down
2 changes: 1 addition & 1 deletion silkworm/sentry/discovery/disc_v4/server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
14 changes: 8 additions & 6 deletions silkworm/sentry/discovery/discovery.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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{
Expand Down
14 changes: 7 additions & 7 deletions silkworm/sentry/discovery/enr/enr_codec.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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) {
Expand All @@ -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"));

Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion silkworm/sentry/discovery/node_db/node_db_sqlite.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading