Separate RPC serialization from implementation

[rphmeier]

This PR only does the Eth and EthFilter APIs but it was getting a bit big.

The jsonrpc_core update is for https://github.com/ethcore/jsonrpc-core/pull/27

[coveralls]

Coverage decreased (-0.02%) to 84.694% when pulling fc7098d on rpc-separation into bc9b7cb on master.

[tomusdrw]

Shouldn't we have a non-serialization related Error also?

[rphmeier]

What would it consist of? The errors that can occur still need to be serialized and won't be used anywhere else.

[coveralls]

Coverage decreased (-0.009%) to 84.936% when pulling 4423efd on rpc-separation into 2ba4968 on master.

[arkpar]

@debris please review this

[coveralls]

Coverage decreased (-0.02%) to 85.269% when pulling 73f813c on rpc-separation into 48be609 on master.

[gavofyork]

this seems to increase the amount of effort to expose functionality by another function implementation per RPC. thus increase dev effort and maintenance burden.

i would prefer to reduce the effort, not increase it.

[tomusdrw]

But it keeps parameter deserialization in a single place in case of multiple implementations (which I suppose will be a case with light client) - keeping two APIs in sync in terms of expected parameters is easier.

What about a default implementation for rpc_* methods on the same trait?:

trait Eth {
 fn syncing(&self) -> Result<bool, Error>;
 fn rpc_syncing(&self, params: Params) -> Result<Value, Error> {
  try!(expect_no_params(params));
  self.syncing().map(to_value)
 }
}

[gavofyork]

tbh introducing a pattern that requires a new function foo to split some of its functionality into x_foo and call into that manually is pretty evil.

it's additional clutter which makes adding, renaming and removing functions more tedious than it really should be.

i'm just wondering whether it is something that can be fixed properly in due course (in which case i'd favour the q'n'd solution of just duplicating logic in order to minimise effort when adding functions and keep us reminded that it really does need addressing) or whether this is really the best we can reasonably do (in which case merging is reasonable).

[rphmeier]

Played around with macro and type-based codegen for parameter and output deserialization -- the results are here: ethcore/parity@fd711c6

Only a single trait with strongly-typed methods is required for an RPC-API (with the caveat that all parameters must be deserializable except for optional BlockParam which must be at the end) and are added to a delegate with

delegate.add_method("foo_Bar", move |base, params| foo_bar.wrap_rpc(base, params))

which is the only added complexity. When impl Trait is stabilized even that can be elided.

@gavofyork think that this solution would be acceptable?

[gavofyork]

oh yeah! :)

[rphmeier]

Latest take on it is pushed -- takes a slightly different approach than described in the comment above because I was running into some issues with coercing anonymous function types into concrete function pointers without specifying the args explicitly.

RPC APIs are to be wrapped in the build_rpc_trait! macro, where you can just write out the definition like before.

Trailing parameters are managed by the Trailing<T> struct, where T: Deserializable. This is a general mechanism because we use it not only for the trailing block parameter but also the eth_getWork timeout.

Not supported yet:

• Async methods
• Adding other attributes to RPC functions
• named parameters

[gavofyork]

lgtm

[coveralls]

Coverage decreased (-0.007%) to 85.225% when pulling fa205c1 on rpc-separation into aae6d19 on master.