Fix forced change

core/consensus/babe/impl/block_header_appender_impl.cpp

@@ -87,8 +87,7 @@ namespace kagome::consensus::babe {
         consistency_guard,
         appender_->observeDigestsAndValidateHeader(block, block_context));
 
-    OUTCOME_TRY(appender_->applyJustifications(block_info,
-                                               justification));
+    OUTCOME_TRY(appender_->applyJustifications(block_info, justification));
 
     auto now = std::chrono::high_resolution_clock::now();
 
@@ -102,14 +101,17 @@ namespace kagome::consensus::babe {
     auto block_delta = block_info.number - speed_data_.block_number;
     auto time_delta = now - speed_data_.time;
     if (block_delta >= 10000 or time_delta >= std::chrono::minutes(1)) {
-      SL_LOG(logger_,
-             speed_data_.block_number ? log::Level::INFO
-                                      : static_cast<log::Level>(-1),
-             "Imported {} more headers of blocks. Average speed is {} bps",
-             block_delta,
-             block_delta
-                 / std::chrono::duration_cast<std::chrono::seconds>(time_delta)
-                       .count());
+      SL_LOG(
+          logger_,
+          speed_data_.block_number ? log::Level::INFO
+                                   : static_cast<log::Level>(-1),
+          "Imported {} more headers of blocks {}-{}. Average speed is {} bps",
+          block_delta,
+          speed_data_.block_number,
+          block_info.number,
+          block_delta
+              / std::chrono::duration_cast<std::chrono::seconds>(time_delta)
+                    .count());
       speed_data_.block_number = block_info.number;
       speed_data_.time = now;
     }

core/consensus/grandpa/impl/authority_manager_impl.cpp

@@ -33,15 +33,13 @@ namespace kagome::consensus::grandpa {
       Config config,
       std::shared_ptr<application::AppStateManager> app_state_manager,
       std::shared_ptr<blockchain::BlockTree> block_tree,
-      std::shared_ptr<storage::trie::TrieStorage> trie_storage,
       std::shared_ptr<runtime::GrandpaApi> grandpa_api,
       std::shared_ptr<crypto::Hasher> hasher,
       std::shared_ptr<storage::SpacedStorage> persistent_storage,
       std::shared_ptr<blockchain::BlockHeaderRepository> header_repo,
       primitives::events::ChainSubscriptionEnginePtr chain_events_engine)
       : config_{std::move(config)},
         block_tree_(std::move(block_tree)),
-        trie_storage_(std::move(trie_storage)),
         grandpa_api_(std::move(grandpa_api)),
         hasher_(std::move(hasher)),
         persistent_storage_{
@@ -55,7 +53,6 @@ namespace kagome::consensus::grandpa {
         logger_{log::createLogger("AuthorityManager", "authority")} {
     BOOST_ASSERT(block_tree_ != nullptr);
     BOOST_ASSERT(grandpa_api_ != nullptr);
-    BOOST_ASSERT(trie_storage_ != nullptr);
     BOOST_ASSERT(hasher_ != nullptr);
     BOOST_ASSERT(persistent_storage_ != nullptr);
     BOOST_ASSERT(header_repo_ != nullptr);
@@ -529,6 +526,15 @@ namespace kagome::consensus::grandpa {
              ancestor_node->block,
              ancestor_node->authorities->id);
 
+    for (auto &block : ancestor_node->forced_digests) {
+      if (directChainExists(context.block_info, block)) {
+        SL_DEBUG(logger_,
+                 "Scheduled change digest {} ignored by forced change",
+                 context.block_info.number);
+        return outcome::success();
+      }
+    }
+
     auto schedule_change = [&](const std::shared_ptr<ScheduleNode> &node)
         -> outcome::result<void> {
       auto new_authorities = std::make_shared<primitives::AuthoritySet>(
@@ -630,39 +636,33 @@ namespace kagome::consensus::grandpa {
       const primitives::AuthorityList &authorities,
       primitives::BlockNumber delay_start,
       size_t delay) {
+    auto forced_number = delay_start + delay;
     SL_DEBUG(logger_,
              "Applying forced change (delay start: {}, delay: {}) on block {} "
              "to activate at block {}",
              delay_start,
              delay,
              context.block_info,
-             delay_start + delay);
-    if (delay_start < root_->block.number) {
-      auto lag = root_->block.number - delay_start;
-      if (delay > lag) {
-        delay_start = root_->block.number;
-        delay -= lag;
-      } else {
-        delay_start = context.block_info.number;
-        delay = 0;
-      }
+             forced_number);
+    if (forced_number < root_->block.number) {
       SL_DEBUG(
           logger_,
           "Applying forced change on block {} is delayed {} blocks. "
           "Normalized to (delay start: {}, delay: {}) to activate at block {}",
           context.block_info,
-          lag,
+          root_->block.number - forced_number,
           delay_start,
           delay,
-          delay_start + delay);
+          root_->block.number);
+      forced_number = root_->block.number;
     }
-    auto delay_start_hash_res = header_repo_->getHashByNumber(delay_start);
+    auto delay_start_hash_res = header_repo_->getHashByNumber(forced_number);
     if (delay_start_hash_res.has_error()) {
-      SL_ERROR(logger_, "Failed to obtain hash by number {}", delay_start);
+      SL_ERROR(logger_, "Failed to obtain hash by number {}", forced_number);
     }
     OUTCOME_TRY(delay_start_hash, delay_start_hash_res);
     auto ancestor_node =
-        getNode({.block_info = {delay_start, delay_start_hash}});
+        getNode({.block_info = {forced_number, delay_start_hash}});
 
     if (not ancestor_node) {
       return AuthorityManagerError::ORPHAN_BLOCK_OR_ALREADY_FINALIZED;
@@ -673,26 +673,27 @@ namespace kagome::consensus::grandpa {
              ancestor_node->block,
              ancestor_node->authorities->id);
 
+    for (auto &block : ancestor_node->forced_digests) {
+      if (context.block_info == block) {
+        SL_DEBUG(logger_,
+                 "Forced change digest {} already included",
+                 context.block_info.number);
+        return outcome::success();
+      }
+    }
+
     auto force_change = [&](const std::shared_ptr<ScheduleNode> &node)
         -> outcome::result<void> {
       auto new_authorities = std::make_shared<primitives::AuthoritySet>(
           node->authorities->id + 1, authorities);
 
-      // Force changes
-      if (node->block.number >= delay_start + delay) {
-        node->authorities = new_authorities;
-        SL_VERBOSE(logger_,
-                   "Change has been forced on block #{} (set id={})",
-                   delay_start + delay,
-                   node->authorities->id);
-      } else {
-        node->action =
-            ScheduleNode::ForcedChange{delay_start, delay, new_authorities};
-        SL_VERBOSE(logger_,
-                   "Change will be forced on block #{} (set id={})",
-                   delay_start + delay,
-                   new_authorities->id);
-      }
+      node->forced_digests.emplace_back(context.block_info);
+
+      node->authorities = new_authorities;
+      SL_VERBOSE(logger_,
+                 "Change has been forced on block #{} (set id={})",
+                 forced_number,
+                 node->authorities->id);
 
       size_t index = 0;
       for (auto &authority : *new_authorities) {
@@ -708,7 +709,7 @@ namespace kagome::consensus::grandpa {
     };
 
     auto new_node =
-        ancestor_node->makeDescendant({delay_start, delay_start_hash}, true);
+        ancestor_node->makeDescendant({forced_number, delay_start_hash}, true);
 
     OUTCOME_TRY(force_change(new_node));
 
@@ -1021,14 +1022,6 @@ namespace kagome::consensus::grandpa {
           new_node->block.number < descendant->block.number ? new_node : node;
 
       // Apply if delay will be passed for descendant
-      if (auto *forced_change =
-              boost::get<ScheduleNode::ForcedChange>(&ancestor->action)) {
-        if (descendant->block.number
-            >= forced_change->delay_start + forced_change->delay_length) {
-          descendant->authorities = forced_change->new_authorities;
-          descendant->action = ScheduleNode::NoAction{};
-        }
-      }
       if (auto *resume = boost::get<ScheduleNode::Resume>(&ancestor->action)) {
         if (descendant->block.number >= resume->applied_block) {
           descendant->enabled = true;

core/consensus/grandpa/impl/authority_manager_impl.hpp

@@ -41,10 +41,6 @@ namespace kagome::runtime {
   class Executor;
 }  // namespace kagome::runtime
 
-namespace kagome::storage::trie {
-  class TrieStorage;
-}
-
 namespace kagome::consensus::grandpa {
 
   class AuthorityManagerImpl final
@@ -69,7 +65,6 @@ namespace kagome::consensus::grandpa {
         Config config,
         std::shared_ptr<application::AppStateManager> app_state_manager,
         std::shared_ptr<blockchain::BlockTree> block_tree,
-        std::shared_ptr<storage::trie::TrieStorage> trie_storage,
         std::shared_ptr<runtime::GrandpaApi> grandpa_api,
         std::shared_ptr<crypto::Hasher> hash,
         std::shared_ptr<storage::SpacedStorage> persistent_storage,
@@ -153,7 +148,6 @@ namespace kagome::consensus::grandpa {
 
     Config config_;
     std::shared_ptr<const blockchain::BlockTree> block_tree_;
-    std::shared_ptr<storage::trie::TrieStorage> trie_storage_;
     std::shared_ptr<runtime::GrandpaApi> grandpa_api_;
     std::shared_ptr<crypto::Hasher> hasher_;
     std::shared_ptr<storage::BufferStorage> persistent_storage_;

core/consensus/grandpa/impl/grandpa_impl.cpp

@@ -36,6 +36,15 @@ namespace {
 }  // namespace
 
 namespace kagome::consensus::grandpa {
+  inline auto &westendPastRound() {
+    static primitives::BlockInfo block{
+        198785,
+        primitives::BlockHash::fromHex(
+            "62caf6a8c99d63744f7093bceead8fdf4c7d8ef74f16163ed58b1c1aec67bf18")
+            .value(),
+    };
+    return block;
+  }
 
   namespace {
     Clock::Duration getGossipDuration(const application::ChainSpec &chain) {
@@ -1213,8 +1222,15 @@ namespace kagome::consensus::grandpa {
         return VotingRoundError::JUSTIFICATION_FOR_BLOCK_IN_PAST;
       }
 
+      auto authorities_opt =
+          authority_manager_->authorities(block_info, IsBlockFinalized{false});
+      if (!authorities_opt) {
+        return VotingRoundError::NO_KNOWN_AUTHORITIES_FOR_BLOCK;
+      }
+      auto &authority_set = authorities_opt.value();
+
       auto prev_round_opt =
-          selectRound(justification.round_number - 1, std::nullopt);
+          selectRound(justification.round_number - 1, authority_set->id);
 
       if (prev_round_opt.has_value()) {
         const auto &prev_round = prev_round_opt.value();
@@ -1239,20 +1255,15 @@ namespace kagome::consensus::grandpa {
             .votes = {},
             .finalized = block_info};
 
-        auto authorities_opt = authority_manager_->authorities(
-            block_info, IsBlockFinalized{false});
-        if (!authorities_opt) {
-          return VotingRoundError::NO_KNOWN_AUTHORITIES_FOR_BLOCK;
-        }
-        auto &authority_set = authorities_opt.value();
-
         // This is justification for non-actual round
         if (authority_set->id < current_round_->voterSetId()) {
           return VotingRoundError::JUSTIFICATION_FOR_AUTHORITY_SET_IN_PAST;
         }
         if (authority_set->id == current_round_->voterSetId()
             && justification.round_number < current_round_->roundNumber()) {
-          return VotingRoundError::JUSTIFICATION_FOR_ROUND_IN_PAST;
+          if (justification.block_info != westendPastRound()) {
+            return VotingRoundError::JUSTIFICATION_FOR_ROUND_IN_PAST;
+          }
         }
 
         if (authority_set->id > current_round_->voterSetId() + 1) {

core/consensus/grandpa/impl/schedule_node.cpp

@@ -30,20 +30,14 @@ namespace kagome::consensus::grandpa {
       if (scheduled_change->applied_block <= block.number) {
         authorities = std::move(scheduled_change->new_authorities);
         action = NoAction{};
+        forced_digests = {};
       }
     } else if (auto pause = boost::get<Pause>(&action);
                finalized && pause != nullptr) {
       if (pause->applied_block <= block.number) {
         enabled = false;
         action = NoAction{};
       }
-    } else if (auto forced_change = boost::get<ForcedChange>(&action);
-               forced_change != nullptr) {
-      if (forced_change->delay_start + forced_change->delay_length
-          <= block.number) {
-        authorities = std::move(forced_change->new_authorities);
-        action = NoAction{};
-      }
     } else if (auto resume = boost::get<Resume>(&action); resume != nullptr) {
       if (resume->applied_block <= block.number) {
         enabled = true;
@@ -60,6 +54,7 @@ namespace kagome::consensus::grandpa {
     node->authorities = authorities;
     node->enabled = enabled;
     node->action = action;
+    node->forced_digests = forced_digests;
     node->adjust(finalized);
     return node;
   }

core/consensus/grandpa/impl/schedule_node.hpp

@@ -52,14 +52,6 @@ namespace kagome::consensus::grandpa {
       std::shared_ptr<const primitives::AuthoritySet> new_authorities{};
     };
 
-    struct ForcedChange {
-      SCALE_TIE(3);
-
-      primitives::BlockNumber delay_start{};
-      size_t delay_length{};
-      std::shared_ptr<const primitives::AuthoritySet> new_authorities{};
-    };
-
     struct Pause {
       SCALE_TIE(1);
 
@@ -74,23 +66,23 @@ namespace kagome::consensus::grandpa {
 
     friend inline ::scale::ScaleEncoderStream &operator<<(
         ::scale::ScaleEncoderStream &s, const ScheduleNode &node) {
-      return s << node.enabled << node.block << *node.authorities
-               << node.action;
+      return s << node.enabled << node.block << *node.authorities << node.action
+               << node.forced_digests;
     }
 
     friend inline ::scale::ScaleDecoderStream &operator>>(
         ::scale::ScaleDecoderStream &s, ScheduleNode &node) {
       return s >> node.enabled
-             >> const_cast<primitives::BlockInfo &>(node.block)
-             >> node.authorities >> node.action;
+          >> const_cast<primitives::BlockInfo &>(node.block) >> node.authorities
+          >> node.action >> node.forced_digests;
     }
 
     const primitives::BlockInfo block{};
     std::weak_ptr<const ScheduleNode> parent;
     std::vector<std::shared_ptr<ScheduleNode>> descendants{};
 
-    boost::variant<NoAction, ScheduledChange, ForcedChange, Pause, Resume>
-        action;
+    boost::variant<NoAction, ScheduledChange, Pause, Resume> action;
+    std::vector<primitives::BlockInfo> forced_digests;
     std::shared_ptr<const primitives::AuthoritySet> authorities;
     bool enabled = true;
   };

core/utils/storage_explorer.cpp

@@ -573,7 +573,6 @@ int storage_explorer_main(int argc, const char **argv) {
       std::make_shared<AuthorityManagerImpl>(AuthorityManagerImpl::Config{},
                                              app_state_manager,
                                              block_tree,
-                                             trie_storage,
                                              grandpa_api,
                                              hasher,
                                              persistent_storage,