new feature flag: `deterministic` #709

YaronWittenstein · 2019-08-22T10:32:33Z

The motivation for the PR is for introducing a new feature flag called deterministic.

When deterministic will be enabled (turned-off by default) it'll guarantee deterministic
execution of wasm programs across different hardware/circumstances.

This is critical for Blockchain projects that require execution to be deterministic
in order to reach a consensus of the state transition of each smart-contract transaction.

ethanfrey · 2019-09-19T09:14:27Z

Interesting idea. I am also looking for deterministic execution in for a blockchain project.
I had assumed that simply disabling floating point in the wasm will get you deterministic execution (CosmWasm/cosmos-sdk#6)

I'd be happy for you to explain to me what other sources you found, and how this disables them

MarkMcCaskey · 2019-09-20T19:08:03Z

@ethanfrey in the future, threads, too. I think we'd want to use this flag to be extra strict during the Wasm evaluation step (I don't have context on this PR, just commenting generally)

YaronWittenstein · 2019-10-06T17:31:53Z

@ethanfrey Hi!

You're right. The main motivation is right now disabling floats.
But in the future, it might be more features (as @MarkMcCaskey said)

I'll update the PR soon since the code isn't synced with master

ethanfrey · 2019-10-07T08:18:44Z

Great, I thought this was possible already, but if not, definitely an essential feature.

…it'll guarantee deterministic execution of wasm programs across different hardware/circumstances. This is very useful for Blockchain projects having wasm smart-contracts This is critical for Blockchain projects that require execution to be deterministic in order to reach a consensus of the state transition of each smart-contract transaction.

MarkMcCaskey

We should test this flag with cargo check --features "deterministic" in CI at the very least. It'd also be nice to have test cases of Wasm that fails when it's enabled and an example of Wasm that works. We can add the tests as a follow-up PR (except for the check) if you want to get this shipped quickly though.

MarkMcCaskey · 2019-10-07T19:01:13Z

lib/runtime/Cargo.toml

@@ -41,6 +41,7 @@ singlepass = ["wasmer-singlepass-backend"]
 default-backend-singlepass = ["singlepass"]
 default-backend-llvm = ["llvm"]
 default-backend-cranelift = ["cranelift"]
+deterministic = ["wasmer-singlepass-backend/deterministic"]


This should probably trigger wasmer-runtime-core/deterministic, too

I thought it's redundant because:
https://github.com/wasmerio/wasmer/blob/master/lib/singlepass-backend/Cargo.toml#L12

It's redundant in that all interactions with wasmer-runtime-core through singlepass will be deterministic as defined by that flag, but I believe Rust will compile a separate version of the crate with the other feature flags that are directly accessible from wasmer-runtime

MarkMcCaskey · 2019-10-07T19:02:38Z

Cargo.toml

@@ -26,7 +26,7 @@ wabt = "0.9.1"
 wasmer-clif-backend = { path = "lib/clif-backend" }
 wasmer-singlepass-backend = { path = "lib/singlepass-backend", optional = true }
 wasmer-middleware-common = { path = "lib/middleware-common" }
-wasmer-runtime = { path = "lib/runtime" }
+wasmer-runtime = { path = "lib/runtime", default-features = false }


Hmm, I don't know if we want to change this

otherwise, you force the installation of cranelift and it seems to break to compilation

Yeah, you can only enable one default backend.
And you cannot disable default features from the deterministic feature.

I dealt with this for my metering pr #736 and ended up just documenting that you had to explicitly set the backend with metering on...

cargo test --features metering,backend-singlepass --no-default-features

it is a bit heavier to use it, but still configurable and doesn't break anyone else's compile steps. Then again, this is trying to disable default features of a dependency... hmm...

See https://github.com/wasmerio/wasmer/pull/736/files#diff-0f742359c7cbd0059a44bc3d831b548c

very nice @ethanfrey, thank you!

@ethanfrey
I've reverted that and tried:

cargo +nightly build --features=deterministic --no-default-features

