paupino / rust-decimal

Decimal number implementation written in pure Rust suitable for financial and fixed-precision calculations.
https://docs.rs/rust_decimal/
MIT License
991 stars 180 forks source link

breaking change ? rmp-serde fail to deserialise after serialisation #426

Open jrouaix opened 3 years ago

jrouaix commented 3 years ago

Hello and thank you for this lib !

I have an issue after migrated from v1.14.3 to v1.16.0 of this crate.

This test is failing with new version :

  #[derive(Serialize, Deserialize, Debug)]
  struct TestStruct {
    pub dec: rust_decimal::Decimal,
  }

  #[test]
  fn rmp_serialization_should_be_able_to_read_decimals() {
    use rust_decimal_macros::dec;
    let ts = TestStruct { dec: dec!(42) };
    let bin = rmp_serde::to_vec(&ts).expect("?");
    dbg!(&bin);
    let value_back = rmp_serde::from_slice::<TestStruct>(&bin).expect("?");
    assert_eq!(value_back.dec, ts.dec);
  }

The debug output is this one on v1.14.3 :

&bin = [
    145,
    203,
    64,
    69,
    0,
    0,
    0,
    0,
    0,
    0,
]

While on v1.16.0 it's output is :

&bin = [
    145,
    145,
    162,
    52,
    50,
]

before failing with the error :

thread 'serialization::tests::rmp_serialization_should_be_able_to_read_decimals' panicked at '?: Syntax("invalid type: sequence, expected a Decimal type representing a fixed-point number")'

Here is the features activated in the toml file :

rust_decimal = { version = "1.13", features = ["serde-float", "serde-str", "serde-arbitrary-precision"] }
rust_decimal_macros = "1.13"

weird that my cargo.lock mentioned a 1.14 version ? 🤔

Can you think of a change that could have led to this behavior ? Of course it could be a change behind rmp-serde, but their libs changed less in the same time so I ask you first.

I hope I didn't do something just really stupid ...

paupino commented 3 years ago

To confirm, this does seem like it is caused by a change in rust-decimal. In particular: #404. This highlights the need to clean up the confusing relationship between these features. In the meantime (in lieu of fixing this), there are two options:

  1. Fix the version in your Cargo.toml. e.g.
rust_decimal = { version = "=1.14.3", features = ["serde-float", "serde-str", "serde-arbitrary-precision"] }
rust_decimal_macros = "=1.14.3"
  1. The conflict happens because of the serde-float feature. If you could remove this then it should work. That being said, there could be good reason to keep it!

In the meantime, I'm going to play around with the features a bit. I do see that the combination of all three features is currently not tested at all so I'll start with that and figure out how to close the gap.

schungx commented 3 years ago

Yes, IMHO features is one huge mess in Rust right now...

It gets worse exponentially when you start having more.