Fix finalisation bugs and issues (#1290)

* fix: log of next epoch digest
* fix: send neighbor message in case of passive round
* fix: logging of ping protocol handling
* feature: remove disconnected peer from stream engine
* feature: forget fingerprint of sync-request in case of empty response
* refactor: conditions of sync-request and normal catch-up of round
* feature: catch-up based sync-request in according to applied blocks
* fix: Mixing full and authority roles
* refactor: use short peer_id in log
* fix: crash at execute next round after applying of justification
* refactor: make sit_id and round in peer data optional
* refactor: order of peer data (about grandpa) initialization
* fix: getting of descending chain to block
* feature: suppression of catch-up request to one peer for time
* fix: log and some corner case in sync protocol
* refactor: use cli bootstrap nodes early then spec's ones
* fix: allow to create next round by commit for non finalized

Signed-off-by: Dmitriy Khaustov aka xDimon <khaustov.dm@gmail.com>

Co-authored-by: Igor Egorov <igor@soramitsu.co.jp>

core/application/impl/app_configuration_impl.cpp

@@ -295,6 +295,7 @@ namespace kagome::application {
     bool validator_mode = false;
     load_bool(val, "validator", validator_mode);
     if (validator_mode) {
+      roles_.flags.full = 0;
       roles_.flags.authority = 1;
     }
 
@@ -800,7 +801,7 @@ namespace kagome::application {
           }
         }
 
-        roles_.flags.full = 1;
+        roles_.flags.full = 0;
         roles_.flags.authority = 1;
         p2p_port_ = def_p2p_port;
         rpc_http_host_ = def_rpc_http_host;
@@ -822,6 +823,7 @@ namespace kagome::application {
     });
 
     if (vm.end() != vm.find("validator")) {
+      roles_.flags.full = 0;
       roles_.flags.authority = 1;
     }
 

core/blockchain/impl/block_tree_impl.cpp

@@ -985,11 +985,13 @@ namespace kagome::blockchain {
       }
       const auto &header = header_res.value();
 
+      chain.emplace_back(hash);
+
       if (header.parent_hash == primitives::BlockHash{}) {
         break;
       }
 
-      chain.emplace_back(header.parent_hash);
+      hash = header.parent_hash;
     }
 
     return std::vector<primitives::BlockHash>(chain.begin(), chain.end());

core/consensus/babe/block_executor.hpp

@@ -16,6 +16,10 @@ namespace kagome::consensus {
     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;
   };
 
 }  // namespace kagome::consensus

core/consensus/babe/impl/block_executor_impl.cpp

@@ -209,8 +209,8 @@ namespace kagome::consensus {
       SL_VERBOSE(logger_,
                  "Next epoch digest has gotten in block {} (slot {}). "
                  "Randomness: {}",
-                 slot_number,
                  primitives::BlockInfo(block.header.number, block_hash),
+                 slot_number,
                  next_epoch_digest.randomness);
     }
 
