fix babe slot (#1711)

Signed-off-by: turuslan <turuslan.devbox@gmail.com>
qdrvm · Jul 27, 2023 · c40d94c · c40d94c
1 parent 28047c0
commit c40d94c
Show file tree

Hide file tree

Showing 8 changed files with 20 additions and 149 deletions.
diff --git a/core/consensus/babe/babe_util.hpp b/core/consensus/babe/babe_util.hpp
@@ -20,12 +20,8 @@ namespace kagome::consensus::babe {
    public:
     virtual ~BabeUtil() = default;
 
-    /**
-     * Init inner state by call {@param f} returning first block slot and flag
-     * if first block is already finalized
-     */
-    virtual BabeSlotNumber syncEpoch(
-        std::function<std::tuple<BabeSlotNumber, bool>()> &&f) = 0;
+    // TODO(turuslan): removed in https://github.com/soramitsu/kagome/pull/1700
+    virtual BabeSlotNumber getFirstBlockSlotNumber() = 0;
 
     /**
      * @returns current unix time slot number

diff --git a/core/consensus/babe/impl/babe_config_repository_impl.cpp b/core/consensus/babe/impl/babe_config_repository_impl.cpp
@@ -167,20 +167,6 @@ namespace kagome::consensus::babe {
     return epoch_length_;
   }
 
-  BabeSlotNumber BabeConfigRepositoryImpl::syncEpoch(
-      std::function<std::tuple<BabeSlotNumber, bool>()> &&f) {
-    if (not is_first_block_finalized_) {
-      auto [first_block_slot_number, is_first_block_finalized] = f();
-      first_block_slot_number_.emplace(first_block_slot_number);
-      is_first_block_finalized_ = is_first_block_finalized;
-      SL_TRACE(
-          logger_,
-          "Epoch beginning is synchronized: first block slot number is {} now",
-          first_block_slot_number_.value());
-    }
-    return first_block_slot_number_.value();
-  }
-
   BabeSlotNumber BabeConfigRepositoryImpl::getCurrentSlot() const {
     return static_cast<BabeSlotNumber>(clock_.now().time_since_epoch()
                                        / slotDuration());
@@ -216,6 +202,20 @@ namespace kagome::consensus::babe {
       return first_block_slot_number_.value();
     }
 
+    auto r = [&]() -> outcome::result<BabeSlotNumber> {
+      OUTCOME_TRY(hash, header_repo_->getHashByNumber(1));
+      OUTCOME_TRY(header, header_repo_->getBlockHeader(hash));
+      OUTCOME_TRY(digest, getBabeDigests(header));
+      auto &slot = digest.second.slot_number;
+      if (block_tree_->getLastFinalized().number != 0) {
+        first_block_slot_number_ = slot;
+      }
+      return slot;
+    }();
+    if (r) {
+      return r.value();
+    }
+
     return getCurrentSlot();
   }
 

diff --git a/core/consensus/babe/impl/babe_config_repository_impl.hpp b/core/consensus/babe/impl/babe_config_repository_impl.hpp
@@ -93,8 +93,7 @@ namespace kagome::consensus::babe {
 
     // BabeUtil
 
-    BabeSlotNumber syncEpoch(
-        std::function<std::tuple<BabeSlotNumber, bool>()> &&f) override;
+    BabeSlotNumber getFirstBlockSlotNumber() override;
 
     BabeSlotNumber getCurrentSlot() const override;
 
@@ -109,8 +108,6 @@ namespace kagome::consensus::babe {
     void warp(const primitives::BlockInfo &block) override;
 
    private:
-    BabeSlotNumber getFirstBlockSlotNumber();
-
     outcome::result<std::shared_ptr<const primitives::BabeConfiguration>>
     config(const primitives::BlockInfo &block, bool next_epoch) const;
 
@@ -144,7 +141,6 @@ namespace kagome::consensus::babe {
 
     const BabeClock &clock_;
     std::optional<BabeSlotNumber> first_block_slot_number_;
-    bool is_first_block_finalized_ = false;
 
     log::Logger logger_;
   };

diff --git a/core/consensus/babe/impl/babe_impl.cpp b/core/consensus/babe/impl/babe_impl.cpp
@@ -407,41 +407,7 @@ namespace kagome::consensus::babe {
   }
 
   void BabeImpl::adjustEpochDescriptor() {
-    auto first_slot_number = babe_util_->syncEpoch([&]() {
-      auto hash_res = block_tree_->getBlockHash(primitives::BlockNumber(1));
-      if (hash_res.has_error() || !hash_res.value().has_value()) {
-        SL_TRACE(log_,
-                 "First block slot is {}: no first block (at adjusting)",
-                 babe_util_->getCurrentSlot());
-        return std::tuple(babe_util_->getCurrentSlot(), false);
-      }
-
-      auto first_block_header_res =
-          block_tree_->getBlockHeader(*hash_res.value());
-      if (first_block_header_res.has_error()) {
-        SL_CRITICAL(log_,
-                    "Database is not consistent: "
-                    "Not found block header for existing num-to-hash record");
-        throw std::runtime_error(
-            "Not found block header for existing num-to-hash record");
-      }
-
-      const auto &first_block_header = first_block_header_res.value();
-      auto babe_digest_res = getBabeDigests(first_block_header);
-      BOOST_ASSERT_MSG(babe_digest_res.has_value(),
-                       "Any non genesis block must contain babe digest");
-      auto first_slot_number = babe_digest_res.value().second.slot_number;
-
-      auto is_first_block_finalized =
-          block_tree_->getLastFinalized().number > 0;
-
-      SL_TRACE(
-          log_,
-          "First block slot is {}: by {}finalized first block (at adjusting)",
-          first_slot_number,
-          is_first_block_finalized ? "" : "non-");
-      return std::tuple(first_slot_number, is_first_block_finalized);
-    });
+    auto first_slot_number = babe_util_->getFirstBlockSlotNumber();
 
     auto current_epoch_start_slot =
         first_slot_number
@@ -1377,42 +1343,6 @@ namespace kagome::consensus::babe {
 
     ++current_epoch_.epoch_number;
     current_epoch_.start_slot = current_slot_;
-
-    babe_util_->syncEpoch([&]() {
-      auto hash_opt_res = block_tree_->getBlockHash(primitives::BlockNumber(1));
-      if (hash_opt_res.has_error() || !hash_opt_res.value().has_value()) {
-        SL_TRACE(log_,
-                 "First block slot is {}: no first block (at start next epoch)",
-                 babe_util_->getCurrentSlot());
-        return std::tuple(babe_util_->getCurrentSlot(), false);
-      }
-
-      auto first_block_header_res =
-          block_tree_->getBlockHeader(*hash_opt_res.value());
-      if (first_block_header_res.has_error()) {
-        SL_CRITICAL(log_,
-                    "Database is not consistent: "
-                    "Not found block header for existing num-to-hash record");
-        throw std::runtime_error(
-            "Not found block header for existing num-to-hash record");
-      }
-
-      const auto &first_block_header = first_block_header_res.value();
-      auto babe_digest_res = getBabeDigests(first_block_header);
-      BOOST_ASSERT_MSG(babe_digest_res.has_value(),
-                       "Any non genesis block must contain babe digest");
-      auto first_slot_number = babe_digest_res.value().second.slot_number;
-
-      auto is_first_block_finalized =
-          block_tree_->getLastFinalized().number > 0;
-
-      SL_WARN(log_,
-              "First block slot is {}: "
-              "by {}finalized first block (at start next epoch)",
-              first_slot_number,
-              is_first_block_finalized ? "" : "non-");
-      return std::tuple(first_slot_number, is_first_block_finalized);
-    });
   }
 
 }  // namespace kagome::consensus::babe
diff --git a/core/consensus/babe/impl/block_appender_base.cpp b/core/consensus/babe/impl/block_appender_base.cpp
@@ -153,49 +153,6 @@ namespace kagome::consensus::babe {
 
     auto slot_number = babe_header.slot_number;
 
-    babe_util_->syncEpoch([&] {
-      auto hash_res = block_tree_->getBlockHash(primitives::BlockNumber(1));
-      if (hash_res.has_error() || !hash_res.value().has_value()) {
-        if (block.header.number == 1) {
-          SL_TRACE(logger_,
-                   "First block slot is {}: it is first block (at executing)",
-                   slot_number);
-          return std::tuple(slot_number, false);
-        } else {
-          SL_TRACE(logger_,
-                   "First block slot is {}: no first block (at executing)",
-                   babe_util_->getCurrentSlot());
-          return std::tuple(babe_util_->getCurrentSlot(), false);
-        }
-      }
-
-      auto first_block_header_res =
-          block_tree_->getBlockHeader(*hash_res.value());
-      if (first_block_header_res.has_error()) {
-        SL_CRITICAL(logger_,
-                    "Database is not consistent: "
-                    "Not found block header for existing num-to-hash record");
-        throw std::runtime_error(
-            "Not found block header for existing num-to-hash record");
-      }
-
-      const auto &first_block_header = first_block_header_res.value();
-      auto babe_digest_res = getBabeDigests(first_block_header);
-      BOOST_ASSERT_MSG(babe_digest_res.has_value(),
-                       "Any non genesis block must contain babe digest");
-      auto first_slot_number = babe_digest_res.value().second.slot_number;
-
-      auto is_first_block_finalized =
-          block_tree_->getLastFinalized().number > 0;
-
-      SL_TRACE(
-          logger_,
-          "First block slot is {}: by {}finalized first block (at executing)",
-          first_slot_number,
-          is_first_block_finalized ? "" : "non-");
-      return std::tuple(first_slot_number, is_first_block_finalized);
-    });
-
     auto epoch_number = babe_util_->slotToEpoch(slot_number);
 
     SL_VERBOSE(

diff --git a/test/core/consensus/babe/babe_test.cpp b/test/core/consensus/babe/babe_test.cpp
@@ -313,7 +313,6 @@ TEST_F(BabeTest, Success) {
       .WillRepeatedly(Return(clock::SystemClockMock::zero()));
   EXPECT_CALL(*babe_util_, remainToStartOfSlot(_)).WillRepeatedly(Return(1ms));
   EXPECT_CALL(*babe_util_, remainToFinishOfSlot(_)).WillRepeatedly(Return(1ms));
-  EXPECT_CALL(*babe_util_, syncEpoch(_)).Times(testing::AnyNumber());
 
   testing::Sequence s;
   auto breaker = [](const boost::system::error_code &ec) {
@@ -372,7 +371,6 @@ TEST_F(BabeTest, NotAuthority) {
   EXPECT_CALL(*block_tree_, bestLeaf()).WillRepeatedly(Return(best_leaf));
   EXPECT_CALL(*block_tree_, getBlockHeader(best_block_hash_))
       .WillOnce(Return(best_block_header_));
-  EXPECT_CALL(*babe_util_, syncEpoch(_)).Times(2);
   EXPECT_CALL(*babe_util_, slotStartTime(_));
 
   expected_epoch_digest.authorities.clear();

diff --git a/test/core/consensus/babe/block_executor_test.cpp b/test/core/consensus/babe/block_executor_test.cpp
@@ -130,7 +130,7 @@ class BlockExecutorTest : public testing::Test {
     digest_tracker_ = std::make_shared<DigestTrackerMock>();
 
     babe_util_ = std::make_shared<BabeUtilMock>();
-    ON_CALL(*babe_util_, syncEpoch(_)).WillByDefault(Return(1));
+    ON_CALL(*babe_util_, getFirstBlockSlotNumber()).WillByDefault(Return(1));
     ON_CALL(*babe_util_, slotToEpoch(_)).WillByDefault(Return(1));
 
     offchain_worker_api_ = std::make_shared<OffchainWorkerApiMock>();

diff --git a/test/mock/core/consensus/babe/babe_util_mock.hpp b/test/mock/core/consensus/babe/babe_util_mock.hpp
@@ -14,13 +14,7 @@ namespace kagome::consensus::babe {
 
   class BabeUtilMock : public BabeUtil {
    public:
-    using SyncFunctor = std::function<std::tuple<BabeSlotNumber, bool>()>;
-
-    MOCK_METHOD(BabeSlotNumber, syncEpoch, (SyncFunctor &), ());
-
-    BabeSlotNumber syncEpoch(SyncFunctor &&func) override {
-      return syncEpoch(func);
-    }
+    MOCK_METHOD(BabeSlotNumber, getFirstBlockSlotNumber, (), ());
 
     MOCK_METHOD(BabeSlotNumber, getCurrentSlot, (), (const, override));