Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Basic support for ewasm precompiles #431

Closed
wants to merge 2 commits into from
Closed

Basic support for ewasm precompiles #431

wants to merge 2 commits into from

Conversation

s1na
Copy link
Contributor

@s1na s1na commented Jan 30, 2019

This is the first step for integrating ewasm into ethereumjs-vm. It adds very basic support for using ewasm-precompiles instead of those written natively in JS. The interface only implements methods which the precompiles use (all of them are sync). The code is heavily inspired by ewasm-kernel :)

It passes state and blockchain tests (commands npm run testStateEwasm and npm run testBlockchainEwasm), except for cases which use ripemd160 (that's why I've commented that out for now). The ripemd160 precompile written in rust doesn't pad the output to 32 bytes, and I wasn't sure where whether padding was intentionally left to be done in hosts.

Another point is the wasm binaries. I committed the binaries as well, but I'm not sure if this is the best way to distribute them, and whether using fs.readFileSync is a good idea (specially for browser support).

To enable these changes, the option enableEwasm needs to be set to true when instantiating VM.

@s1na s1na force-pushed the ewasm-precompiles branch from b820cc4 to 43013a2 Compare January 30, 2019 15:42
@coveralls

This comment has been minimized.

@axic
Copy link
Member

axic commented Jan 30, 2019

@s1na wow! Will review shortly.

@axic
Copy link
Member

axic commented Jan 30, 2019

The ripemd160 precompile written in rust doesn't pad the output to 32 bytes, and I wasn't sure where whether padding was intentionally left to be done in hosts.

Behaviour should match. Do you want to submit a PR to fix that?

lib/index.js Outdated Show resolved Hide resolved
lib/ewasm/env.js Show resolved Hide resolved
lib/ewasm/contract.js Outdated Show resolved Hide resolved
lib/ewasm/env.js Show resolved Hide resolved
.circleci/config.yml Outdated Show resolved Hide resolved
lib/ewasm/index.js Outdated Show resolved Hide resolved
@s1na s1na force-pushed the ewasm-precompiles branch from 4bfa311 to c82e211 Compare January 31, 2019 14:05
@s1na s1na force-pushed the ewasm-precompiles branch from f16b3c1 to c0a768d Compare February 1, 2019 11:25
@s1na
Copy link
Contributor Author

s1na commented Feb 1, 2019

A few small updates:

  • finish now throws FinishExecution which Contract.run catches
  • Coverage now additionally runs state tests with ewasm precompiles enabled
  • Fixed indentation of transform-i64.wast
  • Added some jsdoc comments to Contract and Env

P.S. Sorry that I keep rebasing. If that makes reviewing more difficult I can keep full commit history, and rebase once at the end.

@axic
Copy link
Member

axic commented Feb 1, 2019

I think this looks great, but I am not at all sure merging in those big binaries. They supposed to be way smaller once we fix an optimisation step. Ideally compilation of it would happen at build time, but unfortunately that would put a requirement on rust being available.

Perhaps the best way is to release the binaries on the ewasm-precompiles repo and the build script here would download them with curl.

@s1na what do you think?

@s1na
Copy link
Contributor Author

s1na commented Feb 1, 2019

If I'm not mistaken, github allows distributing files for release (if having them in git is not desirable). It definitely wouldn't hurt to have the binaries downloadable from ewasm-precompiles.

I'd say if the precompiles change very frequently, fetching them via curl in the build scripts is much better, otherwise there are advantages and disadvantages to both approaches.

If the optimization is close, we could also wait and check in the smaller binaries.

@axic
Copy link
Member

axic commented Feb 1, 2019

If I'm not mistaken, github allows distributing files for release (if having them in git is not desirable).

Yes, to clarify that is what I meant. Tagging the repo and making a proper release on github - there one can upload files.

@s1na
Copy link
Contributor Author

s1na commented Feb 19, 2019

Tagging the repo and making a proper release on github

Is there anything I can help with for that?

I was wondering if publishing them as an npm package would be better in any way? Or alternatively, could the rust compiler produce wast files in addition to wasm binaries?

@axic
Copy link
Member

axic commented Feb 19, 2019

Is there anything I can help with for that?

I was waiting for the modexp precompile to be merged to make a "stable" release.

@holgerd77
Copy link
Member

Do I get this correctly that this didn't stand the test of time and we therefore might want to close here? 🤔 Or are there remaining applications and/or use cases that I am missing?

@ryanio ryanio marked this pull request as draft October 10, 2021 09:01
@holgerd77 holgerd77 closed this Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants