Limiting result of the execution to execution-specific errors #1071
Conversation
| let sender = try!(t.sender()); | ||
| let sender = try!(t.sender().map_err(|e| { | ||
| warn!(target: "evm", "Transaction mallformed: {:?}", e); | ||
| ExecutionError::TransactionMallformed |
There was a problem hiding this comment.
You don't want to wrap the original error inside TransactionMalformed? Maybe at least a string representation of the error to have some more details?
There was a problem hiding this comment.
Yeah just thought about this as well, thanx
There was a problem hiding this comment.
And by implementing From trait you should be able to get rid of map_err functions (unless we want to print warning here - altough it could also be a side effect in From::from implementation (not sure if it's a good practice though))
There was a problem hiding this comment.
Well perhaps from "pattern" should not be used here, because conversion of generic error into concrete error of the function is not one-to-one in general (unlike when you transform in the opposite direction), this is just one case here
| @@ -439,7 +439,10 @@ impl<V> BlockChainClient for Client<V> 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 mallformed: {:?}", e); | |||
There was a problem hiding this comment.
spelling, "Malformed transaction"
arkpar
added
A5-grumble 🔥
|
There were previous "mallformed" all around the source ) |
NikVolf
added
A8-looksgood 🦄
| @@ -59,7 +59,9 @@ pub enum ExecutionError { | |||
| got: U512 | |||
| }, | |||
| /// Returned when internal evm error occurs. | |||
| Internal | |||
| Internal, | |||
| /// Returned when generic transaction occurs | |||
There was a problem hiding this comment.
generic transaction error occurs.
gavofyork
added
A8-looksgood 🦄
Before this refactoring api calling parity should have an idea, what kind of errors it should or should not handle, because
fn call/fn transactreturned the most broad error type combined from erros of several crates (including some external ones)After refactoring it should return error type designed specifically to the execution logic (or initial condition), and calling party has a complete number of errors it can possibly encounter in the single enum
ExecitionResult