UnverifiedTransaction type (#4134)

* Introducing ValidSignedTransaction

* Verifiying transactions in engines

* Widening use of VerifiedSignedTransaction

* Renaming Transactions

* Uncommenting banning queue & Fixing tests

* Fixing json tests

* Fixing pre-homestead test

* Fixing imports

* Addressing grumbles

* Fixing test

Cargo.lock

@@ -368,7 +368,6 @@ dependencies = [
  "lru-cache 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "num_cpus 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)",
  "rand 0.3.14 (registry+https://github.com/rust-lang/crates.io-index)",
- "rayon 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "rlp 0.1.0",
  "rust-crypto 0.2.36 (registry+https://github.com/rust-lang/crates.io-index)",
  "rustc-serialize 0.3.19 (registry+https://github.com/rust-lang/crates.io-index)",

ethcore/Cargo.toml

@@ -20,7 +20,6 @@ num_cpus = "0.2"
 crossbeam = "0.2.9"
 lazy_static = "0.2"
 bloomchain = "0.1"
-rayon = "0.4.2"
 semver = "0.5"
 bit-set = "0.4"
 time = "0.1"

ethcore/light/src/net/mod.rs

@@ -19,7 +19,7 @@
 //! This uses a "Provider" to answer requests.
 //! See https://github.com/ethcore/parity/wiki/Light-Ethereum-Subprotocol-(LES)
 
-use ethcore::transaction::SignedTransaction;
+use ethcore::transaction::UnverifiedTransaction;
 use ethcore::receipt::Receipt;
 
 use io::TimerToken;
@@ -179,7 +179,7 @@ pub trait Handler: Send + Sync {
 	/// Called when a peer makes an announcement.
 	fn on_announcement(&self, _ctx: &EventContext, _announcement: &Announcement) { }
 	/// Called when a peer requests relay of some transactions.
-	fn on_transactions(&self, _ctx: &EventContext, _relay: &[SignedTransaction]) { }
+	fn on_transactions(&self, _ctx: &EventContext, _relay: &[UnverifiedTransaction]) { }
 	/// Called when a peer responds with block bodies.
 	fn on_block_bodies(&self, _ctx: &EventContext, _req_id: ReqId, _bodies: &[Bytes]) { }
 	/// Called when a peer responds with block headers.
@@ -1135,7 +1135,7 @@ impl LightProtocol {
 
 		let txs: Vec<_> = data.iter()
 			.take(MAX_TRANSACTIONS)
-			.map(|x| x.as_val::<SignedTransaction>())
+			.map(|x| x.as_val::<UnverifiedTransaction>())
 			.collect::<Result<_,_>>()?;
 
 		debug!(target: "les", "Received {} transactions to relay from peer {}", txs.len(), peer);

ethcore/src/block.rs

@@ -34,7 +34,7 @@ use receipt::Receipt;
 use state::State;
 use state_db::StateDB;
 use trace::FlatTrace;
-use transaction::SignedTransaction;
+use transaction::{UnverifiedTransaction, SignedTransaction};
 use verification::PreverifiedBlock;
 use views::BlockView;
 
@@ -44,7 +44,7 @@ pub struct Block {
 	/// The header of this block.
 	pub header: Header,
 	/// The transactions in this block.
-	pub transactions: Vec<SignedTransaction>,
+	pub transactions: Vec<UnverifiedTransaction>,
 	/// The uncles of this block.
 	pub uncles: Vec<Header>,
 }
@@ -86,8 +86,9 @@ impl Decodable for Block {
 /// An internal type for a block's common elements.
 #[derive(Clone)]
 pub struct ExecutedBlock {
-	base: Block,
+	header: Header,
-
+	transactions: Vec<SignedTransaction>,
+	uncles: Vec<Header>,
 	receipts: Vec<Receipt>,
 	transactions_set: HashSet<H256>,
 	state: State,
@@ -130,7 +131,9 @@ impl ExecutedBlock {
 	/// Create a new block from the given `state`.
 	fn new(state: State, tracing: bool) -> ExecutedBlock {
 		ExecutedBlock {
-			base: Default::default(),
+			header: Default::default(),
+			transactions: Default::default(),
+			uncles: Default::default(),
 			receipts: Default::default(),
 			transactions_set: Default::default(),
 			state: state,
@@ -141,9 +144,9 @@ impl ExecutedBlock {
 	/// Get a structure containing individual references to all public fields.
 	pub fn fields_mut(&mut self) -> BlockRefMut {
 		BlockRefMut {
-			header: &mut self.base.header,
+			header: &mut self.header,
-			transactions: &self.base.transactions,
+			transactions: &self.transactions,
-			uncles: &self.base.uncles,
+			uncles: &self.uncles,
 			state: &mut self.state,
 			receipts: &self.receipts,
 			traces: &self.traces,
@@ -153,9 +156,9 @@ impl ExecutedBlock {
 	/// Get a structure containing individual references to all public fields.
 	pub fn fields(&self) -> BlockRef {
 		BlockRef {
-			header: &self.base.header,
+			header: &self.header,
-			transactions: &self.base.transactions,
+			transactions: &self.transactions,
-			uncles: &self.base.uncles,
+			uncles: &self.uncles,
 			state: &self.state,
 			receipts: &self.receipts,
 			traces: &self.traces,
@@ -169,16 +172,22 @@ pub trait IsBlock {
 	fn block(&self) -> &ExecutedBlock;
 
 	/// Get the base `Block` object associated with this.
-	fn base(&self) -> &Block { &self.block().base }
+	fn to_base(&self) -> Block {
+		Block {
+			header: self.header().clone(),
+			transactions: self.transactions().iter().cloned().map(Into::into).collect(),
+			uncles: self.uncles().to_vec(),
+		}
+	}
 
 	/// Get the header associated with this object's block.
-	fn header(&self) -> &Header { &self.block().base.header }
+	fn header(&self) -> &Header { &self.block().header }
 
 	/// Get the final state associated with this object's block.
 	fn state(&self) -> &State { &self.block().state }
 
 	/// Get all information on transactions in this block.
-	fn transactions(&self) -> &[SignedTransaction] { &self.block().base.transactions }
+	fn transactions(&self) -> &[SignedTransaction] { &self.block().transactions }
 
 	/// Get all information on receipts in this block.
 	fn receipts(&self) -> &[Receipt] { &self.block().receipts }
@@ -187,7 +196,7 @@ pub trait IsBlock {
 	fn traces(&self) -> &Option<Vec<Vec<FlatTrace>>> { &self.block().traces }
 
 	/// Get all uncles in this block.
-	fn uncles(&self) -> &[Header] { &self.block().base.uncles }
+	fn uncles(&self) -> &[Header] { &self.block().uncles }
 }
 
 /// Trait for a object that has a state database.
@@ -260,50 +269,50 @@ impl<'x> OpenBlock<'x> {
 			last_hashes: last_hashes,
 		};
 
-		r.block.base.header.set_parent_hash(parent.hash());
+		r.block.header.set_parent_hash(parent.hash());
-		r.block.base.header.set_number(parent.number() + 1);
+		r.block.header.set_number(parent.number() + 1);
-		r.block.base.header.set_author(author);
+		r.block.header.set_author(author);
-		r.block.base.header.set_timestamp_now(parent.timestamp());
+		r.block.header.set_timestamp_now(parent.timestamp());
-		r.block.base.header.set_extra_data(extra_data);
+		r.block.header.set_extra_data(extra_data);
-		r.block.base.header.note_dirty();
+		r.block.header.note_dirty();
 
 		let gas_floor_target = cmp::max(gas_range_target.0, engine.params().min_gas_limit);
 		let gas_ceil_target = cmp::max(gas_range_target.1, gas_floor_target);
-		engine.populate_from_parent(&mut r.block.base.header, parent, gas_floor_target, gas_ceil_target);
+		engine.populate_from_parent(&mut r.block.header, parent, gas_floor_target, gas_ceil_target);
 		engine.on_new_block(&mut r.block);
 		Ok(r)
 	}
 
 	/// Alter the author for the block.
-	pub fn set_author(&mut self, author: Address) { self.block.base.header.set_author(author); }
+	pub fn set_author(&mut self, author: Address) { self.block.header.set_author(author); }
 
 	/// Alter the timestamp of the block.
-	pub fn set_timestamp(&mut self, timestamp: u64) { self.block.base.header.set_timestamp(timestamp); }
+	pub fn set_timestamp(&mut self, timestamp: u64) { self.block.header.set_timestamp(timestamp); }
 
 	/// Alter the difficulty for the block.
-	pub fn set_difficulty(&mut self, a: U256) { self.block.base.header.set_difficulty(a); }
+	pub fn set_difficulty(&mut self, a: U256) { self.block.header.set_difficulty(a); }
 
 	/// Alter the gas limit for the block.
-	pub fn set_gas_limit(&mut self, a: U256) { self.block.base.header.set_gas_limit(a); }
+	pub fn set_gas_limit(&mut self, a: U256) { self.block.header.set_gas_limit(a); }
 
 	/// Alter the gas limit for the block.
-	pub fn set_gas_used(&mut self, a: U256) { self.block.base.header.set_gas_used(a); }
+	pub fn set_gas_used(&mut self, a: U256) { self.block.header.set_gas_used(a); }
 
 	/// Alter the uncles hash the block.
-	pub fn set_uncles_hash(&mut self, h: H256) { self.block.base.header.set_uncles_hash(h); }
+	pub fn set_uncles_hash(&mut self, h: H256) { self.block.header.set_uncles_hash(h); }
 
 	/// Alter transactions root for the block.
-	pub fn set_transactions_root(&mut self, h: H256) { self.block.base.header.set_transactions_root(h); }
+	pub fn set_transactions_root(&mut self, h: H256) { self.block.header.set_transactions_root(h); }
 
 	/// Alter the receipts root for the block.
-	pub fn set_receipts_root(&mut self, h: H256) { self.block.base.header.set_receipts_root(h); }
+	pub fn set_receipts_root(&mut self, h: H256) { self.block.header.set_receipts_root(h); }
 
 	/// 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: None, max: Some(self.engine.maximum_extra_data_size()), found: extra_data.len()}))
 		} else {
-			self.block.base.header.set_extra_data(extra_data);
+			self.block.header.set_extra_data(extra_data);
 			Ok(())
 		}
 	}
@@ -313,12 +322,12 @@ 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() + 1 > self.engine.maximum_uncle_count() {
+		if self.block.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}));
+			return Err(BlockError::TooManyUncles(OutOfBounds{min: None, max: Some(self.engine.maximum_uncle_count()), found: self.block.uncles.len() + 1}));
 		}
 		// TODO: check number
 		// TODO: check not a direct ancestor (use last_hashes for that)
-		self.block.base.uncles.push(valid_uncle_header);
+		self.block.uncles.push(valid_uncle_header);
 		Ok(())
 	}
 
@@ -326,13 +335,13 @@ impl<'x> OpenBlock<'x> {
 	pub fn env_info(&self) -> EnvInfo {
 		// TODO: memoise.
 		EnvInfo {
-			number: self.block.base.header.number(),
+			number: self.block.header.number(),
-			author: self.block.base.header.author().clone(),
+			author: self.block.header.author().clone(),
-			timestamp: self.block.base.header.timestamp(),
+			timestamp: self.block.header.timestamp(),
-			difficulty: self.block.base.header.difficulty().clone(),
+			difficulty: self.block.header.difficulty().clone(),
 			last_hashes: self.last_hashes.clone(),
 			gas_used: self.block.receipts.last().map_or(U256::zero(), |r| r.gas_used),
-			gas_limit: self.block.base.header.gas_limit().clone(),
+			gas_limit: self.block.header.gas_limit().clone(),
 		}
 	}
 
@@ -349,7 +358,7 @@ impl<'x> OpenBlock<'x> {
 		match self.block.state.apply(&env_info, self.engine, &t, self.block.traces.is_some()) {
 			Ok(outcome) => {
 				self.block.transactions_set.insert(h.unwrap_or_else(||t.hash()));
-				self.block.base.transactions.push(t);
+				self.block.transactions.push(t.into());
 				let t = outcome.trace;
 				self.block.traces.as_mut().map(|traces| traces.push(t));
 				self.block.receipts.push(outcome.receipt);
@@ -366,13 +375,13 @@ impl<'x> OpenBlock<'x> {
 		let unclosed_state = s.block.state.clone();
 
 		s.engine.on_close_block(&mut s.block);
-		s.block.base.header.set_transactions_root(ordered_trie_root(s.block.base.transactions.iter().map(|e| e.rlp_bytes().to_vec())));
+		s.block.header.set_transactions_root(ordered_trie_root(s.block.transactions.iter().map(|e| e.rlp_bytes().to_vec())));
-		let uncle_bytes = s.block.base.uncles.iter().fold(RlpStream::new_list(s.block.base.uncles.len()), |mut s, u| {s.append_raw(&u.rlp(Seal::With), 1); s} ).out();
+		let uncle_bytes = s.block.uncles.iter().fold(RlpStream::new_list(s.block.uncles.len()), |mut s, u| {s.append_raw(&u.rlp(Seal::With), 1); s} ).out();
-		s.block.base.header.set_uncles_hash(uncle_bytes.sha3());
+		s.block.header.set_uncles_hash(uncle_bytes.sha3());
-		s.block.base.header.set_state_root(s.block.state.root().clone());
+		s.block.header.set_state_root(s.block.state.root().clone());
-		s.block.base.header.set_receipts_root(ordered_trie_root(s.block.receipts.iter().map(|r| r.rlp_bytes().to_vec())));
+		s.block.header.set_receipts_root(ordered_trie_root(s.block.receipts.iter().map(|r| r.rlp_bytes().to_vec())));
-		s.block.base.header.set_log_bloom(s.block.receipts.iter().fold(LogBloom::zero(), |mut b, r| {b = &b | &r.log_bloom; b})); //TODO: use |= operator
+		s.block.header.set_log_bloom(s.block.receipts.iter().fold(LogBloom::zero(), |mut b, r| {b = &b | &r.log_bloom; b})); //TODO: use |= operator
-		s.block.base.header.set_gas_used(s.block.receipts.last().map_or(U256::zero(), |r| r.gas_used));
+		s.block.header.set_gas_used(s.block.receipts.last().map_or(U256::zero(), |r| r.gas_used));
 
 		ClosedBlock {
 			block: s.block,
@@ -387,20 +396,20 @@ impl<'x> OpenBlock<'x> {
 		let mut s = self;
 
 		s.engine.on_close_block(&mut s.block);
-		if s.block.base.header.transactions_root().is_zero() || s.block.base.header.transactions_root() == &SHA3_NULL_RLP {
+		if s.block.header.transactions_root().is_zero() || s.block.header.transactions_root() == &SHA3_NULL_RLP {
-			s.block.base.header.set_transactions_root(ordered_trie_root(s.block.base.transactions.iter().map(|e| e.rlp_bytes().to_vec())));
+			s.block.header.set_transactions_root(ordered_trie_root(s.block.transactions.iter().map(|e| e.rlp_bytes().to_vec())));
 		}
-		let uncle_bytes = s.block.base.uncles.iter().fold(RlpStream::new_list(s.block.base.uncles.len()), |mut s, u| {s.append_raw(&u.rlp(Seal::With), 1); s} ).out();
+		let uncle_bytes = s.block.uncles.iter().fold(RlpStream::new_list(s.block.uncles.len()), |mut s, u| {s.append_raw(&u.rlp(Seal::With), 1); s} ).out();
-		if s.block.base.header.uncles_hash().is_zero() {
+		if s.block.header.uncles_hash().is_zero() {
-			s.block.base.header.set_uncles_hash(uncle_bytes.sha3());
+			s.block.header.set_uncles_hash(uncle_bytes.sha3());
 		}
-		if s.block.base.header.receipts_root().is_zero() || s.block.base.header.receipts_root() == &SHA3_NULL_RLP {
+		if s.block.header.receipts_root().is_zero() || s.block.header.receipts_root() == &SHA3_NULL_RLP {
-			s.block.base.header.set_receipts_root(ordered_trie_root(s.block.receipts.iter().map(|r| r.rlp_bytes().to_vec())));
+			s.block.header.set_receipts_root(ordered_trie_root(s.block.receipts.iter().map(|r| r.rlp_bytes().to_vec())));
 		}
 
-		s.block.base.header.set_state_root(s.block.state.root().clone());
+		s.block.header.set_state_root(s.block.state.root().clone());
-		s.block.base.header.set_log_bloom(s.block.receipts.iter().fold(LogBloom::zero(), |mut b, r| {b = &b | &r.log_bloom; b})); //TODO: use |= operator
+		s.block.header.set_log_bloom(s.block.receipts.iter().fold(LogBloom::zero(), |mut b, r| {b = &b | &r.log_bloom; b})); //TODO: use |= operator
-		s.block.base.header.set_gas_used(s.block.receipts.last().map_or(U256::zero(), |r| r.gas_used));
+		s.block.header.set_gas_used(s.block.receipts.last().map_or(U256::zero(), |r| r.gas_used));
 
 		LockedBlock {
 			block: s.block,
@@ -462,7 +471,7 @@ impl LockedBlock {
 		if seal.len() != engine.seal_fields() {
 			return Err(BlockError::InvalidSealArity(Mismatch{expected: engine.seal_fields(), found: seal.len()}));
 		}
-		s.block.base.header.set_seal(seal);
+		s.block.header.set_seal(seal);
 		Ok(SealedBlock { block: s.block, uncle_bytes: s.uncle_bytes })
 	}
 
@@ -471,8 +480,8 @@ impl LockedBlock {
 	/// Returns the `ClosedBlock` back again if the seal is no good.
 	pub fn try_seal(self, engine: &Engine, seal: Vec<Bytes>) -> Result<SealedBlock, (Error, LockedBlock)> {
 		let mut s = self;
-		s.block.base.header.set_seal(seal);
+		s.block.header.set_seal(seal);
-		match engine.verify_block_seal(&s.block.base.header) {
+		match engine.verify_block_seal(&s.block.header) {
 			Err(e) => Err((e, s)),
 			_ => Ok(SealedBlock { block: s.block, uncle_bytes: s.uncle_bytes }),
 		}
@@ -490,8 +499,8 @@ impl SealedBlock {
 	/// Get the RLP-encoding of the block.
 	pub fn rlp_bytes(&self) -> Bytes {
 		let mut block_rlp = RlpStream::new_list(3);
-		self.block.base.header.stream_rlp(&mut block_rlp, Seal::With);
+		self.block.header.stream_rlp(&mut block_rlp, Seal::With);
-		block_rlp.append(&self.block.base.transactions);
+		block_rlp.append(&self.block.transactions);
 		block_rlp.append_raw(&self.uncle_bytes, 1);
 		block_rlp.out()
 	}
@@ -571,6 +580,7 @@ fn push_transactions(block: &mut OpenBlock, transactions: &[SignedTransaction])
 	Ok(())
 }
 
+// TODO [ToDr] Pass `PreverifiedBlock` by move, this will avoid unecessary allocation
 /// Enact the block given by `block_bytes` using `engine` on the database `db` with given `parent` block header
 #[cfg_attr(feature="dev", allow(too_many_arguments))]
 pub fn enact_verified(
@@ -600,6 +610,7 @@ mod tests {
 	use util::Address;
 	use util::hash::FixedHash;
 	use std::sync::Arc;
+	use transaction::SignedTransaction;
 
 	/// Enact the block given by `block_bytes` using `engine` on the database `db` with given `parent` block header
 	#[cfg_attr(feature="dev", allow(too_many_arguments))]
@@ -614,7 +625,9 @@ mod tests {
 	) -> Result<LockedBlock, Error> {
 		let block = BlockView::new(block_bytes);
 		let header = block.header();
-		enact(&header, &block.transactions(), &block.uncles(), engine, tracing, db, parent, last_hashes, factories)
+		let transactions: Result<Vec<_>, Error> = block.transactions().into_iter().map(SignedTransaction::new).collect();
+		let transactions = transactions?;
+		enact(&header, &transactions, &block.uncles(), engine, tracing, db, parent, last_hashes, factories)
 	}
 
 	/// Enact the block given by `block_bytes` using `engine` on the database `db` with given `parent` block header. Seal the block aferwards

ethcore/src/client/client.rs

@@ -44,7 +44,7 @@ use env_info::LastHashes;
 use verification;
 use verification::{PreverifiedBlock, Verifier};
 use block::*;
-use transaction::{LocalizedTransaction, SignedTransaction, Transaction, PendingTransaction, Action};
+use transaction::{LocalizedTransaction, UnverifiedTransaction, SignedTransaction, Transaction, PendingTransaction, Action};
 use blockchain::extras::TransactionAddress;
 use types::filter::Filter;
 use types::mode::Mode as IpcMode;
@@ -596,7 +596,7 @@ impl Client {
 		trace!(target: "external_tx", "Importing queued");
 		let _timer = PerfTimer::new("import_queued_transactions");
 		self.queue_transactions.fetch_sub(transactions.len(), AtomicOrdering::SeqCst);
-		let txs: Vec<SignedTransaction> = transactions.iter().filter_map(|bytes| UntrustedRlp::new(bytes).as_val().ok()).collect();
+		let txs: Vec<UnverifiedTransaction> = transactions.iter().filter_map(|bytes| UntrustedRlp::new(bytes).as_val().ok()).collect();
 		let hashes: Vec<_> = txs.iter().map(|tx| tx.hash()).collect();
 		self.notify(|notify| {
 			notify.transactions_received(hashes.clone(), peer_id);
@@ -854,10 +854,7 @@ impl BlockChainClient for Client {
 		let mut state = self.state_at(block).ok_or(CallError::StatePruned)?;
 		let original_state = if analytics.state_diffing { Some(state.clone()) } else { None };
 
-		let sender = t.sender().map_err(|e| {
+		let sender = t.sender();
-			let message = format!("Transaction malformed: {:?}", e);
-			ExecutionError::TransactionMalformed(message)
-		})?;
 		let balance = state.balance(&sender);
 		let needed_balance = t.value + t.gas * t.gas_price;
 		if balance < needed_balance {
@@ -888,16 +885,14 @@ impl BlockChainClient for Client {
 		};
 		// that's just a copy of the state.
 		let original_state = self.state_at(block).ok_or(CallError::StatePruned)?;
-		let sender = t.sender().map_err(|e| {
+		let sender = t.sender();
-			let message = format!("Transaction malformed: {:?}", e);
-			ExecutionError::TransactionMalformed(message)
-		})?;
 		let balance = original_state.balance(&sender);
 		let options = TransactOptions { tracing: true, vm_tracing: false, check_nonce: false };
-		let mut tx = t.clone();
 
-		let mut cond = |gas| {
+		let cond = |gas| {
+			let mut tx = t.as_unsigned().clone();
 			tx.gas = gas;
+			let tx = tx.fake_sign(sender);
 
 			let mut state = original_state.clone();
 			let needed_balance = tx.value + tx.gas * tx.gas_price;
@@ -955,7 +950,7 @@ impl BlockChainClient for Client {
 		let header = self.block_header(BlockId::Hash(address.block_hash)).ok_or(CallError::StatePruned)?;
 		let body = self.block_body(BlockId::Hash(address.block_hash)).ok_or(CallError::StatePruned)?;
 		let mut state = self.state_at_beginning(BlockId::Hash(address.block_hash)).ok_or(CallError::StatePruned)?;
-		let txs = body.transactions();
+		let mut txs = body.transactions();
 
 		if address.index >= txs.len() {
 			return Err(CallError::TransactionNotFound);
@@ -972,16 +967,19 @@ impl BlockChainClient for Client {
 			gas_used: U256::default(),
 			gas_limit: header.gas_limit(),
 		};
-		for t in txs.iter().take(address.index) {
+		const PROOF: &'static str = "Transactions fetched from blockchain; blockchain transactions are valid; qed";
-			match Executive::new(&mut state, &env_info, &*self.engine, &self.factories.vm).transact(t, Default::default()) {
+		let rest = txs.split_off(address.index);
+		for t in txs {
+			let t = SignedTransaction::new(t).expect(PROOF);
+			match Executive::new(&mut state, &env_info, &*self.engine, &self.factories.vm).transact(&t, Default::default()) {
 				Ok(x) => { env_info.gas_used = env_info.gas_used + x.gas_used; }
 				Err(ee) => { return Err(CallError::Execution(ee)) }
 			}
 		}
-		let t = &txs[address.index];
+		let first = rest.into_iter().next().expect("We split off < `address.index`; Length is checked earlier; qed");
-
+		let t = SignedTransaction::new(first).expect(PROOF);
 		let original_state = if analytics.state_diffing { Some(state.clone()) } else { None };
-		let mut ret = Executive::new(&mut state, &env_info, &*self.engine, &self.factories.vm).transact(t, options)?;
+		let mut ret = Executive::new(&mut state, &env_info, &*self.engine, &self.factories.vm).transact(&t, options)?;
 		ret.state_diff = original_state.map(|original| state.diff_from(original));
 
 		Ok(ret)
@@ -1584,11 +1582,10 @@ impl Drop for Client {
 
 /// Returns `LocalizedReceipt` given `LocalizedTransaction`
 /// and a vector of receipts from given block up to transaction index.
-fn transaction_receipt(tx: LocalizedTransaction, mut receipts: Vec<Receipt>) -> LocalizedReceipt {
+fn transaction_receipt(mut tx: LocalizedTransaction, mut receipts: Vec<Receipt>) -> LocalizedReceipt {
 	assert_eq!(receipts.len(), tx.transaction_index + 1, "All previous receipts are provided.");
 
-	let sender = tx.sender()
+	let sender = tx.sender();
-		.expect("LocalizedTransaction is part of the blockchain; We have only valid transactions in chain; qed");
 	let receipt = receipts.pop().expect("Current receipt is provided; qed");
 	let prior_gas_used = match tx.transaction_index {
 		0 => 0.into(),
@@ -1688,10 +1685,11 @@ mod tests {
 		};
 		let tx1 = raw_tx.clone().sign(secret, None);
 		let transaction = LocalizedTransaction {
-			signed: tx1.clone(),
+			signed: tx1.clone().into(),
 			block_number: block_number,
 			block_hash: block_hash,
 			transaction_index: 1,
+			cached_sender: Some(tx1.sender()),
 		};
 		let logs = vec![LogEntry {
 			address: 5.into(),

ethcore/src/client/test_client.rs

@@ -21,7 +21,7 @@ use util::*;
 use rlp::*;
 use ethkey::{Generator, Random};
 use devtools::*;
-use transaction::{Transaction, LocalizedTransaction, SignedTransaction, PendingTransaction, Action};
+use transaction::{Transaction, LocalizedTransaction, PendingTransaction, SignedTransaction, Action};
 use blockchain::TreeRoute;
 use client::{
 	BlockChainClient, MiningBlockChainClient, EngineClient, BlockChainInfo, BlockStatus, BlockId,
@@ -320,8 +320,8 @@ impl TestBlockChainClient {
 			nonce: U256::zero()
 		};
 		let signed_tx = tx.sign(keypair.secret(), None);
-		self.set_balance(signed_tx.sender().unwrap(), 10_000_000.into());
+		self.set_balance(signed_tx.sender(), 10_000_000.into());
-		let res = self.miner.import_external_transactions(self, vec![signed_tx]);
+		let res = self.miner.import_external_transactions(self, vec![signed_tx.into()]);
 		let res = res.into_iter().next().unwrap().expect("Successful import");
 		assert_eq!(res, TransactionImportResult::Current);
 	}

ethcore/src/client/traits.rs

@@ -21,7 +21,7 @@ use blockchain::TreeRoute;
 use verification::queue::QueueInfo as BlockQueueInfo;
 use block::{OpenBlock, SealedBlock};
 use header::{BlockNumber};
-use transaction::{LocalizedTransaction, SignedTransaction, PendingTransaction};
+use transaction::{LocalizedTransaction, PendingTransaction, SignedTransaction};
 use log_entry::LocalizedLogEntry;
 use filter::Filter;
 use error::{ImportResult, CallError};

ethcore/src/engines/authority_round.rs

@@ -33,7 +33,6 @@ use views::HeaderView;
 use evm::Schedule;
 use ethjson;
 use io::{IoContext, IoHandler, TimerToken, IoService};
-use transaction::SignedTransaction;
 use env_info::EnvInfo;
 use builtin::Builtin;
 use client::{Client, EngineClient};
@@ -312,15 +311,6 @@ impl Engine for AuthorityRound {
 		Ok(())
 	}
 
-	fn verify_transaction_basic(&self, t: &SignedTransaction, _header: &Header) -> Result<(), Error> {
-		t.check_low_s()?;
-		Ok(())
-	}
-
-	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 {
 		let new_number = new_header.number();
 		let best_number = best_header.number();

ethcore/src/engines/basic_authority.rs

@@ -29,7 +29,6 @@ use error::{BlockError, Error};
 use evm::Schedule;
 use ethjson;
 use header::Header;
-use transaction::SignedTransaction;
 use client::Client;
 use super::validator_set::{ValidatorSet, new_validator_set};
 
@@ -171,15 +170,6 @@ impl Engine for BasicAuthority {
 		Ok(())
 	}
 
-	fn verify_transaction_basic(&self, t: &SignedTransaction, _header: &Header) -> result::Result<(), Error> {
-		t.check_low_s()?;
-		Ok(())
-	}
-
-	fn verify_transaction(&self, t: &SignedTransaction, _header: &Header) -> Result<(), Error> {
-		t.sender().map(|_|()) // Perform EC recovery and cache sender
-	}
-
 	fn register_client(&self, client: Weak<Client>) {
 		self.validators.register_call_contract(client);
 	}

ethcore/src/engines/mod.rs

@@ -39,7 +39,7 @@ use error::Error;
 use spec::CommonParams;
 use evm::Schedule;
 use header::Header;
-use transaction::SignedTransaction;
+use transaction::{UnverifiedTransaction, SignedTransaction};
 use ethereum::ethash;
 use blockchain::extras::BlockDetails;
 use views::HeaderView;
@@ -155,9 +155,15 @@ pub trait Engine : Sync + Send {
 	/// Additional verification for transactions in blocks.
 	// TODO: Add flags for which bits of the transaction to check.
 	// TODO: consider including State in the params.
-	fn verify_transaction_basic(&self, _t: &SignedTransaction, _header: &Header) -> Result<(), Error> { Ok(()) }
+	fn verify_transaction_basic(&self, t: &UnverifiedTransaction, _header: &Header) -> Result<(), Error> {
+		t.check_low_s()?;
+		Ok(())
+	}
+
 	/// Verify a particular transaction is valid.
-	fn verify_transaction(&self, _t: &SignedTransaction, _header: &Header) -> Result<(), Error> { Ok(()) }
+	fn verify_transaction(&self, t: UnverifiedTransaction, _header: &Header) -> Result<SignedTransaction, Error> {
+		SignedTransaction::new(t)
+	}
 
 	/// The network ID that transactions should be signed with.
 	fn signing_network_id(&self, _env_info: &EnvInfo) -> Option<u64> { None }

ethcore/src/engines/tendermint/mod.rs

@@ -35,7 +35,6 @@ use error::{Error, BlockError};
 use header::Header;
 use builtin::Builtin;
 use env_info::EnvInfo;
-use transaction::SignedTransaction;
 use rlp::{UntrustedRlp, View};
 use ethkey::{recover, public_to_address};
 use account_provider::AccountProvider;
@@ -564,15 +563,6 @@ impl Engine for Tendermint {
 		Ok(())
 	}
 
-	fn verify_transaction_basic(&self, t: &SignedTransaction, _header: &Header) -> Result<(), Error> {
-		t.check_low_s()?;
-		Ok(())
-	}
-
-	fn verify_transaction(&self, t: &SignedTransaction, _header: &Header) -> Result<(), Error> {
-		t.sender().map(|_|()) // Perform EC recovery and cache sender
-	}
-
 	fn set_signer(&self, address: Address, password: String) {
 		*self.authority.write()	= address;
 		*self.password.write() = Some(password);

ethcore/src/ethereum/ethash.rs

@@ -24,7 +24,7 @@ use header::Header;
 use views::HeaderView;
 use state::CleanupMode;
 use spec::CommonParams;
-use transaction::SignedTransaction;
+use transaction::UnverifiedTransaction;
 use engines::Engine;
 use evm::Schedule;
 use ethjson;
@@ -185,7 +185,7 @@ impl Engine for Ethash {
 			} else if gas_limit > gas_ceil_target {
 				max(gas_ceil_target, lower_limit)
 			} else {
-				max(gas_floor_target, min(min(gas_ceil_target, upper_limit), 
+				max(gas_floor_target, min(min(gas_ceil_target, upper_limit),
 					lower_limit + (header.gas_used().clone() * 6.into() / 5.into()) / bound_divisor))
 			}
 		};
@@ -310,7 +310,7 @@ impl Engine for Ethash {
 		Ok(())
 	}
 
-	fn verify_transaction_basic(&self, t: &SignedTransaction, header: &Header) -> result::Result<(), Error> {
+	fn verify_transaction_basic(&self, t: &UnverifiedTransaction, header: &Header) -> result::Result<(), Error> {
 		if header.number() >= self.ethash_params.homestead_transition {
 			t.check_low_s()?;
 		}
@@ -324,10 +324,6 @@ impl Engine for Ethash {
 		Ok(())
 	}
 
-	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)
 	}

ethcore/src/executive.rs

@@ -122,10 +122,7 @@ impl<'a> Executive<'a> {
 		mut tracer: T,
 		mut vm_tracer: V
 	) -> Result<Executed, ExecutionError> where T: Tracer, V: VMTracer {
-		let sender = t.sender().map_err(|e| {
+		let sender = t.sender();
-			let message = format!("Transaction malformed: {:?}", e);
-			ExecutionError::TransactionMalformed(message)
-		})?;
 		let nonce = self.state.nonce(&sender);
 
 		let schedule = self.engine.schedule(self.info);
@@ -435,14 +432,7 @@ impl<'a> Executive<'a> {
 		trace!("exec::finalize: t.gas={}, sstore_refunds={}, suicide_refunds={}, refunds_bound={}, gas_left_prerefund={}, refunded={}, gas_left={}, gas_used={}, refund_value={}, fees_value={}\n",
 			t.gas, sstore_refunds, suicide_refunds, refunds_bound, gas_left_prerefund, refunded, gas_left, gas_used, refund_value, fees_value);
 
-		let sender = match t.sender() {
+		let sender = t.sender();
-			Ok(sender) => sender,
-			Err(e) => {
-				debug!(target: "executive", "attempted to finalize transaction without sender: {}", e);
-				return Err(ExecutionError::Internal);
-			}
-		};
-
 		trace!("exec::finalize: Refunding refund_value={}, sender={}\n", refund_value, sender);
 		// Below: NoEmpty is safe since the sender must already be non-null to have sent this transaction
 		self.state.add_balance(&sender, &refund_value, CleanupMode::NoEmpty);
@@ -1057,7 +1047,7 @@ mod tests {
 			gas_price: U256::zero(),
 			nonce: U256::zero()
 		}.sign(keypair.secret(), None);
-		let sender = t.sender().unwrap();
+		let sender = t.sender();
 		let contract = contract_address(&sender, &U256::zero());
 
 		let mut state_result = get_temp_state();
@@ -1085,34 +1075,6 @@ mod tests {
 		assert_eq!(state.storage_at(&contract, &H256::new()), H256::from(&U256::from(1)));
 	}
 
-	evm_test!{test_transact_invalid_sender: test_transact_invalid_sender_jit, test_transact_invalid_sender_int}
-	fn test_transact_invalid_sender(factory: Factory) {
-		let t = Transaction {
-			action: Action::Create,
-			value: U256::from(17),
-			data: "3331600055".from_hex().unwrap(),
-			gas: U256::from(100_000),
-			gas_price: U256::zero(),
-			nonce: U256::zero()
-		}.invalid_sign();
-		let mut state_result = get_temp_state();
-		let mut state = state_result.reference_mut();
-		let mut info = EnvInfo::default();
-		info.gas_limit = U256::from(100_000);
-		let engine = TestEngine::new(0);
-
-		let res = {
-			let mut ex = Executive::new(&mut state, &info, &engine, &factory);
-			let opts = TransactOptions { check_nonce: true, tracing: false, vm_tracing: false };
-			ex.transact(&t, opts)
-		};
-
-		match res {
-			Err(ExecutionError::TransactionMalformed(_)) => (),
-			_ => assert!(false, "Expected an invalid transaction error.")
-		}
-	}
-
 	evm_test!{test_transact_invalid_nonce: test_transact_invalid_nonce_jit, test_transact_invalid_nonce_int}
 	fn test_transact_invalid_nonce(factory: Factory) {
 		let keypair = Random.generate().unwrap();
@@ -1124,7 +1086,7 @@ mod tests {
 			gas_price: U256::zero(),
 			nonce: U256::one()
 		}.sign(keypair.secret(), None);
-		let sender = t.sender().unwrap();
+		let sender = t.sender();
 
 		let mut state_result = get_temp_state();
 		let mut state = state_result.reference_mut();
@@ -1157,7 +1119,7 @@ mod tests {
 			gas_price: U256::zero(),
 			nonce: U256::zero()
 		}.sign(keypair.secret(), None);
-		let sender = t.sender().unwrap();
+		let sender = t.sender();
 
 		let mut state_result = get_temp_state();
 		let mut state = state_result.reference_mut();
@@ -1192,7 +1154,7 @@ mod tests {
 			gas_price: U256::one(),
 			nonce: U256::zero()
 		}.sign(keypair.secret(), None);
-		let sender = t.sender().unwrap();
+		let sender = t.sender();
 
 		let mut state_result = get_temp_state();
 		let mut state = state_result.reference_mut();

ethcore/src/json_tests/homestead_chain.rs

@@ -26,8 +26,7 @@ declare_test!{BlockchainTests_Homestead_bcBlockGasLimitTest, "BlockchainTests/Ho
 declare_test!{BlockchainTests_Homestead_bcForkStressTest, "BlockchainTests/Homestead/bcForkStressTest"}
 declare_test!{BlockchainTests_Homestead_bcGasPricerTest, "BlockchainTests/Homestead/bcGasPricerTest"}
 declare_test!{BlockchainTests_Homestead_bcInvalidHeaderTest, "BlockchainTests/Homestead/bcInvalidHeaderTest"}
-// TODO [ToDr] Ignored because of incorrect JSON (https://github.com/ethereum/tests/pull/113)
+declare_test!{BlockchainTests_Homestead_bcInvalidRLPTest, "BlockchainTests/Homestead/bcInvalidRLPTest"}
-declare_test!{ignore => BlockchainTests_Homestead_bcInvalidRLPTest, "BlockchainTests/Homestead/bcInvalidRLPTest"}
 declare_test!{BlockchainTests_Homestead_bcMultiChainTest, "BlockchainTests/Homestead/bcMultiChainTest"}
 declare_test!{BlockchainTests_Homestead_bcRPC_API_Test, "BlockchainTests/Homestead/bcRPC_API_Test"}
 declare_test!{BlockchainTests_Homestead_bcStateTest, "BlockchainTests/Homestead/bcStateTest"}

ethcore/src/json_tests/transaction.rs

@@ -18,7 +18,8 @@ use super::test_common::*;
 use evm;
 use ethjson;
 use rlp::{UntrustedRlp, View};
-use transaction::{Action, SignedTransaction};
+use transaction::{Action, UnverifiedTransaction};
+use ethstore::ethkey::public_to_address;
 
 fn do_json_test(json_data: &[u8]) -> Vec<String> {
 	let tests = ethjson::transaction::Test::load(json_data).unwrap();
@@ -34,18 +35,18 @@ fn do_json_test(json_data: &[u8]) -> Vec<String> {
 			Some(x) if x < 1_150_000 => &old_schedule,
 			Some(_) => &new_schedule
 		};
-		let allow_network_id_of_one = number.map_or(false, |n| n >= 2_675_000); 
+		let allow_network_id_of_one = number.map_or(false, |n| n >= 2_675_000);
 
 		let rlp: Vec<u8> = test.rlp.into();
 		let res = UntrustedRlp::new(&rlp)
 			.as_val()
 			.map_err(From::from)
-			.and_then(|t: SignedTransaction| t.validate(schedule, schedule.have_delegate_call, allow_network_id_of_one));
+			.and_then(|t: UnverifiedTransaction| t.validate(schedule, schedule.have_delegate_call, allow_network_id_of_one));
 
 		fail_unless(test.transaction.is_none() == res.is_err(), "Validity different");
 		if let (Some(tx), Some(sender)) = (test.transaction, test.sender) {
 			let t = res.unwrap();
-			fail_unless(t.sender().unwrap() == sender.into(), "sender mismatch");
+			fail_unless(public_to_address(&t.recover_public().unwrap()) == sender.into(), "sender mismatch");
 			let is_acceptable_network_id = match t.network_id() {
 				None => true,
 				Some(1) if allow_network_id_of_one => true,

ethcore/src/lib.rs

@@ -89,7 +89,6 @@ extern crate num_cpus;
 extern crate crossbeam;
 extern crate ethjson;
 extern crate bloomchain;
-extern crate rayon;
 extern crate hyper;
 extern crate ethash;
 extern crate ethkey;

ethcore/src/miner/banning_queue.rs

@@ -90,12 +90,11 @@ impl BanningTransactionQueue {
 			// NOTE In all checks use direct query to avoid increasing ban timeout.
 
 			// Check sender
-			if let Ok(sender) = transaction.sender() {
+			let sender = transaction.sender();
-				let count = self.senders_bans.direct().get(&sender).map(|v| v.get()).unwrap_or(0);
+			let count = self.senders_bans.direct().get(&sender).map(|v| v.get()).unwrap_or(0);
-				if count > threshold {
+			if count > threshold {
-					debug!(target: "txqueue", "Ignoring transaction {:?} because sender is banned.", transaction.hash());
+				debug!(target: "txqueue", "Ignoring transaction {:?} because sender is banned.", transaction.hash());
-					return Err(Error::Transaction(TransactionError::SenderBanned));
+				return Err(Error::Transaction(TransactionError::SenderBanned));
-				}
 			}
 
 			// Check recipient
@@ -128,7 +127,7 @@ impl BanningTransactionQueue {
 		let transaction = self.queue.find(hash);
 		match transaction {
 			Some(transaction) => {
-				let sender = transaction.sender().expect("Transaction is in queue, so the sender is already validated; qed");
+				let sender = transaction.sender();
 				// Ban sender
 				let sender_banned = self.ban_sender(sender);
 				// Ban recipient and codehash
@@ -278,14 +277,14 @@ mod tests {
 		let tx = transaction(Action::Create);
 		let mut txq = queue();
 		// Banlist once (threshold not reached)
-		let banlist1 = txq.ban_sender(tx.sender().unwrap());
+		let banlist1 = txq.ban_sender(tx.sender());
 		assert!(!banlist1, "Threshold not reached yet.");
 		// Insert once
 		let import1 = txq.add_with_banlist(tx.clone(), 0, &default_account_details, &gas_required).unwrap();
 		assert_eq!(import1, TransactionImportResult::Current);
 
 		// when
-		let banlist2 = txq.ban_sender(tx.sender().unwrap());
+		let banlist2 = txq.ban_sender(tx.sender());
 		let import2 = txq.add_with_banlist(tx.clone(), 0, &default_account_details, &gas_required);
 
 		// then

ethcore/src/miner/miner.rs

@@ -14,7 +14,6 @@
 // You should have received a copy of the GNU General Public License
 // along with Parity.  If not, see <http://www.gnu.org/licenses/>.
 
-use rayon::prelude::*;
 use std::time::{Instant, Duration};
 
 use util::*;
@@ -26,7 +25,7 @@ use client::TransactionImportResult;
 use executive::contract_address;
 use block::{ClosedBlock, IsBlock, Block};
 use error::*;
-use transaction::{Action, SignedTransaction, PendingTransaction};
+use transaction::{Action, UnverifiedTransaction, PendingTransaction, SignedTransaction};
 use receipt::{Receipt, RichReceipt};
 use spec::Spec;
 use engines::{Engine, Seal};
@@ -299,9 +298,9 @@ impl Miner {
 		self.sealing_work.lock().queue.peek_last_ref().map(|b| b.block().fields().state.clone())
 	}
 
-	/// Get `Some` `clone()` of the current pending block's state or `None` if we're not sealing.
+	/// Get `Some` `clone()` of the current pending block or `None` if we're not sealing.
 	pub fn pending_block(&self) -> Option<Block> {
-		self.sealing_work.lock().queue.peek_last_ref().map(|b| b.base().clone())
+		self.sealing_work.lock().queue.peek_last_ref().map(|b| b.to_base())
 	}
 
 	#[cfg_attr(feature="dev", allow(match_same_arms))]
@@ -576,11 +575,11 @@ impl Miner {
 	fn add_transactions_to_queue(
 		&self,
 		chain: &MiningBlockChainClient,
-		transactions: Vec<SignedTransaction>,
+		transactions: Vec<UnverifiedTransaction>,
 		default_origin: TransactionOrigin,
 		min_block: Option<BlockNumber>,
-		transaction_queue: &mut BanningTransactionQueue)
+		transaction_queue: &mut BanningTransactionQueue,
-		-> Vec<Result<TransactionImportResult, Error>> {
+	) -> Vec<Result<TransactionImportResult, Error>> {
 
 		let fetch_account = |a: &Address| AccountDetails {
 			nonce: chain.latest_nonce(a),
@@ -598,29 +597,32 @@ impl Miner {
 
 		transactions.into_iter()
 			.map(|tx| {
-				if chain.transaction_block(TransactionId::Hash(tx.hash())).is_some() {
+				let hash = tx.hash();
-					debug!(target: "miner", "Rejected tx {:?}: already in the blockchain", tx.hash());
+				if chain.transaction_block(TransactionId::Hash(hash)).is_some() {
+					debug!(target: "miner", "Rejected tx {:?}: already in the blockchain", hash);
 					return Err(Error::Transaction(TransactionError::AlreadyImported));
 				}
-				match self.engine.verify_transaction_basic(&tx, &best_block_header) {
+				match self.engine.verify_transaction_basic(&tx, &best_block_header)
+					.and_then(|_| self.engine.verify_transaction(tx, &best_block_header))
+				{
 					Err(e) => {
-						debug!(target: "miner", "Rejected tx {:?} with invalid signature: {:?}", tx.hash(), e);
+						debug!(target: "miner", "Rejected tx {:?} with invalid signature: {:?}", hash, e);
 						Err(e)
 					},
-					Ok(()) => {
+					Ok(transaction) => {
 						let origin = accounts.as_ref().and_then(|accounts| {
-							tx.sender().ok().and_then(|sender| match accounts.contains(&sender) {
+							match accounts.contains(&transaction.sender()) {
 								true => Some(TransactionOrigin::Local),
 								false => None,
-							})
+							}
 						}).unwrap_or(default_origin);
 
 						match origin {
 							TransactionOrigin::Local | TransactionOrigin::RetractedBlock => {
-								transaction_queue.add(tx, origin, insertion_time, min_block, &fetch_account, &gas_required)
+								transaction_queue.add(transaction, origin, insertion_time, min_block, &fetch_account, &gas_required)
 							},
 							TransactionOrigin::External => {
-								transaction_queue.add_with_banlist(tx, insertion_time, &fetch_account, &gas_required)
+								transaction_queue.add_with_banlist(transaction, insertion_time, &fetch_account, &gas_required)
 							}
 						}
 					},
@@ -695,10 +697,7 @@ impl MinerService for Miner {
 				let mut state = block.state().clone();
 				let original_state = if analytics.state_diffing { Some(state.clone()) } else { None };
 
-				let sender = t.sender().map_err(|e| {
+				let sender = t.sender();
-					let message = format!("Transaction malformed: {:?}", e);
-					ExecutionError::TransactionMalformed(message)
-				})?;
 				let balance = state.balance(&sender);
 				let needed_balance = t.value + t.gas * t.gas_price;
 				if balance < needed_balance {
@@ -843,7 +842,7 @@ impl MinerService for Miner {
 	fn import_external_transactions(
 		&self,
 		chain: &MiningBlockChainClient,
-		transactions: Vec<SignedTransaction>
+		transactions: Vec<UnverifiedTransaction>
 	) -> Vec<Result<TransactionImportResult, Error>> {
 		trace!(target: "external_tx", "Importing external transactions");
 		let results = {
@@ -876,8 +875,9 @@ impl MinerService for Miner {
 		let imported = {
 			// Be sure to release the lock before we call prepare_work_sealing
 			let mut transaction_queue = self.transaction_queue.lock();
+			// We need to re-validate transactions
 			let import = self.add_transactions_to_queue(
-				chain, vec![pending.transaction], TransactionOrigin::Local, pending.min_block, &mut transaction_queue
+				chain, vec![pending.transaction.into()], TransactionOrigin::Local, pending.min_block, &mut transaction_queue
 			).pop().expect("one result returned per added transaction; one added => one result; qed");
 
 			match import {
@@ -1013,8 +1013,7 @@ impl MinerService for Miner {
 							contract_address: match tx.action {
 								Action::Call(_) => None,
 								Action::Create => {
-									let sender = tx.sender()
+									let sender = tx.sender();
-										.expect("transactions in pending block have already been checked for valid sender; qed");
 									Some(contract_address(&sender, &tx.nonce))
 								}
 							},
@@ -1126,22 +1125,16 @@ impl MinerService for Miner {
 
 		// Then import all transactions...
 		{
-			retracted.par_iter()
+
-				.map(|hash| {
+			let mut transaction_queue = self.transaction_queue.lock();
-					let block = chain.block(BlockId::Hash(*hash))
+			for hash in retracted {
-						.expect("Client is sending message after commit to db and inserting to chain; the block is available; qed");
+				let block = chain.block(BlockId::Hash(*hash))
-					let txs = block.transactions();
+					.expect("Client is sending message after commit to db and inserting to chain; the block is available; qed");
-					// populate sender
+				let txs = block.transactions();
-					for tx in &txs {
+				let _ = self.add_transactions_to_queue(
-						let _sender = tx.sender();
+					chain, txs, TransactionOrigin::RetractedBlock, None, &mut transaction_queue
-					}
+				);
-					txs
+			}
-				}).for_each(|txs| {
-					let mut transaction_queue = self.transaction_queue.lock();
-					let _ = self.add_transactions_to_queue(
-						chain, txs, TransactionOrigin::RetractedBlock, None, &mut transaction_queue
-					);
-				});
 		}
 
 		// ...and at the end remove the old ones
@@ -1177,7 +1170,7 @@ mod tests {
 	use ethkey::{Generator, Random};
 	use client::{BlockChainClient, TestBlockChainClient, EachBlockWith, TransactionImportResult};
 	use header::BlockNumber;
-	use types::transaction::{Transaction, SignedTransaction, PendingTransaction, Action};
+	use types::transaction::{SignedTransaction, Transaction, PendingTransaction, Action};
 	use spec::Spec;
 	use tests::helpers::{generate_dummy_client};
 
@@ -1291,7 +1284,7 @@ mod tests {
 		// given
 		let client = TestBlockChainClient::default();
 		let miner = miner();
-		let transaction = transaction();
+		let transaction = transaction().into();
 		let best_block = 0;
 		// when
 		let res = miner.import_external_transactions(&client, vec![transaction]).pop().unwrap();
@@ -1313,7 +1306,7 @@ mod tests {
 		// By default resealing is not required.
 		assert!(!miner.requires_reseal(1u8.into()));
 
-		miner.import_external_transactions(&client, vec![transaction()]).pop().unwrap().unwrap();
+		miner.import_external_transactions(&client, vec![transaction().into()]).pop().unwrap().unwrap();
 		assert!(miner.prepare_work_sealing(&client));
 		// Unless asked to prepare work.
 		assert!(miner.requires_reseal(1u8.into()));
@@ -1326,14 +1319,14 @@ mod tests {
 		let c = generate_dummy_client(2);
 		let client = c.reference().as_ref();
 
-		assert_eq!(miner.import_external_transactions(client, vec![transaction()]).pop().unwrap().unwrap(), TransactionImportResult::Current);
+		assert_eq!(miner.import_external_transactions(client, vec![transaction().into()]).pop().unwrap().unwrap(), TransactionImportResult::Current);
 
 		miner.update_sealing(client);
 		client.flush_queue();
 		assert!(miner.pending_block().is_none());
 		assert_eq!(client.chain_info().best_block_number, 3 as BlockNumber);
 
-		assert_eq!(miner.import_own_transaction(client, PendingTransaction::new(transaction(), None)).unwrap(), TransactionImportResult::Current);
+		assert_eq!(miner.import_own_transaction(client, PendingTransaction::new(transaction().into(), None)).unwrap(), TransactionImportResult::Current);
 
 		miner.update_sealing(client);
 		client.flush_queue();

ethcore/src/miner/mod.rs

@@ -62,7 +62,7 @@ use block::ClosedBlock;
 use header::BlockNumber;
 use receipt::{RichReceipt, Receipt};
 use error::{Error, CallError};
-use transaction::{SignedTransaction, PendingTransaction};
+use transaction::{UnverifiedTransaction, PendingTransaction, SignedTransaction};
 
 /// Miner client API
 pub trait MinerService : Send + Sync {
@@ -114,7 +114,7 @@ pub trait MinerService : Send + Sync {
 	fn set_tx_gas_limit(&self, limit: U256);
 
 	/// Imports transactions to transaction queue.
-	fn import_external_transactions(&self, chain: &MiningBlockChainClient, transactions: Vec<SignedTransaction>) ->
+	fn import_external_transactions(&self, chain: &MiningBlockChainClient, transactions: Vec<UnverifiedTransaction>) ->
 		Vec<Result<TransactionImportResult, Error>>;
 
 	/// Imports own (node owner) transaction to queue.

ethcore/src/miner/transaction_queue.rs

@@ -251,7 +251,7 @@ impl Ord for TransactionOrder {
 	}
 }
 
-/// Verified transaction (with sender)
+/// Verified transaction
 #[derive(Debug)]
 struct VerifiedTransaction {
 	/// Transaction.
@@ -265,14 +265,13 @@ struct VerifiedTransaction {
 }
 
 impl VerifiedTransaction {
-	fn new(transaction: SignedTransaction, origin: TransactionOrigin, time: QueuingInstant, min_block: Option<BlockNumber>) -> Result<Self, Error> {
+	fn new(transaction: SignedTransaction, origin: TransactionOrigin, time: QueuingInstant, min_block: Option<BlockNumber>) -> Self {
-		transaction.sender()?;
+		VerifiedTransaction {
-		Ok(VerifiedTransaction {
 			transaction: transaction,
 			origin: origin,
 			insertion_time: time,
 			min_block: min_block,
-		})
+		}
 	}
 
 	fn hash(&self) -> H256 {
@@ -284,7 +283,7 @@ impl VerifiedTransaction {
 	}
 
 	fn sender(&self) -> Address {
-		self.transaction.sender().expect("Sender is verified in new; qed")
+		self.transaction.sender()
 	}
 
 	fn cost(&self) -> U256 {
@@ -663,7 +662,7 @@ impl TransactionQueue {
 				Err(Error::Transaction(ref err)) => {
 					// Sometimes transactions are re-imported, so
 					// don't overwrite transactions if they are already on the list
-					if !self.local_transactions.contains(&cloned_tx.hash()) {
+					if !self.local_transactions.contains(&hash) {
 						self.local_transactions.mark_rejected(cloned_tx, err.clone());
 					}
 				},
@@ -749,17 +748,12 @@ impl TransactionQueue {
 			}));
 		}
 
-		// Verify signature
+		let client_account = fetch_account(&tx.sender());
-		tx.check_low_s()?;
+		let cost = tx.value + tx.gas_price * tx.gas;
-
-		let vtx = VerifiedTransaction::new(tx, origin, time, min_block)?;
-		let client_account = fetch_account(&vtx.sender());
-
-		let cost = vtx.cost();
 		if client_account.balance < cost {
 			trace!(target: "txqueue",
 				"Dropping transaction without sufficient balance: {:?} ({} < {})",
-				vtx.hash(),
+				tx.hash(),
 				client_account.balance,
 				cost
 			);
@@ -769,7 +763,9 @@ impl TransactionQueue {
 				balance: client_account.balance
 			}));
 		}
-
+		tx.check_low_s()?;
+		// No invalid transactions beyond this point.
+		let vtx = VerifiedTransaction::new(tx, origin, time, min_block);
 		let r = self.import_tx(vtx, client_account.nonce).map_err(Error::Transaction);
 		assert_eq!(self.future.by_priority.len() + self.current.by_priority.len(), self.by_hash.len());
 		r
@@ -918,7 +914,7 @@ impl TransactionQueue {
 
 		// Mark in locals
 		if self.local_transactions.contains(transaction_hash) {
-			self.local_transactions.mark_invalid(transaction.transaction.clone());
+			self.local_transactions.mark_invalid(transaction.transaction.into());
 		}
 
 		// Remove from future
@@ -1391,7 +1387,7 @@ mod test {
 
 		let keypair = Random.generate().unwrap();
 		let secret = &keypair.secret();
-		(tx1.sign(secret, None), tx2.sign(secret, None))
+		(tx1.sign(secret, None).into(), tx2.sign(secret, None).into())
 	}
 
 	/// Returns two consecutive transactions, both with increased gas price
@@ -1402,7 +1398,7 @@ mod test {
 
 		let keypair = Random.generate().unwrap();
 		let secret = &keypair.secret();
-		(tx1.sign(secret, None), tx2.sign(secret, None))
+		(tx1.sign(secret, None).into(), tx2.sign(secret, None).into())
 	}
 
 	fn new_tx_pair_default(nonce_increment: U256, gas_price_increment: U256) -> (SignedTransaction, SignedTransaction) {
@@ -1434,7 +1430,7 @@ mod test {
 		// given
 		let mut txq = TransactionQueue::with_limits(PrioritizationStrategy::GasPriceOnly, 2, !U256::zero(), !U256::zero());
 		let (tx1, tx2) = new_tx_pair(123.into(), 1.into(), 1.into(), 0.into());
-		let sender = tx1.sender().unwrap();
+		let sender = tx1.sender();
 		let nonce = tx1.nonce;
 		txq.add(tx1.clone(), TransactionOrigin::External, 0, None, &default_account_details, &gas_estimator).unwrap();
 		txq.add(tx2.clone(), TransactionOrigin::External, 0, None, &default_account_details, &gas_estimator).unwrap();
@@ -1475,12 +1471,12 @@ mod test {
 			gas_limit: !U256::zero(),
 		};
 		let (tx1, tx2) = new_tx_pair_default(1.into(), 0.into());
-		let tx1 = VerifiedTransaction::new(tx1, TransactionOrigin::External, 0, None).unwrap();
+		let tx1 = VerifiedTransaction::new(tx1, TransactionOrigin::External, 0, None);
-		let tx2 = VerifiedTransaction::new(tx2, TransactionOrigin::External, 0, None).unwrap();
+		let tx2 = VerifiedTransaction::new(tx2, TransactionOrigin::External, 0, None);
 		let mut by_hash = {
 			let mut x = HashMap::new();
-			let tx1 = VerifiedTransaction::new(tx1.transaction.clone(), TransactionOrigin::External, 0, None).unwrap();
+			let tx1 = VerifiedTransaction::new(tx1.transaction.clone(), TransactionOrigin::External, 0, None);
-			let tx2 = VerifiedTransaction::new(tx2.transaction.clone(), TransactionOrigin::External, 0, None).unwrap();
+			let tx2 = VerifiedTransaction::new(tx2.transaction.clone(), TransactionOrigin::External, 0, None);
 			x.insert(tx1.hash(), tx1);
 			x.insert(tx2.hash(), tx2);
 			x
@@ -1518,12 +1514,12 @@ mod test {
 		// Create two transactions with same nonce
 		// (same hash)
 		let (tx1, tx2) = new_tx_pair_default(0.into(), 0.into());
-		let tx1 = VerifiedTransaction::new(tx1, TransactionOrigin::External, 0, None).unwrap();
+		let tx1 = VerifiedTransaction::new(tx1, TransactionOrigin::External, 0, None);
-		let tx2 = VerifiedTransaction::new(tx2, TransactionOrigin::External, 0, None).unwrap();
+		let tx2 = VerifiedTransaction::new(tx2, TransactionOrigin::External, 0, None);
 		let by_hash = {
 			let mut x = HashMap::new();
-			let tx1 = VerifiedTransaction::new(tx1.transaction.clone(), TransactionOrigin::External, 0, None).unwrap();
+			let tx1 = VerifiedTransaction::new(tx1.transaction.clone(), TransactionOrigin::External, 0, None);
-			let tx2 = VerifiedTransaction::new(tx2.transaction.clone(), TransactionOrigin::External, 0, None).unwrap();
+			let tx2 = VerifiedTransaction::new(tx2.transaction.clone(), TransactionOrigin::External, 0, None);
 			x.insert(tx1.hash(), tx1);
 			x.insert(tx2.hash(), tx2);
 			x
@@ -1565,10 +1561,10 @@ mod test {
 			gas_limit: !U256::zero(),
 		};
 		let tx = new_tx_default();
-		let tx1 = VerifiedTransaction::new(tx.clone(), TransactionOrigin::External, 0, None).unwrap();
+		let tx1 = VerifiedTransaction::new(tx.clone(), TransactionOrigin::External, 0, None);
 		let order1 = TransactionOrder::for_transaction(&tx1, 0.into(), 1.into(), PrioritizationStrategy::GasPriceOnly);
 		assert!(set.insert(tx1.sender(), tx1.nonce(), order1).is_none());
-		let tx2 = VerifiedTransaction::new(tx, TransactionOrigin::External, 0, None).unwrap();
+		let tx2 = VerifiedTransaction::new(tx, TransactionOrigin::External, 0, None);
 		let order2 = TransactionOrder::for_transaction(&tx2, 0.into(), 1.into(), PrioritizationStrategy::GasPriceOnly);
 		assert!(set.insert(tx2.sender(), tx2.nonce(), order2).is_some());
 	}
@@ -1585,7 +1581,7 @@ mod test {
 
 		assert_eq!(set.gas_price_entry_limit(), 0.into());
 		let tx = new_tx_default();
-		let tx1 = VerifiedTransaction::new(tx.clone(), TransactionOrigin::External, 0, None).unwrap();
+		let tx1 = VerifiedTransaction::new(tx.clone(), TransactionOrigin::External, 0, None);
 		let order1 = TransactionOrder::for_transaction(&tx1, 0.into(), 1.into(), PrioritizationStrategy::GasPriceOnly);
 		assert!(set.insert(tx1.sender(), tx1.nonce(), order1.clone()).is_none());
 		assert_eq!(set.gas_price_entry_limit(), 2.into());
@@ -1613,7 +1609,8 @@ mod test {
 		assert_eq!(txq.status().future, 0);
 		assert_eq!(txq.current.by_priority.len(), 1);
 		assert_eq!(txq.current.by_address.len(), 1);
-		assert_eq!(txq.top_transactions()[0], tx2);
+		let top = txq.top_transactions();
+		assert_eq!(top[0], tx2);
 	}
 
 	#[test]
@@ -1638,8 +1635,9 @@ mod test {
 		assert_eq!(txq.status().future, 0);
 		assert_eq!(txq.current.by_priority.len(), 2);
 		assert_eq!(txq.current.by_address.len(), 2);
-		assert_eq!(txq.top_transactions()[0], tx);
+		let top = txq.top_transactions();
-		assert_eq!(txq.top_transactions()[1], tx2);
+		assert_eq!(top[0], tx);
+		assert_eq!(top[1], tx2);
 	}
 
 	#[test]
@@ -1821,33 +1819,6 @@ mod test {
 		assert_eq!(stats.future, 0);
 	}
 
-	#[test]
-	fn should_reject_incorectly_signed_transaction() {
-		use rlp::{self, RlpStream, Stream};
-
-		// given
-		let mut txq = TransactionQueue::default();
-		let tx = new_unsigned_tx(123.into(), 100.into(), 1.into());
-		let stx = {
-			let mut s = RlpStream::new_list(9);
-			s.append(&tx.nonce);
-			s.append(&tx.gas_price);
-			s.append(&tx.gas);
-			s.append_empty_data(); // action=create
-			s.append(&tx.value);
-			s.append(&tx.data);
-			s.append(&0u64); // v
-			s.append(&U256::zero()); // r
-			s.append(&U256::zero()); // s
-			rlp::decode(s.as_raw())
-		};
-		// when
-		let res = txq.add(stx, TransactionOrigin::External, 0, None, &default_account_details, &gas_estimator);
-
-		// then
-		assert!(res.is_err());
-	}
-
 	#[test]
 	fn should_import_txs_from_same_sender() {
 		// given
@@ -1898,8 +1869,9 @@ mod test {
 		txq.add(tx0.clone(), TransactionOrigin::External, 0, None, &default_account_details, &gas_estimator).unwrap();
 		txq.add(tx1.clone(), TransactionOrigin::External, 0, None, &default_account_details, &gas_estimator).unwrap();
 		// the one with higher gas price is first
-		assert_eq!(txq.top_transactions()[0], tx0);
+		let top = txq.top_transactions();
-		assert_eq!(txq.top_transactions()[1], tx1);
+		assert_eq!(top[0], tx0);
+		assert_eq!(top[1], tx1);
 
 		// when
 		// insert second as local
@@ -1907,9 +1879,10 @@ mod test {
 
 		// then
 		// the order should be updated
-		assert_eq!(txq.top_transactions()[0], tx1);
+		let top = txq.top_transactions();
-		assert_eq!(txq.top_transactions()[1], tx2);
+		assert_eq!(top[0], tx1);
-		assert_eq!(txq.top_transactions()[2], tx0);
+		assert_eq!(top[1], tx2);
+		assert_eq!(top[2], tx0);
 	}
 
 	#[test]
@@ -1971,11 +1944,11 @@ mod test {
 		txq.penalize(&tx1.hash());
 
 		// then
-		let top = txq.future_transactions();
+		let top: Vec<_> = txq.future_transactions().into_iter().map(|tx| tx.transaction).collect();
-		assert_eq!(top[0].transaction, txa);
+		assert_eq!(top[0], txa);
-		assert_eq!(top[1].transaction, txb);
+		assert_eq!(top[1], txb);
-		assert_eq!(top[2].transaction, tx1);
+		assert_eq!(top[2], tx1);
-		assert_eq!(top[3].transaction, tx2);
+		assert_eq!(top[3], tx2);
 		assert_eq!(top.len(), 4);
 	}
 
@@ -2120,7 +2093,7 @@ mod test {
 		assert_eq!(txq.status().future, 2);
 
 		// when
-		txq.cull(tx.sender().unwrap(), next2_nonce);
+		txq.cull(tx.sender(), next2_nonce);
 		// should remove both transactions since they are not valid
 
 		// then
@@ -2134,9 +2107,9 @@ mod test {
 		let mut txq = TransactionQueue::default();
 		let kp = Random.generate().unwrap();
 		let secret = kp.secret();
-		let tx = new_unsigned_tx(123.into(), default_gas_val(), 1.into()).sign(secret, None);
+		let tx = new_unsigned_tx(123.into(), default_gas_val(), 1.into()).sign(secret, None).into();
-		let tx1 = new_unsigned_tx(124.into(), default_gas_val(), 1.into()).sign(secret, None);
+		let tx1 = new_unsigned_tx(124.into(), default_gas_val(), 1.into()).sign(secret, None).into();
-		let tx2 = new_unsigned_tx(125.into(), default_gas_val(), 1.into()).sign(secret, None);
+		let tx2 = new_unsigned_tx(125.into(), default_gas_val(), 1.into()).sign(secret, None).into();
 
 		txq.add(tx, TransactionOrigin::External, 0, None, &default_account_details, &gas_estimator).unwrap();
 		assert_eq!(txq.status().pending, 1);
@@ -2166,9 +2139,8 @@ mod test {
 		assert_eq!(txq2.status().future, 1);
 
 		// when
-		txq2.cull(tx.sender().unwrap(), tx.nonce + U256::one());
+		txq2.cull(tx.sender(), tx.nonce + U256::one());
-		txq2.cull(tx2.sender().unwrap(), tx2.nonce + U256::one());
+		txq2.cull(tx2.sender(), tx2.nonce + U256::one());
-
 
 		// then
 		let stats = txq2.status();
@@ -2222,7 +2194,7 @@ mod test {
 		// given
 		let mut txq = TransactionQueue::with_limits(PrioritizationStrategy::GasPriceOnly, 1, !U256::zero(), !U256::zero());
 		let (tx, tx2) = new_tx_pair_default(1.into(), 0.into());
-		let sender = tx.sender().unwrap();
+		let sender = tx.sender();
 		let nonce = tx.nonce;
 		txq.add(tx.clone(), TransactionOrigin::External, 0, None, &default_account_details, &gas_estimator).unwrap();
 		assert_eq!(txq.status().pending, 1);
@@ -2358,8 +2330,7 @@ mod test {
 		assert_eq!(txq.status().pending, 3);
 
 		// when
-		let sender = tx.sender().unwrap();
+		txq.cull(tx.sender(), default_nonce() + U256::one());
-		txq.cull(sender, default_nonce() + U256::one());
 
 		// then
 		let stats = txq.status();
@@ -2375,7 +2346,7 @@ mod test {
 		let keypair = Random.generate().unwrap();
 		let tx = new_unsigned_tx(123.into(), default_gas_val(), 1.into()).sign(keypair.secret(), None);
 		let tx2 = {
-			let mut tx2 = (*tx).clone();
+			let mut tx2 = (**tx).clone();
 			tx2.gas_price = U256::from(200);
 			tx2.sign(keypair.secret(), None)
 		};
@@ -2398,12 +2369,12 @@ mod test {
 		let keypair = Random.generate().unwrap();
 		let tx0 = new_unsigned_tx(123.into(), default_gas_val(), 1.into()).sign(keypair.secret(), None);
 		let tx1 = {
-			let mut tx1 = (*tx0).clone();
+			let mut tx1 = (**tx0).clone();
 			tx1.nonce = U256::from(124);
 			tx1.sign(keypair.secret(), None)
 		};
 		let tx2 = {
-			let mut tx2 = (*tx1).clone();
+			let mut tx2 = (**tx1).clone();
 			tx2.gas_price = U256::from(200);
 			tx2.sign(keypair.secret(), None)
 		};
@@ -2455,7 +2426,7 @@ mod test {
 		// given
 		let mut txq = TransactionQueue::default();
 		let tx = new_tx_default();
-		let from = tx.sender().unwrap();
+		let from = tx.sender();
 		let nonce = tx.nonce;
 		let details = |_a: &Address| AccountDetails { nonce: nonce, balance: !U256::zero() };
 
@@ -2478,7 +2449,7 @@ mod test {
 		txq.add(tx1, TransactionOrigin::External, 0, None, &details1, &gas_estimator).unwrap();
 
 		// when
-		txq.cull(tx2.sender().unwrap(), nonce2 + U256::one());
+		txq.cull(tx2.sender(), nonce2 + U256::one());
 
 		// then
 		assert!(txq.top_transactions().is_empty());
@@ -2489,7 +2460,7 @@ mod test {
 		// given
 		let mut txq = TransactionQueue::default();
 		let (tx1, tx2) = new_tx_pair_default(4.into(), 0.into());
-		let sender = tx1.sender().unwrap();
+		let sender = tx1.sender();
 		let (nonce1, nonce2) = (tx1.nonce, tx2.nonce);
 		let details1 = |_a: &Address| AccountDetails { nonce: nonce1, balance: !U256::zero() };
 
@@ -2558,7 +2529,7 @@ mod test {
 
 			(tx.sign(secret, None), tx2.sign(secret, None), tx2_2.sign(secret, None), tx3.sign(secret, None))
 		};
-		let sender = tx1.sender().unwrap();
+		let sender = tx1.sender();
 		txq.add(tx1, TransactionOrigin::Local, 0, None, &default_account_details, &gas_estimator).unwrap();
 		txq.add(tx2, TransactionOrigin::Local, 0, None, &default_account_details, &gas_estimator).unwrap();
 		txq.add(tx3, TransactionOrigin::Local, 0, None, &default_account_details, &gas_estimator).unwrap();

ethcore/src/snapshot/block.rs

@@ -183,8 +183,8 @@ mod tests {
 			data: "Eep!".into(),
 		}.fake_sign(Address::from(0x55));
 
-		b.transactions.push(t1);
+		b.transactions.push(t1.into());
-		b.transactions.push(t2);
+		b.transactions.push(t2.into());
 
 		let receipts_root = b.header.receipts_root().clone();
 		b.header.set_transactions_root(::util::triehash::ordered_trie_root(

ethcore/src/state/mod.rs

@@ -879,7 +879,7 @@ mod tests {
 			data: FromHex::from_hex("601080600c6000396000f3006000355415600957005b60203560003555").unwrap(),
 		}.sign(&secret(), None);
 
-		state.add_balance(t.sender().as_ref().unwrap(), &(100.into()), CleanupMode::NoEmpty);
+		state.add_balance(&t.sender(), &(100.into()), CleanupMode::NoEmpty);
 		let result = state.apply(&info, &engine, &t, true).unwrap();
 		let expected_trace = vec![FlatTrace {
 			trace_address: Default::default(),
@@ -939,7 +939,7 @@ mod tests {
 			data: FromHex::from_hex("5b600056").unwrap(),
 		}.sign(&secret(), None);
 
-		state.add_balance(t.sender().as_ref().unwrap(), &(100.into()), CleanupMode::NoEmpty);
+		state.add_balance(&t.sender(), &(100.into()), CleanupMode::NoEmpty);
 		let result = state.apply(&info, &engine, &t, true).unwrap();
 		let expected_trace = vec![FlatTrace {
 			trace_address: Default::default(),
@@ -977,7 +977,7 @@ mod tests {
 		}.sign(&secret(), None);
 
 		state.init_code(&0xa.into(), FromHex::from_hex("6000").unwrap());
-		state.add_balance(t.sender().as_ref().unwrap(), &(100.into()), CleanupMode::NoEmpty);
+		state.add_balance(&t.sender(), &(100.into()), CleanupMode::NoEmpty);
 		let result = state.apply(&info, &engine, &t, true).unwrap();
 		let expected_trace = vec![FlatTrace {
 			trace_address: Default::default(),
@@ -1019,7 +1019,7 @@ mod tests {
 			data: vec![],
 		}.sign(&secret(), None);
 
-		state.add_balance(t.sender().as_ref().unwrap(), &(100.into()), CleanupMode::NoEmpty);
+		state.add_balance(&t.sender(), &(100.into()), CleanupMode::NoEmpty);
 		let result = state.apply(&info, &engine, &t, true).unwrap();
 		let expected_trace = vec![FlatTrace {
 			trace_address: Default::default(),
@@ -1268,7 +1268,7 @@ mod tests {
 		}.sign(&secret(), None);
 
 		state.init_code(&0xa.into(), FromHex::from_hex("5b600056").unwrap());
-		state.add_balance(t.sender().as_ref().unwrap(), &(100.into()), CleanupMode::NoEmpty);
+		state.add_balance(&t.sender(), &(100.into()), CleanupMode::NoEmpty);
 		let result = state.apply(&info, &engine, &t, true).unwrap();
 		let expected_trace = vec![FlatTrace {
 			trace_address: Default::default(),
@@ -1309,7 +1309,7 @@ mod tests {
 
 		state.init_code(&0xa.into(), FromHex::from_hex("60006000600060006000600b602b5a03f1").unwrap());
 		state.init_code(&0xb.into(), FromHex::from_hex("6000").unwrap());
-		state.add_balance(t.sender().as_ref().unwrap(), &(100.into()), CleanupMode::NoEmpty);
+		state.add_balance(&t.sender(), &(100.into()), CleanupMode::NoEmpty);
 		let result = state.apply(&info, &engine, &t, true).unwrap();
 
 		let expected_trace = vec![FlatTrace {
@@ -1368,7 +1368,7 @@ mod tests {
 		}.sign(&secret(), None);
 
 		state.init_code(&0xa.into(), FromHex::from_hex("60006000600060006045600b6000f1").unwrap());
-		state.add_balance(t.sender().as_ref().unwrap(), &(100.into()), CleanupMode::NoEmpty);
+		state.add_balance(&t.sender(), &(100.into()), CleanupMode::NoEmpty);
 		let result = state.apply(&info, &engine, &t, true).unwrap();
 		let expected_trace = vec![FlatTrace {
 			trace_address: Default::default(),
@@ -1423,7 +1423,7 @@ mod tests {
 		}.sign(&secret(), None);
 
 		state.init_code(&0xa.into(), FromHex::from_hex("600060006000600060ff600b6000f1").unwrap());	// not enough funds.
-		state.add_balance(t.sender().as_ref().unwrap(), &(100.into()), CleanupMode::NoEmpty);
+		state.add_balance(&t.sender(), &(100.into()), CleanupMode::NoEmpty);
 		let result = state.apply(&info, &engine, &t, true).unwrap();
 		let expected_trace = vec![FlatTrace {
 			trace_address: Default::default(),
@@ -1467,7 +1467,7 @@ mod tests {
 
 		state.init_code(&0xa.into(), FromHex::from_hex("60006000600060006000600b602b5a03f1").unwrap());
 		state.init_code(&0xb.into(), FromHex::from_hex("5b600056").unwrap());
-		state.add_balance(t.sender().as_ref().unwrap(), &(100.into()), CleanupMode::NoEmpty);
+		state.add_balance(&t.sender(), &(100.into()), CleanupMode::NoEmpty);
 		let result = state.apply(&info, &engine, &t, true).unwrap();
 		let expected_trace = vec![FlatTrace {
 			trace_address: Default::default(),
@@ -1524,7 +1524,7 @@ mod tests {
 		state.init_code(&0xa.into(), FromHex::from_hex("60006000600060006000600b602b5a03f1").unwrap());
 		state.init_code(&0xb.into(), FromHex::from_hex("60006000600060006000600c602b5a03f1").unwrap());
 		state.init_code(&0xc.into(), FromHex::from_hex("6000").unwrap());
-		state.add_balance(t.sender().as_ref().unwrap(), &(100.into()), CleanupMode::NoEmpty);
+		state.add_balance(&t.sender(), &(100.into()), CleanupMode::NoEmpty);
 		let result = state.apply(&info, &engine, &t, true).unwrap();
 		let expected_trace = vec![FlatTrace {
 			trace_address: Default::default(),
@@ -1599,7 +1599,7 @@ mod tests {
 		state.init_code(&0xa.into(), FromHex::from_hex("60006000600060006000600b602b5a03f1").unwrap());
 		state.init_code(&0xb.into(), FromHex::from_hex("60006000600060006000600c602b5a03f1505b601256").unwrap());
 		state.init_code(&0xc.into(), FromHex::from_hex("6000").unwrap());
-		state.add_balance(t.sender().as_ref().unwrap(), &(100.into()), CleanupMode::NoEmpty);
+		state.add_balance(&t.sender(), &(100.into()), CleanupMode::NoEmpty);
 		let result = state.apply(&info, &engine, &t, true).unwrap();
 
 		let expected_trace = vec![FlatTrace {
@@ -1671,7 +1671,7 @@ mod tests {
 
 		state.init_code(&0xa.into(), FromHex::from_hex("73000000000000000000000000000000000000000bff").unwrap());
 		state.add_balance(&0xa.into(), &50.into(), CleanupMode::NoEmpty);
-		state.add_balance(t.sender().as_ref().unwrap(), &100.into(), CleanupMode::NoEmpty);
+		state.add_balance(&t.sender(), &100.into(), CleanupMode::NoEmpty);
 		let result = state.apply(&info, &engine, &t, true).unwrap();
 		let expected_trace = vec![FlatTrace {
 			trace_address: Default::default(),

ethcore/src/tests/helpers.rs

@@ -32,7 +32,7 @@ use ethereum::ethash::EthashParams;
 use devtools::*;
 use miner::Miner;
 use header::Header;
-use transaction::{Action, SignedTransaction, Transaction};
+use transaction::{Action, Transaction, SignedTransaction};
 use rlp::{self, RlpStream, Stream};
 use views::BlockView;
 
@@ -126,7 +126,7 @@ pub fn create_test_block_with_data(header: &Header, transactions: &[SignedTransa
 	rlp.append(header);
 	rlp.begin_list(transactions.len());
 	for t in transactions {
-		rlp.append_raw(&rlp::encode::<SignedTransaction>(t).to_vec(), 1);
+		rlp.append_raw(&rlp::encode(t).to_vec(), 1);
 	}
 	rlp.append(&uncles);
 	rlp.out()

ethcore/src/types/encoded.rs

@@ -25,7 +25,7 @@
 
 use block::Block as FullBlock;
 use header::{BlockNumber, Header as FullHeader};
-use transaction::SignedTransaction;
+use transaction::UnverifiedTransaction;
 use views;
 
 use util::{Address, Hashable, H256, H2048, U256};
@@ -126,7 +126,7 @@ impl Body {
 	pub fn view(&self) -> views::BodyView { views::BodyView::new(&self.0) }
 
 	/// Fully decode this block body.
-	pub fn decode(&self) -> (Vec<SignedTransaction>, Vec<FullHeader>) {
+	pub fn decode(&self) -> (Vec<UnverifiedTransaction>, Vec<FullHeader>) {
 		(self.view().transactions(), self.view().uncles())
 	}
 
@@ -143,7 +143,7 @@ impl Body {
 // forwarders to borrowed view.
 impl Body {
 	/// Get a vector of all transactions.
-	pub fn transactions(&self) -> Vec<SignedTransaction> { self.view().transactions() }
+	pub fn transactions(&self) -> Vec<UnverifiedTransaction> { self.view().transactions() }
 
 	/// Number of transactions in the block.
 	pub fn transactions_count(&self) -> usize { self.view().transactions_count() }
@@ -248,7 +248,7 @@ impl Block {
 // forwarders to body view.
 impl Block {
 	/// Get a vector of all transactions.
-	pub fn transactions(&self) -> Vec<SignedTransaction> { self.view().transactions() }
+	pub fn transactions(&self) -> Vec<UnverifiedTransaction> { self.view().transactions() }
 
 	/// Number of transactions in the block.
 	pub fn transactions_count(&self) -> usize { self.view().transactions_count() }

ethcore/src/types/transaction.rs

@@ -16,8 +16,7 @@
 
 //! Transaction data structure.
 
-use std::ops::{Deref, DerefMut};
+use std::ops::Deref;
-use std::cell::*;
 use rlp::*;
 use util::sha3::Hashable;
 use util::{H256, Address, U256, Bytes, HeapSizeOf};
@@ -117,10 +116,10 @@ impl From<ethjson::state::Transaction> for SignedTransaction {
 	}
 }
 
-impl From<ethjson::transaction::Transaction> for SignedTransaction {
+impl From<ethjson::transaction::Transaction> for UnverifiedTransaction {
 	fn from(t: ethjson::transaction::Transaction) -> Self {
 		let to: Option<ethjson::hash::Address> = t.to.into();
-		SignedTransaction {
+		UnverifiedTransaction {
 			unsigned: Transaction {
 				nonce: t.nonce.into(),
 				gas_price: t.gas_price.into(),
@@ -135,9 +134,8 @@ impl From<ethjson::transaction::Transaction> for SignedTransaction {
 			r: t.r.into(),
 			s: t.s.into(),
 			v: t.v.into(),
-			sender: Cell::new(None),
+			hash: 0.into(),
-			hash: Cell::new(None)
+		}.compute_hash()
-		}
 	}
 }
 
@@ -153,43 +151,45 @@ impl Transaction {
 	pub fn sign(self, secret: &Secret, network_id: Option<u64>) -> SignedTransaction {
 		let sig = ::ethkey::sign(secret, &self.hash(network_id))
 			.expect("data is valid and context has signing capabilities; qed");
-		self.with_signature(sig, network_id)
+		SignedTransaction::new(self.with_signature(sig, network_id))
+			.expect("secret is valid so it's recoverable")
 	}
 
 	/// Signs the transaction with signature.
-	pub fn with_signature(self, sig: Signature, network_id: Option<u64>) -> SignedTransaction {
+	pub fn with_signature(self, sig: Signature, network_id: Option<u64>) -> UnverifiedTransaction {
-		SignedTransaction {
+		UnverifiedTransaction {
 			unsigned: self,
 			r: sig.r().into(),
 			s: sig.s().into(),
 			v: sig.v() as u64 + if let Some(n) = network_id { 35 + n * 2 } else { 27 },
-			hash: Cell::new(None),
+			hash: 0.into(),
-			sender: Cell::new(None),
+		}.compute_hash()
-		}
 	}
 
 	/// Useful for test incorrectly signed transactions.
 	#[cfg(test)]
-	pub fn invalid_sign(self) -> SignedTransaction {
+	pub fn invalid_sign(self) -> UnverifiedTransaction {
-		SignedTransaction {
+		UnverifiedTransaction {
 			unsigned: self,
 			r: U256::default(),
 			s: U256::default(),
 			v: 0,
-			hash: Cell::new(None),
+			hash: 0.into(),
-			sender: Cell::new(None),
+		}.compute_hash()
-		}
 	}
 
 	/// Specify the sender; this won't survive the serialize/deserialize process, but can be cloned.
 	pub fn fake_sign(self, from: Address) -> SignedTransaction {
 		SignedTransaction {
-			unsigned: self,
+			transaction: UnverifiedTransaction {
-			r: U256::default(),
+				unsigned: self,
-			s: U256::default(),
+				r: U256::default(),
-			v: 0,
+				s: U256::default(),
-			hash: Cell::new(None),
+				v: 0,
-			sender: Cell::new(Some(from)),
+				hash: 0.into(),
+			}.compute_hash(),
+			sender: from,
+			public: Public::default(),
 		}
 	}
 
@@ -208,9 +208,9 @@ impl Transaction {
 }
 
 /// Signed transaction information.
-#[derive(Debug, Clone, Eq)]
+#[derive(Debug, Clone, Eq, PartialEq)]
 #[cfg_attr(feature = "ipc", binary)]
-pub struct SignedTransaction {
+pub struct UnverifiedTransaction {
 	/// Plain Transaction.
 	unsigned: Transaction,
 	/// The V field of the signature; the LS bit described which half of the curve our point falls
@@ -220,19 +220,11 @@ pub struct SignedTransaction {
 	r: U256,
 	/// The S field of the signature; helps describe the point on the curve.
 	s: U256,
-	/// Cached hash.
+	/// Hash of the transaction
-	hash: Cell<Option<H256>>,
+	hash: H256,
-	/// Cached sender.
-	sender: Cell<Option<Address>>,
 }
 
-impl PartialEq for SignedTransaction {
+impl Deref for UnverifiedTransaction {
-	fn eq(&self, other: &SignedTransaction) -> bool {
-		self.unsigned == other.unsigned && self.v == other.v && self.r == other.r && self.s == other.s
-	}
-}
-
-impl Deref for SignedTransaction {
 	type Target = Transaction;
 
 	fn deref(&self) -> &Self::Target {
@@ -240,19 +232,14 @@ impl Deref for SignedTransaction {
 	}
 }
 
-impl DerefMut for SignedTransaction {
+impl Decodable for UnverifiedTransaction {
-	fn deref_mut(&mut self) -> &mut Self::Target {
-		&mut self.unsigned
-	}
-}
-
-impl Decodable for SignedTransaction {
 	fn decode<D>(decoder: &D) -> Result<Self, DecoderError> where D: Decoder {
 		let d = decoder.as_rlp();
 		if d.item_count() != 9 {
 			return Err(DecoderError::RlpIncorrectListLen);
 		}
-		Ok(SignedTransaction {
+		let hash = decoder.as_raw().sha3();
+		Ok(UnverifiedTransaction {
 			unsigned: Transaction {
 				nonce: d.val_at(0)?,
 				gas_price: d.val_at(1)?,
@@ -264,23 +251,23 @@ impl Decodable for SignedTransaction {
 			v: d.val_at(6)?,
 			r: d.val_at(7)?,
 			s: d.val_at(8)?,
-			hash: Cell::new(None),
+			hash: hash,
-			sender: Cell::new(None),
 		})
 	}
 }
 
-impl Encodable for SignedTransaction {
+impl Encodable for UnverifiedTransaction {
 	fn rlp_append(&self, s: &mut RlpStream) { self.rlp_append_sealed_transaction(s) }
 }
 
-impl HeapSizeOf for SignedTransaction {
+impl UnverifiedTransaction {
-	fn heap_size_of_children(&self) -> usize {
+	/// Used to compute hash of created transactions
-		self.unsigned.heap_size_of_children()
+	fn compute_hash(mut self) -> UnverifiedTransaction {
+		let hash = (&*self.rlp_bytes()).sha3();
+		self.hash = hash;
+		self
 	}
-}
 
-impl SignedTransaction {
 	/// Append object with a signature into RLP stream
 	fn rlp_append_sealed_transaction(&self, s: &mut RlpStream) {
 		s.begin_list(9);
@@ -298,17 +285,9 @@ impl SignedTransaction {
 		s.append(&self.s);
 	}
 
-	/// Get the hash of this header (sha3 of the RLP).
+	///	Reference to unsigned part of this transaction.
-	pub fn hash(&self) -> H256 {
+	pub fn as_unsigned(&self) -> &Transaction {
-		let hash = self.hash.get();
+		&self.unsigned
-		match hash {
-			Some(h) => h,
-			None => {
-				let h = (&*self.rlp_bytes()).sha3();
-				self.hash.set(Some(h));
-				h
-			}
-		}
 	}
 
 	/// 0 if `v` would have been 27 under "Electrum" notation, 1 if 28 or 4 if invalid.
@@ -339,21 +318,13 @@ impl SignedTransaction {
 		}
 	}
 
-	/// Returns transaction sender.
+	/// Get the hash of this header (sha3 of the RLP).
-	pub fn sender(&self) -> Result<Address, Error> {
+	pub fn hash(&self) -> H256 {
-		let sender = self.sender.get();
+		self.hash
-		match sender {
-			Some(s) => Ok(s),
-			None => {
-				let s = public_to_address(&self.public_key()?);
-				self.sender.set(Some(s));
-				Ok(s)
-			}
-		}
 	}
 
-	/// Returns the public key of the sender.
+	/// Recovers the public key of the sender.
-	pub fn public_key(&self) -> Result<Public, Error> {
+	pub fn recover_public(&self) -> Result<Public, Error> {
 		Ok(recover(&self.signature(), &self.unsigned.hash(self.network_id()))?)
 	}
 
@@ -361,7 +332,7 @@ impl SignedTransaction {
 	// TODO: consider use in block validation.
 	#[cfg(test)]
 	#[cfg(feature = "json-tests")]
-	pub fn validate(self, schedule: &Schedule, require_low: bool, allow_network_id_of_one: bool) -> Result<SignedTransaction, Error> {
+	pub fn validate(self, schedule: &Schedule, require_low: bool, allow_network_id_of_one: bool) -> Result<UnverifiedTransaction, Error> {
 		if require_low && !self.signature().is_low_s() {
 			return Err(EthkeyError::InvalidSignature.into())
 		}
@@ -370,7 +341,7 @@ impl SignedTransaction {
 			Some(1) if allow_network_id_of_one => {},
 			_ => return Err(TransactionError::InvalidNetworkId.into()),
 		}
-		self.sender()?;
+		self.recover_public()?;
 		if self.gas < U256::from(self.gas_required(&schedule)) {
 			Err(TransactionError::InvalidGasLimit(::util::OutOfBounds{min: Some(U256::from(self.gas_required(&schedule))), max: None, found: self.gas}).into())
 		} else {
@@ -379,22 +350,92 @@ impl SignedTransaction {
 	}
 }
 
+/// A `UnverifiedTransaction` with successfully recovered `sender`.
+#[derive(Debug, Clone, Eq, PartialEq)]
+pub struct SignedTransaction {
+	transaction: UnverifiedTransaction,
+	sender: Address,
+	public: Public,
+}
+
+impl HeapSizeOf for SignedTransaction {
+	fn heap_size_of_children(&self) -> usize {
+		self.transaction.unsigned.heap_size_of_children()
+	}
+}
+
+impl Encodable for SignedTransaction {
+	fn rlp_append(&self, s: &mut RlpStream) { self.transaction.rlp_append_sealed_transaction(s) }
+}
+
+impl Deref for SignedTransaction {
+	type Target = UnverifiedTransaction;
+	fn deref(&self) -> &Self::Target {
+		&self.transaction
+	}
+}
+
+impl From<SignedTransaction> for UnverifiedTransaction {
+	fn from(tx: SignedTransaction) -> Self {
+		tx.transaction
+	}
+}
+
+impl SignedTransaction {
+	/// Try to verify transaction and recover sender.
+	pub fn new(transaction: UnverifiedTransaction) -> Result<Self, Error> {
+		let public = transaction.recover_public()?;
+		let sender = public_to_address(&public);
+		Ok(SignedTransaction {
+			transaction: transaction,
+			sender: sender,
+			public: public,
+		})
+	}
+
+	/// Returns transaction sender.
+	pub fn sender(&self) -> Address {
+		self.sender
+	}
+
+	/// Returns a public key of the sender.
+	pub fn public_key(&self) -> Public {
+		self.public
+	}
+}
+
 /// Signed Transaction that is a part of canon blockchain.
 #[derive(Debug, Clone, PartialEq, Eq)]
 #[cfg_attr(feature = "ipc", binary)]
 pub struct LocalizedTransaction {
 	/// Signed part.
-	pub signed: SignedTransaction,
+	pub signed: UnverifiedTransaction,
 	/// Block number.
 	pub block_number: BlockNumber,
 	/// Block hash.
 	pub block_hash: H256,
 	/// Transaction index within block.
-	pub transaction_index: usize
+	pub transaction_index: usize,
+	/// Cached sender
+	pub cached_sender: Option<Address>,
+}
+
+impl LocalizedTransaction {
+	/// Returns transaction sender.
+	/// Panics if `LocalizedTransaction` is constructed using invalid `UnverifiedTransaction`.
+	pub fn sender(&mut self) -> Address {
+		if let Some(sender) = self.cached_sender {
+			return sender;
+		}
+		let sender = public_to_address(&self.recover_public()
+			.expect("LocalizedTransaction is always constructed from transaction from blockchain; Blockchain only stores verified transactions; qed"));
+		self.cached_sender = Some(sender);
+		sender
+	}
 }
 
 impl Deref for LocalizedTransaction {
-	type Target = SignedTransaction;
+	type Target = UnverifiedTransaction;
 
 	fn deref(&self) -> &Self::Target {
 		&self.signed
@@ -432,7 +473,7 @@ impl From<SignedTransaction> for PendingTransaction {
 
 #[test]
 fn sender_test() {
-	let t: SignedTransaction = decode(&::rustc_serialize::hex::FromHex::from_hex("f85f800182520894095e7baea6a6c7c4c2dfeb977efac326af552d870a801ba048b55bfa915ac795c431978d8a6a992b628d557da5ff759b307d495a36649353a0efffd310ac743f371de3b9f7f9cb56c0b28ad43601b4ab949f53faa07bd2c804").unwrap());
+	let t: UnverifiedTransaction = decode(&::rustc_serialize::hex::FromHex::from_hex("f85f800182520894095e7baea6a6c7c4c2dfeb977efac326af552d870a801ba048b55bfa915ac795c431978d8a6a992b628d557da5ff759b307d495a36649353a0efffd310ac743f371de3b9f7f9cb56c0b28ad43601b4ab949f53faa07bd2c804").unwrap());
 	assert_eq!(t.data, b"");
 	assert_eq!(t.gas, U256::from(0x5208u64));
 	assert_eq!(t.gas_price, U256::from(0x01u64));
@@ -441,7 +482,7 @@ fn sender_test() {
 		assert_eq!(*to, "095e7baea6a6c7c4c2dfeb977efac326af552d87".into());
 	} else { panic!(); }
 	assert_eq!(t.value, U256::from(0x0au64));
-	assert_eq!(t.sender().unwrap(), "0f65fe9276bc9a24ae7083ae28e2660ef72df99e".into());
+	assert_eq!(public_to_address(&t.recover_public().unwrap()), "0f65fe9276bc9a24ae7083ae28e2660ef72df99e".into());
 	assert_eq!(t.network_id(), None);
 }
 
@@ -458,7 +499,7 @@ fn signing() {
 		value: U256::from(1),
 		data: b"Hello!".to_vec()
 	}.sign(&key.secret(), None);
-	assert_eq!(Address::from(key.public().sha3()), t.sender().unwrap());
+	assert_eq!(Address::from(key.public().sha3()), t.sender());
 	assert_eq!(t.network_id(), None);
 }
 
@@ -472,11 +513,11 @@ fn fake_signing() {
 		value: U256::from(1),
 		data: b"Hello!".to_vec()
 	}.fake_sign(Address::from(0x69));
-	assert_eq!(Address::from(0x69), t.sender().unwrap());
+	assert_eq!(Address::from(0x69), t.sender());
 	assert_eq!(t.network_id(), None);
 
 	let t = t.clone();
-	assert_eq!(Address::from(0x69), t.sender().unwrap());
+	assert_eq!(Address::from(0x69), t.sender());
 	assert_eq!(t.network_id(), None);
 }
 
@@ -492,7 +533,7 @@ fn should_recover_from_network_specific_signing() {
 		value: U256::from(1),
 		data: b"Hello!".to_vec()
 	}.sign(&key.secret(), Some(69));
-	assert_eq!(Address::from(key.public().sha3()), t.sender().unwrap());
+	assert_eq!(Address::from(key.public().sha3()), t.sender());
 	assert_eq!(t.network_id(), Some(69));
 }
 
@@ -501,9 +542,9 @@ fn should_agree_with_vitalik() {
 	use rustc_serialize::hex::FromHex;
 
 	let test_vector = |tx_data: &str, address: &'static str| {
-		let signed: SignedTransaction = decode(&FromHex::from_hex(tx_data).unwrap());
+		let signed = decode(&FromHex::from_hex(tx_data).unwrap());
-		signed.check_low_s().unwrap();
+		let signed = SignedTransaction::new(signed).unwrap();
-		assert_eq!(signed.sender().unwrap(), address.into());
+		assert_eq!(signed.sender(), address.into());
 		flushln!("networkid: {:?}", signed.network_id());
 	};
 

ethcore/src/verification/verification.rs

@@ -83,7 +83,7 @@ pub fn verify_block_unordered(header: Header, bytes: Bytes, engine: &Engine, che
 	{
 		let v = BlockView::new(&bytes);
 		for t in v.transactions() {
-			engine.verify_transaction(&t, &header)?;
+			let t = engine.verify_transaction(t, &header)?;
 			transactions.push(t);
 		}
 	}
@@ -460,7 +460,7 @@ mod tests {
 		let mut uncles_rlp = RlpStream::new();
 		uncles_rlp.append(&good_uncles);
 		let good_uncles_hash = uncles_rlp.as_raw().sha3();
-		let good_transactions_root = ordered_trie_root(good_transactions.iter().map(|t| ::rlp::encode::<SignedTransaction>(t).to_vec()));
+		let good_transactions_root = ordered_trie_root(good_transactions.iter().map(|t| ::rlp::encode::<UnverifiedTransaction>(t).to_vec()));
 
 		let mut parent = good.clone();
 		parent.set_number(9);

ethcore/src/views/block.rs

@@ -68,7 +68,7 @@ impl<'a> BlockView<'a> {
 	}
 
 	/// Return List of transactions in given block.
-	pub fn transactions(&self) -> Vec<SignedTransaction> {
+	pub fn transactions(&self) -> Vec<UnverifiedTransaction> {
 		self.rlp.val_at(1)
 	}
 
@@ -84,7 +84,8 @@ impl<'a> BlockView<'a> {
 				signed: t,
 				block_hash: block_hash.clone(),
 				block_number: block_number,
-				transaction_index: i
+				transaction_index: i,
+				cached_sender: None,
 			}).collect()
 	}
 
@@ -104,7 +105,7 @@ impl<'a> BlockView<'a> {
 	}
 
 	/// Returns transaction at given index without deserializing unnecessary data.
-	pub fn transaction_at(&self, index: usize) -> Option<SignedTransaction> {
+	pub fn transaction_at(&self, index: usize) -> Option<UnverifiedTransaction> {
 		self.rlp.at(1).iter().nth(index).map(|rlp| rlp.as_val())
 	}
 
@@ -117,7 +118,8 @@ impl<'a> BlockView<'a> {
 			signed: t,
 			block_hash: block_hash,
 			block_number: block_number,
-			transaction_index: index
+			transaction_index: index,
+			cached_sender: None,
 		})
 	}
 

ethcore/src/views/body.rs

@@ -48,7 +48,7 @@ impl<'a> BodyView<'a> {
 	}
 
 	/// Return List of transactions in given block.
-	pub fn transactions(&self) -> Vec<SignedTransaction> {
+	pub fn transactions(&self) -> Vec<UnverifiedTransaction> {
 		self.rlp.val_at(0)
 	}
 
@@ -61,7 +61,8 @@ impl<'a> BodyView<'a> {
 				signed: t,
 				block_hash: block_hash.clone(),
 				block_number: block_number,
-				transaction_index: i
+				transaction_index: i,
+				cached_sender: None,
 			}).collect()
 	}
 
@@ -81,7 +82,7 @@ impl<'a> BodyView<'a> {
 	}
 
 	/// Returns transaction at given index without deserializing unnecessary data.
-	pub fn transaction_at(&self, index: usize) -> Option<SignedTransaction> {
+	pub fn transaction_at(&self, index: usize) -> Option<UnverifiedTransaction> {
 		self.rlp.at(0).iter().nth(index).map(|rlp| rlp.as_val())
 	}
 
@@ -91,7 +92,8 @@ impl<'a> BodyView<'a> {
 			signed: t,
 			block_hash: block_hash.clone(),
 			block_number: block_number,
-			transaction_index: index
+			transaction_index: index,
+			cached_sender: None,
 		})
 	}
 

rpc/src/v1/helpers/dispatch.rs

@@ -179,7 +179,8 @@ pub fn sign_no_dispatch<C, M>(client: &C, miner: &M, accounts: &AccountProvider,
 		let hash = t.hash(network_id);
 		let signature = signature(accounts, address, hash, password)?;
 		signature.map(|sig| {
-			t.with_signature(sig, network_id)
+			SignedTransaction::new(t.with_signature(sig, network_id))
+				.expect("Transaction was signed by AccountsProvider; it never produces invalid signatures; qed")
 		})
 	};
 	Ok(signed_transaction)

rpc/src/v1/impls/eth.rs

@@ -647,11 +647,13 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM> Eth for EthClient<C, SN, S, M, EM> where
 	fn send_raw_transaction(&self, raw: Bytes) -> Result<RpcH256, Error> {
 		self.active()?;
 
-		let raw_transaction = raw.to_vec();
+		UntrustedRlp::new(&raw.into_vec()).as_val()
-		match UntrustedRlp::new(&raw_transaction).as_val() {
+			.map_err(errors::from_rlp_error)
-			Ok(signed_transaction) => dispatch_transaction(&*take_weak!(self.client), &*take_weak!(self.miner), PendingTransaction::new(signed_transaction, None)).map(Into::into),
+			.and_then(|tx| SignedTransaction::new(tx).map_err(errors::from_transaction_error))
-			Err(e) => Err(errors::from_rlp_error(e)),
+			.and_then(|signed_transaction| {
-		}
+				dispatch_transaction(&*take_weak!(self.client), &*take_weak!(self.miner), PendingTransaction::new(signed_transaction, None))
+			})
+			.map(Into::into)
 	}
 
 	fn submit_transaction(&self, raw: Bytes) -> Result<RpcH256, Error> {

rpc/src/v1/impls/parity_set.rs

@@ -101,7 +101,7 @@ impl<C, M, U, F> ParitySet for ParitySetClient<C, M, U, F> where
 	fn set_extra_data(&self, extra_data: Bytes) -> Result<bool, Error> {
 		self.active()?;
 
-		take_weak!(self.miner).set_extra_data(extra_data.to_vec());
+		take_weak!(self.miner).set_extra_data(extra_data.into_vec());
 		Ok(true)
 	}
 

rpc/src/v1/impls/signer.rs

@@ -140,8 +140,9 @@ impl<C: 'static, M: 'static> Signer for SignerClient<C, M> where C: MiningBlockC
 		signer.peek(&id).map(|confirmation| {
 			let result = match confirmation.payload {
 				ConfirmationPayload::SendTransaction(request) => {
-					let signed_transaction: SignedTransaction = UntrustedRlp::new(&bytes.0).as_val().map_err(errors::from_rlp_error)?;
+					let signed_transaction = UntrustedRlp::new(&bytes.0).as_val().map_err(errors::from_rlp_error)?;
-					let sender = signed_transaction.sender().map_err(|e| errors::invalid_params("Invalid signature.", e))?;
+					let signed_transaction = SignedTransaction::new(signed_transaction).map_err(|e| errors::invalid_params("Invalid signature.", e))?;
+					let sender = signed_transaction.sender();
 
 					// Verification
 					let sender_matches = sender == request.from;

rpc/src/v1/impls/traces.rs

@@ -128,14 +128,15 @@ impl<C, M> Traces for TracesClient<C, M> where C: BlockChainClient + 'static, M:
 		self.active()?;
 		let block = block.0;
 
-		let raw_transaction = Bytes::to_vec(raw_transaction);
+		UntrustedRlp::new(&raw_transaction.into_vec()).as_val()
-		match UntrustedRlp::new(&raw_transaction).as_val() {
+			.map_err(|e| errors::invalid_params("Transaction is not valid RLP", e))
-			Ok(signed) => Ok(match take_weak!(self.client).call(&signed, block.into(), to_call_analytics(flags)) {
+			.and_then(|tx| SignedTransaction::new(tx).map_err(errors::from_transaction_error))
-				Ok(e) => Some(TraceResults::from(e)),
+			.and_then(|signed| {
-				_ => None,
+				Ok(match take_weak!(self.client).call(&signed, block.into(), to_call_analytics(flags)) {
-			}),
+					Ok(e) => Some(TraceResults::from(e)),
-			Err(e) => Err(errors::invalid_params("Transaction is not valid RLP", e)),
+					_ => None,
-		}
+				})
+			})
 	}
 
 	fn replay_transaction(&self, transaction_hash: H256, flags: Vec<String>) -> Result<Option<TraceResults>, Error> {

rpc/src/v1/tests/helpers/miner_service.rs

@@ -22,7 +22,7 @@ use ethcore::error::{Error, CallError};
 use ethcore::client::{MiningBlockChainClient, Executed, CallAnalytics};
 use ethcore::block::{ClosedBlock, IsBlock};
 use ethcore::header::BlockNumber;
-use ethcore::transaction::{SignedTransaction, PendingTransaction};
+use ethcore::transaction::{UnverifiedTransaction, SignedTransaction, PendingTransaction};
 use ethcore::receipt::{Receipt, RichReceipt};
 use ethcore::miner::{MinerService, MinerStatus, TransactionImportResult, LocalTransactionStatus};
 use ethcore::account_provider::Error as AccountError;
@@ -144,12 +144,13 @@ impl MinerService for TestMinerService {
 	}
 
 	/// Imports transactions to transaction queue.
-	fn import_external_transactions(&self, _chain: &MiningBlockChainClient, transactions: Vec<SignedTransaction>) ->
+	fn import_external_transactions(&self, _chain: &MiningBlockChainClient, transactions: Vec<UnverifiedTransaction>) ->
 		Vec<Result<TransactionImportResult, Error>> {
 		// lets assume that all txs are valid
+		let transactions: Vec<_> = transactions.into_iter().map(|tx| SignedTransaction::new(tx).unwrap()).collect();
 		self.imported_transactions.lock().extend_from_slice(&transactions);
 
-		for sender in transactions.iter().filter_map(|t| t.sender().ok()) {
+		for sender in transactions.iter().map(|tx| tx.sender()) {
 			let nonce = self.last_nonce(&sender).expect("last_nonce must be populated in tests");
 			self.last_nonces.write().insert(sender, nonce + U256::from(1));
 		}
@@ -164,10 +165,9 @@ impl MinerService for TestMinerService {
 		Result<TransactionImportResult, Error> {
 
 		// keep the pending nonces up to date
-		if let Ok(ref sender) = pending.transaction.sender() {
+		let sender = pending.transaction.sender();
-			let nonce = self.last_nonce(sender).unwrap_or(chain.latest_nonce(sender));
+		let nonce = self.last_nonce(&sender).unwrap_or(chain.latest_nonce(&sender));
-			self.last_nonces.write().insert(sender.clone(), nonce + U256::from(1));
+		self.last_nonces.write().insert(sender, nonce + U256::from(1));
-		}
 
 		// lets assume that all txs are valid
 		self.imported_transactions.lock().push(pending.transaction);

rpc/src/v1/tests/mocked/eth.rs

@@ -498,12 +498,14 @@ fn rpc_eth_transaction_count_by_number_pending() {
 
 #[test]
 fn rpc_eth_pending_transaction_by_hash() {
-	use util::*;
+	use util::{H256, FromHex};
-	use ethcore::transaction::*;
+	use rlp;
+	use ethcore::transaction::SignedTransaction;
 
 	let tester = EthTester::default();
 	{
-		let tx: SignedTransaction = ::rlp::decode(&FromHex::from_hex("f85f800182520894095e7baea6a6c7c4c2dfeb977efac326af552d870a801ba048b55bfa915ac795c431978d8a6a992b628d557da5ff759b307d495a36649353a0efffd310ac743f371de3b9f7f9cb56c0b28ad43601b4ab949f53faa07bd2c804").unwrap());
+		let tx = rlp::decode(&FromHex::from_hex("f85f800182520894095e7baea6a6c7c4c2dfeb977efac326af552d870a801ba048b55bfa915ac795c431978d8a6a992b628d557da5ff759b307d495a36649353a0efffd310ac743f371de3b9f7f9cb56c0b28ad43601b4ab949f53faa07bd2c804").unwrap());
+		let tx = SignedTransaction::new(tx).unwrap();
 		tester.miner.pending_transactions.lock().insert(H256::zero(), tx);
 	}
 
@@ -831,7 +833,7 @@ fn rpc_eth_sign_transaction() {
 		r#""minBlock":null,"# +
 		&format!("\"networkId\":{},", t.network_id().map_or("null".to_owned(), |n| format!("{}", n))) +
 		r#""nonce":"0x1","# +
-		&format!("\"publicKey\":\"0x{:?}\",", t.public_key().unwrap()) +
+		&format!("\"publicKey\":\"0x{:?}\",", t.recover_public().unwrap()) +
 		&format!("\"r\":\"0x{}\",", U256::from(signature.r()).to_hex()) +
 		&format!("\"raw\":\"0x{}\",", rlp.to_hex()) +
 		&format!("\"s\":\"0x{}\",", U256::from(signature.s()).to_hex()) +

rpc/src/v1/tests/mocked/signing.rs

@@ -29,7 +29,7 @@ use v1::tests::mocked::parity;
 use util::{Address, FixedHash, Uint, U256, ToPretty, Hashable};
 use ethcore::account_provider::AccountProvider;
 use ethcore::client::TestBlockChainClient;
-use ethcore::transaction::{Transaction, Action};
+use ethcore::transaction::{Transaction, Action, SignedTransaction};
 use ethstore::ethkey::{Generator, Random};
 use futures::Future;
 use serde_json;
@@ -269,6 +269,7 @@ fn should_add_sign_transaction_to_the_queue() {
 	};
 	let signature = tester.accounts.sign(address, Some("test".into()), t.hash(None)).unwrap();
 	let t = t.with_signature(signature, None);
+	let t = SignedTransaction::new(t).unwrap();
 	let signature = t.signature();
 	let rlp = rlp::encode(&t);
 
@@ -283,7 +284,7 @@ fn should_add_sign_transaction_to_the_queue() {
 		r#""minBlock":null,"# +
 		&format!("\"networkId\":{},", t.network_id().map_or("null".to_owned(), |n| format!("{}", n))) +
 		r#""nonce":"0x1","# +
-		&format!("\"publicKey\":\"0x{:?}\",", t.public_key().unwrap()) +
+		&format!("\"publicKey\":\"0x{:?}\",", t.public_key()) +
 		&format!("\"r\":\"0x{}\",", U256::from(signature.r()).to_hex()) +
 		&format!("\"raw\":\"0x{}\",", rlp.to_hex()) +
 		&format!("\"s\":\"0x{}\",", U256::from(signature.s()).to_hex()) +

rpc/src/v1/types/bytes.rs

@@ -31,7 +31,7 @@ impl Bytes {
 		Bytes(bytes)
 	}
 	/// Convert back to vector
-	pub fn to_vec(self) -> Vec<u8> {
+	pub fn into_vec(self) -> Vec<u8> {
 		self.0
 	}
 }

rpc/src/v1/types/transaction.rs

@@ -162,7 +162,7 @@ impl From<SignedTransaction> for RichRawTransaction {
 }
 
 impl From<LocalizedTransaction> for Transaction {
-	fn from(t: LocalizedTransaction) -> Transaction {
+	fn from(mut t: LocalizedTransaction) -> Transaction {
 		let signature = t.signature();
 		Transaction {
 			hash: t.hash().into(),
@@ -170,7 +170,7 @@ impl From<LocalizedTransaction> for Transaction {
 			block_hash: Some(t.block_hash.clone().into()),
 			block_number: Some(t.block_number.into()),
 			transaction_index: Some(t.transaction_index.into()),
-			from: t.sender().unwrap().into(),
+			from: t.sender().into(),
 			to: match t.action {
 				Action::Create => None,
 				Action::Call(ref address) => Some(address.clone().into())
@@ -180,11 +180,11 @@ impl From<LocalizedTransaction> for Transaction {
 			gas: t.gas.into(),
 			input: Bytes::new(t.data.clone()),
 			creates: match t.action {
-				Action::Create => Some(contract_address(&t.sender().unwrap(), &t.nonce).into()),
+				Action::Create => Some(contract_address(&t.sender(), &t.nonce).into()),
 				Action::Call(_) => None,
 			},
 			raw: ::rlp::encode(&t.signed).to_vec().into(),
-			public_key: t.public_key().ok().map(Into::into),
+			public_key: t.recover_public().ok().map(Into::into),
 			network_id: t.network_id(),
 			standard_v: t.standard_v().into(),
 			v: t.original_v().into(),
@@ -204,7 +204,7 @@ impl From<SignedTransaction> for Transaction {
 			block_hash: None,
 			block_number: None,
 			transaction_index: None,
-			from: t.sender().unwrap().into(),
+			from: t.sender().into(),
 			to: match t.action {
 				Action::Create => None,
 				Action::Call(ref address) => Some(address.clone().into())
@@ -214,11 +214,11 @@ impl From<SignedTransaction> for Transaction {
 			gas: t.gas.into(),
 			input: Bytes::new(t.data.clone()),
 			creates: match t.action {
-				Action::Create => Some(contract_address(&t.sender().unwrap(), &t.nonce).into()),
+				Action::Create => Some(contract_address(&t.sender(), &t.nonce).into()),
 				Action::Call(_) => None,
 			},
 			raw: ::rlp::encode(&t).to_vec().into(),
-			public_key: t.public_key().ok().map(Into::into),
+			public_key: Some(t.public_key().into()),
 			network_id: t.network_id(),
 			standard_v: t.standard_v().into(),
 			v: t.original_v().into(),

sync/src/api.rs

@@ -364,7 +364,7 @@ impl ChainNotify for EthSync {
 struct TxRelay(Arc<BlockChainClient>);
 
 impl LightHandler for TxRelay {
-	fn on_transactions(&self, ctx: &EventContext, relay: &[::ethcore::transaction::SignedTransaction]) {
+	fn on_transactions(&self, ctx: &EventContext, relay: &[::ethcore::transaction::UnverifiedTransaction]) {
 		trace!(target: "les", "Relaying {} transactions from peer {}", relay.len(), ctx.peer());
 		self.0.queue_transactions(relay.iter().map(|tx| ::rlp::encode(tx).to_vec()).collect(), ctx.peer())
 	}

sync/src/chain.rs

@@ -2124,7 +2124,7 @@ mod tests {
 	use std::collections::{HashSet, VecDeque};
 	use tests::helpers::*;
 	use tests::snapshot::TestSnapshotService;
-	use util::{U256, RwLock};
+	use util::{U256, Address, RwLock};
 	use util::sha3::Hashable;
 	use util::hash::{H256, FixedHash};
 	use util::bytes::Bytes;
@@ -2132,8 +2132,10 @@ mod tests {
 	use super::*;
 	use ::SyncConfig;
 	use super::{PeerInfo, PeerAsking};
+	use ethkey;
 	use ethcore::header::*;
 	use ethcore::client::*;
+	use ethcore::transaction::UnverifiedTransaction;
 	use ethcore::miner::MinerService;
 
 	fn get_dummy_block(order: u32, parent_hash: H256) -> Bytes {
@@ -2762,6 +2764,10 @@ mod tests {
 
 	#[test]
 	fn should_add_transactions_to_queue() {
+		fn sender(tx: &UnverifiedTransaction) -> Address {
+			ethkey::public_to_address(&tx.recover_public().unwrap())
+		}
+
 		// given
 		let mut client = TestBlockChainClient::new();
 		client.add_blocks(98, EachBlockWith::Uncle);
@@ -2775,8 +2781,9 @@ mod tests {
 		// Add some balance to clients and reset nonces
 		for h in &[good_blocks[0], retracted_blocks[0]] {
 			let block = client.block(BlockId::Hash(*h)).unwrap();
-			client.set_balance(block.transactions()[0].sender().unwrap(), U256::from(1_000_000_000));
+			let sender = sender(&block.transactions()[0]);;
-			client.set_nonce(block.transactions()[0].sender().unwrap(), U256::from(0));
+			client.set_balance(sender, U256::from(1_000_000_000));
+			client.set_nonce(sender, U256::from(0));
 		}
 
 
@@ -2793,7 +2800,7 @@ mod tests {
 		// We need to update nonce status (because we say that the block has been imported)
 		for h in &[good_blocks[0]] {
 			let block = client.block(BlockId::Hash(*h)).unwrap();
-			client.set_nonce(block.transactions()[0].sender().unwrap(), U256::from(1));
+			client.set_nonce(sender(&block.transactions()[0]), U256::from(1));
 		}
 		{
 			let queue = RwLock::new(VecDeque::new());