and for some reason, it installs cranelift...

Yeah, what I was thinking was that if a user wanted to do this, then they could pass default-features = false in their Cargo.toml of this.

@MarkMcCaskey it means that a user will have to use a forked version then?

A forked version of what exactly?

It's my understanding that this is a fully configurable way to do this. In Rust this is done in cargo.toml. In other things like the go-ext-wasm example, it uses an empty wrapper crate to pass configuration options into the Rust for the dynamic library that it creates.

Hywan

Could we rename the feature deterministic-execution please? I feel like the subject is missing. We should also add a comment in the lowest-denominator Cargo.toml file to explain what this feature is about. Thoughts?

Hywan · 2019-10-07T19:22:26Z

Cargo.toml

@@ -101,6 +101,8 @@ backend-singlepass = [
 wasi = ["wasmer-wasi"]
 managed = ["backend-singlepass", "wasmer-runtime-core/managed"]



Remove the empty line between the features please.

@Hywan I wanted to stay with the feature flag name of wasmparser
(also a shorter name)

but it's your call of course

@MarkMcCaskey @Hywan
please LMK what's your decision

@MarkMcCaskey @Hywan
I've renamed the feature flag to deterministic-execution

ethanfrey · 2019-10-10T17:38:47Z

This also seems to just be for single pass.
Is there a way to return an error on detecting floating point (disable it) for other compilers?

Failing that maybe some other code that can parse wasm and detect if there are any floating point ops, as a pre processor? Hmmm... I guess this coild be a Middleware, right?

YaronWittenstein · 2019-10-10T17:48:58Z

This also seems to just be for single pass.
Is there a way to return an error on detecting floating point (disable it) for other compilers?

Failing that maybe some other code that can parse wasm and detect if there are any floating point ops, as a pre processor? Hmmm... I guess this coild be a Middleware, right?

I thought that smart contracts will use only the single-pass complier

MarkMcCaskey · 2019-10-10T18:35:58Z

@ethanfrey I believe the feature passed to wasmparser does this, though I have not verified this.

It's probably good for this feature to apply to all backends. @YaronWittenstein is correct that most users in blockchain or smart contract settings want to avoid super-linear compilation time on malicious input ("JIT-bombs"). That said, it would be nice to avoid fragmentation on the backends and I can imagine that this flag might be more generally useful.

ethanfrey · 2019-10-11T10:08:56Z

@MarkMcCaskey wasmparser is a nice link. Also nice as a quick pre-compile filter to ensure the proper imports and exports sections, etc. And quickly reject data that we don't even want to compile.

I couldn't figure out how to quickly check for floating ops, but it does seem there are some hooks I could use, such as https://docs.rs/wasmparser/0.39.2/wasmparser/struct.CodeSectionReader.html

ethanfrey · 2019-10-11T10:21:11Z

@YaronWittenstein

That is a good point with single-pass to avoid attack vectors. Not sure what the routes are, I just figured you could use a heavier backend and raise the gas cost for creating a contract very high (per byte) and lower the cost for running them. My current design is actually only a few well designed contracts uploaded (can be expensive) and then instantiated many times (making a new erc20 token reuses the same code, just different initial config). But that is just my design, not a general rule.

Also, the support matrix seems to be:

Backend	Gas Metering	Serialization	Compile Speed	Run Speed	Toolchain
Singlepass	Yes	No	Fast	Slow	Nightly
Cranelift	No	Yes	Medium	Medium	Stable
LLVM	Yes	Yes	Slow	Fast	Stable

I want gas metering and serialization, and would prefer the stable toolchain. Which locks me to llvm now. I would consider singlepass (even with nightly) if it had serialization support - which means only compile once on upload, not on every run. And would gladly jump to cranelift when it supports gas metering #819

I say this to explain my thinking now. I am not sure it is the proper choice, and am very interested in feedback.

YaronWittenstein · 2019-10-11T11:11:43Z

@MarkMcCaskey wasmparser is a nice link. Also nice as a quick pre-compile filter to ensure the proper imports and exports sections, etc. And quickly reject data that we don't even want to compile.

I couldn't figure out how to quickly check for floating ops, but it does seem there are some hooks I could use, such as https://docs.rs/wasmparser/0.39.2/wasmparser/struct.CodeSectionReader.html

@ethanfrey The determinstic feature has been implemented here:
yurydelendik/wasmparser.rs@dd0a114

I guess can have deterministic for more backends. maybe we can have backend type feature
orthogonal to the deterministic feature.

YaronWittenstein · 2019-11-30T08:02:48Z

What's the status here? It seems like a very nice feature for the blockchain use-case, but I don't see any movement here.

I am sticking with singlepass for my integration now as well. Especially since llvm support is even harder than I thought (it cannot be published to crates.io due to dependencies inkwell that are structured in a way to prevent that). And I don't see much movement on metering coming to cranelift.

I agree. I'm just not sure what I should do next here... like do we need to support more compilers?

MarkMcCaskey · 2019-12-02T19:27:34Z

I agree, I'm not sure what happened with this PR -- this is a feature we want to ship. I'll follow up with the team

AdamSLevy · 2019-12-03T20:58:51Z

Is there a way to return an error on detecting floating point (disable it) for other compilers?

@ethanfrey it should be trivial to implement a middleware chain that can throw an error if any floating point Operators are detected. This would then be totally agnostic to the backend.

YaronWittenstein · 2019-12-04T08:22:09Z

Is there a way to return an error on detecting floating point (disable it) for other compilers?

@ethanfrey it should be trivial to implement a middleware chain that can throw an error if any floating point Operators are detected. This would then be totally agnostic to the backend.

https://github.com/spacemeshos/svm/blob/develop/crates/svm-compiler/src/middleware/validation.rs#L154

ethanfrey · 2019-12-04T16:11:50Z

@YaronWittenstein awesome link, great to see you have implemented the function as middleware.

If I want to add this to my app, what is the suggested approach? I can just copy that file into my repo (it is MIT), but if this will hit upstream wasmer, I would import there. Or maybe I just import your svm-compiler crate (which is basically what I do in my singlepass compiler backend logic), but I don't think you are making stability guarantees there yet.

Anyway, I'd love to integrate this code, let me know what you would suggest/prefer. Personally, I think this would be a great addition to wasmer-runtime

YaronWittenstein · 2019-12-05T12:49:46Z

@YaronWittenstein awesome link, great to see you have implemented the function as middleware.

If I want to add this to my app, what is the suggested approach? I can just copy that file into my repo (it is MIT), but if this will hit upstream wasmer, I would import there. Or maybe I just import your svm-compiler crate (which is basically what I do in my singlepass compiler backend logic), but I don't think you are making stability guarantees there yet.

Anyway, I'd love to integrate this code, let me know what you would suggest/prefer. Personally, I think this would be a great addition to wasmer-runtime

@syrusakbary @MarkMcCaskey @Hywan
How about that I'll open a PR with the code of the middleware rejecting instructions related to floats / other non-deterministic related features?

Ideally, I'd like to have wasmer expose a feature-flag for deterministic execution.
However, some clients might choose to execute different inputs with different middlewares-chains
(so such middleware can help with that).

MarkMcCaskey · 2019-12-05T19:40:23Z

@YaronWittenstein yeah, that middleware sounds like it'd be good to have -- thanks!

I'm actually going to merge master into this PR and then ship it and we can iterate on it after it's shipped. Sorry about dropping the ball on this one for so long!

MarkMcCaskey · 2019-12-05T20:02:12Z

bors r+

bors · 2019-12-05T20:02:14Z

👎 Rejected by too few approved reviews

MarkMcCaskey · 2019-12-05T20:02:35Z

bors r+

709: new feature flag: `deterministic` r=MarkMcCaskey a=YaronWittenstein The motivation for the PR is for introducing a new feature flag called `deterministic`. When `deterministic` will be enabled (turned-off by default) it'll guarantee deterministic execution of wasm programs across different hardware/circumstances. This is critical for Blockchain projects that require execution to be deterministic in order to reach a consensus of the state transition of each smart-contract transaction. Co-authored-by: Yaron Wittenstein <[email protected]> Co-authored-by: Yaron Wittenstein <[email protected]> Co-authored-by: Mark McCaskey <[email protected]>

bors · 2019-12-05T20:52:03Z

Build failed

wasmerio.wasmer

MarkMcCaskey · 2019-12-05T22:03:25Z

Okay, so I ran into some issues with this that need to be resolved:

I think the deterministic-execution flag needs to be completely orthogonal to the default-backendflags. It makes the conditional logic really complicated and if someone wants to use determinism than they should be using a backend explicitly, not relying on the the automatic backend logic (which could break due to being extremely complex and then break their system). We'll need to document how to go about using this properly somewhere though and talk about the strengths and weaknesses of our backends so that users can make an informed choice.

We'll have to update the cfg flag checking tests too (should be easier now that there's less interaction) and ideally have other tests for something this important, but we can avoid most of that in this PR

