[Merged by Bors] - Add broadcast validation routes to Beacon Node HTTP API by jmcph4 · Pull Request #4316 · sigp/lighthouse

jmcph4 · 2023-05-22T05:22:04Z

Issue Addressed

Proposed Changes

Changes largely follow those suggested in the main issue.

Add new routes to HTTP API
- post_beacon_blocks_v2
- post_blinded_beacon_blocks_v2
Add new routes to BeaconNodeHttpClient
- post_beacon_blocks_v2
- post_blinded_beacon_blocks_v2
Define new Eth2 common types
- BroadcastValidation, enum representing the level of validation to apply to blocks prior to broadcast
- BroadcastValidationQuery, the corresponding HTTP query string type for the above type
~~Define _checked variants of both publish_block and publish_blinded_block that enforce a validation level at a type level~~
Add interactive tests to the bn_http_api_tests test target covering each validation level (to their own test module, broadcast_validation_tests)
- beacon/blocks
  - broadcast_validation=gossip
    - Invalid (400)
    - Full Pass (200)
    - Partial Pass (202)
  - broadcast_validation=consensus
    - Invalid (400)
    - Only gossip (400)
    - Only consensus pass (i.e., equivocates) (200)
    - Full pass (200)
  - broadcast_validation=consensus_and_equivocation
    - Invalid (400)
    - Invalid due to early equivocation (400)
    - Only gossip (400)
    - Only consensus (400)
    - Pass (200)
- beacon/blinded_blocks
  - broadcast_validation=gossip
    - Invalid (400)
    - Full Pass (200)
    - Partial Pass (202)
  - broadcast_validation=consensus
    - Invalid (400)
    - Only gossip (400)
    - ~~Only consensus pass (i.e., equivocates) (200)~~
    - Full pass (200)
  - broadcast_validation=consensus_and_equivocation
    - Invalid (400)
    - Invalid due to early equivocation (400)
    - Only gossip (400)
    - Only consensus (400)
    - Pass (200)
Add a new trait, IntoGossipVerifiedBlock, which allows type-level guarantees to be made as to gossip validity
Modify the structure of the ObservedBlockProducers cache from a (slot, validator_index) mapping to a ((slot, validator_index), block_root) mapping
Modify ObservedBlockProducers::proposer_has_been_observed to return a SeenBlock rather than a boolean on success
Punish gossip peer (low) for submitting equivocating blocks
Rename BlockError::SlashablePublish to BlockError::SlashableProposal

Additional Info

This PR contains changes that directly modify how blocks are verified within the client. For more context, consult comments in-thread.

This reverts commit 23c804e.

…ghthouse into lighthouse-relay

…thouse-relay

…ghthouse-relay

This commit defines a new type, `BroadcastValidation`, which represents the level of block validation to perform prior to broadcasting.

This commit adds in plumbing to the existing `publish_block` function to allow broadcast validation to take place. Wrappers are also added to both the full and blinded variants for block publication -- these enforce validation by accepting a `BroadcastValidation` type directly, rather than an `Option`al one.

This commit adds the two new routes specified in the new Beacon API specification.

This commit `match`es on the provided validation level in `publish_block` in order to pass the correct closure into `process_block`.

This commit adds the new API routes to the `BeaconNodeHttpClient` wrapper type.

This commit adds an interactive test case for the case where a block is only gossip-valid, not consensus-valid nor equivocation-free.

This commit fixes path construction in the relevant routes on the `BeaconNodeHttpClient` wrapper type.

This commit moves the interactive broadcast validation tests to their own module.

This commit corrects the route definitions to accept query strings rather than URL parameters, in line with the Beacon APIs specification.

realbigsean

great job @jmcph4 ! Just had some small requests

beacon_node/network/src/beacon_processor/worker/gossip_methods.rs

beacon_node/beacon_chain/src/block_verification.rs

michaelsproul · 2023-06-29T04:34:50Z

Some of the tests are broken, we're looking into it

…hthouse into 4293-broadcast-validation-api

michaelsproul

Pesky tests fixed! Let's merge!

michaelsproul · 2023-06-29T08:03:10Z

bors r+

