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

u128 (and i128) serialization are not supported #540

Open
ozgunozerk opened this issue Apr 6, 2023 · 4 comments
Open

u128 (and i128) serialization are not supported #540

ozgunozerk opened this issue Apr 6, 2023 · 4 comments
Labels
A-serde Area: Serde integration C-enhancement Category: Raise on the bar on expectations M-breaking-change Meta: Implementing or merging this will introduce a breaking change.

Comments

@ozgunozerk
Copy link

Minimal reproducible error:
https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=a9c27b8a6032ebfbbef9cea1908be5dc

In this example, u128/i128 support is already present in serde. Yet, when used with toml, it fails and says u128 is not supported for serialization.

@epage epage added C-enhancement Category: Raise on the bar on expectations A-serde Area: Serde integration M-breaking-change Meta: Implementing or merging this will introduce a breaking change. labels Apr 6, 2023
@epage
Copy link
Member

epage commented Apr 6, 2023

So support this, our choices are

  • Make the entire call stack generic over the type which which is less than ideal
  • Make the entire call stack use i128 (which leaves off parts of u128) which is likely overkill for most applications
  • Use a feature flag for controlling this, requiring wrapping the numbers in a newtype to avoid this being a breaking change (one dependency enabling a feature shouldn't break other dependencies) which both adds implementation complexity and complexity on the users side
  • Only error when value doesn't fit within i64 but that shifts the problem from easily discoverable to requiring very specific input

For reference, serde_json supports serializing and deserializing i128/u128. However, we parse to an intermediate representation (not just for reuse but to deal with toml complexities) which is more akin to serde_json::Value. For that, it only supports u128/i128 if arbitrary_precision is enabled at which point it uses a String with a newtype to abstract the internal representation.

  • The only way for callers to access the String is with Display. There is no way to generate this
  • If its not representable within 64 bits, they use the { "<private key>": "<string>"} way of encoding this within serde

If we support this, a arbitrary_precision feature is most likely the way for us to go with this. It'd need to be supported in toml_edit and toml

@epage
Copy link
Member

epage commented Apr 6, 2023

I'm curious, normally toml is focused more on human-editable use cases. What is it you are using this for that you want to use u128? u128 isn't exactly a type easily used by humans and I would normally expect a string to be used instead so people could use suffixes like 2M (2 million)

@ozgunozerk
Copy link
Author

It is being used in a blockchain application, where u128 will be used for storing the rewards earned by the user. Due to precision issues (like satoshi's for bitcoin), u128 was a safer choice.

@ozgunozerk
Copy link
Author

I can understand the hardships. I could work around this by having a custom type which wraps u128 and serialize it into/from string.
So, it's a good to have from my perspective.

JamesHinshelwood added a commit to Zilliqa/zq2 that referenced this issue Sep 8, 2023
Annoyingly `toml` (the crate) doesn't support `u128`
(toml-rs/toml#540), so we instead take
the configured values as strings and parse them into `u128`s
later.
JamesHinshelwood added a commit to Zilliqa/zq2 that referenced this issue Sep 8, 2023
Annoyingly `toml` (the crate) doesn't support `u128`
(toml-rs/toml#540), so we instead take
the configured values as strings and parse them into `u128`s
later.
JamesHinshelwood added a commit to Zilliqa/zq2 that referenced this issue Sep 11, 2023
Annoyingly `toml` (the crate) doesn't support `u128`
(toml-rs/toml#540), so we instead take
the configured values as strings and parse them into `u128`s
later.
JamesHinshelwood added a commit to Zilliqa/zq2 that referenced this issue Sep 11, 2023
Annoyingly `toml` (the crate) doesn't support `u128`
(toml-rs/toml#540), so we instead take
the configured values as strings and parse them into `u128`s
later.
JamesHinshelwood added a commit to Zilliqa/zq2 that referenced this issue Sep 11, 2023
Annoyingly `toml` (the crate) doesn't support `u128`
(toml-rs/toml#540), so we instead take
the configured values as strings and parse them into `u128`s
later.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-serde Area: Serde integration C-enhancement Category: Raise on the bar on expectations M-breaking-change Meta: Implementing or merging this will introduce a breaking change.
Projects
None yet
Development

No branches or pull requests

2 participants