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

Contract including rand or time can be stored in chain #530

Closed
loloicci opened this issue Sep 15, 2020 · 14 comments
Closed

Contract including rand or time can be stored in chain #530

loloicci opened this issue Sep 15, 2020 · 14 comments
Milestone

Comments

@loloicci
Copy link
Contributor

loloicci commented Sep 15, 2020

Although contracts compiled from rust code including rand::Rng.gen() or SystemTime::now() are non-deterministic, they can be stored in chain. (They should not)

The followings are examples of these contracts.
https://github.com/loloicci/cosmwasm-contract-rand
https://github.com/loloicci/cosmwasm-contract-time

(Executing rand::Rng.gen() or SystemTime::now() issues out of gas.
The cause of this error is not a lack of gas and it is an error during executing a contract with wasmer.
It is concerned with #501 and #375.)

@ethanfrey
Copy link
Member

This contract should error. Better yet, it should error on store code (or compilng).

The misnamed error message on instantiation is covered by other issues, but this highlights missing static checks on the wasm code before storing it. (I would consider this issue closed if said contracts are rejected by store code)

@loloicci loloicci changed the title Contract including rand or time can be stored in chain and executing them issues "out of gas" error Contract including rand or time can be stored in chain Sep 15, 2020
@loloicci
Copy link
Contributor Author

loloicci commented Sep 15, 2020

Thank you @ethanfrey. I agree with you.
I changed the title to make clear the problem.

@loloicci
Copy link
Contributor Author

loloicci commented Sep 24, 2020

Contracts try to read files in the local-machine behaves the same.
It should not be stored in the chain, too.

https://github.com/loloicci/cosmwasm-contract-file

@ethanfrey
Copy link
Member

I saw this rust-lang/rust@8fe65da

Maybe the wasm_syscall feature flag is somehow being set when compiling?

@SatoshiSakamori
Copy link

I think this problem is unsupported function will panic or just generate a message. For example, System.now() will panic.
So generated WASM binary by compiler is consisted by permitted op codes. To avoid this, I think Rust compiler should generate unsupported op code when that function used using inline assembler.

@webmaster128
Copy link
Member

For example, System.now() will panic.

Thank you for looking this up. That was my guess as well. Then #501 would be most important and to make sure the panic's error message is propagated to the user.

generate unsupported op code

What kind of op code should that be?

@SatoshiSakamori
Copy link

SatoshiSakamori commented Sep 28, 2020

For example, System.now() will panic.

Thank you for looking this up. That was my guess as well. Then #501 would be most important and to make sure the panic's error message is propagated to the user.

generate unsupported op code

What kind of op code should that be?

I'm sorry if I'm wrong because I'm not familiar with Rust lang.
Permitted op codes are here. So, I thought I could embed other op codes. I'm not sure it really can embed other op codes.

@webmaster128
Copy link
Member

Permitted op codes are here. So, I thought I could embed other op codes. I'm not sure it really can embed other op codes.

This only works for op codes that already exist in the Wasm standard. CosmWasm does not create them. It would be the same for any language that compiles to Wasm.

@loloicci
Copy link
Contributor Author

loloicci commented Oct 2, 2020

This means compiling environment unpatched rust-lang/rust@8fe65da can generate wasm binary including systemcall to get the time, right?
Although I am interested in how the wasm binary generated in this way works in CosmWasm (of course, it is expected to issue some error), I could not make the compiling environment. Does anyone know how to make it?

@webmaster128
Copy link
Member

This means compiling environment unpatched rust-lang/rust@8fe65da can generate wasm binary including systemcall to get the time, right?

I don't know what wasm_syscall exactly, but all Wasm code runs in a sandbox and can only access functions that are explicitely imported into that sandbox. Since we don't important any system functions into the CosmWasm VM, the Wasm code cannot call them.

@webmaster128
Copy link
Member

webmaster128 commented Oct 2, 2020

i.e. even if you can upload a Wasm that tries to call system functions, it will fail to do so

@loloicci
Copy link
Contributor Author

loloicci commented Oct 2, 2020

Thanks for your reply. It helps me!
Maybe wasm_syscall uses WASI (WebAssembly System Interface). You mean that wasmer does not use WASI without explicitly import, so we cannot use system function in the contract, right?

@webmaster128
Copy link
Member

Wasmer has support for WASI, but we are not using it. So no WASI functionality is exposed to contracts.

@webmaster128
Copy link
Member

Closing this as a duplicate of #501

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

No branches or pull requests

4 participants