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

Maximum uncle count transition #7196

Merged
merged 4 commits into from
Dec 5, 2017
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
1 change: 1 addition & 0 deletions ethcore/res/ethereum/kovan.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
},
"validateScoreTransition": 1000000,
"validateStepTransition": 1500000,
"maximumUncleCountTransition": 5000000,
"maximumUncleCount": 2
}
}
Expand Down
9 changes: 7 additions & 2 deletions ethcore/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -380,8 +380,13 @@ impl<'x> OpenBlock<'x> {
/// NOTE Will check chain constraints and the uncle number but will NOT check
/// that the header itself is actually valid.
pub fn push_uncle(&mut self, valid_uncle_header: Header) -> Result<(), BlockError> {
if self.block.uncles.len() + 1 > self.engine.maximum_uncle_count() {
return Err(BlockError::TooManyUncles(OutOfBounds{min: None, max: Some(self.engine.maximum_uncle_count()), found: self.block.uncles.len() + 1}));
let max_uncles = self.engine.maximum_uncle_count(self.block.header().number());
if self.block.uncles.len() + 1 > max_uncles {
return Err(BlockError::TooManyUncles(OutOfBounds{
min: None,
max: Some(max_uncles),
found: self.block.uncles.len() + 1,
}));
}
// TODO: check number
// TODO: check not a direct ancestor (use last_hashes for that)
Expand Down
4 changes: 2 additions & 2 deletions ethcore/src/client/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1872,7 +1872,7 @@ impl MiningBlockChainClient for Client {
.find_uncle_headers(&h, engine.maximum_uncle_age())
.unwrap_or_else(Vec::new)
.into_iter()
.take(engine.maximum_uncle_count())
.take(engine.maximum_uncle_count(open_block.header().number()))
.foreach(|h| {
open_block.push_uncle(h).expect("pushing maximum_uncle_count;
open_block was just created;
Expand All @@ -1887,7 +1887,7 @@ impl MiningBlockChainClient for Client {
fn reopen_block(&self, block: ClosedBlock) -> OpenBlock {
let engine = &*self.engine;
let mut block = block.reopen(engine);
let max_uncles = engine.maximum_uncle_count();
let max_uncles = engine.maximum_uncle_count(block.header().number());
if block.uncles().len() < max_uncles {
let chain = self.chain.read();
let h = chain.best_block_hash();
Expand Down
42 changes: 41 additions & 1 deletion ethcore/src/engines/authority_round/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ pub struct AuthorityRoundParams {
pub immediate_transitions: bool,
/// Block reward in base units.
pub block_reward: U256,
/// Number of accepted uncles transition block.
pub maximum_uncle_count_transition: u64,
/// Number of accepted uncles.
pub maximum_uncle_count: usize,
}
Expand All @@ -79,6 +81,7 @@ impl From<ethjson::spec::AuthorityRoundParams> for AuthorityRoundParams {
validate_step_transition: p.validate_step_transition.map_or(0, Into::into),
immediate_transitions: p.immediate_transitions.unwrap_or(false),
block_reward: p.block_reward.map_or_else(Default::default, Into::into),
maximum_uncle_count_transition: p.maximum_uncle_count_transition.map_or(0, Into::into),
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will silently break existing networks for Aura users. Consider setting to u64::max_value() instead.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's keeping the behavour introduced in #7006, I think we don't really want anyone running AuRa with uncles. @keorn ?

maximum_uncle_count: p.maximum_uncle_count.map_or(0, Into::into),
}
}
Expand Down Expand Up @@ -221,6 +224,7 @@ pub struct AuthorityRound {
epoch_manager: Mutex<EpochManager>,
immediate_transitions: bool,
block_reward: U256,
maximum_uncle_count_transition: u64,
maximum_uncle_count: usize,
machine: EthereumMachine,
}
Expand Down Expand Up @@ -369,6 +373,7 @@ impl AuthorityRound {
epoch_manager: Mutex::new(EpochManager::blank()),
immediate_transitions: our_params.immediate_transitions,
block_reward: our_params.block_reward,
maximum_uncle_count_transition: our_params.maximum_uncle_count_transition,
maximum_uncle_count: our_params.maximum_uncle_count,
machine: machine,
});
Expand Down Expand Up @@ -441,7 +446,14 @@ impl Engine<EthereumMachine> for AuthorityRound {
]
}

fn maximum_uncle_count(&self) -> usize { self.maximum_uncle_count }
fn maximum_uncle_count(&self, block: BlockNumber) -> usize {
if block >= self.maximum_uncle_count_transition {
self.maximum_uncle_count
} else {
// fallback to default value
2
}
}

fn populate_from_parent(&self, header: &mut Header, parent: &Header) {
// Chain scoring: total weight is sqrt(U256::max_value())*height - step
Expand Down Expand Up @@ -956,6 +968,7 @@ mod tests {
validate_score_transition: 0,
validate_step_transition: 0,
immediate_transitions: true,
maximum_uncle_count_transition: 0,
maximum_uncle_count: 0,
block_reward: Default::default(),
};
Expand Down Expand Up @@ -984,4 +997,31 @@ mod tests {
assert!(aura.verify_block_family(&header, &parent_header).is_ok());
assert_eq!(last_benign.load(AtomicOrdering::SeqCst), 1);
}

#[test]
fn test_uncles_transition() {
let last_benign = Arc::new(AtomicUsize::new(0));
let params = AuthorityRoundParams {
step_duration: Default::default(),
start_step: Some(1),
validators: Box::new(TestSet::new(Default::default(), last_benign.clone())),
validate_score_transition: 0,
validate_step_transition: 0,
immediate_transitions: true,
maximum_uncle_count_transition: 1,
maximum_uncle_count: 0,
block_reward: Default::default(),
};

let aura = {
let mut c_params = ::spec::CommonParams::default();
c_params.gas_limit_bound_divisor = 5.into();
let machine = ::machine::EthereumMachine::regular(c_params, Default::default());
AuthorityRound::new(params, machine).unwrap()
};

assert_eq!(aura.maximum_uncle_count(0), 2);
assert_eq!(aura.maximum_uncle_count(1), 0);
assert_eq!(aura.maximum_uncle_count(100), 0);
}
}
5 changes: 3 additions & 2 deletions ethcore/src/engines/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,8 @@ pub trait Engine<M: Machine>: Sync + Send {
fn extra_info(&self, _header: &M::Header) -> BTreeMap<String, String> { BTreeMap::new() }

/// Maximum number of uncles a block is allowed to declare.
fn maximum_uncle_count(&self) -> usize { 0 }
fn maximum_uncle_count(&self, _block: BlockNumber) -> usize { 0 }

/// The number of generations back that uncles can be.
fn maximum_uncle_age(&self) -> usize { 6 }

Expand Down Expand Up @@ -363,7 +364,7 @@ pub trait EthEngine: Engine<::machine::EthereumMachine> {
}

/// The nonce with which accounts begin at given block.
fn account_start_nonce(&self, block: u64) -> U256 {
fn account_start_nonce(&self, block: BlockNumber) -> U256 {
self.machine().account_start_nonce(block)
}

Expand Down
3 changes: 2 additions & 1 deletion ethcore/src/engines/null_engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

use bigint::prelude::U256;
use engines::Engine;
use header::BlockNumber;
use parity_machine::{Header, LiveBlock, WithBalances};

/// Params for a null engine.
Expand Down Expand Up @@ -95,7 +96,7 @@ impl<M: WithBalances> Engine<M> for NullEngine<M> {
self.machine.note_rewards(block, &[(author, result_block_reward)], &uncle_rewards)
}

fn maximum_uncle_count(&self) -> usize { 2 }
fn maximum_uncle_count(&self, _block: BlockNumber) -> usize { 2 }

fn verify_local_seal(&self, _header: &M::Header) -> Result<(), M::Error> {
Ok(())
Expand Down
2 changes: 1 addition & 1 deletion ethcore/src/engines/tendermint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@ impl Engine<EthereumMachine> for Tendermint {

fn machine(&self) -> &EthereumMachine { &self.machine }

fn maximum_uncle_count(&self) -> usize { 0 }
fn maximum_uncle_count(&self, _block: BlockNumber) -> usize { 0 }

fn maximum_uncle_age(&self) -> usize { 0 }

Expand Down
6 changes: 3 additions & 3 deletions ethcore/src/ethereum/ethash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use util::Address;
use unexpected::{OutOfBounds, Mismatch};
use block::*;
use error::{BlockError, Error};
use header::Header;
use header::{Header, BlockNumber};
use engines::{self, Engine};
use ethjson;
use rlp::{self, UntrustedRlp};
Expand Down Expand Up @@ -181,7 +181,7 @@ impl Engine<EthereumMachine> for Arc<Ethash> {
}
}

fn maximum_uncle_count(&self) -> usize { 2 }
fn maximum_uncle_count(&self, _block: BlockNumber) -> usize { 2 }

fn populate_from_parent(&self, header: &mut Header, parent: &Header) {
let difficulty = self.calculate_difficulty(header, parent);
Expand Down Expand Up @@ -520,7 +520,7 @@ mod tests {
let (eras, reward) = ecip1017_eras_block_reward(eras_rounds, start_reward, block_number);
assert_eq!(15, eras);
assert_eq!(U256::from_str("271000000000000").unwrap(), reward);

let block_number = 250000000;
let (eras, reward) = ecip1017_eras_block_reward(eras_rounds, start_reward, block_number);
assert_eq!(49, eras);
Expand Down
11 changes: 8 additions & 3 deletions ethcore/src/verification/verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,14 @@ pub fn verify_block_family(header: &Header, parent: &Header, engine: &EthEngine,

fn verify_uncles(header: &Header, bytes: &[u8], bc: &BlockProvider, engine: &EthEngine) -> Result<(), Error> {
let num_uncles = UntrustedRlp::new(bytes).at(2)?.item_count()?;
let max_uncles = engine.maximum_uncle_count(header.number());
if num_uncles != 0 {
if num_uncles > engine.maximum_uncle_count() {
return Err(From::from(BlockError::TooManyUncles(OutOfBounds { min: None, max: Some(engine.maximum_uncle_count()), found: num_uncles })));
if num_uncles > max_uncles {
return Err(From::from(BlockError::TooManyUncles(OutOfBounds {
min: None,
max: Some(max_uncles),
found: num_uncles,
})));
}

let mut excluded = HashSet::new();
Expand Down Expand Up @@ -653,7 +658,7 @@ mod tests {
let mut bad_uncles = good_uncles.clone();
bad_uncles.push(good_uncle1.clone());
check_fail(family_test(&create_test_block_with_data(&header, &good_transactions, &bad_uncles), engine, &bc),
TooManyUncles(OutOfBounds { max: Some(engine.maximum_uncle_count()), min: None, found: bad_uncles.len() }));
TooManyUncles(OutOfBounds { max: Some(engine.maximum_uncle_count(header.number())), min: None, found: bad_uncles.len() }));

header = good.clone();
bad_uncles = vec![ good_uncle1.clone(), good_uncle1.clone() ];
Expand Down
10 changes: 9 additions & 1 deletion json/src/spec/authority_round.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ pub struct AuthorityRoundParams {
/// Reward per block in wei.
#[serde(rename="blockReward")]
pub block_reward: Option<Uint>,
/// Block at which maximum uncle count should be considered.
#[serde(rename="maximumUncleCountTransition")]
pub maximum_uncle_count_transition: Option<Uint>,
/// Maximum number of accepted uncles.
#[serde(rename="maximumUncleCount")]
pub maximum_uncle_count: Option<Uint>,
}
Expand Down Expand Up @@ -73,7 +77,9 @@ mod tests {
},
"startStep" : 24,
"validateStepTransition": 150,
"blockReward": 5000000
"blockReward": 5000000,
"maximumUncleCountTransition": 10000000,
"maximumUncleCount": 5
}
}"#;

Expand All @@ -82,6 +88,8 @@ mod tests {
assert_eq!(deserialized.params.validators, ValidatorSet::List(vec![Address(H160::from("0xc6d9d2cd449a754c494264e1809c50e34d64562b"))]));
assert_eq!(deserialized.params.start_step, Some(Uint(U256::from(24))));
assert_eq!(deserialized.params.immediate_transitions, None);
assert_eq!(deserialized.params.maximum_uncle_count_transition, Some(Uint(10_000_000.into())));
assert_eq!(deserialized.params.maximum_uncle_count, Some(Uint(5.into())));

}
}