From e72fc5398ac67d120912ef61d60e6bfc0074beab Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Fri, 5 Aug 2016 23:33:55 +0200 Subject: [PATCH] miner and client take spec reference (#1853) * miner and client take spec reference * fix tests --- ethcore/src/client/client.rs | 1 + ethcore/src/client/test_client.rs | 6 +++--- ethcore/src/json_tests/chain.rs | 7 +++---- ethcore/src/miner/miner.rs | 31 +++++++++++++------------------ ethcore/src/miner/mod.rs | 4 ++-- ethcore/src/service.rs | 7 ++++--- ethcore/src/snapshot/service.rs | 16 +++++++++------- ethcore/src/tests/client.rs | 9 ++++++--- ethcore/src/tests/helpers.rs | 5 +++-- ethcore/src/tests/rpc.rs | 5 +++-- parity/blockchain.rs | 8 ++++---- parity/run.rs | 8 ++------ parity/snapshot.rs | 4 ++-- rpc/src/v1/tests/eth.rs | 20 +++++++++----------- sync/src/lib.rs | 5 +++-- 15 files changed, 67 insertions(+), 69 deletions(-) diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index 67937ff45..ca23fb5a1 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -193,6 +193,7 @@ impl Client { } let engine = spec.engine.clone(); + let block_queue = BlockQueue::new(config.queue, engine.clone(), message_channel.clone()); let panic_handler = PanicHandler::new_in_arc(); panic_handler.forward_from(&block_queue); diff --git a/ethcore/src/client/test_client.rs b/ethcore/src/client/test_client.rs index 0e7157f38..113974dee 100644 --- a/ethcore/src/client/test_client.rs +++ b/ethcore/src/client/test_client.rs @@ -96,7 +96,7 @@ impl Default for TestBlockChainClient { impl TestBlockChainClient { /// Creates new test client. pub fn new() -> Self { - + let spec = Spec::new_test(); let mut client = TestBlockChainClient { blocks: RwLock::new(HashMap::new()), numbers: RwLock::new(HashMap::new()), @@ -110,8 +110,8 @@ impl TestBlockChainClient { execution_result: RwLock::new(None), receipts: RwLock::new(HashMap::new()), queue_size: AtomicUsize::new(0), - miner: Arc::new(Miner::with_spec(Spec::new_test())), - spec: Spec::new_test(), + miner: Arc::new(Miner::with_spec(&spec)), + spec: spec, vm_factory: EvmFactory::new(VMType::Interpreter), }; client.add_blocks(1, EachBlockWith::Nothing); // add genesis block diff --git a/ethcore/src/json_tests/chain.rs b/ethcore/src/json_tests/chain.rs index 37a70e48e..16161e158 100644 --- a/ethcore/src/json_tests/chain.rs +++ b/ethcore/src/json_tests/chain.rs @@ -22,7 +22,6 @@ use tests::helpers::*; use devtools::*; use spec::Genesis; use ethjson; -use ethjson::blockchain::BlockChain; use miner::Miner; use io::IoChannel; @@ -43,7 +42,7 @@ pub fn json_chain_test(json_data: &[u8], era: ChainEra) -> Vec { flush!(" - {}...", name); - let spec = |blockchain: &BlockChain| { + let spec = { let genesis = Genesis::from(blockchain.genesis()); let state = From::from(blockchain.pre_state.clone()); let mut spec = match era { @@ -61,9 +60,9 @@ pub fn json_chain_test(json_data: &[u8], era: ChainEra) -> Vec { { let client = Client::new( ClientConfig::default(), - &spec(&blockchain), + &spec, temp.as_path(), - Arc::new(Miner::with_spec(spec(&blockchain))), + Arc::new(Miner::with_spec(&spec)), IoChannel::disconnected() ).unwrap(); for b in &blockchain.blocks_rlp() { diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index f2296de00..421532278 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -177,7 +177,7 @@ pub struct Miner { gas_range_target: RwLock<(U256, U256)>, author: RwLock
, extra_data: RwLock, - spec: Spec, + engine: Arc, accounts: Option>, work_poster: Option, @@ -186,7 +186,7 @@ pub struct Miner { impl Miner { /// Creates new instance of miner without accounts, but with given spec. - pub fn with_spec(spec: Spec) -> Miner { + pub fn with_spec(spec: &Spec) -> Miner { Miner { transaction_queue: Arc::new(Mutex::new(TransactionQueue::new())), options: Default::default(), @@ -197,14 +197,14 @@ impl Miner { author: RwLock::new(Address::default()), extra_data: RwLock::new(Vec::new()), accounts: None, - spec: spec, + engine: spec.engine.clone(), work_poster: None, gas_pricer: Mutex::new(GasPricer::new_fixed(20_000_000_000u64.into())), } } /// Creates new instance of miner - pub fn new(options: MinerOptions, gas_pricer: GasPricer, spec: Spec, accounts: Option>) -> Arc { + pub fn new(options: MinerOptions, gas_pricer: GasPricer, spec: &Spec, accounts: Option>) -> Arc { let work_poster = if !options.new_work_notify.is_empty() { Some(WorkPoster::new(&options.new_work_notify)) } else { None }; let txq = Arc::new(Mutex::new(TransactionQueue::with_limits(options.tx_queue_size, options.tx_gas_limit))); Arc::new(Miner { @@ -217,16 +217,12 @@ impl Miner { extra_data: RwLock::new(Vec::new()), options: options, accounts: accounts, - spec: spec, + engine: spec.engine.clone(), work_poster: work_poster, gas_pricer: Mutex::new(gas_pricer), }) } - fn engine(&self) -> &Engine { - self.spec.engine.deref() - } - fn forced_sealing(&self) -> bool { self.options.force_sealing || !self.options.new_work_notify.is_empty() } @@ -274,8 +270,7 @@ impl Miner { Some(old_block) => { trace!(target: "miner", "Already have previous work; updating and returning"); // add transactions to old_block - let e = self.engine(); - old_block.reopen(e, chain.vm_factory()) + old_block.reopen(&*self.engine, chain.vm_factory()) } None => { // block not found - create it. @@ -338,13 +333,13 @@ impl Miner { if !block.transactions().is_empty() { trace!(target: "miner", "prepare_sealing: block has transaction - attempting internal seal."); // block with transactions - see if we can seal immediately. - let s = self.engine().generate_seal(block.block(), match self.accounts { + let s = self.engine.generate_seal(block.block(), match self.accounts { Some(ref x) => Some(&**x), None => None, }); if let Some(seal) = s { trace!(target: "miner", "prepare_sealing: managed internal seal. importing..."); - if let Ok(sealed) = block.lock().try_seal(self.engine(), seal) { + if let Ok(sealed) = block.lock().try_seal(&*self.engine, seal) { if let Ok(_) = chain.import_block(sealed.rlp_bytes()) { trace!(target: "miner", "prepare_sealing: sealed internally and imported. leaving."); } else { @@ -497,7 +492,7 @@ impl MinerService for Miner { state.add_balance(&sender, &(needed_balance - balance)); } let options = TransactOptions { tracing: analytics.transaction_tracing, vm_tracing: analytics.vm_tracing, check_nonce: false }; - let mut ret = try!(Executive::new(&mut state, &env_info, self.engine(), chain.vm_factory()).transact(t, options)); + let mut ret = try!(Executive::new(&mut state, &env_info, &*self.engine, chain.vm_factory()).transact(t, options)); // TODO gav move this into Executive. ret.state_diff = original_state.map(|original| state.diff_from(original)); @@ -795,7 +790,7 @@ impl MinerService for Miner { fn submit_seal(&self, chain: &MiningBlockChainClient, pow_hash: H256, seal: Vec) -> Result<(), Error> { let result = if let Some(b) = self.sealing_work.lock().queue.get_used_if(if self.options.enable_resubmission { GetAction::Clone } else { GetAction::Take }, |b| &b.hash() == &pow_hash) { - b.lock().try_seal(self.engine(), seal).or_else(|_| { + b.lock().try_seal(&*self.engine, seal).or_else(|_| { warn!(target: "miner", "Mined solution rejected: Invalid."); Err(Error::PowInvalid) }) @@ -897,7 +892,7 @@ mod tests { fn should_prepare_block_to_seal() { // given let client = TestBlockChainClient::default(); - let miner = Miner::with_spec(Spec::new_test()); + let miner = Miner::with_spec(&Spec::new_test()); // when let sealing_work = miner.map_sealing_work(&client, |_| ()); @@ -908,7 +903,7 @@ mod tests { fn should_still_work_after_a_couple_of_blocks() { // given let client = TestBlockChainClient::default(); - let miner = Miner::with_spec(Spec::new_test()); + let miner = Miner::with_spec(&Spec::new_test()); let res = miner.map_sealing_work(&client, |b| b.block().fields().header.hash()); assert!(res.is_some()); @@ -940,7 +935,7 @@ mod tests { enable_resubmission: true, }, GasPricer::new_fixed(0u64.into()), - Spec::new_test(), + &Spec::new_test(), None, // accounts provider )).ok().expect("Miner was just created.") } diff --git a/ethcore/src/miner/mod.rs b/ethcore/src/miner/mod.rs index a6cff9f55..f6d41b566 100644 --- a/ethcore/src/miner/mod.rs +++ b/ethcore/src/miner/mod.rs @@ -32,12 +32,12 @@ //! use ethcore::miner::{Miner, MinerService}; //! //! fn main() { -//! let miner: Miner = Miner::with_spec(ethereum::new_frontier()); +//! let miner: Miner = Miner::with_spec(ðereum::new_frontier()); //! // get status //! assert_eq!(miner.status().transactions_in_pending_queue, 0); //! //! // Check block for sealing -//! //assert!(miner.sealing_block(client.deref()).lock().is_some()); +//! //assert!(miner.sealing_block(&*client).lock().is_some()); //! } //! ``` diff --git a/ethcore/src/service.rs b/ethcore/src/service.rs index c942416db..8c3283f87 100644 --- a/ethcore/src/service.rs +++ b/ethcore/src/service.rs @@ -58,7 +58,7 @@ impl ClientService { /// Start the service in a separate thread. pub fn start( config: ClientConfig, - spec: Spec, + spec: &Spec, db_path: &Path, miner: Arc, ) -> Result @@ -198,11 +198,12 @@ mod tests { path.push("pruning"); path.push("db"); + let spec = get_test_spec(); let service = ClientService::start( ClientConfig::default(), - get_test_spec(), + &spec, &path, - Arc::new(Miner::with_spec(get_test_spec())), + Arc::new(Miner::with_spec(&spec)), ); assert!(service.is_ok()); } diff --git a/ethcore/src/snapshot/service.rs b/ethcore/src/snapshot/service.rs index d01c00f68..dafce75cf 100644 --- a/ethcore/src/snapshot/service.rs +++ b/ethcore/src/snapshot/service.rs @@ -98,12 +98,12 @@ struct Restoration { impl Restoration { // make a new restoration, building databases in the given path. - fn new(manifest: &ManifestData, pruning: Algorithm, path: &Path, spec: &Spec) -> Result { + fn new(manifest: &ManifestData, pruning: Algorithm, path: &Path, gb: &[u8]) -> Result { let cfg = DatabaseConfig::with_columns(::client::DB_NO_OF_COLUMNS); let raw_db = Arc::new(try!(Database::open(&cfg, &*path.to_string_lossy()) .map_err(|s| UtilError::SimpleString(s)))); - let chain = BlockChain::new(Default::default(), &spec.genesis_block(), raw_db.clone()); + let chain = BlockChain::new(Default::default(), gb, raw_db.clone()); let blocks = try!(BlockRebuilder::new(chain, manifest.block_number)); Ok(Restoration { @@ -173,14 +173,15 @@ pub struct Service { pruning: Algorithm, status: Mutex, reader: Option, - spec: Spec, + engine: Arc, + genesis_block: Bytes, state_chunks: AtomicUsize, block_chunks: AtomicUsize, } impl Service { /// Create a new snapshot service. - pub fn new(spec: Spec, pruning: Algorithm, client_db: PathBuf, io_channel: Channel) -> Result { + pub fn new(spec: &Spec, pruning: Algorithm, client_db: PathBuf, io_channel: Channel) -> Result { let db_path = try!(client_db.parent().and_then(Path::parent) .ok_or_else(|| UtilError::SimpleString("Failed to find database root.".into()))).to_owned(); @@ -199,7 +200,8 @@ impl Service { pruning: pruning, status: Mutex::new(RestorationStatus::Inactive), reader: reader, - spec: spec, + engine: spec.engine.clone(), + genesis_block: spec.genesis_block(), state_chunks: AtomicUsize::new(0), block_chunks: AtomicUsize::new(0), }; @@ -324,7 +326,7 @@ impl Service { match is_state { true => rest.feed_state(hash, chunk), - false => rest.feed_blocks(hash, chunk, &*self.spec.engine), + false => rest.feed_blocks(hash, chunk, &*self.engine), }.map(|_| rest.is_done()) }; @@ -411,7 +413,7 @@ impl SnapshotService for Service { // make new restoration. let db_path = self.restoration_db(); - *res = match Restoration::new(&manifest, self.pruning, &db_path, &self.spec) { + *res = match Restoration::new(&manifest, self.pruning, &db_path, &self.genesis_block) { Ok(b) => Some(b), Err(e) => { warn!("encountered error {} while beginning snapshot restoration.", e); diff --git a/ethcore/src/tests/client.rs b/ethcore/src/tests/client.rs index ef97c7528..e6f930485 100644 --- a/ethcore/src/tests/client.rs +++ b/ethcore/src/tests/client.rs @@ -25,7 +25,8 @@ use miner::Miner; #[test] fn imports_from_empty() { let dir = RandomTempPath::new(); - let client = Client::new(ClientConfig::default(), &get_test_spec(), dir.as_path(), Arc::new(Miner::with_spec(get_test_spec())), IoChannel::disconnected()).unwrap(); + let spec = get_test_spec(); + let client = Client::new(ClientConfig::default(), &spec, dir.as_path(), Arc::new(Miner::with_spec(&spec)), IoChannel::disconnected()).unwrap(); client.import_verified_blocks(); client.flush_queue(); } @@ -43,7 +44,8 @@ fn returns_state_root_basic() { #[test] fn imports_good_block() { let dir = RandomTempPath::new(); - let client = Client::new(ClientConfig::default(), &get_test_spec(), dir.as_path(), Arc::new(Miner::with_spec(get_test_spec())), IoChannel::disconnected()).unwrap(); + let spec = get_test_spec(); + let client = Client::new(ClientConfig::default(), &spec, dir.as_path(), Arc::new(Miner::with_spec(&spec)), IoChannel::disconnected()).unwrap(); let good_block = get_good_dummy_block(); if let Err(_) = client.import_block(good_block) { panic!("error importing block being good by definition"); @@ -58,7 +60,8 @@ fn imports_good_block() { #[test] fn query_none_block() { let dir = RandomTempPath::new(); - let client = Client::new(ClientConfig::default(), &get_test_spec(), dir.as_path(), Arc::new(Miner::with_spec(get_test_spec())), IoChannel::disconnected()).unwrap(); + let spec = get_test_spec(); + let client = Client::new(ClientConfig::default(), &spec, dir.as_path(), Arc::new(Miner::with_spec(&spec)), IoChannel::disconnected()).unwrap(); let non_existant = client.block_header(BlockID::Number(188)); assert!(non_existant.is_none()); diff --git a/ethcore/src/tests/helpers.rs b/ethcore/src/tests/helpers.rs index b84f72c02..4dc4f953d 100644 --- a/ethcore/src/tests/helpers.rs +++ b/ethcore/src/tests/helpers.rs @@ -133,7 +133,7 @@ pub fn generate_dummy_client_with_spec_and_data(get_test_spec: F, block_numbe let dir = RandomTempPath::new(); let test_spec = get_test_spec(); - let client = Client::new(ClientConfig::default(), &get_test_spec(), dir.as_path(), Arc::new(Miner::with_spec(get_test_spec())), IoChannel::disconnected()).unwrap(); + let client = Client::new(ClientConfig::default(), &test_spec, dir.as_path(), Arc::new(Miner::with_spec(&test_spec)), IoChannel::disconnected()).unwrap(); let test_engine = &test_spec.engine; let mut db_result = get_temp_journal_db(); @@ -232,7 +232,8 @@ pub fn push_blocks_to_client(client: &Arc, timestamp_salt: u64, starting pub fn get_test_client_with_blocks(blocks: Vec) -> GuardedTempResult> { let dir = RandomTempPath::new(); - let client = Client::new(ClientConfig::default(), &get_test_spec(), dir.as_path(), Arc::new(Miner::with_spec(get_test_spec())), IoChannel::disconnected()).unwrap(); + let test_spec = get_test_spec(); + let client = Client::new(ClientConfig::default(), &test_spec, dir.as_path(), Arc::new(Miner::with_spec(&test_spec)), IoChannel::disconnected()).unwrap(); for block in &blocks { if let Err(_) = client.import_block(block.clone()) { panic!("panic importing block which is well-formed"); diff --git a/ethcore/src/tests/rpc.rs b/ethcore/src/tests/rpc.rs index 9d6ac0774..afd2cd6a7 100644 --- a/ethcore/src/tests/rpc.rs +++ b/ethcore/src/tests/rpc.rs @@ -30,11 +30,12 @@ pub fn run_test_worker(scope: &crossbeam::Scope, stop: Arc, socket_p let socket_path = socket_path.to_owned(); scope.spawn(move || { let temp = RandomTempPath::create_dir(); + let spec = get_test_spec(); let client = Client::new( ClientConfig::default(), - &get_test_spec(), + &spec, temp.as_path(), - Arc::new(Miner::with_spec(get_test_spec())), + Arc::new(Miner::with_spec(&spec)), IoChannel::disconnected()).unwrap(); let mut worker = nanoipc::Worker::new(&(client as Arc)); worker.add_reqrep(&socket_path).unwrap(); diff --git a/parity/blockchain.rs b/parity/blockchain.rs index 36795edc7..b0c5d95a7 100644 --- a/parity/blockchain.rs +++ b/parity/blockchain.rs @@ -136,9 +136,9 @@ fn execute_import(cmd: ImportBlockchain) -> Result { // build client let service = try!(ClientService::start( client_config, - spec, + &spec, Path::new(&client_path), - Arc::new(Miner::with_spec(try!(cmd.spec.spec()))), + Arc::new(Miner::with_spec(&spec)), ).map_err(|e| format!("Client service error: {:?}", e))); panic_handler.forward_from(&service); @@ -246,9 +246,9 @@ fn execute_export(cmd: ExportBlockchain) -> Result { let service = try!(ClientService::start( client_config, - spec, + &spec, Path::new(&client_path), - Arc::new(Miner::with_spec(try!(cmd.spec.spec()))) + Arc::new(Miner::with_spec(&spec)), ).map_err(|e| format!("Client service error: {:?}", e))); panic_handler.forward_from(&service); diff --git a/parity/run.rs b/parity/run.rs index 04037c9b6..7d8ef1ac7 100644 --- a/parity/run.rs +++ b/parity/run.rs @@ -139,7 +139,7 @@ pub fn execute(cmd: RunCmd) -> Result<(), String> { let account_provider = Arc::new(try!(prepare_account_provider(&cmd.dirs, cmd.acc_conf))); // create miner - let miner = Miner::new(cmd.miner_options, cmd.gas_pricer.into(), spec, Some(account_provider.clone())); + 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); @@ -161,10 +161,6 @@ pub fn execute(cmd: RunCmd) -> Result<(), String> { fork_name.as_ref(), ); - // load spec - // TODO: make it clonable and load it only once! - let spec = try!(cmd.spec.spec()); - // set up bootnodes let mut net_conf = cmd.net_conf; if !cmd.custom_bootnodes { @@ -174,7 +170,7 @@ pub fn execute(cmd: RunCmd) -> Result<(), String> { // create client service. let service = try!(ClientService::start( client_config, - spec, + &spec, Path::new(&client_path), miner.clone(), ).map_err(|e| format!("Client service error: {:?}", e))); diff --git a/parity/snapshot.rs b/parity/snapshot.rs index c357b7b22..790e001e6 100644 --- a/parity/snapshot.rs +++ b/parity/snapshot.rs @@ -89,9 +89,9 @@ impl SnapshotCommand { let service = try!(ClientService::start( client_config, - spec, + &spec, Path::new(&client_path), - Arc::new(Miner::with_spec(try!(self.spec.spec()))) + Arc::new(Miner::with_spec(&spec)) ).map_err(|e| format!("Client service error: {:?}", e))); Ok((service, panic_handler)) diff --git a/rpc/src/v1/tests/eth.rs b/rpc/src/v1/tests/eth.rs index 20c83b49b..7c99e9a6e 100644 --- a/rpc/src/v1/tests/eth.rs +++ b/rpc/src/v1/tests/eth.rs @@ -50,7 +50,7 @@ fn sync_provider() -> Arc { })) } -fn miner_service(spec: Spec, accounts: Arc) -> Arc { +fn miner_service(spec: &Spec, accounts: Arc) -> Arc { Miner::new( MinerOptions { new_work_notify: vec![], @@ -65,8 +65,8 @@ fn miner_service(spec: Spec, accounts: Arc) -> Arc { enable_resubmission: true, }, GasPricer::new_fixed(20_000_000_000u64.into()), - spec, - Some(accounts) + &spec, + Some(accounts), ) } @@ -89,7 +89,7 @@ struct EthTester { impl EthTester { fn from_chain(chain: &BlockChain) -> Self { - let tester = Self::from_spec_provider(|| make_spec(chain)); + let tester = Self::from_spec(make_spec(chain)); for b in &chain.blocks_rlp() { if Block::is_good(&b) { @@ -105,13 +105,11 @@ impl EthTester { tester } - fn from_spec_provider(spec_provider: F) -> Self - where F: Fn() -> Spec { - + fn from_spec(spec: Spec) -> Self { let dir = RandomTempPath::new(); let account_provider = account_provider(); - let miner_service = miner_service(spec_provider(), account_provider.clone()); - let client = Client::new(ClientConfig::default(), &spec_provider(), dir.as_path(), miner_service.clone(), IoChannel::disconnected()).unwrap(); + let miner_service = miner_service(&spec, account_provider.clone()); + let client = Client::new(ClientConfig::default(), &spec, dir.as_path(), miner_service.clone(), IoChannel::disconnected()).unwrap(); let sync_provider = sync_provider(); let external_miner = Arc::new(ExternalMiner::default()); @@ -291,7 +289,7 @@ fn eth_transaction_count() { use util::crypto::Secret; let secret = Secret::from_str("8a283037bb19c4fed7b1c569e40c7dcff366165eb869110a1b11532963eb9cb2").unwrap(); - let tester = EthTester::from_spec_provider(|| Spec::load(TRANSACTION_COUNT_SPEC)); + let tester = EthTester::from_spec(Spec::load(TRANSACTION_COUNT_SPEC)); let address = tester.accounts.insert_account(secret, "").unwrap(); tester.accounts.unlock_account_permanently(address, "".into()).unwrap(); @@ -417,7 +415,7 @@ fn verify_transaction_counts(name: String, chain: BlockChain) { #[test] fn starting_nonce_test() { - let tester = EthTester::from_spec_provider(|| Spec::load(POSITIVE_NONCE_SPEC)); + let tester = EthTester::from_spec(Spec::load(POSITIVE_NONCE_SPEC)); let address = ::util::hash::Address::from(10); let sample = tester.handler.handle_request(&(r#" diff --git a/sync/src/lib.rs b/sync/src/lib.rs index 3aa93662e..69dd03a2a 100644 --- a/sync/src/lib.rs +++ b/sync/src/lib.rs @@ -43,15 +43,16 @@ //! //! fn main() { //! let dir = env::temp_dir(); +//! let spec = ethereum::new_frontier(); //! let miner = Miner::new( //! Default::default(), //! GasPricer::new_fixed(20_000_000_000u64.into()), -//! ethereum::new_frontier(), +//! &spec, //! None //! ); //! let client = Client::new( //! ClientConfig::default(), -//! ðereum::new_frontier(), +//! &spec, //! &dir, //! miner, //! IoChannel::disconnected()