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

[Merged by Bors] - Fix timing issue in obtaining the Fork #2158

Closed

Conversation

ackintosh
Copy link
Member

@ackintosh ackintosh commented Jan 17, 2021

Issue Addressed

Related PR: #2137 (comment)

The Fork is required for VC to perform signing. Currently, it is not guaranteed that the Fork has been obtained at the point of the signing as the Fork is obtained at after ForkService starts. We will see the error if VC could not perform the signing due to the timing issue.

Unable to get Fork for signing

Proposed Changes

Obtain the Fork on init_from_beacon_node to fix the timing issue.

because the Fork is required that VC perform signing.
ref sigp#2137 (comment)
@ackintosh
Copy link
Member Author

ackintosh commented Jan 17, 2021

Because of this fix, fork_service.fork will no longer to be optional. I believe removing Option from fork_service.fork is good for strictness.

error: using `Option.and_then(|x| Some(y))`, which is more succinctly expressed as `map(|x| y)`
@ackintosh
Copy link
Member Author

ackintosh commented Jan 17, 2021

The clippy error below will be fixed by #2156 👀

https://github.com/sigp/lighthouse/pull/2158/checks?check_run_id=1716046878

error: explicit lifetimes given in parameter types where they could be elided (or replaced with '_ if needed by type declaration)
--> consensus/ssz_derive/src/lib.rs:17:1
|
17 | / fn get_serializable_named_field_idents<'a>(
18 | | struct_data: &'a syn::DataStruct,
19 | | ) -> Vec<&'a syn::Ident> {
| |________________________^
|
= note: -D clippy::needless-lifetimes implied by -D warnings
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes

error: explicit lifetimes given in parameter types where they could be elided (or replaced with '_ if needed by type declaration)
--> consensus/ssz_derive/src/lib.rs:38:1
|
38 | fn get_serializable_field_types<'a>(struct_data: &'a syn::DataStruct) -> Vec<&'a syn::Type> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes

error: aborting due to 2 previous errors

@ackintosh
Copy link
Member Author

I've verified that VC performs get_beacon_states_fork until the BC becomes online.