@@ -284,8 +284,7 @@ namespace kagome::consensus {
       if (not justifications_.empty()) {
         std::vector<primitives::BlockInfo> to_remove;
         for (const auto &[block_info, justification] : justifications_) {
-          auto res = grandpa_environment_->applyJustification(block_info,
-                                                              justification);
+          auto res = applyJustification(block_info, justification);
           if (res) {
             to_remove.push_back(block_info);
           }
@@ -297,7 +296,7 @@ namespace kagome::consensus {
         }
       }
 
-      auto res = grandpa_environment_->applyJustification(
+      auto res = applyJustification(
           primitives::BlockInfo(block.header.number, block_hash),
           b.justification.value());
       if (res.has_error()) {
@@ -358,6 +357,12 @@ namespace kagome::consensus {
     return outcome::success();
   }
 
+  outcome::result<void> BlockExecutorImpl::applyJustification(
+      const primitives::BlockInfo &block_info,
+      const primitives::Justification &justification) {
+    return grandpa_environment_->applyJustification(block_info, justification);
+  }
+
   void BlockExecutorImpl::rollbackBlock(const primitives::BlockInfo &block) {
     // Remove possible authority changes scheduled on block
     authority_update_observer_->cancel(block);

core/consensus/babe/impl/block_executor_impl.hpp

@@ -52,6 +52,10 @@ namespace kagome::consensus {
 
     outcome::result<void> applyBlock(primitives::BlockData &&block) override;
 
+    outcome::result<void> applyJustification(
+        const primitives::BlockInfo &block_info,
+        const primitives::Justification &justification) override;
+
    private:
     void rollbackBlock(const primitives::BlockInfo &block);
 

core/consensus/grandpa/impl/grandpa_impl.cpp

@@ -164,9 +164,8 @@ namespace kagome::consensus::grandpa {
 
   std::shared_ptr<VotingRound> GrandpaImpl::makeNextRound(
       const std::shared_ptr<VotingRound> &round) {
-    BOOST_ASSERT(round->finalizedBlock().has_value());
-
-    BlockInfo best_block = round->finalizedBlock().value();
+    BlockInfo best_block =
+        round->finalizedBlock().value_or(round->lastFinalizedBlock());
 
     auto authorities_opt =
         authority_manager_->authorities(best_block, IsBlockFinalized{true});
@@ -298,6 +297,11 @@ namespace kagome::consensus::grandpa {
     metric_highest_round_->set(current_round_->roundNumber());
     if (keypair_) {
       current_round_->play();
+    } else {
+      auto round = std::dynamic_pointer_cast<VotingRoundImpl>(current_round_);
+      if (round) {
+        round->sendNeighborMessage();
+      }
     }
   }
 
@@ -322,28 +326,66 @@ namespace kagome::consensus::grandpa {
              msg.last_finalized,
              peer_id);
 
-    // Ignore peer whose voter_set is different
-    if (msg.voter_set_id != current_round_->voterSetId()) {
-      return;
-    }
-
-    // Check if needed to catch-up peer, then do that
-    if (msg.round_number >= current_round_->roundNumber() + kCatchUpThreshold) {
-      std::ignore = environment_->onCatchUpRequested(
-          peer_id, msg.voter_set_id, msg.round_number - 1);
-      return;
-    }
+    auto info = peer_manager_->getPeerState(peer_id);
 
     // Iff peer just reached one of recent round, then share known votes
-    auto info = peer_manager_->getPeerState(peer_id);
-    if (not info.has_value() || msg.voter_set_id != info->set_id
-        || msg.round_number > info->round_number) {
+    if (not info.has_value()
+        or (info->set_id.has_value() and msg.voter_set_id != info->set_id)
+        or (info->round_number.has_value()
+            and msg.round_number > info->round_number)) {
       if (auto opt_round = selectRound(msg.round_number, msg.voter_set_id);
           opt_round.has_value()) {
         auto &round = opt_round.value();
         environment_->sendState(peer_id, round->state(), msg.voter_set_id);
       }
     }
+
+    peer_manager_->updatePeerState(peer_id, msg);
+
+    // If peer has the same voter set id
+    if (msg.voter_set_id == current_round_->voterSetId()) {
+      // Check if needed to catch-up peer, then do that
+      if (msg.round_number
+          >= current_round_->roundNumber() + kCatchUpThreshold) {
+        std::ignore = environment_->onCatchUpRequested(
+            peer_id, msg.voter_set_id, msg.round_number - 1);
+      }
+
+      return;
+    }
+
+    // Ignore peer whose voter set id lower than our current
+    if (msg.voter_set_id < current_round_->voterSetId()) {
+      return;
+    }
+
+    if (info->last_finalized <= block_tree_->deepestLeaf().number) {
+      //  Trying to substitute with justifications' request only
+      auto last_finalized = block_tree_->getLastFinalized();
+      synchronizer_->syncMissingJustifications(
+          peer_id,
+          last_finalized,
+          std::nullopt,
+          [wp = weak_from_this(), last_finalized, msg](auto res) {
+            auto self = wp.lock();
+            if (not self) {
+              return;
+            }
+            if (res.has_error()) {
+              SL_WARN(self->logger_,
+                      "Missing justifications between blocks {} and "
+                      "{} was not loaded: {}",
+                      last_finalized,
+                      msg.last_finalized,
+                      res.error().message());
+            } else {
+              SL_DEBUG(self->logger_,
+                       "Loaded justifications for blocks in range {} - {}",
+                       last_finalized,
+                       res.value());
+            }
+          });
+    }
   }
 
   void GrandpaImpl::onCatchUpRequest(const libp2p::peer::PeerId &peer_id,
@@ -766,73 +808,88 @@ namespace kagome::consensus::grandpa {
 
   outcome::result<void> GrandpaImpl::applyJustification(
       const BlockInfo &block_info, const GrandpaJustification &justification) {
-    auto opt_round = selectRound(justification.round_number, std::nullopt);
+    auto round_opt = selectRound(justification.round_number, std::nullopt);
     std::shared_ptr<VotingRound> round;
     bool need_to_make_round_current = false;
-    if (!opt_round.has_value()) {
+    if (round_opt.has_value()) {
+      round = std::move(round_opt.value());
+    } else {
       // This is justification for already finalized block
       if (current_round_->lastFinalizedBlock().number > block_info.number) {
         return VotingRoundError::JUSTIFICATION_FOR_BLOCK_IN_PAST;
       }
 
-      MovableRoundState round_state{
-          .round_number = justification.round_number,
-          .last_finalized_block = current_round_->lastFinalizedBlock(),
-          .votes = {},
-          .finalized = block_info};
-
-      auto authorities_opt =
-          authority_manager_->authorities(block_info, IsBlockFinalized{false});
-      if (!authorities_opt) {
-        SL_WARN(logger_,
-                "Can't retrieve authorities to apply a justification "
-                "at block {}",
-                block_info);
-        return VotingRoundError::NO_KNOWN_AUTHORITIES_FOR_BLOCK;
-      }
-      auto &authorities = authorities_opt.value();
-
-      // This is justification for non-actual round
-      if (authorities->id < current_round_->voterSetId()
-          or (authorities->id == current_round_->voterSetId()
-              && justification.round_number < current_round_->roundNumber())) {
-        return VotingRoundError::JUSTIFICATION_FOR_ROUND_IN_PAST;
-      }
+      auto prev_round_opt =
+          selectRound(justification.round_number - 1, std::nullopt);
+
+      if (prev_round_opt.has_value()) {
+        const auto &prev_round = prev_round_opt.value();
+        round = makeNextRound(prev_round);
+        need_to_make_round_current = true;
+        BOOST_ASSERT(round);
+
+        SL_DEBUG(logger_,
+                 "Hop grandpa to round #{} by received justification",
+                 justification.round_number);
+      } else {
+        MovableRoundState round_state{
+            .round_number = justification.round_number,
+            .last_finalized_block = current_round_->lastFinalizedBlock(),
+            .votes = {},
+            .finalized = block_info};
+
+        auto authorities_opt = authority_manager_->authorities(
+            block_info, IsBlockFinalized{false});
+        if (!authorities_opt) {
+          SL_WARN(logger_,
+                  "Can't retrieve authorities to apply a justification "
+                  "at block {}",
+                  block_info);
+          return VotingRoundError::NO_KNOWN_AUTHORITIES_FOR_BLOCK;
+        }
+        auto &authorities = authorities_opt.value();
+
+        // This is justification for non-actual round
+        if (authorities->id < current_round_->voterSetId()
+            or (authorities->id == current_round_->voterSetId()
+                && justification.round_number
+                       < current_round_->roundNumber())) {
+          return VotingRoundError::JUSTIFICATION_FOR_ROUND_IN_PAST;
+        }
 
-      if (authorities->id > current_round_->voterSetId() + 1) {
-        return VotingRoundError::WRONG_ORDER_OF_VOTER_SET_ID;
-      }
+        if (authorities->id > current_round_->voterSetId() + 1) {
+          return VotingRoundError::WRONG_ORDER_OF_VOTER_SET_ID;
+        }
 
-      auto voters = std::make_shared<VoterSet>(authorities->id);
-      for (const auto &authority : *authorities) {
-        auto res = voters->insert(
-            primitives::GrandpaSessionKey(authority.id.id), authority.weight);
-        if (res.has_error()) {
-          SL_CRITICAL(
-              logger_, "Can't make voter set: {}", res.error().message());
-          return res.as_failure();
+        auto voters = std::make_shared<VoterSet>(authorities->id);
+        for (const auto &authority : *authorities) {
+          auto res = voters->insert(
+              primitives::GrandpaSessionKey(authority.id.id), authority.weight);
+          if (res.has_error()) {
+            SL_CRITICAL(
+                logger_, "Can't make voter set: {}", res.error().message());
+            return res.as_failure();
+          }
         }
-      }
 
-      round = makeInitialRound(round_state, std::move(voters));
-      need_to_make_round_current = true;
-      BOOST_ASSERT(round);
+        round = makeInitialRound(round_state, std::move(voters));
+        need_to_make_round_current = true;
+        BOOST_ASSERT(round);
 
-      SL_DEBUG(logger_,
-               "Rewind grandpa till round #{} by received justification",
-               justification.round_number);
-    } else {
-      round = *opt_round;
+        SL_DEBUG(logger_,
+                 "Rewind grandpa till round #{} by received justification",
+                 justification.round_number);
+      }
     }
 
     OUTCOME_TRY(round->applyJustification(block_info, justification));
 
     if (need_to_make_round_current) {
       current_round_->end();
-      current_round_ = std::move(round);
+      current_round_ = round;
     }
 
-    tryExecuteNextRound(current_round_);
+    tryExecuteNextRound(round);
 
     // if round == current round, then execution of the next round will be
     // elsewhere

core/consensus/grandpa/impl/voting_round_impl.cpp

@@ -114,10 +114,10 @@ namespace kagome::consensus::grandpa {
                         clock,
                         scheduler) {
     BOOST_ASSERT(previous_round != nullptr);
-    BOOST_ASSERT(previous_round->finalizedBlock().has_value());
 
     previous_round_ = previous_round;
-    last_finalized_block_ = previous_round->finalizedBlock().value();
+    last_finalized_block_ = previous_round->finalizedBlock().value_or(
+        previous_round->lastFinalizedBlock());
   }
 
   VotingRoundImpl::VotingRoundImpl(

core/consensus/grandpa/impl/voting_round_impl.hpp

@@ -239,6 +239,8 @@ namespace kagome::consensus::grandpa {
      */
     MovableRoundState state() const override;
 
+    void sendNeighborMessage();
+
    private:
     /// Check if peer \param id is primary
     bool isPrimary(const Id &id) const;
@@ -290,7 +292,6 @@ namespace kagome::consensus::grandpa {
     outcome::result<void> validatePrecommitJustification(
         const BlockInfo &vote, const GrandpaJustification &justification) const;
 
-    void sendNeighborMessage();
     void sendProposal(const PrimaryPropose &primary_proposal);
     void sendPrevote(const Prevote &prevote);
     void sendPrecommit(const Precommit &precommit);