Refactor block reward application and tracing

ethcore/src/engines/authority_round/mod.rs

@@ -1015,8 +1015,10 @@ impl Engine<EthereumMachine> 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<EthereumMachine> 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()
 			},
 		};
 

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 <http://www.gnu.org/licenses/>.
 
+//! 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<RewardKind> for u16 {
@@ -45,6 +51,17 @@ impl From<RewardKind> for u16 {
 	}
 }
 
+impl Into<trace::RewardType> 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<M: Machine + WithBalances + WithRewards>(
+	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)]

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;

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<M: Default> Default for NullEngine<M> {
 	}
 }
 
-impl<M: WithBalances> Engine<M> for NullEngine<M> {
+impl<M: WithBalances + WithRewards> Engine<M> for NullEngine<M> {
 	fn name(&self) -> &str {
 		"NullEngine"
 	}
@@ -74,26 +76,20 @@ impl<M: WithBalances> Engine<M> for NullEngine<M> {
 
 		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));
-		}
-
-		for &(ref a, ref reward) in &uncle_rewards {
-			self.machine.add_balance(block, a, reward)?;
+			rewards.push((*uncle_author, RewardKind::Uncle, result_uncle_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 }

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<EthereumMachine> 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> {

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<EthereumMachine> for Arc<Ethash> {
 	/// 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<EthereumMachine> for Arc<Ethash> {
 
 		// 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<EthereumMachine> for Arc<Ethash> {
 				reward.shr(5)
 			};
 
-			uncle_rewards.push((*uncle_author, result_uncle_reward));
-		}
-
-		for &(ref a, ref reward) in &uncle_rewards {
-			self.machine.add_balance(block, a, reward)?;
+			rewards.push((*uncle_author, RewardKind::Uncle, result_uncle_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> {

ethcore/src/machine.rs

@@ -426,22 +426,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,
+		rewards: &[(Address, RewardType, U256)],
+	) -> Result<(), Self::Error>;
+}
 
+impl WithRewards for EthereumMachine {
 	fn note_rewards(
 		&self,
 		live: &mut Self::LiveBlock,
-		direct: &[(Address, U256)],
-		indirect: &[(Address, U256)],
+		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());

ethcore/src/trace/types/trace.rs

@@ -141,13 +141,19 @@ 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 {
 	fn rlp_append(&self, s: &mut RlpStream) {
 		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")),
 		}))
 	}

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(()) }
 }

rpc/src/v1/types/trace.rs

@@ -308,13 +308,21 @@ 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<trace::RewardType> for RewardType {
 	fn from(c: trace::RewardType) -> Self {
 		match c {
 			trace::RewardType::Block => RewardType::Block,
 			trace::RewardType::Uncle => RewardType::Uncle,
+			trace::RewardType::EmptyStep => RewardType::EmptyStep,
+			trace::RewardType::External => RewardType::External,
 		}
 	}
 }