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 committed Oct 31, 2022
1 parent 6cbce4a commit c37ecf4
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 25 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
18 changes: 11 additions & 7 deletions tests/src/native_vp/pos.rs
Original file line number Diff line number Diff line change
Expand Up @@ -594,6 +594,10 @@ pub mod testing {

use crate::tx::{self, tx_host_env};

const TOKENS_PER_NAM: i128 =
namada::ledger::pos::namada_proof_of_stake::parameters::TOKENS_PER_NAM
as i128;

#[derive(Clone, Debug, Default)]
pub struct TestValidator {
pub address: Option<Address>,
Expand Down Expand Up @@ -940,9 +944,9 @@ pub mod testing {
// 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);
params.votes_per_token * (total_delta / TOKENS_PER_NAM);
let vp_after = params.votes_per_token
* ((total_delta + token_delta) / 1_000_000);
* ((total_delta + token_delta) / TOKENS_PER_NAM);
// voting power delta
let vp_delta = vp_after - vp_before;

Expand Down Expand Up @@ -1001,12 +1005,12 @@ pub mod testing {
let total_delta = validator_total_deltas
.get(epoch)
.unwrap_or_default();
// We convert the tokens from micro units to whole
// 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);
* (total_delta / TOKENS_PER_NAM);
let vp_after = params.votes_per_token
* ((total_delta + token_delta) / 1_000_000);
* ((total_delta + token_delta) / TOKENS_PER_NAM);
// voting power delta
let vp_delta_at_unbonding =
vp_after - vp_before - vp_delta - total_vp_delta;
Expand Down Expand Up @@ -1080,9 +1084,9 @@ pub mod testing {
// 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);
* (total_delta_cur / TOKENS_PER_NAM);
let vp_after = params.votes_per_token
* ((total_delta_cur + token_delta) / 1_000_000);
* ((total_delta_cur + token_delta) / TOKENS_PER_NAM);
// voting power delta
let vp_delta = vp_after - vp_before;

Expand Down

0 comments on commit c37ecf4

Please sign in to comment.