[Merged by Bors] - Implement standard eth2.0 API #1569

paulhauner · 2020-08-26T08:20:48Z

Issue Addressed

Resolves Do not require subnet ID for /validator/attestations HTTP call #1550
Resolves Slow API calls for slots that are not synced #824
Resolves Parameters too loose on API calls #825
Resolves API Content type header bug #1131
Resolves Separate metrics server and API server (different ports) #1411
Resolves Document Prometheus port in lighthouse-book #1256
Resolve CORS headers on API #1177

Proposed Changes

Includes the ShufflingId struct initially defined in Swap to ShufflingId for keying shuffling cache #1492. That PR is now closed and the changes are included here, with significant bug fixes.
Implement the https://github.com/ethereum/eth2.0-APIs in a new http_api crate using warp. This replaces the rest_api crate.
Add a new common/eth2 crate which provides a wrapper around reqwest, providing the HTTP client that is used by the validator client and for testing. This replaces the common/remote_beacon_node crate.
Create a http_metrics crate which is a dedicated server for Prometheus metrics (they are no longer served on the same port as the REST API). We now have flags for --metrics, --metrics-address, etc.
Allow the subnet_id to be an optional parameter for VerifiedUnaggregatedAttestation::verify. This means it does not need to be provided unnecessarily by the validator client.
Move fn map_attestation_committee in mod beacon_chain::attestation_verification to a new fn with_committee_cache on the BeaconChain so the same cache can be used for obtaining validator duties.
Add some other helpers to BeaconChain to assist with common API duties (e.g., block_root_at_slot, head_beacon_block_root).
Change the NaiveAggregationPool so it can index attestations by hash_tree_root(attestation.data). This is a requirement of the API.
Add functions to BeaconChainHarness to allow it to create slashings and exits.
Allow for eth1::Eth1NetworkId to go to/from a String.
Add functions to the OperationPool to allow getting all objects in the pool.
Add function to BeaconState to check if a committee cache is initialized.
Fix bug where seconds_per_eth1_block was not transferring over from YamlConfig to ChainSpec.
Add the deposit_contract_address to YamlConfig and ChainSpec. We needed to be able to return it in an API response.
Change some uses of serde serialize_with and deserialize_with to a single use of with (code quality).
Impl Display and FromStr for several BLS fields.
Check for clock discrepancy when VC polls BN for sync state (with +/- 1 slot tolerance). This is not intended to be comprehensive, it was just easy to do.

Additional Info

See Implement the standard HTTP for BN #1434 for a per-endpoint overview.
Seeking clarity here: Clarity on /beacon/headers ethereum/beacon-APIs#75

TODO

Blocked On

~~Blocked on [Merged by Bors] - Fix validator lockfiles #1586~~

michaelsproul · 2020-09-03T04:21:49Z

I've been using your QuotedIntVisitor in #1544 and noticed that it only deserializes quoted integers, and errors on unquoted ones. This breaks the YAML parsing in the EF tests, but it can be fixed by adding an implementation of visit_u64:

fn visit_u64<E>(self, v: u64) -> Result<Self::Value, E>
where
    E: serde::de::Error,
{
    Ok(v)
}

For the slashing interchange format I actually need it to error on unquoted ints, so I made a variant only_quoted module that preserves the current behaviour. I was thinking we could have flexible quoting on by default for most of the types in consensus/types, and then opt into stricter behaviour where it's desired? Does that work for you?

## Proposed Changes This is an extraction of the quoted int code from #1569, that I've come to rely on for #1544. It allows us to parse integers from serde strings in YAML, JSON, etc. The main differences from the code in Paul's original PR are: * Added a submodule that makes quoting mandatory (`require_quotes`). * Decoding is generic over the type `T` being decoded. You can use `#[serde(with = "serde_utils::quoted_u64::require_quotes")]` on `Epoch` and `Slot` fields (this is what I do in my slashing protection PR). I've turned on quoting for `Epoch` and `Slot` in this PR, but will leave the other `types` changes to you Paul. I opted to put everything in the `conseus/serde_utils` module so that BLS can use it without a circular dependency. In future when we want to publish `types` I think we could publish `serde_utils` as `lighthouse_serde_utils` or something. Open to other ideas on this front too.

