Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

WASM runtime update #6467

Merged
merged 6 commits into from
Sep 10, 2017
Merged

WASM runtime update #6467

merged 6 commits into from
Sep 10, 2017

Conversation

NikVolf
Copy link
Contributor

@NikVolf NikVolf commented Sep 5, 2017

  • WASM contracts will be able to utilize various blockchain runtime externs (gas_limit, difficulty, coinbase, etc..)
  • corresponding happy test
  • also updates to use latest parity-wasm and corresponding refactoring

@NikVolf NikVolf added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Sep 5, 2017
Copy link
Collaborator

@svyatonik svyatonik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but I would replace exact gas_left checks in tests with something like assert!(gas_left < params.gas) or assert!(params.gas - gas_left < 5000).
Not sure that exact value is important - imho it depends on too many factors like compiler version (next version may generate slightly different wasm code), compiler flags (something like optimization/no optimization, etc), code itself.

@svyatonik svyatonik added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Sep 8, 2017
@NikVolf
Copy link
Contributor Author

NikVolf commented Sep 8, 2017

@svyatonik
I want gas price to match exactly when we don't change wasm itself, it's consensus-critical issue

@rphmeier
Copy link
Contributor

rphmeier commented Sep 8, 2017

@NikVolf I agree. Related: what made the gas costs for examples smaller? I was worried that they aren't remaining consistent.

@NikVolf
Copy link
Contributor Author

NikVolf commented Sep 8, 2017

@rphmeier
oh we change the pwasm-std currently a lot, so the way wasm produced in tests repository will change from time to time

I am not sure about what exactly caused this particular change though, nor it does matter much

@gavofyork gavofyork merged commit ee14a3f into master Sep 10, 2017
@gavofyork gavofyork deleted the wasm-update branch September 10, 2017 16:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants