Merge pull request #578 from ethcore/finduncles

Uncle inclusion in block authoring.

ethcore/src/block.rs

@@ -220,8 +220,8 @@ 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() {
-			return Err(BlockError::TooManyUncles(OutOfBounds{min: None, max: Some(self.engine.maximum_uncle_count()), found: self.block.base.uncles.len()}));
+		if self.block.base.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.base.uncles.len() + 1}));
 		}
 		// TODO: check number
 		// TODO: check not a direct ancestor (use last_hashes for that)

ethcore/src/blockchain/blockchain.rs

@@ -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())
 	}
@@ -227,6 +227,24 @@ impl BlockProvider for BlockChain {
 
 const COLLECTION_QUEUE_SIZE: usize = 8;
 
+pub struct AncestryIter<'a> {
+	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 {
@@ -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.
@@ -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() {

ethcore/src/client.rs

@@ -17,6 +17,7 @@
 //! Blockchain database client.
 
 use std::marker::PhantomData;
+use std::sync::atomic::AtomicBool;
 use util::*;
 use util::panics::*;
 use blockchain::{BlockChain, BlockProvider};
@@ -212,6 +213,7 @@ pub struct Client<V = CanonVerifier> where V: Verifier {
 	panic_handler: Arc<PanicHandler>,
 
 	// for sealing...
+	sealing_enabled: AtomicBool,
 	sealing_block: Mutex<Option<ClosedBlock>>,
 	author: RwLock<Address>,
 	extra_data: RwLock<Bytes>,
@@ -263,6 +265,7 @@ impl<V> Client<V> where V: Verifier {
 			report: RwLock::new(Default::default()),
 			import_lock: Mutex::new(()),
 			panic_handler: panic_handler,
+			sealing_enabled: AtomicBool::new(false),
 			sealing_block: Mutex::new(None),
 			author: RwLock::new(Address::new()),
 			extra_data: RwLock::new(Vec::new()),
@@ -410,7 +413,7 @@ impl<V> Client<V> where V: Verifier {
 			}
 		}
 
-		if self.chain_info().best_block_hash != original_best {
+		if self.chain_info().best_block_hash != original_best && self.sealing_enabled.load(atomic::Ordering::Relaxed) {
 			self.prepare_sealing();
 		}
 
@@ -493,7 +496,7 @@ impl<V> Client<V> where V: Verifier {
 			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().take(self.engine.deref().deref().maximum_uncle_count()).foreach(|h| { b.push_uncle(h).unwrap(); });
 
 		// TODO: push transactions.
 
@@ -505,6 +508,8 @@ impl<V> Client<V> where V: Verifier {
 	/// Grab the `ClosedBlock` that we want to be sealed. Comes as a mutex that you have to lock.
 	pub fn sealing_block(&self) -> &Mutex<Option<ClosedBlock>> {
 		if self.sealing_block.lock().unwrap().is_none() {
+			self.sealing_enabled.store(true, atomic::Ordering::Relaxed);
+			// TODO: Above should be on a timer that resets after two blocks have arrived without being asked for.
 			self.prepare_sealing();
 		}
 		&self.sealing_block

ethcore/src/engine.rs

@@ -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 }
 	/// The nonce with which accounts begin.
 	fn account_start_nonce(&self) -> U256 { decode(&self.spec().engine_params.get("accountStartNonce").unwrap()) }
 

ethcore/src/header.rs

@@ -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.
@@ -70,6 +70,25 @@ pub struct Header {
 	pub bare_hash: RefCell<Option<H256>>,
 }
 
+impl PartialEq for Header {
+	fn eq(&self, c: &Header) -> bool {
+		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 {

ethcore/src/verification/verification.rs

@@ -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());
@@ -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 {