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
Slasher proto and function renames #4797
Slasher proto and function renames #4797
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4797 +/- ##
========================================
Coverage ? 3.35%
========================================
Files ? 78
Lines ? 6433
Branches ? 0
========================================
Hits ? 216
Misses ? 6179
Partials ? 38 |
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 for doing it @0xKiwi , much clearer naming. did you run it with an archive endpoint to see it doesn't break?
lgtm
@@ -25,8 +25,8 @@ func unmarshalIdxAtt(enc []byte) (*ethpb.IndexedAttestation, error) { | |||
return protoIdxAtt, nil | |||
} | |||
|
|||
func unmarshalValIDsToIdxAttList(enc []byte) (*slashpb.ValidatorIDToIdxAttList, error) { | |||
protoIdxAtt := &slashpb.ValidatorIDToIdxAttList{} | |||
func unmarshalCompressedIdxAttList(enc []byte) (*slashpb.CompressedIdxAttList, 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.
CompressedIdx sounds too vague imo, is it validator index? attestation index? block index?... etc
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.
It is an indexed attestation, but the AttestationData
is changed to its []byte
attestation data root. That's why I refer to it as "compressed".
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.
Makes sense 👍
slasher/service/data_update.go
Outdated
} | ||
} | ||
|
||
// finalizedChangeUpdater this is a stub for the coming PRs #3133 |
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.
// finalizedChangeUpdater this is a stub for the coming PRs #3133 | |
// finalizedChangeUpdater this is a stub for the coming PRs #3133. |
slasher/service/data_update.go
Outdated
} | ||
|
||
// finalizedChangeUpdater this is a stub for the coming PRs #3133 | ||
// Store validator index to public key map Validate attestation signature. |
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.
// Store validator index to public key map Validate attestation signature. | |
// Store validator index to public key map to validate attestation signature. |
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.
lgtm
* Rename elements for clarity * Merge branch 'master' of https://github.com/prysmaticlabs/Prysm into slasher-renames * Fix test * Rename more functions * Cleanup * Fix logs * Merge branch 'master' of https://github.com/prysmaticlabs/Prysm into slasher-renames * Reorganize and clean up logs * Address comments * Add comments
* Rename elements for clarity * Merge branch 'master' of https://github.com/prysmaticlabs/Prysm into slasher-renames * Fix test * Rename more functions * Cleanup * Fix logs * Merge branch 'master' of https://github.com/prysmaticlabs/Prysm into slasher-renames * Reorganize and clean up logs * Address comments * Add comments
This PR renames and cleans up the slasher further to reduce confusion.
Renames:
ValidatorIDToIdxAtt
->CompressedIdxAtt
ValidatorIDToIdxAttList
->CompressedIdxAttList
BlockHeader()
->BlockHeaders()
dataForDetection()
->attsAndCommitteesForEpoch()
slasherOldAttestationFeeder()
->historicalAttestationFeeder()
Changed
indexedAttestationsIndicesBucket
tocompressedIdxAttsBucket
detectAttestation()
split intoconvertToIndexed()
anddetectSlashings()
Changes the
ClearDB()
function to only clear the slasher db, not the whole DB folder.