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 missing filename in 'Unable to read keypair file' errors #2932

Merged

Conversation

mikemaccana
Copy link
Contributor

Fixes #2884

Copy link

vercel bot commented Apr 25, 2024

@mikemaccana is attempting to deploy a commit to the coral-xyz Team on Vercel.

A member of the Team first needs to authorize it.

@mikemaccana mikemaccana force-pushed the fix-missing-keypair-path branch from 1082d2d to 0b9540c Compare April 25, 2024 18:17
@acheroncrypto acheroncrypto added enhancement New feature or request cli labels Apr 25, 2024
Copy link
Collaborator

@acheroncrypto acheroncrypto left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

This currently is missing some parts, please search for read_keypair_file in your editor and fix all of them. Just to give an example:

anchor/cli/src/lib.rs

Lines 3620 to 3622 in 0b9540c

solana_sdk::signature::read_keypair_file(path)
.map_err(|_| anyhow!("Unable to read keypair file"))?
.pubkey(),

is not handled in this PR.

cli/src/lib.rs Outdated Show resolved Hide resolved
cli/src/lib.rs Outdated Show resolved Hide resolved
cli/src/lib.rs Outdated Show resolved Hide resolved
@mikemaccana mikemaccana force-pushed the fix-missing-keypair-path branch from 254da4c to 876f404 Compare April 26, 2024 19:11
@mikemaccana
Copy link
Contributor Author

mikemaccana commented Apr 26, 2024

Thanks. I did try and make get_keypair() take a path instead originally but read_keypair_file() really wants the trait AsRef<std::path::Path> and WalletPath doesn't implement it. I ended up converting to a string in this most recent update, is there a better way?

read_keypair_file in your editor and fix all of them

Missed that one (originally I skipped it when playing with WalletPath as it wants a string). Got it now.

Copy link
Collaborator

@acheroncrypto acheroncrypto left a comment

Choose a reason for hiding this comment

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

Thanks. I did try and make get_keypair() take a path instead originally but read_keypair_file() really wants the trait AsRef<std::path::Path> and WalletPath doesn't implement it. I ended up converting to a string in this most recent update, is there a better way?

Taking in &str would be better than String.

read_keypair_file in your editor and fix all of them

Missed that one (originally I skipped it when playing with WalletPath as it wants a string). Got it now.

Thanks, there are 2 more though:

  • anchor/cli/src/config.rs

    Lines 1268 to 1269 in 499e1e6

    solana_sdk::signature::read_keypair_file(file.path())
    .map_err(|_| anyhow!("failed to read keypair for program: {}", self.lib_name))
  • anchor/cli/src/config.rs

    Lines 534 to 535 in 499e1e6

    solana_sdk::signature::read_keypair_file(&self.provider.wallet.to_string())
    .map_err(|_| anyhow!("Unable to read keypair file"))

cli/src/lib.rs Outdated Show resolved Hide resolved
cli/src/lib.rs Outdated Show resolved Hide resolved
@mikemaccana mikemaccana force-pushed the fix-missing-keypair-path branch 2 times, most recently from 1285166 to b5a7577 Compare April 29, 2024 15:55
CHANGELOG.md Outdated Show resolved Hide resolved
cli/src/config.rs Outdated Show resolved Hide resolved
cli/src/config.rs Outdated Show resolved Hide resolved
cli/src/lib.rs Outdated Show resolved Hide resolved
cli/src/lib.rs Outdated Show resolved Hide resolved
Use named placeholders in formatting

Co-authored-by: acheron <[email protected]>
@mikemaccana mikemaccana force-pushed the fix-missing-keypair-path branch from ac9beec to e9797a1 Compare May 1, 2024 16:18
@mikemaccana
Copy link
Contributor Author

cargo clippy bits fixed too.

Copy link
Collaborator

@acheroncrypto acheroncrypto left a comment

Choose a reason for hiding this comment

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

Thank you!

@acheroncrypto acheroncrypto merged commit 940de79 into coral-xyz:master May 1, 2024
48 of 49 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feature suggestion: 'Unable to read keypair file' should include name of keypair file it cannot read
2 participants