Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Rewards on closing blocks #6194

Merged
merged 31 commits into from
Aug 30, 2017
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
ab61538
Tracing for rewards added. Without tests for now
grbIzl Jul 18, 2017
16f3948
Warnings removed
grbIzl Jul 18, 2017
7312803
Working test with block reward added
grbIzl Jul 25, 2017
53c3b77
Complete version of tracing test with reward
grbIzl Jul 26, 2017
af1fbb3
Unit tests for tracing reward added
grbIzl Jul 27, 2017
b193534
Fixed errors after merge with master
grbIzl Jul 27, 2017
1f3f911
Transaction ids made optional in order to reflect not transactional t…
grbIzl Jul 28, 2017
94efa3a
Miner field renamed to author
grbIzl Jul 31, 2017
bb043ba
Tabs corrected
grbIzl Jul 31, 2017
06862c7
Tracing for rewards added. Without tests for now
grbIzl Jul 18, 2017
204a63a
Warnings removed
grbIzl Jul 18, 2017
5086dc3
Working test with block reward added
grbIzl Jul 25, 2017
3fdb912
Complete version of tracing test with reward
grbIzl Jul 26, 2017
f2d12ae
Unit tests for tracing reward added
grbIzl Jul 27, 2017
45043c3
Fixed errors after merge with master
grbIzl Jul 27, 2017
1a3f3ff
Transaction ids made optional in order to reflect not transactional t…
grbIzl Jul 28, 2017
9374e31
Miner field renamed to author
grbIzl Jul 31, 2017
e809582
Tabs corrected
grbIzl Jul 31, 2017
8826fdb
Merge with head
grbIzl Jul 31, 2017
141c2fd
Fixed comments after review and test after rebase
grbIzl Jul 31, 2017
2e840bc
Fixed comments after the review
grbIzl Aug 2, 2017
01ea968
Merge with master
grbIzl Aug 2, 2017
01a02a8
Wasm test link changed
grbIzl Aug 2, 2017
3043432
Modification of traces moved to engines
grbIzl Aug 3, 2017
655ed93
Common engine method for bestowing rewards created
grbIzl Aug 4, 2017
dd91121
Common method for tracing refactored due to comments on review
grbIzl Aug 4, 2017
bfd238e
Style fixed after review
grbIzl Aug 10, 2017
50495c6
Merge branch 'master' into Fix-4858
gavofyork Aug 29, 2017
e366645
Consistent use of `,`s
gavofyork Aug 29, 2017
88200a1
Merge branch 'master' into Fix-4858
debris Aug 30, 2017
8a420d6
fixed merge
debris Aug 30, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions ethcore/res/null_morden_with_reward.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
{
"name": "Morden",
"engine": {
"null": null
},
"params": {
"gasLimitBoundDivisor": "0x0400",
"accountStartNonce": "0x0",
"maximumExtraDataSize": "0x20",
"minGasLimit": "0x1388",
"networkID" : "0x2",
"blockReward": "0x4563918244F40000"
},
"genesis": {
"seal": {
"ethereum": {
"nonce": "0x00006d6f7264656e",
"mixHash": "0x00000000000000000000000000000000000000647572616c65787365646c6578"
}
},
"difficulty": "0x20000",
"author": "0x0000000000000000000000000000000000000000",
"timestamp": "0x00",
"parentHash": "0x0000000000000000000000000000000000000000000000000000000000000000",
"extraData": "0x",
"gasLimit": "0x2fefd8"
},
"accounts": {
"0000000000000000000000000000000000000001": { "balance": "1", "nonce": "1048576", "builtin": { "name": "ecrecover", "pricing": { "linear": { "base": 3000, "word": 0 } } } },
"0000000000000000000000000000000000000002": { "balance": "1", "nonce": "1048576", "builtin": { "name": "sha256", "pricing": { "linear": { "base": 60, "word": 12 } } } },
"0000000000000000000000000000000000000003": { "balance": "1", "nonce": "1048576", "builtin": { "name": "ripemd160", "pricing": { "linear": { "base": 600, "word": 120 } } } },
"0000000000000000000000000000000000000004": { "balance": "1", "nonce": "1048576", "builtin": { "name": "identity", "pricing": { "linear": { "base": 15, "word": 3 } } } },
"102e61f5d8f9bc71d0ad4a084df4e65e05ce0e1c": { "balance": "1606938044258990275541962092341162602522202993782792835301376", "nonce": "1048576" }
}
}
18 changes: 11 additions & 7 deletions ethcore/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use std::sync::Arc;
use std::collections::HashSet;

