Skip to content
This repository has been archived by the owner on May 4, 2023. It is now read-only.

Merging parables and sol-rs #40

Open
udoprog opened this issue Oct 9, 2018 · 4 comments
Open

Merging parables and sol-rs #40

udoprog opened this issue Oct 9, 2018 · 4 comments

Comments

@udoprog
Copy link

udoprog commented Oct 9, 2018

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
Copy link

dvdplm commented Oct 10, 2018

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
Copy link
Author

udoprog commented Oct 10, 2018

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
Copy link

cheme commented Oct 10, 2018

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 :

  • evmbin which is used by hive for instance
  • json-tests running from parity. Those are evm tests but they are worth it. Currently the bytecode for those test is produce by 'lll'.
  • a private evm code fuzzer

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
Copy link

folsen commented Oct 11, 2018

@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?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants
@folsen @dvdplm @udoprog @cheme and others