rpcdaemon: remove optional in read chain config (#2130)

Author: canepat

silkworm/rpc/commands/debug_api.cpp

@@ -302,16 +302,14 @@ Task<void> DebugRpcApi::handle_debug_account_at(const nlohmann::json& request, n
 
         SILK_TRACE << "Block number: " << block_number << " #tnx: " << transactions.size();
 
-        auto chain_config_ptr = co_await chain_storage->read_chain_config();
-        ensure(chain_config_ptr.has_value(), "cannot read chain config");
-
+        const auto chain_config = co_await chain_storage->read_chain_config();
         auto this_executor = co_await boost::asio::this_coro::executor;
         auto result = co_await async_task(workers_.executor(), [&]() -> nlohmann::json {
             auto state = tx->create_state(this_executor, *chain_storage, block_number - 1);
             auto account_opt = state->read_account(address);
             account_opt.value_or(silkworm::Account{});
 
-            EVMExecutor executor{*chain_config_ptr, workers_, state};
+            EVMExecutor executor{chain_config, workers_, state};
 
             uint64_t index = std::min(static_cast<uint64_t>(transactions.size()), tx_index);
             for (uint64_t idx{0}; idx < index; idx++) {
@@ -710,9 +708,9 @@ Task<void> DebugRpcApi::handle_debug_get_raw_header(const nlohmann::json& reques
         const auto chain_storage = tx->create_storage();
         const auto block_number = co_await core::get_block_number(block_id, *tx);
         const auto block_hash = co_await chain_storage->read_canonical_hash(block_number);
-        auto header = co_await chain_storage->read_header(block_number, block_hash->bytes);
+        const auto header = co_await chain_storage->read_header(block_number, block_hash->bytes);
         if (!header) {
-            throw std::invalid_argument("header not found");
+            throw std::invalid_argument("header " + std::to_string(block_number) + " not found");
         }
         Bytes encoded_header;
         rlp::encode(encoded_header, *header);

silkworm/rpc/commands/engine_api.cpp

@@ -318,25 +318,24 @@ Task<void> EngineRpcApi::handle_engine_new_payload_v2(const nlohmann::json& requ
 #endif
         const auto storage{tx->create_storage()};
         const auto config{co_await storage->read_chain_config()};
-        ensure(config.has_value(), "execution layer has invalid configuration");
-        ensure(config->shanghai_time.has_value(), "execution layer has no Shanghai timestamp in configuration");
+        ensure(config.shanghai_time.has_value(), "execution layer has no Shanghai timestamp in configuration");
 
         // We MUST check that CL has sent the expected ExecutionPayload version [Specification for params]
-        if (payload.timestamp < config->shanghai_time && payload.version != ExecutionPayload::V1) {
+        if (payload.timestamp < config.shanghai_time && payload.version != ExecutionPayload::V1) {
             const auto error_msg = "consensus layer must use ExecutionPayloadV1 if timestamp lower than Shanghai";
             SILK_ERROR << error_msg;
             reply = make_json_error(request, kInvalidParams, error_msg);
             co_await tx->close();
             co_return;
         }
-        if (payload.timestamp >= config->shanghai_time && payload.version != ExecutionPayload::V2) {
+        if (payload.timestamp >= config.shanghai_time && payload.version != ExecutionPayload::V2) {
             const auto error_msg = "consensus layer must use ExecutionPayloadV2 if timestamp greater or equal to Shanghai";
             SILK_ERROR << error_msg;
             reply = make_json_error(request, kInvalidParams, error_msg);
             co_await tx->close();
             co_return;
         }
-        if (config->cancun_time && payload.timestamp >= config->cancun_time) {
+        if (config.cancun_time && payload.timestamp >= config.cancun_time) {
             const auto error_msg = "consensus layer must use ExecutionPayloadV3 if timestamp greater or equal to Cancun";
             SILK_ERROR << error_msg;
             reply = make_json_error(request, kUnsupportedFork, error_msg);
@@ -382,12 +381,11 @@ Task<void> EngineRpcApi::handle_engine_new_payload_v3(const nlohmann::json& requ
 #endif
         const auto storage{tx->create_storage()};
         const auto config{co_await storage->read_chain_config()};
-        ensure(config.has_value(), "execution layer has invalid configuration");
-        ensure(config->shanghai_time.has_value(), "execution layer has no Shanghai timestamp in configuration");
-        ensure(config->cancun_time.has_value(), "execution layer has no Cancun timestamp in configuration");
+        ensure(config.shanghai_time.has_value(), "execution layer has no Shanghai timestamp in configuration");
+        ensure(config.cancun_time.has_value(), "execution layer has no Cancun timestamp in configuration");
 
         // We MUST check that CL has sent the expected ExecutionPayload version [Specification for params]
-        if (payload.timestamp >= config->cancun_time && payload.version != ExecutionPayload::V3) {
+        if (payload.timestamp >= config.cancun_time && payload.version != ExecutionPayload::V3) {
             const auto error_msg = "consensus layer must use ExecutionPayloadV3 if timestamp greater or equal to Cancun";
             SILK_ERROR << error_msg;
             reply = make_json_error(request, kUnsupportedFork, error_msg);
@@ -581,13 +579,12 @@ Task<void> EngineRpcApi::handle_engine_exchange_transition_configuration_v1(cons
 #endif
         const auto storage{tx->create_storage()};
         const auto config{co_await storage->read_chain_config()};
-        ensure(config.has_value(), "execution layer has invalid configuration");
-        ensure(config->terminal_total_difficulty.has_value(), "execution layer does not have terminal total difficulty");
+        ensure(config.terminal_total_difficulty.has_value(), "execution layer does not have terminal total difficulty");
 
         // We SHOULD check for any configuration mismatch except `terminalBlockNumber` [Specification 2.]
-        if (config->terminal_total_difficulty != cl_configuration.terminal_total_difficulty) {
+        if (config.terminal_total_difficulty != cl_configuration.terminal_total_difficulty) {
             SILK_ERROR << "execution layer has the incorrect terminal total difficulty, expected: "
-                       << cl_configuration.terminal_total_difficulty << " got: " << config->terminal_total_difficulty.value();
+                       << cl_configuration.terminal_total_difficulty << " got: " << config.terminal_total_difficulty.value();
             reply = make_json_error(request, kInvalidParams, "consensus layer terminal total difficulty does not match");
             co_await tx->close();
             co_return;
@@ -602,7 +599,7 @@ Task<void> EngineRpcApi::handle_engine_exchange_transition_configuration_v1(cons
 
         // We MUST respond with configurable setting values set according to EIP-3675 [Specification 1.]
         const TransitionConfiguration transition_configuration{
-            .terminal_total_difficulty = config->terminal_total_difficulty.value(),
+            .terminal_total_difficulty = config.terminal_total_difficulty.value(),
             .terminal_block_hash = kZeroHash,  // terminal_block_hash removed from chain_config, return zero
             .terminal_block_number = 0         // terminal_block_number removed from chain_config, return zero
         };

silkworm/rpc/commands/erigon_api.cpp

@@ -129,9 +129,12 @@ Task<void> ErigonRpcApi::handle_erigon_get_block_by_timestamp(const nlohmann::js
 
         // Lookup the first and last block headers
         const auto first_header = co_await chain_storage->read_canonical_header(kEarliestBlockNumber);
+        ensure(first_header.has_value(), "cannot find earliest header");
         const auto head_header_hash = co_await core::rawdb::read_head_header_hash(*tx);
-        const auto header_header_block_number = co_await chain_storage->read_block_number(head_header_hash);
-        const auto current_header = co_await chain_storage->read_header(*header_header_block_number, head_header_hash);
+        const auto head_header_block_number = co_await chain_storage->read_block_number(head_header_hash);
+        ensure(head_header_block_number.has_value(), "cannot find head header hash");
+        const auto current_header = co_await chain_storage->read_header(*head_header_block_number, head_header_hash);
+        ensure(current_header.has_value(), "cannot find head header");
         const BlockNum current_block_number = current_header->number;
 
         // Find the lowest block header w/ timestamp greater or equal to provided timestamp
@@ -142,13 +145,14 @@ Task<void> ErigonRpcApi::handle_erigon_get_block_by_timestamp(const nlohmann::js
             block_number = kEarliestBlockNumber;
         } else {
             // Good-old binary search to find the lowest block header matching timestamp
+            // NOLINTNEXTLINE(cppcoreguidelines-avoid-capturing-lambda-coroutines)
             auto matching_block_number = co_await binary_search(current_block_number, [&](uint64_t bn) -> Task<bool> {
                 const auto header = co_await chain_storage->read_canonical_header(bn);
-                co_return header->timestamp >= timestamp;
+                co_return header && header->timestamp >= timestamp;
             });
             // TODO(canepat) we should try to avoid this block header lookup (just done in search)
             auto matching_header = co_await chain_storage->read_canonical_header(matching_block_number);
-            while (matching_header->timestamp > timestamp) {
+            while (matching_header && matching_header->timestamp > timestamp) {
                 const auto header = co_await chain_storage->read_canonical_header(matching_block_number - 1);
                 if (!header || header->timestamp < timestamp) {
                     break;
@@ -161,14 +165,9 @@ Task<void> ErigonRpcApi::handle_erigon_get_block_by_timestamp(const nlohmann::js
 
         // Lookup and return the matching block
         const auto block_with_hash = co_await core::read_block_by_number(*block_cache_, *chain_storage, block_number);
-        if (!block_with_hash) {
-            const std::string error_msg = "block not found ";
-            SILK_ERROR << "erigon_get_block_by_timestamp: core::read_block_by_number: " << error_msg << request.dump();
-            make_glaze_json_error(request, 100, error_msg, reply);
-            co_await tx->close();  // RAII not (yet) available with coroutines
-            co_return;
-        }
+        ensure(block_with_hash != nullptr, [&]() { return "block " + std::to_string(block_number) + " not found"; });
         const auto total_difficulty = co_await chain_storage->read_total_difficulty(block_with_hash->hash, block_number);
+        ensure(total_difficulty.has_value(), [&]() { return "no total difficulty for block " + std::to_string(block_number); });
         const Block extended_block{block_with_hash, *total_difficulty, full_tx};
 
         make_glaze_json_content(request, extended_block, reply);
@@ -449,11 +448,7 @@ Task<void> ErigonRpcApi::handle_erigon_forks(const nlohmann::json& request, nloh
         const auto chain_storage = tx->create_storage();
 
         const auto chain_config{co_await chain_storage->read_chain_config()};
-        if (!chain_config) {
-            throw std::runtime_error("Chain config missing");
-        }
-        SILK_DEBUG << "chain config: " << *chain_config;
-        Forks forks{*chain_config};
+        Forks forks{chain_config};
 
         reply = make_json_content(request, forks);
     } catch (const std::exception& e) {