Merge pull request #85 from gavofyork/gav

Nicer transaction validation API. Nicer OutOfBounds API in general.

src/block.rs

@@ -129,7 +129,7 @@ impl<'x, 'y> OpenBlock<'x, 'y> {
 	/// Alter the extra_data for the block.
 	pub fn set_extra_data(&mut self, extra_data: Bytes) -> Result<(), BlockError> {
 		if extra_data.len() > self.engine.maximum_extra_data_size() {
-			Err(BlockError::ExtraDataOutOfBounds(OutOfBounds{min: 0, max: self.engine.maximum_extra_data_size(), found: extra_data.len()}))
+			Err(BlockError::ExtraDataOutOfBounds(OutOfBounds{min: None, max: Some(self.engine.maximum_extra_data_size()), found: extra_data.len()}))
 		} else {
 			self.block.header.set_extra_data(extra_data);
 			Ok(())
@@ -142,7 +142,7 @@ impl<'x, 'y> OpenBlock<'x, 'y> {
 	/// that the header itself is actually valid.
 	pub fn push_uncle(&mut self, valid_uncle_header: Header) -> Result<(), BlockError> {
 		if self.block.uncles.len() >= self.engine.maximum_uncle_count() {
-			return Err(BlockError::TooManyUncles(OutOfBounds{min: 0, max: self.engine.maximum_uncle_count(), found: self.block.uncles.len()}));
+			return Err(BlockError::TooManyUncles(OutOfBounds{min: None, max: Some(self.engine.maximum_uncle_count()), found: self.block.uncles.len()}));
 		}
 		// TODO: check number
 		// TODO: check not a direct ancestor (use last_hashes for that)

src/error.rs

@@ -11,8 +11,8 @@ pub struct Mismatch<T: fmt::Debug> {
 
 #[derive(Debug)]
 pub struct OutOfBounds<T: fmt::Debug> {
-	pub min: T,
+	pub min: Option<T>,
-	pub max: T,
+	pub max: Option<T>,
 	pub found: T,
 }
 
@@ -33,6 +33,11 @@ pub enum ExecutionError {
 	Internal
 }
 
+#[derive(Debug)]
+pub enum TransactionError {
+	InvalidGasLimit(OutOfBounds<U256>),
+}
+
 #[derive(Debug)]
 pub enum BlockError {
 	TooManyUncles(OutOfBounds<usize>),
@@ -83,6 +88,13 @@ pub enum Error {
 	Block(BlockError),
 	UnknownEngineName(String),
 	Execution(ExecutionError),
+	Transaction(TransactionError),
+}
+
+impl From<TransactionError> for Error {
+	fn from(err: TransactionError) -> Error {
+		Error::Transaction(err)
+	}
 }
 
 impl From<BlockError> for Error {
@@ -103,6 +115,12 @@ impl From<CryptoError> for Error {
 	}
 }
 
+impl From<DecoderError> for Error {
+	fn from(err: DecoderError) -> Error {
+		Error::Util(UtilError::Decoder(err))
+	}
+}
+
 // TODO: uncomment below once https://github.com/rust-lang/rust/issues/27336 sorted.
 /*#![feature(concat_idents)]
 macro_rules! assimilate {

src/ethereum/ethash.rs

@@ -53,11 +53,11 @@ impl Engine for Ethash {
 		}
 		let min_gas_limit = decode(self.spec().engine_params.get("minGasLimit").unwrap());
 		if header.gas_limit < min_gas_limit {
-			return Err(From::from(BlockError::InvalidGasLimit(OutOfBounds { min: min_gas_limit, max: From::from(0), found: header.gas_limit }))); 
+			return Err(From::from(BlockError::InvalidGasLimit(OutOfBounds { min: Some(min_gas_limit), max: None, found: header.gas_limit }))); 
 		}
 		let maximum_extra_data_size = self.maximum_extra_data_size();
 		if header.number != 0 && header.extra_data.len() > maximum_extra_data_size {
-			return Err(From::from(BlockError::ExtraDataOutOfBounds(OutOfBounds { min: 0, max: maximum_extra_data_size, found: header.extra_data.len() }))); 
+			return Err(From::from(BlockError::ExtraDataOutOfBounds(OutOfBounds { min: None, max: Some(maximum_extra_data_size), found: header.extra_data.len() }))); 
 		}
 		// TODO: Verify seal (quick)
 		Ok(())
@@ -78,7 +78,7 @@ impl Engine for Ethash {
 		let min_gas = parent.gas_limit - parent.gas_limit / gas_limit_divisor;
 		let max_gas = parent.gas_limit + parent.gas_limit / gas_limit_divisor;
 		if header.gas_limit <= min_gas || header.gas_limit >= max_gas {
-			return Err(From::from(BlockError::InvalidGasLimit(OutOfBounds { min: min_gas, max: max_gas, found: header.gas_limit }))); 
+			return Err(From::from(BlockError::InvalidGasLimit(OutOfBounds { min: Some(min_gas), max: Some(max_gas), found: header.gas_limit }))); 
 		}
 		Ok(())
 	}

src/transaction.rs

@@ -1,6 +1,6 @@
 use util::*;
 use basic_types::*;
-use error::Error;
+use error::*;
 use evm::Schedule;
 
 pub enum Action {
@@ -91,17 +91,27 @@ impl Transaction {
 	pub fn sender(&self) -> Result<Address, Error> { Ok(From::from(try!(ec::recover(&self.signature(), &self.message_hash())).sha3())) }
 
 	/// Get the transaction cost in gas for the given params.
-	pub fn gas_required_for(is_create: bool, data: &[u8], schedule: &Schedule, gas: &U256) -> U256 {
+	pub fn gas_required_for(is_create: bool, data: &[u8], schedule: &Schedule) -> U256 {
 		// CRITICAL TODO XXX FIX NEED BIGINT!!!!!
 		data.iter().fold(
-			U256::from(if is_create {schedule.tx_create_gas} else {schedule.tx_gas}) + *gas,
+			U256::from(if is_create {schedule.tx_create_gas} else {schedule.tx_gas}),
 			|g, b| g + U256::from(match *b { 0 => schedule.tx_data_zero_gas, _ => schedule.tx_data_non_zero_gas})
 		)
 	}
 
-	/// Get the transaction cost in gas for this transaction
+	/// Get the transaction cost in gas for this transaction.
-	pub fn gas_required(&self, schedule: &Schedule, gas: &U256) -> U256 {
+	pub fn gas_required(&self, schedule: &Schedule) -> U256 {
-		Self::gas_required_for(match self.action{Action::Create=>true, Action::Call(_)=>false}, &self.data, schedule, gas)
+		Self::gas_required_for(match self.action{Action::Create=>true, Action::Call(_)=>false}, &self.data, schedule)
+	}
+
+	/// Do basic validation, checking for valid signature and minimum gas,
+	pub fn validate(self, schedule: &Schedule) -> Result<Transaction, Error> {
+		try!(self.sender());
+		if self.gas < self.gas_required(&schedule) {
+			Err(From::from(TransactionError::InvalidGasLimit(OutOfBounds{min: Some(self.gas_required(&schedule)), max: None, found: self.gas})))
+		} else {
+			Ok(self)
+		}
 	}
 }
 
@@ -200,10 +210,10 @@ fn json_tests() {
 		let mut fail_unless = |cond: bool| if !cond && fail { failed.push(name.to_string()); fail = true };
 		let _ = BlockNumber::from_str(test["blocknumber"].as_string().unwrap()).unwrap();
 		let rlp = bytes_from_json(&test["rlp"]);
-		let r: Result<Transaction, DecoderError> = UntrustedRlp::new(&rlp).as_val();
+		let res = UntrustedRlp::new(&rlp).as_val().map_err(|e| From::from(e)).and_then(|t: Transaction| t.validate(&schedule));
-		if let Ok(t) = r {
+		fail_unless(test.find("transaction").is_none() == res.is_err());
-			if t.sender().is_ok() && t.gas >= t.gas_required(&schedule, &U256::zero()) {
 		if let (Some(&Json::Object(ref tx)), Some(&Json::String(ref expect_sender))) = (test.find("transaction"), test.find("sender")) {
+			let t = res.unwrap();
 			fail_unless(t.sender().unwrap() == address_from_hex(clean(expect_sender)));
 			fail_unless(t.data == bytes_from_json(&tx["data"]));
 			fail_unless(t.gas == u256_from_json(&tx["gasLimit"]));
@@ -216,11 +226,6 @@ fn json_tests() {
 				fail_unless(bytes_from_json(&tx["to"]).len() == 0);
 			}
 		}
-				else { fail_unless(false) }
-			}
-			else { fail_unless(test.find("transaction").is_none()) }
-		}
-		else { fail_unless(test.find("transaction").is_none()) }
 	}
 	for f in failed.iter() {
 		println!("FAILED: {:?}", f);

src/verification.rs

@@ -47,7 +47,7 @@ pub fn verify_block_final(bytes: &[u8], engine: &Engine, bc: &BlockChain) -> Res
 	let num_uncles = Rlp::new(bytes).at(2).item_count();
 	if num_uncles != 0 {
 		if num_uncles > engine.maximum_uncle_count() {
-			return Err(From::from(BlockError::TooManyUncles(OutOfBounds { min: 0, max: engine.maximum_uncle_count(), found: num_uncles })));
+			return Err(From::from(BlockError::TooManyUncles(OutOfBounds { min: None, max: Some(engine.maximum_uncle_count()), found: num_uncles })));
 		}
 
 		let mut excluded = HashSet::new();
@@ -83,10 +83,10 @@ pub fn verify_block_final(bytes: &[u8], engine: &Engine, bc: &BlockChain) -> Res
 
 			let depth = if header.number > uncle.number { header.number - uncle.number } else { 0 };
 			if depth > 6 {
-				return Err(From::from(BlockError::UncleTooOld(OutOfBounds { min: header.number - depth, max: header.number - 1, found: uncle.number })));
+				return Err(From::from(BlockError::UncleTooOld(OutOfBounds { min: Some(header.number - depth), max: Some(header.number - 1), found: uncle.number })));
 			}
 			else if depth < 1 {
-				return Err(From::from(BlockError::UncleIsBrother(OutOfBounds { min: header.number - depth, max: header.number - 1, found: uncle.number })));
+				return Err(From::from(BlockError::UncleIsBrother(OutOfBounds { min: Some(header.number - depth), max: Some(header.number - 1), found: uncle.number })));
 			}
 
 			// cB
@@ -115,10 +115,10 @@ pub fn verify_block_final(bytes: &[u8], engine: &Engine, bc: &BlockChain) -> Res
 /// Check basic header parameters.
 fn verify_header(header: &Header) -> Result<(), Error> {
 	if header.number > From::from(BlockNumber::max_value()) {
-		return Err(From::from(BlockError::InvalidNumber(OutOfBounds { max: From::from(BlockNumber::max_value()), min: 0, found: header.number })))
+		return Err(From::from(BlockError::InvalidNumber(OutOfBounds { max: Some(From::from(BlockNumber::max_value())), min: None, found: header.number })))
 	}
 	if header.gas_used > header.gas_limit {
-		return Err(From::from(BlockError::TooMuchGasUsed(OutOfBounds { max: header.gas_limit, min: From::from(0), found: header.gas_used })));
+		return Err(From::from(BlockError::TooMuchGasUsed(OutOfBounds { max: Some(header.gas_limit), min: None, found: header.gas_used })));
 	}
 	Ok(())
 }
@@ -129,10 +129,10 @@ fn verify_parent(header: &Header, parent: &Header) -> Result<(), Error> {
 		return Err(From::from(BlockError::InvalidParentHash(Mismatch { expected: parent.hash(), found: header.parent_hash.clone() })))
 	}
 	if header.timestamp <= parent.timestamp {
-		return Err(From::from(BlockError::InvalidTimestamp(OutOfBounds { max: u64::max_value(), min: parent.timestamp + 1, found: header.timestamp })))
+		return Err(From::from(BlockError::InvalidTimestamp(OutOfBounds { max: None, min: Some(parent.timestamp + 1), found: header.timestamp })))
 	}
 	if header.number <= parent.number {
-		return Err(From::from(BlockError::InvalidNumber(OutOfBounds { max: BlockNumber::max_value(), min: parent.number + 1, found: header.number })));
+		return Err(From::from(BlockError::InvalidNumber(OutOfBounds { max: None, min: Some(parent.number + 1), found: header.number })));
 	}
 	Ok(())
 }