Use 2-chain for finality

libraries/chain/finality/finality_core.cpp

@@ -212,6 +212,7 @@ const qc_link& finality_core::get_qc_link_from(block_num_type block_num) const
 std::tuple<block_num_type, block_num_type, block_num_type> get_new_block_numbers(const finality_core& c, const qc_claim_t& most_recent_ancestor_with_qc)
 {
    assert(most_recent_ancestor_with_qc.block_num <= c.current_block_num()); // Satisfied by the precondition.
+   assert(c.latest_qc_claim().block_num <= most_recent_ancestor_with_qc.block_num); // Satisfied by the precondition.
 
    // Invariant 2 of core guarantees that:
    // c.last_final_block_num() <= c.links.front().source_block_num <= c.final_on_strong_qc_block_num  <= c.latest_qc_claim().block_num
@@ -223,53 +224,34 @@ std::tuple<block_num_type, block_num_type, block_num_type> get_new_block_numbers
       return {c.last_final_block_num(), c.links.front().source_block_num, c.final_on_strong_qc_block_num};
    }
 
-   const auto& link1 = c.get_qc_link_from(most_recent_ancestor_with_qc.block_num);
-
-   // By the post-condition of get_qc_link_from, link1.source_block_num == most_recent_ancestor_with_qc.block_num.
-   // By the invariant on qc_link, link1.target_block_num <= link1.source_block_num.
-   // Therefore, link1.target_block_num <= most_recent_ancestor_with_qc.block_num.
-   // And also by the precondition, link1.target_block_num <= c.current_block_num().
-
-   // If c.refs.empty() == true, then by invariant 3 of core, link1 == c.links.front() == c.links.back() and so
-   // link1.target_block_num == c.current_block_num().
-
-   // Otherwise, if c.refs.empty() == false, consider two cases.
-   // Case 1: link1 != c.links.back()
-   //   In this case, link1.target_block_num <= link1.source_block_num < c.links.back().source_block_num.
-   //   The strict inequality is justified by invariant 7 of core.
-   //   Therefore, link1.target_block_num < c.current_block_num().
-   // Case 2: link1 == c.links.back()
-   //   In this case, link1.target_block_num < link1.source_block_num == c.links.back().source_block_num.
-   //   The strict inequality is justified because target_block_num and source_block_num of a qc_link can only be equal for a
-   //   genesis block. And a link mapping genesis block number to genesis block number can only possibly exist for c.links.front().
-   //   Therefore, link1.target_block_num < c.current_block_num().
-
-   // There must exist some link, call it link0, within c.links where 
-   // link0.target_block_num == c.final_on_strong_qc_block_num and link0.source_block_num <= c.latest_qc_claim().block_num.
-   // By the precondition, link0.source_block_num <= most_recent_ancestor_with_qc.block_num.
-   // If c.links.size() > 1, then by invariant 7 of core, link0.target_block_num <= link1.target_block_num.
-   // Otherwise if c.links.size() == 1, then link0 == link1 and so link0.target_block_num == link1.target_block_num.
-   // Therefore, c.final_on_strong_qc_block_num <= link1.target_block_num.
-
-   assert(c.final_on_strong_qc_block_num <= link1.target_block_num); // Satisfied by justification above.
-
-   // Finality does not advance if a better 3-chain is not found.
-   if (!link1.is_link_strong || (link1.target_block_num < c.links.front().source_block_num)) {
-      return {c.last_final_block_num(), c.links.front().source_block_num, link1.target_block_num};
-   }
-
-   const auto& link2 = c.get_qc_link_from(link1.target_block_num);
-
-   // By the post-condition of get_qc_link_from, link2.source_block_num == link1.target_block_num.
-   // By the invariant on qc_link, link2.target_block_num <= link2.source_block_num.
-   // Therefore, link2.target_block_num <= link1.target_block_num.
-
-   // Wherever link2 is found within c.links, it must be the case that c.links.front().target_block_num <= link2.target_block_num.
-   // This is obvious if c.links.size() == 1 (even though the code would even not get to this point if c.links.size() == 1), and 
-   // for the case where c.links.size() > 1, it is justified by invariant 7 of core.
-   // Therefore, c.last_final_block_num() <= link2.target_block_num.
-
-   return {link2.target_block_num, link2.source_block_num, link1.target_block_num}; 
+   const auto& link = c.get_qc_link_from(most_recent_ancestor_with_qc.block_num);
+
+   // Wherever link is found within c.links, it must be either c.links.front() or
+   // a link after c.links.front() in c.links.
+   // In both cases, by invariant 7 of core,
+   // c.links.front().target_block_num <= link.target_block_num
+   // c.links.front().source_block_num <= link.source_block_num
+   assert(c.last_final_block_num() <= link.target_block_num); // by justification above and c.last_final_block_num() == c.links.front().target_block_num
+   assert(c.links.front().source_block_num <= link.source_block_num); // by justification above
+
+   // 1. By the post-condition of get_qc_link_from, link.source_block_num == most_recent_ancestor_with_qc.block_num.
+   //    By the invariant on qc_link, link.target_block_num <= link.source_block_num.
+   //    Therefore, link.target_block_num <= most_recent_ancestor_with_qc.block_num.
+   //
+   // 2. There must exist some link, call it link0, within c.links where
+   //    link0.target_block_num == c.final_on_strong_qc_block_num and link0.source_block_num <= c.latest_qc_claim().block_num.
+   //    By the precondition, link0.source_block_num <= most_recent_ancestor_with_qc.block_num.
+   //    Since most_recent_ancestor_with_qc.block_num == link.source_block_num,
+   //    we have link0.source_block_num <= link.source_block_num.
+   //    If c.links.size() > 1, then by invariant 7 of core, link0.target_block_num <= link.target_block_num.
+   //    Otherwise if c.links.size() == 1, then link0 == link and so link0.target_block_num == link.target_block_num.
+   //    Therefore, c.final_on_strong_qc_block_num <= link.target_block_num.
+   //
+   //    From 1 and 2, we have c.final_on_strong_qc_block_num <= most_recent_ancestor_with_qc.block_num
+   assert(c.final_on_strong_qc_block_num <= most_recent_ancestor_with_qc.block_num);
+
+   // Use two-chain for finality advance
+   return {link.target_block_num, link.source_block_num, most_recent_ancestor_with_qc.block_num};
 }
 
 core_metadata finality_core::next_metadata(const qc_claim_t& most_recent_ancestor_with_qc) const

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

