Calculate post-constructors state root in spec at load time (#5523)

* apply contract constructors at spec load time

* debugging

* do trie updates in two stages

* add lint exception to newly-dead constructor

* squash warning in RPC tests

* kill dead accountdb constructors

* restore accountdb constructors under test configuration

* use provided state root in spec, if it exists

ethcore/src/account_db.rs

@@ -78,6 +78,7 @@ pub struct AccountDB<'db> {
 
 impl<'db> AccountDB<'db> {
 	/// Create a new AccountDB from an address.
+	#[cfg(test)]
 	pub fn new(db: &'db HashDB, address: &Address) -> Self {
 		Self::from_hash(db, address.sha3())
 	}
@@ -131,6 +132,7 @@ pub struct AccountDBMut<'db> {
 
 impl<'db> AccountDBMut<'db> {
 	/// Create a new AccountDB from an address.
+	#[cfg(test)]
 	pub fn new(db: &'db mut HashDB, address: &Address) -> Self {
 		Self::from_hash(db, address.sha3())
 	}
@@ -143,7 +145,7 @@ impl<'db> AccountDBMut<'db> {
 		}
 	}
 
-	#[allow(dead_code)]
+	#[cfg(test)]
 	pub fn immutable(&'db self) -> AccountDB<'db> {
 		AccountDB { db: self.db, address_hash: self.address_hash.clone() }
 	}

ethcore/src/client/client.rs

@@ -166,8 +166,7 @@ impl Client {
 		db: Arc<KeyValueDB>,
 		miner: Arc<Miner>,
 		message_channel: IoChannel<ClientIoMessage>,
-	) -> Result<Arc<Client>, ClientError> {
+	) -> Result<Arc<Client>, ::error::Error> {
-
 		let trie_spec = match config.fat_db {
 			true => TrieSpec::Fat,
 			false => TrieSpec::Secure,

ethcore/src/ethereum/mod.rs

@@ -95,6 +95,7 @@ mod tests {
 		let engine = &spec.engine;
 		let genesis_header = spec.genesis_header();
 		let db = spec.ensure_db_good(get_temp_state_db(), &Default::default()).unwrap();
+
 		let s = State::from_existing(db, genesis_header.state_root().clone(), engine.account_start_nonce(), Default::default()).unwrap();
 		assert_eq!(s.balance(&"0000000000000000000000000000000000000001".into()).unwrap(), 1u64.into());
 		assert_eq!(s.balance(&"0000000000000000000000000000000000000002".into()).unwrap(), 1u64.into());

ethcore/src/json_tests/chain.rs

@@ -51,7 +51,7 @@ pub fn json_chain_test(json_data: &[u8], era: ChainEra) -> Vec<String> {
 					ChainEra::_Eip161 => ethereum::new_eip161_test(),
 					ChainEra::TransitionTest => ethereum::new_transition_test(),
 				};
-				spec.set_genesis_state(state);
+				spec.set_genesis_state(state).expect("Failed to overwrite genesis state");
 				spec.overwrite_genesis_params(genesis);
 				assert!(spec.is_state_root_valid());
 				spec

ethcore/src/pod_account.rs

@@ -16,7 +16,6 @@
 
 use util::*;
 use state::Account;
-use account_db::AccountDBMut;
 use ethjson;
 use types::account_diff::*;
 use rlp::{self, RlpStream};
@@ -64,7 +63,7 @@ impl PodAccount {
 	}
 
 	/// Place additional data into given hash DB.
-	pub fn insert_additional(&self, db: &mut AccountDBMut, factory: &TrieFactory) {
+	pub fn insert_additional(&self, db: &mut HashDB, factory: &TrieFactory) {
 		match self.code {
 			Some(ref c) if !c.is_empty() => { db.insert(c); }
 			_ => {}

ethcore/src/spec/spec.rs

@@ -16,25 +16,27 @@
 
 //! Parameters for a block chain.
 
-use util::*;
-use builtin::Builtin;
-use engines::{Engine, NullEngine, InstantSeal, BasicAuthority, AuthorityRound, Tendermint};
-use factory::Factories;
-use executive::Executive;
-use trace::{NoopTracer, NoopVMTracer};
-use action_params::{ActionValue, ActionParams};
-use types::executed::CallType;
-use state::{Backend, State, Substate};
-use env_info::EnvInfo;
-use pod_state::*;
-use account_db::*;
-use header::{BlockNumber, Header};
-use state_db::StateDB;
 use super::genesis::Genesis;
 use super::seal::Generic as GenericSeal;
+
+use action_params::{ActionValue, ActionParams};
+use builtin::Builtin;
+use engines::{Engine, NullEngine, InstantSeal, BasicAuthority, AuthorityRound, Tendermint};
+use env_info::EnvInfo;
+use error::Error;
 use ethereum;
 use ethjson;
+use executive::Executive;
+use factory::Factories;
+use header::{BlockNumber, Header};
+use pod_state::*;
 use rlp::{Rlp, RlpStream};
+use state_db::StateDB;
+use state::{Backend, State, Substate};
+use state::backend::Basic as BasicBackend;
+use trace::{NoopTracer, NoopVMTracer};
+use types::executed::CallType;
+use util::*;
 
 /// Parameters common to all engines.
 #[derive(Debug, PartialEq, Clone, Default)]
@@ -119,39 +121,46 @@ pub struct Spec {
 	constructors: Vec<(Address, Bytes)>,
 
 	/// May be prepopulated if we know this in advance.
-	state_root_memo: RwLock<Option<H256>>,
+	state_root_memo: RwLock<H256>,
 
 	/// Genesis state as plain old data.
 	genesis_state: PodState,
 }
 
-impl From<ethjson::spec::Spec> for Spec {
+fn load_from(s: ethjson::spec::Spec) -> Result<Spec, Error> {
-	fn from(s: ethjson::spec::Spec) -> Self {
+	let builtins = s.accounts.builtins().into_iter().map(|p| (p.0.into(), From::from(p.1))).collect();
-		let builtins = s.accounts.builtins().into_iter().map(|p| (p.0.into(), From::from(p.1))).collect();
+	let g = Genesis::from(s.genesis);
-		let g = Genesis::from(s.genesis);
+	let GenericSeal(seal_rlp) = g.seal.into();
-		let GenericSeal(seal_rlp) = g.seal.into();
+	let params = CommonParams::from(s.params);
-		let params = CommonParams::from(s.params);
+
-		Spec {
+	let mut s = Spec {
-			name: s.name.clone().into(),
+		name: s.name.clone().into(),
-			params: params.clone(),
+		params: params.clone(),
-			engine: Spec::engine(s.engine, params, builtins),
+		engine: Spec::engine(s.engine, params, builtins),
-			data_dir: s.data_dir.unwrap_or(s.name).into(),
+		data_dir: s.data_dir.unwrap_or(s.name).into(),
-			nodes: s.nodes.unwrap_or_else(Vec::new),
+		nodes: s.nodes.unwrap_or_else(Vec::new),
-			parent_hash: g.parent_hash,
+		parent_hash: g.parent_hash,
-			transactions_root: g.transactions_root,
+		transactions_root: g.transactions_root,
-			receipts_root: g.receipts_root,
+		receipts_root: g.receipts_root,
-			author: g.author,
+		author: g.author,
-			difficulty: g.difficulty,
+		difficulty: g.difficulty,
-			gas_limit: g.gas_limit,
+		gas_limit: g.gas_limit,
-			gas_used: g.gas_used,
+		gas_used: g.gas_used,
-			timestamp: g.timestamp,
+		timestamp: g.timestamp,
-			extra_data: g.extra_data,
+		extra_data: g.extra_data,
-			seal_rlp: seal_rlp,
+		seal_rlp: seal_rlp,
-			constructors: s.accounts.constructors().into_iter().map(|(a, c)| (a.into(), c.into())).collect(),
+		constructors: s.accounts.constructors().into_iter().map(|(a, c)| (a.into(), c.into())).collect(),
-			state_root_memo: RwLock::new(g.state_root),
+		state_root_memo: RwLock::new(Default::default()), // will be overwritten right after.
-			genesis_state: From::from(s.accounts),
+		genesis_state: s.accounts.into(),
-		}
+	};
+
+	// use memoized state root if provided.
+	match g.state_root {
+		Some(root) => *s.state_root_memo.get_mut() = root,
+		None => { let _ = s.run_constructors(&Default::default(), BasicBackend(MemoryDB::new()))?; },
 	}
+
+	Ok(s)
 }
 
 macro_rules! load_bundled {
@@ -174,13 +183,93 @@ impl Spec {
 		}
 	}
 
+	// given a pre-constructor state, run all the given constructors and produce a new state and state root.
+	fn run_constructors<T: Backend>(&self, factories: &Factories, mut db: T) -> Result<T, Error> {
+		let mut root = SHA3_NULL_RLP;
+
+		// basic accounts in spec.
+		{
+			let mut t = factories.trie.create(db.as_hashdb_mut(), &mut root);
+
+			for (address, account) in self.genesis_state.get().iter() {
+				t.insert(&**address, &account.rlp())?;
+			}
+		}
+
+		for (address, account) in self.genesis_state.get().iter() {
+			db.note_non_null_account(address);
+			account.insert_additional(
+				&mut *factories.accountdb.create(db.as_hashdb_mut(), address.sha3()),
+				&factories.trie
+			);
+		}
+
+		let start_nonce = self.engine.account_start_nonce();
+
+		let (root, db) = {
+			let mut state = State::from_existing(
+				db,
+				root,
+				start_nonce,
+				factories.clone(),
+			)?;
+
+			// Execute contract constructors.
+			let env_info = EnvInfo {
+				number: 0,
+				author: self.author,
+				timestamp: self.timestamp,
+				difficulty: self.difficulty,
+				last_hashes: Default::default(),
+				gas_used: U256::zero(),
+				gas_limit: U256::max_value(),
+			};
+
+			let from = Address::default();
+			for &(ref address, ref constructor) in self.constructors.iter() {
+				trace!(target: "spec", "run_constructors: Creating a contract at {}.", address);
+				trace!(target: "spec", "  .. root before = {}", state.root());
+				let params = ActionParams {
+					code_address: address.clone(),
+					code_hash: constructor.sha3(),
+					address: address.clone(),
+					sender: from.clone(),
+					origin: from.clone(),
+					gas: U256::max_value(),
+					gas_price: Default::default(),
+					value: ActionValue::Transfer(Default::default()),
+					code: Some(Arc::new(constructor.clone())),
+					data: None,
+					call_type: CallType::None,
+				};
+
+				let mut substate = Substate::new();
+				state.kill_account(&address);
+
+				{
+					let mut exec = Executive::new(&mut state, &env_info, self.engine.as_ref(), &factories.vm);
+					if let Err(e) = exec.create(params, &mut substate, &mut NoopTracer, &mut NoopVMTracer) {
+						warn!(target: "spec", "Genesis constructor execution at {} failed: {}.", address, e);
+					}
+				}
+
+				if let Err(e) = state.commit() {
+					warn!(target: "spec", "Genesis constructor trie commit at {} failed: {}.", address, e);
+				}
+
+				trace!(target: "spec", "  .. root after = {}", state.root());
+			}
+
+			state.drop()
+		};
+
+		*self.state_root_memo.write() = root;
+		Ok(db)
+	}
+
 	/// Return the state root for the genesis state, memoising accordingly.
 	pub fn state_root(&self) -> H256 {
-		if self.state_root_memo.read().is_none() {
+		self.state_root_memo.read().clone()
-			*self.state_root_memo.write() = Some(self.genesis_state.root());
-		}
-		self.state_root_memo.read().as_ref().cloned()
-			.expect("state root memo ensured to be set at this point; qed")
 	}
 
 	/// Get the known knodes of the network in enode format.
@@ -243,95 +332,46 @@ impl Spec {
 		self.timestamp = g.timestamp;
 		self.extra_data = g.extra_data;
 		self.seal_rlp = seal_rlp;
-		self.state_root_memo = RwLock::new(g.state_root);
 	}
 
 	/// Alter the value of the genesis state.
-	pub fn set_genesis_state(&mut self, s: PodState) {
+	pub fn set_genesis_state(&mut self, s: PodState) -> Result<(), Error> {
 		self.genesis_state = s;
-		*self.state_root_memo.write() = None;
+		let _ = self.run_constructors(&Default::default(), BasicBackend(MemoryDB::new()))?;
+
+		Ok(())
 	}
 
 	/// Returns `false` if the memoized state root is invalid. `true` otherwise.
 	pub fn is_state_root_valid(&self) -> bool {
-		self.state_root_memo.read().clone().map_or(true, |sr| sr == self.genesis_state.root())
+		// TODO: get rid of this function and ensure state root always is valid.
+		// we're mostly there, but `self.genesis_state.root()` doesn't encompass
+		// post-constructor state.
+		*self.state_root_memo.read() == self.genesis_state.root()
 	}
 
 	/// Ensure that the given state DB has the trie nodes in for the genesis state.
-	pub fn ensure_db_good(&self, mut db: StateDB, factories: &Factories) -> Result<StateDB, Box<TrieError>> {
+	pub fn ensure_db_good(&self, db: StateDB, factories: &Factories) -> Result<StateDB, Error> {
 		if db.as_hashdb().contains(&self.state_root()) {
 			return Ok(db)
 		}
-		trace!(target: "spec", "ensure_db_good: Fresh database? Cannot find state root {}", self.state_root());
-		let mut root = H256::new();
 
-		{
+		// TODO: could optimize so we don't re-run, but `ensure_db_good` is barely ever
-			let mut t = factories.trie.create(db.as_hashdb_mut(), &mut root);
+		// called anyway.
-			for (address, account) in self.genesis_state.get().iter() {
+		let db = self.run_constructors(factories, db)?;
-				t.insert(&**address, &account.rlp())?;
-			}
-		}
 
-		trace!(target: "spec", "ensure_db_good: Populated sec trie; root is {}", root);
-		for (address, account) in self.genesis_state.get().iter() {
-			db.note_non_null_account(address);
-			account.insert_additional(&mut AccountDBMut::new(db.as_hashdb_mut(), address), &factories.trie);
-		}
-
-		// Execute contract constructors.
-		let env_info = EnvInfo {
-			number: 0,
-			author: self.author,
-			timestamp: self.timestamp,
-			difficulty: self.difficulty,
-			last_hashes: Default::default(),
-			gas_used: U256::zero(),
-			gas_limit: U256::max_value(),
-		};
-		let from = Address::default();
-		let start_nonce = self.engine.account_start_nonce();
-
-		let mut state = State::from_existing(db, root, start_nonce, factories.clone())?;
-		// Mutate the state with each constructor.
-		for &(ref address, ref constructor) in self.constructors.iter() {
-			trace!(target: "spec", "ensure_db_good: Creating a contract at {}.", address);
-			let params = ActionParams {
-				code_address: address.clone(),
-				code_hash: constructor.sha3(),
-				address: address.clone(),
-				sender: from.clone(),
-				origin: from.clone(),
-				gas: U256::max_value(),
-				gas_price: Default::default(),
-				value: ActionValue::Transfer(Default::default()),
-				code: Some(Arc::new(constructor.clone())),
-				data: None,
-				call_type: CallType::None,
-			};
-			let mut substate = Substate::new();
-			state.kill_account(address);
-			{
-				let mut exec = Executive::new(&mut state, &env_info, self.engine.as_ref(), &factories.vm);
-				if let Err(e) = exec.create(params, &mut substate, &mut NoopTracer, &mut NoopVMTracer) {
-					warn!(target: "spec", "Genesis constructor execution at {} failed: {}.", address, e);
-				}
-			}
-			if let Err(e) = state.commit() {
-				warn!(target: "spec", "Genesis constructor trie commit at {} failed: {}.", address, e);
-			}
-		}
-		let (root, db) = state.drop();
-
-		*self.state_root_memo.write() = Some(root);
 		Ok(db)
 	}
 
-	/// Loads spec from json file.
+	/// Loads spec from json file. Provide factories for executing contracts and ensuring
+	/// storage goes to the right place.
 	pub fn load<R>(reader: R) -> Result<Self, String> where R: Read {
-		match ethjson::spec::Spec::load(reader) {
+		fn fmt<F: ::std::fmt::Display>(f: F) -> String {
-			Ok(spec) => Ok(spec.into()),
+			format!("Spec json is invalid: {}", f)
-			Err(e) => Err(format!("Spec json is invalid: {}", e)),
 		}
+
+		ethjson::spec::Spec::load(reader).map_err(fmt)
+			.and_then(|x| load_from(x).map_err(fmt))
 	}
 
 	/// Create a new Spec which conforms to the Frontier-era Morden chain except that it's a NullEngine consensus.

ethcore/src/state/backend.rs

@@ -206,7 +206,8 @@ impl<H: AsHashDB> Proving<H> {
 		}
 	}
 
-	/// Consume the backend, extracting the gathered proof.
+	/// Consume the backend, extracting the gathered proof in lexicographical order
+	/// by value.
 	pub fn extract_proof(self) -> Vec<DBValue> {
 		self.proof.into_inner().into_iter().collect()
 	}
@@ -221,3 +222,33 @@ impl<H: AsHashDB + Clone> Clone for Proving<H> {
 		}
 	}
 }
+
+/// A basic backend. Just wraps the given database, directly inserting into and deleting from
+/// it. Doesn't cache anything.
+pub struct Basic<H>(pub H);
+
+impl<H: AsHashDB + Send + Sync> Backend for Basic<H> {
+	fn as_hashdb(&self) -> &HashDB {
+		self.0.as_hashdb()
+	}
+
+	fn as_hashdb_mut(&mut self) -> &mut HashDB {
+		self.0.as_hashdb_mut()
+	}
+
+	fn add_to_account_cache(&mut self, _: Address, _: Option<Account>, _: bool) { }
+
+	fn cache_code(&self, _: H256, _: Arc<Vec<u8>>) { }
+
+	fn get_cached_account(&self, _: &Address) -> Option<Option<Account>> { None }
+
+	fn get_cached<F, U>(&self, _: &Address, _: F) -> Option<U>
+		where F: FnOnce(Option<&mut Account>) -> U
+	{
+		None
+	}
+
+	fn get_cached_code(&self, _: &H256) -> Option<Arc<Vec<u8>>> { None }
+	fn note_non_null_account(&self, _: &Address) { }
+	fn is_known_null(&self, _: &Address) -> bool { false }
+}

ethcore/src/state_db.rs

@@ -448,7 +448,8 @@ impl state::Backend for StateDB {
 	fn is_known_null(&self, address: &Address) -> bool {
 		trace!(target: "account_bloom", "Check account bloom: {:?}", address);
 		let bloom = self.account_bloom.lock();
-		!bloom.check(&*address.sha3())
+		let is_null = !bloom.check(&*address.sha3());
+		is_null
 	}
 }
 

rpc/src/v1/tests/eth.rs

@@ -83,7 +83,7 @@ fn make_spec(chain: &BlockChain) -> Spec {
 	let genesis = Genesis::from(chain.genesis());
 	let mut spec = ethereum::new_frontier_test();
 	let state = chain.pre_state.clone().into();
-	spec.set_genesis_state(state);
+	spec.set_genesis_state(state).expect("unable to set genesis state");
 	spec.overwrite_genesis_params(genesis);
 	assert!(spec.is_state_root_valid());
 	spec