Skip to content
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

Cleanup stategen pkg #7127

Merged
merged 9 commits into from
Aug 27, 2020
Merged

Cleanup stategen pkg #7127

merged 9 commits into from
Aug 27, 2020

Conversation

terencechain
Copy link
Member

@terencechain terencechain commented Aug 27, 2020

What type of PR is this?

Clean up

What does this PR do? Why is it needed?
As we are heading towards further audits / mainnet, it's important we go through each package and perform the necessary clean ups. This PR cleans up the stategen pkg

  • Removed unnecessary hot and cold abstractions. Instead of loadHotStateByRoot, it's just loadStatebyRoot
  • Removed a few unused functions ComputeStateUpToSlot, processStateUpTo, and archivedState... etc
  • Moved functions from hot.go and cold.go under getter.go and setter.go as helpers
  • There's no logic changes here, this is just renaming and move stuff around

Which issues(s) does this PR fix?

N/A

Other notes for review
Tested:
1.)Validator run time
2.)Syncing from genesis to head using 32 slots archived point
3.)The following script

#!/bin/bash
for i in {0..2000}
do
   time curl -X GET "http://localhost:3051/eth/v1alpha1/validators/assignments?epoch=$i" -H "accept: application/json" | jq . | grep epoch
done

prestonvanloon
prestonvanloon previously approved these changes Aug 27, 2020
@codecov
Copy link

codecov bot commented Aug 27, 2020

Codecov Report

Merging #7127 into master will decrease coverage by 0.28%.
The diff coverage is 67.30%.

@@            Coverage Diff             @@
##           master    #7127      +/-   ##
==========================================
- Coverage   61.95%   61.67%   -0.29%     
==========================================
  Files         410      406       -4     
  Lines       32826    32109     -717     
==========================================
- Hits        20338    19803     -535     
+ Misses       9618     9489     -129     
+ Partials     2870     2817      -53     

return cachedInfo.state, nil
}

summary, err := s.stateSummary(ctx, blockRoot)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be nil?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -0,0 +1,16 @@
package stategen
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe rename this file to metrics.go for consistency

rauljordan
rauljordan previously approved these changes Aug 27, 2020
@prylabs-bulldozer prylabs-bulldozer bot merged commit fea2cc9 into master Aug 27, 2020
@delete-merged-branch delete-merged-branch bot deleted the cleanup-newstate branch August 27, 2020 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cleanup Code health!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants