From 856a0755888a30d4dc52a68cb2638a8bfd5786ac Mon Sep 17 00:00:00 2001 From: David Date: Fri, 14 Feb 2020 19:01:17 +0100 Subject: [PATCH] Include the seal when populating the header for a new block (#11475) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * debug signer * Don't panic if empty_steps_transition already happened Before this the `header_empty_steps_raw` would panic if the chain has already progressed beyond the block number set in `emptyStepsTransition`. As this is a user accessible configuration option I don't think we should panic. * Cleanup some code in Aura Nothing really interesting here, renames or removes some methods. Adds some docs and extends a test a bit to clarify the behaviour of the code. * Include the seal when populating the header for a new block (fixes #11445) * cleanup * cleanup2 * Review grumbles * Update ethcore/engines/authority-round/src/lib.rs Co-Authored-By: André Silva * Update ethcore/engines/authority-round/src/lib.rs Co-Authored-By: André Silva * Update ethcore/src/block.rs Co-Authored-By: André Silva Co-authored-by: André Silva --- ethcore/engines/authority-round/src/lib.rs | 34 ++++++++++++++-------- ethcore/src/block.rs | 6 +++- parity/run.rs | 2 ++ 3 files changed, 29 insertions(+), 13 deletions(-) diff --git a/ethcore/engines/authority-round/src/lib.rs b/ethcore/engines/authority-round/src/lib.rs index 18978d7af..69ac2075e 100644 --- a/ethcore/engines/authority-round/src/lib.rs +++ b/ethcore/engines/authority-round/src/lib.rs @@ -720,17 +720,22 @@ fn header_signature(header: &Header, empty_steps_transition: u64) -> Result().map(Into::into) } -// extracts the raw empty steps vec from the header seal. should only be called when there are 3 fields in the seal -// (i.e. header.number() >= self.empty_steps_transition) -fn header_empty_steps_raw(header: &Header) -> &[u8] { - header.seal().get(2).expect("was checked with verify_block_basic; has 3 fields; qed") +// Extracts the RLP bytes of the empty steps from the header seal. Returns data only when there are +// 3 fields in the seal. (i.e. header.number() >= self.empty_steps_transition). +fn header_empty_steps_raw(header: &Header) -> Option<&[u8]> { + header.seal().get(2).map(Vec::as_slice) } -// extracts the empty steps from the header seal. should only be called when there are 3 fields in the seal +// Extracts the empty steps from the header seal. Returns data only when there are 3 fields in the seal // (i.e. header.number() >= self.empty_steps_transition). fn header_empty_steps(header: &Header) -> Result, ::rlp::DecoderError> { - let empty_steps = Rlp::new(header_empty_steps_raw(header)).as_list::()?; - Ok(empty_steps.into_iter().map(|s| EmptyStep::from_sealed(s, header.parent_hash())).collect()) + header_empty_steps_raw(header).map_or(Ok(vec![]), |raw| { + let empty_steps = Rlp::new(raw).as_list::()?; + Ok(empty_steps + .into_iter() + .map(|s| EmptyStep::from_sealed(s, header.parent_hash())) + .collect()) + }) } // gets the signers of empty step messages for the given header, does not include repeated signers @@ -788,7 +793,7 @@ fn verify_external(header: &Header, validators: &dyn ValidatorSet, empty_steps_t let correct_proposer = validators.get(header.parent_hash(), header_step as usize); let is_invalid_proposer = *header.author() != correct_proposer || { let empty_steps_rlp = if header.number() >= empty_steps_transition { - Some(header_empty_steps_raw(header)) + header_empty_steps_raw(header) } else { None }; @@ -970,11 +975,15 @@ impl AuthorityRound { /// Drops all `EmptySteps` less than or equal to the passed `step`, irregardless of the parent hash. fn clear_empty_steps(&self, step: u64) { let mut empty_steps = self.empty_steps.lock(); - *empty_steps = empty_steps.split_off(&EmptyStep { + if empty_steps.is_empty() { + return; + } + let next_empty_steps = empty_steps.split_off(&EmptyStep { step: step + 1, parent_hash: Default::default(), signature: Default::default(), }); + *empty_steps = next_empty_steps } fn store_empty_step(&self, empty_step: EmptyStep) { @@ -3013,9 +3022,10 @@ mod tests { let params = AuthorityRoundParams::from(deserialized.params); for ((block_num1, address1), (block_num2, address2)) in params.block_reward_contract_transitions.iter().zip( - [(0u64, BlockRewardContract::new_from_address(Address::from_str("2000000000000000000000000000000000000002").unwrap())), - (7u64, BlockRewardContract::new_from_address(Address::from_str("3000000000000000000000000000000000000003").unwrap())), - (42u64, BlockRewardContract::new_from_address(Address::from_str("4000000000000000000000000000000000000004").unwrap())), + [ + (0u64, BlockRewardContract::new_from_address(Address::from_str("2000000000000000000000000000000000000002").unwrap())), + (7u64, BlockRewardContract::new_from_address(Address::from_str("3000000000000000000000000000000000000003").unwrap())), + (42u64, BlockRewardContract::new_from_address(Address::from_str("4000000000000000000000000000000000000004").unwrap())), ].iter()) { assert_eq!(block_num1, block_num2); diff --git a/ethcore/src/block.rs b/ethcore/src/block.rs index 381e21875..06127a77e 100644 --- a/ethcore/src/block.rs +++ b/ethcore/src/block.rs @@ -222,6 +222,10 @@ impl<'x> OpenBlock<'x> { self.block.header.set_timestamp(header.timestamp()); self.block.header.set_uncles_hash(*header.uncles_hash()); self.block.header.set_transactions_root(*header.transactions_root()); + // For Aura-based chains, the seal may contain EmptySteps which are used to bestow rewards; + // such rewards affect the state and the state root (see + // https://github.com/paritytech/parity-ethereum/pull/11475). + self.block.header.set_seal(header.seal().to_vec()); // TODO: that's horrible. set only for backwards compatibility if header.extra_data().len() > self.engine.maximum_extra_data_size() { warn!("Couldn't set extradata. Ignoring."); @@ -554,7 +558,7 @@ mod tests { b.close_and_lock() } - /// Enact the block given by `block_bytes` using `engine` on the database `db` with given `parent` block header. Seal the block aferwards + /// Enact the block given by `block_bytes` using `engine` on the database `db` with given `parent` block header. Seal the block afterwards fn enact_and_seal( block_bytes: Vec, engine: &dyn Engine, diff --git a/parity/run.rs b/parity/run.rs index 5e1d43d6e..ad69e3df8 100644 --- a/parity/run.rs +++ b/parity/run.rs @@ -492,6 +492,7 @@ fn execute_impl( let fetch = fetch::Client::new(FETCH_FULL_NUM_DNS_THREADS).map_err(|e| format!("Error starting fetch client: {:?}", e))?; let txpool_size = cmd.miner_options.pool_limits.max_count; + // create miner let miner = Arc::new(Miner::new( cmd.miner_options, @@ -502,6 +503,7 @@ fn execute_impl( account_utils::miner_local_accounts(account_provider.clone()), ) )); + miner.set_author(miner::Author::External(cmd.miner_extras.author)); miner.set_gas_range_target(cmd.miner_extras.gas_range_target); miner.set_extra_data(cmd.miner_extras.extra_data);