## Issue Addressed - #4293 - #4264 ## Proposed Changes *Changes largely follow those suggested in the main issue*. - Add new routes to HTTP API - `post_beacon_blocks_v2` - `post_blinded_beacon_blocks_v2` - Add new routes to `BeaconNodeHttpClient` - `post_beacon_blocks_v2` - `post_blinded_beacon_blocks_v2` - Define new Eth2 common types - `BroadcastValidation`, enum representing the level of validation to apply to blocks prior to broadcast - `BroadcastValidationQuery`, the corresponding HTTP query string type for the above type - ~~Define `_checked` variants of both `publish_block` and `publish_blinded_block` that enforce a validation level at a type level~~ - Add interactive tests to the `bn_http_api_tests` test target covering each validation level (to their own test module, `broadcast_validation_tests`) - `beacon/blocks` - `broadcast_validation=gossip` - Invalid (400) - Full Pass (200) - Partial Pass (202) - `broadcast_validation=consensus` - Invalid (400) - Only gossip (400) - Only consensus pass (i.e., equivocates) (200) - Full pass (200) - `broadcast_validation=consensus_and_equivocation` - Invalid (400) - Invalid due to early equivocation (400) - Only gossip (400) - Only consensus (400) - Pass (200) - `beacon/blinded_blocks` - `broadcast_validation=gossip` - Invalid (400) - Full Pass (200) - Partial Pass (202) - `broadcast_validation=consensus` - Invalid (400) - Only gossip (400) - ~~Only consensus pass (i.e., equivocates) (200)~~ - Full pass (200) - `broadcast_validation=consensus_and_equivocation` - Invalid (400) - Invalid due to early equivocation (400) - Only gossip (400) - Only consensus (400) - Pass (200) - Add a new trait, `IntoGossipVerifiedBlock`, which allows type-level guarantees to be made as to gossip validity - Modify the structure of the `ObservedBlockProducers` cache from a `(slot, validator_index)` mapping to a `((slot, validator_index), block_root)` mapping - Modify `ObservedBlockProducers::proposer_has_been_observed` to return a `SeenBlock` rather than a boolean on success - Punish gossip peer (low) for submitting equivocating blocks - Rename `BlockError::SlashablePublish` to `BlockError::SlashableProposal` ## Additional Info This PR contains changes that directly modify how blocks are verified within the client. For more context, consult [comments in-thread](#4316 (comment)). Co-authored-by: Michael Sproul <[email protected]>

bors · 2023-06-29T09:09:07Z

Build failed (retrying...):

no-eth1-simulator-ubuntu

## Issue Addressed - #4293 - #4264 ## Proposed Changes *Changes largely follow those suggested in the main issue*. - Add new routes to HTTP API - `post_beacon_blocks_v2` - `post_blinded_beacon_blocks_v2` - Add new routes to `BeaconNodeHttpClient` - `post_beacon_blocks_v2` - `post_blinded_beacon_blocks_v2` - Define new Eth2 common types - `BroadcastValidation`, enum representing the level of validation to apply to blocks prior to broadcast - `BroadcastValidationQuery`, the corresponding HTTP query string type for the above type - ~~Define `_checked` variants of both `publish_block` and `publish_blinded_block` that enforce a validation level at a type level~~ - Add interactive tests to the `bn_http_api_tests` test target covering each validation level (to their own test module, `broadcast_validation_tests`) - `beacon/blocks` - `broadcast_validation=gossip` - Invalid (400) - Full Pass (200) - Partial Pass (202) - `broadcast_validation=consensus` - Invalid (400) - Only gossip (400) - Only consensus pass (i.e., equivocates) (200) - Full pass (200) - `broadcast_validation=consensus_and_equivocation` - Invalid (400) - Invalid due to early equivocation (400) - Only gossip (400) - Only consensus (400) - Pass (200) - `beacon/blinded_blocks` - `broadcast_validation=gossip` - Invalid (400) - Full Pass (200) - Partial Pass (202) - `broadcast_validation=consensus` - Invalid (400) - Only gossip (400) - ~~Only consensus pass (i.e., equivocates) (200)~~ - Full pass (200) - `broadcast_validation=consensus_and_equivocation` - Invalid (400) - Invalid due to early equivocation (400) - Only gossip (400) - Only consensus (400) - Pass (200) - Add a new trait, `IntoGossipVerifiedBlock`, which allows type-level guarantees to be made as to gossip validity - Modify the structure of the `ObservedBlockProducers` cache from a `(slot, validator_index)` mapping to a `((slot, validator_index), block_root)` mapping - Modify `ObservedBlockProducers::proposer_has_been_observed` to return a `SeenBlock` rather than a boolean on success - Punish gossip peer (low) for submitting equivocating blocks - Rename `BlockError::SlashablePublish` to `BlockError::SlashableProposal` ## Additional Info This PR contains changes that directly modify how blocks are verified within the client. For more context, consult [comments in-thread](#4316 (comment)). Co-authored-by: Michael Sproul <[email protected]>

bors · 2023-06-29T14:31:03Z

Pull request successfully merged into unstable.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

## Issue Addressed - sigp#4293 - sigp#4264 ## Proposed Changes *Changes largely follow those suggested in the main issue*. - Add new routes to HTTP API - `post_beacon_blocks_v2` - `post_blinded_beacon_blocks_v2` - Add new routes to `BeaconNodeHttpClient` - `post_beacon_blocks_v2` - `post_blinded_beacon_blocks_v2` - Define new Eth2 common types - `BroadcastValidation`, enum representing the level of validation to apply to blocks prior to broadcast - `BroadcastValidationQuery`, the corresponding HTTP query string type for the above type - ~~Define `_checked` variants of both `publish_block` and `publish_blinded_block` that enforce a validation level at a type level~~ - Add interactive tests to the `bn_http_api_tests` test target covering each validation level (to their own test module, `broadcast_validation_tests`) - `beacon/blocks` - `broadcast_validation=gossip` - Invalid (400) - Full Pass (200) - Partial Pass (202) - `broadcast_validation=consensus` - Invalid (400) - Only gossip (400) - Only consensus pass (i.e., equivocates) (200) - Full pass (200) - `broadcast_validation=consensus_and_equivocation` - Invalid (400) - Invalid due to early equivocation (400) - Only gossip (400) - Only consensus (400) - Pass (200) - `beacon/blinded_blocks` - `broadcast_validation=gossip` - Invalid (400) - Full Pass (200) - Partial Pass (202) - `broadcast_validation=consensus` - Invalid (400) - Only gossip (400) - ~~Only consensus pass (i.e., equivocates) (200)~~ - Full pass (200) - `broadcast_validation=consensus_and_equivocation` - Invalid (400) - Invalid due to early equivocation (400) - Only gossip (400) - Only consensus (400) - Pass (200) - Add a new trait, `IntoGossipVerifiedBlock`, which allows type-level guarantees to be made as to gossip validity - Modify the structure of the `ObservedBlockProducers` cache from a `(slot, validator_index)` mapping to a `((slot, validator_index), block_root)` mapping - Modify `ObservedBlockProducers::proposer_has_been_observed` to return a `SeenBlock` rather than a boolean on success - Punish gossip peer (low) for submitting equivocating blocks - Rename `BlockError::SlashablePublish` to `BlockError::SlashableProposal` ## Additional Info This PR contains changes that directly modify how blocks are verified within the client. For more context, consult [comments in-thread](sigp#4316 (comment)). Co-authored-by: Michael Sproul <[email protected]>

pawanjay176 · 2023-07-14T23:34:27Z

beacon_node/beacon_chain/src/block_verification.rs

        chain: &Arc<BeaconChain<T>>,
        notify_execution_layer: NotifyExecutionLayer,
    ) -> Result<Self, BlockError<T::EthSpec>> {
+        chain


@jmcph4 Sorry for the late ping here. Stumbled into this while reviewing some deneb stuff.
Wondering why we observe the block here. We would hit this condition from multiple paths:

When verifying a chain segment during range sync: we attempt to observe proposals from before the finalized slot which would be redundant.

For verification of RPC blocks: doesn't really make sense to observe RPC blocks in a cache for blocks received over gossip

@pawanjay176 We've kind of expanded the scope of the cache beyond just gossip, because the relays need to detect all equivocations, regardless of whether the block is fetched via gossip or RPC.

I agree that observing finalized blocks is not useful though, so we should probably refactor to only observe blocks within a small distance of the current slot.

Is this causing problems on Deneb?

Ahh gotcha. No issues on deneb, was just a bit confusing to me.
We should probably also change the docs to reflect the scope if we are refactoring to only observe in a small distance

- sigp#4293 - sigp#4264 *Changes largely follow those suggested in the main issue*. - Add new routes to HTTP API - `post_beacon_blocks_v2` - `post_blinded_beacon_blocks_v2` - Add new routes to `BeaconNodeHttpClient` - `post_beacon_blocks_v2` - `post_blinded_beacon_blocks_v2` - Define new Eth2 common types - `BroadcastValidation`, enum representing the level of validation to apply to blocks prior to broadcast - `BroadcastValidationQuery`, the corresponding HTTP query string type for the above type - ~~Define `_checked` variants of both `publish_block` and `publish_blinded_block` that enforce a validation level at a type level~~ - Add interactive tests to the `bn_http_api_tests` test target covering each validation level (to their own test module, `broadcast_validation_tests`) - `beacon/blocks` - `broadcast_validation=gossip` - Invalid (400) - Full Pass (200) - Partial Pass (202) - `broadcast_validation=consensus` - Invalid (400) - Only gossip (400) - Only consensus pass (i.e., equivocates) (200) - Full pass (200) - `broadcast_validation=consensus_and_equivocation` - Invalid (400) - Invalid due to early equivocation (400) - Only gossip (400) - Only consensus (400) - Pass (200) - `beacon/blinded_blocks` - `broadcast_validation=gossip` - Invalid (400) - Full Pass (200) - Partial Pass (202) - `broadcast_validation=consensus` - Invalid (400) - Only gossip (400) - ~~Only consensus pass (i.e., equivocates) (200)~~ - Full pass (200) - `broadcast_validation=consensus_and_equivocation` - Invalid (400) - Invalid due to early equivocation (400) - Only gossip (400) - Only consensus (400) - Pass (200) - Add a new trait, `IntoGossipVerifiedBlock`, which allows type-level guarantees to be made as to gossip validity - Modify the structure of the `ObservedBlockProducers` cache from a `(slot, validator_index)` mapping to a `((slot, validator_index), block_root)` mapping - Modify `ObservedBlockProducers::proposer_has_been_observed` to return a `SeenBlock` rather than a boolean on success - Punish gossip peer (low) for submitting equivocating blocks - Rename `BlockError::SlashablePublish` to `BlockError::SlashableProposal` This PR contains changes that directly modify how blocks are verified within the client. For more context, consult [comments in-thread](sigp#4316 (comment)). Co-authored-by: Michael Sproul <[email protected]>

realbigsean and others added 26 commits April 5, 2023 12:34

fully validate a block before publishing

09ba87c

remove error on publish

23c804e

Revert "remove error on publish"

7ce5ad5

This reverts commit 23c804e.

publish block prior to payload verification and fork choice

2625007

don't publish blocks that are slashable

5cbdd5f

get beacon chain tests compiling

771abe7

use observed proposals cache

2394c97

update observed proposals cache to report slashability correctly

0531088

improve logging

8f38805

fix compile

451288e

consolidate block proposal caches

1a3afdb

Use head state for exit verification

1da556e

Empty commit

db6ace5

Bump versions

35d8c98

Merge branch 'hotfix-exit-verification' of https://github.com/sigp/li…

12c1b46

…ghthouse into lighthouse-relay

Merge branch 'stable' of https://github.com/sigp/lighthouse into ligh…

01431d4

…thouse-relay

Merge branch 'unstable' of https://github.com/sigp/lighthouse into li…

93837e3

…ghthouse-relay

add publish delay to logs for lighthouse relay

746b4ef

remove 202 errors

5ae9ec9

return 200 on non-slashable duplicates

00cd8da

Add BroadcastValidation type to API types

fe8d68d

This commit defines a new type, `BroadcastValidation`, which represents the level of block validation to perform prior to broadcasting.

Define new v2 routes for block broadcast

8693f8a

This commit adds the two new routes specified in the new Beacon API specification.

Modify publish callback based on validation level

27dc3f9

This commit `match`es on the provided validation level in `publish_block` in order to pass the correct closure into `process_block`.

Add new routes to HTTP client wrapper

198b55b

This commit adds the new API routes to the `BeaconNodeHttpClient` wrapper type.

Add test for gossip validity case

1284080

This commit adds an interactive test case for the case where a block is only gossip-valid, not consensus-valid nor equivocation-free.

jmcph4 self-assigned this May 22, 2023

jmcph4 added 3 commits May 23, 2023 11:23

Fix path construction in wrapper routes

dc0b23d

This commit fixes path construction in the relevant routes on the `BeaconNodeHttpClient` wrapper type.

Move broadcast validation tests to their own module

92512ce

This commit moves the interactive broadcast validation tests to their own module.

Correct route definitions to accept query strings

091c566

This commit corrects the route definitions to accept query strings rather than URL parameters, in line with the Beacon APIs specification.

jmcph4 added 3 commits June 21, 2023 12:03

Restore log output for BlockIsAlreadyKnown

8f99103

Remove remaining instances of RepeatProposal

d14541e

Patch block verification test

493fff9

realbigsean requested changes Jun 26, 2023

View reviewed changes

beacon_node/network/src/beacon_processor/worker/gossip_methods.rs Outdated Show resolved Hide resolved

beacon_node/beacon_chain/src/block_verification.rs Outdated Show resolved Hide resolved

beacon_node/beacon_chain/src/block_verification.rs Outdated Show resolved Hide resolved

jmcph4 and others added 2 commits June 29, 2023 10:20

Adjust peer scoring to a softer penalty for submitting equivocations

d5aca48

Clean up BlockError variants

0066e6e

michaelsproul added the ready-for-merge This PR is ready to merge. label Jun 29, 2023

Patch v1 test

7603b23

michaelsproul added work-in-progress PR is a work-in-progress and removed ready-for-merge This PR is ready to merge. labels Jun 29, 2023

jmcph4 added 2 commits June 29, 2023 14:59

Patch get_events reorg test

3fe968f

Merge branch '4293-broadcast-validation-api' of github.com:jmcph4/lig…

350de4a

…hthouse into 4293-broadcast-validation-api

michaelsproul approved these changes Jun 29, 2023

View reviewed changes

michaelsproul added ready-for-merge This PR is ready to merge. and removed work-in-progress PR is a work-in-progress labels Jun 29, 2023

bors bot changed the title ~~Add broadcast validation routes to Beacon Node HTTP API~~ [Merged by Bors] - Add broadcast validation routes to Beacon Node HTTP API Jun 29, 2023

bors bot closed this Jun 29, 2023

jmcph4 mentioned this pull request Jul 3, 2023

Store equivocating blocks on disk #4325

Closed

michaelsproul mentioned this pull request Jul 5, 2023

No gossip validation before block broadcast #4264

Closed

jmcph4 mentioned this pull request Jul 11, 2023

Implement broadcast_validation API #4293

Closed

pawanjay176 reviewed Jul 14, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Merged by Bors] - Add broadcast validation routes to Beacon Node HTTP API#4316

