Skip to content

Commit

Permalink
GH-376 Set better received qc if a better qc is received then current…
Browse files Browse the repository at this point in the history
…ly available.
  • Loading branch information
heifner committed Jul 23, 2024
1 parent 606c940 commit d03d750
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 21 deletions.
13 changes: 5 additions & 8 deletions libraries/chain/controller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3972,18 +3972,15 @@ struct controller_impl {
return;
}

// Don't save the QC from block extension if the claimed block has a better or same received_qc_sig.
// claimed->received_qc_sig_is_strong() acquires a mutex.
if (received_qc.is_weak() || claimed->valid_qc_is_strong()) {
dlog("qc not better, claimed->valid: ${qbn} ${qid}, strong=${s}, received: ${rqc}, for block ${bn} ${id}",
("qbn", claimed->block_num())("qid", claimed->id())("s", !received_qc.is_weak()) // use is_weak() to avoid mutex on received_qc_sig_is_strong()
// Don't save the QC from block extension if the claimed block has a better or same received_qc
if (!claimed->set_received_qc(received_qc)) {
dlog("qc not better, claimed->received: ${qbn} ${qid}, strong=${s}, received: ${rqc}, for block ${bn} ${id}",
("qbn", claimed->block_num())("qid", claimed->id())("s", !received_qc.is_weak()) // use is_weak() to avoid mutex on received_qc_is_strong()
("rqc", qc_ext.qc.to_qc_claim())("bn", bsp_in->block_num())("id", bsp_in->id()));
return;
}

// Save the QC.
dlog("setting valid qc: ${rqc} into claimed block ${bn} ${id}", ("rqc", qc_ext.qc.to_qc_claim())("bn", claimed->block_num())("id", claimed->id()));
claimed->set_valid_qc(received_qc);
dlog("set received qc: ${rqc} into claimed block ${bn} ${id}", ("rqc", qc_ext.qc.to_qc_claim())("bn", claimed->block_num())("id", claimed->id()));

if( received_qc.is_strong() ) {
// Update finalizer safety information based on vote evidence
Expand Down
26 changes: 21 additions & 5 deletions libraries/chain/finality/qc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ inline std::vector<uint32_t> bitset_to_vector(const vote_bitset& bs) {
return r;
}

size_t qc_sig_t::vote_count() const {
return (strong_votes ? strong_votes->count() : 0u) +
(weak_votes ? weak_votes->count() : 0u);
}

void qc_sig_t::verify(const finalizer_policy_ptr& fin_policy,
const digest_type& strong_digest,
const weak_digest_t& weak_digest) const {
Expand Down Expand Up @@ -277,9 +282,18 @@ std::optional<qc_sig_t> open_qc_sig_t::get_best_qc() const {
return std::optional{qc_sig_t{ std::move(qc_sig_from_open) }};
}

void open_qc_sig_t::set_received_qc_sig(const qc_sig_t& qc) {
bool open_qc_sig_t::set_received_qc_sig(const qc_sig_t& qc) {
std::lock_guard g(*_mtx);
received_qc_sig = qc;
const bool qc_is_better =
!received_qc_sig
|| (received_qc_sig->is_weak() && qc.is_strong())
|| received_qc_sig->vote_count() < qc.vote_count();

if (qc_is_better) {
received_qc_sig = qc;
return true;
}
return false;
}

bool open_qc_sig_t::received_qc_sig_is_strong() const {
Expand Down Expand Up @@ -322,12 +336,14 @@ void open_qc_t::verify_qc(const qc_t& qc, const digest_type& strong_digest, cons
}
}

void open_qc_t::set_received_qc(const qc_t& qc) {
active_policy_sig.set_received_qc_sig(qc.active_policy_sig);
bool open_qc_t::set_received_qc(const qc_t& qc) {
bool active_better = active_policy_sig.set_received_qc_sig(qc.active_policy_sig);
bool pending_better = false;
if (qc.pending_policy_sig) {
assert(pending_policy_sig);
pending_policy_sig->set_received_qc_sig(*qc.pending_policy_sig);
pending_better = pending_policy_sig->set_received_qc_sig(*qc.pending_policy_sig);
}
return active_better || pending_better;
}

bool open_qc_t::received_qc_is_strong() const {
Expand Down
5 changes: 3 additions & 2 deletions libraries/chain/include/eosio/chain/block_state.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,9 @@ struct block_state : public block_header_state { // block_header_state provi
uint32_t final_on_strong_qc_block_num() const { return core.final_on_strong_qc_block_num; }

std::optional<qc_t> get_best_qc() const { return open_qc.get_best_qc(block_num()); } // thread safe
bool valid_qc_is_strong() const { return open_qc.received_qc_is_strong(); } // thread safe
void set_valid_qc(const qc_t& qc) { open_qc.set_received_qc(qc); }
bool received_qc_is_strong() const { return open_qc.received_qc_is_strong(); } // thread safe
// return true if better qc, thread safe
bool set_received_qc(const qc_t& qc) { return open_qc.set_received_qc(qc); }
// extract the qc_claim from block header finality_extension
qc_claim_t extract_qc_claim() const;

Expand Down
12 changes: 6 additions & 6 deletions libraries/chain/include/eosio/chain/finality/qc.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ namespace eosio::chain {
struct qc_sig_t {
bool is_weak() const { return !!weak_votes; }
bool is_strong() const { return !weak_votes; }
size_t vote_count() const;

std::optional<vote_bitset> strong_votes;
std::optional<vote_bitset> weak_votes;
Expand All @@ -72,10 +73,7 @@ namespace eosio::chain {
bool is_strong() const {
return active_policy_sig.is_strong() && (!pending_policy_sig || pending_policy_sig->is_strong());
}

bool is_weak() const {
return active_policy_sig.is_weak() || (pending_policy_sig && pending_policy_sig->is_weak());
}
bool is_weak() const { return !is_strong(); }

qc_claim_t to_qc_claim() const {
return { .block_num = block_num, .is_strong_qc = is_strong() };
Expand Down Expand Up @@ -170,7 +168,8 @@ namespace eosio::chain {
state_t state() const { std::lock_guard g(*_mtx); return pending_state; };

std::optional<qc_sig_t> get_best_qc() const;
void set_received_qc_sig(const qc_sig_t& qc);
// return true if better qc
bool set_received_qc_sig(const qc_sig_t& qc);
bool received_qc_sig_is_strong() const;
private:
friend struct fc::reflector<open_qc_sig_t>;
Expand Down Expand Up @@ -229,7 +228,8 @@ namespace eosio::chain {
qc_vote_metrics_t vote_metrics(const qc_t& qc) const;
// return qc missing vote's finalizers
std::set<finalizer_authority_ptr> missing_votes(const qc_t& qc) const;
void set_received_qc(const qc_t& qc);
// return true if better qc
bool set_received_qc(const qc_t& qc);
bool received_qc_is_strong() const;
vote_status aggregate_vote(uint32_t connection_id, const vote_message& vote,
block_num_type block_num, std::span<const uint8_t> finalizer_digest);
Expand Down
5 changes: 5 additions & 0 deletions libraries/libfc/include/fc/bitutil.hpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
#pragma once

#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wshift-count-overflow"
// boost dynamic_bitset count() generates warning
#include <boost/dynamic_bitset.hpp>
#pragma GCC diagnostic pop

#include <stdint.h>

namespace fc {
Expand Down

0 comments on commit d03d750

Please sign in to comment.