From cc8c0ed6aeb6c5c6e87dff1f4c4b80a800981ea4 Mon Sep 17 00:00:00 2001 From: Harrm Date: Sat, 16 Apr 2022 22:22:50 +0300 Subject: [PATCH 1/6] Fix corner case in upper bound --- .../impl/block_builder_factory_impl.cpp | 2 + .../host_api/impl/child_storage_extension.cpp | 1 + .../polkadot_trie_cursor_impl.cpp | 24 ++++--- .../trie/polkadot_trie/polkadot_trie_impl.cpp | 12 ++-- core/storage/trie/polkadot_trie/trie_node.hpp | 64 +++++++++++++++++++ .../trie/serialization/polkadot_codec.cpp | 46 +------------ .../trie/serialization/polkadot_codec.hpp | 11 ---- .../polkadot_trie_cursor_test.cpp | 31 +++++++++ .../polkadot_codec_nibbles_test.cpp | 4 +- .../storage/polkadot_trie_printer.hpp | 4 +- 10 files changed, 125 insertions(+), 74 deletions(-) diff --git a/core/authorship/impl/block_builder_factory_impl.cpp b/core/authorship/impl/block_builder_factory_impl.cpp index 4de067c1db..e28235f9cc 100644 --- a/core/authorship/impl/block_builder_factory_impl.cpp +++ b/core/authorship/impl/block_builder_factory_impl.cpp @@ -25,7 +25,9 @@ namespace kagome::authorship { outcome::result> BlockBuilderFactoryImpl::make( const kagome::primitives::BlockInfo &parent, primitives::Digest inherent_digest) const { +#ifndef BOOST_ASSERT_IS_VOID OUTCOME_TRY(parent_number, header_backend_->getNumberById(parent.hash)); +#endif BOOST_ASSERT(parent_number == parent.number); auto number = parent.number + 1; diff --git a/core/host_api/impl/child_storage_extension.cpp b/core/host_api/impl/child_storage_extension.cpp index 6964bb7d81..103ed5a7d6 100644 --- a/core/host_api/impl/child_storage_extension.cpp +++ b/core/host_api/impl/child_storage_extension.cpp @@ -189,6 +189,7 @@ namespace kagome::host_api { auto child_batch = child_batch_outcome.value(); auto cursor = child_batch->trieCursor(); storage_provider_->clearChildBatches(); + auto seek_result = cursor->seekUpperBound(key_buffer); if (seek_result.has_error()) { logger_->error( diff --git a/core/storage/trie/polkadot_trie/polkadot_trie_cursor_impl.cpp b/core/storage/trie/polkadot_trie/polkadot_trie_cursor_impl.cpp index 0d6701154c..fc5f58e434 100644 --- a/core/storage/trie/polkadot_trie/polkadot_trie_cursor_impl.cpp +++ b/core/storage/trie/polkadot_trie/polkadot_trie_cursor_impl.cpp @@ -180,12 +180,17 @@ namespace kagome::storage::trie { SAFE_CALL(child, visitChildWithMinIdx(branch, sought_nibbles[mismatch_pos])) if (child) { + uint8_t child_idx = std::get(state_).getPath().back().child_idx; SL_TRACE( log_, - "We're in a branch and proceed to child {}", - (int)std::get(state_).getPath().back().child_idx); - return seekLowerBoundInternal( - *child, sought_nibbles.subspan(mismatch_pos + 1)); + "We're in a branch and proceed to child {:x}", + (int)child_idx); + if (child_idx > sought_nibbles[mismatch_pos]) { + return nextNodeWithValueInSubTree(*child); + } else { + return seekLowerBoundInternal( + *child, sought_nibbles.subspan(mismatch_pos + 1)); + } } break; // go to case3 } @@ -223,7 +228,7 @@ namespace kagome::storage::trie { return outcome::success(); } state_ = SearchState{*trie_->getRoot()}; - auto nibbles = PolkadotCodec::keyToNibbles(key); + auto nibbles = KeyNibbles::fromByteBuffer(key); SAFE_VOID_CALL(seekLowerBoundInternal(*trie_->getRoot(), nibbles)) return outcome::success(); } @@ -258,7 +263,7 @@ namespace kagome::storage::trie { } SAFE_CALL(child, visitChildWithMinIdx(*current)) SL_TRACE(log_, - "Proceed to child {}", + "Proceed to child {:x}", (int)std::get(state_).getPath().back().child_idx); BOOST_ASSERT_MSG(child != nullptr, "Branch node must contain a leaf"); current = child; @@ -353,7 +358,7 @@ namespace kagome::storage::trie { } key_nibbles.put(search_state.getCurrent().key_nibbles); using Codec = kagome::storage::trie::PolkadotCodec; - return Codec::nibblesToKey(key_nibbles); + return key_nibbles.toByteBuffer(); } std::optional PolkadotTrieCursorImpl::key() const { @@ -393,13 +398,12 @@ namespace kagome::storage::trie { return outcome::success(); }; auto res = trie_->forNodeInPath( - trie_->getRoot(), codec_.keyToNibbles(key), add_visited_child); + trie_->getRoot(), KeyNibbles::fromByteBuffer(key), add_visited_child); if (res.has_error()) { if (res.error() == TrieError::NO_VALUE) { return Error::KEY_NOT_FOUND; - } else { - return res.error(); } + return res.error(); } return search_state; } diff --git a/core/storage/trie/polkadot_trie/polkadot_trie_impl.cpp b/core/storage/trie/polkadot_trie/polkadot_trie_impl.cpp index 053aaf52f4..062b7de515 100644 --- a/core/storage/trie/polkadot_trie/polkadot_trie_impl.cpp +++ b/core/storage/trie/polkadot_trie/polkadot_trie_impl.cpp @@ -157,7 +157,7 @@ namespace { outcome::result notifyOnDetached( PolkadotTrie::NodePtr &node, const PolkadotTrie::OnDetachCallback &callback) { - auto key = PolkadotCodec::nibblesToKey(node->key_nibbles); + auto key = node->key_nibbles.toByteBuffer(); OUTCOME_TRY(callback(key, std::move(node->value))); return outcome::success(); } @@ -283,7 +283,7 @@ namespace kagome::storage::trie { outcome::result PolkadotTrieImpl::put(const BufferView &key, Buffer &&value) { - auto k_enc = PolkadotCodec::keyToNibbles(key); + auto k_enc = KeyNibbles::fromByteBuffer(key); NodePtr root = nodes_->getRoot(); @@ -303,7 +303,7 @@ namespace kagome::storage::trie { const OnDetachCallback &callback) { bool finished = true; uint32_t count = 0; - auto key_nibbles = PolkadotCodec::keyToNibbles(prefix); + auto key_nibbles = KeyNibbles::fromByteBuffer(prefix); auto root = nodes_->getRoot(); OUTCOME_TRY(detachNode( root, key_nibbles, limit, finished, count, callback, *nodes_)); @@ -429,7 +429,7 @@ namespace kagome::storage::trie { if (not nodes_->getRoot()) { return std::nullopt; } - auto nibbles = PolkadotCodec::keyToNibbles(key); + auto nibbles = KeyNibbles::fromByteBuffer(key); OUTCOME_TRY(node, getNode(nodes_->getRoot(), nibbles)); if (node && node->value) { return node->value.value(); @@ -536,7 +536,7 @@ namespace kagome::storage::trie { } OUTCOME_TRY(node, - getNode(nodes_->getRoot(), PolkadotCodec::keyToNibbles(key))); + getNode(nodes_->getRoot(), KeyNibbles::fromByteBuffer(key))); return node != nullptr && node->value; } @@ -546,7 +546,7 @@ namespace kagome::storage::trie { outcome::result PolkadotTrieImpl::remove( const common::BufferView &key) { - auto key_nibbles = PolkadotCodec::keyToNibbles(key); + auto key_nibbles = KeyNibbles::fromByteBuffer(key); // delete node will fetch nodes that it needs from the storage (the // nodes typically are a path in the trie) and work on them in memory auto root = nodes_->getRoot(); diff --git a/core/storage/trie/polkadot_trie/trie_node.hpp b/core/storage/trie/polkadot_trie/trie_node.hpp index 01a27d627c..38c75fb0c6 100644 --- a/core/storage/trie/polkadot_trie/trie_node.hpp +++ b/core/storage/trie/polkadot_trie/trie_node.hpp @@ -8,6 +8,8 @@ #include +#include + #include "common/blob.hpp" #include "common/buffer.hpp" #include "storage/trie/node.hpp" @@ -28,6 +30,54 @@ namespace kagome::storage::trie { return *this; } + /** + * Def. 14 KeyEncode + * Splits a key to an array of nibbles (a nibble is a half of a byte) + * TODO(Harrm): Good candidate to rewrite with SIMD + */ + static KeyNibbles fromByteBuffer(const common::BufferView &key) { + if (key.empty()) { + return {}; + } + if (key.size() == 1 && key[0] == 0) { + return KeyNibbles{common::Buffer{0, 0}}; + } + + auto l = key.size() * 2; + KeyNibbles res(common::Buffer(l, 0)); + for (ssize_t i = 0; i < key.size(); i++) { + res[2 * i] = key[i] >> 4u; + res[2 * i + 1] = key[i] & 0xfu; + } + + return res; + } + + /** + * Collects an array of nibbles to a key + * TODO(Harrm): Good candidate to rewrite with SIMD + */ + Buffer toByteBuffer() const { + Buffer res; + if (size() % 2 == 0) { + res = Buffer(size() / 2, 0); + for (size_t i = 0; i < size() / 2; i++) { + res[i] = toByte((*this)[i * 2], (*this)[i * 2 + 1]); + } + } else { + res = Buffer(size() / 2 + 1, 0); + res[0] = (*this)[0]; + for (size_t i = 1; i < size() / 2 + 1; i++) { + res[i] = toByte((*this)[i * 2 - 1], (*this)[i * 2]); + } + } + return res; + } + + static uint8_t toByte(uint8_t high, uint8_t low) { + return (high << 4u) | (low & 0xfu); + } + NibblesView subspan(size_t offset = 0, size_t length = -1) const { return NibblesView{*this}.subspan(offset, length); } @@ -123,4 +173,18 @@ namespace kagome::storage::trie { } // namespace kagome::storage::trie +template <> +struct fmt::formatter { + template + auto format(const kagome::storage::trie::KeyNibbles &p, FormatContext &ctx) + -> decltype(ctx.out()) { + if (p.size() % 2 != 0) { + format_to(ctx.out(), "{:x}", p[0]); + } + for (size_t i = p.size() % 2; i < p.size() - 1; i += 2) { + format_to(ctx.out(), "{:02x}", p.toByte(p[i], p[i + 1])); + } + } +}; + #endif // KAGOME_STORAGE_TRIE_POLKADOT_NODE diff --git a/core/storage/trie/serialization/polkadot_codec.cpp b/core/storage/trie/serialization/polkadot_codec.cpp index e5a02a9d66..a837a057a8 100644 --- a/core/storage/trie/serialization/polkadot_codec.cpp +++ b/core/storage/trie/serialization/polkadot_codec.cpp @@ -30,11 +30,6 @@ OUTCOME_CPP_DEFINE_CATEGORY(kagome::storage::trie, PolkadotCodec::Error, e) { namespace kagome::storage::trie { - inline uint8_t byteFromNibbles(uint8_t high, uint8_t low) { - // get low4 from nibbles to avoid check: if(low > 0xff) return error - return (high << 4u) | (low & 0xfu); - } - inline common::Buffer ushortToBytes(uint16_t b) { common::Buffer out(2, 0); out[1] = (b >> 8u) & 0xffu; @@ -42,41 +37,6 @@ namespace kagome::storage::trie { return out; } - common::Buffer PolkadotCodec::nibblesToKey(const KeyNibbles &nibbles) { - Buffer res; - if (nibbles.size() % 2 == 0) { - res = Buffer(nibbles.size() / 2, 0); - for (size_t i = 0; i < nibbles.size(); i += 2) { - res[i / 2] = byteFromNibbles(nibbles[i], nibbles[i + 1]); - } - } else { - res = Buffer(nibbles.size() / 2 + 1, 0); - res[0] = nibbles[0]; - for (size_t i = 2; i < nibbles.size(); i += 2) { - res[i / 2] = byteFromNibbles(nibbles[i - 1], nibbles[i]); - } - } - return res; - } - - KeyNibbles PolkadotCodec::keyToNibbles(const common::BufferView &key) { - if (key.empty()) { - return {}; - } - if (key.size() == 1 && key[0] == 0) { - return KeyNibbles{common::Buffer{0, 0}}; - } - - auto l = key.size() * 2; - KeyNibbles res(common::Buffer(l, 0)); - for (ssize_t i = 0; i < key.size(); i++) { - res[2 * i] = key[i] >> 4u; - res[2 * i + 1] = key[i] & 0xfu; - } - - return res; - } - common::Buffer PolkadotCodec::merkleValue( const common::BufferView &buf) const { // if a buffer size is less than the size of a would-be hash, just return @@ -167,7 +127,7 @@ namespace kagome::storage::trie { OUTCOME_TRY(encoding, encodeHeader(node)); // key - encoding += nibblesToKey(node.key_nibbles); + encoding += node.key_nibbles.toByteBuffer(); // children bitmap encoding += ushortToBytes(node.childrenBitmap()); @@ -202,7 +162,7 @@ namespace kagome::storage::trie { OUTCOME_TRY(encoding, encodeHeader(node)); // key - encoding += nibblesToKey(node.key_nibbles); + encoding += node.key_nibbles.toByteBuffer(); if (!node.value) return Error::NO_NODE_VALUE; // scale encoded value @@ -278,7 +238,7 @@ namespace kagome::storage::trie { } // array of nibbles is much more convenient than array of bytes, though it // wastes some memory - KeyNibbles partial_key_nibbles = keyToNibbles(partial_key); + auto partial_key_nibbles = KeyNibbles::fromByteBuffer(partial_key); if (nibbles_num % 2 == 1) { partial_key_nibbles = partial_key_nibbles.subbuffer(1); } diff --git a/core/storage/trie/serialization/polkadot_codec.hpp b/core/storage/trie/serialization/polkadot_codec.hpp index e0673351d9..e5278dfc4b 100644 --- a/core/storage/trie/serialization/polkadot_codec.hpp +++ b/core/storage/trie/serialization/polkadot_codec.hpp @@ -40,17 +40,6 @@ namespace kagome::storage::trie { common::Hash256 hash256(const BufferView &buf) const override; - /** - * Def. 14 KeyEncode - * Splits a key to an array of nibbles (a nibble is a half of a byte) - */ - static KeyNibbles keyToNibbles(const BufferView &key); - - /** - * Collects an array of nibbles to a key - */ - static Buffer nibblesToKey(const KeyNibbles &nibbles); - /** * Encodes a node header according to the specification * @see Algorithm 3: partial key length encoding diff --git a/test/core/storage/trie/polkadot_trie/polkadot_trie_cursor_test.cpp b/test/core/storage/trie/polkadot_trie/polkadot_trie_cursor_test.cpp index 2cf02eb6d3..197f344c5c 100644 --- a/test/core/storage/trie/polkadot_trie/polkadot_trie_cursor_test.cpp +++ b/test/core/storage/trie/polkadot_trie/polkadot_trie_cursor_test.cpp @@ -359,3 +359,34 @@ TEST_F(PolkadotTrieCursorTest, SeekLowerBoundLeaf) { EXPECT_OUTCOME_TRUE_1(cursor->seekLowerBound(lex_sorted_vals[3].first)) ASSERT_TRUE(cursor->isValid()); } + +/** + * GIVEN A tree where the beginning of upper bound key for the given key lays + * through child indices (and not in key parts inside nodes). + * WHEN Searching for upper bound. + * THEN Correct upper bound is returned. + * @note There was a bug in the cursor implementation, where the cursor ignored + * the fact that it followed through a child index larger than required and thus + * could return any node with value that it finds next. Instead, it continued + * searching for nibbles larger or equal than in the required key, thus skipping + * the actual upper bound in corner cases + */ +TEST_F(PolkadotTrieCursorTest, Broken) { + kagome::log::setLevelOfGroup(kagome::log::defaultGroupName, soralog::Level::TRACE); + std::vector> vals = { + {"00289e629fac633384f461a8e9a7bc63bce825350e4548ed2a06ab661909af3c"_hex2buf, + "00"_hex2buf}, + {"002f7f49bfd6648427ffdbce670e4019fa96f7a96031763ad241c981c85de627"_hex2buf, + "00"_hex2buf}, + {"11"_hex2buf, "00"_hex2buf}, + {"01"_hex2buf, "00"_hex2buf}, + {"10"_hex2buf, "00"_hex2buf}, + {"0000"_hex2buf, "00"_hex2buf}, + {"0030"_hex2buf, "00"_hex2buf} + }; + auto trie = makeTrie(vals); + auto cursor = trie->trieCursor(); + cursor->seekUpperBound( + "001bc05a925467574025104b405941493d67d3d3cbf1a66bc21aea056916463c"_hex2buf).value(); + ASSERT_EQ(cursor->key().value(), vals[0].first); +} \ No newline at end of file diff --git a/test/core/storage/trie/trie_storage/polkadot_codec_nibbles_test.cpp b/test/core/storage/trie/trie_storage/polkadot_codec_nibbles_test.cpp index beec3c4e5f..2546e12a26 100644 --- a/test/core/storage/trie/trie_storage/polkadot_codec_nibbles_test.cpp +++ b/test/core/storage/trie/trie_storage/polkadot_codec_nibbles_test.cpp @@ -24,14 +24,14 @@ struct KeyToNibbles TEST_P(NibblesToKey, nibblesToKey) { auto codec = std::make_unique(); auto [nibbles, key] = GetParam(); - auto actualKey = codec->nibblesToKey(nibbles); + auto actualKey = nibbles.toByteBuffer(); ASSERT_EQ(key, actualKey); } TEST_P(KeyToNibbles, keyToNibbles) { auto codec = std::make_unique(); auto [nibbles, key] = GetParam(); - auto actualNibbles = codec->keyToNibbles(key); + auto actualNibbles = KeyNibbles::fromByteBuffer(key); ASSERT_EQ(nibbles, actualNibbles); } diff --git a/test/testutil/storage/polkadot_trie_printer.hpp b/test/testutil/storage/polkadot_trie_printer.hpp index 6619231567..ce66794f40 100644 --- a/test/testutil/storage/polkadot_trie_printer.hpp +++ b/test/testutil/storage/polkadot_trie_printer.hpp @@ -46,7 +46,7 @@ namespace kagome::storage::trie { case T::Leaf: { stream_ << std::setfill('-') << std::setw(nest_level) << "" << std::setw(0) << "(leaf) key: <" - << hex_lower(codec_.nibblesToKey(node->key_nibbles)) + << hex_lower(node->key_nibbles.toByteBuffer()) << "> value: " << node->value.value().toHex() << "\n"; break; } @@ -66,7 +66,7 @@ namespace kagome::storage::trie { auto branch = std::dynamic_pointer_cast(node); stream_ << std::setfill('-') << std::setw(nest_level) << "" << std::setw(0) << "(branch) key: <" - << hex_lower(codec_.nibblesToKey(node->key_nibbles)) + << hex_lower(node->key_nibbles.toByteBuffer()) << "> value: " << value << " children: "; for (size_t i = 0; i < branch->children.size(); i++) { if (branch->children[i]) { From 03e29603e7565dd573fe96d04d88add9bffdfbad Mon Sep 17 00:00:00 2001 From: Harrm Date: Wed, 20 Apr 2022 15:16:51 +0300 Subject: [PATCH 2/6] Fix bug in topper batch remove --- core/host_api/impl/storage_extension.cpp | 25 ++++++--- .../common/trie_storage_provider_impl.cpp | 7 ++- .../trie/impl/ephemeral_trie_batch_impl.cpp | 7 +++ .../trie/impl/ephemeral_trie_batch_impl.hpp | 1 + .../trie/impl/persistent_trie_batch_impl.cpp | 6 +++ .../trie/impl/persistent_trie_batch_impl.hpp | 2 + .../trie/impl/topper_trie_batch_impl.cpp | 5 +- .../trie/impl/topper_trie_batch_impl.hpp | 5 ++ .../trie/polkadot_trie/polkadot_trie_impl.cpp | 51 +++++++++++++------ .../trie/polkadot_trie/polkadot_trie_impl.hpp | 3 ++ core/storage/trie/trie_batches.hpp | 2 + .../trie/trie_storage/trie_batch_test.cpp | 23 ++++++++- 12 files changed, 107 insertions(+), 30 deletions(-) diff --git a/core/host_api/impl/storage_extension.cpp b/core/host_api/impl/storage_extension.cpp index 4bd6b16ca3..822d220419 100644 --- a/core/host_api/impl/storage_extension.cpp +++ b/core/host_api/impl/storage_extension.cpp @@ -11,13 +11,13 @@ #include "clock/impl/clock_impl.hpp" #include "log/profiling_logger.hpp" #include "runtime/common/runtime_transaction_error.hpp" -#include "runtime/memory.hpp" #include "runtime/memory_provider.hpp" #include "runtime/ptr_size.hpp" #include "runtime/trie_storage_provider.hpp" #include "scale/encode_append.hpp" #include "storage/predefined_keys.hpp" #include "storage/trie/polkadot_trie/trie_error.hpp" +#include "storage/trie/impl/topper_trie_batch_impl.hpp" #include "storage/trie/serialization/ordered_trie_hash.hpp" using kagome::common::Buffer; @@ -126,6 +126,8 @@ namespace kagome::host_api { "ext_set_storage failed, due to fail in trie db with reason: {}", put_result.error().message()); } + auto root = batch->calculateRoot().value(); + SL_TRACE(logger_, "Root: {}", root.toHex()); } runtime::WasmSpan StorageExtension::ext_storage_get_version_1( @@ -140,12 +142,7 @@ namespace kagome::host_api { auto result = get(key_buffer); if (result) { - auto& opt_buf = result.value(); - if (opt_buf.has_value()) { - SL_TRACE_FUNC_CALL(logger_, opt_buf.value().get().toHex(), key_buffer); - } else { - SL_TRACE_FUNC_CALL(logger_, std::string_view{"none"}, key_buffer); - } + SL_TRACE_FUNC_CALL(logger_, result.value(), key_buffer); } else { logger_->error( error_message, key_buffer.toHex(), result.error().message()); @@ -162,6 +159,10 @@ namespace kagome::host_api { auto batch = storage_provider_->getCurrentBatch(); auto &memory = memory_provider_->getCurrentMemory()->get(); auto key = memory.loadN(key_ptr, key_size); + if (key.toHex() == "26aa394eea5630e07c48ae0c9558cef7b99d880ec681799c0cf30e8886371da9a094f85c668358da445fc075a634d9d4402d3faf81bd73f954786c979879ab6350a53adf0e84499c01bf98d63c429453") { + SL_INFO(logger_, "FLAAAAG"); + } + SL_TRACE(logger_, "batch: {}, persistent batch: {}", fmt::ptr(batch.get()), fmt::ptr(dynamic_cast(batch.get()))); auto del_result = batch->remove(key); SL_TRACE_FUNC_CALL(logger_, del_result.has_value(), key); if (not del_result) { @@ -171,6 +172,8 @@ namespace kagome::host_api { key_data, del_result.error().message()); } + auto root = batch->calculateRoot().value(); + SL_TRACE(logger_, "Root: {}", root.toHex()); } runtime::WasmSize StorageExtension::ext_storage_exists_version_1( @@ -212,7 +215,7 @@ namespace kagome::host_api { if (limit_opt) { SL_TRACE_VOID_FUNC_CALL(logger_, prefix, limit_opt.value()); } else { - SL_TRACE_VOID_FUNC_CALL(logger_, prefix, std::string_view{"none"}); + SL_TRACE_VOID_FUNC_CALL(logger_, prefix, std::string_view {"none"}); } return clearPrefix(prefix, limit_opt); } @@ -222,6 +225,8 @@ namespace kagome::host_api { removeEmptyChildStorages(); if (auto opt_batch = storage_provider_->tryGetPersistentBatch(); opt_batch.has_value() and opt_batch.value() != nullptr) { + auto root = opt_batch.value()->calculateRoot().value(); + SL_TRACE(logger_, "Root: {}", root.toHex()); res = opt_batch.value()->commit(); } else { logger_->warn("ext_storage_root called in an ephemeral extension"); @@ -310,6 +315,8 @@ namespace kagome::host_api { "with reason: {}", put_result.error().message()); } + auto root = batch->calculateRoot().value(); + SL_TRACE(logger_, "Root: {}", root.toHex()); return; } } @@ -486,6 +493,8 @@ namespace kagome::host_api { logger_->error(msg); throw std::runtime_error(msg); } + auto root = batch->calculateRoot().value(); + SL_TRACE(logger_, "Root: {}", root.toHex()); return memory.storeBuffer(enc_res.value()); } diff --git a/core/runtime/common/trie_storage_provider_impl.cpp b/core/runtime/common/trie_storage_provider_impl.cpp index abc92fc6fd..4543b7d24a 100644 --- a/core/runtime/common/trie_storage_provider_impl.cpp +++ b/core/runtime/common/trie_storage_provider_impl.cpp @@ -108,13 +108,16 @@ namespace kagome::runtime { current_batch_)) { // won't actually write any data to the storage but will calculate the // root hash for the state represented by the batch - return ephemeral->hash(); + OUTCOME_TRY(root, ephemeral->hash()); + SL_TRACE(logger_, "Force commit ephemeral batch, root: {}", root); + return root; } return Error::NO_BATCH; } outcome::result TrieStorageProviderImpl::startTransaction() { stack_of_batches_.emplace(current_batch_); + SL_TRACE(logger_, "Start storage transaction, depth {}", stack_of_batches_.size()); current_batch_ = std::make_shared(std::move(current_batch_)); return outcome::success(); @@ -126,6 +129,7 @@ namespace kagome::runtime { } current_batch_ = std::move(stack_of_batches_.top()); + SL_TRACE(logger_, "Rollback storage transaction, depth {}", stack_of_batches_.size()); stack_of_batches_.pop(); return outcome::success(); } @@ -141,6 +145,7 @@ namespace kagome::runtime { OUTCOME_TRY(commitee_batch->writeBack()); current_batch_ = std::move(stack_of_batches_.top()); + SL_TRACE(logger_, "Commit storage transaction, depth {}", stack_of_batches_.size()); stack_of_batches_.pop(); return outcome::success(); } diff --git a/core/storage/trie/impl/ephemeral_trie_batch_impl.cpp b/core/storage/trie/impl/ephemeral_trie_batch_impl.cpp index 7900d296c6..dabdcfbe0d 100644 --- a/core/storage/trie/impl/ephemeral_trie_batch_impl.cpp +++ b/core/storage/trie/impl/ephemeral_trie_batch_impl.cpp @@ -70,4 +70,11 @@ namespace kagome::storage::trie { } return empty_hash; } + + outcome::result EphemeralTrieBatchImpl::calculateRoot() const { + OUTCOME_TRY(enc, codec_->encodeNode(*trie_->getRoot())); + auto root = codec_->hash256(enc); + return root; + } + } // namespace kagome::storage::trie diff --git a/core/storage/trie/impl/ephemeral_trie_batch_impl.hpp b/core/storage/trie/impl/ephemeral_trie_batch_impl.hpp index fd7761b626..15293c2b9b 100644 --- a/core/storage/trie/impl/ephemeral_trie_batch_impl.hpp +++ b/core/storage/trie/impl/ephemeral_trie_batch_impl.hpp @@ -32,6 +32,7 @@ namespace kagome::storage::trie { outcome::result put(const BufferView &key, Buffer &&value) override; outcome::result remove(const BufferView &key) override; outcome::result hash() override; + outcome::result calculateRoot() const override; private: std::shared_ptr codec_; diff --git a/core/storage/trie/impl/persistent_trie_batch_impl.cpp b/core/storage/trie/impl/persistent_trie_batch_impl.cpp index 1c4e0450da..9c4e46687a 100644 --- a/core/storage/trie/impl/persistent_trie_batch_impl.cpp +++ b/core/storage/trie/impl/persistent_trie_batch_impl.cpp @@ -73,6 +73,12 @@ namespace kagome::storage::trie { return std::move(root); } + outcome::result PersistentTrieBatchImpl::calculateRoot() const { + OUTCOME_TRY(enc, codec_->encodeNode(*trie_->getRoot())); + auto root = codec_->hash256(enc); + return root; + } + std::unique_ptr PersistentTrieBatchImpl::batchOnTop() { return std::make_unique(shared_from_this()); } diff --git a/core/storage/trie/impl/persistent_trie_batch_impl.hpp b/core/storage/trie/impl/persistent_trie_batch_impl.hpp index 36f9bc8091..5b372c7a4c 100644 --- a/core/storage/trie/impl/persistent_trie_batch_impl.hpp +++ b/core/storage/trie/impl/persistent_trie_batch_impl.hpp @@ -33,6 +33,8 @@ namespace kagome::storage::trie { outcome::result commit() override; std::unique_ptr batchOnTop() override; + outcome::result calculateRoot() const override; + outcome::result get(const BufferView &key) const override; outcome::result> tryGet( const BufferView &key) const override; diff --git a/core/storage/trie/impl/topper_trie_batch_impl.cpp b/core/storage/trie/impl/topper_trie_batch_impl.cpp index d56cb91391..b7870a2cf9 100644 --- a/core/storage/trie/impl/topper_trie_batch_impl.cpp +++ b/core/storage/trie/impl/topper_trie_batch_impl.cpp @@ -100,9 +100,8 @@ namespace kagome::storage::trie { } outcome::result TopperTrieBatchImpl::remove(const BufferView &key) { - if (auto it = cache_.find(key); it != cache_.end()) { - it->second = std::nullopt; - } + cache_.insert_or_assign(Buffer{key}, std::nullopt); + return outcome::success(); } diff --git a/core/storage/trie/impl/topper_trie_batch_impl.hpp b/core/storage/trie/impl/topper_trie_batch_impl.hpp index c46b806d60..2927371345 100644 --- a/core/storage/trie/impl/topper_trie_batch_impl.hpp +++ b/core/storage/trie/impl/topper_trie_batch_impl.hpp @@ -36,6 +36,11 @@ namespace kagome::storage::trie { outcome::result contains(const BufferView &key) const override; bool empty() const override; + outcome::result calculateRoot() const override { + OUTCOME_TRY(const_cast(this)->writeBack()); + return parent_.lock()->calculateRoot(); + } + outcome::result put(const BufferView &key, const Buffer &value) override; outcome::result put(const BufferView &key, Buffer &&value) override; diff --git a/core/storage/trie/polkadot_trie/polkadot_trie_impl.cpp b/core/storage/trie/polkadot_trie/polkadot_trie_impl.cpp index 062b7de515..6bc3134361 100644 --- a/core/storage/trie/polkadot_trie/polkadot_trie_impl.cpp +++ b/core/storage/trie/polkadot_trie/polkadot_trie_impl.cpp @@ -50,8 +50,8 @@ namespace kagome::storage::trie { root_ = root; } - [[nodiscard]] outcome::result> - getChild(BranchNode const &parent, uint8_t idx) const { + [[nodiscard]] outcome::result> getChild( + BranchNode const &parent, uint8_t idx) const { // SAFETY: changing a parent's opaque child node from a handle to a node // to the actual node doesn't break it's const correctness, because opaque // nodes are meant to hide their content @@ -97,17 +97,19 @@ namespace { its key, and copy children if any */ [[nodiscard]] outcome::result handleDeletion( - PolkadotTrie::NodePtr & parent, OpaqueNodeStorage &node_storage) { + const kagome::log::Logger& logger, PolkadotTrie::NodePtr &parent, OpaqueNodeStorage &node_storage) { if (!parent->isBranch()) return outcome::success(); - auto& branch = dynamic_cast(*parent); + auto &branch = dynamic_cast(*parent); auto bitmap = branch.childrenBitmap(); if (bitmap == 0) { if (parent->value) { // turn branch node left with no children to a leaf node parent = std::make_shared(parent->key_nibbles, parent->value); + SL_TRACE(logger, "handleDeletion: turn childless branch into a leaf"); } else { // this case actual only for clearPrefix, unreal situation in deletion parent = nullptr; + SL_TRACE(logger, "handleDeletion: nullify valueless branch parent"); } } else if (branch.childrenNum() == 1 && !branch.value) { size_t idx = 0; @@ -117,9 +119,11 @@ namespace { using T = TrieNode::Type; if (child->getTrieType() == T::Leaf) { parent = std::make_shared(parent->key_nibbles, child->value); + SL_TRACE(logger, "handleDeletion: turn a branch with single leaf child into its child"); } else if (child->isBranch()) { branch.children = dynamic_cast(*child).children; parent->value = child->value; + SL_TRACE(logger, "handleDeletion: turn a branch with single branch child into its child"); } parent->key_nibbles.putUint8(idx).putBuffer(child->key_nibbles); } @@ -130,25 +134,34 @@ namespace { * @return if the node should be deleted itself from parent */ [[nodiscard]] outcome::result deleteNode( - PolkadotTrie::NodePtr & node, + const kagome::log::Logger &logger, + PolkadotTrie::NodePtr &node, const NibblesView &sought_key, OpaqueNodeStorage &node_storage) { if (node == nullptr) { return outcome::success(); } + SL_TRACE(logger, + "deleteNode: currently in {}, sought key is {}", + node->key_nibbles, + sought_key); if (node->isBranch()) { auto &branch = dynamic_cast(*node); if (node->key_nibbles == sought_key) { + SL_TRACE(logger, "deleteNode: deleting value in branch; stop"); node->value = std::nullopt; } else { auto length = getCommonPrefixLength(node->key_nibbles, sought_key); OUTCOME_TRY(child, node_storage.getChild(branch, sought_key[length])); - OUTCOME_TRY(deleteNode(child, sought_key.subspan(length + 1), node_storage)); + SL_TRACE(logger, "deleteNode: go to child {:x}", (int)sought_key[length]); + OUTCOME_TRY( + deleteNode(logger, child, sought_key.subspan(length + 1), node_storage)); branch.children[sought_key[length]] = child; } - OUTCOME_TRY(handleDeletion(node, node_storage)); + OUTCOME_TRY(handleDeletion(logger, node, node_storage)); } else if (node->key_nibbles == sought_key) { + SL_TRACE(logger, "deleteNode: nullifying leaf node; stop"); node = nullptr; } return outcome::success(); @@ -169,7 +182,8 @@ namespace { * @param count is a number of values deleted */ [[nodiscard]] outcome::result detachNode( - std::shared_ptr & parent, + const kagome::log::Logger& logger, + std::shared_ptr &parent, const KeyNibbles &prefix, std::optional limit, bool &finished, @@ -196,7 +210,8 @@ namespace { child_idx++) { if (branch.children[child_idx] != nullptr) { OUTCOME_TRY(child_node, node_storage.getChild(branch, child_idx)); - OUTCOME_TRY(detachNode(child_node, + OUTCOME_TRY(detachNode(logger, + child_node, KeyNibbles(), limit, finished, @@ -220,7 +235,7 @@ namespace { } if (parent->isBranch()) { // fix block after children removal - OUTCOME_TRY(handleDeletion(parent, node_storage)); + OUTCOME_TRY(handleDeletion(logger, parent, node_storage)); } } return outcome::success(); @@ -241,7 +256,8 @@ namespace { auto &child = branch.children.at(prefix[length]); if (child != nullptr) { OUTCOME_TRY(child_node, node_storage.getChild(branch, prefix[length])); - OUTCOME_TRY(detachNode(child_node, + OUTCOME_TRY(detachNode(logger, + child_node, prefix.subspan(length + 1), limit, finished, @@ -249,7 +265,7 @@ namespace { callback, node_storage)); branch.children[prefix[length]] = child_node; - OUTCOME_TRY(handleDeletion(parent, node_storage)); + OUTCOME_TRY(handleDeletion(logger, parent, node_storage)); } } return outcome::success(); @@ -260,10 +276,12 @@ namespace { namespace kagome::storage::trie { PolkadotTrieImpl::PolkadotTrieImpl(NodeRetrieveFunctor f) - : nodes_{std::make_unique(std::move(f), nullptr)} {} + : nodes_{std::make_unique(std::move(f), nullptr)}, + logger_{log::createLogger("PolkadotTrie", "trie")} {} PolkadotTrieImpl::PolkadotTrieImpl(NodePtr root, NodeRetrieveFunctor f) - : nodes_{std::make_unique(std::move(f), root)} {} + : nodes_{std::make_unique(std::move(f), root)}, + logger_{log::createLogger("PolkadotTrie", "trie")} {} PolkadotTrieImpl::~PolkadotTrieImpl() {} @@ -306,7 +324,7 @@ namespace kagome::storage::trie { auto key_nibbles = KeyNibbles::fromByteBuffer(prefix); auto root = nodes_->getRoot(); OUTCOME_TRY(detachNode( - root, key_nibbles, limit, finished, count, callback, *nodes_)); + logger_, root, key_nibbles, limit, finished, count, callback, *nodes_)); nodes_->setRoot(root); return {finished, count}; } @@ -550,7 +568,8 @@ namespace kagome::storage::trie { // delete node will fetch nodes that it needs from the storage (the // nodes typically are a path in the trie) and work on them in memory auto root = nodes_->getRoot(); - OUTCOME_TRY(deleteNode(root, key_nibbles, *nodes_)); + SL_TRACE(logger_, "Remove by key {:l} (nibbles {:l})", key, key_nibbles); + OUTCOME_TRY(deleteNode(logger_, root, key_nibbles, *nodes_)); nodes_->setRoot(root); return outcome::success(); } diff --git a/core/storage/trie/polkadot_trie/polkadot_trie_impl.hpp b/core/storage/trie/polkadot_trie/polkadot_trie_impl.hpp index 96b1d5a2b6..781c3a0d9d 100644 --- a/core/storage/trie/polkadot_trie/polkadot_trie_impl.hpp +++ b/core/storage/trie/polkadot_trie/polkadot_trie_impl.hpp @@ -8,6 +8,7 @@ #include "storage/trie/polkadot_trie/polkadot_trie.hpp" +#include "log/logger.hpp" #include "storage/buffer_map_types.hpp" #include "storage/trie/serialization/polkadot_codec.hpp" @@ -94,6 +95,8 @@ namespace kagome::storage::trie { const NodePtr &node); std::unique_ptr nodes_; + + log::Logger logger_; }; } // namespace kagome::storage::trie diff --git a/core/storage/trie/trie_batches.hpp b/core/storage/trie/trie_batches.hpp index 7edd492e38..d16344cb66 100644 --- a/core/storage/trie/trie_batches.hpp +++ b/core/storage/trie/trie_batches.hpp @@ -27,6 +27,8 @@ namespace kagome::storage::trie { virtual std::unique_ptr trieCursor() = 0; + virtual outcome::result calculateRoot() const = 0; + /** * Remove all trie entries which key begins with the supplied prefix */ diff --git a/test/core/storage/trie/trie_storage/trie_batch_test.cpp b/test/core/storage/trie/trie_storage/trie_batch_test.cpp index bc4ca39359..0044d3257a 100644 --- a/test/core/storage/trie/trie_storage/trie_batch_test.cpp +++ b/test/core/storage/trie/trie_storage/trie_batch_test.cpp @@ -97,8 +97,7 @@ class MockDb : public kagome::storage::InMemoryStorage { // to retain the ability to call the actual implementation of put from the // superclass - outcome::result true_put(const BufferView &key, - const Buffer &value) { + outcome::result true_put(const BufferView &key, const Buffer &value) { return InMemoryStorage::put(key, value); } }; @@ -240,4 +239,24 @@ TEST_F(TrieBatchTest, TopperBatchAtomic) { ASSERT_OUTCOME_IS_FALSE(p_batch->contains("123"_buf)) } +/** + * GIVEN a key present in a persistent batch but not present in its child topper + * batch WHEN issuing a remove of this key from the topper batch THEN the key + * must be removed from the persistent batch after a writeback of the topper + * batch + */ +TEST_F(TrieBatchTest, TopperBatchRemove) { + std::shared_ptr p_batch = + trie->getPersistentBatchAt(empty_hash).value(); + + ASSERT_OUTCOME_SUCCESS_TRY(p_batch->put("102030"_hex2buf, "010203"_hex2buf)); + + auto t_batch = p_batch->batchOnTop(); + + t_batch->remove("102030"_hex2buf).value(); + t_batch->writeBack().value(); + + ASSERT_FALSE(p_batch->contains("102030"_hex2buf).value()); +} + // TODO(Harrm): #595 test clearPrefix From cbc0bad620a9430dd176dad84f8312be1ff25efb Mon Sep 17 00:00:00 2001 From: Harrm Date: Wed, 20 Apr 2022 20:40:05 +0300 Subject: [PATCH 3/6] Remove debug ad-hocs --- core/host_api/impl/storage_extension.cpp | 6 ------ .../trie/polkadot_trie/polkadot_trie_cursor_impl.cpp | 1 - core/storage/trie/polkadot_trie/polkadot_trie_impl.cpp | 4 ++-- 3 files changed, 2 insertions(+), 9 deletions(-) diff --git a/core/host_api/impl/storage_extension.cpp b/core/host_api/impl/storage_extension.cpp index 822d220419..5183879637 100644 --- a/core/host_api/impl/storage_extension.cpp +++ b/core/host_api/impl/storage_extension.cpp @@ -159,10 +159,6 @@ namespace kagome::host_api { auto batch = storage_provider_->getCurrentBatch(); auto &memory = memory_provider_->getCurrentMemory()->get(); auto key = memory.loadN(key_ptr, key_size); - if (key.toHex() == "26aa394eea5630e07c48ae0c9558cef7b99d880ec681799c0cf30e8886371da9a094f85c668358da445fc075a634d9d4402d3faf81bd73f954786c979879ab6350a53adf0e84499c01bf98d63c429453") { - SL_INFO(logger_, "FLAAAAG"); - } - SL_TRACE(logger_, "batch: {}, persistent batch: {}", fmt::ptr(batch.get()), fmt::ptr(dynamic_cast(batch.get()))); auto del_result = batch->remove(key); SL_TRACE_FUNC_CALL(logger_, del_result.has_value(), key); if (not del_result) { @@ -172,8 +168,6 @@ namespace kagome::host_api { key_data, del_result.error().message()); } - auto root = batch->calculateRoot().value(); - SL_TRACE(logger_, "Root: {}", root.toHex()); } runtime::WasmSize StorageExtension::ext_storage_exists_version_1( diff --git a/core/storage/trie/polkadot_trie/polkadot_trie_cursor_impl.cpp b/core/storage/trie/polkadot_trie/polkadot_trie_cursor_impl.cpp index fc5f58e434..f159f41d51 100644 --- a/core/storage/trie/polkadot_trie/polkadot_trie_cursor_impl.cpp +++ b/core/storage/trie/polkadot_trie/polkadot_trie_cursor_impl.cpp @@ -357,7 +357,6 @@ namespace kagome::storage::trie { key_nibbles.putUint8(idx); } key_nibbles.put(search_state.getCurrent().key_nibbles); - using Codec = kagome::storage::trie::PolkadotCodec; return key_nibbles.toByteBuffer(); } diff --git a/core/storage/trie/polkadot_trie/polkadot_trie_impl.cpp b/core/storage/trie/polkadot_trie/polkadot_trie_impl.cpp index 6bc3134361..a2cd86390a 100644 --- a/core/storage/trie/polkadot_trie/polkadot_trie_impl.cpp +++ b/core/storage/trie/polkadot_trie/polkadot_trie_impl.cpp @@ -143,7 +143,7 @@ namespace { } SL_TRACE(logger, "deleteNode: currently in {}, sought key is {}", - node->key_nibbles, + node->key_nibbles.toHex(), sought_key); if (node->isBranch()) { @@ -568,7 +568,7 @@ namespace kagome::storage::trie { // delete node will fetch nodes that it needs from the storage (the // nodes typically are a path in the trie) and work on them in memory auto root = nodes_->getRoot(); - SL_TRACE(logger_, "Remove by key {:l} (nibbles {:l})", key, key_nibbles); + SL_TRACE(logger_, "Remove by key {:l} (nibbles {})", key, key_nibbles.toHex()); OUTCOME_TRY(deleteNode(logger_, root, key_nibbles, *nodes_)); nodes_->setRoot(root); return outcome::success(); From 003078b335966a278aecd62232befef5bdec02c6 Mon Sep 17 00:00:00 2001 From: Harrm Date: Thu, 21 Apr 2022 12:11:10 +0300 Subject: [PATCH 4/6] Fix transaction functionality in storage --- core/host_api/impl/storage_extension.cpp | 10 ++-------- core/storage/trie/impl/ephemeral_trie_batch_impl.cpp | 6 ------ core/storage/trie/impl/ephemeral_trie_batch_impl.hpp | 1 - core/storage/trie/impl/persistent_trie_batch_impl.cpp | 6 ------ core/storage/trie/impl/persistent_trie_batch_impl.hpp | 2 -- core/storage/trie/impl/topper_trie_batch_impl.cpp | 2 +- core/storage/trie/impl/topper_trie_batch_impl.hpp | 5 ----- core/storage/trie/trie_batches.hpp | 2 -- 8 files changed, 3 insertions(+), 31 deletions(-) diff --git a/core/host_api/impl/storage_extension.cpp b/core/host_api/impl/storage_extension.cpp index 5183879637..a1396d2284 100644 --- a/core/host_api/impl/storage_extension.cpp +++ b/core/host_api/impl/storage_extension.cpp @@ -126,8 +126,6 @@ namespace kagome::host_api { "ext_set_storage failed, due to fail in trie db with reason: {}", put_result.error().message()); } - auto root = batch->calculateRoot().value(); - SL_TRACE(logger_, "Root: {}", root.toHex()); } runtime::WasmSpan StorageExtension::ext_storage_get_version_1( @@ -219,8 +217,6 @@ namespace kagome::host_api { removeEmptyChildStorages(); if (auto opt_batch = storage_provider_->tryGetPersistentBatch(); opt_batch.has_value() and opt_batch.value() != nullptr) { - auto root = opt_batch.value()->calculateRoot().value(); - SL_TRACE(logger_, "Root: {}", root.toHex()); res = opt_batch.value()->commit(); } else { logger_->warn("ext_storage_root called in an ephemeral extension"); @@ -309,8 +305,6 @@ namespace kagome::host_api { "with reason: {}", put_result.error().message()); } - auto root = batch->calculateRoot().value(); - SL_TRACE(logger_, "Root: {}", root.toHex()); return; } } @@ -326,6 +320,7 @@ namespace kagome::host_api { void StorageExtension::ext_storage_commit_transaction_version_1() { auto res = storage_provider_->commitTransaction(); + SL_TRACE_VOID_FUNC_CALL(logger_); if (res.has_error()) { logger_->error("Storage transaction rollback has failed: {}", res.error().message()); @@ -335,6 +330,7 @@ namespace kagome::host_api { void StorageExtension::ext_storage_rollback_transaction_version_1() { auto res = storage_provider_->rollbackTransaction(); + SL_TRACE_VOID_FUNC_CALL(logger_); if (res.has_error()) { logger_->error("Storage transaction commit has failed: {}", res.error().message()); @@ -487,8 +483,6 @@ namespace kagome::host_api { logger_->error(msg); throw std::runtime_error(msg); } - auto root = batch->calculateRoot().value(); - SL_TRACE(logger_, "Root: {}", root.toHex()); return memory.storeBuffer(enc_res.value()); } diff --git a/core/storage/trie/impl/ephemeral_trie_batch_impl.cpp b/core/storage/trie/impl/ephemeral_trie_batch_impl.cpp index dabdcfbe0d..9b75dc93dd 100644 --- a/core/storage/trie/impl/ephemeral_trie_batch_impl.cpp +++ b/core/storage/trie/impl/ephemeral_trie_batch_impl.cpp @@ -71,10 +71,4 @@ namespace kagome::storage::trie { return empty_hash; } - outcome::result EphemeralTrieBatchImpl::calculateRoot() const { - OUTCOME_TRY(enc, codec_->encodeNode(*trie_->getRoot())); - auto root = codec_->hash256(enc); - return root; - } - } // namespace kagome::storage::trie diff --git a/core/storage/trie/impl/ephemeral_trie_batch_impl.hpp b/core/storage/trie/impl/ephemeral_trie_batch_impl.hpp index 15293c2b9b..fd7761b626 100644 --- a/core/storage/trie/impl/ephemeral_trie_batch_impl.hpp +++ b/core/storage/trie/impl/ephemeral_trie_batch_impl.hpp @@ -32,7 +32,6 @@ namespace kagome::storage::trie { outcome::result put(const BufferView &key, Buffer &&value) override; outcome::result remove(const BufferView &key) override; outcome::result hash() override; - outcome::result calculateRoot() const override; private: std::shared_ptr codec_; diff --git a/core/storage/trie/impl/persistent_trie_batch_impl.cpp b/core/storage/trie/impl/persistent_trie_batch_impl.cpp index 9c4e46687a..1c4e0450da 100644 --- a/core/storage/trie/impl/persistent_trie_batch_impl.cpp +++ b/core/storage/trie/impl/persistent_trie_batch_impl.cpp @@ -73,12 +73,6 @@ namespace kagome::storage::trie { return std::move(root); } - outcome::result PersistentTrieBatchImpl::calculateRoot() const { - OUTCOME_TRY(enc, codec_->encodeNode(*trie_->getRoot())); - auto root = codec_->hash256(enc); - return root; - } - std::unique_ptr PersistentTrieBatchImpl::batchOnTop() { return std::make_unique(shared_from_this()); } diff --git a/core/storage/trie/impl/persistent_trie_batch_impl.hpp b/core/storage/trie/impl/persistent_trie_batch_impl.hpp index 5b372c7a4c..36f9bc8091 100644 --- a/core/storage/trie/impl/persistent_trie_batch_impl.hpp +++ b/core/storage/trie/impl/persistent_trie_batch_impl.hpp @@ -33,8 +33,6 @@ namespace kagome::storage::trie { outcome::result commit() override; std::unique_ptr batchOnTop() override; - outcome::result calculateRoot() const override; - outcome::result get(const BufferView &key) const override; outcome::result> tryGet( const BufferView &key) const override; diff --git a/core/storage/trie/impl/topper_trie_batch_impl.cpp b/core/storage/trie/impl/topper_trie_batch_impl.cpp index b7870a2cf9..74ac5e5dcd 100644 --- a/core/storage/trie/impl/topper_trie_batch_impl.cpp +++ b/core/storage/trie/impl/topper_trie_batch_impl.cpp @@ -112,7 +112,7 @@ namespace kagome::storage::trie { ++it) it->second = std::nullopt; - cleared_prefixes_.push_back(Buffer{prefix}); + cleared_prefixes_.emplace_back(prefix); if (parent_.lock() != nullptr) { return outcome::success(std::make_tuple(true, 0ULL)); } diff --git a/core/storage/trie/impl/topper_trie_batch_impl.hpp b/core/storage/trie/impl/topper_trie_batch_impl.hpp index 2927371345..c46b806d60 100644 --- a/core/storage/trie/impl/topper_trie_batch_impl.hpp +++ b/core/storage/trie/impl/topper_trie_batch_impl.hpp @@ -36,11 +36,6 @@ namespace kagome::storage::trie { outcome::result contains(const BufferView &key) const override; bool empty() const override; - outcome::result calculateRoot() const override { - OUTCOME_TRY(const_cast(this)->writeBack()); - return parent_.lock()->calculateRoot(); - } - outcome::result put(const BufferView &key, const Buffer &value) override; outcome::result put(const BufferView &key, Buffer &&value) override; diff --git a/core/storage/trie/trie_batches.hpp b/core/storage/trie/trie_batches.hpp index d16344cb66..7edd492e38 100644 --- a/core/storage/trie/trie_batches.hpp +++ b/core/storage/trie/trie_batches.hpp @@ -27,8 +27,6 @@ namespace kagome::storage::trie { virtual std::unique_ptr trieCursor() = 0; - virtual outcome::result calculateRoot() const = 0; - /** * Remove all trie entries which key begins with the supplied prefix */ From 199aeb73127075114fd0b5cc3c401e91746f781d Mon Sep 17 00:00:00 2001 From: Harrm Date: Thu, 21 Apr 2022 18:20:40 +0300 Subject: [PATCH 5/6] Fix db editor and trie tests --- .../trie/polkadot_trie/polkadot_trie_cursor_impl.cpp | 9 +++++++-- test/core/storage/trie/polkadot_trie/CMakeLists.txt | 1 + .../storage/trie/polkadot_trie/polkadot_trie_test.cpp | 6 +++--- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/core/storage/trie/polkadot_trie/polkadot_trie_cursor_impl.cpp b/core/storage/trie/polkadot_trie/polkadot_trie_cursor_impl.cpp index f159f41d51..9a3977b7b5 100644 --- a/core/storage/trie/polkadot_trie/polkadot_trie_cursor_impl.cpp +++ b/core/storage/trie/polkadot_trie/polkadot_trie_cursor_impl.cpp @@ -311,7 +311,11 @@ namespace kagome::storage::trie { return outcome::success(); } - SL_TRACE(log_, "Searching next key, current is {}", key().value()); + if (key().has_value()) { + SL_TRACE(log_, "Searching next key, current is {}", key().value()); + } else { + SL_TRACE(log_, "Searching next key, no current key"); + } if (std::holds_alternative(state_)) { state_ = SearchState{*trie_->getRoot()}; @@ -339,8 +343,9 @@ namespace kagome::storage::trie { if (not found) { SL_TRACE(log_, "Not found anything"); state_ = ReachedEndState{}; + } else { + SL_TRACE(log_, "Found {}", key().value()); } - SL_TRACE(log_, "Found {}", key().value()); return outcome::success(); } diff --git a/test/core/storage/trie/polkadot_trie/CMakeLists.txt b/test/core/storage/trie/polkadot_trie/CMakeLists.txt index 2c66163ac9..bbe93c2008 100644 --- a/test/core/storage/trie/polkadot_trie/CMakeLists.txt +++ b/test/core/storage/trie/polkadot_trie/CMakeLists.txt @@ -10,6 +10,7 @@ target_link_libraries(polkadot_trie_test Boost::boost trie_serializer trie_error + log_configurator ) addtest(polkadot_trie_cursor_test diff --git a/test/core/storage/trie/polkadot_trie/polkadot_trie_test.cpp b/test/core/storage/trie/polkadot_trie/polkadot_trie_test.cpp index 162b62248f..8e5b75f537 100644 --- a/test/core/storage/trie/polkadot_trie/polkadot_trie_test.cpp +++ b/test/core/storage/trie/polkadot_trie/polkadot_trie_test.cpp @@ -8,6 +8,7 @@ #include "storage/in_memory/in_memory_storage.hpp" #include "storage/trie/polkadot_trie/polkadot_trie_impl.hpp" #include "storage/trie/polkadot_trie/trie_error.hpp" +#include "testutil/prepare_loggers.hpp" #include "testutil/literals.hpp" #include "testutil/outcome.hpp" #include "testutil/storage/polkadot_trie_printer.hpp" @@ -39,6 +40,8 @@ class TrieTest TrieTest() {} void SetUp() override { + testutil::prepareLoggers(soralog::Level::OFF); + trie = std::make_unique(); } @@ -323,7 +326,6 @@ TEST_F(TrieTest, ClearPrefix) { for (auto &entry : data) { ASSERT_OUTCOME_SUCCESS_TRY(trie->put(entry.first, entry.second)); } - std::cout << *trie << "\n"; ASSERT_OUTCOME_SUCCESS_TRY( trie->clearPrefix("bar"_buf, std::nullopt, [](const auto &, auto &&) { return outcome::success(); @@ -394,9 +396,7 @@ TEST_P(DeleteTest, DeleteData) { for (auto &entry : GetParam().data) { ASSERT_OUTCOME_SUCCESS_TRY(trie->put(entry, "123"_buf)); } - std::cout << *trie << "\n==================\n"; ASSERT_OUTCOME_SUCCESS_TRY(trie->remove(GetParam().key)); - std::cout << *trie << "\n==================\n"; ASSERT_EQ(size(trie->getRoot()), GetParam().size); } From 4ed0f9f165b97b815cf02f0b7da6e401a0ea4ca5 Mon Sep 17 00:00:00 2001 From: Harrm Date: Wed, 27 Apr 2022 17:12:11 +0300 Subject: [PATCH 6/6] Fix test --- test/core/authorship/block_builder_factory_test.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/core/authorship/block_builder_factory_test.cpp b/test/core/authorship/block_builder_factory_test.cpp index eedc3648da..f5f93ffc22 100644 --- a/test/core/authorship/block_builder_factory_test.cpp +++ b/test/core/authorship/block_builder_factory_test.cpp @@ -38,8 +38,8 @@ class BlockBuilderFactoryTest : public ::testing::Test { expected_header_.number = expected_number_; expected_header_.digest = inherent_digests_; - EXPECT_CALL(*header_backend_, getNumberByHash(parent_hash_)) - .WillOnce(Return(parent_number_)); + ON_CALL(*header_backend_, getNumberByHash(parent_hash_)) + .WillByDefault(Return(parent_number_)); } std::shared_ptr core_ = std::make_shared();