diff --git a/balances/src/lib.rs b/balances/src/lib.rs index 63b1d109..a83b9ed5 100644 --- a/balances/src/lib.rs +++ b/balances/src/lib.rs @@ -364,7 +364,7 @@ impl Pallet { /// 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 { >::try_get(cid).unwrap_or_else(|_| T::DefaultDemurrage::get()) } diff --git a/balances/src/mock.rs b/balances/src/mock.rs index 6d567072..bff9043e 100644 --- a/balances/src/mock.rs +++ b/balances/src/mock.rs @@ -26,7 +26,7 @@ type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic(-DefaultDemurrage::get()).unwrap() + to_U64F64(exp::(-DefaultDemurrage::get()).unwrap()).unwrap() ); //one year later System::set_block_number(86400 / 5 * 356); @@ -309,7 +310,7 @@ fn transfer_all_works() { let balance: f64 = EncointerBalances::balance(cid, &bob).lossy_into(); let demurrage_factor: f64 = - exp::(Demurrage::from_num(3.0) * -DefaultDemurrage::get()) + exp::(Demurrage::from_num(3.0) * -DefaultDemurrage::get()) .unwrap() .lossy_into(); assert_relative_eq!(balance, 50.0 * demurrage_factor, epsilon = 1.0e-9); @@ -493,67 +494,43 @@ mod impl_fungibles { let ferdie = AccountKeyring::Ferdie.to_account_id(); assert_ok!(EncointerBalances::issue(cid, &alice, BalanceType::from_num(50))); - assert!( - EncointerBalances::can_deposit(wrong_cid, &alice, 10, Provenance::Minted) == - DepositConsequence::UnknownAsset + assert_eq!( + EncointerBalances::can_deposit(wrong_cid, &alice, 10, Provenance::Minted), + DepositConsequence::UnknownAsset ); assert_ok!(EncointerBalances::issue( cid, &alice, - BalanceType::from_num(4.5 * 10f64.powf(18f64)) - )); - assert_ok!(EncointerBalances::issue( - cid, - &bob, - BalanceType::from_num(4.5 * 10f64.powf(18f64)) - )); - - assert!( - EncointerBalances::can_deposit( - cid, - &ferdie, - fungible(BalanceType::from_num(4.5 * 10f64.powf(18f64))), - Provenance::Minted - ) == DepositConsequence::Overflow - ); - - // in the very weird case where some some balances are negative we need to test for overflow of - // and account balance, because now an account can overflow but the total issuance does not. - assert_ok!(EncointerBalances::burn( - cid, - &bob, - BalanceType::from_num(4.5 * 10f64.powf(18f64)) + // u64::Max is uneven, so subtract 1 to be explicit about the result. + BalanceType::from_num((u64::MAX - 1) / 2) )); assert_ok!(EncointerBalances::issue( cid, &bob, - BalanceType::from_num(-4.5 * 10f64.powf(18f64)) - )); - - assert_ok!(EncointerBalances::issue( - cid, - &alice, - BalanceType::from_num(4.5 * 10f64.powf(18f64)) + // Subtract the 50 we issued to Alice above. + BalanceType::from_num((u64::MAX - 1) / 2 - 50) )); - assert!( + assert_eq!( EncointerBalances::can_deposit( cid, - &alice, - fungible(BalanceType::from_num(4.5 * 10f64.powf(18f64))), - Provenance::Minted - ) == DepositConsequence::Overflow + &ferdie, + fungible(BalanceType::from_num(2)), + Provenance::Minted, + ), + DepositConsequence::Overflow ); - assert!( + assert_eq!( EncointerBalances::can_deposit( cid, &alice, fungible(BalanceType::from_num(1)), Provenance::Minted - ) == DepositConsequence::Success + ), + DepositConsequence::Success ); }) } @@ -568,29 +545,29 @@ mod impl_fungibles { assert_ok!(EncointerBalances::issue(cid, &alice, BalanceType::from_num(10))); assert_ok!(EncointerBalances::issue(cid, &bob, BalanceType::from_num(1))); - assert!( - EncointerBalances::can_withdraw(wrong_cid, &alice, 10) == - WithdrawConsequence::UnknownAsset + assert_eq!( + EncointerBalances::can_withdraw(wrong_cid, &alice, 10), + WithdrawConsequence::UnknownAsset ); - assert!( - EncointerBalances::can_withdraw(cid, &bob, fungible(BalanceType::from_num(12))) == - WithdrawConsequence::Underflow + assert_eq!( + EncointerBalances::can_withdraw(cid, &bob, fungible(BalanceType::from_num(12))), + WithdrawConsequence::Underflow ); - assert!( - EncointerBalances::can_withdraw(cid, &bob, fungible(BalanceType::from_num(0))) == - WithdrawConsequence::Success + assert_eq!( + EncointerBalances::can_withdraw(cid, &bob, fungible(BalanceType::from_num(0))), + WithdrawConsequence::Success ); - assert!( - EncointerBalances::can_withdraw(cid, &bob, fungible(BalanceType::from_num(2))) == - WithdrawConsequence::BalanceLow + assert_eq!( + EncointerBalances::can_withdraw(cid, &bob, fungible(BalanceType::from_num(2))), + WithdrawConsequence::BalanceLow ); - assert!( - EncointerBalances::can_withdraw(cid, &bob, fungible(BalanceType::from_num(1))) == - WithdrawConsequence::Success + assert_eq!( + EncointerBalances::can_withdraw(cid, &bob, fungible(BalanceType::from_num(1))), + WithdrawConsequence::Success ); }) } diff --git a/communities/src/lib.rs b/communities/src/lib.rs index e433fc6e..5185fc44 100644 --- a/communities/src/lib.rs +++ b/communities/src/lib.rs @@ -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, @@ -106,9 +105,6 @@ pub mod pallet { community_metadata .validate() .map_err(|_| >::InvalidCommunityMetadata)?; - if let Some(i) = nominal_income { - validate_nominal_income(&i).map_err(|_| >::InvalidNominalIncome)?; - } let cid = CommunityIdentifier::new(location, bootstrappers.clone()) .map_err(|_| Error::::InvalidLocation)?; @@ -275,7 +271,7 @@ pub mod pallet { pub fn update_demurrage( origin: OriginFor, cid: CommunityIdentifier, - demurrage: BalanceType, + demurrage: Demurrage, ) -> DispatchResultWithPostInfo { T::CommunityMaster::ensure_origin(origin)?; @@ -481,8 +477,6 @@ impl Pallet { nominal_income: NominalIncomeType, ) -> DispatchResultWithPostInfo { Self::ensure_cid_exists(&cid)?; - validate_nominal_income(&nominal_income).map_err(|_| >::InvalidNominalIncome)?; - Self::ensure_cid_exists(&cid)?; >::insert(cid, nominal_income); diff --git a/democracy/src/tests.rs b/democracy/src/tests.rs index ab503be6..d0df6760 100644 --- a/democracy/src/tests.rs +++ b/democracy/src/tests.rs @@ -89,7 +89,7 @@ fn proposal_submission_works() { let cid = create_cid(); let block = System::block_number(); let proposal_action = - ProposalAction::UpdateNominalIncome(cid, NominalIncomeType::from(100i32)); + ProposalAction::UpdateNominalIncome(cid, NominalIncomeType::from(100u32)); assert_ok!(EncointerDemocracy::submit_proposal( RuntimeOrigin::signed(alice()), @@ -109,7 +109,7 @@ fn proposal_submission_fails_if_proposal_in_enactment_queue() { new_test_ext().execute_with(|| { let cid = create_cid(); let proposal_action = - ProposalAction::UpdateNominalIncome(cid, NominalIncomeType::from(100i32)); + ProposalAction::UpdateNominalIncome(cid, NominalIncomeType::from(100u32)); EnactmentQueue::::insert(proposal_action.get_identifier(), 100); @@ -254,7 +254,7 @@ fn eligible_reputations_works_with_cids() { let alice = alice(); let proposal_action = - ProposalAction::UpdateNominalIncome(cid, NominalIncomeType::from(100i32)); + ProposalAction::UpdateNominalIncome(cid, NominalIncomeType::from(100u32)); assert_ok!(EncointerDemocracy::submit_proposal( RuntimeOrigin::signed(alice.clone()), proposal_action @@ -384,7 +384,7 @@ fn do_update_proposal_state_fails_with_wrong_state() { let proposal: Proposal = Proposal { start: BlockNumber::from(1u64), start_cindex: 1, - action: ProposalAction::UpdateNominalIncome(cid, NominalIncomeType::from(100i32)), + action: ProposalAction::UpdateNominalIncome(cid, NominalIncomeType::from(100u32)), state: ProposalState::Cancelled, }; Proposals::::insert(1, proposal); @@ -392,7 +392,7 @@ fn do_update_proposal_state_fails_with_wrong_state() { let proposal2: Proposal = Proposal { start: BlockNumber::from(1u64), start_cindex: 1, - action: ProposalAction::UpdateNominalIncome(cid, NominalIncomeType::from(100i32)), + action: ProposalAction::UpdateNominalIncome(cid, NominalIncomeType::from(100u32)), state: ProposalState::Approved, }; Proposals::::insert(2, proposal2); @@ -570,7 +570,7 @@ fn test_get_electorate_works() { )); let proposal_action = - ProposalAction::UpdateNominalIncome(cid, NominalIncomeType::from(100i32)); + ProposalAction::UpdateNominalIncome(cid, NominalIncomeType::from(100u32)); assert_ok!(EncointerDemocracy::submit_proposal( RuntimeOrigin::signed(alice.clone()), proposal_action @@ -638,7 +638,7 @@ fn enactment_updates_proposal_metadata_and_enactment_queue() { )); let proposal_action2 = - ProposalAction::UpdateNominalIncome(cid, NominalIncomeType::from(100i32)); + ProposalAction::UpdateNominalIncome(cid, NominalIncomeType::from(100u32)); assert_ok!(EncointerDemocracy::submit_proposal( RuntimeOrigin::signed(alice.clone()), proposal_action2 @@ -668,7 +668,7 @@ fn proposal_happy_flow() { let cid2 = register_test_community::(None, 10.0, 10.0); let alice = alice(); let proposal_action = - ProposalAction::UpdateNominalIncome(cid, NominalIncomeType::from(13037i32)); + ProposalAction::UpdateNominalIncome(cid, NominalIncomeType::from(13037u32)); assert_ok!(EncointerDemocracy::submit_proposal( RuntimeOrigin::signed(alice.clone()), proposal_action @@ -700,7 +700,7 @@ fn proposal_happy_flow() { assert_eq!(EncointerDemocracy::proposals(1).unwrap().state, ProposalState::Enacted); assert_eq!(EncointerDemocracy::enactment_queue(proposal_action.get_identifier()), None); - assert_eq!(EncointerCommunities::nominal_income(cid), NominalIncomeType::from(13037i32)); + assert_eq!(EncointerCommunities::nominal_income(cid), NominalIncomeType::from(13037u32)); }); } @@ -710,7 +710,7 @@ fn enact_update_nominal_income_works() { let cid = create_cid(); let alice = alice(); let proposal_action = - ProposalAction::UpdateNominalIncome(cid, NominalIncomeType::from(13037i32)); + ProposalAction::UpdateNominalIncome(cid, NominalIncomeType::from(13037u32)); assert_ok!(EncointerDemocracy::submit_proposal( RuntimeOrigin::signed(alice.clone()), proposal_action @@ -724,7 +724,7 @@ fn enact_update_nominal_income_works() { assert_eq!(EncointerDemocracy::proposals(1).unwrap().state, ProposalState::Enacted); assert_eq!(EncointerDemocracy::enactment_queue(proposal_action.get_identifier()), None); - assert_eq!(EncointerCommunities::nominal_income(cid), NominalIncomeType::from(13037i32)); + assert_eq!(EncointerCommunities::nominal_income(cid), NominalIncomeType::from(13037u32)); }); } diff --git a/primitives/src/balances.rs b/primitives/src/balances.rs index 12dc45d3..a71cb8d2 100644 --- a/primitives/src/balances.rs +++ b/primitives/src/balances.rs @@ -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 @@ -137,11 +137,20 @@ 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()); + + to_U64F64(f).unwrap_or_else(|| { + // Should never happen, but we absolutely don't want to panic in code that gets executed + // upon every transaction, which would brick the chain. + log::error!("Exponential function has returned a negative value. Critical bug!"); + + // In this critical scenario we return 1, which is the no-op in terms of demurrage. + 1u32.into() + }) } /// Our BalanceType is I64F64, so the smallest possible number is @@ -161,10 +170,10 @@ impl Convert for EncointerBalanceConverter { let bits = balance.to_bits(); let mut result: u128 = 0; - result += (bits >> 64) as u128 * ONE_ENCOINTER_BALANCE_UNIT; + result += (bits >> 64) * 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::(); result } @@ -195,6 +204,16 @@ impl Convert for EncointerBalanceConverter { } } +#[allow(non_snake_case)] +pub fn to_U64F64(source: I64F64) -> Option { + if source.is_negative() { + return None + } + + // Safe conversion because we made sure that it is not negative above. + Some(U64F64::from_bits(source.to_bits() as u128)) +} + #[cfg(test)] mod tests { use super::*; @@ -215,26 +234,26 @@ mod tests { #[test] fn demurrage_works() { - let bal = BalanceEntry::::new(1.into(), 0); + let bal = BalanceEntry::::new(1u32.into(), 0); 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::::new(0.into(), 0); + let bal = BalanceEntry::::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::::new(1.into(), 0); + let bal = BalanceEntry::::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::::new(1.into(), 0); + let bal = BalanceEntry::::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); @@ -249,7 +268,7 @@ 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::::new(1.into(), 0); + let bal = BalanceEntry::::new(1u32.into(), 0); assert_abs_diff_eq(bal.apply_demurrage(demurrage, u32::MAX as u64 + 1).principal, 0f64); } @@ -257,15 +276,15 @@ mod tests { #[test] fn apply_demurrage_with_block_number_not_monotonically_rising_just_updates_last_block() { let demurrage = Demurrage::from_num(DEFAULT_DEMURRAGE); - let bal = BalanceEntry::::new(1.into(), 1); + let bal = BalanceEntry::::new(1u32.into(), 1); - assert_eq!(bal.apply_demurrage(demurrage, 0), BalanceEntry::::new(1.into(), 0)) + assert_eq!(bal.apply_demurrage(demurrage, 0), BalanceEntry::::new(1u32.into(), 0)) } #[test] fn apply_demurrage_with_zero_demurrage_works() { let demurrage = Demurrage::from_num(0.0); - let bal = BalanceEntry::::new(1.into(), 0); + let bal = BalanceEntry::::new(1u32.into(), 0); assert_abs_diff_eq(bal.apply_demurrage(demurrage, ONE_YEAR).principal, 1f64); } @@ -273,7 +292,7 @@ mod tests { #[test] fn apply_demurrage_with_zero_elapsed_blocks_works() { let demurrage = Demurrage::from_num(DEFAULT_DEMURRAGE); - let bal = BalanceEntry::::new(1.into(), 0); + let bal = BalanceEntry::::new(1u32.into(), 0); assert_abs_diff_eq(bal.apply_demurrage(demurrage, 0).principal, 1f64); } @@ -285,7 +304,7 @@ mod tests { // // Not critical as we safeguard against this with `validate_demurrage`. let demurrage = Demurrage::from_num(i64::MAX - 1); - let bal = BalanceEntry::::new(1.into(), 0); + let bal = BalanceEntry::::new(1u32.into(), 0); assert_abs_diff_eq(bal.apply_demurrage(demurrage, 1).principal, 0f64); } diff --git a/primitives/src/communities.rs b/primitives/src/communities.rs index 3afdf0b0..a5455dc5 100644 --- a/primitives/src/communities.rs +++ b/primitives/src/communities.rs @@ -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, @@ -47,7 +47,7 @@ pub type GeoHash = GeohashGeneric; 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; @@ -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, )] @@ -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; @@ -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(())); diff --git a/test-utils/src/lib.rs b/test-utils/src/lib.rs index 27c85fb5..d51cee1f 100644 --- a/test-utils/src/lib.rs +++ b/test-utils/src/lib.rs @@ -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]