Cli: promote commitment to a global arg + config.yml (#14684)

* Make commitment a global arg

* Add commitment to solana/cli/config.yml

* Fixup a couple Display/Verbose bugs

clap-utils/src/commitment.rs

@@ -1,22 +0,0 @@
-use crate::ArgConstant;
-use clap::Arg;
-
-pub const COMMITMENT_ARG: ArgConstant<'static> = ArgConstant {
-    name: "commitment",
-    long: "commitment",
-    help: "Return information at the selected commitment level",
-};
-
-pub fn commitment_arg<'a, 'b>() -> Arg<'a, 'b> {
-    commitment_arg_with_default("recent")
-}
-
-pub fn commitment_arg_with_default<'a, 'b>(default_value: &'static str) -> Arg<'a, 'b> {
-    Arg::with_name(COMMITMENT_ARG.name)
-        .long(COMMITMENT_ARG.long)
-        .takes_value(true)
-        .possible_values(&["recent", "single", "singleGossip", "root", "max"])
-        .default_value(default_value)
-        .value_name("COMMITMENT_LEVEL")
-        .help(COMMITMENT_ARG.help)
-}

clap-utils/src/input_parsers.rs

@@ -184,14 +184,9 @@ pub fn cluster_type_of(matches: &ArgMatches<'_>, name: &str) -> Option<ClusterTy
 }
 
 pub fn commitment_of(matches: &ArgMatches<'_>, name: &str) -> Option<CommitmentConfig> {
-    matches.value_of(name).map(|value| match value {
-        "max" => CommitmentConfig::max(),
-        "recent" => CommitmentConfig::recent(),
-        "root" => CommitmentConfig::root(),
-        "single" => CommitmentConfig::single(),
-        "singleGossip" => CommitmentConfig::single_gossip(),
-        _ => CommitmentConfig::default(),
-    })
+    matches
+        .value_of(name)
+        .map(|value| CommitmentConfig::from_str(value).unwrap_or_default())
 }
 
 #[cfg(test)]

clap-utils/src/lib.rs

@@ -23,7 +23,6 @@ impl std::fmt::Debug for DisplayError {
     }
 }
 
-pub mod commitment;
 pub mod fee_payer;
 pub mod input_parsers;
 pub mod input_validators;

cli-config/src/config.rs

@@ -17,9 +17,10 @@ pub struct Config {
     pub json_rpc_url: String,
     pub websocket_url: String,
     pub keypair_path: String,
-
     #[serde(default)]
     pub address_labels: HashMap<String, String>,
+    #[serde(default)]
+    pub commitment: String,
 }
 
 impl Default for Config {
@@ -41,11 +42,14 @@ impl Default for Config {
             "System Program".to_string(),
         );
 
+        let commitment = "singleGossip".to_string();
+
         Self {
             json_rpc_url,
             websocket_url,
             keypair_path,
             address_labels,
+            commitment,
         }
     }
 }

cli/src/cli.rs

@@ -9,7 +9,6 @@ use serde_json::{self, Value};
 use solana_account_decoder::{UiAccount, UiAccountEncoding};
 use solana_clap_utils::{
     self,
-    commitment::commitment_arg_with_default,
     fee_payer::{fee_payer_arg, FEE_PAYER_ARG},
     input_parsers::*,
     input_validators::*,
@@ -431,6 +430,10 @@ impl CliConfig<'_> {
         solana_cli_config::Config::default().websocket_url
     }
 
+    fn default_commitment() -> CommitmentConfig {
+        CommitmentConfig::single_gossip()
+    }
+
     fn first_nonempty_setting(
         settings: std::vec::Vec<(SettingType, String)>,
     ) -> (SettingType, String) {
@@ -440,6 +443,16 @@ impl CliConfig<'_> {
             .expect("no nonempty setting")
     }
 
