diff --git a/crates/ethcore/src/engines/hbbft/contracts/keygen_history.rs b/crates/ethcore/src/engines/hbbft/contracts/keygen_history.rs index e008790c2..4ebcd1a87 100644 --- a/crates/ethcore/src/engines/hbbft/contracts/keygen_history.rs +++ b/crates/ethcore/src/engines/hbbft/contracts/keygen_history.rs @@ -179,6 +179,13 @@ pub struct KeyPairWrapper { pub inner: Arc>>>, } +impl PublicWrapper { + /// Check if the public key is valid. + pub fn is_valid(&self) -> bool { + self.encrypt(b"a", &mut rand::thread_rng()).is_ok() + } +} + impl<'a> PublicKey for PublicWrapper { type Error = crypto::publickey::Error; type SecretKey = KeyPairWrapper; diff --git a/crates/ethcore/src/engines/hbbft/hbbft_engine.rs b/crates/ethcore/src/engines/hbbft/hbbft_engine.rs index 64ee9d24e..a235d92ba 100644 --- a/crates/ethcore/src/engines/hbbft/hbbft_engine.rs +++ b/crates/ethcore/src/engines/hbbft/hbbft_engine.rs @@ -1009,6 +1009,7 @@ impl HoneyBadgerBFT { Some(client_arc) => { if self.is_syncing(&client_arc) { // we are syncing - do not do anything. + trace!(target: "engine", "do_validator_engine_actions: skipping because we are syncing."); return Ok(()); } diff --git a/crates/ethcore/src/engines/hbbft/keygen_transactions.rs b/crates/ethcore/src/engines/hbbft/keygen_transactions.rs index e0d6ee567..440b7c479 100644 --- a/crates/ethcore/src/engines/hbbft/keygen_transactions.rs +++ b/crates/ethcore/src/engines/hbbft/keygen_transactions.rs @@ -16,12 +16,14 @@ use engines::{ }, signer::EngineSigner, }; -use ethereum_types::{Address, U256}; +use ethereum_types::{Address, Public, U256}; use itertools::Itertools; use parking_lot::RwLock; use std::{collections::BTreeMap, sync::Arc}; use types::ids::BlockId; +use crate::client::BlockChainClient; + pub struct KeygenTransactionSender { last_keygen_mode: KeyGenMode, keygen_mode_counter: u64, @@ -36,6 +38,21 @@ enum ShouldSendKeyAnswer { Yes, } +#[derive(Debug)] +pub enum KeyGenError { + NoSigner, + NoFullClient, + NoPartToWrite, + CallError(CallError), + Unexpected, +} + +impl From for KeyGenError { + fn from(e: CallError) -> Self { + KeyGenError::CallError(e) + } +} + static KEYGEN_TRANSACTION_SEND_DELAY: u64 = 3; static KEYGEN_TRANSACTION_RESEND_DELAY: u64 = 10; @@ -102,17 +119,17 @@ impl KeygenTransactionSender { &mut self, client: &dyn EngineClient, signer: &Arc>>>, - ) -> Result<(), CallError> { + ) -> Result<(), KeyGenError> { // If we have no signer there is nothing for us to send. let address = match signer.read().as_ref() { Some(signer) => signer.address(), None => { warn!(target: "engine", "Could not send keygen transactions, because signer module could not be retrieved"); - return Err(CallError::ReturnValueInvalid); + return Err(KeyGenError::NoSigner); } }; - let full_client = client.as_full_client().ok_or(CallError::NotFullClient)?; + let full_client = client.as_full_client().ok_or(KeyGenError::NoFullClient)?; // If the chain is still syncing, do not send Parts or Acks. if full_client.is_major_syncing() { @@ -122,30 +139,82 @@ impl KeygenTransactionSender { trace!(target:"engine", " get_validator_pubkeys..."); - let vmap = get_validator_pubkeys(&*client, BlockId::Latest, ValidatorType::Pending)?; + let vmap = get_validator_pubkeys(&*client, BlockId::Latest, ValidatorType::Pending) + .map_err(|e| KeyGenError::CallError(e))?; let pub_keys: BTreeMap<_, _> = vmap .values() .map(|p| (*p, PublicWrapper { inner: p.clone() })) .collect(); + let pub_keys_arc = Arc::new(pub_keys); + let upcoming_epoch = + get_posdao_epoch(client, BlockId::Latest).map_err(|e| KeyGenError::CallError(e))? + 1; + + //let pub_key_len = pub_keys.len(); // if synckeygen creation fails then either signer or validator pub keys are problematic. // Todo: We should expect up to f clients to write invalid pub keys. Report and re-start pending validator set selection. - let (mut synckeygen, part) = engine_signer_to_synckeygen(signer, Arc::new(pub_keys)) - .map_err(|e| { - warn!(target:"engine", "engine_signer_to_synckeygen error {:?}", e); - CallError::ReturnValueInvalid - })?; + let (mut synckeygen, part) = match engine_signer_to_synckeygen(signer, pub_keys_arc.clone()) + { + Ok((mut synckeygen_, part_)) => (synckeygen_, part_), + Err(e) => { + warn!(target:"engine", "engine_signer_to_synckeygen pub keys count {:?} error {:?}", pub_keys_arc.len(), e); + //let mut failure_pub_keys: Vec = Vec::new(); + let mut failure_pub_keys: Vec = Vec::new(); + pub_keys_arc.iter().for_each(|(k, v)| { + warn!(target:"engine", "pub key {}", k.as_bytes().iter().join("")); + + if !v.is_valid() { + warn!(target:"engine", "INVALID pub key {}", k); + + // append the bytes of the public key to the failure_pub_keys. + k.as_bytes().iter().for_each(|b| { + failure_pub_keys.push(*b); + }); + } + }); + + // if we should send our parts, we will send the public keys of the troublemakers instead. + + match self + .should_send_part(client, &address) + .map_err(|e| KeyGenError::CallError(e))? + { + ShouldSendKeyAnswer::NoNotThisKeyGenMode => { + return Err(KeyGenError::Unexpected) + } + ShouldSendKeyAnswer::NoWaiting => return Err(KeyGenError::Unexpected), + ShouldSendKeyAnswer::Yes => { + let serialized_part = match bincode::serialize(&failure_pub_keys) { + Ok(part) => part, + Err(e) => { + warn!(target:"engine", "could not serialize part: {:?}", e); + return Err(KeyGenError::Unexpected); + } + }; + + send_part_transaction( + full_client, + client, + &address, + upcoming_epoch, + serialized_part, + )?; + trace!(target:"engine", "PART Transaction send for moving forward key gen phase"); + return Ok(()); + } + } + } + }; // If there is no part then we are not part of the pending validator set and there is nothing for us to do. let part_data = match part { Some(part) => part, None => { warn!(target:"engine", "no part to write."); - return Err(CallError::ReturnValueInvalid); + return Err(KeyGenError::NoPartToWrite); } }; - let upcoming_epoch = get_posdao_epoch(client, BlockId::Latest)? + 1; trace!(target:"engine", "preparing to send PARTS for upcoming epoch: {}", upcoming_epoch); // Check if we already sent our part. @@ -155,36 +224,17 @@ impl KeygenTransactionSender { Ok(part) => part, Err(e) => { warn!(target:"engine", "could not serialize part: {:?}", e); - return Err(CallError::ReturnValueInvalid); + return Err(KeyGenError::Unexpected); } }; - let serialized_part_len = serialized_part.len(); - let current_round = get_current_key_gen_round(client)?; - let write_part_data = key_history_contract::functions::write_part::call( + + send_part_transaction( + full_client, + client, + &address, upcoming_epoch, - current_round, serialized_part, - ); - - // the required gas values have been approximated by - // experimenting and it's a very rough estimation. - // it can be further fine tuned to be just above the real consumption. - // ACKs require much more gas, - // and usually run into the gas limit problems. - let gas: usize = serialized_part_len * 800 + 100_000; - - let part_transaction = - TransactionRequest::call(*KEYGEN_HISTORY_ADDRESS, write_part_data.0) - .gas(U256::from(gas)) - .nonce(full_client.nonce(&address, BlockId::Latest).unwrap()) - .gas_price(U256::from(10000000000u64)); - full_client - .transact_silently(part_transaction) - .map_err(|e| { - warn!(target:"engine", "could not transact_silently: {:?}", e); - CallError::ReturnValueInvalid - })?; - + )?; trace!(target:"engine", "PART Transaction send."); return Ok(()); } @@ -213,7 +263,7 @@ impl KeygenTransactionSender { } Err(err) => { error!(target:"engine", "could not retrieve part for {} call failed. Error: {:?}", *v, err); - return Err(err); + return Err(KeyGenError::CallError(err)); } } ); @@ -230,7 +280,7 @@ impl KeygenTransactionSender { for ack in acks { let ack_to_push = match bincode::serialize(&ack) { Ok(serialized_ack) => serialized_ack, - Err(_) => return Err(CallError::ReturnValueInvalid), + Err(e) => return Err(KeyGenError::Unexpected), }; total_bytes_for_acks += ack_to_push.len(); serialized_acks.push(ack_to_push); @@ -263,3 +313,36 @@ impl KeygenTransactionSender { Ok(()) } } + +fn send_part_transaction( + full_client: &dyn BlockChainClient, + client: &dyn EngineClient, + mining_address: &Address, + upcoming_epoch: U256, + data: Vec, +) -> Result<(), KeyGenError> { + // the required gas values have been approximated by + // experimenting and it's a very rough estimation. + // it can be further fine tuned to be just above the real consumption. + // ACKs require much more gas, + // and usually run into the gas limit problems. + let gas: usize = data.len() * 800 + 100_000; + + let nonce = full_client.nonce(&mining_address, BlockId::Latest).unwrap(); + let current_round = get_current_key_gen_round(client)?; + let write_part_data = + key_history_contract::functions::write_part::call(upcoming_epoch, current_round, data); + + let part_transaction = TransactionRequest::call(*KEYGEN_HISTORY_ADDRESS, write_part_data.0) + .gas(U256::from(gas)) + .nonce(nonce) + .gas_price(U256::from(10000000000u64)); + full_client + .transact_silently(part_transaction) + .map_err(|e| { + warn!(target:"engine", "could not transact_silently: {:?}", e); + CallError::ReturnValueInvalid + })?; + + return Ok(()); +}