openethereum / sol-rs

Solaris - Solidity testing framework in Rust.
GNU General Public License v3.0
54 stars 14 forks source link

Merging parables and sol-rs #40

Open udoprog opened 5 years ago

udoprog commented 5 years ago

Parables is a project started to attempt and improve the experience in writing soldity tests in Rust against parity. In order to accomplish what we wanted, we ended up having to deviate quite a bit from how sol-rs did things.

Among these deviations, we've forked ethabi derive and made it part of the project so that we can quickly experiment on how to get an easier to use contract interface. We've also adapted the internals of EvmTestClient into Evm to among other things make it easier to clone and use through references. Effectively sharing a mutable reference of Evm and passing it across threads was one of the pain points when we started expanding our test base (note that most methods of parables' Evm takes &self). We've also expanded the API a fair bit and tweaked it to make among other things make it easier to assert for failures while still taking things like gas costs into account.

A number of issues listed in this tracker are now supported in parables, like:

Internally we've ported a fair amount of tests to parables, and have found it quite nice to work with. Dumping expression state is especially helpful to determine why a given expression fails. And having the ability to pin expected failures to a function and expression makes tests much less fragile.

So I'd like to start a dialogue here, since working together is generally more beneficial. Would you like to merge these projects? And if so, how should we do it?

dvdplm commented 5 years ago

Which version of ethabi-derive did you fork? The latest 6.0 introduced quite a few changes. Any chance we can make it work for you guys? (/cc @debris) I know there's been work on our end to improve state dumping, but not sure if that's similar to what you've built? (/cc @cheme).

/cc @folsen

udoprog commented 5 years ago

I'll try to outline the major differences compared to what was the current state of ethabi.

Here's the complete ABI we use in parables: https://github.com/PrimaBlock/parables/blob/master/testing/abi.rs We've coupled it with Evm to make it easier to use and the Constructor trait ships a fair amount of context (source maps, paths, ...) that are later used when tracing.

The biggest difference when interfacing with contracts was that we've (re-)added a Vm abstraction that you can use to bind contract interfaces like this: https://github.com/PrimaBlock/parables/blob/master/example/src/main.rs#L40

There we associate a reference to a vm, an address, and a default set of call parameters (sender, gas_price, ...) so they don't have to be specified when calling functions on a contract.

The Call<T> return type is also important, since we distinguish between internal errors, and errors during contract execution. Internal errors are communicated through Result<Call<T>, Error>, while contract errors are captured in Call<T> so we can access stuff like the amount of gas consumed, or if an error happened and where. This type is part of the ABI since it's propagated through the simplified contract abstraction.

cheme commented 5 years ago

I just discovered both project (solaris and parables), and I think you are doing a nice job, for instance having book is quite nice.

I like the fact that parables plugs directly on solc (and not solcjs), so no need for js to run it.

I do not really know how 'merging' parables and solaris could be done (at the time I think there is not a lot of activity on solaris side and both project being gpl everything can be reused). Maybe it could be a good thing for Parables if we point out to your project from our README as an active project alternative, but it is really not my place to say so.

Talking about things I know, EVMTestClient may be a good target to share some functionalities. I understand your primary concern for not using it is being able to use EVMTestClient with threads.   We can probably manage to run both

pub struct EvmTestClient<'a> {
    pub state: state::State<state_db::StateDB>,
    pub spec: spec::Spec,
  pub _ph: PhantomData<&'a()>,
}

which we can put in an 'Arc<Mutex<_>>', and

pub struct EvmTestClient<'a> {
    state: state::State<state_db::StateDB>,
    spec: &'a spec::Spec,
}

by using a feature or whatever solution, and then get everyone behind EvmTestClient.

I am really not sure it is worth it (at the time EvmTestClient is not a lot of code but could later benefit from new functionalities).

For information currently EvmTestClient is the main entry point for :

there may be other use that I do not know/remember /cc @sorpaas . Also as dvdplm already say you could probably use state functions from this pr when/if it get merged.

folsen commented 5 years ago

@udoprog sol-rs is a project started out of our own pain and it's a project we love but unfortunately don't have enough resources to properly develop and maintain, so it's been stale for a while (as you've noticed).

Would you like to merge these projects?

I think that'd be awesome.

And if so, how should we do it?

I'm not sure, we have a new version of ethabi as previously mentioned and that's used for a lot of other things as well not just solaris so duplicating that and having two different versions out in the wild seems suboptimal. I'll leave it up to others (you guys, @snd, @tomusdrw, @maciejhirsz et al) to decide.

There's also a question of where it would live, do you guys want to merge your changes upstream and have it live under the parity org or do you want to keep it in your own org?