From e258cdf0731f27475225ab28946cf9c3687d10ac Mon Sep 17 00:00:00 2001 From: JerryKwan Date: Tue, 6 Sep 2022 20:33:50 -0700 Subject: [PATCH 1/4] add more validation and more error display add more validation when create AccountPrivateKeyProvider add more error display in deploying --- account/provider/src/private_key_provider.rs | 9 +++++++++ cmd/starcoin/src/cli_state.rs | 5 ++++- vm/move-package-manager/Cargo.toml | 1 + vm/move-package-manager/src/deployment.rs | 12 +++++++++++- 4 files changed, 25 insertions(+), 2 deletions(-) diff --git a/account/provider/src/private_key_provider.rs b/account/provider/src/private_key_provider.rs index 9f701dc9e6..6045260627 100644 --- a/account/provider/src/private_key_provider.rs +++ b/account/provider/src/private_key_provider.rs @@ -43,6 +43,15 @@ impl AccountPrivateKeyProvider { }; let private_key = AccountPrivateKey::from_encoded_string(data)?; + if address.is_some() { + if address.unwrap() != private_key.public_key().derived_address() { + bail!( + "account address {} and private key derived address {} are not equal", + address.unwrap(), + private_key.public_key().derived_address() + ); + } + } let address = address.unwrap_or_else(|| private_key.public_key().derived_address()); let _account = manager.import_account(address, private_key.to_bytes().to_vec(), "")?; Ok(Self { manager }) diff --git a/cmd/starcoin/src/cli_state.rs b/cmd/starcoin/src/cli_state.rs index 2ec087e9b5..085d126a74 100644 --- a/cmd/starcoin/src/cli_state.rs +++ b/cmd/starcoin/src/cli_state.rs @@ -287,7 +287,10 @@ impl CliState { TransactionStatusView::Executed ) { - eprintln!("txn dry run result:"); + eprintln!( + "txn dry run result: {:?}", + execute_result.dry_run_output.txn_output + ); return Ok(execute_result); } diff --git a/vm/move-package-manager/Cargo.toml b/vm/move-package-manager/Cargo.toml index 04434edf02..c436f90d9c 100644 --- a/vm/move-package-manager/Cargo.toml +++ b/vm/move-package-manager/Cargo.toml @@ -63,6 +63,7 @@ starcoin-types = {path = "../../types"} starcoin-vm-runtime = {path = "../vm-runtime", features = ["testing"]} starcoin-vm-types = {path = "../../vm/types"} stdlib = {path = "../stdlib"} +vm-status-translator = {path = "../../vm/vm-status-translator"} [dev-dependencies] diff --git a/vm/move-package-manager/src/deployment.rs b/vm/move-package-manager/src/deployment.rs index d395365788..267ec1050d 100644 --- a/vm/move-package-manager/src/deployment.rs +++ b/vm/move-package-manager/src/deployment.rs @@ -14,6 +14,7 @@ use starcoin_rpc_client::RpcClient; use starcoin_vm_types::transaction::TransactionPayload; use std::path::PathBuf; use std::time::Duration; +use vm_status_translator::VmStatusExplainView; #[derive(Parser)] pub struct DeploymentCommand { @@ -102,7 +103,16 @@ pub fn handle_deployment(_move_args: &Move, cmd: DeploymentCommand) -> anyhow::R let item = state.build_and_execute_transaction(cmd.txn_opts, TransactionPayload::Package(package)); match item { - Ok(_) => println!("The deployment is successful."), + Ok(execute_result_view) => { + // check dry run result + if VmStatusExplainView::Executed == execute_result_view.dry_run_output.explained_status + { + println!("The deployment is successful."); + } else { + println!("The deployment is failed. execute result view is: "); + println!("{:?}", execute_result_view); + } + } Err(e) => { println!("The deployment is failed. Reason: "); println!("{:?}", e); From 185524284061d6670fa25be8a600d13e932e95d3 Mon Sep 17 00:00:00 2001 From: JerryKwan Date: Tue, 6 Sep 2022 23:30:34 -0700 Subject: [PATCH 2/4] reformat code according suggestions reformat code according suggestions --- Cargo.lock | 1 + account/provider/src/private_key_provider.rs | 23 +++++++++++--------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6901db3596..03e002d39e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5355,6 +5355,7 @@ dependencies = [ "stdlib", "tempfile", "tokio 1.20.1", + "vm-status-translator", "walkdir", ] diff --git a/account/provider/src/private_key_provider.rs b/account/provider/src/private_key_provider.rs index 6045260627..65177aa37e 100644 --- a/account/provider/src/private_key_provider.rs +++ b/account/provider/src/private_key_provider.rs @@ -43,17 +43,20 @@ impl AccountPrivateKeyProvider { }; let private_key = AccountPrivateKey::from_encoded_string(data)?; - if address.is_some() { - if address.unwrap() != private_key.public_key().derived_address() { - bail!( - "account address {} and private key derived address {} are not equal", - address.unwrap(), - private_key.public_key().derived_address() - ); + let address = match address { + Some(address_value) => { + if address_value != private_key.public_key().derived_address() { + bail!( + "account address {} and private key derived address {} are not equal", + address_value, + private_key.public_key().derived_address() + ); + } + address_value } - } - let address = address.unwrap_or_else(|| private_key.public_key().derived_address()); - let _account = manager.import_account(address, private_key.to_bytes().to_vec(), "")?; + None => private_key.public_key().derived_address(), + }; + manager.import_account(address, private_key.to_bytes().to_vec(), "")?; Ok(Self { manager }) } } From 4521096cea5697319f380589bcca80c8c1b4d0de Mon Sep 17 00:00:00 2001 From: JerryKwan Date: Wed, 7 Sep 2022 01:16:22 -0700 Subject: [PATCH 3/4] remove address and private key validation remove address and private key validation when creating AccountPrivateKeyProvider --- account/provider/src/private_key_provider.rs | 314 +++++++++---------- 1 file changed, 151 insertions(+), 163 deletions(-) diff --git a/account/provider/src/private_key_provider.rs b/account/provider/src/private_key_provider.rs index 65177aa37e..1e625606ef 100644 --- a/account/provider/src/private_key_provider.rs +++ b/account/provider/src/private_key_provider.rs @@ -1,163 +1,151 @@ -use anyhow::{bail, Result}; -use starcoin_account::{account_storage::AccountStorage, AccountManager}; -use starcoin_account_api::{AccountInfo, AccountPrivateKey, AccountProvider}; -use starcoin_config::account_provider_config::G_ENV_PRIVATE_KEY; -use starcoin_crypto::{ValidCryptoMaterial, ValidCryptoMaterialStringExt}; -use starcoin_types::account_address::AccountAddress; -use starcoin_types::account_config::token_code::TokenCode; -use starcoin_types::genesis_config::ChainId; -use starcoin_types::sign_message::{SignedMessage, SigningMessage}; -use starcoin_types::transaction::{RawUserTransaction, SignedUserTransaction}; -use std::env; -use std::path::PathBuf; -use std::time::Duration; - -pub struct AccountPrivateKeyProvider { - manager: AccountManager, -} - -impl AccountPrivateKeyProvider { - pub fn create( - secret_file: Option, - address: Option, - from_env: bool, - chain_id: ChainId, - ) -> Result { - if !(secret_file.is_some() ^ from_env) { - bail!("Please input one and only one in args [secret_file, from_env].") - } - let storage = AccountStorage::mock(); - let manager = AccountManager::new(storage, chain_id)?; - - let data = if secret_file.is_some() { - std::fs::read_to_string(secret_file.unwrap())? - } else { - match env::var_os(G_ENV_PRIVATE_KEY) { - Some(value) => value.into_string().unwrap_or_else(|_| String::from("")), - None => String::from(""), - } - }; - let data = data.trim_end_matches('\n').trim_end_matches('\r'); - if data.is_empty() { - bail!("Invalid private key.") - }; - - let private_key = AccountPrivateKey::from_encoded_string(data)?; - let address = match address { - Some(address_value) => { - if address_value != private_key.public_key().derived_address() { - bail!( - "account address {} and private key derived address {} are not equal", - address_value, - private_key.public_key().derived_address() - ); - } - address_value - } - None => private_key.public_key().derived_address(), - }; - manager.import_account(address, private_key.to_bytes().to_vec(), "")?; - Ok(Self { manager }) - } -} -impl AccountProvider for AccountPrivateKeyProvider { - fn create_account(&self, _password: String) -> anyhow::Result { - bail!("Unsupported") - } - - fn get_default_account(&self) -> anyhow::Result> { - self.manager.default_account_info().map_err(|e| e.into()) - } - - fn set_default_account(&self, _address: AccountAddress) -> anyhow::Result { - bail!("Unsupported") - } - - fn get_accounts(&self) -> anyhow::Result> { - self.manager.list_account_infos().map_err(|e| e.into()) - } - - fn get_account(&self, address: AccountAddress) -> anyhow::Result> { - self.manager.account_info(address).map_err(|e| e.into()) - } - - fn sign_message( - &self, - address: AccountAddress, - message: SigningMessage, - ) -> anyhow::Result { - self.manager - .sign_message(address, message) - .map_err(|e| e.into()) - } - - fn sign_txn( - &self, - raw_txn: RawUserTransaction, - signer_address: AccountAddress, - ) -> anyhow::Result { - self.manager - .sign_txn(signer_address, raw_txn) - .map_err(|e| e.into()) - } - - fn unlock_account( - &self, - address: AccountAddress, - _password: String, - duration: Duration, - ) -> anyhow::Result { - self.manager - .unlock_account(address, "", duration) - .map_err(|e| e.into()) - } - - fn lock_account(&self, _address: AccountAddress) -> anyhow::Result { - bail!("Unsupported") - } - - fn import_account( - &self, - _address: AccountAddress, - _private_key: Vec, - _password: String, - ) -> anyhow::Result { - bail!("Unsupported") - } - - fn import_readonly_account( - &self, - _address: AccountAddress, - _public_key: Vec, - ) -> anyhow::Result { - bail!("Unsupported") - } - - fn export_account( - &self, - _address: AccountAddress, - _password: String, - ) -> anyhow::Result> { - bail!("Unsupported") - } - - fn accepted_tokens(&self, address: AccountAddress) -> anyhow::Result> { - self.manager.accepted_tokens(address).map_err(|e| e.into()) - } - - fn change_account_password( - &self, - _address: AccountAddress, - _new_password: String, - ) -> anyhow::Result { - bail!("Unsupported") - } - - fn remove_account( - &self, - _address: AccountAddress, - _password: Option, - ) -> anyhow::Result { - bail!("Unsupported") - } -} +use anyhow::{bail, Result}; +use starcoin_account::{account_storage::AccountStorage, AccountManager}; +use starcoin_account_api::{AccountInfo, AccountPrivateKey, AccountProvider}; +use starcoin_config::account_provider_config::G_ENV_PRIVATE_KEY; +use starcoin_crypto::{ValidCryptoMaterial, ValidCryptoMaterialStringExt}; +use starcoin_types::account_address::AccountAddress; +use starcoin_types::account_config::token_code::TokenCode; +use starcoin_types::genesis_config::ChainId; +use starcoin_types::sign_message::{SignedMessage, SigningMessage}; +use starcoin_types::transaction::{RawUserTransaction, SignedUserTransaction}; +use std::env; +use std::path::PathBuf; +use std::time::Duration; + +pub struct AccountPrivateKeyProvider { + manager: AccountManager, +} + +impl AccountPrivateKeyProvider { + pub fn create( + secret_file: Option, + address: Option, + from_env: bool, + chain_id: ChainId, + ) -> Result { + if !(secret_file.is_some() ^ from_env) { + bail!("Please input one and only one in args [secret_file, from_env].") + } + let storage = AccountStorage::mock(); + let manager = AccountManager::new(storage, chain_id)?; + + let data = if secret_file.is_some() { + std::fs::read_to_string(secret_file.unwrap())? + } else { + match env::var_os(G_ENV_PRIVATE_KEY) { + Some(value) => value.into_string().unwrap_or_else(|_| String::from("")), + None => String::from(""), + } + }; + let data = data.trim_end_matches('\n').trim_end_matches('\r'); + if data.is_empty() { + bail!("Invalid private key.") + }; + + let private_key = AccountPrivateKey::from_encoded_string(data)?; + let address = address.unwrap_or_else(|| private_key.public_key().derived_address()); + let _account = manager.import_account(address, private_key.to_bytes().to_vec(), "")?; + Ok(Self { manager }) + } +} +impl AccountProvider for AccountPrivateKeyProvider { + fn create_account(&self, _password: String) -> anyhow::Result { + bail!("Unsupported") + } + + fn get_default_account(&self) -> anyhow::Result> { + self.manager.default_account_info().map_err(|e| e.into()) + } + + fn set_default_account(&self, _address: AccountAddress) -> anyhow::Result { + bail!("Unsupported") + } + + fn get_accounts(&self) -> anyhow::Result> { + self.manager.list_account_infos().map_err(|e| e.into()) + } + + fn get_account(&self, address: AccountAddress) -> anyhow::Result> { + self.manager.account_info(address).map_err(|e| e.into()) + } + + fn sign_message( + &self, + address: AccountAddress, + message: SigningMessage, + ) -> anyhow::Result { + self.manager + .sign_message(address, message) + .map_err(|e| e.into()) + } + + fn sign_txn( + &self, + raw_txn: RawUserTransaction, + signer_address: AccountAddress, + ) -> anyhow::Result { + self.manager + .sign_txn(signer_address, raw_txn) + .map_err(|e| e.into()) + } + + fn unlock_account( + &self, + address: AccountAddress, + _password: String, + duration: Duration, + ) -> anyhow::Result { + self.manager + .unlock_account(address, "", duration) + .map_err(|e| e.into()) + } + + fn lock_account(&self, _address: AccountAddress) -> anyhow::Result { + bail!("Unsupported") + } + + fn import_account( + &self, + _address: AccountAddress, + _private_key: Vec, + _password: String, + ) -> anyhow::Result { + bail!("Unsupported") + } + + fn import_readonly_account( + &self, + _address: AccountAddress, + _public_key: Vec, + ) -> anyhow::Result { + bail!("Unsupported") + } + + fn export_account( + &self, + _address: AccountAddress, + _password: String, + ) -> anyhow::Result> { + bail!("Unsupported") + } + + fn accepted_tokens(&self, address: AccountAddress) -> anyhow::Result> { + self.manager.accepted_tokens(address).map_err(|e| e.into()) + } + + fn change_account_password( + &self, + _address: AccountAddress, + _new_password: String, + ) -> anyhow::Result { + bail!("Unsupported") + } + + fn remove_account( + &self, + _address: AccountAddress, + _password: Option, + ) -> anyhow::Result { + bail!("Unsupported") + } +} From fc0e154623be4d18b3b404c49bed4f21317b2bea Mon Sep 17 00:00:00 2001 From: JerryKwan Date: Wed, 7 Sep 2022 02:57:31 -0700 Subject: [PATCH 4/4] change line separator change line separator --- account/provider/src/private_key_provider.rs | 302 +++++++++---------- 1 file changed, 151 insertions(+), 151 deletions(-) diff --git a/account/provider/src/private_key_provider.rs b/account/provider/src/private_key_provider.rs index 1e625606ef..9f701dc9e6 100644 --- a/account/provider/src/private_key_provider.rs +++ b/account/provider/src/private_key_provider.rs @@ -1,151 +1,151 @@ -use anyhow::{bail, Result}; -use starcoin_account::{account_storage::AccountStorage, AccountManager}; -use starcoin_account_api::{AccountInfo, AccountPrivateKey, AccountProvider}; -use starcoin_config::account_provider_config::G_ENV_PRIVATE_KEY; -use starcoin_crypto::{ValidCryptoMaterial, ValidCryptoMaterialStringExt}; -use starcoin_types::account_address::AccountAddress; -use starcoin_types::account_config::token_code::TokenCode; -use starcoin_types::genesis_config::ChainId; -use starcoin_types::sign_message::{SignedMessage, SigningMessage}; -use starcoin_types::transaction::{RawUserTransaction, SignedUserTransaction}; -use std::env; -use std::path::PathBuf; -use std::time::Duration; - -pub struct AccountPrivateKeyProvider { - manager: AccountManager, -} - -impl AccountPrivateKeyProvider { - pub fn create( - secret_file: Option, - address: Option, - from_env: bool, - chain_id: ChainId, - ) -> Result { - if !(secret_file.is_some() ^ from_env) { - bail!("Please input one and only one in args [secret_file, from_env].") - } - let storage = AccountStorage::mock(); - let manager = AccountManager::new(storage, chain_id)?; - - let data = if secret_file.is_some() { - std::fs::read_to_string(secret_file.unwrap())? - } else { - match env::var_os(G_ENV_PRIVATE_KEY) { - Some(value) => value.into_string().unwrap_or_else(|_| String::from("")), - None => String::from(""), - } - }; - let data = data.trim_end_matches('\n').trim_end_matches('\r'); - if data.is_empty() { - bail!("Invalid private key.") - }; - - let private_key = AccountPrivateKey::from_encoded_string(data)?; - let address = address.unwrap_or_else(|| private_key.public_key().derived_address()); - let _account = manager.import_account(address, private_key.to_bytes().to_vec(), "")?; - Ok(Self { manager }) - } -} -impl AccountProvider for AccountPrivateKeyProvider { - fn create_account(&self, _password: String) -> anyhow::Result { - bail!("Unsupported") - } - - fn get_default_account(&self) -> anyhow::Result> { - self.manager.default_account_info().map_err(|e| e.into()) - } - - fn set_default_account(&self, _address: AccountAddress) -> anyhow::Result { - bail!("Unsupported") - } - - fn get_accounts(&self) -> anyhow::Result> { - self.manager.list_account_infos().map_err(|e| e.into()) - } - - fn get_account(&self, address: AccountAddress) -> anyhow::Result> { - self.manager.account_info(address).map_err(|e| e.into()) - } - - fn sign_message( - &self, - address: AccountAddress, - message: SigningMessage, - ) -> anyhow::Result { - self.manager - .sign_message(address, message) - .map_err(|e| e.into()) - } - - fn sign_txn( - &self, - raw_txn: RawUserTransaction, - signer_address: AccountAddress, - ) -> anyhow::Result { - self.manager - .sign_txn(signer_address, raw_txn) - .map_err(|e| e.into()) - } - - fn unlock_account( - &self, - address: AccountAddress, - _password: String, - duration: Duration, - ) -> anyhow::Result { - self.manager - .unlock_account(address, "", duration) - .map_err(|e| e.into()) - } - - fn lock_account(&self, _address: AccountAddress) -> anyhow::Result { - bail!("Unsupported") - } - - fn import_account( - &self, - _address: AccountAddress, - _private_key: Vec, - _password: String, - ) -> anyhow::Result { - bail!("Unsupported") - } - - fn import_readonly_account( - &self, - _address: AccountAddress, - _public_key: Vec, - ) -> anyhow::Result { - bail!("Unsupported") - } - - fn export_account( - &self, - _address: AccountAddress, - _password: String, - ) -> anyhow::Result> { - bail!("Unsupported") - } - - fn accepted_tokens(&self, address: AccountAddress) -> anyhow::Result> { - self.manager.accepted_tokens(address).map_err(|e| e.into()) - } - - fn change_account_password( - &self, - _address: AccountAddress, - _new_password: String, - ) -> anyhow::Result { - bail!("Unsupported") - } - - fn remove_account( - &self, - _address: AccountAddress, - _password: Option, - ) -> anyhow::Result { - bail!("Unsupported") - } -} +use anyhow::{bail, Result}; +use starcoin_account::{account_storage::AccountStorage, AccountManager}; +use starcoin_account_api::{AccountInfo, AccountPrivateKey, AccountProvider}; +use starcoin_config::account_provider_config::G_ENV_PRIVATE_KEY; +use starcoin_crypto::{ValidCryptoMaterial, ValidCryptoMaterialStringExt}; +use starcoin_types::account_address::AccountAddress; +use starcoin_types::account_config::token_code::TokenCode; +use starcoin_types::genesis_config::ChainId; +use starcoin_types::sign_message::{SignedMessage, SigningMessage}; +use starcoin_types::transaction::{RawUserTransaction, SignedUserTransaction}; +use std::env; +use std::path::PathBuf; +use std::time::Duration; + +pub struct AccountPrivateKeyProvider { + manager: AccountManager, +} + +impl AccountPrivateKeyProvider { + pub fn create( + secret_file: Option, + address: Option, + from_env: bool, + chain_id: ChainId, + ) -> Result { + if !(secret_file.is_some() ^ from_env) { + bail!("Please input one and only one in args [secret_file, from_env].") + } + let storage = AccountStorage::mock(); + let manager = AccountManager::new(storage, chain_id)?; + + let data = if secret_file.is_some() { + std::fs::read_to_string(secret_file.unwrap())? + } else { + match env::var_os(G_ENV_PRIVATE_KEY) { + Some(value) => value.into_string().unwrap_or_else(|_| String::from("")), + None => String::from(""), + } + }; + let data = data.trim_end_matches('\n').trim_end_matches('\r'); + if data.is_empty() { + bail!("Invalid private key.") + }; + + let private_key = AccountPrivateKey::from_encoded_string(data)?; + let address = address.unwrap_or_else(|| private_key.public_key().derived_address()); + let _account = manager.import_account(address, private_key.to_bytes().to_vec(), "")?; + Ok(Self { manager }) + } +} +impl AccountProvider for AccountPrivateKeyProvider { + fn create_account(&self, _password: String) -> anyhow::Result { + bail!("Unsupported") + } + + fn get_default_account(&self) -> anyhow::Result> { + self.manager.default_account_info().map_err(|e| e.into()) + } + + fn set_default_account(&self, _address: AccountAddress) -> anyhow::Result { + bail!("Unsupported") + } + + fn get_accounts(&self) -> anyhow::Result> { + self.manager.list_account_infos().map_err(|e| e.into()) + } + + fn get_account(&self, address: AccountAddress) -> anyhow::Result> { + self.manager.account_info(address).map_err(|e| e.into()) + } + + fn sign_message( + &self, + address: AccountAddress, + message: SigningMessage, + ) -> anyhow::Result { + self.manager + .sign_message(address, message) + .map_err(|e| e.into()) + } + + fn sign_txn( + &self, + raw_txn: RawUserTransaction, + signer_address: AccountAddress, + ) -> anyhow::Result { + self.manager + .sign_txn(signer_address, raw_txn) + .map_err(|e| e.into()) + } + + fn unlock_account( + &self, + address: AccountAddress, + _password: String, + duration: Duration, + ) -> anyhow::Result { + self.manager + .unlock_account(address, "", duration) + .map_err(|e| e.into()) + } + + fn lock_account(&self, _address: AccountAddress) -> anyhow::Result { + bail!("Unsupported") + } + + fn import_account( + &self, + _address: AccountAddress, + _private_key: Vec, + _password: String, + ) -> anyhow::Result { + bail!("Unsupported") + } + + fn import_readonly_account( + &self, + _address: AccountAddress, + _public_key: Vec, + ) -> anyhow::Result { + bail!("Unsupported") + } + + fn export_account( + &self, + _address: AccountAddress, + _password: String, + ) -> anyhow::Result> { + bail!("Unsupported") + } + + fn accepted_tokens(&self, address: AccountAddress) -> anyhow::Result> { + self.manager.accepted_tokens(address).map_err(|e| e.into()) + } + + fn change_account_password( + &self, + _address: AccountAddress, + _new_password: String, + ) -> anyhow::Result { + bail!("Unsupported") + } + + fn remove_account( + &self, + _address: AccountAddress, + _password: Option, + ) -> anyhow::Result { + bail!("Unsupported") + } +}