-
Notifications
You must be signed in to change notification settings - Fork 72
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
Changes to support new block_header_state
and block_state
#1975
Conversation
block_header_state
and block_state
block_header_state
and block_state
131/150 Test #98: producer_schedule_hs_unit_test_eos-vm-jit ..........***Failed 1.36 sec 132/150 Test #97: producer_schedule_hs_unit_test_eos-vm ..............***Failed 1.57 sec 133/150 Test #96: producer_schedule_hs_unit_test_eos-vm-oc ...........***Failed 2.86 sec 144/150 Test #21: api_unit_test_eos-vm-oc ............................***Failed 79.26 sec 146/150 Test #23: api_unit_test_eos-vm-jit ...........................***Failed 96.61 sec 150/150 Test #22: api_unit_test_eos-vm ...............................***Failed 467.14 sec
…o gh_1941 Also removed tabs and aligned struct members
Note:start |
return bsp->valid_block_signing_authority; | ||
}, | ||
[](const block_state_ptr& bsp) -> const block_signing_authority& { | ||
static block_signing_authority bsa; return bsa; //return bsp->header.producer; [greg todo] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a warning here so that the remaining work is easy to be found.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added the [greg todo]
comments instead of warnings, which I think obfuscate the compilation output.
@@ -0,0 +1,27 @@ | |||
The following diagram describes Leap block production, as implemented in `libraries/chain/controller.cpp`: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file probably should be located inside /docs/01_nodeos. But you can move it later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I followed @ericpassmore 's suggestion for the file path, but happy to move it if desirable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no changes for now.
direction TB | ||
start -- "stage = Ø" --> sb | ||
sb("start_block()"):::fun -- "stage = building_block" --> et | ||
et["execute transactions" ] -- "stage = building_block" --> fb("finalize_block()"):::fun |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does stage
still stay in building_block
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not understanding exactly what you are asking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After executing transactions, is the stage still
building_block
? ("stage = building_block" in the chart)
yes, it is.
libraries/chain/controller.cpp
Outdated
// if the _unsigned_block pre-dates block-signing authorities this may be present. | ||
std::optional<producer_authority_schedule> _new_producer_authority_cache; | ||
}; | ||
// if the _unsigned_block pre-dates block-signing authorities this may be present. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove _
in _unsigned_block
block_id_type id; | ||
pending_block_header_state_legacy pending_block_header_state; | ||
deque<transaction_metadata_ptr> trx_metas; | ||
signed_block_ptr unsigned_block; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I always get confused with unsigned_block
having signed_block_ptr
as its type. But this is from existing code. No changes needed.
|
||
using block_stage_type = std::variant<building_block, assembled_block, completed_block>; | ||
// -------------------------------------------------------------------------------- | ||
struct assembled_block_if { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know how to handle this. As if
is such common reserved word in the code, it can cause confusion when reading a variable containing if
. It is ugly to have a capitalized IF
. I start to move away from if
and spell out if possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can change it to just assembled_block
maybe?
const pending_block_header_state_legacy& get_pending_block_header_state_legacy()const { | ||
if( std::holds_alternative<building_block>(_block_stage) ) | ||
return std::get<building_block>(_block_stage)._pending_block_header_state_legacy; | ||
template <class R, class F> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does R imply for? (F is for function)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
R
is the result_type
, what F
returns.
|
||
}; | ||
|
||
std::variant<building_block_dpos, building_block_if> v; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is v
too short? Not obvious when reading functions 100 line away from it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so, it is used only locally within the class building_block
.
No description provided.