From a6c3fa4c6401d145db4506a30edd78cee99d423a Mon Sep 17 00:00:00 2001 From: turuslan Date: Mon, 28 Aug 2023 10:53:23 +0300 Subject: [PATCH 1/5] simplify grandpa api Signed-off-by: turuslan --- core/consensus/grandpa/impl/grandpa_impl.cpp | 3 - core/consensus/grandpa/impl/grandpa_impl.hpp | 11 +-- core/runtime/runtime_api/grandpa_api.hpp | 40 +--------- core/runtime/runtime_api/impl/CMakeLists.txt | 1 - core/runtime/runtime_api/impl/grandpa_api.cpp | 40 ++-------- core/runtime/runtime_api/impl/grandpa_api.hpp | 24 +----- core/utils/storage_explorer.cpp | 14 ++-- test/core/runtime/binaryen/CMakeLists.txt | 14 ---- .../runtime/binaryen/grandpa_api_test.cpp | 80 ------------------- test/mock/core/runtime/grandpa_api_mock.hpp | 17 +--- 10 files changed, 22 insertions(+), 222 deletions(-) delete mode 100644 test/core/runtime/binaryen/grandpa_api_test.cpp diff --git a/core/consensus/grandpa/impl/grandpa_impl.cpp b/core/consensus/grandpa/impl/grandpa_impl.cpp index e60dd6ed77..05b0134ebb 100644 --- a/core/consensus/grandpa/impl/grandpa_impl.cpp +++ b/core/consensus/grandpa/impl/grandpa_impl.cpp @@ -72,7 +72,6 @@ namespace kagome::consensus::grandpa { std::shared_ptr hasher, std::shared_ptr environment, std::shared_ptr crypto_provider, - std::shared_ptr grandpa_api, std::shared_ptr session_keys, const application::ChainSpec &chain_spec, std::shared_ptr authority_manager, @@ -86,7 +85,6 @@ namespace kagome::consensus::grandpa { hasher_{std::move(hasher)}, environment_{std::move(environment)}, crypto_provider_{std::move(crypto_provider)}, - grandpa_api_{std::move(grandpa_api)}, session_keys_{std::move(session_keys)}, authority_manager_(std::move(authority_manager)), synchronizer_(std::move(synchronizer)), @@ -103,7 +101,6 @@ namespace kagome::consensus::grandpa { libp2p::basic::Scheduler::Config{})} { BOOST_ASSERT(environment_ != nullptr); BOOST_ASSERT(crypto_provider_ != nullptr); - BOOST_ASSERT(grandpa_api_ != nullptr); BOOST_ASSERT(scheduler_ != nullptr); BOOST_ASSERT(authority_manager_ != nullptr); BOOST_ASSERT(synchronizer_ != nullptr); diff --git a/core/consensus/grandpa/impl/grandpa_impl.hpp b/core/consensus/grandpa/impl/grandpa_impl.hpp index dbca64ddf8..2010e93201 100644 --- a/core/consensus/grandpa/impl/grandpa_impl.hpp +++ b/core/consensus/grandpa/impl/grandpa_impl.hpp @@ -3,8 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -#ifndef KAGOME_CONSENSUS_GRANDPA_GRANDPAIMPL -#define KAGOME_CONSENSUS_GRANDPA_GRANDPAIMPL +#pragma once #include "consensus/grandpa/grandpa.hpp" #include "consensus/grandpa/grandpa_observer.hpp" @@ -46,10 +45,6 @@ namespace kagome::network { class Synchronizer; } // namespace kagome::network -namespace kagome::runtime { - class GrandpaApi; -} - namespace kagome::consensus::grandpa { // clang-format off @@ -98,7 +93,6 @@ namespace kagome::consensus::grandpa { std::shared_ptr hasher, std::shared_ptr environment, std::shared_ptr crypto_provider, - std::shared_ptr grandpa_api, std::shared_ptr session_keys, const application::ChainSpec &chain_spec, std::shared_ptr authority_manager, @@ -307,7 +301,6 @@ namespace kagome::consensus::grandpa { std::shared_ptr hasher_; std::shared_ptr environment_; std::shared_ptr crypto_provider_; - std::shared_ptr grandpa_api_; std::shared_ptr session_keys_; std::shared_ptr authority_manager_; std::shared_ptr synchronizer_; @@ -343,5 +336,3 @@ namespace kagome::consensus::grandpa { }; } // namespace kagome::consensus::grandpa - -#endif // KAGOME_CONSENSUS_GRANDPA_GRANDPAIMPL diff --git a/core/runtime/runtime_api/grandpa_api.hpp b/core/runtime/runtime_api/grandpa_api.hpp index ba7fa2313f..662a8e4303 100644 --- a/core/runtime/runtime_api/grandpa_api.hpp +++ b/core/runtime/runtime_api/grandpa_api.hpp @@ -3,19 +3,10 @@ * SPDX-License-Identifier: Apache-2.0 */ -#ifndef KAGOME_RUNTIME_GRANDPAAPI -#define KAGOME_RUNTIME_GRANDPAAPI +#pragma once -#include - -#include "common/buffer.hpp" -#include "outcome/outcome.hpp" #include "primitives/authority.hpp" -#include "primitives/block_id.hpp" #include "primitives/common.hpp" -#include "primitives/digest.hpp" -#include "primitives/scheduled_change.hpp" -#include "primitives/session_key.hpp" namespace kagome::runtime { // https://github.com/paritytech/substrate/blob/8bf08ca63491961fafe6adf414a7411cb3953dcf/core/finality-grandpa/primitives/src/lib.rs#L56 @@ -25,42 +16,17 @@ namespace kagome::runtime { */ class GrandpaApi { protected: - using Digest = primitives::Digest; - using ScheduledChange = primitives::ScheduledChange; - using BlockNumber = primitives::BlockNumber; using AuthorityList = primitives::AuthorityList; - using ForcedChange = primitives::ForcedChange; - using BlockId = primitives::BlockId; public: virtual ~GrandpaApi() = default; - /** - * @brief calls Grandpa_pending_change runtime api function, - * which checks a digest for pending changes. - * @param digest digest to check - * @return nullopt if there are no pending changes, - * scheduled change item if exists or error if error occured - */ - virtual outcome::result> pending_change( - primitives::BlockHash const &block, const Digest &digest) = 0; - - /** - * @brief calls Grandpa_forced_change runtime api function - * which checks a digest for forced changes - * @param digest digest to check - * @return nullopt if no forced changes, - * forced change item if exists or error if error occured - * - */ - virtual outcome::result> forced_change( - primitives::BlockHash const &block, const Digest &digest) = 0; /** * @brief calls Grandpa_authorities runtime api function * @return collection of current grandpa authorities with their weights */ virtual outcome::result authorities( - const primitives::BlockId &block_id) = 0; + const primitives::BlockHash &block_hash) = 0; /** * @return the id of the current voter set at the provided block @@ -70,5 +36,3 @@ namespace kagome::runtime { }; } // namespace kagome::runtime - -#endif // KAGOME_RUNTIME_GRANDPAAPI diff --git a/core/runtime/runtime_api/impl/CMakeLists.txt b/core/runtime/runtime_api/impl/CMakeLists.txt index 40877c2e61..a89018b2c3 100644 --- a/core/runtime/runtime_api/impl/CMakeLists.txt +++ b/core/runtime/runtime_api/impl/CMakeLists.txt @@ -38,7 +38,6 @@ add_library(grandpa_api grandpa_api.cpp ) target_link_libraries(grandpa_api - blockchain executor ) diff --git a/core/runtime/runtime_api/impl/grandpa_api.cpp b/core/runtime/runtime_api/impl/grandpa_api.cpp index 5d1687f467..bcfc0062da 100644 --- a/core/runtime/runtime_api/impl/grandpa_api.cpp +++ b/core/runtime/runtime_api/impl/grandpa_api.cpp @@ -9,47 +9,21 @@ namespace kagome::runtime { - GrandpaApiImpl::GrandpaApiImpl( - std::shared_ptr block_header_repo, - std::shared_ptr executor) - : block_header_repo_{std::move(block_header_repo)}, - executor_{std::move(executor)} { - BOOST_ASSERT(block_header_repo_); + GrandpaApiImpl::GrandpaApiImpl(std::shared_ptr executor) + : executor_{std::move(executor)} { BOOST_ASSERT(executor_); } - outcome::result> - GrandpaApiImpl::pending_change(const primitives::BlockHash &block, - const Digest &digest) { - return executor_->callAt>( - block, "GrandpaApi_pending_change", digest); - } - - outcome::result> - GrandpaApiImpl::forced_change(const primitives::BlockHash &block, - const Digest &digest) { - return executor_->callAt>( - block, "GrandpaApi_forced_change", digest); - } - outcome::result GrandpaApiImpl::authorities( - const primitives::BlockId &block_id) { - OUTCOME_TRY(block_hash, block_header_repo_->getHashById(block_id)); - - OUTCOME_TRY(ref, authorities_.get_else(block_hash, [&] { - return executor_->callAt(block_hash, - "GrandpaApi_grandpa_authorities"); - })); - return *ref; + const primitives::BlockHash &block_hash) { + return executor_->callAt(block_hash, + "GrandpaApi_grandpa_authorities"); } outcome::result GrandpaApiImpl::current_set_id( const primitives::BlockHash &block_hash) { - OUTCOME_TRY(ref, set_id_.get_else(block_hash, [&] { - return executor_->callAt( - block_hash, "GrandpaApi_current_set_id"); - })); - return *ref; + return executor_->callAt( + block_hash, "GrandpaApi_current_set_id"); } } // namespace kagome::runtime diff --git a/core/runtime/runtime_api/impl/grandpa_api.hpp b/core/runtime/runtime_api/impl/grandpa_api.hpp index 108c097f6e..63c29cca81 100644 --- a/core/runtime/runtime_api/impl/grandpa_api.hpp +++ b/core/runtime/runtime_api/impl/grandpa_api.hpp @@ -3,44 +3,26 @@ * SPDX-License-Identifier: Apache-2.0 */ -#ifndef KAGOME_RUNTIME_IMPL_GRANDPAAPI -#define KAGOME_RUNTIME_IMPL_GRANDPAAPI +#pragma once #include "runtime/runtime_api/grandpa_api.hpp" -#include "blockchain/block_header_repository.hpp" -#include "common/lru_cache.hpp" - namespace kagome::runtime { class Executor; class GrandpaApiImpl final : public GrandpaApi { public: - GrandpaApiImpl( - std::shared_ptr block_header_repo, - std::shared_ptr executor); - - outcome::result> pending_change( - const primitives::BlockHash &block, const Digest &digest) override; - - outcome::result> forced_change( - const primitives::BlockHash &block, const Digest &digest) override; + GrandpaApiImpl(std::shared_ptr executor); outcome::result authorities( - const primitives::BlockId &block_id) override; + const primitives::BlockHash &block_hash) override; outcome::result current_set_id( const primitives::BlockHash &block) override; private: - std::shared_ptr block_header_repo_; std::shared_ptr executor_; - - LruCache authorities_{10}; - LruCache set_id_{10}; }; } // namespace kagome::runtime - -#endif // KAGOME_RUNTIME_GRANDPAAPI diff --git a/core/utils/storage_explorer.cpp b/core/utils/storage_explorer.cpp index 95d6928f91..cfc0aaf933 100644 --- a/core/utils/storage_explorer.cpp +++ b/core/utils/storage_explorer.cpp @@ -39,7 +39,7 @@ class CommandExecutionError : public std::runtime_error { : std::runtime_error{what}, command_name{command_name} {} friend std::ostream &operator<<(std::ostream &out, - CommandExecutionError const &err) { + const CommandExecutionError &err) { return out << "Error in command '" << err.command_name << "': " << err.what() << "\n"; } @@ -162,7 +162,7 @@ std::optional parseBlockId(const char *string) { class PrintHelpCommand final : public Command { public: - explicit PrintHelpCommand(CommandParser const &parser) + explicit PrintHelpCommand(const CommandParser &parser) : Command{"help", "print help message"}, parser{parser} {} virtual void execute(std::ostream &out, const ArgumentList &args) override { @@ -171,7 +171,7 @@ class PrintHelpCommand final : public Command { } private: - CommandParser const &parser; + const CommandParser &parser; }; class InspectBlockCommand : public Command { @@ -423,7 +423,7 @@ class SearchChainCommand : public Command { } void searchBlock(std::ostream &out, - BlockHeader const &header, + const BlockHeader &header, Target target) const { switch (target) { case Target::Justification: @@ -454,7 +454,7 @@ class SearchChainCommand : public Command { } void searchForAuthorityUpdate(std::ostream &out, - BlockHeader const &header) const { + const BlockHeader &header) const { for (auto &digest_item : header.digest) { auto *consensus_digest = boost::get(&digest_item); @@ -471,7 +471,7 @@ class SearchChainCommand : public Command { void reportAuthorityUpdate(std::ostream &out, BlockNumber digest_origin, - GrandpaDigest const &digest) const { + const GrandpaDigest &digest) const { using namespace kagome::primitives; if (auto *scheduled_change = boost::get(&digest); scheduled_change) { @@ -553,7 +553,7 @@ int storage_explorer_main(int argc, const char **argv) { std::make_shared( persistent_storage, hasher); auto grandpa_api = - std::make_shared(header_repo, executor); + std::make_shared(executor); auto chain_events_engine = std::make_shared(); diff --git a/test/core/runtime/binaryen/CMakeLists.txt b/test/core/runtime/binaryen/CMakeLists.txt index b86c2a97f0..aba2259a5c 100644 --- a/test/core/runtime/binaryen/CMakeLists.txt +++ b/test/core/runtime/binaryen/CMakeLists.txt @@ -60,20 +60,6 @@ target_link_libraries(parachain_test network ) -addtest(grandpa_api_test - grandpa_api_test.cpp - ) -target_link_libraries(grandpa_api_test - binaryen_runtime_test - grandpa_api - storage - basic_code_provider - key_file_storage - core_api - logger_for_tests - filesystem - ) - addtest(metadata_test metadata_test.cpp ) diff --git a/test/core/runtime/binaryen/grandpa_api_test.cpp b/test/core/runtime/binaryen/grandpa_api_test.cpp deleted file mode 100644 index 400348c349..0000000000 --- a/test/core/runtime/binaryen/grandpa_api_test.cpp +++ /dev/null @@ -1,80 +0,0 @@ -/** - * Copyright Soramitsu Co., Ltd. All Rights Reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -#include "runtime/runtime_api/impl/grandpa_api.hpp" - -#include - -#include "core/runtime/binaryen/binaryen_runtime_test.hpp" -#include "host_api/impl/host_api_impl.hpp" -#include "mock/core/blockchain/block_header_repository_mock.hpp" - -using kagome::blockchain::BlockHeaderRepositoryMock; -using kagome::common::Buffer; -using kagome::host_api::HostApiImpl; -using kagome::primitives::BlockId; -using kagome::primitives::BlockNumber; -using kagome::primitives::Digest; -using kagome::primitives::PreRuntime; -using kagome::runtime::GrandpaApi; -using kagome::runtime::GrandpaApiImpl; - -using ::testing::_; -using ::testing::Return; - -namespace fs = kagome::filesystem; - -class GrandpaTest : public BinaryenRuntimeTest { - public: - void SetUp() override { - BinaryenRuntimeTest::SetUp(); - - api_ = std::make_shared( - std::make_shared(), executor_); - } - - Digest createDigest() const { - return Digest{PreRuntime{}}; - } - - BlockId createBlockId() const { - return BlockId(BlockNumber{0}); - } - - protected: - std::shared_ptr api_; -}; - -// TODO(yuraz): PRE-157 find out do we need to give block_id to api functions -/** - * @given initialized Grandpa api - * @when pendingChange() is invoked - * @then successful result is returned - */ -TEST_F(GrandpaTest, DISABLED_PendingChange) { - auto &&digest = createDigest(); - ASSERT_TRUE(api_->pending_change("block_hash"_hash256, digest)); -} - -/** - * @given initialized Grandpa api - * @when pendingChange() is invoked - * @then successful result is returned - */ -TEST_F(GrandpaTest, DISABLED_ForcedChange) { - auto &&digest = createDigest(); - ASSERT_TRUE(api_->forced_change("block_hash"_hash256, digest)); -} - -/** - * @given initialized Grandpa api - * @when authorities() is invoked - * @then successful result is returned - * @brief writes "Uninteresting mock function call - returning default value" - */ -TEST_F(GrandpaTest, DISABLED_Authorities) { - auto block_id = createBlockId(); - ASSERT_TRUE(api_->authorities(block_id)); -} diff --git a/test/mock/core/runtime/grandpa_api_mock.hpp b/test/mock/core/runtime/grandpa_api_mock.hpp index 82a5730e51..bd4a7128f9 100644 --- a/test/mock/core/runtime/grandpa_api_mock.hpp +++ b/test/mock/core/runtime/grandpa_api_mock.hpp @@ -3,8 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -#ifndef KAGOME_RUNTIME_GRANDPAAPIMOCK -#define KAGOME_RUNTIME_GRANDPAAPIMOCK +#pragma once #include "runtime/runtime_api/grandpa_api.hpp" @@ -14,19 +13,9 @@ namespace kagome::runtime { class GrandpaApiMock : public GrandpaApi { public: - MOCK_METHOD(outcome::result>, - pending_change, - (primitives::BlockHash const &block, const Digest &digest), - (override)); - - MOCK_METHOD(outcome::result>, - forced_change, - (primitives::BlockHash const &block, const Digest &digest), - (override)); - MOCK_METHOD(outcome::result, authorities, - (const primitives::BlockId &block_id), + (const primitives::BlockHash &), (override)); MOCK_METHOD(outcome::result, @@ -36,5 +25,3 @@ namespace kagome::runtime { }; } // namespace kagome::runtime - -#endif // KAGOME_RUNTIME_GRANDPAAPIMOCK From 5f169ae50f79acf67a09dc1284e566f43d2cd8ac Mon Sep 17 00:00:00 2001 From: turuslan Date: Mon, 28 Aug 2023 11:58:54 +0300 Subject: [PATCH 2/5] simplify babe api Signed-off-by: turuslan --- core/parachain/approval/approval_distribution.cpp | 14 ++++++++------ core/parachain/approval/approval_distribution.hpp | 14 +++++++------- core/runtime/runtime_api/impl/babe_api.cpp | 7 ++----- core/runtime/runtime_api/impl/babe_api.hpp | 9 +-------- 4 files changed, 18 insertions(+), 26 deletions(-) diff --git a/core/parachain/approval/approval_distribution.cpp b/core/parachain/approval/approval_distribution.cpp index c9194ee5a9..56deadd378 100644 --- a/core/parachain/approval/approval_distribution.cpp +++ b/core/parachain/approval/approval_distribution.cpp @@ -8,6 +8,7 @@ #include "clock/impl/basic_waitable_timer.hpp" #include "common/visitor.hpp" +#include "consensus/babe/babe_config_repository.hpp" #include "consensus/babe/impl/babe_digests_util.hpp" #include "crypto/crypto_store.hpp" #include "crypto/hasher.hpp" @@ -463,7 +464,7 @@ namespace { namespace kagome::parachain { ApprovalDistribution::ApprovalDistribution( - std::shared_ptr babe_api, + std::shared_ptr babe_config_repo, std::shared_ptr app_state_manager, std::shared_ptr thread_pool, std::shared_ptr parachain_host, @@ -494,7 +495,7 @@ namespace kagome::parachain { crypto_provider_(std::move(crypto_provider)), pm_(std::move(pm)), router_(std::move(router)), - babe_api_(std::move(babe_api)), + babe_config_repo_(std::move(babe_config_repo)), block_tree_(std::move(block_tree)), pvf_(std::move(pvf)), recovery_(std::move(recovery)), @@ -510,7 +511,7 @@ namespace kagome::parachain { BOOST_ASSERT(crypto_provider_); BOOST_ASSERT(pm_); BOOST_ASSERT(router_); - BOOST_ASSERT(babe_api_); + BOOST_ASSERT(babe_config_repo_); BOOST_ASSERT(block_tree_); BOOST_ASSERT(pvf_); BOOST_ASSERT(recovery_); @@ -895,15 +896,16 @@ namespace kagome::parachain { const primitives::BlockHeader &block_header, const primitives::BlockHash &block_hash) { OUTCOME_TRY(babe_digests, consensus::babe::getBabeDigests(block_header)); - OUTCOME_TRY(babe_config, babe_api_->configuration(block_hash)); OUTCOME_TRY(epoch, babe_util_->slotToEpoch(*block_header.parentInfo(), babe_digests.second.slot_number)); + OUTCOME_TRY(babe_config, + babe_config_repo_->config(*block_header.parentInfo(), epoch)); return std::make_tuple(epoch, std::move(babe_digests.second), - std::move(babe_config.authorities), - std::move(babe_config.randomness)); + std::move(babe_config->authorities), + std::move(babe_config->randomness)); } outcome::result diff --git a/core/parachain/approval/approval_distribution.hpp b/core/parachain/approval/approval_distribution.hpp index 3e3bb15ee2..aa41c539ee 100644 --- a/core/parachain/approval/approval_distribution.hpp +++ b/core/parachain/approval/approval_distribution.hpp @@ -3,8 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -#ifndef KAGOME_APPROVAL_DISTRIBUTION_HPP -#define KAGOME_APPROVAL_DISTRIBUTION_HPP +#pragma once #include #include @@ -32,12 +31,15 @@ #include "parachain/approval/store.hpp" #include "parachain/availability/recovery/recovery.hpp" #include "parachain/validator/parachain_processor.hpp" -#include "runtime/runtime_api/babe_api.hpp" #include "runtime/runtime_api/parachain_host.hpp" #include "runtime/runtime_api/parachain_host_types.hpp" #include "utils/safe_object.hpp" #include "utils/thread_pool.hpp" +namespace kagome::consensus::babe { + class BabeConfigRepository; +} // namespace kagome::consensus::babe + namespace kagome::parachain { using DistributeAssignment = network::Assignment; @@ -257,7 +259,7 @@ namespace kagome::parachain { }; ApprovalDistribution( - std::shared_ptr babe_api, + std::shared_ptr babe_config_repo, std::shared_ptr app_state_manager, std::shared_ptr thread_pool, std::shared_ptr parachain_host, @@ -696,7 +698,7 @@ namespace kagome::parachain { std::shared_ptr crypto_provider_; std::shared_ptr pm_; std::shared_ptr router_; - std::shared_ptr babe_api_; + std::shared_ptr babe_config_repo_; std::shared_ptr block_tree_; std::shared_ptr pvf_; std::shared_ptr recovery_; @@ -732,5 +734,3 @@ namespace kagome::parachain { } // namespace kagome::parachain OUTCOME_HPP_DECLARE_ERROR(kagome::parachain, ApprovalDistribution::Error); - -#endif // KAGOME_APPROVAL_DISTRIBUTION_HPP diff --git a/core/runtime/runtime_api/impl/babe_api.cpp b/core/runtime/runtime_api/impl/babe_api.cpp index 36678660e1..5af51d80ce 100644 --- a/core/runtime/runtime_api/impl/babe_api.cpp +++ b/core/runtime/runtime_api/impl/babe_api.cpp @@ -16,11 +16,8 @@ namespace kagome::runtime { outcome::result BabeApiImpl::configuration( const primitives::BlockHash &block) { - OUTCOME_TRY(ref, cache_.get_else(block, [&] { - return executor_->callAt( - block, "BabeApi_configuration"); - })); - return *ref; + return executor_->callAt( + block, "BabeApi_configuration"); } outcome::result BabeApiImpl::next_epoch( diff --git a/core/runtime/runtime_api/impl/babe_api.hpp b/core/runtime/runtime_api/impl/babe_api.hpp index bd6bb37ee1..ca3703e748 100644 --- a/core/runtime/runtime_api/impl/babe_api.hpp +++ b/core/runtime/runtime_api/impl/babe_api.hpp @@ -3,13 +3,10 @@ * SPDX-License-Identifier: Apache-2.0 */ -#ifndef KAGOME_CORE_RUNTIME_IMPL_BABE_API_HPP -#define KAGOME_CORE_RUNTIME_IMPL_BABE_API_HPP +#pragma once #include "runtime/runtime_api/babe_api.hpp" -#include "common/lru_cache.hpp" - namespace kagome::runtime { class Executor; @@ -26,10 +23,6 @@ namespace kagome::runtime { private: std::shared_ptr executor_; - - LruCache cache_{10}; }; } // namespace kagome::runtime - -#endif // KAGOME_CORE_RUNTIME_IMPL_BABE_API_HPP From 663af44f3b5f285c4891c2c772e26222fe215b27 Mon Sep 17 00:00:00 2001 From: turuslan Date: Thu, 31 Aug 2023 09:45:34 +0300 Subject: [PATCH 3/5] disable AppStateManagerImpl signals Signed-off-by: turuslan --- core/application/impl/app_state_manager_impl.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/core/application/impl/app_state_manager_impl.cpp b/core/application/impl/app_state_manager_impl.cpp index 334fa23d20..2b630dc999 100644 --- a/core/application/impl/app_state_manager_impl.cpp +++ b/core/application/impl/app_state_manager_impl.cpp @@ -9,6 +9,8 @@ #include namespace kagome::application { + // handling signals causes threads to live longer than global variables + constexpr auto kHandleSignals = false; std::weak_ptr AppStateManagerImpl::wp_to_myself; @@ -20,6 +22,9 @@ namespace kagome::application { AppStateManagerImpl::AppStateManagerImpl() : logger_(log::createLogger("AppStateManager", "application")) { + if (not kHandleSignals) { + return; + } struct sigaction act {}; memset(&act, 0, sizeof(act)); act.sa_handler = shuttingDownSignalsHandler; // NOLINT @@ -36,6 +41,9 @@ namespace kagome::application { } AppStateManagerImpl::~AppStateManagerImpl() { + if (not kHandleSignals) { + return; + } struct sigaction act {}; memset(&act, 0, sizeof(act)); act.sa_handler = SIG_DFL; // NOLINT From 0f5b81f5b162240df36848ecf95220d731d20afd Mon Sep 17 00:00:00 2001 From: turuslan Date: Thu, 31 Aug 2023 09:54:07 +0300 Subject: [PATCH 4/5] fix test Signed-off-by: turuslan --- test/core/application/app_state_manager_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/core/application/app_state_manager_test.cpp b/test/core/application/app_state_manager_test.cpp index c87cf4ba34..8fcc15648c 100644 --- a/test/core/application/app_state_manager_test.cpp +++ b/test/core/application/app_state_manager_test.cpp @@ -291,7 +291,7 @@ TEST_F(AppStateManagerTest, RegCallbacks) { * @when register callbacks by reg() method and run() AppStateManager * @then each callcack execudted according to the stages order */ -TEST_F(AppStateManagerTest, Run_CallSequence) { +TEST_F(AppStateManagerTest, DISABLED_Run_CallSequence) { EXPECT_THROW(run(), std::logic_error); auto app_state_manager = std::make_shared(); From 48e9c008c970e3a1b23484f891bd734b5130c4c9 Mon Sep 17 00:00:00 2001 From: turuslan Date: Wed, 6 Sep 2023 13:50:39 +0300 Subject: [PATCH 5/5] revert Signed-off-by: turuslan --- core/application/impl/app_state_manager_impl.cpp | 8 -------- test/core/application/app_state_manager_test.cpp | 2 +- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/core/application/impl/app_state_manager_impl.cpp b/core/application/impl/app_state_manager_impl.cpp index 2b630dc999..334fa23d20 100644 --- a/core/application/impl/app_state_manager_impl.cpp +++ b/core/application/impl/app_state_manager_impl.cpp @@ -9,8 +9,6 @@ #include namespace kagome::application { - // handling signals causes threads to live longer than global variables - constexpr auto kHandleSignals = false; std::weak_ptr AppStateManagerImpl::wp_to_myself; @@ -22,9 +20,6 @@ namespace kagome::application { AppStateManagerImpl::AppStateManagerImpl() : logger_(log::createLogger("AppStateManager", "application")) { - if (not kHandleSignals) { - return; - } struct sigaction act {}; memset(&act, 0, sizeof(act)); act.sa_handler = shuttingDownSignalsHandler; // NOLINT @@ -41,9 +36,6 @@ namespace kagome::application { } AppStateManagerImpl::~AppStateManagerImpl() { - if (not kHandleSignals) { - return; - } struct sigaction act {}; memset(&act, 0, sizeof(act)); act.sa_handler = SIG_DFL; // NOLINT diff --git a/test/core/application/app_state_manager_test.cpp b/test/core/application/app_state_manager_test.cpp index 8fcc15648c..c87cf4ba34 100644 --- a/test/core/application/app_state_manager_test.cpp +++ b/test/core/application/app_state_manager_test.cpp @@ -291,7 +291,7 @@ TEST_F(AppStateManagerTest, RegCallbacks) { * @when register callbacks by reg() method and run() AppStateManager * @then each callcack execudted according to the stages order */ -TEST_F(AppStateManagerTest, DISABLED_Run_CallSequence) { +TEST_F(AppStateManagerTest, Run_CallSequence) { EXPECT_THROW(run(), std::logic_error); auto app_state_manager = std::make_shared();