-
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
IF: Implement computing finality digest #2282
Conversation
…s of header.action_mroot about its different roles in Legacy and Savanna
…o finality_mroots in valid structure
… ID does not match
Note:start |
…shot IF: Include `block_state::valid` in the snapshot.
…roller to block_state
finalizer_policy_ptr active_finalizer_policy; | ||
proposer_policy_ptr active_proposer_policy; | ||
flat_map<block_timestamp_type, proposer_policy_ptr> proposer_policies; | ||
flat_map<uint32_t, finalizer_policy_ptr> finalizer_policies; | ||
|
||
// from block_state | ||
using valid_t = uint32_t; // snapshot todo | ||
std::optional<valid_t> valid; | ||
std::optional<valid_t> valid; |
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.
There is no need for this to be optional within a snapshot.
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.
Yes, I remember you mentioned that it should always be present, and the std::optional was just because the valid_t
is not be available when block_state
is initially created.
I assume that when the snapshot is initiated, and we save the chain head to the snapshot, we can FC_ASSERT
that the valid
member is populated, right?
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.
Yes, we can assert that the valid
structure exists at the time the snapshot is being produced. It must always be the case.
In the snapshot itself, the valid
structure must always be present, which is why I say it shouldn't be an optional type within this struct. It should still be optional within block_state
. But when loading the root block_state
from a snapshot, valid
should always end up populated.
… an equal relation
…r_schedule.version; restore code in get_next_proposer_schedule_version and proposer_policy_progression_test removed wrongly
…ch argument can be ensured
… the same time as finality extensioni during block header validation; small changes to incor[orate review comments in block_header_state
@@ -59,8 +117,6 @@ block_header_state block_header_state::next(block_header_state_input& input) con | |||
// +1 since this is called after the block is built, this will be the active schedule for the next block | |||
if (it->first.slot <= input.timestamp.slot + 1) { | |||
result.active_proposer_policy = it->second; | |||
result.header.schedule_version = header.schedule_version + 1; | |||
result.active_proposer_policy->proposer_schedule.version = result.header.schedule_version; |
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 believe this change means it is now possible to skip propser_schedule.version
numbers. We should modify the set_proposed_producers
logic so we don't skip version numbers. Need a test to cover that case so we don't break it again.
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
leap/libraries/chain/controller.cpp
Lines 481 to 484 in 7f3d5e6
return (--parent.proposer_policies.end())->second->proposer_schedule.version + 1; | |
} | |
assert(active_proposer_policy); | |
return active_proposer_policy->proposer_schedule.version + 1; |
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, that doesn't help. We need to make sure that if a proposer policy is scheduled for the same proposer_policy.active_time
that it replaces the current one there but doesn't increment the schedule version. I think we will need to change controller::set_proposed_producers
to check if there is an existing one that would have the same active_time
and replace it and use its version. We should return the correct version to the contract.
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.
Feel free to create a follow-on issue for this.
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.
Thanks.
…cy digest and base_digest explicitly
…erkle_t to incremental_legacy_merkle_tree
A finality digest is a hash of a series sub-digests required for IBC verification. It is built from a hash of a serialization of
light_header_protocol_version
,active_finalizer_policy_generation
,finality_tree_digest
, and a combination ofbase_digest
andactive_finalizer_policy_digest
.base_digest
is a hash of a serialization of deterministic fields ofblock_header_state
, includingheader
,core
,finalizer_policies
,active_proposer_policy
,proposer_policies
, andactivated_protocol_features
.finality_tree
is defined as following:one for each block starting from the IF Genesis Block and ending at some
specified descendant block.
Tree over Finality Leaf Nodes starting with the one for the IF Genesis Block
and ending with the one for the block that is the parent of the the target Block.
block referenced by the target block's final_on_strong_qc_block_num.
That is, validation_tree(core.final_on_strong_qc_block_num))
A
valid
structure is introduced intoblock_state
to hold all necessary information.This PR implemented computing finality digest based on finality merkle tree, validation tree, finality tree, and validated structure.
See #2080 for design details.
Resolved #2080