gaspersic/solana
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Cli error cleanup 1.0 (#8834)

Browse Source 
* Don't use move semantics if not needed (#8793)

* SDK: Deboilerplate `TransportError` with thiserror

* Enable interchange between `TransportError` and `ClientError`

* SDK: Retval consistency between `Client` and `AsyncClient` traits

* Client: Introduce/use `Result` type

* Client: Remove unused `RpcResponseIn` type

* Client: Rename `RpcResponse` to more appropriate `RpcResult`

* Client: Death to `io::Result` return types

* Client: Struct-ify `ClientError`

* Client: Add optional `command` parameter to `ClientError`

* RpcClient: Stop abusing `io::Error` (low-fruit)

* ClientError: Use `thiserror`'s `Display` impl

* Extend `RpcError`'s utility

* RpcClient: Stop abusing `io::Error` (the rest)

* CLI: Shim `main()` so we can `Display` format errors

* claputils: format input validator errors with `Display`

They are intended to be displayed to users

* SDK: `thiserror` for hash and sig parse erros

* Keygen: Shim main to format errors with `Display`

* SDK: `thiserror` for `InstructionError`

* CLI: `thiserror` for `CliError`

* CLI: Format user messages with `Display`

* Client: Tweak `Display` for `ClientError`

* RpcClient: Improve messaging when TX cannot be confirmed

* fu death io res retval

* CLI/Keygen - fix shell return value on error

* Tweak `InstructionError` `Display` messages as per review

* Cleanup hackjob return code fix

* Embrace that which you hate most

* Too much...

Co-authored-by: Jack May <jack@solana.com>
This commit is contained in:
Trent Nelson
2020-03-13 07:42:25 -06:00
committed by GitHub
parent 976d744b0d
commit 4a42cfc42a
52 changed files with 689 additions and 760 deletions
Download Patch File Download Diff File Expand all files Collapse all files

78
cli/src/cli.rs
View File

@@ -18,7 +18,10 @@ use solana_clap_utils::{
input_parsers::*, input_validators::*, keypair::signer_from_path, offline::SIGN_ONLY_ARG,
ArgConstant,
};
use solana_client::{client_error::ClientError, rpc_client::RpcClient};
use solana_client::{
client_error::{ClientErrorKind, Result as ClientResult},
rpc_client::RpcClient,
};
#[cfg(not(test))]
use solana_faucet::faucet::request_airdrop_transaction;
#[cfg(test)]
@@ -47,14 +50,15 @@ use solana_stake_program::{
use solana_storage_program::storage_instruction::StorageAccountType;
use solana_vote_program::vote_state::VoteAuthorize;
use std::{
error,
fs::File,
io::{Read, Write},
net::{IpAddr, SocketAddr},
sync::Arc,
thread::sleep,
time::Duration,
{error, fmt},
};
use thiserror::Error;
use url::Url;
pub type CliSigners = Vec<Box<dyn Signer>>;
@@ -406,46 +410,34 @@ pub struct CliCommandInfo {
pub signers: CliSigners,
}
#[derive(Debug, PartialEq)]
#[derive(Debug, Error, PartialEq)]
pub enum CliError {
#[error("bad parameter: {0}")]
BadParameter(String),
#[error("command not recognized: {0}")]
CommandNotRecognized(String),
#[error("insuficient funds for fee")]
InsufficientFundsForFee,
#[error(transparent)]
InvalidNonce(CliNonceError),
#[error("dynamic program error: {0}")]
DynamicProgramError(String),
#[error("rpc request error: {0}")]
RpcRequestError(String),
#[error("keypair file not found: {0}")]
KeypairFileNotFound(String),
}
impl fmt::Display for CliError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "invalid")
}
}
impl error::Error for CliError {
fn description(&self) -> &str {
"invalid"
}
fn cause(&self) -> Option<&dyn error::Error> {
// Generic error, underlying cause isn't tracked.
None
}
}
impl From<Box<dyn error::Error>> for CliError {
fn from(error: Box<dyn error::Error>) -> Self {
CliError::DynamicProgramError(format!("{:?}", error))
CliError::DynamicProgramError(error.to_string())
}
}
impl From<CliNonceError> for CliError {
fn from(error: CliNonceError) -> Self {
match error {
CliNonceError::Client(client_error) => {
Self::RpcRequestError(format!("{:?}", client_error))
}
CliNonceError::Client(client_error) => Self::RpcRequestError(client_error),
_ => Self::InvalidNonce(error),
}
}
@@ -717,7 +709,7 @@ pub fn parse_command(
.parse()
.or_else(|err| {
Err(CliError::BadParameter(format!(
"Invalid faucet port: {:?}",
"Invalid faucet port: {}",
err
)))
})?;
@@ -725,7 +717,7 @@ pub fn parse_command(
let faucet_host = if let Some(faucet_host) = matches.value_of("faucet_host") {
Some(solana_net_utils::parse_host(faucet_host).or_else(|err| {
Err(CliError::BadParameter(format!(
"Invalid faucet host: {:?}",
"Invalid faucet host: {}",
err
)))
})?)
@@ -1137,13 +1129,13 @@ fn process_confirm(rpc_client: &RpcClient, signature: &Signature) -> ProcessResu
if let Some(result) = status {
match result {
Ok(_) => Ok("Confirmed".to_string()),
Err(err) => Ok(format!("Transaction failed with error {:?}", err)),
Err(err) => Ok(format!("Transaction failed with error: {}", err)),
}
} else {
Ok("Not found".to_string())
}
}
Err(err) => Err(CliError::RpcRequestError(format!("Unable to confirm: {:?}", err)).into()),
Err(err) => Err(CliError::RpcRequestError(format!("Unable to confirm: {}", err)).into()),
}
}
@@ -1203,7 +1195,7 @@ fn process_deploy(
program_data.len() as u64,
&bpf_loader::id(),
);
let message = Message::new(vec![ix]);
let message = Message::new(&[ix]);
let mut create_account_tx = Transaction::new_unsigned(message);
create_account_tx.try_sign(&[config.signers[0], &program_id], blockhash)?;
messages.push(&create_account_tx.message);
@@ -1216,7 +1208,7 @@ fn process_deploy(
(i * DATA_CHUNK_SIZE) as u32,
chunk.to_vec(),
);
let message = Message::new_with_payer(vec![instruction], Some(&signers[0].pubkey()));
let message = Message::new_with_payer(&[instruction], Some(&signers[0].pubkey()));
let mut tx = Transaction::new_unsigned(message);
tx.try_sign(&signers, blockhash)?;
write_transactions.push(tx);
@@ -1226,7 +1218,7 @@ fn process_deploy(
}
let instruction = loader_instruction::finalize(&program_id.pubkey(), &bpf_loader::id());
let message = Message::new_with_payer(vec![instruction], Some(&signers[0].pubkey()));
let message = Message::new_with_payer(&[instruction], Some(&signers[0].pubkey()));
let mut finalize_tx = Transaction::new_unsigned(message);
finalize_tx.try_sign(&signers, blockhash)?;
messages.push(&finalize_tx.message);
@@ -1294,7 +1286,7 @@ fn process_pay(
let message = if let Some(nonce_account) = &nonce_account {
Message::new_with_nonce(vec![ix], None, nonce_account, &nonce_authority.pubkey())
} else {
Message::new(vec![ix])
Message::new(&[ix])
};
let mut tx = Transaction::new_unsigned(message);
tx.try_sign(&config.signers, blockhash)?;
@@ -1334,7 +1326,7 @@ fn process_pay(
cancelable,
lamports,
);
let message = Message::new(ixs);
let message = Message::new(&ixs);
let mut tx = Transaction::new_unsigned(message);
tx.try_sign(&[config.signers[0], &contract_state], blockhash)?;
if sign_only {
@@ -1377,7 +1369,7 @@ fn process_pay(
cancelable,
lamports,
);
let message = Message::new(ixs);
let message = Message::new(&ixs);
let mut tx = Transaction::new_unsigned(message);
tx.try_sign(&[config.signers[0], &contract_state], blockhash)?;
if sign_only {
@@ -1411,7 +1403,7 @@ fn process_cancel(rpc_client: &RpcClient, config: &CliConfig, pubkey: &Pubkey) -
pubkey,
&config.signers[0].pubkey(),
);
let message = Message::new(vec![ix]);
let message = Message::new(&[ix]);
let mut tx = Transaction::new_unsigned(message);
tx.try_sign(&config.signers, blockhash)?;
check_account_for_fee(
@@ -1434,7 +1426,7 @@ fn process_time_elapsed(
let (blockhash, fee_calculator) = rpc_client.get_recent_blockhash()?;
let ix = budget_instruction::apply_timestamp(&config.signers[0].pubkey(), pubkey, to, dt);
let message = Message::new(vec![ix]);
let message = Message::new(&[ix]);
let mut tx = Transaction::new_unsigned(message);
tx.try_sign(&config.signers, blockhash)?;
check_account_for_fee(
@@ -1482,7 +1474,7 @@ fn process_transfer(
&nonce_authority.pubkey(),
)
} else {
Message::new_with_payer(ixs, Some(&fee_payer.pubkey()))
Message::new_with_payer(&ixs, Some(&fee_payer.pubkey()))
};
let mut tx = Transaction::new_unsigned(message);
tx.try_sign(&config.signers, recent_blockhash)?;
@@ -1514,7 +1506,7 @@ fn process_witness(
let (blockhash, fee_calculator) = rpc_client.get_recent_blockhash()?;
let ix = budget_instruction::apply_signature(&config.signers[0].pubkey(), pubkey, to);
let message = Message::new(vec![ix]);
let message = Message::new(&[ix]);
let mut tx = Transaction::new_unsigned(message);
tx.try_sign(&config.signers, blockhash)?;
check_account_for_fee(
@@ -2111,18 +2103,18 @@ pub fn request_and_confirm_airdrop(
log_instruction_custom_error::<SystemError>(result)
}
pub fn log_instruction_custom_error<E>(result: Result<String, ClientError>) -> ProcessResult
pub fn log_instruction_custom_error<E>(result: ClientResult<String>) -> ProcessResult
where
E: 'static + std::error::Error + DecodeError<E> + FromPrimitive,
{
match result {
Err(err) => {
if let ClientError::TransactionError(TransactionError::InstructionError(
if let ClientErrorKind::TransactionError(TransactionError::InstructionError(
_,
InstructionError::CustomError(code),
)) = err
)) = err.kind()
{
if let Some(specific_error) = E::decode_custom_error_to_enum(code) {
if let Some(specific_error) = E::decode_custom_error_to_enum(*code) {
error!("{}::{:?}", E::type_of(), specific_error);
eprintln!(
"Program Error ({}::{:?}): {}",
@@ -3325,7 +3317,7 @@ mod tests {
assert_eq!(
process_command(&config).unwrap(),
format!(
"Transaction failed with error {:?}",
"Transaction failed with error: {}",
TransactionError::AccountInUse
)
);

4
cli/src/cluster_query.rs
View File

@@ -789,7 +789,7 @@ pub fn process_ping(
last_blockhash = recent_blockhash;
let ix = system_instruction::transfer(&config.signers[0].pubkey(), &to, lamports);
let message = Message::new(vec![ix]);
let message = Message::new(&[ix]);
let mut transaction = Transaction::new_unsigned(message);
transaction.try_sign(&config.signers, recent_blockhash)?;
check_account_for_fee(
@@ -972,7 +972,7 @@ pub fn process_live_slots(url: &str) -> ProcessResult {
current = Some(new_info);
}
Err(err) => {
eprintln!("disconnected: {:?}", err);
eprintln!("disconnected: {}", err);
break;
}
}

10
cli/src/main.rs
View File

@@ -1,7 +1,9 @@
use clap::{crate_description, crate_name, AppSettings, Arg, ArgGroup, ArgMatches, SubCommand};
use console::style;
use solana_clap_utils::{input_validators::is_url, keypair::SKIP_SEED_PHRASE_VALIDATION_ARG};
use solana_clap_utils::{
input_validators::is_url, keypair::SKIP_SEED_PHRASE_VALIDATION_ARG, DisplayError,
};
use solana_cli::{
cli::{app, parse_command, process_command, CliCommandInfo, CliConfig, CliSigners},
display::{println_name_value, println_name_value_or},
@@ -230,6 +232,10 @@ fn main() -> Result<(), Box<dyn error::Error>> {
)
.get_matches();
do_main(&matches).map_err(|err| DisplayError::new_as_boxed(err).into())
}
fn do_main(matches: &ArgMatches<'_>) -> Result<(), Box<dyn error::Error>> {
if parse_settings(&matches)? {
let wallet_manager = maybe_wallet_manager()?;
@@ -237,6 +243,6 @@ fn main() -> Result<(), Box<dyn error::Error>> {
config.signers = signers.iter().map(|s| s.as_ref()).collect();
let result = process_command(&config)?;
println!("{}", result);
}
};
Ok(())
}

10
cli/src/nonce.rs
View File

@@ -238,7 +238,7 @@ pub fn get_account(
) -> Result<Account, CliNonceError> {
rpc_client
.get_account(nonce_pubkey)
.map_err(|e| CliNonceError::Client(format!("{:?}", e)))
.map_err(|e| CliNonceError::Client(format!("{}", e)))
.and_then(|a| match account_identity_ok(&a) {
Ok(()) => Ok(a),
Err(e) => Err(e),
@@ -441,7 +441,7 @@ pub fn process_authorize_nonce_account(
let nonce_authority = config.signers[nonce_authority];
let ix = authorize_nonce_account(nonce_account, &nonce_authority.pubkey(), new_authority);
let message = Message::new_with_payer(vec![ix], Some(&config.signers[0].pubkey()));
let message = Message::new_with_payer(&[ix], Some(&config.signers[0].pubkey()));
let mut tx = Transaction::new_unsigned(message);
tx.try_sign(&config.signers, recent_blockhash)?;
@@ -518,7 +518,7 @@ pub fn process_create_nonce_account(
let (recent_blockhash, fee_calculator) = rpc_client.get_recent_blockhash()?;
let message = Message::new_with_payer(ixs, Some(&config.signers[0].pubkey()));
let message = Message::new_with_payer(&ixs, Some(&config.signers[0].pubkey()));
let mut tx = Transaction::new_unsigned(message);
tx.try_sign(&config.signers, recent_blockhash)?;
@@ -560,7 +560,7 @@ pub fn process_new_nonce(
let nonce_authority = config.signers[nonce_authority];
let ix = advance_nonce_account(&nonce_account, &nonce_authority.pubkey());
let (recent_blockhash, fee_calculator) = rpc_client.get_recent_blockhash()?;
let message = Message::new_with_payer(vec![ix], Some(&config.signers[0].pubkey()));
let message = Message::new_with_payer(&[ix], Some(&config.signers[0].pubkey()));
let mut tx = Transaction::new_unsigned(message);
tx.try_sign(&config.signers, recent_blockhash)?;
check_account_for_fee(
@@ -633,7 +633,7 @@ pub fn process_withdraw_from_nonce_account(
destination_account_pubkey,
lamports,
);
let message = Message::new_with_payer(vec![ix], Some(&config.signers[0].pubkey()));
let message = Message::new_with_payer(&[ix], Some(&config.signers[0].pubkey()));
let mut tx = Transaction::new_unsigned(message);
tx.try_sign(&config.signers, recent_blockhash)?;
check_account_for_fee(

20
cli/src/stake.rs
View File

@@ -827,7 +827,7 @@ pub fn process_create_stake_account(
&nonce_authority.pubkey(),
)
} else {
Message::new_with_payer(ixs, Some(&fee_payer.pubkey()))
Message::new_with_payer(&ixs, Some(&fee_payer.pubkey()))
};
let mut tx = Transaction::new_unsigned(message);
tx.try_sign(&config.signers, recent_blockhash)?;
@@ -889,7 +889,7 @@ pub fn process_stake_authorize(
&nonce_authority.pubkey(),
)
} else {
Message::new_with_payer(ixs, Some(&fee_payer.pubkey()))
Message::new_with_payer(&ixs, Some(&fee_payer.pubkey()))
};
let mut tx = Transaction::new_unsigned(message);
tx.try_sign(&config.signers, recent_blockhash)?;
@@ -942,7 +942,7 @@ pub fn process_deactivate_stake_account(
&nonce_authority.pubkey(),
)
} else {
Message::new_with_payer(ixs, Some(&fee_payer.pubkey()))
Message::new_with_payer(&ixs, Some(&fee_payer.pubkey()))
};
let mut tx = Transaction::new_unsigned(message);
tx.try_sign(&config.signers, recent_blockhash)?;
@@ -1001,7 +1001,7 @@ pub fn process_withdraw_stake(
&nonce_authority.pubkey(),
)
} else {
Message::new_with_payer(ixs, Some(&fee_payer.pubkey()))
Message::new_with_payer(&ixs, Some(&fee_payer.pubkey()))
};
let mut tx = Transaction::new_unsigned(message);
tx.try_sign(&config.signers, recent_blockhash)?;
@@ -1134,7 +1134,7 @@ pub fn process_split_stake(
&nonce_authority.pubkey(),
)
} else {
Message::new_with_payer(ixs, Some(&fee_payer.pubkey()))
Message::new_with_payer(&ixs, Some(&fee_payer.pubkey()))
};
let mut tx = Transaction::new_unsigned(message);
tx.try_sign(&config.signers, recent_blockhash)?;
@@ -1190,7 +1190,7 @@ pub fn process_stake_set_lockup(
&nonce_authority.pubkey(),
)
} else {
Message::new_with_payer(ixs, Some(&fee_payer.pubkey()))
Message::new_with_payer(&ixs, Some(&fee_payer.pubkey()))
};
let mut tx = Transaction::new_unsigned(message);
tx.try_sign(&config.signers, recent_blockhash)?;
@@ -1300,7 +1300,7 @@ pub fn process_show_stake_account(
Ok("".to_string())
}
Err(err) => Err(CliError::RpcRequestError(format!(
"Account data could not be deserialized to stake state: {:?}",
"Account data could not be deserialized to stake state: {}",
err
))
.into()),
@@ -1396,11 +1396,11 @@ pub fn process_delegate_stake(
}
};
if sanity_check_result.is_err() {
if let Err(err) = &sanity_check_result {
if !force {
sanity_check_result?;
} else {
println!("--force supplied, ignoring: {:?}", sanity_check_result);
println!("--force supplied, ignoring: {}", err);
}
}
}
@@ -1424,7 +1424,7 @@ pub fn process_delegate_stake(
&nonce_authority.pubkey(),
)
} else {
Message::new_with_payer(ixs, Some(&fee_payer.pubkey()))
Message::new_with_payer(&ixs, Some(&fee_payer.pubkey()))
};
let mut tx = Transaction::new_unsigned(message);
tx.try_sign(&config.signers, recent_blockhash)?;

6
cli/src/storage.rs
View File

@@ -212,7 +212,7 @@ pub fn process_create_storage_account(
);
let (recent_blockhash, fee_calculator) = rpc_client.get_recent_blockhash()?;
let message = Message::new(ixs);
let message = Message::new(&ixs);
let mut tx = Transaction::new_unsigned(message);
tx.try_sign(&config.signers, recent_blockhash)?;
check_account_for_fee(
@@ -236,7 +236,7 @@ pub fn process_claim_storage_reward(
let instruction =
storage_instruction::claim_reward(node_account_pubkey, storage_account_pubkey);
let signers = [config.signers[0]];
let message = Message::new_with_payer(vec![instruction], Some(&signers[0].pubkey()));
let message = Message::new_with_payer(&[instruction], Some(&signers[0].pubkey()));
let mut tx = Transaction::new_unsigned(message);
tx.try_sign(&signers, recent_blockhash)?;
check_account_for_fee(
@@ -266,7 +266,7 @@ pub fn process_show_storage_account(
use solana_storage_program::storage_contract::StorageContract;
let storage_contract: StorageContract = account.state().map_err(|err| {
CliError::RpcRequestError(format!("Unable to deserialize storage account: {:?}", err))
CliError::RpcRequestError(format!("Unable to deserialize storage account: {}", err))
})?;
println!("{:#?}", storage_contract);
println!("Account Lamports: {}", account.lamports);

6
cli/src/validator_info.rs
View File

@@ -274,7 +274,7 @@ pub fn process_set_validator_info(
println!("--force supplied, ignoring: {:?}", result);
} else {
result.map_err(|err| {
CliError::BadParameter(format!("Invalid validator keybase username: {:?}", err))
CliError::BadParameter(format!("Invalid validator keybase username: {}", err))
})?;
}
}
@@ -339,7 +339,7 @@ pub fn process_set_validator_info(
&validator_info,
)]);
let signers = vec![config.signers[0], &info_keypair];
let message = Message::new(instructions);
let message = Message::new(&instructions);
(message, signers)
} else {
println!(
@@ -353,7 +353,7 @@ pub fn process_set_validator_info(
keys,
&validator_info,
)];
let message = Message::new_with_payer(instructions, Some(&config.signers[0].pubkey()));
let message = Message::new_with_payer(&instructions, Some(&config.signers[0].pubkey()));
let signers = vec![config.signers[0]];
(message, signers)
};

6
cli/src/vote.rs
View File

@@ -359,7 +359,7 @@ pub fn process_create_vote_account(
};
let (recent_blockhash, fee_calculator) = rpc_client.get_recent_blockhash()?;
let message = Message::new(ixs);
let message = Message::new(&ixs);
let mut tx = Transaction::new_unsigned(message);
tx.try_sign(&config.signers, recent_blockhash)?;
check_account_for_fee(
@@ -391,7 +391,7 @@ pub fn process_vote_authorize(
vote_authorize, // vote or withdraw
)];
let message = Message::new_with_payer(ixs, Some(&config.signers[0].pubkey()));
let message = Message::new_with_payer(&ixs, Some(&config.signers[0].pubkey()));
let mut tx = Transaction::new_unsigned(message);
tx.try_sign(&config.signers, recent_blockhash)?;
check_account_for_fee(
@@ -422,7 +422,7 @@ pub fn process_vote_update_validator(
new_identity_pubkey,
)];
let message = Message::new_with_payer(ixs, Some(&config.signers[0].pubkey()));
let message = Message::new_with_payer(&ixs, Some(&config.signers[0].pubkey()));
let mut tx = Transaction::new_unsigned(message);
tx.try_sign(&config.signers, recent_blockhash)?;
check_account_for_fee(