From 32c32ecfdac7787ab8d4271b5245661bf253d471 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Mon, 7 May 2018 11:57:03 +0100 Subject: [PATCH] ethcore, rpc, machine: refactor block reward application and tracing (#8490) --- ethcore/src/engines/authority_round/mod.rs | 9 ++++-- ethcore/src/engines/block_reward.rs | 34 +++++++++++++++++----- ethcore/src/engines/mod.rs | 2 +- ethcore/src/engines/null_engine.rs | 20 +++++-------- ethcore/src/engines/tendermint/mod.rs | 10 +++++-- ethcore/src/ethereum/ethash.rs | 25 ++++++++-------- ethcore/src/machine.rs | 24 ++++++++++----- ethcore/src/trace/types/trace.rs | 8 +++++ machine/src/lib.rs | 8 ----- rpc/src/v1/types/trace.rs | 8 +++++ 10 files changed, 93 insertions(+), 55 deletions(-) diff --git a/ethcore/src/engines/authority_round/mod.rs b/ethcore/src/engines/authority_round/mod.rs index 4807d6c3f..c2aee7c6e 100644 --- a/ethcore/src/engines/authority_round/mod.rs +++ b/ethcore/src/engines/authority_round/mod.rs @@ -1015,8 +1015,10 @@ impl Engine for AuthorityRound { let author = *block.header().author(); benefactors.push((author, RewardKind::Author)); - let rewards = match self.block_reward_contract { + let rewards: Vec<_> = match self.block_reward_contract { Some(ref c) if block.header().number() >= self.block_reward_contract_transition => { + // NOTE: this logic should be moved to a function when another + // engine needs support for block reward contract. let mut call = |to, data| { let result = self.machine.execute_as_system( block, @@ -1027,10 +1029,11 @@ impl Engine for AuthorityRound { result.map_err(|e| format!("{}", e)) }; - c.reward(&benefactors, &mut call)? + let rewards = c.reward(&benefactors, &mut call)?; + rewards.into_iter().map(|(author, amount)| (author, RewardKind::External, amount)).collect() }, _ => { - benefactors.into_iter().map(|(author, _)| (author, self.block_reward)).collect() + benefactors.into_iter().map(|(author, reward_kind)| (author, reward_kind, self.block_reward)).collect() }, }; diff --git a/ethcore/src/engines/block_reward.rs b/ethcore/src/engines/block_reward.rs index 510a5255f..9a9d54e4a 100644 --- a/ethcore/src/engines/block_reward.rs +++ b/ethcore/src/engines/block_reward.rs @@ -14,13 +14,17 @@ // You should have received a copy of the GNU General Public License // along with Parity. If not, see . +//! A module with types for declaring block rewards and a client interface for interacting with a +//! block reward contract. + use ethabi; use ethabi::ParamType; use ethereum_types::{H160, Address, U256}; -use block::ExecutedBlock; use error::Error; -use machine::EthereumMachine; +use machine::WithRewards; +use parity_machine::{Machine, WithBalances}; +use trace; use super::SystemCall; use_contract!(block_reward_contract, "BlockReward", "res/contracts/block_reward.json"); @@ -37,6 +41,8 @@ pub enum RewardKind { Uncle = 1, /// Reward attributed to the author(s) of empty step(s) included in the block (AuthorityRound engine). EmptyStep = 2, + /// Reward attributed by an external protocol (e.g. block reward contract). + External = 3, } impl From for u16 { @@ -45,6 +51,17 @@ impl From for u16 { } } +impl Into for RewardKind { + fn into(self) -> trace::RewardType { + match self { + RewardKind::Author => trace::RewardType::Block, + RewardKind::Uncle => trace::RewardType::Uncle, + RewardKind::EmptyStep => trace::RewardType::EmptyStep, + RewardKind::External => trace::RewardType::External, + } + } +} + /// A client for the block reward contract. pub struct BlockRewardContract { /// Address of the contract. @@ -112,14 +129,17 @@ impl BlockRewardContract { /// Applies the given block rewards, i.e. adds the given balance to each benefactors' address. /// If tracing is enabled the operations are recorded. -pub fn apply_block_rewards(rewards: &[(Address, U256)], block: &mut ExecutedBlock, machine: &EthereumMachine) -> Result<(), Error> { - use parity_machine::WithBalances; - - for &(ref author, ref block_reward) in rewards { +pub fn apply_block_rewards( + rewards: &[(Address, RewardKind, U256)], + block: &mut M::LiveBlock, + machine: &M, +) -> Result<(), M::Error> { + for &(ref author, _, ref block_reward) in rewards { machine.add_balance(block, author, block_reward)?; } - machine.note_rewards(block, &rewards, &[]) + let rewards: Vec<_> = rewards.into_iter().map(|&(a, k, r)| (a, k.into(), r)).collect(); + machine.note_rewards(block, &rewards) } #[cfg(test)] diff --git a/ethcore/src/engines/mod.rs b/ethcore/src/engines/mod.rs index a17ae356e..e019636f5 100644 --- a/ethcore/src/engines/mod.rs +++ b/ethcore/src/engines/mod.rs @@ -18,7 +18,6 @@ mod authority_round; mod basic_authority; -mod block_reward; mod instant_seal; mod null_engine; mod signer; @@ -27,6 +26,7 @@ mod transition; mod validator_set; mod vote_collector; +pub mod block_reward; pub mod epoch; pub use self::authority_round::AuthorityRound; diff --git a/ethcore/src/engines/null_engine.rs b/ethcore/src/engines/null_engine.rs index f20a9cdfd..278eb037c 100644 --- a/ethcore/src/engines/null_engine.rs +++ b/ethcore/src/engines/null_engine.rs @@ -16,7 +16,9 @@ use ethereum_types::U256; use engines::Engine; +use engines::block_reward::{self, RewardKind}; use header::BlockNumber; +use machine::WithRewards; use parity_machine::{Header, LiveBlock, WithBalances}; /// Params for a null engine. @@ -56,7 +58,7 @@ impl Default for NullEngine { } } -impl Engine for NullEngine { +impl Engine for NullEngine { fn name(&self) -> &str { "NullEngine" } @@ -74,26 +76,20 @@ impl Engine for NullEngine { let n_uncles = LiveBlock::uncles(&*block).len(); + let mut rewards = Vec::new(); + // Bestow block reward let result_block_reward = reward + reward.shr(5) * U256::from(n_uncles); - let mut uncle_rewards = Vec::with_capacity(n_uncles); - - self.machine.add_balance(block, &author, &result_block_reward)?; + rewards.push((author, RewardKind::Author, result_block_reward)); // bestow uncle rewards. for u in LiveBlock::uncles(&*block) { let uncle_author = u.author(); let result_uncle_reward = (reward * U256::from(8 + u.number() - number)).shr(3); - - uncle_rewards.push((*uncle_author, result_uncle_reward)); + rewards.push((*uncle_author, RewardKind::Uncle, result_uncle_reward)); } - for &(ref a, ref reward) in &uncle_rewards { - self.machine.add_balance(block, a, reward)?; - } - - // note and trace. - self.machine.note_rewards(block, &[(author, result_block_reward)], &uncle_rewards) + block_reward::apply_block_rewards(&rewards, block, &self.machine) } fn maximum_uncle_count(&self, _block: BlockNumber) -> usize { 2 } diff --git a/ethcore/src/engines/tendermint/mod.rs b/ethcore/src/engines/tendermint/mod.rs index 289beaad0..d80a5e182 100644 --- a/ethcore/src/engines/tendermint/mod.rs +++ b/ethcore/src/engines/tendermint/mod.rs @@ -41,6 +41,7 @@ use ethkey::{self, Message, Signature}; use account_provider::AccountProvider; use block::*; use engines::{Engine, Seal, EngineError, ConstructedVerifier}; +use engines::block_reward::{self, RewardKind}; use io::IoService; use super::signer::EngineSigner; use super::validator_set::{ValidatorSet, SimpleList}; @@ -550,10 +551,13 @@ impl Engine for Tendermint { /// Apply the block reward on finalisation of the block. fn on_close_block(&self, block: &mut ExecutedBlock) -> Result<(), Error>{ - use parity_machine::WithBalances; let author = *block.header().author(); - self.machine.add_balance(block, &author, &self.block_reward)?; - self.machine.note_rewards(block, &[(author, self.block_reward)], &[]) + + block_reward::apply_block_rewards( + &[(author, RewardKind::Author, self.block_reward)], + block, + &self.machine, + ) } fn verify_local_seal(&self, _header: &Header) -> Result<(), Error> { diff --git a/ethcore/src/ethereum/ethash.rs b/ethcore/src/ethereum/ethash.rs index 09e9caf72..09151415c 100644 --- a/ethcore/src/ethereum/ethash.rs +++ b/ethcore/src/ethereum/ethash.rs @@ -19,6 +19,7 @@ use std::cmp; use std::collections::BTreeMap; use std::sync::Arc; use hash::{KECCAK_EMPTY_LIST_RLP}; +use engines::block_reward::{self, RewardKind}; use ethash::{quick_get_difficulty, slow_hash_block_number, EthashManager, OptimizeFor}; use ethereum_types::{H256, H64, U256, Address}; use unexpected::{OutOfBounds, Mismatch}; @@ -233,11 +234,13 @@ impl Engine for Arc { /// This assumes that all uncles are valid uncles (i.e. of at least one generation before the current). fn on_close_block(&self, block: &mut ExecutedBlock) -> Result<(), Error> { use std::ops::Shr; - use parity_machine::{LiveBlock, WithBalances}; + use parity_machine::LiveBlock; let author = *LiveBlock::header(&*block).author(); let number = LiveBlock::header(&*block).number(); + let mut rewards = Vec::new(); + // Applies EIP-649 reward. let reward = if number >= self.ethash_params.eip649_transition { self.ethash_params.eip649_reward.unwrap_or(self.ethash_params.block_reward) @@ -253,20 +256,21 @@ impl Engine for Arc { // Bestow block rewards. let mut result_block_reward = reward + reward.shr(5) * U256::from(n_uncles); - let mut uncle_rewards = Vec::with_capacity(n_uncles); if number >= self.ethash_params.mcip3_transition { result_block_reward = self.ethash_params.mcip3_miner_reward; + let ubi_contract = self.ethash_params.mcip3_ubi_contract; let ubi_reward = self.ethash_params.mcip3_ubi_reward; let dev_contract = self.ethash_params.mcip3_dev_contract; let dev_reward = self.ethash_params.mcip3_dev_reward; - self.machine.add_balance(block, &author, &result_block_reward)?; - self.machine.add_balance(block, &ubi_contract, &ubi_reward)?; - self.machine.add_balance(block, &dev_contract, &dev_reward)?; + rewards.push((author, RewardKind::Author, result_block_reward)); + rewards.push((ubi_contract, RewardKind::External, ubi_reward)); + rewards.push((dev_contract, RewardKind::External, dev_reward)); + } else { - self.machine.add_balance(block, &author, &result_block_reward)?; + rewards.push((author, RewardKind::Author, result_block_reward)); } // Bestow uncle rewards. @@ -278,15 +282,10 @@ impl Engine for Arc { reward.shr(5) }; - uncle_rewards.push((*uncle_author, result_uncle_reward)); + rewards.push((*uncle_author, RewardKind::Uncle, result_uncle_reward)); } - for &(ref a, ref reward) in &uncle_rewards { - self.machine.add_balance(block, a, reward)?; - } - - // Note and trace. - self.machine.note_rewards(block, &[(author, result_block_reward)], &uncle_rewards) + block_reward::apply_block_rewards(&rewards, block, &self.machine) } fn verify_local_seal(&self, header: &Header) -> Result<(), Error> { diff --git a/ethcore/src/machine.rs b/ethcore/src/machine.rs index e3bf7d340..4aa72b50d 100644 --- a/ethcore/src/machine.rs +++ b/ethcore/src/machine.rs @@ -437,22 +437,30 @@ impl ::parity_machine::WithBalances for EthereumMachine { fn add_balance(&self, live: &mut ExecutedBlock, address: &Address, amount: &U256) -> Result<(), Error> { live.state_mut().add_balance(address, amount, CleanupMode::NoEmpty).map_err(Into::into) } +} +/// A state machine that uses block rewards. +pub trait WithRewards: ::parity_machine::Machine { + /// Note block rewards, traces each reward storing information about benefactor, amount and type + /// of reward. fn note_rewards( &self, live: &mut Self::LiveBlock, - direct: &[(Address, U256)], - indirect: &[(Address, U256)], + rewards: &[(Address, RewardType, U256)], + ) -> Result<(), Self::Error>; +} + +impl WithRewards for EthereumMachine { + fn note_rewards( + &self, + live: &mut Self::LiveBlock, + rewards: &[(Address, RewardType, U256)], ) -> Result<(), Self::Error> { if let Tracing::Enabled(ref mut traces) = *live.traces_mut() { let mut tracer = ExecutiveTracer::default(); - for &(address, amount) in direct { - tracer.trace_reward(address, amount, RewardType::Block); - } - - for &(address, amount) in indirect { - tracer.trace_reward(address, amount, RewardType::Uncle); + for &(address, ref reward_type, amount) in rewards { + tracer.trace_reward(address, amount, reward_type.clone()); } traces.push(tracer.drain().into()); diff --git a/ethcore/src/trace/types/trace.rs b/ethcore/src/trace/types/trace.rs index 06f24efac..cdb00a522 100644 --- a/ethcore/src/trace/types/trace.rs +++ b/ethcore/src/trace/types/trace.rs @@ -141,6 +141,10 @@ pub enum RewardType { Block, /// Uncle Uncle, + /// Empty step (AuthorityRound) + EmptyStep, + /// A reward directly attributed by an external protocol (e.g. block reward contract) + External, } impl Encodable for RewardType { @@ -148,6 +152,8 @@ impl Encodable for RewardType { let v = match *self { RewardType::Block => 0u32, RewardType::Uncle => 1, + RewardType::EmptyStep => 2, + RewardType::External => 3, }; Encodable::rlp_append(&v, s); } @@ -158,6 +164,8 @@ impl Decodable for RewardType { rlp.as_val().and_then(|v| Ok(match v { 0u32 => RewardType::Block, 1 => RewardType::Uncle, + 2 => RewardType::EmptyStep, + 3 => RewardType::External, _ => return Err(DecoderError::Custom("Invalid value of RewardType item")), })) } diff --git a/machine/src/lib.rs b/machine/src/lib.rs index 3a45c38d2..54ee403d9 100644 --- a/machine/src/lib.rs +++ b/machine/src/lib.rs @@ -106,12 +106,4 @@ pub trait WithBalances: Machine { /// Increment the balance of an account in the state of the live block. fn add_balance(&self, live: &mut Self::LiveBlock, address: &Address, amount: &U256) -> Result<(), Self::Error>; - - /// Note block rewards. "direct" rewards are for authors, "indirect" are for e.g. uncles. - fn note_rewards( - &self, - _live: &mut Self::LiveBlock, - _direct: &[(Address, U256)], - _indirect: &[(Address, U256)], - ) -> Result<(), Self::Error> { Ok(()) } } diff --git a/rpc/src/v1/types/trace.rs b/rpc/src/v1/types/trace.rs index a984c64ba..6eb222f5e 100644 --- a/rpc/src/v1/types/trace.rs +++ b/rpc/src/v1/types/trace.rs @@ -308,6 +308,12 @@ pub enum RewardType { /// Uncle #[serde(rename="uncle")] Uncle, + /// EmptyStep (AuthorityRound) + #[serde(rename="emptyStep")] + EmptyStep, + /// External (attributed as part of an external protocol) + #[serde(rename="external")] + External, } impl From for RewardType { @@ -315,6 +321,8 @@ impl From for RewardType { match c { trace::RewardType::Block => RewardType::Block, trace::RewardType::Uncle => RewardType::Uncle, + trace::RewardType::EmptyStep => RewardType::EmptyStep, + trace::RewardType::External => RewardType::External, } } }