Find a stable and convenient solution for floats implicit loading before cosmwasm 1.0 - Githubissues

CosmWasm / cosmwasm

Framework for building smart contracts in Wasm for the Cosmos SDK

https://www.cosmwasm.com/

Apache License 2.0

1.07k stars 333 forks source link

Find a stable and convenient solution for floats implicit loading before cosmwasm 1.0 #458

Closed KamiD closed 1 year ago

KamiD commented 4 years ago

Summary of Bug

there is a problem about floating number type support differentiation in tinyGo and rust-wasm.

in cosmwasm, Floating number is disabled, so the function "parse_wasm_opcode" which in source file primitives.rs will denied all floating opcode, such as F32Load or any opcode about floating number,
when i using tinygo to build some code like json parser or make a custom marshal interface, since i used a package include float type, the F32Load OPCODE will complie into wasm file by tinyGo, so i have to check and recheck.

that's not a finally solution, I think there at least have two way to fix that:

disable all float support in tinyGo, that's not very easy~
allow wasm file which inluding floating opcode loaded,that's not cool for security of cosmwasm

Version

Cosmwasm v0.8

KamiD commented 4 years ago

hi guys, checking the unsupported symbol during compiling~ how do you think ? I wrote a tool for wasm file check, it will translate wasm file to wat file, then analyze every single line to make sure no unsupported opcode include, it will generate calling map and relationship between functions, find out where is the unsupported opcode using~ Also it will check tables such as export\import\memory or anything else. after some resolving, it will translated from wat file to wasm file, that's a way to determined wasm file is suitable with cosmwasm-vm(pass through some checkXXX in cosmwasm-vm) during compiling

webmaster128 commented 3 years ago

@KamiD such a tool seems very helpful for debugging. However, I would only check the Wasm and explain. Manipulating the Wasm can easily become dangerous when the tool does not 100 % work as intended.

Building a call tree for debugging is nice until you hit call_indirect, where the call tree suddenly depends on runtime variable values.

I was thinking about a different approach to rejecting Wasm ops codes: we can replace every float operation with Operator::Unreachable. This would allow us to accept floats as long as they are never used at runtime.

ethanfrey commented 3 years ago

I was thinking about a different approach to rejecting Wasm ops codes: we can replace every float operation with Operator::Unreachable. This would allow us to accept floats as long as they are never used at runtime.

I did this in a preprocessing compile step, before uploading wasm blobs, when working on the TinyGo prototype. There were float operations that were never executed in runtime, but still not removed by the compiler/optimizer.

This is a bit dangerous, as it may cause contracts that really use floats to fail unexpectedly, but it can resolve the issue. I would not do this on the server, but require the user uploading contracts to do this as a pre-processing step, maybe with some warning if there were more floats removed than typical

webmaster128 commented 3 years ago

Removing from the 1.0 milestone as the state of this is too vague to block the release. We can open up floats later on. It would be consensus breaking but existing contracts would not be affected.

ethanfrey commented 3 years ago

Agree

webmaster128 commented 3 years ago

NaN canonicalization is available for all Wasmer backends now. NEAR is using to overcome the only non-deterministic aspect of floats in Wasm and seems to be happy with it.

Should we just open up? That would allow all kinds of calculations to become possible in CosmWasm contracts that you cannot do in native Cosmos SDK because of the additional sandbox layer.

ethanfrey commented 3 years ago

If we can guarantee they produce the same results on ARM (M1 and "Rasperry Pi"), Intel and AMD, then I am happy to allow them. It would make lots of things much easier. I just want to make sure this is safe.

ethanfrey commented 3 years ago

Maybe you can show me the NEAR test vectors / fuzzing that I can use to convince myself this is safe.

webmaster128 commented 3 years ago

Singlepass v1 is x86_64 only and looking at https://github.com/wasmerio/wasmer/issues/2463 this will not change anymore. So for a cross-chipset test you first need a cross-backend test. I wonder if that is desired? Why should e.g. cranelift and singlepass calculate deterministic results. Maybe it works though. There are tests in the Wasmer code base I can look into.

ethanfrey commented 3 years ago

The issue for me is if floats are safe in x86 only singlepass v1, but no longer safe in multi-architecture singlepass v2, but we already enabled float support with v1, then we can never upgrade to v2.

The conservative alternative is to not allow them until v2 is out and we are sure this is deterministic cross-architecture. At that point we can remove the "no-float" rule with a minor release of CosmWasm, either together with v2 support or separate. But sure that we will not regret that step.

Personally, it would make PoE and bonding curves much easier to code. Just too much time at Tendermint having "floats are evil in blockchain" drilled into my head.

AmitPr commented 1 year ago

Hey all, any update on the state of support across various architectures that would allow this to move forward?

betterclever commented 1 year ago

Hey everyone! Any updates on the support of Float ops? I encountered an issue while trying to use pbjson with prost-serde plugin. I think it uses FP32 internally somewhere. Currently, trying to debug it using the tool mentioned in the thread.

chipshort commented 1 year ago

I just started working on float support, probably coming in CosmWasm 1.5. Keep an eye on #1845 and any PRs related to that.

webmaster128 commented 1 year ago

Closing in favour of https://github.com/CosmWasm/cosmwasm/issues/1845