+    fn first_setting_is_some<T>(
+        settings: std::vec::Vec<(SettingType, Option<T>)>,
+    ) -> (SettingType, T) {
+        let (setting_type, setting_option) = settings
+            .into_iter()
+            .find(|(_, value)| value.is_some())
+            .expect("all settings none");
+        (setting_type, setting_option.unwrap())
+    }
+
     pub fn compute_websocket_url_setting(
         websocket_cmd_url: &str,
         websocket_cfg_url: &str,
@@ -484,6 +497,23 @@ impl CliConfig<'_> {
         ])
     }
 
+    pub fn compute_commitment_config(
+        commitment_cmd: &str,
+        commitment_cfg: &str,
+    ) -> (SettingType, CommitmentConfig) {
+        Self::first_setting_is_some(vec![
+            (
+                SettingType::Explicit,
+                CommitmentConfig::from_str(commitment_cmd).ok(),
+            ),
+            (
+                SettingType::Explicit,
+                CommitmentConfig::from_str(commitment_cfg).ok(),
+            ),
+            (SettingType::SystemDefault, Some(Self::default_commitment())),
+        ])
+    }
+
     pub(crate) fn pubkey(&self) -> Result<Pubkey, SignerError> {
         if !self.signers.is_empty() {
             self.signers[0].try_pubkey()
@@ -1019,7 +1049,9 @@ fn process_show_account(
 
     let mut account_string = config.output_format.formatted_string(&cli_account);
 
-    if config.output_format == OutputFormat::Display {
+    if config.output_format == OutputFormat::Display
+        || config.output_format == OutputFormat::DisplayVerbose
+    {
         if let Some(output_file) = output_file {
             let mut f = File::create(output_file)?;
             f.write_all(&data)?;
@@ -1107,12 +1139,13 @@ fn process_transfer(
 }
 
 pub fn process_command(config: &CliConfig) -> ProcessResult {
-    if config.verbose && config.output_format == OutputFormat::Display {
+    if config.verbose && config.output_format == OutputFormat::DisplayVerbose {
         println_name_value("RPC URL:", &config.json_rpc_url);
         println_name_value("Default Signer Path:", &config.keypair_path);
         if config.keypair_path.starts_with("usb://") {
             println_name_value("Pubkey:", &format!("{:?}", config.pubkey()?));
         }
+        println_name_value("Commitment:", &config.commitment.commitment.to_string());
     }
 
     let mut _rpc_client;
@@ -1851,8 +1884,7 @@ pub fn app<'ab, 'v>(name: &str, about: &'ab str, version: &'v str) -> App<'ab, '
                         .long("lamports")
                         .takes_value(false)
                         .help("Display balance in lamports instead of SOL"),
-                )
-                .arg(commitment_arg_with_default("singleGossip")),
+                ),
         )
         .subcommand(
             SubCommand::with_name("confirm")
@@ -1949,8 +1981,7 @@ pub fn app<'ab, 'v>(name: &str, about: &'ab str, version: &'v str) -> App<'ab, '
                         .long("allow-excessive-deploy-account-balance")
                         .takes_value(false)
                         .help("Use the designated program id, even if the account already holds a large balance of SOL")
-                )
-                .arg(commitment_arg_with_default("singleGossip")),
+                ),
         )
         .subcommand(
             SubCommand::with_name("pay")

cli/src/cluster_query.rs

@@ -7,7 +7,6 @@ use chrono::{Local, TimeZone};
 use clap::{value_t, value_t_or_exit, App, AppSettings, Arg, ArgMatches, SubCommand};
 use console::{style, Emoji};
 use solana_clap_utils::{
-    commitment::{commitment_arg, commitment_arg_with_default},
     input_parsers::*,
     input_validators::*,
     keypair::DefaultSigner,
@@ -121,20 +120,17 @@ impl ClusterQuerySubCommands for App<'_, '_> {
                         .long("log")
                         .takes_value(false)
                         .help("Don't update the progress inplace; instead show updates with its own new lines"),
-                )
-                .arg(commitment_arg()),
+                ),
         )
         .subcommand(
             SubCommand::with_name("cluster-date")
-                .about("Get current cluster date, computed from genesis creation time and network time")
-                .arg(commitment_arg()),
+                .about("Get current cluster date, computed from genesis creation time and network time"),
         )
         .subcommand(
             SubCommand::with_name("cluster-version")
                 .about("Get the version of the cluster entrypoint"),
         )
-        .subcommand(SubCommand::with_name("fees").about("Display current cluster fees")
-            .arg(commitment_arg()),
+        .subcommand(SubCommand::with_name("fees").about("Display current cluster fees"),
         )
         .subcommand(
             SubCommand::with_name("first-available-block")
@@ -165,8 +161,7 @@ impl ClusterQuerySubCommands for App<'_, '_> {
         .subcommand(
             SubCommand::with_name("epoch-info")
             .about("Get information about the current epoch")
-            .alias("get-epoch-info")
-            .arg(commitment_arg()),
+            .alias("get-epoch-info"),
         )
         .subcommand(
             SubCommand::with_name("genesis-hash")
@@ -175,16 +170,13 @@ impl ClusterQuerySubCommands for App<'_, '_> {
         )
         .subcommand(
             SubCommand::with_name("slot").about("Get current slot")
-            .alias("get-slot")
-            .arg(commitment_arg()),
+            .alias("get-slot"),
         )
         .subcommand(
-            SubCommand::with_name("block-height").about("Get current block height")
-            .arg(commitment_arg()),
+            SubCommand::with_name("block-height").about("Get current block height"),
         )
         .subcommand(
-            SubCommand::with_name("epoch").about("Get current epoch")
-            .arg(commitment_arg()),
+            SubCommand::with_name("epoch").about("Get current epoch"),
         )
         .subcommand(
             SubCommand::with_name("largest-accounts").about("Get addresses of largest cluster accounts")
@@ -200,8 +192,7 @@ impl ClusterQuerySubCommands for App<'_, '_> {
                     .takes_value(false)
                     .conflicts_with("circulating")
                     .help("Filter address list to only non-circulating accounts")
-            )
-            .arg(commitment_arg()),
+            ),
         )
         .subcommand(
             SubCommand::with_name("supply").about("Get information about the cluster supply of SOL")
@@ -210,18 +201,15 @@ impl ClusterQuerySubCommands for App<'_, '_> {
                     .long("print-accounts")
                     .takes_value(false)
                     .help("Print list of non-circualting account addresses")
-            )
-            .arg(commitment_arg()),
+            ),
         )
         .subcommand(
             SubCommand::with_name("total-supply").about("Get total number of SOL")
-            .setting(AppSettings::Hidden)
-            .arg(commitment_arg()),
+            .setting(AppSettings::Hidden),
         )
         .subcommand(
             SubCommand::with_name("transaction-count").about("Get current transaction count")
-            .alias("get-transaction-count")
-            .arg(commitment_arg()),
+            .alias("get-transaction-count"),
         )
         .subcommand(
             SubCommand::with_name("ping")
@@ -268,8 +256,7 @@ impl ClusterQuerySubCommands for App<'_, '_> {
                         .default_value("15")
                         .help("Wait up to timeout seconds for transaction confirmation"),
                 )
-                .arg(blockhash_arg())
-                .arg(commitment_arg()),
+                .arg(blockhash_arg()),
         )
         .subcommand(
             SubCommand::with_name("live-slots")
@@ -292,8 +279,7 @@ impl ClusterQuerySubCommands for App<'_, '_> {
                         .takes_value(false)
                         .conflicts_with("address")
                         .help("Include vote transactions when monitoring all transactions")
-                )
-                .arg(commitment_arg_with_default("singleGossip")),
+                ),
         )
         .subcommand(
             SubCommand::with_name("block-production")
@@ -343,8 +329,7 @@ impl ClusterQuerySubCommands for App<'_, '_> {
                         .long("lamports")
                         .takes_value(false)
                         .help("Display balance in lamports instead of SOL"),
-                )
-                .arg(commitment_arg()),
+                ),
         )
         .subcommand(
             SubCommand::with_name("transaction-history")
@@ -1984,8 +1969,6 @@ mod tests {
             "-t",
             "3",
             "-D",
-            "--commitment",
-            "max",
             "--blockhash",
             "4CCNp28j6AhGq7PkjPDP4wbQWBS8LLbQin2xV5n8frKX",
         ]);

cli/src/main.rs

@@ -3,10 +3,7 @@ use clap::{
     SubCommand,
 };
 use console::style;
-
 use solana_clap_utils::{
-    commitment::COMMITMENT_ARG,
-    input_parsers::commitment_of,
     input_validators::{is_url, is_url_or_moniker},
     keypair::{CliSigners, DefaultSigner, SKIP_SEED_PHRASE_VALIDATION_ARG},
     DisplayError,
@@ -19,7 +16,6 @@ use solana_cli_config::{Config, CONFIG_FILE};
 use solana_cli_output::{display::println_name_value, OutputFormat};
 use solana_client::rpc_config::RpcSendTransactionConfig;
 use solana_remote_wallet::remote_wallet::RemoteWalletManager;
-use solana_sdk::commitment_config::CommitmentConfig;
 use std::{collections::HashMap, error, path::PathBuf, sync::Arc, time::Duration};
 
 pub fn println_name_value_or(name: &str, value: &str, setting_type: SettingType) {
@@ -64,12 +60,19 @@ fn parse_settings(matches: &ArgMatches<'_>) -> Result<bool, Box<dyn error::Error
                     );
                     let (keypair_setting_type, keypair_path) =
                         CliConfig::compute_keypair_path_setting("", &config.keypair_path);
+                    let (commitment_setting_type, commitment) =
+                        CliConfig::compute_commitment_config("", &config.commitment);
 
                     if let Some(field) = subcommand_matches.value_of("specific_setting") {
                         let (field_name, value, setting_type) = match field {
                             "json_rpc_url" => ("RPC URL", json_rpc_url, url_setting_type),
                             "websocket_url" => ("WebSocket URL", websocket_url, ws_setting_type),
                             "keypair" => ("Key Path", keypair_path, keypair_setting_type),
+                            "commitment" => (
+                                "Commitment",
+                                commitment.commitment.to_string(),
+                                commitment_setting_type,
+                            ),
                             _ => unreachable!(),
                         };
                         println_name_value_or(&format!("{}:", field_name), &value, setting_type);
@@ -78,6 +81,11 @@ fn parse_settings(matches: &ArgMatches<'_>) -> Result<bool, Box<dyn error::Error
                         println_name_value_or("RPC URL:", &json_rpc_url, url_setting_type);
                         println_name_value_or("WebSocket URL:", &websocket_url, ws_setting_type);
                         println_name_value_or("Keypair Path:", &keypair_path, keypair_setting_type);
+                        println_name_value_or(
+                            "Commitment:",
+                            &commitment.commitment.to_string(),
+                            commitment_setting_type,
+                        );
                     }
                 }
                 ("set", Some(subcommand_matches)) => {
@@ -93,6 +101,9 @@ fn parse_settings(matches: &ArgMatches<'_>) -> Result<bool, Box<dyn error::Error
                     if let Some(keypair) = subcommand_matches.value_of("keypair") {
                         config.keypair_path = keypair.to_string();
                     }
+                    if let Some(commitment) = subcommand_matches.value_of("commitment") {
+                        config.commitment = commitment.to_string();
+                    }
 
                     config.save(config_file)?;
 
@@ -106,11 +117,18 @@ fn parse_settings(matches: &ArgMatches<'_>) -> Result<bool, Box<dyn error::Error
                     );
                     let (keypair_setting_type, keypair_path) =
                         CliConfig::compute_keypair_path_setting("", &config.keypair_path);
+                    let (commitment_setting_type, commitment) =
+                        CliConfig::compute_commitment_config("", &config.commitment);
 
                     println_name_value("Config File:", config_file);
                     println_name_value_or("RPC URL:", &json_rpc_url, url_setting_type);
                     println_name_value_or("WebSocket URL:", &websocket_url, ws_setting_type);
                     println_name_value_or("Keypair Path:", &keypair_path, keypair_setting_type);
+                    println_name_value_or(
+                        "Commitment:",
+                        &commitment.commitment.to_string(),
+                        commitment_setting_type,
+                    );
                 }
                 ("import-address-labels", Some(subcommand_matches)) => {
                     let filename = value_t_or_exit!(subcommand_matches, "filename", PathBuf);
@@ -183,16 +201,10 @@ pub fn parse_args<'a>(
             OutputFormat::Display
         });
 
-    let commitment = {
-        let mut sub_matches = matches;
-        while let Some(subcommand_name) = sub_matches.subcommand_name() {
-            sub_matches = sub_matches
-                .subcommand_matches(subcommand_name)
-                .expect("subcommand_matches");
-        }
-        commitment_of(sub_matches, COMMITMENT_ARG.long)
-    }
-    .unwrap_or_else(CommitmentConfig::single_gossip);
+    let (_, commitment) = CliConfig::compute_commitment_config(
+        matches.value_of("commitment").unwrap_or(""),
+        &config.commitment,
+    );
 
     let address_labels = if matches.is_present("no_address_labels") {
         HashMap::new()
@@ -274,6 +286,15 @@ fn main() -> Result<(), Box<dyn error::Error>> {
             .takes_value(true)
             .help("Filepath or URL to a keypair"),
     )
+    .arg(
+        Arg::with_name("commitment")
+            .long("commitment")
+            .takes_value(true)
+            .possible_values(&["recent", "single", "singleGossip", "root", "max"])
+            .value_name("COMMITMENT_LEVEL")
+            .global(true)
+            .help("Return information at the selected commitment level"),
+    )
     .arg(
         Arg::with_name("verbose")
             .long("verbose")
@@ -325,7 +346,12 @@ fn main() -> Result<(), Box<dyn error::Error>> {
                             .index(1)
                             .value_name("CONFIG_FIELD")
                             .takes_value(true)
-                            .possible_values(&["json_rpc_url", "websocket_url", "keypair"])
+                            .possible_values(&[
+                                "json_rpc_url",
+                                "websocket_url",
+                                "keypair",
+                                "commitment",
+                            ])
                             .help("Return a specific config setting"),
                     ),
             )
@@ -334,7 +360,7 @@ fn main() -> Result<(), Box<dyn error::Error>> {
                     .about("Set a config setting")
                     .group(
                         ArgGroup::with_name("config_settings")
-                            .args(&["json_rpc_url", "websocket_url", "keypair"])
+                            .args(&["json_rpc_url", "websocket_url", "keypair", "commitment"])
                             .multiple(true)
                             .required(true),
                     ),

cli/src/program.rs

@@ -12,10 +12,7 @@ use clap::{App, AppSettings, Arg, ArgMatches, SubCommand};
 use log::*;
 use serde_json::{self, json};
 use solana_bpf_loader_program::{bpf_verifier, BPFError, ThisInstructionMeter};
-use solana_clap_utils::{
-    self, commitment::commitment_arg_with_default, input_parsers::*, input_validators::*,
-    keypair::*,
-};
+use solana_clap_utils::{self, input_parsers::*, input_validators::*, keypair::*};
 use solana_cli_output::display::new_spinner_progress_bar;
 use solana_client::{
     rpc_client::RpcClient, rpc_config::RpcSendTransactionConfig,
@@ -148,8 +145,7 @@ impl ProgramSubCommands for App<'_, '_> {
                                 .long("allow-excessive-deploy-account-balance")
                                 .takes_value(false)
                                 .help("Use the designated program id even if the account already holds a large balance of SOL")
-                        )
-                        .arg(commitment_arg_with_default("singleGossip")),
+                        ),
                 )
                 .subcommand(
                     SubCommand::with_name("write-buffer")
@@ -191,8 +187,7 @@ impl ProgramSubCommands for App<'_, '_> {
                                 .required(false)
                                 .help("Maximum length of the upgradeable program \
                                       [default: twice the length of the original deployed program]")
-                        )
-                        .arg(commitment_arg_with_default("singleGossip")),
+                        ),
                 )
                 .subcommand(
                     SubCommand::with_name("set-buffer-authority")
@@ -270,8 +265,7 @@ impl ProgramSubCommands for App<'_, '_> {
                                 .takes_value(true)
                                 .required(true)
                                 .help("Public key of the account to query")
-                        )
-                        .arg(commitment_arg_with_default("singleGossip")),
+                        ),
                 )
         )
     }

cli/src/stake.rs

@@ -10,7 +10,6 @@ use crate::{
 use chrono::{Local, TimeZone};
 use clap::{App, Arg, ArgGroup, ArgMatches, SubCommand};
 use solana_clap_utils::{
-    commitment::commitment_arg_with_default,
     fee_payer::{fee_payer_arg, FEE_PAYER_ARG},
     input_parsers::*,
     input_validators::*,
@@ -404,8 +403,7 @@ impl StakeSubCommands for App<'_, '_> {
                         .long("lamports")
                         .takes_value(false)
                         .help("Display balance in lamports instead of SOL")
-                )
-                .arg(commitment_arg_with_default("singleGossip")),
+                ),
         )
         .subcommand(
             SubCommand::with_name("stake-history")

cli/src/vote.rs

@@ -8,7 +8,6 @@ use crate::{
 };
 use clap::{value_t_or_exit, App, Arg, ArgMatches, SubCommand};
 use solana_clap_utils::{
-    commitment::commitment_arg_with_default,
     input_parsers::*,
     input_validators::*,
     keypair::{DefaultSigner, SignerIndex},
@@ -208,8 +207,7 @@ impl VoteSubCommands for App<'_, '_> {
                         .long("lamports")
                         .takes_value(false)
                         .help("Display balance in lamports instead of SOL"),
-                )
-                .arg(commitment_arg_with_default("singleGossip")),
+                ),
         )
         .subcommand(
             SubCommand::with_name("withdraw-from-vote-account")

sdk/src/commitment_config.rs

@@ -1,3 +1,6 @@
+use std::str::FromStr;
+use thiserror::Error;
+
 #[derive(Serialize, Deserialize, Default, Clone, Copy, Debug, PartialEq)]
 #[serde(rename_all = "camelCase")]
 pub struct CommitmentConfig {
@@ -44,6 +47,14 @@ impl CommitmentConfig {
     }
 }
 
+impl FromStr for CommitmentConfig {
+    type Err = ParseCommitmentLevelError;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        CommitmentLevel::from_str(s).map(|commitment| Self { commitment })
+    }
+}
+
 #[derive(Serialize, Deserialize, Clone, Copy, Debug, PartialEq, Eq, Hash)]
 #[serde(rename_all = "camelCase")]
 /// An attribute of a slot. It describes how finalized a block is at some point in time. For example, a slot
@@ -79,3 +90,37 @@ impl Default for CommitmentLevel {
         Self::Max
     }
 }
+
+impl FromStr for CommitmentLevel {
+    type Err = ParseCommitmentLevelError;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        match s {
+            "max" => Ok(CommitmentLevel::Max),
+            "recent" => Ok(CommitmentLevel::Recent),
+            "root" => Ok(CommitmentLevel::Root),
+            "single" => Ok(CommitmentLevel::Single),
+            "singleGossip" => Ok(CommitmentLevel::SingleGossip),
+            _ => Err(ParseCommitmentLevelError::Invalid),
+        }
+    }
+}
+
+impl std::fmt::Display for CommitmentLevel {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        let s = match self {
+            CommitmentLevel::Max => "max",
+            CommitmentLevel::Recent => "recent",
+            CommitmentLevel::Root => "root",
+            CommitmentLevel::Single => "single",
+            CommitmentLevel::SingleGossip => "singleGossip",
+        };
+        write!(f, "{}", s)
+    }
+}
+
+#[derive(Error, Debug)]
+pub enum ParseCommitmentLevelError {
+    #[error("invalid variant")]
+    Invalid,
+}