diff --git a/CHANGELOG.md b/CHANGELOG.md index e56002baf..623cba754 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,14 +1,21 @@ ## Diamond Node Software 4.0.2 -OPTIONAL Update -New behavior for validator nodes as main feature +### New behavior for validator nodes - [Autoshutdown if a Node becomes a regular Node](https://github.com/DMDcoin/diamond-node/issues/322) +- [Remove empty blocks during key gen phases behaviour](https://github.com/DMDcoin/diamond-node/issues/327) +- [Service Transaction cleanup (garbage collect)](https://github.com/DMDcoin/diamond-node/issues/172) -Further stability improvements +### RPC +- [Gas price from contracts](https://github.com/DMDcoin/diamond-node/issues/159) + +### Further stability improvements - [FIXED: received transactions are getting pooled, if announced by another peer](https://github.com/DMDcoin/diamond-node/issues/304) - [FIXED: dropped transactions are getting pooled](https://github.com/DMDcoin/diamond-node/issues/303) - [FIXED: key generation can panic if faulty validators write malicious parts](https://github.com/DMDcoin/diamond-node/issues/100) +- [FIXED: already included transactions are refretched from peers](https://github.com/DMDcoin/diamond-node/issues/196) +- [Gracefull Node Shutdown: increase to 15 seconds](https://github.com/DMDcoin/diamond-node/issues/321) + ## Diamond Node Software 4.0.1 diff --git a/crates/concensus/miner/src/pool/queue.rs b/crates/concensus/miner/src/pool/queue.rs index 3034123d4..fcf9659cb 100644 --- a/crates/concensus/miner/src/pool/queue.rs +++ b/crates/concensus/miner/src/pool/queue.rs @@ -41,6 +41,8 @@ use crate::pool::{ verifier, PendingOrdering, PendingSettings, PrioritizationStrategy, }; +use super::VerifiedTransaction; + type Listener = ( LocalTransactionsList, (listener::Notifier, listener::Logger), @@ -413,6 +415,17 @@ impl TransactionQueue { .collect() } + /// Performs garbage collection of the pool of this transactionqueue for free service transactions. + /// Removes transaction that are not valid anymore. + /// The process executes listener calls. + pub fn garbage_collect bool>( + &self, + service_transaction_check: F, + ) { + let mut pool = self.pool.write(); + pool.garbage_collect(service_transaction_check); + } + /// Computes unordered set of pending hashes. /// /// Since strict nonce-checking is not required, you may get some false positive future transactions as well. diff --git a/crates/ethcore/service/src/service.rs b/crates/ethcore/service/src/service.rs index 9885ea874..67319b6f6 100644 --- a/crates/ethcore/service/src/service.rs +++ b/crates/ethcore/service/src/service.rs @@ -233,7 +233,9 @@ impl IoHandler for ClientIoHandler { ClientIoMessage::Execute(ref exec) => { (*exec.0)(&self.client); } - _ => {} // ignore other messages + ClientIoMessage::NewChainHead => { + self.client.garbage_collect_in_queue(); + } } } } diff --git a/crates/ethcore/src/client/client.rs b/crates/ethcore/src/client/client.rs index e4cc85d3f..9410070b3 100644 --- a/crates/ethcore/src/client/client.rs +++ b/crates/ethcore/src/client/client.rs @@ -306,6 +306,10 @@ pub struct Client { shutdown: Arc, + /// block number and block has of latest gc. + /// this information is used to avoid double garbage collection. + garbage_collect_latest_block: Mutex<(u64, H256)>, + statistics: ClientStatistics, } @@ -842,6 +846,13 @@ impl Importer { warn!("Failed to prune ancient state data: {}", e); } + client.schedule_garbage_collect_in_queue(); + + //self.on_block_commit_finalized(); + + //client.miner().g + //client.check_garbage();.garbage_collect(Duration::from_secs(1)); + route } @@ -1107,6 +1118,7 @@ impl Client { importer, config, shutdown, + garbage_collect_latest_block: Mutex::new((0, H256::zero())), statistics, }); @@ -1540,6 +1552,52 @@ impl Client { } } + /// Schedule garbage collection of invalid service transactions from the transaction queue based on the given block hash. + pub fn schedule_garbage_collect_in_queue(&self) { + let m = ClientIoMessage::execute(|c| c.garbage_collect_in_queue()); + if let Err(e) = self.io_channel.read().send(m) { + error!(target: "client", "Failed to schedule garbage collection in transaction queue for block {:?}", e); + } + } + + /// Garbage collect invalid servive transactions from the transaction queue based on the given block header. + pub fn garbage_collect_in_queue(&self) { + let machine = self.engine().machine(); + + match &self.block_header_decoded(BlockId::Latest) { + Some(block_header) => { + { + // scope for mutex. + let mut last_gc = self.garbage_collect_latest_block.lock(); + + if block_header.number() == last_gc.0 && block_header.hash() == last_gc.1 { + // already gced for this block, or gc is ongoing. + // we can return here. + return; + } + + // we treat ongoing gc as DONE, to avoid blocking of the message channel + last_gc.0 = block_header.number(); + last_gc.1 = block_header.hash(); + } + + // here hides an accepted race condition. + // latest block could change during loing ongoing GCs. + // this could be avoided developing a more complex GC logic. + // but the GC blocks the tx queue, so it has to be placing fast. + self.importer.miner.collect_garbage(|tx| + match machine.verify_transaction(tx.signed(), block_header, self) { + Ok(_) => true, + Err(e) => { + info!(target: "client", "collected garbage transaction from {:?}: {:?} reason: {:?}", tx.signed().sender(), tx.signed().hash, e); + false + }, + }); + } + None => {} + } + } + fn check_garbage(&self) { self.chain.read().collect_garbage(); self.importer.block_queue.collect_garbage(); diff --git a/crates/ethcore/src/engines/hbbft/contracts/staking.rs b/crates/ethcore/src/engines/hbbft/contracts/staking.rs index 84094ccc5..58b0ee653 100644 --- a/crates/ethcore/src/engines/hbbft/contracts/staking.rs +++ b/crates/ethcore/src/engines/hbbft/contracts/staking.rs @@ -35,11 +35,6 @@ pub fn get_posdao_epoch_start( call_const_staking!(c, staking_epoch_start_block) } -pub fn start_time_of_next_phase_transition(client: &dyn EngineClient) -> Result { - let c = BoundContract::bind(client, BlockId::Latest, *STAKING_CONTRACT_ADDRESS); - call_const_staking!(c, start_time_of_next_phase_transition) -} - pub fn candidate_min_stake(client: &dyn EngineClient) -> Result { let c = BoundContract::bind(client, BlockId::Latest, *STAKING_CONTRACT_ADDRESS); call_const_staking!(c, candidate_min_stake) diff --git a/crates/ethcore/src/engines/hbbft/hbbft_early_epoch_end_manager.rs b/crates/ethcore/src/engines/hbbft/hbbft_early_epoch_end_manager.rs index 398246ab4..3134d2fcb 100644 --- a/crates/ethcore/src/engines/hbbft/hbbft_early_epoch_end_manager.rs +++ b/crates/ethcore/src/engines/hbbft/hbbft_early_epoch_end_manager.rs @@ -112,7 +112,7 @@ impl HbbftEarlyEpochEndManager { signing_address: signing_address.clone(), }; - info!(target: "engine", "early-epoch-end: HbbftEarlyEpochEndManager created. start_time {now:?}, start_block: {epoch_start_block}"); + trace!(target: "engine", "early-epoch-end: HbbftEarlyEpochEndManager created. start_time {now:?}, start_block: {epoch_start_block}"); return Some(result); } diff --git a/crates/ethcore/src/engines/hbbft/hbbft_engine.rs b/crates/ethcore/src/engines/hbbft/hbbft_engine.rs index c3eca3362..b0fc2e614 100644 --- a/crates/ethcore/src/engines/hbbft/hbbft_engine.rs +++ b/crates/ethcore/src/engines/hbbft/hbbft_engine.rs @@ -51,7 +51,6 @@ use super::{ NodeId, contracts::{ keygen_history::{all_parts_acks_available, initialize_synckeygen}, - staking::start_time_of_next_phase_transition, validator_set::{ValidatorType, get_pending_validators, is_pending_validator}, }, contribution::{unix_now_millis, unix_now_secs}, @@ -329,9 +328,6 @@ impl TransitionHandler { // If the minimum block time has passed we are ready to trigger new blocks. if timer_duration == Duration::from_secs(0) { - // Always create blocks if we are in the keygen phase. - self.engine.start_hbbft_epoch_if_next_phase(); - // If the maximum block time has been reached we trigger a new block in any case. if self.max_block_time_remaining(client.clone()) == Duration::from_secs(0) { self.engine.start_hbbft_epoch(client); @@ -1104,27 +1100,6 @@ impl HoneyBadgerBFT { self.client.read().as_ref().and_then(Weak::upgrade) } - fn start_hbbft_epoch_if_next_phase(&self) { - // experimental deactivation of empty blocks. - // see: https://github.com/DMDcoin/diamond-node/issues/160 - - match self.client_arc() { - None => return, - Some(client) => { - // Get the next phase start time - let genesis_transition_time = match start_time_of_next_phase_transition(&*client) { - Ok(time) => time, - Err(_) => return, - }; - - // If current time larger than phase start time, start a new block. - if genesis_transition_time.as_u64() < unix_now_secs() { - self.start_hbbft_epoch(client); - } - } - } - } - fn replay_cached_messages(&self) -> Option<()> { let client = self.client_arc()?; diff --git a/crates/ethcore/src/engines/hbbft/hbbft_state.rs b/crates/ethcore/src/engines/hbbft/hbbft_state.rs index fe94ec2bd..3bdb78c20 100644 --- a/crates/ethcore/src/engines/hbbft/hbbft_state.rs +++ b/crates/ethcore/src/engines/hbbft/hbbft_state.rs @@ -209,10 +209,7 @@ impl HbbftState { // apply DAO updates here. // update the current minimum gas price. - match get_minimum_gas_from_permission_contract( - client.as_ref(), - BlockId::Number(self.current_posdao_epoch_start_block), - ) { + match get_minimum_gas_from_permission_contract(client.as_ref(), BlockId::Latest) { Ok(min_gas) => { *current_minimum_gas_price.lock() = Some(min_gas); } diff --git a/crates/ethcore/src/engines/hbbft/test/mod.rs b/crates/ethcore/src/engines/hbbft/test/mod.rs index 4c935ff7a..e95a4c576 100644 --- a/crates/ethcore/src/engines/hbbft/test/mod.rs +++ b/crates/ethcore/src/engines/hbbft/test/mod.rs @@ -1,12 +1,11 @@ use super::{ contracts::{ staking::{ - get_posdao_epoch, start_time_of_next_phase_transition, + get_posdao_epoch, tests::{create_staker, is_pool_active}, }, validator_set::{is_pending_validator, mining_by_staking_address}, }, - contribution::unix_now_secs, test::hbbft_test_client::{HbbftTestClient, create_hbbft_client, create_hbbft_clients}, }; use crate::{client::traits::BlockInfo, types::ids::BlockId}; @@ -129,12 +128,6 @@ fn test_epoch_transition() { // To avoid performing external transactions with the MoC we create and fund a random address. let transactor: KeyPair = Random.generate(); - let genesis_transition_time = start_time_of_next_phase_transition(moc.client.as_ref()) - .expect("start_time_of_next_phase_transition call must succeed"); - - // Genesis block is at time 0, current unix time must be much larger. - assert!(genesis_transition_time.as_u64() < unix_now_secs()); - // We should not be in the pending validator set at the genesis block. assert!( !is_pending_validator(moc.client.as_ref(), &moc.address()) diff --git a/crates/ethcore/src/miner/miner.rs b/crates/ethcore/src/miner/miner.rs index ee2135764..bc2a08bb8 100644 --- a/crates/ethcore/src/miner/miner.rs +++ b/crates/ethcore/src/miner/miner.rs @@ -420,6 +420,17 @@ impl Miner { self.service_transaction_checker.clone() } + /// Performs garbage collection of the pool for free service transactions. + /// Removes transaction that are not valid anymore. + /// The process executes listener calls. + pub fn collect_garbage bool>( + &self, + service_transaction_filter: F, + ) { + self.transaction_queue + .garbage_collect(service_transaction_filter); + } + /// Retrieves an existing pending block iff it's not older than given block number. /// /// NOTE: This will not prepare a new pending block if it's not existing. diff --git a/crates/transaction-pool/Cargo.toml b/crates/transaction-pool/Cargo.toml index db2478e95..a04fb955a 100644 --- a/crates/transaction-pool/Cargo.toml +++ b/crates/transaction-pool/Cargo.toml @@ -8,7 +8,6 @@ authors = ["Dragan Rakita . -use log::{trace, warn}; +use log::{info, trace, warn}; use std::{ collections::{hash_map, BTreeSet, HashMap}, slice, @@ -236,6 +236,44 @@ where } } + /// Performs garbage collection of the pool for free service transactions. + /// Removes transaction that are not valid anymore. + /// The process executes listener calls. + pub fn garbage_collect bool>(&mut self, service_transaction_check: F) { + if self.best_transactions.is_empty() { + return; + } + + let mut txs_to_remove = Vec::::new(); + + for sender in self.transactions.iter() { + for tx in sender.1.iter_transactions() { + if tx.transaction.has_zero_gas_price() { + if service_transaction_check(&tx.transaction) { + txs_to_remove.push(tx.hash().clone()); + } + } else { + // if the next transaction has not zero gas price, + // we are not continuing. + break; + }; + } + } + + if txs_to_remove.is_empty() { + return; + } + + info!( + "Garbage collection: removing invalid {} service transactions from pool.", + txs_to_remove.len() + ); + + for tx in txs_to_remove.iter() { + self.remove(tx, true); + } + } + /// Updates state of the pool statistics if the transaction was added to a set. fn finalize_insert(&mut self, new: &Transaction, old: Option<&Transaction>) { self.mem_usage += new.mem_usage();