From da21294ac566b71112668e05a8e1dec34d09d10f Mon Sep 17 00:00:00 2001 From: Jeremy Rubin Date: Thu, 14 Jan 2021 10:45:51 -0800 Subject: [PATCH 1/2] Fix Optional Amount Serialization --- src/util/amount.rs | 131 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 124 insertions(+), 7 deletions(-) diff --git a/src/util/amount.rs b/src/util/amount.rs index 8c3be954de..ad7b57d99d 100644 --- a/src/util/amount.rs +++ b/src/util/amount.rs @@ -880,6 +880,9 @@ pub mod serde { fn des_sat<'d, D: Deserializer<'d>>(d: D) -> Result; fn ser_btc(self, s: S) -> Result; fn des_btc<'d, D: Deserializer<'d>>(d: D) -> Result; + fn type_prefix() -> &'static str; + fn ser_sat_opt(self, s: S) -> Result; + fn ser_btc_opt(self, s: S) -> Result; } impl SerdeAmount for Amount { @@ -896,6 +899,15 @@ pub mod serde { use serde::de::Error; Ok(Amount::from_btc(f64::deserialize(d)?).map_err(D::Error::custom)?) } + fn type_prefix() -> &'static str { + "u" + } + fn ser_sat_opt(self, s: S) -> Result { + s.serialize_some(&self.as_sat()) + } + fn ser_btc_opt(self, s: S) -> Result { + s.serialize_some(&self.as_btc()) + } } impl SerdeAmount for SignedAmount { @@ -912,6 +924,15 @@ pub mod serde { use serde::de::Error; Ok(SignedAmount::from_btc(f64::deserialize(d)?).map_err(D::Error::custom)?) } + fn type_prefix() -> &'static str { + "i" + } + fn ser_sat_opt(self, s: S) -> Result { + s.serialize_some(&self.as_sat()) + } + fn ser_btc_opt(self, s: S) -> Result { + s.serialize_some(&self.as_btc()) + } } pub mod as_sat { @@ -933,15 +954,17 @@ pub mod serde { //! Serialize and deserialize [Optoin] as real numbers denominated in satoshi. //! Use with `#[serde(default, with = "amount::serde::as_sat::opt")]`. - use serde::{Deserializer, Serializer}; + use serde::{Deserializer, Serializer, de}; use util::amount::serde::SerdeAmount; + use std::fmt; + use std::marker::PhantomData; pub fn serialize( a: &Option, s: S, ) -> Result { match *a { - Some(a) => a.ser_sat(s), + Some(a) => a.ser_sat_opt(s), None => s.serialize_none(), } } @@ -949,7 +972,28 @@ pub mod serde { pub fn deserialize<'d, A: SerdeAmount, D: Deserializer<'d>>( d: D, ) -> Result, D::Error> { - Ok(Some(A::des_sat(d)?)) + struct VisitOptAmt(PhantomData); + + impl<'de, X: SerdeAmount> de::Visitor<'de> for VisitOptAmt { + type Value = Option; + + fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + write!(formatter, "An Option<{}64>", X::type_prefix()) + } + + fn visit_none(self) -> Result + where + E: de::Error { + Ok(None) + } + fn visit_some(self, d: D) -> Result + where + D: Deserializer<'de> + { + Ok(Some(X::des_sat(d)?)) + } + } + d.deserialize_option(VisitOptAmt::(PhantomData)) } } } @@ -973,15 +1017,17 @@ pub mod serde { //! Serialize and deserialize [Option] as JSON numbers denominated in BTC. //! Use with `#[serde(default, with = "amount::serde::as_btc::opt")]`. - use serde::{Deserializer, Serializer}; + use serde::{Deserializer, Serializer, de}; use util::amount::serde::SerdeAmount; + use std::fmt; + use std::marker::PhantomData; pub fn serialize( a: &Option, s: S, ) -> Result { match *a { - Some(a) => a.ser_btc(s), + Some(a) => a.ser_btc_opt(s), None => s.serialize_none(), } } @@ -989,7 +1035,28 @@ pub mod serde { pub fn deserialize<'d, A: SerdeAmount, D: Deserializer<'d>>( d: D, ) -> Result, D::Error> { - Ok(Some(A::des_btc(d)?)) + struct VisitOptAmt(PhantomData); + + impl<'de, X :SerdeAmount> de::Visitor<'de> for VisitOptAmt { + type Value = Option; + + fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + write!(formatter, "An Option") + } + + fn visit_none(self) -> Result + where + E: de::Error { + Ok(None) + } + fn visit_some(self, d: D) -> Result + where + D: Deserializer<'de>, + { + Ok(Some(X::des_btc(d)?)) + } + } + d.deserialize_option(VisitOptAmt::(PhantomData)) } } } @@ -1345,7 +1412,7 @@ mod tests { fn serde_as_btc_opt() { use serde_json; - #[derive(Serialize, Deserialize, PartialEq, Debug)] + #[derive(Serialize, Deserialize, PartialEq, Debug, Eq)] struct T { #[serde(default, with = "::util::amount::serde::as_btc::opt")] pub amt: Option, @@ -1362,6 +1429,13 @@ mod tests { samt: None, }; + // Test Roundtripping + for s in [&with, &without].iter() { + let v = serde_json::to_string(s).unwrap(); + let w : T = serde_json::from_str(&v).unwrap(); + assert_eq!(w, **s); + } + let t: T = serde_json::from_str("{\"amt\": 2.5, \"samt\": -2.5}").unwrap(); assert_eq!(t, with); @@ -1375,4 +1449,47 @@ mod tests { let value_without: serde_json::Value = serde_json::from_str("{}").unwrap(); assert_eq!(without, serde_json::from_value(value_without).unwrap()); } + + #[cfg(feature = "serde")] + #[test] + fn serde_as_sat_opt() { + use serde_json; + + #[derive(Serialize, Deserialize, PartialEq, Debug, Eq)] + struct T { + #[serde(default, with = "::util::amount::serde::as_sat::opt")] + pub amt: Option, + #[serde(default, with = "::util::amount::serde::as_sat::opt")] + pub samt: Option, + } + + let with = T { + amt: Some(Amount::from_sat(2__500_000_00)), + samt: Some(SignedAmount::from_sat(-2__500_000_00)), + }; + let without = T { + amt: None, + samt: None, + }; + + // Test Roundtripping + for s in [&with, &without].iter() { + let v = serde_json::to_string(s).unwrap(); + let w : T = serde_json::from_str(&v).unwrap(); + assert_eq!(w, **s); + } + + let t: T = serde_json::from_str("{\"amt\": 250000000, \"samt\": -250000000}").unwrap(); + assert_eq!(t, with); + + let t: T = serde_json::from_str("{}").unwrap(); + assert_eq!(t, without); + + let value_with: serde_json::Value = + serde_json::from_str("{\"amt\": 250000000, \"samt\": -250000000}").unwrap(); + assert_eq!(with, serde_json::from_value(value_with).unwrap()); + + let value_without: serde_json::Value = serde_json::from_str("{}").unwrap(); + assert_eq!(without, serde_json::from_value(value_without).unwrap()); + } } From a0c7f530bac543cf73f8a9e33ee8ac3ec3de066f Mon Sep 17 00:00:00 2001 From: Jeremy Rubin Date: Sun, 28 Feb 2021 09:13:46 -0800 Subject: [PATCH 2/2] Localize breaking changes of fixing the Amount serialization to only the broken Option serializer. --- src/util/amount.rs | 36 ++++++++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/src/util/amount.rs b/src/util/amount.rs index ad7b57d99d..f3ab4ec3c1 100644 --- a/src/util/amount.rs +++ b/src/util/amount.rs @@ -875,11 +875,25 @@ pub mod serde { /// This trait is used only to avoid code duplication and naming collisions /// of the different serde serialization crates. + /// + /// TODO: Add the private::Sealed bound in next breaking release pub trait SerdeAmount: Copy + Sized { fn ser_sat(self, s: S) -> Result; fn des_sat<'d, D: Deserializer<'d>>(d: D) -> Result; fn ser_btc(self, s: S) -> Result; fn des_btc<'d, D: Deserializer<'d>>(d: D) -> Result; + } + + mod private { + /// add this as a trait bound to traits which consumers of this library + /// should not be able to implement. + pub trait Sealed {} + impl Sealed for super::Amount {} + impl Sealed for super::SignedAmount {} + } + + /// This trait is only for internal Amount type serialization/deserialization + pub trait SerdeAmountForOpt: Copy + Sized + SerdeAmount + private::Sealed { fn type_prefix() -> &'static str; fn ser_sat_opt(self, s: S) -> Result; fn ser_btc_opt(self, s: S) -> Result; @@ -899,6 +913,9 @@ pub mod serde { use serde::de::Error; Ok(Amount::from_btc(f64::deserialize(d)?).map_err(D::Error::custom)?) } + } + + impl SerdeAmountForOpt for Amount { fn type_prefix() -> &'static str { "u" } @@ -924,6 +941,9 @@ pub mod serde { use serde::de::Error; Ok(SignedAmount::from_btc(f64::deserialize(d)?).map_err(D::Error::custom)?) } + } + + impl SerdeAmountForOpt for SignedAmount { fn type_prefix() -> &'static str { "i" } @@ -955,11 +975,11 @@ pub mod serde { //! Use with `#[serde(default, with = "amount::serde::as_sat::opt")]`. use serde::{Deserializer, Serializer, de}; - use util::amount::serde::SerdeAmount; + use util::amount::serde::SerdeAmountForOpt; use std::fmt; use std::marker::PhantomData; - pub fn serialize( + pub fn serialize( a: &Option, s: S, ) -> Result { @@ -969,12 +989,12 @@ pub mod serde { } } - pub fn deserialize<'d, A: SerdeAmount, D: Deserializer<'d>>( + pub fn deserialize<'d, A: SerdeAmountForOpt, D: Deserializer<'d>>( d: D, ) -> Result, D::Error> { struct VisitOptAmt(PhantomData); - impl<'de, X: SerdeAmount> de::Visitor<'de> for VisitOptAmt { + impl<'de, X: SerdeAmountForOpt> de::Visitor<'de> for VisitOptAmt { type Value = Option; fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { @@ -1018,11 +1038,11 @@ pub mod serde { //! Use with `#[serde(default, with = "amount::serde::as_btc::opt")]`. use serde::{Deserializer, Serializer, de}; - use util::amount::serde::SerdeAmount; + use util::amount::serde::SerdeAmountForOpt; use std::fmt; use std::marker::PhantomData; - pub fn serialize( + pub fn serialize( a: &Option, s: S, ) -> Result { @@ -1032,12 +1052,12 @@ pub mod serde { } } - pub fn deserialize<'d, A: SerdeAmount, D: Deserializer<'d>>( + pub fn deserialize<'d, A: SerdeAmountForOpt, D: Deserializer<'d>>( d: D, ) -> Result, D::Error> { struct VisitOptAmt(PhantomData); - impl<'de, X :SerdeAmount> de::Visitor<'de> for VisitOptAmt { + impl<'de, X :SerdeAmountForOpt> de::Visitor<'de> for VisitOptAmt { type Value = Option; fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {