From 2b945ecab1e16631e1a1e93b3b8d1c78f0fb5891 Mon Sep 17 00:00:00 2001 From: turuslan Date: Fri, 29 Jul 2022 11:17:36 +0300 Subject: [PATCH 01/20] optional Signed-off-by: turuslan --- core/api/service/impl/api_service_impl.cpp | 2 +- core/api/service/impl/api_service_impl.hpp | 2 +- core/primitives/event_types.hpp | 2 +- .../impl/storage_changes_tracker_impl.cpp | 13 +++++++------ .../impl/storage_changes_tracker_impl.hpp | 3 ++- 5 files changed, 12 insertions(+), 10 deletions(-) diff --git a/core/api/service/impl/api_service_impl.cpp b/core/api/service/impl/api_service_impl.cpp index 41f02ffd83..eb8c5608f8 100644 --- a/core/api/service/impl/api_service_impl.cpp +++ b/core/api/service/impl/api_service_impl.cpp @@ -515,7 +515,7 @@ namespace kagome::api { void ApiServiceImpl::onStorageEvent(SubscriptionSetId set_id, SessionPtr &session, const Buffer &key, - const Buffer &data, + const std::optional &data, const common::Hash256 &block) { sendEvent(server_, session, diff --git a/core/api/service/impl/api_service_impl.hpp b/core/api/service/impl/api_service_impl.hpp index 5b54a93681..df8eaa27bc 100644 --- a/core/api/service/impl/api_service_impl.hpp +++ b/core/api/service/impl/api_service_impl.hpp @@ -197,7 +197,7 @@ namespace kagome::api { void onStorageEvent(SubscriptionSetId set_id, SessionPtr &session, const Buffer &key, - const Buffer &data, + const std::optional &data, const common::Hash256 &block); void onChainEvent(SubscriptionSetId set_id, SessionPtr &session, diff --git a/core/primitives/event_types.hpp b/core/primitives/event_types.hpp index 95ee4f4681..825d84a2de 100644 --- a/core/primitives/event_types.hpp +++ b/core/primitives/event_types.hpp @@ -216,7 +216,7 @@ namespace kagome::primitives::events { using StorageSubscriptionEngine = subscription::SubscriptionEngine, - common::Buffer, + std::optional, primitives::BlockHash>; using StorageSubscriptionEnginePtr = std::shared_ptr; diff --git a/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp b/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp index 71b01d2d3b..1d8356e010 100644 --- a/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp +++ b/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp @@ -55,10 +55,11 @@ namespace kagome::storage::changes_trie { primitives::events::ChainEventType::kNewRuntime, hash); } for (auto &pair : actual_val_) { - SL_TRACE(logger_, - "Key: 0x{}; Value 0x{};", - pair.first.toHex(), - pair.second.toHex()); + if (pair.second) { + SL_TRACE(logger_, "Key: {:l}; Value {:l};", pair.first, *pair.second); + } else { + SL_TRACE(logger_, "Key: {:l}; Removed;", pair.first); + } storage_subscription_engine_->notify( pair.first, pair.second, parent_hash_); } @@ -71,7 +72,7 @@ namespace kagome::storage::changes_trie { && prefix.size() <= static_cast(it->first.size()) && it->first.view(0, prefix.size()) == prefix; ++it) { - it->second.clear(); + it->second.reset(); } } @@ -101,7 +102,7 @@ namespace kagome::storage::changes_trie { outcome::result StorageChangesTrackerImpl::onRemove( common::BufferView extrinsic_index, const common::BufferView &key) { if (auto it = actual_val_.find(key); it != actual_val_.end()) { - it->second.clear(); + it->second.reset(); } auto change_it = extrinsics_changes_.find(key); diff --git a/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp b/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp index cfaf9e2169..93603e4a03 100644 --- a/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp +++ b/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp @@ -59,7 +59,8 @@ namespace kagome::storage::changes_trie { std::set> new_entries_; // entries that do not yet exist in // the underlying storage - std::map> actual_val_; + std::map, std::less<>> + actual_val_; primitives::BlockHash parent_hash_; primitives::BlockNumber parent_number_; From 87a5261e4f6e7bc16ad8240fefa593b001138590 Mon Sep 17 00:00:00 2001 From: turuslan Date: Fri, 29 Jul 2022 11:36:16 +0300 Subject: [PATCH 02/20] remove onClearPrefix Signed-off-by: turuslan --- core/storage/changes_trie/changes_tracker.hpp | 5 ----- .../impl/storage_changes_tracker_impl.cpp | 11 ----------- .../impl/storage_changes_tracker_impl.hpp | 1 - core/storage/trie/impl/persistent_trie_batch_impl.cpp | 1 - .../storage/changes_trie/changes_tracker_mock.hpp | 2 -- 5 files changed, 20 deletions(-) diff --git a/core/storage/changes_trie/changes_tracker.hpp b/core/storage/changes_trie/changes_tracker.hpp index 185c8f42a6..17f081319d 100644 --- a/core/storage/changes_trie/changes_tracker.hpp +++ b/core/storage/changes_trie/changes_tracker.hpp @@ -40,11 +40,6 @@ namespace kagome::storage::changes_trie { */ virtual void onBlockAdded(const primitives::BlockHash &hash) = 0; - /** - * Supposed to be called when clear by prefix called. - */ - virtual void onClearPrefix(const common::BufferView &prefix) = 0; - /** * Supposed to be called when an entry is removed from the tracked storage */ diff --git a/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp b/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp index 1d8356e010..f59f1b8435 100644 --- a/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp +++ b/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp @@ -65,17 +65,6 @@ namespace kagome::storage::changes_trie { } } - void StorageChangesTrackerImpl::onClearPrefix( - const common::BufferView &prefix) { - for (auto it = actual_val_.lower_bound(prefix); - it != actual_val_.end() - && prefix.size() <= static_cast(it->first.size()) - && it->first.view(0, prefix.size()) == prefix; - ++it) { - it->second.reset(); - } - } - outcome::result StorageChangesTrackerImpl::onPut( common::BufferView extrinsic_index, const common::BufferView &key, diff --git a/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp b/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp index 93603e4a03..826932f8dd 100644 --- a/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp +++ b/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp @@ -40,7 +40,6 @@ namespace kagome::storage::changes_trie { const common::BufferView &value, bool new_entry) override; void onBlockAdded(const primitives::BlockHash &hash) override; - void onClearPrefix(const common::BufferView &prefix) override; outcome::result onRemove(common::BufferView extrinsic_index, const common::BufferView &key) override; diff --git a/core/storage/trie/impl/persistent_trie_batch_impl.cpp b/core/storage/trie/impl/persistent_trie_batch_impl.cpp index 1c4e0450da..eb99a7394d 100644 --- a/core/storage/trie/impl/persistent_trie_batch_impl.cpp +++ b/core/storage/trie/impl/persistent_trie_batch_impl.cpp @@ -103,7 +103,6 @@ namespace kagome::storage::trie { outcome::result> PersistentTrieBatchImpl::clearPrefix(const BufferView &prefix, std::optional limit) { - if (changes_.has_value()) changes_.value()->onClearPrefix(prefix); SL_TRACE_VOID_FUNC_CALL(logger_, prefix); OUTCOME_TRY(ext_idx, getExtrinsicIndex()); return trie_->clearPrefix( diff --git a/test/mock/core/storage/changes_trie/changes_tracker_mock.hpp b/test/mock/core/storage/changes_trie/changes_tracker_mock.hpp index eb9c8e691d..3bbbdedbd3 100644 --- a/test/mock/core/storage/changes_trie/changes_tracker_mock.hpp +++ b/test/mock/core/storage/changes_trie/changes_tracker_mock.hpp @@ -29,8 +29,6 @@ namespace kagome::storage::changes_trie { (const primitives::BlockHash &block_hash), (override)); - MOCK_METHOD(void, onClearPrefix, (const common::BufferView &), (override)); - MOCK_METHOD(outcome::result, onPut, (common::BufferView ext_idx, From a79a8fa0ac4fce889d611d0bcd22af0a007244e9 Mon Sep 17 00:00:00 2001 From: turuslan Date: Fri, 29 Jul 2022 14:28:42 +0300 Subject: [PATCH 03/20] remove ChangesTrieSignal Signed-off-by: turuslan --- core/primitives/digest.hpp | 29 +------------------ test/core/authorship/proposer_test.cpp | 3 -- .../authority/authority_manager_test.cpp | 3 -- test/core/consensus/babe/babe_test.cpp | 3 -- 4 files changed, 1 insertion(+), 37 deletions(-) diff --git a/core/primitives/digest.hpp b/core/primitives/digest.hpp index bbca039751..97b2f89545 100644 --- a/core/primitives/digest.hpp +++ b/core/primitives/digest.hpp @@ -36,33 +36,6 @@ namespace kagome::primitives { /// trie creation. struct ChangesTrieRoot : public common::Hash256 {}; - // TODO (kamilsa): workaround unless we bump gtest version to 1.8.1+ - // after gtest update use `data` type directly - struct ChangesTrieSignal { - boost::variant> - data; - - bool operator==(const ChangesTrieSignal &rhs) const { - return data == rhs.data; - } - - bool operator!=(const ChangesTrieSignal &rhs) const { - return !operator==(rhs); - } - }; - - template > - Stream &operator<<(Stream &s, const ChangesTrieSignal &sig) { - return s << sig.data; - } - - template > - Stream &operator>>(Stream &s, ChangesTrieSignal &sig) { - return s >> sig.data; - } - struct Other : public common::Buffer {}; namespace detail { @@ -225,7 +198,7 @@ namespace kagome::primitives { Consensus, // 4 Seal, // 5 PreRuntime, // 6 - ChangesTrieSignal, // 7 + Unused<7>, // 7 RuntimeEnvironmentUpdated>; // 8 /** diff --git a/test/core/authorship/proposer_test.cpp b/test/core/authorship/proposer_test.cpp index 66091cc814..1da81f05bc 100644 --- a/test/core/authorship/proposer_test.cpp +++ b/test/core/authorship/proposer_test.cpp @@ -51,9 +51,6 @@ namespace kagome::primitives { const detail::DigestItemCommon &dic) { return s; } - std::ostream &operator<<(std::ostream &s, const ChangesTrieSignal &) { - return s; - } } // namespace kagome::primitives class ProposerTest : public ::testing::Test { diff --git a/test/core/consensus/authority/authority_manager_test.cpp b/test/core/consensus/authority/authority_manager_test.cpp index 3452f344b7..529d2d8c4f 100644 --- a/test/core/consensus/authority/authority_manager_test.cpp +++ b/test/core/consensus/authority/authority_manager_test.cpp @@ -37,9 +37,6 @@ namespace kagome::primitives { const detail::DigestItemCommon &dic) { return s; } - std::ostream &operator<<(std::ostream &s, const ChangesTrieSignal &) { - return s; - } } // namespace kagome::primitives class AuthorityManagerTest : public testing::Test { diff --git a/test/core/consensus/babe/babe_test.cpp b/test/core/consensus/babe/babe_test.cpp index c6eda22ff7..d94179d519 100644 --- a/test/core/consensus/babe/babe_test.cpp +++ b/test/core/consensus/babe/babe_test.cpp @@ -61,9 +61,6 @@ namespace kagome::primitives { const detail::DigestItemCommon &dic) { return s; } - std::ostream &operator<<(std::ostream &s, const ChangesTrieSignal &) { - return s; - } } // namespace kagome::primitives static Digest make_digest(BabeSlotNumber slot) { From 9313799a5a495a7bb229ad6d887051a30782c67b Mon Sep 17 00:00:00 2001 From: turuslan Date: Fri, 29 Jul 2022 14:29:16 +0300 Subject: [PATCH 04/20] remove ChangesTrieRoot Signed-off-by: turuslan --- core/primitives/digest.hpp | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/core/primitives/digest.hpp b/core/primitives/digest.hpp index 97b2f89545..528473b3b5 100644 --- a/core/primitives/digest.hpp +++ b/core/primitives/digest.hpp @@ -31,11 +31,6 @@ namespace kagome::primitives { inline const auto kUnsupportedEngineId_BEEF = ConsensusEngineId::fromString("BEEF").value(); - /// System digest item that contains the root of changes trie at given - /// block. It is created for every block iff runtime supports changes - /// trie creation. - struct ChangesTrieRoot : public common::Hash256 {}; - struct Other : public common::Buffer {}; namespace detail { @@ -193,7 +188,7 @@ namespace kagome::primitives { /// https://github.com/paritytech/substrate/blob/polkadot-v0.9.12/primitives/runtime/src/generic/digest.rs#L272 using DigestItem = boost::variant, // 1 - ChangesTrieRoot, // 2 + Unused<2>, // 2 Unused<3>, // 3 Consensus, // 4 Seal, // 5 From da139e4cf59ef3a72b0fa0e1f243df1e34b5196b Mon Sep 17 00:00:00 2001 From: turuslan Date: Fri, 29 Jul 2022 14:34:18 +0300 Subject: [PATCH 05/20] ext_storage_changes_root_version_1 return null Signed-off-by: turuslan --- core/host_api/impl/storage_extension.cpp | 58 +--------------- core/host_api/impl/storage_extension.hpp | 3 - test/core/host_api/storage_extension_test.cpp | 69 ------------------- 3 files changed, 2 insertions(+), 128 deletions(-) diff --git a/core/host_api/impl/storage_extension.cpp b/core/host_api/impl/storage_extension.cpp index 129b85c585..fe4167f12c 100644 --- a/core/host_api/impl/storage_extension.cpp +++ b/core/host_api/impl/storage_extension.cpp @@ -22,8 +22,6 @@ using kagome::common::Buffer; namespace { - const auto CHANGES_CONFIG_KEY = kagome::common::Buffer{}.put(":changes_trie"); - [[nodiscard]] kagome::storage::trie::StateVersion toStateVersion( kagome::runtime::WasmI32 state_version_int) { if (state_version_int == 0) { @@ -254,20 +252,9 @@ namespace kagome::host_api { runtime::WasmSpan StorageExtension::ext_storage_changes_root_version_1( runtime::WasmSpan parent_hash_data) { - auto parent_hash_span = runtime::PtrSize(parent_hash_data); auto &memory = memory_provider_->getCurrentMemory()->get(); - auto parent_hash_bytes = - memory.loadN(parent_hash_span.ptr, parent_hash_span.size); - common::Hash256 parent_hash; - std::copy_n(parent_hash_bytes.begin(), - common::Hash256::size(), - parent_hash.begin()); - - auto &&result = calcStorageChangesRoot(parent_hash); - auto &&res = result.has_value() - ? std::make_optional(std::move(result.value())) - : std::nullopt; - return memory.storeBuffer(scale::encode(std::move(res)).value()); + // https://github.com/paritytech/substrate/pull/10080 + return memory.storeBuffer(scale::encode(std::nullopt).value()); } runtime::WasmSpan StorageExtension::ext_storage_next_key_version_1( @@ -452,47 +439,6 @@ namespace kagome::host_api { return runtime::PtrSize(res).ptr; } - std::optional StorageExtension::calcStorageChangesRoot( - common::Hash256 parent_hash) const { - if (not storage_provider_->tryGetPersistentBatch()) { - logger_->error("ext_storage_changes_root persistent batch not found"); - return std::nullopt; - } - auto batch = storage_provider_->tryGetPersistentBatch().value(); - auto config_bytes_res = batch->tryGet(CHANGES_CONFIG_KEY); - if (config_bytes_res.has_error()) { - logger_->error("ext_storage_changes_root resulted with an error: {}", - config_bytes_res.error().message()); - throw std::runtime_error(config_bytes_res.error().message()); - } - if (config_bytes_res.value() == std::nullopt) { - return std::nullopt; - } - auto config_res = scale::decode( - config_bytes_res.value().value().get()); - if (config_res.has_error()) { - logger_->error("ext_storage_changes_root resulted with an error: {}", - config_res.error().message()); - throw std::runtime_error(config_res.error().message()); - } - storage::changes_trie::ChangesTrieConfig trie_config = config_res.value(); - - SL_DEBUG(logger_, - "ext_storage_changes_root constructing changes trie with " - "parent_hash: {}", - parent_hash.toHex()); - auto trie_hash_res = - changes_tracker_->constructChangesTrie(parent_hash, trie_config); - if (trie_hash_res.has_error()) { - logger_->error("ext_storage_changes_root resulted with an error: {}", - trie_hash_res.error().message()); - throw std::runtime_error(trie_hash_res.error().message()); - } - common::Buffer result_buf(trie_hash_res.value()); - SL_TRACE_FUNC_CALL(logger_, result_buf, parent_hash); - return result_buf; - } - runtime::WasmSpan StorageExtension::clearPrefix( common::BufferView prefix, std::optional limit) { auto batch = storage_provider_->getCurrentBatch(); diff --git a/core/host_api/impl/storage_extension.hpp b/core/host_api/impl/storage_extension.hpp index 9033dbbf6e..ca86edbe0a 100644 --- a/core/host_api/impl/storage_extension.hpp +++ b/core/host_api/impl/storage_extension.hpp @@ -163,9 +163,6 @@ namespace kagome::host_api { outcome::result> getStorageNextKey( const common::Buffer &key) const; - std::optional calcStorageChangesRoot( - common::Hash256 parent) const; - runtime::WasmSpan clearPrefix(common::BufferView prefix, std::optional limit); diff --git a/test/core/host_api/storage_extension_test.cpp b/test/core/host_api/storage_extension_test.cpp index d88be45222..1034cba820 100644 --- a/test/core/host_api/storage_extension_test.cpp +++ b/test/core/host_api/storage_extension_test.cpp @@ -704,72 +704,3 @@ TEST_F(StorageExtensionTest, Blake2_256_TrieRootV1) { ASSERT_EQ(result, storage_extension_->ext_trie_blake2_256_root_version_1(dict_data)); } - -/** - * @given no :changes_trie key (which sets the changes trie config) in storage - * @when calling ext_storage_changes_root_version_1 - * @then none is returned, changes trie is empty - */ -TEST_F(StorageExtensionTest, ChangesRootEmpty) { - auto parent_hash = "123456"_hash256; - Buffer parent_hash_buf{gsl::span(parent_hash.data(), parent_hash.size())}; - PtrSize parent_root_ptr{1, Hash256::size()}; - EXPECT_CALL(*memory_, loadN(parent_root_ptr.ptr, Hash256::size())) - .WillOnce(Return(parent_hash_buf)); - - auto changes_trie_buf = kagome::common::Buffer{}.put(":changes_trie"); - EXPECT_CALL(*trie_batch_, tryGet(changes_trie_buf.view())) - .WillOnce(Return(std::nullopt)); - - WasmPointer result = 1984; - Buffer none_bytes{0}; - EXPECT_CALL(*memory_, storeBuffer(gsl::span(none_bytes))) - .WillOnce(Return(result)); - - auto res = storage_extension_->ext_storage_changes_root_version_1( - parent_root_ptr.combine()); - ASSERT_EQ(res, result); -} - -MATCHER_P(configsAreEqual, n, "") { - return (arg.digest_interval == n.digest_interval) - and (arg.digest_levels == n.digest_levels); -} - -/** - * @given existing :changes_trie key (which sets the changes trie config) in - * storage and there are accumulated changes - * @when calling ext_storage_changes_root_version_1 - * @then a valid trie root is returned - */ -TEST_F(StorageExtensionTest, ChangesRootNotEmpty) { - auto parent_hash = "123456"_hash256; - Buffer parent_hash_buf{gsl::span(parent_hash.data(), parent_hash.size())}; - PtrSize parent_root_ptr{1, Hash256::size()}; - EXPECT_CALL(*memory_, loadN(parent_root_ptr.ptr, Hash256::size())) - .WillOnce(Return(parent_hash_buf)); - - kagome::storage::changes_trie::ChangesTrieConfig config{.digest_interval = 0, - .digest_levels = 0}; - auto changes_trie_buf = kagome::common::Buffer{}.put(":changes_trie"); - EXPECT_CALL(*trie_batch_, tryGet(changes_trie_buf.view())) - .WillOnce(Return(Buffer(scale::encode(config).value()))); - - auto trie_hash = "deadbeef"_hash256; - auto enc_trie_hash = - scale::encode( - std::make_optional((gsl::span(trie_hash.data(), Hash256::size())))) - .value(); - EXPECT_CALL(*changes_tracker_, - constructChangesTrie(parent_hash, configsAreEqual(config))) - .WillOnce(Return(trie_hash)); - - WasmPointer result = 1984; - Buffer none_bytes{0}; - EXPECT_CALL(*memory_, storeBuffer(gsl::span(enc_trie_hash))) - .WillOnce(Return(result)); - - auto res = storage_extension_->ext_storage_changes_root_version_1( - parent_root_ptr.combine()); - ASSERT_EQ(res, result); -} From b537f0a6dd08df30320aa6cb97007b47a83dee9f Mon Sep 17 00:00:00 2001 From: turuslan Date: Fri, 29 Jul 2022 14:37:49 +0300 Subject: [PATCH 06/20] remove constructChangesTrie Signed-off-by: turuslan --- core/storage/changes_trie/changes_tracker.hpp | 7 ------- .../impl/storage_changes_tracker_impl.cpp | 14 -------------- .../impl/storage_changes_tracker_impl.hpp | 4 ---- .../storage/changes_trie/changes_tracker_test.cpp | 2 -- .../storage/changes_trie/changes_tracker_mock.hpp | 6 ------ 5 files changed, 33 deletions(-) diff --git a/core/storage/changes_trie/changes_tracker.hpp b/core/storage/changes_trie/changes_tracker.hpp index 17f081319d..501b972411 100644 --- a/core/storage/changes_trie/changes_tracker.hpp +++ b/core/storage/changes_trie/changes_tracker.hpp @@ -45,13 +45,6 @@ namespace kagome::storage::changes_trie { */ virtual outcome::result onRemove(common::BufferView extrinsic_index, const common::BufferView &key) = 0; - - /** - * Sinks accumulated changes for the latest registered block to the changes - * trie and returns its root hash - */ - virtual outcome::result constructChangesTrie( - const primitives::BlockHash &parent, const ChangesTrieConfig &conf) = 0; }; } // namespace kagome::storage::changes_trie diff --git a/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp b/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp index f59f1b8435..6643e7794f 100644 --- a/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp +++ b/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp @@ -115,18 +115,4 @@ namespace kagome::storage::changes_trie { } return outcome::success(); } - - outcome::result - StorageChangesTrackerImpl::constructChangesTrie( - const primitives::BlockHash &parent, const ChangesTrieConfig &conf) { - if (parent != parent_hash_) { - return Error::INVALID_PARENT_HASH; - } - OUTCOME_TRY( - trie, - ChangesTrie::buildFromChanges( - parent_number_, trie_factory_, codec_, extrinsics_changes_, conf)); - return trie->getHash(); - } - } // namespace kagome::storage::changes_trie diff --git a/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp b/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp index 826932f8dd..522a7f549c 100644 --- a/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp +++ b/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp @@ -43,10 +43,6 @@ namespace kagome::storage::changes_trie { outcome::result onRemove(common::BufferView extrinsic_index, const common::BufferView &key) override; - outcome::result constructChangesTrie( - const primitives::BlockHash &parent, - const ChangesTrieConfig &conf) override; - private: std::shared_ptr trie_factory_; std::shared_ptr codec_; diff --git a/test/core/storage/changes_trie/changes_tracker_test.cpp b/test/core/storage/changes_trie/changes_tracker_test.cpp index fe7ffd719c..bcb32eba3d 100644 --- a/test/core/storage/changes_trie/changes_tracker_test.cpp +++ b/test/core/storage/changes_trie/changes_tracker_test.cpp @@ -79,7 +79,5 @@ TEST(ChangesTrieTest, IntegrationWithOverlay) { auto repo = std::make_shared(); EXPECT_CALL(*repo, getNumberByHash(_)).WillRepeatedly(Return(42)); - EXPECT_OUTCOME_TRUE_1( - changes_tracker->constructChangesTrie("aaa"_hash256, {})); // THEN SUCCESS } diff --git a/test/mock/core/storage/changes_trie/changes_tracker_mock.hpp b/test/mock/core/storage/changes_trie/changes_tracker_mock.hpp index 3bbbdedbd3..4d7a91d591 100644 --- a/test/mock/core/storage/changes_trie/changes_tracker_mock.hpp +++ b/test/mock/core/storage/changes_trie/changes_tracker_mock.hpp @@ -41,12 +41,6 @@ namespace kagome::storage::changes_trie { onRemove, (common::BufferView ext_idx, const common::BufferView &key), (override)); - - MOCK_METHOD(outcome::result, - constructChangesTrie, - (const primitives::BlockHash &parent, - const ChangesTrieConfig &conf), - (override)); }; } // namespace kagome::storage::changes_trie From 063094f03fe2a5f793718ec31ee0c3da8aa5bd93 Mon Sep 17 00:00:00 2001 From: turuslan Date: Fri, 29 Jul 2022 14:41:44 +0300 Subject: [PATCH 07/20] remove ChangesTrie Signed-off-by: turuslan --- core/storage/changes_trie/CMakeLists.txt | 1 - .../changes_trie/impl/changes_trie.cpp | 66 --------------- .../changes_trie/impl/changes_trie.hpp | 84 ------------------- .../impl/storage_changes_tracker_impl.cpp | 1 - 4 files changed, 152 deletions(-) delete mode 100644 core/storage/changes_trie/impl/changes_trie.cpp delete mode 100644 core/storage/changes_trie/impl/changes_trie.hpp diff --git a/core/storage/changes_trie/CMakeLists.txt b/core/storage/changes_trie/CMakeLists.txt index c5881b2df5..8c4803a9ce 100644 --- a/core/storage/changes_trie/CMakeLists.txt +++ b/core/storage/changes_trie/CMakeLists.txt @@ -3,7 +3,6 @@ add_library(changes_tracker impl/storage_changes_tracker_impl.cpp - impl/changes_trie.cpp ) target_link_libraries(changes_tracker buffer diff --git a/core/storage/changes_trie/impl/changes_trie.cpp b/core/storage/changes_trie/impl/changes_trie.cpp deleted file mode 100644 index 889bec59ff..0000000000 --- a/core/storage/changes_trie/impl/changes_trie.cpp +++ /dev/null @@ -1,66 +0,0 @@ -/** - * Copyright Soramitsu Co., Ltd. All Rights Reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -#include "storage/changes_trie/impl/changes_trie.hpp" - -#include "scale/scale.hpp" - -namespace kagome::storage::changes_trie { - - outcome::result> ChangesTrie::buildFromChanges( - const primitives::BlockNumber &parent_number, - const std::shared_ptr &trie_factory, - std::shared_ptr codec, - const ExtrinsicsChanges &extinsics_changes, - const ChangesTrieConfig &config) { - BOOST_ASSERT(trie_factory != nullptr); - auto changes_storage = trie_factory->createEmpty(); - - for (auto &change : extinsics_changes) { - auto &key = change.first; - auto &changers = change.second; - auto current_number = parent_number + 1; - KeyIndexVariant keyIndex{ExtrinsicsChangesKey{{current_number, key}}}; - OUTCOME_TRY(key_enc, scale::encode(keyIndex)); - OUTCOME_TRY(value, scale::encode(changers)); - common::Buffer value_buf{std::move(value)}; - OUTCOME_TRY(changes_storage->put(common::Buffer{std::move(key_enc)}, - std::move(value_buf))); - } - - return std::unique_ptr{ - new ChangesTrie{std::move(changes_storage), std::move(codec)}}; - } - - ChangesTrie::ChangesTrie(std::unique_ptr trie, - std::shared_ptr codec) - : changes_trie_{std::move(trie)}, - codec_{std::move(codec)}, - logger_(log::createLogger("ChangesTrie", "changes_trie")) { - BOOST_ASSERT(changes_trie_ != nullptr); - BOOST_ASSERT(codec_ != nullptr); - } - - common::Hash256 ChangesTrie::getHash() const { - if (changes_trie_ == nullptr) { - return {}; - } - - auto root = changes_trie_->getRoot(); - if (root == nullptr) { - logger_->warn("Get root of empty changes trie"); - return codec_->hash256(common::Buffer{0}); - } - auto enc_res = codec_->encodeNode(*root); - if (enc_res.has_error()) { - logger_->error("Encoding Changes trie failed" - + enc_res.error().message()); - return {}; - } - auto hash_bytes = codec_->hash256(enc_res.value()); - return hash_bytes; - } - -} // namespace kagome::storage::changes_trie diff --git a/core/storage/changes_trie/impl/changes_trie.hpp b/core/storage/changes_trie/impl/changes_trie.hpp deleted file mode 100644 index bbb5e43a78..0000000000 --- a/core/storage/changes_trie/impl/changes_trie.hpp +++ /dev/null @@ -1,84 +0,0 @@ -/** - * Copyright Soramitsu Co., Ltd. All Rights Reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -#ifndef KAGOME_CORE_STORAGE_CHANGES_TRIE_IMPL_CHANGES_TRIE -#define KAGOME_CORE_STORAGE_CHANGES_TRIE_IMPL_CHANGES_TRIE - -#include - -#include - -#include "blockchain/block_header_repository.hpp" -#include "common/buffer.hpp" -#include "log/logger.hpp" -#include "primitives/common.hpp" -#include "primitives/extrinsic.hpp" -#include "storage/changes_trie/changes_trie_config.hpp" -#include "storage/trie/codec.hpp" -#include "storage/trie/polkadot_trie/polkadot_trie_factory.hpp" - -namespace kagome::storage::changes_trie { - - class ChangesTrie { - public: - using ExtrinsicsChanges = std::map, - std::less<>>; - - static outcome::result> buildFromChanges( - const primitives::BlockNumber &parent_block, - const std::shared_ptr &trie_factory, - std::shared_ptr codec, - const ExtrinsicsChanges &extinsics_changes, - const ChangesTrieConfig &config); - - common::Hash256 getHash() const; - - private: - ChangesTrie(std::unique_ptr trie, - std::shared_ptr codec); - - /** - * key of a changes trie - */ - struct KeyIndex { - // block in which the change occurred - primitives::BlockNumber block; - // changed key - common::Buffer key; - }; - // Mapping between storage key and extrinsics - struct ExtrinsicsChangesKey : public KeyIndex {}; - // Mapping between storage key and blocks - struct BlocksChangesKey : public KeyIndex {}; - // Mapping between storage key and child changes Trie - struct ChildChangesKey : public KeyIndex {}; - - // the key used for the Changes Trie must be the varying datatype, not the - // individual, appended KeyIndex. - using KeyIndexVariant = boost::variant, - ExtrinsicsChangesKey, - BlocksChangesKey, - ChildChangesKey>; - - template > - friend Stream &operator<<(Stream &s, const KeyIndex &b) { - return s << b.block << b.key; - } - template > - friend Stream &operator>>(Stream &s, KeyIndex &b) { - return s >> b.block >> b.key; - } - - std::unique_ptr changes_trie_; - std::shared_ptr codec_; - log::Logger logger_; - }; - -} // namespace kagome::storage::changes_trie - -#endif // KAGOME_CORE_STORAGE_CHANGES_TRIE_IMPL_CHANGES_TRIE diff --git a/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp b/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp index 6643e7794f..da56b963a8 100644 --- a/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp +++ b/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp @@ -2,7 +2,6 @@ #include "runtime/common/storage_code_provider.hpp" #include "scale/scale.hpp" -#include "storage/changes_trie/impl/changes_trie.hpp" #include "storage/predefined_keys.hpp" OUTCOME_CPP_DEFINE_CATEGORY(kagome::storage::changes_trie, From 440ee12e5f2b150474e0b911a481841edc7cea2f Mon Sep 17 00:00:00 2001 From: turuslan Date: Fri, 29 Jul 2022 14:43:30 +0300 Subject: [PATCH 08/20] remove ChangesTrieConfig Signed-off-by: turuslan --- core/primitives/digest.hpp | 1 - core/storage/changes_trie/changes_tracker.hpp | 1 - .../changes_trie/changes_trie_config.hpp | 70 ------------------- .../core/host_api/offchain_extension_test.cpp | 1 - test/core/host_api/storage_extension_test.cpp | 1 - .../changes_trie/changes_tracker_mock.hpp | 2 - 6 files changed, 76 deletions(-) delete mode 100644 core/storage/changes_trie/changes_trie_config.hpp diff --git a/core/primitives/digest.hpp b/core/primitives/digest.hpp index 528473b3b5..c399811c9d 100644 --- a/core/primitives/digest.hpp +++ b/core/primitives/digest.hpp @@ -13,7 +13,6 @@ #include "common/unused.hpp" #include "primitives/scheduled_change.hpp" #include "scale/scale.hpp" -#include "storage/changes_trie/changes_trie_config.hpp" namespace kagome::primitives { /// Consensus engine unique ID. diff --git a/core/storage/changes_trie/changes_tracker.hpp b/core/storage/changes_trie/changes_tracker.hpp index 501b972411..a149a9e81f 100644 --- a/core/storage/changes_trie/changes_tracker.hpp +++ b/core/storage/changes_trie/changes_tracker.hpp @@ -4,7 +4,6 @@ #include "common/buffer.hpp" #include "primitives/common.hpp" #include "primitives/extrinsic.hpp" -#include "storage/changes_trie/changes_trie_config.hpp" namespace kagome::storage::changes_trie { diff --git a/core/storage/changes_trie/changes_trie_config.hpp b/core/storage/changes_trie/changes_trie_config.hpp deleted file mode 100644 index 8377bcec77..0000000000 --- a/core/storage/changes_trie/changes_trie_config.hpp +++ /dev/null @@ -1,70 +0,0 @@ -/** - * Copyright Soramitsu Co., Ltd. All Rights Reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -#ifndef KAGOME_STORAGE_CHANGES_TRIE_CHANGES_TRIE_CONFIG -#define KAGOME_STORAGE_CHANGES_TRIE_CHANGES_TRIE_CONFIG - -#include - -namespace kagome::storage::changes_trie { - /// https://github.com/paritytech/substrate/blob/polkadot-v0.9.8/primitives/core/src/changes_trie.rs#L28 - struct ChangesTrieConfig { - /** - * The interval (in blocks) at which - * block mappings are created. Block mappings are not created when this is - * less or equal to 1. - */ - uint32_t digest_interval; - - /** - * Maximal number of levels in the - * hierarchy. 0 means that block mappings are not created at - * all. 1 means only the regular digest_interval block - * mappings are created. Any other level means that the block mappings are - * created every (digest_interval in power of digest_levels) block for - * each level in 1 to digest_levels. - */ - uint32_t digest_levels; - - bool operator==(const ChangesTrieConfig &rhs) const { - return digest_interval == rhs.digest_interval - and digest_levels == rhs.digest_levels; - } - - bool operator!=(const ChangesTrieConfig &rhs) const { - return !operator==(rhs); - } - }; - - /** - * @brief scale-encodes blob instance to stream - * @tparam Stream output stream type - * @tparam size blob size - * @param s output stream reference - * @param blob value to encode - * @return reference to stream - */ - template > - Stream &operator<<(Stream &s, const ChangesTrieConfig &config) { - return s << config.digest_interval << config.digest_levels; - } - - /** - * @brief decodes blob instance from stream - * @tparam Stream output stream type - * @tparam size blob size - * @param s input stream reference - * @param blob value to encode - * @return reference to stream - */ - template > - Stream &operator>>(Stream &s, ChangesTrieConfig &config) { - return s >> config.digest_interval >> config.digest_levels; - } -} // namespace kagome::storage::changes_trie - -#endif // KAGOME_STORAGE_CHANGES_TRIE_CHANGES_TRIE_CONFIG diff --git a/test/core/host_api/offchain_extension_test.cpp b/test/core/host_api/offchain_extension_test.cpp index 4addd641c0..59f4443c27 100644 --- a/test/core/host_api/offchain_extension_test.cpp +++ b/test/core/host_api/offchain_extension_test.cpp @@ -19,7 +19,6 @@ #include "offchain/types.hpp" #include "runtime/ptr_size.hpp" #include "scale/encode_append.hpp" -#include "storage/changes_trie/changes_trie_config.hpp" #include "testutil/literals.hpp" #include "testutil/outcome.hpp" #include "testutil/outcome/dummy_error.hpp" diff --git a/test/core/host_api/storage_extension_test.cpp b/test/core/host_api/storage_extension_test.cpp index 1034cba820..f487c0e47a 100644 --- a/test/core/host_api/storage_extension_test.cpp +++ b/test/core/host_api/storage_extension_test.cpp @@ -19,7 +19,6 @@ #include "mock/core/storage/trie/trie_batches_mock.hpp" #include "runtime/ptr_size.hpp" #include "scale/encode_append.hpp" -#include "storage/changes_trie/changes_trie_config.hpp" #include "storage/predefined_keys.hpp" #include "testutil/literals.hpp" #include "testutil/outcome.hpp" diff --git a/test/mock/core/storage/changes_trie/changes_tracker_mock.hpp b/test/mock/core/storage/changes_trie/changes_tracker_mock.hpp index 4d7a91d591..9c2d6e2044 100644 --- a/test/mock/core/storage/changes_trie/changes_tracker_mock.hpp +++ b/test/mock/core/storage/changes_trie/changes_tracker_mock.hpp @@ -16,8 +16,6 @@ namespace kagome::storage::changes_trie { public: MOCK_METHOD(void, setBlockHash, (const primitives::BlockHash &hash), ()); - MOCK_METHOD(void, setConfig, (const ChangesTrieConfig &conf), ()); - MOCK_METHOD(outcome::result, onBlockExecutionStart, (primitives::BlockHash new_parent_hash, From 0e2d972495a136be95aba6c365524468242d0012 Mon Sep 17 00:00:00 2001 From: turuslan Date: Fri, 29 Jul 2022 14:44:18 +0300 Subject: [PATCH 09/20] remove setBlockHash Signed-off-by: turuslan --- test/mock/core/storage/changes_trie/changes_tracker_mock.hpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/mock/core/storage/changes_trie/changes_tracker_mock.hpp b/test/mock/core/storage/changes_trie/changes_tracker_mock.hpp index 9c2d6e2044..2f8e30429b 100644 --- a/test/mock/core/storage/changes_trie/changes_tracker_mock.hpp +++ b/test/mock/core/storage/changes_trie/changes_tracker_mock.hpp @@ -14,8 +14,6 @@ namespace kagome::storage::changes_trie { class ChangesTrackerMock : public ChangesTracker { public: - MOCK_METHOD(void, setBlockHash, (const primitives::BlockHash &hash), ()); - MOCK_METHOD(outcome::result, onBlockExecutionStart, (primitives::BlockHash new_parent_hash, From 4920233dcfc70c215c206bef6d5615f690110f8b Mon Sep 17 00:00:00 2001 From: turuslan Date: Fri, 29 Jul 2022 15:04:53 +0300 Subject: [PATCH 10/20] remove extrinsics_changes_ Signed-off-by: turuslan --- .../impl/storage_changes_tracker_impl.cpp | 38 ++++--------------- .../impl/storage_changes_tracker_impl.hpp | 4 -- 2 files changed, 7 insertions(+), 35 deletions(-) diff --git a/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp b/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp index da56b963a8..2d1c3eb297 100644 --- a/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp +++ b/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp @@ -41,8 +41,6 @@ namespace kagome::storage::changes_trie { parent_number_ = new_parent_number; // new block -- new extrinsics actual_val_.clear(); - - extrinsics_changes_.clear(); new_entries_.clear(); return outcome::success(); } @@ -69,48 +67,26 @@ namespace kagome::storage::changes_trie { const common::BufferView &key, const common::BufferView &value, bool is_new_entry) { - auto change_it = extrinsics_changes_.find(key); - OUTCOME_TRY(idx, - scale::decode(extrinsic_index)); - - // if key was already changed in the same block, just add extrinsic to - // the changers list - if (change_it != extrinsics_changes_.end()) { - change_it->second.push_back(idx); + auto it = actual_val_.find(key); + if (it != actual_val_.end()) { + it->second.emplace(value); } else { - extrinsics_changes_.insert(std::make_pair(key, std::vector{idx})); + actual_val_.emplace(key, value); if (is_new_entry) { new_entries_.insert(common::Buffer{key}); } } - actual_val_.insert_or_assign(common::Buffer{key}, common::Buffer{value}); return outcome::success(); } outcome::result StorageChangesTrackerImpl::onRemove( common::BufferView extrinsic_index, const common::BufferView &key) { if (auto it = actual_val_.find(key); it != actual_val_.end()) { - it->second.reset(); - } - - auto change_it = extrinsics_changes_.find(key); - OUTCOME_TRY(idx, - scale::decode(extrinsic_index)); - - // if key was already changed in the same block, just add extrinsic to - // the changers list - if (change_it != extrinsics_changes_.end()) { - // if new entry, i.e. it doesn't exist in the persistent storage, then - // don't track it, because it's just temporary - if (auto i = new_entries_.find(key); i != new_entries_.end()) { - extrinsics_changes_.erase(change_it); - new_entries_.erase(i); + if (new_entries_.erase(it->first) != 0) { + actual_val_.erase(it); } else { - change_it->second.push_back(idx); + it->second.reset(); } - - } else { - extrinsics_changes_.insert(std::make_pair(key, std::vector{idx})); } return outcome::success(); } diff --git a/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp b/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp index 522a7f549c..b0aaec948b 100644 --- a/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp +++ b/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp @@ -47,10 +47,6 @@ namespace kagome::storage::changes_trie { std::shared_ptr trie_factory_; std::shared_ptr codec_; - std::map, - std::less<>> - extrinsics_changes_; std::set> new_entries_; // entries that do not yet exist in // the underlying storage From cd2aee91b481024040e1767b065750071330b5ea Mon Sep 17 00:00:00 2001 From: turuslan Date: Fri, 29 Jul 2022 15:05:41 +0300 Subject: [PATCH 11/20] remove parent_number_ Signed-off-by: turuslan --- core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp | 2 -- core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp | 1 - 2 files changed, 3 deletions(-) diff --git a/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp b/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp index 2d1c3eb297..29a7b56275 100644 --- a/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp +++ b/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp @@ -26,7 +26,6 @@ namespace kagome::storage::changes_trie { primitives::events::ChainSubscriptionEnginePtr chain_subscription_engine) : trie_factory_(std::move(trie_factory)), codec_(std::move(codec)), - parent_number_{std::numeric_limits::max()}, storage_subscription_engine_(std::move(storage_subscription_engine)), chain_subscription_engine_(std::move(chain_subscription_engine)), logger_{log::createLogger("Storage Changes Tracker", "changes_trie")} { @@ -38,7 +37,6 @@ namespace kagome::storage::changes_trie { primitives::BlockHash new_parent_hash, primitives::BlockNumber new_parent_number) { parent_hash_ = new_parent_hash; - parent_number_ = new_parent_number; // new block -- new extrinsics actual_val_.clear(); new_entries_.clear(); diff --git a/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp b/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp index b0aaec948b..308ecaba3c 100644 --- a/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp +++ b/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp @@ -54,7 +54,6 @@ namespace kagome::storage::changes_trie { actual_val_; primitives::BlockHash parent_hash_; - primitives::BlockNumber parent_number_; primitives::events::StorageSubscriptionEnginePtr storage_subscription_engine_; primitives::events::ChainSubscriptionEnginePtr chain_subscription_engine_; From b47d00dbc505525888443f6257cd236abeca4832 Mon Sep 17 00:00:00 2001 From: turuslan Date: Fri, 29 Jul 2022 15:09:37 +0300 Subject: [PATCH 12/20] remove trie_factory_ codec_ Signed-off-by: turuslan --- .../impl/storage_changes_tracker_impl.cpp | 11 ++--------- .../impl/storage_changes_tracker_impl.hpp | 19 ++++--------------- .../changes_trie/changes_tracker_test.cpp | 4 +--- 3 files changed, 7 insertions(+), 27 deletions(-) diff --git a/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp b/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp index 29a7b56275..d37d539ec4 100644 --- a/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp +++ b/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp @@ -19,19 +19,12 @@ OUTCOME_CPP_DEFINE_CATEGORY(kagome::storage::changes_trie, namespace kagome::storage::changes_trie { StorageChangesTrackerImpl::StorageChangesTrackerImpl( - std::shared_ptr trie_factory, - std::shared_ptr codec, primitives::events::StorageSubscriptionEnginePtr storage_subscription_engine, primitives::events::ChainSubscriptionEnginePtr chain_subscription_engine) - : trie_factory_(std::move(trie_factory)), - codec_(std::move(codec)), - storage_subscription_engine_(std::move(storage_subscription_engine)), + : storage_subscription_engine_(std::move(storage_subscription_engine)), chain_subscription_engine_(std::move(chain_subscription_engine)), - logger_{log::createLogger("Storage Changes Tracker", "changes_trie")} { - BOOST_ASSERT(trie_factory_ != nullptr); - BOOST_ASSERT(codec_ != nullptr); - } + logger_{log::createLogger("Storage Changes Tracker", "changes_trie")} {} outcome::result StorageChangesTrackerImpl::onBlockExecutionStart( primitives::BlockHash new_parent_hash, diff --git a/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp b/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp index 308ecaba3c..9a34c750ef 100644 --- a/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp +++ b/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp @@ -6,24 +6,16 @@ #include "log/logger.hpp" #include "primitives/event_types.hpp" -namespace kagome::storage::trie { - class Codec; - class PolkadotTrieFactory; -} // namespace kagome::storage::trie - namespace kagome::storage::changes_trie { class StorageChangesTrackerImpl : public ChangesTracker { public: enum class Error { INVALID_PARENT_HASH }; - StorageChangesTrackerImpl( - std::shared_ptr trie_factory, - std::shared_ptr codec, - primitives::events::StorageSubscriptionEnginePtr - storage_subscription_engine, - primitives::events::ChainSubscriptionEnginePtr - chain_subscription_engine); + StorageChangesTrackerImpl(primitives::events::StorageSubscriptionEnginePtr + storage_subscription_engine, + primitives::events::ChainSubscriptionEnginePtr + chain_subscription_engine); /** * Functor that returns the current extrinsic index, which is supposed to @@ -44,9 +36,6 @@ namespace kagome::storage::changes_trie { const common::BufferView &key) override; private: - std::shared_ptr trie_factory_; - std::shared_ptr codec_; - std::set> new_entries_; // entries that do not yet exist in // the underlying storage diff --git a/test/core/storage/changes_trie/changes_tracker_test.cpp b/test/core/storage/changes_trie/changes_tracker_test.cpp index bcb32eba3d..58b139336c 100644 --- a/test/core/storage/changes_trie/changes_tracker_test.cpp +++ b/test/core/storage/changes_trie/changes_tracker_test.cpp @@ -59,9 +59,7 @@ TEST(ChangesTrieTest, IntegrationWithOverlay) { std::make_shared(); auto chain_subscription_engine = std::make_shared(); std::shared_ptr changes_tracker = - std::make_shared(factory, - codec, - storage_subscription_engine, + std::make_shared(storage_subscription_engine, chain_subscription_engine); EXPECT_OUTCOME_TRUE_1( changes_tracker->onBlockExecutionStart("aaa"_hash256, 42)); From 841855417e89832ddda3519eeb0aadb44057fbf3 Mon Sep 17 00:00:00 2001 From: turuslan Date: Fri, 29 Jul 2022 15:24:23 +0300 Subject: [PATCH 13/20] remove extrinsic index Signed-off-by: turuslan --- core/storage/changes_trie/changes_tracker.hpp | 11 +++---- .../impl/storage_changes_tracker_impl.cpp | 15 +++------ .../impl/storage_changes_tracker_impl.hpp | 14 +++----- .../trie/impl/persistent_trie_batch_impl.cpp | 32 ++++--------------- .../trie/impl/persistent_trie_batch_impl.hpp | 3 -- .../changes_trie/changes_tracker_mock.hpp | 10 ++---- 6 files changed, 22 insertions(+), 63 deletions(-) diff --git a/core/storage/changes_trie/changes_tracker.hpp b/core/storage/changes_trie/changes_tracker.hpp index a149a9e81f..804cfed65a 100644 --- a/core/storage/changes_trie/changes_tracker.hpp +++ b/core/storage/changes_trie/changes_tracker.hpp @@ -3,7 +3,6 @@ #include "common/buffer.hpp" #include "primitives/common.hpp" -#include "primitives/extrinsic.hpp" namespace kagome::storage::changes_trie { @@ -29,10 +28,9 @@ namespace kagome::storage::changes_trie { * @arg new_entry states whether the entry is new, or just an update of a * present value */ - virtual outcome::result onPut(common::BufferView extrinsic_index, - const common::BufferView &key, - const common::BufferView &value, - bool new_entry) = 0; + virtual void onPut(const common::BufferView &key, + const common::BufferView &value, + bool new_entry) = 0; /** * Supposed to be called when a block is added to the block tree. @@ -42,8 +40,7 @@ namespace kagome::storage::changes_trie { /** * Supposed to be called when an entry is removed from the tracked storage */ - virtual outcome::result onRemove(common::BufferView extrinsic_index, - const common::BufferView &key) = 0; + virtual void onRemove(const common::BufferView &key) = 0; }; } // namespace kagome::storage::changes_trie diff --git a/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp b/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp index d37d539ec4..9acf7e68ad 100644 --- a/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp +++ b/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp @@ -1,7 +1,5 @@ #include "storage/changes_trie/impl/storage_changes_tracker_impl.hpp" -#include "runtime/common/storage_code_provider.hpp" -#include "scale/scale.hpp" #include "storage/predefined_keys.hpp" OUTCOME_CPP_DEFINE_CATEGORY(kagome::storage::changes_trie, @@ -53,11 +51,9 @@ namespace kagome::storage::changes_trie { } } - outcome::result StorageChangesTrackerImpl::onPut( - common::BufferView extrinsic_index, - const common::BufferView &key, - const common::BufferView &value, - bool is_new_entry) { + void StorageChangesTrackerImpl::onPut(const common::BufferView &key, + const common::BufferView &value, + bool is_new_entry) { auto it = actual_val_.find(key); if (it != actual_val_.end()) { it->second.emplace(value); @@ -67,11 +63,9 @@ namespace kagome::storage::changes_trie { new_entries_.insert(common::Buffer{key}); } } - return outcome::success(); } - outcome::result StorageChangesTrackerImpl::onRemove( - common::BufferView extrinsic_index, const common::BufferView &key) { + void StorageChangesTrackerImpl::onRemove(const common::BufferView &key) { if (auto it = actual_val_.find(key); it != actual_val_.end()) { if (new_entries_.erase(it->first) != 0) { actual_val_.erase(it); @@ -79,6 +73,5 @@ namespace kagome::storage::changes_trie { it->second.reset(); } } - return outcome::success(); } } // namespace kagome::storage::changes_trie diff --git a/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp b/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp index 9a34c750ef..9eea40f88f 100644 --- a/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp +++ b/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp @@ -17,23 +17,17 @@ namespace kagome::storage::changes_trie { primitives::events::ChainSubscriptionEnginePtr chain_subscription_engine); - /** - * Functor that returns the current extrinsic index, which is supposed to - * be stored in the state trie - */ ~StorageChangesTrackerImpl() override = default; outcome::result onBlockExecutionStart( primitives::BlockHash new_parent_hash, primitives::BlockNumber new_parent_number) override; - outcome::result onPut(common::BufferView extrinsic_index, - const common::BufferView &key, - const common::BufferView &value, - bool new_entry) override; + void onPut(const common::BufferView &key, + const common::BufferView &value, + bool new_entry) override; void onBlockAdded(const primitives::BlockHash &hash) override; - outcome::result onRemove(common::BufferView extrinsic_index, - const common::BufferView &key) override; + void onRemove(const common::BufferView &key) override; private: std::set> diff --git a/core/storage/trie/impl/persistent_trie_batch_impl.cpp b/core/storage/trie/impl/persistent_trie_batch_impl.cpp index eb99a7394d..ce00bd203b 100644 --- a/core/storage/trie/impl/persistent_trie_batch_impl.cpp +++ b/core/storage/trie/impl/persistent_trie_batch_impl.cpp @@ -7,8 +7,6 @@ #include -#include "scale/scale.hpp" -#include "storage/predefined_keys.hpp" #include "storage/trie/impl/topper_trie_batch_impl.hpp" #include "storage/trie/polkadot_trie/polkadot_trie_cursor_impl.hpp" #include "storage/trie/polkadot_trie/trie_error.hpp" @@ -25,10 +23,6 @@ OUTCOME_CPP_DEFINE_CATEGORY(kagome::storage::trie, } namespace kagome::storage::trie { - // sometimes there is no extrinsic index for a runtime call - const common::Buffer NO_EXTRINSIC_INDEX_VALUE{ - scale::encode(0xffffffff).value()}; - std::unique_ptr PersistentTrieBatchImpl::create( std::shared_ptr codec, std::shared_ptr serializer, @@ -58,15 +52,6 @@ namespace kagome::storage::trie { BOOST_ASSERT(trie_ != nullptr); } - outcome::result - PersistentTrieBatchImpl::getExtrinsicIndex() const { - OUTCOME_TRY(res, trie_->tryGet(kExtrinsicIndexKey)); - if (res) { - return res.value(); - } - return NO_EXTRINSIC_INDEX_VALUE; - } - outcome::result PersistentTrieBatchImpl::commit() { OUTCOME_TRY(root, serializer_->storeTrie(*trie_)); SL_TRACE_FUNC_CALL(logger_, root); @@ -104,11 +89,10 @@ namespace kagome::storage::trie { PersistentTrieBatchImpl::clearPrefix(const BufferView &prefix, std::optional limit) { SL_TRACE_VOID_FUNC_CALL(logger_, prefix); - OUTCOME_TRY(ext_idx, getExtrinsicIndex()); return trie_->clearPrefix( prefix, limit, [&](const auto &key, auto &&) -> outcome::result { if (changes_.has_value()) { - OUTCOME_TRY(changes_.value()->onRemove(ext_idx.get(), key)); + changes_.value()->onRemove(key); } return outcome::success(); }); @@ -120,10 +104,9 @@ namespace kagome::storage::trie { bool is_new_entry = not contains; auto res = trie_->put(key, value); if (res and changes_.has_value()) { - OUTCOME_TRY(ext_idx, getExtrinsicIndex()); SL_TRACE_VOID_FUNC_CALL(logger_, key, value); - OUTCOME_TRY( - changes_.value()->onPut(ext_idx.get(), key, value, is_new_entry)); + + changes_.value()->onPut(key, value, is_new_entry); } return res; } @@ -135,13 +118,12 @@ namespace kagome::storage::trie { } outcome::result PersistentTrieBatchImpl::remove(const BufferView &key) { - auto res = trie_->remove(key); - if (res and changes_.has_value()) { + OUTCOME_TRY(trie_->remove(key)); + if (changes_.has_value()) { SL_TRACE_VOID_FUNC_CALL(logger_, key); - OUTCOME_TRY(ext_idx, getExtrinsicIndex()); - OUTCOME_TRY(changes_.value()->onRemove(ext_idx.get(), key)); + changes_.value()->onRemove(key); } - return res; + return outcome::success(); } } // namespace kagome::storage::trie diff --git a/core/storage/trie/impl/persistent_trie_batch_impl.hpp b/core/storage/trie/impl/persistent_trie_batch_impl.hpp index 36f9bc8091..e09170764e 100644 --- a/core/storage/trie/impl/persistent_trie_batch_impl.hpp +++ b/core/storage/trie/impl/persistent_trie_batch_impl.hpp @@ -54,9 +54,6 @@ namespace kagome::storage::trie { std::optional> changes, std::shared_ptr trie); - // retrieves the current extrinsic index from the storage - outcome::result getExtrinsicIndex() const; - std::shared_ptr codec_; std::shared_ptr serializer_; std::optional> changes_; diff --git a/test/mock/core/storage/changes_trie/changes_tracker_mock.hpp b/test/mock/core/storage/changes_trie/changes_tracker_mock.hpp index 2f8e30429b..94c750c749 100644 --- a/test/mock/core/storage/changes_trie/changes_tracker_mock.hpp +++ b/test/mock/core/storage/changes_trie/changes_tracker_mock.hpp @@ -25,18 +25,14 @@ namespace kagome::storage::changes_trie { (const primitives::BlockHash &block_hash), (override)); - MOCK_METHOD(outcome::result, + MOCK_METHOD(void, onPut, - (common::BufferView ext_idx, - const common::BufferView &key, + (const common::BufferView &key, const common::BufferView &value, bool is_new_entry), (override)); - MOCK_METHOD(outcome::result, - onRemove, - (common::BufferView ext_idx, const common::BufferView &key), - (override)); + MOCK_METHOD(void, onRemove, (const common::BufferView &key), (override)); }; } // namespace kagome::storage::changes_trie From 2f1726d85a999e86d6e97bdb83e4c922b09665b1 Mon Sep 17 00:00:00 2001 From: turuslan Date: Fri, 29 Jul 2022 15:36:32 +0300 Subject: [PATCH 14/20] remove block number Signed-off-by: turuslan --- core/runtime/runtime_api/impl/core.cpp | 8 +++----- core/storage/changes_trie/changes_tracker.hpp | 3 +-- .../changes_trie/impl/storage_changes_tracker_impl.cpp | 3 +-- .../changes_trie/impl/storage_changes_tracker_impl.hpp | 3 +-- test/core/runtime/wavm/core_integration_test.cpp | 8 +++----- test/core/storage/changes_trie/changes_tracker_test.cpp | 3 +-- .../core/storage/changes_trie/changes_tracker_mock.hpp | 3 +-- 7 files changed, 11 insertions(+), 20 deletions(-) diff --git a/core/runtime/runtime_api/impl/core.cpp b/core/runtime/runtime_api/impl/core.cpp index ee877f7d6d..3ccf4bb9df 100644 --- a/core/runtime/runtime_api/impl/core.cpp +++ b/core/runtime/runtime_api/impl/core.cpp @@ -36,8 +36,8 @@ namespace kagome::runtime { const primitives::Block &block) { OUTCOME_TRY(parent, header_repo_->getBlockHeader(block.header.parent_hash)); BOOST_ASSERT(parent.number == block.header.number - 1); - OUTCOME_TRY(changes_tracker_->onBlockExecutionStart( - block.header.parent_hash, parent.number)); + OUTCOME_TRY( + changes_tracker_->onBlockExecutionStart(block.header.parent_hash)); OUTCOME_TRY(executor_->persistentCallAt( block.header.parent_hash, "Core_execute_block", block)); return outcome::success(); @@ -45,9 +45,7 @@ namespace kagome::runtime { outcome::result CoreImpl::initialize_block( const primitives::BlockHeader &header) { - OUTCOME_TRY(changes_tracker_->onBlockExecutionStart( - header.parent_hash, - header.number - 1)); // parent's number + OUTCOME_TRY(changes_tracker_->onBlockExecutionStart(header.parent_hash)); const auto res = executor_->persistentCallAt( header.parent_hash, "Core_initialize_block", header); if (res.has_value()) { diff --git a/core/storage/changes_trie/changes_tracker.hpp b/core/storage/changes_trie/changes_tracker.hpp index 804cfed65a..110710cc29 100644 --- a/core/storage/changes_trie/changes_tracker.hpp +++ b/core/storage/changes_trie/changes_tracker.hpp @@ -20,8 +20,7 @@ namespace kagome::storage::changes_trie { * Supposed to be called when a block execution starts */ virtual outcome::result onBlockExecutionStart( - primitives::BlockHash new_parent_hash, - primitives::BlockNumber new_parent_number) = 0; + primitives::BlockHash new_parent_hash) = 0; /** * Supposed to be called when an entry is put into the tracked storage diff --git a/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp b/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp index 9acf7e68ad..7a8cd9ac36 100644 --- a/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp +++ b/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp @@ -25,8 +25,7 @@ namespace kagome::storage::changes_trie { logger_{log::createLogger("Storage Changes Tracker", "changes_trie")} {} outcome::result StorageChangesTrackerImpl::onBlockExecutionStart( - primitives::BlockHash new_parent_hash, - primitives::BlockNumber new_parent_number) { + primitives::BlockHash new_parent_hash) { parent_hash_ = new_parent_hash; // new block -- new extrinsics actual_val_.clear(); diff --git a/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp b/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp index 9eea40f88f..e3cd892323 100644 --- a/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp +++ b/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp @@ -20,8 +20,7 @@ namespace kagome::storage::changes_trie { ~StorageChangesTrackerImpl() override = default; outcome::result onBlockExecutionStart( - primitives::BlockHash new_parent_hash, - primitives::BlockNumber new_parent_number) override; + primitives::BlockHash new_parent_hash) override; void onPut(const common::BufferView &key, const common::BufferView &value, diff --git a/test/core/runtime/wavm/core_integration_test.cpp b/test/core/runtime/wavm/core_integration_test.cpp index 021cccbf23..384ed5f0b7 100644 --- a/test/core/runtime/wavm/core_integration_test.cpp +++ b/test/core/runtime/wavm/core_integration_test.cpp @@ -66,9 +66,8 @@ TEST_F(CoreTest, DISABLED_VersionTest) { */ TEST_F(CoreTest, DISABLED_ExecuteBlockTest) { auto block = createBlock("block_hash"_hash256, 42); - EXPECT_CALL( - *changes_tracker_, - onBlockExecutionStart(block.header.parent_hash, block.header.number - 1)) + EXPECT_CALL(*changes_tracker_, + onBlockExecutionStart(block.header.parent_hash)) .WillOnce(Return(outcome::success())); ASSERT_TRUE(core_->execute_block(block)); @@ -81,8 +80,7 @@ TEST_F(CoreTest, DISABLED_ExecuteBlockTest) { */ TEST_F(CoreTest, DISABLED_InitializeBlockTest) { auto header = createBlockHeader("block_hash"_hash256, 42); - EXPECT_CALL(*changes_tracker_, - onBlockExecutionStart(header.parent_hash, header.number - 1)) + EXPECT_CALL(*changes_tracker_, onBlockExecutionStart(header.parent_hash)) .WillOnce(Return(outcome::success())); ASSERT_TRUE(core_->initialize_block(header)); diff --git a/test/core/storage/changes_trie/changes_tracker_test.cpp b/test/core/storage/changes_trie/changes_tracker_test.cpp index 58b139336c..2f115ab50d 100644 --- a/test/core/storage/changes_trie/changes_tracker_test.cpp +++ b/test/core/storage/changes_trie/changes_tracker_test.cpp @@ -61,8 +61,7 @@ TEST(ChangesTrieTest, IntegrationWithOverlay) { std::shared_ptr changes_tracker = std::make_shared(storage_subscription_engine, chain_subscription_engine); - EXPECT_OUTCOME_TRUE_1( - changes_tracker->onBlockExecutionStart("aaa"_hash256, 42)); + EXPECT_OUTCOME_TRUE_1(changes_tracker->onBlockExecutionStart("aaa"_hash256)); auto batch = PersistentTrieBatchImpl::create( codec, serializer, diff --git a/test/mock/core/storage/changes_trie/changes_tracker_mock.hpp b/test/mock/core/storage/changes_trie/changes_tracker_mock.hpp index 94c750c749..537ab78a37 100644 --- a/test/mock/core/storage/changes_trie/changes_tracker_mock.hpp +++ b/test/mock/core/storage/changes_trie/changes_tracker_mock.hpp @@ -16,8 +16,7 @@ namespace kagome::storage::changes_trie { public: MOCK_METHOD(outcome::result, onBlockExecutionStart, - (primitives::BlockHash new_parent_hash, - primitives::BlockNumber new_parent_number), + (primitives::BlockHash new_parent_hash), (override)); MOCK_METHOD(void, From af4270ed526fea08e5498a0d03466cc4e868ce88 Mon Sep 17 00:00:00 2001 From: turuslan Date: Fri, 29 Jul 2022 15:38:39 +0300 Subject: [PATCH 15/20] scale workaround Signed-off-by: turuslan --- core/host_api/impl/storage_extension.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/host_api/impl/storage_extension.cpp b/core/host_api/impl/storage_extension.cpp index fe4167f12c..1195ec9adb 100644 --- a/core/host_api/impl/storage_extension.cpp +++ b/core/host_api/impl/storage_extension.cpp @@ -254,7 +254,7 @@ namespace kagome::host_api { runtime::WasmSpan parent_hash_data) { auto &memory = memory_provider_->getCurrentMemory()->get(); // https://github.com/paritytech/substrate/pull/10080 - return memory.storeBuffer(scale::encode(std::nullopt).value()); + return memory.storeBuffer(scale::encode(std::optional()).value()); } runtime::WasmSpan StorageExtension::ext_storage_next_key_version_1( From b21e4e6e4b5b8a92505ee0997543998126f99b6e Mon Sep 17 00:00:00 2001 From: turuslan Date: Fri, 29 Jul 2022 15:42:09 +0300 Subject: [PATCH 16/20] remove error Signed-off-by: turuslan --- .../impl/storage_changes_tracker_impl.cpp | 12 ------------ .../impl/storage_changes_tracker_impl.hpp | 5 ----- 2 files changed, 17 deletions(-) diff --git a/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp b/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp index 7a8cd9ac36..53607462c8 100644 --- a/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp +++ b/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp @@ -2,18 +2,6 @@ #include "storage/predefined_keys.hpp" -OUTCOME_CPP_DEFINE_CATEGORY(kagome::storage::changes_trie, - StorageChangesTrackerImpl::Error, - e) { - using E = kagome::storage::changes_trie::StorageChangesTrackerImpl::Error; - switch (e) { - case E::INVALID_PARENT_HASH: - return "The supplied parent hash doesn't match the one of the current " - "block"; - } - return "Unknown error"; -} - namespace kagome::storage::changes_trie { StorageChangesTrackerImpl::StorageChangesTrackerImpl( diff --git a/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp b/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp index e3cd892323..c6f315243f 100644 --- a/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp +++ b/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp @@ -10,8 +10,6 @@ namespace kagome::storage::changes_trie { class StorageChangesTrackerImpl : public ChangesTracker { public: - enum class Error { INVALID_PARENT_HASH }; - StorageChangesTrackerImpl(primitives::events::StorageSubscriptionEnginePtr storage_subscription_engine, primitives::events::ChainSubscriptionEnginePtr @@ -44,7 +42,4 @@ namespace kagome::storage::changes_trie { } // namespace kagome::storage::changes_trie -OUTCOME_HPP_DECLARE_ERROR(kagome::storage::changes_trie, - StorageChangesTrackerImpl::Error); - #endif // KAGOME_STORAGE_CHANGES_TRIE_STORAGE_CHANGES_TRACKER_IMPL From 3122351c86fa2be21138806bb894a0eb6ead0658 Mon Sep 17 00:00:00 2001 From: turuslan Date: Fri, 29 Jul 2022 15:44:28 +0300 Subject: [PATCH 17/20] remove outcome Signed-off-by: turuslan --- core/runtime/runtime_api/impl/core.cpp | 5 ++--- core/storage/changes_trie/changes_tracker.hpp | 2 +- .../changes_trie/impl/storage_changes_tracker_impl.cpp | 3 +-- .../changes_trie/impl/storage_changes_tracker_impl.hpp | 3 +-- test/core/runtime/wavm/core_integration_test.cpp | 4 ++-- test/core/storage/changes_trie/changes_tracker_test.cpp | 2 +- test/mock/core/storage/changes_trie/changes_tracker_mock.hpp | 2 +- 7 files changed, 9 insertions(+), 12 deletions(-) diff --git a/core/runtime/runtime_api/impl/core.cpp b/core/runtime/runtime_api/impl/core.cpp index 3ccf4bb9df..bffd3bc176 100644 --- a/core/runtime/runtime_api/impl/core.cpp +++ b/core/runtime/runtime_api/impl/core.cpp @@ -36,8 +36,7 @@ namespace kagome::runtime { const primitives::Block &block) { OUTCOME_TRY(parent, header_repo_->getBlockHeader(block.header.parent_hash)); BOOST_ASSERT(parent.number == block.header.number - 1); - OUTCOME_TRY( - changes_tracker_->onBlockExecutionStart(block.header.parent_hash)); + changes_tracker_->onBlockExecutionStart(block.header.parent_hash); OUTCOME_TRY(executor_->persistentCallAt( block.header.parent_hash, "Core_execute_block", block)); return outcome::success(); @@ -45,7 +44,7 @@ namespace kagome::runtime { outcome::result CoreImpl::initialize_block( const primitives::BlockHeader &header) { - OUTCOME_TRY(changes_tracker_->onBlockExecutionStart(header.parent_hash)); + changes_tracker_->onBlockExecutionStart(header.parent_hash); const auto res = executor_->persistentCallAt( header.parent_hash, "Core_initialize_block", header); if (res.has_value()) { diff --git a/core/storage/changes_trie/changes_tracker.hpp b/core/storage/changes_trie/changes_tracker.hpp index 110710cc29..91605bfd99 100644 --- a/core/storage/changes_trie/changes_tracker.hpp +++ b/core/storage/changes_trie/changes_tracker.hpp @@ -19,7 +19,7 @@ namespace kagome::storage::changes_trie { /** * Supposed to be called when a block execution starts */ - virtual outcome::result onBlockExecutionStart( + virtual void onBlockExecutionStart( primitives::BlockHash new_parent_hash) = 0; /** diff --git a/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp b/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp index 53607462c8..2a903aa4aa 100644 --- a/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp +++ b/core/storage/changes_trie/impl/storage_changes_tracker_impl.cpp @@ -12,13 +12,12 @@ namespace kagome::storage::changes_trie { chain_subscription_engine_(std::move(chain_subscription_engine)), logger_{log::createLogger("Storage Changes Tracker", "changes_trie")} {} - outcome::result StorageChangesTrackerImpl::onBlockExecutionStart( + void StorageChangesTrackerImpl::onBlockExecutionStart( primitives::BlockHash new_parent_hash) { parent_hash_ = new_parent_hash; // new block -- new extrinsics actual_val_.clear(); new_entries_.clear(); - return outcome::success(); } void StorageChangesTrackerImpl::onBlockAdded( diff --git a/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp b/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp index c6f315243f..eb7d4be187 100644 --- a/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp +++ b/core/storage/changes_trie/impl/storage_changes_tracker_impl.hpp @@ -17,8 +17,7 @@ namespace kagome::storage::changes_trie { ~StorageChangesTrackerImpl() override = default; - outcome::result onBlockExecutionStart( - primitives::BlockHash new_parent_hash) override; + void onBlockExecutionStart(primitives::BlockHash new_parent_hash) override; void onPut(const common::BufferView &key, const common::BufferView &value, diff --git a/test/core/runtime/wavm/core_integration_test.cpp b/test/core/runtime/wavm/core_integration_test.cpp index 384ed5f0b7..df4daa2623 100644 --- a/test/core/runtime/wavm/core_integration_test.cpp +++ b/test/core/runtime/wavm/core_integration_test.cpp @@ -68,7 +68,7 @@ TEST_F(CoreTest, DISABLED_ExecuteBlockTest) { auto block = createBlock("block_hash"_hash256, 42); EXPECT_CALL(*changes_tracker_, onBlockExecutionStart(block.header.parent_hash)) - .WillOnce(Return(outcome::success())); + .WillOnce(Return()); ASSERT_TRUE(core_->execute_block(block)); } @@ -81,7 +81,7 @@ TEST_F(CoreTest, DISABLED_ExecuteBlockTest) { TEST_F(CoreTest, DISABLED_InitializeBlockTest) { auto header = createBlockHeader("block_hash"_hash256, 42); EXPECT_CALL(*changes_tracker_, onBlockExecutionStart(header.parent_hash)) - .WillOnce(Return(outcome::success())); + .WillOnce(Return()); ASSERT_TRUE(core_->initialize_block(header)); } diff --git a/test/core/storage/changes_trie/changes_tracker_test.cpp b/test/core/storage/changes_trie/changes_tracker_test.cpp index 2f115ab50d..18db45989c 100644 --- a/test/core/storage/changes_trie/changes_tracker_test.cpp +++ b/test/core/storage/changes_trie/changes_tracker_test.cpp @@ -61,7 +61,7 @@ TEST(ChangesTrieTest, IntegrationWithOverlay) { std::shared_ptr changes_tracker = std::make_shared(storage_subscription_engine, chain_subscription_engine); - EXPECT_OUTCOME_TRUE_1(changes_tracker->onBlockExecutionStart("aaa"_hash256)); + changes_tracker->onBlockExecutionStart("aaa"_hash256); auto batch = PersistentTrieBatchImpl::create( codec, serializer, diff --git a/test/mock/core/storage/changes_trie/changes_tracker_mock.hpp b/test/mock/core/storage/changes_trie/changes_tracker_mock.hpp index 537ab78a37..df656ee505 100644 --- a/test/mock/core/storage/changes_trie/changes_tracker_mock.hpp +++ b/test/mock/core/storage/changes_trie/changes_tracker_mock.hpp @@ -14,7 +14,7 @@ namespace kagome::storage::changes_trie { class ChangesTrackerMock : public ChangesTracker { public: - MOCK_METHOD(outcome::result, + MOCK_METHOD(void, onBlockExecutionStart, (primitives::BlockHash new_parent_hash), (override)); From 63736ff61ca60982f355c29ceb4a6706215dcd3a Mon Sep 17 00:00:00 2001 From: turuslan Date: Fri, 29 Jul 2022 15:54:24 +0300 Subject: [PATCH 18/20] remove assert Signed-off-by: turuslan --- core/consensus/grandpa/impl/voting_round_impl.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core/consensus/grandpa/impl/voting_round_impl.cpp b/core/consensus/grandpa/impl/voting_round_impl.cpp index 51f7ae7423..cdd0055f60 100644 --- a/core/consensus/grandpa/impl/voting_round_impl.cpp +++ b/core/consensus/grandpa/impl/voting_round_impl.cpp @@ -1229,7 +1229,9 @@ namespace kagome::consensus::grandpa { return false; } - BOOST_ASSERT(prevote_ghost_.has_value()); + if (not prevote_ghost_) { + return false; + } const auto &prevote_ghost = prevote_ghost_.value(); // anything new finalized? finalized blocks are those which have both From c2b2c8c82299ee2ec2f953cee50d0f9989603f7f Mon Sep 17 00:00:00 2001 From: turuslan Date: Fri, 29 Jul 2022 17:08:12 +0300 Subject: [PATCH 19/20] fix external project Signed-off-by: turuslan --- test/external-project-test/src/main.cpp | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/test/external-project-test/src/main.cpp b/test/external-project-test/src/main.cpp index ebd2b5ca9b..c41b76714a 100644 --- a/test/external-project-test/src/main.cpp +++ b/test/external-project-test/src/main.cpp @@ -75,7 +75,7 @@ int main() { leveldb::Options db_options{}; db_options.create_if_missing = true; - std::shared_ptr database = + std::shared_ptr database = kagome::storage::LevelDB::create("/tmp/kagome_tmp_db", db_options) .value(); auto hasher = std::make_shared(); @@ -108,15 +108,12 @@ int main() { auto changes_tracker = std::make_shared< kagome::storage::changes_trie::StorageChangesTrackerImpl>( - trie_factory, - codec, - storage_subscription_engine, - chain_subscription_engine); + storage_subscription_engine, chain_subscription_engine); - auto trie_storage = - std::shared_ptr(kagome::storage::trie::TrieStorageImpl::createEmpty( - trie_factory, codec, serializer, changes_tracker) - .value()); + std::shared_ptr trie_storage = + kagome::storage::trie::TrieStorageImpl::createEmpty( + trie_factory, codec, serializer, changes_tracker) + .value(); auto batch = trie_storage->getPersistentBatchAt(serializer->getEmptyRootHash()) @@ -169,9 +166,9 @@ int main() { std::make_shared(ed25519_provider); auto sr_suite = std::make_shared(sr25519_provider); - auto key_fs = std::shared_ptr( + std::shared_ptr key_fs = kagome::crypto::KeyFileStorage::createAt("/tmp/kagome_tmp_key_storage") - .value()); + .value(); auto crypto_store = std::make_shared( ecdsa_suite, ed_suite, sr_suite, bip39_provider, key_fs); From cd25bdc13113d4ee8aea23b5add4d18c0d2211d2 Mon Sep 17 00:00:00 2001 From: turuslan Date: Mon, 1 Aug 2022 12:33:12 +0300 Subject: [PATCH 20/20] remove unused Signed-off-by: turuslan --- core/host_api/impl/host_api_factory_impl.cpp | 4 ---- core/host_api/impl/host_api_factory_impl.hpp | 3 --- core/host_api/impl/host_api_impl.cpp | 3 +-- core/host_api/impl/host_api_impl.hpp | 1 - core/host_api/impl/storage_extension.cpp | 5 +---- core/host_api/impl/storage_extension.hpp | 5 +---- test/core/host_api/storage_extension_test.cpp | 8 ++------ test/core/runtime/runtime_test_base.hpp | 1 - test/core/runtime/wavm/wasm_executor_test.cpp | 1 - test/external-project-test/src/main.cpp | 1 - 10 files changed, 5 insertions(+), 27 deletions(-) diff --git a/core/host_api/impl/host_api_factory_impl.cpp b/core/host_api/impl/host_api_factory_impl.cpp index 20b09d2c8e..7929ba4565 100644 --- a/core/host_api/impl/host_api_factory_impl.cpp +++ b/core/host_api/impl/host_api_factory_impl.cpp @@ -11,7 +11,6 @@ namespace kagome::host_api { HostApiFactoryImpl::HostApiFactoryImpl( const OffchainExtensionConfig &offchain_config, - std::shared_ptr tracker, std::shared_ptr sr25519_provider, std::shared_ptr ecdsa_provider, std::shared_ptr ed25519_provider, @@ -23,7 +22,6 @@ namespace kagome::host_api { offchain_persistent_storage, std::shared_ptr offchain_worker_pool) : offchain_config_(offchain_config), - changes_tracker_{std::move(tracker)}, sr25519_provider_(std::move(sr25519_provider)), ecdsa_provider_(std::move(ecdsa_provider)), ed25519_provider_(std::move(ed25519_provider)), @@ -33,7 +31,6 @@ namespace kagome::host_api { bip39_provider_(std::move(bip39_provider)), offchain_persistent_storage_(std::move(offchain_persistent_storage)), offchain_worker_pool_(std::move(offchain_worker_pool)) { - BOOST_ASSERT(changes_tracker_ != nullptr); BOOST_ASSERT(sr25519_provider_ != nullptr); BOOST_ASSERT(ed25519_provider_ != nullptr); BOOST_ASSERT(secp256k1_provider_ != nullptr); @@ -52,7 +49,6 @@ namespace kagome::host_api { memory_provider, core_provider, storage_provider, - changes_tracker_, sr25519_provider_, ecdsa_provider_, ed25519_provider_, diff --git a/core/host_api/impl/host_api_factory_impl.hpp b/core/host_api/impl/host_api_factory_impl.hpp index 71afb34616..0dad9ea206 100644 --- a/core/host_api/impl/host_api_factory_impl.hpp +++ b/core/host_api/impl/host_api_factory_impl.hpp @@ -16,7 +16,6 @@ #include "crypto/secp256k1_provider.hpp" #include "crypto/sr25519_provider.hpp" #include "host_api/impl/offchain_extension.hpp" -#include "storage/changes_trie/changes_tracker.hpp" namespace kagome::offchain { class OffchainPersistentStorage; @@ -31,7 +30,6 @@ namespace kagome::host_api { HostApiFactoryImpl( const OffchainExtensionConfig &offchain_config, - std::shared_ptr tracker, std::shared_ptr sr25519_provider, std::shared_ptr ecdsa_provider, std::shared_ptr ed25519_provider, @@ -51,7 +49,6 @@ namespace kagome::host_api { private: OffchainExtensionConfig offchain_config_; - std::shared_ptr changes_tracker_; std::shared_ptr sr25519_provider_; std::shared_ptr ecdsa_provider_; std::shared_ptr ed25519_provider_; diff --git a/core/host_api/impl/host_api_impl.cpp b/core/host_api/impl/host_api_impl.cpp index 3e806e65f8..c83e60f339 100644 --- a/core/host_api/impl/host_api_impl.cpp +++ b/core/host_api/impl/host_api_impl.cpp @@ -23,7 +23,6 @@ namespace kagome::host_api { std::shared_ptr memory_provider, std::shared_ptr core_provider, std::shared_ptr storage_provider, - std::shared_ptr tracker, std::shared_ptr sr25519_provider, std::shared_ptr ecdsa_provider, std::shared_ptr ed25519_provider, @@ -56,7 +55,7 @@ namespace kagome::host_api { hasher, memory_provider_, std::move(core_provider)}, - storage_ext_(storage_provider_, memory_provider_, std::move(tracker)), + storage_ext_(storage_provider_, memory_provider_), child_storage_ext_(storage_provider_, memory_provider_), offchain_ext_(offchain_config, memory_provider_, diff --git a/core/host_api/impl/host_api_impl.hpp b/core/host_api/impl/host_api_impl.hpp index 32ea77117a..aa40b074f2 100644 --- a/core/host_api/impl/host_api_impl.hpp +++ b/core/host_api/impl/host_api_impl.hpp @@ -38,7 +38,6 @@ namespace kagome::host_api { std::shared_ptr memory_provider, std::shared_ptr core_provider, std::shared_ptr storage_provider, - std::shared_ptr tracker, std::shared_ptr sr25519_provider, std::shared_ptr ecdsa_provider, std::shared_ptr ed25519_provider, diff --git a/core/host_api/impl/storage_extension.cpp b/core/host_api/impl/storage_extension.cpp index 1195ec9adb..5c5d9900b3 100644 --- a/core/host_api/impl/storage_extension.cpp +++ b/core/host_api/impl/storage_extension.cpp @@ -40,15 +40,12 @@ namespace { namespace kagome::host_api { StorageExtension::StorageExtension( std::shared_ptr storage_provider, - std::shared_ptr memory_provider, - std::shared_ptr changes_tracker) + std::shared_ptr memory_provider) : storage_provider_(std::move(storage_provider)), memory_provider_(std::move(memory_provider)), - changes_tracker_{std::move(changes_tracker)}, logger_{log::createLogger("StorageExtension", "storage_extension")} { BOOST_ASSERT_MSG(storage_provider_ != nullptr, "storage batch is nullptr"); BOOST_ASSERT_MSG(memory_provider_ != nullptr, "memory provider is nullptr"); - BOOST_ASSERT_MSG(changes_tracker_ != nullptr, "changes tracker is nullptr"); } void StorageExtension::reset() { diff --git a/core/host_api/impl/storage_extension.hpp b/core/host_api/impl/storage_extension.hpp index ca86edbe0a..98014ef8c8 100644 --- a/core/host_api/impl/storage_extension.hpp +++ b/core/host_api/impl/storage_extension.hpp @@ -10,7 +10,6 @@ #include "log/logger.hpp" #include "runtime/types.hpp" -#include "storage/changes_trie/changes_tracker.hpp" #include "storage/trie/serialization/polkadot_codec.hpp" namespace kagome::runtime { @@ -26,8 +25,7 @@ namespace kagome::host_api { public: StorageExtension( std::shared_ptr storage_provider, - std::shared_ptr memory_provider, - std::shared_ptr changes_tracker); + std::shared_ptr memory_provider); void reset(); @@ -174,7 +172,6 @@ namespace kagome::host_api { std::shared_ptr storage_provider_; std::shared_ptr memory_provider_; - std::shared_ptr changes_tracker_; storage::trie::PolkadotCodec codec_; log::Logger logger_; diff --git a/test/core/host_api/storage_extension_test.cpp b/test/core/host_api/storage_extension_test.cpp index f487c0e47a..e51b2ba683 100644 --- a/test/core/host_api/storage_extension_test.cpp +++ b/test/core/host_api/storage_extension_test.cpp @@ -14,7 +14,6 @@ #include "mock/core/runtime/memory_mock.hpp" #include "mock/core/runtime/memory_provider_mock.hpp" #include "mock/core/runtime/trie_storage_provider_mock.hpp" -#include "mock/core/storage/changes_trie/changes_tracker_mock.hpp" #include "mock/core/storage/trie/polkadot_trie_cursor_mock.h" #include "mock/core/storage/trie/trie_batches_mock.hpp" #include "runtime/ptr_size.hpp" @@ -38,7 +37,6 @@ using kagome::runtime::WasmOffset; using kagome::runtime::WasmPointer; using kagome::runtime::WasmSize; using kagome::runtime::WasmSpan; -using kagome::storage::changes_trie::ChangesTrackerMock; using kagome::storage::trie::EphemeralTrieBatchMock; using kagome::storage::trie::PersistentTrieBatchMock; using kagome::storage::trie::PolkadotCodec; @@ -71,9 +69,8 @@ class StorageExtensionTest : public ::testing::Test { EXPECT_CALL(*memory_provider_, getCurrentMemory()) .WillRepeatedly( Return(std::optional>(*memory_))); - changes_tracker_ = std::make_shared(); - storage_extension_ = std::make_shared( - storage_provider_, memory_provider_, changes_tracker_); + storage_extension_ = + std::make_shared(storage_provider_, memory_provider_); } protected: @@ -82,7 +79,6 @@ class StorageExtensionTest : public ::testing::Test { std::shared_ptr memory_; std::shared_ptr memory_provider_; std::shared_ptr storage_extension_; - std::shared_ptr changes_tracker_; PolkadotCodec codec_; constexpr static uint32_t kU32Max = std::numeric_limits::max(); diff --git a/test/core/runtime/runtime_test_base.hpp b/test/core/runtime/runtime_test_base.hpp index a92f298466..7fc6152fc5 100644 --- a/test/core/runtime/runtime_test_base.hpp +++ b/test/core/runtime/runtime_test_base.hpp @@ -99,7 +99,6 @@ class RuntimeTestBase : public ::testing::Test { host_api_factory_ = std::make_shared( kagome::host_api::OffchainExtensionConfig{}, - changes_tracker_, sr25519_provider, ecdsa_provider, ed25519_provider, diff --git a/test/core/runtime/wavm/wasm_executor_test.cpp b/test/core/runtime/wavm/wasm_executor_test.cpp index 49f3649930..482aa7bb3b 100644 --- a/test/core/runtime/wavm/wasm_executor_test.cpp +++ b/test/core/runtime/wavm/wasm_executor_test.cpp @@ -143,7 +143,6 @@ class WasmExecutorTest : public ::testing::Test { auto host_api_factory = std::make_shared( kagome::host_api::OffchainExtensionConfig{}, - std::make_shared(), sr25519_provider, ecdsa_provider, ed25519_provider, diff --git a/test/external-project-test/src/main.cpp b/test/external-project-test/src/main.cpp index c41b76714a..255a052016 100644 --- a/test/external-project-test/src/main.cpp +++ b/test/external-project-test/src/main.cpp @@ -182,7 +182,6 @@ int main() { auto host_api_factory = std::make_shared( kagome::host_api::OffchainExtensionConfig{}, - changes_tracker, sr25519_provider, ecdsa_provider, ed25519_provider,