Skip to content

[Merged by Bors] - Pass failed gossip blocks to the slasher #2047

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.

Sign up for GitHub

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

Jump to bottom
Closed
michaelsproul wants to merge 2 commits into from
Closed

[Merged by Bors] - Pass failed gossip blocks to the slasher #2047

michaelsproul wants to merge 2 commits into from

Conversation

@michaelsproul
Copy link
Member

@michaelsproul michaelsproul commented Dec 4, 2020

Issue Addressed

Closes #2042

Proposed Changes

Pass blocks that fail gossip verification to the slasher. Blocks that are successfully verified are not passed immediately, but will be passed as part of full block verification.

@michaelsproul michaelsproul added ready-for-review The code is ready for review A0 labels Dec 4, 2020
@michaelsproul
Copy link
Member Author

michaelsproul commented Dec 4, 2020

Here's a sketch of how with this change, all entrypoints to block verification pass their blocks to the slasher regardless of error.

Entrypoint verify_block_for_gossip

On success: goes to process_block flow, see below.

On failure: passed to slasher via GossipVerifiedBlock::new

Entrypoint process_block

Could be called with any type implementing IntoFullyVerifiedBlock.

Each type implementing this calls into_fully_verified_block, which has a default implementation in terms of into_fully_verified_block_slashable. The default impl handles pushing blocks to the slasher, and into_fully_verified_block_slashable does different checks based on the receiver type, while producing slash info:

If GossipVerifiedBlock, gets transformed into SignatureVerifiedBlock via from_gossip_verified_block_check_slashable, which produces slash info.

If SignedBeaconBlock, gets transformed into SignatureVerifiedBlock via SignatureVerifiedBlock::check_slashable, which produces slash info.

If SignatureVerifiedBlock, does some more verification and then wraps any errors in BlockSlashInfo::SignatureValid.

paulhauner
paulhauner approved these changes Dec 4, 2020
View reviewed changes
Copy link
Member

@paulhauner paulhauner left a comment

Choose a reason for hiding this comment

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

Nice! Merge at will!

@michaelsproul
Copy link
Member Author

michaelsproul commented Dec 4, 2020

bors r+

bors bot pushed a commit that referenced this pull request Dec 4, 2020
@michaelsproul
Pass failed gossip blocks to the slasher (#2047)
e860fd6 
## Issue Addressed

Closes #2042

## Proposed Changes

Pass blocks that fail gossip verification to the slasher. Blocks that are successfully verified are not passed immediately, but will be passed as part of full block verification.
@michaelsproul
Copy link
Member Author

michaelsproul commented Dec 4, 2020

Hmm that ENR test failed again

@bors
Copy link

bors bot commented Dec 4, 2020

Build failed:

@michaelsproul
Copy link
Member Author

michaelsproul commented Dec 4, 2020

bors r+

@michaelsproul michaelsproul mentioned this pull request Dec 4, 2020
Closed
bors bot pushed a commit that referenced this pull request Dec 4, 2020
@michaelsproul
Pass failed gossip blocks to the slasher (#2047)
c1ec386 
## Issue Addressed

Closes #2042

## Proposed Changes

Pass blocks that fail gossip verification to the slasher. Blocks that are successfully verified are not passed immediately, but will be passed as part of full block verification.
@bors
Copy link

bors bot commented Dec 4, 2020

Pull request successfully merged into unstable.

Build succeeded:

@bors bors bot changed the title Pass failed gossip blocks to the slasher [Merged by Bors] - Pass failed gossip blocks to the slasher Dec 4, 2020
@bors bors bot closed this Dec 4, 2020
@michaelsproul michaelsproul deleted the slasher-proposers branch December 4, 2020 06:13
@AgeManning AgeManning mentioned this pull request Dec 7, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@paulhauner paulhauner paulhauner approved these changes

Assignees

No one assigned

Labels

A0 ready-for-review The code is ready for review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@michaelsproul @paulhauner