From b88d50dc9b44ab705599b21dc27b41e27337700d Mon Sep 17 00:00:00 2001 From: keorn Date: Thu, 15 Dec 2016 19:27:06 +0100 Subject: [PATCH 1/4] fix naming collision --- ethcore/src/engines/authority_round.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/ethcore/src/engines/authority_round.rs b/ethcore/src/engines/authority_round.rs index 8d1c004c5..cf9325dd4 100644 --- a/ethcore/src/engines/authority_round.rs +++ b/ethcore/src/engines/authority_round.rs @@ -118,13 +118,9 @@ impl AuthorityRound { Ok(engine) } - fn step(&self) -> usize { - self.step.load(AtomicOrdering::SeqCst) - } - fn remaining_step_duration(&self) -> Duration { let now = unix_now(); - let step_end = self.our_params.step_duration * (self.step() as u32 + 1); + let step_end = self.our_params.step_duration * (self.step.load(AtomicOrdering::SeqCst) as u32 + 1); if step_end > now { step_end - now } else { @@ -228,7 +224,7 @@ impl Engine for AuthorityRound { fn generate_seal(&self, block: &ExecutedBlock) -> Seal { if self.proposed.load(AtomicOrdering::SeqCst) { return Seal::None; } let header = block.header(); - let step = self.step(); + let step = self.step.load(AtomicOrdering::SeqCst); if self.is_step_proposer(step, header.author()) { if let Some(ref ap) = *self.account_provider.lock() { // Account should be permanently unlocked, otherwise sealing will fail. @@ -265,7 +261,7 @@ impl Engine for AuthorityRound { fn verify_block_unordered(&self, header: &Header, _block: Option<&[u8]>) -> Result<(), Error> { let header_step = try!(header_step(header)); // Give one step slack if step is lagging, double vote is still not possible. - if header_step <= self.step() + 1 { + if header_step <= self.step.load(AtomicOrdering::SeqCst) + 1 { let proposer_signature = try!(header_signature(header)); let ok_sig = try!(verify_address(self.step_proposer(header_step), &proposer_signature, &header.bare_hash())); if ok_sig { From b7b531ca75111ee0b02afd9ab3308bb599b9041f Mon Sep 17 00:00:00 2001 From: keorn Date: Thu, 15 Dec 2016 19:54:28 +0100 Subject: [PATCH 2/4] more slack in test --- ethcore/src/engines/authority_round.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ethcore/src/engines/authority_round.rs b/ethcore/src/engines/authority_round.rs index cf9325dd4..4e7d564fe 100644 --- a/ethcore/src/engines/authority_round.rs +++ b/ethcore/src/engines/authority_round.rs @@ -462,7 +462,7 @@ mod tests { // Spec starts with step 2. header.set_seal(vec![encode(&1usize).to_vec(), encode(&(&*signature as &[u8])).to_vec()]); assert!(engine.verify_block_seal(&header).is_ok()); - header.set_seal(vec![encode(&5usize).to_vec(), encode(&(&*signature as &[u8])).to_vec()]); + header.set_seal(vec![encode(&11usize).to_vec(), encode(&(&*signature as &[u8])).to_vec()]); assert!(engine.verify_block_seal(&header).is_err()); } } From 447196398aab89d9d41fa3b10f4f4dfd7ab7c63a Mon Sep 17 00:00:00 2001 From: keorn Date: Thu, 15 Dec 2016 23:36:06 +0100 Subject: [PATCH 3/4] fix start_step --- ethcore/src/engines/authority_round.rs | 8 ++++++-- sync/src/tests/consensus.rs | 15 ++++++++++----- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/ethcore/src/engines/authority_round.rs b/ethcore/src/engines/authority_round.rs index 4e7d564fe..57bb513ae 100644 --- a/ethcore/src/engines/authority_round.rs +++ b/ethcore/src/engines/authority_round.rs @@ -100,6 +100,7 @@ impl AsMillis for Duration { impl AuthorityRound { /// Create a new instance of AuthorityRound engine. pub fn new(params: CommonParams, our_params: AuthorityRoundParams, builtins: BTreeMap) -> Result, Error> { + let should_timeout = our_params.start_step.is_none(); let initial_step = our_params.start_step.unwrap_or_else(|| (unix_now().as_secs() / our_params.step_duration.as_secs())) as usize; let engine = Arc::new( AuthorityRound { @@ -113,8 +114,11 @@ impl AuthorityRound { account_provider: Mutex::new(None), password: RwLock::new(None), }); - let handler = TransitionHandler { engine: Arc::downgrade(&engine) }; - try!(engine.transition_service.register_handler(Arc::new(handler))); + // Do not initialize timeouts for tests. + if should_timeout { + let handler = TransitionHandler { engine: Arc::downgrade(&engine) }; + try!(engine.transition_service.register_handler(Arc::new(handler))); + } Ok(engine) } diff --git a/sync/src/tests/consensus.rs b/sync/src/tests/consensus.rs index c1fa8fce7..e1f73d066 100644 --- a/sync/src/tests/consensus.rs +++ b/sync/src/tests/consensus.rs @@ -82,12 +82,14 @@ fn authority_round() { net.sync(); // Trigger block proposal net.peer(0).chain.miner().import_own_transaction(&*net.peer(0).chain, new_tx(s0.secret(), 0.into())).unwrap(); + net.peer(1).chain.miner().import_own_transaction(&*net.peer(1).chain, new_tx(s1.secret(), 0.into())).unwrap(); // Sync a block net.sync(); assert_eq!(net.peer(0).chain.chain_info().best_block_number, 1); assert_eq!(net.peer(1).chain.chain_info().best_block_number, 1); - net.peer(1).chain.miner().import_own_transaction(&*net.peer(1).chain, new_tx(s1.secret(), 0.into())).unwrap(); + net.peer(0).chain.miner().import_own_transaction(&*net.peer(0).chain, new_tx(s0.secret(), 1.into())).unwrap(); + net.peer(1).chain.miner().import_own_transaction(&*net.peer(1).chain, new_tx(s1.secret(), 1.into())).unwrap(); // Move to next proposer step net.peer(0).chain.engine().step(); net.peer(1).chain.engine().step(); @@ -96,14 +98,17 @@ fn authority_round() { assert_eq!(net.peer(1).chain.chain_info().best_block_number, 2); // Fork the network - net.peer(0).chain.miner().import_own_transaction(&*net.peer(0).chain, new_tx(s0.secret(), 1.into())).unwrap(); + net.peer(0).chain.miner().import_own_transaction(&*net.peer(0).chain, new_tx(s0.secret(), 2.into())).unwrap(); + net.peer(1).chain.miner().import_own_transaction(&*net.peer(1).chain, new_tx(s1.secret(), 2.into())).unwrap(); net.peer(0).chain.engine().step(); net.peer(1).chain.engine().step(); - assert_eq!(net.peer(0).chain.chain_info().best_block_number, 3); - net.peer(1).chain.miner().import_own_transaction(&*net.peer(1).chain, new_tx(s1.secret(), 1.into())).unwrap(); net.peer(0).chain.engine().step(); net.peer(1).chain.engine().step(); - assert_eq!(net.peer(1).chain.chain_info().best_block_number, 3); + let ci0 = net.peer(0).chain.chain_info(); + let ci1 = net.peer(1).chain.chain_info(); + assert_eq!(ci0.best_block_number, 3); + assert_eq!(ci1.best_block_number, 3); + assert!(ci0.best_block_hash != ci1.best_block_hash); // Reorg to the correct one. net.sync(); let ci0 = net.peer(0).chain.chain_info(); From d8470cc5ca4dc0e3c21d95972d5333ac2e5cc555 Mon Sep 17 00:00:00 2001 From: keorn Date: Fri, 16 Dec 2016 00:07:07 +0100 Subject: [PATCH 4/4] revert slack --- ethcore/src/engines/authority_round.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ethcore/src/engines/authority_round.rs b/ethcore/src/engines/authority_round.rs index 57bb513ae..11d77bc78 100644 --- a/ethcore/src/engines/authority_round.rs +++ b/ethcore/src/engines/authority_round.rs @@ -466,7 +466,7 @@ mod tests { // Spec starts with step 2. header.set_seal(vec![encode(&1usize).to_vec(), encode(&(&*signature as &[u8])).to_vec()]); assert!(engine.verify_block_seal(&header).is_ok()); - header.set_seal(vec![encode(&11usize).to_vec(), encode(&(&*signature as &[u8])).to_vec()]); + header.set_seal(vec![encode(&5usize).to_vec(), encode(&(&*signature as &[u8])).to_vec()]); assert!(engine.verify_block_seal(&header).is_err()); } }