Skip to content

Commit

Permalink
more renamings
Browse files Browse the repository at this point in the history
  • Loading branch information
yperbasis committed Sep 17, 2024
1 parent c5d810d commit a61ef0e
Show file tree
Hide file tree
Showing 8 changed files with 70 additions and 70 deletions.
18 changes: 9 additions & 9 deletions silkworm/infra/grpc/client/call_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,17 +84,17 @@ struct CallTest : public silkworm::test_util::ContextTestBase {
std::unique_ptr<StrictMockKVStub> stub_{std::make_unique<StrictMockKVStub>()};

//! Mocked reader for Version unary RPC of gRPC KV interface
std::unique_ptr<StrictMockKVVersionAsyncResponseReader> version_reader_ptr_{
std::unique_ptr<StrictMockKVVersionAsyncResponseReader> version_reader_ptr{
std::make_unique<StrictMockKVVersionAsyncResponseReader>()};
StrictMockKVVersionAsyncResponseReader& version_reader_{*version_reader_ptr_};
StrictMockKVVersionAsyncResponseReader& version_reader{*version_reader_ptr};
};

TEST_CASE_METHOD(CallTest, "Unary gRPC threading: unary_rpc", "[grpc][client]") {
// Set the call expectations:
// 1. remote::KV::StubInterface::AsyncVersionRaw call succeeds
EXPECT_CALL(*stub_, AsyncVersionRaw).WillOnce(Return(version_reader_ptr_.get()));
EXPECT_CALL(*stub_, AsyncVersionRaw).WillOnce(Return(version_reader_ptr.get()));
// 2. AsyncResponseReader<types::VersionReply>::Finish call succeeds w/ status OK
EXPECT_CALL(version_reader_, Finish).WillOnce(test::finish_ok(grpc_context_));
EXPECT_CALL(version_reader, Finish).WillOnce(test::finish_ok(grpc_context_));

// Trick necessary because expectations require MockKVStub, whilst production code wants remote::KV::StubInterface
std::unique_ptr<proto::KV::StubInterface> stub{std::move(stub_)};
Expand All @@ -106,11 +106,11 @@ TEST_CASE_METHOD(CallTest, "Unary gRPC threading: unary_rpc", "[grpc][client]")
TEST_CASE_METHOD(CallTest, "Unary gRPC threading: agrpc::ClientRPC", "[grpc][client]") {
// Set the call expectations:
// 1. remote::KV::StubInterface::PrepareAsyncVersionRaw call succeeds
EXPECT_CALL(*stub_, PrepareAsyncVersionRaw).WillOnce(Return(version_reader_ptr_.get()));
EXPECT_CALL(*stub_, PrepareAsyncVersionRaw).WillOnce(Return(version_reader_ptr.get()));
// 2. AsyncResponseReader<types::VersionReply>::StartCall call succeeds
EXPECT_CALL(version_reader_, StartCall).WillOnce([&]() {});
EXPECT_CALL(version_reader, StartCall).WillOnce([&]() {});
// 3. AsyncResponseReader<types::VersionReply>::Finish call succeeds w/ status OK
EXPECT_CALL(version_reader_, Finish).WillOnce(test::finish_ok(grpc_context_));
EXPECT_CALL(version_reader, Finish).WillOnce(test::finish_ok(grpc_context_));

// Execute the test: check threading assumptions during async Version RPC execution
spawn_and_wait(check_unary_agrpc_client_threading(*stub_, google::protobuf::Empty{}, grpc_context_));
Expand All @@ -119,14 +119,14 @@ TEST_CASE_METHOD(CallTest, "Unary gRPC threading: agrpc::ClientRPC", "[grpc][cli
TEST_CASE_METHOD(CallTest, "Unary gRPC cancelling: unary_rpc", "[grpc][client]") {
// Set the call expectations:
// 1. remote::KV::StubInterface::AsyncVersionRaw call succeeds
EXPECT_CALL(*stub_, AsyncVersionRaw).WillOnce(Return(version_reader_ptr_.get()));
EXPECT_CALL(*stub_, AsyncVersionRaw).WillOnce(Return(version_reader_ptr.get()));
// 2. AsyncResponseReader<types::VersionReply>::Finish call fails w/ status CANCELLED
boost::asio::cancellation_signal cancellation_signal;
auto cancellation_slot = cancellation_signal.slot();

// Trick: we use Finish as a hook to signal cancellation, but then we must trigger tag by hand anyway
// Better solution possible with agrpc::ClientRPC: use StartCall as a hook to signal cancellation
EXPECT_CALL(version_reader_, Finish).WillOnce([&](auto&&, ::grpc::Status* status, void* tag) {
EXPECT_CALL(version_reader, Finish).WillOnce([&](auto&&, ::grpc::Status* status, void* tag) {
cancellation_signal.emit(boost::asio::cancellation_type::all);
*status = ::grpc::Status::CANCELLED;
agrpc::process_grpc_tag(grpc_context_, tag, /*ok=*/true);
Expand Down
18 changes: 9 additions & 9 deletions silkworm/sync/internals/chain_elements.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,14 @@ namespace silkworm {
// A link corresponds to a block header, links are connected to each other by reverse of parent_hash relation
struct Link {
std::shared_ptr<BlockHeader> header; // Header to which this link point to
BlockNum blockHeight = 0; // Block height of the header, repeated here for convenience (remove?)
BlockNum block_height = 0; // Block height of the header, repeated here for convenience (remove?)
Hash hash; // Hash of the header
std::vector<std::shared_ptr<Link>> next; // Reverse of parent_hash,allows iter.over links in asc. block height order
bool persisted = false; // Whether this link comes from the database record
bool preverified = false; // Ancestor of pre-verified header

Link(BlockHeader h, bool persisted_)
: blockHeight{h.number},
: block_height{h.number},
hash{h.hash()}, // save computation
persisted{persisted_} {
header = std::make_shared<BlockHeader>(std::move(h));
Expand Down Expand Up @@ -106,15 +106,15 @@ struct Anchor {
// Binary relations to use in priority queues
struct LinkOlderThan : public std::function<bool(std::shared_ptr<Link>, std::shared_ptr<Link>)> {
bool operator()(const std::shared_ptr<Link>& x, const std::shared_ptr<Link>& y) const {
return x->blockHeight != y->blockHeight ? x->blockHeight < y->blockHeight : // cause ordering
x < y; // preserve identity
return x->block_height != y->block_height ? x->block_height < y->block_height : // cause ordering
x < y; // preserve identity
}
};

struct LinkYoungerThan : public std::function<bool(std::shared_ptr<Link>, std::shared_ptr<Link>)> {
bool operator()(const std::shared_ptr<Link>& x, const std::shared_ptr<Link>& y) const {
return x->blockHeight != y->blockHeight ? x->blockHeight > y->blockHeight : // cause ordering
x > y; // preserve identity
return x->block_height != y->block_height ? x->block_height > y->block_height : // cause ordering
x > y; // preserve identity
}
};

Expand Down Expand Up @@ -160,9 +160,9 @@ struct BlockOlderThan : public std::function<bool(BlockNum, BlockNum)> {

} // namespace silkworm
template <>
struct MbpqKey<std::shared_ptr<silkworm::Link>> { // extract key type and value
using type = silkworm::BlockNum; // type of the key
static type value(const std::shared_ptr<silkworm::Link>& link) { return link->blockHeight; } // value of the key
struct MbpqKey<std::shared_ptr<silkworm::Link>> { // extract key type and value
using type = silkworm::BlockNum; // type of the key
static type value(const std::shared_ptr<silkworm::Link>& link) { return link->block_height; } // value of the key
};
namespace silkworm { // reopen namespace

Expand Down
8 changes: 4 additions & 4 deletions silkworm/sync/internals/chain_elements_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,15 @@ TEST_CASE("links") {

SECTION("construction") {
REQUIRE(*(link1.header) == headers[1]);
REQUIRE(link1.blockHeight == headers[1].number);
REQUIRE(link1.block_height == headers[1].number);
REQUIRE(link1.hash == headers[1].hash());
REQUIRE(link1.persisted == persisted);
REQUIRE(link1.preverified == false);
REQUIRE(link1.next.empty());

headers[1].number = 100; // only for the following test
REQUIRE(link1.blockHeight == 1); // link1 has a copy of headers[1]
headers[1].number = 1; // ok
headers[1].number = 100; // only for the following test
REQUIRE(link1.block_height == 1); // link1 has a copy of headers[1]
headers[1].number = 1; // ok
}

SECTION("children") {
Expand Down
32 changes: 16 additions & 16 deletions silkworm/sync/internals/header_chain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ Headers HeaderChain::withdraw_stable_headers() {
assessing_list.pop();

// If it is in the pre-verified headers range do not verify it, wait for pre-verification
if (link->blockHeight <= last_preverified_hash_ && !link->preverified) {
if (link->block_height <= last_preverified_hash_ && !link->preverified) {
insert_list_.push(link);
continue; // header should be pre-verified, but not yet, try again later
}
Expand All @@ -171,14 +171,14 @@ Headers HeaderChain::withdraw_stable_headers() {
if (assessment == kPostpone) {
insert_list_.push(link);
log::Warning() << "HeaderChain: added future link,"
<< " hash=" << link->hash << " height=" << link->blockHeight
<< " hash=" << link->hash << " height=" << link->block_height
<< " timestamp=" << link->header->timestamp << ")";
continue;
}

if (assessment == kSkip) {
links_.erase(link->hash);
log::Warning() << "HeaderChain: skipping link at " << link->blockHeight;
log::Warning() << "HeaderChain: skipping link at " << link->block_height;
continue; // todo: do we need to invalidate all the descendants?
}

Expand All @@ -187,15 +187,15 @@ Headers HeaderChain::withdraw_stable_headers() {
// If we received an announcement for this header we must propagate it
if (seen_announces_.get(link->hash)) {
seen_announces_.remove(link->hash);
announces_to_do_.push_back({link->hash, link->blockHeight});
announces_to_do_.push_back({link->hash, link->block_height});
}

// Insert in the list of headers to persist
stable_headers.push_back(link->header); // will be persisted by HeaderPersistence

// Update persisted height, and state
if (link->blockHeight > highest_in_db_) {
highest_in_db_ = link->blockHeight;
if (link->block_height > highest_in_db_) {
highest_in_db_ = link->block_height;
}
link->persisted = true;
persisted_link_queue_.push(link);
Expand Down Expand Up @@ -811,7 +811,7 @@ std::tuple<std::optional<std::shared_ptr<Anchor>>, HeaderChain::DeepLink> Header
if (a == anchors_.end()) {
log::Trace()
<< "[ERROR] HeaderChain: segment cut&paste error, segment without anchor or persisted attach point, "
<< "starting bn=" << link->blockHeight << " ending bn=" << parent_link->blockHeight << " "
<< "starting bn=" << link->block_height << " ending bn=" << parent_link->block_height << " "
<< "parent=" << to_hex(parent_link->header->parent_hash);
return {std::nullopt, parent_link}; // wrong, invariant violation, no anchor but there should be
}
Expand Down Expand Up @@ -865,9 +865,9 @@ void HeaderChain::connect(const std::shared_ptr<Link>& attachment_link, Segment:
log::Trace() << "[INFO] HeaderChain, segment op: "
<< (deep_a.has_value()
? "A " + to_string(deep_a.value()->block_height)
: "X " + to_string(deep_link->blockHeight) + (deep_link->persisted ? " (P)" : " (!P)"))
<< " --- " << attachment_link->blockHeight << (attachment_link->preverified ? " (V)" : "")
<< " <-connect-> " << segment_slice.rbegin()->operator*().number << " --- " << prev_link->blockHeight
: "X " + to_string(deep_link->block_height) + (deep_link->persisted ? " (P)" : " (!P)"))
<< " --- " << attachment_link->block_height << (attachment_link->preverified ? " (V)" : "")
<< " <-connect-> " << segment_slice.rbegin()->operator*().number << " --- " << prev_link->block_height
<< " <-connect-> " << anchor->block_height << " --- " << anchor->last_link_height
<< (anchor_preverified ? " (V)" : "");
}
Expand Down Expand Up @@ -906,7 +906,7 @@ HeaderChain::RequestMoreHeaders HeaderChain::extend_down(Segment::Slice segment_
std::ranges::any_of(new_anchor->links, [](const auto& link) -> bool { return link->preverified; });

log::Trace() << "[INFO] HeaderChain, segment op: " << new_anchor->block_height
<< (newanchor_preverified ? " (V)" : "") << " --- " << prev_link->blockHeight << " <-extend down "
<< (newanchor_preverified ? " (V)" : "") << " --- " << prev_link->block_height << " <-extend down "
<< anchor->block_height << " --- " << anchor->last_link_height << (anchor_preverified ? " (V)" : "");

return !pre_existing;
Expand All @@ -920,7 +920,7 @@ void HeaderChain::extend_up(const std::shared_ptr<Link>& attachment_link, Segmen
throw SegmentCutAndPasteError(
"connection to bad headers,"
" height=" +
std::to_string(attachment_link->blockHeight) +
std::to_string(attachment_link->block_height) +
" hash=" + to_hex(attachment_link->hash));
}

Expand All @@ -939,16 +939,16 @@ void HeaderChain::extend_up(const std::shared_ptr<Link>& attachment_link, Segmen
auto [deep_a, deep_link] = find_anchor(attachment_link);
if (deep_a.has_value()) {
auto deepest_anchor = deep_a.value();
deepest_anchor->last_link_height = std::max(deepest_anchor->last_link_height, prev_link->blockHeight);
deepest_anchor->last_link_height = std::max(deepest_anchor->last_link_height, prev_link->block_height);
} else {
// if (!deep_link->persisted) error, else attachment to special anchor
}

log::Trace() << "[INFO] HeaderChain, segment op: "
<< (deep_a.has_value()
? "A " + to_string(deep_a.value()->block_height)
: "X " + to_string(deep_link->blockHeight) + (deep_link->persisted ? " (P)" : " (!P)"))
<< " --- " << attachment_link->blockHeight << (attachment_link->preverified ? " (V)" : "")
: "X " + to_string(deep_link->block_height) + (deep_link->persisted ? " (P)" : " (!P)"))
<< " --- " << attachment_link->block_height << (attachment_link->preverified ? " (V)" : "")
<< " extend up-> " << segment_slice.rbegin()->operator*().number << " --- "
<< (segment_slice.rend() - 1)->operator*().number;
}
Expand All @@ -974,7 +974,7 @@ HeaderChain::RequestMoreHeaders HeaderChain::new_anchor(Segment::Slice segment_s
if (preverified_hashes_.contains(link->hash)) mark_as_preverified(link);
}

anchor->last_link_height = std::max(anchor->last_link_height, prev_link->blockHeight);
anchor->last_link_height = std::max(anchor->last_link_height, prev_link->block_height);

bool anchor_preverified =
std::ranges::any_of(anchor->links, [](const auto& link) -> bool { return link->preverified; });
Expand Down
4 changes: 2 additions & 2 deletions silkworm/sync/internals/header_chain_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1408,7 +1408,7 @@ TEST_CASE("HeaderChain: (8) sibling with anchor invalidation and links reduction

auto link5b = chain.links_[h5p.hash()];
REQUIRE(link5b != nullptr);
REQUIRE(link5b->blockHeight == 5);
REQUIRE(link5b->block_height == 5);
REQUIRE(link5b->hash == h5p.hash());

auto anchor2 = chain.anchors_[headers[3].parent_hash];
Expand Down Expand Up @@ -1454,7 +1454,7 @@ TEST_CASE("HeaderChain: (8) sibling with anchor invalidation and links reduction

auto link5b = chain.links_[h5p.hash()];
REQUIRE(link5b != nullptr);
REQUIRE(link5b->blockHeight == 5);
REQUIRE(link5b->block_height == 5);
REQUIRE(link5b->hash == h5p.hash());

auto anchor2 = chain.anchors_[headers[3].parent_hash];
Expand Down
2 changes: 1 addition & 1 deletion silkworm/sync/internals/header_only_state.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ std::optional<BlockHeader> CustomHeaderOnlyChainState::read_header(BlockNum bloc
auto [initial_link, final_link] = persisted_link_queue_.equal_range(block_number);

for (auto link = initial_link; link != final_link; link++) {
if (link->second->blockHeight == block_number && link->second->hash == hash) {
if (link->second->block_height == block_number && link->second->hash == hash) {
return *link->second->header;
}
}
Expand Down
2 changes: 1 addition & 1 deletion silkworm/sync/internals/priority_queue.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ class SetBasedPriorityQueue { // use boost::priority_queue instead?
* template <>
* struct key<Link> {
* using type = BlockNum;
* static BlockNum value(const Link& l) {return l.blockHeight;}
* static BlockNum value(const Link& l) {return l.block_height;}
* };
*
* MapBasedPriorityQueue<Link, BlockOlderThan> queue;
Expand Down
Loading

0 comments on commit a61ef0e

Please sign in to comment.