From 86bb45758287bd3d44b84db585299abeec14396c Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Fri, 11 Oct 2024 13:42:52 -0400 Subject: [PATCH 1/7] Remove `controller_impl::verify_qc`. --- libraries/chain/controller.cpp | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index efd241bbaa..cd6f9058c8 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -4112,11 +4112,6 @@ struct controller_impl { return {}; } - // This verifies BLS signatures and is expensive. - void verify_qc( const block_state& prev, const qc_t& qc ) { - prev.verify_qc(qc); - } - // thread safe, expected to be called from thread other than the main thread // tuple template @@ -4129,7 +4124,7 @@ struct controller_impl { if constexpr (is_proper_savanna_block) { if (qc) { verify_qc_future = post_async_task(thread_pool.get_executor(), [this, &qc, &prev] { - verify_qc(prev, *qc); + prev.verify_qc(*qc); }); } } @@ -4279,7 +4274,7 @@ struct controller_impl { if constexpr (std::is_same_v, block_state_ptr>) { if (conf.force_all_checks && qc) { - verify_qc(*head, *qc); + head->verify_qc(*qc); } } From 7b10b8ab383f34b0ba37563e514825c501a30110 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Fri, 11 Oct 2024 15:47:27 -0400 Subject: [PATCH 2/7] split qc verif into `verify_qc_basic` and `verify_qc_signatures` --- libraries/chain/block_state.cpp | 14 ++++- libraries/chain/controller.cpp | 16 ++++-- libraries/chain/finality/qc.cpp | 56 ++++++++++++------- .../chain/include/eosio/chain/block_state.hpp | 5 +- .../chain/include/eosio/chain/exceptions.hpp | 4 ++ .../chain/include/eosio/chain/finality/qc.hpp | 23 ++++---- 6 files changed, 81 insertions(+), 37 deletions(-) diff --git a/libraries/chain/block_state.cpp b/libraries/chain/block_state.cpp index 82d645ac6e..2ab111d913 100644 --- a/libraries/chain/block_state.cpp +++ b/libraries/chain/block_state.cpp @@ -266,9 +266,21 @@ vote_status_t block_state::has_voted(const bls_public_key& key) const { } // Called from net threads +void block_state::verify_qc_signatures(const qc_t& qc) const { + finalizer_policies_t policies = get_finalizer_policies(qc.block_num); // get policies active at claimed block number + qc.verify_signatures(policies); +} + +// Called from net threads +void block_state::verify_qc_basic(const qc_t& qc) const { + finalizer_policies_t policies = get_finalizer_policies(qc.block_num); // get policies active at claimed block number + qc.verify_basic(policies); +} + void block_state::verify_qc(const qc_t& qc) const { finalizer_policies_t policies = get_finalizer_policies(qc.block_num); // get policies active at claimed block number - qc.verify(policies); + qc.verify_basic(policies); + qc.verify_signatures(policies); } qc_claim_t block_state::extract_qc_claim() const { diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index cd6f9058c8..25fd002631 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -3915,7 +3915,8 @@ struct controller_impl { // Verify basic proper_block invariants. // Called from net-threads. It is thread safe as signed_block is never modified after creation. // ----------------------------------------------------------------------------- - std::optional verify_basic_proper_block_invariants( const block_id_type& id, const signed_block_ptr& b, const block_header_state& prev ) { + std::optional verify_basic_proper_block_invariants( const block_id_type& id, const signed_block_ptr& b, + const block_header_state& prev ) { assert(b->is_proper_svnn_block()); auto qc_ext_id = quorum_certificate_extension::extension_id(); @@ -4123,8 +4124,11 @@ struct controller_impl { log_and_drop_future verify_qc_future; if constexpr (is_proper_savanna_block) { if (qc) { - verify_qc_future = post_async_task(thread_pool.get_executor(), [this, &qc, &prev] { - prev.verify_qc(*qc); + // do basic checks on provided qc immediately (excluding signature verification) + prev.verify_qc_basic(*qc); + + verify_qc_future = post_async_task(thread_pool.get_executor(), [&qc, &prev] { + prev.verify_qc_signatures(*qc); }); } } @@ -4273,8 +4277,12 @@ struct controller_impl { std::optional qc = verify_basic_block_invariants({}, b, *head); if constexpr (std::is_same_v, block_state_ptr>) { + // do basic checks always (excluding signature verification) + head->verify_qc_basic(*qc); + if (conf.force_all_checks && qc) { - head->verify_qc(*qc); + // verify signatures only if `conf.force_all_checks` + head->verify_qc_signatures(*qc); } } diff --git a/libraries/chain/finality/qc.cpp b/libraries/chain/finality/qc.cpp index b7d97efe18..a97b83b992 100644 --- a/libraries/chain/finality/qc.cpp +++ b/libraries/chain/finality/qc.cpp @@ -39,7 +39,7 @@ inline void verify_dual_finalizers_votes(const finalizer_policies_t& policies, for (const auto& pending_fin: policies.pending_finalizer_policy->finalizers) { if (active_fin.public_key == pending_fin.public_key) { EOS_ASSERT(active_policy_sig.vote_same_at(pending_policy_sig, active_vote_index, pending_vote_index), - invalid_qc_claim, + invalid_qc, "qc ${bn} contains a dual finalizer ${k} which does not vote the same on active and pending policies", ("bn", block_num)("k", active_fin.public_key)); break; @@ -50,30 +50,39 @@ inline void verify_dual_finalizers_votes(const finalizer_policies_t& policies, } } -void qc_t::verify(const finalizer_policies_t& policies) const { +void qc_t::verify_signatures(const finalizer_policies_t& policies) const { const auto& strong_digest = policies.finality_digest; - auto weak_digest = create_weak_digest(strong_digest); + auto weak_digest = create_weak_digest(strong_digest); + active_policy_sig.verify_signatures(policies.active_finalizer_policy, strong_digest, weak_digest); + + if (pending_policy_sig) { + EOS_ASSERT(policies.pending_finalizer_policy, invalid_qc, + "qc ${bn} contains pending policy signature for nonexistent pending finalizer policy", ("bn", block_num)); + pending_policy_sig->verify_signatures(policies.pending_finalizer_policy, strong_digest, weak_digest); + } +} + +void qc_t::verify_basic(const finalizer_policies_t& policies) const { active_policy_sig.verify_vote_format(policies.active_finalizer_policy); - active_policy_sig.verify(policies.active_finalizer_policy, strong_digest, weak_digest); + active_policy_sig.verify_weights(policies.active_finalizer_policy); if (pending_policy_sig) { - EOS_ASSERT(policies.pending_finalizer_policy, invalid_qc_claim, + EOS_ASSERT(policies.pending_finalizer_policy, invalid_qc, "qc ${bn} contains pending policy signature for nonexistent pending finalizer policy", ("bn", block_num)); // verify that every finalizer included in both policies voted the same verify_dual_finalizers_votes(policies, active_policy_sig, *pending_policy_sig, block_num); pending_policy_sig->verify_vote_format(policies.pending_finalizer_policy); - pending_policy_sig->verify(policies.pending_finalizer_policy, strong_digest, weak_digest); + pending_policy_sig->verify_weights(policies.pending_finalizer_policy); } else { - EOS_ASSERT(!policies.pending_finalizer_policy, invalid_qc_claim, + EOS_ASSERT(!policies.pending_finalizer_policy, invalid_qc, "qc ${bn} does not contain pending policy signature for pending finalizer policy", ("bn", block_num)); } - } -// returns true iff the other and I voted iin the same way. +// returns true iff the other and I voted in the same way. bool qc_sig_t::vote_same_at(const qc_sig_t& other, uint32_t my_vote_index, uint32_t other_vote_index) const { assert(!strong_votes || my_vote_index < strong_votes->size()); assert(!weak_votes || my_vote_index < weak_votes->size()); @@ -94,19 +103,19 @@ void qc_sig_t::verify_vote_format(const finalizer_policy_ptr& fin_policy) const const auto& finalizers = fin_policy->finalizers; auto num_finalizers = finalizers.size(); - EOS_ASSERT( strong_votes || weak_votes, invalid_qc_claim, + EOS_ASSERT( strong_votes || weak_votes, invalid_qc, "Neither strong_votes nor weak_votes present for finalizer policy, generation ${n}", ("n", fin_policy->generation) ); // verify number of finalizers matches with vote bitset size if (strong_votes) { - EOS_ASSERT( num_finalizers == strong_votes->size(), invalid_qc_claim, + EOS_ASSERT( num_finalizers == strong_votes->size(), invalid_qc, "vote bitset size is not the same as the number of finalizers for the policy it refers to, " "vote bitset size: ${s}, num of finalizers for the policy: ${n}", ("s", strong_votes->size())("n", num_finalizers) ); } if (weak_votes) { - EOS_ASSERT( num_finalizers == weak_votes->size(), invalid_qc_claim, + EOS_ASSERT( num_finalizers == weak_votes->size(), invalid_qc, "vote bitset size is not the same as the number of finalizers for the policy it refers to, " "vote bitset size: ${s}, num of finalizers for the policy: ${n}", ("s", weak_votes->size())("n", num_finalizers) ); @@ -116,7 +125,7 @@ void qc_sig_t::verify_vote_format(const finalizer_policy_ptr& fin_policy) const if (strong_votes && weak_votes) { for (size_t i=0; isize(); ++i) { // at most one is true - EOS_ASSERT( !((*strong_votes)[i] && (*weak_votes)[i]), invalid_qc_claim, + EOS_ASSERT( !((*strong_votes)[i] && (*weak_votes)[i]), invalid_qc, "finalizer (bit index ${i}) voted both strong and weak", ("i", i) ); @@ -124,9 +133,7 @@ void qc_sig_t::verify_vote_format(const finalizer_policy_ptr& fin_policy) const } } -void qc_sig_t::verify(const finalizer_policy_ptr& fin_policy, - const digest_type& strong_digest, - const weak_digest_t& weak_digest) const { +void qc_sig_t::verify_weights(const finalizer_policy_ptr& fin_policy) const { const auto& finalizers = fin_policy->finalizers; auto num_finalizers = finalizers.size(); @@ -148,14 +155,21 @@ void qc_sig_t::verify(const finalizer_policy_ptr& fin_policy, // verfify quorum is met if( is_strong() ) { - EOS_ASSERT( strong_weights >= fin_policy->threshold, invalid_qc_claim, + EOS_ASSERT( strong_weights >= fin_policy->threshold, invalid_qc, "strong quorum is not met, strong_weights: ${s}, threshold: ${t}", ("s", strong_weights)("t", fin_policy->threshold) ); } else { - EOS_ASSERT( strong_weights + weak_weights >= fin_policy->threshold, invalid_qc_claim, + EOS_ASSERT( strong_weights + weak_weights >= fin_policy->threshold, invalid_qc, "weak quorum is not met, strong_weights: ${s}, weak_weights: ${w}, threshold: ${t}", ("s", strong_weights)("w", weak_weights)("t", fin_policy->threshold) ); } +} + +void qc_sig_t::verify_signatures(const finalizer_policy_ptr& fin_policy, + const digest_type& strong_digest, + const weak_digest_t& weak_digest) const { + const auto& finalizers = fin_policy->finalizers; + auto num_finalizers = finalizers.size(); // no reason to use bls_public_key wrapper std::vector pubkeys; @@ -190,7 +204,7 @@ void qc_sig_t::verify(const finalizer_policy_ptr& fin_policy, // validate aggregated signature EOS_ASSERT( bls12_381::aggregate_verify(pubkeys, digests, sig.jacobian_montgomery_le()), - invalid_qc_claim, "qc signature validation failed" ); + invalid_qc_signature, "qc signature validation failed" ); } @@ -409,7 +423,7 @@ std::optional aggregating_qc_t::get_best_qc(block_num_type block_num) cons bool aggregating_qc_t::set_received_qc(const qc_t& qc) { // qc should have already been verified via verify_qc, this EOS_ASSERT should never fire - EOS_ASSERT(!pending_policy_sig || qc.pending_policy_sig, invalid_qc_claim, + EOS_ASSERT(!pending_policy_sig || qc.pending_policy_sig, invalid_qc, "qc ${bn} expected to have a pending policy signature", ("bn", qc.block_num)); bool active_better = active_policy_sig.set_received_qc_sig(qc.active_policy_sig); bool pending_better = false; @@ -503,7 +517,7 @@ vote_status_t aggregating_qc_t::has_voted(const bls_public_key& key) const { return active_status; } - EOS_ASSERT(pending_policy_sig, invalid_qc_claim, + EOS_ASSERT(pending_policy_sig, invalid_qc, "qc does not contain pending policy signature for pending finalizer policy"); vote_status_t pending_status = finalizer_has_voted(pending_finalizer_policy, *pending_policy_sig, key); diff --git a/libraries/chain/include/eosio/chain/block_state.hpp b/libraries/chain/include/eosio/chain/block_state.hpp index 21e825bf1a..cd6df7dd72 100644 --- a/libraries/chain/include/eosio/chain/block_state.hpp +++ b/libraries/chain/include/eosio/chain/block_state.hpp @@ -151,7 +151,10 @@ struct block_state : public block_header_state { // block_header_state provi // connection_id only for logging aggregate_vote_result_t aggregate_vote(uint32_t connection_id, const vote_message& vote); // aggregate vote into aggregating_qc vote_status_t has_voted(const bls_public_key& key) const; - void verify_qc(const qc_t& qc) const; // verify given qc_t is valid with respect block_state + + void verify_qc_signatures(const qc_t& qc) const; // validate qc signatures (slow) + void verify_qc_basic(const qc_t& qc) const; // do basic checks on provided qc, excluding signature verification + void verify_qc(const qc_t& qc) const; // full qc verification incl. signatures using bhs_t = block_header_state; using bhsp_t = block_header_state_ptr; diff --git a/libraries/chain/include/eosio/chain/exceptions.hpp b/libraries/chain/include/eosio/chain/exceptions.hpp index 2a663a68c9..fbf9311128 100644 --- a/libraries/chain/include/eosio/chain/exceptions.hpp +++ b/libraries/chain/include/eosio/chain/exceptions.hpp @@ -257,6 +257,10 @@ namespace eosio { namespace chain { 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( invalid_qc, block_validate_exception, + 3030014, "Block includes an invalid QC" ) + FC_DECLARE_DERIVED_EXCEPTION( invalid_qc_signature, block_validate_exception, + 3030014, "Block includes a QC with invalid signature(s)" ) FC_DECLARE_DERIVED_EXCEPTION( transaction_exception, chain_exception, 3040000, "Transaction exception" ) diff --git a/libraries/chain/include/eosio/chain/finality/qc.hpp b/libraries/chain/include/eosio/chain/finality/qc.hpp index ec8173a7e4..38a9f4b345 100644 --- a/libraries/chain/include/eosio/chain/finality/qc.hpp +++ b/libraries/chain/include/eosio/chain/finality/qc.hpp @@ -63,15 +63,19 @@ namespace eosio::chain { }; struct qc_sig_t { - bool is_weak() const { return !!weak_votes; } - bool is_strong() const { return !weak_votes; } - std::optional strong_votes; std::optional weak_votes; bls_aggregate_signature sig; + bool is_weak() const { return !!weak_votes; } + bool is_strong() const { return !weak_votes; } + + // called from net threads + void verify_signatures(const finalizer_policy_ptr& fin_policy, const digest_type& strong_digest, const weak_digest_t& weak_digest) const; + // called from net threads - void verify(const finalizer_policy_ptr& fin_policy, const digest_type& strong_digest, const weak_digest_t& weak_digest) const; + void verify_weights(const finalizer_policy_ptr& fin_policy) const; + void verify_vote_format(const finalizer_policy_ptr& fin_policy) const; // returns true if vote indicated by my_vote_index in strong_votes or @@ -81,11 +85,9 @@ namespace eosio::chain { }; struct qc_t { - uint32_t block_num{0}; - // signatures corresponding to the active finalizer policy - qc_sig_t active_policy_sig; - // signatures corresponding to the pending finalizer policy if there is one - std::optional pending_policy_sig; + uint32_t block_num{0}; + qc_sig_t active_policy_sig; // signatures for the active finalizer policy + std::optional pending_policy_sig; // signatures for the pending finalizer policy (if any) bool is_strong() const { return active_policy_sig.is_strong() && (!pending_policy_sig || pending_policy_sig->is_strong()); @@ -96,7 +98,8 @@ namespace eosio::chain { return { .block_num = block_num, .is_strong_qc = is_strong() }; } - void verify(const finalizer_policies_t& policies) const; + void verify_signatures(const finalizer_policies_t& policies) const; // validate qc signatures + void verify_basic(const finalizer_policies_t& policies) const; // do basic checks on provided qc, excluding signature verification }; struct qc_data_t { From 499eb1252edbc53c07f1047c4db0115e5a326172 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Fri, 11 Oct 2024 16:22:13 -0400 Subject: [PATCH 3/7] Update tests according to new exception names. --- unittests/block_state_tests.cpp | 34 ++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/unittests/block_state_tests.cpp b/unittests/block_state_tests.cpp index 4a65e69d38..dbb05198db 100644 --- a/unittests/block_state_tests.cpp +++ b/unittests/block_state_tests.cpp @@ -421,7 +421,7 @@ BOOST_AUTO_TEST_CASE(verify_qc_test) try { qc_sig_t qc_sig(strong_votes, {}, agg_sig); qc_t qc{bsp->block_num(), qc_sig, {}}; - BOOST_CHECK_EXCEPTION( bsp->verify_qc(qc), invalid_qc_claim, eosio::testing::fc_exception_message_starts_with("strong quorum is not met") ); + BOOST_CHECK_EXCEPTION( bsp->verify_qc(qc), invalid_qc, eosio::testing::fc_exception_message_starts_with("strong quorum is not met") ); } { // weak QC quorum not met @@ -436,7 +436,7 @@ BOOST_AUTO_TEST_CASE(verify_qc_test) try { qc_sig_t qc_sig({}, weak_votes, agg_sig); qc_t qc{bsp->block_num(), qc_sig, {}}; - BOOST_CHECK_EXCEPTION( bsp->verify_qc(qc), invalid_qc_claim, eosio::testing::fc_exception_message_starts_with("weak quorum is not met") ); + BOOST_CHECK_EXCEPTION( bsp->verify_qc(qc), invalid_qc, eosio::testing::fc_exception_message_starts_with("weak quorum is not met") ); } { // strong QC bitset size does not match number of finalizers in the policy @@ -456,7 +456,7 @@ BOOST_AUTO_TEST_CASE(verify_qc_test) try { qc_sig_t qc_sig(strong_votes, {}, agg_sig); qc_t qc{bsp->block_num(), qc_sig, {}}; - BOOST_CHECK_EXCEPTION( bsp->verify_qc(qc), invalid_qc_claim, eosio::testing::fc_exception_message_starts_with("vote bitset size is not the same as the number of finalizers") ); + BOOST_CHECK_EXCEPTION( bsp->verify_qc(qc), invalid_qc, eosio::testing::fc_exception_message_starts_with("vote bitset size is not the same as the number of finalizers") ); } { // weak QC bitset size does not match number of finalizers in the policy @@ -476,7 +476,7 @@ BOOST_AUTO_TEST_CASE(verify_qc_test) try { qc_sig_t qc_sig({}, weak_votes, agg_sig); qc_t qc{bsp->block_num(), qc_sig, {}}; - BOOST_CHECK_EXCEPTION( bsp->verify_qc(qc), invalid_qc_claim, eosio::testing::fc_exception_message_starts_with("vote bitset size is not the same as the number of finalizers") ); + BOOST_CHECK_EXCEPTION( bsp->verify_qc(qc), invalid_qc, eosio::testing::fc_exception_message_starts_with("vote bitset size is not the same as the number of finalizers") ); } { // strong QC with a wrong signing private key @@ -494,7 +494,7 @@ BOOST_AUTO_TEST_CASE(verify_qc_test) try { qc_sig_t qc_sig(strong_votes, {}, sig); qc_t qc{bsp->block_num(), qc_sig, {}}; - BOOST_CHECK_EXCEPTION( bsp->verify_qc(qc), invalid_qc_claim, eosio::testing::fc_exception_message_is("qc signature validation failed") ); + BOOST_CHECK_EXCEPTION( bsp->verify_qc(qc), invalid_qc_signature, eosio::testing::fc_exception_message_is("qc signature validation failed") ); } { // strong QC with a wrong digest @@ -512,7 +512,7 @@ BOOST_AUTO_TEST_CASE(verify_qc_test) try { qc_sig_t qc_sig(strong_votes, {}, sig); qc_t qc{bsp->block_num(), qc_sig, {}}; - BOOST_CHECK_EXCEPTION( bsp->verify_qc(qc), invalid_qc_claim, eosio::testing::fc_exception_message_is("qc signature validation failed") ); + BOOST_CHECK_EXCEPTION( bsp->verify_qc(qc), invalid_qc_signature, eosio::testing::fc_exception_message_is("qc signature validation failed") ); } { // weak QC with a wrong signing private key @@ -530,7 +530,7 @@ BOOST_AUTO_TEST_CASE(verify_qc_test) try { qc_sig_t qc_sig(strong_votes, weak_votes, sig); qc_t qc{bsp->block_num(), qc_sig, {}}; - BOOST_CHECK_EXCEPTION( bsp->verify_qc(qc), invalid_qc_claim, eosio::testing::fc_exception_message_is("qc signature validation failed") ); + BOOST_CHECK_EXCEPTION( bsp->verify_qc(qc), invalid_qc_signature, eosio::testing::fc_exception_message_is("qc signature validation failed") ); } { // weak QC with a wrong digest @@ -548,7 +548,7 @@ BOOST_AUTO_TEST_CASE(verify_qc_test) try { qc_sig_t qc_sig(strong_votes, weak_votes, sig); qc_t qc{bsp->block_num(), qc_sig, {}}; - BOOST_CHECK_EXCEPTION( bsp->verify_qc(qc), invalid_qc_claim, eosio::testing::fc_exception_message_is("qc signature validation failed") ); + BOOST_CHECK_EXCEPTION( bsp->verify_qc(qc), invalid_qc_signature, eosio::testing::fc_exception_message_is("qc signature validation failed") ); } } FC_LOG_AND_RETHROW(); @@ -695,7 +695,7 @@ BOOST_AUTO_TEST_CASE(verify_qc_test_with_pending) try { qc_sig_t pending_qc_sig(strong_votes, {}, pending_agg_sig); qc_t qc{bsp->block_num(), active_qc_sig, pending_qc_sig}; - BOOST_CHECK_EXCEPTION( bsp->verify_qc(qc), invalid_qc_claim, eosio::testing::fc_exception_message_starts_with("strong quorum is not met") ); + BOOST_CHECK_EXCEPTION( bsp->verify_qc(qc), invalid_qc, eosio::testing::fc_exception_message_starts_with("strong quorum is not met") ); } { // weak QC quorum not met @@ -712,7 +712,7 @@ BOOST_AUTO_TEST_CASE(verify_qc_test_with_pending) try { qc_sig_t pending_qc_sig({}, weak_votes, pending_agg_sig); qc_t qc{bsp->block_num(), active_qc_sig, pending_qc_sig}; - BOOST_CHECK_EXCEPTION( bsp->verify_qc(qc), invalid_qc_claim, eosio::testing::fc_exception_message_starts_with("weak quorum is not met") ); + BOOST_CHECK_EXCEPTION( bsp->verify_qc(qc), invalid_qc, eosio::testing::fc_exception_message_starts_with("weak quorum is not met") ); } { // strong QC bitset size does not match number of finalizers in the policy @@ -734,7 +734,7 @@ BOOST_AUTO_TEST_CASE(verify_qc_test_with_pending) try { qc_sig_t pending_qc_sig(strong_votes, {}, pending_agg_sig); qc_t qc{bsp->block_num(), active_qc_sig, pending_qc_sig}; - BOOST_CHECK_EXCEPTION( bsp->verify_qc(qc), invalid_qc_claim, eosio::testing::fc_exception_message_starts_with("vote bitset size is not the same as the number of finalizers") ); + BOOST_CHECK_EXCEPTION( bsp->verify_qc(qc), invalid_qc, eosio::testing::fc_exception_message_starts_with("vote bitset size is not the same as the number of finalizers") ); } { // weak QC bitset size does not match number of finalizers in the policy @@ -756,7 +756,7 @@ BOOST_AUTO_TEST_CASE(verify_qc_test_with_pending) try { qc_sig_t pending_qc_sig({}, weak_votes, pending_agg_sig); qc_t qc{bsp->block_num(), active_qc_sig, pending_qc_sig}; - BOOST_CHECK_EXCEPTION( bsp->verify_qc(qc), invalid_qc_claim, eosio::testing::fc_exception_message_starts_with("vote bitset size is not the same as the number of finalizers") ); + BOOST_CHECK_EXCEPTION( bsp->verify_qc(qc), invalid_qc, eosio::testing::fc_exception_message_starts_with("vote bitset size is not the same as the number of finalizers") ); } { // strong QC with a wrong signing private key @@ -776,7 +776,7 @@ BOOST_AUTO_TEST_CASE(verify_qc_test_with_pending) try { qc_sig_t pending_qc_sig(strong_votes, {}, pending_agg_sig); qc_t qc{bsp->block_num(), active_qc_sig, pending_qc_sig}; - BOOST_CHECK_EXCEPTION( bsp->verify_qc(qc), invalid_qc_claim, eosio::testing::fc_exception_message_is("qc signature validation failed") ); + BOOST_CHECK_EXCEPTION( bsp->verify_qc(qc), invalid_qc_signature, eosio::testing::fc_exception_message_is("qc signature validation failed") ); } { // strong QC with a wrong digest @@ -796,7 +796,7 @@ BOOST_AUTO_TEST_CASE(verify_qc_test_with_pending) try { qc_sig_t pending_qc_sig(strong_votes, {}, pending_agg_sig); qc_t qc{bsp->block_num(), active_qc_sig, pending_qc_sig}; - BOOST_CHECK_EXCEPTION( bsp->verify_qc(qc), invalid_qc_claim, eosio::testing::fc_exception_message_is("qc signature validation failed") ); + BOOST_CHECK_EXCEPTION( bsp->verify_qc(qc), invalid_qc_signature, eosio::testing::fc_exception_message_is("qc signature validation failed") ); } { // weak QC with a wrong signing private key @@ -816,7 +816,7 @@ BOOST_AUTO_TEST_CASE(verify_qc_test_with_pending) try { qc_sig_t active_qc_sig(strong_votes, weak_votes, active_agg_sig); qc_sig_t pending_qc_sig(strong_votes, weak_votes, pending_agg_sig); qc_t qc{bsp->block_num(), active_qc_sig, pending_qc_sig}; - BOOST_CHECK_EXCEPTION( bsp->verify_qc(qc), invalid_qc_claim, eosio::testing::fc_exception_message_is("qc signature validation failed") ); + BOOST_CHECK_EXCEPTION( bsp->verify_qc(qc), invalid_qc_signature, eosio::testing::fc_exception_message_is("qc signature validation failed") ); } { // weak QC with a wrong digest @@ -836,7 +836,7 @@ BOOST_AUTO_TEST_CASE(verify_qc_test_with_pending) try { qc_sig_t active_qc_sig(strong_votes, weak_votes, active_agg_sig); qc_sig_t pending_qc_sig(strong_votes, weak_votes, active_agg_sig); qc_t qc{bsp->block_num(), active_qc_sig, pending_qc_sig}; - BOOST_CHECK_EXCEPTION( bsp->verify_qc(qc), invalid_qc_claim, eosio::testing::fc_exception_message_is("qc signature validation failed") ); + BOOST_CHECK_EXCEPTION( bsp->verify_qc(qc), invalid_qc_signature, eosio::testing::fc_exception_message_is("qc signature validation failed") ); } } FC_LOG_AND_RETHROW(); @@ -952,7 +952,7 @@ BOOST_AUTO_TEST_CASE(verify_qc_dual_finalizers) try { if (expected_same) { BOOST_CHECK_NO_THROW( bsp->verify_qc(qc) ); } else { - BOOST_CHECK_EXCEPTION( bsp->verify_qc(qc), invalid_qc_claim, eosio::testing::fc_exception_message_contains("does not vote the same on active and pending policies") ); + BOOST_CHECK_EXCEPTION( bsp->verify_qc(qc), invalid_qc, eosio::testing::fc_exception_message_contains("does not vote the same on active and pending policies") ); } }; From d6db8799409e6cd619b08fae06a71ed7ba88aa29 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Fri, 11 Oct 2024 16:22:36 -0400 Subject: [PATCH 4/7] Forgot to check optional. --- libraries/chain/controller.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 25fd002631..2c122d302d 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -4278,7 +4278,8 @@ struct controller_impl { if constexpr (std::is_same_v, block_state_ptr>) { // do basic checks always (excluding signature verification) - head->verify_qc_basic(*qc); + if (qc) + head->verify_qc_basic(*qc); if (conf.force_all_checks && qc) { // verify signatures only if `conf.force_all_checks` From 557cfb07ac62d5cc3770bd733c206f73866818f5 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Fri, 11 Oct 2024 16:24:56 -0400 Subject: [PATCH 5/7] Simplify code. --- libraries/chain/controller.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 2c122d302d..2bcce4122a 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -4278,12 +4278,13 @@ struct controller_impl { if constexpr (std::is_same_v, block_state_ptr>) { // do basic checks always (excluding signature verification) - if (qc) + if (qc) { head->verify_qc_basic(*qc); - if (conf.force_all_checks && qc) { - // verify signatures only if `conf.force_all_checks` - head->verify_qc_signatures(*qc); + if (conf.force_all_checks) { + // verify signatures only if `conf.force_all_checks` + head->verify_qc_signatures(*qc); + } } } From d134711fe9ba076f5a14084e64c3be2e1e5318fd Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Fri, 11 Oct 2024 22:49:40 -0400 Subject: [PATCH 6/7] Change the exception #. --- libraries/chain/include/eosio/chain/exceptions.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/chain/include/eosio/chain/exceptions.hpp b/libraries/chain/include/eosio/chain/exceptions.hpp index fbf9311128..1539b7c3be 100644 --- a/libraries/chain/include/eosio/chain/exceptions.hpp +++ b/libraries/chain/include/eosio/chain/exceptions.hpp @@ -258,9 +258,9 @@ namespace eosio { namespace chain { FC_DECLARE_DERIVED_EXCEPTION( invalid_qc_claim, block_validate_exception, 3030014, "Block includes an invalid QC claim" ) FC_DECLARE_DERIVED_EXCEPTION( invalid_qc, block_validate_exception, - 3030014, "Block includes an invalid QC" ) + 3030015, "Block includes an invalid QC" ) FC_DECLARE_DERIVED_EXCEPTION( invalid_qc_signature, block_validate_exception, - 3030014, "Block includes a QC with invalid signature(s)" ) + 3030016, "Block includes a QC with invalid signature(s)" ) FC_DECLARE_DERIVED_EXCEPTION( transaction_exception, chain_exception, 3040000, "Transaction exception" ) From d4b6861248443223d09a273ace17fe5db9d633f3 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Fri, 11 Oct 2024 22:54:01 -0400 Subject: [PATCH 7/7] Do both signature verification and basic checks in the async task --- libraries/chain/controller.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 2bcce4122a..fd77004684 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -4124,11 +4124,9 @@ struct controller_impl { log_and_drop_future verify_qc_future; if constexpr (is_proper_savanna_block) { if (qc) { - // do basic checks on provided qc immediately (excluding signature verification) - prev.verify_qc_basic(*qc); - verify_qc_future = post_async_task(thread_pool.get_executor(), [&qc, &prev] { - prev.verify_qc_signatures(*qc); + // do both signature verification and basic checks in the async task + prev.verify_qc(*qc); }); } }