Skip to content

fix: Show correct default path in signer error messages#7849

Merged
KirillLykov merged 2 commits intoanza-xyz:masterfrom
swarna1101:fix-solana-keygen-default-signer-path
Sep 3, 2025
Merged

fix: Show correct default path in signer error messages#7849
KirillLykov merged 2 commits intoanza-xyz:masterfrom
swarna1101:fix-solana-keygen-default-signer-path

Conversation

@swarna1101
Copy link
Copy Markdown

Problem

When Solana CLI configuration contains an invalid keypair_path, error messages suggest creating new keypairs at the misconfigured location instead of the user's actual default directory. This leads to confusion and keypairs being created in wrong locations (e.g., /home/sol/identity/id.json instead of $HOME/.config/solana/id.json).

Solution

Modified error message generation in DefaultSigner::path() to always suggest the system default path (~/.config/solana/id.json) based on the user's actual home directory, regardless of any misconfigured paths in config files.

@KirillLykov can you check this, closes #6637

@mergify mergify Bot requested a review from a team September 3, 2025 06:53
@KirillLykov KirillLykov added the CI Pull Request is ready to enter CI label Sep 3, 2025
@anza-team anza-team removed the CI Pull Request is ready to enter CI label Sep 3, 2025
@KirillLykov KirillLykov added the CI Pull Request is ready to enter CI label Sep 3, 2025
@anza-team anza-team removed the CI Pull Request is ready to enter CI label Sep 3, 2025
Copy link
Copy Markdown

@KirillLykov KirillLykov left a comment

Choose a reason for hiding this comment

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

I double checked, it fixes the problem. Thanks a lot!

@KirillLykov KirillLykov added automerge automerge Merge this Pull Request automatically once CI passes v3.0 labels Sep 3, 2025
@mergify
Copy link
Copy Markdown

mergify Bot commented Sep 3, 2025

Backports to the beta branch are to be avoided unless absolutely necessary for fixing bugs, security issues, and perf regressions. Changes intended for backport should be structured such that a minimum effective diff can be committed separately from any refactoring, plumbing, cleanup, etc that are not strictly necessary to achieve the goal. Any of the latter should go only into master and ride the normal stabilization schedule. Exceptions include CI/metrics changes, CLI improvements and documentation updates on a case by case basis.

@mergify
Copy link
Copy Markdown

mergify Bot commented Sep 3, 2025

automerge label removed due to a CI failure

@mergify mergify Bot removed the automerge automerge Merge this Pull Request automatically once CI passes label Sep 3, 2025
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.0%. Comparing base (2933846) to head (94f0251).
⚠️ Report is 5 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #7849   +/-   ##
=======================================
  Coverage    83.0%    83.0%           
=======================================
  Files         809      809           
  Lines      356483   356497   +14     
=======================================
+ Hits       296203   296227   +24     
+ Misses      60280    60270   -10     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@KirillLykov KirillLykov merged commit 7c3e9e7 into anza-xyz:master Sep 3, 2025
59 checks passed
mergify Bot pushed a commit that referenced this pull request Sep 3, 2025
Fixes the problem with Solana CLI when configuration contains an invalid `keypair_path`: error messages suggest creating new keypairs at the misconfigured location instead of the user's actual default directory. This leads to confusion and keypairs being created in wrong locations.
Modified error message generation in `efaultSigner::path()` to always suggest the system default path based on the user's actual home directory, regardless of any misconfigured paths in config files.

(cherry picked from commit 7c3e9e7)

# Conflicts:
#	clap-utils/src/keypair.rs
#	clap-v3-utils/src/keypair.rs
@t-nelson
Copy link
Copy Markdown

t-nelson commented Sep 3, 2025

i don't understand why
a) the original message was wrong, nor
b) the fix committed here is correct

creating a key at the code default path does not fix there not being a keypair file at the path specified in the config. all tools should respect the values in the config file. if the user has specified that they wish to use a keypair file at a path that does not, or no longer, contains one, the correctness of that decision is not for us to decide. it may well be exactly that they intend to use the path and that creating a key there is the correct advice

i think this change should be reverted

@KirillLykov
Copy link
Copy Markdown

KirillLykov commented Sep 4, 2025

Actually,

i don't understand why a) the original message was wrong, nor b) the fix committed here is correct

creating a key at the code default path does not fix there not being a keypair file at the path specified in the config. all tools should respect the values in the config file. if the user has specified that they wish to use a keypair file at a path that does not, or no longer, contains one, the correctness of that decision is not for us to decide. it may well be exactly that they intend to use the path and that creating a key there is the correct advice

i think this change should be reverted

You are right, that's my misunderstanding of where this path to /home/sol/ comes from. I completely forgot about this config.yaml file existence. Please approve: #7882

@KirillLykov KirillLykov mentioned this pull request Sep 4, 2025
KirillLykov added a commit that referenced this pull request Sep 4, 2025
* Revert "Resolving linter issue with keypair commenting (#7870)"

This reverts commit ea34d3b.

* Revert "fix: Show correct default path in signer error messages (#7849)"

This reverts commit 7c3e9e7.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

solana-keygen expects that default signer is in wrong folder

5 participants