Refactor: change sat/vB to sat/kvB

CHANGELOG.md

@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
 
 - Fee strategy `range` support is now under the new crate feature `fee_range` and disable by default ([#314](https://github.com/farcaster-project/farcaster-core/pull/314))
+- Change Bitcoin fee unit from `sat/vB` to `sat/kvB` ([#315](https://github.com/farcaster-project/farcaster-core/pull/315))
 
 ### Fixed
 

src/bitcoin/fee.rs

@@ -14,21 +14,21 @@
 // License along with this library; if not, write to the Free Software
 // Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA
 
-//! Transaction fee unit type and implementation. Defines the [`SatPerVByte`] unit used in methods
+//! Transaction fee unit type and implementation. Defines the [`SatPerKvB`] unit used in methods
 //! that set the fee and check the fee on transactions given a [`FeeStrategy`] and a
 //! [`FeePriority`].
 //!
 //! ```rust
-//! use farcaster_core::bitcoin::fee::SatPerVByte;
+//! use farcaster_core::bitcoin::fee::SatPerKvB;
 //!
 //!# fn main() -> Result<(), farcaster_core::consensus::Error> {
 //! // Parse a Bitcoin amount suffixed with '/vByte'
-//! let rate = "100 satoshi/vByte".parse::<SatPerVByte>()?;
+//! let rate = "100 satoshi/kvB".parse::<SatPerKvB>()?;
 //! // ...also work with any other valid Bitcoin denomination
-//! let rate = "0.000001 BTC/vByte".parse::<SatPerVByte>()?;
+//! let rate = "0.000001 BTC/kvB".parse::<SatPerKvB>()?;
 //!
 //! // Always displayed as 'statoshi/vByte'
-//! assert_eq!("100 satoshi/vByte", format!("{}", rate));
+//! assert_eq!("100 satoshi/kvB", format!("{}", rate));
 //!# Ok(())
 //!# }
 //! ```
@@ -47,25 +47,30 @@ use std::str::FromStr;
 use serde::ser::{Serialize, Serializer};
 use serde::{de, Deserialize, Deserializer};
 
+/// The unit used to mesure a quantity, or weight, for a Bitcoin transaction. This represent a
+/// 1'000 of virtual Bytes.
+pub const WEIGHT_UNIT: &str = "kvB";
+
 /// An amount of Bitcoin (internally in satoshis) representing the number of satoshis per virtual
 /// byte a transaction must use for its fee. A [`FeeStrategy`] can use one of more of this type
 /// depending of its complexity (fixed, range, etc).
 #[derive(Debug, Clone, Copy, PartialOrd, PartialEq, Hash, Eq, Display)]
 #[display(display_sats_per_vbyte)]
-pub struct SatPerVByte(Amount);
+pub struct SatPerKvB(Amount);
 
-fn display_sats_per_vbyte(rate: &SatPerVByte) -> String {
+fn display_sats_per_vbyte(rate: &SatPerKvB) -> String {
     format!(
-        "{}/vByte",
+        "{}/{}",
         rate.as_native_unit()
-            .to_string_with_denomination(Denomination::Satoshi)
+            .to_string_with_denomination(Denomination::Satoshi),
+        WEIGHT_UNIT
     )
 }
 
-impl SatPerVByte {
+impl SatPerKvB {
     /// Create a fee quantity per virtual byte of given satoshis.
     pub fn from_sat(satoshis: u64) -> Self {
-        SatPerVByte(Amount::from_sat(satoshis))
+        SatPerKvB(Amount::from_sat(satoshis))
     }
 
     /// Return the number of satoshis per virtual byte to use for calculating the fee.
@@ -75,7 +80,7 @@ impl SatPerVByte {
 
     /// Create a fee quantity per virtual byte of given `bitcoin` crate amount.
     pub fn from_native_unit(amount: Amount) -> Self {
-        SatPerVByte(amount)
+        SatPerKvB(amount)
     }
 
     /// Return the number of bitcoins per virtual byte to use for calculating the fee as the native
@@ -85,7 +90,7 @@ impl SatPerVByte {
     }
 }
 
-impl Serialize for SatPerVByte {
+impl Serialize for SatPerKvB {
     fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
     where
         S: Serializer,
@@ -94,16 +99,16 @@ impl Serialize for SatPerVByte {
     }
 }
 
-impl<'de> Deserialize<'de> for SatPerVByte {
-    fn deserialize<D>(deserializer: D) -> Result<SatPerVByte, D::Error>
+impl<'de> Deserialize<'de> for SatPerKvB {
+    fn deserialize<D>(deserializer: D) -> Result<SatPerKvB, D::Error>
     where
         D: Deserializer<'de>,
     {
-        SatPerVByte::from_str(&String::deserialize(deserializer)?).map_err(de::Error::custom)
+        SatPerKvB::from_str(&String::deserialize(deserializer)?).map_err(de::Error::custom)
     }
 }
 
-impl CanonicalBytes for SatPerVByte {
+impl CanonicalBytes for SatPerKvB {
     fn as_canonical_bytes(&self) -> Vec<u8> {
         bitcoin::consensus::encode::serialize(&self.0.as_sat())
     }
@@ -112,26 +117,26 @@ impl CanonicalBytes for SatPerVByte {
     where
         Self: Sized,
     {
-        Ok(SatPerVByte(Amount::from_sat(
+        Ok(SatPerKvB(Amount::from_sat(
             bitcoin::consensus::encode::deserialize(bytes).map_err(consensus::Error::new)?,
         )))
     }
 }
 
-impl FromStr for SatPerVByte {
+impl FromStr for SatPerKvB {
     type Err = consensus::Error;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         let parts = s.split('/').collect::<Vec<&str>>();
         if parts.len() != 2 {
             return Err(consensus::Error::ParseFailed(
-                "SatPerVByte format is not respected",
+                "sat/kvB format is not respected",
             ));
         }
         let amount = parts[0].parse::<Amount>().map_err(consensus::Error::new)?;
         match parts[1] {
-            "vByte" => Ok(Self(amount)),
-            _ => Err(consensus::Error::ParseFailed("SatPerVByte parse failed")),
+            WEIGHT_UNIT => Ok(Self(amount)),
+            _ => Err(consensus::Error::ParseFailed("Weight unit parse failed")),
         }
     }
 }
@@ -154,15 +159,15 @@ fn get_available_input_sat(tx: &PartiallySignedTransaction) -> Result<Amount, Fe
 }
 
 impl Fee for PartiallySignedTransaction {
-    type FeeUnit = SatPerVByte;
+    type FeeUnit = SatPerKvB;
 
     type Amount = Amount;
 
     /// Calculates and sets the fees on the given transaction and return the fees set
     #[allow(unused_variables)]
     fn set_fee(
         &mut self,
-        strategy: &FeeStrategy<SatPerVByte>,
+        strategy: &FeeStrategy<SatPerKvB>,
         politic: FeePriority,
     ) -> Result<Self::Amount, FeeStrategyError> {
         if self.unsigned_tx.output.len() != 1 {
@@ -181,18 +186,21 @@ impl Fee for PartiallySignedTransaction {
         // times four. For transactions with a witness, this is the non-witness
         // consensus-serialized size multiplied by three plus the with-witness consensus-serialized
         // size.
-        let weight = self.unsigned_tx.weight() as u64;
+        let weight = self.unsigned_tx.weight() as f64;
 
         // Compute the fee amount to set in total
-        let fee_amount = match strategy {
-            FeeStrategy::Fixed(sat_per_vbyte) => sat_per_vbyte.as_native_unit().checked_mul(weight),
+        let fee_rate = match strategy {
+            FeeStrategy::Fixed(sat_per_kvb) => sat_per_kvb
+                .as_native_unit()
+                .to_float_in(Denomination::Satoshi),
             #[cfg(feature = "fee_range")]
             FeeStrategy::Range { min_inc, max_inc } => match politic {
-                FeePriority::Low => min_inc.as_native_unit().checked_mul(weight),
-                FeePriority::High => max_inc.as_native_unit().checked_mul(weight),
+                FeePriority::Low => min_inc.as_native_unit().to_float_in(Denomination::Satoshi),
+                FeePriority::High => max_inc.as_native_unit().to_float_in(Denomination::Satoshi),
             },
-        }
-        .ok_or(FeeStrategyError::AmountOfFeeTooHigh)?;
+        };
+        let fee_amount = fee_rate / 1000f64 * weight;
+        let fee_amount = Amount::from_sat(fee_amount.round() as u64);
 
         // Apply the fee on the first output
         self.unsigned_tx.output[0].value = input_sum
@@ -205,7 +213,7 @@ impl Fee for PartiallySignedTransaction {
     }
 
     /// Validates that the fees for the given transaction are set accordingly to the strategy
-    fn validate_fee(&self, strategy: &FeeStrategy<SatPerVByte>) -> Result<bool, FeeStrategyError> {
+    fn validate_fee(&self, strategy: &FeeStrategy<SatPerKvB>) -> Result<bool, FeeStrategyError> {
         if self.unsigned_tx.output.len() != 1 {
             return Err(FeeStrategyError::new(
                 transaction::Error::MultiUTXOUnsuported,
@@ -219,13 +227,13 @@ impl Fee for PartiallySignedTransaction {
             .ok_or(FeeStrategyError::AmountOfFeeTooHigh)?;
         let weight = self.unsigned_tx.weight() as u64;
 
-        let effective_sat_per_vbyte = SatPerVByte::from_sat(
-            weight
+        let effective_sat_per_kvb = SatPerKvB::from_sat(
+            (weight * 1000)
                 .checked_div(fee)
                 .ok_or(FeeStrategyError::AmountOfFeeTooLow)?,
         );
 
-        Ok(strategy.check(&effective_sat_per_vbyte))
+        Ok(strategy.check(&effective_sat_per_kvb))
     }
 }
 
@@ -235,52 +243,52 @@ mod tests {
 
     #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
     struct SerdeTest {
-        fee: SatPerVByte,
+        fee: SatPerKvB,
     }
 
     #[test]
     fn parse_sats_per_vbyte() {
         for s in [
-            "0.0001 BTC/vByte",
-            "100 satoshi/vByte",
-            "100 satoshis/vByte",
-            "10 satoshi/vByte",
-            "1 satoshi/vByte",
+            "0.0001 BTC/kvB",
+            "100 satoshi/kvB",
+            "100 satoshis/kvB",
+            "10 satoshi/kvB",
+            "1 satoshi/kvB",
         ]
         .iter()
         {
-            let parse = SatPerVByte::from_str(s);
+            let parse = SatPerKvB::from_str(s);
             assert!(parse.is_ok());
         }
         // MUST fail
-        for s in ["1 satoshi", "100 vByte"].iter() {
-            let parse = SatPerVByte::from_str(s);
+        for s in ["1 satoshi", "100 kvB"].iter() {
+            let parse = SatPerKvB::from_str(s);
             assert!(parse.is_err());
         }
     }
 
     #[test]
     fn display_sats_per_vbyte() {
-        let fee_rate = SatPerVByte::from_sat(100);
-        assert_eq!(format!("{}", fee_rate), "100 satoshi/vByte".to_string());
+        let fee_rate = SatPerKvB::from_sat(100);
+        assert_eq!(format!("{}", fee_rate), "100 satoshi/kvB".to_string());
     }
 
     #[test]
     fn serialize_fee_rate_in_yaml() {
         let fee_rate = SerdeTest {
-            fee: SatPerVByte::from_sat(10),
+            fee: SatPerKvB::from_sat(10),
         };
         let s = serde_yaml::to_string(&fee_rate).expect("Encode fee rate in yaml");
-        assert_eq!("---\nfee: 10 satoshi/vByte\n", s);
+        assert_eq!("---\nfee: 10 satoshi/kvB\n", s);
     }
 
     #[test]
     fn deserialize_fee_rate_in_yaml() {
-        let s = "---\nfee: 10 satoshi/vByte\n";
+        let s = "---\nfee: 10 satoshi/kvB\n";
         let fee_rate = serde_yaml::from_str(&s).expect("Decode fee rate from yaml");
         assert_eq!(
             SerdeTest {
-                fee: SatPerVByte::from_sat(10)
+                fee: SatPerKvB::from_sat(10)
             },
             fee_rate
         );

src/blockchain.rs

@@ -522,7 +522,7 @@ impl_strict_encoding!(Network);
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::bitcoin::fee::SatPerVByte;
+    use crate::bitcoin::fee::SatPerKvB;
 
     #[test]
     fn parse_fee_politic() {
@@ -590,38 +590,35 @@ mod tests {
 
     #[test]
     fn fee_strategy_display() {
-        let strategy = FeeStrategy::Fixed(SatPerVByte::from_sat(100));
-        assert_eq!(&format!("{}", strategy), "100 satoshi/vByte");
+        let strategy = FeeStrategy::Fixed(SatPerKvB::from_sat(100));
+        assert_eq!(&format!("{}", strategy), "100 satoshi/kvB");
         #[cfg(feature = "fee_range")]
         {
             let strategy = FeeStrategy::Range {
-                min_inc: SatPerVByte::from_sat(50),
-                max_inc: SatPerVByte::from_sat(150),
+                min_inc: SatPerKvB::from_sat(50),
+                max_inc: SatPerKvB::from_sat(150),
             };
-            assert_eq!(
-                &format!("{}", strategy),
-                "50 satoshi/vByte-150 satoshi/vByte"
-            )
+            assert_eq!(&format!("{}", strategy), "50 satoshi/kvB-150 satoshi/kvB")
         }
     }
 
     #[test]
     fn fee_strategy_parse() {
         let strings = [
-            "100 satoshi/vByte",
+            "100 satoshi/kvB",
             #[cfg(feature = "fee_range")]
-            "50 satoshi/vByte-150 satoshi/vByte",
+            "50 satoshi/kvB-150 satoshi/kvB",
         ];
         let res = [
-            FeeStrategy::Fixed(SatPerVByte::from_sat(100)),
+            FeeStrategy::Fixed(SatPerKvB::from_sat(100)),
             #[cfg(feature = "fee_range")]
             FeeStrategy::Range {
-                min_inc: SatPerVByte::from_sat(50),
-                max_inc: SatPerVByte::from_sat(150),
+                min_inc: SatPerKvB::from_sat(50),
+                max_inc: SatPerKvB::from_sat(150),
             },
         ];
         for (s, r) in strings.iter().zip(res) {
-            let strategy = FeeStrategy::<SatPerVByte>::from_str(s);
+            let strategy = FeeStrategy::<SatPerKvB>::from_str(s);
             assert!(strategy.is_ok());
             assert_eq!(strategy.unwrap(), r);
         }
@@ -630,16 +627,16 @@ mod tests {
     #[test]
     fn fee_strategy_to_str_from_str() {
         let strats = [
-            FeeStrategy::Fixed(SatPerVByte::from_sat(1)),
+            FeeStrategy::Fixed(SatPerKvB::from_sat(1)),
             #[cfg(feature = "fee_range")]
             FeeStrategy::Range {
-                min_inc: SatPerVByte::from_sat(1),
-                max_inc: SatPerVByte::from_sat(7),
+                min_inc: SatPerKvB::from_sat(1),
+                max_inc: SatPerKvB::from_sat(7),
             },
         ];
         for strat in strats.iter() {
             assert_eq!(
-                FeeStrategy::<SatPerVByte>::from_str(&strat.to_string()).unwrap(),
+                FeeStrategy::<SatPerKvB>::from_str(&strat.to_string()).unwrap(),
                 *strat
             )
         }
@@ -649,12 +646,12 @@ mod tests {
     #[cfg(feature = "fee_range")]
     fn fee_strategy_check_range() {
         let strategy = FeeStrategy::Range {
-            min_inc: SatPerVByte::from_sat(50),
-            max_inc: SatPerVByte::from_sat(150),
+            min_inc: SatPerKvB::from_sat(50),
+            max_inc: SatPerKvB::from_sat(150),
         };
-        assert!(!strategy.check(&SatPerVByte::from_sat(49)));
-        assert!(strategy.check(&SatPerVByte::from_sat(50)));
-        assert!(strategy.check(&SatPerVByte::from_sat(150)));
-        assert!(!strategy.check(&SatPerVByte::from_sat(151)));
+        assert!(!strategy.check(&SatPerKvB::from_sat(49)));
+        assert!(strategy.check(&SatPerKvB::from_sat(50)));
+        assert!(strategy.check(&SatPerKvB::from_sat(150)));
+        assert!(!strategy.check(&SatPerKvB::from_sat(151)));
     }
 }