use rlp::{UntrustedRlp, RlpStream, Encodable, Decodable, DecoderError};
use util::{Bytes, Address, Hashable, U256, H256, ordered_trie_root, SHA3_NULL_RLP};
use util::{Bytes, Address, Hashable, U256, H256, ordered_trie_root, SHA3_NULL_RLP, SHA3_EMPTY_LIST_RLP};
use util::error::{Mismatch, OutOfBounds};

use basic_types::{LogBloom, Seal};
Expand Down Expand Up @@ -91,7 +91,7 @@ pub struct ExecutedBlock {
receipts: Vec<Receipt>,
transactions_set: HashSet<H256>,
state: State<StateDB>,
traces: Option<Vec<Vec<FlatTrace>>>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: don't kill the trailing ,

traces: Option<Vec<Vec<FlatTrace>>>
}

/// A set of references to `ExecutedBlock` fields that are publicly accessible.
Expand All @@ -107,7 +107,7 @@ pub struct BlockRefMut<'a> {
/// State.
pub state: &'a mut State<StateDB>,
/// Traces.
pub traces: &'a Option<Vec<Vec<FlatTrace>>>,
pub traces: &'a mut Option<Vec<Vec<FlatTrace>>>,
}

/// A set of immutable references to `ExecutedBlock` fields that are publicly accessible.
Expand Down Expand Up @@ -148,7 +148,7 @@ impl ExecutedBlock {
uncles: &self.uncles,
state: &mut self.state,
receipts: &self.receipts,
traces: &self.traces,
traces: &mut self.traces,
}
}

Expand Down Expand Up @@ -196,6 +196,9 @@ pub trait IsBlock {

/// Get all uncles in this block.
fn uncles(&self) -> &[Header] { &self.block().uncles }

/// Get tracing enabled flag for this block.
fn tracing_enabled(&self) -> bool { self.block().traces.is_some() }
}

