From 87d6f1718c28fbba07b70ecaee271aa05532a70f Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 14 May 2025 12:13:38 +1000 Subject: [PATCH 1/4] Make serde attribute usage more terse The `serde` attribute can be made more terse in docs and tests with no loss of clarity. Refactor and docs only, no logic change. --- units/src/amount/serde.rs | 11 ++++++----- units/src/fee_rate/serde.rs | 4 ++-- units/tests/serde.rs | 30 +++++++++++++++--------------- 3 files changed, 23 insertions(+), 22 deletions(-) diff --git a/units/src/amount/serde.rs b/units/src/amount/serde.rs index 1db2e9eea..fc4ca82e3 100644 --- a/units/src/amount/serde.rs +++ b/units/src/amount/serde.rs @@ -13,11 +13,11 @@ //! //! ``` //! use serde::{Serialize, Deserialize}; -//! use bitcoin_units::Amount; +//! use bitcoin_units::{amount, Amount}; //! //! #[derive(Serialize, Deserialize)] //! pub struct HasAmount { -//! #[serde(with = "bitcoin_units::amount::serde::as_sat")] +//! #[serde(with = "amount::serde::as_sat")] //! pub amount: Amount, //! } //! ``` @@ -396,6 +396,7 @@ pub mod as_str { #[cfg(test)] mod tests { use super::*; + use crate::amount; #[test] fn sanity_check() { @@ -407,7 +408,7 @@ mod tests { fn can_serde_as_sat() { #[derive(Debug, PartialEq, Eq, Serialize, Deserialize)] pub struct HasAmount { - #[serde(with = "crate::amount::serde::as_sat")] + #[serde(with = "amount::serde::as_sat")] pub amount: Amount, } @@ -426,7 +427,7 @@ mod tests { fn can_serde_as_btc() { #[derive(Debug, PartialEq, Eq, Serialize, Deserialize)] pub struct HasAmount { - #[serde(with = "crate::amount::serde::as_btc")] + #[serde(with = "amount::serde::as_btc")] pub amount: Amount, } @@ -445,7 +446,7 @@ mod tests { fn can_serde_as_str() { #[derive(Debug, PartialEq, Eq, Serialize, Deserialize)] pub struct HasAmount { - #[serde(with = "crate::amount::serde::as_str")] + #[serde(with = "amount::serde::as_str")] pub amount: Amount, } diff --git a/units/src/fee_rate/serde.rs b/units/src/fee_rate/serde.rs index fdcb4d67e..b9c40b77a 100644 --- a/units/src/fee_rate/serde.rs +++ b/units/src/fee_rate/serde.rs @@ -14,11 +14,11 @@ //! //! ``` //! use serde::{Serialize, Deserialize}; -//! use bitcoin_units::FeeRate; +//! use bitcoin_units::{fee_rate, FeeRate}; //! //! #[derive(Serialize, Deserialize)] //! pub struct Foo { -//! #[serde(with = "bitcoin_units::fee_rate::serde::as_sat_per_kwu")] +//! #[serde(with = "fee_rate::serde::as_sat_per_kwu")] //! pub fee_rate: FeeRate, //! } //! ``` diff --git a/units/tests/serde.rs b/units/tests/serde.rs index e6c7d0af6..453f218c6 100644 --- a/units/tests/serde.rs +++ b/units/tests/serde.rs @@ -7,41 +7,41 @@ use bincode::serialize; use bitcoin_units::locktime::{absolute, relative}; -use bitcoin_units::{Amount, BlockHeight, BlockInterval, FeeRate, SignedAmount, Weight}; +use bitcoin_units::{amount, fee_rate, Amount, BlockHeight, BlockInterval, FeeRate, SignedAmount, Weight}; use serde::{Deserialize, Serialize}; /// A struct that includes all the types that implement or support `serde` traits. #[derive(Debug, Serialize, Deserialize)] struct Serde { - #[serde(with = "bitcoin_units::amount::serde::as_sat")] + #[serde(with = "amount::serde::as_sat")] unsigned_as_sat: Amount, - #[serde(with = "bitcoin_units::amount::serde::as_btc")] + #[serde(with = "amount::serde::as_btc")] unsigned_as_btc: Amount, - #[serde(with = "bitcoin_units::amount::serde::as_sat::opt")] + #[serde(with = "amount::serde::as_sat::opt")] unsigned_opt_as_sat: Option, - #[serde(with = "bitcoin_units::amount::serde::as_btc::opt")] + #[serde(with = "amount::serde::as_btc::opt")] unsigned_opt_as_btc: Option, - #[serde(with = "bitcoin_units::amount::serde::as_sat")] + #[serde(with = "amount::serde::as_sat")] signed_as_sat: SignedAmount, - #[serde(with = "bitcoin_units::amount::serde::as_btc")] + #[serde(with = "amount::serde::as_btc")] signed_as_btc: SignedAmount, - #[serde(with = "bitcoin_units::amount::serde::as_sat::opt")] + #[serde(with = "amount::serde::as_sat::opt")] signed_opt_as_sat: Option, - #[serde(with = "bitcoin_units::amount::serde::as_btc::opt")] + #[serde(with = "amount::serde::as_btc::opt")] signed_opt_as_btc: Option, - #[serde(with = "bitcoin_units::fee_rate::serde::as_sat_per_vb_floor")] + #[serde(with = "fee_rate::serde::as_sat_per_vb_floor")] vb_floor: FeeRate, - #[serde(with = "bitcoin_units::fee_rate::serde::as_sat_per_vb_ceil")] + #[serde(with = "fee_rate::serde::as_sat_per_vb_ceil")] vb_ceil: FeeRate, - #[serde(with = "bitcoin_units::fee_rate::serde::as_sat_per_kwu")] + #[serde(with = "fee_rate::serde::as_sat_per_kwu")] kwu: FeeRate, - #[serde(with = "bitcoin_units::fee_rate::serde::as_sat_per_vb_floor::opt")] + #[serde(with = "fee_rate::serde::as_sat_per_vb_floor::opt")] opt_vb_floor: Option, - #[serde(with = "bitcoin_units::fee_rate::serde::as_sat_per_vb_ceil::opt")] + #[serde(with = "fee_rate::serde::as_sat_per_vb_ceil::opt")] opt_vb_ceil: Option, - #[serde(with = "bitcoin_units::fee_rate::serde::as_sat_per_kwu::opt")] + #[serde(with = "fee_rate::serde::as_sat_per_kwu::opt")] opt_kwu: Option, a: BlockHeight, From d6940497fdf71a37009e0433f615530121e6cd36 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 14 May 2025 12:25:33 +1000 Subject: [PATCH 2/4] Simplify fee_rate serde deserialize opt We can just call through to the `deserialize` function. Reduces code duplication and increases maintainability. Refactor only, no logic change. --- units/src/fee_rate/serde.rs | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/units/src/fee_rate/serde.rs b/units/src/fee_rate/serde.rs index b9c40b77a..283a1c2c1 100644 --- a/units/src/fee_rate/serde.rs +++ b/units/src/fee_rate/serde.rs @@ -50,7 +50,7 @@ pub mod as_sat_per_kwu { use core::fmt; - use serde::{de, Deserialize, Deserializer, Serializer}; + use serde::{de, Deserializer, Serializer}; use crate::FeeRate; @@ -83,7 +83,7 @@ pub mod as_sat_per_kwu { where D: Deserializer<'de>, { - Ok(Some(FeeRate::from_sat_per_kwu(u64::deserialize(d)?))) + Ok(Some(super::deserialize(d)?)) } } d.deserialize_option(VisitOpt) @@ -121,9 +121,8 @@ pub mod as_sat_per_vb_floor { use core::fmt; - use serde::{de, Deserialize, Deserializer, Serializer}; + use serde::{de, Deserializer, Serializer}; - use crate::fee_rate::serde::OverflowError; use crate::fee_rate::FeeRate; #[allow(clippy::ref_option)] // API forced by serde. @@ -155,11 +154,7 @@ pub mod as_sat_per_vb_floor { where D: Deserializer<'de>, { - Ok(Some( - FeeRate::from_sat_per_vb(u64::deserialize(d)?) - .ok_or(OverflowError) - .map_err(serde::de::Error::custom)?, - )) + Ok(Some(super::deserialize(d)?)) } } d.deserialize_option(VisitOpt) @@ -197,9 +192,8 @@ pub mod as_sat_per_vb_ceil { use core::fmt; - use serde::{de, Deserialize, Deserializer, Serializer}; + use serde::{de, Deserializer, Serializer}; - use crate::fee_rate::serde::OverflowError; use crate::fee_rate::FeeRate; #[allow(clippy::ref_option)] // API forced by serde. @@ -231,11 +225,7 @@ pub mod as_sat_per_vb_ceil { where D: Deserializer<'de>, { - Ok(Some( - FeeRate::from_sat_per_vb(u64::deserialize(d)?) - .ok_or(OverflowError) - .map_err(serde::de::Error::custom)?, - )) + Ok(Some(super::deserialize(d)?)) } } d.deserialize_option(VisitOpt) From 3658865a18ed875704c28fa47358a610bf266ae5 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 14 May 2025 12:28:49 +1000 Subject: [PATCH 3/4] Fix expecting string for fee_rate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit According to the `serde` docs: > This is used in error messages. The message should complete the > sentence “This Visitor expects to receive …”, for example the message > could be “an integer between 0 and 64”. The message should not be > capitalized and should not end with a period. However we have the `expecting` str using the converted type not the thing the visitor expects. Use `u64` instead of `FeeRate` since that is what is being parsed. Note that in `amount` we got it _almost_ correct, subsequent patch will fix the case. --- units/src/fee_rate/serde.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/units/src/fee_rate/serde.rs b/units/src/fee_rate/serde.rs index 283a1c2c1..ae2e2fb8f 100644 --- a/units/src/fee_rate/serde.rs +++ b/units/src/fee_rate/serde.rs @@ -69,7 +69,7 @@ pub mod as_sat_per_kwu { type Value = Option; fn expecting(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "An Option") + write!(f, "an Option") } fn visit_none(self) -> Result @@ -140,7 +140,7 @@ pub mod as_sat_per_vb_floor { type Value = Option; fn expecting(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "An Option") + write!(f, "an Option") } fn visit_none(self) -> Result @@ -211,7 +211,7 @@ pub mod as_sat_per_vb_ceil { type Value = Option; fn expecting(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "An Option") + write!(f, "an Option") } fn visit_none(self) -> Result From 980365097d44fc8ebb8e0b1327058a59ae62a439 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 14 May 2025 12:31:16 +1000 Subject: [PATCH 4/4] Fix expecting string for amount types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit According to the `serde` docs: > This is used in error messages. The message should complete the > sentence “This Visitor expects to receive …”, for example the message > could be “an integer between 0 and 64”. The message should not be > capitalized and should not end with a period. Use a lower case character to start the string. --- units/src/amount/serde.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/units/src/amount/serde.rs b/units/src/amount/serde.rs index fc4ca82e3..4be766058 100644 --- a/units/src/amount/serde.rs +++ b/units/src/amount/serde.rs @@ -230,7 +230,7 @@ pub mod as_sat { type Value = Option; fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - write!(formatter, "An Option<{}64>", X::type_prefix(private::Token)) + write!(formatter, "an Option<{}64>", X::type_prefix(private::Token)) } fn visit_none(self) -> Result @@ -301,7 +301,7 @@ pub mod as_btc { type Value = Option; fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - write!(formatter, "An Option") + write!(formatter, "an Option") } fn visit_none(self) -> Result @@ -372,7 +372,7 @@ pub mod as_str { type Value = Option; fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - write!(formatter, "An Option") + write!(formatter, "an Option") } fn visit_none(self) -> Result