Skip to content

Commit

Permalink
replace floating point arithm from token module with rust_decimal
Browse files Browse the repository at this point in the history
  • Loading branch information
brentstone authored and tzemanovic committed Oct 25, 2022
1 parent 836a6de commit f583f67
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 35 deletions.
5 changes: 4 additions & 1 deletion proof_of_stake/src/parameters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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: <https://github.com/tendermint/tendermint/blob/master/spec/abci/apps.md#updating-the-validator-set>
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
Expand Down
29 changes: 12 additions & 17 deletions shared/src/types/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -109,21 +109,16 @@ impl<'de> serde::Deserialize<'de> for Amount {
}
}

impl From<Amount> for f64 {
/// Warning: `f64` loses precision and it should not be used when exact
/// values are required.
impl From<Amount> for Decimal {
fn from(amount: Amount) -> Self {
amount.micro as f64 / SCALE_F64
Into::<Decimal>::into(amount.micro) / Into::<Decimal>::into(SCALE)
}
}

impl From<f64> 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<Decimal> for Amount {
fn from(micro: Decimal) -> Self {
let res = (micro * Into::<Decimal>::into(SCALE)).to_u64().unwrap();
Self { micro: res }
}
}

Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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);
}
}
Expand Down
29 changes: 12 additions & 17 deletions tests/src/native_vp/pos.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down

0 comments on commit f583f67

Please sign in to comment.