-
Notifications
You must be signed in to change notification settings - Fork 34
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
Refactor: slot leadership lottery #1867
Conversation
Signed-off-by: Dmitriy Khaustov aka xDimon <khaustov.dm@gmail.com>
Signed-off-by: Dmitriy Khaustov aka xDimon <khaustov.dm@gmail.com>
Signed-off-by: Dmitriy Khaustov aka xDimon <khaustov.dm@gmail.com>
Signed-off-by: Dmitriy Khaustov aka xDimon <khaustov.dm@gmail.com>
…ion) Signed-off-by: Dmitriy Khaustov aka xDimon <khaustov.dm@gmail.com>
Signed-off-by: Dmitriy Khaustov aka xDimon <khaustov.dm@gmail.com>
Signed-off-by: Dmitriy Khaustov aka xDimon <khaustov.dm@gmail.com>
Signed-off-by: Dmitriy Khaustov aka xDimon <khaustov.dm@gmail.com>
Signed-off-by: Dmitriy Khaustov aka xDimon <khaustov.dm@gmail.com>
Signed-off-by: Dmitriy Khaustov aka xDimon <khaustov.dm@gmail.com>
// TODO optimize by avoiding of copying | ||
common::Buffer pre_runtime_data{std::move(encode_res.value())}; |
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.
[minor] Could be named makeCurrentSlotPreDigest
} | ||
return false; | ||
} |
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.
Should we proceed if consensus
is some unknown implementation or it is initialized by nullptr (which could be the case for getProductionConsensus
returned value)?
Probably makes sense adding else branch that handles such cases
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, this returns false for error, because we have already make sure the consensus is 'Babe'
|
||
return res; | ||
SL_TRACE(logger_, "Epoch changed to epoch {}", epoch_); | ||
return keypair_.has_value(); |
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.
If this function returns true when epoch has changed, does it makes sense to return true when keypair has value? What if we are running as syncing node?
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.
if we have no one key, it is one case of non-having key actual for epoch. In all such case we can not be validator.
Signed-off-by: Dmitriy Khaustov aka xDimon <khaustov.dm@gmail.com>
Referenced issues
Needed for #1591 (as part of preparation for sassafras lottery develop)
Description of the Change
Makes block building consensus agnostic as more as possible: slot leadership computation encapsulated into lottery.
This is needed for future extraction common block production mechanisms to intermediate base class.