From 18554b8f0086c7412a2fb6eb573ccdc6a3e41390 Mon Sep 17 00:00:00 2001 From: Boris Erakhtin Date: Thu, 5 Dec 2024 18:21:36 +0500 Subject: [PATCH] tryGetBlockHeader for BlockStorage --- core/blockchain/block_storage.hpp | 13 ++++++- core/blockchain/impl/block_storage_impl.cpp | 37 +++++++++++++------ core/blockchain/impl/block_storage_impl.hpp | 6 +++ test/core/blockchain/block_storage_test.cpp | 30 +++++++++++++++ .../core/blockchain/block_storage_mock.hpp | 5 +++ 5 files changed, 78 insertions(+), 13 deletions(-) diff --git a/core/blockchain/block_storage.hpp b/core/blockchain/block_storage.hpp index 975636eee5..5c0b447473 100644 --- a/core/blockchain/block_storage.hpp +++ b/core/blockchain/block_storage.hpp @@ -93,8 +93,17 @@ namespace kagome::blockchain { * Tries to get block header by {@param block_hash} * @returns block header or error */ - virtual outcome::result - getBlockHeader(const primitives::BlockHash &block_hash) const = 0; + virtual outcome::result getBlockHeader( + const primitives::BlockHash &block_hash) const = 0; + + /** + * Attempts to retrieve the block header for the given {@param block_hash}. + * @param block_hash The hash of the block whose header is to be retrieved. + * @returns An optional containing the block header if found, std::nullopt + * if not found, or an error if the operation fails. + */ + virtual outcome::result> + tryGetBlockHeader(const primitives::BlockHash &block_hash) const = 0; // -- body -- diff --git a/core/blockchain/impl/block_storage_impl.cpp b/core/blockchain/impl/block_storage_impl.cpp index 666e553bb3..63a34460b6 100644 --- a/core/blockchain/impl/block_storage_impl.cpp +++ b/core/blockchain/impl/block_storage_impl.cpp @@ -201,18 +201,19 @@ namespace kagome::blockchain { outcome::result BlockStorageImpl::getBlockHeader( const primitives::BlockHash &block_hash) const { - OUTCOME_TRY(encoded_header_opt, - getFromSpace(*storage_, Space::kHeader, block_hash)); - if (encoded_header_opt.has_value()) { - OUTCOME_TRY( - header, - scale::decode(encoded_header_opt.value())); - header.hash_opt.emplace(block_hash); - return header; + OUTCOME_TRY(header_opt, fetchBlockHeader(block_hash)); + if (header_opt.has_value()) { + return header_opt.value(); } return BlockStorageError::HEADER_NOT_FOUND; } + outcome::result> + BlockStorageImpl::tryGetBlockHeader( + const primitives::BlockHash &block_hash) const { + return fetchBlockHeader(block_hash); + } + outcome::result BlockStorageImpl::putBlockBody( const primitives::BlockHash &block_hash, const primitives::BlockBody &block_body) { @@ -320,11 +321,11 @@ namespace kagome::blockchain { outcome::result BlockStorageImpl::removeBlock( const primitives::BlockHash &block_hash) { // Check if block still in storage - const auto header_res = getBlockHeader(block_hash); - if (not header_res) { + OUTCOME_TRY(header_opt, fetchBlockHeader(block_hash)); + if (not header_opt) { return outcome::success(); } - const auto &header = header_res.value(); + const auto &header = header_opt.value(); primitives::BlockInfo block_info(header.number, block_hash); @@ -385,4 +386,18 @@ namespace kagome::blockchain { return outcome::success(); } + outcome::result> + BlockStorageImpl::fetchBlockHeader( + const primitives::BlockHash &block_hash) const { + OUTCOME_TRY(encoded_header_opt, + getFromSpace(*storage_, Space::kHeader, block_hash)); + if (encoded_header_opt.has_value()) { + OUTCOME_TRY( + header, + scale::decode(encoded_header_opt.value())); + header.hash_opt.emplace(block_hash); + return std::make_optional(std::move(header)); + } + return std::nullopt; + } } // namespace kagome::blockchain diff --git a/core/blockchain/impl/block_storage_impl.hpp b/core/blockchain/impl/block_storage_impl.hpp index 43445c7035..3f229dc713 100644 --- a/core/blockchain/impl/block_storage_impl.hpp +++ b/core/blockchain/impl/block_storage_impl.hpp @@ -63,6 +63,9 @@ namespace kagome::blockchain { outcome::result getBlockHeader( const primitives::BlockHash &block_hash) const override; + outcome::result> tryGetBlockHeader( + const primitives::BlockHash &block_hash) const override; + // -- body -- outcome::result putBlockBody( @@ -102,6 +105,9 @@ namespace kagome::blockchain { BlockStorageImpl(std::shared_ptr storage, std::shared_ptr hasher); + outcome::result> fetchBlockHeader( + const primitives::BlockHash &block_hash) const; + std::shared_ptr storage_; std::shared_ptr hasher_; diff --git a/test/core/blockchain/block_storage_test.cpp b/test/core/blockchain/block_storage_test.cpp index ec6dd94c9a..87ad0ae0ce 100644 --- a/test/core/blockchain/block_storage_test.cpp +++ b/test/core/blockchain/block_storage_test.cpp @@ -173,6 +173,36 @@ TEST_F(BlockStorageTest, PutBlock) { ASSERT_OUTCOME_SUCCESS_TRY(block_storage->putBlock(block)); } +/* + * @given a block storage and a block that is not in storage yet + * @when trying to get a block from the storage + * @then an error is returned + */ +TEST_F(BlockStorageTest, GetBlockNotFound) { + ASSERT_OUTCOME_SUCCESS( + block_storage, + BlockStorageImpl::create(root_hash, spaced_storage, hasher)); + + EXPECT_OUTCOME_ERROR(get_res, + block_storage->getBlockHeader(genesis_block_hash), + BlockStorageError::HEADER_NOT_FOUND); +} + +/* + * @given a block storage and a block that is not in storage yet + * @when trying to get a block from the storage + * @then success value containing nullopt is returned + */ +TEST_F(BlockStorageTest, TryGetBlockNotFound) { + ASSERT_OUTCOME_SUCCESS( + block_storage, + BlockStorageImpl::create(root_hash, spaced_storage, hasher)); + + ASSERT_OUTCOME_SUCCESS(try_get_res, + block_storage->tryGetBlockHeader(genesis_block_hash)); + ASSERT_FALSE(try_get_res.has_value()); +} + /** * @given a block storage and a block that is not in storage yet * @when putting a block in the storage and underlying storage throws an diff --git a/test/mock/core/blockchain/block_storage_mock.hpp b/test/mock/core/blockchain/block_storage_mock.hpp index 7031b13390..0d25e1d537 100644 --- a/test/mock/core/blockchain/block_storage_mock.hpp +++ b/test/mock/core/blockchain/block_storage_mock.hpp @@ -62,6 +62,11 @@ namespace kagome::blockchain { (const primitives::BlockHash &), (const, override)); + MOCK_METHOD(outcome::result>, + tryGetBlockHeader, + (const primitives::BlockHash &), + (const, override)); + MOCK_METHOD(outcome::result, putBlockBody, (const primitives::BlockHash &, const primitives::BlockBody &),