Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Improve subkey error reporting #4504

Merged
merged 1 commit into from
Dec 27, 2019
Merged

Improve subkey error reporting #4504

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions bin/utils/subkey/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ pallet-balances = { version = "2.0.0", path = "../../../frame/balances" }
pallet-transaction-payment = { version = "2.0.0", path = "../../../frame/transaction-payment" }
rpassword = "4.0.1"
itertools = "0.8.2"
derive_more = { version = "0.99.2" }

[features]
bench = []
182 changes: 106 additions & 76 deletions bin/utils/subkey/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use sp_core::{
};
use sp_runtime::{traits::{IdentifyAccount, Verify}, generic::Era};
use std::{
convert::{TryInto, TryFrom}, io::{stdin, Read}, str::FromStr, path::PathBuf, fs,
convert::{TryInto, TryFrom}, io::{stdin, Read}, str::FromStr, path::PathBuf, fs, fmt,
};

mod vanity;
Expand Down Expand Up @@ -234,12 +234,12 @@ fn get_app<'a, 'b>(usage: &'a str) -> App<'a, 'b> {
])
}

fn main() {
fn main() -> Result<(), Error> {
let usage = get_usage();
let matches = get_app(&usage).get_matches();

if matches.is_present("ed25519") {
return execute::<Ed25519>(matches)
return execute::<Ed25519>(matches);
}
if matches.is_present("secp256k1") {
return execute::<Ecdsa>(matches)
Expand All @@ -253,24 +253,41 @@ fn main() {
///
/// If the `URI` given as CLI argument is a file, the file content is taken as `URI`.
/// If no `URI` is given to the CLI, the user is prompted for it.
fn get_uri(match_name: &str, matches: &ArgMatches) -> String {
if let Some(uri) = matches.value_of(match_name) {
fn get_uri(match_name: &str, matches: &ArgMatches) -> Result<String, Error> {
let uri = if let Some(uri) = matches.value_of(match_name) {
let file = PathBuf::from(uri);
if file.is_file() {
fs::read_to_string(uri)
.expect(&format!("Failed to read `URI` file: {}", uri))
fs::read_to_string(uri)?
.trim_end()
.into()
} else {
uri.into()
}
} else {
rpassword::read_password_from_tty(Some("URI: "))
.expect("Failed to read `URI`")
rpassword::read_password_from_tty(Some("URI: "))?
};

Ok(uri)
}

#[derive(derive_more::Display, derive_more::From)]
Copy link
Member

Choose a reason for hiding this comment

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

Could you please change the function above to return an error as well? (get_uri)

enum Error {
Static(&'static str),
Io(std::io::Error),
Formatted(String),
}

impl fmt::Debug for Error {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
fmt::Display::fmt(self, f)
}
}

fn execute<C: Crypto>(matches: ArgMatches)
fn static_err(msg: &'static str) -> Result<(), Error> {
Err(Error::Static(msg))
}

fn execute<C: Crypto>(matches: ArgMatches) -> Result<(), Error>
where
SignatureOf<C>: SignatureT,
PublicOf<C>: PublicT,
Expand All @@ -279,77 +296,83 @@ where
let password = matches.value_of("password");

let password = if password.is_some() && password_interactive {
panic!("`--password` given and `--password-interactive` selected!");
return static_err("`--password` given and `--password-interactive` selected!");
} else if password_interactive {
Some(
rpassword::read_password_from_tty(Some("Key password: "))
.expect("Reads password from tty")
rpassword::read_password_from_tty(Some("Key password: "))?
)
} else {
password.map(Into::into)
};
let password = password.as_ref().map(String::as_str);

let maybe_network: Option<Ss58AddressFormat> = matches.value_of("network").map(|network| {
let maybe_network: Option<Ss58AddressFormat> = match matches.value_of("network").map(|network| {
network
.try_into()
.expect("Invalid network name. See --help for available networks.")
});
.map_err(|_| Error::Static("Invalid network name. See --help for available networks."))
}) {
Some(Err(e)) => return Err(e),
Some(Ok(v)) => Some(v),
None => None,
};

if let Some(network) = maybe_network {
set_default_ss58_version(network);
}
match matches.subcommand() {
("generate", Some(matches)) => {
let mnemonic = generate_mnemonic(matches);
let mnemonic = generate_mnemonic(matches)?;
C::print_from_uri(mnemonic.phrase(), password, maybe_network);
}
("inspect", Some(matches)) => {
C::print_from_uri(&get_uri("uri", &matches), password, maybe_network);
C::print_from_uri(&get_uri("uri", &matches)?, password, maybe_network);
}
("sign", Some(matches)) => {
let suri = get_uri("suri", &matches);
let suri = get_uri("suri", &matches)?;
let should_decode = matches.is_present("hex");
let message = read_message_from_stdin(should_decode);
let signature = do_sign::<C>(&suri, message, password);

let message = read_message_from_stdin(should_decode)?;
let signature = do_sign::<C>(&suri, message, password)?;
println!("{}", signature);
}
("verify", Some(matches)) => {
let uri = get_uri("uri", &matches);
let uri = get_uri("uri", &matches)?;
let should_decode = matches.is_present("hex");
let message = read_message_from_stdin(should_decode);
let is_valid_signature = do_verify::<C>(matches, &uri, message);

let message = read_message_from_stdin(should_decode)?;
let is_valid_signature = do_verify::<C>(matches, &uri, message)?;
if is_valid_signature {
println!("Signature verifies correctly.");
} else {
println!("Signature invalid.");
return static_err("Signature invalid.");
}
}
("vanity", Some(matches)) => {
let desired: String = matches
.value_of("pattern")
.map(str::to_string)
.unwrap_or_default();
let result = vanity::generate_key::<C>(&desired).expect("Key generation failed");
let result = vanity::generate_key::<C>(&desired)?;
let formated_seed = format_seed::<C>(result.seed);
C::print_from_uri(&formated_seed, None, maybe_network);
}
("transfer", Some(matches)) => {
let signer = read_pair::<C>(matches.value_of("from"), password);
let index = read_required_parameter::<Index>(matches, "index");
let genesis_hash = read_genesis_hash(matches);
let signer = read_pair::<C>(matches.value_of("from"), password)?;
let index = read_required_parameter::<Index>(matches, "index")?;
let genesis_hash = read_genesis_hash(matches)?;

let to: AccountId = read_account_id(matches.value_of("to"));
let amount = read_required_parameter::<Balance>(matches, "amount");
let amount = read_required_parameter::<Balance>(matches, "amount")?;
let function = Call::Balances(BalancesCall::transfer(to.into(), amount));

let extrinsic = create_extrinsic::<C>(function, index, signer, genesis_hash);

print_extrinsic(extrinsic);
}
("sign-transaction", Some(matches)) => {
let signer = read_pair::<C>(matches.value_of("suri"), password);
let index = read_required_parameter::<Index>(matches, "nonce");
let genesis_hash = read_genesis_hash(matches);
let signer = read_pair::<C>(matches.value_of("suri"), password)?;
let index = read_required_parameter::<Index>(matches, "nonce")?;
let genesis_hash = read_genesis_hash(matches)?;

let call = matches.value_of("call").expect("call is required; qed");
let function: Call = hex::decode(&call)
Expand All @@ -363,101 +386,107 @@ where
}
_ => print_usage(&matches),
}

Ok(())
}

/// Creates a new randomly generated mnemonic phrase.
fn generate_mnemonic(matches: &ArgMatches) -> Mnemonic {
let words = matches
.value_of("words")
.map(|x| usize::from_str(x).expect("Invalid number given for --words"))
.map(|x| {
MnemonicType::for_word_count(x)
.expect("Invalid number of words given for phrase: must be 12/15/18/21/24")
})
.unwrap_or(MnemonicType::Words12);
Mnemonic::new(words, Language::English)
}

fn do_sign<C: Crypto>(suri: &str, message: Vec<u8>, password: Option<&str>) -> String
fn generate_mnemonic(matches: &ArgMatches) -> Result<Mnemonic, Error> {
let words = match matches.value_of("words") {
Some(words) => {
let num = usize::from_str(words).map_err(|_| Error::Static("Invalid number given for --words"))?;
MnemonicType::for_word_count(num)
.map_err(|_| Error::Static("Invalid number of words given for phrase: must be 12/15/18/21/24"))?
},
None => MnemonicType::Words12,
};
Ok(Mnemonic::new(words, Language::English))
}

fn do_sign<C: Crypto>(suri: &str, message: Vec<u8>, password: Option<&str>) -> Result<String, Error>
where
SignatureOf<C>: SignatureT,
PublicOf<C>: PublicT,
{
let pair = read_pair::<C>(Some(suri), password);
let pair = read_pair::<C>(Some(suri), password)?;
let signature = pair.sign(&message);
format_signature::<C>(&signature)
Ok(format_signature::<C>(&signature))
}

fn do_verify<C: Crypto>(matches: &ArgMatches, uri: &str, message: Vec<u8>) -> bool
fn do_verify<C: Crypto>(matches: &ArgMatches, uri: &str, message: Vec<u8>) -> Result<bool, Error>
where
SignatureOf<C>: SignatureT,
PublicOf<C>: PublicT,
{
let signature = read_signature::<C>(matches);

let signature = read_signature::<C>(matches)?;
let pubkey = read_public_key::<C>(Some(uri));
<<C as Crypto>::Pair as Pair>::verify(&signature, &message, &pubkey)
Ok(<<C as Crypto>::Pair as Pair>::verify(&signature, &message, &pubkey))
}

fn decode_hex<T: AsRef<[u8]>>(message: T) -> Result<Vec<u8>, Error> {
hex::decode(message).map_err(|e| Error::Formatted(format!("Invalid hex ({})", e)))
}

fn read_message_from_stdin(should_decode: bool) -> Vec<u8> {
fn read_message_from_stdin(should_decode: bool) -> Result<Vec<u8>, Error> {
let mut message = vec![];
stdin()
.lock()
.read_to_end(&mut message)
.expect("Error reading from stdin");
.read_to_end(&mut message)?;
if should_decode {
message = hex::decode(&message).expect("Invalid hex in message");
message = decode_hex(&message)?;
}
message
Ok(message)
}

fn read_required_parameter<T: FromStr>(matches: &ArgMatches, name: &str) -> T where
fn read_required_parameter<T: FromStr>(matches: &ArgMatches, name: &str) -> Result<T, Error> where
<T as FromStr>::Err: std::fmt::Debug,
{
let str_value = matches
.value_of(name)
.expect("parameter is required; thus it can't be None; qed");
str::parse::<T>(str_value).unwrap_or_else(|_|
panic!("Invalid `{}' parameter; expecting an integer.", name)
str::parse::<T>(str_value).map_err(|_|
Error::Formatted(format!("Invalid `{}' parameter; expecting an integer.", name))
)
}

fn read_genesis_hash(matches: &ArgMatches) -> H256 {
fn read_genesis_hash(matches: &ArgMatches) -> Result<H256, Error> {
let genesis_hash: Hash = match matches.value_of("genesis").unwrap_or("alex") {
"elm" => hex!["10c08714a10c7da78f40a60f6f732cf0dba97acfb5e2035445b032386157d5c3"].into(),
"alex" => hex!["dcd1346701ca8396496e52aa2785b1748deb6db09551b72159dcb3e08991025b"].into(),
h => hex::decode(h)
.ok()
.and_then(|x| Decode::decode(&mut &x[..]).ok())
h => Decode::decode(&mut &decode_hex(h)?[..])
.expect("Invalid genesis hash or unrecognised chain identifier"),
};
println!(
"Using a genesis hash of {}",
HexDisplay::from(&genesis_hash.as_ref())
);
genesis_hash
Ok(genesis_hash)
}

fn read_signature<C: Crypto>(matches: &ArgMatches) -> SignatureOf<C> where
fn read_signature<C: Crypto>(matches: &ArgMatches) -> Result<SignatureOf<C>, Error>
where
SignatureOf<C>: SignatureT,
PublicOf<C>: PublicT,
{
let sig_data = matches
.value_of("sig")
.expect("signature parameter is required; thus it can't be None; qed");
let mut signature = <<C as Crypto>::Pair as Pair>::Signature::default();
let sig_data = hex::decode(sig_data).expect("signature is invalid hex");
let sig_data = decode_hex(sig_data)?;
if sig_data.len() != signature.as_ref().len() {
panic!(
return Err(Error::Formatted(format!(
"signature has an invalid length. read {} bytes, expected {} bytes",
sig_data.len(),
signature.as_ref().len(),
);
)));
}
signature.as_mut().copy_from_slice(&sig_data);
signature
Ok(signature)
}

fn read_public_key<C: Crypto>(matched_uri: Option<&str>) -> PublicOf<C> where
fn read_public_key<C: Crypto>(matched_uri: Option<&str>) -> PublicOf<C>
where
PublicOf<C>: PublicT,
{
let uri = matched_uri.expect("parameter is required; thus it can't be None; qed");
Expand Down Expand Up @@ -494,12 +523,12 @@ fn read_account_id(matched_uri: Option<&str>) -> AccountId {
fn read_pair<C: Crypto>(
matched_suri: Option<&str>,
password: Option<&str>,
) -> <C as Crypto>::Pair where
) -> Result<<C as Crypto>::Pair, Error> where
SignatureOf<C>: SignatureT,
PublicOf<C>: PublicT,
{
let suri = matched_suri.expect("parameter is required; thus it can't be None; qed");
C::pair_from_suri(suri, password)
let suri = matched_suri.ok_or(Error::Static("parameter is required; thus it can't be None; qed"))?;
Ok(C::pair_from_suri(suri, password))
}

fn format_signature<C: Crypto>(signature: &SignatureOf<C>) -> String {
Expand Down Expand Up @@ -591,7 +620,7 @@ mod tests {

let matches = app.clone().get_matches_from(arg_vec);
let matches = matches.subcommand().1.unwrap();
let mnemonic = generate_mnemonic(matches);
let mnemonic = generate_mnemonic(matches).expect("generate failed");

let (pair, seed) =
<<CryptoType as Crypto>::Pair as Pair>::from_phrase(mnemonic.phrase(), password)
Expand All @@ -601,14 +630,15 @@ mod tests {
let seed = format_seed::<CryptoType>(seed);
let message = "Blah Blah\n".as_bytes().to_vec();

let signature = do_sign::<CryptoType>(&seed, message.clone(), password);
let signature = do_sign::<CryptoType>(&seed, message.clone(), password).expect("signing failed");

// Verify the previous signature.
let arg_vec = vec!["subkey", "verify", &signature[..], &public_key[..]];

let matches = get_app(&usage).get_matches_from(arg_vec);
let matches = matches.subcommand().1.unwrap();
assert!(do_verify::<CryptoType>(matches, &public_key, message));

assert!(do_verify::<CryptoType>(matches, &public_key, message).expect("verify failed"));
}

#[test]
Expand Down
2 changes: 1 addition & 1 deletion bin/utils/subkey/src/vanity.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ fn calculate_score(_desired: &str, key: &str) -> usize {
0
}

pub(super) fn generate_key<C: Crypto>(desired: &str) -> Result<KeyPair<C>, &str> where
pub(super) fn generate_key<C: Crypto>(desired: &str) -> Result<KeyPair<C>, &'static str> where
PublicOf<C>: PublicT,
{
if desired.is_empty() {
Expand Down