Skip to content

Commit

Permalink
GH-2125 Vote before applying/validating block if possible
Browse files Browse the repository at this point in the history
  • Loading branch information
@heifner
heifner committed Mar 2, 2024
1 parent 71c5479 commit 25309df
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 43 deletions.
92 changes: 55 additions & 37 deletions libraries/chain/controller.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3172,43 +3172,6 @@ struct controller_impl {
});
}

// expected to be called from application thread as it modifies bsp->valid_qc and if_irreversible_block_id
void integrate_received_qc_to_block(const block_state_ptr& bsp_in) {
// extract QC from block extension
const auto& block_exts = bsp_in->block->validate_and_extract_extensions();
auto qc_ext_id = quorum_certificate_extension::extension_id();

if( block_exts.count(qc_ext_id) == 0 ) {
return;
}
const auto& qc_ext = std::get<quorum_certificate_extension>(block_exts.lower_bound(qc_ext_id)->second);
const auto& received_qc = qc_ext.qc.qc;

const auto claimed = fetch_bsp_on_branch_by_num( bsp_in->previous(), qc_ext.qc.block_num );
if( !claimed ) {
return;
}

// Don't save the QC from block extension if the claimed block has a better valid_qc.
if (claimed->valid_qc && (claimed->valid_qc->is_strong() || received_qc.is_weak())) {
return;
}

// Save the QC. This is safe as the function is called by push_block from application thread.
claimed->valid_qc = received_qc;

// advance LIB if QC is strong
if( received_qc.is_strong() ) {
// We evaluate a block extension qc and advance lib if strong.
// This is done before evaluating the block. It is possible the block
// will not be valid or forked out. This is safe because the block is
// just acting as a carrier of this info. It doesn't matter if the block
// is actually valid as it simply is used as a network message for this data.
auto& final_on_strong_qc_block_ref = claimed->core.get_block_reference(claimed->core.final_on_strong_qc_block_num);
set_if_irreversible_block_id(final_on_strong_qc_block_ref.block_id);
}
}

// Verify QC claim made by instant_finality_extension in header extension
// and quorum_certificate_extension in block extension are valid.
// Called from net-threads. It is thread safe as signed_block is never modified
Expand Down Expand Up @@ -3417,13 +3380,67 @@ struct controller_impl {
return fork_db.apply<std::optional<block_handle>>(f);
}

// expected to be called from application thread as it modifies bsp->valid_qc and if_irreversible_block_id
void integrate_received_qc_to_block(const block_state_ptr& bsp_in) {
// extract QC from block extension
const auto& block_exts = bsp_in->block->validate_and_extract_extensions();
auto qc_ext_id = quorum_certificate_extension::extension_id();

if( block_exts.count(qc_ext_id) == 0 ) {
return;
}
const auto& qc_ext = std::get<quorum_certificate_extension>(block_exts.lower_bound(qc_ext_id)->second);
const auto& received_qc = qc_ext.qc.qc;

const auto claimed = fetch_bsp_on_branch_by_num( bsp_in->previous(), qc_ext.qc.block_num );
if( !claimed ) {
return;
}

// Don't save the QC from block extension if the claimed block has a better valid_qc.
if (claimed->valid_qc && (claimed->valid_qc->is_strong() || received_qc.is_weak())) {
return;
}

// Save the QC. This is safe as the function is called by push_block & accept_block from application thread.
claimed->valid_qc = received_qc;

// advance LIB if QC is strong
if( received_qc.is_strong() ) {
// We evaluate a block extension qc and advance lib if strong.
// This is done before evaluating the block. It is possible the block
// will not be valid or forked out. This is safe because the block is
// just acting as a carrier of this info. It doesn't matter if the block
// is actually valid as it simply is used as a network message for this data.
const auto& final_on_strong_qc_block_ref = claimed->core.get_block_reference(claimed->core.final_on_strong_qc_block_num);
set_if_irreversible_block_id(final_on_strong_qc_block_ref.block_id);
}
}