MarkMcCaskey · 2019-12-05T23:08:17Z

Okay! I've made a PR spacemeshos#1 ! when that gets merged, we can merge here. Once it's merged we can follow up on some of the specifics if you want to change them

MarkMcCaskey · 2019-12-06T18:17:49Z

@YaronWittenstein ping me when you have a chance to review and merge the PR ^ ! Then I'll ship this one

Deterministic

MarkMcCaskey · 2019-12-06T20:07:52Z

bors r+

709: new feature flag: `deterministic` r=MarkMcCaskey a=YaronWittenstein The motivation for the PR is for introducing a new feature flag called `deterministic`. When `deterministic` will be enabled (turned-off by default) it'll guarantee deterministic execution of wasm programs across different hardware/circumstances. This is critical for Blockchain projects that require execution to be deterministic in order to reach a consensus of the state transition of each smart-contract transaction. 865: adding tests for `state_creator` of `import_object` r=MarkMcCaskey a=YaronWittenstein Part of the PR #807 changes was adding support for shared import objects between threads. https://github.com/wasmerio/wasmer/pull/807/files#diff-d20cb4c5a883566b85be4cc046f45aa9R49 I've added tests/examples on how to create an `import object` with a state_creator (function or closure) 1042: Make regression test work in release builds too. r=nlewycky a=nlewycky Fix this regression test to detect the bug it was looking for in release builds too. This bug triggered an assertion failure in debug, and by examining the pre-opt IR, we can check for the bug in release mode too. Co-authored-by: Yaron Wittenstein <[email protected]> Co-authored-by: Yaron Wittenstein <[email protected]> Co-authored-by: Mark McCaskey <[email protected]> Co-authored-by: Mark McCaskey <[email protected]> Co-authored-by: Nick Lewycky <[email protected]>

