Skip to content

Commit

Permalink
keytool: Remove default key scheme as ed25519 (MystenLabs#4351)
Browse files Browse the repository at this point in the history
* keytool: remove default value

* update new-address command doc

* use SignatureScheme instead of u8 flag
  • Loading branch information
joyqvq authored Aug 31, 2022
1 parent 214d5ff commit 0e7ddb9
Show file tree
Hide file tree
Showing 10 changed files with 128 additions and 81 deletions.
3 changes: 2 additions & 1 deletion crates/sui-sdk/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@ This directory contains examples of interacting with a Move language smart contr
1. [Connect to Sui Devnet](https://github.com/MystenLabs/sui/blob/main/doc/src/build/devnet.md).
1. [Make sure you have two addresses with gas](https://github.com/MystenLabs/sui/blob/main/doc/src/build/cli-client.md#adding-accounts-to-the-client) by using the `new-address` command to create new addresses:
```shell
sui client new-address
sui client new-address ed25519
```
New address can be created with key scheme flag `{secp256k1 | ed25519}`.
You can skip this step if you are going to play with a friend. :)
1. [Request Sui tokens](https://github.com/MystenLabs/sui/blob/main/doc/src/build/install.md#sui-tokens) for all addresses that will be used to join the game.

Expand Down
12 changes: 6 additions & 6 deletions crates/sui-sdk/src/crypto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use rand::rngs::adapter::ReadRng;
use sui_types::base_types::SuiAddress;
use sui_types::crypto::{
get_key_pair_from_rng, random_key_pair_by_type_from_rng, EncodeDecodeBase64, PublicKey,
Signature, SuiKeyPair,
Signature, SignatureScheme, SuiKeyPair,
};

#[derive(Serialize, Deserialize)]
Expand Down Expand Up @@ -156,16 +156,16 @@ impl SuiKeystore {

pub fn generate_new_key(
&mut self,
key_scheme: Option<String>,
) -> Result<(SuiAddress, String, u8), anyhow::Error> {
key_scheme: SignatureScheme,
) -> Result<(SuiAddress, String, SignatureScheme), anyhow::Error> {
let mnemonic = Mnemonic::generate(12)?;
let seed = mnemonic.to_seed("");
let mut rng = RngWrapper(ReadRng::new(&seed));
match random_key_pair_by_type_from_rng(key_scheme, &mut rng) {
Ok((address, kp)) => {
let flag = kp.public().flag();
let k = kp.public();
self.0.add_key(kp)?;
Ok((address, mnemonic.to_string(), flag))
Ok((address, mnemonic.to_string(), k.scheme()))
}
Err(e) => Err(anyhow!("error generating key {:?}", e)),
}
Expand All @@ -186,7 +186,7 @@ impl SuiKeystore {
pub fn import_from_mnemonic(
&mut self,
phrase: &str,
key_scheme: Option<String>,
key_scheme: SignatureScheme,
) -> Result<SuiAddress, anyhow::Error> {
let seed = &Mnemonic::from_str(phrase).unwrap().to_seed("");
let mut rng = RngWrapper(ReadRng::new(seed));
Expand Down
14 changes: 9 additions & 5 deletions crates/sui-sdk/tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use sha3::{Digest, Sha3_256};
use tempfile::TempDir;

use sui_sdk::crypto::KeystoreType;
use sui_types::crypto::SuiSignatureInner;
use sui_types::crypto::{SignatureScheme, SuiSignatureInner};
use sui_types::{
base_types::{SuiAddress, SUI_ADDRESS_LENGTH},
crypto::Ed25519SuiSignature,
Expand All @@ -17,12 +17,14 @@ fn mnemonic_test() {
let keystore_path = temp_dir.path().join("sui.keystore");
let mut keystore = KeystoreType::File(keystore_path).init().unwrap();

let (address, phrase, flag) = keystore.generate_new_key(None).unwrap();
let (address, phrase, scheme) = keystore.generate_new_key(SignatureScheme::ED25519).unwrap();

let keystore_path_2 = temp_dir.path().join("sui2.keystore");
let mut keystore2 = KeystoreType::File(keystore_path_2).init().unwrap();
let imported_address = keystore2.import_from_mnemonic(&phrase, None).unwrap();
assert_eq!(flag, Ed25519SuiSignature::SCHEME.flag());
let imported_address = keystore2
.import_from_mnemonic(&phrase, SignatureScheme::ED25519)
.unwrap();
assert_eq!(scheme.flag(), Ed25519SuiSignature::SCHEME.flag());
assert_eq!(address, imported_address);
}

Expand All @@ -37,7 +39,9 @@ fn sui_wallet_address_mnemonic_test() -> Result<(), anyhow::Error> {
let keystore_path = temp_dir.path().join("sui.keystore");
let mut keystore = KeystoreType::File(keystore_path).init().unwrap();

keystore.import_from_mnemonic(phrase, None).unwrap();
keystore
.import_from_mnemonic(phrase, SignatureScheme::ED25519)
.unwrap();

let pubkey = keystore.keys()[0].clone();
assert_eq!(pubkey.flag(), Ed25519SuiSignature::SCHEME.flag());
Expand Down
60 changes: 50 additions & 10 deletions crates/sui-types/src/crypto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,12 @@ impl PublicKey {
PublicKey::Secp256k1KeyPair(_) => Secp256k1SuiSignature::SCHEME.flag(),
}
}
pub fn scheme(&self) -> SignatureScheme {
match self {
PublicKey::Ed25519KeyPair(_) => Ed25519SuiSignature::SCHEME,
PublicKey::Secp256k1KeyPair(_) => Secp256k1SuiSignature::SCHEME,
}
}
}

//
Expand Down Expand Up @@ -439,18 +445,18 @@ where

/// Wrapper function to return SuiKeypair based on key scheme string
pub fn random_key_pair_by_type(
key_scheme: Option<String>,
key_scheme: SignatureScheme,
) -> Result<(SuiAddress, SuiKeyPair), anyhow::Error> {
match key_scheme.as_deref() {
Some("secp256k1") => {
match key_scheme.to_string().as_ref() {
"secp256k1" => {
let (addr, key_pair): (_, Secp256k1KeyPair) = get_key_pair();
Ok((addr, SuiKeyPair::Secp256k1SuiKeyPair(key_pair)))
}
Some("ed25519") | None => {
"ed25519" => {
let (addr, key_pair): (_, Ed25519KeyPair) = get_key_pair();
Ok((addr, SuiKeyPair::Ed25519SuiKeyPair(key_pair)))
}
Some(_) => Err(anyhow!("Unrecognized key scheme")),
_ => Err(anyhow!("Unrecognized key scheme")),
}
}

Expand All @@ -474,22 +480,21 @@ where

/// Wrapper function to return SuiKeypair based on key scheme string with seedable rng.
pub fn random_key_pair_by_type_from_rng<R>(
key_scheme: Option<String>,
key_scheme: SignatureScheme,
csprng: &mut R,
) -> Result<(SuiAddress, SuiKeyPair), anyhow::Error>
where
R: rand::CryptoRng + rand::RngCore,
{
match key_scheme.as_deref() {
Some("secp256k1") => {
match key_scheme {
SignatureScheme::Secp256k1 => {
let (addr, key_pair): (_, Secp256k1KeyPair) = get_key_pair_from_rng(csprng);
Ok((addr, SuiKeyPair::Secp256k1SuiKeyPair(key_pair)))
}
Some("ed25519") | None => {
SignatureScheme::ED25519 => {
let (addr, key_pair): (_, Ed25519KeyPair) = get_key_pair_from_rng(csprng);
Ok((addr, SuiKeyPair::Ed25519SuiKeyPair(key_pair)))
}
Some(_) => Err(anyhow!("Unrecognized key scheme")),
}
}

Expand Down Expand Up @@ -1332,4 +1337,39 @@ impl SignatureScheme {
SignatureScheme::Secp256k1 => 0x01,
}
}

pub fn from_flag(flag: &str) -> Result<SignatureScheme, SuiError> {
let byte_int = flag
.parse::<u8>()
.map_err(|_| SuiError::KeyConversionError("Invalid key scheme".to_string()))?;
match byte_int {
0x00 => Ok(SignatureScheme::ED25519),
0x01 => Ok(SignatureScheme::Secp256k1),
_ => Err(SuiError::KeyConversionError(
"Invalid key scheme".to_string(),
)),
}
}
}

impl FromStr for SignatureScheme {
type Err = SuiError;
fn from_str(s: &str) -> Result<Self, Self::Err> {
match s {
"ed25519" => Ok(SignatureScheme::ED25519),
"secp256k1" => Ok(SignatureScheme::Secp256k1),
_ => Err(SuiError::KeyConversionError(
"Invalid key scheme".to_string(),
)),
}
}
}

impl ToString for SignatureScheme {
fn to_string(&self) -> String {
match self {
SignatureScheme::ED25519 => "ed25519".to_string(),
SignatureScheme::Secp256k1 => "secp256k1".to_string(),
}
}
}
20 changes: 12 additions & 8 deletions crates/sui/src/client_commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ use sui_json_rpc_types::{
use sui_json_rpc_types::{SuiCertifiedTransaction, SuiExecutionStatus, SuiTransactionEffects};
use sui_sdk::crypto::SuiKeystore;
use sui_sdk::{ClientType, SuiClient};
use sui_types::sui_serde::{Base64, Encoding};
use sui_types::{
base_types::{ObjectID, SuiAddress},
gas_coin::GasCoin,
Expand All @@ -35,6 +34,10 @@ use sui_types::{
object::Owner,
parse_sui_type_tag, SUI_FRAMEWORK_ADDRESS,
};
use sui_types::{
crypto::SignatureScheme,
sui_serde::{Base64, Encoding},
};
use tracing::info;

use crate::config::{Config, PersistedConfig, SuiClientConfig};
Expand Down Expand Up @@ -188,9 +191,9 @@ pub enum SuiClientCommands {
#[clap(name = "addresses")]
Addresses,

/// Generate new address and keypair, with optional keypair scheme {ed25519 | secp256k1}, default to ed25519.
/// Generate new address and keypair with keypair scheme flag {ed25519 | secp256k1}.
#[clap(name = "new-address")]
NewAddress { key_scheme: Option<String> },
NewAddress { key_scheme: SignatureScheme },

/// Obtain all objects owned by the address.
#[clap(name = "objects")]
Expand Down Expand Up @@ -407,8 +410,8 @@ impl SuiClientCommands {
SuiClientCommandResult::SyncClientState
}
SuiClientCommands::NewAddress { key_scheme } => {
let (address, phrase, flag) = context.keystore.generate_new_key(key_scheme)?;
SuiClientCommandResult::NewAddress((address, phrase, flag))
let (address, phrase, scheme) = context.keystore.generate_new_key(key_scheme)?;
SuiClientCommandResult::NewAddress((address, phrase, scheme))
}
SuiClientCommands::Gas { address } => {
let address = address.unwrap_or(context.active_address()?);
Expand Down Expand Up @@ -771,10 +774,11 @@ impl Display for SuiClientCommandResult {
SuiClientCommandResult::SyncClientState => {
writeln!(writer, "Client state sync complete.")?;
}
SuiClientCommandResult::NewAddress((address, recovery_phrase, flag)) => {
SuiClientCommandResult::NewAddress((address, recovery_phrase, scheme)) => {
writeln!(
writer,
"Created new keypair for address with flag {flag}: [{address}]"
"Created new keypair for address with scheme {:?}: [{address}]",
scheme
)?;
writeln!(writer, "Secret Recovery Phrase : [{recovery_phrase}]")?;
}
Expand Down Expand Up @@ -943,7 +947,7 @@ pub enum SuiClientCommandResult {
Addresses(Vec<SuiAddress>),
Objects(Vec<SuiObjectInfo>),
SyncClientState,
NewAddress((SuiAddress, String, u8)),
NewAddress((SuiAddress, String, SignatureScheme)),
Gas(Vec<GasCoin>),
SplitCoin(SuiTransactionResponse),
MergeCoin(SuiTransactionResponse),
Expand Down
19 changes: 8 additions & 11 deletions crates/sui/src/keytool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use sui_sdk::crypto::SuiKeystore;
use sui_types::base_types::SuiAddress;
use sui_types::base_types::{decode_bytes_hex, encode_bytes_hex};
use sui_types::crypto::{
random_key_pair_by_type, AuthorityKeyPair, EncodeDecodeBase64, SuiKeyPair,
random_key_pair_by_type, AuthorityKeyPair, EncodeDecodeBase64, SignatureScheme, SuiKeyPair,
};
use sui_types::sui_serde::{Base64, Encoding};

Expand All @@ -24,9 +24,9 @@ mod keytool_tests;
#[derive(Subcommand)]
#[clap(rename_all = "kebab-case")]
pub enum KeyToolCommand {
/// Generate a new keypair with optional keypair scheme flag, default using ed25519. Output file to current dir (to generate keypair to sui.keystore, use `sui client new-address`)
/// Generate a new keypair with keypair scheme flag {ed25519 | secp256k1}. Output file to current dir (to generate keypair to sui.keystore, use `sui client new-address`)
Generate {
key_scheme: Option<String>,
key_scheme: SignatureScheme,
},
Show {
file: PathBuf,
Expand All @@ -35,7 +35,7 @@ pub enum KeyToolCommand {
Unpack {
keypair: SuiKeyPair,
},
/// List all keys in the keystore
/// List all keys by its address, public key, key scheme in the keystore
List,
/// Create signature using the sui keystore and provided data.
Sign {
Expand All @@ -44,10 +44,10 @@ pub enum KeyToolCommand {
#[clap(long)]
data: String,
},
/// Import mnemonic phrase and generate keypair based on key scheme, default using ed25519.
/// Import mnemonic phrase and generate keypair based on key scheme flag {ed25519 | secp256k1}.
Import {
mnemonic_phrase: String,
key_scheme: Option<String>,
key_scheme: SignatureScheme,
},
/// This is a temporary helper function to ensure that testnet genesis does not break while
/// we transition towards BLS signatures.
Expand All @@ -60,15 +60,12 @@ impl KeyToolCommand {
pub fn execute(self, keystore: &mut SuiKeystore) -> Result<(), anyhow::Error> {
match self {
KeyToolCommand::Generate { key_scheme } => {
let k = key_scheme.clone();
let k = key_scheme.to_string();
match random_key_pair_by_type(key_scheme) {
Ok((address, keypair)) => {
let file_name = format!("{address}.key");
write_keypair_to_file(&keypair, &file_name)?;
println!(
"{:?} key generated and saved to '{file_name}'",
k.unwrap_or_else(|| "ed25519".to_string())
)
println!("{:?} key generated and saved to '{file_name}'", k);
}
Err(e) => {
println!("Failed to generate keypair: {:?}", e)
Expand Down
19 changes: 10 additions & 9 deletions crates/sui/src/sui_commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use sui_config::{
use sui_sdk::crypto::KeystoreType;
use sui_sdk::ClientType;
use sui_swarm::memory::Swarm;
use sui_types::crypto::{KeypairTraits, SuiKeyPair};
use sui_types::crypto::{KeypairTraits, SignatureScheme, SuiKeyPair};
use tracing::info;

#[allow(clippy::large_enum_variant)]
Expand Down Expand Up @@ -400,15 +400,16 @@ async fn prompt_if_no_config(wallet_conf_path: &Path) -> Result<(), anyhow::Erro
.unwrap_or(&sui_config_dir()?)
.join(SUI_KEYSTORE_FILENAME);
let keystore = KeystoreType::File(keystore_path);
println!("Generating keypair ...\n");

println!("Do you want to generate a secp256k1 keypair instead? [y/N] No will select Ed25519 by default. ");
let key_scheme = match read_line()?.trim() {
"y" => Some(String::from("secp256k1")),
_ => None,
println!("Select key scheme to generate keypair (0 for ed25519, 1 for secp256k1):");
let key_scheme = match SignatureScheme::from_flag(read_line()?.trim()) {
Ok(s) => s,
Err(e) => return Err(anyhow!("{e}")),
};
let (new_address, phrase, flag) = keystore.init()?.generate_new_key(key_scheme)?;
println!("Generated new keypair for address with flag {flag} [{new_address}]");
let (new_address, phrase, scheme) = keystore.init()?.generate_new_key(key_scheme)?;
println!(
"Generated new keypair for address with scheme {:?} [{new_address}]",
scheme.to_string()
);
println!("Secret Recovery Phrase : [{phrase}]");
SuiClientConfig {
keystore,
Expand Down
Loading

0 comments on commit 0e7ddb9

Please sign in to comment.