GH-2125 Fixed qc_claim comparison in block_header_state is_needed(qc_…

…claim). Fixed qc choice logic to use parent
AntelopeIO · Mar 8, 2024 · 27f2557 · 27f2557
1 parent 88451b4
commit 27f2557
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 13 deletions.
diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp
@@ -689,30 +689,28 @@ struct building_block {
                } else {
                   fork_db.apply_s<void>([&](const auto& forkdb) {
                      auto branch = forkdb.fetch_branch(parent_id());
-                     std::optional<quorum_certificate> qc;
                      for( auto it = branch.begin(); it != branch.end(); ++it ) {
-                        qc = (*it)->get_best_qc();
-                        if( qc ) {
+                        if( auto qc = (*it)->get_best_qc(); qc ) {
                            EOS_ASSERT( qc->block_num <= block_header::num_from_id(parent_id()), block_validate_exception,
                                        "most recent ancestor QC block number (${a}) cannot be greater than parent's block number (${p})",
                                        ("a", qc->block_num)("p", block_header::num_from_id(parent_id())) );
-                           auto qc_claim = qc_claim_t { qc->block_num, qc->qc.is_strong() };
+                           auto qc_claim = qc->to_qc_claim();
                            ilog("integrate qc is_needed qc: ${qc}, strong=${s}", ("qc", qc->block_num)("s", qc->qc.is_strong()));
-                           if( bb.parent.is_needed(*qc) ) {
+                           if( bb.parent.is_needed(qc_claim) ) {
                               ilog("integrate qc and qc claim ${qc} into block ${bn}", ("qc", qc_claim)("bn", block_header::num_from_id(parent_id())+1));
                               qc_data = qc_data_t{ *qc, qc_claim };
                            } else {
+                              // no new qc info, repeat existing
                               ilog("integrate only qc claim ${qc} into block ${bn}", ("qc", qc_claim)("bn", block_header::num_from_id(parent_id())+1));
-                              qc_data = qc_data_t{ {},  qc_claim };
+                              qc_data = qc_data_t{ {},  bb.parent.core.latest_qc_claim() };
                            }
                            break;
                         }
                      }
 
-                     if (!qc) {
-                        // This only happens when parent block is the IF genesis block.
-                        // There is no ancestor block which has a QC.
-                        // Construct a default QC claim.
+                     if (!qc_data) {
+                        // This only happens when parent block is the IF genesis block or starting from snapshot.
+                        // There is no ancestor block which has a QC. Construct a default QC claim.
                         qc_data = qc_data_t{ {}, bb.parent.core.latest_qc_claim() };
                      }
                   });

diff --git a/libraries/chain/include/eosio/chain/block_header_state.hpp b/libraries/chain/include/eosio/chain/block_header_state.hpp
@@ -72,9 +72,8 @@ struct block_header_state {
    block_header_state next(const signed_block_header& h, validator_t& validator) const;
 
    // block descending from this need the provided qc in the block extension
-   bool is_needed(const quorum_certificate& qc) const {
-      ilog("qc is_needed: ${qc} > ${lc} = ${r}", ("qc", qc.block_num)("lc", core.latest_qc_claim().block_num)("r", qc.block_num > core.latest_qc_claim().block_num));
-      return qc.block_num > core.latest_qc_claim().block_num;
+   bool is_needed(const qc_claim_t& qc_claim) const {
+      return qc_claim > core.latest_qc_claim();
    }
 
    const vector<digest_type>& get_new_protocol_feature_activations() const;

diff --git a/libraries/chain/include/eosio/chain/hotstuff/hotstuff.hpp b/libraries/chain/include/eosio/chain/hotstuff/hotstuff.hpp
@@ -1,6 +1,7 @@
 #pragma once
 
 #include <eosio/chain/block_timestamp.hpp>
+#include <eosio/chain/finality_core.hpp>
 #include <fc/crypto/bls_private_key.hpp>
 #include <fc/crypto/bls_public_key.hpp>
 #include <fc/crypto/bls_signature.hpp>
@@ -60,6 +61,10 @@ namespace eosio::chain {
    struct quorum_certificate {
       uint32_t                 block_num;
       valid_quorum_certificate qc;
+
+      qc_claim_t to_qc_claim() const {
+         return {.block_num = block_num, .is_strong_qc = qc.is_strong()};
+      }
    };