Provide finalizer policy in SHiP finality_data only when it gets promoted to pending

libraries/chain/block_header_state.cpp

@@ -153,7 +153,6 @@ void evaluate_finalizer_policies_for_promotion(const block_header_state& prev,
    if (target != prev_proposed.end()) {
       if (pending_slot_open) {
          // promote the target to pending
-         // https://github.com/AntelopeIO/spring/issues/306 will update generation properly
          auto block_num = next_header_state.block_num();
          next_pending.emplace(block_num, target->second);
       } else {

libraries/chain/block_state.cpp

@@ -341,18 +341,16 @@ finality_data_t block_state::get_finality_data() {
       base_digest = compute_base_digest(); // cache it
    }
 
-   // Check if there is any proposed finalizer policy in the block
-   std::optional<finalizer_policy> proposed_finalizer_policy;
+   // Check if there is a finalizer policy promoted to pending in the block
+   std::optional<finalizer_policy> pending_fin_pol;
    if (is_savanna_genesis_block()) {
-      // For Genesis Block, use the active finalizer policy which was proposed in the block.
-      proposed_finalizer_policy = *active_finalizer_policy;
-   } else {
-      for (const auto& p: proposed_finalizer_policies) {
-         if (p.first == block_num()) {
-            proposed_finalizer_policy = *p.second;
-            break;
-         }
-      }
+      // For Genesis Block, use the active finalizer policy which went through
+      // proposed to pending to active in the single block.
+      pending_fin_pol = *active_finalizer_policy;
+   } else if (pending_finalizer_policy.has_value() && pending_finalizer_policy->first == block_num()) {
+      // The `first` element of `pending_finalizer_policy` pair is the block number
+      // when the policy becomes pending
+      pending_fin_pol = *pending_finalizer_policy->second;
    }
 
    return {
@@ -361,7 +359,7 @@ finality_data_t block_state::get_finality_data() {
       .final_on_strong_qc_block_num       = core.final_on_strong_qc_block_num,
       .action_mroot                       = action_mroot,
       .base_digest                        = *base_digest,
-      .proposed_finalizer_policy          = std::move(proposed_finalizer_policy)
+      .pending_finalizer_policy           = std::move(pending_fin_pol)
    };
 }
 

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

@@ -65,7 +65,7 @@ struct finality_data_t {
    uint32_t     final_on_strong_qc_block_num{0};
    digest_type  action_mroot{};
    digest_type  base_digest{};
-   std::optional<finalizer_policy> proposed_finalizer_policy; // finalizer policy, if proposed in the block
+   std::optional<finalizer_policy> pending_finalizer_policy; // finalizer policy if one is promoted to pending in the block
 };
 
 struct block_state : public block_header_state {     // block_header_state provides parent link
@@ -183,5 +183,5 @@ using block_state_pair      = std::pair<std::shared_ptr<block_state_legacy>, blo
 // not exporting pending_qc or valid_qc
 FC_REFLECT( eosio::chain::valid_t::finality_leaf_node_t, (major_version)(minor_version)(block_num)(finality_digest)(action_mroot) )
 FC_REFLECT( eosio::chain::valid_t, (validation_tree)(validation_mroots))
-FC_REFLECT( eosio::chain::finality_data_t, (major_version)(minor_version)(active_finalizer_policy_generation)(final_on_strong_qc_block_num)(action_mroot)(base_digest)(proposed_finalizer_policy) )
+FC_REFLECT( eosio::chain::finality_data_t, (major_version)(minor_version)(active_finalizer_policy_generation)(final_on_strong_qc_block_num)(action_mroot)(base_digest)(pending_finalizer_policy) )
 FC_REFLECT_DERIVED( eosio::chain::block_state, (eosio::chain::block_header_state), (block)(strong_digest)(weak_digest)(pending_qc)(valid)(validated) )

libraries/state_history/abi.cpp

@@ -610,7 +610,7 @@ extern const char* const state_history_plugin_abi = R"({
                 { "name": "final_on_strong_qc_block_num", "type": "uint32" },
                 { "name": "action_mroot", "type": "checksum256" },
                 { "name": "base_digest", "type": "checksum256" },
-                { "name": "proposed_finalizer_policy", "type": "finalizer_policy?" }
+                { "name": "pending_finalizer_policy", "type": "finalizer_policy?" }
             ]
         }
     ],

unittests/CMakeLists.txt

@@ -75,6 +75,7 @@ target_link_libraries( unit_test eosio_chain_wrap state_history chainbase eosio_
 
 target_compile_options(unit_test PUBLIC -DDISABLE_EOSLIB_SERIALIZE)
 target_include_directories( unit_test PUBLIC
+                            ${CMAKE_CURRENT_SOURCE_DIR}
                             ${CMAKE_SOURCE_DIR}/libraries/testing/include
                             ${CMAKE_SOURCE_DIR}/test-contracts
                             ${CMAKE_BINARY_DIR}/contracts

unittests/block_state_tests.cpp

@@ -1,3 +1,5 @@
+#include <finality_test_cluster.hpp>
+
 #include <eosio/chain/block_state.hpp>
 #include <eosio/testing/tester.hpp>
 
@@ -7,12 +9,12 @@
 
 #include <boost/test/unit_test.hpp>
 
+using namespace eosio::chain;
+using namespace fc::crypto::blslib;
+
 BOOST_AUTO_TEST_SUITE(block_state_tests)
 
 BOOST_AUTO_TEST_CASE(aggregate_vote_test) try {
-   using namespace eosio::chain;
-   using namespace fc::crypto::blslib;
-
    digest_type block_id(fc::sha256("0000000000000000000000000000001"));
 
    digest_type strong_digest(fc::sha256("0000000000000000000000000000002"));
@@ -91,9 +93,6 @@ void do_quorum_test(const std::vector<uint64_t>& weights,
                     bool strong,
                     const std::vector<bool>& to_vote,
                     bool expected_quorum) {
-   using namespace eosio::chain;
-   using namespace fc::crypto::blslib;
-
    digest_type block_id(fc::sha256("0000000000000000000000000000001"));
    digest_type strong_digest(fc::sha256("0000000000000000000000000000002"));
    auto weak_digest(create_weak_digest(fc::sha256("0000000000000000000000000000003")));
@@ -180,9 +179,6 @@ BOOST_AUTO_TEST_CASE(quorum_test) try {
 } FC_LOG_AND_RETHROW();
 
 BOOST_AUTO_TEST_CASE(verify_qc_test) try {
-   using namespace eosio::chain;
-   using namespace fc::crypto::blslib;
-
    // prepare digests
    digest_type strong_digest(fc::sha256("0000000000000000000000000000002"));
    auto weak_digest(create_weak_digest(fc::sha256("0000000000000000000000000000003")));
@@ -377,4 +373,47 @@ BOOST_AUTO_TEST_CASE(verify_qc_test) try {
    }
 } FC_LOG_AND_RETHROW();
 
+BOOST_FIXTURE_TEST_CASE(get_finality_data_test, finality_test_cluster<4>) try {
+   // The test cluster consists of only 4 node -- node0 is both a producer and a finalizer.
+   // It has transitioned to Savanna after startup.
+
+   // fin_policy_0 is the active finalizer policy
+   BOOST_REQUIRE(fin_policy_0);
+
+   // fin_policy_indices_0 is the set of indices used in active finalizer policy
+   // to indicate which key of a node is used in the policy
+   auto key_indices = fin_policy_indices_0;
+   BOOST_REQUIRE(key_indices[0] == 0u);  // index 0 for node0 was used in active policy
+
+   // Propose a finalizer policy by changing the index of the key used by node0 to 1
+   key_indices[0] = 1;
+   node0.finkeys.set_finalizer_policy(key_indices);
+
+   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) {
+      produce_and_push_block();
+      process_votes(1, num_nodes - 1); // all non-producing nodes (starting from node1) vote
+
+      // We should not see pending_finalizer_policy in finality_data
+      finality_data = *node0.control->head_finality_data();
+      BOOST_REQUIRE(!finality_data.pending_finalizer_policy.has_value());
+   }
+
+   // Produce one more block. The proposed finalizer policy is promoted to pending in this block.
+   // We should see pending_finalizer_policy in finality_data
+   produce_and_push_block();
+   process_votes(1, num_nodes - 1); // all non-producing nodes (starting from node1) vote
+   finality_data = *node0.control->head_finality_data();
+   BOOST_REQUIRE(finality_data.pending_finalizer_policy.has_value());
+
+   // Produce another block. We should not see pending_finalizer_policy as
+   // no proposed finalizer policy is promoted to pending in this block
+   produce_and_push_block();
+   process_votes(1, num_nodes - 1); // all non-producing nodes (starting from node1) vote
+   finality_data = *node0.control->head_finality_data();
+   BOOST_REQUIRE(!finality_data.pending_finalizer_policy.has_value());
+} FC_LOG_AND_RETHROW();
+
 BOOST_AUTO_TEST_SUITE_END()