AuthorityEngine: Minor cleanups. (#11408)

This commit is contained in:
Andreas Fackler 2020-01-27 14:30:42 +01:00 committed by Andronik Ordian
parent 6ab7789604
commit e69539357f

View File

@ -370,7 +370,6 @@ impl EpochManager {
debug!(target: "engine", "Zooming to epoch after block {}", hash); debug!(target: "engine", "Zooming to epoch after block {}", hash);
trace!(target: "engine", "Current validator set: {:?}", self.validators()); trace!(target: "engine", "Current validator set: {:?}", self.validators());
// epoch_transition_for can be an expensive call, but in the absence of // 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 // 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 // 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) let (signal_number, set_proof, _) = destructure_proofs(&last_transition.proof)
.expect("proof produced by this engine; therefore it is valid; qed"); .expect("proof produced by this engine; therefore it is valid; qed");
trace!(target: "engine", "extracting epoch validator set for epoch ({}, {}) signalled at #{}", trace!(
last_transition.block_number, last_transition.block_hash, signal_number); 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 first = signal_number == 0;
let epoch_set = validators.epoch_set( let (list, _) = validators.epoch_set(
first, first,
machine, machine,
signal_number, // use signal number so multi-set first calculation is correct. signal_number, // use signal number so multi-set first calculation is correct.
set_proof, set_proof,
) ).expect("proof produced by this engine; therefore it is valid; qed");
.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);
list.into_inner() trace!(
}) target: "engine",
.expect("proof produced by this engine; therefore it is valid; qed"); "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(); let two_thirds_majority_transition = self.finality_checker.two_thirds_majority_transition();
self.finality_checker = RollingFinality::blank(epoch_set, 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 /// 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 /// transactions. Other authorities accumulate these messages and later include them in the seal as
/// proof. /// 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)] #[derive(Clone, Debug, PartialEq, Eq)]
struct EmptyStep { struct EmptyStep {
/// The signature of the other two fields, by the message's author.
signature: H520, signature: H520,
/// This message's step number.
step: u64, step: u64,
/// The hash of the most recent block.
parent_hash: H256, parent_hash: H256,
} }
@ -447,6 +460,7 @@ impl PartialOrd for EmptyStep {
Some(self.cmp(other)) Some(self.cmp(other))
} }
} }
impl Ord for EmptyStep { impl Ord for EmptyStep {
fn cmp(&self, other: &Self) -> cmp::Ordering { fn cmp(&self, other: &Self) -> cmp::Ordering {
self.step.cmp(&other.step) self.step.cmp(&other.step)
@ -463,6 +477,7 @@ impl EmptyStep {
EmptyStep { signature, step, parent_hash } 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<bool, Error> { fn verify(&self, validators: &dyn ValidatorSet) -> Result<bool, Error> {
let message = keccak(empty_step_rlp(self.step, &self.parent_hash)); let message = keccak(empty_step_rlp(self.step, &self.parent_hash));
let correct_proposer = step_proposer(validators, &self.parent_hash, self.step); 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<u8> { fn combine_proofs(signal_number: BlockNumber, set_proof: &[u8], finality_proof: &[u8]) -> Vec<u8> {
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.append(&signal_number).append(&set_proof).append(&finality_proof);
stream.out() stream.out()
} }
@ -830,30 +845,21 @@ impl AuthorityRound {
let initial_step = our_params.start_step.unwrap_or(0); let initial_step = our_params.start_step.unwrap_or(0);
let mut durations = Vec::new(); let mut durations = Vec::new();
let mut prev_step = 0u64; {
let mut prev_time = 0u64; let mut dur_info = StepDurationInfo {
let mut prev_dur = our_params.step_durations[&0]; transition_step: 0u64,
durations.push(StepDurationInfo { transition_timestamp: 0u64,
transition_step: prev_step, step_duration: our_params.step_durations[&0],
transition_timestamp: prev_time, };
step_duration: prev_dur durations.push(dur_info);
}); for (time, dur) in our_params.step_durations.iter().skip(1) {
for (time, dur) in our_params.step_durations.iter().skip(1) { let (step, time) = next_step_time_duration(dur_info, *time)
let (step, time) = next_step_time_duration( .ok_or(BlockError::TimestampOverflow)?;
StepDurationInfo{ dur_info.transition_step = step;
transition_step: prev_step, dur_info.transition_timestamp = time;
transition_timestamp: prev_time, dur_info.step_duration = *dur;
step_duration: prev_dur, durations.push(dur_info);
}, *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 step = Step { let step = Step {
@ -907,13 +913,7 @@ impl AuthorityRound {
(CowLike::Borrowed(&*self.validators), header.number()) (CowLike::Borrowed(&*self.validators), header.number())
} else { } else {
let mut epoch_manager = self.epoch_manager.lock(); let mut epoch_manager = self.epoch_manager.lock();
let client = match self.client.read().as_ref().and_then(|weak| weak.upgrade()) { let client = self.upgrade_client_or("Unable to verify sig")?;
Some(client) => client,
None => {
debug!(target: "engine", "Unable to verify sig: missing client ref.");
return Err(EngineError::RequiresClient.into())
}
};
if !epoch_manager.zoom_to_after(&*client, &self.machine, &*self.validators, *header.parent_hash()) { if !epoch_manager.zoom_to_after(&*client, &self.machine, &*self.validators, *header.parent_hash()) {
debug!(target: "engine", "Unable to zoom to epoch."); debug!(target: "engine", "Unable to zoom to epoch.");
@ -981,14 +981,19 @@ impl AuthorityRound {
} }
fn broadcast_message(&self, message: Vec<u8>) { fn broadcast_message(&self, message: Vec<u8>) {
if let Some(ref weak) = *self.client.read() { if let Ok(c) = self.upgrade_client_or(None) {
if let Some(c) = weak.upgrade() { c.broadcast_consensus_message(message);
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 // we're building on top of the genesis block so don't report any skipped steps
if header.number() == 1 { if header.number() == 1 {
return; return;
@ -1004,8 +1009,12 @@ impl AuthorityRound {
if skipped_primary != me { if skipped_primary != me {
// Stop reporting once validators start repeating. // Stop reporting once validators start repeating.
if !reported.insert(skipped_primary) { break; } 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: {}", trace!(
header.number(), set_number, skipped_primary, me); 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()); self.validators.report_benign(&skipped_primary, set_number, header.number());
} else { } else {
trace!(target: "engine", "Primary that skipped is self, not self-reporting. Own address: {}", me); 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<Item=Header>) -> Vec<H256> { fn build_finality(&self, chain_head: &Header, ancestry: &mut dyn Iterator<Item=Header>) -> Vec<H256> {
if self.immediate_transitions { return Vec::new() } if self.immediate_transitions { return Vec::new() }
let client = match self.client.read().as_ref().and_then(|weak| weak.upgrade()) { let client = match self.upgrade_client_or("Unable to apply ancestry actions") {
Some(client) => client, Ok(client) => client,
None => { Err(_) => return Vec::new(),
warn!(target: "engine", "Unable to apply ancestry actions: missing client ref.");
return Vec::new();
}
}; };
let mut epoch_manager = self.epoch_manager.lock(); let mut epoch_manager = self.epoch_manager.lock();
@ -1079,7 +1085,7 @@ impl AuthorityRound {
} }
fn address(&self) -> Option<Address> { fn address(&self) -> Option<Address> {
self.signer.read().as_ref().map(|s| s.address() ) self.signer.read().as_ref().map(|s| s.address())
} }
/// Make calls to the randomness contract. /// 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. None => return Ok(Vec::new()), // We are not a validator, so we shouldn't call the contracts.
}; };
let our_addr = signer.address(); let our_addr = signer.address();
let client = self.client.read().as_ref().and_then(|weak| weak.upgrade()).ok_or_else(|| { let client = self.upgrade_client_or("Unable to prepare block")?;
debug!(target: "engine", "Unable to prepare block: missing client ref.");
EngineError::RequiresClient
})?;
let full_client = client.as_full_client() let full_client = client.as_full_client()
.ok_or_else(|| EngineError::FailedSystemCall("Failed to upgrade to BlockchainClient.".to_string()))?; .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); let tx_request = TransactionRequest::call(contract_addr, data).gas_price(U256::zero()).nonce(nonce);
Ok(vec![full_client.create_transaction(tx_request)?]) 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<Arc<dyn EngineClient>, EngineError>
where T: Into<Option<&'a str>>,
{
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 { fn unix_now() -> Duration {
@ -1174,10 +1189,8 @@ impl Engine for AuthorityRound {
fn step(&self) { fn step(&self) {
self.step.inner.increment(); self.step.inner.increment();
self.step.can_propose.store(true, AtomicOrdering::SeqCst); self.step.can_propose.store(true, AtomicOrdering::SeqCst);
if let Some(ref weak) = *self.client.read() { if let Ok(c) = self.upgrade_client_or(None) {
if let Some(c) = weak.upgrade() { c.update_sealing(ForceUpdateSealing::No);
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()) { let client = match self.upgrade_client_or("Not preparing block") {
Some(client) => client, Ok(client) => client,
None => { Err(_) => return SealingState::NotReady,
warn!(target: "engine", "Not preparing block: missing client ref.");
return SealingState::NotReady;
}
}; };
let parent = match client.as_full_client() { let parent = match client.as_full_client() {
@ -1296,7 +1306,7 @@ impl Engine for AuthorityRound {
} }
fn handle_message(&self, rlp: &[u8]) -> Result<(), EngineError> { fn handle_message(&self, rlp: &[u8]) -> Result<(), EngineError> {
fn fmt_err<T: ::std::fmt::Debug>(x: T) -> EngineError { fn fmt_err<T: fmt::Debug>(x: T) -> EngineError {
EngineError::MalformedMessage(format!("{:?}", x)) EngineError::MalformedMessage(format!("{:?}", x))
} }
@ -1625,8 +1635,12 @@ impl Engine for AuthorityRound {
match validate_empty_steps() { match validate_empty_steps() {
Ok(len) => len, Ok(len) => len,
Err(err) => { Err(err) => {
trace!(target: "engine", "Reporting benign misbehaviour (cause: invalid empty steps) at block #{}, epoch set number {}. Own address: {}", trace!(
header.number(), set_number, self.address().unwrap_or_default()); 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()); self.validators.report_benign(header.author(), set_number, header.number());
return Err(err); return Err(err);
}, },
@ -1640,7 +1654,10 @@ impl Engine for AuthorityRound {
if header.number() >= self.validate_score_transition { if header.number() >= self.validate_score_transition {
let expected_difficulty = calculate_score(parent_step.into(), step.into(), empty_steps_len.into()); let expected_difficulty = calculate_score(parent_step.into(), step.into(), empty_steps_len.into());
if header.difficulty() != &expected_difficulty { 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); let res = verify_external(header, &*validators, self.empty_steps_transition);
match res { match res {
Err(Error::Engine(EngineError::NotProposer(_))) => { 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()); header.number(), set_number, self.address().unwrap_or_default());
self.validators.report_benign(header.author(), set_number, header.number()); self.validators.report_benign(header.author(), set_number, header.number());
}, },
@ -1692,13 +1712,7 @@ impl Engine for AuthorityRound {
if self.immediate_transitions { return None } if self.immediate_transitions { return None }
let epoch_transition_hash = { let epoch_transition_hash = {
let client = match self.client.read().as_ref().and_then(|weak| weak.upgrade()) { let client = self.upgrade_client_or("Unable to check for epoch end").ok()?;
Some(client) => client,
None => {
warn!(target: "engine", "Unable to check for epoch end: missing client ref.");
return None;
}
};
let mut epoch_manager = self.epoch_manager.lock(); let mut epoch_manager = self.epoch_manager.lock();
if !epoch_manager.zoom_to_after(&*client, &self.machine, &*self.validators, *chain_head.parent_hash()) { 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 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| { chain(hash).and_then(|header| {
if header.number() == 0 { return None } if header.number() == 0 { return None }
hash = *header.parent_hash(); 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) // 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) { 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(); self.epoch_manager.lock().note_new_epoch();
let change = combine_proofs(chain_head.number(), &change, &[]); let change = combine_proofs(chain_head.number(), &change, &[]);
return Some(change) return Some(change)
@ -1752,7 +1770,7 @@ impl Engine for AuthorityRound {
// to construct transition proof. author == ec_recover(sig) known // to construct transition proof. author == ec_recover(sig) known
// since the blocks are in the DB. // since the blocks are in the DB.
let mut hash = chain_head.hash(); 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| { chain(hash).and_then(|header| {
hash = *header.parent_hash(); hash = *header.parent_hash();
if header.number() == 0 { None } if header.number() == 0 { None }
@ -1865,13 +1883,7 @@ impl Engine for AuthorityRound {
fn gas_limit_override(&self, header: &Header) -> Option<U256> { fn gas_limit_override(&self, header: &Header) -> Option<U256> {
let (_, &address) = self.block_gas_limit_contract_transitions.range(..=header.number()).last()?; 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()) { let client = self.upgrade_client_or("Unable to prepare block").ok()?;
Some(client) => client,
None => {
error!(target: "engine", "Unable to prepare block: missing client ref.");
return None;
}
};
let full_client = match client.as_full_client() { let full_client = match client.as_full_client() {
Some(full_client) => full_client, Some(full_client) => full_client,
None => { None => {
@ -2392,7 +2404,7 @@ mod tests {
]); ]);
} }
fn assert_insufficient_proof<T: ::std::fmt::Debug>(result: Result<T, Error>, contains: &str) { fn assert_insufficient_proof<T: std::fmt::Debug>(result: Result<T, Error>, contains: &str) {
match result { match result {
Err(Error::Engine(EngineError::InsufficientProof(ref s))) =>{ Err(Error::Engine(EngineError::InsufficientProof(ref s))) =>{
assert!(s.contains(contains), "Expected {:?} to contain {:?}", s, contains); assert!(s.contains(contains), "Expected {:?} to contain {:?}", s, contains);