Refactor block executor and block appender to reduce code duplication

core/blockchain/impl/digest_tracker_impl.cpp

@@ -25,47 +25,47 @@ namespace kagome::blockchain {
   outcome::result<void> DigestTrackerImpl::onDigest(
       const primitives::BlockContext &context,
       const primitives::Digest &digest) {
-    SL_TRACE(logger_, "Start process digest on block {}", context.block);
+    SL_TRACE(logger_, "Start process digest on block {}", context.block_info);
     for (auto &item : digest) {
       auto res = visit_in_place(
           item,
           [&](const primitives::Consensus &item) {
             SL_TRACE(logger_,
                      "Consensus-digest on block {}, engine '{}'",
-                     context.block,
+                     context.block_info,
                      item.consensus_engine_id.toString());
             return onConsensus(context, item);
           },
           [&](const primitives::Seal &item) {
             SL_TRACE(logger_,
                      "Seal-digest on block {}, engine '{}'",
-                     context.block,
+                     context.block_info,
                      item.consensus_engine_id.toString());
             return outcome::success();  // It does not processed by tracker
           },
           [&](const primitives::PreRuntime &item) {
             SL_TRACE(logger_,
                      "PreRuntime-digest on block {}, engine '{}'",
-                     context.block,
+                     context.block_info,
                      item.consensus_engine_id.toString());
             return onPreRuntime(context, item);
           },
           [&](const primitives::RuntimeEnvironmentUpdated &item) {
             SL_TRACE(logger_,
                      "RuntimeEnvironmentUpdated-digest on block {}",
-                     context.block);
+                     context.block_info);
             return outcome::success();  // It does not processed by tracker
           },
           [&](const auto &) {
             SL_WARN(logger_,
                     "Unsupported digest on block {}: variant #{}",
-                    context.block,
+                    context.block_info,
                     item.which());
             return outcome::success();
           });
       OUTCOME_TRY(res);
     }
-    SL_TRACE(logger_, "End process digest on block {}", context.block);
+    SL_TRACE(logger_, "End process digest on block {}", context.block_info);
     return outcome::success();
   }
 
@@ -97,14 +97,14 @@ namespace kagome::blockchain {
                       == primitives::kUnsupportedEngineId_POL1) {
       SL_TRACE(logger_,
                "Unsupported consensus engine id in block {}: {}",
-               context.block,
+               context.block_info,
                message.consensus_engine_id.toString());
       return outcome::success();
 
     } else {
       SL_WARN(logger_,
               "Unknown consensus engine id in block {}: {}",
-              context.block,
+              context.block_info,
               message.consensus_engine_id.toString());
       return outcome::success();
     }
@@ -122,7 +122,7 @@ namespace kagome::blockchain {
     } else {
       SL_WARN(logger_,
               "Unknown consensus engine id in block {}: {}",
-              context.block,
+              context.block_info,
               message.consensus_engine_id.toString());
       return outcome::success();
     }

core/consensus/CMakeLists.txt

@@ -11,7 +11,8 @@ add_library(consensus
     babe/impl/threshold_util.cpp
     babe/impl/babe_lottery_impl.cpp
     babe/impl/babe_config_repository_impl.cpp
-    babe/impl/block_appender_impl.cpp
+    babe/impl/block_header_appender_impl.cpp
+    babe/impl/block_appender_base.cpp
     babe/impl/consistency_keeper_impl.cpp
     babe/babe_error.cpp
     validation/babe_block_validator.cpp

core/consensus/babe/babe_config_repository.hpp

@@ -16,18 +16,19 @@ namespace kagome::consensus::babe {
    public:
     virtual ~BabeConfigRepository() = default;
 
-    /// Returns duration of slot in milliseconds
-    /// @return slot duration
+    /// Returns the duration of a slot in milliseconds
+    /// @return the duration of a slot in milliseconds
     virtual BabeDuration slotDuration() const = 0;
 
-    /// Returns epoch length in number of slot
-    /// @return slot duration
+    /// @return the epoch length in slots
     virtual EpochLength epochLength() const = 0;
 
-    /// Returns actual babe configuration
-    /// @return actual babe configuration
-    virtual std::shared_ptr<const primitives::BabeConfiguration> config(
-        const primitives::BlockContext &context, EpochNumber epoch_number) = 0;
+    /// Returns the actual babe configuration
+    /// @return the actual babe configuration
+    virtual std::optional<
+        std::reference_wrapper<const primitives::BabeConfiguration>>
+    config(const primitives::BlockContext &context,
+           EpochNumber epoch_number) const = 0;
   };
 
 }  // namespace kagome::consensus::babe

core/consensus/babe/babe_error.cpp

@@ -17,3 +17,17 @@ OUTCOME_CPP_DEFINE_CATEGORY(kagome::consensus::babe, BabeError, e) {
   }
   return "unknown error";
 }
