From ef9d8ba8e11eda677a0122da4fc60cc0a84bfb18 Mon Sep 17 00:00:00 2001 From: Ruslan Tushov Date: Thu, 20 Apr 2023 10:34:59 +0300 Subject: [PATCH] Babe time (#1568) Signed-off-by: turuslan --- core/authorship/impl/proposer_impl.cpp | 16 +++-- core/authorship/impl/proposer_impl.hpp | 3 + core/authorship/proposer.hpp | 3 + core/consensus/babe/impl/babe_impl.cpp | 69 +++++++++++---------- core/consensus/babe/impl/babe_impl.hpp | 4 +- test/core/authorship/proposer_test.cpp | 48 +++++++++----- test/core/consensus/babe/babe_test.cpp | 2 +- test/mock/core/authorship/proposer_mock.hpp | 1 + 8 files changed, 93 insertions(+), 53 deletions(-) diff --git a/core/authorship/impl/proposer_impl.cpp b/core/authorship/impl/proposer_impl.cpp index eec394427c..0a341394db 100644 --- a/core/authorship/impl/proposer_impl.cpp +++ b/core/authorship/impl/proposer_impl.cpp @@ -16,12 +16,14 @@ namespace kagome::authorship { ProposerImpl::ProposerImpl( std::shared_ptr block_builder_factory, + std::shared_ptr clock, std::shared_ptr transaction_pool, std::shared_ptr ext_sub_engine, std::shared_ptr extrinsic_event_key_repo) : block_builder_factory_{std::move(block_builder_factory)}, + clock_{std::move(clock)}, transaction_pool_{std::move(transaction_pool)}, ext_sub_engine_{std::move(ext_sub_engine)}, extrinsic_event_key_repo_{std::move(extrinsic_event_key_repo)} { @@ -39,6 +41,7 @@ namespace kagome::authorship { outcome::result ProposerImpl::propose( const primitives::BlockInfo &parent_block, + std::optional deadline, const primitives::InherentData &inherent_data, const primitives::Digest &inherent_digest, TrieChangesTrackerOpt changes_tracker) { @@ -100,10 +103,14 @@ namespace kagome::authorship { // number of transactions to be pushed to the block size_t included_tx_count = 0; + std::vector included_hashes; for (const auto &[hash, tx] : ready_txs) { - const auto &tx_ref = tx; + if (deadline && clock_->now() >= deadline) { + break; + } + scale::ScaleEncoderStream s(true); - s << tx_ref->ext; + s << tx->ext; auto estimate_tx_size = s.size(); if (block_size + estimate_tx_size > block_size_limit) { @@ -121,7 +128,7 @@ namespace kagome::authorship { break; } - SL_DEBUG(logger_, "Adding extrinsic: {}", tx_ref->ext.data); + SL_DEBUG(logger_, "Adding extrinsic: {}", tx->ext.data); auto inserted_res = block_builder->pushExtrinsic(tx->ext); if (not inserted_res) { if (BlockBuilderError::EXHAUSTS_RESOURCES == inserted_res.error()) { @@ -144,6 +151,7 @@ namespace kagome::authorship { block_size += estimate_tx_size; transaction_pushed = true; ++included_tx_count; + included_hashes.emplace_back(hash); } } metric_tx_included_in_block_->set(included_tx_count); @@ -156,7 +164,7 @@ namespace kagome::authorship { OUTCOME_TRY(block, block_builder->bake()); - for (const auto &[hash, tx] : ready_txs) { + for (const auto &hash : included_hashes) { auto removed_res = transaction_pool_->removeOne(hash); if (not removed_res) { logger_->error( diff --git a/core/authorship/impl/proposer_impl.hpp b/core/authorship/impl/proposer_impl.hpp index 145a56637f..3539ca22db 100644 --- a/core/authorship/impl/proposer_impl.hpp +++ b/core/authorship/impl/proposer_impl.hpp @@ -30,6 +30,7 @@ namespace kagome::authorship { ProposerImpl( std::shared_ptr block_builder_factory, + std::shared_ptr clock, std::shared_ptr transaction_pool, std::shared_ptr ext_sub_engine, @@ -38,12 +39,14 @@ namespace kagome::authorship { outcome::result propose( const primitives::BlockInfo &parent_block, + std::optional deadline, const primitives::InherentData &inherent_data, const primitives::Digest &inherent_digest, TrieChangesTrackerOpt changes_tracker) override; private: std::shared_ptr block_builder_factory_; + std::shared_ptr clock_; std::shared_ptr transaction_pool_; std::shared_ptr ext_sub_engine_; diff --git a/core/authorship/proposer.hpp b/core/authorship/proposer.hpp index 05826a8fe1..331a558b87 100644 --- a/core/authorship/proposer.hpp +++ b/core/authorship/proposer.hpp @@ -20,6 +20,8 @@ namespace kagome::authorship { */ class Proposer { public: + using Clock = clock::SystemClock; + virtual ~Proposer() = default; /** @@ -31,6 +33,7 @@ namespace kagome::authorship { */ virtual outcome::result propose( const primitives::BlockInfo &parent_block, + std::optional deadline, const primitives::InherentData &inherent_data, const primitives::Digest &inherent_digest, TrieChangesTrackerOpt changes_tracker) = 0; diff --git a/core/consensus/babe/impl/babe_impl.cpp b/core/consensus/babe/impl/babe_impl.cpp index b04aa0e019..2a44a947dc 100644 --- a/core/consensus/babe/impl/babe_impl.cpp +++ b/core/consensus/babe/impl/babe_impl.cpp @@ -259,10 +259,10 @@ namespace kagome::consensus::babe { * @param authority_key authority * @return index of authority in list of authorities */ - std::optional getAuthorityIndex( + std::optional getAuthorityIndex( const primitives::AuthorityList &authorities, const primitives::BabeSessionKey &authority_key) { - uint64_t n = 0; + primitives::AuthorityIndex n = 0; for (auto &authority : authorities) { if (authority.id.id == authority_key) { return n; @@ -665,11 +665,12 @@ namespace kagome::consensus::babe { bool rewind_slots; // NOLINT auto slot = current_slot_; + clock::SystemClock::TimePoint now; do { // check that we are really in the middle of the slot, as expected; we // can cooperate with a relatively little (kMaxLatency) latency, as our // node will be able to retrieve - auto now = clock_->now(); + now = clock_->now(); auto finish_time = babe_util_->slotFinishTime(current_slot_); @@ -693,9 +694,9 @@ namespace kagome::consensus::babe { } } while (rewind_slots); - // Slot processing begins in 1/3 slot time before end - auto finish_time = babe_util_->slotFinishTime(current_slot_) - - babe_config_repo_->slotDuration() / 3; + // Slot processing begins in 1/3 slot time after start + auto finish_time = babe_util_->slotStartTime(current_slot_) + + babe_config_repo_->slotDuration() / 3; SL_VERBOSE(log_, "Starting a slot {} in epoch {} (remains {:.2f} sec.)", @@ -708,16 +709,16 @@ namespace kagome::consensus::babe { // everything is OK: wait for the end of the slot timer_->expiresAt(finish_time); - timer_->asyncWait([&](auto &&ec) { + timer_->asyncWait([this, now](auto &&ec) { if (ec) { log_->error("error happened while waiting on the timer: {}", ec); return; } - processSlot(); + processSlot(now); }); } - void BabeImpl::processSlot() { + void BabeImpl::processSlot(clock::SystemClock::TimePoint slot_timestamp) { BOOST_ASSERT(keypair_ != nullptr); best_block_ = block_tree_->bestLeaf(); @@ -763,7 +764,7 @@ namespace kagome::consensus::babe { const auto &authority_index = authority_index_res.value(); if (lottery_->getEpoch() != current_epoch_) { - changeLotteryEpoch(current_epoch_, babe_config); + changeLotteryEpoch(current_epoch_, authority_index, babe_config); } auto slot_leadership = lottery_->getSlotLeadership(current_slot_); @@ -777,8 +778,10 @@ namespace kagome::consensus::babe { common::Buffer(vrf_result.output), common::Buffer(vrf_result.proof)); - processSlotLeadership( - SlotType::Primary, std::cref(vrf_result), authority_index); + processSlotLeadership(SlotType::Primary, + slot_timestamp, + std::cref(vrf_result), + authority_index); } else if (babe_config.allowed_slots == primitives::AllowedSlots::PrimaryAndSecondaryPlain or babe_config.allowed_slots @@ -800,16 +803,20 @@ namespace kagome::consensus::babe { common::Buffer(vrf.output), common::Buffer(vrf.proof)); - processSlotLeadership( - SlotType::SecondaryVRF, std::cref(vrf), authority_index); + processSlotLeadership(SlotType::SecondaryVRF, + slot_timestamp, + std::cref(vrf), + authority_index); } else { // plain secondary slots mode SL_DEBUG( log_, "Babe author {} is block producer in secondary plain slot", keypair_->public_key); - processSlotLeadership( - SlotType::SecondaryPlain, std::nullopt, authority_index); + processSlotLeadership(SlotType::SecondaryPlain, + slot_timestamp, + std::nullopt, + authority_index); } } else { SL_TRACE(log_, @@ -848,7 +855,7 @@ namespace kagome::consensus::babe { // everything is OK: wait for the end of the slot timer_->expiresAt(start_time); - timer_->asyncWait([&](auto &&ec) { + timer_->asyncWait([this](auto &&ec) { if (ec) { log_->error("error happened while waiting on the timer: {}", ec); return; @@ -912,6 +919,7 @@ namespace kagome::consensus::babe { void BabeImpl::processSlotLeadership( SlotType slot_type, + clock::SystemClock::TimePoint slot_timestamp, std::optional> output, primitives::AuthorityIndex authority_index) { BOOST_ASSERT(keypair_ != nullptr); @@ -930,7 +938,7 @@ namespace kagome::consensus::babe { primitives::InherentData inherent_data; auto now = std::chrono::duration_cast( - clock_->now().time_since_epoch()) + slot_timestamp.time_since_epoch()) .count(); if (auto res = inherent_data.putData(kTimestampId, now); @@ -986,8 +994,13 @@ namespace kagome::consensus::babe { std::make_shared(); // create new block - auto pre_seal_block_res = proposer_->propose( - best_block_, inherent_data, {babe_pre_digest}, changes_tracker); + auto pre_seal_block_res = + proposer_->propose(best_block_, + babe_util_->slotFinishTime(current_slot_) + - babe_config_repo_->slotDuration() / 3, + inherent_data, + {babe_pre_digest}, + changes_tracker); if (!pre_seal_block_res) { SL_ERROR(log_, "Cannot propose a block: {}", pre_seal_block_res.error()); return; @@ -1121,22 +1134,12 @@ namespace kagome::consensus::babe { void BabeImpl::changeLotteryEpoch( const EpochDescriptor &epoch, + primitives::AuthorityIndex authority_index, const primitives::BabeConfiguration &babe_config) const { BOOST_ASSERT(keypair_ != nullptr); - auto authority_index_res = - getAuthorityIndex(babe_config.authorities, keypair_->public_key); - if (not authority_index_res) { - SL_CRITICAL(log_, - "Block production failed: This node is not in the list of " - "authorities. (public key: {})", - keypair_->public_key); - return; - } - - auto threshold = calculateThreshold(babe_config.leadership_rate, - babe_config.authorities, - authority_index_res.value()); + auto threshold = calculateThreshold( + babe_config.leadership_rate, babe_config.authorities, authority_index); lottery_->changeEpoch(epoch, babe_config.randomness, threshold, *keypair_); } diff --git a/core/consensus/babe/impl/babe_impl.hpp b/core/consensus/babe/impl/babe_impl.hpp index 6a11abec9b..04ef86e965 100644 --- a/core/consensus/babe/impl/babe_impl.hpp +++ b/core/consensus/babe/impl/babe_impl.hpp @@ -153,7 +153,7 @@ namespace kagome::consensus::babe { /** * Process the current Babe slot */ - void processSlot(); + void processSlot(clock::SystemClock::TimePoint slot_timestamp); /** * Gather block and broadcast it @@ -163,6 +163,7 @@ namespace kagome::consensus::babe { */ void processSlotLeadership( SlotType slot_type, + clock::SystemClock::TimePoint slot_timestamp, std::optional> output, primitives::AuthorityIndex authority_index); @@ -173,6 +174,7 @@ namespace kagome::consensus::babe { void changeLotteryEpoch( const EpochDescriptor &epoch, + primitives::AuthorityIndex authority_index, const primitives::BabeConfiguration &babe_config) const; outcome::result babePreDigest( diff --git a/test/core/authorship/proposer_test.cpp b/test/core/authorship/proposer_test.cpp index a7309ade13..286effc976 100644 --- a/test/core/authorship/proposer_test.cpp +++ b/test/core/authorship/proposer_test.cpp @@ -10,6 +10,7 @@ #include "authorship/impl/block_builder_error.hpp" #include "mock/core/authorship/block_builder_factory_mock.hpp" #include "mock/core/authorship/block_builder_mock.hpp" +#include "mock/core/clock/clock_mock.hpp" #include "mock/core/runtime/block_builder_api_mock.hpp" #include "mock/core/transaction_pool/transaction_pool_mock.hpp" #include "primitives/event_types.hpp" @@ -29,6 +30,7 @@ using kagome::authorship::BlockBuilderError; using kagome::authorship::BlockBuilderFactoryMock; using kagome::authorship::BlockBuilderMock; using kagome::authorship::ProposerImpl; +using kagome::clock::SystemClockMock; using kagome::common::Buffer; using kagome::primitives::Block; using kagome::primitives::BlockId; @@ -78,6 +80,7 @@ class ProposerTest : public ::testing::Test { protected: std::shared_ptr block_builder_factory_ = std::make_shared(); + std::shared_ptr clock_ = std::make_shared(); std::shared_ptr transaction_pool_ = std::make_shared(); std::shared_ptr extrinsic_sub_engine_ = @@ -88,6 +91,7 @@ class ProposerTest : public ::testing::Test { BlockBuilderMock *block_builder_; ProposerImpl proposer_{block_builder_factory_, + clock_, transaction_pool_, extrinsic_sub_engine_, extrinsic_event_key_repo_}; @@ -135,8 +139,11 @@ TEST_F(ProposerTest, CreateBlockSuccess) { EXPECT_CALL(*block_builder_, bake()).WillOnce(Return(expected_block)); // when - auto block_res = proposer_.propose( - expected_block_, inherent_data_, inherent_digests_, std::nullopt); + auto block_res = proposer_.propose(expected_block_, + std::nullopt, + inherent_data_, + inherent_digests_, + std::nullopt); // then ASSERT_TRUE(block_res); @@ -158,8 +165,11 @@ TEST_F(ProposerTest, CreateBlockFailsWhenXtNotPushed) { .WillOnce(Return(outcome::failure(BlockBuilderError::BAD_MANDATORY))); // when - auto block_res = proposer_.propose( - expected_block_, inherent_data_, inherent_digests_, std::nullopt); + auto block_res = proposer_.propose(expected_block_, + std::nullopt, + inherent_data_, + inherent_digests_, + std::nullopt); // then ASSERT_FALSE(block_res); @@ -177,8 +187,11 @@ TEST_F(ProposerTest, CreateBlockFailsToGetInhetentExtr) { .WillOnce(Return(outcome::failure(boost::system::error_code{}))); // when - auto block_res = proposer_.propose( - expected_block_, inherent_data_, inherent_digests_, std::nullopt); + auto block_res = proposer_.propose(expected_block_, + std::nullopt, + inherent_data_, + inherent_digests_, + std::nullopt); // then ASSERT_FALSE(block_res); @@ -210,16 +223,17 @@ TEST_F(ProposerTest, PushFailed) { std::map> ready_transactions{ std::make_pair("fakeHash"_hash256, std::make_shared())}; - EXPECT_CALL(*transaction_pool_, removeOne("fakeHash"_hash256)) - .WillOnce(Return(Transaction{})); EXPECT_CALL(*transaction_pool_, getReadyTransactions()) .WillOnce(Return(ready_transactions)); EXPECT_CALL(*transaction_pool_, removeStale(BlockId(expected_block_.number))) .WillOnce(Return(outcome::success())); // when - auto block_res = proposer_.propose( - expected_block_, inherent_data_, inherent_digests_, std::nullopt); + auto block_res = proposer_.propose(expected_block_, + std::nullopt, + inherent_data_, + inherent_digests_, + std::nullopt); // then ASSERT_TRUE(block_res); @@ -264,8 +278,11 @@ TEST_F(ProposerTest, TrxSkippedDueToOverflow) { .WillRepeatedly(Return(outcome::success())); // when - auto block_res = proposer_.propose( - expected_block_, inherent_data_, inherent_digests_, std::nullopt); + auto block_res = proposer_.propose(expected_block_, + std::nullopt, + inherent_data_, + inherent_digests_, + std::nullopt); // then ASSERT_TRUE(block_res); @@ -310,8 +327,11 @@ TEST_F(ProposerTest, TrxSkippedDueToResourceExhausted) { .WillRepeatedly(Return(outcome::success())); // when - auto block_res = proposer_.propose( - expected_block_, inherent_data_, inherent_digests_, std::nullopt); + auto block_res = proposer_.propose(expected_block_, + std::nullopt, + inherent_data_, + inherent_digests_, + std::nullopt); // then ASSERT_TRUE(block_res); diff --git a/test/core/consensus/babe/babe_test.cpp b/test/core/consensus/babe/babe_test.cpp index 7f3c2696dc..ff72086a14 100644 --- a/test/core/consensus/babe/babe_test.cpp +++ b/test/core/consensus/babe/babe_test.cpp @@ -341,7 +341,7 @@ TEST_F(BabeTest, Success) { EXPECT_CALL(*block_tree_, getBlockHeader(created_block_hash_)) .WillRepeatedly(Return(outcome::success(block_header_))); - EXPECT_CALL(*proposer_, propose(best_leaf, _, _, _)) + EXPECT_CALL(*proposer_, propose(best_leaf, _, _, _, _)) .WillOnce(Return(created_block_)); EXPECT_CALL(*hasher_, blake2b_256(_)) diff --git a/test/mock/core/authorship/proposer_mock.hpp b/test/mock/core/authorship/proposer_mock.hpp index 5a8aa59f09..abddb685d3 100644 --- a/test/mock/core/authorship/proposer_mock.hpp +++ b/test/mock/core/authorship/proposer_mock.hpp @@ -16,6 +16,7 @@ namespace kagome::authorship { MOCK_METHOD(outcome::result, propose, (const primitives::BlockInfo &, + std::optional, const primitives::InherentData &, const primitives::Digest &, TrieChangesTrackerOpt),