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

Better error messages for PoA chains #4034

Merged
merged 4 commits into from
Jan 5, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions ethcore/src/account_provider/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,11 @@ impl AccountProvider {
Ok(Address::from(address).into())
}

/// Checks whether an account with a given address is present.
pub fn has_account(&self, address: Address) -> Result<bool, Error> {
Ok(self.accounts()?.iter().any(|&a| a == address))
}

/// Returns addresses of all accounts.
pub fn accounts(&self) -> Result<Vec<Address>, Error> {
let accounts = self.sstore.accounts()?;
Expand Down
29 changes: 28 additions & 1 deletion parity/params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
// You should have received a copy of the GNU General Public License
// along with Parity. If not, see <http://www.gnu.org/licenses/>.

use std::{str, fs};
use std::{str, fs, fmt};
use std::time::Duration;
use util::{Address, U256, version_data};
use util::journaldb::Algorithm;
Expand Down Expand Up @@ -60,6 +60,21 @@ impl str::FromStr for SpecType {
}
}

impl fmt::Display for SpecType {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
f.write_str(match *self {
SpecType::Mainnet => "homestead",
SpecType::Morden => "morden",
SpecType::Ropsten => "ropsten",
SpecType::Olympic => "olympic",
SpecType::Classic => "classic",
SpecType::Expanse => "expanse",
SpecType::Dev => "dev",
SpecType::Custom(ref custom) => custom,
})
}
}

impl SpecType {
pub fn spec(&self) -> Result<Spec, String> {
match *self {
Expand Down Expand Up @@ -305,6 +320,18 @@ mod tests {
assert_eq!(SpecType::Mainnet, SpecType::default());
}

#[test]
fn test_spec_type_display() {
assert_eq!(format!("{}", SpecType::Mainnet), "homestead");
assert_eq!(format!("{}", SpecType::Ropsten), "ropsten");
assert_eq!(format!("{}", SpecType::Morden), "morden");
assert_eq!(format!("{}", SpecType::Olympic), "olympic");
assert_eq!(format!("{}", SpecType::Classic), "classic");
assert_eq!(format!("{}", SpecType::Expanse), "expanse");
assert_eq!(format!("{}", SpecType::Dev), "dev");
assert_eq!(format!("{}", SpecType::Custom("foo/bar".into())), "foo/bar");
}

#[test]
fn test_pruning_parsing() {
assert_eq!(Pruning::Auto, "auto".parse().unwrap());
Expand Down
44 changes: 37 additions & 7 deletions parity/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ const SNAPSHOT_PERIOD: u64 = 10000;
// how many blocks to wait before starting a periodic snapshot.
const SNAPSHOT_HISTORY: u64 = 100;

// Pops along with error messages when a password is missing or invalid.
const VERIFY_PASSWORD_HINT: &'static str = "Make sure valid password is present in files passed using `--password` or in the configuration file.";

#[derive(Debug, PartialEq)]
pub struct RunCmd {
pub cache_config: CacheConfig,
Expand Down Expand Up @@ -217,7 +220,7 @@ pub fn execute(cmd: RunCmd, can_restart: bool, logger: Arc<RotatingLogger>) -> R
let passwords = passwords_from_files(&cmd.acc_conf.password_files)?;

// prepare account provider
let account_provider = Arc::new(prepare_account_provider(&cmd.dirs, &spec.data_dir, cmd.acc_conf, &passwords)?);
let account_provider = Arc::new(prepare_account_provider(&cmd.spec, &cmd.dirs, &spec.data_dir, cmd.acc_conf, &passwords)?);

// let the Engine access the accounts
spec.engine.register_account_provider(account_provider.clone());
Expand All @@ -230,9 +233,21 @@ pub fn execute(cmd: RunCmd, can_restart: bool, logger: Arc<RotatingLogger>) -> R
miner.set_extra_data(cmd.miner_extras.extra_data);
miner.set_transactions_limit(cmd.miner_extras.transactions_limit);
let engine_signer = cmd.miner_extras.engine_signer;

if engine_signer != Default::default() {
// Check if engine signer exists
if !account_provider.has_account(engine_signer).unwrap_or(false) {
return Err(format!("Consensus signer account not found for the current chain. {}", build_create_account_hint(&cmd.spec, &cmd.dirs.keys)));
}

// Check if any passwords have been read from the password file(s)
if passwords.is_empty() {
return Err(format!("No password found for the consensus signer {}. {}", engine_signer, VERIFY_PASSWORD_HINT));
}

// Attempt to sign in the engine signer.
if !passwords.into_iter().any(|p| miner.set_engine_signer(engine_signer, p).is_ok()) {
return Err(format!("No password found for the consensus signer {}. Make sure valid password is present in files passed using `--password`.", engine_signer));
return Err(format!("No valid password for the consensus signer {}. {}", engine_signer, VERIFY_PASSWORD_HINT));
}
}

Expand Down Expand Up @@ -471,24 +486,39 @@ fn daemonize(_pid_file: String) -> Result<(), String> {
Err("daemon is no supported on windows".into())
}

fn prepare_account_provider(dirs: &Directories, data_dir: &str, cfg: AccountsConfig, passwords: &[String]) -> Result<AccountProvider, String> {
fn prepare_account_provider(spec: &SpecType, dirs: &Directories, data_dir: &str, cfg: AccountsConfig, passwords: &[String]) -> Result<AccountProvider, String> {
use ethcore::ethstore::EthStore;
use ethcore::ethstore::dir::DiskDirectory;

let path = dirs.keys_path(data_dir);
upgrade_key_location(&dirs.legacy_keys_path(cfg.testnet), &path);
let dir = Box::new(DiskDirectory::create(&path).map_err(|e| format!("Could not open keys directory: {}", e))?);
let account_service = AccountProvider::new(Box::new(
let account_provider = AccountProvider::new(Box::new(
EthStore::open_with_iterations(dir, cfg.iterations).map_err(|e| format!("Could not open keys directory: {}", e))?
));

for a in cfg.unlocked_accounts {
if !passwords.iter().any(|p| account_service.unlock_account_permanently(a, (*p).clone()).is_ok()) {
return Err(format!("No password found to unlock account {}. Make sure valid password is present in files passed using `--password`.", a));
// Check if the account exists
if !account_provider.has_account(a).unwrap_or(false) {
return Err(format!("Account {} not found for the current chain. {}", a, build_create_account_hint(spec, &dirs.keys)));
}

// Check if any passwords have been read from the password file(s)
if passwords.is_empty() {
return Err(format!("No password found to unlock account {}. {}", a, VERIFY_PASSWORD_HINT));
}

if !passwords.iter().any(|p| account_provider.unlock_account_permanently(a, (*p).clone()).is_ok()) {
return Err(format!("No valid password to unlock account {}. {}", a, VERIFY_PASSWORD_HINT));
}
}

Ok(account_service)
Ok(account_provider)
}

// Construct an error `String` with an adaptive hint on how to create an account.
fn build_create_account_hint(spec: &SpecType, keys: &str) -> String {
format!("You can create an account via RPC, UI or `parity account new --chain {} --keys-path {}`.", spec, keys)
}

fn wait_for_exit(
Expand Down