Fixes a breaking change to our API that was unnecessary and can wait until #1569 is merged

paulhauner

Initial self review :)

beacon_node/beacon_chain/src/shuffling_cache.rs

beacon_node/client/src/builder.rs

beacon_node/eth1/src/lib.rs

beacon_node/http_api/src/beacon_proposer_cache.rs

beacon_node/http_api/src/consensus.rs

common/http_api_common/Cargo.toml

common/lighthouse_metrics/src/health.rs

consensus/serde_utils/src/hex.rs

validator_client/src/validator_duty.rs

book/src/advanced_metrics.md

book/src/api-bn.md

guybrush · 2020-09-23T16:36:28Z

Currently some values are numbers which should be strings.

For example in the response of /eth/v1/beacon/states/{stateID}/validators .data[].validator.{effective_balance,activation_eligibilty_epoch,activation_epoch,exit_epoch,withdrawable_epoch} are numbers.

michaelsproul

(submitting early to discuss review-so-far)

beacon_node/beacon_chain/src/beacon_chain.rs

consensus/types/src/shuffling_id.rs

michaelsproul · 2020-09-24T04:23:27Z

beacon_node/beacon_chain/src/shuffling_cache.rs

+        } else if epoch > self.next.shuffling_epoch {
+            let mut shuffling_id = self.next.clone();
+            shuffling_id.shuffling_epoch = epoch;
+            Some(shuffling_id)
+        } else {


This doesn't seem safe to me... if we have the shuffling IDs for the current and next epochs, why can we assume we know the shuffling for future epochs?

I kinda convinced myself that this was OK during our call, but I think it's still not quite right.

Our goal here is to return a shuffling ID that accurately describes the shuffling at a future epoch where no new blocks have been produced since the given head block. The problem is, the shuffling ID for the next epoch (from the perspective of the head), only includes RANDAO information up to the last slot of the previous epoch. The shuffling for subsequent epochs (>next_epoch) is affected by blocks after that point, namely those in the current epoch, including the head block itself. Diagram for clarity:

(big boxes are epochs, little boxes are blocks within them)

I think the correct shuffling ID to return for an epoch after next_epoch would be:

ShufflingId { shuffling_epoch: epoch, shuffling_decision_block: head_block_root, }

Which would require incorporating the head_block_root into the BlockShufflingIds struct.

Yep, you're totally right. Thank you!

michaelsproul · 2020-09-24T04:27:38Z

consensus/types/src/shuffling_id.rs

+        let shuffling_decision_slot =
+            (state.current_epoch() - 1).start_slot(E::slots_per_epoch()) - 1;


Shouldn't this subtract from the shuffling_epoch in question instead of the current epoch? I.e.

(shuffling_epoch - 1).start_slot(E::slots_per_epoch()) - 1

Or even clearer:

(shuffling_epoch - 2).end_slot(E::slots_per_epoch())

We could also replace the 2 by 1 + spec.min_seed_lookahead, to be robust against potential spec updates (unlikely, but possible).

You're right about the shuffling_epoch, thanks!

(shuffling_epoch - 2).end_slot(E::slots_per_epoch())

Unfortunately this doesn't work for cases like state.slot ==1, where you'll end up with a shuffling decision slot of 31.

beacon_node/beacon_chain/src/beacon_chain.rs

paulhauner · 2020-09-29T03:25:19Z

Yay! Thanks all involved! I'll try an optimistic bors and see how it goes!

bors r+

## Issue Addressed - Resolves #1550 - Resolves #824 - Resolves #825 - Resolves #1131 - Resolves #1411 - Resolves #1256 - Resolve #1177 ## Proposed Changes - Includes the `ShufflingId` struct initially defined in #1492. That PR is now closed and the changes are included here, with significant bug fixes. - Implement the https://github.com/ethereum/eth2.0-APIs in a new `http_api` crate using `warp`. This replaces the `rest_api` crate. - Add a new `common/eth2` crate which provides a wrapper around `reqwest`, providing the HTTP client that is used by the validator client and for testing. This replaces the `common/remote_beacon_node` crate. - Create a `http_metrics` crate which is a dedicated server for Prometheus metrics (they are no longer served on the same port as the REST API). We now have flags for `--metrics`, `--metrics-address`, etc. - Allow the `subnet_id` to be an optional parameter for `VerifiedUnaggregatedAttestation::verify`. This means it does not need to be provided unnecessarily by the validator client. - Move `fn map_attestation_committee` in `mod beacon_chain::attestation_verification` to a new `fn with_committee_cache` on the `BeaconChain` so the same cache can be used for obtaining validator duties. - Add some other helpers to `BeaconChain` to assist with common API duties (e.g., `block_root_at_slot`, `head_beacon_block_root`). - Change the `NaiveAggregationPool` so it can index attestations by `hash_tree_root(attestation.data)`. This is a requirement of the API. - Add functions to `BeaconChainHarness` to allow it to create slashings and exits. - Allow for `eth1::Eth1NetworkId` to go to/from a `String`. - Add functions to the `OperationPool` to allow getting all objects in the pool. - Add function to `BeaconState` to check if a committee cache is initialized. - Fix bug where `seconds_per_eth1_block` was not transferring over from `YamlConfig` to `ChainSpec`. - Add the `deposit_contract_address` to `YamlConfig` and `ChainSpec`. We needed to be able to return it in an API response. - Change some uses of serde `serialize_with` and `deserialize_with` to a single use of `with` (code quality). - Impl `Display` and `FromStr` for several BLS fields. - Check for clock discrepancy when VC polls BN for sync state (with +/- 1 slot tolerance). This is not intended to be comprehensive, it was just easy to do. ## Additional Info - See #1434 for a per-endpoint overview. - Seeking clarity here: ethereum/beacon-APIs#75 ## TODO - [x] Add docs for prom port to close #1256 - [x] Follow up on this #1177 - [x] ~~Follow up with #1424~~ Will fix in future PR. - [x] Follow up with #1411 - [x] ~~Follow up with #1260~~ Will fix in future PR. - [x] Add quotes to all integers. - [x] Remove `rest_types` - [x] Address missing beacon block error. (#1629) - [x] ~~Add tests for lighthouse/peers endpoints~~ Wontfix - [x] ~~Follow up with validator status proposal~~ Tracked in #1434 - [x] Unify graffiti structs - [x] ~~Start server when waiting for genesis?~~ Will fix in future PR. - [x] TODO in http_api tests - [x] Move lighthouse endpoints off /eth/v1 - [x] Update docs to link to standard ## Blocked On - ~~Blocked on #1586~~ Co-authored-by: Michael Sproul <[email protected]>

bors · 2020-09-29T04:55:15Z

Pull request successfully merged into v0.3.0-staging.

Build succeeded:

- Resolves #1550 - Resolves #824 - Resolves #825 - Resolves #1131 - Resolves #1411 - Resolves #1256 - Resolve #1177 - Includes the `ShufflingId` struct initially defined in #1492. That PR is now closed and the changes are included here, with significant bug fixes. - Implement the https://github.com/ethereum/eth2.0-APIs in a new `http_api` crate using `warp`. This replaces the `rest_api` crate. - Add a new `common/eth2` crate which provides a wrapper around `reqwest`, providing the HTTP client that is used by the validator client and for testing. This replaces the `common/remote_beacon_node` crate. - Create a `http_metrics` crate which is a dedicated server for Prometheus metrics (they are no longer served on the same port as the REST API). We now have flags for `--metrics`, `--metrics-address`, etc. - Allow the `subnet_id` to be an optional parameter for `VerifiedUnaggregatedAttestation::verify`. This means it does not need to be provided unnecessarily by the validator client. - Move `fn map_attestation_committee` in `mod beacon_chain::attestation_verification` to a new `fn with_committee_cache` on the `BeaconChain` so the same cache can be used for obtaining validator duties. - Add some other helpers to `BeaconChain` to assist with common API duties (e.g., `block_root_at_slot`, `head_beacon_block_root`). - Change the `NaiveAggregationPool` so it can index attestations by `hash_tree_root(attestation.data)`. This is a requirement of the API. - Add functions to `BeaconChainHarness` to allow it to create slashings and exits. - Allow for `eth1::Eth1NetworkId` to go to/from a `String`. - Add functions to the `OperationPool` to allow getting all objects in the pool. - Add function to `BeaconState` to check if a committee cache is initialized. - Fix bug where `seconds_per_eth1_block` was not transferring over from `YamlConfig` to `ChainSpec`. - Add the `deposit_contract_address` to `YamlConfig` and `ChainSpec`. We needed to be able to return it in an API response. - Change some uses of serde `serialize_with` and `deserialize_with` to a single use of `with` (code quality). - Impl `Display` and `FromStr` for several BLS fields. - Check for clock discrepancy when VC polls BN for sync state (with +/- 1 slot tolerance). This is not intended to be comprehensive, it was just easy to do. - See #1434 for a per-endpoint overview. - Seeking clarity here: ethereum/beacon-APIs#75 - [x] Add docs for prom port to close #1256 - [x] Follow up on this #1177 - [x] ~~Follow up with #1424~~ Will fix in future PR. - [x] Follow up with #1411 - [x] ~~Follow up with #1260~~ Will fix in future PR. - [x] Add quotes to all integers. - [x] Remove `rest_types` - [x] Address missing beacon block error. (#1629) - [x] ~~Add tests for lighthouse/peers endpoints~~ Wontfix - [x] ~~Follow up with validator status proposal~~ Tracked in #1434 - [x] Unify graffiti structs - [x] ~~Start server when waiting for genesis?~~ Will fix in future PR. - [x] TODO in http_api tests - [x] Move lighthouse endpoints off /eth/v1 - [x] Update docs to link to standard - ~~Blocked on #1586~~ Co-authored-by: Michael Sproul <[email protected]>

paulhauner added the work-in-progress PR is a work-in-progress label Aug 26, 2020

paulhauner changed the title ~~Standard standard eth2.0 API~~ Implement standard eth2.0 API Aug 31, 2020

paulhauner mentioned this pull request Sep 4, 2020

HTTP API for validator client #1347

Closed

paulhauner added the v0.3.0 For inclusion in v0.3.0 label Sep 4, 2020

paulhauner mentioned this pull request Sep 4, 2020

Tracking: October security review #1436

Closed

8 tasks

michaelsproul mentioned this pull request Sep 5, 2020

[Merged by Bors] - Add serde_utils module with quoted u64 support #1588

Closed

paulhauner force-pushed the standard-http branch from 56aa87c to 2e8941d Compare September 5, 2020 06:55

michaelsproul mentioned this pull request Sep 8, 2020

[Merged by Bors] - Don't quote slot and epoch, for now #1597

Closed

bors bot pushed a commit that referenced this pull request Sep 8, 2020

Don't quote slot and epoch, for now (#1597)

19be7ab

Fixes a breaking change to our API that was unnecessary and can wait until #1569 is merged

This was referenced Sep 14, 2020

Separate metrics server and API server (different ports) #1411

Closed

BN failover for VC #1619

Closed

paulhauner added the blocked label Sep 16, 2020

paulhauner commented Sep 16, 2020

View reviewed changes

paulhauner mentioned this pull request Sep 21, 2020

Implement the standard HTTP for BN #1434

Closed

39 tasks

paulhauner commented Sep 21, 2020

View reviewed changes

book/src/advanced_metrics.md Outdated Show resolved Hide resolved

paulhauner commented Sep 21, 2020

View reviewed changes

book/src/api-bn.md Show resolved Hide resolved

paulhauner commented Sep 21, 2020

View reviewed changes

book/src/api-bn.md Outdated Show resolved Hide resolved

This was referenced Sep 22, 2020

Remove ValidatorDuty shim #1643

Closed

Missing integer quoting for API #1650

Closed

Swap to ShufflingId for keying shuffling cache #1492

Closed

paulhauner added the A0 label Sep 23, 2020

paulhauner mentioned this pull request Sep 23, 2020

Add a CLI command for exiting validators #1652

Closed

paulhauner changed the base branch from master to v0.3.0-staging September 23, 2020 01:29

paulhauner force-pushed the standard-http branch 2 times, most recently from 47d09a0 to 3291d91 Compare September 23, 2020 03:21

paulhauner marked this pull request as ready for review September 23, 2020 06:49

michaelsproul reviewed Sep 24, 2020

View reviewed changes

paulhauner and others added 14 commits September 29, 2020 13:16

Fix clippy lint

29fd583

Fix safe arith violation

ad9b774

Fix shuffling id error

f143b69

Address review comments

5b8dbde

Fix deposit contract address issue

077be72

Address docs, aggregation pool comments

076c3f8

Change beacon proposer cache key

d535fc3

Disallow some endpoints whilst syncing

d8c31e0

Pass shuffling id to fork choice on init

ac0ce72

Fix clippy lint

31bb71d

Fix arith error

1c99f59

Simplify serde_utils for hex ints

0c24493

Minor fixes

04f08a2

Fix warning from merge

4458012

paulhauner force-pushed the standard-http branch from 31b0ac1 to 4458012 Compare September 29, 2020 03:24

bors bot changed the title ~~Implement standard eth2.0 API~~ [Merged by Bors] - Implement standard eth2.0 API Sep 29, 2020

bors bot closed this Sep 29, 2020

paulhauner deleted the standard-http branch October 5, 2020 08:27

guybrush mentioned this pull request Oct 12, 2020

Feature/api v1 gobitfly/eth2-beaconchain-explorer#292

Closed

ackintosh added a commit to ackintosh/lighthouse that referenced this pull request Jan 13, 2022

/beacon/validators/all has been removed in sigp#1569

79d8c76

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Merged by Bors] - Implement standard eth2.0 API #1569

[Merged by Bors] - Implement standard eth2.0 API #1569

paulhauner commented Aug 26, 2020 •

edited

Loading

michaelsproul commented Sep 3, 2020 •

edited

Loading

paulhauner left a comment

guybrush commented Sep 23, 2020

michaelsproul left a comment

michaelsproul Sep 24, 2020

michaelsproul Sep 24, 2020 •

edited

Loading

paulhauner Sep 28, 2020

michaelsproul Sep 24, 2020

paulhauner Sep 28, 2020

michaelsproul Sep 28, 2020

paulhauner commented Sep 29, 2020

bors bot commented Sep 29, 2020

		let shuffling_decision_slot =
		(state.current_epoch() - 1).start_slot(E::slots_per_epoch()) - 1;

[Merged by Bors] - Implement standard eth2.0 API #1569

[Merged by Bors] - Implement standard eth2.0 API #1569

Conversation

paulhauner commented Aug 26, 2020 • edited Loading

Issue Addressed

Proposed Changes

Additional Info

TODO

Blocked On

michaelsproul commented Sep 3, 2020 • edited Loading

paulhauner left a comment

Choose a reason for hiding this comment

guybrush commented Sep 23, 2020

michaelsproul left a comment

Choose a reason for hiding this comment

michaelsproul Sep 24, 2020

Choose a reason for hiding this comment

michaelsproul Sep 24, 2020 • edited Loading

Choose a reason for hiding this comment

paulhauner Sep 28, 2020

Choose a reason for hiding this comment

michaelsproul Sep 24, 2020

Choose a reason for hiding this comment

paulhauner Sep 28, 2020

Choose a reason for hiding this comment

michaelsproul Sep 28, 2020

Choose a reason for hiding this comment

paulhauner commented Sep 29, 2020

bors bot commented Sep 29, 2020

paulhauner commented Aug 26, 2020 •

edited

Loading

michaelsproul commented Sep 3, 2020 •

edited

Loading

michaelsproul Sep 24, 2020 •

edited

Loading