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 incorrect VC default HTTP token path when the --datadir flag is present #6748

Merged
merged 1 commit into from
Jan 8, 2025

Conversation

jimmygchen
Copy link
Member

Issue Addressed

Fixes a validator client bug introduced in #6577.

When a custom --datadir flag is provided, the default http token path changes to $HOME/.lighthouse/mainnet/validators/api-token.txt, rather than the previous default <datadir>/validators/api-token.txt

This causes a CRIT in some setups where the validator client does not have write access to the home directory (e.g. eth-docker):

CRIT Failed to start validator client        reason: Unable to create parent directories for "/nonexistent/.lighthouse/mainnet/validators/api-token.txt": Os { code: 13, kind: PermissionDenied, message: "Permission denied" }

Proposed Changes

Set the correct default VC API token path, if a datadir or validators-dir is provided.

@jimmygchen jimmygchen added bug Something isn't working val-client Relates to the validator client binary labels Jan 1, 2025
@jimmygchen
Copy link
Member Author

This wasn't included in the v6.0.1 release, so a patch isn't required 😌

@jimmygchen jimmygchen added ready-for-review The code is ready for review v6.1.0 New release c. Q1 2025 labels Jan 1, 2025
Copy link
Member

@pawanjay176 pawanjay176 left a comment

Choose a reason for hiding this comment

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

Took me a while to see the difference. Nice catch!

@pawanjay176 pawanjay176 added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Jan 8, 2025
@jimmygchen
Copy link
Member Author

@mergify requeue

Copy link

mergify bot commented Jan 8, 2025

requeue

❌ This pull request head commit has not been previously disembarked from queue.

@jimmygchen
Copy link
Member Author

@mergify queue

Copy link

mergify bot commented Jan 8, 2025

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 87b72de

mergify bot added a commit that referenced this pull request Jan 8, 2025
@mergify mergify bot merged commit 87b72de into sigp:unstable Jan 8, 2025
30 checks passed
@syjn99
Copy link

syjn99 commented Jan 8, 2025

Thanks for catching this! I made a mistake in my previous contribution, and I appreciate your help in fixing it.

@jimmygchen jimmygchen deleted the fix-api-token-path branch January 8, 2025 07:07
@jimmygchen
Copy link
Member Author

All good, I missed it in review too 🙏

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. v6.1.0 New release c. Q1 2025 val-client Relates to the validator client binary
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants