From f583f675a1ec9e3167175d57c8329f8f00d3bf90 Mon Sep 17 00:00:00 2001 From: brentstone Date: Tue, 30 Aug 2022 20:29:47 +0300 Subject: [PATCH] replace floating point arithm from token module with rust_decimal --- proof_of_stake/src/parameters.rs | 5 ++++- shared/src/types/token.rs | 29 ++++++++++++----------------- tests/src/native_vp/pos.rs | 29 ++++++++++++----------------- 3 files changed, 28 insertions(+), 35 deletions(-) diff --git a/proof_of_stake/src/parameters.rs b/proof_of_stake/src/parameters.rs index 7ee0abdf986..1c265bf1a84 100644 --- a/proof_of_stake/src/parameters.rs +++ b/proof_of_stake/src/parameters.rs @@ -71,11 +71,14 @@ pub enum ValidationError { UnbondingLenTooShort(u64, u64), } +/// The number of fundamental units per whole token of the native staking token +pub const TOKENS_PER_NAM: u64 = 1_000_000; + /// From Tendermint: const MAX_TOTAL_VOTING_POWER: i64 = i64::MAX / 8; /// Assuming token amount is `u64` in micro units. -const TOKEN_MAX_AMOUNT: u64 = u64::MAX / 1_000_000; +const TOKEN_MAX_AMOUNT: u64 = u64::MAX / TOKENS_PER_NAM; impl PosParams { /// Validate PoS parameters values. Returns an empty list if the values are diff --git a/shared/src/types/token.rs b/shared/src/types/token.rs index 787a8855dc5..b19642b85af 100644 --- a/shared/src/types/token.rs +++ b/shared/src/types/token.rs @@ -6,6 +6,7 @@ use std::ops::{Add, AddAssign, Mul, Sub, SubAssign}; use std::str::FromStr; use borsh::{BorshDeserialize, BorshSchema, BorshSerialize}; +use rust_decimal::prelude::{Decimal, ToPrimitive}; use serde::{Deserialize, Serialize}; use thiserror::Error; @@ -37,7 +38,6 @@ pub struct Amount { pub const MAX_DECIMAL_PLACES: u32 = 6; /// Decimal scale of token [`Amount`] and [`Change`]. pub const SCALE: u64 = 1_000_000; -const SCALE_F64: f64 = SCALE as f64; /// A change in tokens amount pub type Change = i128; @@ -109,21 +109,16 @@ impl<'de> serde::Deserialize<'de> for Amount { } } -impl From for f64 { - /// Warning: `f64` loses precision and it should not be used when exact - /// values are required. +impl From for Decimal { fn from(amount: Amount) -> Self { - amount.micro as f64 / SCALE_F64 + Into::::into(amount.micro) / Into::::into(SCALE) } } -impl From for Amount { - /// Warning: `f64` loses precision and it should not be used when exact - /// values are required. - fn from(micro: f64) -> Self { - Self { - micro: (micro * SCALE_F64).round() as u64, - } +impl From for Amount { + fn from(micro: Decimal) -> Self { + let res = (micro * Into::::into(SCALE)).to_u64().unwrap(); + Self { micro: res } } } @@ -205,7 +200,7 @@ impl FromStr for Amount { match rust_decimal::Decimal::from_str(s) { Ok(decimal) => { let scale = decimal.scale(); - if scale > 6 { + if scale > MAX_DECIMAL_PLACES { return Err(AmountParseError::ScaleTooLarge(scale)); } let whole = @@ -440,11 +435,11 @@ mod tests { /// The upper limit is set to `2^51`, because then the float is /// starting to lose precision. #[test] - fn test_token_amount_f64_conversion(raw_amount in 0..2_u64.pow(51)) { + fn test_token_amount_decimal_conversion(raw_amount in 0..2_u64.pow(51)) { let amount = Amount::from(raw_amount); - // A round-trip conversion to and from f64 should be an identity - let float = f64::from(amount); - let identity = Amount::from(float); + // A round-trip conversion to and from Decimal should be an identity + let decimal = Decimal::from(amount); + let identity = Amount::from(decimal); assert_eq!(amount, identity); } } diff --git a/tests/src/native_vp/pos.rs b/tests/src/native_vp/pos.rs index ec6f75a15f0..4145f43a915 100644 --- a/tests/src/native_vp/pos.rs +++ b/tests/src/native_vp/pos.rs @@ -937,12 +937,11 @@ pub mod testing { let total_delta = validator_total_deltas .get_at_offset(current_epoch, offset, params) .unwrap_or_default(); - // We convert the tokens from micro units to whole tokens - // with division by 10^6 - let vp_before = - params.votes_per_token * ((total_delta) / 1_000_000); - let vp_after = params.votes_per_token - * ((total_delta + token_delta) / 1_000_000); + // Votes_per_token is relative to micro units so no need to + // convert + let vp_before = params.votes_per_token * total_delta; + let vp_after = + params.votes_per_token * (total_delta + token_delta); // voting power delta let vp_delta = vp_after - vp_before; @@ -1001,12 +1000,9 @@ pub mod testing { let total_delta = validator_total_deltas .get(epoch) .unwrap_or_default(); - // We convert the tokens from micro units to whole - // tokens with division by 10^6 - let vp_before = params.votes_per_token - * ((total_delta) / 1_000_000); - let vp_after = params.votes_per_token - * ((total_delta + token_delta) / 1_000_000); + // Votes_per_token is relative to micro units so no need to convert + let vp_before = params.votes_per_token * total_delta; + let vp_after = params.votes_per_token * (total_delta + token_delta); // voting power delta let vp_delta_at_unbonding = vp_after - vp_before - vp_delta - total_vp_delta; @@ -1077,12 +1073,11 @@ pub mod testing { let total_delta_cur = validator_total_deltas_cur .get_at_offset(current_epoch, offset, params) .unwrap_or_default(); - // We convert the tokens from micro units to whole tokens - // with division by 10^6 - let vp_before = params.votes_per_token - * ((total_delta_cur) / 1_000_000); + // Votes_per_token is relative to micro units so no need to + // convert + let vp_before = params.votes_per_token * total_delta_cur; let vp_after = params.votes_per_token - * ((total_delta_cur + token_delta) / 1_000_000); + * (total_delta_cur + token_delta); // voting power delta let vp_delta = vp_after - vp_before;