search

CosmWasm / serde-json-wasm

serde_json for Wasm programs (small, deterministic, no floats)
Apache License 2.0
30 stars 15 forks source link

`serde::​de​::Error::custom()` impl introduces floats downstream due to `to_string()` (`fmt::Display`) usage #42

Closed chris-ricketts closed 1 year ago

chris-ricketts commented 2 years ago

Hi, thanks for this crate - it's incredibly useful.

I am implementing a crate that ports serde_json::Value (deserialisation only) to be used in WASM smart contracts.

However, uploading contracts that use the crate are rejected due to F64 operations being present.

I have tracked this down to the serde::de::Unexpected type, specifically the core::fmt::Display impl (similar to this blog post).

Here is branch where I've removed explicit references to both serde::de::Unexpected and serde::de::Error: https://github.com/chris-ricketts/serde-json-value-wasm/tree/fix-float-inclusion

Unexpected is still in the WASM output however and in turn core::fmt functions involving floats. Both the unmangled WASM text and call path analysis can be found in this gist or run the read-wasm-output.sh script in the repo root.

I have found that if I remove the msg.to_string() call in the serde::de::Error impl for serde_json_wasm::de::Error then no Unexpected type or float fmt operations are included in the output, and the contract upload succeeds.

This serde_json_value_wasm branch uses a patched version of serde_json_wasm. The WASM text and call paths without Unexpected or f64 can be seen in this gist

The twiggy diff between the two WASM files can be seen in this mainline vs patched gist.

Is the patch removing the call to to_string in the serde::de::Error impl something that you would consider accepting a PR for?

There is some precedent for discarding this by default in serde_json_core.

I am at a bit of a loss as to finding where the serde_json_value_wasm crate is implicitly invoking it, if you have any insights on that front it would be greatly appreciated.

chris-ricketts commented 1 year ago

Hi - this issue has come up for me again on a different project when the std::num::NonZeroU128 type is deserialized. Using my patch avoids the F64Load.

Would you be open to a PR to feature gate the custom error messages like it is in serde_json_core or discard the error message completely?