[1.0] Use other_branch_latest_time for liveness

libraries/chain/controller.cpp

@@ -1425,9 +1425,9 @@ struct controller_impl {
                        [&](const block_state_ptr& head) { return head->make_block_ref(); }});
          // doesn't matter chain_head is not updated for IRREVERSIBLE, cannot be in irreversible mode and be a finalizer
          my_finalizers.set_default_safety_information(
-            finalizer_safety_information{ .last_vote                             = ref,
-                                          .lock                                  = ref,
-                                          .votes_forked_since_latest_strong_vote = false});
+            finalizer_safety_information{ .last_vote                = ref,
+                                          .lock                     = ref,
+                                          .other_branch_latest_time = block_timestamp_type{} });
       }
    }
 
@@ -1636,9 +1636,9 @@ struct controller_impl {
                      // we create the non-legacy fork_db, as from this point we may need to cast votes to participate
                      // to the IF consensus. See https://github.com/AntelopeIO/leap/issues/2070#issuecomment-1941901836
                      my_finalizers.set_default_safety_information(
-                        finalizer_safety_information{.last_vote                             = prev->make_block_ref(),
-                                                     .lock                                  = prev->make_block_ref(),
-                                                     .votes_forked_since_latest_strong_vote = false});
+                        finalizer_safety_information{.last_vote                = prev->make_block_ref(),
+                                                     .lock                     = prev->make_block_ref(),
+                                                     .other_branch_latest_time = block_timestamp_type{} });
                   }
                }
             });
@@ -2040,19 +2040,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 = {},
-                                                .lock      = lib->make_block_ref(),
-                                                .votes_forked_since_latest_strong_vote = false });
+                  finalizer_safety_information{ .last_vote                = {},
+                                                .lock                     = lib->make_block_ref(),
+                                                .other_branch_latest_time = block_timestamp_type{} });
             };
             fork_db.apply_s<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 = {},
-                                                .lock      = lib->make_block_ref(),
-                                                .votes_forked_since_latest_strong_vote = false });
+                  finalizer_safety_information{.last_vote                = {},
+                                               .lock                     = lib->make_block_ref(),
+                                               .other_branch_latest_time = block_timestamp_type{} });
             };
             fork_db.apply_s<void>(set_finalizer_defaults);
          }

libraries/chain/finality/finalizer.cpp

