Skip to content

Fix code provider race #1463

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

Merged
merged 8 commits into from
Jan 11, 2023
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
7 changes: 4 additions & 3 deletions core/blockchain/impl/block_storage_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,19 +38,20 @@ namespace kagome::blockchain {
}

if (not res.value()) {
auto extrinsics_root = trieRoot({});

// genesis block initialization
primitives::Block genesis_block;
genesis_block.header.number = 0;
genesis_block.header.extrinsics_root = extrinsics_root;
genesis_block.header.extrinsics_root = storage::trie::kEmptyRootHash;
genesis_block.header.state_root = state_root;
// the rest of the fields have default value

OUTCOME_TRY(genesis_block_hash, block_storage->putBlock(genesis_block));
OUTCOME_TRY(block_storage->putNumberToIndexKey({0, genesis_block_hash}));

OUTCOME_TRY(block_storage->setBlockTreeLeaves({genesis_block_hash}));

block_storage->logger_->info(
"Genesis block {}, state {}", genesis_block_hash, state_root);
}

return block_storage;
Expand Down
3 changes: 0 additions & 3 deletions core/blockchain/impl/block_tree_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,6 @@ namespace kagome::blockchain {
"Must be known or calculated at least one leaf");

// Find the least and best leaf
auto least_leaf = *block_tree_leaves.begin();
auto best_leaf = *block_tree_leaves.rbegin();

OUTCOME_TRY(last_finalized_block_info, storage->getLastFinalized());
Expand All @@ -155,8 +154,6 @@ namespace kagome::blockchain {
best_leaf,
last_finalized_block_info);

auto hash_tmp = last_finalized_block_info.hash;

// Load non-finalized block from block storage
std::multimap<primitives::BlockInfo, primitives::BlockHeader> collected;

Expand Down
19 changes: 0 additions & 19 deletions core/blockchain/impl/common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,23 +28,4 @@ namespace kagome::blockchain {

return std::move(key_opt);
}

storage::trie::RootHash trieRoot(
const std::vector<std::pair<common::Buffer, common::Buffer>> &key_vals) {
auto trie = storage::trie::PolkadotTrieImpl();
auto codec = storage::trie::PolkadotCodec();

for (const auto &[key, val] : key_vals) {
[[maybe_unused]] auto res = trie.put(key, common::BufferView{val});
BOOST_ASSERT_MSG(res.has_value(), "Insertion into trie failed");
}
auto root = trie.getRoot();
if (root == nullptr) {
static const auto zero_hash = codec.hash256(common::Buffer{0});
return zero_hash;
}
auto encode_res = codec.encodeNode(*root);
BOOST_ASSERT_MSG(encode_res.has_value(), "Trie encoding failed");
return codec.hash256(encode_res.value());
}
} // namespace kagome::blockchain
7 changes: 0 additions & 7 deletions core/blockchain/impl/common.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,6 @@ namespace kagome::blockchain {
*/
outcome::result<std::optional<common::BufferOrView>> idToLookupKey(
const ReadableBufferStorage &map, const primitives::BlockId &id);

/**
* Instantiate empty merkle trie, insert \param key_vals pairs and \return
* Buffer containing merkle root of resulting trie
*/
storage::trie::RootHash trieRoot(
const std::vector<std::pair<common::Buffer, common::Buffer>> &key_vals);
} // namespace kagome::blockchain

