From 6dfb4529f0507bad3dd95d8a86749f79cf5b1661 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 11 Sep 2024 13:40:57 -0500 Subject: [PATCH 1/3] Log received block with qc_claim even if no qc --- libraries/chain/controller.cpp | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index bf2dfbfad5..ae34348c73 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -3887,7 +3887,7 @@ 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 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(); @@ -3931,6 +3931,15 @@ struct controller_impl { "Block #${b} claims a block_num (${n1}) less than the previous block's (${n2})", ("n1", new_qc_claim.block_num)("n2", prev_qc_claim.block_num)("b", block_num) ); + if (!replaying && fc::logger::get(DEFAULT_LOGGER).is_enabled(fc::log_level::debug)) { + fc::time_point now = fc::time_point::now(); + if (now - b->timestamp < fc::minutes(5) || (b->block_num() % 1000 == 0)) { + dlog("received block: #${bn} ${t} ${prod} ${id}, qc claim: ${qc_claim}, qc ${qc}, previous: ${p}", + ("bn", b->block_num())("t", b->timestamp)("prod", b->producer)("id", id) + ("qc_claim", new_qc_claim)("qc", qc_extension_present ? "present" : "not present")("p", b->previous)); + } + } + if( new_qc_claim.block_num == prev_qc_claim.block_num ) { if( new_qc_claim.is_strong_qc == prev_qc_claim.is_strong_qc ) { // QC block extension is redundant @@ -3949,7 +3958,7 @@ struct controller_impl { ("s1", new_qc_claim.is_strong_qc)("s2", prev_qc_claim.is_strong_qc)("b", block_num) ); } - // At this point, we are making a new claim in this block, so it must includes a QC to justify this claim. + // At this point, we are making a new claim in this block, so it must include a QC to justify this claim. EOS_ASSERT( qc_extension_present, block_validate_exception, "Block #${b} is making a new finality claim, but doesn't include a qc to justify this claim", ("b", block_num) ); @@ -4051,7 +4060,7 @@ struct controller_impl { // verify basic_block invariants template - std::optional verify_basic_block_invariants(const signed_block_ptr& b, const BS& prev) { + std::optional verify_basic_block_invariants(const block_id_type& id, const signed_block_ptr& b, const BS& prev) { constexpr bool is_proper_savanna_block = std::is_same_v, block_state>; assert(is_proper_savanna_block == b->is_proper_svnn_block()); @@ -4059,7 +4068,7 @@ struct controller_impl { EOS_ASSERT( b->is_proper_svnn_block(), block_validate_exception, "create_block_state_i cannot be called on block #${b} which is not a Proper Savanna block unless the prev block state provided is of type block_state", ("b", b->block_num()) ); - return verify_basic_proper_block_invariants(b, prev); + return verify_basic_proper_block_invariants(id, b, prev); } else { EOS_ASSERT( !b->is_proper_svnn_block(), block_validate_exception, "create_block_state_i cannot be called on block #${b} which is a Proper Savanna block unless the prev block state provided is of type block_state_legacy", @@ -4092,15 +4101,11 @@ struct controller_impl { constexpr bool is_proper_savanna_block = std::is_same_v, block_state>; assert(is_proper_savanna_block == b->is_proper_svnn_block()); - std::optional qc = verify_basic_block_invariants(b, prev); + std::optional qc = verify_basic_block_invariants(id, b, prev); if constexpr (is_proper_savanna_block) { if (qc) { verify_qc(b, prev, *qc); - - dlog("received block: #${bn} ${t} ${prod} ${id}, qc claim: ${qc_claim}, previous: ${p}", - ("bn", b->block_num())("t", b->timestamp)("prod", b->producer)("id", id) - ("qc_claim", qc->to_qc_claim())("p", b->previous)); } } @@ -4337,7 +4342,7 @@ struct controller_impl { auto do_push = [&](const auto& head) { if constexpr (std::is_same_v>) { - std::optional qc = verify_basic_block_invariants(b, *head); + std::optional qc = verify_basic_block_invariants({}, b, *head); if constexpr (std::is_same_v, block_state_ptr>) { if (conf.force_all_checks && qc) { From b878a05efadae9724662da8e1ca8389be139e24b Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 11 Sep 2024 16:22:54 -0500 Subject: [PATCH 2/3] Log as soon as we can --- libraries/chain/controller.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index ae34348c73..32fc901570 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -3912,6 +3912,15 @@ struct controller_impl { const auto& f_ext = std::get(*finality_ext); const auto new_qc_claim = f_ext.qc_claim; + if (!replaying && fc::logger::get(DEFAULT_LOGGER).is_enabled(fc::log_level::debug)) { + fc::time_point now = fc::time_point::now(); + if (now - b->timestamp < fc::minutes(5) || (b->block_num() % 1000 == 0)) { + dlog("received block: #${bn} ${t} ${prod} ${id}, qc claim: ${qc_claim}, qc ${qc}, previous: ${p}", + ("bn", b->block_num())("t", b->timestamp)("prod", b->producer)("id", id) + ("qc_claim", new_qc_claim)("qc", qc_extension_present ? "present" : "not present")("p", b->previous)); + } + } + // The only time a block should have a finality block header extension but // its parent block does not, is if it is a Savanna Genesis block (which is // necessarily a Transition block). Since verify_proper_block_exts will not be called @@ -3931,15 +3940,6 @@ struct controller_impl { "Block #${b} claims a block_num (${n1}) less than the previous block's (${n2})", ("n1", new_qc_claim.block_num)("n2", prev_qc_claim.block_num)("b", block_num) ); - if (!replaying && fc::logger::get(DEFAULT_LOGGER).is_enabled(fc::log_level::debug)) { - fc::time_point now = fc::time_point::now(); - if (now - b->timestamp < fc::minutes(5) || (b->block_num() % 1000 == 0)) { - dlog("received block: #${bn} ${t} ${prod} ${id}, qc claim: ${qc_claim}, qc ${qc}, previous: ${p}", - ("bn", b->block_num())("t", b->timestamp)("prod", b->producer)("id", id) - ("qc_claim", new_qc_claim)("qc", qc_extension_present ? "present" : "not present")("p", b->previous)); - } - } - if( new_qc_claim.block_num == prev_qc_claim.block_num ) { if( new_qc_claim.is_strong_qc == prev_qc_claim.is_strong_qc ) { // QC block extension is redundant From 3a1c0ad5f34537db50d5a4086c3ab31d9c3b4580 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 11 Sep 2024 16:37:53 -0500 Subject: [PATCH 3/3] Fix merge issue --- libraries/chain/controller.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 59356140a3..5bac0e8ee9 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -4104,7 +4104,7 @@ struct controller_impl { if constexpr (is_proper_savanna_block) { if (qc) { - verify_qc(b, prev, *qc); + verify_qc(prev, *qc); } }