Skip to content

workaround serde deserializing incorrectly when arbitrary_precision enabled#521

Merged
insipx merged 6 commits intoparitytech:masterfrom
insipx:insipx/fix-serde-arbitrary-precision
Nov 22, 2019
Merged

workaround serde deserializing incorrectly when arbitrary_precision enabled#521
insipx merged 6 commits intoparitytech:masterfrom
insipx:insipx/fix-serde-arbitrary-precision

Conversation

@insipx
Copy link
Contributor

@insipx insipx commented Nov 21, 2019

serde deserializes incorrectly when arbitrary precision is enabled.

fixes #520

The workaround is to deserialize into Value before ClientResponse. This is gated under the feature arbitrary_precision in core-client and transports.

serde deserializes incorrectly when arbitrary precision is enabled.
Fix by first deserializing into `Value` before `ClientResponse`
@parity-cla-bot
Copy link

It looks like @insipx signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

@insipx insipx requested a review from tomusdrw November 21, 2019 12:07
Copy link
Contributor

@tomusdrw tomusdrw left a comment

Choose a reason for hiding this comment

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

lgtm! Would be cool to get an integration test for this.

@insipx
Copy link
Contributor Author

insipx commented Nov 21, 2019

there is now a generic fn serde_from_str which checks for cfg!(feature = "arbitrary_precision").

Also made sure that cargo test --feature arbitrary_precision works

@insipx insipx requested a review from tomusdrw November 21, 2019 19:03
response: &str,
) -> Result<(Id, Result<Value, RpcError>, Option<String>, Option<SubscriptionId>), RpcError> {
serde_json::from_str::<ClientResponse>(&response)
jsonrpc_core::serde_from_str::<ClientResponse>(response)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we now get rid of serde_json dependency?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

serde_json is still used for all to_string serializations

Co-Authored-By: Tomasz Drwięga <tomusdrw@users.noreply.github.com>
@insipx insipx merged commit 591e916 into paritytech:master Nov 22, 2019
koushiro added a commit to chainx-org/ChainX that referenced this pull request Sep 17, 2020
JSON serialization/deserialization of `i128`/`u128` can only be supported when the `arbitrary_precision` feature enabled.

But `arbitrary_precision` numbers don't work with `serde(tag = ..)` and `serde(flatten)` syntax (serde-rs/json#505), as a result, the deserialization of structures in `jsonrpc-core` will be error.

Fortunately, the crate `jsonrpc-core` provides a workround(paritytech/jsonrpc#521).
Although this workround will cause more extra serialization/deserialization work, but I think it's worth it.

Signed-off-by: koushiro <koushiro.cqx@gmail.com>
ethan-rep added a commit to ethan-rep/ChainX that referenced this pull request Sep 27, 2025
JSON serialization/deserialization of `i128`/`u128` can only be supported when the `arbitrary_precision` feature enabled.

But `arbitrary_precision` numbers don't work with `serde(tag = ..)` and `serde(flatten)` syntax (serde-rs/json#505), as a result, the deserialization of structures in `jsonrpc-core` will be error.

Fortunately, the crate `jsonrpc-core` provides a workround(paritytech/jsonrpc#521).
Although this workround will cause more extra serialization/deserialization work, but I think it's worth it.

Signed-off-by: koushiro <koushiro.cqx@gmail.com>
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

Successfully merging this pull request may close these issues.

[jsonrpc-client-transports] jsonrpc does not deserialize if user has "arbitrary_precision" enabled in serde

3 participants