IF: Net plugin takes appropriate actions on invalid QC claims and invalid vote messages

libraries/chain/block_state.cpp

@@ -69,7 +69,7 @@ void block_state::set_trxs_metas( deque<transaction_metadata_ptr>&& trxs_metas,
 }
 
 // Called from net threads
-std::pair<bool, std::optional<uint32_t>> block_state::aggregate_vote(const vote_message& vote) {
+std::pair<vote_status, std::optional<uint32_t>> block_state::aggregate_vote(const vote_message& vote) {
    const auto& finalizers = active_finalizer_policy->finalizers;
    auto it = std::find_if(finalizers.begin(),
                           finalizers.end(),
@@ -78,17 +78,17 @@ std::pair<bool, std::optional<uint32_t>> block_state::aggregate_vote(const vote_
    if (it != finalizers.end()) {
       auto index = std::distance(finalizers.begin(), it);
       const digest_type& digest = vote.strong ? strong_digest : weak_digest;
-      auto [valid, strong] = pending_qc.add_vote(vote.strong,
+      auto [status, strong] = pending_qc.add_vote(vote.strong,
 #warning TODO change to use std::span if possible
                                  std::vector<uint8_t>{digest.data(), digest.data() + digest.data_size()},
                                  index,
                                  vote.finalizer_key,
                                  vote.sig,
                                  finalizers[index].weight);
-      return {valid, strong ? core.final_on_strong_qc_block_num : std::optional<uint32_t>{}};
+      return {status, strong ? core.final_on_strong_qc_block_num : std::optional<uint32_t>{}};
    } else {
       wlog( "finalizer_key (${k}) in vote is not in finalizer policy", ("k", vote.finalizer_key) );
-      return {};
+      return {vote_status::unknown_public_key, {}};
    }
 }
 
@@ -116,12 +116,12 @@ void block_state::verify_qc(const valid_quorum_certificate& qc) const {
    // verfify quorum is met
    if( qc.is_strong() ) {
       EOS_ASSERT( strong_weights >= active_finalizer_policy->threshold,
-                  block_validate_exception,
+                  invalid_qc_claim,
                   "strong quorum is not met, strong_weights: ${s}, threshold: ${t}",
                   ("s", strong_weights)("t", active_finalizer_policy->threshold) );
    } else {
       EOS_ASSERT( strong_weights + weak_weights >= active_finalizer_policy->threshold,
-                  block_validate_exception,
+                  invalid_qc_claim,
                   "weak quorum is not met, strong_weights: ${s}, weak_weights: ${w}, threshold: ${t}",
                   ("s", strong_weights)("w", weak_weights)("t", active_finalizer_policy->threshold) );
    }
@@ -155,7 +155,7 @@ void block_state::verify_qc(const valid_quorum_certificate& qc) const {
    }
 
    // validate aggregated signature
-   EOS_ASSERT( fc::crypto::blslib::aggregate_verify( pubkeys, digests, qc._sig ),  block_validate_exception, "signature validation failed" );
+   EOS_ASSERT( fc::crypto::blslib::aggregate_verify( pubkeys, digests, qc._sig ),  invalid_qc_claim, "signature validation failed" );
 }
 
 std::optional<quorum_certificate> block_state::get_best_qc() const {

libraries/chain/controller.cpp

@@ -3082,12 +3082,12 @@ struct controller_impl {
          // block doesn't have a header extension either. Then return early.
          // ------------------------------------------------------------------------------------------
          EOS_ASSERT( !qc_extension_present,
-                     block_validate_exception,
+                     invalid_qc_claim,
                      "Block #${b} includes a QC block extension, but doesn't have a finality header extension",
                      ("b", block_num) );
 
          EOS_ASSERT( !prev_header_ext,
-                     block_validate_exception,
+                     invalid_qc_claim,
                      "Block #${b} doesn't have a finality header extension even though its predecessor does.",
                      ("b", block_num) );
          return;
@@ -3103,7 +3103,7 @@ struct controller_impl {
       // -------------------------------------------------------------------------------------------------
       if (!prev_header_ext) {
          EOS_ASSERT( !qc_extension_present && qc_claim.last_qc_block_num == block_num && qc_claim.is_last_qc_strong == false,
-                     block_validate_exception,
+                     invalid_qc_claim,
                      "Block #${b}, which is the finality transition block, doesn't have the expected extensions",
                      ("b", block_num) );
          return;
@@ -3121,15 +3121,15 @@ struct controller_impl {
 
       // new claimed QC block number cannot be smaller than previous block's
       EOS_ASSERT( qc_claim.last_qc_block_num >= prev_qc_claim.last_qc_block_num,
-                  block_validate_exception,
+                  invalid_qc_claim,
                   "Block #${b} claims a last_qc_block_num (${n1}) less than the previous block's (${n2})",
                   ("n1", qc_claim.last_qc_block_num)("n2", prev_qc_claim.last_qc_block_num)("b", block_num) );
 
       if( qc_claim.last_qc_block_num == prev_qc_claim.last_qc_block_num ) {
          if( qc_claim.is_last_qc_strong == prev_qc_claim.is_last_qc_strong ) {
             // QC block extension is redundant
             EOS_ASSERT( !qc_extension_present,
-                        block_validate_exception,
+                        invalid_qc_claim,
                         "Block #${b} should not provide a QC block extension since its QC claim is the same as the previous block's",
                         ("b", block_num) );
 
@@ -3140,35 +3140,35 @@ struct controller_impl {
 
          // new claimed QC must be stronger than previous if the claimed block number is the same
          EOS_ASSERT( qc_claim.is_last_qc_strong,
-                     block_validate_exception,
+                     invalid_qc_claim,
                      "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_claim.is_last_qc_strong)("b", block_num) );
       }
 
       // At this point, we are making a new claim in this block, so it better include a QC to justify this claim.
       EOS_ASSERT( qc_extension_present,
-                  block_validate_exception,
+                  invalid_qc_claim,
                   "Block #${b} is making a new finality claim, but doesn't include a qc to justify this claim", ("b", block_num) );
 
       const auto& qc_ext   = std::get<quorum_certificate_extension>(block_exts.lower_bound(qc_ext_id)->second);
       const auto& qc_proof = qc_ext.qc;
 
       // Check QC information in header extension and block extension match
       EOS_ASSERT( qc_proof.block_num == qc_claim.last_qc_block_num,
-                  block_validate_exception,
+                  invalid_qc_claim,
                   "Block #${b}: Mismatch between qc.block_num (${n1}) in block extension and last_qc_block_num (${n2}) in header extension",
                   ("n1", qc_proof.block_num)("n2", qc_claim.last_qc_block_num)("b", block_num) );
 
       // Verify claimed strictness is the same as in proof
       EOS_ASSERT( qc_proof.qc.is_strong() == qc_claim.is_last_qc_strong,
-                  block_validate_exception,
+                  invalid_qc_claim,
                   "QC is_strong (${s1}) in block extension does not match is_last_qc_strong (${s2}) in header extension. Block number: ${b}",
                   ("s1", qc_proof.qc.is_strong())("s2", qc_claim.is_last_qc_strong)("b", block_num) );
 
       // find the claimed block's block state on branch of id
       auto bsp = fork_db_fetch_bsp_by_num( prev.id, qc_claim.last_qc_block_num );
       EOS_ASSERT( bsp,
-                  block_validate_exception,
+                  invalid_qc_claim,
                   "Block state was not found in forkdb for last_qc_block_num ${q}. Block number: ${b}",
                   ("q", qc_claim.last_qc_block_num)("b", block_num) );
 
@@ -4387,18 +4387,18 @@ void controller::set_proposed_finalizers( const finalizer_policy& fin_pol ) {
 }
 
 // called from net threads
-bool controller::process_vote_message( const vote_message& vote ) {
-   auto do_vote = [&vote](auto& forkdb) -> std::pair<bool, std::optional<uint32_t>> {
+vote_status controller::process_vote_message( const vote_message& vote ) {
+   auto do_vote = [&vote](auto& forkdb) -> std::pair<vote_status, std::optional<uint32_t>> {
        auto bsp = forkdb.get_block(vote.proposal_id);
        if (bsp)
           return bsp->aggregate_vote(vote);
-       return {false, {}};
+       return {vote_status::unknown_block, {}};
    };
-   auto [valid, new_lib] = my->fork_db.apply_if<std::pair<bool, std::optional<uint32_t>>>(do_vote);
+   auto [status, new_lib] = my->fork_db.apply_if<std::pair<vote_status, std::optional<uint32_t>>>(do_vote);
    if (new_lib) {
       my->set_if_irreversible_block_num(*new_lib);
    }
-   return valid;
+   return status;
 };
 
 const producer_authority_schedule& controller::active_producers()const {

libraries/chain/hotstuff/hotstuff.cpp

@@ -20,18 +20,18 @@ inline std::vector<uint32_t> bitset_to_vector(const hs_bitset& bs) {
    return r;
 }
 
-bool pending_quorum_certificate::votes_t::add_vote(const std::vector<uint8_t>& proposal_digest, size_t index,
-                                                   const bls_public_key& pubkey, const bls_signature& new_sig) {
+vote_status pending_quorum_certificate::votes_t::add_vote(const std::vector<uint8_t>& proposal_digest, size_t index,
+                                                          const bls_public_key& pubkey, const bls_signature& new_sig) {
    if (_bitset[index]) {
-      return false; // shouldn't be already present
+      return vote_status::duplicate; // shouldn't be already present
    }
    if (!fc::crypto::blslib::verify(pubkey, proposal_digest, new_sig)) {
       wlog( "signature from finalizer ${i} cannot be verified", ("i", index) );
-      return false;
+      return vote_status::invalid_signature;
    }
    _bitset.set(index);
    _sig = fc::crypto::blslib::aggregate({_sig, new_sig}); // works even if _sig is default initialized (fp2::zero())
-   return true;
+   return vote_status::success;
 }
 
 void pending_quorum_certificate::votes_t::reset(size_t num_finalizers) {
@@ -59,11 +59,11 @@ bool pending_quorum_certificate::is_quorum_met() const {
 }
 
 // called by add_vote, already protected by mutex
-bool pending_quorum_certificate::add_strong_vote(const std::vector<uint8_t>& proposal_digest, size_t index,
-                                                 const bls_public_key& pubkey, const bls_signature& sig,
-                                                 uint64_t weight) {
-   if (!_strong_votes.add_vote(proposal_digest, index, pubkey, sig))
-      return false;
+vote_status pending_quorum_certificate::add_strong_vote(const std::vector<uint8_t>& proposal_digest, size_t index,
+                                                        const bls_public_key& pubkey, const bls_signature& sig,
+                                                        uint64_t weight) {
+   if (auto s = _strong_votes.add_vote(proposal_digest, index, pubkey, sig); s != vote_status::success)
+      return s;
    _strong_sum += weight;
 
    switch (_state) {
@@ -86,15 +86,15 @@ bool pending_quorum_certificate::add_strong_vote(const std::vector<uint8_t>& pro
       // getting another strong vote...nothing to do
       break;
    }
-   return true;
+   return vote_status::success;
 }
 
 // called by add_vote, already protected by mutex
-bool pending_quorum_certificate::add_weak_vote(const std::vector<uint8_t>& proposal_digest, size_t index,
-                                               const bls_public_key& pubkey, const bls_signature& sig,
-                                               uint64_t weight) {
-   if (!_weak_votes.add_vote(proposal_digest, index, pubkey, sig))
-      return false;
+vote_status pending_quorum_certificate::add_weak_vote(const std::vector<uint8_t>& proposal_digest, size_t index,
+                                                      const bls_public_key& pubkey, const bls_signature& sig,
+                                                      uint64_t weight) {
+   if (auto s = _weak_votes.add_vote(proposal_digest, index, pubkey, sig); s != vote_status::success)
+      return s;
    _weak_sum += weight;
 
    switch (_state) {
@@ -121,17 +121,17 @@ bool pending_quorum_certificate::add_weak_vote(const std::vector<uint8_t>& propo
       // getting another weak vote... nothing to do
       break;
    }
-   return true;
+   return vote_status::success;
 }
 
 // thread safe, <valid, strong>
-std::pair<bool, bool> pending_quorum_certificate::add_vote(bool strong, const std::vector<uint8_t>& proposal_digest, size_t index,
-                                                           const bls_public_key& pubkey, const bls_signature& sig,
-                                                           uint64_t weight) {
+std::pair<vote_status, bool> pending_quorum_certificate::add_vote(bool strong, const std::vector<uint8_t>& proposal_digest, size_t index,
+                                                                  const bls_public_key& pubkey, const bls_signature& sig,
+                                                                  uint64_t weight) {
    std::lock_guard g(*_mtx);
-   bool valid = strong ? add_strong_vote(proposal_digest, index, pubkey, sig, weight)
-                       : add_weak_vote(proposal_digest, index, pubkey, sig, weight);
-   return {valid, _state == state_t::strong};
+   vote_status s = strong ? add_strong_vote(proposal_digest, index, pubkey, sig, weight)
+                          : add_weak_vote(proposal_digest, index, pubkey, sig, weight);
+   return {s, _state == state_t::strong};
 }
 
 // thread safe

libraries/chain/hotstuff/test/finality_misc_tests.cpp

@@ -74,8 +74,8 @@ BOOST_AUTO_TEST_CASE(qc_state_transitions) try {
 
       // add duplicate weak vote
       // -----------------------
-      bool ok = weak_vote(qc, digest, 0, weight);
-      BOOST_CHECK(!ok); // vote was a duplicate
+      auto ok = weak_vote(qc, digest, 0, weight);
+      BOOST_CHECK(ok != vote_status::success); // vote was a duplicate
       BOOST_CHECK_EQUAL(qc.state(), state_t::weak_achieved);
       BOOST_CHECK(qc.is_quorum_met());
 

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

@@ -39,7 +39,7 @@ struct block_state : public block_header_state {     // block_header_state provi
    void                                set_trxs_metas(deque<transaction_metadata_ptr>&& trxs_metas, bool keys_recovered);
    const deque<transaction_metadata_ptr>& trxs_metas()  const { return cached_trxs; }
 
-   std::pair<bool, std::optional<uint32_t>> aggregate_vote(const vote_message& vote); // aggregate vote into pending_qc
+   std::pair<vote_status, std::optional<uint32_t>> aggregate_vote(const vote_message& vote); // aggregate vote into pending_qc
    void verify_qc(const valid_quorum_certificate& qc) const; // verify given qc is valid with respect block_state
 
    using bhs_t  = block_header_state;

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

@@ -8,6 +8,7 @@
 #include <eosio/chain/fork_database.hpp>
 #include <eosio/chain/protocol_feature_manager.hpp>
 #include <eosio/chain/webassembly/eos-vm-oc/config.hpp>
+#include <eosio/chain/hotstuff/hotstuff.hpp>
 
 #include <chainbase/pinnable_mapped_file.hpp>
 
@@ -326,7 +327,7 @@ namespace eosio::chain {
          // called by host function set_finalizers
          void set_proposed_finalizers( const finalizer_policy& fin_set );
          // called from net threads
-         bool process_vote_message( const vote_message& msg );
+         vote_status process_vote_message( const vote_message& msg );
 
          bool light_validation_allowed() const;
          bool skip_auth_check()const;

libraries/chain/include/eosio/chain/exceptions.hpp

@@ -255,6 +255,8 @@ namespace eosio { namespace chain {
                                     3030012, "Invalid block extension" )
       FC_DECLARE_DERIVED_EXCEPTION( ill_formed_additional_block_signatures_extension, block_validate_exception,
                                     3030013, "Block includes an ill-formed additional block signature extension" )
+      FC_DECLARE_DERIVED_EXCEPTION( invalid_qc_claim, block_validate_exception,
+                                    3030014, "Block includes an invalid QC claim" )
 
    FC_DECLARE_DERIVED_EXCEPTION( transaction_exception,             chain_exception,
                                  3040000, "Transaction exception" )

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

@@ -92,6 +92,14 @@ namespace eosio::chain {
       invalid                  // invalid message (other reason)
    };
 
+   enum class vote_status {
+      success,
+      duplicate,
+      unknown_public_key,
+      invalid_signature,
+      unknown_block
+   };
+
    using bls_public_key  = fc::crypto::blslib::bls_public_key;
    using bls_signature   = fc::crypto::blslib::bls_signature;
    using bls_private_key = fc::crypto::blslib::bls_private_key;
@@ -151,8 +159,8 @@ namespace eosio::chain {
          void resize(size_t num_finalizers) { _bitset.resize(num_finalizers); }
          size_t count() const { return _bitset.count(); }
 
-         bool add_vote(const std::vector<uint8_t>& proposal_digest, size_t index, const bls_public_key& pubkey,
-                       const bls_signature& new_sig);
+         vote_status add_vote(const std::vector<uint8_t>& proposal_digest, size_t index, const bls_public_key& pubkey,
+                              const bls_signature& new_sig);
 
          void reset(size_t num_finalizers);
       };
@@ -165,12 +173,12 @@ namespace eosio::chain {
       bool   is_quorum_met() const;
 
       // thread safe
-      std::pair<bool, bool> add_vote(bool strong,
-                                     const std::vector<uint8_t>&proposal_digest,
-                                     size_t index,
-                                     const bls_public_key&pubkey,
-                                     const bls_signature&sig,
-                                     uint64_t weight);
+      std::pair<vote_status, bool> add_vote(bool strong,
+                                            const std::vector<uint8_t>&proposal_digest,
+                                            size_t index,
+                                            const bls_public_key&pubkey,
+                                            const bls_signature&sig,
+                                            uint64_t weight);
 
       state_t state()  const { std::lock_guard g(*_mtx); return _state; };
       valid_quorum_certificate to_valid_quorum_certificate() const;
@@ -198,18 +206,18 @@ namespace eosio::chain {
       votes_t              _strong_votes;
 
       // called by add_vote, already protected by mutex
-      bool add_strong_vote(const std::vector<uint8_t>& proposal_digest,
-                           size_t index,
-                           const bls_public_key& pubkey,
-                           const bls_signature& sig,
-                           uint64_t weight);
+      vote_status add_strong_vote(const std::vector<uint8_t>& proposal_digest,
+                                  size_t index,
+                                  const bls_public_key& pubkey,
+                                  const bls_signature& sig,
+                                  uint64_t weight);
 
       // called by add_vote, already protected by mutex
-      bool add_weak_vote(const std::vector<uint8_t>& proposal_digest,
-                         size_t index,
-                         const bls_public_key& pubkey,
-                         const bls_signature& sig,
-                         uint64_t weight);
+      vote_status add_weak_vote(const std::vector<uint8_t>& proposal_digest,
+                                size_t index,
+                                const bls_public_key& pubkey,
+                                const bls_signature& sig,
+                                uint64_t weight);
    };
 } //eosio::chain