Merge pull request #3719 from ethcore/engine-block-ordering

Engine block ordering

ethcore/src/blockchain/blockchain.rs

@@ -34,6 +34,7 @@ use blockchain::update::ExtrasUpdate;
 use blockchain::{CacheSize, ImportRoute, Config};
 use db::{self, Writable, Readable, CacheUpdatePolicy};
 use cache_manager::CacheManager;
+use engines::Engine;
 
 const LOG_BLOOMS_LEVELS: usize = 3;
 const LOG_BLOOMS_ELEMENTS_PER_INDEX: usize = 16;
@@ -198,6 +199,9 @@ pub struct BlockChain {
 	pending_block_hashes: RwLock<HashMap<BlockNumber, H256>>,
 	pending_block_details: RwLock<HashMap<H256, BlockDetails>>,
 	pending_transaction_addresses: RwLock<HashMap<H256, Option<TransactionAddress>>>,
+
+	// Used for block ordering.
+	engine: Arc<Engine>,
 }
 
 impl BlockProvider for BlockChain {
@@ -415,9 +419,8 @@ impl<'a> Iterator for AncestryIter<'a> {
 }
 
 impl BlockChain {
-	#[cfg_attr(feature="dev", allow(useless_let_if_seq))]
+	/// Create new instance of blockchain from given Genesis and block picking rules of Engine.
-	/// Create new instance of blockchain from given Genesis
+	pub fn new(config: Config, genesis: &[u8], db: Arc<Database>, engine: Arc<Engine>) -> BlockChain {
-	pub fn new(config: Config, genesis: &[u8], db: Arc<Database>) -> BlockChain {
 		// 400 is the avarage size of the key
 		let cache_man = CacheManager::new(config.pref_cache_size, config.max_cache_size, 400);
 
@@ -442,6 +445,7 @@ impl BlockChain {
 			pending_block_hashes: RwLock::new(HashMap::new()),
 			pending_block_details: RwLock::new(HashMap::new()),
 			pending_transaction_addresses: RwLock::new(HashMap::new()),
+			engine: engine,
 		};
 
 		// load best block
@@ -858,13 +862,12 @@ impl BlockChain {
 		let number = header.number();
 		let parent_hash = header.parent_hash();
 		let parent_details = self.block_details(&parent_hash).unwrap_or_else(|| panic!("Invalid parent hash: {:?}", parent_hash));
-		let total_difficulty = parent_details.total_difficulty + header.difficulty();
+		let is_new_best = self.engine.is_new_best_block(self.best_block_total_difficulty(), HeaderView::new(&self.best_block_header()), &parent_details, header);
-		let is_new_best = total_difficulty > self.best_block_total_difficulty();
 
 		BlockInfo {
 			hash: hash,
 			number: number,
-			total_difficulty: total_difficulty,
+			total_difficulty: parent_details.total_difficulty + header.difficulty(),
 			location: if is_new_best {
 				// on new best block we need to make sure that all ancestors
 				// are moved to "canon chain"
@@ -1319,11 +1322,16 @@ mod tests {
 	use views::BlockView;
 	use transaction::{Transaction, Action};
 	use log_entry::{LogEntry, LocalizedLogEntry};
+	use spec::Spec;
 
 	fn new_db(path: &str) -> Arc<Database> {
 		Arc::new(Database::open(&DatabaseConfig::with_columns(::db::NUM_COLUMNS), path).unwrap())
 	}
 
+	fn new_chain(genesis: &[u8], db: Arc<Database>) -> BlockChain {
+		BlockChain::new(Config::default(), genesis, db, Spec::new_null().engine) 
+	}
+
 	#[test]
 	fn should_cache_best_block() {
 		// given
@@ -1334,7 +1342,7 @@ mod tests {
 
 		let temp = RandomTempPath::new();
 		let db = new_db(temp.as_str());
-		let bc = BlockChain::new(Config::default(), &genesis, db.clone());
+		let bc = new_chain(&genesis, db.clone());
 		assert_eq!(bc.best_block_number(), 0);
 
 		// when
@@ -1360,7 +1368,7 @@ mod tests {
 
 		let temp = RandomTempPath::new();
 		let db = new_db(temp.as_str());
-		let bc = BlockChain::new(Config::default(), &genesis, db.clone());
+		let bc = new_chain(&genesis, db.clone());
 
 		assert_eq!(bc.genesis_hash(), genesis_hash.clone());
 		assert_eq!(bc.best_block_hash(), genesis_hash.clone());
@@ -1391,7 +1399,7 @@ mod tests {
 
 		let temp = RandomTempPath::new();
 		let db = new_db(temp.as_str());
-		let bc = BlockChain::new(Config::default(), &genesis, db.clone());
+		let bc = new_chain(&genesis, db.clone());
 
 		let mut block_hashes = vec![genesis_hash.clone()];
 		let mut batch = db.transaction();
@@ -1427,7 +1435,7 @@ mod tests {
 
 		let temp = RandomTempPath::new();
 		let db = new_db(temp.as_str());
-		let bc = BlockChain::new(Config::default(), &genesis, db.clone());
+		let bc = new_chain(&genesis, db.clone());
 
 		let mut batch =db.transaction();
 		for b in &[&b1a, &b1b, &b2a, &b2b, &b3a, &b3b, &b4a, &b4b, &b5a, &b5b] {
@@ -1489,7 +1497,7 @@ mod tests {
 
 		let temp = RandomTempPath::new();
 		let db = new_db(temp.as_str());
-		let bc = BlockChain::new(Config::default(), &genesis, db.clone());
+		let bc = new_chain(&genesis, db.clone());
 
 		let mut batch = db.transaction();
 		let _ = bc.insert_block(&mut batch, &b1a, vec![]);
@@ -1577,7 +1585,7 @@ mod tests {
 
 		let temp = RandomTempPath::new();
 		let db = new_db(temp.as_str());
-		let bc = BlockChain::new(Config::default(), &genesis, db.clone());
+		let bc = new_chain(&genesis, db.clone());
 
 		let mut batch = db.transaction();
 		let _ = bc.insert_block(&mut batch, &b1a, vec![]);
@@ -1639,7 +1647,7 @@ mod tests {
 
 		let temp = RandomTempPath::new();
 		let db = new_db(temp.as_str());
-		let bc = BlockChain::new(Config::default(), &genesis, db.clone());
+		let bc = new_chain(&genesis, db.clone());
 
 		let mut batch = db.transaction();
 		let ir1 = bc.insert_block(&mut batch, &b1, vec![]);
@@ -1755,7 +1763,7 @@ mod tests {
 		let temp = RandomTempPath::new();
 		{
 			let db = new_db(temp.as_str());
-			let bc = BlockChain::new(Config::default(), &genesis, db.clone());
+			let bc = new_chain(&genesis, db.clone());
 			assert_eq!(bc.best_block_hash(), genesis_hash);
 			let mut batch =db.transaction();
 			bc.insert_block(&mut batch, &first, vec![]);
@@ -1766,7 +1774,7 @@ mod tests {
 
 		{
 			let db = new_db(temp.as_str());
-			let bc = BlockChain::new(Config::default(), &genesis, db.clone());
+			let bc = new_chain(&genesis, db.clone());
 
 			assert_eq!(bc.best_block_hash(), first_hash);
 		}
@@ -1821,7 +1829,7 @@ mod tests {
 
 		let temp = RandomTempPath::new();
 		let db = new_db(temp.as_str());
-		let bc = BlockChain::new(Config::default(), &genesis, db.clone());
+		let bc = new_chain(&genesis, db.clone());
 		let mut batch =db.transaction();
 		bc.insert_block(&mut batch, &b1, vec![]);
 		db.write(batch).unwrap();
@@ -1881,7 +1889,7 @@ mod tests {
 
 		let temp = RandomTempPath::new();
 		let db = new_db(temp.as_str());
-		let bc = BlockChain::new(Config::default(), &genesis, db.clone());
+		let bc = new_chain(&genesis, db.clone());
 		insert_block(&db, &bc, &b1, vec![Receipt {
 			state_root: H256::default(),
 			gas_used: 10_000.into(),
@@ -1985,7 +1993,7 @@ mod tests {
 
 		let temp = RandomTempPath::new();
 		let db = new_db(temp.as_str());
-		let bc = BlockChain::new(Config::default(), &genesis, db.clone());
+		let bc = new_chain(&genesis, db.clone());
 
 		let blocks_b1 = bc.blocks_with_bloom(&bloom_b1, 0, 5);
 		let blocks_b2 = bc.blocks_with_bloom(&bloom_b2, 0, 5);
@@ -2042,7 +2050,7 @@ mod tests {
 
 		{
 			let db = new_db(temp.as_str());
-			let bc = BlockChain::new(Config::default(), &genesis, db.clone());
+			let bc = new_chain(&genesis, db.clone());
 			let uncle = canon_chain.fork(1).generate(&mut finalizer.fork()).unwrap();
 
 			let mut batch =db.transaction();
@@ -2061,7 +2069,7 @@ mod tests {
 
 		// re-loading the blockchain should load the correct best block.
 		let db = new_db(temp.as_str());
-		let bc = BlockChain::new(Config::default(), &genesis, db.clone());
+		let bc = new_chain(&genesis, db.clone());
 		assert_eq!(bc.best_block_number(), 5);
 	}
 
@@ -2078,7 +2086,7 @@ mod tests {
 
 		let temp = RandomTempPath::new();
 		let db = new_db(temp.as_str());
-		let bc = BlockChain::new(Config::default(), &genesis, db.clone());
+		let bc = new_chain(&genesis, db.clone());
 
 		let mut batch =db.transaction();
 		bc.insert_block(&mut batch, &first, vec![]);

ethcore/src/client/client.rs

@@ -164,7 +164,7 @@ impl Client {
 		let gb = spec.genesis_block();
 
 		let db = Arc::new(try!(Database::open(&db_config, &path.to_str().expect("DB path could not be converted to string.")).map_err(ClientError::Database)));
-		let chain = Arc::new(BlockChain::new(config.blockchain.clone(), &gb, db.clone()));
+		let chain = Arc::new(BlockChain::new(config.blockchain.clone(), &gb, db.clone(), spec.engine.clone()));
 		let tracedb = RwLock::new(TraceDB::new(config.tracing.clone(), db.clone(), chain.clone()));
 
 		let trie_spec = match config.fat_db {
@@ -787,7 +787,7 @@ impl snapshot::DatabaseRestore for Client {
 
 		let cache_size = state_db.cache_size();
 		*state_db = StateDB::new(journaldb::new(db.clone(), self.pruning, ::db::COL_STATE), cache_size);
-		*chain = Arc::new(BlockChain::new(self.config.blockchain.clone(), &[], db.clone()));
+		*chain = Arc::new(BlockChain::new(self.config.blockchain.clone(), &[], db.clone(), self.engine.clone()));
 		*tracedb = TraceDB::new(self.config.tracing.clone(), db.clone(), chain.clone());
 		Ok(())
 	}

ethcore/src/engines/authority_round.rs

@@ -21,7 +21,7 @@ use std::sync::Weak;
 use std::time::{UNIX_EPOCH, Duration};
 use util::*;
 use ethkey::{verify_address, Signature};
-use rlp::{UntrustedRlp, View, encode};
+use rlp::{Rlp, UntrustedRlp, View, encode};
 use account_provider::AccountProvider;
 use block::*;
 use spec::CommonParams;
@@ -35,6 +35,8 @@ use service::ClientIoMessage;
 use transaction::SignedTransaction;
 use env_info::EnvInfo;
 use builtin::Builtin;
+use blockchain::extras::BlockDetails;
+use views::HeaderView;
 
 /// `AuthorityRound` params.
 #[derive(Debug, PartialEq)]
@@ -272,7 +274,6 @@ impl Engine for AuthorityRound {
 	}
 
 	fn verify_block_family(&self, header: &Header, parent: &Header, _block: Option<&[u8]>) -> Result<(), Error> {
-		// Don't calculate difficulty for genesis blocks.
 		if header.number() == 0 {
 			return Err(From::from(BlockError::RidiculousNumber(OutOfBounds { min: Some(1), max: None, found: header.number() })));
 		}
@@ -284,10 +285,6 @@ impl Engine for AuthorityRound {
 			try!(Err(BlockError::DoubleVote(header.author().clone())));
 		}
 
-		// Check difficulty is correct given the two timestamps.
-		if header.difficulty() != parent.difficulty() {
-			return Err(From::from(BlockError::InvalidDifficulty(Mismatch { expected: *parent.difficulty(), found: *header.difficulty() })))
-		}
 		let gas_limit_divisor = self.our_params.gas_limit_bound_divisor;
 		let min_gas = parent.gas_limit().clone() - parent.gas_limit().clone() / gas_limit_divisor;
 		let max_gas = parent.gas_limit().clone() + parent.gas_limit().clone() / gas_limit_divisor;
@@ -310,6 +307,19 @@ impl Engine for AuthorityRound {
 		let mut guard = self.message_channel.lock();
 		*guard = Some(message_channel);
 	}
+
+	fn is_new_best_block(&self, _best_total_difficulty: U256, best_header: HeaderView, _parent_details: &BlockDetails, new_header: &HeaderView) -> bool {
+		let new_number = new_header.number();
+		let best_number = best_header.number();
+		if new_number != best_number {
+			new_number > best_number
+		} else {
+			// Take the oldest step at given height.
+			let new_step: usize = Rlp::new(&new_header.seal()[0]).as_val();
+			let best_step: usize = Rlp::new(&best_header.seal()[0]).as_val();
+			new_step < best_step
+		}
+	}
 }
 
 #[cfg(test)]

ethcore/src/engines/mod.rs

@@ -38,6 +38,9 @@ use io::IoChannel;
 use service::ClientIoMessage;
 use header::Header;
 use transaction::SignedTransaction;
+use ethereum::ethash;
+use blockchain::extras::BlockDetails;
+use views::HeaderView;
 
 /// A consensus mechanism for the chain. Generally either proof-of-work or proof-of-stake-based.
 /// Provides hooks into each of the major parts of block import.
@@ -146,5 +149,9 @@ pub trait Engine : Sync + Send {
 
 	/// Add a channel for communication with Client which can be used for sealing.
 	fn register_message_channel(&self, _message_channel: IoChannel<ClientIoMessage>) {}
-	// TODO: sealing stuff - though might want to leave this for later.
+
+	/// Check if new block should be chosen as the one  in chain.
+	fn is_new_best_block(&self, best_total_difficulty: U256, _best_header: HeaderView, parent_details: &BlockDetails, new_header: &HeaderView) -> bool {
+		ethash::is_new_best_block(best_total_difficulty, parent_details, new_header)
+	}
 }

ethcore/src/engines/null_engine.rs

@@ -38,6 +38,12 @@ impl NullEngine {
 	}
 }
 
+impl Default for NullEngine {
+	fn default() -> Self {
+		Self::new(Default::default(), Default::default())
+	}
+}
+
 impl Engine for NullEngine {
 	fn name(&self) -> &str {
 		"NullEngine"

ethcore/src/ethereum/ethash.rs

@@ -21,6 +21,7 @@ use builtin::Builtin;
 use env_info::EnvInfo;
 use error::{BlockError, TransactionError, Error};
 use header::Header;
+use views::HeaderView;
 use state::CleanupMode;
 use spec::CommonParams;
 use transaction::SignedTransaction;
@@ -28,6 +29,7 @@ use engines::Engine;
 use evm::Schedule;
 use ethjson;
 use rlp::{self, UntrustedRlp, View};
+use blockchain::extras::BlockDetails;
 
 /// Ethash params.
 #[derive(Debug, PartialEq)]
@@ -325,6 +327,15 @@ impl Engine for Ethash {
 	fn verify_transaction(&self, t: &SignedTransaction, _header: &Header) -> Result<(), Error> {
 		t.sender().map(|_|()) // Perform EC recovery and cache sender
 	}
+
+	fn is_new_best_block(&self, best_total_difficulty: U256, _best_header: HeaderView, parent_details: &BlockDetails, new_header: &HeaderView) -> bool {
+		is_new_best_block(best_total_difficulty, parent_details, new_header)
+	}
+}
+
+/// Check if a new block should replace the best blockchain block.
+pub fn is_new_best_block(best_total_difficulty: U256, parent_details: &BlockDetails, new_header: &HeaderView) -> bool {
+	parent_details.total_difficulty + new_header.difficulty() > best_total_difficulty
 }
 
 #[cfg_attr(feature="dev", allow(wrong_self_convention))]

ethcore/src/snapshot/service.rs

@@ -81,6 +81,7 @@ struct Restoration {
 struct RestorationParams<'a> {
 	manifest: ManifestData, // manifest to base restoration on.
 	pruning: Algorithm, // pruning algorithm for the database.
+	engine: Arc<Engine>, // consensus engine of the chain.
 	db_path: PathBuf, // database path
 	db_config: &'a DatabaseConfig, // configuration for the database.
 	writer: Option<LooseWriter>, // writer for recovered snapshot.
@@ -99,7 +100,7 @@ impl Restoration {
 		let raw_db = Arc::new(try!(Database::open(params.db_config, &*params.db_path.to_string_lossy())
 			.map_err(UtilError::SimpleString)));
 
-		let chain = BlockChain::new(Default::default(), params.genesis, raw_db.clone());
+		let chain = BlockChain::new(Default::default(), params.genesis, raw_db.clone(), params.engine);
 		let blocks = try!(BlockRebuilder::new(chain, raw_db.clone(), &manifest));
 
 		let root = manifest.state_root.clone();
@@ -420,6 +421,7 @@ impl Service {
 		let params = RestorationParams {
 			manifest: manifest,
 			pruning: self.pruning,
+			engine: self.engine.clone(),
 			db_path: self.restoration_db(),
 			db_config: &self.db_config,
 			writer: writer,

ethcore/src/snapshot/tests/blocks.rs

@@ -37,13 +37,14 @@ fn chunk_and_restore(amount: u64) {
 	let genesis = canon_chain.generate(&mut finalizer).unwrap();
 	let db_cfg = DatabaseConfig::with_columns(::db::NUM_COLUMNS);
 
+	let engine = Arc::new(::engines::NullEngine::default());
 	let orig_path = RandomTempPath::create_dir();
 	let new_path = RandomTempPath::create_dir();
 	let mut snapshot_path = new_path.as_path().to_owned();
 	snapshot_path.push("SNAP");
 
 	let old_db = Arc::new(Database::open(&db_cfg, orig_path.as_str()).unwrap());
-	let bc = BlockChain::new(Default::default(), &genesis, old_db.clone());
+	let bc = BlockChain::new(Default::default(), &genesis, old_db.clone(), engine.clone());
 
 	// build the blockchain.
 	let mut batch = old_db.transaction();
@@ -73,21 +74,20 @@ fn chunk_and_restore(amount: u64) {
 
 	// restore it.
 	let new_db = Arc::new(Database::open(&db_cfg, new_path.as_str()).unwrap());
-	let new_chain = BlockChain::new(Default::default(), &genesis, new_db.clone());
+	let new_chain = BlockChain::new(Default::default(), &genesis, new_db.clone(), engine.clone());
 	let mut rebuilder = BlockRebuilder::new(new_chain, new_db.clone(), &manifest).unwrap();
 	let reader = PackedReader::new(&snapshot_path).unwrap().unwrap();
-	let engine = ::engines::NullEngine::new(Default::default(), Default::default());
 	let flag = AtomicBool::new(true);
 	for chunk_hash in &reader.manifest().block_hashes {
 		let compressed = reader.chunk(*chunk_hash).unwrap();
 		let chunk = snappy::decompress(&compressed).unwrap();
-		rebuilder.feed(&chunk, &engine, &flag).unwrap();
+		rebuilder.feed(&chunk, engine.as_ref(), &flag).unwrap();
 	}
 
 	rebuilder.finalize(HashMap::new()).unwrap();
 
 	// and test it.
-	let new_chain = BlockChain::new(Default::default(), &genesis, new_db);
+	let new_chain = BlockChain::new(Default::default(), &genesis, new_db, engine);
 	assert_eq!(new_chain.best_block_hash(), best_hash);
 }
 
@@ -121,8 +121,8 @@ fn checks_flag() {
 
 	let db_cfg = DatabaseConfig::with_columns(::db::NUM_COLUMNS);
 	let db = Arc::new(Database::open(&db_cfg, path.as_str()).unwrap());
-	let chain = BlockChain::new(Default::default(), &genesis, db.clone());
+	let engine = Arc::new(::engines::NullEngine::default());
-	let engine = ::engines::NullEngine::new(Default::default(), Default::default());
+	let chain = BlockChain::new(Default::default(), &genesis, db.clone(), engine.clone());
 
 	let manifest = ::snapshot::ManifestData {
 		state_hashes: Vec::new(),
@@ -134,7 +134,7 @@ fn checks_flag() {
 
 	let mut rebuilder = BlockRebuilder::new(chain, db.clone(), &manifest).unwrap();
 
-	match rebuilder.feed(&chunk, &engine, &AtomicBool::new(false)) {
+	match rebuilder.feed(&chunk, engine.as_ref(), &AtomicBool::new(false)) {
 		Err(Error::Snapshot(SnapshotError::RestorationAborted)) => {}
 		_ => panic!("Wrong result on abort flag set")
 	}

ethcore/src/spec/spec.rs

@@ -30,8 +30,7 @@ use ethjson;
 use rlp::{Rlp, RlpStream, View, Stream};
 
 /// Parameters common to all engines.
-#[derive(Debug, PartialEq, Clone)]
+#[derive(Debug, PartialEq, Clone, Default)]
-#[cfg_attr(test, derive(Default))]
 pub struct CommonParams {
 	/// Account start nonce.
 	pub account_start_nonce: U256,

ethcore/src/tests/helpers.rs

@@ -286,7 +286,7 @@ fn new_db(path: &str) -> Arc<Database> {
 pub fn generate_dummy_blockchain(block_number: u32) -> GuardedTempResult<BlockChain> {
 	let temp = RandomTempPath::new();
 	let db = new_db(temp.as_str());
-	let bc = BlockChain::new(BlockChainConfig::default(), &create_unverifiable_block(0, H256::zero()), db.clone());
+	let bc = BlockChain::new(BlockChainConfig::default(), &create_unverifiable_block(0, H256::zero()), db.clone(), Spec::new_null().engine);
 
 	let mut batch = db.transaction();
 	for block_order in 1..block_number {
@@ -304,7 +304,7 @@ pub fn generate_dummy_blockchain(block_number: u32) -> GuardedTempResult<BlockCh
 pub fn generate_dummy_blockchain_with_extra(block_number: u32) -> GuardedTempResult<BlockChain> {
 	let temp = RandomTempPath::new();
 	let db = new_db(temp.as_str());
-	let bc = BlockChain::new(BlockChainConfig::default(), &create_unverifiable_block(0, H256::zero()), db.clone());
+	let bc = BlockChain::new(BlockChainConfig::default(), &create_unverifiable_block(0, H256::zero()), db.clone(), Spec::new_null().engine);
 
 
 	let mut batch = db.transaction();
@@ -323,7 +323,7 @@ pub fn generate_dummy_blockchain_with_extra(block_number: u32) -> GuardedTempRes
 pub fn generate_dummy_empty_blockchain() -> GuardedTempResult<BlockChain> {
 	let temp = RandomTempPath::new();
 	let db = new_db(temp.as_str());
-	let bc = BlockChain::new(BlockChainConfig::default(), &create_unverifiable_block(0, H256::zero()), db.clone());
+	let bc = BlockChain::new(BlockChainConfig::default(), &create_unverifiable_block(0, H256::zero()), db.clone(), Spec::new_null().engine);
 
 	GuardedTempResult::<BlockChain> {
 		_temp: temp,