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

Fix proposer-nodes cli flag name #6125

Merged
merged 2 commits into from
Jul 18, 2024

Conversation

jimmygchen
Copy link
Member

@jimmygchen jimmygchen commented Jul 18, 2024

Issue Addressed

Getting this error while trying to run validator client locally (with a custom built docker image) and it shuts down immediately with:

Unable to initialize validator config: Unable to parse proposer_nodes: Unknown argument or group id.  Make sure you are using the argument id and not the short or long flags

This PR fixes the --proposer-node flag parsing to use the correct arg name.

Arg::new("proposer-nodes")
.long("proposer-nodes")
.value_name("NETWORK_ADDRESSES")

@jimmygchen jimmygchen added ready-for-review The code is ready for review bug Something isn't working val-client Relates to the validator client binary labels Jul 18, 2024
@jimmygchen
Copy link
Member Author

jimmygchen commented Jul 18, 2024

Note this was tested on a custom built image. I can't reproduce the crash using an image built using our Dockerfile, so it could be that the image wasn't built with --locked, and potentially using a different clap version.

@michaelsproul
Copy link
Member

michaelsproul commented Jul 18, 2024

I can't reproduce the crash, even building without the lockfile (cargo install --path lighthouse). Were you using the --proposer-nodes flag?

Regardless, I think we should fix it

@jimmygchen
Copy link
Member Author

Nope I wasn’t using the flag. Hmm really weird!

@michaelsproul
Copy link
Member

When did you build the image? I notice there were two clap versions released really close together: https://crates.io/crates/clap/versions

@michaelsproul
Copy link
Member

Can we add a test in lighthouse/tests/validator_client.rs so this doesn't regress?

@michaelsproul michaelsproul added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Jul 18, 2024
@jimmygchen
Copy link
Member Author

I used this image: kevaundray/lighthouse-eth:das-dbg-04d9eef8a

built by kev for this issue:
#6105 (comment)

I’ve reach out to him and asked whether this was produced with our local testnet scripts.

@jimmygchen
Copy link
Member Author

Good idea, I’ll add a test!

@jimmygchen jimmygchen added the ready-for-review The code is ready for review label Jul 18, 2024
@jimmygchen jimmygchen removed the waiting-on-author The reviewer has suggested changes and awaits thier implementation. label Jul 18, 2024
Copy link
Member

@michaelsproul michaelsproul left a comment

Choose a reason for hiding this comment

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

Nice

@michaelsproul
Copy link
Member

@mergify queue

Copy link

mergify bot commented Jul 18, 2024

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 549035d

@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Jul 18, 2024
mergify bot added a commit that referenced this pull request Jul 18, 2024
@mergify mergify bot merged commit 549035d into sigp:unstable Jul 18, 2024
28 checks passed
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ready-for-merge This PR is ready to merge. val-client Relates to the validator client binary
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants