From 2851b0d36cd8635713fc10143e24cc6c19cb3bcf Mon Sep 17 00:00:00 2001 From: Vladimir Komendantskiy Date: Thu, 12 Sep 2019 16:44:38 +0100 Subject: [PATCH] moved clear_signer functionality to set_signer --- ethcore/engine/src/engine.rs | 5 +- ethcore/engines/authority-round/src/lib.rs | 58 ++++++++++------------ ethcore/engines/basic-authority/src/lib.rs | 14 ++---- ethcore/engines/clique/src/lib.rs | 5 +- ethcore/src/miner/miner.rs | 4 +- 5 files changed, 37 insertions(+), 49 deletions(-) diff --git a/ethcore/engine/src/engine.rs b/ethcore/engine/src/engine.rs index 31476d40498..357a184c7af 100644 --- a/ethcore/engine/src/engine.rs +++ b/ethcore/engine/src/engine.rs @@ -293,10 +293,7 @@ pub trait Engine: Sync + Send { fn handle_message(&self, _message: &[u8]) -> Result<(), EngineError> { Err(EngineError::UnexpectedMessage) } /// Register a component which signs consensus messages. - fn set_signer(&self, _signer: Box) {} - - /// Unregisters the engine signer address to stop signing consensus messages. - fn clear_signer(&self) {} + fn set_signer(&self, _signer: Option>) {} /// Sign using the EngineSigner, to be used for consensus tx signing. fn sign(&self, _hash: H256) -> Result { unimplemented!() } diff --git a/ethcore/engines/authority-round/src/lib.rs b/ethcore/engines/authority-round/src/lib.rs index d23f2a9a220..dd832aa5aa9 100644 --- a/ethcore/engines/authority-round/src/lib.rs +++ b/ethcore/engines/authority-round/src/lib.rs @@ -1622,12 +1622,8 @@ impl Engine for AuthorityRound { self.validators.register_client(client); } - fn set_signer(&self, signer: Box) { - *self.signer.write() = Some(signer); - } - - fn clear_signer(&self) { - *self.signer.write() = Default::default(); + fn set_signer(&self, signer: Option>) { + *self.signer.write() = signer; } fn sign(&self, hash: H256) -> Result { @@ -1763,7 +1759,7 @@ mod tests { let b1 = OpenBlock::new(engine, Default::default(), false, db1, &genesis_header, last_hashes.clone(), addr1, (3141562.into(), 31415620.into()), vec![], false).unwrap(); let b1 = b1.close_and_lock().unwrap(); - engine.set_signer(Box::new((tap.clone(), addr1, "1".into()))); + engine.set_signer(Some(Box::new((tap.clone(), addr1, "1".into())))); if let Seal::Regular(seal) = engine.generate_seal(&b1, &genesis_header) { assert!(b1.clone().try_seal(engine, seal).is_ok()); // Second proposal is forbidden. @@ -1789,7 +1785,7 @@ mod tests { // Not a signer. A seal cannot be generated. assert!(engine.generate_seal(&b1, &genesis_header) == Seal::None); // Become a signer. - engine.set_signer(Box::new((tap.clone(), addr1, "1".into()))); + engine.set_signer(Some(Box::new((tap.clone(), addr1, "1".into())))); if let Seal::Regular(seal) = engine.generate_seal(&b1, &genesis_header) { assert!(b1.clone().try_seal(engine, seal).is_ok()); // Second proposal is forbidden. @@ -1798,7 +1794,7 @@ mod tests { panic!("block 1 not sealed"); } // Stop being a signer. - engine.clear_signer(); + engine.set_signer(None); // Make a step first and then create a new block in that new step. engine.step(); let addr2 = tap.insert_account(keccak("0").into(), &"0".into()).unwrap(); @@ -1814,7 +1810,7 @@ mod tests { // Not a signer. A seal cannot be generated. assert!(engine.generate_seal(&b2, &header2) == Seal::None); // Become a signer once more. - engine.set_signer(Box::new((tap, addr2, "0".into()))); + engine.set_signer(Some(Box::new((tap, addr2, "0".into())))); if let Seal::Regular(seal) = engine.generate_seal(&b2, &header2) { assert!(b2.clone().try_seal(engine, seal).is_ok()); // Second proposal is forbidden. @@ -1843,13 +1839,13 @@ mod tests { let b2 = OpenBlock::new(engine, Default::default(), false, db2, &genesis_header, last_hashes, addr2, (3141562.into(), 31415620.into()), vec![], false).unwrap(); let b2 = b2.close_and_lock().unwrap(); - engine.set_signer(Box::new((tap.clone(), addr1, "1".into()))); + engine.set_signer(Some(Box::new((tap.clone(), addr1, "1".into())))); match engine.generate_seal(&b1, &genesis_header) { Seal::None => panic!("wrong seal"), Seal::Regular(_) => { engine.step(); - engine.set_signer(Box::new((tap.clone(), addr2, "0".into()))); + engine.set_signer(Some(Box::new((tap.clone(), addr2, "0".into())))); match engine.generate_seal(&b2, &genesis_header) { Seal::Regular(_) => panic!("sealed despite wrong difficulty"), Seal::None => {} @@ -1967,11 +1963,11 @@ mod tests { assert!(aura.verify_block_family(&header, &parent_header).is_ok()); assert_eq!(last_benign.load(AtomicOrdering::SeqCst), 0); - aura.set_signer(Box::new(( + aura.set_signer(Some(Box::new(( Arc::new(AccountProvider::transient_provider()), validator2, "".into(), - ))); + )))); // Do not report on steps skipped between genesis and first block. header.set_number(1); @@ -2086,7 +2082,7 @@ mod tests { client.add_notify(notify.clone()); engine.register_client(Arc::downgrade(&client) as _); - engine.set_signer(Box::new((tap.clone(), addr1, "1".into()))); + engine.set_signer(Some(Box::new((tap.clone(), addr1, "1".into())))); let b1 = OpenBlock::new(engine, Default::default(), false, db1, &genesis_header, last_hashes.clone(), addr1, (3141562.into(), 31415620.into()), vec![], false).unwrap(); let b1 = b1.close_and_lock().unwrap(); @@ -2130,7 +2126,7 @@ mod tests { let b1 = b1.close_and_lock().unwrap(); // since the block is empty it isn't sealed and we generate empty steps - engine.set_signer(Box::new((tap.clone(), addr1, "1".into()))); + engine.set_signer(Some(Box::new((tap.clone(), addr1, "1".into())))); assert_eq!(engine.generate_seal(&b1, &genesis_header), Seal::None); engine.step(); @@ -2147,9 +2143,9 @@ mod tests { let b2 = b2.close_and_lock().unwrap(); // we will now seal a block with 1tx and include the accumulated empty step message - engine.set_signer(Box::new((tap.clone(), addr2, "0".into()))); + engine.set_signer(Some(Box::new((tap.clone(), addr2, "0".into())))); if let Seal::Regular(seal) = engine.generate_seal(&b2, &genesis_header) { - engine.set_signer(Box::new((tap.clone(), addr1, "1".into()))); + engine.set_signer(Some(Box::new((tap.clone(), addr1, "1".into())))); let empty_step2 = sealed_empty_step(engine, 2, &genesis_header.hash()); let empty_steps = ::rlp::encode_list(&vec![empty_step2]); @@ -2183,14 +2179,14 @@ mod tests { let b1 = b1.close_and_lock().unwrap(); // since the block is empty it isn't sealed and we generate empty steps - engine.set_signer(Box::new((tap.clone(), addr1, "1".into()))); + engine.set_signer(Some(Box::new((tap.clone(), addr1, "1".into())))); assert_eq!(engine.generate_seal(&b1, &genesis_header), Seal::None); engine.step(); // step 3 let b2 = OpenBlock::new(engine, Default::default(), false, db2, &genesis_header, last_hashes.clone(), addr2, (3141562.into(), 31415620.into()), vec![], false).unwrap(); let b2 = b2.close_and_lock().unwrap(); - engine.set_signer(Box::new((tap.clone(), addr2, "0".into()))); + engine.set_signer(Some(Box::new((tap.clone(), addr2, "0".into())))); assert_eq!(engine.generate_seal(&b2, &genesis_header), Seal::None); engine.step(); @@ -2199,10 +2195,10 @@ mod tests { let b3 = OpenBlock::new(engine, Default::default(), false, db3, &genesis_header, last_hashes.clone(), addr1, (3141562.into(), 31415620.into()), vec![], false).unwrap(); let b3 = b3.close_and_lock().unwrap(); - engine.set_signer(Box::new((tap.clone(), addr1, "1".into()))); + engine.set_signer(Some(Box::new((tap.clone(), addr1, "1".into())))); if let Seal::Regular(seal) = engine.generate_seal(&b3, &genesis_header) { let empty_step2 = sealed_empty_step(engine, 2, &genesis_header.hash()); - engine.set_signer(Box::new((tap.clone(), addr2, "0".into()))); + engine.set_signer(Some(Box::new((tap.clone(), addr2, "0".into())))); let empty_step3 = sealed_empty_step(engine, 3, &genesis_header.hash()); let empty_steps = ::rlp::encode_list(&vec![empty_step2, empty_step3]); @@ -2233,7 +2229,7 @@ mod tests { let b1 = b1.close_and_lock().unwrap(); // since the block is empty it isn't sealed and we generate empty steps - engine.set_signer(Box::new((tap.clone(), addr1, "1".into()))); + engine.set_signer(Some(Box::new((tap.clone(), addr1, "1".into())))); assert_eq!(engine.generate_seal(&b1, &genesis_header), Seal::None); engine.step(); @@ -2287,7 +2283,7 @@ mod tests { ); // empty step with valid signature from incorrect proposer for step - engine.set_signer(Box::new((tap.clone(), addr1, "1".into()))); + engine.set_signer(Some(Box::new((tap.clone(), addr1, "1".into())))); let empty_steps = vec![sealed_empty_step(engine, 1, &parent_header.hash())]; set_empty_steps_seal(&mut header, 2, &signature, &empty_steps); @@ -2297,9 +2293,9 @@ mod tests { ); // valid empty steps - engine.set_signer(Box::new((tap.clone(), addr1, "1".into()))); + engine.set_signer(Some(Box::new((tap.clone(), addr1, "1".into())))); let empty_step2 = sealed_empty_step(engine, 2, &parent_header.hash()); - engine.set_signer(Box::new((tap.clone(), addr2, "0".into()))); + engine.set_signer(Some(Box::new((tap.clone(), addr2, "0".into())))); let empty_step3 = sealed_empty_step(engine, 3, &parent_header.hash()); let empty_steps = vec![empty_step2, empty_step3]; @@ -2343,7 +2339,7 @@ mod tests { let b1 = b1.close_and_lock().unwrap(); // since the block is empty it isn't sealed and we generate empty steps - engine.set_signer(Box::new((tap.clone(), addr1, "1".into()))); + engine.set_signer(Some(Box::new((tap.clone(), addr1, "1".into())))); assert_eq!(engine.generate_seal(&b1, &genesis_header), Seal::None); engine.step(); @@ -2380,7 +2376,7 @@ mod tests { let engine = &*spec.engine; let addr1 = accounts[0]; - engine.set_signer(Box::new((tap.clone(), addr1, "1".into()))); + engine.set_signer(Some(Box::new((tap.clone(), addr1, "1".into())))); let mut header: Header = Header::default(); let empty_step = empty_step(engine, 1, &header.parent_hash()); @@ -2461,7 +2457,7 @@ mod tests { header.set_author(accounts[0]); // when - engine.set_signer(Box::new((tap.clone(), accounts[1], "0".into()))); + engine.set_signer(Some(Box::new((tap.clone(), accounts[1], "0".into())))); let empty_steps = vec![ sealed_empty_step(&*engine, 1, &parent.hash()), sealed_empty_step(&*engine, 1, &parent.hash()), @@ -2498,9 +2494,9 @@ mod tests { header.set_author(accounts[0]); // when - engine.set_signer(Box::new((tap.clone(), accounts[1], "0".into()))); + engine.set_signer(Some(Box::new((tap.clone(), accounts[1], "0".into())))); let es1 = sealed_empty_step(&*engine, 1, &parent.hash()); - engine.set_signer(Box::new((tap.clone(), accounts[0], "1".into()))); + engine.set_signer(Some(Box::new((tap.clone(), accounts[0], "1".into())))); let es2 = sealed_empty_step(&*engine, 2, &parent.hash()); let mut empty_steps = vec![es2, es1]; diff --git a/ethcore/engines/basic-authority/src/lib.rs b/ethcore/engines/basic-authority/src/lib.rs index 37a70272b53..bae548781b4 100644 --- a/ethcore/engines/basic-authority/src/lib.rs +++ b/ethcore/engines/basic-authority/src/lib.rs @@ -194,12 +194,8 @@ impl Engine for BasicAuthority { } } - fn set_signer(&self, signer: Box) { - *self.signer.write() = Some(signer); - } - - fn clear_signer(&self) { - *self.signer.write() = Default::default(); + fn set_signer(&self, signer: Option>) { + *self.signer.write() = signer; } fn sign(&self, hash: H256) -> Result { @@ -273,7 +269,7 @@ mod tests { let spec = new_test_authority(); let engine = &*spec.engine; - engine.set_signer(Box::new((Arc::new(tap), addr, "".into()))); + engine.set_signer(Some(Box::new((Arc::new(tap), addr, "".into())))); let genesis_header = spec.genesis_header(); let db = spec.ensure_db_good(get_temp_state_db(), &Default::default()).unwrap(); let last_hashes = Arc::new(vec![genesis_header.hash()]); @@ -291,9 +287,9 @@ mod tests { let engine = new_test_authority().engine; assert_eq!(SealingState::NotReady, engine.sealing_state()); - engine.set_signer(Box::new((Arc::new(tap), authority, "".into()))); + engine.set_signer(Some(Box::new((Arc::new(tap), authority, "".into())))); assert_eq!(SealingState::Ready, engine.sealing_state()); - engine.clear_signer(); + engine.set_signer(None); assert_eq!(SealingState::NotReady, engine.sealing_state()); } } diff --git a/ethcore/engines/clique/src/lib.rs b/ethcore/engines/clique/src/lib.rs index 05cb5d2621c..d5343360b5f 100644 --- a/ethcore/engines/clique/src/lib.rs +++ b/ethcore/engines/clique/src/lib.rs @@ -762,9 +762,8 @@ impl Engine for Clique { } } - fn set_signer(&self, signer: Box) { - trace!(target: "engine", "set_signer: {}", signer.address()); - *self.signer.write() = Some(signer); + fn set_signer(&self, signer: Option>) { + *self.signer.write() = signer; } fn register_client(&self, client: Weak) { diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 23c679fb58b..566a02bf233 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -911,7 +911,7 @@ impl miner::MinerService for Miner { // | (some `Engine`s call `EngineClient.update_sealing()`) | // | Make sure to release the locks before calling that method. | // -------------------------------------------------------------------------- - self.engine.set_signer(signer); + self.engine.set_signer(Some(signer)); } else { warn!("Setting an EngineSigner while Engine does not require one."); } @@ -924,7 +924,7 @@ impl miner::MinerService for Miner { if self.engine.sealing_state() != SealingState::External { // Disable sealing. self.sealing.lock().enabled = false; - self.engine.clear_signer(); + self.engine.set_signer(None); } }