Skip to content

Commit

Permalink
[types, consensus, crypto] Converting validator_(verifier|signer) and…
Browse files Browse the repository at this point in the history
… dependencies to the new API

## This diff modifies this behaviour:
- **Makes `ValidatorSigner`, `ValidatorVerifier` and their dependencies use the new API,**
  + most of the changes are in `types/validator_(signer|verifier).rs`, the rest is propagation
- Hides terms that should only be used in tests from our binaries, through conditional compilation
  + those are mostly `Clone` of `PrivateKey`s and `Arbitrary` impls, plus proptest strategies
  + this uses a `#[cfg(any(test, feature="testing")]` flag. `cfg(test)` works within-crate, `cfg(feature="testing")` helps transfer out-of-crate dependencies.
  + Introduces corresponding `"testing"` feature flag on a couple of lower crates (nextgen_crypto, types) so that you can import crates under a test context.
- `Validator(Verifier|Signer)::random()`, and `ValidatorSigner::new()` constructor now take  [optional arguments](http://xion.io/post/code/rust-optional-args.html) reflecting a default value.
- creating a `ValidatorSigner` does not need the public key any more and deduces it from the private key at constructor call (and then stores it).
- there's a compatibility `generate_keypair` for tests that has an optional pluggable rng argument with safe default,

## Why this is better:
- **`Validator(Verifier|Signer)` are safer, cryptographically agile, and expose one single true signing API which implementation is managed in the crypto module, irrespective of scheme.**
- random tests based on cryptographic material are deterministic (customizable rng with safe, set default)
- Cloning of Private Keys now can't happen outside of tests
- proptest-related terms and strategies do not bloat our binaries any more

## Why this is worse:
- temporarily this reintroduces a bincode dependency (end of `ed25519::compat`)
- there are ugly Legacy <-> Nextgen key material conversions, esp in `libra_node`, (parts of) `config`, `network`

## Future improvements:
- pull `libra_node`, `config`, `network` into this brave `nextgen_crypto` new world
- in this temporary state, Cloning of Private Keys still happen in tests — much can be removed

## TODO in next diffs:
- make a few more structures generic in a private / public Key parameter, esp. NodeConfig
  • Loading branch information
huitseeker authored and calibra-opensource committed Jul 19, 2019
1 parent 23e8bb6 commit 44764e5
Show file tree
Hide file tree
Showing 53 changed files with 710 additions and 452 deletions.
1 change: 1 addition & 0 deletions client/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ admission_control_proto = { version = "0.1.0", path = "../admission_control/admi
config = { path = "../config" }
crash_handler = { path = "../common/crash_handler" }
crypto = { path = "../crypto/legacy_crypto" }
nextgen_crypto = { path = "../crypto/nextgen_crypto" }
failure = { package = "failure_ext", path = "../common/failure_ext" }
libc = "0.2.48"
libra_wallet = { path = "./libra_wallet" }
Expand Down
7 changes: 6 additions & 1 deletion client/src/client_proxy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use futures::{future::Future, stream::Stream};
use hyper;
use libra_wallet::{io_utils, wallet_library::WalletLibrary};
use logger::prelude::*;
use nextgen_crypto::ed25519::Ed25519PublicKey;
use num_traits::{
cast::{FromPrimitive, ToPrimitive},
identities::Zero,
Expand Down Expand Up @@ -117,7 +118,11 @@ impl ClientProxy {
);
// Total 3f + 1 validators, 2f + 1 correct signatures are required.
// If < 4 validators, all validators have to agree.
let validator_verifier = Arc::new(ValidatorVerifier::new(validators));
let validator_pubkeys: HashMap<AccountAddress, Ed25519PublicKey> = validators
.into_iter()
.map(|(key, value)| (key, value.into()))
.collect();
let validator_verifier = Arc::new(ValidatorVerifier::new(validator_pubkeys));
let client = GRPCClient::new(host, ac_port, validator_verifier)?;

let accounts = vec![];
Expand Down
9 changes: 7 additions & 2 deletions client/src/grpc_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use failure::prelude::*;
use futures::Future;
use grpcio::{CallOption, ChannelBuilder, EnvBuilder};
use logger::prelude::*;
use nextgen_crypto::ed25519::*;
use proto_conv::{FromProto, IntoProto};
use std::sync::Arc;
use types::{
Expand All @@ -36,12 +37,16 @@ const MAX_GRPC_RETRY_COUNT: u64 = 1;
/// Struct holding dependencies of client.
pub struct GRPCClient {
client: AdmissionControlClient,
validator_verifier: Arc<ValidatorVerifier>,
validator_verifier: Arc<ValidatorVerifier<Ed25519PublicKey>>,
}

impl GRPCClient {
/// Construct a new Client instance.
pub fn new(host: &str, port: &str, validator_verifier: Arc<ValidatorVerifier>) -> Result<Self> {
pub fn new(
host: &str,
port: &str,
validator_verifier: Arc<ValidatorVerifier<Ed25519PublicKey>>,
) -> Result<Self> {
let conn_addr = format!("{}:{}", host, port);

// Create a GRPC client
Expand Down
1 change: 1 addition & 0 deletions config/config_builder/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ toml = "0.4"

config = { path = ".." }
crypto = { path = "../../crypto/legacy_crypto" }
nextgen_crypto = { path = "../../crypto/nextgen_crypto" }
failure = { path = "../../common/failure_ext", package = "failure_ext" }
generate_keypair = { path = "../generate_keypair" }
proto_conv = { path = "../../common/proto_conv", features = ["derive"] }
Expand Down
8 changes: 4 additions & 4 deletions config/config_builder/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use config::{
config::{NodeConfig, NodeConfigHelpers},
trusted_peers::{TrustedPeersConfig, TrustedPeersConfigHelpers},
};
use crypto::signing::KeyPair;
use crypto::signing::{self, KeyPair};
use failure::prelude::*;
use proto_conv::IntoProtoBytes;
use std::{convert::TryFrom, fs::File, io::prelude::*, path::Path};
Expand All @@ -23,8 +23,8 @@ pub fn gen_genesis_transaction<P: AsRef<Path>>(
.map(|(peer_id, peer)| {
ValidatorPublicKeys::new(
AccountAddress::try_from(peer_id.clone()).expect("[config] invalid peer_id"),
peer.get_consensus_public(),
peer.get_network_signing_public(),
peer.get_consensus_public().into(),
peer.get_network_signing_public().into(),
peer.get_network_identity_public(),
)
})
Expand All @@ -43,7 +43,7 @@ pub fn gen_genesis_transaction<P: AsRef<Path>>(
pub fn get_test_config() -> (NodeConfig, KeyPair) {
// TODO: test config should be moved here instead of config crate
let config = NodeConfigHelpers::get_single_node_test_config(true);
let (private_key, _) = ::crypto::signing::generate_keypair();
let (private_key, _) = signing::generate_keypair();
let keypair = KeyPair::new(private_key);

gen_genesis_transaction(
Expand Down
9 changes: 9 additions & 0 deletions consensus/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ canonical_serialization = { path = "../common/canonical_serialization" }
channel = { path = "../common/channel" }
config = { path = "../config" }
crypto = { path = "../crypto/legacy_crypto" }
nextgen_crypto = { path = "../crypto/nextgen_crypto" }
execution_proto = { path = "../execution/execution_proto" }
failure = { path = "../common/failure_ext", package = "failure_ext" }
grpc_helpers = { path = "../common/grpc_helpers" }
Expand Down Expand Up @@ -61,3 +62,11 @@ execution_service = { path = "../execution/execution_service" }
storage_service = { path = "../storage/storage_service" }
vm_genesis = { path = "../language/vm/vm_genesis" }
vm_validator = { path = "../vm_validator" }

[dev-dependencies.nextgen_crypto]
features = ["testing"]
path = "../crypto/nextgen_crypto"

[dev-dependencies.types]
features = ["testing"]
path = "../types"
7 changes: 4 additions & 3 deletions consensus/src/chained_bft/block_storage/block_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use logger::prelude::*;
use crate::{chained_bft::persistent_storage::RecoveryData, state_replication::StateComputeResult};
use crypto::hash::CryptoHash;
use mirai_annotations::checked_precondition;
use nextgen_crypto::ed25519::*;
use std::{
collections::{vec_deque::VecDeque, HashMap},
sync::{Arc, RwLock},
Expand Down Expand Up @@ -56,7 +57,7 @@ pub enum NeedFetchResult {
/// | -------------> D3
pub struct BlockStore<T> {
inner: Arc<RwLock<BlockTree<T>>>,
validator_signer: ValidatorSigner,
validator_signer: ValidatorSigner<Ed25519PrivateKey>,
state_computer: Arc<dyn StateComputer<Payload = T>>,
enforce_increasing_timestamps: bool,
/// The persistent storage backing up the in-memory data structure, every write should go
Expand All @@ -68,7 +69,7 @@ impl<T: Payload> BlockStore<T> {
pub async fn new(
storage: Arc<dyn PersistentStorage<T>>,
initial_data: RecoveryData<T>,
validator_signer: ValidatorSigner,
validator_signer: ValidatorSigner<Ed25519PrivateKey>,
state_computer: Arc<dyn StateComputer<Payload = T>>,
enforce_increasing_timestamps: bool,
max_pruned_blocks_in_mem: usize,
Expand Down Expand Up @@ -160,7 +161,7 @@ impl<T: Payload> BlockStore<T> {
*self.inner.write().unwrap() = tree;
}

pub fn signer(&self) -> &ValidatorSigner {
pub fn signer(&self) -> &ValidatorSigner<Ed25519PrivateKey> {
&self.validator_signer
}

Expand Down
30 changes: 16 additions & 14 deletions consensus/src/chained_bft/block_storage/block_store_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use crate::chained_bft::{
};
use crypto::HashValue;
use futures::executor::block_on;
use nextgen_crypto::{ed25519::*, *};
use proptest::prelude::*;
use std::{cmp::min, collections::HashSet, sync::Arc};
use types::{account_address::AccountAddress, validator_signer::ValidatorSigner};
Expand Down Expand Up @@ -162,16 +163,16 @@ proptest! {

#[test]
fn test_block_store_insert(
(keypairs, blocks) in block_test::block_forest_and_its_keys(
(mut private_keys, blocks) in block_test::block_forest_and_its_keys(
// quorum size
10,
// recursion depth
50)
){
let authors: HashSet<Author> = keypairs.iter().map(|(_, public_key)| AccountAddress::from(*public_key)).collect();
let (priv_key, pub_key) = keypairs.first().expect("several keypairs generated");
let signer = ValidatorSigner::new(AccountAddress::from(*pub_key), *pub_key, priv_key.clone());
let block_store = build_empty_tree_with_custom_signing(signer.clone());
let authors: HashSet<Author> = private_keys.iter().map(|private_key| AccountAddress::from_public_key(&private_key.public_key())).collect();
let priv_key = private_keys.pop().expect("several keypairs generated");
let signer = ValidatorSigner::new(None, priv_key);
let block_store = build_empty_tree_with_custom_signing(signer);
for block in blocks {
if block.round() > 0 && authors.contains(&block.author()) {
let known_parent = block_store.block_exists(block.parent_id());
Expand Down Expand Up @@ -312,17 +313,18 @@ fn test_insert_vote() {
let qc_size = 10;
let mut signers = vec![];
let mut author_public_keys = vec![];
for _ in 0..qc_size {
let signer = ValidatorSigner::random();

for i in 0..qc_size {
let signer = ValidatorSigner::<Ed25519PrivateKey>::random([i as u8; 32]);
author_public_keys.push((
AccountAddress::from(signer.public_key()),
AccountAddress::from_public_key(&signer.public_key()),
signer.public_key(),
));
signers.push(signer);
}
let my_signer = ValidatorSigner::random();
let my_signer = ValidatorSigner::random([qc_size as u8; 32]);
author_public_keys.push((
AccountAddress::from(my_signer.public_key()),
AccountAddress::from_public_key(&my_signer.public_key()),
my_signer.public_key(),
));
let block_store = build_empty_tree_with_custom_signing(my_signer);
Expand Down Expand Up @@ -425,13 +427,13 @@ fn test_need_fetch_for_qc() {
let a3 = inserter.insert_block(a2.as_ref(), 3);
block_on(block_tree.prune_tree(a2.id()));
let need_fetch_qc = placeholder_certificate_for_block(
vec![block_tree.signer().clone()],
vec![block_tree.signer()],
HashValue::zero(),
a3.round() + 1,
);
let too_old_qc = QuorumCert::certificate_for_genesis();
let can_insert_qc =
placeholder_certificate_for_block(vec![block_tree.signer().clone()], a3.id(), a3.round());
placeholder_certificate_for_block(vec![block_tree.signer()], a3.id(), a3.round());
let duplicate_qc = block_tree.get_quorum_cert_for_block(a2.id()).unwrap();
assert_eq!(
block_tree.need_fetch_for_quorum_cert(&need_fetch_qc),
Expand Down Expand Up @@ -464,7 +466,7 @@ fn test_need_sync_for_qc() {
let a3 = inserter.insert_block(a2.as_ref(), 3);
block_on(block_tree.prune_tree(a3.id()));
let qc = placeholder_certificate_for_block(
vec![block_tree.signer().clone()],
vec![block_tree.signer()],
HashValue::zero(),
a3.round() + 3,
);
Expand All @@ -473,7 +475,7 @@ fn test_need_sync_for_qc() {
true
);
let qc = placeholder_certificate_for_block(
vec![block_tree.signer().clone()],
vec![block_tree.signer()],
HashValue::zero(),
a3.round() + 2,
);
Expand Down
16 changes: 11 additions & 5 deletions consensus/src/chained_bft/chained_bft_consensus_provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use crate::{
txn_manager::MempoolProxy,
};
use network::validator_network::{ConsensusNetworkEvents, ConsensusNetworkSender};
use nextgen_crypto::ed25519::*;

use crate::{
chained_bft::{
Expand All @@ -34,10 +35,10 @@ use types::{

struct InitialSetup {
author: Author,
signer: ValidatorSigner,
signer: ValidatorSigner<Ed25519PrivateKey>,
quorum_size: usize,
peers: Arc<Vec<Author>>,
validator: Arc<ValidatorVerifier>,
validator: Arc<ValidatorVerifier<Ed25519PublicKey>>,
}

/// Supports the implementation of ConsensusProvider using LibraBFT.
Expand Down Expand Up @@ -115,16 +116,21 @@ impl ChainedBftProvider {
let author =
AccountAddress::try_from(peer_id_str).expect("Failed to parse peer id of a validator");
let private_key = node_config.base.peer_keypairs.get_consensus_private();
let public_key = node_config.base.peer_keypairs.get_consensus_public();
let signer = ValidatorSigner::new(author, public_key, private_key);
let _public_key = node_config.base.peer_keypairs.get_consensus_public();
let signer = ValidatorSigner::new(author, private_key.into());
let peers_with_public_keys = node_config.base.trusted_peers.get_trusted_consensus_peers();
let peers_with_nextgen_public_keys = peers_with_public_keys
.clone()
.into_iter()
.map(|(k, v)| (AccountAddress::clone(&k), v.into()))
.collect();
let peers = Arc::new(
peers_with_public_keys
.keys()
.map(AccountAddress::clone)
.collect(),
);
let validator = Arc::new(ValidatorVerifier::new(peers_with_public_keys));
let validator = Arc::new(ValidatorVerifier::new(peers_with_nextgen_public_keys));
counters::EPOCH_NUM.set(0); // No reconfiguration yet, so it is always zero
counters::CURRENT_EPOCH_NUM_VALIDATORS.set(validator.len() as i64);
counters::CURRENT_EPOCH_QUORUM_SIZE.set(validator.quorum_size() as i64);
Expand Down
Loading

0 comments on commit 44764e5

Please sign in to comment.