/// Trait for a object that has a state database.
Expand Down Expand Up @@ -393,8 +396,9 @@ impl<'x> OpenBlock<'x> {
let unclosed_state = s.block.state.clone();

if let Err(e) = s.engine.on_close_block(&mut s.block) {
warn!("Encountered error on closing the block: {}", e);
warn!("Encountered error on closing the block: {}", e);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dodgy space added

}

if let Err(e) = s.block.state.commit() {
warn!("Encountered error on state commit: {}", e);
}
Expand All @@ -419,7 +423,7 @@ impl<'x> OpenBlock<'x> {
let mut s = self;

if let Err(e) = s.engine.on_close_block(&mut s.block) {
warn!("Encountered error on closing the block: {}", e);
warn!("Encountered error on closing the block: {}", e);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dodgy space added (beginning of line)

}

if let Err(e) = s.block.state.commit() {
Expand All @@ -429,7 +433,7 @@ impl<'x> OpenBlock<'x> {
s.block.header.set_transactions_root(ordered_trie_root(s.block.transactions.iter().map(|e| e.rlp_bytes().into_vec())));
}
let uncle_bytes = s.block.uncles.iter().fold(RlpStream::new_list(s.block.uncles.len()), |mut s, u| {s.append_raw(&u.rlp(Seal::With), 1); s} ).out();
if s.block.header.uncles_hash().is_zero() {
if s.block.header.uncles_hash().is_zero() || s.block.header.uncles_hash() == &SHA3_EMPTY_LIST_RLP {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any particular reason for this change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. From my point of view it's a logical flaw. If block is initialized using default construction, uncle_hash won't be zero and won't be set in this method. I assume, that original intent was to set it in this case. You can see the same pattern (checking default values and zero) in this function for other fields. As a result simple integration test with close_and_seal for block failed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok that's fair enough.

s.block.header.set_uncles_hash(uncle_bytes.sha3());
}
if s.block.header.receipts_root().is_zero() || s.block.header.receipts_root() == &SHA3_NULL_RLP {
Expand Down
13 changes: 1 addition & 12 deletions ethcore/src/engines/authority_round/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ use error::{Error, TransactionError, BlockError};
use ethjson;
use header::{Header, BlockNumber};
use spec::CommonParams;
use state::CleanupMode;
use transaction::UnverifiedTransaction;

use super::signer::EngineSigner;
Expand Down Expand Up @@ -546,17 +545,7 @@ impl Engine for AuthorityRound {

/// Apply the block reward on finalisation of the block.
fn on_close_block(&self, block: &mut ExecutedBlock) -> Result<(), Error> {
let fields = block.fields_mut();
// Bestow block reward
let reward = self.params().block_reward;
let res = fields.state.add_balance(fields.header.author(), &reward, CleanupMode::NoEmpty)
.map_err(::error::Error::from)
.and_then(|_| fields.state.commit());
// Commit state so that we can actually figure out the state root.
if let Err(ref e) = res {
warn!("Encountered error on closing block: {}", e);
}
res
::engines::common::bestow_block_reward(block, self)
}

/// Check the number of seal fields.
Expand Down
28 changes: 26 additions & 2 deletions ethcore/src/engines/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ pub trait Engine : Sync + Send {

/// Block transformation functions, after the transactions.
fn on_close_block(&self, _block: &mut ExecutedBlock) -> Result<(), Error> {
Ok(())
Ok(())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dodgy space added

}

/// None means that it requires external input (e.g. PoW) to seal a block.
Expand Down Expand Up @@ -399,8 +399,9 @@ pub mod common {
use transaction::SYSTEM_ADDRESS;
use executive::Executive;
use vm::{CallType, ActionParams, ActionValue, EnvInfo, LastHashes};
use trace::{NoopTracer, NoopVMTracer};
use trace::{NoopTracer, NoopVMTracer, Tracer, ExecutiveTracer, RewardType};
use state::Substate;
use state::CleanupMode;

use util::*;
use super::Engine;
Expand Down Expand Up @@ -469,4 +470,27 @@ pub mod common {
}
Ok(())
}

/// Trace rewards on closing block
pub fn bestow_block_reward<E: Engine + ?Sized>(block: &mut ExecutedBlock, engine: &E) -> Result<(), Error> {
let fields = block.fields_mut();
// Bestow block reward
let reward = engine.params().block_reward;
let res = fields.state.add_balance(fields.header.author(), &reward, CleanupMode::NoEmpty)
.map_err(::error::Error::from)
.and_then(|_| fields.state.commit());

let block_author = fields.header.author().clone();
fields.traces.as_mut().map(|mut traces| {
let mut tracer = ExecutiveTracer::default();
tracer.trace_reward(block_author, engine.params().block_reward, RewardType::Block);
traces.push(tracer.traces())
});

// Commit state so that we can actually figure out the state root.
if let Err(ref e) = res {
warn!("Encountered error on bestowing reward: {}", e);
}
res
}
}
49 changes: 49 additions & 0 deletions ethcore/src/engines/null_engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,15 @@
use std::collections::BTreeMap;
use util::Address;
use builtin::Builtin;
use block::{ExecutedBlock, IsBlock};
use util::U256;
use engines::Engine;
use spec::CommonParams;
use evm::Schedule;
use header::BlockNumber;
use error::Error;
use state::CleanupMode;
use trace::{Tracer, ExecutiveTracer, RewardType};

/// An engine which does not provide any consensus mechanism and does not seal blocks.
pub struct NullEngine {
Expand Down Expand Up @@ -64,4 +69,48 @@ impl Engine for NullEngine {
fn snapshot_components(&self) -> Option<Box<::snapshot::SnapshotComponents>> {
Some(Box::new(::snapshot::PowSnapshot(10000)))
}

fn on_close_block(&self, block: &mut ExecutedBlock) -> Result<(), Error> {
if self.params.block_reward == U256::zero() {
// we don't have to apply reward in this case
return Ok(())
}

/// Block reward
let tracing_enabled = block.tracing_enabled();
let fields = block.fields_mut();
let mut tracer = ExecutiveTracer::default();

let result_block_reward = U256::from(1000000000);
fields.state.add_balance(
fields.header.author(),
&result_block_reward,
CleanupMode::NoEmpty
)?;

if tracing_enabled {
let block_author = fields.header.author().clone();
tracer.trace_reward(block_author, result_block_reward, RewardType::Block);
}

/// Uncle rewards
let result_uncle_reward = U256::from(10000000);
for u in fields.uncles.iter() {
let uncle_author = u.author().clone();
fields.state.add_balance(
u.author(),
&(result_uncle_reward),
CleanupMode::NoEmpty
)?;
if tracing_enabled {
tracer.trace_reward(uncle_author, result_uncle_reward, RewardType::Uncle);
}
}

fields.state.commit()?;
if tracing_enabled {
fields.traces.as_mut().map(|mut traces| traces.push(tracer.traces()));
}
Ok(())
}
}
13 changes: 1 addition & 12 deletions ethcore/src/engines/tendermint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ use account_provider::AccountProvider;
use block::*;
use spec::CommonParams;
use engines::{Engine, Seal, EngineError, ConstructedVerifier};
use state::CleanupMode;
use io::IoService;
use super::signer::EngineSigner;
use super::validator_set::{ValidatorSet, SimpleList};
Expand Down Expand Up @@ -541,17 +540,7 @@ impl Engine for Tendermint {

/// Apply the block reward on finalisation of the block.
fn on_close_block(&self, block: &mut ExecutedBlock) -> Result<(), Error>{
let fields = block.fields_mut();
// Bestow block reward
let reward = self.params().block_reward;
let res = fields.state.add_balance(fields.header.author(), &reward, CleanupMode::NoEmpty)
.map_err(::error::Error::from)
.and_then(|_| fields.state.commit());
// Commit state so that we can actually figure out the state root.
if let Err(ref e) = res {
warn!("Encountered error on closing block: {}", e);
}
res
::engines::common::bestow_block_reward(block, self)
}

fn verify_block_basic(&self, header: &Header, _block: Option<&[u8]>) -> Result<(), Error> {
Expand Down
31 changes: 27 additions & 4 deletions ethcore/src/ethereum/ethash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use block::*;
use builtin::Builtin;
use vm::EnvInfo;
use error::{BlockError, Error, TransactionError};
use trace::{Tracer, ExecutiveTracer,RewardType};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inconsistent spaces.

use header::{Header, BlockNumber};
use state::CleanupMode;
use spec::CommonParams;
Expand Down Expand Up @@ -275,38 +276,60 @@ impl Engine for Arc<Ethash> {
/// Apply the block reward on finalisation of the block.
/// 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;
let reward = self.params().block_reward;
let tracing_enabled = block.tracing_enabled();
let fields = block.fields_mut();
let eras_rounds = self.ethash_params.ecip1017_era_rounds;
let (eras, reward) = ecip1017_eras_block_reward(eras_rounds, reward, fields.header.number());
let mut tracer = ExecutiveTracer::default();

// Bestow block reward
let result_block_reward = reward + reward.shr(5) * U256::from(fields.uncles.len());
fields.state.add_balance(
fields.header.author(),
&(reward + reward / U256::from(32) * U256::from(fields.uncles.len())),
&result_block_reward,
CleanupMode::NoEmpty
)?;

if tracing_enabled {
let block_author = fields.header.author().clone();
tracer.trace_reward(block_author, result_block_reward, RewardType::Block);
}

// Bestow uncle rewards
let current_number = fields.header.number();
for u in fields.uncles.iter() {
let uncle_author = u.author().clone();
let result_uncle_reward: U256;

if eras == 0 {
result_uncle_reward = (reward * U256::from(8 + u.number() - current_number)).shr(3);
fields.state.add_balance(
u.author(),
&(reward * U256::from(8 + u.number() - current_number) / U256::from(8)),
&result_uncle_reward,
CleanupMode::NoEmpty
)
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

trailing whitespace

} else {
result_uncle_reward = reward.shr(5);
fields.state.add_balance(
u.author(),
&(reward / U256::from(32)),
&result_uncle_reward,
CleanupMode::NoEmpty
)
}?;

// Trace uncle rewards
if tracing_enabled {
tracer.trace_reward(uncle_author, result_uncle_reward, RewardType::Uncle);
}
}

// Commit state so that we can actually figure out the state root.
fields.state.commit()?;
if tracing_enabled {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here (avoid to produce traces when tracing is disabled)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure? In this case it seems to be covered with if.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, you're right sorry!

fields.traces.as_mut().map(|mut traces| traces.push(tracer.traces()));
}
Ok(())
}

Expand Down
5 changes: 4 additions & 1 deletion ethcore/src/spec/spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ impl From<ethjson::spec::Params> for CommonParams {
wasm: p.wasm.unwrap_or(false),
gas_limit_bound_divisor: p.gas_limit_bound_divisor.into(),
block_reward: p.block_reward.map_or_else(U256::zero, Into::into),
registrar: p.registrar.map_or_else(Address::new, Into::into),
registrar: p.registrar.map_or_else(Address::new, Into::into)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't kill the trailing ,

}
}
}
Expand Down Expand Up @@ -478,6 +478,9 @@ impl Spec {
/// Create a new Spec which conforms to the Frontier-era Morden chain except that it's a NullEngine consensus.
pub fn new_test() -> Spec { load_bundled!("null_morden") }

/// Create a new Spec which conforms to the Frontier-era Morden chain except that it's a NullEngine consensus with applying reward on block close.
pub fn new_test_with_reward() -> Spec { load_bundled!("null_morden_with_reward") }

/// Create a new Spec which is a NullEngine consensus with a premine of address whose secret is sha3('').
pub fn new_null() -> Spec { load_bundled!("null") }

Expand Down
1 change: 1 addition & 0 deletions ethcore/src/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
pub mod helpers;
mod client;
mod evm;
mod trace;

#[cfg(feature="ipc")]
mod rpc;
Expand Down
Loading