log
Jan 17 19:59:35.486 INFO Initialized validators                  enabled: 1, disabled: 0
Jan 17 19:59:35.507 INFO Connected to beacon node                endpoint: http://localhost:5052/, version: Lighthouse/v1.0.6-bfb5ed1+/x86_64-macos
Jan 17 19:59:35.510 INFO Initialized beacon node connections     available: 1, total: 1
Jan 17 19:59:45.525 ERRO Failed to get fork                      error: All endpoints failed http://localhost:5052/ => RequestFailed(Reqwest(reqwest::Error { kind: Request, url: Url { scheme: "http", host: Some(Domain("localhost")), port: Some(5052), path: "/eth/v1/beacon/states/head/fork", query: None, fragment: None }, source: hyper::Error(Connect, ConnectError("tcp connect error", Os { code: 61, kind: ConnectionRefused, message: "Connection refused" })) }))
Jan 17 19:59:47.535 WARN Offline beacon node                     endpoint: http://localhost:5052/, error: Reqwest(reqwest::Error { kind: Request, url: Url { scheme: "http", host: Some(Domain("localhost")), port: Some(5052), path: "/eth/v1/node/version", query: None, fragment: None }, source: hyper::Error(Connect, ConnectError("tcp connect error", Os { code: 61, kind: ConnectionRefused, message: "Connection refused" })) })
Jan 17 19:59:47.536 ERRO Failed to get fork                      error: All endpoints failed http://localhost:5052/ => Unavailable(Offline), http://localhost:5052/ => Unavailable(Offline)
Jan 17 19:59:49.539 WARN Offline beacon node                     endpoint: http://localhost:5052/, error: Reqwest(reqwest::Error { kind: Request, url: Url { scheme: "http", host: Some(Domain("localhost")), port: Some(5052), path: "/eth/v1/node/version", query: None, fragment: None }, source: hyper::Error(Connect, ConnectError("tcp connect error", Os { code: 61, kind: ConnectionRefused, message: "Connection refused" })) })
Jan 17 19:59:49.540 ERRO Failed to get fork                      error: All endpoints failed http://localhost:5052/ => Unavailable(Offline), http://localhost:5052/ => Unavailable(Offline)
Jan 17 19:59:51.546 WARN Offline beacon node                     endpoint: http://localhost:5052/, error: Reqwest(reqwest::Error { kind: Request, url: Url { scheme: "http", host: Some(Domain("localhost")), port: Some(5052), path: "/eth/v1/node/version", query: None, fragment: None }, source: hyper::Error(Connect, ConnectError("tcp connect error", Os { code: 61, kind: ConnectionRefused, message: "Connection refused" })) })
Jan 17 19:59:51.546 ERRO Failed to get fork                      error: All endpoints failed http://localhost:5052/ => Unavailable(Offline), http://localhost:5052/ => Unavailable(Offline)
Jan 17 19:59:53.552 WARN Offline beacon node                     endpoint: http://localhost:5052/, error: Reqwest(reqwest::Error { kind: Request, url: Url { scheme: "http", host: Some(Domain("localhost")), port: Some(5052), path: "/eth/v1/node/version", query: None, fragment: None }, source: hyper::Error(Connect, ConnectError("tcp connect error", Os { code: 61, kind: ConnectionRefused, message: "Connection refused" })) })
Jan 17 19:59:53.553 ERRO Failed to get fork                      error: All endpoints failed http://localhost:5052/ => Unavailable(Offline), http://localhost:5052/ => Unavailable(Offline)
Jan 17 19:59:55.558 WARN Offline beacon node                     endpoint: http://localhost:5052/, error: Reqwest(reqwest::Error { kind: Request, url: Url { scheme: "http", host: Some(Domain("localhost")), port: Some(5052), path: "/eth/v1/node/version", query: None, fragment: None }, source: hyper::Error(Connect, ConnectError("tcp connect error", Os { code: 61, kind: ConnectionRefused, message: "Connection refused" })) })
Jan 17 19:59:55.558 ERRO Failed to get fork                      error: All endpoints failed http://localhost:5052/ => Unavailable(Offline), http://localhost:5052/ => Unavailable(Offline)
Jan 17 19:59:57.567 WARN Offline beacon node                     endpoint: http://localhost:5052/, error: Reqwest(reqwest::Error { kind: Request, url: Url { scheme: "http", host: Some(Domain("localhost")), port: Some(5052), path: "/eth/v1/node/version", query: None, fragment: None }, source: hyper::Error(Connect, ConnectError("tcp connect error", Os { code: 61, kind: ConnectionRefused, message: "Connection refused" })) })
Jan 17 19:59:57.567 ERRO Failed to get fork                      error: All endpoints failed http://localhost:5052/ => Unavailable(Offline), http://localhost:5052/ => Unavailable(Offline)
Jan 17 19:59:59.574 WARN Offline beacon node                     endpoint: http://localhost:5052/, error: Reqwest(reqwest::Error { kind: Request, url: Url { scheme: "http", host: Some(Domain("localhost")), port: Some(5052), path: "/eth/v1/node/version", query: None, fragment: None }, source: hyper::Error(Connect, ConnectError("tcp connect error", Os { code: 61, kind: ConnectionRefused, message: "Connection refused" })) })
Jan 17 19:59:59.574 ERRO Failed to get fork                      error: All endpoints failed http://localhost:5052/ => Unavailable(Offline), http://localhost:5052/ => Unavailable(Offline)
Jan 17 20:00:01.579 WARN Offline beacon node                     endpoint: http://localhost:5052/, error: Reqwest(reqwest::Error { kind: Request, url: Url { scheme: "http", host: Some(Domain("localhost")), port: Some(5052), path: "/eth/v1/node/version", query: None, fragment: None }, source: hyper::Error(Connect, ConnectError("tcp connect error", Os { code: 61, kind: ConnectionRefused, message: "Connection refused" })) })
Jan 17 20:00:01.579 ERRO Failed to get fork                      error: All endpoints failed http://localhost:5052/ => Unavailable(Offline), http://localhost:5052/ => Unavailable(Offline)
Jan 17 20:00:03.642 WARN Offline beacon node                     endpoint: http://localhost:5052/, error: Reqwest(reqwest::Error { kind: Request, url: Url { scheme: "http", host: Some(Domain("localhost")), port: Some(5052), path: "/eth/v1/node/version", query: None, fragment: None }, source: hyper::Error(Connect, ConnectError("tcp connect error", Os { code: 61, kind: ConnectionRefused, message: "Connection refused" })) })
Jan 17 20:00:03.642 ERRO Failed to get fork                      error: All endpoints failed http://localhost:5052/ => Unavailable(Offline), http://localhost:5052/ => Unavailable(Offline)
Jan 17 20:00:05.650 WARN Offline beacon node                     endpoint: http://localhost:5052/, error: Reqwest(reqwest::Error { kind: Request, url: Url { scheme: "http", host: Some(Domain("localhost")), port: Some(5052), path: "/eth/v1/node/version", query: None, fragment: None }, source: hyper::Error(Connect, ConnectError("tcp connect error", Os { code: 61, kind: ConnectionRefused, message: "Connection refused" })) })
Jan 17 20:00:05.650 ERRO Failed to get fork                      error: All endpoints failed http://localhost:5052/ => Unavailable(Offline), http://localhost:5052/ => Unavailable(Offline)
Jan 17 20:00:07.656 WARN Offline beacon node                     endpoint: http://localhost:5052/, error: Reqwest(reqwest::Error { kind: Request, url: Url { scheme: "http", host: Some(Domain("localhost")), port: Some(5052), path: "/eth/v1/node/version", query: None, fragment: None }, source: hyper::Error(Connect, ConnectError("tcp connect error", Os { code: 61, kind: ConnectionRefused, message: "Connection refused" })) })
Jan 17 20:00:07.656 ERRO Failed to get fork                      error: All endpoints failed http://localhost:5052/ => Unavailable(Offline), http://localhost:5052/ => Unavailable(Offline)
Jan 17 20:00:09.660 WARN Offline beacon node                     endpoint: http://localhost:5052/, error: Reqwest(reqwest::Error { kind: Request, url: Url { scheme: "http", host: Some(Domain("localhost")), port: Some(5052), path: "/eth/v1/node/version", query: None, fragment: None }, source: hyper::Error(Connect, ConnectError("tcp connect error", Os { code: 61, kind: ConnectionRefused, message: "Connection refused" })) })
Jan 17 20:00:09.660 ERRO Failed to get fork                      error: All endpoints failed http://localhost:5052/ => Unavailable(Offline), http://localhost:5052/ => Unavailable(Offline)
Jan 17 20:00:11.665 WARN Offline beacon node                     endpoint: http://localhost:5052/, error: Reqwest(reqwest::Error { kind: Request, url: Url { scheme: "http", host: Some(Domain("localhost")), port: Some(5052), path: "/eth/v1/node/version", query: None, fragment: None }, source: hyper::Error(Connect, ConnectError("tcp connect error", Os { code: 61, kind: ConnectionRefused, message: "Connection refused" })) })
Jan 17 20:00:11.665 ERRO Failed to get fork                      error: All endpoints failed http://localhost:5052/ => Unavailable(Offline), http://localhost:5052/ => Unavailable(Offline)
Jan 17 20:00:13.682 WARN Offline beacon node                     endpoint: http://localhost:5052/, error: Reqwest(reqwest::Error { kind: Request, url: Url { scheme: "http", host: Some(Domain("localhost")), port: Some(5052), path: "/eth/v1/node/version", query: None, fragment: None }, source: hyper::Error(Connect, ConnectError("tcp connect error", Os { code: 61, kind: ConnectionRefused, message: "Connection refused" })) })
Jan 17 20:00:13.682 ERRO Failed to get fork                      error: All endpoints failed http://localhost:5052/ => Unavailable(Offline), http://localhost:5052/ => Unavailable(Offline)
Jan 17 20:00:15.685 WARN Offline beacon node                     endpoint: http://localhost:5052/, error: Reqwest(reqwest::Error { kind: Request, url: Url { scheme: "http", host: Some(Domain("localhost")), port: Some(5052), path: "/eth/v1/node/version", query: None, fragment: None }, source: hyper::Error(Connect, ConnectError("tcp connect error", Os { code: 61, kind: ConnectionRefused, message: "Connection refused" })) })
Jan 17 20:00:15.686 ERRO Failed to get fork                      error: All endpoints failed http://localhost:5052/ => Unavailable(Offline), http://localhost:5052/ => Unavailable(Offline)
Jan 17 20:00:17.693 INFO Connected to beacon node                endpoint: http://localhost:5052/, version: Lighthouse/v1.0.6-bfb5ed1+/x86_64-macos
Jan 17 20:00:17.697 INFO Loaded validator keypair store          voting_validators: 1
Jan 17 20:00:17.698 INFO Genesis has already occurred            seconds_ago: 5180410
Jan 17 20:00:17.699 INFO Block production service started        service: block
Jan 17 20:00:17.699 INFO Attestation production service started  next_update_millis: 1300, service: attestation

