Only vote on recent blocks

libraries/chain/controller.cpp

@@ -960,7 +960,7 @@ struct controller_impl {
    named_thread_pool<chain>        thread_pool;
    deep_mind_handler*              deep_mind_logger = nullptr;
    bool                            okay_to_print_integrity_hash_on_stop = false;
-   bool                            allow_voting = true; // used in unit tests to create long forks or simulate not getting votes
+   bool                            testing_allow_voting = false; // used in unit tests to create long forks or simulate not getting votes
    async_t                         async_voting = async_t::yes;  // by default we post `create_and_send_vote_msg()` calls, used in tester
    async_t                         async_aggregation = async_t::yes; // by default we process incoming votes asynchronously
    my_finalizers_t                 my_finalizers;
@@ -3296,7 +3296,7 @@ struct controller_impl {
 
             if (!my_finalizers.empty()) {
                block_handle_accessor::apply_s<void>(chain_head, [&](const auto& head) {
-                  if (head->is_recent() || my_finalizers.is_active()) {
+                  if (head->is_recent() || testing_allow_voting) {
                      if (async_voting == async_t::no)
                         create_and_send_vote_msg(head);
                      else
@@ -3695,7 +3695,7 @@ struct controller_impl {
    }
 
    bool is_block_missing_finalizer_votes(const block_handle& bh) const {
-      if (!allow_voting || my_finalizers.empty())
+      if (my_finalizers.empty())
          return false;
 
       return std::visit(
@@ -3735,7 +3735,7 @@ struct controller_impl {
 
    // thread safe
    void create_and_send_vote_msg(const block_state_ptr& bsp) {
-      if (!allow_voting || !bsp->block->is_proper_svnn_block())
+      if (!bsp->block->is_proper_svnn_block())
          return;
 
       // Each finalizer configured on the node which is present in the active finalizer policy may create and sign a vote.
@@ -4002,7 +4002,7 @@ struct controller_impl {
       // 3. Otherwise, consider voting for that block according to the decide_vote rules.
 
       if (!my_finalizers.empty() && bsp->core.latest_qc_claim().block_num > 0) {
-         if (bsp->is_recent() || my_finalizers.is_active()) {
+         if (bsp->is_recent() || testing_allow_voting) {
             if (use_thread_pool == use_thread_pool_t::yes && async_voting == async_t::yes) {
                boost::asio::post(thread_pool.get_executor(), [this, bsp=bsp]() {
                   const auto& latest_qc_claim__block_ref = bsp->core.get_block_reference(bsp->core.latest_qc_claim().block_num);
@@ -5002,12 +5002,12 @@ void controller::commit_block(block_report& br) {
    my->commit_block(br, block_status::incomplete);
 }
 
-void controller::allow_voting(bool val) {
-   my->allow_voting = val;
+void controller::testing_allow_voting(bool val) {
+   my->testing_allow_voting = val;
 }
 
-bool controller::get_allow_voting_flag() {
-   return my->allow_voting;
+bool controller::get_testing_allow_voting_flag() {
+   return my->testing_allow_voting;
 }
 
 void controller::set_async_voting(async_t val) {

libraries/chain/finality/finalizer.cpp

@@ -288,8 +288,6 @@ void my_finalizers_t::set_keys(const std::map<std::string, std::string>& finaliz
 
    // now only inactive finalizers remain in safety_info => move it to inactive_safety_info
    inactive_safety_info = std::move(safety_info);
-
-   enable_voting = enable_immediate_voting;
 }
 
 

libraries/chain/include/eosio/chain/controller.hpp

@@ -182,8 +182,8 @@ namespace eosio::chain {
          void assemble_and_complete_block( block_report& br, const signer_callback_type& signer_callback );
          void sign_block( const signer_callback_type& signer_callback );
          void commit_block(block_report& br);
-         void allow_voting(bool val);
-         bool get_allow_voting_flag();
+         void testing_allow_voting(bool val);
+         bool get_testing_allow_voting_flag();
          void set_async_voting(async_t val);
          void set_async_aggregation(async_t val);
          void maybe_switch_forks(const forked_callback_t& cb, const trx_meta_cache_lookup& trx_lookup);

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

@@ -75,7 +75,6 @@ namespace eosio::chain {
    private:
       const std::filesystem::path       persist_file_path;     // where we save the safety data
       std::atomic<bool>                 has_voted{false};      // true if this node has voted and updated safety info
-      std::atomic<bool>                 enable_voting{false};
       mutable std::mutex                mtx;
       mutable fc::datastream<fc::cfile> persist_file;          // we want to keep the file open for speed
       std::map<bls_public_key, finalizer>  finalizers;         // the active finalizers for this node, loaded at startup, not mutated afterwards
@@ -93,11 +92,6 @@ namespace eosio::chain {
          if (finalizers.empty())
             return;
 
-         if (!enable_voting.load(std::memory_order_relaxed)) { // Avoid extra processing while syncing. Once caught up, consider voting
-            if (!bsp->is_recent())
-               return;
-            enable_voting.store(true, std::memory_order_relaxed);
-         }
          assert(bsp->active_finalizer_policy);
 
          std::vector<vote_message_ptr> votes;
@@ -139,7 +133,6 @@ namespace eosio::chain {
 
       size_t  size() const { return finalizers.size(); }   // doesn't change, thread safe
       bool    empty() const { return finalizers.empty(); } // doesn't change, thread safe
-      bool    is_active() const { return !empty() && enable_voting.load(std::memory_order_relaxed); } // thread safe
 
       template<typename F>
       bool all_of_public_keys(F&& f) const { // only access keys which do not change, thread safe

libraries/testing/include/eosio/testing/tester.hpp

@@ -453,7 +453,7 @@ namespace eosio::testing {
          }
 
          void allow_voting(bool val) {
-            control->allow_voting(val);
+            control->testing_allow_voting(val);
          }
 
          const controller::config& get_config() const {

libraries/testing/tester.cpp

@@ -337,6 +337,7 @@ namespace eosio::testing {
 
       control.reset( new controller(cfg, std::move(pfs), *expected_chain_id) );
       control->add_indices();
+      control->testing_allow_voting(true);
       if (lambda) lambda();
       chain_transactions.clear();
       [[maybe_unused]] auto accepted_block_connection = control->accepted_block().connect([this]( block_signal_params t ){
@@ -538,22 +539,22 @@ namespace eosio::testing {
          // This is not the case for tests with forks, so for these tests we should set
          // `_expect_votes` to false by calling `base_tester::do_check_for_votes(false)`
          // ----------------------------------------------------------------------------
-         FC_ASSERT(c.is_block_missing_finalizer_votes(bh) == false, "Missing expected vote");
+         FC_ASSERT(!c.get_testing_allow_voting_flag() || !c.is_block_missing_finalizer_votes(bh), "Missing expected vote");
       }
    }
 
    signed_block_ptr base_tester::produce_blocks( uint32_t n, bool empty ) {
       signed_block_ptr res;
-      bool allow_voting_originally = control->get_allow_voting_flag();
+      bool allow_voting_originally = control->get_testing_allow_voting_flag();
 
       for (uint32_t i = 0; i < n; ++i) {
          // For performance, only vote on the last four to move finality.
-         // Modify allow_voting only if it was set to true originally;
-         // otherwise the allow_voting would be set to true when `i >= 4` even though the user of
+         // Modify testing_allow_voting only if it was set to true originally;
+         // otherwise the testing_allow_voting would be set to true when `i >= 4` even though the user of
          // `produce_blocks` wants it to be true.
          // This is 4 instead of 3 because the extra block has to be produced to log_irreversible
          if (allow_voting_originally && n > 4)
-            control->allow_voting(i >= n - 4);
+            control->testing_allow_voting(i >= n - 4);
          res = empty ? produce_empty_block() : produce_block();
       }
       return res;

unittests/savanna_cluster.hpp

@@ -78,6 +78,8 @@ namespace savanna_cluster {
       }
 
       bool is_head_missing_finalizer_votes() {
+         if (!control->get_testing_allow_voting_flag())
+            return false;
          return control->is_block_missing_finalizer_votes(head());
       }
    };

unittests/state_history_tests.cpp

@@ -815,7 +815,7 @@ bool test_fork(uint32_t stride, uint32_t max_retained_files) {
       // the first block from chain2 is pushed to chain1. This is to ensure LIBs
       // on chain1 and chain2 are the same, and further blocks from chain2 can be
       // pushed into chain1's forkdb.
-      chain1.control->allow_voting(false);
+      chain1.control->testing_allow_voting(false);
       chain1.produce_block();
    }
 
@@ -831,7 +831,7 @@ bool test_fork(uint32_t stride, uint32_t max_retained_files) {
    if constexpr (std::is_same_v<T, state_history_tester<savanna_tester>>) {
       // Disable voting on chain2 such that chain2's blocks can form a fork when
       // pushed to chain1
-      chain2.control->allow_voting(false);
+      chain2.control->testing_allow_voting(false);
    }
 
    auto b = chain2.produce_block();