From 48070bed07caa022cbabb6e372db1e9d83624d62 Mon Sep 17 00:00:00 2001 From: Nicolas Gotchac Date: Thu, 15 Aug 2019 19:10:36 +0200 Subject: [PATCH 01/17] WiP : clear pending txs cache & tick in Miner --- Cargo.toml | 2 +- ethcore/service/src/service.rs | 10 ++++++++-- ethcore/src/client/client.rs | 8 ++++---- ethcore/src/engines/instant_seal.rs | 7 ++++++- ethcore/src/engines/mod.rs | 9 +++++++++ ethcore/src/miner/miner.rs | 19 +++++++++++++++++++ miner/src/pool/queue.rs | 5 +++++ util/network-devp2p/src/handshake.rs | 3 ++- 8 files changed, 54 insertions(+), 9 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index ea85995a556..508e6161b48 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -121,7 +121,7 @@ name = "parity" [profile.release] debug = false -lto = true +lto = false [workspace] # This should only list projects that are not diff --git a/ethcore/service/src/service.rs b/ethcore/service/src/service.rs index 737d77254f2..2fbe3d9e0a7 100644 --- a/ethcore/service/src/service.rs +++ b/ethcore/service/src/service.rs @@ -133,7 +133,7 @@ impl ClientService { )); let provider = Arc::new(ethcore_private_tx::Provider::new( client.clone(), - miner, + miner.clone(), signer, encryptor, private_tx_conf, @@ -144,6 +144,7 @@ impl ClientService { let client_io = Arc::new(ClientIoHandler { client: client.clone(), + miner: miner, snapshot: snapshot.clone(), }); io_service.register_handler(client_io)?; @@ -200,18 +201,22 @@ impl ClientService { /// IO interface for the Client handler struct ClientIoHandler { client: Arc, + miner: Arc, snapshot: Arc, } const CLIENT_TICK_TIMER: TimerToken = 0; -const SNAPSHOT_TICK_TIMER: TimerToken = 1; +const MINER_TICK_TIMER: TimerToken = 1; +const SNAPSHOT_TICK_TIMER: TimerToken = 2; const CLIENT_TICK: Duration = Duration::from_secs(5); +const MINER_TICK: Duration = Duration::from_secs(5); const SNAPSHOT_TICK: Duration = Duration::from_secs(10); impl IoHandler for ClientIoHandler { fn initialize(&self, io: &IoContext) { io.register_timer(CLIENT_TICK_TIMER, CLIENT_TICK).expect("Error registering client timer"); + io.register_timer(MINER_TICK_TIMER, MINER_TICK).expect("Error registering miner timer"); io.register_timer(SNAPSHOT_TICK_TIMER, SNAPSHOT_TICK).expect("Error registering snapshot timer"); } @@ -223,6 +228,7 @@ impl IoHandler for ClientIoHandler { let snapshot_restoration = if let RestorationStatus::Ongoing{..} = self.snapshot.status() { true } else { false }; self.client.tick(snapshot_restoration) }, + MINER_TICK_TIMER => self.miner.tick(&*self.client), SNAPSHOT_TICK_TIMER => self.snapshot.tick(), _ => warn!("IO service triggered unregistered timer '{}'", timer), } diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index 207a92da03b..031838579f0 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -532,10 +532,8 @@ impl Importer { let route = chain.tree_route(best_hash, *parent).expect("forks are only kept when it has common ancestors; tree route from best to prospective's parent always exists; qed"); let fork_choice = if route.is_from_route_finalized { ForkChoice::Old - } else if new_total_difficulty > best_total_difficulty { - ForkChoice::New } else { - ForkChoice::Old + self.engine.fork_choice(new_total_difficulty, best_total_difficulty) }; // CHECK! I *think* this is fine, even if the state_root is equal to another @@ -2417,6 +2415,7 @@ impl ScheduleInfo for Client { impl ImportSealedBlock for Client { fn import_sealed_block(&self, block: SealedBlock) -> EthcoreResult { let start = Instant::now(); + let num_txs = block.transactions.len(); let raw = block.rlp_bytes(); let header = block.header.clone(); let hash = header.hash(); @@ -2452,7 +2451,8 @@ impl ImportSealedBlock for Client { pending, self ); - trace!(target: "client", "Imported sealed block #{} ({})", header.number(), hash); + trace!(target: "client", "Imported sealed block #{} ({} // {}txs)", + header.number(), hash, num_txs); self.state_db.write().sync_cache(&route.enacted, &route.retracted, false); route }; diff --git a/ethcore/src/engines/instant_seal.rs b/ethcore/src/engines/instant_seal.rs index 8b2be486575..0071b386557 100644 --- a/ethcore/src/engines/instant_seal.rs +++ b/ethcore/src/engines/instant_seal.rs @@ -14,11 +14,12 @@ // You should have received a copy of the GNU General Public License // along with Parity Ethereum. If not, see . -use engines::{Engine, Seal}; +use engines::{Engine, Seal, ForkChoice}; use machine::{ ExecutedBlock, Machine }; +use ethereum_types::{U256}; use types::{ header::Header, engines::{ @@ -101,6 +102,10 @@ impl Engine for InstantSeal { self.machine.params() } + /// Always choose the newest block + fn fork_choice(&self, _new_total_difficulty: U256, _best_total_difficulty: U256) -> ForkChoice { + ForkChoice::New + } } diff --git a/ethcore/src/engines/mod.rs b/ethcore/src/engines/mod.rs index b5391c0b226..a084175dfbb 100644 --- a/ethcore/src/engines/mod.rs +++ b/ethcore/src/engines/mod.rs @@ -420,6 +420,15 @@ pub trait Engine: Sync + Send { fn decode_transaction(&self, transaction: &[u8]) -> Result { self.machine().decode_transaction(transaction) } + + /// Returns the fork choice given the new and current best total difficulties + fn fork_choice(&self, new_total_difficulty: U256, best_total_difficulty: U256) -> ForkChoice { + if new_total_difficulty > best_total_difficulty { + ForkChoice::New + } else { + ForkChoice::Old + } + } } /// Verifier for all blocks within an epoch with self-contained state. diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index aeb664b57ad..63bac52bae2 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -339,6 +339,15 @@ impl Miner { }, GasPricer::new_fixed(minimal_gas_price), spec, accounts.unwrap_or_default()) } + /// Tick the Miner for pending local transactions + pub fn tick( + &self, + chain: &C, + ) { + // self.transaction_queue.clear_pending_cache(); + // self.prepare_and_update_sealing(chain); + } + /// Sets `IoChannel` pub fn set_io_channel(&self, io_channel: IoChannel) { *self.io_channel.write() = Some(io_channel); @@ -487,6 +496,9 @@ impl Miner { MAX_SKIPPED_TRANSACTIONS.saturating_add(cmp::min(*open_block.header.gas_limit() / min_tx_gas, u64::max_value().into()).as_u64() as usize) }; + let all_tx = self.transaction_queue.all_transactions(); + debug!(target: "miner", "Found {} transactions in total", all_tx.len()); + let pending: Vec> = self.transaction_queue.pending( client.clone(), pool::PendingSettings { @@ -859,6 +871,7 @@ impl Miner { false } } + /// Prepare pending block, check whether sealing is needed, and then update sealing. fn prepare_and_update_sealing(&self, chain: &C) { use miner::MinerService; @@ -1372,6 +1385,12 @@ impl miner::MinerService for Miner { let gas_limit = *chain.best_block_header().gas_limit(); self.update_transaction_queue_limits(gas_limit); + trace!(target: "client", "chain_new_blocks: {} imported ; {} enacted ; {} retracted", + imported.len(), + enacted.len(), + retracted.len(), + ); + // Then import all transactions from retracted blocks. let client = self.pool_client(chain); { diff --git a/miner/src/pool/queue.rs b/miner/src/pool/queue.rs index bc79d34246d..ff9d32ae0a9 100644 --- a/miner/src/pool/queue.rs +++ b/miner/src/pool/queue.rs @@ -337,6 +337,11 @@ impl TransactionQueue { self.pool.read().unordered_pending(ready).map(|tx| tx.hash).collect() } + /// Clear the pending transactions cache + pub fn clear_pending_cache(&self) { + self.cached_pending.write().clear(); + } + /// Returns current pending transactions ordered by priority. /// /// NOTE: This may return a cached version of pending transaction set. diff --git a/util/network-devp2p/src/handshake.rs b/util/network-devp2p/src/handshake.rs index d0aadd313df..3242d8c72db 100644 --- a/util/network-devp2p/src/handshake.rs +++ b/util/network-devp2p/src/handshake.rs @@ -180,7 +180,8 @@ impl Handshake { self.set_auth(secret, sig, pubk, nonce, PROTOCOL_VERSION)?; self.write_ack(io)?; } - Err(_) => { + Err(e) => { + trace!(target: "network", "Failed to decrypt auth message: {}", e); // Try to interpret as EIP-8 packet let total = ((u16::from(data[0]) << 8 | (u16::from(data[1]))) as usize) + 2; if total < V4_AUTH_PACKET_SIZE { From e9c110f03f44214e377fa976f51d85ccbcd67a7c Mon Sep 17 00:00:00 2001 From: Nicolas Gotchac Date: Fri, 16 Aug 2019 17:55:10 +0200 Subject: [PATCH 02/17] Fixed pending transactions --- ethcore/src/engines/instant_seal.rs | 8 ++-- ethcore/src/miner/miner.rs | 72 +++++++++++++++-------------- miner/src/pool/queue.rs | 1 + miner/src/pool/ready.rs | 29 ++++++++++-- 4 files changed, 66 insertions(+), 44 deletions(-) diff --git a/ethcore/src/engines/instant_seal.rs b/ethcore/src/engines/instant_seal.rs index 0071b386557..95fdeeba5f2 100644 --- a/ethcore/src/engines/instant_seal.rs +++ b/ethcore/src/engines/instant_seal.rs @@ -102,10 +102,10 @@ impl Engine for InstantSeal { self.machine.params() } - /// Always choose the newest block - fn fork_choice(&self, _new_total_difficulty: U256, _best_total_difficulty: U256) -> ForkChoice { - ForkChoice::New - } + // /// Always choose the newest block + // fn fork_choice(&self, _new_total_difficulty: U256, _best_total_difficulty: U256) -> ForkChoice { + // ForkChoice::New + // } } diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 63bac52bae2..36e8d865948 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -1409,54 +1409,56 @@ impl miner::MinerService for Miner { }); } + if has_new_best_block { + // // Make sure to cull transactions after we update sealing. + // // Not culling won't lead to old transactions being added to the block + // // (thanks to Ready), but culling can take significant amount of time, + // // so best to leave it after we create some work for miners to prevent increased + // // uncle rate. + // // If the io_channel is available attempt to offload culling to a separate task + // // to avoid blocking chain_new_blocks + // if let Some(ref channel) = *self.io_channel.read() { + // let queue = self.transaction_queue.clone(); + // let nonce_cache = self.nonce_cache.clone(); + // let engine = self.engine.clone(); + // let accounts = self.accounts.clone(); + // let service_transaction_checker = self.service_transaction_checker.clone(); + + // let cull = move |chain: &::client::Client| { + // let client = PoolClient::new( + // chain, + // &nonce_cache, + // &*engine, + // &*accounts, + // service_transaction_checker.as_ref(), + // ); + // queue.cull(client); + // }; + + // if let Err(e) = channel.send(ClientIoMessage::execute(cull)) { + // warn!(target: "miner", "Error queueing cull: {:?}", e); + // } + // } else { + // self.transaction_queue.cull(client); + // } + self.transaction_queue.cull(client); + } + if has_new_best_block || (imported.len() > 0 && self.options.reseal_on_uncle) { // Reset `next_allowed_reseal` in case a block is imported. // Even if min_period is high, we will always attempt to create // new pending block. self.sealing.lock().next_allowed_reseal = Instant::now(); - if !is_internal_import { + // if !is_internal_import { // -------------------------------------------------------------------------- // | NOTE Code below requires sealing locks. | // | Make sure to release the locks before calling that method. | // -------------------------------------------------------------------------- self.update_sealing(chain); - } + // } } - if has_new_best_block { - // Make sure to cull transactions after we update sealing. - // Not culling won't lead to old transactions being added to the block - // (thanks to Ready), but culling can take significant amount of time, - // so best to leave it after we create some work for miners to prevent increased - // uncle rate. - // If the io_channel is available attempt to offload culling to a separate task - // to avoid blocking chain_new_blocks - if let Some(ref channel) = *self.io_channel.read() { - let queue = self.transaction_queue.clone(); - let nonce_cache = self.nonce_cache.clone(); - let engine = self.engine.clone(); - let accounts = self.accounts.clone(); - let service_transaction_checker = self.service_transaction_checker.clone(); - - let cull = move |chain: &::client::Client| { - let client = PoolClient::new( - chain, - &nonce_cache, - &*engine, - &*accounts, - service_transaction_checker.as_ref(), - ); - queue.cull(client); - }; - - if let Err(e) = channel.send(ClientIoMessage::execute(cull)) { - warn!(target: "miner", "Error queueing cull: {:?}", e); - } - } else { - self.transaction_queue.cull(client); - } - } if let Some(ref service_transaction_checker) = self.service_transaction_checker { match service_transaction_checker.refresh_cache(chain) { Ok(true) => { diff --git a/miner/src/pool/queue.rs b/miner/src/pool/queue.rs index ff9d32ae0a9..45280933ec6 100644 --- a/miner/src/pool/queue.rs +++ b/miner/src/pool/queue.rs @@ -463,6 +463,7 @@ impl TransactionQueue { let state_readiness = ready::State::new(client.clone(), stale_id, nonce_cap); removed += self.pool.write().cull(Some(chunk), state_readiness); } + self.cached_pending.write().clear(); debug!(target: "txqueue", "Removed {} stalled transactions. {}", removed, self.status()); } diff --git a/miner/src/pool/ready.rs b/miner/src/pool/ready.rs index 3accba13903..f64b117feae 100644 --- a/miner/src/pool/ready.rs +++ b/miner/src/pool/ready.rs @@ -75,9 +75,11 @@ impl State { impl txpool::Ready for State { fn is_ready(&mut self, tx: &VerifiedTransaction) -> txpool::Readiness { + trace!(target: "txqueue", "Checking readiness for {}::State", tx.hash()); // Check max nonce match self.max_nonce { Some(nonce) if tx.transaction.nonce > nonce => { + trace!(target: "txqueue", "[{}::State] Ready? Nonce too high", tx.hash()); return txpool::Readiness::Future; }, _ => {}, @@ -90,11 +92,21 @@ impl txpool::Ready for State { match tx.transaction.nonce.cmp(nonce) { // Before marking as future check for stale ids cmp::Ordering::Greater => match self.stale_id { - Some(id) if tx.insertion_id() < id => txpool::Readiness::Stale, - _ => txpool::Readiness::Future, + Some(id) if tx.insertion_id() < id => { + trace!(target: "txqueue", "[{}::State] Ready? Nonce higher but stale", tx.hash()); + txpool::Readiness::Stale + }, + _ => { + trace!(target: "txqueue", "[{}::State] Ready? Nonce higher", tx.hash()); + txpool::Readiness::Future + }, + }, + cmp::Ordering::Less => { + trace!(target: "txqueue", "[{}::State] Ready? Nonce too low", tx.hash()); + txpool::Readiness::Stale }, - cmp::Ordering::Less => txpool::Readiness::Stale, cmp::Ordering::Equal => { + trace!(target: "txqueue", "[{}::State] Ready? Nonce ==", tx.hash()); *nonce = nonce.saturating_add(U256::from(1)); txpool::Readiness::Ready }, @@ -121,9 +133,16 @@ impl Condition { impl txpool::Ready for Condition { fn is_ready(&mut self, tx: &VerifiedTransaction) -> txpool::Readiness { + trace!(target: "txqueue", "Checking readiness for {}::Condition", tx.hash()); match tx.transaction.condition { - Some(transaction::Condition::Number(block)) if block > self.block_number => txpool::Readiness::Future, - Some(transaction::Condition::Timestamp(time)) if time > self.now => txpool::Readiness::Future, + Some(transaction::Condition::Number(block)) if block > self.block_number => { + trace!(target: "txqueue", "[{}::Condition] Ready? Block number too high", tx.hash()); + txpool::Readiness::Future + }, + Some(transaction::Condition::Timestamp(time)) if time > self.now => { + trace!(target: "txqueue", "[{}::Condition] Ready? Time too high", tx.hash()); + txpool::Readiness::Future + }, _ => txpool::Readiness::Ready, } } From cc653a06c6e0d9f798c44ee9207364feb54b5a7b Mon Sep 17 00:00:00 2001 From: Nicolas Gotchac Date: Mon, 26 Aug 2019 17:16:33 +0200 Subject: [PATCH 03/17] Revert debugging code --- ethcore/service/src/service.rs | 10 ++-------- ethcore/src/client/client.rs | 8 ++++---- ethcore/src/engines/instant_seal.rs | 7 +------ ethcore/src/engines/mod.rs | 9 --------- ethcore/src/miner/miner.rs | 18 ----------------- miner/src/pool/queue.rs | 5 ----- miner/src/pool/ready.rs | 29 +++++----------------------- util/network-devp2p/src/handshake.rs | 3 +-- 8 files changed, 13 insertions(+), 76 deletions(-) diff --git a/ethcore/service/src/service.rs b/ethcore/service/src/service.rs index 2fbe3d9e0a7..737d77254f2 100644 --- a/ethcore/service/src/service.rs +++ b/ethcore/service/src/service.rs @@ -133,7 +133,7 @@ impl ClientService { )); let provider = Arc::new(ethcore_private_tx::Provider::new( client.clone(), - miner.clone(), + miner, signer, encryptor, private_tx_conf, @@ -144,7 +144,6 @@ impl ClientService { let client_io = Arc::new(ClientIoHandler { client: client.clone(), - miner: miner, snapshot: snapshot.clone(), }); io_service.register_handler(client_io)?; @@ -201,22 +200,18 @@ impl ClientService { /// IO interface for the Client handler struct ClientIoHandler { client: Arc, - miner: Arc, snapshot: Arc, } const CLIENT_TICK_TIMER: TimerToken = 0; -const MINER_TICK_TIMER: TimerToken = 1; -const SNAPSHOT_TICK_TIMER: TimerToken = 2; +const SNAPSHOT_TICK_TIMER: TimerToken = 1; const CLIENT_TICK: Duration = Duration::from_secs(5); -const MINER_TICK: Duration = Duration::from_secs(5); const SNAPSHOT_TICK: Duration = Duration::from_secs(10); impl IoHandler for ClientIoHandler { fn initialize(&self, io: &IoContext) { io.register_timer(CLIENT_TICK_TIMER, CLIENT_TICK).expect("Error registering client timer"); - io.register_timer(MINER_TICK_TIMER, MINER_TICK).expect("Error registering miner timer"); io.register_timer(SNAPSHOT_TICK_TIMER, SNAPSHOT_TICK).expect("Error registering snapshot timer"); } @@ -228,7 +223,6 @@ impl IoHandler for ClientIoHandler { let snapshot_restoration = if let RestorationStatus::Ongoing{..} = self.snapshot.status() { true } else { false }; self.client.tick(snapshot_restoration) }, - MINER_TICK_TIMER => self.miner.tick(&*self.client), SNAPSHOT_TICK_TIMER => self.snapshot.tick(), _ => warn!("IO service triggered unregistered timer '{}'", timer), } diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index 031838579f0..207a92da03b 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -532,8 +532,10 @@ impl Importer { let route = chain.tree_route(best_hash, *parent).expect("forks are only kept when it has common ancestors; tree route from best to prospective's parent always exists; qed"); let fork_choice = if route.is_from_route_finalized { ForkChoice::Old + } else if new_total_difficulty > best_total_difficulty { + ForkChoice::New } else { - self.engine.fork_choice(new_total_difficulty, best_total_difficulty) + ForkChoice::Old }; // CHECK! I *think* this is fine, even if the state_root is equal to another @@ -2415,7 +2417,6 @@ impl ScheduleInfo for Client { impl ImportSealedBlock for Client { fn import_sealed_block(&self, block: SealedBlock) -> EthcoreResult { let start = Instant::now(); - let num_txs = block.transactions.len(); let raw = block.rlp_bytes(); let header = block.header.clone(); let hash = header.hash(); @@ -2451,8 +2452,7 @@ impl ImportSealedBlock for Client { pending, self ); - trace!(target: "client", "Imported sealed block #{} ({} // {}txs)", - header.number(), hash, num_txs); + trace!(target: "client", "Imported sealed block #{} ({})", header.number(), hash); self.state_db.write().sync_cache(&route.enacted, &route.retracted, false); route }; diff --git a/ethcore/src/engines/instant_seal.rs b/ethcore/src/engines/instant_seal.rs index 95fdeeba5f2..8b2be486575 100644 --- a/ethcore/src/engines/instant_seal.rs +++ b/ethcore/src/engines/instant_seal.rs @@ -14,12 +14,11 @@ // You should have received a copy of the GNU General Public License // along with Parity Ethereum. If not, see . -use engines::{Engine, Seal, ForkChoice}; +use engines::{Engine, Seal}; use machine::{ ExecutedBlock, Machine }; -use ethereum_types::{U256}; use types::{ header::Header, engines::{ @@ -102,10 +101,6 @@ impl Engine for InstantSeal { self.machine.params() } - // /// Always choose the newest block - // fn fork_choice(&self, _new_total_difficulty: U256, _best_total_difficulty: U256) -> ForkChoice { - // ForkChoice::New - // } } diff --git a/ethcore/src/engines/mod.rs b/ethcore/src/engines/mod.rs index a084175dfbb..b5391c0b226 100644 --- a/ethcore/src/engines/mod.rs +++ b/ethcore/src/engines/mod.rs @@ -420,15 +420,6 @@ pub trait Engine: Sync + Send { fn decode_transaction(&self, transaction: &[u8]) -> Result { self.machine().decode_transaction(transaction) } - - /// Returns the fork choice given the new and current best total difficulties - fn fork_choice(&self, new_total_difficulty: U256, best_total_difficulty: U256) -> ForkChoice { - if new_total_difficulty > best_total_difficulty { - ForkChoice::New - } else { - ForkChoice::Old - } - } } /// Verifier for all blocks within an epoch with self-contained state. diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 36e8d865948..f20fcd211e8 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -339,15 +339,6 @@ impl Miner { }, GasPricer::new_fixed(minimal_gas_price), spec, accounts.unwrap_or_default()) } - /// Tick the Miner for pending local transactions - pub fn tick( - &self, - chain: &C, - ) { - // self.transaction_queue.clear_pending_cache(); - // self.prepare_and_update_sealing(chain); - } - /// Sets `IoChannel` pub fn set_io_channel(&self, io_channel: IoChannel) { *self.io_channel.write() = Some(io_channel); @@ -496,9 +487,6 @@ impl Miner { MAX_SKIPPED_TRANSACTIONS.saturating_add(cmp::min(*open_block.header.gas_limit() / min_tx_gas, u64::max_value().into()).as_u64() as usize) }; - let all_tx = self.transaction_queue.all_transactions(); - debug!(target: "miner", "Found {} transactions in total", all_tx.len()); - let pending: Vec> = self.transaction_queue.pending( client.clone(), pool::PendingSettings { @@ -1385,12 +1373,6 @@ impl miner::MinerService for Miner { let gas_limit = *chain.best_block_header().gas_limit(); self.update_transaction_queue_limits(gas_limit); - trace!(target: "client", "chain_new_blocks: {} imported ; {} enacted ; {} retracted", - imported.len(), - enacted.len(), - retracted.len(), - ); - // Then import all transactions from retracted blocks. let client = self.pool_client(chain); { diff --git a/miner/src/pool/queue.rs b/miner/src/pool/queue.rs index 45280933ec6..5f788ab8235 100644 --- a/miner/src/pool/queue.rs +++ b/miner/src/pool/queue.rs @@ -337,11 +337,6 @@ impl TransactionQueue { self.pool.read().unordered_pending(ready).map(|tx| tx.hash).collect() } - /// Clear the pending transactions cache - pub fn clear_pending_cache(&self) { - self.cached_pending.write().clear(); - } - /// Returns current pending transactions ordered by priority. /// /// NOTE: This may return a cached version of pending transaction set. diff --git a/miner/src/pool/ready.rs b/miner/src/pool/ready.rs index f64b117feae..3accba13903 100644 --- a/miner/src/pool/ready.rs +++ b/miner/src/pool/ready.rs @@ -75,11 +75,9 @@ impl State { impl txpool::Ready for State { fn is_ready(&mut self, tx: &VerifiedTransaction) -> txpool::Readiness { - trace!(target: "txqueue", "Checking readiness for {}::State", tx.hash()); // Check max nonce match self.max_nonce { Some(nonce) if tx.transaction.nonce > nonce => { - trace!(target: "txqueue", "[{}::State] Ready? Nonce too high", tx.hash()); return txpool::Readiness::Future; }, _ => {}, @@ -92,21 +90,11 @@ impl txpool::Ready for State { match tx.transaction.nonce.cmp(nonce) { // Before marking as future check for stale ids cmp::Ordering::Greater => match self.stale_id { - Some(id) if tx.insertion_id() < id => { - trace!(target: "txqueue", "[{}::State] Ready? Nonce higher but stale", tx.hash()); - txpool::Readiness::Stale - }, - _ => { - trace!(target: "txqueue", "[{}::State] Ready? Nonce higher", tx.hash()); - txpool::Readiness::Future - }, - }, - cmp::Ordering::Less => { - trace!(target: "txqueue", "[{}::State] Ready? Nonce too low", tx.hash()); - txpool::Readiness::Stale + Some(id) if tx.insertion_id() < id => txpool::Readiness::Stale, + _ => txpool::Readiness::Future, }, + cmp::Ordering::Less => txpool::Readiness::Stale, cmp::Ordering::Equal => { - trace!(target: "txqueue", "[{}::State] Ready? Nonce ==", tx.hash()); *nonce = nonce.saturating_add(U256::from(1)); txpool::Readiness::Ready }, @@ -133,16 +121,9 @@ impl Condition { impl txpool::Ready for Condition { fn is_ready(&mut self, tx: &VerifiedTransaction) -> txpool::Readiness { - trace!(target: "txqueue", "Checking readiness for {}::Condition", tx.hash()); match tx.transaction.condition { - Some(transaction::Condition::Number(block)) if block > self.block_number => { - trace!(target: "txqueue", "[{}::Condition] Ready? Block number too high", tx.hash()); - txpool::Readiness::Future - }, - Some(transaction::Condition::Timestamp(time)) if time > self.now => { - trace!(target: "txqueue", "[{}::Condition] Ready? Time too high", tx.hash()); - txpool::Readiness::Future - }, + Some(transaction::Condition::Number(block)) if block > self.block_number => txpool::Readiness::Future, + Some(transaction::Condition::Timestamp(time)) if time > self.now => txpool::Readiness::Future, _ => txpool::Readiness::Ready, } } diff --git a/util/network-devp2p/src/handshake.rs b/util/network-devp2p/src/handshake.rs index 3242d8c72db..d0aadd313df 100644 --- a/util/network-devp2p/src/handshake.rs +++ b/util/network-devp2p/src/handshake.rs @@ -180,8 +180,7 @@ impl Handshake { self.set_auth(secret, sig, pubk, nonce, PROTOCOL_VERSION)?; self.write_ack(io)?; } - Err(e) => { - trace!(target: "network", "Failed to decrypt auth message: {}", e); + Err(_) => { // Try to interpret as EIP-8 packet let total = ((u16::from(data[0]) << 8 | (u16::from(data[1]))) as usize) + 2; if total < V4_AUTH_PACKET_SIZE { From 55772a051f51fc12d1cbe5da48b6e4e1fde5e292 Mon Sep 17 00:00:00 2001 From: Nicolas Gotchac Date: Mon, 26 Aug 2019 17:23:45 +0200 Subject: [PATCH 04/17] Add ToDo comment --- ethcore/src/miner/miner.rs | 65 ++++++++++++++++++++------------------ 1 file changed, 34 insertions(+), 31 deletions(-) diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index f20fcd211e8..0440d8b7a49 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -1391,38 +1391,41 @@ impl miner::MinerService for Miner { }); } + // @todo Use IoChannel if available + // if has_new_best_block { + // // Make sure to cull transactions after we update sealing. + // // Not culling won't lead to old transactions being added to the block + // // (thanks to Ready), but culling can take significant amount of time, + // // so best to leave it after we create some work for miners to prevent increased + // // uncle rate. + // // If the io_channel is available attempt to offload culling to a separate task + // // to avoid blocking chain_new_blocks + // if let Some(ref channel) = *self.io_channel.read() { + // let queue = self.transaction_queue.clone(); + // let nonce_cache = self.nonce_cache.clone(); + // let engine = self.engine.clone(); + // let accounts = self.accounts.clone(); + // let service_transaction_checker = self.service_transaction_checker.clone(); + + // let cull = move |chain: &::client::Client| { + // let client = PoolClient::new( + // chain, + // &nonce_cache, + // &*engine, + // &*accounts, + // service_transaction_checker.as_ref(), + // ); + // queue.cull(client); + // }; + + // if let Err(e) = channel.send(ClientIoMessage::execute(cull)) { + // warn!(target: "miner", "Error queueing cull: {:?}", e); + // } + // } else { + // self.transaction_queue.cull(client); + // } + // } if has_new_best_block { - // // Make sure to cull transactions after we update sealing. - // // Not culling won't lead to old transactions being added to the block - // // (thanks to Ready), but culling can take significant amount of time, - // // so best to leave it after we create some work for miners to prevent increased - // // uncle rate. - // // If the io_channel is available attempt to offload culling to a separate task - // // to avoid blocking chain_new_blocks - // if let Some(ref channel) = *self.io_channel.read() { - // let queue = self.transaction_queue.clone(); - // let nonce_cache = self.nonce_cache.clone(); - // let engine = self.engine.clone(); - // let accounts = self.accounts.clone(); - // let service_transaction_checker = self.service_transaction_checker.clone(); - - // let cull = move |chain: &::client::Client| { - // let client = PoolClient::new( - // chain, - // &nonce_cache, - // &*engine, - // &*accounts, - // service_transaction_checker.as_ref(), - // ); - // queue.cull(client); - // }; - - // if let Err(e) = channel.send(ClientIoMessage::execute(cull)) { - // warn!(target: "miner", "Error queueing cull: {:?}", e); - // } - // } else { - // self.transaction_queue.cull(client); - // } self.transaction_queue.cull(client); } From 75921ede9111b406db2f2736b11ca6395f5c62d7 Mon Sep 17 00:00:00 2001 From: Nicolas Gotchac Date: Tue, 27 Aug 2019 14:52:36 +0200 Subject: [PATCH 05/17] Remove commented-out code --- ethcore/src/miner/miner.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 0440d8b7a49..7c3b234ac97 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -1353,7 +1353,7 @@ impl miner::MinerService for Miner { Ok(sealed) } - fn chain_new_blocks(&self, chain: &C, imported: &[H256], _invalid: &[H256], enacted: &[H256], retracted: &[H256], is_internal_import: bool) + fn chain_new_blocks(&self, chain: &C, imported: &[H256], _invalid: &[H256], enacted: &[H256], retracted: &[H256], _is_internal_import: bool) where C: miner::BlockChainClient, { trace!(target: "miner", "chain_new_blocks"); @@ -1435,13 +1435,11 @@ impl miner::MinerService for Miner { // new pending block. self.sealing.lock().next_allowed_reseal = Instant::now(); - // if !is_internal_import { - // -------------------------------------------------------------------------- - // | NOTE Code below requires sealing locks. | - // | Make sure to release the locks before calling that method. | - // -------------------------------------------------------------------------- - self.update_sealing(chain); - // } + // -------------------------------------------------------------------------- + // | NOTE Code below requires sealing locks. | + // | Make sure to release the locks before calling that method. | + // -------------------------------------------------------------------------- + self.update_sealing(chain); } if let Some(ref service_transaction_checker) = self.service_transaction_checker { From 897eef0191860785d3b40d79268bc85c54955ff1 Mon Sep 17 00:00:00 2001 From: Nicolas Gotchac Date: Tue, 27 Aug 2019 15:07:02 +0200 Subject: [PATCH 06/17] Reverse LTO setting --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 508e6161b48..ea85995a556 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -121,7 +121,7 @@ name = "parity" [profile.release] debug = false -lto = false +lto = true [workspace] # This should only list projects that are not From 9abf8acad8101e56d770c22a9a3f970ba900a33b Mon Sep 17 00:00:00 2001 From: Nicolas Gotchac Date: Mon, 2 Sep 2019 18:31:13 +0200 Subject: [PATCH 07/17] WiP --- Cargo.lock | 1 + ethcore/engines/instant-seal/Cargo.toml | 1 + ethcore/src/miner/miner.rs | 94 +++++++++++++------------ 3 files changed, 50 insertions(+), 46 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d4867ad3362..7e959195fd8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2110,6 +2110,7 @@ dependencies = [ "ethjson 0.1.0", "keccak-hash 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)", "machine 0.1.0", + "parking_lot 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)", "rlp 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)", "spec 0.1.0", "trace 0.1.0", diff --git a/ethcore/engines/instant-seal/Cargo.toml b/ethcore/engines/instant-seal/Cargo.toml index 42a74e999d5..80a0d394b5e 100644 --- a/ethcore/engines/instant-seal/Cargo.toml +++ b/ethcore/engines/instant-seal/Cargo.toml @@ -13,6 +13,7 @@ ethjson = { path = "../../../json" } ethereum-types = "0.6.0" keccak-hash = "0.2.0" machine = { path = "../../machine" } +parking_lot = "0.8" trace = { path = "../../trace" } [dev-dependencies] diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 5b46640e8ea..6dac80325b1 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -675,8 +675,8 @@ impl Miner { fn seal_and_import_block_internally(&self, chain: &C, block: ClosedBlock) -> bool where C: BlockChain + SealedBlockImporter, { + let mut sealing = self.sealing.lock(); { - let sealing = self.sealing.lock(); if block.transactions.is_empty() && !self.forced_sealing() && Instant::now() <= sealing.next_mandatory_reseal @@ -709,7 +709,7 @@ impl Miner { Seal::Regular(seal) => { trace!(target: "miner", "Block #{}: Received a Regular seal.", block_number); { - let mut sealing = self.sealing.lock(); + // let mut sealing = self.sealing.lock(); sealing.next_mandatory_reseal = Instant::now() + self.options.reseal_max_period; } @@ -1359,7 +1359,7 @@ impl miner::MinerService for Miner { Ok(sealed) } - fn chain_new_blocks(&self, chain: &C, imported: &[H256], _invalid: &[H256], enacted: &[H256], retracted: &[H256], _is_internal_import: bool) + fn chain_new_blocks(&self, chain: &C, imported: &[H256], _invalid: &[H256], enacted: &[H256], retracted: &[H256], is_internal_import: bool) where C: miner::BlockChainClient, { trace!(target: "miner", "chain_new_blocks"); @@ -1397,57 +1397,59 @@ impl miner::MinerService for Miner { }); } - // @todo Use IoChannel if available - // if has_new_best_block { - // // Make sure to cull transactions after we update sealing. - // // Not culling won't lead to old transactions being added to the block - // // (thanks to Ready), but culling can take significant amount of time, - // // so best to leave it after we create some work for miners to prevent increased - // // uncle rate. - // // If the io_channel is available attempt to offload culling to a separate task - // // to avoid blocking chain_new_blocks - // if let Some(ref channel) = *self.io_channel.read() { - // let queue = self.transaction_queue.clone(); - // let nonce_cache = self.nonce_cache.clone(); - // let engine = self.engine.clone(); - // let accounts = self.accounts.clone(); - // let service_transaction_checker = self.service_transaction_checker.clone(); - - // let cull = move |chain: &::client::Client| { - // let client = PoolClient::new( - // chain, - // &nonce_cache, - // &*engine, - // &*accounts, - // service_transaction_checker.as_ref(), - // ); - // queue.cull(client); - // }; - - // if let Err(e) = channel.send(ClientIoMessage::execute(cull)) { - // warn!(target: "miner", "Error queueing cull: {:?}", e); - // } - // } else { - // self.transaction_queue.cull(client); - // } - // } - if has_new_best_block { - self.transaction_queue.cull(client); - } - if has_new_best_block || (imported.len() > 0 && self.options.reseal_on_uncle) { // Reset `next_allowed_reseal` in case a block is imported. // Even if min_period is high, we will always attempt to create // new pending block. self.sealing.lock().next_allowed_reseal = Instant::now(); - // -------------------------------------------------------------------------- - // | NOTE Code below requires sealing locks. | - // | Make sure to release the locks before calling that method. | - // -------------------------------------------------------------------------- - self.update_sealing(chain); + if !is_internal_import { + // -------------------------------------------------------------------------- + // | NOTE Code below requires sealing locks. | + // | Make sure to release the locks before calling that method. | + // -------------------------------------------------------------------------- + self.update_sealing(chain); + } } + // @todo Use IoChannel if available + if has_new_best_block { + // Make sure to cull transactions after we update sealing. + // Not culling won't lead to old transactions being added to the block + // (thanks to Ready), but culling can take significant amount of time, + // so best to leave it after we create some work for miners to prevent increased + // uncle rate. + // If the io_channel is available attempt to offload culling to a separate task + // to avoid blocking chain_new_blocks + if let Some(ref channel) = *self.io_channel.read() { + let queue = self.transaction_queue.clone(); + let nonce_cache = self.nonce_cache.clone(); + let engine = self.engine.clone(); + let accounts = self.accounts.clone(); + let service_transaction_checker = self.service_transaction_checker.clone(); + + let cull = move |chain: &::client::Client| { + let client = PoolClient::new( + chain, + &nonce_cache, + &*engine, + &*accounts, + service_transaction_checker.as_ref(), + ); + queue.cull(client); + }; + + if let Err(e) = channel.send(ClientIoMessage::execute(cull)) { + warn!(target: "miner", "Error queueing cull: {:?}", e); + } + } else { + self.transaction_queue.cull(client); + } + } + // if has_new_best_block { + // self.transaction_queue.cull(client); + // } + if let Some(ref service_transaction_checker) = self.service_transaction_checker { match service_transaction_checker.refresh_cache(chain) { Ok(true) => { From 6980320f04c2e1b1773cd770b8bdaa6574c83f78 Mon Sep 17 00:00:00 2001 From: Nicolas Gotchac Date: Wed, 4 Sep 2019 16:28:58 +0200 Subject: [PATCH 08/17] Try to seal a new block if there are pending transactions --- ethcore/engines/instant-seal/src/lib.rs | 15 ++++++++++++++- ethcore/src/client/client.rs | 5 +++++ ethcore/src/miner/miner.rs | 20 +++++++++++++------- ethcore/src/miner/mod.rs | 2 +- 4 files changed, 33 insertions(+), 9 deletions(-) diff --git a/ethcore/engines/instant-seal/src/lib.rs b/ethcore/engines/instant-seal/src/lib.rs index a103401a7e7..009ca799f8e 100644 --- a/ethcore/engines/instant-seal/src/lib.rs +++ b/ethcore/engines/instant-seal/src/lib.rs @@ -14,6 +14,8 @@ // You should have received a copy of the GNU General Public License // along with Parity Ethereum. If not, see . +use std::collections::HashSet; + use common_types::{ header::Header, engines::{ @@ -22,6 +24,7 @@ use common_types::{ params::CommonParams, }, errors::EthcoreError as Error, + BlockNumber, }; use engine::Engine; use ethjson; @@ -29,6 +32,7 @@ use machine::{ ExecutedBlock, Machine }; +use parking_lot::Mutex; /// `InstantSeal` params. @@ -51,6 +55,7 @@ impl From for InstantSealParams { pub struct InstantSeal { params: InstantSealParams, machine: Machine, + sealed_blocks: Mutex>, } impl InstantSeal { @@ -59,6 +64,7 @@ impl InstantSeal { InstantSeal { params, machine, + sealed_blocks: Mutex::new(HashSet::new()), } } } @@ -74,7 +80,14 @@ impl Engine for InstantSeal { if block.transactions.is_empty() { Seal::None } else { - Seal::Regular(Vec::new()) + let mut sealed_blocks = self.sealed_blocks.lock(); + let block_number = block.header.number(); + if !sealed_blocks.contains(&block_number) { + sealed_blocks.insert(block_number); + Seal::Regular(Vec::new()) + } else { + Seal::None + } } } diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index bb4855e189e..a413466f32a 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -1233,6 +1233,11 @@ impl Client { _ => self.block_header(id).and_then(|h| h.decode().ok()) } } + + /// Maybe call `update_sealing` method on the Miner + pub fn maybe_update_sealing(&self) { + self.importer.miner.maybe_update_sealing(self); + } } impl DatabaseRestore for Client { diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index fff402d55ea..9a65e715450 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -669,8 +669,8 @@ impl Miner { fn seal_and_import_block_internally(&self, chain: &C, block: ClosedBlock) -> bool where C: BlockChain + SealedBlockImporter, { - let mut sealing = self.sealing.lock(); { + let sealing = self.sealing.lock(); if block.transactions.is_empty() && !self.forced_sealing() && Instant::now() <= sealing.next_mandatory_reseal @@ -703,7 +703,7 @@ impl Miner { Seal::Regular(seal) => { trace!(target: "miner", "Block #{}: Received a Regular seal.", block_number); { - // let mut sealing = self.sealing.lock(); + let mut sealing = self.sealing.lock(); sealing.next_mandatory_reseal = Instant::now() + self.options.reseal_max_period; } @@ -877,6 +877,14 @@ impl Miner { SealingState::NotReady => { self.maybe_enable_sealing(); }, } } + + /// Call `update_sealing` if needed + pub fn maybe_update_sealing(&self, chain: &C) { + if self.transaction_queue.has_local_pending_transactions() { + eprintln!("Has local pending txs. Calling `update_sealing`."); + self.prepare_and_update_sealing(chain); + } + } } impl miner::MinerService for Miner { @@ -1243,7 +1251,7 @@ impl miner::MinerService for Miner { /// Update sealing if required. /// Prepare the block and work if the Engine does not seal internally. fn update_sealing(&self, chain: &C) where - C: BlockChain + CallContract + BlockProducer + SealedBlockImporter + Nonce + Sync, + C: miner::BlockChainClient, { trace!(target: "miner", "update_sealing"); @@ -1406,7 +1414,6 @@ impl miner::MinerService for Miner { } } - // @todo Use IoChannel if available if has_new_best_block { // Make sure to cull transactions after we update sealing. // Not culling won't lead to old transactions being added to the block @@ -1431,6 +1438,7 @@ impl miner::MinerService for Miner { service_transaction_checker.as_ref(), ); queue.cull(client); + chain.maybe_update_sealing(); }; if let Err(e) = channel.send(ClientIoMessage::::execute(cull)) { @@ -1438,11 +1446,9 @@ impl miner::MinerService for Miner { } } else { self.transaction_queue.cull(client); + self.maybe_update_sealing(chain); } } - // if has_new_best_block { - // self.transaction_queue.cull(client); - // } if let Some(ref service_transaction_checker) = self.service_transaction_checker { match service_transaction_checker.refresh_cache(chain) { diff --git a/ethcore/src/miner/mod.rs b/ethcore/src/miner/mod.rs index 0de9390259d..452e152b1d9 100644 --- a/ethcore/src/miner/mod.rs +++ b/ethcore/src/miner/mod.rs @@ -84,7 +84,7 @@ pub trait MinerService : Send + Sync { /// Update current pending block fn update_sealing(&self, chain: &C) - where C: BlockChain + CallContract + BlockProducer + SealedBlockImporter + Nonce + Sync; + where C: BlockChainClient; // Notifications From 8080777622c3bb3c466d4568aafd083a7d5f6eb8 Mon Sep 17 00:00:00 2001 From: Nicolas Gotchac Date: Wed, 4 Sep 2019 16:40:31 +0200 Subject: [PATCH 09/17] Try resealing only for internal imports --- ethcore/src/miner/miner.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 9a65e715450..2a52e85148e 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -1438,7 +1438,9 @@ impl miner::MinerService for Miner { service_transaction_checker.as_ref(), ); queue.cull(client); - chain.maybe_update_sealing(); + if is_internal_import { + chain.maybe_update_sealing(); + } }; if let Err(e) = channel.send(ClientIoMessage::::execute(cull)) { @@ -1446,7 +1448,9 @@ impl miner::MinerService for Miner { } } else { self.transaction_queue.cull(client); - self.maybe_update_sealing(chain); + if is_internal_import { + self.maybe_update_sealing(chain); + } } } From 6b5c31dd77e83861eda53403b32f1a3da14c3fc0 Mon Sep 17 00:00:00 2001 From: Nicolas Gotchac Date: Wed, 4 Sep 2019 16:46:05 +0200 Subject: [PATCH 10/17] Remove logging --- ethcore/src/miner/miner.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 2a52e85148e..bf436a8d9d5 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -881,7 +881,6 @@ impl Miner { /// Call `update_sealing` if needed pub fn maybe_update_sealing(&self, chain: &C) { if self.transaction_queue.has_local_pending_transactions() { - eprintln!("Has local pending txs. Calling `update_sealing`."); self.prepare_and_update_sealing(chain); } } From 7004b5487ccd4279dccdcb5fd6d54a67a3c9063b Mon Sep 17 00:00:00 2001 From: Nicolas Gotchac Date: Wed, 4 Sep 2019 18:23:11 +0200 Subject: [PATCH 11/17] Use AtomicU64 instead of Mutex --- ethcore/engines/instant-seal/src/lib.rs | 14 ++++++-------- ethcore/src/miner/miner.rs | 5 +++-- ethcore/src/miner/mod.rs | 2 +- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/ethcore/engines/instant-seal/src/lib.rs b/ethcore/engines/instant-seal/src/lib.rs index 009ca799f8e..2fda3eec515 100644 --- a/ethcore/engines/instant-seal/src/lib.rs +++ b/ethcore/engines/instant-seal/src/lib.rs @@ -14,7 +14,7 @@ // You should have received a copy of the GNU General Public License // along with Parity Ethereum. If not, see . -use std::collections::HashSet; +use std::sync::atomic::{AtomicU64, Ordering}; use common_types::{ header::Header, @@ -24,7 +24,6 @@ use common_types::{ params::CommonParams, }, errors::EthcoreError as Error, - BlockNumber, }; use engine::Engine; use ethjson; @@ -32,7 +31,6 @@ use machine::{ ExecutedBlock, Machine }; -use parking_lot::Mutex; /// `InstantSeal` params. @@ -55,7 +53,7 @@ impl From for InstantSealParams { pub struct InstantSeal { params: InstantSealParams, machine: Machine, - sealed_blocks: Mutex>, + last_sealed_block: AtomicU64, } impl InstantSeal { @@ -64,7 +62,7 @@ impl InstantSeal { InstantSeal { params, machine, - sealed_blocks: Mutex::new(HashSet::new()), + last_sealed_block: AtomicU64::new(0), } } } @@ -80,10 +78,10 @@ impl Engine for InstantSeal { if block.transactions.is_empty() { Seal::None } else { - let mut sealed_blocks = self.sealed_blocks.lock(); let block_number = block.header.number(); - if !sealed_blocks.contains(&block_number) { - sealed_blocks.insert(block_number); + let last_sealed_block = self.last_sealed_block.load(Ordering::SeqCst); + if block_number > last_sealed_block { + self.last_sealed_block.store(block_number, Ordering::SeqCst); Seal::Regular(Vec::new()) } else { Seal::None diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index bf436a8d9d5..896f6e61094 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -880,8 +880,9 @@ impl Miner { /// Call `update_sealing` if needed pub fn maybe_update_sealing(&self, chain: &C) { + use miner::MinerService; if self.transaction_queue.has_local_pending_transactions() { - self.prepare_and_update_sealing(chain); + self.update_sealing(chain); } } } @@ -1250,7 +1251,7 @@ impl miner::MinerService for Miner { /// Update sealing if required. /// Prepare the block and work if the Engine does not seal internally. fn update_sealing(&self, chain: &C) where - C: miner::BlockChainClient, + C: BlockChain + CallContract + BlockProducer + SealedBlockImporter + Nonce + Sync, { trace!(target: "miner", "update_sealing"); diff --git a/ethcore/src/miner/mod.rs b/ethcore/src/miner/mod.rs index 452e152b1d9..0de9390259d 100644 --- a/ethcore/src/miner/mod.rs +++ b/ethcore/src/miner/mod.rs @@ -84,7 +84,7 @@ pub trait MinerService : Send + Sync { /// Update current pending block fn update_sealing(&self, chain: &C) - where C: BlockChainClient; + where C: BlockChain + CallContract + BlockProducer + SealedBlockImporter + Nonce + Sync; // Notifications From 33e5809a3356e3a3ec276bc407feb1ccece09a26 Mon Sep 17 00:00:00 2001 From: Nicolas Gotchac Date: Tue, 10 Sep 2019 11:14:00 +0200 Subject: [PATCH 12/17] Remove TxQueue cache clear // Update AtomicUint logic --- ethcore/engines/instant-seal/src/lib.rs | 15 ++++++++------- miner/src/pool/queue.rs | 1 - 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/ethcore/engines/instant-seal/src/lib.rs b/ethcore/engines/instant-seal/src/lib.rs index 2fda3eec515..9626be86b30 100644 --- a/ethcore/engines/instant-seal/src/lib.rs +++ b/ethcore/engines/instant-seal/src/lib.rs @@ -75,18 +75,19 @@ impl Engine for InstantSeal { fn sealing_state(&self) -> SealingState { SealingState::Ready } fn generate_seal(&self, block: &ExecutedBlock, _parent: &Header) -> Seal { - if block.transactions.is_empty() { - Seal::None - } else { + if !block.transactions.is_empty() { let block_number = block.header.number(); let last_sealed_block = self.last_sealed_block.load(Ordering::SeqCst); + // Return a regular seal if the given block is _higher_ than + // the last sealed one if block_number > last_sealed_block { - self.last_sealed_block.store(block_number, Ordering::SeqCst); - Seal::Regular(Vec::new()) - } else { - Seal::None + let prev_last_sealed_block = self.last_sealed_block.compare_and_swap(last_sealed_block, block_number, Ordering::SeqCst); + if prev_last_sealed_block == last_sealed_block { + return Seal::Regular(Vec::new()) + } } } + Seal::None } fn verify_local_seal(&self, _header: &Header) -> Result<(), Error> { diff --git a/miner/src/pool/queue.rs b/miner/src/pool/queue.rs index 7e286c8688d..a6dbe3450a8 100644 --- a/miner/src/pool/queue.rs +++ b/miner/src/pool/queue.rs @@ -458,7 +458,6 @@ impl TransactionQueue { let state_readiness = ready::State::new(client.clone(), stale_id, nonce_cap); removed += self.pool.write().cull(Some(chunk), state_readiness); } - self.cached_pending.write().clear(); debug!(target: "txqueue", "Removed {} stalled transactions. {}", removed, self.status()); } From eea5d32349f15196b7782085a246c15d70c94074 Mon Sep 17 00:00:00 2001 From: Nicolas Gotchac Date: Tue, 10 Sep 2019 11:26:59 +0200 Subject: [PATCH 13/17] Update comments in Miner --- ethcore/src/miner/miner.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 896f6e61094..e8b645bb757 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -33,7 +33,7 @@ use futures::sync::mpsc; use io::IoChannel; use miner::filter_options::{FilterOptions, FilterOperator}; use miner::pool_client::{PoolClient, CachedNonceClient, NonceCache}; -use miner; +use miner::{self, MinerService}; use parking_lot::{Mutex, RwLock}; use rayon::prelude::*; use types::{ @@ -862,7 +862,6 @@ impl Miner { /// Prepare pending block, check whether sealing is needed, and then update sealing. fn prepare_and_update_sealing(&self, chain: &C) { - use miner::MinerService; match self.engine.sealing_state() { SealingState::Ready => { self.maybe_enable_sealing(); @@ -878,9 +877,10 @@ impl Miner { } } - /// Call `update_sealing` if needed + /// Call `update_sealing` if there are some local pending transactions. + /// This should be called after an internal block import, which might not have + /// included all the pending local transactions. pub fn maybe_update_sealing(&self, chain: &C) { - use miner::MinerService; if self.transaction_queue.has_local_pending_transactions() { self.update_sealing(chain); } From 40e243aa1d2a24049577c959e7280e6687bf8c2f Mon Sep 17 00:00:00 2001 From: Nicolas Gotchac Date: Tue, 10 Sep 2019 11:28:43 +0200 Subject: [PATCH 14/17] Revert import of `parking_lot` --- Cargo.lock | 1 - ethcore/engines/instant-seal/Cargo.toml | 1 - 2 files changed, 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 40e889ecee3..98940990fae 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2128,7 +2128,6 @@ dependencies = [ "ethjson 0.1.0", "keccak-hash 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)", "machine 0.1.0", - "parking_lot 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)", "rlp 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)", "spec 0.1.0", "trace 0.1.0", diff --git a/ethcore/engines/instant-seal/Cargo.toml b/ethcore/engines/instant-seal/Cargo.toml index 80a0d394b5e..42a74e999d5 100644 --- a/ethcore/engines/instant-seal/Cargo.toml +++ b/ethcore/engines/instant-seal/Cargo.toml @@ -13,7 +13,6 @@ ethjson = { path = "../../../json" } ethereum-types = "0.6.0" keccak-hash = "0.2.0" machine = { path = "../../machine" } -parking_lot = "0.8" trace = { path = "../../trace" } [dev-dependencies] From 47c102b6519a21b7721fe0f169d5f2890a35be72 Mon Sep 17 00:00:00 2001 From: Nicolas Gotchac Date: Wed, 11 Sep 2019 11:14:18 +0200 Subject: [PATCH 15/17] Update `transaction-pool` dependency --- Cargo.lock | 10 +++++----- ethcore/private-tx/Cargo.toml | 2 +- miner/Cargo.toml | 2 +- rpc/Cargo.toml | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ee93d28f5b7..3d9227e9c7c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1263,7 +1263,7 @@ dependencies = [ "serde_derive 1.0.89 (registry+https://github.com/rust-lang/crates.io-index)", "serde_json 1.0.40 (registry+https://github.com/rust-lang/crates.io-index)", "trace-time 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)", - "transaction-pool 2.0.0 (registry+https://github.com/rust-lang/crates.io-index)", + "transaction-pool 2.0.1 (registry+https://github.com/rust-lang/crates.io-index)", "url 1.7.1 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -1366,7 +1366,7 @@ dependencies = [ "time-utils 0.1.0", "tiny-keccak 1.4.2 (registry+https://github.com/rust-lang/crates.io-index)", "trace 0.1.0", - "transaction-pool 2.0.0 (registry+https://github.com/rust-lang/crates.io-index)", + "transaction-pool 2.0.1 (registry+https://github.com/rust-lang/crates.io-index)", "trie-db 0.15.0 (registry+https://github.com/rust-lang/crates.io-index)", "url 1.7.1 (registry+https://github.com/rust-lang/crates.io-index)", "vm 0.1.0", @@ -3141,7 +3141,7 @@ dependencies = [ "tiny-keccak 1.4.2 (registry+https://github.com/rust-lang/crates.io-index)", "tokio-timer 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", "trace 0.1.0", - "transaction-pool 2.0.0 (registry+https://github.com/rust-lang/crates.io-index)", + "transaction-pool 2.0.1 (registry+https://github.com/rust-lang/crates.io-index)", "transient-hashmap 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)", "verification 0.1.0", "vm 0.1.0", @@ -4693,7 +4693,7 @@ dependencies = [ [[package]] name = "transaction-pool" -version = "2.0.0" +version = "2.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)", @@ -5521,7 +5521,7 @@ dependencies = [ "checksum toml 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)" = "b8c96d7873fa7ef8bdeb3a9cda3ac48389b4154f32b9803b4bc26220b677b039" "checksum toolshed 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "450441e131c7663af72e63a33c02a6a1fbaaa8601dc652ed6757813bb55aeec7" "checksum trace-time 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "dbe82f2f0bf1991e163e757baf044282823155dd326e70f44ce2186c3c320cc9" -"checksum transaction-pool 2.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "f8d8bd3123931aa6e49dd03bc8a2400490e14701d779458d1f1fff1f04c6f666" +"checksum transaction-pool 2.0.1 (registry+https://github.com/rust-lang/crates.io-index)" = "454adc482e32785c3beab9415dd0f3c689f29cc2d16717eb62f6a784d53544b4" "checksum transient-hashmap 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)" = "aeb4b191d033a35edfce392a38cdcf9790b6cebcb30fa690c312c29da4dc433e" "checksum trie-db 0.15.0 (registry+https://github.com/rust-lang/crates.io-index)" = "9b65d609ae631d808c6c1cc23a622733d5a0b66a7d67e9f5cd5171562a1f4cb5" "checksum trie-standardmap 0.15.0 (registry+https://github.com/rust-lang/crates.io-index)" = "64fda153c00484d640bc91334624be22ead0e5baca917d9fd53ff29bdebcf9b2" diff --git a/ethcore/private-tx/Cargo.toml b/ethcore/private-tx/Cargo.toml index 0d13bf41bb5..620148ea8c6 100644 --- a/ethcore/private-tx/Cargo.toml +++ b/ethcore/private-tx/Cargo.toml @@ -47,7 +47,7 @@ state-db = { path = "../state-db" } time-utils = { path = "../../util/time-utils" } tiny-keccak = "1.4" trace = { path = "../trace" } -transaction-pool = "2.0" +transaction-pool = "2.0.1" url = "1" vm = { path = "../vm" } diff --git a/miner/Cargo.toml b/miner/Cargo.toml index d24cdb904bb..16ea5b3c361 100644 --- a/miner/Cargo.toml +++ b/miner/Cargo.toml @@ -34,7 +34,7 @@ serde = "1.0" serde_derive = "1.0" serde_json = "1.0" trace-time = "0.1" -transaction-pool = "2.0" +transaction-pool = "2.0.1" [dev-dependencies] env_logger = "0.5" diff --git a/rpc/Cargo.toml b/rpc/Cargo.toml index aa555d75fdf..8a85c80e5a2 100644 --- a/rpc/Cargo.toml +++ b/rpc/Cargo.toml @@ -79,7 +79,7 @@ fake-fetch = { path = "../util/fake-fetch" } macros = { path = "../util/macros" } spec = { path = "../ethcore/spec" } pretty_assertions = "0.1" -transaction-pool = "2.0" +transaction-pool = "2.0.1" verification = { path = "../ethcore/verification" } [features] From 159cc84d47322cbfbd91e0898d03464b17816124 Mon Sep 17 00:00:00 2001 From: Nicolas Gotchac Date: Wed, 11 Sep 2019 15:29:03 +0200 Subject: [PATCH 16/17] Call directly `update_sealing` --- ethcore/src/miner/miner.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index e8b645bb757..be06f76a727 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -881,9 +881,7 @@ impl Miner { /// This should be called after an internal block import, which might not have /// included all the pending local transactions. pub fn maybe_update_sealing(&self, chain: &C) { - if self.transaction_queue.has_local_pending_transactions() { - self.update_sealing(chain); - } + self.update_sealing(chain); } } From 51c188d40e6efc36c3dd98527cd96a3561d7172f Mon Sep 17 00:00:00 2001 From: Nicolas Gotchac Date: Wed, 11 Sep 2019 15:49:50 +0200 Subject: [PATCH 17/17] Call `update_sealing` directly --- ethcore/src/client/client.rs | 5 ----- ethcore/src/miner/miner.rs | 13 +++---------- 2 files changed, 3 insertions(+), 15 deletions(-) diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index 82376639c27..b9fa829be83 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -1268,11 +1268,6 @@ impl Client { _ => self.block_header(id).and_then(|h| h.decode().ok()) } } - - /// Maybe call `update_sealing` method on the Miner - pub fn maybe_update_sealing(&self) { - self.importer.miner.maybe_update_sealing(self); - } } impl DatabaseRestore for Client { diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index be06f76a727..5c15d73799f 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -58,7 +58,7 @@ use using_queue::{UsingQueue, GetAction}; use block::{ClosedBlock, SealedBlock}; use client::{BlockProducer, SealedBlockImporter, Client}; -use client_traits::{BlockChain, ChainInfo, Nonce, TransactionInfo}; +use client_traits::{BlockChain, ChainInfo, EngineClient, Nonce, TransactionInfo}; use engine::{Engine, signer::EngineSigner}; use machine::executive::contract_address; use spec::Spec; @@ -876,13 +876,6 @@ impl Miner { SealingState::NotReady => { self.maybe_enable_sealing(); }, } } - - /// Call `update_sealing` if there are some local pending transactions. - /// This should be called after an internal block import, which might not have - /// included all the pending local transactions. - pub fn maybe_update_sealing(&self, chain: &C) { - self.update_sealing(chain); - } } impl miner::MinerService for Miner { @@ -1437,7 +1430,7 @@ impl miner::MinerService for Miner { ); queue.cull(client); if is_internal_import { - chain.maybe_update_sealing(); + chain.update_sealing(); } }; @@ -1447,7 +1440,7 @@ impl miner::MinerService for Miner { } else { self.transaction_queue.cull(client); if is_internal_import { - self.maybe_update_sealing(chain); + self.update_sealing(chain); } } }