Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
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
Prev Previous commit
Next Next commit
fix global test issues
  • Loading branch information
kianenigma committed Mar 6, 2019
commit f969335c5b567781a3e7944c92568def35c4166e
14 changes: 7 additions & 7 deletions core/sr-primitives/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,11 +250,14 @@ impl From<codec::Compact<Perbill>> for Perbill {

/// Perquill is parts-per-quintillion. It stores a value between 0 and 1 in fixed point and
/// provides a means to multiply some other value by that.
#[cfg_attr(feature = "std", derive(Serialize, Deserialize))]
#[cfg_attr(feature = "std", derive(Serialize, Deserialize, Debug))]
#[derive(Encode, Decode, Default, Copy, Clone, PartialEq, Eq)]
pub struct Perquill(u64);

impl Perquill {
/// Returns the internal u64 value
pub fn extract(&self) -> u64 { self.0 }
Copy link
Member

Choose a reason for hiding this comment

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

Please remove this function and implement Deref to get the same functionality.

Copy link
Contributor

Choose a reason for hiding this comment

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

Deref is meant to be for smart pointers

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well... Its currently changed to Deref instead of extract().

Docs says:

Implementing Deref for smart pointers makes accessing the data behind them convenient, ...

From which I understand it is good for smart pointers, not exclusive.

Though I am personally in favor of .extract() or anything else since I hate this type of code.
747d650#diff-ba0a45203e7c14aedb758fe4d3653b35R138


/// Nothing.
pub fn zero() -> Perquill { Perquill(0) }
Copy link
Member

Choose a reason for hiding this comment

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

Is this function even required?
If we don't need 0, we could use https://doc.rust-lang.org/core/num/struct.NonZeroU64.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one actually yes. But some others are not used atm but exist to have a concise set of functions w.r.t Permill. Can be removed if priority is to not have unused code?

Copy link
Member

Choose a reason for hiding this comment

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

It's just if you don't need 0, you can use the linked type to get some optimization for certain operations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

0 is needed then. What's the preferred way regarding having unused functions for the sake of consistency with other similar type?
My preference: They should go away.


Expand All @@ -270,17 +273,14 @@ impl Perquill {
/// Construct new instance where `x` is in millionths. Value equivalent to `x / 1,000,000`.
pub fn from_millionths(x: u64) -> Perquill { Perquill(x.min(1_000_000) * 1000_000_000_000) }

/// Construct new instance where `x` is denominator and the nominator is 1.
pub fn from_xth(x: u64) -> Perquill { Perquill(1_000_000_000_000_000_000 / x.min(1_000_000_000_000_000_000)) }

#[cfg(feature = "std")]
/// Construct new instance whose value is equal to `x` (between 0 and 1).
pub fn from_fraction(x: f64) -> Perquill { Perquill((x.max(0.0).min(1.0) * 1_000_000_000_000_000_000.0) as u64) }
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, maybe we should return Option<Self> here. If x is greater than 1 or smaller than 0, we should return None.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's first decide on this bc. this is not even used atm
#1915 (comment)

But if it stays it is again a matter of consistency with the behavior of other primitives. Currently, all of them have a don't care attitude toward from_fraction where you just get .min() / .max()'ed and always get a value, so Option is not needed. I am personally happy with the current way of doing it.

}

impl std::fmt::Debug for Perquill {
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
write!(f, "Perquill(~{}) ", self.0 as f64/1_000_000_000_000_000_000.0)
}
}

impl<N> ::rstd::ops::Mul<N> for Perquill
where
N: traits::As<u64>
Expand Down
2 changes: 2 additions & 0 deletions node/cli/src/chain_spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ fn staging_testnet_config_genesis() -> GenesisConfig {
minimum_validator_count: 4,
stakers: initial_authorities.iter().map(|x| (x.0.into(), x.1.into(), STASH)).collect(),
invulnerables: initial_authorities.iter().map(|x| x.1.into()).collect(),
nominators: vec![],
}),
democracy: Some(DemocracyConfig {
launch_period: 10 * MINUTES, // 1 day per public referendum
Expand Down Expand Up @@ -256,6 +257,7 @@ pub fn testnet_genesis(
offline_slash_grace: 0,
stakers: initial_authorities.iter().map(|x| (x.0.into(), x.1.into(), STASH)).collect(),
invulnerables: initial_authorities.iter().map(|x| x.1.into()).collect(),
nominators: vec![],
}),
democracy: Some(DemocracyConfig {
launch_period: 9,
Expand Down
1 change: 1 addition & 0 deletions node/executor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,7 @@ mod tests {
current_session_reward: 0,
offline_slash_grace: 0,
invulnerables: vec![alice(), bob(), charlie()],
nominators: vec![],
}),
democracy: Some(Default::default()),
council_seats: Some(Default::default()),
Expand Down
17 changes: 8 additions & 9 deletions srml/staking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -724,7 +724,7 @@ impl<T: Trait> Module<T> {
let candidate = &vote.who;
if let Some(index) = candidates.iter().position(|i| i.who == *candidate) {
let approval_stake = candidates[index].approval_stake;
candidates[index].score = Perquill::from_fraction(1f64/approval_stake.as_() as f64);
candidates[index].score = Perquill::from_xth(approval_stake.as_());
}
}
}
Expand All @@ -734,18 +734,17 @@ impl<T: Trait> Module<T> {
let candidate = &vote.who;
if let Some(index) = candidates.iter().position(|i| i.who == *candidate) {
let approval_stake = candidates[index].approval_stake;
// TODO: casting, casting everywhere... Perquill::from_Xth()?
let temp =
nominaotion.stake.as_()
* *nominaotion.load.encode_as() as u64
* nominaotion.load.extract()
/ approval_stake.as_();
candidates[index].score = Perquill::decode_from(candidates[index].score.encode_as() + temp);
candidates[index].score = Perquill::decode_from(candidates[index].score.extract() + temp);
}
}
}

// Find the best
let (winner_index, _) = candidates.iter().enumerate().min_by_key(|&(_i, c)| c.score.encode_as())
let (winner_index, _) = candidates.iter().enumerate().min_by_key(|&(_i, c)| c.score.extract())
.expect("candidates length is checked to be >0; qed");

// update nominator and vote load
Expand All @@ -754,8 +753,8 @@ impl<T: Trait> Module<T> {
for vote_idx in 0..nominations[nominator_idx].nominees.len() {
if nominations[nominator_idx].nominees[vote_idx].who == winner.who {
nominations[nominator_idx].nominees[vote_idx].load =
Perquill::decode_from(winner.score.encode_as()
- nominations[nominator_idx].load.encode_as());
Perquill::decode_from(winner.score.extract()
- nominations[nominator_idx].load.extract());
nominations[nominator_idx].load = winner.score;
}
}
Expand All @@ -770,8 +769,8 @@ impl<T: Trait> Module<T> {
if let Some(index) = elected_candidates.iter().position(|c| c.who == vote.who) {
vote.backing_stake = <BalanceOf<T> as As<u64>>::sa(
nomination.stake.as_()
* vote.load.encode_as()
/ nomination.load.encode_as()
* vote.load.extract()
/ nomination.load.extract()
);
elected_candidates[index].exposure.total += vote.backing_stake;
// Update IndividualExposure of those who nominated and their vote won
Expand Down