Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use the unsigned fixed type for balances: U64F64 #353

Merged
merged 12 commits into from
Sep 22, 2023
2 changes: 1 addition & 1 deletion balances/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ impl<T: Config> Pallet<T> {

/// Returns the community-specific demurrage if it is set. Otherwise returns the
/// the demurrage defined in the genesis config
fn demurrage(cid: &CommunityIdentifier) -> BalanceType {
fn demurrage(cid: &CommunityIdentifier) -> Demurrage {
<DemurragePerBlock<T>>::try_get(cid).unwrap_or_else(|_| T::DefaultDemurrage::get())
}

Expand Down
14 changes: 4 additions & 10 deletions communities/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,11 @@
use codec::Encode;
use core::marker::PhantomData;
use encointer_primitives::{
balances::{BalanceEntry, BalanceType, Demurrage},
balances::{BalanceEntry, Demurrage},
common::PalletString,
communities::{
consts::*, validate_nominal_income, CommunityIdentifier,
CommunityMetadata as CommunityMetadataType, Degree, GeoHash, Location, LossyFrom,
NominalIncome as NominalIncomeType,
consts::*, CommunityIdentifier, CommunityMetadata as CommunityMetadataType, Degree,
GeoHash, Location, LossyFrom, NominalIncome as NominalIncomeType,
},
fixed::transcendental::{asin, cos, powi, sin, sqrt},
scheduler::CeremonyPhaseType,
Expand Down Expand Up @@ -106,9 +105,6 @@ pub mod pallet {
community_metadata
.validate()
.map_err(|_| <Error<T>>::InvalidCommunityMetadata)?;
if let Some(i) = nominal_income {
validate_nominal_income(&i).map_err(|_| <Error<T>>::InvalidNominalIncome)?;
}
Comment on lines -109 to -111
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Validate only validated if the value is negative; type safety gives us this guarantee now already.


let cid = CommunityIdentifier::new(location, bootstrappers.clone())
.map_err(|_| Error::<T>::InvalidLocation)?;
Expand Down Expand Up @@ -275,7 +271,7 @@ pub mod pallet {
pub fn update_demurrage(
origin: OriginFor<T>,
cid: CommunityIdentifier,
demurrage: BalanceType,
demurrage: Demurrage,
) -> DispatchResultWithPostInfo {
T::CommunityMaster::ensure_origin(origin)?;

Expand Down Expand Up @@ -481,8 +477,6 @@ impl<T: Config> Pallet<T> {
nominal_income: NominalIncomeType,
) -> DispatchResultWithPostInfo {
Self::ensure_cid_exists(&cid)?;
validate_nominal_income(&nominal_income).map_err(|_| <Error<T>>::InvalidNominalIncome)?;
Self::ensure_cid_exists(&cid)?;

<NominalIncome<T>>::insert(cid, nominal_income);

Expand Down
33 changes: 18 additions & 15 deletions primitives/src/balances.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ const LOG: &str = "encointer::demurrage";
// We're working with fixpoint here.

/// Encointer balances are fixpoint values
pub type BalanceType = I64F64;
pub type BalanceType = U64F64;

/// Demurrage is the rate of evanescence of balances per block
/// it must be positive
Expand Down Expand Up @@ -137,11 +137,14 @@ pub fn demurrage_factor(demurrage_per_block: Demurrage, elapsed_blocks: u32) ->
// demurrage >= 0; hence exponent <= 0
let exponent = match (-demurrage_per_block.abs()).checked_mul(elapsed_blocks.into()) {
Some(exp) => exp,
None => return 0.into(),
None => return 0u64.into(),
};

// exponent <= 0; hence return value [0, 1)
exp(exponent).unwrap_or_else(|_| 0.into())
let f: I64F64 = exp(exponent).unwrap_or_else(|_| 0.into());

// Safe conversion. The result of an exponential function can't be negative.
BalanceType::from_le_bytes(f.to_le_bytes())
}

/// Our BalanceType is I64F64, so the smallest possible number is
Expand All @@ -163,8 +166,8 @@ impl Convert<BalanceType, u128> for EncointerBalanceConverter {

result += (bits >> 64) as u128 * ONE_ENCOINTER_BALANCE_UNIT;

result += BalanceType::from_bits((bits as u64) as i128) // <- to truncate
.saturating_mul_int(ONE_ENCOINTER_BALANCE_UNIT as i128)
result += BalanceType::from_bits((bits as u64) as u128) // <- to truncate
.saturating_mul_int(ONE_ENCOINTER_BALANCE_UNIT)
.to_num::<u128>();
result
}
Expand Down Expand Up @@ -215,26 +218,26 @@ mod tests {

#[test]
fn demurrage_works() {
let bal = BalanceEntry::<u32>::new(1.into(), 0);
let bal = BalanceEntry::<u32>::new(1u32.into(), 0);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implicit type is i32, which can obviously not be into-ed.

assert_abs_diff_eq(bal.apply_demurrage(DEFAULT_DEMURRAGE, ONE_YEAR).principal, 0.5);
}

#[test]
fn apply_demurrage_when_principal_is_zero_works() {
let bal = BalanceEntry::<u32>::new(0.into(), 0);
let bal = BalanceEntry::<u32>::new(0u32.into(), 0);
assert_abs_diff_eq(bal.apply_demurrage(DEFAULT_DEMURRAGE, ONE_YEAR).principal, 0f64);
}

#[test]
fn apply_demurrage_when_demurrage_is_negative_works() {
let bal = BalanceEntry::<u32>::new(1.into(), 0);
let bal = BalanceEntry::<u32>::new(1u32.into(), 0);
assert_abs_diff_eq(bal.apply_demurrage(-DEFAULT_DEMURRAGE, ONE_YEAR).principal, 0.5);
}

#[test]
fn apply_demurrage_with_overflowing_values_works() {
let demurrage = Demurrage::from_num(0.000048135220872218395);
let bal = BalanceEntry::<u32>::new(1.into(), 0);
let bal = BalanceEntry::<u32>::new(1u32.into(), 0);

// This produced a overflow before: https://github.com/encointer/encointer-node/issues/290
assert_abs_diff_eq(bal.apply_demurrage(demurrage, ONE_YEAR).principal, 0f64);
Expand All @@ -249,31 +252,31 @@ mod tests {
#[test]
fn apply_demurrage_with_block_number_bigger_than_u32max_does_not_overflow() {
let demurrage = Demurrage::from_num(DEFAULT_DEMURRAGE);
let bal = BalanceEntry::<u64>::new(1.into(), 0);
let bal = BalanceEntry::<u64>::new(1u32.into(), 0);

assert_abs_diff_eq(bal.apply_demurrage(demurrage, u32::MAX as u64 + 1).principal, 0f64);
}

#[test]
fn apply_demurrage_with_block_number_not_monotonically_rising_just_updates_last_block() {
let demurrage = Demurrage::from_num(DEFAULT_DEMURRAGE);
let bal = BalanceEntry::<u32>::new(1.into(), 1);
let bal = BalanceEntry::<u32>::new(1u32.into(), 1);

assert_eq!(bal.apply_demurrage(demurrage, 0), BalanceEntry::<u32>::new(1.into(), 0))
assert_eq!(bal.apply_demurrage(demurrage, 0), BalanceEntry::<u32>::new(1u32.into(), 0))
}

#[test]
fn apply_demurrage_with_zero_demurrage_works() {
let demurrage = Demurrage::from_num(0.0);
let bal = BalanceEntry::<u32>::new(1.into(), 0);
let bal = BalanceEntry::<u32>::new(1u32.into(), 0);

assert_abs_diff_eq(bal.apply_demurrage(demurrage, ONE_YEAR).principal, 1f64);
}

#[test]
fn apply_demurrage_with_zero_elapsed_blocks_works() {
let demurrage = Demurrage::from_num(DEFAULT_DEMURRAGE);
let bal = BalanceEntry::<u32>::new(1.into(), 0);
let bal = BalanceEntry::<u32>::new(1u32.into(), 0);

assert_abs_diff_eq(bal.apply_demurrage(demurrage, 0).principal, 1f64);
}
Expand All @@ -285,7 +288,7 @@ mod tests {
//
// Not critical as we safeguard against this with `validate_demurrage`.
let demurrage = Demurrage::from_num(i64::MAX - 1);
let bal = BalanceEntry::<u32>::new(1.into(), 0);
let bal = BalanceEntry::<u32>::new(1u32.into(), 0);

assert_abs_diff_eq(bal.apply_demurrage(demurrage, 1).principal, 0f64);
}
Expand Down
25 changes: 4 additions & 21 deletions primitives/src/communities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use serde::{Deserialize, Serialize};
use ep_core::serde::{serialize_array, serialize_fixed};

use crate::{
balances::Demurrage,
balances::{BalanceType, Demurrage},
common::{
validate_ascii, validate_ipfs_cid, AsByteOrNoop, BoundedIpfsCid, IpfsValidationError,
PalletString,
Expand All @@ -47,7 +47,7 @@ pub type GeoHash = GeohashGeneric<GEO_HASH_BUCKET_RESOLUTION>;
pub type CommunityIndexType = u32;
pub type LocationIndexType = u32;
pub type Degree = I64F64;
pub type NominalIncome = I64F64;
pub type NominalIncome = BalanceType;
pub type MinSolarTripTimeType = u32;
pub type MaxSpeedMpsType = u32;

Expand Down Expand Up @@ -91,14 +91,6 @@ pub fn validate_demurrage(demurrage: &Demurrage) -> Result<(), RangeError> {
Ok(())
}

/// Ensure that the nominal is in a sane range.
pub fn validate_nominal_income(nominal_income: &NominalIncome) -> Result<(), RangeError> {
if nominal_income <= &NominalIncome::from_num(0) {
return Err(RangeError::LessThanOrEqualZero)
}
Ok(())
}

#[derive(
Encode, Decode, Copy, Clone, PartialEq, Eq, Default, PartialOrd, Ord, TypeInfo, MaxEncodedLen,
)]
Expand Down Expand Up @@ -427,9 +419,8 @@ mod tests {
bs58_verify::Bs58Error,
common::{FromStr as CrateFromStr, IpfsValidationError},
communities::{
validate_demurrage, validate_nominal_income, CommunityIdentifier, CommunityMetadata,
CommunityMetadataError, Degree, Demurrage, Location, NominalIncome, PalletString,
RangeError,
validate_demurrage, CommunityIdentifier, CommunityMetadata, CommunityMetadataError,
Degree, Demurrage, Location, PalletString, RangeError,
},
};
use sp_std::str::FromStr;
Expand All @@ -440,14 +431,6 @@ mod tests {
assert_eq!(validate_demurrage(&Demurrage::from_num(-1)), Err(RangeError::LessThanZero));
}

#[test]
fn nominal_income_smaller_0_fails() {
assert_eq!(
validate_nominal_income(&NominalIncome::from_num(-1)),
Err(RangeError::LessThanOrEqualZero)
);
}

#[test]
fn validate_metadata_works() {
assert_eq!(CommunityMetadata::default().validate(), Ok(()));
Expand Down
2 changes: 1 addition & 1 deletion test-utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ macro_rules! impl_balances {
parameter_types! {
pub const DefaultDemurrage: Demurrage = Demurrage::from_bits(0x0000000000000000000001E3F0A8A973_i128);
/// 0.000005
pub const EncointerBalancesExistentialDeposit: BalanceType = BalanceType::from_bits(0x0000000000000000000053e2d6238da4_i128);
pub const EncointerBalancesExistentialDeposit: BalanceType = BalanceType::from_bits(0x0000000000000000000053e2d6238da4_u128);
}

#[macro_export]
Expand Down