-
Notifications
You must be signed in to change notification settings - Fork 682
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
ProposerVM Extend windows 2- extend windowing #2401
Conversation
8d51f96
to
0d95f1b
Compare
0d95f1b
to
86153c0
Compare
ctx, | ||
parentHeight+1, | ||
parentPChainHeight, | ||
currentSlot+1, // We know we aren't the proposer for the current slot |
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.
Nice catch
return nil | ||
} | ||
|
||
func (p *postForkCommonComponents) shouldBuildBlockPostDurango( |
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.
nit: I think scheduleBuildBlockPostDurango
is a better name for what this actually is
shouldBuild*
at least implies a bool
is returned?
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.
hmmm I don't know what to call this function... I don't like scheduleBuildBlockPostDurango
- because it only schedules the build block if it returns an error...
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 going to add a TODO to restructure this function after durango activates... I think it'll be easier to clean up the code during that than now
if weight < uint64(numToSample) { | ||
numToSample = int(weight) | ||
} | ||
source.Seed(w.chainSource ^ blockHeight) |
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.
Calling out that this was chainHeight ^ w.chainSource
before
utils.Sort(validators) | ||
|
||
weights := make([]uint64, len(validators)) | ||
for i, validator := range validators { |
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 return totalWeight
from this function, we can remove some additional logic from Proposers
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.
We only need to calculate the total weight once in Proposers
- so I don't really see a reason to do that for every call we make to this 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.
I left some nits but the code is sound AFAICT
…valanchego into proposervm-upgraded-windowing
Why this should be merged
Change proposerVM windowing system, but extending the number of validators which can propose and allowing them a specific slot to do so
How this works
Based on #2404
How this was tested