+
+OUTCOME_CPP_DEFINE_CATEGORY(kagome::consensus::babe, BlockAdditionError, e) {
+  using E = kagome::consensus::babe::BlockAdditionError;
+  switch (e) {
+    case E::ORPHAN_BLOCK:
+      return "Attempt to append a block which is either already finalized or "
+             "not a descendant of any known block";
+    case E::BLOCK_MISSING_HEADER:
+      return "Block without a header cannot be appended";
+    case E::PARENT_NOT_FOUND:
+      return "Parent not found";
+  }
+  return "Unknown error";
+}

core/consensus/babe/babe_error.hpp

@@ -14,8 +14,13 @@ namespace kagome::consensus::babe {
     BAD_ORDER_OF_DIGEST_ITEM,
     UNKNOWN_DIGEST_TYPE,
   };
+
+  enum class BlockAdditionError {
+    ORPHAN_BLOCK = 1, BLOCK_MISSING_HEADER, PARENT_NOT_FOUND
+  };
 }
 
 OUTCOME_HPP_DECLARE_ERROR(kagome::consensus::babe, BabeError)
+OUTCOME_HPP_DECLARE_ERROR(kagome::consensus::babe, BlockAdditionError)
 
 #endif  // KAGOME_BABE_ERROR_HPP

core/consensus/babe/block_executor.hpp

@@ -15,11 +15,9 @@ namespace kagome::consensus::babe {
    public:
     virtual ~BlockExecutor() = default;
 
-    virtual outcome::result<void> applyBlock(primitives::BlockData &&block) = 0;
-
-    virtual outcome::result<void> applyJustification(
-        const primitives::BlockInfo &block_info,
-        const primitives::Justification &justification) = 0;
+    virtual outcome::result<void> applyBlock(
+        primitives::Block &&block,
+        std::optional<primitives::Justification> const &justification) = 0;
   };
 
 }  // namespace kagome::consensus::babe

core/consensus/babe/block_appender.hpp → core/consensus/babe/block_header_appender.hpp

@@ -11,15 +11,16 @@
 
 namespace kagome::consensus::babe {
 
-  class BlockAppender {
+  /**
+   * Adds a new block header to the block storage
+   */
+  class BlockHeaderAppender {
    public:
-    virtual ~BlockAppender() = default;
+    virtual ~BlockHeaderAppender() = default;
 
-    virtual outcome::result<void> appendBlock(primitives::BlockData &&b) = 0;
-
-    virtual outcome::result<void> applyJustification(
-        const primitives::BlockInfo &block_info,
-        const primitives::Justification &justification) = 0;
+    virtual outcome::result<void> appendHeader(
+        primitives::BlockHeader &&block_header,
+        std::optional<primitives::Justification> const &justification) = 0;
   };
 
 }  // namespace kagome::consensus::babe

core/consensus/babe/consistency_keeper.hpp

@@ -13,42 +13,54 @@ namespace kagome::consensus::babe {
   /// Class to provide transactional applying of block and rollback that on
   /// start if last applied block was applied partially
   class ConsistencyKeeper {
-   public:
-    class Guard final {
-     public:
-      Guard(ConsistencyKeeper &keeper, primitives::BlockInfo block)
-          : keeper_(keeper), block_(block){};
-      ~Guard() {
-        rollback();
-      }
-
-      void commit() {
-        if (block_.has_value()) {
-          keeper_.commit(block_.value());
-          block_.reset();
-        }
-      }
-      void rollback() {
-        if (block_.has_value()) {
-          keeper_.rollback(block_.value());
-          block_.reset();
-        }
-      }
-
-     private:
-      ConsistencyKeeper &keeper_;
-      std::optional<primitives::BlockInfo> block_;
-    };
+    friend class ConsistencyGuard;
 
+   public:
     virtual ~ConsistencyKeeper() = default;
 
-    virtual Guard start(primitives::BlockInfo block) = 0;
+    virtual class ConsistencyGuard start(primitives::BlockInfo block) = 0;
 
    protected:
     virtual void commit(primitives::BlockInfo block) = 0;
     virtual void rollback(primitives::BlockInfo block) = 0;
   };
 
+  class ConsistencyGuard final {
+   public:
+    ConsistencyGuard(ConsistencyKeeper &keeper, primitives::BlockInfo block)
+        : keeper_(keeper), block_(block){};
+
+    ConsistencyGuard(const ConsistencyGuard &) = delete;
+    ConsistencyGuard &operator=(const ConsistencyGuard &) = delete;
+
+    ConsistencyGuard(ConsistencyGuard &&other) : keeper_{other.keeper_} {
+      block_ = other.block_;
+      other.block_.reset();
+    }
+    ConsistencyGuard &operator=(ConsistencyGuard &&other) = delete;
+
+    ~ConsistencyGuard() {
+      rollback();
+    }
+
+    void commit() {
+      if (block_.has_value()) {
+        keeper_.commit(block_.value());
+        block_.reset();
+      }
+    }
+    void rollback() {
+      if (block_.has_value()) {
+        keeper_.rollback(block_.value());
+        block_.reset();
+      }
+    }
+
+   private:
+    ConsistencyKeeper &keeper_;
+    std::optional<primitives::BlockInfo> block_;
+  };
+
 }  // namespace kagome::consensus::babe
 
 #endif  // KAGOME_CONSENSUS_BABE_CONSISTENCYKEEPER