@@ -77,6 +77,10 @@ namespace eosio::testing {
       yes // tester calls startup() during initialization.
    };
 
+   // Number of chains required for a block to become final.
+   // Current protocol is 2: strong-strong or weak-strong.
+   constexpr size_t num_chains_to_final = 2;
+
    std::ostream& operator<<(std::ostream& os, setup_policy p);
 
    std::vector<uint8_t> read_wasm( const char* fn );
@@ -957,9 +961,9 @@ namespace eosio::testing {
             BOOST_REQUIRE(pt_block->is_proper_svnn_block());
          }
 
-         // lib must advance after 3 blocks
+         // lib must advance after num_chains_to_final blocks
          // -------------------------------
-         for (size_t i=0; i<3; ++i)
+         for (size_t i=0; i<num_chains_to_final; ++i)
             auto b = produce_block();
 
          BOOST_REQUIRE_EQUAL(t.lib_block->block_num(), pt_block->block_num());

unittests/block_header_state_tests.cpp

@@ -2,6 +2,7 @@
 #include <eosio/chain/block_header_state.hpp>
 
 using namespace eosio::chain;
+using namespace eosio::testing;
 
 // Function to be tested. Implemented in block_header_state.cpp`
 namespace eosio::chain {
@@ -366,15 +367,15 @@ BOOST_FIXTURE_TEST_CASE(finalizer_policies_change_edge_case_strong_qc_test, fina
    key_indices[0] = 2;
    auto policy_c_pubkeys = node0.finkeys.set_finalizer_policy(key_indices).pubkeys;
 
-   // We did produce_and_push_block() after proposing Policy B; need 5 more rounds
-   // of strong QC to make two 3-chains for Policy B to be activated.
-   for (size_t i=0; i<5; ++i) {
+   // We did produce_and_push_block() after proposing Policy B; need `2*num_chains_to_final - 1`
+   // more rounds of strong QC to make two 2-chains for Policy B to be activated.
+   for (size_t i=0; i<(2*num_chains_to_final - 1); ++i) {
       produce_and_push_block();
       process_votes(node1_index, num_nodes - 1); // all non-producing nodes vote strong
       node0.check_head_finalizer_policy(policy_a_generation, fin_policy_pubkeys_0); // original policy still active
    }
 
-   // we just completed the two 3-chains, so the next block we produce will have
+   // we just completed the two 2-chains, so the next block we produce will have
    // Policy B activated
    produce_and_push_block();
    node0.check_head_finalizer_policy(policy_b_generation, policy_b_pubkeys);
@@ -418,29 +419,27 @@ BOOST_FIXTURE_TEST_CASE(finalizer_policies_change_edge_case_weak_qc_test, finali
    key_indices[0] = 2;
    auto policy_c_pubkeys = node0.finkeys.set_finalizer_policy(key_indices).pubkeys;
 
-   // Produce 4 rounds; all the votes are strong votes.
-   for (size_t i=0; i<4; ++i) {
+   // Require 2*num_chains_to_final rounds for a policy to become active;
+   // reserve 2 rounds below. That's why produce (2*num_chains_to_final - 2) first.
+   for (size_t i=0; i<(2*num_chains_to_final - 2); ++i) {
       produce_and_push_block();
       process_votes(node1_index, num_nodes - 1); // all non-producing nodes vote strong
       node0.check_head_finalizer_policy(policy_a_generation, fin_policy_pubkeys_0); // original policy still active
    }
 
    produce_and_push_block();
-   // node1 votes strong
-   process_vote(node1_index);
-   // node2 votes weak
+   // make a weak QC
+   process_vote(node1_index); // node1 votes strong
    process_vote(node2_index, (size_t)-1 /*not used*/, vote_mode::weak); // node2 votes weak
    // active policy should still stay at Policy A as LIB has not advanced due to weak vote
    node0.check_head_finalizer_policy(policy_a_generation, fin_policy_pubkeys_0);
 
-   // produce 2 rounds of strong QC blocks
-   for (size_t i=0; i<2; ++i) {
-      produce_and_push_block();
-      process_votes(node1_index, num_nodes - 1); // all non-producing nodes vote strong
-      node0.check_head_finalizer_policy(policy_a_generation, fin_policy_pubkeys_0); // original policy still active
-   }
+   // produce 1 round of strong QC block
+   produce_and_push_block();
+   process_votes(node1_index, num_nodes - 1); // all non-producing nodes vote strong
+   node0.check_head_finalizer_policy(policy_a_generation, fin_policy_pubkeys_0); // original policy still active
 
-   // Now a weak-strong-strong chain is formed. LIB advances. Policy B becomes active.
+   // Now a weak-strong chain is formed. LIB advances. Policy B becomes active.
    produce_and_push_block();
    node0.check_head_finalizer_policy(policy_b_generation, policy_b_pubkeys);
    node1.check_head_finalizer_policy(policy_b_generation, policy_b_pubkeys);

unittests/block_state_tests.cpp

@@ -429,8 +429,8 @@ BOOST_FIXTURE_TEST_CASE(get_finality_data_test, finality_test_cluster<4>) try {
 
    finality_data_t finality_data;
 
-   // It takes one 3-chain for LIB to advance and 1 LIB proposed finalizer to be promoted to pending.
-   for (size_t i=0; i<3; ++i) {
+   // It takes one 2-chain for LIB to advance and 1 LIB proposed finalizer to be promoted to pending.
+   for (size_t i=0; i<eosio::testing::num_chains_to_final; ++i) {
       produce_and_push_block();
       process_votes(1, num_nodes - 1); // all non-producing nodes (starting from node1) vote
 

unittests/database_tests.cpp

@@ -56,7 +56,7 @@ BOOST_AUTO_TEST_SUITE(database_tests)
          // Check the last irreversible block number is set correctly.
          if constexpr (std::is_same_v<T, savanna_validating_tester>) {
             // In Savanna, after 3-chain finality is achieved.
-            const auto expected_last_irreversible_block_number = test.head().block_num() - 3;
+            const auto expected_last_irreversible_block_number = test.head().block_num() - num_chains_to_final;
             BOOST_TEST(test.last_irreversible_block_num() == expected_last_irreversible_block_number);
          } else {
             // With one producer, irreversibility should only just 1 block before
@@ -73,7 +73,7 @@ BOOST_AUTO_TEST_SUITE(database_tests)
 
          // Check the last irreversible block number is updated correctly
          if constexpr (std::is_same_v<T, savanna_validating_tester>) {
-            const auto next_expected_last_irreversible_block_number = test.head().block_num() - 3;
+            const auto next_expected_last_irreversible_block_number = test.head().block_num() - num_chains_to_final;
             BOOST_TEST(test.last_irreversible_block_num() == next_expected_last_irreversible_block_number);
          } else {
             const auto next_expected_last_irreversible_block_number = test.head().block_num() - 1;