From 354ac7d6e596ae5c5752d3863884a43ecb625a84 Mon Sep 17 00:00:00 2001 From: Nikolay Volf Date: Sat, 14 May 2016 15:28:44 +0300 Subject: [PATCH] Limiting result of the execution to execution-specific errors (#1071) * execution error/result limiting * missing trailing comma * fix executive tests * adding original error as string to the generic transaction error * 'mallformed'-s all around --- ethcore/src/client/client.rs | 7 +++++-- ethcore/src/client/mod.rs | 4 ++-- ethcore/src/client/test_client.rs | 4 ++-- ethcore/src/error.rs | 4 +++- ethcore/src/executive.rs | 19 +++++++++++-------- miner/src/lib.rs | 4 ++-- miner/src/miner.rs | 7 +++++-- rpc/src/v1/tests/helpers/miner_service.rs | 4 ++-- sync/src/chain.rs | 4 ++-- util/src/keys/directory.rs | 2 +- 10 files changed, 35 insertions(+), 24 deletions(-) diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index 23b81b34e..e385f39a8 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -424,7 +424,7 @@ impl Client where V: Verifier { } impl BlockChainClient for Client where V: Verifier { - fn call(&self, t: &SignedTransaction) -> Result { + fn call(&self, t: &SignedTransaction) -> Result { let header = self.block_header(BlockId::Latest).unwrap(); let view = HeaderView::new(&header); let last_hashes = self.build_last_hashes(view.hash()); @@ -439,7 +439,10 @@ impl BlockChainClient for Client where V: Verifier { }; // that's just a copy of the state. let mut state = self.state(); - let sender = try!(t.sender()); + let sender = try!(t.sender().map_err(|e| { + let message = format!("Transaction malformed: {:?}", e); + ExecutionError::TransactionMalformed(message) + })); let balance = state.balance(&sender); // give the sender max balance state.sub_balance(&sender, &balance); diff --git a/ethcore/src/client/mod.rs b/ethcore/src/client/mod.rs index 90dd78015..9dddeceb7 100644 --- a/ethcore/src/client/mod.rs +++ b/ethcore/src/client/mod.rs @@ -42,7 +42,7 @@ use header::{BlockNumber, Header}; use transaction::{LocalizedTransaction, SignedTransaction}; use log_entry::LocalizedLogEntry; use filter::Filter; -use error::{ImportResult, Error}; +use error::{ImportResult, ExecutionError}; use receipt::LocalizedReceipt; use engine::{Engine}; use trace::LocalizedTrace; @@ -133,7 +133,7 @@ pub trait BlockChainClient : Sync + Send { fn try_seal(&self, block: LockedBlock, seal: Vec) -> Result; /// Makes a non-persistent transaction call. - fn call(&self, t: &SignedTransaction) -> Result; + fn call(&self, t: &SignedTransaction) -> Result; /// Attempt to seal the block internally. See `Engine`. fn generate_seal(&self, block: &ExecutedBlock, accounts: Option<&AccountProvider>) -> Option> { self.engine().generate_seal(block, accounts) } diff --git a/ethcore/src/client/test_client.rs b/ethcore/src/client/test_client.rs index c3df57b76..4ec993fe5 100644 --- a/ethcore/src/client/test_client.rs +++ b/ethcore/src/client/test_client.rs @@ -31,7 +31,7 @@ use error::{ImportResult}; use block_queue::BlockQueueInfo; use block::{SealedBlock, ClosedBlock, LockedBlock}; use executive::Executed; -use error::Error; +use error::{ExecutionError}; use engine::Engine; use trace::LocalizedTrace; @@ -221,7 +221,7 @@ impl TestBlockChainClient { } impl BlockChainClient for TestBlockChainClient { - fn call(&self, _t: &SignedTransaction) -> Result { + fn call(&self, _t: &SignedTransaction) -> Result { Ok(self.execution_result.read().unwrap().clone().unwrap()) } diff --git a/ethcore/src/error.rs b/ethcore/src/error.rs index 922d72700..2e007703b 100644 --- a/ethcore/src/error.rs +++ b/ethcore/src/error.rs @@ -59,7 +59,9 @@ pub enum ExecutionError { got: U512 }, /// Returned when internal evm error occurs. - Internal + Internal, + /// Returned when generic transaction occurs + TransactionMalformed(String), } #[derive(Debug, PartialEq)] diff --git a/ethcore/src/executive.rs b/ethcore/src/executive.rs index 4247114ef..179195078 100644 --- a/ethcore/src/executive.rs +++ b/ethcore/src/executive.rs @@ -119,7 +119,7 @@ impl<'a> Executive<'a> { } /// This function should be used to execute transaction. - pub fn transact(&'a mut self, t: &SignedTransaction, options: TransactOptions) -> Result { + pub fn transact(&'a mut self, t: &SignedTransaction, options: TransactOptions) -> Result { let check = options.check_nonce; match options.tracing { true => self.transact_with_tracer(t, check, ExecutiveTracer::default()), @@ -128,8 +128,11 @@ impl<'a> Executive<'a> { } /// Execute transaction/call with tracing enabled - pub fn transact_with_tracer(&'a mut self, t: &SignedTransaction, check_nonce: bool, mut tracer: T) -> Result where T: Tracer { - let sender = try!(t.sender()); + pub fn transact_with_tracer(&'a mut self, t: &SignedTransaction, check_nonce: bool, mut tracer: T) -> Result where T: Tracer { + let sender = try!(t.sender().map_err(|e| { + let message = format!("Transaction malformed: {:?}", e); + ExecutionError::TransactionMalformed(message) + })); let nonce = self.state.nonce(&sender); let schedule = self.engine.schedule(self.info); @@ -983,8 +986,8 @@ mod tests { }; match res { - Err(Error::Util(UtilError::Crypto(CryptoError::InvalidSignature))) => (), - _ => assert!(false, "Expected invalid signature error.") + Err(ExecutionError::TransactionMalformed(_)) => (), + _ => assert!(false, "Expected an invalid transaction error.") } } @@ -1015,7 +1018,7 @@ mod tests { }; match res { - Err(Error::Execution(ExecutionError::InvalidNonce { expected, got })) + Err(ExecutionError::InvalidNonce { expected, got }) if expected == U256::zero() && got == U256::one() => (), _ => assert!(false, "Expected invalid nonce error.") } @@ -1049,7 +1052,7 @@ mod tests { }; match res { - Err(Error::Execution(ExecutionError::BlockGasLimitReached { gas_limit, gas_used, gas })) + Err(ExecutionError::BlockGasLimitReached { gas_limit, gas_used, gas }) if gas_limit == U256::from(100_000) && gas_used == U256::from(20_000) && gas == U256::from(80_001) => (), _ => assert!(false, "Expected block gas limit error.") } @@ -1083,7 +1086,7 @@ mod tests { }; match res { - Err(Error::Execution(ExecutionError::NotEnoughCash { required , got })) + Err(ExecutionError::NotEnoughCash { required , got }) if required == U512::from(100_018) && got == U512::from(100_017) => (), _ => assert!(false, "Expected not enough cash error. {:?}", res) } diff --git a/miner/src/lib.rs b/miner/src/lib.rs index bd2904fc2..f92e0de52 100644 --- a/miner/src/lib.rs +++ b/miner/src/lib.rs @@ -63,7 +63,7 @@ pub use external::{ExternalMiner, ExternalMinerService}; use util::{H256, U256, Address, Bytes}; use ethcore::client::{BlockChainClient, Executed}; use ethcore::block::{ClosedBlock}; -use ethcore::error::{Error}; +use ethcore::error::{Error, ExecutionError}; use ethcore::transaction::SignedTransaction; /// Miner client API @@ -150,7 +150,7 @@ pub trait MinerService : Send + Sync { fn balance(&self, chain: &BlockChainClient, address: &Address) -> U256; /// Call into contract code using pending state. - fn call(&self, chain: &BlockChainClient, t: &SignedTransaction) -> Result; + fn call(&self, chain: &BlockChainClient, t: &SignedTransaction) -> Result; /// Get storage value in pending state. fn storage_at(&self, chain: &BlockChainClient, address: &Address, position: &H256) -> H256; diff --git a/miner/src/miner.rs b/miner/src/miner.rs index 78ff824f6..33d21613f 100644 --- a/miner/src/miner.rs +++ b/miner/src/miner.rs @@ -240,7 +240,7 @@ impl MinerService for Miner { } } - fn call(&self, chain: &BlockChainClient, t: &SignedTransaction) -> Result { + fn call(&self, chain: &BlockChainClient, t: &SignedTransaction) -> Result { let sealing_work = self.sealing_work.lock().unwrap(); match sealing_work.peek_last_ref() { Some(work) => { @@ -258,7 +258,10 @@ impl MinerService for Miner { }; // that's just a copy of the state. let mut state = block.state().clone(); - let sender = try!(t.sender()); + let sender = try!(t.sender().map_err(|e| { + let message = format!("Transaction malformed: {:?}", e); + ExecutionError::TransactionMalformed(message) + })); let balance = state.balance(&sender); // give the sender max balance state.sub_balance(&sender, &balance); diff --git a/rpc/src/v1/tests/helpers/miner_service.rs b/rpc/src/v1/tests/helpers/miner_service.rs index cd4157fa9..1fbe15ca4 100644 --- a/rpc/src/v1/tests/helpers/miner_service.rs +++ b/rpc/src/v1/tests/helpers/miner_service.rs @@ -18,7 +18,7 @@ use util::{Address, H256, Bytes, U256, FixedHash, Uint}; use util::standard::*; -use ethcore::error::Error; +use ethcore::error::{Error, ExecutionError}; use ethcore::client::{BlockChainClient, Executed}; use ethcore::block::{ClosedBlock, IsBlock}; use ethcore::transaction::SignedTransaction; @@ -179,7 +179,7 @@ impl MinerService for TestMinerService { self.latest_closed_block.lock().unwrap().as_ref().map_or_else(U256::zero, |b| b.block().fields().state.balance(address).clone()) } - fn call(&self, _chain: &BlockChainClient, _t: &SignedTransaction) -> Result { + fn call(&self, _chain: &BlockChainClient, _t: &SignedTransaction) -> Result { unimplemented!(); } diff --git a/sync/src/chain.rs b/sync/src/chain.rs index 14809fdc4..bd94fb9be 100644 --- a/sync/src/chain.rs +++ b/sync/src/chain.rs @@ -1193,7 +1193,7 @@ impl ChainSync { let mut rlp_stream = RlpStream::new_list(route.blocks.len()); for block_hash in route.blocks { let mut hash_rlp = RlpStream::new_list(2); - let difficulty = chain.block_total_difficulty(BlockId::Hash(block_hash.clone())).expect("Mallformed block without a difficulty on the chain!"); + let difficulty = chain.block_total_difficulty(BlockId::Hash(block_hash.clone())).expect("Malformed block without a difficulty on the chain!"); hash_rlp.append(&block_hash); hash_rlp.append(&difficulty); rlp_stream.append_raw(&hash_rlp.out(), 1); @@ -1570,7 +1570,7 @@ mod tests { } #[test] - fn handles_peer_new_block_mallformed() { + fn handles_peer_new_block_malformed() { let mut client = TestBlockChainClient::new(); client.add_blocks(10, EachBlockWith::Uncle); diff --git a/util/src/keys/directory.rs b/util/src/keys/directory.rs index d9268a95f..3f4100163 100644 --- a/util/src/keys/directory.rs +++ b/util/src/keys/directory.rs @@ -381,7 +381,7 @@ impl KeyFileContent { } } - /// Loads key from valid json, returns error and records warning if key is mallformed + /// Loads key from valid json, returns error and records warning if key is malformed pub fn load(json: &Json) -> Result { match Self::from_json(json) { Ok(key_file) => Ok(key_file),