Integrate state diffing into the ethcore JSONRPC #1206
Conversation
| @@ -277,13 +276,19 @@ impl MinerService for Miner { | |||
| // give the sender max balance | |||
| state.sub_balance(&sender, &balance); | |||
There was a problem hiding this comment.
Seems it needs a similar workaround as the code in client.
Isn't it possible to reuse the code here and in client?
There was a problem hiding this comment.
not sure what you're referring to here. this code hasn't changed in the PR.
There was a problem hiding this comment.
But similar code in client did. So if the code was invalid (possible overflow?) then I guess this here is too (only difference being pending vs latest/blocknumber)
There was a problem hiding this comment.
right. in that case it should really be filed as a new PR (my fault for including a half a fix, but that ship has sailed now).
i'll put in a fix for miner.rs now, but i think properly refactoring will have to wait for a future PR.
tomusdrw
added
A5-grumble 🔥
gavofyork
added
A0-pleasereview 🤓
rphmeier
added
A8-looksgood 🦄
| use ipc::binary::BinaryConvertError; | ||
| use std::fmt; | ||
| use std::mem; | ||
| use std::collections::VecDeque; | ||
|
|
||
| /// Transaction execution receipt. | ||
| #[derive(Debug, PartialEq, Clone, Binary)] | ||
| #[derive(Debug, PartialEq, Clone)] |
There was a problem hiding this comment.
Executed is in types directory, so I assume that it should implement Binary. It's probably not an issue right now, cause no tests are failing, but it may interfere with Nikolay's pull requests and might be difficult to debug, because compiler plugins errors are hardly readable.
imo, we should keep derive from Binary and also implement it for StateDiff
There was a problem hiding this comment.
this was discussed with @NikVolf and decided that although it will eventually need to be binary, the infrastructure is not yet in place to easily allow that and so it will remain for another PR.
debris
added
A5-grumble 🔥
gavofyork
added
A0-pleasereview 🤓
Aside from the JSONRPC formatting, there is some repotting needed to being the PoD Account/State out of a tests-only config.
There's also a fix for
eth_call. Mea culpa.