-
Notifications
You must be signed in to change notification settings - Fork 214
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
[Merged by Bors] - remove read/write for ballot.ActiveSet #5024
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #5024 +/- ##
=========================================
- Coverage 77.2% 77.1% -0.1%
=========================================
Files 254 254
Lines 30102 30113 +11
=========================================
- Hits 23247 23242 -5
- Misses 5351 5362 +11
- Partials 1504 1509 +5
|
bors try |
tryBuild failed: |
bors try |
tryBuild failed: |
bors try |
tryBuild succeeded! The publicly hosted instance of bors-ng is deprecated and will go away soon. If you want to self-host your own instance, instructions are here. If you want to switch to GitHub's built-in merge queue, visit their help page. |
proposals/eligibility_validator.go
Outdated
@@ -140,11 +141,15 @@ func (v *Validator) validateReference(ballot *types.Ballot, owned *types.Activat | |||
if ballot.EpochData.Beacon == types.EmptyBeacon { | |||
return nil, fmt.Errorf("%w: ref ballot %v", errMissingBeacon, ballot.ID()) | |||
} | |||
if len(ballot.ActiveSet) == 0 { | |||
activeSet, err := activesets.Get(v.cdb, ballot.EpochData.ActiveSetHash) |
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 should be a way to avoid this read. we should have loaded this data earlier, as we need to check data availability. and then we need to pass it together with the rest of the ballot for eligibility validation
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. changed to pass it from the original lookup.
bors merge |
## Motivation Closes #4984 ## Changes - drop EmitEmptyActiveSet config - remove all access to Ballot.ActiveSet and look up from db instead - remove all writes of Ballot.ActiveSet after this PR, miner will still accept proposal/ballot with non-empty active sets, but will save ballots without active set.
bors cancel |
Canceled. |
bors merge |
## Motivation Closes #4984 ## Changes - drop EmitEmptyActiveSet config - remove all access to Ballot.ActiveSet and look up from db instead - remove all writes of Ballot.ActiveSet after this PR, miner will still accept proposal/ballot with non-empty active sets, but will save ballots without active set.
Pull request successfully merged into develop. Build succeeded! The publicly hosted instance of bors-ng is deprecated and will go away soon. If you want to self-host your own instance, instructions are here. If you want to switch to GitHub's built-in merge queue, visit their help page. |
Motivation
Closes #4984
Changes
after this PR, miner will still accept proposal/ballot with non-empty active sets, but will save ballots without active set.