void consider_voting(const block_state_ptr& bsp) {
// 1. Get the `core.final_on_strong_qc_block_num` for the block you are considering to vote on and use that to find the actual block ID
// of the ancestor block that has that block number.
// 2. If that block ID is not an ancestor of the current head block, then do not vote for that block.
// 3. Otherwise, consider voting for that block according to the decide_vote rules.

if (bsp->core.final_on_strong_qc_block_num > 0) {
const auto& final_on_strong_qc_block_ref = bsp->core.get_block_reference(bsp->core.final_on_strong_qc_block_num);
auto final = fetch_bsp_on_head_branch_by_num(final_on_strong_qc_block_ref.block_num());
if (final) {
assert(final->is_valid()); // if found on head branch then it must be validated
create_and_send_vote_msg(bsp);
}
}
}

template <class BSP>
void accept_block(const BSP& bsp) {
assert(bsp && bsp->block);

// Save the received QC as soon as possible, no matter whether the block itself is valid or not
if constexpr (std::is_same_v<BSP, block_state_ptr>) {
integrate_received_qc_to_block(bsp);
consider_voting(bsp);
}

auto do_accept_block = [&](auto& forkdb) {
Expand All @@ -3447,6 +3464,7 @@ struct controller_impl {
// Save the received QC as soon as possible, no matter whether the block itself is valid or not
if constexpr (std::is_same_v<BSP, block_state_ptr>) {
integrate_received_qc_to_block(bsp);
consider_voting(bsp);
}

controller::block_status s = controller::block_status::complete;
Expand Down
11 changes: 5 additions & 6 deletions libraries/chain/hotstuff/finalizer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
namespace eosio::chain {

// ----------------------------------------------------------------------------------------
finalizer::vote_result finalizer::decide_vote(const finality_core& core, const block_id_type &proposal_id,
finalizer::vote_result finalizer::decide_vote(const finality_core& core, const block_id_type& proposal_id,
const block_timestamp_type proposal_timestamp) {
vote_result res;

Expand All @@ -14,7 +14,7 @@ finalizer::vote_result finalizer::decide_vote(const finality_core& core, const b
// just activated and we can proceed

if (!res.monotony_check) {
dlog("monotony check failed for proposal ${p}, cannot vote", ("p", proposal_id));
dlog("monotony check failed for proposal ${bn} ${p}, cannot vote", ("bn", block_header::num_from_id(proposal_id))("p", proposal_id));
return res;
}

Expand All @@ -38,8 +38,6 @@ finalizer::vote_result finalizer::decide_vote(const finality_core& core, const b
}

bool can_vote = res.liveness_check || res.safety_check;
dlog("liveness_check=${l}, safety_check=${s}, monotony_check=${m}, can vote=${can_vote}",
("l",res.liveness_check)("s",res.safety_check)("m",res.monotony_check)("can_vote",can_vote));

// Figure out if we can vote and wether our vote will be strong or weak
// If we vote, update `fsi.last_vote` and also `fsi.lock` if we have a newer commit qc
Expand All @@ -64,8 +62,9 @@ finalizer::vote_result finalizer::decide_vote(const finality_core& core, const b
res.decision = voting_strong ? vote_decision::strong_vote : vote_decision::weak_vote;
}

dlog("liveness_check=${l}, safety_check=${s}, monotony_check=${m}, can vote=${can_vote}, voting=${v}",
("l",res.liveness_check)("s",res.safety_check)("m",res.monotony_check)("can_vote",can_vote)("v", res.decision));
dlog("block=${bn}, liveness_check=${l}, safety_check=${s}, monotony_check=${m}, can vote=${can_vote}, voting=${v}",
("bn", block_header::num_from_id(proposal_id))("l",res.liveness_check)("s",res.safety_check)("m",res.monotony_check)
("can_vote",can_vote)("v", res.decision));
return res;
}

Expand Down

0 comments on commit 25309df

Please sign in to comment.