bors · 2019-12-06T21:24:26Z

Build failed (retrying...)

wasmerio.wasmer

709: new feature flag: `deterministic` r=MarkMcCaskey a=YaronWittenstein The motivation for the PR is for introducing a new feature flag called `deterministic`. When `deterministic` will be enabled (turned-off by default) it'll guarantee deterministic execution of wasm programs across different hardware/circumstances. This is critical for Blockchain projects that require execution to be deterministic in order to reach a consensus of the state transition of each smart-contract transaction. Co-authored-by: Yaron Wittenstein <[email protected]> Co-authored-by: Yaron Wittenstein <[email protected]> Co-authored-by: Mark McCaskey <[email protected]> Co-authored-by: Mark McCaskey <[email protected]>

bors · 2019-12-06T22:07:29Z

Build succeeded

wasmerio.wasmer

ethanfrey · 2019-12-08T20:22:08Z

Awesome! I can start using this with the next release

YaronWittenstein changed the title ~~Deterministic~~ new feature flag: deterministic Aug 22, 2019

YaronWittenstein force-pushed the deterministic branch from 46c2e82 to 6ca5812 Compare October 7, 2019 13:59

YaronWittenstein marked this pull request as ready for review October 7, 2019 13:59

YaronWittenstein requested review from bjfish, Hywan, losfair, MarkMcCaskey and nlewycky as code owners October 7, 2019 13:59

