IF: make qc_info in instant_finality_extension non-optional

libraries/chain/block_header_state.cpp

@@ -88,10 +88,6 @@ block_header_state block_header_state::next(block_header_state_input& input) con
       result.activated_protocol_features = activated_protocol_features;
    }
 
-   // block_header_state_core
-   // -----------------------
-   result.core = input.qc_info ? core.next(*input.qc_info) : core;
-
    // proposal_mtree and finality_mtree
    // ---------------------------------
    // [greg todo] ??
@@ -127,17 +123,32 @@ block_header_state block_header_state::next(block_header_state_input& input) con
    //      ++input.new_finalizer_policy->generation;
 
 
-   // add IF block header extension
-   // -----------------------------
+   qc_info_t qc_info;
    uint16_t if_ext_id = instant_finality_extension::extension_id();
-   auto     if_entry  = header_exts.lower_bound(if_ext_id);
-   auto&    if_ext    = std::get<instant_finality_extension>(if_entry->second);
 
-   instant_finality_extension new_if_ext {if_ext.qc_info,
+   if (input.qc_info) {
+      qc_info = *input.qc_info;
+      dlog("qc_info from input -> final value: ${qci}",("qci", qc_info));
+   } else {
+      // copy previous qc_claim if we are not provided with a new one
+      // ------------------------------------------------------------
+      auto  if_entry  = header_exts.lower_bound(if_ext_id);
+      if (if_entry != header_exts.end()) {
+         const auto& qci = std::get<instant_finality_extension>(if_entry->second).qc_info;
+         qc_info = qci;
+         dlog("qc_info from existing extension -> final value: ${qci}",("qci",qc_info));
+      } else {
+         assert(0); // we should always get a previous if extension when in IF mode.
+      }
+   }
+
+   instant_finality_extension new_if_ext {qc_info,
                                           std::move(input.new_finalizer_policy),
                                           std::move(input.new_proposer_policy)};
-   if (input.qc_info)
-      new_if_ext.qc_info = *input.qc_info;
+
+   // block_header_state_core
+   // -----------------------
+   result.core = core.next(new_if_ext.qc_info);
 
    emplace_extension(result.header.header_extensions, if_ext_id, fc::raw::pack(new_if_ext));
    result.header_exts.emplace(if_ext_id, std::move(new_if_ext));

libraries/chain/block_state.cpp

@@ -39,7 +39,9 @@ block_state::block_state(const block_state_legacy& bsp) {
    header = bsp.header;
    core.last_final_block_num = bsp.block_num(); // [if todo] instant transition is not acceptable
    activated_protocol_features = bsp.activated_protocol_features;
-   std::optional<block_header_extension> ext = bsp.block->extract_header_extension(instant_finality_extension::extension_id());
+
+   auto if_ext_id = instant_finality_extension::extension_id();
+   std::optional<block_header_extension> ext = bsp.block->extract_header_extension(if_ext_id);
    assert(ext); // required by current transition mechanism
    const auto& if_extension = std::get<instant_finality_extension>(*ext);
    assert(if_extension.new_finalizer_policy); // required by current transition mechanism
@@ -48,6 +50,9 @@ block_state::block_state(const block_state_legacy& bsp) {
    active_proposer_policy->active_time = bsp.timestamp();
    active_proposer_policy->proposer_schedule = bsp.active_schedule;
    header_exts = bsp.header_exts;
+
+   auto& updated_if_extension = std::get<instant_finality_extension>(header_exts.lower_bound(if_ext_id)->second);
+   updated_if_extension.qc_info = { bsp.block_num(), false };
    block = bsp.block;
    validated = bsp.is_valid();
    pub_keys_recovered = bsp._pub_keys_recovered;

libraries/chain/controller.cpp

@@ -2854,16 +2854,17 @@ struct controller_impl {
 
    static std::optional<qc_data_t> extract_qc_data(const signed_block_ptr& b) {
       std::optional<qc_data_t> qc_data;
-      auto exts = b->validate_and_extract_extensions();
-      if (auto entry = exts.lower_bound(quorum_certificate_extension::extension_id()); entry != exts.end()) {
-         auto& qc_ext = std::get<quorum_certificate_extension>(entry->second);
-
-         // get the matching header extension... should always be present
-         auto hexts = b->validate_and_extract_header_extensions();
-         auto if_entry = hexts.lower_bound(instant_finality_extension::extension_id());
-         assert(if_entry != hexts.end());
+      auto hexts = b->validate_and_extract_header_extensions();
+      if (auto if_entry = hexts.lower_bound(instant_finality_extension::extension_id()); if_entry != hexts.end()) {
          auto& if_ext   = std::get<instant_finality_extension>(if_entry->second);
-         return qc_data_t{ std::move(qc_ext.qc), *if_ext.qc_info };
+
+         // get the matching qc extension if present
+         auto exts = b->validate_and_extract_extensions();
+         if (auto entry = exts.lower_bound(quorum_certificate_extension::extension_id()); entry != exts.end()) {
+            auto& qc_ext = std::get<quorum_certificate_extension>(entry->second);
+            return qc_data_t{ std::move(qc_ext.qc), if_ext.qc_info };
+         }
+         return qc_data_t{ {}, if_ext.qc_info };
       }
       return {};
    }
@@ -3110,19 +3111,9 @@ struct controller_impl {
       }
 
       const auto& if_ext = std::get<instant_finality_extension>(*header_ext);
-      if( !if_ext.qc_info ) {
-         EOS_ASSERT( block_exts.count(quorum_certificate_extension::extension_id()) == 0,
-                     block_validate_exception,
-                     "A block must have QC claim if it provides QC block extension. Block number: ${b}",
-                     ("b", b->block_num()) );
-
-         // If header extension does not have QC claim,
-         // do not continue.
-         return;
-      }
 
       // extract QC claim
-      qc_info_t qc_claim{ *if_ext.qc_info };
+      qc_info_t qc_claim{ if_ext.qc_info };
 
       // A block should not be able to claim there was a QC on a block that
       // is prior to the transition to IF.
@@ -3135,32 +3126,31 @@ struct controller_impl {
       auto prev_qc_info = prev_if_ext.qc_info;
 
       // validate QC claim against previous block QC info
-      if( prev_qc_info ) {
-         // new claimed QC block nubmber cannot be smaller than previous block's
-         EOS_ASSERT( qc_claim.last_qc_block_num >= prev_qc_info->last_qc_block_num,
-                     block_validate_exception,
-                     "claimed last_qc_block_num (${n1}) must be equal to or greater than previous block's last_qc_block_num (${n2}). Block number: ${b}",
-                     ("n1", qc_claim.last_qc_block_num)("n2", prev_qc_info->last_qc_block_num)("b", b->block_num()) );
-
-         if( qc_claim.last_qc_block_num == prev_qc_info->last_qc_block_num ) {
-            if( qc_claim.is_last_qc_strong == prev_qc_info->is_last_qc_strong ) {
-               // QC block extension is redundant
-               EOS_ASSERT( block_exts.count(quorum_certificate_extension::extension_id()) == 0,
-                           block_validate_exception,
-                           "A block should not provide QC block extension if QC claim is the same as previous block. Block number: ${b}",
-                           ("b", b->block_num()) );
-
-               // if previous block's header extension has the same claim, just return
-               // (previous block already validated the claim)
-               return;
-            }
 
-            // new claimed QC must be stricter than previous if block number is the same
-            EOS_ASSERT( qc_claim.is_last_qc_strong || !prev_qc_info->is_last_qc_strong,
+      // new claimed QC block number cannot be smaller than previous block's
+      EOS_ASSERT( qc_claim.last_qc_block_num >= prev_qc_info.last_qc_block_num,
+                  block_validate_exception,
+                  "claimed last_qc_block_num (${n1}) must be equal to or greater than previous block's last_qc_block_num (${n2}). Block number: ${b}",
+                  ("n1", qc_claim.last_qc_block_num)("n2", prev_qc_info.last_qc_block_num)("b", b->block_num()) );
+
+      if( qc_claim.last_qc_block_num == prev_qc_info.last_qc_block_num ) {
+         if( qc_claim.is_last_qc_strong == prev_qc_info.is_last_qc_strong ) {
+            // QC block extension is redundant
+            EOS_ASSERT( block_exts.count(quorum_certificate_extension::extension_id()) == 0,
                         block_validate_exception,
-                        "claimed QC (${s1}) must be stricter than previous block's (${s2}) if block number is the same. Block number: ${b}",
-                       ("s1", qc_claim.is_last_qc_strong)("s2", prev_qc_info->is_last_qc_strong)("b", b->block_num()) );
+                        "A block should not provide QC block extension if QC claim is the same as previous block. Block number: ${b}",
+                        ("b", b->block_num()) );
+
+            // if previous block's header extension has the same claim, just return
+            // (previous block already validated the claim)
+            return;
          }
+
+         // new claimed QC must be stricter than previous if block number is the same
+         EOS_ASSERT( qc_claim.is_last_qc_strong || !prev_qc_info.is_last_qc_strong,
+                     block_validate_exception,
+                     "claimed QC (${s1}) must be stricter than previous block's (${s2}) if block number is the same. Block number: ${b}",
+                     ("s1", qc_claim.is_last_qc_strong)("s2", prev_qc_info.is_last_qc_strong)("b", b->block_num()) );
       }
 
       if( block_exts.count(quorum_certificate_extension::extension_id()) == 0 ) {

libraries/chain/include/eosio/chain/hotstuff/instant_finality_extension.hpp

@@ -15,7 +15,7 @@ struct instant_finality_extension : fc::reflect_init {
    static constexpr bool     enforce_unique() { return true; }
 
    instant_finality_extension() = default;
-   instant_finality_extension(std::optional<qc_info_t> qc_info,
+   instant_finality_extension(qc_info_t qc_info,
                               std::optional<finalizer_policy> new_finalizer_policy,
                               std::shared_ptr<proposer_policy> new_proposer_policy) :
       qc_info(qc_info),
@@ -25,7 +25,7 @@ struct instant_finality_extension : fc::reflect_init {
 
    void reflector_init();
 
-   std::optional<qc_info_t>           qc_info;
+   qc_info_t                          qc_info;
    std::optional<finalizer_policy>    new_finalizer_policy;
    std::shared_ptr<proposer_policy>   new_proposer_policy;
 };

unittests/block_header_tests.cpp

@@ -31,8 +31,8 @@ BOOST_AUTO_TEST_CASE(instant_finality_extension_with_empty_values_test)
    BOOST_REQUIRE( !!ext );
 
    const auto& if_extension = std::get<instant_finality_extension>(*ext);
-   BOOST_REQUIRE_EQUAL( if_extension.qc_info->last_qc_block_num, last_qc_block_num );
-   BOOST_REQUIRE_EQUAL( if_extension.qc_info->is_last_qc_strong, is_last_qc_strong );
+   BOOST_REQUIRE_EQUAL( if_extension.qc_info.last_qc_block_num, last_qc_block_num );
+   BOOST_REQUIRE_EQUAL( if_extension.qc_info.is_last_qc_strong, is_last_qc_strong );
    BOOST_REQUIRE( !if_extension.new_finalizer_policy );
    BOOST_REQUIRE( !if_extension.new_proposer_policy );
 }
@@ -91,8 +91,8 @@ BOOST_AUTO_TEST_CASE(instant_finality_extension_with_values_test)
 
    const auto& if_extension = std::get<instant_finality_extension>(*ext);
 
-   BOOST_REQUIRE_EQUAL( if_extension.qc_info->last_qc_block_num, last_qc_block_num );
-   BOOST_REQUIRE_EQUAL( if_extension.qc_info->is_last_qc_strong, is_last_qc_strong );
+   BOOST_REQUIRE_EQUAL( if_extension.qc_info.last_qc_block_num, last_qc_block_num );
+   BOOST_REQUIRE_EQUAL( if_extension.qc_info.is_last_qc_strong, is_last_qc_strong );
 
    BOOST_REQUIRE( !!if_extension.new_finalizer_policy );
    BOOST_REQUIRE_EQUAL(if_extension.new_finalizer_policy->generation, 1u);