From f0c0da85515c9163a8ff06d1ac56b72545ab2cbe Mon Sep 17 00:00:00 2001 From: Matthew Martin Date: Tue, 31 Jul 2018 04:52:49 -0700 Subject: [PATCH 1/6] Check if synced when using eth_getWork (#9193) (#9210) * Check if synced when using eth_getWork (#9193) * Don't use fn syncing * Fix identation * Fix typo * Don't check for warping * rpc: avoid calling queue_info twice on eth_getWork --- rpc/src/v1/impls/eth.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/rpc/src/v1/impls/eth.rs b/rpc/src/v1/impls/eth.rs index d20b2feb267..f8934b26e68 100644 --- a/rpc/src/v1/impls/eth.rs +++ b/rpc/src/v1/impls/eth.rs @@ -744,9 +744,11 @@ impl Eth for EthClient< // check if we're still syncing and return empty strings in that case { - //TODO: check if initial sync is complete here - //let sync = self.sync; - if /*sync.status().state != SyncState::Idle ||*/ self.client.queue_info().total_queue_size() > MAX_QUEUE_SIZE_TO_MINE_ON { + let sync_status = self.sync.status(); + let queue_info = self.client.queue_info(); + let total_queue_size = queue_info.total_queue_size(); + + if is_major_importing(Some(sync_status.state), queue_info) || total_queue_size > MAX_QUEUE_SIZE_TO_MINE_ON { trace!(target: "miner", "Syncing. Cannot give any work."); return Err(errors::no_work()); } From 637883f52b249be8284d77ac7a76968ebff4aefa Mon Sep 17 00:00:00 2001 From: Wei Tang Date: Wed, 1 Aug 2018 19:17:04 +0800 Subject: [PATCH 2/6] Comply EIP-86 with the new definition (#9140) * Comply EIP-86 with the new CREATE2 opcode * Fix rpc compile * Fix interpreter CREATE/CREATE2 stack pop difference * Add unreachable! to fix compile * Fix instruction_info * Fix gas check due to new stack item * Add new tests in executive * Fix have_create2 comment * Remove all unused references of eip86_transition and block_number --- ethcore/evm/src/instructions.rs | 2 +- ethcore/evm/src/interpreter/gasometer.rs | 6 +++- ethcore/evm/src/interpreter/mod.rs | 6 +++- ethcore/src/executive.rs | 8 +++-- ethcore/src/externalities.rs | 40 ++++++++++++++++++++++++ ethcore/src/machine.rs | 8 ++--- ethcore/vm/src/ext.rs | 8 ++--- ethcore/vm/src/schedule.rs | 2 +- rpc/src/v1/helpers/dispatch.rs | 6 ++-- rpc/src/v1/helpers/light_fetch.rs | 8 ++--- rpc/src/v1/impls/eth.rs | 11 +++---- rpc/src/v1/impls/light/eth.rs | 17 +++------- rpc/src/v1/impls/light/parity.rs | 8 ++--- rpc/src/v1/impls/parity.rs | 12 ++----- rpc/src/v1/impls/parity_set.rs | 5 +-- rpc/src/v1/tests/mocked/signing.rs | 2 +- rpc/src/v1/types/transaction.rs | 20 ++++++------ 17 files changed, 96 insertions(+), 73 deletions(-) diff --git a/ethcore/evm/src/instructions.rs b/ethcore/evm/src/instructions.rs index 3be4556942c..67d390d4dff 100644 --- a/ethcore/evm/src/instructions.rs +++ b/ethcore/evm/src/instructions.rs @@ -594,7 +594,7 @@ lazy_static! { arr[DELEGATECALL as usize] = Some(InstructionInfo::new("DELEGATECALL", 6, 1, GasPriceTier::Special)); arr[STATICCALL as usize] = Some(InstructionInfo::new("STATICCALL", 6, 1, GasPriceTier::Special)); arr[SUICIDE as usize] = Some(InstructionInfo::new("SUICIDE", 1, 0, GasPriceTier::Special)); - arr[CREATE2 as usize] = Some(InstructionInfo::new("CREATE2", 3, 1, GasPriceTier::Special)); + arr[CREATE2 as usize] = Some(InstructionInfo::new("CREATE2", 4, 1, GasPriceTier::Special)); arr[REVERT as usize] = Some(InstructionInfo::new("REVERT", 2, 0, GasPriceTier::Zero)); arr }; diff --git a/ethcore/evm/src/interpreter/gasometer.rs b/ethcore/evm/src/interpreter/gasometer.rs index 943a8320bb4..78a33ef3ead 100644 --- a/ethcore/evm/src/interpreter/gasometer.rs +++ b/ethcore/evm/src/interpreter/gasometer.rs @@ -228,7 +228,11 @@ impl Gasometer { }, instructions::CREATE | instructions::CREATE2 => { let gas = Gas::from(schedule.create_gas); - let mem = mem_needed(stack.peek(1), stack.peek(2))?; + let mem = match instruction { + instructions::CREATE => mem_needed(stack.peek(1), stack.peek(2))?, + instructions::CREATE2 => mem_needed(stack.peek(2), stack.peek(3))?, + _ => unreachable!("instruction can only be CREATE/CREATE2 checked above; qed"), + }; Request::GasMemProvide(gas, mem, None) }, diff --git a/ethcore/evm/src/interpreter/mod.rs b/ethcore/evm/src/interpreter/mod.rs index 6b8abcc17f7..3af1e34dd36 100644 --- a/ethcore/evm/src/interpreter/mod.rs +++ b/ethcore/evm/src/interpreter/mod.rs @@ -317,6 +317,11 @@ impl Interpreter { }, instructions::CREATE | instructions::CREATE2 => { let endowment = stack.pop_back(); + let address_scheme = match instruction { + instructions::CREATE => CreateContractAddress::FromSenderAndNonce, + instructions::CREATE2 => CreateContractAddress::FromSenderSaltAndCodeHash(stack.pop_back().into()), + _ => unreachable!("instruction can only be CREATE/CREATE2 checked above; qed"), + }; let init_off = stack.pop_back(); let init_size = stack.pop_back(); @@ -336,7 +341,6 @@ impl Interpreter { } let contract_code = self.mem.read_slice(init_off, init_size); - let address_scheme = if instruction == instructions::CREATE { CreateContractAddress::FromSenderAndNonce } else { CreateContractAddress::FromSenderAndCodeHash }; let create_result = ext.create(&create_gas.as_u256(), &endowment, contract_code, address_scheme); return match create_result { diff --git a/ethcore/src/executive.rs b/ethcore/src/executive.rs index 9bb0bc8f9f5..b77cb050e41 100644 --- a/ethcore/src/executive.rs +++ b/ethcore/src/executive.rs @@ -61,10 +61,12 @@ pub fn contract_address(address_scheme: CreateContractAddress, sender: &Address, stream.append(nonce); (From::from(keccak(stream.as_raw())), None) }, - CreateContractAddress::FromCodeHash => { + CreateContractAddress::FromSenderSaltAndCodeHash(salt) => { let code_hash = keccak(code); - let mut buffer = [0xffu8; 20 + 32]; - &mut buffer[20..].copy_from_slice(&code_hash[..]); + let mut buffer = [0u8; 20 + 32 + 32]; + &mut buffer[0..20].copy_from_slice(&sender[..]); + &mut buffer[20..(20+32)].copy_from_slice(&salt[..]); + &mut buffer[(20+32)..].copy_from_slice(&code_hash[..]); (From::from(keccak(&buffer[..])), Some(code_hash)) }, CreateContractAddress::FromSenderAndCodeHash => { diff --git a/ethcore/src/externalities.rs b/ethcore/src/externalities.rs index c1bd785c4a7..2dbc82f604d 100644 --- a/ethcore/src/externalities.rs +++ b/ethcore/src/externalities.rs @@ -581,4 +581,44 @@ mod tests { assert_eq!(setup.sub_state.suicides.len(), 1); } + + #[test] + fn can_create() { + use std::str::FromStr; + + let mut setup = TestSetup::new(); + let state = &mut setup.state; + let mut tracer = NoopTracer; + let mut vm_tracer = NoopVMTracer; + + let address = { + let mut ext = Externalities::new(state, &setup.env_info, &setup.machine, 0, get_test_origin(), &mut setup.sub_state, OutputPolicy::InitContract(None), &mut tracer, &mut vm_tracer, false); + match ext.create(&U256::max_value(), &U256::zero(), &[], CreateContractAddress::FromSenderAndNonce) { + ContractCreateResult::Created(address, _) => address, + _ => panic!("Test create failed; expected Created, got Failed/Reverted."), + } + }; + + assert_eq!(address, Address::from_str("bd770416a3345f91e4b34576cb804a576fa48eb1").unwrap()); + } + + #[test] + fn can_create2() { + use std::str::FromStr; + + let mut setup = TestSetup::new(); + let state = &mut setup.state; + let mut tracer = NoopTracer; + let mut vm_tracer = NoopVMTracer; + + let address = { + let mut ext = Externalities::new(state, &setup.env_info, &setup.machine, 0, get_test_origin(), &mut setup.sub_state, OutputPolicy::InitContract(None), &mut tracer, &mut vm_tracer, false); + match ext.create(&U256::max_value(), &U256::zero(), &[], CreateContractAddress::FromSenderSaltAndCodeHash(H256::default())) { + ContractCreateResult::Created(address, _) => address, + _ => panic!("Test create failed; expected Created, got Failed/Reverted."), + } + }; + + assert_eq!(address, Address::from_str("b7c227636666831278bacdb8d7f52933b8698ab9").unwrap()); + } } diff --git a/ethcore/src/machine.rs b/ethcore/src/machine.rs index d58a3878d7d..5b2170609a3 100644 --- a/ethcore/src/machine.rs +++ b/ethcore/src/machine.rs @@ -310,12 +310,8 @@ impl EthereumMachine { } /// Returns new contract address generation scheme at given block number. - pub fn create_address_scheme(&self, number: BlockNumber) -> CreateContractAddress { - if number >= self.params().eip86_transition { - CreateContractAddress::FromCodeHash - } else { - CreateContractAddress::FromSenderAndNonce - } + pub fn create_address_scheme(&self, _number: BlockNumber) -> CreateContractAddress { + CreateContractAddress::FromSenderAndNonce } /// Verify a particular transaction is valid, regardless of order. diff --git a/ethcore/vm/src/ext.rs b/ethcore/vm/src/ext.rs index b6d41a39521..3e6ee1e0265 100644 --- a/ethcore/vm/src/ext.rs +++ b/ethcore/vm/src/ext.rs @@ -53,11 +53,11 @@ pub enum MessageCallResult { /// Specifies how an address is calculated for a new contract. #[derive(Copy, Clone, PartialEq, Eq)] pub enum CreateContractAddress { - /// Address is calculated from nonce and sender. Pre EIP-86 (Metropolis) + /// Address is calculated from sender and nonce. Pre EIP-86 (Metropolis) FromSenderAndNonce, - /// Address is calculated from code hash. Default since EIP-86 - FromCodeHash, - /// Address is calculated from code hash and sender. Used by CREATE_P2SH instruction. + /// Address is calculated from sender, salt and code hash. EIP-86 CREATE2 scheme. + FromSenderSaltAndCodeHash(H256), + /// Address is calculated from code hash and sender. Used by pwasm create ext. FromSenderAndCodeHash, } diff --git a/ethcore/vm/src/schedule.rs b/ethcore/vm/src/schedule.rs index 6215aed78d9..757d8a16d5d 100644 --- a/ethcore/vm/src/schedule.rs +++ b/ethcore/vm/src/schedule.rs @@ -22,7 +22,7 @@ pub struct Schedule { pub exceptional_failed_code_deposit: bool, /// Does it have a delegate cal pub have_delegate_call: bool, - /// Does it have a CREATE_P2SH instruction + /// Does it have a CREATE2 instruction pub have_create2: bool, /// Does it have a REVERT instruction pub have_revert: bool, diff --git a/rpc/src/v1/helpers/dispatch.rs b/rpc/src/v1/helpers/dispatch.rs index c6f10400a58..9b789a56bb2 100644 --- a/rpc/src/v1/helpers/dispatch.rs +++ b/rpc/src/v1/helpers/dispatch.rs @@ -178,8 +178,7 @@ impl Dispatcher } fn enrich(&self, signed_transaction: SignedTransaction) -> RpcRichRawTransaction { - let block_number = self.client.best_block_header().number(); - RpcRichRawTransaction::from_signed(signed_transaction, block_number, self.client.eip86_transition()) + RpcRichRawTransaction::from_signed(signed_transaction) } fn dispatch_transaction(&self, signed_transaction: PendingTransaction) -> Result { @@ -405,8 +404,7 @@ impl Dispatcher for LightDispatcher { } fn enrich(&self, signed_transaction: SignedTransaction) -> RpcRichRawTransaction { - let block_number = self.client.best_block_header().number(); - RpcRichRawTransaction::from_signed(signed_transaction, block_number, self.client.eip86_transition()) + RpcRichRawTransaction::from_signed(signed_transaction) } fn dispatch_transaction(&self, signed_transaction: PendingTransaction) -> Result { diff --git a/rpc/src/v1/helpers/light_fetch.rs b/rpc/src/v1/helpers/light_fetch.rs index ae8aadf94a7..1da2fdf1ad3 100644 --- a/rpc/src/v1/helpers/light_fetch.rs +++ b/rpc/src/v1/helpers/light_fetch.rs @@ -66,7 +66,7 @@ pub struct LightFetch { } /// Extract a transaction at given index. -pub fn extract_transaction_at_index(block: encoded::Block, index: usize, eip86_transition: u64) -> Option { +pub fn extract_transaction_at_index(block: encoded::Block, index: usize) -> Option { block.transactions().into_iter().nth(index) // Verify if transaction signature is correct. .and_then(|tx| SignedTransaction::new(tx).ok()) @@ -85,7 +85,7 @@ pub fn extract_transaction_at_index(block: encoded::Block, index: usize, eip86_t cached_sender, } }) - .map(|tx| Transaction::from_localized(tx, eip86_transition)) + .map(|tx| Transaction::from_localized(tx)) } // extract the header indicated by the given `HeaderRef` from the given responses. @@ -365,7 +365,7 @@ impl LightFetch { // Get a transaction by hash. also returns the index in the block. // Only returns transactions in the canonical chain. - pub fn transaction_by_hash(&self, tx_hash: H256, eip86_transition: u64) + pub fn transaction_by_hash(&self, tx_hash: H256) -> impl Future, Error = Error> + Send { let params = (self.sync.clone(), self.on_demand.clone()); @@ -397,7 +397,7 @@ impl LightFetch { } let index = index.index as usize; - let transaction = extract_transaction_at_index(blk, index, eip86_transition); + let transaction = extract_transaction_at_index(blk, index); if transaction.as_ref().map_or(true, |tx| tx.hash != tx_hash.into()) { // index is actively wrong: indicated block has diff --git a/rpc/src/v1/impls/eth.rs b/rpc/src/v1/impls/eth.rs index f8934b26e68..84c0aa6ed95 100644 --- a/rpc/src/v1/impls/eth.rs +++ b/rpc/src/v1/impls/eth.rs @@ -107,7 +107,6 @@ pub struct EthClient where external_miner: Arc, seed_compute: Mutex, options: EthClientOptions, - eip86_transition: u64, } #[derive(Debug)] @@ -169,7 +168,6 @@ impl EthClient EthClient BlockTransactions::Full(block.view().localized_transactions().into_iter().map(|t| Transaction::from_localized(t, self.eip86_transition)).collect()), + true => BlockTransactions::Full(block.view().localized_transactions().into_iter().map(|t| Transaction::from_localized(t)).collect()), false => BlockTransactions::Hashes(block.transaction_hashes().into_iter().map(Into::into).collect()), }, extra_data: Bytes::new(view.extra_data()), @@ -268,7 +266,7 @@ impl EthClient Result> { let client_transaction = |id| match self.client.transaction(id) { - Some(t) => Ok(Some(Transaction::from_localized(t, self.eip86_transition))), + Some(t) => Ok(Some(Transaction::from_localized(t))), None => Ok(None), }; @@ -305,7 +303,7 @@ impl EthClient Eth for EthClient< fn transaction_by_hash(&self, hash: RpcH256) -> BoxFuture> { let hash: H256 = hash.into(); - let block_number = self.client.chain_info().best_block_number; let tx = try_bf!(self.transaction(PendingTransactionId::Hash(hash))).or_else(|| { self.miner.transaction(&hash) - .map(|t| Transaction::from_pending(t.pending().clone(), block_number + 1, self.eip86_transition)) + .map(|t| Transaction::from_pending(t.pending().clone())) }); Box::new(future::ok(tx)) diff --git a/rpc/src/v1/impls/light/eth.rs b/rpc/src/v1/impls/light/eth.rs index e88ac2dab35..042720c5cae 100644 --- a/rpc/src/v1/impls/light/eth.rs +++ b/rpc/src/v1/impls/light/eth.rs @@ -142,7 +142,6 @@ impl EthClient { fn rich_block(&self, id: BlockId, include_txs: bool) -> BoxFuture { let (on_demand, sync) = (self.on_demand.clone(), self.sync.clone()); let (client, engine) = (self.client.clone(), self.client.engine().clone()); - let eip86_transition = self.client.eip86_transition(); // helper for filling out a rich block once we've got a block and a score. let fill_rich = move |block: encoded::Block, score: Option| { @@ -169,7 +168,7 @@ impl EthClient { seal_fields: header.seal().into_iter().cloned().map(Into::into).collect(), uncles: block.uncle_hashes().into_iter().map(Into::into).collect(), transactions: match include_txs { - true => BlockTransactions::Full(block.view().localized_transactions().into_iter().map(|t| Transaction::from_localized(t, eip86_transition)).collect()), + true => BlockTransactions::Full(block.view().localized_transactions().into_iter().map(|t| Transaction::from_localized(t)).collect()), _ => BlockTransactions::Hashes(block.transaction_hashes().into_iter().map(Into::into).collect()), }, extra_data: Bytes::new(header.extra_data().clone()), @@ -419,40 +418,34 @@ impl Eth for EthClient { fn transaction_by_hash(&self, hash: RpcH256) -> BoxFuture> { let hash = hash.into(); - let eip86 = self.client.eip86_transition(); { let tx_queue = self.transaction_queue.read(); if let Some(tx) = tx_queue.get(&hash) { return Box::new(future::ok(Some(Transaction::from_pending( tx.clone(), - self.client.chain_info().best_block_number, - eip86, )))); } } - Box::new(self.fetcher().transaction_by_hash(hash, eip86).map(|x| x.map(|(tx, _)| tx))) + Box::new(self.fetcher().transaction_by_hash(hash).map(|x| x.map(|(tx, _)| tx))) } fn transaction_by_block_hash_and_index(&self, hash: RpcH256, idx: Index) -> BoxFuture> { - let eip86 = self.client.eip86_transition(); Box::new(self.fetcher().block(BlockId::Hash(hash.into())).map(move |block| { - light_fetch::extract_transaction_at_index(block, idx.value(), eip86) + light_fetch::extract_transaction_at_index(block, idx.value()) })) } fn transaction_by_block_number_and_index(&self, num: BlockNumber, idx: Index) -> BoxFuture> { - let eip86 = self.client.eip86_transition(); Box::new(self.fetcher().block(Self::num_to_id(num)).map(move |block| { - light_fetch::extract_transaction_at_index(block, idx.value(), eip86) + light_fetch::extract_transaction_at_index(block, idx.value()) })) } fn transaction_receipt(&self, hash: RpcH256) -> BoxFuture> { - let eip86 = self.client.eip86_transition(); let fetcher = self.fetcher(); - Box::new(fetcher.transaction_by_hash(hash.clone().into(), eip86).and_then(move |tx| { + Box::new(fetcher.transaction_by_hash(hash.clone().into()).and_then(move |tx| { // the block hash included in the transaction object here has // already been checked for canonicality and whether it contains // the transaction. diff --git a/rpc/src/v1/impls/light/parity.rs b/rpc/src/v1/impls/light/parity.rs index e7dcbe9e9ab..2045a4a595f 100644 --- a/rpc/src/v1/impls/light/parity.rs +++ b/rpc/src/v1/impls/light/parity.rs @@ -57,7 +57,6 @@ pub struct ParityClient { settings: Arc, signer: Option>, ws_address: Option, - eip86_transition: u64, gas_price_percentile: usize, } @@ -80,7 +79,6 @@ impl ParityClient { settings, signer, ws_address, - eip86_transition: client.eip86_transition(), client, gas_price_percentile, } @@ -264,7 +262,7 @@ impl Parity for ParityClient { txq.ready_transactions(chain_info.best_block_number, chain_info.best_block_timestamp) .into_iter() .take(limit.unwrap_or_else(usize::max_value)) - .map(|tx| Transaction::from_pending(tx, chain_info.best_block_number, self.eip86_transition)) + .map(|tx| Transaction::from_pending(tx)) .collect::>() ) } @@ -279,7 +277,7 @@ impl Parity for ParityClient { current .into_iter() .chain(future.into_iter()) - .map(|tx| Transaction::from_pending(tx, chain_info.best_block_number, self.eip86_transition)) + .map(|tx| Transaction::from_pending(tx)) .collect::>() ) } @@ -290,7 +288,7 @@ impl Parity for ParityClient { Ok( txq.future_transactions(chain_info.best_block_number, chain_info.best_block_timestamp) .into_iter() - .map(|tx| Transaction::from_pending(tx, chain_info.best_block_number, self.eip86_transition)) + .map(|tx| Transaction::from_pending(tx)) .collect::>() ) } diff --git a/rpc/src/v1/impls/parity.rs b/rpc/src/v1/impls/parity.rs index c4c2ac9dba8..2bdf09df486 100644 --- a/rpc/src/v1/impls/parity.rs +++ b/rpc/src/v1/impls/parity.rs @@ -62,7 +62,6 @@ pub struct ParityClient { settings: Arc, signer: Option>, ws_address: Option, - eip86_transition: u64, } impl ParityClient where @@ -81,7 +80,6 @@ impl ParityClient where signer: Option>, ws_address: Option, ) -> Self { - let eip86_transition = client.eip86_transition(); ParityClient { client, miner, @@ -93,7 +91,6 @@ impl ParityClient where settings, signer, ws_address, - eip86_transition, } } } @@ -296,7 +293,6 @@ impl Parity for ParityClient where } fn pending_transactions(&self, limit: Trailing) -> Result> { - let block_number = self.client.chain_info().best_block_number; let ready_transactions = self.miner.ready_transactions( &*self.client, limit.unwrap_or_else(usize::max_value), @@ -305,18 +301,17 @@ impl Parity for ParityClient where Ok(ready_transactions .into_iter() - .map(|t| Transaction::from_pending(t.pending().clone(), block_number, self.eip86_transition)) + .map(|t| Transaction::from_pending(t.pending().clone())) .collect() ) } fn all_transactions(&self) -> Result> { - let block_number = self.client.chain_info().best_block_number; let all_transactions = self.miner.queued_transactions(); Ok(all_transactions .into_iter() - .map(|t| Transaction::from_pending(t.pending().clone(), block_number, self.eip86_transition)) + .map(|t| Transaction::from_pending(t.pending().clone())) .collect() ) } @@ -335,10 +330,9 @@ impl Parity for ParityClient where fn local_transactions(&self) -> Result> { let transactions = self.miner.local_transactions(); - let block_number = self.client.chain_info().best_block_number; Ok(transactions .into_iter() - .map(|(hash, status)| (hash.into(), LocalTransactionStatus::from(status, block_number, self.eip86_transition))) + .map(|(hash, status)| (hash.into(), LocalTransactionStatus::from(status))) .collect() ) } diff --git a/rpc/src/v1/impls/parity_set.rs b/rpc/src/v1/impls/parity_set.rs index c811b3e5daf..9bbb7ceab14 100644 --- a/rpc/src/v1/impls/parity_set.rs +++ b/rpc/src/v1/impls/parity_set.rs @@ -41,7 +41,6 @@ pub struct ParitySetClient { net: Arc, fetch: F, pool: CpuPool, - eip86_transition: u64, } impl ParitySetClient @@ -63,7 +62,6 @@ impl ParitySetClient net: net.clone(), fetch: fetch, pool: pool, - eip86_transition: client.eip86_transition(), } } } @@ -191,11 +189,10 @@ impl ParitySet for ParitySetClient where } fn remove_transaction(&self, hash: H256) -> Result> { - let block_number = self.client.chain_info().best_block_number; let hash = hash.into(); Ok(self.miner.remove_transaction(&hash) - .map(|t| Transaction::from_pending(t.pending().clone(), block_number + 1, self.eip86_transition)) + .map(|t| Transaction::from_pending(t.pending().clone())) ) } } diff --git a/rpc/src/v1/tests/mocked/signing.rs b/rpc/src/v1/tests/mocked/signing.rs index 632d8fb76b7..c063ee09601 100644 --- a/rpc/src/v1/tests/mocked/signing.rs +++ b/rpc/src/v1/tests/mocked/signing.rs @@ -339,7 +339,7 @@ fn should_add_sign_transaction_to_the_queue() { // respond let sender = signer.take(&1.into()).unwrap(); signer.request_confirmed(sender, Ok(ConfirmationResponse::SignTransaction( - RichRawTransaction::from_signed(t.into(), 0x0, u64::max_value()) + RichRawTransaction::from_signed(t.into()) ))); break } diff --git a/rpc/src/v1/types/transaction.rs b/rpc/src/v1/types/transaction.rs index d41fc84e001..4266f60b623 100644 --- a/rpc/src/v1/types/transaction.rs +++ b/rpc/src/v1/types/transaction.rs @@ -161,8 +161,8 @@ pub struct RichRawTransaction { impl RichRawTransaction { /// Creates new `RichRawTransaction` from `SignedTransaction`. - pub fn from_signed(tx: SignedTransaction, block_number: u64, eip86_transition: u64) -> Self { - let tx = Transaction::from_signed(tx, block_number, eip86_transition); + pub fn from_signed(tx: SignedTransaction) -> Self { + let tx = Transaction::from_signed(tx); RichRawTransaction { raw: tx.raw.clone(), transaction: tx, @@ -172,9 +172,9 @@ impl RichRawTransaction { impl Transaction { /// Convert `LocalizedTransaction` into RPC Transaction. - pub fn from_localized(mut t: LocalizedTransaction, eip86_transition: u64) -> Transaction { + pub fn from_localized(mut t: LocalizedTransaction) -> Transaction { let signature = t.signature(); - let scheme = if t.block_number >= eip86_transition { CreateContractAddress::FromCodeHash } else { CreateContractAddress::FromSenderAndNonce }; + let scheme = CreateContractAddress::FromSenderAndNonce; Transaction { hash: t.hash().into(), nonce: t.nonce.into(), @@ -206,9 +206,9 @@ impl Transaction { } /// Convert `SignedTransaction` into RPC Transaction. - pub fn from_signed(t: SignedTransaction, block_number: u64, eip86_transition: u64) -> Transaction { + pub fn from_signed(t: SignedTransaction) -> Transaction { let signature = t.signature(); - let scheme = if block_number >= eip86_transition { CreateContractAddress::FromCodeHash } else { CreateContractAddress::FromSenderAndNonce }; + let scheme = CreateContractAddress::FromSenderAndNonce; Transaction { hash: t.hash().into(), nonce: t.nonce.into(), @@ -240,8 +240,8 @@ impl Transaction { } /// Convert `PendingTransaction` into RPC Transaction. - pub fn from_pending(t: PendingTransaction, block_number: u64, eip86_transition: u64) -> Transaction { - let mut r = Transaction::from_signed(t.transaction, block_number, eip86_transition); + pub fn from_pending(t: PendingTransaction) -> Transaction { + let mut r = Transaction::from_signed(t.transaction); r.condition = t.condition.map(|b| b.into()); r } @@ -249,9 +249,9 @@ impl Transaction { impl LocalTransactionStatus { /// Convert `LocalTransactionStatus` into RPC `LocalTransactionStatus`. - pub fn from(s: miner::pool::local_transactions::Status, block_number: u64, eip86_transition: u64) -> Self { + pub fn from(s: miner::pool::local_transactions::Status) -> Self { let convert = |tx: Arc| { - Transaction::from_signed(tx.signed().clone(), block_number, eip86_transition) + Transaction::from_signed(tx.signed().clone()) }; use miner::pool::local_transactions::Status::*; match s { From c22498066b6546c4a2bd58f1ff4a664d00142f43 Mon Sep 17 00:00:00 2001 From: Nicolas Gotchac Date: Wed, 1 Aug 2018 18:03:41 +0200 Subject: [PATCH 3/6] Update ref to `parity-common` and update `seek` behaviour (#9257) * Update ref to `parity-common` and update `seek` behaviour * Remove reference to `ng-fix-triedb-seek` branch --- Cargo.lock | 38 ++++++++++++++++++------------------ ethcore/src/client/client.rs | 8 +++++++- 2 files changed, 26 insertions(+), 20 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6ecec53882c..54a05ba05be 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -532,7 +532,7 @@ dependencies = [ "lru-cache 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)", "macros 0.1.0", "memory-cache 0.1.0", - "memorydb 0.2.0 (git+https://github.com/paritytech/parity-common)", + "memorydb 0.2.1 (git+https://github.com/paritytech/parity-common)", "num 0.1.42 (registry+https://github.com/rust-lang/crates.io-index)", "num_cpus 1.8.0 (registry+https://github.com/rust-lang/crates.io-index)", "parity-bytes 0.1.0 (git+https://github.com/paritytech/parity-common)", @@ -607,7 +607,7 @@ dependencies = [ "kvdb-memorydb 0.1.0 (git+https://github.com/paritytech/parity-common)", "log 0.3.9 (registry+https://github.com/rust-lang/crates.io-index)", "memory-cache 0.1.0", - "memorydb 0.2.0 (git+https://github.com/paritytech/parity-common)", + "memorydb 0.2.1 (git+https://github.com/paritytech/parity-common)", "parity-bytes 0.1.0 (git+https://github.com/paritytech/parity-common)", "parking_lot 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)", "patricia-trie 0.2.1 (git+https://github.com/paritytech/parity-common)", @@ -1172,7 +1172,7 @@ dependencies = [ [[package]] name = "hashdb" version = "0.2.0" -source = "git+https://github.com/paritytech/parity-common#5f05acd90cf173f2e1ce477665be2a40502fef42" +source = "git+https://github.com/paritytech/parity-common#0045887fecd2fec39e56c962a0b27e2caf3b9474" dependencies = [ "elastic-array 0.10.0 (registry+https://github.com/rust-lang/crates.io-index)", "heapsize 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1330,7 +1330,7 @@ dependencies = [ "kvdb 0.1.0 (git+https://github.com/paritytech/parity-common)", "kvdb-memorydb 0.1.0 (git+https://github.com/paritytech/parity-common)", "log 0.3.9 (registry+https://github.com/rust-lang/crates.io-index)", - "memorydb 0.2.0 (git+https://github.com/paritytech/parity-common)", + "memorydb 0.2.1 (git+https://github.com/paritytech/parity-common)", "parity-bytes 0.1.0 (git+https://github.com/paritytech/parity-common)", "parking_lot 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)", "plain_hasher 0.1.0 (git+https://github.com/paritytech/parity-common)", @@ -1436,7 +1436,7 @@ dependencies = [ [[package]] name = "keccak-hash" version = "0.1.2" -source = "git+https://github.com/paritytech/parity-common#5f05acd90cf173f2e1ce477665be2a40502fef42" +source = "git+https://github.com/paritytech/parity-common#0045887fecd2fec39e56c962a0b27e2caf3b9474" dependencies = [ "ethereum-types 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)", "tiny-keccak 1.4.2 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1464,7 +1464,7 @@ dependencies = [ [[package]] name = "kvdb" version = "0.1.0" -source = "git+https://github.com/paritytech/parity-common#5f05acd90cf173f2e1ce477665be2a40502fef42" +source = "git+https://github.com/paritytech/parity-common#0045887fecd2fec39e56c962a0b27e2caf3b9474" dependencies = [ "elastic-array 0.10.0 (registry+https://github.com/rust-lang/crates.io-index)", "parity-bytes 0.1.0 (git+https://github.com/paritytech/parity-common)", @@ -1473,7 +1473,7 @@ dependencies = [ [[package]] name = "kvdb-memorydb" version = "0.1.0" -source = "git+https://github.com/paritytech/parity-common#5f05acd90cf173f2e1ce477665be2a40502fef42" +source = "git+https://github.com/paritytech/parity-common#0045887fecd2fec39e56c962a0b27e2caf3b9474" dependencies = [ "kvdb 0.1.0 (git+https://github.com/paritytech/parity-common)", "parking_lot 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1482,7 +1482,7 @@ dependencies = [ [[package]] name = "kvdb-rocksdb" version = "0.1.0" -source = "git+https://github.com/paritytech/parity-common#5f05acd90cf173f2e1ce477665be2a40502fef42" +source = "git+https://github.com/paritytech/parity-common#0045887fecd2fec39e56c962a0b27e2caf3b9474" dependencies = [ "elastic-array 0.10.0 (registry+https://github.com/rust-lang/crates.io-index)", "ethereum-types 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1652,8 +1652,8 @@ source = "registry+https://github.com/rust-lang/crates.io-index" [[package]] name = "memorydb" -version = "0.2.0" -source = "git+https://github.com/paritytech/parity-common#5f05acd90cf173f2e1ce477665be2a40502fef42" +version = "0.2.1" +source = "git+https://github.com/paritytech/parity-common#0045887fecd2fec39e56c962a0b27e2caf3b9474" dependencies = [ "elastic-array 0.10.0 (registry+https://github.com/rust-lang/crates.io-index)", "hashdb 0.2.0 (git+https://github.com/paritytech/parity-common)", @@ -1922,7 +1922,7 @@ dependencies = [ [[package]] name = "parity-bytes" version = "0.1.0" -source = "git+https://github.com/paritytech/parity-common#5f05acd90cf173f2e1ce477665be2a40502fef42" +source = "git+https://github.com/paritytech/parity-common#0045887fecd2fec39e56c962a0b27e2caf3b9474" [[package]] name = "parity-clib" @@ -1934,7 +1934,7 @@ dependencies = [ [[package]] name = "parity-crypto" version = "0.1.0" -source = "git+https://github.com/paritytech/parity-common#5f05acd90cf173f2e1ce477665be2a40502fef42" +source = "git+https://github.com/paritytech/parity-common#0045887fecd2fec39e56c962a0b27e2caf3b9474" dependencies = [ "ethereum-types 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)", "quick-error 1.2.2 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2297,12 +2297,12 @@ dependencies = [ [[package]] name = "path" version = "0.1.1" -source = "git+https://github.com/paritytech/parity-common#5f05acd90cf173f2e1ce477665be2a40502fef42" +source = "git+https://github.com/paritytech/parity-common#0045887fecd2fec39e56c962a0b27e2caf3b9474" [[package]] name = "patricia-trie" version = "0.2.1" -source = "git+https://github.com/paritytech/parity-common#5f05acd90cf173f2e1ce477665be2a40502fef42" +source = "git+https://github.com/paritytech/parity-common#0045887fecd2fec39e56c962a0b27e2caf3b9474" dependencies = [ "elastic-array 0.10.0 (registry+https://github.com/rust-lang/crates.io-index)", "hashdb 0.2.0 (git+https://github.com/paritytech/parity-common)", @@ -2377,7 +2377,7 @@ dependencies = [ [[package]] name = "plain_hasher" version = "0.1.0" -source = "git+https://github.com/paritytech/parity-common#5f05acd90cf173f2e1ce477665be2a40502fef42" +source = "git+https://github.com/paritytech/parity-common#0045887fecd2fec39e56c962a0b27e2caf3b9474" dependencies = [ "crunchy 0.1.6 (registry+https://github.com/rust-lang/crates.io-index)", "ethereum-types 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2630,7 +2630,7 @@ dependencies = [ [[package]] name = "rlp" version = "0.2.1" -source = "git+https://github.com/paritytech/parity-common#5f05acd90cf173f2e1ce477665be2a40502fef42" +source = "git+https://github.com/paritytech/parity-common#0045887fecd2fec39e56c962a0b27e2caf3b9474" dependencies = [ "byteorder 1.2.3 (registry+https://github.com/rust-lang/crates.io-index)", "elastic-array 0.10.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -3307,7 +3307,7 @@ dependencies = [ [[package]] name = "trie-standardmap" version = "0.1.0" -source = "git+https://github.com/paritytech/parity-common#5f05acd90cf173f2e1ce477665be2a40502fef42" +source = "git+https://github.com/paritytech/parity-common#0045887fecd2fec39e56c962a0b27e2caf3b9474" dependencies = [ "ethereum-types 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)", "keccak-hash 0.1.2 (git+https://github.com/paritytech/parity-common)", @@ -3318,7 +3318,7 @@ dependencies = [ [[package]] name = "triehash" version = "0.2.0" -source = "git+https://github.com/paritytech/parity-common#5f05acd90cf173f2e1ce477665be2a40502fef42" +source = "git+https://github.com/paritytech/parity-common#0045887fecd2fec39e56c962a0b27e2caf3b9474" dependencies = [ "elastic-array 0.10.0 (registry+https://github.com/rust-lang/crates.io-index)", "ethereum-types 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)", @@ -3729,7 +3729,7 @@ dependencies = [ "checksum memmap 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)" = "e2ffa2c986de11a9df78620c01eeaaf27d94d3ff02bf81bfcca953102dd0c6ff" "checksum memoffset 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)" = "0f9dc261e2b62d7a622bf416ea3c5245cdd5d9a7fcc428c0d06804dfce1775b3" "checksum memory_units 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "71d96e3f3c0b6325d8ccd83c33b28acb183edcb6c67938ba104ec546854b0882" -"checksum memorydb 0.2.0 (git+https://github.com/paritytech/parity-common)" = "" +"checksum memorydb 0.2.1 (git+https://github.com/paritytech/parity-common)" = "" "checksum mime 0.3.4 (registry+https://github.com/rust-lang/crates.io-index)" = "e3d709ffbb330e1566dc2f2a3c9b58a5ad4a381f740b810cd305dc3f089bc160" "checksum mime_guess 2.0.0-alpha.2 (registry+https://github.com/rust-lang/crates.io-index)" = "27a5e6679a0614e25adc14c6434ba84e41632b765a6d9cb2031a0cca682699ae" "checksum mio 0.6.14 (registry+https://github.com/rust-lang/crates.io-index)" = "6d771e3ef92d58a8da8df7d6976bfca9371ed1de6619d9d5a5ce5b1f29b85bfe" diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index f63615701e2..d69803980a2 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -1692,6 +1692,9 @@ impl BlockChainClient for Client { if let Some(after) = after { if let Err(e) = iter.seek(after) { trace!(target: "fatdb", "list_accounts: Couldn't seek the DB: {:?}", e); + } else { + // Position the iterator after the `after` element + iter.next(); } } @@ -1735,7 +1738,10 @@ impl BlockChainClient for Client { if let Some(after) = after { if let Err(e) = iter.seek(after) { - trace!(target: "fatdb", "list_accounts: Couldn't seek the DB: {:?}", e); + trace!(target: "fatdb", "list_storage: Couldn't seek the DB: {:?}", e); + } else { + // Position the iterator after the `after` element + iter.next(); } } From f442665c461cbfef5ac35ab7c0fbd6ca22976efb Mon Sep 17 00:00:00 2001 From: cheme Date: Thu, 2 Aug 2018 11:15:22 +0200 Subject: [PATCH 4/6] Fix eternalities tests can_create (missing parameter) (#9270) --- ethcore/src/externalities.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ethcore/src/externalities.rs b/ethcore/src/externalities.rs index 2dbc82f604d..68d2c0817f0 100644 --- a/ethcore/src/externalities.rs +++ b/ethcore/src/externalities.rs @@ -592,7 +592,7 @@ mod tests { let mut vm_tracer = NoopVMTracer; let address = { - let mut ext = Externalities::new(state, &setup.env_info, &setup.machine, 0, get_test_origin(), &mut setup.sub_state, OutputPolicy::InitContract(None), &mut tracer, &mut vm_tracer, false); + let mut ext = Externalities::new(state, &setup.env_info, &setup.machine, &setup.schedule, 0, get_test_origin(), &mut setup.sub_state, OutputPolicy::InitContract(None), &mut tracer, &mut vm_tracer, false); match ext.create(&U256::max_value(), &U256::zero(), &[], CreateContractAddress::FromSenderAndNonce) { ContractCreateResult::Created(address, _) => address, _ => panic!("Test create failed; expected Created, got Failed/Reverted."), @@ -612,7 +612,8 @@ mod tests { let mut vm_tracer = NoopVMTracer; let address = { - let mut ext = Externalities::new(state, &setup.env_info, &setup.machine, 0, get_test_origin(), &mut setup.sub_state, OutputPolicy::InitContract(None), &mut tracer, &mut vm_tracer, false); + let mut ext = Externalities::new(state, &setup.env_info, &setup.machine, &setup.schedule, 0, get_test_origin(), &mut setup.sub_state, OutputPolicy::InitContract(None), &mut tracer, &mut vm_tracer, false); + match ext.create(&U256::max_value(), &U256::zero(), &[], CreateContractAddress::FromSenderSaltAndCodeHash(H256::default())) { ContractCreateResult::Created(address, _) => address, _ => panic!("Test create failed; expected Created, got Failed/Reverted."), From b4ae1b6528fb13821ea6524cc67fded852f776e2 Mon Sep 17 00:00:00 2001 From: Marek Kotewicz Date: Thu, 2 Aug 2018 11:20:46 +0200 Subject: [PATCH 5/6] decode block rlp less often (#9252) in total: - removed 4 redundant rlp deserializations - avoid 1 redundant block data copy --- ethcore/src/block.rs | 59 +++++-------------- ethcore/src/client/client.rs | 55 ++++++++--------- ethcore/src/client/test_client.rs | 16 ++--- ethcore/src/client/traits.rs | 5 +- ethcore/src/engines/validator_set/multi.rs | 3 +- .../engines/validator_set/safe_contract.rs | 3 +- ethcore/src/json_tests/chain.rs | 8 +-- ethcore/src/miner/miner.rs | 2 +- ethcore/src/test_helpers.rs | 9 +-- ethcore/src/tests/client.rs | 22 +------ ethcore/src/tests/trace.rs | 7 ++- ethcore/sync/src/block_sync.rs | 24 ++++---- ethcore/sync/src/blocks.rs | 2 +- ethcore/sync/src/chain/handler.rs | 44 +++++++------- parity/blockchain.rs | 4 +- rpc/src/lib.rs | 1 - rpc/src/v1/tests/eth.rs | 17 +++--- 17 files changed, 116 insertions(+), 165 deletions(-) diff --git a/ethcore/src/block.rs b/ethcore/src/block.rs index 6f1ba7a2f00..9fd3957fb34 100644 --- a/ethcore/src/block.rs +++ b/ethcore/src/block.rs @@ -40,7 +40,7 @@ use engines::EthEngine; use error::{Error, BlockError}; use ethereum_types::{H256, U256, Address, Bloom}; use factory::Factories; -use hash::{keccak, KECCAK_NULL_RLP, KECCAK_EMPTY_LIST_RLP}; +use hash::keccak; use header::{Header, ExtendedHeader}; use receipt::{Receipt, TransactionOutcome}; use rlp::{Rlp, RlpStream, Encodable, Decodable, DecoderError, encode_list}; @@ -51,7 +51,6 @@ use transaction::{UnverifiedTransaction, SignedTransaction, Error as Transaction use triehash::ordered_trie_root; use unexpected::{Mismatch, OutOfBounds}; use verification::PreverifiedBlock; -use views::BlockView; use vm::{EnvInfo, LastHashes}; /// A block, encoded as it is on the block chain. @@ -66,11 +65,6 @@ pub struct Block { } impl Block { - /// Returns true if the given bytes form a valid encoding of a block in RLP. - pub fn is_good(b: &[u8]) -> bool { - Rlp::new(b).as_val::().is_ok() - } - /// Get the RLP-encoding of the block with the seal. pub fn rlp_bytes(&self) -> Bytes { let mut block_rlp = RlpStream::new_list(3); @@ -398,26 +392,11 @@ impl<'x> OpenBlock<'x> { /// Turn this into a `ClosedBlock`. pub fn close(self) -> Result { - let mut s = self; - - let unclosed_state = s.block.state.clone(); - - s.engine.on_close_block(&mut s.block)?; - s.block.state.commit()?; - - s.block.header.set_transactions_root(ordered_trie_root(s.block.transactions.iter().map(|e| e.rlp_bytes()))); - let uncle_bytes = encode_list(&s.block.uncles); - s.block.header.set_uncles_hash(keccak(&uncle_bytes)); - s.block.header.set_state_root(s.block.state.root().clone()); - s.block.header.set_receipts_root(ordered_trie_root(s.block.receipts.iter().map(|r| r.rlp_bytes()))); - s.block.header.set_log_bloom(s.block.receipts.iter().fold(Bloom::zero(), |mut b, r| { - b.accrue_bloom(&r.log_bloom); - b - })); - s.block.header.set_gas_used(s.block.receipts.last().map_or_else(U256::zero, |r| r.gas_used)); + let unclosed_state = self.block.state.clone(); + let locked = self.close_and_lock()?; Ok(ClosedBlock { - block: s.block, + block: locked.block, unclosed_state, }) } @@ -429,18 +408,11 @@ impl<'x> OpenBlock<'x> { s.engine.on_close_block(&mut s.block)?; s.block.state.commit()?; - if s.block.header.transactions_root().is_zero() || s.block.header.transactions_root() == &KECCAK_NULL_RLP { - s.block.header.set_transactions_root(ordered_trie_root(s.block.transactions.iter().map(|e| e.rlp_bytes()))); - } - if s.block.header.uncles_hash().is_zero() || s.block.header.uncles_hash() == &KECCAK_EMPTY_LIST_RLP { - let uncle_bytes = encode_list(&s.block.uncles); - s.block.header.set_uncles_hash(keccak(&uncle_bytes)); - } - if s.block.header.receipts_root().is_zero() || s.block.header.receipts_root() == &KECCAK_NULL_RLP { - s.block.header.set_receipts_root(ordered_trie_root(s.block.receipts.iter().map(|r| r.rlp_bytes()))); - } - + s.block.header.set_transactions_root(ordered_trie_root(s.block.transactions.iter().map(|e| e.rlp_bytes()))); + let uncle_bytes = encode_list(&s.block.uncles); + s.block.header.set_uncles_hash(keccak(&uncle_bytes)); s.block.header.set_state_root(s.block.state.root().clone()); + s.block.header.set_receipts_root(ordered_trie_root(s.block.receipts.iter().map(|r| r.rlp_bytes()))); s.block.header.set_log_bloom(s.block.receipts.iter().fold(Bloom::zero(), |mut b, r| { b.accrue_bloom(&r.log_bloom); b @@ -537,18 +509,16 @@ impl LockedBlock { self, engine: &EthEngine, seal: Vec, - ) -> Result { + ) -> Result { let mut s = self; s.block.header.set_seal(seal); s.block.header.compute_hash(); // TODO: passing state context to avoid engines owning it? - match engine.verify_local_seal(&s.block.header) { - Err(e) => Err((e, s)), - _ => Ok(SealedBlock { - block: s.block - }), - } + engine.verify_local_seal(&s.block.header)?; + Ok(SealedBlock { + block: s.block + }) } } @@ -637,12 +607,11 @@ pub fn enact_verified( is_epoch_begin: bool, ancestry: &mut Iterator, ) -> Result { - let view = view!(BlockView, &block.bytes); enact( block.header, block.transactions, - view.uncles(), + block.uncles, engine, tracing, db, diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index d69803980a2..02af5ff0ad8 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -73,6 +73,8 @@ use types::filter::Filter; use types::ancestry_action::AncestryAction; use verification; use verification::{PreverifiedBlock, Verifier, BlockQueue}; +use verification::queue::kind::blocks::Unverified; +use verification::queue::kind::BlockLike; // re-export pub use types::blockchain_info::BlockChainInfo; @@ -208,7 +210,7 @@ pub struct Client { /// Queued ancient blocks, make sure they are imported in order. queued_ancient_blocks: Arc, - VecDeque<(Header, encoded::Block, Bytes)> + VecDeque<(Unverified, Bytes)> )>>, ancient_blocks_import_lock: Arc>, /// Consensus messages import queue @@ -428,7 +430,7 @@ impl Importer { /// /// The block is guaranteed to be the next best blocks in the /// first block sequence. Does no sealing or transaction validation. - fn import_old_block(&self, header: &Header, block: encoded::Block, receipts_bytes: &[u8], db: &KeyValueDB, chain: &BlockChain) -> Result<(), ::error::Error> { + fn import_old_block(&self, unverified: Unverified, receipts_bytes: &[u8], db: &KeyValueDB, chain: &BlockChain) -> Result<(), ::error::Error> { let receipts = ::rlp::decode_list(receipts_bytes); let _import_lock = self.import_lock.lock(); @@ -436,11 +438,11 @@ impl Importer { trace_time!("import_old_block"); // verify the block, passing the chain for updating the epoch verifier. let mut rng = OsRng::new()?; - self.ancient_verifier.verify(&mut rng, &header, &chain)?; + self.ancient_verifier.verify(&mut rng, &unverified.header, &chain)?; // Commit results let mut batch = DBTransaction::new(); - chain.insert_unordered_block(&mut batch, block, receipts, None, false, true); + chain.insert_unordered_block(&mut batch, encoded::Block::new(unverified.bytes), receipts, None, false, true); // Final commit to the DB db.write_buffered(batch); chain.commit(); @@ -1381,22 +1383,15 @@ impl CallContract for Client { } impl ImportBlock for Client { - fn import_block(&self, bytes: Bytes) -> Result { - use verification::queue::kind::BlockLike; - use verification::queue::kind::blocks::Unverified; - - // create unverified block here so the `keccak` calculation can be cached. - let unverified = Unverified::from_rlp(bytes)?; - - { - if self.chain.read().is_known(&unverified.hash()) { - bail!(BlockImportErrorKind::Import(ImportErrorKind::AlreadyInChain)); - } - let status = self.block_status(BlockId::Hash(unverified.parent_hash())); - if status == BlockStatus::Unknown || status == BlockStatus::Pending { - bail!(BlockImportErrorKind::Block(BlockError::UnknownParent(unverified.parent_hash()))); - } + fn import_block(&self, unverified: Unverified) -> Result { + if self.chain.read().is_known(&unverified.hash()) { + bail!(BlockImportErrorKind::Import(ImportErrorKind::AlreadyInChain)); + } + let status = self.block_status(BlockId::Hash(unverified.parent_hash())); + if status == BlockStatus::Unknown || status == BlockStatus::Pending { + bail!(BlockImportErrorKind::Block(BlockError::UnknownParent(unverified.parent_hash()))); } + Ok(self.importer.block_queue.import(unverified)?) } } @@ -2027,24 +2022,23 @@ impl IoClient for Client { }); } - fn queue_ancient_block(&self, block_bytes: Bytes, receipts_bytes: Bytes) -> Result { + fn queue_ancient_block(&self, unverified: Unverified, receipts_bytes: Bytes) -> Result { trace_time!("queue_ancient_block"); - let header: Header = ::rlp::Rlp::new(&block_bytes).val_at(0)?; - let hash = header.hash(); + let hash = unverified.hash(); { // check block order if self.chain.read().is_known(&hash) { bail!(BlockImportErrorKind::Import(ImportErrorKind::AlreadyInChain)); } - let parent_hash = header.parent_hash(); + let parent_hash = unverified.parent_hash(); // NOTE To prevent race condition with import, make sure to check queued blocks first // (and attempt to acquire lock) - let is_parent_pending = self.queued_ancient_blocks.read().0.contains(parent_hash); + let is_parent_pending = self.queued_ancient_blocks.read().0.contains(&parent_hash); if !is_parent_pending { - let status = self.block_status(BlockId::Hash(*parent_hash)); + let status = self.block_status(BlockId::Hash(parent_hash)); if status == BlockStatus::Unknown || status == BlockStatus::Pending { - bail!(BlockImportErrorKind::Block(BlockError::UnknownParent(*parent_hash))); + bail!(BlockImportErrorKind::Block(BlockError::UnknownParent(parent_hash))); } } } @@ -2053,7 +2047,7 @@ impl IoClient for Client { { let mut queued = self.queued_ancient_blocks.write(); queued.0.insert(hash); - queued.1.push_back((header, encoded::Block::new(block_bytes), receipts_bytes)); + queued.1.push_back((unverified, receipts_bytes)); } let queued = self.queued_ancient_blocks.clone(); @@ -2065,11 +2059,10 @@ impl IoClient for Client { let _lock = lock.lock(); for _i in 0..MAX_ANCIENT_BLOCKS_TO_IMPORT { let first = queued.write().1.pop_front(); - if let Some((header, block_bytes, receipts_bytes)) = first { - let hash = header.hash(); + if let Some((unverified, receipts_bytes)) = first { + let hash = unverified.hash(); let result = client.importer.import_old_block( - &header, - block_bytes, + unverified, &receipts_bytes, &**client.db.read().key_value(), &*client.chain.read(), diff --git a/ethcore/src/client/test_client.rs b/ethcore/src/client/test_client.rs index 7a3dfcd0075..627d844aeb6 100644 --- a/ethcore/src/client/test_client.rs +++ b/ethcore/src/client/test_client.rs @@ -53,6 +53,7 @@ use spec::Spec; use types::basic_account::BasicAccount; use types::pruning_info::PruningInfo; use verification::queue::QueueInfo; +use verification::queue::kind::blocks::Unverified; use block::{OpenBlock, SealedBlock, ClosedBlock}; use executive::Executed; use error::CallError; @@ -280,7 +281,8 @@ impl TestBlockChainClient { rlp.append(&header); rlp.append_raw(&txs, 1); rlp.append_raw(uncles.as_raw(), 1); - self.import_block(rlp.as_raw().to_vec()).unwrap(); + let unverified = Unverified::from_rlp(rlp.out()).unwrap(); + self.import_block(unverified).unwrap(); } } @@ -512,8 +514,8 @@ impl RegistryInfo for TestBlockChainClient { } impl ImportBlock for TestBlockChainClient { - fn import_block(&self, b: Bytes) -> Result { - let header = view!(BlockView, &b).header(); + fn import_block(&self, unverified: Unverified) -> Result { + let header = unverified.header; let h = header.hash(); let number: usize = header.number() as usize; if number > self.blocks.read().len() { @@ -539,7 +541,7 @@ impl ImportBlock for TestBlockChainClient { *difficulty = *difficulty + header.difficulty().clone(); } mem::replace(&mut *self.last_hash.write(), h.clone()); - self.blocks.write().insert(h.clone(), b); + self.blocks.write().insert(h.clone(), unverified.bytes); self.numbers.write().insert(number, h.clone()); let mut parent_hash = header.parent_hash().clone(); if number > 0 { @@ -552,7 +554,7 @@ impl ImportBlock for TestBlockChainClient { } } else { - self.blocks.write().insert(h.clone(), b.to_vec()); + self.blocks.write().insert(h.clone(), unverified.bytes); } Ok(h) } @@ -856,8 +858,8 @@ impl IoClient for TestBlockChainClient { self.miner.import_external_transactions(self, txs); } - fn queue_ancient_block(&self, b: Bytes, _r: Bytes) -> Result { - self.import_block(b) + fn queue_ancient_block(&self, unverified: Unverified, _r: Bytes) -> Result { + self.import_block(unverified) } fn queue_consensus_message(&self, message: Bytes) { diff --git a/ethcore/src/client/traits.rs b/ethcore/src/client/traits.rs index 65bf0092118..3f5595f6193 100644 --- a/ethcore/src/client/traits.rs +++ b/ethcore/src/client/traits.rs @@ -34,6 +34,7 @@ use receipt::LocalizedReceipt; use trace::LocalizedTrace; use transaction::{self, LocalizedTransaction, SignedTransaction}; use verification::queue::QueueInfo as BlockQueueInfo; +use verification::queue::kind::blocks::Unverified; use state::StateInfo; use header::Header; use engines::EthEngine; @@ -167,7 +168,7 @@ pub trait RegistryInfo { /// Provides methods to import block into blockchain pub trait ImportBlock { /// Import a block into the blockchain. - fn import_block(&self, bytes: Bytes) -> Result; + fn import_block(&self, block: Unverified) -> Result; } /// Provides `call_contract` method @@ -204,7 +205,7 @@ pub trait IoClient: Sync + Send { fn queue_transactions(&self, transactions: Vec, peer_id: usize); /// Queue block import with transaction receipts. Does no sealing and transaction validation. - fn queue_ancient_block(&self, block_bytes: Bytes, receipts_bytes: Bytes) -> Result; + fn queue_ancient_block(&self, block_bytes: Unverified, receipts_bytes: Bytes) -> Result; /// Queue conensus engine message. fn queue_consensus_message(&self, message: Bytes); diff --git a/ethcore/src/engines/validator_set/multi.rs b/ethcore/src/engines/validator_set/multi.rs index a23208cd2a8..24fb2d890bd 100644 --- a/ethcore/src/engines/validator_set/multi.rs +++ b/ethcore/src/engines/validator_set/multi.rs @@ -158,6 +158,7 @@ mod tests { use test_helpers::{generate_dummy_client_with_spec_and_accounts, generate_dummy_client_with_spec_and_data}; use types::ids::BlockId; use ethereum_types::Address; + use verification::queue::kind::blocks::Unverified; use super::Multi; @@ -198,7 +199,7 @@ mod tests { let sync_client = generate_dummy_client_with_spec_and_data(Spec::new_validator_multi, 0, 0, &[]); sync_client.engine().register_client(Arc::downgrade(&sync_client) as _); for i in 1..4 { - sync_client.import_block(client.block(BlockId::Number(i)).unwrap().into_inner()).unwrap(); + sync_client.import_block(Unverified::from_rlp(client.block(BlockId::Number(i)).unwrap().into_inner()).unwrap()).unwrap(); } sync_client.flush_queue(); assert_eq!(sync_client.chain_info().best_block_number, 3); diff --git a/ethcore/src/engines/validator_set/safe_contract.rs b/ethcore/src/engines/validator_set/safe_contract.rs index a7f4f2c731a..adaa63f0b42 100644 --- a/ethcore/src/engines/validator_set/safe_contract.rs +++ b/ethcore/src/engines/validator_set/safe_contract.rs @@ -458,6 +458,7 @@ mod tests { use test_helpers::{generate_dummy_client_with_spec_and_accounts, generate_dummy_client_with_spec_and_data}; use super::super::ValidatorSet; use super::{ValidatorSafeContract, EVENT_NAME_HASH}; + use verification::queue::kind::blocks::Unverified; #[test] fn fetches_validators() { @@ -530,7 +531,7 @@ mod tests { let sync_client = generate_dummy_client_with_spec_and_data(Spec::new_validator_safe_contract, 0, 0, &[]); sync_client.engine().register_client(Arc::downgrade(&sync_client) as _); for i in 1..4 { - sync_client.import_block(client.block(BlockId::Number(i)).unwrap().into_inner()).unwrap(); + sync_client.import_block(Unverified::from_rlp(client.block(BlockId::Number(i)).unwrap().into_inner()).unwrap()).unwrap(); } sync_client.flush_queue(); assert_eq!(sync_client.chain_info().best_block_number, 3); diff --git a/ethcore/src/json_tests/chain.rs b/ethcore/src/json_tests/chain.rs index 2d643e75fe4..83a940fcb64 100644 --- a/ethcore/src/json_tests/chain.rs +++ b/ethcore/src/json_tests/chain.rs @@ -17,12 +17,12 @@ use std::path::Path; use std::sync::Arc; use client::{EvmTestClient, Client, ClientConfig, ChainInfo, ImportBlock}; -use block::Block; use spec::Genesis; use ethjson; use miner::Miner; use io::IoChannel; use test_helpers; +use verification::queue::kind::blocks::Unverified; use super::HookType; @@ -83,9 +83,9 @@ pub fn json_chain_test(json_data: &[u8], start_stop_ho Arc::new(Miner::new_for_tests(&spec, None)), IoChannel::disconnected(), ).unwrap(); - for b in &blockchain.blocks_rlp() { - if Block::is_good(&b) { - let _ = client.import_block(b.clone()); + for b in blockchain.blocks_rlp() { + if let Ok(block) = Unverified::from_rlp(b) { + let _ = client.import_block(block); client.flush_queue(); client.import_verified_blocks(); } diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 233e28139be..38acf9e1e50 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -1148,7 +1148,7 @@ impl miner::MinerService for Miner { |b| &b.hash() == &block_hash ) { trace!(target: "miner", "Submitted block {}={}={} with seal {:?}", block_hash, b.hash(), b.header().bare_hash(), seal); - b.lock().try_seal(&*self.engine, seal).or_else(|(e, _)| { + b.lock().try_seal(&*self.engine, seal).or_else(|e| { warn!(target: "miner", "Mined solution rejected: {}", e); Err(ErrorKind::PowInvalid.into()) }) diff --git a/ethcore/src/test_helpers.rs b/ethcore/src/test_helpers.rs index 408714571e8..c873db5d14f 100644 --- a/ethcore/src/test_helpers.rs +++ b/ethcore/src/test_helpers.rs @@ -43,6 +43,7 @@ use blooms_db; use kvdb::KeyValueDB; use kvdb_rocksdb; use tempdir::TempDir; +use verification::queue::kind::blocks::Unverified; use encoded; /// Creates test block with corresponding header @@ -175,7 +176,7 @@ pub fn generate_dummy_client_with_spec_accounts_and_data(test_spec: F, accoun let b = b.close_and_lock().unwrap().seal(test_engine, vec![]).unwrap(); - if let Err(e) = client.import_block(b.rlp_bytes()) { + if let Err(e) = client.import_block(Unverified::from_rlp(b.rlp_bytes()).unwrap()) { panic!("error importing block which is valid by definition: {:?}", e); } @@ -211,7 +212,7 @@ pub fn push_blocks_to_client(client: &Arc, timestamp_salt: u64, starting rolling_block_number = rolling_block_number + 1; rolling_timestamp = rolling_timestamp + 10; - if let Err(e) = client.import_block(create_test_block(&header)) { + if let Err(e) = client.import_block(Unverified::from_rlp(create_test_block(&header)).unwrap()) { panic!("error importing block which is valid by definition: {:?}", e); } } @@ -231,7 +232,7 @@ pub fn push_block_with_transactions(client: &Arc, transactions: &[Signed } let b = b.close_and_lock().unwrap().seal(test_engine, vec![]).unwrap(); - if let Err(e) = client.import_block(b.rlp_bytes()) { + if let Err(e) = client.import_block(Unverified::from_rlp(b.rlp_bytes()).unwrap()) { panic!("error importing block which is valid by definition: {:?}", e); } @@ -253,7 +254,7 @@ pub fn get_test_client_with_blocks(blocks: Vec) -> Arc { ).unwrap(); for block in blocks { - if let Err(e) = client.import_block(block) { + if let Err(e) = client.import_block(Unverified::from_rlp(block).unwrap()) { panic!("error importing block which is well-formed: {:?}", e); } } diff --git a/ethcore/src/tests/client.rs b/ethcore/src/tests/client.rs index a7629313d17..4031d30b08e 100644 --- a/ethcore/src/tests/client.rs +++ b/ethcore/src/tests/client.rs @@ -35,9 +35,9 @@ use views::BlockView; use ethkey::KeyPair; use transaction::{PendingTransaction, Transaction, Action, Condition}; use miner::MinerService; -use rlp::{RlpStream, EMPTY_LIST_RLP}; use tempdir::TempDir; use test_helpers; +use verification::queue::kind::blocks::Unverified; #[test] fn imports_from_empty() { @@ -97,7 +97,7 @@ fn imports_good_block() { IoChannel::disconnected(), ).unwrap(); let good_block = get_good_dummy_block(); - if client.import_block(good_block).is_err() { + if client.import_block(Unverified::from_rlp(good_block).unwrap()).is_err() { panic!("error importing block being good by definition"); } client.flush_queue(); @@ -107,24 +107,6 @@ fn imports_good_block() { assert!(!block.into_inner().is_empty()); } -#[test] -fn fails_to_import_block_with_invalid_rlp() { - use error::{BlockImportError, BlockImportErrorKind}; - - let client = generate_dummy_client(6); - let mut rlp = RlpStream::new_list(3); - rlp.append_raw(&EMPTY_LIST_RLP, 1); // empty header - rlp.append_raw(&EMPTY_LIST_RLP, 1); - rlp.append_raw(&EMPTY_LIST_RLP, 1); - let invalid_header_block = rlp.out(); - - match client.import_block(invalid_header_block) { - Err(BlockImportError(BlockImportErrorKind::Decoder(_), _)) => (), // all good - Err(_) => panic!("Should fail with a decoder error"), - Ok(_) => panic!("Should not import block with invalid header"), - } -} - #[test] fn query_none_block() { let db = test_helpers::new_db(); diff --git a/ethcore/src/tests/trace.rs b/ethcore/src/tests/trace.rs index fd7c932cbdc..24ef3780043 100644 --- a/ethcore/src/tests/trace.rs +++ b/ethcore/src/tests/trace.rs @@ -33,6 +33,7 @@ use views::BlockView; use trace::{RewardType, LocalizedTrace}; use trace::trace::Action::Reward; use test_helpers; +use verification::queue::kind::blocks::Unverified; #[test] fn can_trace_block_and_uncle_reward() { @@ -91,7 +92,7 @@ fn can_trace_block_and_uncle_reward() { let root_block = root_block.close_and_lock().unwrap().seal(engine, vec![]).unwrap(); - if let Err(e) = client.import_block(root_block.rlp_bytes()) { + if let Err(e) = client.import_block(Unverified::from_rlp(root_block.rlp_bytes()).unwrap()) { panic!("error importing block which is valid by definition: {:?}", e); } @@ -120,7 +121,7 @@ fn can_trace_block_and_uncle_reward() { let parent_block = parent_block.close_and_lock().unwrap().seal(engine, vec![]).unwrap(); - if let Err(e) = client.import_block(parent_block.rlp_bytes()) { + if let Err(e) = client.import_block(Unverified::from_rlp(parent_block.rlp_bytes()).unwrap()) { panic!("error importing block which is valid by definition: {:?}", e); } @@ -170,7 +171,7 @@ fn can_trace_block_and_uncle_reward() { let block = block.close_and_lock().unwrap().seal(engine, vec![]).unwrap(); - let res = client.import_block(block.rlp_bytes()); + let res = client.import_block(Unverified::from_rlp(block.rlp_bytes()).unwrap()); if res.is_err() { panic!("error importing block: {:#?}", res.err().unwrap()); } diff --git a/ethcore/sync/src/block_sync.rs b/ethcore/sync/src/block_sync.rs index cc04fb04d73..588bfc0c711 100644 --- a/ethcore/sync/src/block_sync.rs +++ b/ethcore/sync/src/block_sync.rs @@ -23,11 +23,10 @@ use std::cmp; use heapsize::HeapSizeOf; use ethereum_types::H256; use rlp::{self, Rlp}; -use ethcore::views::BlockView; use ethcore::header::{BlockNumber, Header as BlockHeader}; use ethcore::client::{BlockStatus, BlockId, BlockImportError, BlockImportErrorKind}; -use ethcore::block::Block; use ethcore::error::{ImportErrorKind, BlockError}; +use ethcore::verification::queue::kind::blocks::Unverified; use sync_io::SyncIo; use blocks::BlockCollection; @@ -484,18 +483,19 @@ impl BlockDownloader { let block = block_and_receipts.block; let receipts = block_and_receipts.receipts; - // Perform basic block verification - if !Block::is_good(&block) { - debug!(target: "sync", "Bad block rlp: {:?}", block); - bad = true; - break; - } - - let (h, number, parent) = { - let header = view!(BlockView, &block).header_view(); - (header.hash(), header.number(), header.parent_hash()) + let block = match Unverified::from_rlp(block) { + Ok(block) => block, + Err(_) => { + debug!(target: "sync", "Bad block rlp"); + bad = true; + break; + } }; + let h = block.header.hash(); + let number = block.header.number(); + let parent = *block.header.parent_hash(); + if self.target_hash.as_ref().map_or(false, |t| t == &h) { self.state = State::Complete; trace!(target: "sync", "Sync target reached"); diff --git a/ethcore/sync/src/blocks.rs b/ethcore/sync/src/blocks.rs index df7d7a3bfde..248180b281d 100644 --- a/ethcore/sync/src/blocks.rs +++ b/ethcore/sync/src/blocks.rs @@ -294,7 +294,7 @@ impl BlockCollection { let header = view!(HeaderView, &block.header); let block_view = Block::new_from_header_and_body(&header, &body); drained.push(BlockAndReceipts { - block: block_view.rlp().as_raw().to_vec(), + block: block_view.into_inner(), receipts: block.receipts.clone(), }); } diff --git a/ethcore/sync/src/chain/handler.rs b/ethcore/sync/src/chain/handler.rs index 136ff3395a1..8547be7b3ff 100644 --- a/ethcore/sync/src/chain/handler.rs +++ b/ethcore/sync/src/chain/handler.rs @@ -19,8 +19,9 @@ use block_sync::{BlockDownloaderImportError as DownloaderImportError, DownloadAc use bytes::Bytes; use ethcore::client::{BlockStatus, BlockId, BlockImportError, BlockImportErrorKind}; use ethcore::error::*; -use ethcore::header::{BlockNumber, Header as BlockHeader}; +use ethcore::header::BlockNumber; use ethcore::snapshot::{ManifestData, RestorationStatus}; +use ethcore::verification::queue::kind::blocks::Unverified; use ethereum_types::{H256, U256}; use hash::keccak; use network::PeerId; @@ -162,44 +163,43 @@ impl SyncHandler { peer.difficulty = Some(difficulty); } } - let block_rlp = r.at(0)?; - let header_rlp = block_rlp.at(0)?; - let h = keccak(&header_rlp.as_raw()); - trace!(target: "sync", "{} -> NewBlock ({})", peer_id, h); - let header: BlockHeader = header_rlp.as_val()?; - if header.number() > sync.highest_block.unwrap_or(0) { - sync.highest_block = Some(header.number()); + let block = Unverified::from_rlp(r.at(0)?.as_raw().to_vec())?; + let hash = block.header.hash(); + let number = block.header.number(); + trace!(target: "sync", "{} -> NewBlock ({})", peer_id, hash); + if number > sync.highest_block.unwrap_or(0) { + sync.highest_block = Some(number); } let mut unknown = false; - { - if let Some(ref mut peer) = sync.peers.get_mut(&peer_id) { - peer.latest_hash = header.hash(); - } + + if let Some(ref mut peer) = sync.peers.get_mut(&peer_id) { + peer.latest_hash = hash; } + let last_imported_number = sync.new_blocks.last_imported_block_number(); - if last_imported_number > header.number() && last_imported_number - header.number() > MAX_NEW_BLOCK_AGE { - trace!(target: "sync", "Ignored ancient new block {:?}", h); + if last_imported_number > number && last_imported_number - number > MAX_NEW_BLOCK_AGE { + trace!(target: "sync", "Ignored ancient new block {:?}", hash); return Err(DownloaderImportError::Invalid); } - match io.chain().import_block(block_rlp.as_raw().to_vec()) { + match io.chain().import_block(block) { Err(BlockImportError(BlockImportErrorKind::Import(ImportErrorKind::AlreadyInChain), _)) => { - trace!(target: "sync", "New block already in chain {:?}", h); + trace!(target: "sync", "New block already in chain {:?}", hash); }, Err(BlockImportError(BlockImportErrorKind::Import(ImportErrorKind::AlreadyQueued), _)) => { - trace!(target: "sync", "New block already queued {:?}", h); + trace!(target: "sync", "New block already queued {:?}", hash); }, Ok(_) => { // abort current download of the same block sync.complete_sync(io); - sync.new_blocks.mark_as_known(&header.hash(), header.number()); - trace!(target: "sync", "New block queued {:?} ({})", h, header.number()); + sync.new_blocks.mark_as_known(&hash, number); + trace!(target: "sync", "New block queued {:?} ({})", hash, number); }, Err(BlockImportError(BlockImportErrorKind::Block(BlockError::UnknownParent(p)), _)) => { unknown = true; - trace!(target: "sync", "New block with unknown parent ({:?}) {:?}", p, h); + trace!(target: "sync", "New block with unknown parent ({:?}) {:?}", p, hash); }, Err(e) => { - debug!(target: "sync", "Bad new block {:?} : {:?}", h, e); + debug!(target: "sync", "Bad new block {:?} : {:?}", hash, e); return Err(DownloaderImportError::Invalid); } }; @@ -207,7 +207,7 @@ impl SyncHandler { if sync.state != SyncState::Idle { trace!(target: "sync", "NewBlock ignored while seeking"); } else { - trace!(target: "sync", "New unknown block {:?}", h); + trace!(target: "sync", "New unknown block {:?}", hash); //TODO: handle too many unknown blocks sync.sync_peer(io, peer_id, true); } diff --git a/parity/blockchain.rs b/parity/blockchain.rs index 21af2968ef5..cc92419dabf 100644 --- a/parity/blockchain.rs +++ b/parity/blockchain.rs @@ -30,6 +30,7 @@ use ethcore::client::{Mode, DatabaseCompactionProfile, VMType, BlockImportError, use ethcore::error::{ImportErrorKind, BlockImportErrorKind}; use ethcore::miner::Miner; use ethcore::verification::queue::VerifierSettings; +use ethcore::verification::queue::kind::blocks::Unverified; use ethcore_service::ClientService; use cache::CacheConfig; use informant::{Informant, FullNodeInformantData, MillisecondDuration}; @@ -417,8 +418,9 @@ fn execute_import(cmd: ImportBlockchain) -> Result<(), String> { service.register_io_handler(informant).map_err(|_| "Unable to register informant handler".to_owned())?; let do_import = |bytes| { + let block = Unverified::from_rlp(bytes).map_err(|_| "Invalid block rlp")?; while client.queue_info().is_full() { sleep(Duration::from_secs(1)); } - match client.import_block(bytes) { + match client.import_block(block) { Err(BlockImportError(BlockImportErrorKind::Import(ImportErrorKind::AlreadyInChain), _)) => { trace!("Skipping block already in chain."); } diff --git a/rpc/src/lib.rs b/rpc/src/lib.rs index 96926700c06..2e731cd3475 100644 --- a/rpc/src/lib.rs +++ b/rpc/src/lib.rs @@ -43,7 +43,6 @@ extern crate jsonrpc_ipc_server as ipc; extern crate jsonrpc_pubsub; extern crate ethash; -#[cfg_attr(test, macro_use)] extern crate ethcore; extern crate parity_bytes as bytes; extern crate parity_crypto as crypto; diff --git a/rpc/src/v1/tests/eth.rs b/rpc/src/v1/tests/eth.rs index 6e3b9855d65..217e032902c 100644 --- a/rpc/src/v1/tests/eth.rs +++ b/rpc/src/v1/tests/eth.rs @@ -20,14 +20,13 @@ use std::sync::Arc; use ethereum_types::{H256, Address}; use ethcore::account_provider::AccountProvider; -use ethcore::block::Block; use ethcore::client::{BlockChainClient, Client, ClientConfig, ChainInfo, ImportBlock}; use ethcore::ethereum; use ethcore::ids::BlockId; use ethcore::miner::Miner; use ethcore::spec::{Genesis, Spec}; use ethcore::test_helpers; -use ethcore::views::BlockView; +use ethcore::verification::queue::kind::blocks::Unverified; use ethjson::blockchain::BlockChain; use ethjson::state::test::ForkSpec; use io::IoChannel; @@ -85,9 +84,9 @@ impl EthTester { fn from_chain(chain: &BlockChain) -> Self { let tester = Self::from_spec(make_spec(chain)); - for b in &chain.blocks_rlp() { - if Block::is_good(&b) { - let _ = tester.client.import_block(b.clone()); + for b in chain.blocks_rlp() { + if let Ok(block) = Unverified::from_rlp(b) { + let _ = tester.client.import_block(block); tester.client.flush_queue(); tester.client.import_verified_blocks(); } @@ -423,11 +422,11 @@ fn verify_transaction_counts(name: String, chain: BlockChain) { let tester = EthTester::from_chain(&chain); let mut id = 1; - for b in chain.blocks_rlp().iter().filter(|b| Block::is_good(b)).map(|b| view!(BlockView, b)) { - let count = b.transactions_count(); + for b in chain.blocks_rlp().into_iter().filter_map(|b| Unverified::from_rlp(b).ok()) { + let count = b.transactions.len(); - let hash = b.hash(); - let number = b.header_view().number(); + let hash = b.header.hash(); + let number = b.header.number(); let (req, res) = by_hash(hash, count, &mut id); assert_eq!(tester.handler.handle_request_sync(&req), Some(res)); From 90d7823acb3ff1e46520a5bae5c0c170f3b69b7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 2 Aug 2018 12:58:02 +0200 Subject: [PATCH 6/6] Propagate transactions for next 4 blocks. (#9265) Closes #9255 This PR also removes the limit of max 64 transactions per packet, currently we only attempt to prevent the packet size to go over 8MB. This will only be the case for super-large transactions or high-block-gas-limit chains. Patching this is important only for chains that have blocks that can fit more than 4k transactions (over 86M block gas limit) For mainnet, we should actually see a tiny bit faster propagation since instead of computing 4k pending set, we only need `4 * 8M / 21k = 1523` transactions. Running some tests on `dekompile` node right now, to check how it performs in the wild. --- ethcore/light/src/net/mod.rs | 5 +---- ethcore/light/src/net/tests/mod.rs | 4 ++-- ethcore/light/src/provider.rs | 14 ++++++-------- ethcore/src/client/client.rs | 21 ++++++++++++++++++++- ethcore/src/client/test_client.rs | 4 ++-- ethcore/src/client/traits.rs | 4 ++-- ethcore/src/tests/client.rs | 4 ++-- ethcore/sync/src/chain/mod.rs | 6 ------ ethcore/sync/src/chain/propagator.rs | 7 ++----- 9 files changed, 37 insertions(+), 32 deletions(-) diff --git a/ethcore/light/src/net/mod.rs b/ethcore/light/src/net/mod.rs index 27d240feda9..5e73681a55a 100644 --- a/ethcore/light/src/net/mod.rs +++ b/ethcore/light/src/net/mod.rs @@ -70,9 +70,6 @@ const PROPAGATE_TIMEOUT_INTERVAL: Duration = Duration::from_secs(5); const RECALCULATE_COSTS_TIMEOUT: TimerToken = 3; const RECALCULATE_COSTS_INTERVAL: Duration = Duration::from_secs(60 * 60); -/// Max number of transactions in a single packet. -const MAX_TRANSACTIONS_TO_PROPAGATE: usize = 64; - // minimum interval between updates. const UPDATE_INTERVAL: Duration = Duration::from_millis(5000); @@ -648,7 +645,7 @@ impl LightProtocol { fn propagate_transactions(&self, io: &IoContext) { if self.capabilities.read().tx_relay { return } - let ready_transactions = self.provider.ready_transactions(MAX_TRANSACTIONS_TO_PROPAGATE); + let ready_transactions = self.provider.transactions_to_propagate(); if ready_transactions.is_empty() { return } trace!(target: "pip", "propagate transactions: {} ready", ready_transactions.len()); diff --git a/ethcore/light/src/net/tests/mod.rs b/ethcore/light/src/net/tests/mod.rs index 0c9971d75bd..6bc6751b1cd 100644 --- a/ethcore/light/src/net/tests/mod.rs +++ b/ethcore/light/src/net/tests/mod.rs @@ -171,8 +171,8 @@ impl Provider for TestProvider { }) } - fn ready_transactions(&self, max_len: usize) -> Vec { - self.0.client.ready_transactions(max_len) + fn transactions_to_propagate(&self) -> Vec { + self.0.client.transactions_to_propagate() } } diff --git a/ethcore/light/src/provider.rs b/ethcore/light/src/provider.rs index 84e7b728394..a066cefb529 100644 --- a/ethcore/light/src/provider.rs +++ b/ethcore/light/src/provider.rs @@ -128,7 +128,7 @@ pub trait Provider: Send + Sync { fn header_proof(&self, req: request::CompleteHeaderProofRequest) -> Option; /// Provide pending transactions. - fn ready_transactions(&self, max_len: usize) -> Vec; + fn transactions_to_propagate(&self) -> Vec; /// Provide a proof-of-execution for the given transaction proof request. /// Returns a vector of all state items necessary to execute the transaction. @@ -283,8 +283,8 @@ impl Provider for T { .map(|(_, proof)| ::request::ExecutionResponse { items: proof }) } - fn ready_transactions(&self, max_len: usize) -> Vec { - BlockChainClient::ready_transactions(self, max_len) + fn transactions_to_propagate(&self) -> Vec { + BlockChainClient::transactions_to_propagate(self) .into_iter() .map(|tx| tx.pending().clone()) .collect() @@ -370,12 +370,10 @@ impl Provider for LightProvider { None } - fn ready_transactions(&self, max_len: usize) -> Vec { + fn transactions_to_propagate(&self) -> Vec { let chain_info = self.chain_info(); - let mut transactions = self.txqueue.read() - .ready_transactions(chain_info.best_block_number, chain_info.best_block_timestamp); - transactions.truncate(max_len); - transactions + self.txqueue.read() + .ready_transactions(chain_info.best_block_number, chain_info.best_block_timestamp) } } diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index 02af5ff0ad8..3a7fd0d0c74 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -15,6 +15,7 @@ // along with Parity. If not, see . use std::collections::{HashSet, BTreeMap, VecDeque}; +use std::cmp; use std::fmt; use std::str::FromStr; use std::sync::atomic::{AtomicUsize, AtomicBool, Ordering as AtomicOrdering}; @@ -1945,7 +1946,25 @@ impl BlockChainClient for Client { (*self.build_last_hashes(&self.chain.read().best_block_hash())).clone() } - fn ready_transactions(&self, max_len: usize) -> Vec> { + fn transactions_to_propagate(&self) -> Vec> { + const PROPAGATE_FOR_BLOCKS: u32 = 4; + const MIN_TX_TO_PROPAGATE: usize = 256; + + let block_gas_limit = *self.best_block_header().gas_limit(); + let min_tx_gas: U256 = self.latest_schedule().tx_gas.into(); + + let max_len = if min_tx_gas.is_zero() { + usize::max_value() + } else { + cmp::max( + MIN_TX_TO_PROPAGATE, + cmp::min( + (block_gas_limit / min_tx_gas) * PROPAGATE_FOR_BLOCKS, + // never more than usize + usize::max_value().into() + ).as_u64() as usize + ) + }; self.importer.miner.ready_transactions(self, max_len, ::miner::PendingOrdering::Priority) } diff --git a/ethcore/src/client/test_client.rs b/ethcore/src/client/test_client.rs index 627d844aeb6..f729b15b7fb 100644 --- a/ethcore/src/client/test_client.rs +++ b/ethcore/src/client/test_client.rs @@ -807,8 +807,8 @@ impl BlockChainClient for TestBlockChainClient { self.traces.read().clone() } - fn ready_transactions(&self, max_len: usize) -> Vec> { - self.miner.ready_transactions(self, max_len, miner::PendingOrdering::Priority) + fn transactions_to_propagate(&self) -> Vec> { + self.miner.ready_transactions(self, 4096, miner::PendingOrdering::Priority) } fn signing_chain_id(&self) -> Option { None } diff --git a/ethcore/src/client/traits.rs b/ethcore/src/client/traits.rs index 3f5595f6193..6ccba5e0f81 100644 --- a/ethcore/src/client/traits.rs +++ b/ethcore/src/client/traits.rs @@ -321,8 +321,8 @@ pub trait BlockChainClient : Sync + Send + AccountData + BlockChain + CallContra /// Get last hashes starting from best block. fn last_hashes(&self) -> LastHashes; - /// List all transactions that are allowed into the next block. - fn ready_transactions(&self, max_len: usize) -> Vec>; + /// List all ready transactions that should be propagated to other peers. + fn transactions_to_propagate(&self) -> Vec>; /// Sorted list of transaction gas prices from at least last sample_size blocks. fn gas_price_corpus(&self, sample_size: usize) -> ::stats::Corpus { diff --git a/ethcore/src/tests/client.rs b/ethcore/src/tests/client.rs index 4031d30b08e..24801cb5756 100644 --- a/ethcore/src/tests/client.rs +++ b/ethcore/src/tests/client.rs @@ -316,11 +316,11 @@ fn does_not_propagate_delayed_transactions() { client.miner().import_own_transaction(&*client, tx0).unwrap(); client.miner().import_own_transaction(&*client, tx1).unwrap(); - assert_eq!(0, client.ready_transactions(10).len()); + assert_eq!(0, client.transactions_to_propagate().len()); assert_eq!(0, client.miner().ready_transactions(&*client, 10, PendingOrdering::Priority).len()); push_blocks_to_client(&client, 53, 2, 2); client.flush_queue(); - assert_eq!(2, client.ready_transactions(10).len()); + assert_eq!(2, client.transactions_to_propagate().len()); assert_eq!(2, client.miner().ready_transactions(&*client, 10, PendingOrdering::Priority).len()); } diff --git a/ethcore/sync/src/chain/mod.rs b/ethcore/sync/src/chain/mod.rs index ef5146f91da..520226a9c7e 100644 --- a/ethcore/sync/src/chain/mod.rs +++ b/ethcore/sync/src/chain/mod.rs @@ -149,12 +149,6 @@ const MAX_NEW_HASHES: usize = 64; const MAX_NEW_BLOCK_AGE: BlockNumber = 20; // maximal packet size with transactions (cannot be greater than 16MB - protocol limitation). const MAX_TRANSACTION_PACKET_SIZE: usize = 8 * 1024 * 1024; -// Maximal number of transactions queried from miner to propagate. -// This set is used to diff with transactions known by the peer and -// we will send a difference of length up to `MAX_TRANSACTIONS_TO_PROPAGATE`. -const MAX_TRANSACTIONS_TO_QUERY: usize = 4096; -// Maximal number of transactions in sent in single packet. -const MAX_TRANSACTIONS_TO_PROPAGATE: usize = 64; // Min number of blocks to be behind for a snapshot sync const SNAPSHOT_RESTORE_THRESHOLD: BlockNumber = 30000; const SNAPSHOT_MIN_PEERS: usize = 3; diff --git a/ethcore/sync/src/chain/propagator.rs b/ethcore/sync/src/chain/propagator.rs index ef5e700bfe0..7cb145f3626 100644 --- a/ethcore/sync/src/chain/propagator.rs +++ b/ethcore/sync/src/chain/propagator.rs @@ -29,11 +29,9 @@ use transaction::SignedTransaction; use super::{ random, ChainSync, + MAX_TRANSACTION_PACKET_SIZE, MAX_PEER_LAG_PROPAGATION, MAX_PEERS_PROPAGATION, - MAX_TRANSACTION_PACKET_SIZE, - MAX_TRANSACTIONS_TO_PROPAGATE, - MAX_TRANSACTIONS_TO_QUERY, MIN_PEERS_PROPAGATION, CONSENSUS_DATA_PACKET, NEW_BLOCK_HASHES_PACKET, @@ -121,7 +119,7 @@ impl SyncPropagator { return 0; } - let transactions = io.chain().ready_transactions(MAX_TRANSACTIONS_TO_QUERY); + let transactions = io.chain().transactions_to_propagate(); if transactions.is_empty() { return 0; } @@ -184,7 +182,6 @@ impl SyncPropagator { // Get hashes of all transactions to send to this peer let to_send = all_transactions_hashes.difference(&peer_info.last_sent_transactions) - .take(MAX_TRANSACTIONS_TO_PROPAGATE) .cloned() .collect::>(); if to_send.is_empty() {