Merge pull request #2272 from AntelopeIO/gh_2233

IF: Add more extensive tests for finalizer voting behavior

libraries/chain/controller.cpp

@@ -699,7 +699,7 @@ struct building_block {
                   qc_data->qc_claim
                };
 
-               assembled_block::assembled_block_if ab{std::move(bb.active_producer_authority), bb.parent.next(bhs_input),
+               assembled_block::assembled_block_if ab{bb.active_producer_authority, bb.parent.next(bhs_input),
                                                       std::move(bb.pending_trx_metas), std::move(bb.pending_trx_receipts),
                                                       qc_data ? std::move(qc_data->qc) : std::optional<quorum_certificate>{}};
 
@@ -1594,19 +1594,19 @@ struct controller_impl {
             auto set_finalizer_defaults = [&](auto& forkdb) -> void {
                auto lib = forkdb.root();
                my_finalizers.set_default_safety_information(
-                  finalizer::safety_information{ .last_vote_range_start = block_timestamp_type(0),
-                                                 .last_vote = {},
-                                                 .lock      = finalizer::proposal_ref(lib) });
+                  finalizer_safety_information{ .last_vote_range_start = block_timestamp_type(0),
+                                                .last_vote = {},
+                                                .lock      = proposal_ref(lib->id(), lib->timestamp()) });
             };
             fork_db.apply_if<void>(set_finalizer_defaults);
          } else {
             // we are past the IF transition.
             auto set_finalizer_defaults = [&](auto& forkdb) -> void {
                auto lib = forkdb.root();
                my_finalizers.set_default_safety_information(
-                  finalizer::safety_information{ .last_vote_range_start = block_timestamp_type(0),
-                                                 .last_vote = {},
-                                                 .lock      = finalizer::proposal_ref(lib) });
+                  finalizer_safety_information{ .last_vote_range_start = block_timestamp_type(0),
+                                                .last_vote = {},
+                                                .lock      = proposal_ref(lib->id(), lib->timestamp()) });
             };
             fork_db.apply_if<void>(set_finalizer_defaults);
          }
@@ -2818,7 +2818,7 @@ struct controller_impl {
             log_irreversible();
          }
 
-         fork_db.apply_if<void>([&](auto& forkdb) { create_and_send_vote_msg(forkdb.chain_head, forkdb); });
+         fork_db.apply_if<void>([&](auto& forkdb) { create_and_send_vote_msg(forkdb.chain_head); });
 
          // TODO: temp transition to instant-finality, happens immediately after block with new_finalizer_policy
          auto transition = [&](auto& forkdb) -> bool {
@@ -2848,9 +2848,9 @@ struct controller_impl {
                      auto start_block = forkdb.chain_head;
                      auto lib_block   = forkdb.chain_head;
                      my_finalizers.set_default_safety_information(
-                        finalizer::safety_information{ .last_vote_range_start = block_timestamp_type(0),
-                                                       .last_vote = finalizer::proposal_ref(start_block),
-                                                       .lock      = finalizer::proposal_ref(lib_block) });
+                        finalizer_safety_information{ .last_vote_range_start = block_timestamp_type(0),
+                                                      .last_vote = proposal_ref(start_block->id(), start_block->timestamp()),
+                                                      .lock      = proposal_ref(lib_block->id(),   lib_block->timestamp()) });
                   }
 
                   log_irreversible();
@@ -3139,7 +3139,7 @@ struct controller_impl {
       return fork_db.apply<vote_status>(aggregate_vote_legacy, aggregate_vote);
    }
 
