Merge pull request #7368 from paritytech/td-future-blocks

Wait for future blocks in AuRa
This commit is contained in:
Marek Kotewicz 2018-01-02 14:21:50 +01:00 committed by GitHub
commit f3e7d94fc4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 94 additions and 42 deletions

View File

@ -112,16 +112,12 @@ impl Step {
let now = unix_now(); let now = unix_now();
let expected_seconds = (self.load() as u64) let expected_seconds = (self.load() as u64)
.checked_add(1) .checked_add(1)
.and_then(|ctr| ctr.checked_mul(self.duration as u64)); .and_then(|ctr| ctr.checked_mul(self.duration as u64))
.map(Duration::from_secs);
match expected_seconds { match expected_seconds {
Some(secs) => { Some(step_end) if step_end > now => step_end - now,
let step_end = Duration::from_secs(secs); Some(_) => Duration::from_secs(0),
if step_end > now {
step_end - now
} else {
Duration::from_secs(0)
}
},
None => { None => {
let ctr = self.load(); let ctr = self.load();
error!(target: "engine", "Step counter is too high: {}, aborting", ctr); error!(target: "engine", "Step counter is too high: {}, aborting", ctr);
@ -130,6 +126,7 @@ impl Step {
} }
} }
fn increment(&self) { fn increment(&self) {
use std::usize; use std::usize;
// fetch_add won't panic on overflow but will rather wrap // fetch_add won't panic on overflow but will rather wrap
@ -141,19 +138,40 @@ impl Step {
} }
} }
fn calibrate(&self) { fn calibrate(&self) {
if self.calibrate { if self.calibrate {
let new_step = unix_now().as_secs() / (self.duration as u64); let new_step = unix_now().as_secs() / (self.duration as u64);
self.inner.store(new_step as usize, AtomicOrdering::SeqCst); self.inner.store(new_step as usize, AtomicOrdering::SeqCst);
} }
} }
fn is_future(&self, given: usize) -> bool {
if given > self.load() + 1 { fn check_future(&self, given: usize) -> Result<(), Option<OutOfBounds<u64>>> {
// Make absolutely sure that the given step is correct. const VALID_STEP_DRIFT: usize = 1;
const REJECTED_STEP_DRIFT: usize = 4;
// Give one step slack if step is lagging, double vote is still not possible.
if given <= self.load() + VALID_STEP_DRIFT {
return Ok(());
}
// Make absolutely sure that the given step is incorrect.
self.calibrate(); self.calibrate();
given > self.load() + 1 let current = self.load();
// reject blocks too far in the future
if given > current + REJECTED_STEP_DRIFT {
Err(None)
// wait a bit for blocks in near future
} else if given > current + VALID_STEP_DRIFT {
let d = self.duration as u64;
Err(Some(OutOfBounds {
min: None,
max: Some(d * (current + VALID_STEP_DRIFT) as u64),
found: d * given as u64,
}))
} else { } else {
false Ok(())
} }
} }
} }
@ -343,12 +361,17 @@ fn verify_external<F: Fn(Report)>(header: &Header, validators: &ValidatorSet, st
{ {
let header_step = header_step(header)?; let header_step = header_step(header)?;
// Give one step slack if step is lagging, double vote is still not possible. match step.check_future(header_step) {
if step.is_future(header_step) { Err(None) => {
trace!(target: "engine", "verify_block_external: block from the future"); trace!(target: "engine", "verify_block_external: block from the future");
report(Report::Benign(*header.author(), header.number())); report(Report::Benign(*header.author(), header.number()));
Err(BlockError::InvalidSeal)? return Err(BlockError::InvalidSeal.into())
} else { },
Err(Some(oob)) => {
trace!(target: "engine", "verify_block_external: block too early");
return Err(BlockError::TemporarilyInvalid(oob).into())
},
Ok(_) => {
let proposer_signature = header_signature(header)?; let proposer_signature = header_signature(header)?;
let correct_proposer = validators.get(header.parent_hash(), header_step); let correct_proposer = validators.get(header.parent_hash(), header_step);
let is_invalid_proposer = *header.author() != correct_proposer || let is_invalid_proposer = *header.author() != correct_proposer ||
@ -362,6 +385,7 @@ fn verify_external<F: Fn(Report)>(header: &Header, validators: &ValidatorSet, st
} }
} }
} }
}
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 = ::rlp::RlpStream::new_list(3);

