diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index d0eb10210..c6be9daf9 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -1718,6 +1718,33 @@ impl MiningBlockChainClient for Client { open_block } + fn reopen_block(&self, block: ClosedBlock) -> OpenBlock { + let engine = &*self.engine; + let mut block = block.reopen(engine); + let max_uncles = engine.maximum_uncle_count(); + if block.uncles().len() < max_uncles { + let chain = self.chain.read(); + let h = chain.best_block_hash(); + // Add new uncles + let uncles = chain + .find_uncle_hashes(&h, engine.maximum_uncle_age()) + .unwrap_or_else(Vec::new); + + for h in uncles { + if !block.uncles().iter().any(|header| header.hash() == h) { + let uncle = chain.block_header(&h).expect("find_uncle_hashes only returns hashes for existing headers; qed"); + block.push_uncle(uncle).expect("pushing up to maximum_uncle_count; + push_uncle is not ok only if more than maximum_uncle_count is pushed; + so all push_uncle are Ok; + qed"); + if block.uncles().len() >= max_uncles { break } + } + } + + } + block + } + fn vm_factory(&self) -> &EvmFactory { &self.factories.vm } diff --git a/ethcore/src/client/test_client.rs b/ethcore/src/client/test_client.rs index e656e6c42..5dfdf5c25 100644 --- a/ethcore/src/client/test_client.rs +++ b/ethcore/src/client/test_client.rs @@ -44,7 +44,7 @@ use types::mode::Mode; use types::pruning_info::PruningInfo; use verification::queue::QueueInfo; -use block::{OpenBlock, SealedBlock}; +use block::{OpenBlock, SealedBlock, ClosedBlock}; use executive::Executed; use error::CallError; use trace::LocalizedTrace; @@ -381,6 +381,10 @@ impl MiningBlockChainClient for TestBlockChainClient { open_block } + fn reopen_block(&self, block: ClosedBlock) -> OpenBlock { + block.reopen(&*self.spec.engine) + } + fn vm_factory(&self) -> &EvmFactory { &self.vm_factory } diff --git a/ethcore/src/client/traits.rs b/ethcore/src/client/traits.rs index 879620f0a..da765ad07 100644 --- a/ethcore/src/client/traits.rs +++ b/ethcore/src/client/traits.rs @@ -19,7 +19,7 @@ use util::{U256, Address, H256, H2048, Bytes, Itertools}; use util::hashdb::DBValue; use blockchain::TreeRoute; use verification::queue::QueueInfo as BlockQueueInfo; -use block::{OpenBlock, SealedBlock}; +use block::{OpenBlock, SealedBlock, ClosedBlock}; use header::{BlockNumber}; use transaction::{LocalizedTransaction, PendingTransaction, SignedTransaction}; use transaction_import::TransactionImportResult; @@ -288,6 +288,9 @@ pub trait MiningBlockChainClient: BlockChainClient { extra_data: Bytes ) -> OpenBlock; + /// Reopens an OpenBlock and updates uncles. + fn reopen_block(&self, block: ClosedBlock) -> OpenBlock; + /// Returns EvmFactory. fn vm_factory(&self) -> &EvmFactory; diff --git a/ethcore/src/error.rs b/ethcore/src/error.rs index e3a32db01..066a9d31a 100644 --- a/ethcore/src/error.rs +++ b/ethcore/src/error.rs @@ -129,6 +129,8 @@ pub enum BlockError { UncleIsBrother(OutOfBounds), /// An uncle is already in the chain. UncleInChain(H256), + /// An uncle is included twice. + DuplicateUncle(H256), /// An uncle has a parent not in the chain. UncleParentNotInChain(H256), /// State root header field is invalid. @@ -188,6 +190,7 @@ impl fmt::Display for BlockError { UncleTooOld(ref oob) => format!("Uncle block is too old. {}", oob), UncleIsBrother(ref oob) => format!("Uncle from same generation as block. {}", oob), UncleInChain(ref hash) => format!("Uncle {} already in chain", hash), + DuplicateUncle(ref hash) => format!("Uncle {} already in the header", hash), UncleParentNotInChain(ref hash) => format!("Uncle {} has a parent not in the chain", hash), InvalidStateRoot(ref mis) => format!("Invalid state root in header: {}", mis), InvalidGasUsed(ref mis) => format!("Invalid gas used in header: {}", mis), diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index df28265cc..139aea2ab 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -88,6 +88,8 @@ pub struct MinerOptions { pub reseal_on_external_tx: bool, /// Reseal on receipt of new local transactions. pub reseal_on_own_tx: bool, + /// Reseal when new uncle block has been imported. + pub reseal_on_uncle: bool, /// Minimum period between transaction-inspired reseals. pub reseal_min_period: Duration, /// Maximum period between blocks (enables force sealing after that). @@ -119,6 +121,7 @@ impl Default for MinerOptions { force_sealing: false, reseal_on_external_tx: false, reseal_on_own_tx: true, + reseal_on_uncle: false, tx_gas_limit: !U256::zero(), tx_queue_size: 1024, tx_queue_gas_limit: GasLimit::Auto, @@ -347,7 +350,7 @@ impl Miner { Some(old_block) => { trace!(target: "miner", "prepare_block: Already have previous work; updating and returning"); // add transactions to old_block - old_block.reopen(&*self.engine) + chain.reopen_block(old_block) } None => { // block not found - create it. @@ -366,7 +369,6 @@ impl Miner { let mut transactions_to_penalize = HashSet::new(); let block_number = open_block.block().fields().header.number(); - // TODO Push new uncles too. let mut tx_count: usize = 0; let tx_total = transactions.len(); for tx in transactions { @@ -1153,11 +1155,10 @@ impl MinerService for Miner { }) } - fn chain_new_blocks(&self, chain: &MiningBlockChainClient, _imported: &[H256], _invalid: &[H256], enacted: &[H256], retracted: &[H256]) { + fn chain_new_blocks(&self, chain: &MiningBlockChainClient, imported: &[H256], _invalid: &[H256], enacted: &[H256], retracted: &[H256]) { trace!(target: "miner", "chain_new_blocks"); - // 1. We ignore blocks that were `imported` (because it means that they are not in canon-chain, and transactions - // should be still available in the queue. + // 1. We ignore blocks that were `imported` unless resealing on new uncles is enabled. // 2. We ignore blocks that are `invalid` because it doesn't have any meaning in terms of the transactions that // are in those blocks @@ -1192,7 +1193,7 @@ impl MinerService for Miner { transaction_queue.remove_old(&fetch_account, time); } - if enacted.len() > 0 { + if enacted.len() > 0 || (imported.len() > 0 && self.options.reseal_on_uncle) { // -------------------------------------------------------------------------- // | NOTE Code below requires transaction_queue and sealing_work locks. | // | Make sure to release the locks before calling that method. | @@ -1312,6 +1313,7 @@ mod tests { force_sealing: false, reseal_on_external_tx: false, reseal_on_own_tx: true, + reseal_on_uncle: false, reseal_min_period: Duration::from_secs(5), reseal_max_period: Duration::from_secs(120), tx_gas_limit: !U256::zero(), diff --git a/ethcore/src/verification/verification.rs b/ethcore/src/verification/verification.rs index d244c2af1..1be636adc 100644 --- a/ethcore/src/verification/verification.rs +++ b/ethcore/src/verification/verification.rs @@ -132,12 +132,17 @@ pub fn verify_block_family(header: &Header, bytes: &[u8], engine: &Engine, bc: & } } + let mut verified = HashSet::new(); for uncle in UntrustedRlp::new(bytes).at(2)?.iter().map(|rlp| rlp.as_val::
()) { let uncle = uncle?; if excluded.contains(&uncle.hash()) { return Err(From::from(BlockError::UncleInChain(uncle.hash()))) } + if verified.contains(&uncle.hash()) { + return Err(From::from(BlockError::DuplicateUncle(uncle.hash()))) + } + // m_currentBlock.number() - uncle.number() m_cB.n - uP.n() // 1 2 // 2 @@ -180,6 +185,7 @@ pub fn verify_block_family(header: &Header, bytes: &[u8], engine: &Engine, bc: & verify_parent(&uncle, &uncle_parent)?; engine.verify_block_family(&uncle, &uncle_parent, Some(bytes))?; + verified.insert(uncle.hash()); } } Ok(()) @@ -568,6 +574,11 @@ mod tests { check_fail(family_test(&create_test_block_with_data(&header, &good_transactions, &bad_uncles), engine, &bc), TooManyUncles(OutOfBounds { max: Some(engine.maximum_uncle_count()), min: None, found: bad_uncles.len() })); + header = good.clone(); + bad_uncles = vec![ good_uncle1.clone(), good_uncle1.clone() ]; + check_fail(family_test(&create_test_block_with_data(&header, &good_transactions, &bad_uncles), engine, &bc), + DuplicateUncle(good_uncle1.hash())); + // TODO: some additional uncle checks } diff --git a/parity/cli/mod.rs b/parity/cli/mod.rs index 198622966..009a6dc38 100644 --- a/parity/cli/mod.rs +++ b/parity/cli/mod.rs @@ -250,6 +250,8 @@ usage! { or |c: &Config| otry!(c.mining).force_sealing.clone(), flag_reseal_on_txs: String = "own", or |c: &Config| otry!(c.mining).reseal_on_txs.clone(), + flag_reseal_on_uncle: bool = false, + or |c: &Config| otry!(c.mining).reseal_on_uncle.clone(), flag_reseal_min_period: u64 = 2000u64, or |c: &Config| otry!(c.mining).reseal_min_period.clone(), flag_reseal_max_period: u64 = 120000u64, @@ -524,6 +526,7 @@ struct Mining { author: Option, engine_signer: Option, force_sealing: Option, + reseal_on_uncle: Option, reseal_on_txs: Option, reseal_min_period: Option, reseal_max_period: Option, @@ -788,6 +791,7 @@ mod tests { flag_reseal_on_txs: "all".into(), flag_reseal_min_period: 4000u64, flag_reseal_max_period: 60000u64, + flag_reseal_on_uncle: false, flag_work_queue_size: 20usize, flag_tx_gas_limit: Some("6283184".into()), flag_tx_time_limit: Some(100u64), @@ -1012,6 +1016,7 @@ mod tests { engine_signer: Some("0xdeadbeefcafe0000000000000000000000000001".into()), force_sealing: Some(true), reseal_on_txs: Some("all".into()), + reseal_on_uncle: None, reseal_min_period: Some(4000), reseal_max_period: Some(60000), work_queue_size: None, diff --git a/parity/cli/usage.txt b/parity/cli/usage.txt index 40be1856f..5020f00ba 100644 --- a/parity/cli/usage.txt +++ b/parity/cli/usage.txt @@ -262,6 +262,9 @@ Sealing/Mining Options: ext - reseal only on a new external transaction; all - reseal on all new transactions (default: {flag_reseal_on_txs}). + --reseal-on-uncle Force the node to author new blocks when a new uncle + block is imported. + (default: {flag_reseal_on_uncle}) --reseal-min-period MS Specify the minimum time between reseals from incoming transactions. MS is time measured in milliseconds (default: {flag_reseal_min_period}). diff --git a/parity/configuration.rs b/parity/configuration.rs index a48d8f716..6d1879779 100644 --- a/parity/configuration.rs +++ b/parity/configuration.rs @@ -526,6 +526,7 @@ impl Configuration { force_sealing: self.args.flag_force_sealing, reseal_on_external_tx: reseal.external, reseal_on_own_tx: reseal.own, + reseal_on_uncle: self.args.flag_reseal_on_uncle, tx_gas_limit: match self.args.flag_tx_gas_limit { Some(ref d) => to_u256(d)?, None => U256::max_value(), diff --git a/rpc/src/v1/tests/eth.rs b/rpc/src/v1/tests/eth.rs index 8d2ca7b88..7ce1dcc29 100644 --- a/rpc/src/v1/tests/eth.rs +++ b/rpc/src/v1/tests/eth.rs @@ -58,6 +58,7 @@ fn miner_service(spec: &Spec, accounts: Arc) -> Arc { force_sealing: true, reseal_on_external_tx: true, reseal_on_own_tx: true, + reseal_on_uncle: false, tx_queue_size: 1024, tx_gas_limit: !U256::zero(), tx_queue_strategy: PrioritizationStrategy::GasPriceOnly,