Rework #17486 (backport #17566) (#17596)

* Revert "Improve missing default signer error messaging (#17486)"

This reverts commit 6d40d0d141.

(cherry picked from commit ca8c1c6c42)

* Improve missing default filepath signer error messaging

(cherry picked from commit 06a926f2f4)

* CI: temporarily skip spl downstream build

(cherry picked from commit d01b4f80f9)

Co-authored-by: Trent Nelson <trent@solana.com>

clap-utils/src/keypair.rs

@@ -24,9 +24,11 @@ use {
         },
     },
     std::{
+        cell::RefCell,
         convert::TryFrom,
         error,
         io::{stdin, stdout, Write},
+        ops::Deref,
         process::exit,
         str::FromStr,
         sync::Arc,
@@ -89,33 +91,49 @@ impl CliSignerInfo {
             .collect()
     }
 }
-#[derive(Debug)]
+
+#[derive(Debug, Default)]
 pub struct DefaultSigner {
     pub arg_name: String,
     pub path: String,
+    is_path_checked: RefCell<bool>,
 }
 
 impl DefaultSigner {
-    pub fn new(path: String) -> Self {
+    pub fn new<AN: AsRef<str>, P: AsRef<str>>(arg_name: AN, path: P) -> Self {
+        let arg_name = arg_name.as_ref().to_string();
+        let path = path.as_ref().to_string();
         Self {
-            arg_name: "keypair".to_string(),
+            arg_name,
             path,
+            ..Self::default()
         }
     }
-    pub fn from_path(path: String) -> Result<Self, Box<dyn error::Error>> {
+
-        std::fs::metadata(&path)
+    fn path(&self) -> Result<&str, Box<dyn std::error::Error>> {
-            .map_err(|_| {
+        if !self.is_path_checked.borrow().deref() {
-                std::io::Error::new(
+            parse_signer_source(&self.path)
-                    std::io::ErrorKind::Other,
+                .and_then(|s| {
-                    format!(
+                    if let SignerSourceKind::Filepath(path) = &s.kind {
-                    "No default signer found, run \"solana-keygen new -o {}\" to create a new one",
+                        std::fs::metadata(path).map(|_| ()).map_err(|e| e.into())
-                    path
+                    } else {
-                ),
+                        Ok(())
-                )
+                    }
-                .into()
+                })
-            })
+                .map_err(|_| {
-            .map(|_| Self::new(path))
+                    std::io::Error::new(
+                        std::io::ErrorKind::Other,
+                        format!(
+                        "No default signer found, run \"solana-keygen new -o {}\" to create a new one",
+                        self.path
+                    ),
+                    )
+                })?;
+            *self.is_path_checked.borrow_mut() = true;
+        }
+        Ok(&self.path)
     }
+
     pub fn generate_unique_signers(
         &self,
         bulk_signers: Vec<Option<Box<dyn Signer>>>,
@@ -145,7 +163,7 @@ impl DefaultSigner {
         matches: &ArgMatches,
         wallet_manager: &mut Option<Arc<RemoteWalletManager>>,
     ) -> Result<Box<dyn Signer>, Box<dyn std::error::Error>> {
-        signer_from_path(matches, &self.path, &self.arg_name, wallet_manager)
+        signer_from_path(matches, self.path()?, &self.arg_name, wallet_manager)
     }
 
     pub fn signer_from_path_with_config(
@@ -154,7 +172,13 @@ impl DefaultSigner {
         wallet_manager: &mut Option<Arc<RemoteWalletManager>>,
         config: &SignerFromPathConfig,
     ) -> Result<Box<dyn Signer>, Box<dyn std::error::Error>> {
-        signer_from_path_with_config(matches, &self.path, &self.arg_name, wallet_manager, config)
+        signer_from_path_with_config(
+            matches,
+            self.path()?,
+            &self.arg_name,
+            wallet_manager,
+            config,
+        )
     }
 }
 
@@ -277,7 +301,9 @@ pub(crate) fn parse_signer_source<S: AsRef<str>>(
                     ASK_KEYWORD => Ok(SignerSource::new_legacy(SignerSourceKind::Prompt)),
                     _ => match Pubkey::from_str(source.as_str()) {
                         Ok(pubkey) => Ok(SignerSource::new(SignerSourceKind::Pubkey(pubkey))),
-                        Err(_) => Ok(SignerSource::new(SignerSourceKind::Filepath(source))),
+                        Err(_) => std::fs::metadata(source.as_str())
+                            .map(|_| SignerSource::new(SignerSourceKind::Filepath(source)))
+                            .map_err(|err| err.into()),
                     },
                 }
             }
@@ -751,6 +777,10 @@ mod tests {
         // Catchall into SignerSource::Filepath fails
         let junk = "sometextthatisnotapubkeyorfile".to_string();
         assert!(Pubkey::from_str(&junk).is_err());
+        assert!(matches!(
+            parse_signer_source(&junk),
+            Err(SignerSourceError::IoError(_))
+        ));
 
         let prompt = "prompt:".to_string();
         assert!(matches!(

cli/src/cli.rs

@@ -2296,7 +2296,7 @@ mod tests {
         let default_keypair_file = make_tmp_path("keypair_file");
         write_keypair_file(&default_keypair, &default_keypair_file).unwrap();
 
-        let default_signer = DefaultSigner::new(default_keypair_file);
+        let default_signer = DefaultSigner::new("keypair", &default_keypair_file);
 
         let signer_info = default_signer
             .generate_unique_signers(vec![], &matches, &mut None)
@@ -2374,7 +2374,7 @@ mod tests {
         let keypair_file = make_tmp_path("keypair_file");
         write_keypair_file(&default_keypair, &keypair_file).unwrap();
         let keypair = read_keypair_file(&keypair_file).unwrap();
-        let default_signer = DefaultSigner::new(keypair_file.clone());
+        let default_signer = DefaultSigner::new("", &keypair_file);
         // Test Airdrop Subcommand
         let test_airdrop =
             test_commands
@@ -2902,7 +2902,7 @@ mod tests {
         let default_keypair = Keypair::new();
         let default_keypair_file = make_tmp_path("keypair_file");
         write_keypair_file(&default_keypair, &default_keypair_file).unwrap();
-        let default_signer = DefaultSigner::new(default_keypair_file.clone());
+        let default_signer = DefaultSigner::new("", &default_keypair_file);
 
         //Test Transfer Subcommand, SOL
         let from_keypair = keypair_from_seed(&[0u8; 32]).unwrap();

cli/src/cluster_query.rs

@@ -2098,7 +2098,7 @@ mod tests {
         let default_keypair = Keypair::new();
         let (default_keypair_file, mut tmp_file) = make_tmp_file();
         write_keypair(&default_keypair, tmp_file.as_file_mut()).unwrap();
-        let default_signer = DefaultSigner::new(default_keypair_file);
+        let default_signer = DefaultSigner::new("", &default_keypair_file);
 
         let test_cluster_version = test_commands
             .clone()

cli/src/main.rs

@@ -173,12 +173,13 @@ pub fn parse_args<'a>(
         matches.value_of("json_rpc_url").unwrap_or(""),
         &config.json_rpc_url,
     );
+    let default_signer_arg_name = "keypair".to_string();
     let (_, default_signer_path) = CliConfig::compute_keypair_path_setting(
-        matches.value_of("keypair").unwrap_or(""),
+        matches.value_of(&default_signer_arg_name).unwrap_or(""),
         &config.keypair_path,
     );
 
-    let default_signer = DefaultSigner::from_path(default_signer_path.clone())?;
+    let default_signer = DefaultSigner::new(default_signer_arg_name, &default_signer_path);
 
     let CliCommandInfo {
         command,

cli/src/nonce.rs

@@ -596,7 +596,7 @@ mod tests {
         let default_keypair = Keypair::new();
         let (default_keypair_file, mut tmp_file) = make_tmp_file();
         write_keypair(&default_keypair, tmp_file.as_file_mut()).unwrap();
-        let default_signer = DefaultSigner::new(default_keypair_file.clone());
+        let default_signer = DefaultSigner::new("", &default_keypair_file);
         let (keypair_file, mut tmp_file) = make_tmp_file();
         let nonce_account_keypair = Keypair::new();
         write_keypair(&nonce_account_keypair, tmp_file.as_file_mut()).unwrap();

cli/src/program.rs

@@ -2131,7 +2131,7 @@ mod tests {
         let default_keypair = Keypair::new();
         let keypair_file = make_tmp_path("keypair_file");
         write_keypair_file(&default_keypair, &keypair_file).unwrap();
-        let default_signer = DefaultSigner::new(keypair_file.clone());
+        let default_signer = DefaultSigner::new("", &keypair_file);
 
         let test_command = test_commands.clone().get_matches_from(vec![
             "test",
@@ -2339,7 +2339,7 @@ mod tests {
         let default_keypair = Keypair::new();
         let keypair_file = make_tmp_path("keypair_file");
         write_keypair_file(&default_keypair, &keypair_file).unwrap();
-        let default_signer = DefaultSigner::new(keypair_file.clone());
+        let default_signer = DefaultSigner::new("", &keypair_file);
 
         // defaults
         let test_command = test_commands.clone().get_matches_from(vec![
@@ -2487,7 +2487,7 @@ mod tests {
         let default_keypair = Keypair::new();
         let keypair_file = make_tmp_path("keypair_file");
         write_keypair_file(&default_keypair, &keypair_file).unwrap();
-        let default_signer = DefaultSigner::new(keypair_file.clone());
+        let default_signer = DefaultSigner::new("", &keypair_file);
 
         let program_pubkey = Pubkey::new_unique();
         let new_authority_pubkey = Pubkey::new_unique();
@@ -2595,7 +2595,7 @@ mod tests {
         let default_keypair = Keypair::new();
         let keypair_file = make_tmp_path("keypair_file");
         write_keypair_file(&default_keypair, &keypair_file).unwrap();
-        let default_signer = DefaultSigner::new(keypair_file.clone());
+        let default_signer = DefaultSigner::new("", &keypair_file);
 
         let buffer_pubkey = Pubkey::new_unique();
         let new_authority_pubkey = Pubkey::new_unique();
@@ -2652,7 +2652,7 @@ mod tests {
         let default_keypair = Keypair::new();
         let keypair_file = make_tmp_path("keypair_file");
         write_keypair_file(&default_keypair, &keypair_file).unwrap();
-        let default_signer = DefaultSigner::new(keypair_file);
+        let default_signer = DefaultSigner::new("", &keypair_file);
 
         // defaults
         let buffer_pubkey = Pubkey::new_unique();
@@ -2751,7 +2751,7 @@ mod tests {
         let default_keypair = Keypair::new();
         let keypair_file = make_tmp_path("keypair_file");
         write_keypair_file(&default_keypair, &keypair_file).unwrap();
-        let default_signer = DefaultSigner::new(keypair_file.clone());
+        let default_signer = DefaultSigner::new("", &keypair_file);
 
         // defaults
         let buffer_pubkey = Pubkey::new_unique();

cli/src/stake.rs

@@ -2104,7 +2104,7 @@ mod tests {
         let default_keypair = Keypair::new();
         let (default_keypair_file, mut tmp_file) = make_tmp_file();
         write_keypair(&default_keypair, tmp_file.as_file_mut()).unwrap();
-        let default_signer = DefaultSigner::new(default_keypair_file.clone());
+        let default_signer = DefaultSigner::new("", &default_keypair_file);
         let (keypair_file, mut tmp_file) = make_tmp_file();
         let stake_account_keypair = Keypair::new();
         write_keypair(&stake_account_keypair, tmp_file.as_file_mut()).unwrap();

cli/src/vote.rs

@@ -826,7 +826,7 @@ mod tests {
         let default_keypair = Keypair::new();
         let (default_keypair_file, mut tmp_file) = make_tmp_file();
         write_keypair(&default_keypair, tmp_file.as_file_mut()).unwrap();
-        let default_signer = DefaultSigner::new(default_keypair_file.clone());
+        let default_signer = DefaultSigner::new("", &default_keypair_file);
 
         let test_authorize_voter = test_commands.clone().get_matches_from(vec![
             "test",

scripts/build-downstream-projects.sh

@@ -100,5 +100,5 @@ EOF
 
 
 _ example_helloworld
-_ spl
+#_ spl
 _ serum_dex