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

Add the additional eip-3076 attestation checks #7966

Merged
merged 14 commits into from
Dec 5, 2020

Conversation

terencechain
Copy link
Member

@terencechain terencechain commented Nov 26, 2020

What type of PR is this?

Feature

What does this PR do? Why is it needed?

Review and merge #7965 first

This PR performs the following eip-3076 checks in preAttSignValidations, and saves the lowest source and target epochs to DB in postAttSignUpdate

Refuse to sign any attestation with source epoch less than the minimum source epoch present in that signer’s attestations

Refuse to sign any attestation with target epoch less than or equal to the minimum target epoch present in that signer’s attestations

Which issues(s) does this PR fix?

Fixes #

Other notes for review

@terencechain terencechain added the Ready For Review A pull request ready for code review label Nov 26, 2020
@terencechain terencechain added this to the v1.0.1 milestone Nov 26, 2020
@terencechain terencechain requested a review from a team as a code owner November 26, 2020 05:42
@terencechain terencechain self-assigned this Nov 26, 2020
@terencechain terencechain changed the title Att check max epochs 1 Check source and target epoch in preAttSignValidations Nov 26, 2020
@terencechain terencechain changed the title Check source and target epoch in preAttSignValidations Add the additional eip-3076 attestation checks Nov 26, 2020
Copy link
Member

@nisdas nisdas left a comment

Choose a reason for hiding this comment

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

LGTM too

Copy link
Member

@nisdas nisdas left a comment

Choose a reason for hiding this comment

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

Sorry rejecting this

time="2020-11-26 05:59:00" level=error msg="Failed attestation slashing protection check" error="could not sign attestation lower than lowest target epoch in db, 0 >= 0" prefix=validator pubKey=0xb3eea1ef03b3 slot=0
time="2020-11-26 05:59:00" level=error msg="Failed attestation slashing protection check" error="could not sign attestation lower than lowest target epoch in db, 0 >= 0" prefix=validator pubKey=0xa626ce67a47d slot=0

nisdas
nisdas previously approved these changes Nov 26, 2020

m.validatorClient.EXPECT().DomainData(
gomock.Any(), // ctx
gomock.Any(), // epoch
Copy link
Member

Choose a reason for hiding this comment

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

Can you specify the value here? What do you expect to be sent? It looks like 0, which is a default value. Also consider a non-default value to make this test more robust.

@terencechain terencechain modified the milestones: v1.0.1, v1.1.0 Nov 26, 2020
@terencechain terencechain removed the Ready For Review A pull request ready for code review label Nov 26, 2020
@rauljordan rauljordan changed the base branch from master to develop November 30, 2020 21:40
@terencechain terencechain removed this from the v1.0.3 milestone Nov 30, 2020
@rauljordan rauljordan merged commit c64de91 into feature/slashing-interchange Dec 5, 2020
@delete-merged-branch delete-merged-branch bot deleted the att-check-max-epochs-1 branch December 5, 2020 03:09
rauljordan added a commit that referenced this pull request Jan 22, 2021
* Change LowestSignedProposal to Also Return a Boolean for Slashing Protection (#8020)

* amend to use bools

* ineff assign

* comment

* Update `LowestSignedTargetEpoch` to include exists (#8004)

* Replace highest with lowerest

* Update validator/db/kv/attestation_history_v2.go

Co-authored-by: Raul Jordan <raul@prysmaticlabs.com>

* Update validator/db/kv/attestation_history_v2.go

Co-authored-by: Raul Jordan <raul@prysmaticlabs.com>

* Invert equality for saveLowestSourceTargetToDB

* Add eip checks to ensure epochs cant be lower than db ones

* Should be less than equal to

* Check if epoch exists in DB getters

* Revert run time checks

Co-authored-by: Raul Jordan <raul@prysmaticlabs.com>
Co-authored-by: prylabs-bulldozer[bot] <58059840+prylabs-bulldozer[bot]@users.noreply.github.com>

* Export Attesting History for Slashing Interchange Standard (#8027)

* added in att history checks

* logic for export

* export return nil

* test for export atts

* round trip passes first try!

* rem println

* fix up tests

* pass test

* Validate Proposers Are Not Slashable With Regard to Data Within Slasher Interchange JSON (#8031)

* filter slashable blocks and atts in same json stub

* add filter blocks func

* add test for filtering out the bad public keys

* Export Slashing Protection History Via CLI (#8040)

* include cli entrypoint for history exports

* builds properly

* test to confirm we export the data as expected

* abstract helpers properly

* full test suite

* gaz

* better errors

* marshal ident

* Add the additional eip-3076 attestation checks (#7966)

* Replace highest with lowerest

* Update validator/db/kv/attestation_history_v2.go

Co-authored-by: Raul Jordan <raul@prysmaticlabs.com>

* Update validator/db/kv/attestation_history_v2.go

Co-authored-by: Raul Jordan <raul@prysmaticlabs.com>

* Invert equality for saveLowestSourceTargetToDB

* Add eip checks to ensure epochs cant be lower than db ones

* Should be less than equal to

* Check if epoch exists in DB getters

Co-authored-by: Raul Jordan <raul@prysmaticlabs.com>

* Add EIP-3076 Invariants for Proposer Slashing Protection (#8067)

* add invariant for proposer protection

* write different test cases

* pass tests

* Add EIP-3076 Interchange JSON CLI command to validator (#7880)

* Import JSON CLI

* CLI impotr

* f

* Begin adding new commands in slashing protection

* Move testing helpers to separate packae

* Add command for importing slashing protection JSONs

* fix import cycle

* fix test

* Undo cleaning changes

* Improvements

* Add better prompts

* Fix prompt

* Fix

* Fix

* Fix

* Fix conflict

* Fix

* Fixes

* Fixes

* Fix exported func

* test func

* Fixes

* fix test

* simplify import and standardize with export

* add round trip test

* true integration test works

* fix up comments

* logrus

* better error

* fix build

* build fix

* Update validator/slashing-protection/cli_export.go

Co-authored-by: Preston Van Loon <preston@prysmaticlabs.com>

* Update validator/slashing-protection/cli_import.go

Co-authored-by: Preston Van Loon <preston@prysmaticlabs.com>

* fmt

Co-authored-by: Raul Jordan <raul@prysmaticlabs.com>
Co-authored-by: Preston Van Loon <preston@prysmaticlabs.com>

* Filter Slashable Attester Public Keys in Slashing Interchange Import (#8051)

* filter slashable attesters from the same JSON

* builds

* fix up initially broken test

* circular dep

* import fix

* giz

* added in attesting history package

* add test for filter slashable attester keys

* pass tests

* Save Slashable Keys to Disk in the Validator Client (#8082)

* begin db funcs

* add in test and bucket

* gaz

* rem changes to import

* ineff assign

* add godoc

* Properly Handle Duplicate Public Key Entries in Slashing Interchange Imports (#8089)

* Prevent Blacklisted Public Keys from Slashing Protection Imports from Having Duties at Runtime (#8084)

* tests on update duties

* ensure the slashable public keys are filtered out from update duties via test

* begin test

* attempt test

* rename for better context

* pass tests

* deep source

* ensure tests pass

* Check for Signing Root Mismatch When Submitting Proposals and Importing Proposals in Slashing Interchange (#8085)

* flexible signing root

* add test

* add tests

* fix test

* Preston's comments

* res tests

* ensure we consider the case for minimum proposals

* pass test

* tests passing

* rem unused code

* Set Empty Epochs in Between Attestations as FAR_FUTURE_EPOCH in Attesting History (#8113)

* set target data

* all tests passing

* ineff assign

* signing root

* Add Slashing Interchange, EIP-3076, Spec Tests to Prysm (#7858)

* Add interchange test framework

* add checks for attestations

* Import genesis root if necessary

* flexible signing root

* add test

* Sync

* fix up test build

* only 3 failing tests now

* two failing

* attempting to debug problems in conformity tests

* include latest changes

* protect test in validator/client passing

* pass tests

* imports

* spec tests passing with bazel

* gh archive link to spectests using tar.gz suffix

* rev

* rev more comment changes

* fix sha

* godoc

* add back save

Co-authored-by: Raul Jordan <raul@prysmaticlabs.com>

* Implement Migration for Unattested Epochs in Attesting History Database (#8121)

* migrate attesting history backbone done

* begin migration logic

* implement migration logic

* migration test

* add test

* migration logic

* bazel

* migration to its own file

* Handle empty blocks and attestations in interchange json and sort interchange json by public key (#8132)

* Handle empty blocks and attestations in interchange json

* add test

* sort json

* easier empty arrays

* pass test

Co-authored-by: Raul Jordan <raul@prysmaticlabs.com>

* builds

* more tests finally build

* Align Slashing Interchange With Optimized Slashing Protection (#8268)

* attestation history should account for multiple targets per source

* attempt at some fixes

* attempt some test fixes

* experimenting with sorting

* only one more failing test

* tests now pass

* slash protect tests passing

* only few tests now failing

* only spec tests failing now

* spec tests passing

* all tests passing

* helper function for verifying double votes

* use helper

* gaz

* deep source

* tests fixed

* expect specific number of times for domain data calls

* final comments

* Batch Save Imported EIP-3076 Attestations (#8304)

* optimize save

* test added

* add test for sad path

Co-authored-by: prylabs-bulldozer[bot] <58059840+prylabs-bulldozer[bot]@users.noreply.github.com>

* revert bad find replace

* add comment to db func

Co-authored-by: terence tsao <terence@prysmaticlabs.com>
Co-authored-by: prylabs-bulldozer[bot] <58059840+prylabs-bulldozer[bot]@users.noreply.github.com>
Co-authored-by: Ivan Martinez <ivanthegreatdev@gmail.com>
Co-authored-by: Preston Van Loon <preston@prysmaticlabs.com>
Co-authored-by: Shay Zluf <thezluf@gmail.com>
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.

4 participants