From f42743c944b84b65ef9229be01868170d08d9716 Mon Sep 17 00:00:00 2001 From: Ruslan Tushov Date: Thu, 14 Nov 2024 14:01:52 +0500 Subject: [PATCH] fix claim queue (#2272) Signed-off-by: turuslan --- core/common/blob.hpp | 16 ++++++-- .../babe/impl/babe_block_validator_impl.cpp | 10 +---- core/parachain/types.hpp | 2 - core/parachain/validator/impl/candidates.hpp | 3 +- .../validator/impl/parachain_processor.cpp | 14 ------- .../validator/parachain_processor.hpp | 5 +-- .../prospective_parachains.cpp | 16 +------- .../prospective_parachains.hpp | 3 -- .../statement_distribution.cpp | 33 +++-------------- .../statement_distribution.hpp | 3 -- core/runtime/runtime_api/impl/babe_api.cpp | 10 ++--- core/runtime/runtime_api/impl/beefy.cpp | 15 +++----- core/runtime/runtime_api/impl/if_export.hpp | 37 +++++++++++++++++++ .../runtime_api/impl/parachain_host.cpp | 31 ++++------------ .../runtime_api/impl/parachain_host.hpp | 6 +-- core/runtime/runtime_api/parachain_host.hpp | 6 +-- .../core/parachain/prospective_parachains.cpp | 27 +++++--------- .../mock/core/runtime/parachain_host_mock.hpp | 10 +---- 18 files changed, 93 insertions(+), 154 deletions(-) create mode 100644 core/runtime/runtime_api/impl/if_export.hpp diff --git a/core/common/blob.hpp b/core/common/blob.hpp index 620d7824c0..87ec763881 100644 --- a/core/common/blob.hpp +++ b/core/common/blob.hpp @@ -14,6 +14,7 @@ #include "common/buffer_view.hpp" #include "common/hexutil.hpp" +#include "common/span_adl.hpp" #include "macro/endianness_utils.hpp" #define KAGOME_BLOB_STRICT_TYPEDEF(space_name, class_name, blob_size) \ @@ -89,8 +90,8 @@ struct fmt::formatter \ : fmt::formatter { \ template \ - auto format(const space_name::class_name &blob, \ - FormatCtx &ctx) const -> decltype(ctx.out()) { \ + auto format(const space_name::class_name &blob, FormatCtx &ctx) const \ + -> decltype(ctx.out()) { \ return fmt::formatter::format(blob, ctx); \ } \ }; @@ -211,6 +212,13 @@ namespace kagome::common { std::ranges::copy(span, blob.begin()); return blob; } + + auto operator<=>(const Blob &other) const { + return SpanAdl{*this} <=> other; + } + bool operator==(const Blob &other) const { + return SpanAdl{*this} == other; + } }; // extern specification of the most frequently instantiated blob @@ -269,8 +277,8 @@ struct fmt::formatter> { // Formats the Blob using the parsed format specification (presentation) // stored in this formatter. template - auto format(const kagome::common::Blob &blob, - FormatContext &ctx) const -> decltype(ctx.out()) { + auto format(const kagome::common::Blob &blob, FormatContext &ctx) const + -> decltype(ctx.out()) { if (presentation == 's') { if constexpr (N > 4) { uint16_t head = static_cast(blob[1]) diff --git a/core/consensus/babe/impl/babe_block_validator_impl.cpp b/core/consensus/babe/impl/babe_block_validator_impl.cpp index 1bf170a0df..68398bb89e 100644 --- a/core/consensus/babe/impl/babe_block_validator_impl.cpp +++ b/core/consensus/babe/impl/babe_block_validator_impl.cpp @@ -148,14 +148,8 @@ namespace kagome::consensus::babe { // If we were synchronized, // we have available runtime to check disabled validators if (was_synchronized_) { - std::vector disabled_validators; - if (auto res = babe_api_->disabled_validators(block_header.parent_hash); - res.has_error()) { - SL_CRITICAL(log_, - "Can't obtain disabled validators list for block {}", - block_header.blockInfo()); - } - + OUTCOME_TRY(disabled_validators, + babe_api_->disabled_validators(block_header.parent_hash)); if (std::ranges::binary_search(disabled_validators, babe_header.authority_index)) { SL_VERBOSE(log_, diff --git a/core/parachain/types.hpp b/core/parachain/types.hpp index dd150d81e3..eb7e7cb530 100644 --- a/core/parachain/types.hpp +++ b/core/parachain/types.hpp @@ -58,8 +58,6 @@ namespace kagome::parachain { /// Signature with which parachain validators sign blocks. using ValidatorSignature = Signature; - constexpr uint32_t CLAIM_QUEUE_RUNTIME_REQUIREMENT = 11; - template struct Indexed { using Type = std::decay_t; diff --git a/core/parachain/validator/impl/candidates.hpp b/core/parachain/validator/impl/candidates.hpp index 3411e34af6..001fc89adb 100644 --- a/core/parachain/validator/impl/candidates.hpp +++ b/core/parachain/validator/impl/candidates.hpp @@ -519,7 +519,8 @@ namespace kagome::parachain { }; retain_if(candidates, [&](auto &pair) { - auto &[c_hash, state] = pair; + auto &[_c_hash, state] = pair; + auto &c_hash = _c_hash; return visit_in_place( state, [&](ConfirmedCandidate &c) { diff --git a/core/parachain/validator/impl/parachain_processor.cpp b/core/parachain/validator/impl/parachain_processor.cpp index f3a5728536..53dc2bb19b 100644 --- a/core/parachain/validator/impl/parachain_processor.cpp +++ b/core/parachain/validator/impl/parachain_processor.cpp @@ -534,20 +534,6 @@ namespace kagome::parachain { } } - outcome::result> - ParachainProcessorImpl::fetch_claim_queue(const RelayHash &relay_parent) { - OUTCOME_TRY(version, parachain_host_->runtime_api_version(relay_parent)); - if (version < CLAIM_QUEUE_RUNTIME_REQUIREMENT) { - SL_TRACE(logger_, "Runtime doesn't support `request_claim_queue`"); - return std::nullopt; - } - - OUTCOME_TRY(claims, parachain_host_->claim_queue(relay_parent)); - return runtime::ClaimQueueSnapshot{ - .claimes = std::move(claims), - }; - } - outcome::result ParachainProcessorImpl::getBabeRandomness(const RelayHash &relay_parent) { OUTCOME_TRY(block_header, block_tree_->getBlockHeader(relay_parent)); diff --git a/core/parachain/validator/parachain_processor.hpp b/core/parachain/validator/parachain_processor.hpp index 03fdc5a9fe..46aabaa007 100644 --- a/core/parachain/validator/parachain_processor.hpp +++ b/core/parachain/validator/parachain_processor.hpp @@ -99,7 +99,8 @@ namespace kagome::parachain { BlockedCollationId(ParachainId pid, const Hash &h) : para_id(pid), parent_head_data_hash(h) {} - constexpr auto operator<=>(const BlockedCollationId &) const = default; + auto operator<=>(const BlockedCollationId &) const = default; + bool operator==(const BlockedCollationId &) const = default; }; } // namespace kagome::parachain @@ -505,8 +506,6 @@ namespace kagome::parachain { outcome::result getBabeRandomness( const RelayHash &relay_parent); - outcome::result> - fetch_claim_queue(const RelayHash &relay_parent); void send_to_validators_group( const RelayHash &relay_parent, const std::deque &messages); diff --git a/core/parachain/validator/prospective_parachains/prospective_parachains.cpp b/core/parachain/validator/prospective_parachains/prospective_parachains.cpp index cd581e9a74..3b5c44b50f 100644 --- a/core/parachain/validator/prospective_parachains/prospective_parachains.cpp +++ b/core/parachain/validator/prospective_parachains/prospective_parachains.cpp @@ -281,25 +281,11 @@ namespace kagome::parachain { return std::nullopt; } - outcome::result> - ProspectiveParachains::fetch_claim_queue(const RelayHash &relay_parent) { - OUTCOME_TRY(version, parachain_host_->runtime_api_version(relay_parent)); - if (version < CLAIM_QUEUE_RUNTIME_REQUIREMENT) { - SL_TRACE(logger, "Runtime doesn't support `request_claim_queue`"); - return std::nullopt; - } - - OUTCOME_TRY(claims, parachain_host_->claim_queue(relay_parent)); - return runtime::ClaimQueueSnapshot{ - .claimes = std::move(claims), - }; - } - outcome::result> ProspectiveParachains::fetchUpcomingParas( const RelayHash &relay_parent, std::unordered_set &pending_availability) { - OUTCOME_TRY(claim, fetch_claim_queue(relay_parent)); + OUTCOME_TRY(claim, parachain_host_->claim_queue(relay_parent)); if (claim) { std::unordered_set result; for (const auto &[_, paras] : claim->claimes) { diff --git a/core/parachain/validator/prospective_parachains/prospective_parachains.hpp b/core/parachain/validator/prospective_parachains/prospective_parachains.hpp index 2a0153a990..ec250fb0bc 100644 --- a/core/parachain/validator/prospective_parachains/prospective_parachains.hpp +++ b/core/parachain/validator/prospective_parachains/prospective_parachains.hpp @@ -120,9 +120,6 @@ namespace kagome::parachain { const RelayHash &relay_parent, std::unordered_set &pending_availability); - outcome::result> - fetch_claim_queue(const RelayHash &relay_parent); - outcome::result> fetchAncestry(const RelayHash &relay_hash, size_t ancestors); diff --git a/core/parachain/validator/statement_distribution/statement_distribution.cpp b/core/parachain/validator/statement_distribution/statement_distribution.cpp index 150207a91c..6970b6787e 100644 --- a/core/parachain/validator/statement_distribution/statement_distribution.cpp +++ b/core/parachain/validator/statement_distribution/statement_distribution.cpp @@ -421,9 +421,11 @@ namespace kagome::parachain::statement_distribution { std::move(authority_lookup))); }); if (per_session_state.has_error()) { - SL_WARN(logger, - "Create session data failed. (error={})", - per_session_state.error()); + if (per_session_state.error() != Error::NOT_A_VALIDATOR) { + SL_WARN(logger, + "Create session data failed. (error={})", + per_session_state.error()); + } continue; } @@ -432,14 +434,7 @@ namespace kagome::parachain::statement_distribution { OUTCOME_TRY(groups, parachain_host->validator_groups(new_relay_parent)); const auto &[_, group_rotation_info] = groups; - auto maybe_claim_queue = - [&]() -> std::optional { - auto r = fetch_claim_queue(new_relay_parent); - if (r.has_value()) { - return r.value(); - } - return std::nullopt; - }(); + OUTCOME_TRY(maybe_claim_queue, parachain_host->claim_queue(relay_parent)); auto local_validator = [&]() -> std::optional { if (!per_session_state.value()->value().v_index) { @@ -658,22 +653,6 @@ namespace kagome::parachain::statement_distribution { return groups_per_para; } - outcome::result> - StatementDistribution::fetch_claim_queue(const RelayHash &relay_parent) { - // constexpr uint32_t CLAIM_QUEUE_RUNTIME_REQUIREMENT = 11; - // OUTCOME_TRY(version, - // parachain_host->runtime_api_version(relay_parent)); if (version < - // CLAIM_QUEUE_RUNTIME_REQUIREMENT) { - // SL_TRACE(logger, "Runtime doesn't support `request_claim_queue`"); - // return std::nullopt; - // } - - OUTCOME_TRY(claims, parachain_host->claim_queue(relay_parent)); - return runtime::ClaimQueueSnapshot{ - .claimes = std::move(claims), - }; - } - bool StatementDistribution::can_disconnect(const libp2p::PeerId &peer) const { auto audi = query_audi->get(peer); if (not audi) { diff --git a/core/parachain/validator/statement_distribution/statement_distribution.hpp b/core/parachain/validator/statement_distribution/statement_distribution.hpp index 6f72effd13..35c7ca81c0 100644 --- a/core/parachain/validator/statement_distribution/statement_distribution.hpp +++ b/core/parachain/validator/statement_distribution/statement_distribution.hpp @@ -323,9 +323,6 @@ namespace kagome::parachain::statement_distribution { outcome::result> is_parachain_validator( const primitives::BlockHash &relay_parent) const; - outcome::result> - fetch_claim_queue(const RelayHash &relay_parent); - std::unordered_map> determine_groups_per_para( const std::vector &availability_cores, diff --git a/core/runtime/runtime_api/impl/babe_api.cpp b/core/runtime/runtime_api/impl/babe_api.cpp index 2a1b2c6eaa..c197827fab 100644 --- a/core/runtime/runtime_api/impl/babe_api.cpp +++ b/core/runtime/runtime_api/impl/babe_api.cpp @@ -8,6 +8,7 @@ #include "runtime/common/runtime_execution_error.hpp" #include "runtime/executor.hpp" +#include "runtime/runtime_api/impl/if_export.hpp" namespace kagome::runtime { @@ -56,12 +57,7 @@ namespace kagome::runtime { outcome::result> BabeApiImpl::disabled_validators(const primitives::BlockHash &block) { OUTCOME_TRY(ctx, executor_->ctx().ephemeralAt(block)); - auto res = executor_->call>( - ctx, "ParachainHost_disabled_validators"); - if (res.has_error() - and res.error() == RuntimeExecutionError::EXPORT_FUNCTION_NOT_FOUND) { - return std::vector{}; - } - return res; + return ifExportVec(executor_->call>( + ctx, "ParachainHost_disabled_validators")); } } // namespace kagome::runtime diff --git a/core/runtime/runtime_api/impl/beefy.cpp b/core/runtime/runtime_api/impl/beefy.cpp index e3cd831e2b..4fdcd51bca 100644 --- a/core/runtime/runtime_api/impl/beefy.cpp +++ b/core/runtime/runtime_api/impl/beefy.cpp @@ -8,6 +8,7 @@ #include "runtime/common/runtime_execution_error.hpp" #include "runtime/executor.hpp" +#include "runtime/runtime_api/impl/if_export.hpp" namespace kagome::runtime { BeefyApiImpl::BeefyApiImpl(std::shared_ptr executor) @@ -18,15 +19,11 @@ namespace kagome::runtime { outcome::result> BeefyApiImpl::genesis( const primitives::BlockHash &block) { OUTCOME_TRY(ctx, executor_->ctx().ephemeralAt(block)); - auto r = executor_->call>( - ctx, "BeefyApi_beefy_genesis"); - if (r) { - return r.value(); - } - if (r.error() == RuntimeExecutionError::EXPORT_FUNCTION_NOT_FOUND) { - return std::nullopt; - } - return r.error(); + OUTCOME_TRY( + r, + ifExport(executor_->call>( + ctx, "BeefyApi_beefy_genesis"))); + return r.value_or(std::nullopt); } outcome::result> diff --git a/core/runtime/runtime_api/impl/if_export.hpp b/core/runtime/runtime_api/impl/if_export.hpp new file mode 100644 index 0000000000..ee5bd1548c --- /dev/null +++ b/core/runtime/runtime_api/impl/if_export.hpp @@ -0,0 +1,37 @@ +/** + * Copyright Quadrivium LLC + * All Rights Reserved + * SPDX-License-Identifier: Apache-2.0 + */ + +#pragma once + +#include +#include + +#include "runtime/common/runtime_execution_error.hpp" + +namespace kagome::runtime { + template + outcome::result> ifExport(outcome::result &&r) { + if (r) { + return std::move(r.value()); + } + if (r.error() == RuntimeExecutionError::EXPORT_FUNCTION_NOT_FOUND) { + return std::nullopt; + } + return r.error(); + } + + template + outcome::result> ifExportVec( + outcome::result> &&r) { + if (r) { + return std::move(r.value()); + } + if (r.error() == RuntimeExecutionError::EXPORT_FUNCTION_NOT_FOUND) { + return std::vector{}; + } + return r.error(); + } +} // namespace kagome::runtime diff --git a/core/runtime/runtime_api/impl/parachain_host.cpp b/core/runtime/runtime_api/impl/parachain_host.cpp index 79e07c90dc..03839557f3 100644 --- a/core/runtime/runtime_api/impl/parachain_host.cpp +++ b/core/runtime/runtime_api/impl/parachain_host.cpp @@ -9,6 +9,7 @@ #include "common/blob.hpp" #include "runtime/common/runtime_execution_error.hpp" #include "runtime/executor.hpp" +#include "runtime/runtime_api/impl/if_export.hpp" #include "runtime/runtime_api/impl/parachain_host_types_serde.hpp" #include "scale/std_variant.hpp" @@ -270,17 +271,11 @@ namespace kagome::runtime { ctx, "ParachainHost_para_backing_state", id); } - outcome::result>> - ParachainHostImpl::claim_queue(const primitives::BlockHash &block) { - OUTCOME_TRY(ctx, executor_->ctx().ephemeralAt(block)); - return executor_->call>>( - ctx, "ParachainHost_claim_queue"); - } - - outcome::result ParachainHostImpl::runtime_api_version( + ParachainHost::ClaimQueueResult ParachainHostImpl::claim_queue( const primitives::BlockHash &block) { OUTCOME_TRY(ctx, executor_->ctx().ephemeralAt(block)); - return executor_->call(ctx, "ParachainHost_runtime_api_version"); + return ifExport( + executor_->call(ctx, "ParachainHost_claim_queue")); } outcome::result @@ -301,26 +296,16 @@ namespace kagome::runtime { outcome::result> ParachainHostImpl::disabled_validators(const primitives::BlockHash &block) { OUTCOME_TRY(ctx, executor_->ctx().ephemeralAt(block)); - auto res = executor_->call>( - ctx, "ParachainHost_disabled_validators"); - if (res.has_error() - and res.error() == RuntimeExecutionError::EXPORT_FUNCTION_NOT_FOUND) { - return outcome::success(std::vector{}); - } - return res; + return ifExportVec(executor_->call>( + ctx, "ParachainHost_disabled_validators")); } outcome::result> ParachainHostImpl::node_features(const primitives::BlockHash &block, SessionIndex index) { OUTCOME_TRY(ctx, executor_->ctx().ephemeralAt(block)); - auto res = executor_->call( - ctx, "ParachainHost_node_features"); - if (res.has_error() - and res.error() == RuntimeExecutionError::EXPORT_FUNCTION_NOT_FOUND) { - return outcome::success(std::nullopt); - } - return res.value(); + return ifExport(executor_->call( + ctx, "ParachainHost_node_features")); } } // namespace kagome::runtime diff --git a/core/runtime/runtime_api/impl/parachain_host.hpp b/core/runtime/runtime_api/impl/parachain_host.hpp index 3c00fcf9a6..2d96568bc2 100644 --- a/core/runtime/runtime_api/impl/parachain_host.hpp +++ b/core/runtime/runtime_api/impl/parachain_host.hpp @@ -118,11 +118,7 @@ namespace kagome::runtime { outcome::result> node_features( const primitives::BlockHash &block, SessionIndex index) override; - outcome::result>> claim_queue( - const primitives::BlockHash &block) override; - - outcome::result runtime_api_version( - const primitives::BlockHash &block) override; + ClaimQueueResult claim_queue(const primitives::BlockHash &block) override; private: bool prepare(); diff --git a/core/runtime/runtime_api/parachain_host.hpp b/core/runtime/runtime_api/parachain_host.hpp index 34f51a54ed..f93aac75d0 100644 --- a/core/runtime/runtime_api/parachain_host.hpp +++ b/core/runtime/runtime_api/parachain_host.hpp @@ -258,10 +258,8 @@ namespace kagome::runtime { virtual outcome::result> node_features( const primitives::BlockHash &block, SessionIndex index) = 0; - virtual outcome::result>> - claim_queue(const primitives::BlockHash &block) = 0; - - virtual outcome::result runtime_api_version( + using ClaimQueueResult = outcome::result>; + virtual ClaimQueueResult claim_queue( const primitives::BlockHash &block) = 0; }; diff --git a/test/core/parachain/prospective_parachains.cpp b/test/core/parachain/prospective_parachains.cpp index d2b0972704..5219de061a 100644 --- a/test/core/parachain/prospective_parachains.cpp +++ b/test/core/parachain/prospective_parachains.cpp @@ -15,6 +15,8 @@ using namespace kagome::parachain; namespace runtime = kagome::runtime; +using kagome::runtime::ClaimQueueSnapshot; + class ProspectiveParachainsTest : public ProspectiveParachainsTestHarness { void SetUp() override { ProspectiveParachainsTestHarness::SetUp(); @@ -43,12 +45,8 @@ class ProspectiveParachainsTest : public ProspectiveParachainsTestHarness { struct TestState { ClaimQueue claim_queue = {{CoreIndex(0), {ParachainId(1)}}, {CoreIndex(1), {ParachainId(2)}}}; - uint32_t runtime_api_version = CLAIM_QUEUE_RUNTIME_REQUIREMENT; + bool enable_claim_queue_api = true; ValidationCodeHash validation_code_hash = fromNumber(42); - - void set_runtime_api_version(uint32_t version) { - runtime_api_version = version; - } }; struct PerParaData { @@ -127,11 +125,9 @@ class ProspectiveParachainsTest : public ProspectiveParachainsTestHarness { EXPECT_CALL(*parachain_api_, staging_async_backing_params(hash)) .WillRepeatedly(Return(outcome::success(async_backing_params))); - EXPECT_CALL(*parachain_api_, runtime_api_version(hash)) - .WillRepeatedly( - Return(outcome::success(test_state.runtime_api_version))); - - if (test_state.runtime_api_version < CLAIM_QUEUE_RUNTIME_REQUIREMENT) { + if (not test_state.enable_claim_queue_api) { + EXPECT_CALL(*parachain_api_, claim_queue(hash)) + .WillRepeatedly(Return(std::nullopt)); std::vector cores; for (const auto &[_, paras] : test_state.claim_queue) { cores.emplace_back(runtime::ScheduledCore{ @@ -143,7 +139,7 @@ class ProspectiveParachainsTest : public ProspectiveParachainsTestHarness { .WillRepeatedly(Return(outcome::success(cores))); } else { EXPECT_CALL(*parachain_api_, claim_queue(hash)) - .WillRepeatedly(Return(outcome::success(test_state.claim_queue))); + .WillRepeatedly(Return(ClaimQueueSnapshot{test_state.claim_queue})); } EXPECT_CALL(*block_tree_, getBlockHeader(hash)) @@ -1437,7 +1433,7 @@ TEST_F(ProspectiveParachainsTest, check_pvd_query) { TEST_F(ProspectiveParachainsTest, correctly_updates_leaves) { TestState test_state; - test_state.set_runtime_api_version(8); + test_state.enable_claim_queue_api = false; // Leaf A const TestLeaf leaf_a{ @@ -1979,13 +1975,8 @@ TEST_F(ProspectiveParachainsTest, uses_ancestry_only_within_session) { .WillRepeatedly(Return(outcome::success(fragment::AsyncBackingParams{ .max_candidate_depth = 0, .allowed_ancestry_len = ancestry_len}))); - EXPECT_CALL(*parachain_api_, runtime_api_version(hash)) - .WillRepeatedly( - Return(outcome::success(CLAIM_QUEUE_RUNTIME_REQUIREMENT))); - EXPECT_CALL(*parachain_api_, claim_queue(hash)) - .WillRepeatedly(Return( - outcome::success(std::map>{}))); + .WillRepeatedly(Return(ClaimQueueSnapshot{})); EXPECT_CALL(*block_tree_, getBlockHeader(hash)) .WillRepeatedly(Return(BlockHeader{ diff --git a/test/mock/core/runtime/parachain_host_mock.hpp b/test/mock/core/runtime/parachain_host_mock.hpp index b785a7f0fb..756426cd2a 100644 --- a/test/mock/core/runtime/parachain_host_mock.hpp +++ b/test/mock/core/runtime/parachain_host_mock.hpp @@ -159,14 +159,8 @@ namespace kagome::runtime { (const primitives::BlockHash &, SessionIndex), (override)); - MOCK_METHOD( - (outcome::result>>), - claim_queue, - (const primitives::BlockHash &), - (override)); - - MOCK_METHOD(outcome::result, - runtime_api_version, + MOCK_METHOD(ClaimQueueResult, + claim_queue, (const primitives::BlockHash &), (override)); };