#endif // KAGOME_BLOCKCHAIN_COMMON_HPP
12 changes: 4 additions & 8 deletions core/common/blob.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ namespace kagome::common {
*/
template <size_t size_>
class Blob : public std::array<byte_t, size_> {
using Array = std::array<byte_t, size_>;

public:
// Next line is required at least for the scale-codec
static constexpr bool is_static_collection = true;
Expand All @@ -123,9 +125,7 @@ namespace kagome::common {
/**
* Initialize blob value
*/
Blob() {
this->fill(0);
}
constexpr Blob() : Array{} {}

const_narref internal_array_reference() const {
return *const_narptr(this->data());
Expand All @@ -135,11 +135,7 @@ namespace kagome::common {
* @brief constructor enabling initializer list
* @param l initializer list
*/
explicit Blob(const std::array<byte_t, size_> &l) {
std::copy(l.begin(), l.end(), this->begin());
}

virtual ~Blob() = default;
constexpr explicit Blob(const Array &l) : Array{l} {}

/**
* In compile-time returns size of current blob.
Expand Down
7 changes: 2 additions & 5 deletions core/host_api/impl/storage_extension.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -370,9 +370,7 @@ namespace kagome::host_api {
auto &&pv = pairs.value();
storage::trie::PolkadotCodec codec;
if (pv.empty()) {
static const auto empty_root =
common::Buffer(codec.hash256(common::Buffer{0}));
auto res = memory.storeBuffer(empty_root);
auto res = memory.storeBuffer(storage::trie::kEmptyRootHash);
return runtime::PtrSize(res).ptr;
}
storage::trie::PolkadotTrieImpl trie;
Expand Down Expand Up @@ -461,7 +459,6 @@ namespace kagome::host_api {

void StorageExtension::removeEmptyChildStorages() {
static const auto &prefix = storage::kChildStorageDefaultPrefix;
static const auto empty_hash = codec_.hash256(common::Buffer{0});
auto current_key = prefix;
auto key_res = getStorageNextKey(current_key);
while (key_res.has_value() and key_res.value().has_value()) {
Expand All @@ -476,7 +473,7 @@ namespace kagome::host_api {
// SAFETY: key obtained by getStorageNextKey method, thus must exist in
// the storage
auto value_opt = get(current_key).value();
if (value_opt == empty_hash) {
if (value_opt == storage::trie::kEmptyRootHash) {
auto batch = storage_provider_->getCurrentBatch();
auto remove_res = batch->remove(current_key);
if (not remove_res) {
Expand Down
2 changes: 1 addition & 1 deletion core/parachain/availability/bitfield/signer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ namespace kagome::parachain {
return outcome::success();
}
scheduler_->schedule(
[weak = weak_from_this(), relay_parent, signer{std::move(*signer)}]() {
[weak = weak_from_this(), signer{std::move(*signer)}]() {
if (auto self = weak.lock()) {
auto r = self->sign(signer);
if (r.has_error()) {
Expand Down
83 changes: 45 additions & 38 deletions core/runtime/common/runtime_environment_factory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,50 @@ namespace kagome::runtime {
BOOST_ASSERT(this->storage_provider);
}

outcome::result<void> resetMemory(const ModuleInstance &instance) {
static auto log = log::createLogger("RuntimeEnvironmentFactory", "runtime");

OUTCOME_TRY(opt_heap_base, instance.getGlobal("__heap_base"));
if (not opt_heap_base) {
log->error(
"__heap_base global variable is not found in a runtime module");
return RuntimeEnvironmentFactory::Error::ABSENT_HEAP_BASE;
}
int32_t heap_base = boost::get<int32_t>(*opt_heap_base);

auto &memory_provider = instance.getEnvironment().memory_provider;
OUTCOME_TRY(
const_cast<MemoryProvider &>(*memory_provider).resetMemory(heap_base));
auto &memory = memory_provider->getCurrentMemory()->get();

static auto heappages_key = ":heappages"_buf;
OUTCOME_TRY(
heappages,
instance.getEnvironment().storage_provider->getCurrentBatch()->tryGet(
heappages_key));
if (heappages) {
if (sizeof(uint64_t) != heappages->size()) {
log->error(
"Unable to read :heappages value. Type size mismatch. "
"Required {} bytes, but {} available",
sizeof(uint64_t),
heappages->size());
} else {
uint64_t pages = common::le_bytes_to_uint64(heappages->view());
memory.resize(pages * kMemoryPageSize);
log->trace(
"Creating wasm module with non-default :heappages value set to {}",
pages);
}
}

instance.forDataSegment([&](auto offset, auto segment) {
memory.storeBuffer(offset, segment);
});

return outcome::success();
}

RuntimeEnvironmentFactory::RuntimeEnvironmentTemplate::
RuntimeEnvironmentTemplate(
std::weak_ptr<const RuntimeEnvironmentFactory> parent_factory,
Expand Down Expand Up @@ -110,44 +154,7 @@ namespace kagome::runtime {
}
}

OUTCOME_TRY(opt_heap_base, instance->getGlobal("__heap_base"));
if (!opt_heap_base.has_value()) {
parent_factory->logger_->error(
"__heap_base global variable is not found in a runtime module");
return Error::ABSENT_HEAP_BASE;
}
int32_t heap_base = boost::get<int32_t>(opt_heap_base.value());

OUTCOME_TRY(env.memory_provider->resetMemory(heap_base));

auto heappages_key = ":heappages"_buf;
auto heappages_res =
env.storage_provider->getCurrentBatch()->get(heappages_key);
if (heappages_res.has_value()) {
const auto &heappages = heappages_res.value();
if (sizeof(uint64_t) != heappages.size()) {
parent_factory->logger_->error(
"Unable to read :heappages value. Type size mismatch. "
"Required {} bytes, but {} available",
sizeof(uint64_t),
heappages.size());
} else {
uint64_t pages = common::le_bytes_to_uint64(heappages.view());
env.memory_provider->getCurrentMemory()->get().resize(
pages * kMemoryPageSize);
parent_factory->logger_->trace(
"Creating wasm module with non-default :heappages value set to {}",
pages);
}
} else if (kagome::storage::trie::TrieError::NO_VALUE
!= heappages_res.error()) {
return heappages_res.error();
}

auto &memory = env.memory_provider->getCurrentMemory()->get();
instance->forDataSegment([&memory](auto offset, auto segment) {
memory.storeBuffer(offset, segment);
});
OUTCOME_TRY(resetMemory(*instance));

SL_DEBUG(parent_factory->logger_,
"Runtime environment at {}, state: {:l}",
Expand Down
14 changes: 8 additions & 6 deletions core/runtime/common/storage_code_provider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,8 @@ namespace kagome::runtime {

outcome::result<gsl::span<const uint8_t>> StorageCodeProvider::getCodeAt(
const storage::trie::RootHash &state) const {
std::unique_lock lock{mutex_};
if (last_state_root_ != state) {
last_state_root_ = state;

auto block_info =
runtime_upgrade_tracker_->getLastCodeUpdateBlockInfo(state);
if (block_info.has_value()) {
Expand All @@ -47,15 +46,18 @@ namespace kagome::runtime {
}
}
OUTCOME_TRY(batch, storage_->getEphemeralBatchAt(state));
OUTCOME_TRY(setCodeFromBatch(*batch.get()));
OUTCOME_TRY(code, setCodeFromBatch(*batch.get()));
cached_code_ = std::move(code);
last_state_root_ = state;
}
return cached_code_;
}

outcome::result<void> StorageCodeProvider::setCodeFromBatch(
outcome::result<common::Buffer> StorageCodeProvider::setCodeFromBatch(
const storage::trie::EphemeralTrieBatch &batch) const {
OUTCOME_TRY(code, batch.get(storage::kRuntimeCodeKey));
OUTCOME_TRY(uncompressCodeIfNeeded(code, cached_code_));
return outcome::success();
common::Buffer uncompressed;
OUTCOME_TRY(uncompressCodeIfNeeded(code, uncompressed));
return std::move(uncompressed);
}
} // namespace kagome::runtime
3 changes: 2 additions & 1 deletion core/runtime/common/storage_code_provider.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ namespace kagome::runtime {
const storage::trie::RootHash &state) const override;

private:
outcome::result<void> setCodeFromBatch(
outcome::result<common::Buffer> setCodeFromBatch(
const storage::trie::EphemeralTrieBatch &batch) const;
std::shared_ptr<const storage::trie::TrieStorage> storage_;
std::shared_ptr<RuntimeUpgradeTracker> runtime_upgrade_tracker_;
Expand All @@ -44,6 +44,7 @@ namespace kagome::runtime {
mutable common::Buffer cached_code_;
mutable storage::trie::RootHash last_state_root_;
log::Logger logger_;
mutable std::mutex mutex_;
};

} // namespace kagome::runtime
Expand Down
2 changes: 2 additions & 0 deletions core/runtime/runtime_environment_factory.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ namespace kagome::runtime {
primitives::BlockInfo blockchain_state_;
};

outcome::result<void> resetMemory(const ModuleInstance &module);

class RuntimeEnvironmentFactory
: public std::enable_shared_from_this<RuntimeEnvironmentFactory> {
public:
Expand Down
3 changes: 1 addition & 2 deletions core/storage/trie/impl/ephemeral_trie_batch_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,12 @@ namespace kagome::storage::trie {
}

outcome::result<RootHash> EphemeralTrieBatchImpl::hash() {
static const auto empty_hash = codec_->hash256(common::Buffer{0});
if (auto root = trie_->getRoot()) {
OUTCOME_TRY(encoded, codec_->encodeNode(*root));
auto hash = codec_->hash256(encoded);
return hash;
}
return empty_hash;
return kEmptyRootHash;
}

} // namespace kagome::storage::trie
5 changes: 2 additions & 3 deletions core/storage/trie/serialization/ordered_trie_hash.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "scale/scale.hpp"
#include "storage/trie/polkadot_trie/polkadot_trie_impl.hpp"
#include "storage/trie/serialization/polkadot_codec.hpp"
#include "storage/trie/types.hpp"

namespace kagome::storage::trie {

Expand All @@ -28,9 +29,7 @@ namespace kagome::storage::trie {
PolkadotCodec codec;
// empty root
if (begin == end) {
static const auto empty_root =
common::Buffer{}.put(codec.hash256(common::Buffer{0}));
return empty_root;
return kEmptyRootHash;
}
static_assert(
std::is_same_v<std::decay_t<decltype(*begin)>, common::Buffer>);
Expand Down
3 changes: 1 addition & 2 deletions core/storage/trie/serialization/trie_serializer_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@ namespace kagome::storage::trie {
}

RootHash TrieSerializerImpl::getEmptyRootHash() const {
static const auto empty_hash = codec_->hash256(common::Buffer{0});
return empty_hash;
return kEmptyRootHash;
}

outcome::result<RootHash> TrieSerializerImpl::storeTrie(PolkadotTrie &trie) {
Expand Down
6 changes: 6 additions & 0 deletions core/storage/trie/types.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@ namespace kagome::storage::trie {

enum class StateVersion { V0, V1 };

/// blake2b_256(0x00)
constexpr RootHash kEmptyRootHash{{
0x03, 0x17, 0x0a, 0x2e, 0x75, 0x97, 0xb7, 0xb7, 0xe3, 0xd8, 0x4c,
0x05, 0x39, 0x1d, 0x13, 0x9a, 0x62, 0xb1, 0x57, 0xe7, 0x87, 0x86,
0xd8, 0xc0, 0x82, 0xf2, 0x9d, 0xcf, 0x4c, 0x11, 0x13, 0x14,
}};
} // namespace kagome::storage::trie

#endif // KAGOME_STORAGE_TRIE_TYPES_HPP
4 changes: 2 additions & 2 deletions node/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@ int main(int argc, const char **argv) {
if (argc > 1) {
std::string_view name{argv[1]};
if (name == "storage-explorer") {
return storage_explorer_main(argc, argv + 1);
return storage_explorer_main(argc - 1, argv + 1);
}
if (name == "db-editor") {
return db_editor_main(argc, argv + 1);
return db_editor_main(argc - 1, argv + 1);
}
}

Expand Down
1 change: 0 additions & 1 deletion test/core/blockchain/block_tree_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -778,7 +778,6 @@ TEST_F(BlockTreeTest, GetBestChain_TwoChains) {
auto T_hash = addHeaderToRepository(kFinalizedBlockInfo.hash, 43);
auto A_hash = addHeaderToRepository(T_hash, 44);
auto B_hash = addHeaderToRepository(A_hash, 45);
auto C1_hash = addHeaderToRepository(B_hash, 46);
auto C2_hash = addHeaderToRepository(B_hash, 46);
auto D2_hash = addHeaderToRepository(C2_hash, 47);

Expand Down
2 changes: 0 additions & 2 deletions test/core/consensus/grandpa/vote_graph/fixture.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,6 @@ inline void AssertGraphCorrect(VoteGraphImpl &graph, std::string json) {
rapidjson::Document document;
document.Parse(json.c_str(), json.size());

auto gb = graph.getBase();
auto jb = jsonToBlockInfo(document);
ASSERT_EQ(graph.getBase(), jsonToBlockInfo(document)) << "base is incorrect";

auto gh_ = graph.getHeads();
Expand Down
Loading