YaronWittenstein closed this Oct 7, 2019

YaronWittenstein reopened this Oct 7, 2019

MarkMcCaskey reviewed Oct 7, 2019

View reviewed changes

Hywan reviewed Oct 7, 2019

View reviewed changes

fixes to PR comments

7ce5ec4

YaronWittenstein force-pushed the deterministic branch from 4ba0bdd to 7ce5ec4 Compare October 7, 2019 19:58

YaronWittenstein and others added 3 commits October 7, 2019 22:58

Cargo.toml - removing deterministic feature flag

899099f

renamed feature flag deterministic to deterministic-execution

3489bfb

Merge branch 'master' into deterministic

420bfa5

ethanfrey mentioned this pull request Nov 29, 2019

Pre-check wasm before compiling CosmWasm/cosmwasm#50

Closed

3 tasks

MarkMcCaskey added 3 commits December 5, 2019 11:50

Merge branch 'master' into deterministic

c7f3abd

Remove wasmparser dep from singlepass

176b938

Manually apply formatting to cfg statement

d144976

MarkMcCaskey approved these changes Dec 5, 2019

View reviewed changes

Remove deterministic flag setting default compiler; add auto logic

3ba355d

Merge pull request #1 from MarkMcCaskey/deterministic

6da3b22

Deterministic

bors bot merged commit 6da3b22 into wasmerio:master Dec 6, 2019

		@@ -101,6 +101,8 @@ backend-singlepass = [
		wasi = ["wasmer-wasi"]
		managed = ["backend-singlepass", "wasmer-runtime-core/managed"]

new feature flag: deterministic #709

new feature flag: deterministic #709

Conversation

YaronWittenstein commented Aug 22, 2019 • edited Loading

ethanfrey commented Sep 19, 2019

MarkMcCaskey commented Sep 20, 2019

YaronWittenstein commented Oct 6, 2019 • edited Loading

ethanfrey commented Oct 7, 2019

MarkMcCaskey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hywan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

YaronWittenstein Oct 7, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ethanfrey commented Oct 10, 2019

YaronWittenstein commented Oct 10, 2019

MarkMcCaskey commented Oct 10, 2019

ethanfrey commented Oct 11, 2019

ethanfrey commented Oct 11, 2019 • edited Loading

YaronWittenstein commented Oct 11, 2019

YaronWittenstein commented Nov 30, 2019

MarkMcCaskey commented Dec 2, 2019

AdamSLevy commented Dec 3, 2019 • edited Loading

YaronWittenstein commented Dec 4, 2019

ethanfrey commented Dec 4, 2019

YaronWittenstein commented Dec 5, 2019

MarkMcCaskey commented Dec 5, 2019

MarkMcCaskey commented Dec 5, 2019

bors bot commented Dec 5, 2019

MarkMcCaskey commented Dec 5, 2019

bors bot commented Dec 5, 2019

Build failed

MarkMcCaskey commented Dec 5, 2019

MarkMcCaskey commented Dec 5, 2019

MarkMcCaskey commented Dec 6, 2019

MarkMcCaskey commented Dec 6, 2019

bors bot commented Dec 6, 2019

Build failed (retrying...)

bors bot commented Dec 6, 2019

Build succeeded

ethanfrey commented Dec 8, 2019

new feature flag: `deterministic` #709

new feature flag: `deterministic` #709

YaronWittenstein commented Aug 22, 2019 •

edited

Loading

YaronWittenstein commented Oct 6, 2019 •

edited

Loading

YaronWittenstein Oct 7, 2019 •

edited

Loading

ethanfrey commented Oct 11, 2019 •

edited

Loading

AdamSLevy commented Dec 3, 2019 •

edited

Loading