Staging: v0.3.0 #1671
Merged
Staging: v0.3.0 #1671
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
55107d0 to
29709c5
Compare
## Issue Addressed #1624 ## Proposed Changes Updates to match [EIP-2333](`https://eips.ethereum.org/EIPS/eip-2333`) ## Additional Info In order to have compatibility with the eth2.0-deposit-cli, [this PR](ethereum/staking-deposit-cli#108) must also be merged
## Issue Addressed - Resolves #1313 ## Proposed Changes Changes the way we start the validator client and beacon node to ensure that we cleanly drop the validator keystores (which therefore ensures we cleanup their lockfiles). Previously we were holding the validator keystores in a tokio task that was being forcefully killed (i.e., without `Drop`). Now, we hold them in a task that can gracefully handle a shutdown. Also, switches the `--strict-lockfiles` flag to `--delete-lockfiles`. This means two things: 1. We are now strict on lockfiles by default (before we weren't). 1. There's a simple way for people delete the lockfiles if they experience a crash. ## Additional Info I've only given the option to ignore *and* delete lockfiles, not just ignore them. I can't see a strong need for ignore-only but could easily add it, if the need arises. I've flagged this as `api-breaking` since users that have lockfiles lingering around will be required to supply `--delete-lockfiles` next time they run.
Closes #1487 Closes #1427 Directory restructure in accordance with #1487. Also has temporary migration code to move the old directories into new structure. Also extracts all default directory names and utility functions into a `directory` crate to avoid repetitio. ~Since `validator_definition.yaml` stores absolute paths, users will have to manually change the keystore paths or delete the file to get the validators picked up by the vc.~. `validator_definition.yaml` is migrated as well from the default directories. Co-authored-by: realbigsean <seananderson33@gmail.com> Co-authored-by: Paul Hauner <paul@paulhauner.com>
- 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 <michael@sigmaprime.io>
## Issue Addressed Closes #673 ## Proposed Changes Store a schema version in the database so that future releases can check they're running against a compatible database version. This would also enable automatic migration on breaking database changes, but that's left as future work. The database config is also stored in the database so that the `slots_per_restore_point` value can be checked for consistency, which closes #673
1ad28c0 to
22aedda
Compare
## Issue Addressed Solution 2 proposed here: #1435 (comment) ## Proposed Changes - Adds an optional `--wss-checkpoint` flag that takes a string `root:epoch` - Verify that the given checkpoint exists in the chain, or that the the chain syncs through this checkpoint. If not, shutdown and prompt the user to purge state before restarting. ## Additional Info Co-authored-by: Paul Hauner <paul@paulhauner.com>
## Issue Addressed Implements support for importing and exporting the slashing protection DB interchange format described here: https://hackmd.io/@sproul/Bk0Y0qdGD Also closes #1584 ## Proposed Changes * [x] Support for serializing and deserializing the format * [x] Support for importing and exporting Lighthouse's database * [x] CLI commands to invoke import and export * [x] Export to minimal format (required when a minimal format has been previously imported) * [x] Tests for export to minimal (utilising mixed importing and attestation signing?) * [x] Tests for import/export of complete format, and import of minimal format * [x] ~~Prevent attestations with sources less than our max source (Danny's suggestion). Required for the fake attestation that we put in for the minimal format to block attestations from source 0.~~ * [x] Add the concept of a "low watermark" for compatibility with the minimal format Bonus! * [x] A fix to a potentially nasty bug involving validators getting re-registered each time the validator client ran! Thankfully, the ordering of keys meant that the validator IDs used for attestations and blocks remained stable -- otherwise we could have had some slashings on our hands! 😱 * [x] Tests to confirm that this bug is indeed vanquished
## Issue Addressed Resolves #1130 ## Proposed Changes Use the sigp fork of tiny-bip39, which includes `Zeroize` for `Mnemonic` and `Seed` ## Additional Info N/A
…ds. (#1697) ## Issue Addressed Fixes #1665. ## Proposed Changes `lighthouse account_manager wallet create` now generates a 24-word mnemonic. The user can override this by passing `--mnemonic-length 12` (or another legal bip39 length). ## Additional Info CLI `--help`: ``` --mnemonic-length <MNEMONIC_LENGTH> The number of words to use for the mnemonic phrase. [default: 24] ``` In case of an invalid argument: ``` % lighthouse account_manager wallet create --mnemonic-length 25 error: Invalid value for '--mnemonic-length <MNEMONIC_LENGTH>': Mnemonic length must be one of 12, 15, 18, 21, 24 ```
Adding UPnP support will help grow the DHT by allowing NAT traversal for peers with UPnP supported routers. ## Issue Addressed #927 ## Proposed Changes Using IGD library: https://docs.rs/igd/0.10.0/igd/ Adding the the libp2p tcp port and discovery udp port. If this fails it simply logs the attempt and moves on ## Additional Info Co-authored-by: Age Manning <Age@AgeManning.com>
## Issue Addressed
NA
## Proposed Changes
- Implements a HTTP API for the validator client.
- Creates EIP-2335 keystores with an empty `description` field, instead of a missing `description` field. Adds option to set name.
- Be more graceful with setups without any validators (yet)
- Remove an error log when there are no validators.
- Create the `validator` dir if it doesn't exist.
- Allow building a `ValidatorDir` without a withdrawal keystore (required for the API method where we only post a voting keystore).
- Add optional `description` field to `validator_definitions.yml`
## TODO
- [x] Signature header, as per #1269 (comment)
- [x] Return validator descriptions
- [x] Return deposit data
- [x] Respect the mnemonic offset
- [x] Check that mnemonic can derive returned keys
- [x] Be strict about non-localhost
- [x] Allow graceful start without any validators (+ create validator dir)
- [x] Docs final pass
- [x] Swap to EIP-2335 description field.
- [x] Fix Zerioze TODO in VC api types.
- [x] Zeroize secp256k1 key
## Endpoints
- [x] `GET /lighthouse/version`
- [x] `GET /lighthouse/health`
- [x] `GET /lighthouse/validators`
- [x] `POST /lighthouse/validators/hd`
- [x] `POST /lighthouse/validators/keystore`
- [x] `PATCH /lighthouse/validators/:validator_pubkey`
- [ ] ~~`POST /lighthouse/validators/:validator_pubkey/exit/:epoch`~~ Future works
## Additional Info
TBC
## Issue Addressed NA ## Proposed Changes Addresses an interesting DoS vector raised by @protolambda by verifying that the head and target are consistent when processing aggregate attestations. This check prevents us from loading very old target blocks and doing lots of work to skip them to the current slot. ## Additional Info NA
This commit was edited by Paul H when rebasing from master to v0.3.0-staging. Solution 2 proposed here: #1435 (comment) - Adds an optional `--wss-checkpoint` flag that takes a string `root:epoch` - Verify that the given checkpoint exists in the chain, or that the the chain syncs through this checkpoint. If not, shutdown and prompt the user to purge state before restarting. Co-authored-by: Paul Hauner <paul@paulhauner.com>
## Issue Addressed Resolves #1130 ## Proposed Changes Use the sigp fork of tiny-bip39, which includes `Zeroize` for `Mnemonic` and `Seed` ## Additional Info N/A
…ds. (#1697) ## Issue Addressed Fixes #1665. ## Proposed Changes `lighthouse account_manager wallet create` now generates a 24-word mnemonic. The user can override this by passing `--mnemonic-length 12` (or another legal bip39 length). ## Additional Info CLI `--help`: ``` --mnemonic-length <MNEMONIC_LENGTH> The number of words to use for the mnemonic phrase. [default: 24] ``` In case of an invalid argument: ``` % lighthouse account_manager wallet create --mnemonic-length 25 error: Invalid value for '--mnemonic-length <MNEMONIC_LENGTH>': Mnemonic length must be one of 12, 15, 18, 21, 24 ```
This commit was modified by Paul H whilst rebasing master onto v0.3.0-staging Adding UPnP support will help grow the DHT by allowing NAT traversal for peers with UPnP supported routers. Using IGD library: https://docs.rs/igd/0.10.0/igd/ Adding the the libp2p tcp port and discovery udp port. If this fails it simply logs the attempt and moves on Co-authored-by: Age Manning <Age@AgeManning.com>
## Issue Addressed NA ## Proposed Changes Addresses an interesting DoS vector raised by @protolambda by verifying that the head and target are consistent when processing aggregate attestations. This check prevents us from loading very old target blocks and doing lots of work to skip them to the current slot. ## Additional Info NA
## Issue Addressed NA ## Proposed Changes Uses a `Drop` implementation to help ensure that `BeaconProcessor` workers are freed. This will help prevent against regression, if someone happens to add an early return and it will also help in the case of a panic. ## Additional Info NA
## Issue Addressed couple of TODOs
## Issue Addressed chain state inconsistencies ## Proposed Changes - a batch can be fake-failed by Range if it needs to move a peer to another chain. The peer will still send blocks/ errors / produce timeouts for those requests, so check when we get a response from the RPC that the request id matches, instead of only the peer, since a re-request can be directed to the same peer. - if an optimistic batch succeeds, store the attempt to avoid trying it again when quickly switching chains. Also, use it only if ahead of our current target, instead of the segment's start epoch
## Issue Addressed N/A ## Proposed Changes Increase gossipsub's content-id length to the full 32 byte hash. ## Additional Info N/A
## Issue Addressed - Resolves #1705 ## Proposed Changes Cleans up some of my TODOs in the code base. - Adds link to issue in this repo for BLST `unsafe` block. - Confirms that the `nextaccount` field *is* required on an EIP-2386 wallet. - Reference: https://github.com/mcdee/EIPs/blob/master/EIPS/eip-2386.md#json-schema - Removes TODO about Zeroize on bip39 that was resolved in #1701 - Removes a TODO about an early randao reveal since we use the slot clock to generate the reveal: https://github.com/sigp/lighthouse/blob/c4bd9c86e6bf23dd66bde150ef85e08caa5e4826/validator_client/src/block_service.rs#L212-L220 ## Additional Info NA
## Issue Addressed Downgrade inconsistent chain segment states from `panic` to `crit`. I don't love this solution but since range can always bounce back from any of those, we don't panic. Co-authored-by: Age Manning <Age@AgeManning.com>
## Issue Addressed N/A ## Proposed Changes Updates the libp2p dependency to the latest version ## Additional Info N/A
* Improve error handling in network processing * Cargo fmt * Cargo fmt * Improve error handling for prior genesis * Remove dep
## Issue Addressed NA ## Proposed Changes Uses a `Drop` implementation to help ensure that `BeaconProcessor` workers are freed. This will help prevent against regression, if someone happens to add an early return and it will also help in the case of a panic. ## Additional Info NA
## Issue Addressed chain state inconsistencies ## Proposed Changes - a batch can be fake-failed by Range if it needs to move a peer to another chain. The peer will still send blocks/ errors / produce timeouts for those requests, so check when we get a response from the RPC that the request id matches, instead of only the peer, since a re-request can be directed to the same peer. - if an optimistic batch succeeds, store the attempt to avoid trying it again when quickly switching chains. Also, use it only if ahead of our current target, instead of the segment's start epoch
## Issue Addressed N/A ## Proposed Changes Increase gossipsub's content-id length to the full 32 byte hash. ## Additional Info N/A
## Issue Addressed Downgrade inconsistent chain segment states from `panic` to `crit`. I don't love this solution but since range can always bounce back from any of those, we don't panic. Co-authored-by: Age Manning <Age@AgeManning.com>
* Improve error handling in network processing * Cargo fmt * Cargo fmt * Improve error handling for prior genesis * Remove dep
* Initial rebase * Remove old code * Correct release tests * Rebase commit * Remove eth2-testnet dep on eth2libp2p * Remove crates lost in rebase * Remove unused dep
Squashed commit of the following: commit f99373c Author: Age Manning <Age@AgeManning.com> Date: Mon Oct 5 18:44:09 2020 +1100 Clean up obsolute TODOs
…ient (#1695) ## Issue Addressed #1618 ## Proposed Changes Adds an encrypted key cache that is loaded on validator_client startup. It stores the keypairs for all enabled keystores and uses as password the concatenation the passwords of all enabled keystores. This reduces the number of time intensive key derivitions for `N` validators from `N` to `1`. On changes the cache gets updated asynchronously to avoid blocking the main thread. ## Additional Info If the cache contains the keypair of a key 9E12 store that is not in the validator_definitions.yml file during loading the cache cannot get decrypted. In this case all the keystores get decrypted and then the cache gets overwritten. To avoid that one can disable keystores in validator_definitions.yml and restart the client which will remove them from the cache, after that one can entirely remove the keystore (from the validator_definitions.yml and from the disk). Other solutions to the above "problem" might be: * Add a CLI and/or API function for removing keystores which will update the cache (asynchronously). * Add a CLI and/or API function that just updates the cache (asynchronously) after a modification of the `validator_definitions.yml` file. Note that the cache file has a lock file which gets removed immediatly after the cache was used or updated.
## Issue Addressed Fix the lockfile after it was broken by the manual merge of #1654
A little help for the future generations.
## Issue Addressed - Resolves #1722 ## Proposed Changes This extends @danielschonfeld's work in #1739 with: - Use an empty boot node list - Remove the genesis state ## Additional Info NA Co-authored-by: Daniel Schonfeld <daniel@schonfeld.org>
## Issue Addressed Resolves #1744 ## Proposed Changes - Add `directory::ensure_dir_exists` to the `ValidatorDefinition::open_or_create` method - As @pawanjay176 suggested, making the `--validator-dir` non-global so users are forced to include the flag after the `validator` subcommand. Current behavior seems to be ignoring the flag if it comes after something like `validator import` ## Additional Info N/A
## Issue Addressed
NA
## Proposed Changes
- Remove Metamask deposits from the docs.
- Restructure docs to be launchpad-centric.
- Remove references to sigp/lighthouse-docker.
- Add section about binaries.
## Additional Info
Please provide any additional information. For example, future considerations
or information useful for reviewers.
## Issue Addressed NA ## Proposed Changes - Bump version to v0.3.0 - Run `cargo update` ## Additional Info NA
## Proposed Changes Replace `--strict-slashing-protection` by `--init-slashing-protection` and remove mentions of `--auto-register`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This branch (
v0.3.0-staging) is being used to shield the breaking changes from the master branch (which is currentlyv0.2.x).API breaking PRs are being merged into this branch. We will then merge/fast-foward this PR onto
masterwhen we're ready to releasev0.3.0.Notes