diff --git a/ethcore/engines/authority-round/src/lib.rs b/ethcore/engines/authority-round/src/lib.rs index 49861c026..adf057453 100644 --- a/ethcore/engines/authority-round/src/lib.rs +++ b/ethcore/engines/authority-round/src/lib.rs @@ -370,7 +370,6 @@ impl EpochManager { debug!(target: "engine", "Zooming to epoch after block {}", hash); trace!(target: "engine", "Current validator set: {:?}", self.validators()); - // epoch_transition_for can be an expensive call, but in the absence of // forks it will only need to be called for the block directly after // epoch transition, in which case it will be O(1) and require a single @@ -390,25 +389,27 @@ impl EpochManager { let (signal_number, set_proof, _) = destructure_proofs(&last_transition.proof) .expect("proof produced by this engine; therefore it is valid; qed"); - trace!(target: "engine", "extracting epoch validator set for epoch ({}, {}) signalled at #{}", - last_transition.block_number, last_transition.block_hash, signal_number); + trace!( + target: "engine", + "extracting epoch validator set for epoch ({}, {}) signalled at #{}", + last_transition.block_number, last_transition.block_hash, signal_number + ); let first = signal_number == 0; - let epoch_set = validators.epoch_set( + let (list, _) = validators.epoch_set( first, machine, signal_number, // use signal number so multi-set first calculation is correct. set_proof, - ) - .ok() - .map(|(list, _)| { - trace!(target: "engine", "Updating finality checker with new validator set extracted from epoch ({}, {}): {:?}", - last_transition.block_number, last_transition.block_hash, &list); + ).expect("proof produced by this engine; therefore it is valid; qed"); - list.into_inner() - }) - .expect("proof produced by this engine; therefore it is valid; qed"); + trace!( + target: "engine", + "Updating finality checker with new validator set extracted from epoch ({}, {}): {:?}", + last_transition.block_number, last_transition.block_hash, &list + ); + let epoch_set = list.into_inner(); let two_thirds_majority_transition = self.finality_checker.two_thirds_majority_transition(); self.finality_checker = RollingFinality::blank(epoch_set, two_thirds_majority_transition); } @@ -435,10 +436,22 @@ impl EpochManager { /// A message broadcast by authorities when it's their turn to seal a block but there are no /// transactions. Other authorities accumulate these messages and later include them in the seal as /// proof. +/// +/// An empty step message is created _instead of_ a block if there are no pending transactions. +/// It cannot itself be a parent, and `parent_hash` always points to the most recent block. E.g.: +/// * Validator A creates block `bA`. +/// * Validator B has no pending transactions, so it signs an empty step message `mB` +/// instead whose hash points to block `bA`. +/// * Validator C also has no pending transactions, so it also signs an empty step message `mC` +/// instead whose hash points to block `bA`. +/// * Validator D creates block `bD`. The parent is block `bA`, and the header includes `mB` and `mC`. #[derive(Clone, Debug, PartialEq, Eq)] struct EmptyStep { + /// The signature of the other two fields, by the message's author. signature: H520, + /// This message's step number. step: u64, + /// The hash of the most recent block. parent_hash: H256, } @@ -447,6 +460,7 @@ impl PartialOrd for EmptyStep { Some(self.cmp(other)) } } + impl Ord for EmptyStep { fn cmp(&self, other: &Self) -> cmp::Ordering { self.step.cmp(&other.step) @@ -463,6 +477,7 @@ impl EmptyStep { EmptyStep { signature, step, parent_hash } } + /// Returns `true` if the message has a valid signature by the expected proposer in the message's step. fn verify(&self, validators: &dyn ValidatorSet) -> Result { let message = keccak(empty_step_rlp(self.step, &self.parent_hash)); let correct_proposer = step_proposer(validators, &self.parent_hash, self.step); @@ -773,7 +788,7 @@ fn verify_external(header: &Header, validators: &dyn ValidatorSet, empty_steps_t } fn combine_proofs(signal_number: BlockNumber, set_proof: &[u8], finality_proof: &[u8]) -> Vec { - let mut stream = ::rlp::RlpStream::new_list(3); + let mut stream = RlpStream::new_list(3); stream.append(&signal_number).append(&set_proof).append(&finality_proof); stream.out() } @@ -830,30 +845,21 @@ impl AuthorityRound { let initial_step = our_params.start_step.unwrap_or(0); let mut durations = Vec::new(); - let mut prev_step = 0u64; - let mut prev_time = 0u64; - let mut prev_dur = our_params.step_durations[&0]; - durations.push(StepDurationInfo { - transition_step: prev_step, - transition_timestamp: prev_time, - step_duration: prev_dur - }); - for (time, dur) in our_params.step_durations.iter().skip(1) { - let (step, time) = next_step_time_duration( - StepDurationInfo{ - transition_step: prev_step, - transition_timestamp: prev_time, - step_duration: prev_dur, - }, *time) - .ok_or(BlockError::TimestampOverflow)?; - durations.push(StepDurationInfo { - transition_step: step, - transition_timestamp: time, - step_duration: *dur - }); - prev_step = step; - prev_time = time; - prev_dur = *dur; + { + let mut dur_info = StepDurationInfo { + transition_step: 0u64, + transition_timestamp: 0u64, + step_duration: our_params.step_durations[&0], + }; + durations.push(dur_info); + for (time, dur) in our_params.step_durations.iter().skip(1) { + let (step, time) = next_step_time_duration(dur_info, *time) + .ok_or(BlockError::TimestampOverflow)?; + dur_info.transition_step = step; + dur_info.transition_timestamp = time; + dur_info.step_duration = *dur; + durations.push(dur_info); + } } let step = Step { @@ -907,13 +913,7 @@ impl AuthorityRound { (CowLike::Borrowed(&*self.validators), header.number()) } else { let mut epoch_manager = self.epoch_manager.lock(); - let client = match self.client.read().as_ref().and_then(|weak| weak.upgrade()) { - Some(client) => client, - None => { - debug!(target: "engine", "Unable to verify sig: missing client ref."); - return Err(EngineError::RequiresClient.into()) - } - }; + let client = self.upgrade_client_or("Unable to verify sig")?; if !epoch_manager.zoom_to_after(&*client, &self.machine, &*self.validators, *header.parent_hash()) { debug!(target: "engine", "Unable to zoom to epoch."); @@ -981,14 +981,19 @@ impl AuthorityRound { } fn broadcast_message(&self, message: Vec) { - if let Some(ref weak) = *self.client.read() { - if let Some(c) = weak.upgrade() { - c.broadcast_consensus_message(message); - } + if let Ok(c) = self.upgrade_client_or(None) { + c.broadcast_consensus_message(message); } } - fn report_skipped(&self, header: &Header, current_step: u64, parent_step: u64, validators: &dyn ValidatorSet, set_number: u64) { + fn report_skipped( + &self, + header: &Header, + current_step: u64, + parent_step: u64, + validators: &dyn ValidatorSet, + set_number: u64 + ) { // we're building on top of the genesis block so don't report any skipped steps if header.number() == 1 { return; @@ -1004,8 +1009,12 @@ impl AuthorityRound { if skipped_primary != me { // Stop reporting once validators start repeating. if !reported.insert(skipped_primary) { break; } - trace!(target: "engine", "Reporting benign misbehaviour (cause: skipped step) at block #{}, epoch set number {}, step proposer={:#x}. Own address: {}", - header.number(), set_number, skipped_primary, me); + trace!( + target: "engine", + "Reporting benign misbehaviour (cause: skipped step) at block #{}, \ + epoch set number {}, step proposer={:#x}. Own address: {}", + header.number(), set_number, skipped_primary, me + ); self.validators.report_benign(&skipped_primary, set_number, header.number()); } else { trace!(target: "engine", "Primary that skipped is self, not self-reporting. Own address: {}", me); @@ -1018,12 +1027,9 @@ impl AuthorityRound { fn build_finality(&self, chain_head: &Header, ancestry: &mut dyn Iterator) -> Vec { if self.immediate_transitions { return Vec::new() } - let client = match self.client.read().as_ref().and_then(|weak| weak.upgrade()) { - Some(client) => client, - None => { - warn!(target: "engine", "Unable to apply ancestry actions: missing client ref."); - return Vec::new(); - } + let client = match self.upgrade_client_or("Unable to apply ancestry actions") { + Ok(client) => client, + Err(_) => return Vec::new(), }; let mut epoch_manager = self.epoch_manager.lock(); @@ -1079,7 +1085,7 @@ impl AuthorityRound { } fn address(&self) -> Option
{ - self.signer.read().as_ref().map(|s| s.address() ) + self.signer.read().as_ref().map(|s| s.address()) } /// Make calls to the randomness contract. @@ -1095,10 +1101,7 @@ impl AuthorityRound { None => return Ok(Vec::new()), // We are not a validator, so we shouldn't call the contracts. }; let our_addr = signer.address(); - let client = self.client.read().as_ref().and_then(|weak| weak.upgrade()).ok_or_else(|| { - debug!(target: "engine", "Unable to prepare block: missing client ref."); - EngineError::RequiresClient - })?; + let client = self.upgrade_client_or("Unable to prepare block")?; let full_client = client.as_full_client() .ok_or_else(|| EngineError::FailedSystemCall("Failed to upgrade to BlockchainClient.".to_string()))?; @@ -1116,6 +1119,18 @@ impl AuthorityRound { let tx_request = TransactionRequest::call(contract_addr, data).gas_price(U256::zero()).nonce(nonce); Ok(vec![full_client.create_transaction(tx_request)?]) } + + /// Returns the reference to the client, if registered. + fn upgrade_client_or<'a, T>(&self, opt_error_msg: T) -> Result, EngineError> + where T: Into>, + { + self.client.read().as_ref().and_then(|weak| weak.upgrade()).ok_or_else(|| { + if let Some(error_msg) = opt_error_msg.into() { + debug!(target: "engine", "{}: missing client ref.", error_msg); + } + EngineError::RequiresClient + }) + } } fn unix_now() -> Duration { @@ -1174,10 +1189,8 @@ impl Engine for AuthorityRound { fn step(&self) { self.step.inner.increment(); self.step.can_propose.store(true, AtomicOrdering::SeqCst); - if let Some(ref weak) = *self.client.read() { - if let Some(c) = weak.upgrade() { - c.update_sealing(ForceUpdateSealing::No); - } + if let Ok(c) = self.upgrade_client_or(None) { + c.update_sealing(ForceUpdateSealing::No); } } @@ -1257,12 +1270,9 @@ impl Engine for AuthorityRound { } }; - let client = match self.client.read().as_ref().and_then(|weak| weak.upgrade()) { - Some(client) => client, - None => { - warn!(target: "engine", "Not preparing block: missing client ref."); - return SealingState::NotReady; - } + let client = match self.upgrade_client_or("Not preparing block") { + Ok(client) => client, + Err(_) => return SealingState::NotReady, }; let parent = match client.as_full_client() { @@ -1296,7 +1306,7 @@ impl Engine for AuthorityRound { } fn handle_message(&self, rlp: &[u8]) -> Result<(), EngineError> { - fn fmt_err(x: T) -> EngineError { + fn fmt_err(x: T) -> EngineError { EngineError::MalformedMessage(format!("{:?}", x)) } @@ -1625,8 +1635,12 @@ impl Engine for AuthorityRound { match validate_empty_steps() { Ok(len) => len, Err(err) => { - trace!(target: "engine", "Reporting benign misbehaviour (cause: invalid empty steps) at block #{}, epoch set number {}. Own address: {}", - header.number(), set_number, self.address().unwrap_or_default()); + trace!( + target: "engine", + "Reporting benign misbehaviour (cause: invalid empty steps) \ + at block #{}, epoch set number {}. Own address: {}", + header.number(), set_number, self.address().unwrap_or_default() + ); self.validators.report_benign(header.author(), set_number, header.number()); return Err(err); }, @@ -1640,7 +1654,10 @@ impl Engine for AuthorityRound { if header.number() >= self.validate_score_transition { let expected_difficulty = calculate_score(parent_step.into(), step.into(), empty_steps_len.into()); if header.difficulty() != &expected_difficulty { - return Err(From::from(BlockError::InvalidDifficulty(Mismatch { expected: expected_difficulty, found: header.difficulty().clone() }))); + return Err(From::from(BlockError::InvalidDifficulty(Mismatch { + expected: expected_difficulty, + found: header.difficulty().clone() + }))); } } @@ -1656,7 +1673,10 @@ impl Engine for AuthorityRound { let res = verify_external(header, &*validators, self.empty_steps_transition); match res { Err(Error::Engine(EngineError::NotProposer(_))) => { - trace!(target: "engine", "Reporting benign misbehaviour (cause: block from incorrect proposer) at block #{}, epoch set number {}. Own address: {}", + trace!( + target: "engine", + "Reporting benign misbehaviour (cause: block from incorrect proposer) \ + at block #{}, epoch set number {}. Own address: {}", header.number(), set_number, self.address().unwrap_or_default()); self.validators.report_benign(header.author(), set_number, header.number()); }, @@ -1692,13 +1712,7 @@ impl Engine for AuthorityRound { if self.immediate_transitions { return None } let epoch_transition_hash = { - let client = match self.client.read().as_ref().and_then(|weak| weak.upgrade()) { - Some(client) => client, - None => { - warn!(target: "engine", "Unable to check for epoch end: missing client ref."); - return None; - } - }; + let client = self.upgrade_client_or("Unable to check for epoch end").ok()?; let mut epoch_manager = self.epoch_manager.lock(); if !epoch_manager.zoom_to_after(&*client, &self.machine, &*self.validators, *chain_head.parent_hash()) { @@ -1710,7 +1724,7 @@ impl Engine for AuthorityRound { let mut hash = *chain_head.parent_hash(); - let mut ancestry = std::iter::repeat_with(move || { + let mut ancestry = iter::repeat_with(move || { chain(hash).and_then(|header| { if header.number() == 0 { return None } hash = *header.parent_hash(); @@ -1739,7 +1753,11 @@ impl Engine for AuthorityRound { // Apply transitions that don't require finality and should be enacted immediately (e.g from chain spec) if let Some(change) = self.validators.is_epoch_end(first, chain_head) { - info!(target: "engine", "Immediately applying validator set change signalled at block {}", chain_head.number()); + info!( + target: "engine", + "Immediately applying validator set change signalled at block {}", + chain_head.number() + ); self.epoch_manager.lock().note_new_epoch(); let change = combine_proofs(chain_head.number(), &change, &[]); return Some(change) @@ -1752,7 +1770,7 @@ impl Engine for AuthorityRound { // to construct transition proof. author == ec_recover(sig) known // since the blocks are in the DB. let mut hash = chain_head.hash(); - let mut finality_proof: Vec<_> = std::iter::repeat_with(move || { + let mut finality_proof: Vec<_> = iter::repeat_with(move || { chain(hash).and_then(|header| { hash = *header.parent_hash(); if header.number() == 0 { None } @@ -1865,13 +1883,7 @@ impl Engine for AuthorityRound { fn gas_limit_override(&self, header: &Header) -> Option { let (_, &address) = self.block_gas_limit_contract_transitions.range(..=header.number()).last()?; - let client = match self.client.read().as_ref().and_then(|weak| weak.upgrade()) { - Some(client) => client, - None => { - error!(target: "engine", "Unable to prepare block: missing client ref."); - return None; - } - }; + let client = self.upgrade_client_or("Unable to prepare block").ok()?; let full_client = match client.as_full_client() { Some(full_client) => full_client, None => { @@ -2392,7 +2404,7 @@ mod tests { ]); } - fn assert_insufficient_proof(result: Result, contains: &str) { + fn assert_insufficient_proof(result: Result, contains: &str) { match result { Err(Error::Engine(EngineError::InsufficientProof(ref s))) =>{ assert!(s.contains(contains), "Expected {:?} to contain {:?}", s, contains);