Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Commit

Permalink
moved clear_signer functionality to set_signer
Browse files Browse the repository at this point in the history
  • Loading branch information
@vkomenda @afck
vkomenda authored and afck committed Oct 8, 2019
1 parent 51bc4e8 commit 972c60d
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 49 deletions.
5 changes: 1 addition & 4 deletions ethcore/engine/src/engine.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<dyn EngineSigner>) {}

/// Unregisters the engine signer address to stop signing consensus messages.
fn clear_signer(&self) {}
fn set_signer(&self, _signer: Option<Box<dyn EngineSigner>>) {}

/// Sign using the EngineSigner, to be used for consensus tx signing.
fn sign(&self, _hash: H256) -> Result<Signature, Error> { unimplemented!() }
Expand Down
58 changes: 27 additions & 31 deletions ethcore/engines/authority-round/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1622,12 +1622,8 @@ impl Engine for AuthorityRound {
self.validators.register_client(client);
}

fn set_signer(&self, signer: Box<dyn EngineSigner>) {
*self.signer.write() = Some(signer);
}

fn clear_signer(&self) {
*self.signer.write() = Default::default();
fn set_signer(&self, signer: Option<Box<dyn EngineSigner>>) {
*self.signer.write() = signer;
}

fn sign(&self, hash: H256) -> Result<Signature, Error> {
Expand Down Expand Up @@ -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.
Expand All @@ -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.
Expand All @@ -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();
Expand All @@ -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.
Expand Down Expand Up @@ -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 => {}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();

Expand All @@ -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]);

Expand Down Expand Up @@ -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();

Expand All @@ -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]);
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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);

Expand All @@ -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];
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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()),
Expand Down Expand Up @@ -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];
Expand Down
14 changes: 5 additions & 9 deletions ethcore/engines/basic-authority/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,12 +194,8 @@ impl Engine for BasicAuthority {
}
}

fn set_signer(&self, signer: Box<dyn EngineSigner>) {
*self.signer.write() = Some(signer);
}

fn clear_signer(&self) {
*self.signer.write() = Default::default();
fn set_signer(&self, signer: Option<Box<dyn EngineSigner>>) {
*self.signer.write() = signer;
}

fn sign(&self, hash: H256) -> Result<Signature, Error> {
Expand Down Expand Up @@ -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()]);
Expand All @@ -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());
}
}
5 changes: 2 additions & 3 deletions ethcore/engines/clique/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -762,9 +762,8 @@ impl Engine for Clique {
}
}

fn set_signer(&self, signer: Box<dyn EngineSigner>) {
trace!(target: "engine", "set_signer: {}", signer.address());
*self.signer.write() = Some(signer);
fn set_signer(&self, signer: Option<Box<dyn EngineSigner>>) {
*self.signer.write() = signer;
}

fn register_client(&self, client: Weak<dyn EngineClient>) {
Expand Down
4 changes: 2 additions & 2 deletions ethcore/src/miner/miner.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -905,7 +905,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.");
}
Expand All @@ -918,7 +918,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);
}
}

Expand Down

0 comments on commit 972c60d

Please sign in to comment.