Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

Remove As trait bound from Currency::Balance #2205

Merged
gavofyork merged 4 commits into from
May 9, 2019
Merged

Remove As trait bound from Currency::Balance #2205

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2fd395b
remove As<64> bond
xlc Apr 4, 2019
dc39843
Currency trait refactor
xlc Apr 4, 2019
01b2d47
bump spec version
xlc May 8, 2019
7bc768c
Merge branch 'master' into update-balance
xlc May 9, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion node/runtime/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
spec_name: create_runtime_str!("node"),
impl_name: create_runtime_str!("substrate-node"),
authoring_version: 10,
spec_version: 72,
spec_version: 73,
impl_version: 73,
apis: RUNTIME_API_VERSIONS,
};
Expand Down
4 changes: 2 additions & 2 deletions srml/balances/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ pub use self::imbalances::{PositiveImbalance, NegativeImbalance};

pub trait Subtrait<I: Instance = DefaultInstance>: system::Trait {
/// The balance of an account.
type Balance: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + As<usize> + As<u64> + MaybeSerializeDebug;
type Balance: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + MaybeSerializeDebug;

/// A function that is invoked when the free-balance has fallen below the existential deposit and
/// has been reduced to zero.
Expand All @@ -181,7 +181,7 @@ pub trait Subtrait<I: Instance = DefaultInstance>: system::Trait {

pub trait Trait<I: Instance = DefaultInstance>: system::Trait {
/// The balance of an account.
type Balance: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + As<usize> + As<u64> + MaybeSerializeDebug;
type Balance: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + MaybeSerializeDebug;

/// A function that is invoked when the free-balance has fallen below the existential deposit and
/// has been reduced to zero.
Expand Down
6 changes: 4 additions & 2 deletions srml/staking/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -894,8 +894,10 @@ impl<T: Trait> Module<T> {
Self::reward_validator(v, reward);
}
Self::deposit_event(RawEvent::Reward(reward));
let total_minted = reward * <BalanceOf<T> as As<usize>>::sa(validators.len());
let total_rewarded_stake = Self::slot_stake() * <BalanceOf<T> as As<usize>>::sa(validators.len());
Comment thread
xlc marked this conversation as resolved.
let len = validators.len() as u64; // validators length can never overflow u64
let len = BalanceOf::<T>::sa(len);

@gui1117 gui1117 May 8, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would have written this in one line but whatever

let total_minted = reward * len;
let total_rewarded_stake = Self::slot_stake() * len;
T::OnRewardMinted::on_dilution(total_minted, total_rewarded_stake);
}

Expand Down
6 changes: 3 additions & 3 deletions srml/support/src/traits.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
use crate::rstd::result;
use crate::codec::{Codec, Encode, Decode};
use crate::runtime_primitives::traits::{
MaybeSerializeDebug, SimpleArithmetic, As
MaybeSerializeDebug, SimpleArithmetic
};

/// The account with the given id was killed.
Expand Down Expand Up @@ -195,7 +195,7 @@ pub enum SignedImbalance<B, P: Imbalance<B>>{
impl<
P: Imbalance<B, Opposite=N>,
N: Imbalance<B, Opposite=P>,
B: SimpleArithmetic + As<usize> + As<u64> + Codec + Copy + MaybeSerializeDebug + Default,
B: SimpleArithmetic + Codec + Copy + MaybeSerializeDebug + Default,
> SignedImbalance<B, P> {
pub fn zero() -> Self {
SignedImbalance::Positive(P::zero())
Expand Down Expand Up @@ -230,7 +230,7 @@ impl<
/// Abstraction over a fungible assets system.
pub trait Currency<AccountId> {
/// The balance of an account.
type Balance: SimpleArithmetic + As<usize> + As<u64> + Codec + Copy + MaybeSerializeDebug + Default;
type Balance: SimpleArithmetic + Codec + Copy + MaybeSerializeDebug + Default;

@gui1117 gui1117 Apr 5, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

it seems to me that only having From<u64> and TryInto<u64> on Currency could be interesting (this would assume that a currency is overset of u64, but this is legit to me)

but SimpleArithmetic bouds As<u64> at the moment, this removal only actually removes As<usize>

@kianenigma kianenigma Apr 8, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also agree with the concept of TryInto<...> (or maybe even TryFrom<...>) instead of the current one. Prevents any API break and seems like a generic-enough approach.

Question though: Balance is SimpleArithmetic and SimpleArithmetic is already As<u64>. Isn't this already indirectly implying that Balance is still bounded by As<u64>?

@gui1117 gui1117 Apr 8, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Question though: Balance is SimpleArithmetic and SimpleArithmetic is already As. Isn't this already indirectly implying that Balance is still bounded by As?

Yes, that's what I tried to say in my second paragraph

@kianenigma kianenigma Apr 8, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Right, I went blind for a sec apparently.

@xlc Did you ever test to see if this PR, with its current state, actually solve your initial problem (storing some kind of Vec as Balance)? I assume no.

@xlc xlc Apr 8, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No. As other pointed out, Vec is not Copyable so I need to do it differently.
I was thinking to do it with [Balance; 4] or something similar. But regardless the approach, it will likely not able to convert into a number and requires this PR.


/// The opaque token type for an imbalance. This is returned by unbalanced operations
/// and must be dealt with. It may be dropped but cannot be cloned.
Expand Down