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

do not use f64/f32/usize as it fails some wasm runtimes (cosmwasm) #800

Closed
dzmitry-lahoda opened this issue Aug 1, 2023 · 10 comments
Closed
Labels
S: wasm Scope: support Wasm envs

Comments

@dzmitry-lahoda
Copy link
Contributor

dzmitry-lahoda commented Aug 1, 2023

Feature Summary

CosmWasm(wasmd), and hence IBC WASM-08 fail to upload wasm file if it has code with f32/f64.
Unfortunately some things have usize which sometimes handled with floats (crazy but fact https://medium.com/cosmwasm/debugging-floating-point-generation-in-rust-wasm-smart-contract-f47d833b5fba ).

Given these things and that ibc-rs to support many runtime (near, solana, aptos, ic) which also may dislike these floats (as constraint and blockchain enviroments in general) preventing usage of this with linter is good.

Usize, except for some reason some crates use floats to handle it (now sure why), is of variable size, which may lead to bugs, e.g. when target would be wasm64.

Proposal

Remove all f64/f32/usize usage and lint that from repo.

LEGACY

Proposal

I used ibc-rs, and in latest version I get

12:54AM DBG invalid tx code=2 log="failed to execute message; message index: 0: Error calling the VM: Error compiling Wasm: Could not compile: WebAssembly translation error: Error in middleware Gatekeeper: Float operator detected: F64Load { memarg: MemoryImmediate { align: 3, offset: 0, memory: 0 } }. The use of floats is not supported.: create wasm contract failed" module=state
12:54AM INF executed block height=159 module=state num_invalid_txs=1 num_valid_txs=0

when it is part of contract.

I see old Cargo.lock

 "tendermint 0.31.1",
 "tendermint-light-client-verifier 0.31.1",
 "tendermint-proto 0.31.1",
 "dyn-clone",
 "erased-serde",
 "ibc-proto 0.30.0",
 "ics23 0.9.0",
 "ibc 0.41.0",

new

 "tendermint 0.32.2",
 "tendermint-light-client-verifier 0.32.2",
 "tendermint-proto 0.32.2",
 "ics23 0.10.1",
 "ibc-derive 0.1.0",
 "ibc-proto 0.32.0",
 "ics23 0.10.1",
informalsystems-pbjson

Also I found f64 usage in this repo when not needed.

So I do not use any things changed directly, and yet get issues.

Run
https://github.com/CosmWasm/cosmwasm/blob/main/packages/check/README.md
in CI.

I did not yet found root cause so.

Thing happend from 0.41 release inclusive and before 0.43. I was able to upload contract.

I think not only cw wasmd will be such, but wasm-08 too.

@dzmitry-lahoda
Copy link
Contributor Author

dzmitry-lahoda commented Aug 1, 2023

I am trying to strip contract, if that works. UPDATE: did not helped

@dzmitry-lahoda
Copy link
Contributor Author

Okey, it was PrefixedDenom

@dzmitry-lahoda
Copy link
Contributor Author

pub(crate) fn calculate_block_delay(
    delay_period_time: &Duration,
    max_expected_time_per_block: &Duration,
) -> u64 {
    if max_expected_time_per_block.is_zero() {
        return 0;
    }

    FloatCore::ceil(delay_period_time.as_secs_f64() / max_expected_time_per_block.as_secs_f64())
        as u64
}

So such code if it runs in ics wasm-08 likely will not work.

@dzmitry-lahoda
Copy link
Contributor Author

I stably reproduce on my contact, but failed to repro on small contract. When I copy paste PrefixedDenom instead of ref, check passes.


All contracts (1) passed checks!
bash-5.1$ nix run github:informalsystems/cosmos.nix#cosmwasm-check /home/dz/github.com/ComposableFi/composable/code/target/wasm32-unknown-unknown/cosmwasm-contracts/cw_xc_gateway.wasm
Available capabilities: {"stargate", "cosmwasm_1_1", "iterator", "staking", "cosmwasm_1_2"}

/home/dz/github.com/ComposableFi/composable/code/target/wasm32-unknown-unknown/cosmwasm-contracts/cw_xc_gateway.wasm: failure
Error compiling Wasm: Could not compile: WebAssembly translation error: Error in middleware Gatekeeper: Float operator detected: F64Load { memarg: MemoryImmediate { align: 3, offset: 8, memory: 0 } }. The use of floats is not supported.

Passes: 0, failures: 1
bash-5.1$ nix run github:informalsystems/cosmos.nix#cosmwasm-check /home/dz/github.com/ComposableFi/composable/code/target/wasm32-unknown-unknown/cosmwasm-contracts/cw_xc_gateway.wasm
Available capabilities: {"stargate", "cosmwasm_1_2", "iterator", "staking", "cosmwasm_1_1"}

/home/dz/github.com/ComposableFi/composable/code/target/wasm32-unknown-unknown/cosmwasm-contracts/cw_xc_gateway.wasm: pass

All contracts (1) passed checks!

The only differnece, I copied PrefixedDenom to our code. We use Channel/Port/Connect-Id, but when adding PrefixedDenom ref it breaks wasm.... Magic.

@dzmitry-lahoda
Copy link
Contributor Author

Added cw-check example https://github.com/dzmitry-lahoda-forks/ibc-rs/tree/dz/9/ci/cw-check . It still may be usefull to do this to ensure it can run in wasm hosts with no floats support (blockchains).

@dzmitry-lahoda
Copy link
Contributor Author

@dzmitry-lahoda dzmitry-lahoda changed the title prevent float usage which fails to compile for cosmwasm do not use f64/f32/usize as it fails some wasm runtimes (cosmwasm) Aug 1, 2023
@dzmitry-lahoda dzmitry-lahoda reopened this Aug 1, 2023
@plafer
Copy link
Contributor

plafer commented Aug 1, 2023

Thank you for bringing this to our attention! Very subtle...

@dzmitry-lahoda
Copy link
Contributor Author

I fixed thing, it works for me now.

@dzmitry-lahoda
Copy link
Contributor Author

dzmitry-lahoda commented Aug 1, 2023

this so works only on wasm https://github.com/chipshort/wasm-float-transpiler, not riskv or bpf or native float free. but it may be alfa version, and having yet another layer of compilation can be buggy.

@Farhad-Shabani
Copy link
Member

Fixed in #894

@Farhad-Shabani Farhad-Shabani added the S: wasm Scope: support Wasm envs label Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S: wasm Scope: support Wasm envs
Projects
Status: Done
3 participants