[Merged by Bors] - Add broadcast validation routes to Beacon Node HTTP API#4316
jmcph4 wants to merge 114 commits intosigp:unstablefrom
jmcph4:4293-broadcast-validation-api

jmcph4 commented May 22, 2023 •

edited

Loading

Uh oh!

realbigsean left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

michaelsproul commented Jun 29, 2023

Uh oh!

michaelsproul left a comment

Uh oh!

michaelsproul commented Jun 29, 2023

Uh oh!

bors bot commented Jun 29, 2023

Uh oh!

bors bot commented Jun 29, 2023

Uh oh!

pawanjay176 Jul 14, 2023

Uh oh!

michaelsproul Jul 15, 2023 •

edited

Loading

Uh oh!

pawanjay176 Jul 17, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

jmcph4 commented May 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue Addressed

Proposed Changes

Additional Info

Uh oh!

realbigsean left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

michaelsproul commented Jun 29, 2023

Uh oh!

michaelsproul left a comment

Choose a reason for hiding this comment

Uh oh!

michaelsproul commented Jun 29, 2023

Uh oh!

bors bot commented Jun 29, 2023

Uh oh!

bors bot commented Jun 29, 2023

Uh oh!

pawanjay176 Jul 14, 2023

Choose a reason for hiding this comment

Uh oh!

michaelsproul Jul 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pawanjay176 Jul 17, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jmcph4 commented May 22, 2023 •

edited

Loading

michaelsproul Jul 15, 2023 •

edited

Loading