From 340d37793058c47d5095d5e1ff871ed1fb7c4163 Mon Sep 17 00:00:00 2001 From: keorn Date: Tue, 22 Nov 2016 16:05:27 +0000 Subject: [PATCH] Revert "dont keep account provider in miner" This reverts commit 11ccacd6d0a9989d07b941a77f7a4e9f5a6f7bb3. --- ethcore/res/tendermint.json | 18 ++--- ethcore/src/engines/authority_round.rs | 6 +- ethcore/src/engines/basic_authority.rs | 2 +- ethcore/src/engines/mod.rs | 6 +- ethcore/src/engines/tendermint/mod.rs | 107 ++++++++++++++++--------- parity/run.rs | 11 ++- 6 files changed, 94 insertions(+), 56 deletions(-) diff --git a/ethcore/res/tendermint.json b/ethcore/res/tendermint.json index 2f40d707b..ae265aa0b 100644 --- a/ethcore/res/tendermint.json +++ b/ethcore/res/tendermint.json @@ -12,16 +12,16 @@ } }, "params": { - "accountStartNonce": "0x0100000", + "accountStartNonce": "0x0", "maximumExtraDataSize": "0x20", "minGasLimit": "0x1388", - "networkID" : "0x69" + "networkID" : "0x2323" }, "genesis": { "seal": { "generic": { - "fields": 1, - "rlp": "0x11bbe8db4e347b4e8c937c1c8370e4b5ed33adb3db69cbdb7a38e1e50b1b82fa" + "fields": 3, + "rlp": "0x40010" } }, "difficulty": "0x20000", @@ -32,10 +32,10 @@ "gasLimit": "0x2fefd8" }, "accounts": { - "0000000000000000000000000000000000000001": { "balance": "1", "nonce": "1048576", "builtin": { "name": "ecrecover", "pricing": { "linear": { "base": 3000, "word": 0 } } } }, - "0000000000000000000000000000000000000002": { "balance": "1", "nonce": "1048576", "builtin": { "name": "sha256", "pricing": { "linear": { "base": 60, "word": 12 } } } }, - "0000000000000000000000000000000000000003": { "balance": "1", "nonce": "1048576", "builtin": { "name": "ripemd160", "pricing": { "linear": { "base": 600, "word": 120 } } } }, - "0000000000000000000000000000000000000004": { "balance": "1", "nonce": "1048576", "builtin": { "name": "identity", "pricing": { "linear": { "base": 15, "word": 3 } } } }, - "9cce34f7ab185c7aba1b7c8140d620b4bda941d6": { "balance": "1606938044258990275541962092341162602522202993782792835301376", "nonce": "1048576" } + "0000000000000000000000000000000000000001": { "balance": "1", "builtin": { "name": "ecrecover", "pricing": { "linear": { "base": 3000, "word": 0 } } } }, + "0000000000000000000000000000000000000002": { "balance": "1", "builtin": { "name": "sha256", "pricing": { "linear": { "base": 60, "word": 12 } } } }, + "0000000000000000000000000000000000000003": { "balance": "1", "builtin": { "name": "ripemd160", "pricing": { "linear": { "base": 600, "word": 120 } } } }, + "0000000000000000000000000000000000000004": { "balance": "1", "builtin": { "name": "identity", "pricing": { "linear": { "base": 15, "word": 3 } } } }, + "9cce34f7ab185c7aba1b7c8140d620b4bda941d6": { "balance": "1606938044258990275541962092341162602522202993782792835301376" } } } diff --git a/ethcore/src/engines/authority_round.rs b/ethcore/src/engines/authority_round.rs index ab2b41aec..e90bc73f2 100644 --- a/ethcore/src/engines/authority_round.rs +++ b/ethcore/src/engines/authority_round.rs @@ -68,9 +68,9 @@ pub struct AuthorityRound { builtins: BTreeMap, transition_service: IoService, message_channel: Mutex>>, - account_provider: Mutex>>, step: AtomicUsize, proposed: AtomicBool, + account_provider: Mutex>>, } fn header_step(header: &Header) -> Result { @@ -102,9 +102,9 @@ impl AuthorityRound { builtins: builtins, transition_service: try!(IoService::::start()), message_channel: Mutex::new(None), - account_provider: Mutex::new(None), step: AtomicUsize::new(initial_step), - proposed: AtomicBool::new(false) + proposed: AtomicBool::new(false), + account_provider: Mutex::new(None), }); let handler = TransitionHandler { engine: Arc::downgrade(&engine) }; try!(engine.transition_service.register_handler(Arc::new(handler))); diff --git a/ethcore/src/engines/basic_authority.rs b/ethcore/src/engines/basic_authority.rs index c43495967..7e0402dab 100644 --- a/ethcore/src/engines/basic_authority.rs +++ b/ethcore/src/engines/basic_authority.rs @@ -58,7 +58,7 @@ pub struct BasicAuthority { params: CommonParams, our_params: BasicAuthorityParams, builtins: BTreeMap, - account_provider: Mutex>> + account_provider: Mutex>>, } impl BasicAuthority { diff --git a/ethcore/src/engines/mod.rs b/ethcore/src/engines/mod.rs index 5aae9ee78..ab50ee9c9 100644 --- a/ethcore/src/engines/mod.rs +++ b/ethcore/src/engines/mod.rs @@ -174,11 +174,11 @@ pub trait Engine : Sync + Send { /// Add a channel for communication with Client which can be used for sealing. fn register_message_channel(&self, _message_channel: IoChannel) {} + /// Add an account provider useful for Engines that sign stuff. + fn register_account_provider(&self, _account_provider: Arc) {} + /// Check if new block should be chosen as the one in chain. fn is_new_best_block(&self, best_total_difficulty: U256, _best_header: HeaderView, parent_details: &BlockDetails, new_header: &HeaderView) -> bool { ethash::is_new_best_block(best_total_difficulty, parent_details, new_header) } - - /// Add an account provider useful for Engines that sign stuff. - fn register_account_provider(&self, _account_provider: Arc) {} } diff --git a/ethcore/src/engines/tendermint/mod.rs b/ethcore/src/engines/tendermint/mod.rs index 48e3b3599..b110a602d 100644 --- a/ethcore/src/engines/tendermint/mod.rs +++ b/ethcore/src/engines/tendermint/mod.rs @@ -202,26 +202,22 @@ impl Tendermint { } } - fn nonce_proposer(&self, proposer_nonce: usize) -> &Address { - let ref p = self.our_params; - p.authorities.get(proposer_nonce % p.authority_n).expect("There are authority_n authorities; taking number modulo authority_n gives number in authority_n range; qed") - } - - fn is_nonce_proposer(&self, proposer_nonce: usize, address: &Address) -> bool { - self.nonce_proposer(proposer_nonce) == address - } - fn is_authority(&self, address: &Address) -> bool { self.our_params.authorities.contains(address) } - fn threshold(&self) -> usize { - self.our_params.authority_n * 2/3 + fn is_above_threshold(&self, n: usize) -> bool { + n > self.our_params.authority_n * 2/3 } /// Round proposer switching. fn is_proposer(&self, address: &Address) -> bool { - self.is_nonce_proposer(self.proposer_nonce.load(AtomicOrdering::SeqCst), address) + let ref p = self.our_params; + let proposer_nonce = self.proposer_nonce.load(AtomicOrdering::SeqCst); + let proposer = p.authorities.get(proposer_nonce % p.authority_n).expect("There are authority_n authorities; taking number modulo authority_n gives number in authority_n range; qed"); + println!("{:?}", &proposer); + println!("{:?}", &address); + proposer == address } fn is_height(&self, message: &ConsensusMessage) -> bool { @@ -251,16 +247,22 @@ impl Tendermint { fn has_enough_any_votes(&self) -> bool { - self.votes.count_step_votes(self.height.load(AtomicOrdering::SeqCst), self.round.load(AtomicOrdering::SeqCst), *self.step.read()) > self.threshold() + let step_votes = self.votes.count_step_votes(self.height.load(AtomicOrdering::SeqCst), self.round.load(AtomicOrdering::SeqCst), *self.step.read()); + self.is_above_threshold(step_votes) } fn has_enough_future_step_votes(&self, message: &ConsensusMessage) -> bool { - message.round > self.round.load(AtomicOrdering::SeqCst) - && self.votes.count_step_votes(message.height, message.round, message.step) > self.threshold() + if message.round > self.round.load(AtomicOrdering::SeqCst) { + let step_votes = self.votes.count_step_votes(message.height, message.round, message.step); + self.is_above_threshold(step_votes) + } else { + false + } } fn has_enough_aligned_votes(&self, message: &ConsensusMessage) -> bool { - self.votes.aligned_votes(&message).len() > self.threshold() + let aligned_votes = self.votes.aligned_votes(&message).len(); + self.is_above_threshold(aligned_votes) } } @@ -476,6 +478,13 @@ mod tests { use super::params::TendermintParams; use super::message::*; + fn setup() -> (Spec, Arc) { + let tap = Arc::new(AccountProvider::transient_provider()); + let spec = Spec::new_test_tendermint(); + spec.engine.register_account_provider(tap.clone()); + (spec, tap) + } + fn propose_default(engine: &Arc, db: &StateDB, proposer: Address) -> Option> { let mut header = Header::default(); let last_hashes = Arc::new(vec![]); @@ -489,9 +498,7 @@ mod tests { engine.handle_message(UntrustedRlp::new(&m)).unwrap(); } - fn proposal_seal(header: &Header, round: Round) -> Vec { - let tap = AccountProvider::transient_provider(); - + fn proposal_seal(tap: &Arc, header: &Header, round: Round) -> Vec { let author = header.author(); let vote_info = message_info_rlp(header.number() as Height, round, Step::Propose, Some(header.bare_hash())); let signature = tap.sign(*author, None, vote_info.sha3()).unwrap(); @@ -502,6 +509,12 @@ mod tests { ] } + fn insert_and_unlock(tap: &Arc, acc: &str) -> Address { + let addr = tap.insert_account(acc.sha3(), acc).unwrap(); + tap.unlock_account_permanently(addr, acc.into()).unwrap(); + addr + } + fn default_seal() -> Vec { vec![vec![160, 39, 191, 179, 126, 80, 124, 233, 13, 161, 65, 48, 114, 4, 177, 198, 186, 36, 25, 67, 128, 97, 53, 144, 172, 80, 202, 75, 29, 113, 152, 255, 101]] } @@ -544,21 +557,44 @@ mod tests { } #[test] - fn verification_fails_on_wrong_signatures() { - let engine = Spec::new_test_tendermint().engine; - let mut header = Header::default(); - let tap = AccountProvider::transient_provider(); + fn allows_correct_proposer() { + ::env_logger::init().unwrap(); + let (spec, tap) = setup(); + let engine = spec.engine; + + let mut header = Header::default(); + let validator = insert_and_unlock(&tap, "0"); + header.set_author(validator); + let seal = proposal_seal(&tap, &header, 0); + header.set_seal(seal); + // Good proposer. + assert!(engine.verify_block_unordered(&header, None).is_ok()); + + let mut header = Header::default(); + let random = insert_and_unlock(&tap, "101"); + header.set_author(random); + let seal = proposal_seal(&tap, &header, 0); + header.set_seal(seal); + // Bad proposer. + assert!(engine.verify_block_unordered(&header, None).is_err()); + } + + #[test] + fn verification_fails_on_wrong_signatures() { + let (spec, tap) = setup(); + let engine = spec.engine; + let mut header = Header::default(); - let mut seal = Vec::new(); let v1 = tap.insert_account("0".sha3(), "0").unwrap(); - let sig1 = tap.sign(v1, Some("0".into()), header.bare_hash()).unwrap(); - seal.push(::rlp::encode(&(&*sig1 as &[u8])).to_vec()); + + header.set_author(v1); + let mut seal = proposal_seal(&tap, &header, 0); header.set_seal(seal.clone()); // Not enough signatures. - assert!(engine.verify_block_basic(&header, None).is_err()); + assert!(engine.verify_block_unordered(&header, None).is_err()); let v2 = tap.insert_account("101".sha3(), "101").unwrap(); let sig2 = tap.sign(v2, Some("101".into()), header.bare_hash()).unwrap(); @@ -567,7 +603,7 @@ mod tests { header.set_seal(seal); // Enough signatures. - assert!(engine.verify_block_basic(&header, None).is_ok()); + assert!(engine.verify_block_unordered(&header, None).is_ok()); let verify_result = engine.verify_block_unordered(&header, None); @@ -581,10 +617,11 @@ mod tests { #[test] fn seal_with_enough_signatures_is_ok() { - let engine = Spec::new_test_tendermint().engine; + let (spec, tap) = setup(); + let engine = spec.engine; let mut header = Header::default(); - let seal = proposal_seal(&header, 0); + let seal = proposal_seal(&tap, &header, 0); header.set_seal(seal); // Enough signatures. @@ -596,20 +633,18 @@ mod tests { #[test] fn can_generate_seal() { - let spec = Spec::new_test_tendermint(); - let ref engine = *spec.engine; - let tender = Tendermint::new(engine.params().clone(), TendermintParams::default(), BTreeMap::new()).unwrap(); + let (spec, _) = setup(); let genesis_header = spec.genesis_header(); let mut db_result = get_temp_state_db(); let mut db = db_result.take(); spec.ensure_db_good(&mut db).unwrap(); let last_hashes = Arc::new(vec![genesis_header.hash()]); - let b = OpenBlock::new(engine, Default::default(), false, db, &genesis_header, last_hashes, Address::default(), (3141562.into(), 31415620.into()), vec![]).unwrap(); + let b = OpenBlock::new(spec.engine.as_ref(), Default::default(), false, db, &genesis_header, last_hashes, Address::default(), (3141562.into(), 31415620.into()), vec![]).unwrap(); let b = b.close_and_lock(); - let seal = tender.generate_seal(b.block()).unwrap(); - assert!(b.try_seal(engine, seal).is_ok()); + let seal = spec.engine.generate_seal(b.block()).unwrap(); + assert!(b.try_seal(spec.engine.as_ref(), seal).is_ok()); } #[test] diff --git a/parity/run.rs b/parity/run.rs index aae4db748..0f00bba97 100644 --- a/parity/run.rs +++ b/parity/run.rs @@ -203,8 +203,14 @@ pub fn execute(cmd: RunCmd, logger: Arc) -> Result<(), String> { sync_config.fork_block = spec.fork_block(); sync_config.warp_sync = cmd.warp_sync; + // prepare account provider + let account_provider = Arc::new(try!(prepare_account_provider(&cmd.dirs, cmd.acc_conf))); + + // let the Engine access the accounts + spec.engine.register_account_provider(account_provider); + // create miner - let miner = Miner::new(cmd.miner_options, cmd.gas_pricer.into(), &spec); + let miner = Miner::new(cmd.miner_options, cmd.gas_pricer.into(), &spec, Some(account_provider.clone())); miner.set_author(cmd.miner_extras.author); miner.set_gas_floor_target(cmd.miner_extras.gas_floor_target); miner.set_gas_ceil_target(cmd.miner_extras.gas_ceil_target); @@ -238,9 +244,6 @@ pub fn execute(cmd: RunCmd, logger: Arc) -> Result<(), String> { // create supervisor let mut hypervisor = modules::hypervisor(&cmd.dirs.ipc_path()); - // prepare account provider - let account_provider = Arc::new(try!(prepare_account_provider(&cmd.dirs, cmd.acc_conf))); - // create client service. let service = try!(ClientService::start( client_config,