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 slasher codebase #4698

Merged
merged 21 commits into from Feb 4, 2020

Conversation

0xKiwi
Copy link
Contributor

@0xKiwi 0xKiwi commented Jan 31, 2020

This PR is me going through the slasher client and cleaning up/improving whatever I find. Includes function renames, improvements to the codebase and readability improvements.

@codecov
Copy link

codecov bot commented Feb 3, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@74adeec). Click here to learn what that means.
The diff coverage is 0%.

@@            Coverage Diff            @@
##             master    #4698   +/-   ##
=========================================
  Coverage          ?   12.26%           
=========================================
  Files             ?       77           
  Lines             ?     6260           
  Branches          ?        0           
=========================================
  Hits              ?      768           
  Misses            ?     5360           
  Partials          ?      132

@0xKiwi 0xKiwi changed the title [WIP] Cleanup slasher Cleanup slasher codebase Feb 3, 2020
Copy link
Contributor

@rauljordan rauljordan left a comment

Choose a reason for hiding this comment

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

Just two comments

var status SlashingStatus
var found bool
key := encodeTypeRoot(SlashingType(Attestation), root)
root, err := hashutil.HashProto(slashing)
Copy link
Contributor

Choose a reason for hiding this comment

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

this isnt really a root, just a hash - maybe rename?

@@ -14,7 +14,7 @@ import (
"google.golang.org/grpc/status"
)

// finalisedChangeUpdater this is a stub for the comming PRs #3133
// finalisedChangeUpdater this is a stub for the coming PRs #3133
Copy link
Contributor

Choose a reason for hiding this comment

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

finalized*

@@ -15,28 +15,28 @@ var highestValidatorIdx uint64
func saveToDB(validatorIdx uint64, _ uint64, value interface{}, cost int64) {
log.Tracef("evicting span map for validator id: %d", validatorIdx)

err := d.batch(func(tx *bolt.Tx) error {
err := d.update(func(tx *bolt.Tx) error {
Copy link
Contributor

@shayzluf shayzluf Feb 4, 2020

Choose a reason for hiding this comment

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

this method might be called from many go routines maybe its better to keep it batch

Copy link
Contributor

@shayzluf shayzluf left a comment

Choose a reason for hiding this comment

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

had one comment. thanks

@prylabs-bulldozer prylabs-bulldozer bot merged commit 923d5fc into prysmaticlabs:master Feb 4, 2020
cryptomental pushed a commit to cryptomental/prysm that referenced this pull request Feb 24, 2020
* First wave of changes
* More changes
* More renames, changes
* Fix errors
* Merge branch 'master' of https://github.com/prysmaticlabs/Prysm into cleanup-slasher
* Fix errors, more cleaning
* Merge branch 'master' of https://github.com/prysmaticlabs/Prysm into cleanup-slasher
* Merge branch 'master' into cleanup-slasher
* fix err
* Merge branch 'cleanup-slasher' of https://github.com/0xKiwi/Prysm into cleanup-slasher
* Fix strings
* More cleanup
* Merge branch 'master' of https://github.com/prysmaticlabs/Prysm into cleanup-slasher
* Fix interface
* Fix
* Merge branch 'master' into cleanup-slasher
* Merge branch 'master' into cleanup-slasher
* Merge branch 'master' into cleanup-slasher
* Address comments
* Merge branch 'master' of https://github.com/prysmaticlabs/Prysm into cleanup-slasher
* Merge branch 'cleanup-slasher' of https://github.com/0xKiwi/Prysm into cleanup-slasher
cryptomental pushed a commit to cryptomental/prysm that referenced this pull request Feb 28, 2020
* First wave of changes
* More changes
* More renames, changes
* Fix errors
* Merge branch 'master' of https://github.com/prysmaticlabs/Prysm into cleanup-slasher
* Fix errors, more cleaning
* Merge branch 'master' of https://github.com/prysmaticlabs/Prysm into cleanup-slasher
* Merge branch 'master' into cleanup-slasher
* fix err
* Merge branch 'cleanup-slasher' of https://github.com/0xKiwi/Prysm into cleanup-slasher
* Fix strings
* More cleanup
* Merge branch 'master' of https://github.com/prysmaticlabs/Prysm into cleanup-slasher
* Fix interface
* Fix
* Merge branch 'master' into cleanup-slasher
* Merge branch 'master' into cleanup-slasher
* Merge branch 'master' into cleanup-slasher
* Address comments
* Merge branch 'master' of https://github.com/prysmaticlabs/Prysm into cleanup-slasher
* Merge branch 'cleanup-slasher' of https://github.com/0xKiwi/Prysm into cleanup-slasher
@0xKiwi 0xKiwi deleted the cleanup-slasher branch March 15, 2020 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants