Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IF: Update fork database fork-choice rule #2273

Merged
merged 45 commits into from
Mar 1, 2024

Conversation

heifner
Copy link
Member

@heifner heifner commented Feb 29, 2024

After this PR, we no longer update LIB via votes. LIB is only updated according to qc claims in block extensions.

Update fork_database fork-choice rule.
Computed fields for index (in order):

  1. core.last_final_block_num
  2. core.latest_qc_claim_block_num
  3. block timestamp
  4. block ID (as final tie breaker to ensure uniqueness)

Potentially switches forks before starting to produce a block. Does not interrupt producing blocks once block production has started.

Includes some misc performance improvements to fork_database.

Currently does not include voting on non-validated blocks. Would like to add the voting change in #2159 or as a separate PR.

Resolves #2125

… instead. Move current_core to block_state.
…ate, use values for fork_database by_best_branch rule.
@heifner heifner added the OCI Work exclusive to OCI team label Feb 29, 2024
@heifner heifner linked an issue Feb 29, 2024 that may be closed by this pull request
libraries/chain/controller.cpp Show resolved Hide resolved
libraries/chain/controller.cpp Show resolved Hide resolved
plugins/producer_plugin/producer_plugin.cpp Show resolved Hide resolved
libraries/chain/fork_database.cpp Outdated Show resolved Hide resolved
libraries/chain/fork_database.cpp Outdated Show resolved Hide resolved
unittests/fork_db_tests.cpp Show resolved Hide resolved
unittests/fork_db_tests.cpp Show resolved Hide resolved
@ericpassmore
Copy link
Contributor

Nice to see the improvements. fork_db_test.cpp looks very nice.

libraries/chain/block_state.cpp Show resolved Hide resolved
libraries/chain/controller.cpp Outdated Show resolved Hide resolved
uint16_t if_ext_id = instant_finality_extension::extension_id();
assert(bsp->header_exts.count(if_ext_id) > 0); // in all instant_finality block headers
const auto& if_ext = std::get<instant_finality_extension>(bsp->header_exts.lower_bound(if_ext_id)->second);
if (if_ext.qc_claim.is_strong_qc) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment here saying the claim has already been verified before.

libraries/chain/controller.cpp Show resolved Hide resolved
libraries/chain/fork_database.cpp Show resolved Hide resolved
libraries/chain/fork_database.cpp Show resolved Hide resolved
Comment on lines 546 to 553
return my->search_on_head_branch_impl(block_num);
}

template<class BSP>
BSP fork_database_impl<BSP>::search_on_head_branch_impl( uint32_t block_num ) const {
for (auto i = index.find(head->id()); i != index.end(); i = index.find((*i)->previous())) {
if ((*i)->block_num() == block_num)
return *i;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return my->search_on_head_branch_impl(block_num);
}
template<class BSP>
BSP fork_database_impl<BSP>::search_on_head_branch_impl( uint32_t block_num ) const {
for (auto i = index.find(head->id()); i != index.end(); i = index.find((*i)->previous())) {
if ((*i)->block_num() == block_num)
return *i;
return my->search_on_branch_impl(head->id(), block_num);

@ericpassmore
Copy link
Contributor

Note:start
group: IF
category: INTERNALS
summary: LIB is only updated according to qc claims in block extensions. Included updated fork choice rules.
Note:end

@heifner heifner merged commit 5a029ee into hotstuff_integration Mar 1, 2024
26 checks passed
@heifner heifner deleted the GH-2125-forkdb-simpler branch March 1, 2024 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCI Work exclusive to OCI team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IF: Update fork-choice rule for fork database
4 participants