Skip to content

Commit

Permalink
[client] add reconfig command when we have mint key and avoid loading…
Browse files Browse the repository at this point in the history
… consensus_peers

We add the command for cli to support adding/removing validators when it
has the mint key.

We always start from empty consensus_peers and rely on storage to send
us back a proof, we'll need waypoint to avoid malicious node cheat.

Closes: diem#2002
Approved by: sblackshear
  • Loading branch information
zekun000 authored and bors-libra committed Dec 12, 2019
1 parent b83fb65 commit e55dc62
Show file tree
Hide file tree
Showing 11 changed files with 113 additions and 99 deletions.
56 changes: 25 additions & 31 deletions client/src/client_proxy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,10 @@
use crate::{commands::*, grpc_client::GRPCClient, AccountData, AccountStatus};
use admission_control_proto::proto::admission_control::SubmitTransactionRequest;
use anyhow::{bail, ensure, format_err, Error, Result};
use libra_config::config::{ConsensusPeersConfig, PersistableConfig};
use libra_crypto::{ed25519::*, test_utils::KeyPair};
use libra_logger::prelude::*;
use libra_tools::tempdir::TempPath;
use libra_types::crypto_proxies::EpochInfo;
use libra_types::crypto_proxies::{EpochInfo, ValidatorVerifier};
use libra_types::{
access_path::AccessPath,
account_address::{AccountAddress, ADDRESS_LENGTH},
Expand Down Expand Up @@ -107,19 +106,19 @@ impl ClientProxy {
pub fn new(
host: &str,
ac_port: u16,
validator_set_file: &str,
faucet_account_file: &str,
sync_on_wallet_recovery: bool,
faucet_server: Option<String>,
mnemonic_file: Option<String>,
) -> Result<Self> {
let verifier =
ConsensusPeersConfig::load_config(validator_set_file)?.get_validator_verifier();
ensure!(
!verifier.is_empty(),
"Not able to load any validators from trusted peers config!"
);
let client = GRPCClient::new(host, ac_port, EpochInfo { epoch: 1, verifier })?;
let client = GRPCClient::new(
host,
ac_port,
EpochInfo {
epoch: 0,
verifier: ValidatorVerifier::new(BTreeMap::new()),
},
)?;

let accounts = vec![];

Expand Down Expand Up @@ -302,9 +301,14 @@ impl ClientProxy {
/// Remove a existing validator.
pub fn remove_validator(
&mut self,
account_address: AccountAddress,
space_delim_strings: &[&str],
is_blocking: bool,
) -> Result<()> {
ensure!(
space_delim_strings.len() == 2,
"Invalid number of arguments for removing validator"
);
let account_address = self.get_account_address_from_parameter(space_delim_strings[1])?;
match self.faucet_account {
Some(_) => self.association_transaction_with_local_faucet_account(
transaction_builder::encode_remove_validator_script(&account_address),
Expand All @@ -315,11 +319,12 @@ impl ClientProxy {
}

/// Add a new validator.
pub fn add_validator(
&mut self,
account_address: AccountAddress,
is_blocking: bool,
) -> Result<()> {
pub fn add_validator(&mut self, space_delim_strings: &[&str], is_blocking: bool) -> Result<()> {
ensure!(
space_delim_strings.len() == 2,
"Invalid number of arguments for removing validator"
);
let account_address = self.get_account_address_from_parameter(space_delim_strings[1])?;
match self.faucet_account {
Some(_) => self.association_transaction_with_local_faucet_account(
transaction_builder::encode_add_validator_script(&account_address),
Expand All @@ -335,7 +340,6 @@ impl ClientProxy {
print!("waiting ");
loop {
stdout().flush().unwrap();
max_iterations -= 1;

match self
.client
Expand All @@ -348,16 +352,17 @@ impl ClientProxy {
}
break;
}
Ok(_) if max_iterations == 0 => {
panic!("wait_for_transaction timeout");
}
Err(e) => {
println!("Response with error: {:?}", e);
}
_ => {
print!(".");
}
}
max_iterations -= 1;
if max_iterations == 0 {
panic!("wait_for_transaction timeout");
}
thread::sleep(time::Duration::from_millis(10));
}
}
Expand Down Expand Up @@ -1130,7 +1135,6 @@ impl fmt::Display for AccountEntry {
#[cfg(test)]
mod tests {
use crate::client_proxy::{parse_bool, AddressAndIndex, ClientProxy};
use libra_config::config::{ConsensusConfig, PersistableConfig};
use libra_tools::tempdir::TempPath;
use libra_wallet::io_utils;
use proptest::prelude::*;
Expand All @@ -1141,21 +1145,11 @@ mod tests {
let file = TempPath::new();
let mnemonic_path = file.path().to_str().unwrap().to_string();

let consensus_config = ConsensusConfig::default();
let consensus_peer_file = TempPath::new();
let consensus_peers_path = consensus_peer_file.path();
consensus_config
.consensus_peers
.save_config(consensus_peers_path)
.expect("Unable to save consensus_peers.config");
let val_set_file = consensus_peers_path.to_str().unwrap().to_string();

// We don't need to specify host/port since the client won't be used to connect, only to
// generate random accounts
let mut client_proxy = ClientProxy::new(
"", /* host */
0, /* port */
&val_set_file,
&"",
false,
None,
Expand Down
56 changes: 56 additions & 0 deletions client/src/dev_commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ impl Command for DevCommand {
Box::new(DevCommandCompile {}),
Box::new(DevCommandPublish {}),
Box::new(DevCommandExecute {}),
Box::new(DevCommandAddValidator {}),
Box::new(DevCommandRemoveValidator {}),
];
subcommand_execute(&params[0], commands, client, &params[1..]);
}
Expand Down Expand Up @@ -104,3 +106,57 @@ impl Command for DevCommandExecute {
}
}
}

pub struct DevCommandAddValidator {}

impl Command for DevCommandAddValidator {
fn get_aliases(&self) -> Vec<&'static str> {
vec!["add_validator"]
}

fn get_params_help(&self) -> &'static str {
"<validator_account_address>"
}

fn get_description(&self) -> &'static str {
"Add an account address to the validator set"
}

fn execute(&self, client: &mut ClientProxy, params: &[&str]) {
if params.len() != 2 {
println!("Invalid number of arguments to add validator");
return;
}
match client.add_validator(params, true) {
Ok(_) => println!("Successfully finished execution"),
Err(e) => println!("{}", e),
}
}
}

pub struct DevCommandRemoveValidator {}

impl Command for DevCommandRemoveValidator {
fn get_aliases(&self) -> Vec<&'static str> {
vec!["remove_validator"]
}

fn get_params_help(&self) -> &'static str {
"<validator_account_address>"
}

fn get_description(&self) -> &'static str {
"Remove an existing account address from the validator set"
}

fn execute(&self, client: &mut ClientProxy, params: &[&str]) {
if params.len() != 2 {
println!("Invalid number of arguments to remove validator");
return;
}
match client.remove_validator(params, true) {
Ok(_) => println!("Successfully finished execution"),
Err(e) => println!("{}", e),
}
}
}
1 change: 1 addition & 0 deletions client/src/grpc_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ impl GRPCClient {
let resp = UpdateToLatestLedgerResponse::try_from(get_with_proof_resp?)?;
let mut wlock = known_version_and_epoch.lock().unwrap();
if let Some(new_epoch_info) = resp.verify(&wlock.1, &req)? {
info!("Trusted epoch change to :{}", new_epoch_info);
wlock.1 = new_epoch_info;
}
wlock.0 = resp.ledger_info_with_sigs.ledger_info().version();
Expand Down
32 changes: 5 additions & 27 deletions client/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,6 @@ struct Args {
/// directory.
#[structopt(short = "n", long)]
pub mnemonic_file: Option<String>,
/// File location from which to load config of trusted validators. It is used to verify
/// validator signatures in validator query response. The file should at least include public
/// key of all validators trusted by the client - which should typically be all validators on
/// the network. To connect to testnet, use 'libra/scripts/cli/consensus_peers.config.toml'.
/// Can be generated by config-builder for local testing:
/// `cargo run --bin config-builder`
/// But the preferred method is to simply use libra-swarm to run local networks
#[structopt(short = "s", long)]
pub validator_set_file: String,
/// If set, client will sync with validator during wallet recovery.
#[structopt(short = "r", long = "sync")]
pub sync: bool,
Expand All @@ -68,7 +59,6 @@ fn main() -> std::io::Result<()> {
let mut client_proxy = ClientProxy::new(
&args.host,
args.port.get(),
&args.validator_set_file,
&faucet_account_file,
args.sync,
args.faucet_server,
Expand Down Expand Up @@ -161,40 +151,28 @@ mod tests {

#[test]
fn test_args_port() {
let args = Args::from_iter(&["test", "--host=h", "--validator-set-file=vsf"]);
let args = Args::from_iter(&["test", "--host=h"]);
assert_eq!(args.port.get(), 8000);
assert_eq!(format!("{}:{}", args.host, args.port.get()), "h:8000");
let args = Args::from_iter(&[
"test",
"--port=65535",
"--host=h",
"--validator-set-file=vsf",
]);
let args = Args::from_iter(&["test", "--port=65535", "--host=h"]);
assert_eq!(args.port.get(), 65535);
}

#[test]
fn test_args_port_too_large() {
let result = Args::from_iter_safe(&[
"test",
"--port=65536",
"--host=h",
"--validator-set-file=vsf",
]);
let result = Args::from_iter_safe(&["test", "--port=65536", "--host=h"]);
assert_eq!(result.is_ok(), false);
}

#[test]
fn test_args_port_invalid() {
let result =
Args::from_iter_safe(&["test", "--port=abc", "--host=h", "--validator-set-file=vsf"]);
let result = Args::from_iter_safe(&["test", "--port=abc", "--host=h"]);
assert_eq!(result.is_ok(), false);
}

#[test]
fn test_args_port_zero() {
let result =
Args::from_iter_safe(&["test", "--port=0", "--host=h", "--validator-set-file=vsf"]);
let result = Args::from_iter_safe(&["test", "--port=0", "--host=h"]);
assert_eq!(result.is_ok(), false);
}
}
8 changes: 1 addition & 7 deletions libra-swarm/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,18 +148,12 @@ pub struct InProcessTestClient {
}

impl InProcessTestClient {
pub fn new(
port: u16,
faucet_key_file_path: &Path,
mnemonic_file_path: &str,
validator_set_file: String,
) -> Self {
pub fn new(port: u16, faucet_key_file_path: &Path, mnemonic_file_path: &str) -> Self {
let (_, alias_to_cmd) = commands::get_commands(true);
Self {
client: ClientProxy::new(
"localhost",
port,
&validator_set_file,
faucet_key_file_path
.canonicalize()
.expect("Unable to get canonical path of faucet key file")
Expand Down
17 changes: 0 additions & 17 deletions scripts/cli/consensus_peers.config.toml

This file was deleted.

2 changes: 1 addition & 1 deletion scripts/cli/start_cli_testnet.sh
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ source "$HOME/.cargo/env"

SCRIPT_PATH="$(dirname $0)"

RUN_PARAMS="--host ac.testnet.libra.org --port 8000 -s $SCRIPT_PATH/consensus_peers.config.toml"
RUN_PARAMS="--host ac.testnet.libra.org --port 8000 "
RELEASE=""

while [[ ! -z "$1" ]]; do
Expand Down
23 changes: 12 additions & 11 deletions testsuite/tests/libratest/smoke_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,13 +99,6 @@ impl TestEnvironment {
}

fn get_ac_client(&self, port: u16) -> ClientProxy {
let config = NodeConfig::load(&self.validator_swarm.config.config_files[0]).unwrap();
let validator_set_file = self
.validator_swarm
.dir
.as_ref()
.join("0")
.join(&config.consensus.consensus_peers_file);
let mnemonic_file_path = self
.mnemonic_file
.path()
Expand All @@ -119,7 +112,6 @@ impl TestEnvironment {
ClientProxy::new(
"localhost",
port,
validator_set_file.to_str().unwrap(),
&self.faucet_key.1,
false,
/* faucet server */ None,
Expand Down Expand Up @@ -721,8 +713,15 @@ fn test_e2e_reconfiguration() {
Decimal::from_f64(10.0),
Decimal::from_str(&client_proxy_0.get_balance(&["b", "0"]).unwrap()).ok()
);
let peer_id = env.get_validator(0).unwrap().validator_peer_id().unwrap();
client_proxy_1.remove_validator(peer_id, true).unwrap();
let peer_id = env
.get_validator(0)
.unwrap()
.validator_peer_id()
.unwrap()
.to_string();
client_proxy_1
.remove_validator(&["remove_validator", &peer_id], true)
.unwrap();
// mint another 10 coins after remove node 0
client_proxy_1
.mint_coins(&["mintb", "0", "10"], true)
Expand All @@ -737,7 +736,9 @@ fn test_e2e_reconfiguration() {
Decimal::from_str(&client_proxy_0.get_balance(&["b", "0"]).unwrap()).ok()
);
// Add the node back
client_proxy_1.add_validator(peer_id, true).unwrap();
client_proxy_1
.add_validator(&["add_validator", &peer_id], true)
.unwrap();
// Wait for it catches up, mint1 + remove + mint2 + add => seq == 4
client_proxy_0.wait_for_transaction(association_address(), 4);
assert_eq!(
Expand Down
Loading

0 comments on commit e55dc62

Please sign in to comment.