checking proofs in safe contract

ethcore/src/client/client.rs

@@ -765,7 +765,7 @@ impl Client {
 							res.map(|(output, proof)| (output, proof.into_iter().map(|x| x.into_vec()).collect()))
 						};
 
-						match (with_state)(&call) {
+						match with_state.generate_proof(&call) {
 							Ok(proof) => proof,
 							Err(e) => {
 								warn!(target: "client", "Failed to generate transition proof for block {}: {}", hash, e);

ethcore/src/engines/mod.rs

@@ -120,12 +120,22 @@ pub type Headers<'a> = Fn(H256) -> Option<Header> + 'a;
 /// Type alias for a function we can query pending transitions by block hash through.
 pub type PendingTransitionStore<'a> = Fn(H256) -> Option<PendingTransition> + 'a;
 
+/// Proof dependent on state.
+pub trait StateDependentProof {
+	/// Generate a proof, given the state.
+	fn generate_proof(&self, caller: &Call) -> Result<Vec<u8>, String>;
+	/// Check a proof generated elsewhere (potentially by a peer).
+	// `engine` needed to check state proofs, while really this should
+	// just be state machine params.
+	fn check_proof(&self, engine: &Engine, proof: &[u8]) -> Result<(), String>;
+}
+
 /// Proof generated on epoch change.
 pub enum Proof {
 	/// Known proof (exctracted from signal)
 	Known(Vec<u8>),
-	/// Extract proof from caller.
+	/// State dependent proof.
-	WithState(Box<Fn(&Call) -> Result<Vec<u8>, String>>),
+	WithState(Box<StateDependentProof>),
 }
 
 /// Generated epoch verifier.

ethcore/src/engines/tendermint/mod.rs

@@ -577,18 +577,35 @@ impl Engine for Tendermint {
 		Ok(())
 	}
 
-	/// Verify validators and gas limit.
+	/// Verify gas limit.
 	fn verify_block_family(&self, header: &Header, parent: &Header, _block: Option<&[u8]>) -> Result<(), Error> {
 		if header.number() == 0 {
 			return Err(BlockError::RidiculousNumber(OutOfBounds { min: Some(1), max: None, found: header.number() }).into());
 		}
 
+		let gas_limit_divisor = self.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;
+		if header.gas_limit() <= &min_gas || header.gas_limit() >= &max_gas {
+			self.validators.report_malicious(header.author(), header.number(), header.number(), Default::default());
+			return Err(BlockError::InvalidGasLimit(OutOfBounds { min: Some(min_gas), max: Some(max_gas), found: header.gas_limit().clone() }).into());
+		}
+
+		Ok(())
+	}
+
+	fn verify_block_external(&self, header: &Header, _block: Option<&[u8]>) -> Result<(), Error> {
 		if let Ok(proposal) = ConsensusMessage::new_proposal(header) {
 			let proposer = proposal.verify()?;
 			if !self.is_authority(&proposer) {
 				return Err(EngineError::NotAuthorized(proposer).into());
 			}
-			self.check_view_proposer(header.parent_hash(), proposal.vote_step.height, proposal.vote_step.view, &proposer)?;
+			self.check_view_proposer(
+				header.parent_hash(),
+				proposal.vote_step.height,
+				proposal.vote_step.view,
+				&proposer
+			).map_err(Into::into)
 		} else {
 			let vote_step = VoteStep::new(header.number() as usize, consensus_view(header)?, Step::Precommit);
 			let precommit_hash = message_hash(vote_step.clone(), header.bare_hash());
@@ -614,18 +631,8 @@ impl Engine for Tendermint {
 				}
 			}
 
-			self.check_above_threshold(origins.len())?
+			self.check_above_threshold(origins.len()).map_err(Into::into)
 		}
-
-		let gas_limit_divisor = self.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;
-		if header.gas_limit() <= &min_gas || header.gas_limit() >= &max_gas {
-			self.validators.report_malicious(header.author(), header.number(), header.number(), Default::default());
-			return Err(BlockError::InvalidGasLimit(OutOfBounds { min: Some(min_gas), max: Some(max_gas), found: header.gas_limit().clone() }).into());
-		}
-
-		Ok(())
 	}
 
 	fn signals_epoch_end(&self, header: &Header, block: Option<&[u8]>, receipts: Option<&[::receipt::Receipt]>)
@@ -892,14 +899,14 @@ mod tests {
 		let seal = proposal_seal(&tap, &header, 0);
 		header.set_seal(seal);
 		// Good proposer.
-		assert!(engine.verify_block_family(&header, &parent_header, None).is_ok());
+		assert!(engine.verify_block_external(&header, None).is_ok());
 
 		let validator = insert_and_unlock(&tap, "0");
 		header.set_author(validator);
 		let seal = proposal_seal(&tap, &header, 0);
 		header.set_seal(seal);
 		// Bad proposer.
-		match engine.verify_block_family(&header, &parent_header, None) {
+		match engine.verify_block_external(&header, None) {
 			Err(Error::Engine(EngineError::NotProposer(_))) => {},
 			_ => panic!(),
 		}
@@ -909,7 +916,7 @@ mod tests {
 		let seal = proposal_seal(&tap, &header, 0);
 		header.set_seal(seal);
 		// Not authority.
-		match engine.verify_block_family(&header, &parent_header, None) {
+		match engine.verify_block_external(&header, None) {
 			Err(Error::Engine(EngineError::NotAuthorized(_))) => {},
 			_ => panic!(),
 		};
@@ -939,7 +946,7 @@ mod tests {
 		header.set_seal(seal.clone());
 
 		// One good signature is not enough.
-		match engine.verify_block_family(&header, &parent_header, None) {
+		match engine.verify_block_external(&header, None) {
 			Err(Error::Engine(EngineError::BadSealFieldSize(_))) => {},
 			_ => panic!(),
 		}
@@ -950,7 +957,7 @@ mod tests {
 		seal[2] = ::rlp::encode_list(&vec![H520::from(signature1.clone()), H520::from(signature0.clone())]).into_vec();
 		header.set_seal(seal.clone());
 
-		assert!(engine.verify_block_family(&header, &parent_header, None).is_ok());
+		assert!(engine.verify_block_external(&header, None).is_ok());
 
 		let bad_voter = insert_and_unlock(&tap, "101");
 		let bad_signature = tap.sign(bad_voter, None, vote_info.sha3()).unwrap();
@@ -959,7 +966,7 @@ mod tests {
 		header.set_seal(seal);
 
 		// One good and one bad signature.
-		match engine.verify_block_family(&header, &parent_header, None) {
+		match engine.verify_block_external(&header, None) {
 			Err(Error::Engine(EngineError::NotAuthorized(_))) => {},
 			_ => panic!(),
 		};

ethcore/src/engines/validator_set/safe_contract.rs

@@ -44,6 +44,29 @@ lazy_static! {
 	static ref EVENT_NAME_HASH: H256 = EVENT_NAME.sha3();
 }
 
+// state-dependent proofs for the safe contract:
+// only "first" proofs are such.
+struct StateProof {
+	header: Header,
+	provider: Provider,
+}
+
+impl ::engines::StateDependentProof for StateProof {
+	fn generate_proof(&self, caller: &Call) -> Result<Vec<u8>, String> {
+		prove_initial(&self.provider, &self.header, caller)
+	}
+
+	fn check_proof(&self, engine: &Engine, proof: &[u8]) -> Result<(), String> {
+		let (header, state_items) = decode_first_proof(&UntrustedRlp::new(proof))
+			.map_err(|e| format!("proof incorrectly encoded: {}", e))?;
+		if header != self.header {
+			return Err("wrong header in proof".into());
+		}
+
+		check_first_proof(engine, &self.provider, header, &state_items).map(|_| ())
+	}
+}
+
 /// The validator contract should have the following interface:
 pub struct ValidatorSafeContract {
 	pub address: Address,
@@ -63,6 +86,59 @@ fn encode_first_proof(header: &Header, state_items: &[Vec<u8>]) -> Bytes {
 	stream.out()
 }
 
+// check a first proof: fetch the validator set at the given block.
+fn check_first_proof(engine: &Engine, provider: &Provider, old_header: Header, state_items: &[DBValue])
+	-> Result<Vec<Address>, String>
+{
+	use transaction::{Action, Transaction};
+
+	// TODO: match client contract_call_tx more cleanly without duplication.
+	const PROVIDED_GAS: u64 = 50_000_000;
+
+	let env_info = ::evm::env_info::EnvInfo {
+		number: old_header.number(),
+		author: *old_header.author(),
+		difficulty: *old_header.difficulty(),
+		gas_limit: PROVIDED_GAS.into(),
+		timestamp: old_header.timestamp(),
+		last_hashes: {
+			// this will break if we don't inclue all 256 last hashes.
+			let mut last_hashes: Vec<_> = (0..256).map(|_| H256::default()).collect();
+			last_hashes[255] = *old_header.parent_hash();
+			Arc::new(last_hashes)
+		},
+		gas_used: 0.into(),
+	};
+
+	// check state proof using given engine.
+	let number = old_header.number();
+	provider.get_validators(move |a, d| {
+		let from = Address::default();
+		let tx = Transaction {
+			nonce: engine.account_start_nonce(number),
+			action: Action::Call(a),
+			gas: PROVIDED_GAS.into(),
+			gas_price: U256::default(),
+			value: U256::default(),
+			data: d,
+		}.fake_sign(from);
+
+		let res = ::state::check_proof(
+			state_items,
+			*old_header.state_root(),
+			&tx,
+			engine,
+			&env_info,
+		);
+
+		match res {
+			::state::ProvedExecution::BadProof => Err("Bad proof".into()),
+			::state::ProvedExecution::Failed(e) => Err(format!("Failed call: {}", e)),
+			::state::ProvedExecution::Complete(e) => Ok(e.output),
+		}
+	}).wait()
+}
+
 fn decode_first_proof(rlp: &UntrustedRlp) -> Result<(Header, Vec<DBValue>), ::error::Error> {
 	let header = rlp.val_at(0)?;
 	let state_items = rlp.at(1)?.iter().map(|x| {
@@ -100,8 +176,7 @@ fn prove_initial(provider: &Provider, header: &Header, caller: &Call) -> Result<
 			Ok(result)
 		};
 
-		provider.get_validators(caller)
+		provider.get_validators(caller).wait()
-			.wait()
 	};
 
 	res.map(|validators| {
@@ -255,9 +330,11 @@ impl ValidatorSet for ValidatorSafeContract {
 		// transition to the first block of a contract requires finality but has no log event.
 		if first {
 			debug!(target: "engine", "signalling transition to fresh contract.");
-			let (provider, header) = (self.provider.clone(), header.clone());
+			let state_proof = Box::new(StateProof {
-			let with_caller: Box<Fn(&Call) -> _> = Box::new(move |caller| prove_initial(&provider, &header, caller));
+				header: header.clone(),
-			return ::engines::EpochChange::Yes(::engines::Proof::WithState(with_caller))
+				provider: self.provider.clone(),
+			});
+			return ::engines::EpochChange::Yes(::engines::Proof::WithState(state_proof as Box<_>));
 		}
 
 		// otherwise, we're checking for logs.
@@ -286,61 +363,16 @@ impl ValidatorSet for ValidatorSafeContract {
 	fn epoch_set(&self, first: bool, engine: &Engine, _number: ::header::BlockNumber, proof: &[u8])
 		-> Result<(SimpleList, Option<H256>), ::error::Error>
 	{
-		use transaction::{Action, Transaction};
-
 		let rlp = UntrustedRlp::new(proof);
 
 		if first {
 			trace!(target: "engine", "Recovering initial epoch set");
 
-			// TODO: match client contract_call_tx more cleanly without duplication.
-			const PROVIDED_GAS: u64 = 50_000_000;
-
 			let (old_header, state_items) = decode_first_proof(&rlp)?;
-			let old_hash = old_header.hash();
-
-			let env_info = ::evm::env_info::EnvInfo {
-				number: old_header.number(),
-				author: *old_header.author(),
-				difficulty: *old_header.difficulty(),
-				gas_limit: PROVIDED_GAS.into(),
-				timestamp: old_header.timestamp(),
-				last_hashes: {
-					// this will break if we don't inclue all 256 last hashes.
-					let mut last_hashes: Vec<_> = (0..256).map(|_| H256::default()).collect();
-					last_hashes[255] = *old_header.parent_hash();
-					Arc::new(last_hashes)
-				},
-				gas_used: 0.into(),
-			};
-
-			// check state proof using given engine.
 			let number = old_header.number();
-			let addresses = self.provider.get_validators(move |a, d| {
+			let old_hash = old_header.hash();
-				let from = Address::default();
+			let addresses = check_first_proof(engine, &self.provider, old_header, &state_items)
-				let tx = Transaction {
+				.map_err(::engines::EngineError::InsufficientProof)?;
-					nonce: engine.account_start_nonce(number),
-					action: Action::Call(a),
-					gas: PROVIDED_GAS.into(),
-					gas_price: U256::default(),
-					value: U256::default(),
-					data: d,
-				}.fake_sign(from);
-
-				let res = ::state::check_proof(
-					&state_items,
-					*old_header.state_root(),
-					&tx,
-					engine,
-					&env_info,
-				);
-
-				match res {
-					::state::ProvedExecution::BadProof => Err("Bad proof".into()),
-					::state::ProvedExecution::Failed(e) => Err(format!("Failed call: {}", e)),
-					::state::ProvedExecution::Complete(e) => Ok(e.output),
-				}
-			}).wait().map_err(::engines::EngineError::InsufficientProof)?;
 
 			trace!(target: "engine", "extracted epoch set at #{}: {} addresses",
 				number, addresses.len());