@@ -61,35 +61,37 @@ finalizer::vote_result finalizer::decide_vote(const block_state_ptr& bsp) {
    // If we vote, update `fsi.last_vote` and also `fsi.lock` if we have a newer commit qc
    // -----------------------------------------------------------------------------------
    if (can_vote) {
-      // if `fsi.last_vote` is not set, it will be initialized with a timestamp slot of 0, so we
-      // don't need to check fsi.last_vote.empty()
-      // ---------------------------------------------------------------------------------------
-      bool voting_strong = fsi.last_vote.timestamp <= bsp->core.latest_qc_block_timestamp();
-
-      if (!voting_strong) {
-         // we can vote strong if the proposal is a descendant of (i.e. extends) our last vote id AND
-         // the latest (weak) vote did not mask a prior (weak) vote for a block not on the same branch.
-         // -------------------------------------------------------------------------------------------
-         voting_strong = !fsi.votes_forked_since_latest_strong_vote && bsp->core.extends(fsi.last_vote.block_id);
+      const auto latest_qc_block_timestamp = bsp->core.latest_qc_block_timestamp();
+
+      // If `fsi.last_vote` is not set, it will be initialized with a timestamp slot of 0,
+      // which means `fsi.last_vote.timestamp` would always be less than or equal
+      // to `latest_qc_block_timestamp`.
+      // So, we don't need to separately check for the case where `fsi.last_vote.empty()` is true.
+      if (fsi.last_vote.timestamp <= latest_qc_block_timestamp) {
+         res.decision = vote_decision::strong_vote;
+      } else if (bsp->core.extends(fsi.last_vote.block_id)) {
+         // If `fsi.other_branch_latest_time` is not present, it will have a timestamp slot of
+         // 0, which means it will always be less than or equal to `latest_qc_block_timestamp`.
+         // So, we don't need to separately check for the case where
+         // `fsi.other_branch_latest_time` is not present.
+         if (fsi.other_branch_latest_time <= latest_qc_block_timestamp) {
+            res.decision = vote_decision::strong_vote;
+         } else {
+            res.decision = vote_decision::weak_vote;
+         }
+      } else {
+         res.decision                 = vote_decision::weak_vote;
+         fsi.other_branch_latest_time = fsi.last_vote.timestamp;
       }
 
-      auto& latest_qc_claim__block_ref = bsp->core.get_block_reference(bsp->core.latest_qc_claim().block_num);
-      if (voting_strong) {
-         fsi.votes_forked_since_latest_strong_vote = false;             // always reset to false on strong vote
-         if (latest_qc_claim__block_ref.timestamp > fsi.lock.timestamp)
-            fsi.lock = latest_qc_claim__block_ref;
-      } else {
-         // On a weak vote, if `votes_forked_since_latest_strong_vote` was already true, then it should remain true.
-         // The only way `votes_forked_since_latest_strong_vote` can change from false to true is on a weak vote
-         // for a block b where the last_vote references a block that is not an ancestor of b
-         // --------------------------------------------------------------------------------------------------------
-         fsi.votes_forked_since_latest_strong_vote =
-            fsi.votes_forked_since_latest_strong_vote || !bsp->core.extends(fsi.last_vote.block_id);
+      if (res.decision == vote_decision::strong_vote) {
+         fsi.other_branch_latest_time = block_timestamp_type{};
+         if (latest_qc_block_timestamp > fsi.lock.timestamp) {
+            fsi.lock = bsp->core.get_block_reference(bsp->core.latest_qc_claim().block_num);
+         }
       }
 
       fsi.last_vote = bsp->make_block_ref();
-
-      res.decision = voting_strong ? vote_decision::strong_vote : vote_decision::weak_vote;
    }
 
    fc_dlog(vote_logger, "block=${bn} ${id}, liveness_check=${l}, safety_check=${s}, monotony_check=${m}, can vote=${can_vote}, voting=${v}, locked=${lbn} ${lid}",
@@ -103,9 +105,9 @@ finalizer::vote_result finalizer::decide_vote(const block_state_ptr& bsp) {
 bool finalizer::maybe_update_fsi(const block_state_ptr& bsp) {
    auto& latest_qc_claim__block_ref = bsp->core.get_block_reference(bsp->core.latest_qc_claim().block_num);
    if (latest_qc_claim__block_ref.timestamp > fsi.lock.timestamp && bsp->timestamp() > fsi.last_vote.timestamp) {
-      fsi.lock                                  = latest_qc_claim__block_ref;
-      fsi.last_vote                             = bsp->make_block_ref();
-      fsi.votes_forked_since_latest_strong_vote = false; // always reset to false on strong vote
+      fsi.lock                     = latest_qc_claim__block_ref;
+      fsi.last_vote                = bsp->make_block_ref();
+      fsi.other_branch_latest_time = block_timestamp_type{}; // always reset on strong vote
       return true;
    }
    return false;
@@ -219,14 +221,27 @@ void my_finalizers_t::save_finalizer_safety_info() const {
 
 // ----------------------------------------------------------------------------------------
 
+// Corresponds to safety_file_version_0
+struct finalizer_safety_information_v0 {
+   block_ref last_vote;
+   block_ref lock;
+   bool      votes_forked_since_latest_strong_vote {false};
+};
+
 void my_finalizers_t::load_finalizer_safety_info_v0(fsi_map& res) {
    uint64_t num_finalizers {0};
    fc::raw::unpack(persist_file, num_finalizers);
    for (size_t i=0; i<num_finalizers; ++i) {
       bls_public_key pubkey;
-      my_finalizers_t::fsi_t fsi;
+      finalizer_safety_information_v0 fsi_v0;
       fc::raw::unpack(persist_file, pubkey);
-      fc::raw::unpack(persist_file, fsi);
+      fc::raw::unpack(persist_file, fsi_v0);
+      my_finalizers_t::fsi_t fsi{
+         .last_vote = fsi_v0.last_vote,
+         .lock = fsi_v0.lock,
+         .other_branch_latest_time = fsi_v0.votes_forked_since_latest_strong_vote ? fsi_v0.last_vote.timestamp
+                                                                                  : block_timestamp_type{}
+      };
       res.emplace(pubkey, fsi);
    }
 }
@@ -371,3 +386,5 @@ void my_finalizers_t::set_default_safety_information(const fsi_t& fsi) {
 }
 
 } // namespace eosio::chain
+
+FC_REFLECT(eosio::chain::finalizer_safety_information_v0, (last_vote)(lock)(votes_forked_since_latest_strong_vote));

libraries/chain/include/eosio/chain/finality/finalizer.hpp

@@ -35,7 +35,7 @@ namespace eosio::chain {
    struct finalizer_safety_information {
       block_ref            last_vote;
       block_ref            lock;
-      bool                 votes_forked_since_latest_strong_vote {false};
+      block_timestamp_type other_branch_latest_time;
 
       static constexpr uint64_t magic = 0x5AFE11115AFE1111ull;
 
@@ -44,7 +44,7 @@ namespace eosio::chain {
       auto operator==(const finalizer_safety_information& o) const {
          return last_vote == o.last_vote &&
             lock == o.lock &&
-            votes_forked_since_latest_strong_vote == o.votes_forked_since_latest_strong_vote;
+            other_branch_latest_time == o.other_branch_latest_time;
       }
    };
 
@@ -74,8 +74,10 @@ namespace eosio::chain {
       ///
       /// Version 0: Spring 1.0.0 RC2 - File has fixed packed sizes with inactive safety info written to the end
       ///                               of the file. Consists of [finalizer public_key, FSI]..
-      /// Version 1: Spring 1.0.0 RC3 - Variable length FSI with inactive safety info written at the beginning of
-      ///                               the file. Uses crc32 checksum to verify data on read.
+      /// Version 1: Spring 1.0.0 RC3 - File has inactive FSIs written at the beginning of the file. Uses crc32
+      ///                               checksum to verify data on read. Removes FSI
+      ///                               votes_forked_since_latest_strong_vote from the version 0 FSI and replaces it
+      ///                               with other_branch_latest_time.
       ///
       static constexpr uint64_t safety_file_version_0       = 0;
       static constexpr uint64_t safety_file_version_1       = 1;
@@ -192,7 +194,7 @@ namespace eosio::chain {
 
 namespace std {
    inline std::ostream& operator<<(std::ostream& os, const eosio::chain::finalizer_safety_information& fsi) {
-      os << "fsi(" << fsi.last_vote << ", " << fsi.lock << ", " << fsi.votes_forked_since_latest_strong_vote << ")";
+      os << "fsi(" << fsi.last_vote << ", " << fsi.lock << ", " << fsi.other_branch_latest_time << ")";
       return os;
    }
 
@@ -210,5 +212,5 @@ namespace std {
    }
 }
 
-FC_REFLECT(eosio::chain::finalizer_safety_information, (last_vote)(lock)(votes_forked_since_latest_strong_vote))
+FC_REFLECT(eosio::chain::finalizer_safety_information, (last_vote)(lock)(other_branch_latest_time))
 FC_REFLECT_ENUM(eosio::chain::finalizer::vote_decision, (strong_vote)(weak_vote)(no_vote))

libraries/libfc/include/fc/io/datastream_crc.hpp

@@ -71,6 +71,7 @@ class datastream_crc {
 
    // only use with p==0, otherwise use seekp() below
    bool seekp(size_t p) {
+      assert(p == 0);
       if (p == 0) {
          crc_.reset();
          return ds_.seekp(0);

unittests/finalizer_tests.cpp

@@ -47,7 +47,7 @@ std::vector<FSI> create_random_fsi(size_t count) {
          .lock                  = block_ref{sha256::hash("lock"s + std::to_string(i)),
                                             tstamp(i * 100),
                                             sha256::hash("lock_digest"s + std::to_string(i))},
-         .votes_forked_since_latest_strong_vote = false
+         .other_branch_latest_time = block_timestamp_type{}
       });
       if (i)
          assert(res.back() != res[0]);
@@ -101,7 +101,7 @@ BOOST_AUTO_TEST_CASE( basic_finalizer_safety_file_io ) try {
 
    fsi_t fsi { .last_vote = proposals[6],
                .lock = proposals[2],
-               .votes_forked_since_latest_strong_vote = false };
+               .other_branch_latest_time = block_timestamp_type{} };
 
    bls_keys_t k("alice"_n);
    bls_pub_priv_key_map_t local_finalizers = { { k.pubkey_str, k.privkey_str } };
@@ -134,7 +134,7 @@ BOOST_AUTO_TEST_CASE( corrupt_finalizer_safety_file ) try {
 
    fsi_t fsi { .last_vote = proposals[6],
                .lock = proposals[2],
-               .votes_forked_since_latest_strong_vote = false };
+               .other_branch_latest_time = block_timestamp_type{} };
 
    bls_keys_t k("alice"_n);
    bls_pub_priv_key_map_t local_finalizers = { { k.pubkey_str, k.privkey_str } };

unittests/finalizer_vote_tests.cpp

@@ -143,7 +143,7 @@ struct simulator_t {
       forkdb.reset_root(genesis);
 
       block_ref genesis_ref(genesis->id(), genesis->timestamp(), genesis->id());
-      my_finalizer.fsi = fsi_t{genesis_ref, genesis_ref, false};
+      my_finalizer.fsi = fsi_t{genesis_ref, genesis_ref, {}};
    }
 
    vote_result vote(const bsp& p) {