@ackintosh ackintosh marked this pull request as ready for review January 17, 2021 11:08
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 one, thank you!

I think bors will work because we have fixes to clippy issues in unstable now.

bors r+

@paulhauner paulhauner added the ready-for-merge This PR is ready to merge. label Jan 19, 2021
@paulhauner paulhauner self-assigned this Jan 19, 2021
bors bot pushed a commit that referenced this pull request Jan 19, 2021
## Issue Addressed

Related PR: #2137 (comment)

The Fork is required for VC to perform signing. Currently, it is not guaranteed that the Fork has been obtained at the point of the signing as the Fork is obtained at after ForkService starts. We will see the [error](https://github.com/sigp/lighthouse/blob/851a4dca3c5a514455006589b1a756a8a994258a/validator_client/src/validator_store.rs#L127) if VC could not perform the signing due to the timing issue.

> Unable to get Fork for signing

## Proposed Changes

Obtain the Fork on `init_from_beacon_node` to fix the timing issue.
@bors bors bot changed the title Fix timing issue in obtaining the Fork [Merged by Bors] - Fix timing issue in obtaining the Fork Jan 19, 2021
@bors bors bot closed this Jan 19, 2021
@ackintosh ackintosh deleted the obtain-fork-on-init-from-beacon-node branch May 5, 2024 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge This PR is ready to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants