IF: Add hs_irreversible_blocknum to block_header_state #1612
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Closing. Going to explore not storing in block_header_state. |
linh2931
reviewed
Sep 9, 2023
| @@ -88,6 +88,9 @@ namespace eosio { namespace chain { | |||
|
|
|||
| void mark_valid( const block_state_ptr& h ); | |||
|
|
|||
| // Called by hotstuff to mark a fork branch irreversible | |||
| void mark_irreversible( const block_id_type& id ); | |||
There was a problem hiding this comment.
Should this be named as `hs_mark_irreversible"?
| "block state ${id} not in fork database; cannot mark as irreversible", ("id", id)); | ||
| const bool at_root = (id == root->id); | ||
| by_id_idx.modify(itr, [&id, lib](block_state_ptr& bsp) { | ||
| bsp->hs_irreversible_blocknum.store(lib); |
There was a problem hiding this comment.
This method only updates s_irreversible_blocknum. Should it be named as hs_mark_irreversible_impl?
| @@ -329,6 +331,25 @@ namespace eosio { namespace chain { | |||
| root = new_root; | |||
| } | |||
|
|
|||
| void fork_database_impl::mark_irreversible_impl( block_id_type id ) { | |||
There was a problem hiding this comment.
You can pass the parameter as const block_id_type&
| const bool at_root = (id == root->id); | ||
| by_id_idx.modify(itr, [&id, lib](block_state_ptr& bsp) { | ||
| bsp->hs_irreversible_blocknum.store(lib); | ||
| id = bsp->header.previous; |
There was a problem hiding this comment.
Don't reuse (reset) input parameter.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
hs_irreversible_blocknumtoblock_header_stateto track hotstuff irreversible block number.fork_databaseto use eitherhs_irreversible_blocknumordpos_irreversible_blocknumfor LIB.mark_irreversibletofork_databaseto be used bychain_pacemakerto advance LIB which will then be used bycontrollerlog_irreversibleto advance LIB.copyable_atomictype for use byblock_header_statehs_irreversible_blocknumsince it will be modified after theblock_stateis signaled and potentially accessed from other threads.fc::atomic_shared_ptrandcopyable_atomic_shared_ptrthese may not actually be needed. But could be used if we wish to store activefinalizer_setinblock_state.Need to add tests for the
fork_database::mark_irreversiblebut getting this PR up so it can be used by #1586