Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix upper bound in cursor and remove in topper batch #1180

Merged
merged 9 commits into from
Apr 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions core/authorship/impl/block_builder_factory_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ namespace kagome::authorship {
outcome::result<std::unique_ptr<BlockBuilder>> BlockBuilderFactoryImpl::make(
const kagome::primitives::BlockInfo &parent,
primitives::Digest inherent_digest) const {
#ifndef BOOST_ASSERT_IS_VOID
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is defined when BOOST_ASSERT is compiled to noop.

OUTCOME_TRY(parent_number, header_backend_->getNumberById(parent.hash));
#endif
BOOST_ASSERT(parent_number == parent.number);

auto number = parent.number + 1;
Expand Down
1 change: 1 addition & 0 deletions core/host_api/impl/child_storage_extension.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
13 changes: 5 additions & 8 deletions core/host_api/impl/storage_extension.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -140,12 +140,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());
Expand Down Expand Up @@ -212,7 +207,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"});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the file missing autoformat

}
return clearPrefix(prefix, limit_opt);
}
Expand Down Expand Up @@ -325,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());
Expand All @@ -334,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());
Expand Down
7 changes: 6 additions & 1 deletion core/runtime/common/trie_storage_provider_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<void> TrieStorageProviderImpl::startTransaction() {
stack_of_batches_.emplace(current_batch_);
SL_TRACE(logger_, "Start storage transaction, depth {}", stack_of_batches_.size());
current_batch_ =
std::make_shared<TopperTrieBatchImpl>(std::move(current_batch_));
return outcome::success();
Expand All @@ -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();
}
Expand All @@ -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();
}
Expand Down
1 change: 1 addition & 0 deletions core/storage/trie/impl/ephemeral_trie_batch_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,5 @@ namespace kagome::storage::trie {
}
return empty_hash;
}

} // namespace kagome::storage::trie
7 changes: 3 additions & 4 deletions core/storage/trie/impl/topper_trie_batch_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,8 @@ namespace kagome::storage::trie {
}

outcome::result<void> 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();
}

Expand All @@ -113,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));
}
Expand Down
34 changes: 21 additions & 13 deletions core/storage/trie/polkadot_trie/polkadot_trie_cursor_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<SearchState>(state_).getPath().back().child_idx;
SL_TRACE(
log_,
"We're in a branch and proceed to child {}",
(int)std::get<SearchState>(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
}
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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<SearchState>(state_).getPath().back().child_idx);
BOOST_ASSERT_MSG(child != nullptr, "Branch node must contain a leaf");
current = child;
Expand Down Expand Up @@ -306,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<UninitializedState>(state_)) {
state_ = SearchState{*trie_->getRoot()};
Expand Down Expand Up @@ -334,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();
}

Expand All @@ -352,8 +362,7 @@ namespace kagome::storage::trie {
key_nibbles.putUint8(idx);
}
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<common::Buffer> PolkadotTrieCursorImpl::key() const {
Expand Down Expand Up @@ -393,13 +402,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;
}
Expand Down
Loading