-   void create_and_send_vote_msg(const block_state_ptr& bsp, const fork_database_if_t& fork_db) {
+   void create_and_send_vote_msg(const block_state_ptr& bsp) {
       auto finalizer_digest = bsp->compute_finalizer_digest();
 
       // Each finalizer configured on the node which is present in the active finalizer policy
@@ -3149,7 +3149,7 @@ struct controller_impl {
       // off the main thread. net_plugin is fine for this to be emitted from any thread.
       // Just need to update the comment in net_plugin
       my_finalizers.maybe_vote(
-          *bsp->active_finalizer_policy, bsp, fork_db, finalizer_digest, [&](const vote_message& vote) {
+          *bsp->active_finalizer_policy, bsp, finalizer_digest, [&](const vote_message& vote) {
               // net plugin subscribed to this signal. it will broadcast the vote message
               // on receiving the signal
               emit(voted_block, vote);

libraries/chain/finality_core.cpp

@@ -78,6 +78,29 @@ qc_claim_t finality_core::latest_qc_claim() const
    return qc_claim_t{.block_num = links.back().target_block_num, .is_strong_qc = links.back().is_link_strong};
 }
 
+/**
+ *  @pre  all finality_core invariants
+ *  @post same
+ *  @returns timestamp of latest qc_claim made by the core
+ */
+block_time_type finality_core::latest_qc_block_timestamp() const {
+   return get_block_reference(links.back().target_block_num).timestamp;
+}
+
+/**
+ *  @pre  all finality_core invariants
+ *  @post same
+ *  @returns boolean indicating whether `id` is an ancestor of this block
+ */
+bool finality_core::extends(const block_id_type& id) const {
+   uint32_t block_num = block_header::num_from_id(id);
+   if (block_num >= last_final_block_num() && block_num < current_block_num()) {
+      const block_ref& ref = get_block_reference(block_num);
+      return ref.block_id == id;
+   }
+   return false;
+}
+
 /**
  *  @pre last_final_block_num() <= block_num < current_block_num()
  *
@@ -260,10 +283,8 @@ finality_core finality_core::next(const block_ref& current_block, const qc_claim
 
       assert(links_index < links.size()); // Satisfied by justification in this->get_qc_link_from(new_links_front_source_block_num).
 
-      next_core.links.reserve(links.size() - links_index + 1);
-
       // Garbage collect unnecessary links
-      std::copy(links.cbegin() + links_index, links.cend(), std::back_inserter(next_core.links));
+      next_core.links = { links.cbegin() + links_index, links.cend() };
 
       assert(next_core.last_final_block_num() == new_last_final_block_num); // Satisfied by choice of links_index.
 
@@ -297,11 +318,8 @@ finality_core finality_core::next(const block_ref& current_block, const qc_claim
       assert(!refs.empty() || (refs_index == 0)); // Satisfied by justification above.
       assert(refs.empty() || (refs_index < refs.size())); // Satisfied by justification above.
 
-      next_core.refs.reserve(refs.size() - refs_index + 1);
-
       // Garbage collect unnecessary block references
-      std::copy(refs.cbegin() + refs_index, refs.cend(), std::back_inserter(next_core.refs));
-
+      next_core.refs = {refs.cbegin() + refs_index, refs.cend()};
       assert(refs.empty() || (next_core.refs.front().block_num() == new_last_final_block_num)); // Satisfied by choice of refs_index.
 
       // Add new block reference

libraries/chain/fork_database.cpp

@@ -146,7 +146,7 @@ namespace eosio::chain {
    {}
 
    template<class BSP>
-   fork_database_t<BSP>::~fork_database_t() = default;
+   fork_database_t<BSP>::~fork_database_t() = default; // close is performed in fork_database::~fork_database()
 
    template<class BSP>
    void fork_database_t<BSP>::open( const std::filesystem::path& fork_db_file, validator_t& validator ) {

libraries/chain/hotstuff/finalizer.cpp

@@ -5,98 +5,75 @@
 namespace eosio::chain {
 
 // ----------------------------------------------------------------------------------------
-block_header_state_ptr get_block_by_num(const fork_database_if_t::full_branch_t& branch, std::optional<uint32_t> block_num) {
-   if (!block_num || branch.empty())
-      return block_state_ptr{};
+finalizer::vote_result finalizer::decide_vote(const finality_core& core, const block_id_type &proposal_id,
+                                               const block_timestamp_type proposal_timestamp) {
+   vote_result res;
 
-   // a branch always contains consecutive block numbers, starting with the highest
-   uint32_t first = branch[0]->block_num();
-   uint32_t dist  = first - *block_num;
-   return dist < branch.size() ? branch[dist] : block_state_ptr{};
-}
-
-// ----------------------------------------------------------------------------------------
-bool extends(const fork_database_if_t::full_branch_t& branch, const block_id_type& id)  {
-   return !branch.empty() &&
-      std::any_of(++branch.cbegin(), branch.cend(), [&](const auto& h) { return h->id() == id; });
-}
-
-// ----------------------------------------------------------------------------------------
-finalizer::vote_decision finalizer::decide_vote(const block_state_ptr& proposal, const fork_database_if_t& fork_db) {
-   bool safety_check   = false;
-   bool liveness_check = false;
-
-   bool monotony_check = !fsi.last_vote || proposal->timestamp() > fsi.last_vote.timestamp;
-   // !fsi.last_vote means we have never voted on a proposal, so the protocol feature just activated and we can proceed
+   res.monotony_check = fsi.last_vote.empty() || proposal_timestamp > fsi.last_vote.timestamp;
+   // fsi.last_vote.empty() means we have never voted on a proposal, so the protocol feature
+   // just activated and we can proceed
 
-   if (!monotony_check) {
-      dlog("monotony check failed for proposal ${p}, cannot vote", ("p", proposal->id()));
-      return vote_decision::no_vote;
+   if (!res.monotony_check) {
+      dlog("monotony check failed for proposal ${p}, cannot vote", ("p", proposal_id));
+      return res;
    }
 
-   std::optional<full_branch_t> p_branch; // a branch that includes the root.
-
    if (!fsi.lock.empty()) {
       // Liveness check : check if the height of this proposal's justification is higher
       // than the height of the proposal I'm locked on.
       // This allows restoration of liveness if a replica is locked on a stale proposal
       // -------------------------------------------------------------------------------
-      liveness_check = proposal->last_qc_block_timestamp() > fsi.lock.timestamp;
+      res.liveness_check = core.latest_qc_block_timestamp() > fsi.lock.timestamp;
 
-      if (!liveness_check) {
+      if (!res.liveness_check) {
          // Safety check : check if this proposal extends the proposal we're locked on
-         p_branch = fork_db.fetch_full_branch(proposal->id());
-         safety_check = extends(*p_branch, fsi.lock.id);
+         res.safety_check = core.extends(fsi.lock.block_id);
       }
    } else {
       // Safety and Liveness both fail if `fsi.lock` is empty. It should not happen.
       // `fsi.lock` is initially set to `lib` when switching to IF or starting from a snapshot.
       // -------------------------------------------------------------------------------------
-      liveness_check = false;
-      safety_check   = false;
+      res.liveness_check = false;
+      res.safety_check   = false;
    }
 
+   bool can_vote = res.liveness_check || res.safety_check;
+   dlog("liveness_check=${l}, safety_check=${s}, monotony_check=${m}, can vote=${can_vote}",
+        ("l",res.liveness_check)("s",res.safety_check)("m",res.monotony_check)("can_vote",can_vote));
+
    // Figure out if we can vote and wether our vote will be strong or weak
    // If we vote, update `fsi.last_vote` and also `fsi.lock` if we have a newer commit qc
    // -----------------------------------------------------------------------------------
-   vote_decision decision = vote_decision::no_vote;
-
-   if (liveness_check || safety_check) {
-      auto [p_start, p_end] = std::make_pair(proposal->last_qc_block_timestamp(), proposal->timestamp());
+   if (can_vote) {
+      auto [p_start, p_end] = std::make_pair(core.latest_qc_block_timestamp(), proposal_timestamp);
 
       bool time_range_disjoint  = fsi.last_vote_range_start >= p_end || fsi.last_vote.timestamp <= p_start;
       bool voting_strong        = time_range_disjoint;
-      if (!voting_strong) {
-         if (!p_branch)
-            p_branch = fork_db.fetch_full_branch(proposal->id());
-         voting_strong = extends(*p_branch, fsi.last_vote.id);
+      if (!voting_strong && !fsi.last_vote.empty()) {
+         // we can vote strong if the proposal is a descendant of (i.e. extends) our last vote id
+         voting_strong = core.extends(fsi.last_vote.block_id);
       }
 
-      fsi.last_vote             = proposal_ref(proposal);
+      fsi.last_vote             = proposal_ref(proposal_id, proposal_timestamp);
       fsi.last_vote_range_start = p_start;
 
-      if (!p_branch)
-         p_branch = fork_db.fetch_full_branch(proposal->id());
-      auto bsp_final_on_strong_qc =  get_block_by_num(*p_branch, proposal->final_on_strong_qc_block_num());
-      if (voting_strong && bsp_final_on_strong_qc && bsp_final_on_strong_qc->timestamp() > fsi.lock.timestamp)
-         fsi.lock = proposal_ref(bsp_final_on_strong_qc);
+      auto& final_on_strong_qc_block_ref = core.get_block_reference(core.final_on_strong_qc_block_num);
+      if (voting_strong && final_on_strong_qc_block_ref.timestamp > fsi.lock.timestamp)
+         fsi.lock = proposal_ref(final_on_strong_qc_block_ref.block_id, final_on_strong_qc_block_ref.timestamp);
 
-      decision = voting_strong ? vote_decision::strong_vote : vote_decision::weak_vote;
-   } else {
-      dlog("last_qc_block_num=${lqc}, fork_db root block_num=${f}",
-           ("lqc",!!proposal->last_qc_block_num())("f",fork_db.root()->block_num()));
-      dlog("last_qc_block_num=${lqc}", ("lqc", proposal->last_qc_block_num()));
+      res.decision = voting_strong ? vote_decision::strong_vote : vote_decision::weak_vote;
    }
 
    dlog("liveness_check=${l}, safety_check=${s}, monotony_check=${m}, can vote=${can_vote}, voting=${v}",
-        ("l",liveness_check)("s",safety_check)("m",monotony_check)("can_vote",(liveness_check || safety_check))("v", decision));
-   return decision;
+        ("l",res.liveness_check)("s",res.safety_check)("m",res.monotony_check)("can_vote",can_vote)("v", res.decision));
+   return res;
 }
 
 // ----------------------------------------------------------------------------------------
-std::optional<vote_message> finalizer::maybe_vote(const bls_public_key& pub_key, const block_state_ptr& p,
-                                                  const digest_type& digest, const fork_database_if_t& fork_db) {
-   finalizer::vote_decision decision = decide_vote(p, fork_db);
+std::optional<vote_message> finalizer::maybe_vote(const bls_public_key& pub_key,
+                                                  const block_header_state_ptr& p,
+                                                  const digest_type& digest) {
+   finalizer::vote_decision decision = decide_vote(p->core, p->id(), p->timestamp()).decision;
    if (decision == vote_decision::strong_vote || decision == vote_decision::weak_vote) {
       bls_signature sig;
       if (decision == vote_decision::weak_vote) {

libraries/chain/include/eosio/chain/block_state.hpp

@@ -59,6 +59,8 @@ struct block_state : public block_header_state {     // block_header_state provi
    uint32_t               irreversible_blocknum() const { return core.last_final_block_num(); } // backwards compatibility
    uint32_t               last_final_block_num() const { return core.last_final_block_num(); }
    std::optional<quorum_certificate> get_best_qc() const;
+
+   protocol_feature_activation_set_ptr get_activated_protocol_features() const { return block_header_state::activated_protocol_features; }
    uint32_t               last_qc_block_num() const { return core.latest_qc_claim().block_num; }
    uint32_t               final_on_strong_qc_block_num() const { return core.final_on_strong_qc_block_num; }
 

libraries/chain/include/eosio/chain/block_timestamp.hpp

@@ -17,7 +17,10 @@ namespace eosio { namespace chain {
    template<uint16_t IntervalMs, uint64_t EpochMs>
    class block_timestamp {
       public:
-         explicit block_timestamp( uint32_t s=0 ) :slot(s){}
+
+         block_timestamp() : slot(0) {}
+
+         explicit block_timestamp( uint32_t s ) :slot(s){}
 
          block_timestamp(const fc::time_point& t) {
             set_time_point(t);
@@ -51,12 +54,11 @@ namespace eosio { namespace chain {
             set_time_point(t);
          }
 
-         bool   operator > ( const block_timestamp& t )const   { return slot >  t.slot; }
-         bool   operator >=( const block_timestamp& t )const   { return slot >= t.slot; }
-         bool   operator < ( const block_timestamp& t )const   { return slot <  t.slot; }
-         bool   operator <=( const block_timestamp& t )const   { return slot <= t.slot; }
-         bool   operator ==( const block_timestamp& t )const   { return slot == t.slot; }
-         bool   operator !=( const block_timestamp& t )const   { return slot != t.slot; }
+         // needed, otherwise deleted because of above version of operator=()
+         block_timestamp& operator=(const block_timestamp&) = default;
+
+         auto operator<=>(const block_timestamp&) const = default;
+
          uint32_t slot;
 
       private:
@@ -76,6 +78,13 @@ namespace eosio { namespace chain {
 
 } } /// eosio::chain
 
+namespace std {
+   inline std::ostream& operator<<(std::ostream& os, const eosio::chain::block_timestamp_type& t) {
+      os << "tstamp(" << t.slot << ")";
+      return os;
+   }
+}
+
 
 #include <fc/reflect/reflect.hpp>
 FC_REFLECT(eosio::chain::block_timestamp_type, (slot))