View File

@ -171,22 +171,36 @@ mod tests {
// Make sure reporting can be done. // Make sure reporting can be done.
client.miner().set_gas_floor_target(1_000_000.into()); client.miner().set_gas_floor_target(1_000_000.into());
client.miner().set_engine_signer(v1, "".into()).unwrap(); client.miner().set_engine_signer(v1, "".into()).unwrap();
// Check a block that is a bit in future, reject it but don't report the validator.
let mut header = Header::default(); let mut header = Header::default();
let seal = vec![encode(&5u8).into_vec(), encode(&(&H520::default() as &[u8])).into_vec()]; let seal = vec![encode(&5u8).into_vec(), encode(&(&H520::default() as &[u8])).into_vec()];
header.set_seal(seal); header.set_seal(seal);
header.set_author(v1); header.set_author(v1);
header.set_number(2); header.set_number(2);
header.set_parent_hash(client.chain_info().best_block_hash); header.set_parent_hash(client.chain_info().best_block_hash);
assert!(client.engine().verify_block_external(&header).is_err());
client.engine().step();
assert_eq!(client.chain_info().best_block_number, 0);
// Now create one that is more in future. That one should be rejected and validator should be reported.
let mut header = Header::default();
let seal = vec![encode(&8u8).into_vec(), encode(&(&H520::default() as &[u8])).into_vec()];
header.set_seal(seal);
header.set_author(v1);
header.set_number(2);
header.set_parent_hash(client.chain_info().best_block_hash);
// `reportBenign` when the designated proposer releases block from the future (bad clock). // `reportBenign` when the designated proposer releases block from the future (bad clock).
assert!(client.engine().verify_block_external(&header).is_err()); assert!(client.engine().verify_block_external(&header).is_err());
// Seal a block. // Seal a block.
client.engine().step(); client.engine().step();
assert_eq!(client.chain_info().best_block_number, 1); assert_eq!(client.chain_info().best_block_number, 1);
// Check if the unresponsive validator is `disliked`. // Check if the unresponsive validator is `disliked`.
assert_eq!(client.call_contract(BlockId::Latest, validator_contract, "d8f2e0bf".from_hex().unwrap()).unwrap().to_hex(), "0000000000000000000000007d577a597b2742b498cb5cf0c26cdcd726d39e6e"); assert_eq!(
client.call_contract(BlockId::Latest, validator_contract, "d8f2e0bf".from_hex().unwrap()).unwrap().to_hex(),
"0000000000000000000000007d577a597b2742b498cb5cf0c26cdcd726d39e6e"
);
// Simulate a misbehaving validator by handling a double proposal. // Simulate a misbehaving validator by handling a double proposal.
let header = client.best_block_header().decode(); let header = client.best_block_header().decode();
assert!(client.engine().verify_block_family(&header, &header).is_err()); assert!(client.engine().verify_block_family(&header, &header).is_err());

View File

@ -167,6 +167,8 @@ pub enum BlockError {
InvalidReceiptsRoot(Mismatch<H256>), InvalidReceiptsRoot(Mismatch<H256>),
/// Timestamp header field is invalid. /// Timestamp header field is invalid.
InvalidTimestamp(OutOfBounds<u64>), InvalidTimestamp(OutOfBounds<u64>),
/// Timestamp header field is too far in future.
TemporarilyInvalid(OutOfBounds<u64>),
/// Log bloom header field is invalid. /// Log bloom header field is invalid.
InvalidLogBloom(Mismatch<LogBloom>), InvalidLogBloom(Mismatch<LogBloom>),
/// Parent hash field of header is invalid; this is an invalid error indicating a logic flaw in the codebase. /// Parent hash field of header is invalid; this is an invalid error indicating a logic flaw in the codebase.
@ -212,6 +214,7 @@ impl fmt::Display for BlockError {
InvalidGasLimit(ref oob) => format!("Invalid gas limit: {}", oob), InvalidGasLimit(ref oob) => format!("Invalid gas limit: {}", oob),
InvalidReceiptsRoot(ref mis) => format!("Invalid receipts trie root in header: {}", mis), InvalidReceiptsRoot(ref mis) => format!("Invalid receipts trie root in header: {}", mis),
InvalidTimestamp(ref oob) => format!("Invalid timestamp in header: {}", oob), InvalidTimestamp(ref oob) => format!("Invalid timestamp in header: {}", oob),
TemporarilyInvalid(ref oob) => format!("Future timestamp in header: {}", oob),
InvalidLogBloom(ref oob) => format!("Invalid log bloom in header: {}", oob), InvalidLogBloom(ref oob) => format!("Invalid log bloom in header: {}", oob),
InvalidParentHash(ref mis) => format!("Invalid parent hash: {}", mis), InvalidParentHash(ref mis) => format!("Invalid parent hash: {}", mis),
InvalidNumber(ref mis) => format!("Invalid number in header: {}", mis), InvalidNumber(ref mis) => format!("Invalid number in header: {}", mis),

View File

@ -503,7 +503,7 @@ impl<K: Kind> VerificationQueue<K> {
Err(err) => { Err(err) => {
match err { match err {
// Don't mark future blocks as bad. // Don't mark future blocks as bad.
Error::Block(BlockError::InvalidTimestamp(ref e)) if e.max.is_some() => {}, Error::Block(BlockError::TemporarilyInvalid(_)) => {},
_ => { _ => {
self.verification.bad.lock().insert(h.clone()); self.verification.bad.lock().insert(h.clone());
} }

View File

@ -273,11 +273,20 @@ pub fn verify_header_params(header: &Header, engine: &EthEngine, is_full: bool)
} }
if is_full { if is_full {
let max_time = get_time().sec as u64 + 15; const ACCEPTABLE_DRIFT_SECS: u64 = 15;
if header.timestamp() > max_time { let max_time = get_time().sec as u64 + ACCEPTABLE_DRIFT_SECS;
return Err(From::from(BlockError::InvalidTimestamp(OutOfBounds { max: Some(max_time), min: None, found: header.timestamp() }))) let invalid_threshold = max_time + ACCEPTABLE_DRIFT_SECS * 9;
let timestamp = header.timestamp();
if timestamp > invalid_threshold {
return Err(From::from(BlockError::InvalidTimestamp(OutOfBounds { max: Some(max_time), min: None, found: timestamp })))
}
if timestamp > max_time {
return Err(From::from(BlockError::TemporarilyInvalid(OutOfBounds { max: Some(max_time), min: None, found: timestamp })))
} }
} }
Ok(()) Ok(())
} }
@ -359,11 +368,13 @@ mod tests {
} }
} }
fn check_fail_timestamp(result: Result<(), Error>) { fn check_fail_timestamp(result: Result<(), Error>, temp: bool) {
let name = if temp { "TemporarilyInvalid" } else { "InvalidTimestamp" };
match result { match result {
Err(Error::Block(BlockError::InvalidTimestamp(_))) => (), Err(Error::Block(BlockError::InvalidTimestamp(_))) if !temp => (),
Err(other) => panic!("Block verification failed.\nExpected: InvalidTimestamp\nGot: {:?}", other), Err(Error::Block(BlockError::TemporarilyInvalid(_))) if temp => (),
Ok(_) => panic!("Block verification failed.\nExpected: InvalidTimestamp\nGot: Ok"), Err(other) => panic!("Block verification failed.\nExpected: {}\nGot: {:?}", name, other),
Ok(_) => panic!("Block verification failed.\nExpected: {}\nGot: Ok", name),
} }
} }
@ -642,11 +653,11 @@ mod tests {
header = good.clone(); header = good.clone();
header.set_timestamp(2450000000); header.set_timestamp(2450000000);
check_fail_timestamp(basic_test(&create_test_block_with_data(&header, &good_transactions, &good_uncles), engine)); check_fail_timestamp(basic_test(&create_test_block_with_data(&header, &good_transactions, &good_uncles), engine), false);
header = good.clone(); header = good.clone();
header.set_timestamp(get_time().sec as u64 + 20); header.set_timestamp(get_time().sec as u64 + 20);
check_fail_timestamp(basic_test(&create_test_block_with_data(&header, &good_transactions, &good_uncles), engine)); check_fail_timestamp(basic_test(&create_test_block_with_data(&header, &good_transactions, &good_uncles), engine), true);
header = good.clone(); header = good.clone();
header.set_timestamp(get_time().sec as u64 + 10); header.set_timestamp(get_time().sec as u64 + 10);