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

Uncle inclusion in block authoring. #578

Merged
merged 13 commits into from
Mar 5, 2016
2 changes: 1 addition & 1 deletion ethcore/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ 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.base.uncles.len() >= self.engine.maximum_uncle_count() {
if self.block.base.uncles.len() > self.engine.maximum_uncle_count() {
return Err(BlockError::TooManyUncles(OutOfBounds{min: None, max: Some(self.engine.maximum_uncle_count()), found: self.block.base.uncles.len()}));
}
// TODO: check number
Expand Down
113 changes: 108 additions & 5 deletions ethcore/src/blockchain/blockchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ pub trait BlockProvider {
}

/// Get a list of uncles for a given block.
/// Returns None if block deos not exist.
/// Returns None if block does not exist.
fn uncles(&self, hash: &H256) -> Option<Vec<Header>> {
self.block(hash).map(|bytes| BlockView::new(&bytes).uncles())
}
Expand Down Expand Up @@ -227,6 +227,24 @@ impl BlockProvider for BlockChain {

const COLLECTION_QUEUE_SIZE: usize = 8;

pub struct AncestryIter<'a> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Iterator for traversing ancestry.

current: H256,
chain: &'a BlockChain,
}

impl<'a> Iterator for AncestryIter<'a> {
type Item = H256;
fn next(&mut self) -> Option<H256> {
if self.current.is_zero() {
Option::None
} else {
let mut n = self.chain.block_details(&self.current).unwrap().parent;
mem::swap(&mut self.current, &mut n);
Some(n)
}
}
}

impl BlockChain {
/// Create new instance of blockchain from given Genesis
pub fn new(config: BlockChainConfig, genesis: &[u8], path: &Path) -> BlockChain {
Expand Down Expand Up @@ -474,10 +492,35 @@ impl BlockChain {
self.extras_db.write(batch).unwrap();
}

/// Given a block's `parent`, find every block header which represents a valid uncle.
pub fn find_uncle_headers(&self, _parent: &H256) -> Vec<Header> {
// TODO
Vec::new()
/// Iterator that lists `first` and then all of `first`'s ancestors, by hash.
pub fn ancestry_iter(&self, first: H256) -> Option<AncestryIter> {
if self.is_known(&first) {
Some(AncestryIter {
current: first,
chain: &self,
})
} else {
None
}
}

/// Given a block's `parent`, find every block header which represents a valid possible uncle.
pub fn find_uncle_headers(&self, parent: &H256, uncle_generations: usize) -> Option<Vec<Header>> {
if !self.is_known(parent) { return None; }

let mut excluded = HashSet::new();
for a in self.ancestry_iter(parent.clone()).unwrap().take(uncle_generations) {
excluded.extend(self.uncle_hashes(&a).unwrap().into_iter());
excluded.insert(a);
}

let mut ret = Vec::new();
for a in self.ancestry_iter(parent.clone()).unwrap().skip(1).take(uncle_generations) {
ret.extend(self.block_details(&a).unwrap().children.iter()
.filter_map(|h| if excluded.contains(h) { None } else { self.block_header(h) })
);
}
Some(ret)
}

/// Get inserted block info which is critical to preapre extras updates.
Expand Down Expand Up @@ -818,6 +861,66 @@ mod tests {
assert_eq!(bc.block_hash(2), None);
}

#[test]
fn check_ancestry_iter() {
let mut canon_chain = ChainGenerator::default();
let mut finalizer = BlockFinalizer::default();
let genesis = canon_chain.generate(&mut finalizer).unwrap();
let genesis_hash = BlockView::new(&genesis).header_view().sha3();

let temp = RandomTempPath::new();
let bc = BlockChain::new(BlockChainConfig::default(), &genesis, temp.as_path());

let mut block_hashes = vec![genesis_hash.clone()];
for _ in 0..10 {
let block = canon_chain.generate(&mut finalizer).unwrap();
block_hashes.push(BlockView::new(&block).header_view().sha3());
bc.insert_block(&block, vec![]);
}

block_hashes.reverse();

assert_eq!(bc.ancestry_iter(block_hashes[0].clone()).unwrap().collect::<Vec<_>>(), block_hashes)
}

#[test]
#[cfg_attr(feature="dev", allow(cyclomatic_complexity))]
fn test_find_uncles() {
let mut canon_chain = ChainGenerator::default();
let mut finalizer = BlockFinalizer::default();
let genesis = canon_chain.generate(&mut finalizer).unwrap();
let b1b = canon_chain.fork(1).generate(&mut finalizer.fork()).unwrap();
let b1a = canon_chain.generate(&mut finalizer).unwrap();
let b2b = canon_chain.fork(1).generate(&mut finalizer.fork()).unwrap();
let b2a = canon_chain.generate(&mut finalizer).unwrap();
let b3b = canon_chain.fork(1).generate(&mut finalizer.fork()).unwrap();
let b3a = canon_chain.generate(&mut finalizer).unwrap();
let b4b = canon_chain.fork(1).generate(&mut finalizer.fork()).unwrap();
let b4a = canon_chain.generate(&mut finalizer).unwrap();
let b5b = canon_chain.fork(1).generate(&mut finalizer.fork()).unwrap();
let b5a = canon_chain.generate(&mut finalizer).unwrap();

let temp = RandomTempPath::new();
let bc = BlockChain::new(BlockChainConfig::default(), &genesis, temp.as_path());
bc.insert_block(&b1a, vec![]);
bc.insert_block(&b1b, vec![]);
bc.insert_block(&b2a, vec![]);
bc.insert_block(&b2b, vec![]);
bc.insert_block(&b3a, vec![]);
bc.insert_block(&b3b, vec![]);
bc.insert_block(&b4a, vec![]);
bc.insert_block(&b4b, vec![]);
bc.insert_block(&b5a, vec![]);
bc.insert_block(&b5b, vec![]);

assert_eq!(
[&b4b, &b3b, &b2b].iter().map(|b| BlockView::new(b).header()).collect::<Vec<_>>(),
bc.find_uncle_headers(&BlockView::new(&b4a).header_view().sha3(), 3).unwrap()
);

// TODO: insert block that already includes one of them as an uncle to check it's not allowed.
}

#[test]
#[cfg_attr(feature="dev", allow(cyclomatic_complexity))]
fn test_small_fork() {
Expand Down
4 changes: 2 additions & 2 deletions ethcore/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ pub struct Client {
extra_data: RwLock<Bytes>,
}

const HISTORY: u64 = 1000;
const HISTORY: u64 = 30;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unintended?

const CLIENT_DB_VER_STR: &'static str = "4.0";

impl Client {
Expand Down Expand Up @@ -460,7 +460,7 @@ impl Client {
self.extra_data()
);

self.chain.read().unwrap().find_uncle_headers(&h).into_iter().foreach(|h| { b.push_uncle(h).unwrap(); });
self.chain.read().unwrap().find_uncle_headers(&h, self.engine.deref().deref().maximum_uncle_age()).unwrap().into_iter().foreach(|h| { b.push_uncle(h).unwrap(); });

// TODO: push transactions.

Expand Down
2 changes: 2 additions & 0 deletions ethcore/src/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ pub trait Engine : Sync + Send {
fn maximum_extra_data_size(&self) -> usize { decode(&self.spec().engine_params.get("maximumExtraDataSize").unwrap()) }
/// Maximum number of uncles a block is allowed to declare.
fn maximum_uncle_count(&self) -> usize { 2 }
/// The number of generations back that uncles can be.
fn maximum_uncle_age(&self) -> usize { 6 }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Additional chain parameter

/// The nonce with which accounts begin.
fn account_start_nonce(&self) -> U256 { decode(&self.spec().engine_params.get("accountStartNonce").unwrap()) }

Expand Down
21 changes: 20 additions & 1 deletion ethcore/src/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ pub type BlockNumber = u64;
/// which is non-specific.
///
/// Doesn't do all that much on its own.
#[derive(Debug, Clone)]
#[derive(Debug, Clone, Eq)]
pub struct Header {
// TODO: make all private.
/// Parent hash.
Expand Down Expand Up @@ -70,6 +70,25 @@ pub struct Header {
pub bare_hash: RefCell<Option<H256>>,
}

impl PartialEq for Header {
fn eq(&self, c: &Header) -> bool {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

comparison without the memoisation fields.

self.parent_hash == c.parent_hash &&
self.timestamp == c.timestamp &&
self.number == c.number &&
self.author == c.author &&
self.transactions_root == c.transactions_root &&
self.uncles_hash == c.uncles_hash &&
self.extra_data == c.extra_data &&
self.state_root == c.state_root &&
self.receipts_root == c.receipts_root &&
self.log_bloom == c.log_bloom &&
self.gas_used == c.gas_used &&
self.gas_limit == c.gas_limit &&
self.difficulty == c.difficulty &&
self.seal == c.seal
}
}

impl Default for Header {
fn default() -> Self {
Header {
Expand Down
4 changes: 2 additions & 2 deletions ethcore/src/verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ pub fn verify_block_family<BC>(header: &Header, bytes: &[u8], engine: &Engine, b
excluded.insert(header.hash());
let mut hash = header.parent_hash.clone();
excluded.insert(hash.clone());
for _ in 0..6 {
for _ in 0..engine.maximum_uncle_age() {
match bc.block_details(&hash) {
Some(details) => {
excluded.insert(details.parent.clone());
Expand All @@ -121,7 +121,7 @@ pub fn verify_block_family<BC>(header: &Header, bytes: &[u8], engine: &Engine, b
// (8 Invalid)

let depth = if header.number > uncle.number { header.number - uncle.number } else { 0 };
if depth > 6 {
if depth > engine.maximum_uncle_age() as u64 {
return Err(From::from(BlockError::UncleTooOld(OutOfBounds { min: Some(header.number - depth), max: Some(header.number - 1), found: uncle.number })));
}
else if depth < 1 {
Expand Down