Refactor: fixed point arithmetic for SRML. #3456

kianenigma · 2019-08-21T16:26:27Z

closes #3189 and resolves #2908

A macro generates a unified code for all per-things.
- Does not work for types that have 64-bit native size while it should. e.g. I want to add:

implement_per_thing!(Perquintill, test_perquintill, 10.pow(18), u64, u128, "_Parts per Quintillion_");

u128 is gone and a generic Rational128 is added with some basic operations.
Phragmen is refactored and uses the Rational128
Adding mul and power impl to per-things (still needed @pepyakin?)
Has still a bunch of TODOs in the code.
Much much much more tests.

cc @thiolliere

kianenigma · 2019-08-29T14:51:42Z

I won't be fully available till mid next week but still could use a round of reviews now. Note the two checklists up top: could still use some docs/test and has some TODOs.

As anticipated it also touches phragmen and uses a Rational128 to store very large and very accurate fractions. tbh it was a bit harder to do actually than the PerU128 but I believe it is a nicer approach. Basically I ended up using a small niche portion of the functionality that a Rational128 can provide in phragmen because I wanted to have the denominator almost always fixed to the largest possible number.

marcio-diaz · 2019-09-01T08:32:46Z

Did you consider using fixed and num-rational crates? I think we may need to implement a few more types to get a better performance/precision tradeoff.

kianenigma · 2019-09-01T13:12:04Z

Did you consider using fixed and num-rational crates? I think we may need to implement a few more types to get a better performance/precision tradeoff.

In the past we dismissed such options because we didn't nee the entire functionality and adding a library to srml as dependency might not be preferable. Maybe in the future. This PR is to mostly refactor what we already have.

core/sr-primitives/src/sr_arithmetic.rs

thiolliere

looks very nice, I haven't took a look yet, just some small comments

core/sr-primitives/src/sr_arithmetic.rs

core/phragmen/src/lib.rs

core/sr-primitives/Cargo.toml

thiolliere · 2019-09-24T11:37:32Z

srml/staking/src/lib.rs

@@ -1190,7 +1191,7 @@ impl<T: Trait> Module<T> {

 			for (v, p) in validators.iter().zip(points.individual.into_iter()) {
 				if p != 0 {
-					let reward = multiply_by_rational(total_payout, p, points.total);
+					let reward = Perbill::from_rational_approximation(p, points.total) * total_payout;


Oh yes so we have concluded that this approximation is fine.

core/sr-primitives/src/sr_arithmetic.rs

HCastano · 2019-09-24T12:12:22Z

core/phragmen/src/lib.rs

-			let diff_per_vote= diff / vote_count;
+			// To ensure an assertion indicating: no stake from the nominator going to waste,
+			// we add a minimal post-processing to equally assign all of the leftover stake ratios.
+			let vote_count = assignment.1.len() as u32;


Do you think it would make sense to de-structure the tuple so that we could have a more descriptive name than assignment.1 throughout this section of code?

we'd have to own it further down to push it to assigned. we could do that, not touch the .0 and construct into a tuple at the end. curious: would that be zero-cost?

I haven't been able to find much online about the implications of destructing, and then reconstructing a tuple. I want to say that the readability will beat the potential performance loss, but I have no good basis for that right now.

From here: https://old.reddit.com/r/rust/comments/79ry4s/tuple_performance/

For those who care, I stumbled upon the flawed benchmark, because I wondered whether using pattern matching for the function argument, i.e. (a, b): (Vec, Vec), is slower than binding the whole tuple and indexing into it, i.e. using x: (Vec, Vec) as the argument and x.0 and X.1 in the body. We do perform an extra copy in the pattern matching case, which seems useless in this example.

quite an interesting read. Yet, let's keep it for an optimisation PR since it needs verification + this code can use a LOT more optimisation here and there.

…l-fixed

core/phragmen/benches/phragmen.rs

@kianenigma

# Membership Request Hi, I am Kian Paimani, known as @kianenigma. I have been working on Polkadot/Kusama through Parity since February 2019 and I can categorize my main contributions to Polkadot's ecosystem as follows: 1. Maintaining and developing the staking sub-system. 2. General FRAME development, especially testing and quality assurance. 3. Polkadot-native side-projects. 4. Education > My first contribution to Polkadot is also indeed related to staking: paritytech/substrate#1915 ### Staking system I joke as the Polkadot staking to be both my blessing and my curse over the years. I started working on it since the first days that I joined this ecosystem and the work [is ongoing ever since](https://github.com/orgs/paritytech/projects/33/views/9). In the past, I focused on making sure that the staking system is secure and to some extent scalable. More recently, I coordinated the (imminent) launch of Nomination Pools. Nowadays I also put an extra effort on making sure that this sub-system of Polkadot is *sustainable*, through code refactor and educating other core developers. Lastly, I have been the main author of the [Polkadot staking newsletter](https://gist.github.com/kianenigma/aa835946455b9a3f167821b9d05ba376), which is my main attempt at making the entire complexity and development of this part of the protocol transparent to the end-users. I expect myself to contribute *directly* to the staking system for at least another ~12, if not more, and afterwards having the role of an advisor. Some notable contributions: - paritytech/substrate#4517 - paritytech/substrate#7910 - paritytech/substrate#6242 - paritytech/substrate#9415 - paritytech/polkadot#3141 - paritytech/substrate#11212 - paritytech/substrate#12129 ### FRAME Historically, I have contributed a variety of domains in FRAME, namely: - Early version of the weight system paritytech/substrate#3816 paritytech/substrate#3157 - Early version of the transaction fee system - Primitive arithmetic types paritytech/substrate#3456 - Council election pallet paritytech/substrate#3364 Many of which were, admittedly, a PoC at most, if not considered "poor". I am happy that nowadays many of the above have been refactored and are being maintained by new domain experts. These days, I put most of my FRAME focus on testing and quality assurance. Through my work in the staking system, I have had to deal with the high sensitivity and liveness requirement of protocol development first hand (I believe I had to do among the [very first storage migrations](paritytech/substrate#3948) in Kusama) and consequently I felt the need to make better testing facilities, all of which have been formulated in https://forum.polkadot.network/t/testing-complex-frame-pallets-discussion-tools/356. Some relevant PRs: - paritytech/substrate#8038 - paritytech/substrate#9788 - paritytech/substrate#10174 Regardless of wearing the staking hat, I plan to remain a direct contributor to FRAME, namely because I consider it to be an important requirements of successfully delivering more features to Polkadot's ecosystem. ### Polkadot-Native Side Projects I have started multiple small, mostly non-RUST projects in the polkadot ecosystem that I am very happy about, and I plan to continue doing so. I have not yet found the time to make a "polished product" out of any of these, but I hope that I can help foster our community such that someday a team will do so. I consider my role, for the time being, to *put ideas out there* through these side projects. - https://github.com/substrate-portfolio/polkadot-portfolio/ - https://github.com/kianenigma/polkadot-basic-notification/ - https://github.com/paritytech/polkadot-scripts/ - https://github.com/paritytech/substrate-debug-kit/ ### Education Lastly, aside from having had a number of educational talks over the years (all of which [are listed](https://hello.kianenigma.nl/talks/) in my personal website), I am a big enthusiast of the newly formed Polkadot Blockchain Academy. I have [been an instructor](https://singular.app/collectibles/statemine/16/2) in the first cohort, and continue to contribute for as long and as much as I can, whilst still attending to the former 3 duties. --- With all of that being said and done, I consider myself at the beginning of the path to Dan 4, but happy to start at a lower one as well.

@kianenigma

# Membership Request Hi, I am Kian Paimani, known as @kianenigma. I have been working on Polkadot/Kusama through Parity since February 2019 and I can categorize my main contributions to Polkadot's ecosystem as follows: 1. Maintaining and developing the staking sub-system. 2. General FRAME development, especially testing and quality assurance. 3. Polkadot-native side-projects. 4. Education > My first contribution to Polkadot is also indeed related to staking: paritytech/substrate#1915 ### Staking system I joke as the Polkadot staking to be both my blessing and my curse over the years. I started working on it since the first days that I joined this ecosystem and the work [is ongoing ever since](https://github.com/orgs/paritytech/projects/33/views/9). In the past, I focused on making sure that the staking system is secure and to some extent scalable. More recently, I coordinated the (imminent) launch of Nomination Pools. Nowadays I also put an extra effort on making sure that this sub-system of Polkadot is *sustainable*, through code refactor and educating other core developers. Lastly, I have been the main author of the [Polkadot staking newsletter](https://gist.github.com/kianenigma/aa835946455b9a3f167821b9d05ba376), which is my main attempt at making the entire complexity and development of this part of the protocol transparent to the end-users. I expect myself to contribute *directly* to the staking system for at least another ~12, if not more, and afterwards having the role of an advisor. Some notable contributions: - paritytech/substrate#4517 - paritytech/substrate#7910 - paritytech/substrate#6242 - paritytech/substrate#9415 - paritytech/polkadot#3141 - paritytech/substrate#11212 - paritytech/substrate#12129 ### FRAME Historically, I have contributed a variety of domains in FRAME, namely: - Early version of the weight system paritytech/substrate#3816 paritytech/substrate#3157 - Early version of the transaction fee system - Primitive arithmetic types paritytech/substrate#3456 - Council election pallet paritytech/substrate#3364 Many of which were, admittedly, a PoC at most, if not considered "poor". I am happy that nowadays many of the above have been refactored and are being maintained by new domain experts. These days, I put most of my FRAME focus on testing and quality assurance. Through my work in the staking system, I have had to deal with the high sensitivity and liveness requirement of protocol development first hand (I believe I had to do among the [very first storage migrations](paritytech/substrate#3948) in Kusama) and consequently I felt the need to make better testing facilities, all of which have been formulated in https://forum.polkadot.network/t/testing-complex-frame-pallets-discussion-tools/356. Some relevant PRs: - paritytech/substrate#8038 - paritytech/substrate#9788 - paritytech/substrate#10174 Regardless of wearing the staking hat, I plan to remain a direct contributor to FRAME, namely because I consider it to be an important requirements of successfully delivering more features to Polkadot's ecosystem. ### Polkadot-Native Side Projects I have started multiple small, mostly non-RUST projects in the polkadot ecosystem that I am very happy about, and I plan to continue doing so. I have not yet found the time to make a "polished product" out of any of these, but I hope that I can help foster our community such that someday a team will do so. I consider my role, for the time being, to *put ideas out there* through these side projects. - https://github.com/substrate-portfolio/polkadot-portfolio/ - https://github.com/kianenigma/polkadot-basic-notification/ - https://github.com/paritytech/polkadot-scripts/ - https://github.com/paritytech/substrate-debug-kit/ ### Education Lastly, aside from having had a number of educational talks over the years (all of which [are listed](https://hello.kianenigma.nl/talks/) in my personal website), I am a big enthusiast of the newly formed Polkadot Blockchain Academy. I have [been an instructor](https://singular.app/collectibles/statemine/16/2) in the first cohort, and continue to contribute for as long and as much as I can, whilst still attending to the former 3 duties. --- With all of that being said and done, I consider myself at the beginning of the path to Dan 4, but happy to start at a lower one as well. Co-authored-by: Bastian Köcher <git@kchr.de>

kianenigma added 4 commits August 1, 2019 15:10

Macro-ify perthings.

48564db

Refactor fixed64

87ad0f9

Half-workign phragmen refactor.

b3fc046

Finalize phragmen refactor.

df550ef

This comment has been minimized.

Sign in to view

paritytech deleted a comment from parity-cla-bot Aug 22, 2019

This was referenced Aug 23, 2019

srml-module: Phragmen election #3364

Merged

Decouple Phragmen from Staking. #3498

Merged

kianenigma added 7 commits August 29, 2019 11:25

Fix creation of perquintill

2786e54

Master.into()

a179dec

Fix build errors

db192c6

Line-width

03785b0

Fix more build errors.

877947e

Line-width

882f904

Fix offence test

1423901

kianenigma marked this pull request as ready for review August 29, 2019 14:41

kianenigma requested a review from thiolliere as a code owner August 29, 2019 14:41

This was referenced Aug 31, 2019

Vision for Substrate Recipes (v2) JoshOrndorff/recipes#32

Closed

Fixed Point Arithmetic JoshOrndorff/recipes#12

Closed

gavofyork added the A0-please_review Pull request needs code review. label Sep 1, 2019

kianenigma requested review from bkchr and pepyakin September 4, 2019 15:09

Master.into()

00006dd

kianenigma commented Sep 5, 2019

View reviewed changes

core/sr-primitives/src/sr_arithmetic.rs Outdated Show resolved Hide resolved

Resolve all TODOs.

14fe8ac

thiolliere reviewed Sep 6, 2019

View reviewed changes

gavofyork reviewed Sep 8, 2019

View reviewed changes

core/phragmen/src/lib.rs Outdated Show resolved Hide resolved

kianenigma added 2 commits September 20, 2019 10:52

Master.into()

a38bc62

Master.into()

711c3fd

kianenigma added A1-onice and removed A0-please_review Pull request needs code review. labels Sep 20, 2019

thiolliere self-requested a review September 23, 2019 08:42

kianenigma mentioned this pull request Sep 23, 2019

Accurately compute operations on u128 #3670

Closed

A bit more hardening.

810fa0e

kianenigma requested review from andresilva, Demi-Marie and jimpo as code owners September 23, 2019 15:23

Master.into()

7952c3d

paritytech-ci added the B2-breaksapi label Sep 23, 2019

thiolliere reviewed Sep 24, 2019

View reviewed changes

thiolliere approved these changes Sep 24, 2019

View reviewed changes

core/sr-primitives/src/sr_arithmetic.rs Outdated Show resolved Hide resolved

core/sr-primitives/src/sr_arithmetic.rs Outdated Show resolved Hide resolved

HCastano reviewed Sep 24, 2019

View reviewed changes

kianenigma added 4 commits September 24, 2019 15:42

Master.into()

2802520

Final nits.

a281e3e

Update lock

177b662

Merge branch 'master' of github.com:paritytech/substrate into kiz-srm…

7396618

…l-fixed

Demi-Marie reviewed Sep 24, 2019

View reviewed changes

core/phragmen/benches/phragmen.rs Outdated Show resolved Hide resolved

kianenigma added 4 commits September 25, 2019 09:50

Master.into()

9065295

Fix indent.

96bba97

Revert lock file.

6c08adf

Bump.

35548be

kianenigma merged commit 759ecfc into master Sep 25, 2019

kianenigma deleted the kiz-srml-fixed branch September 25, 2019 09:21

kianenigma mentioned this pull request Oct 5, 2019

Fuzzing and Organisation for Runtime Arithmetic Types #3745

Closed

3 tasks

kianenigma mentioned this pull request Sep 27, 2022

Member Request polkadot-fellows/seeding#14

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor: fixed point arithmetic for SRML. #3456

Refactor: fixed point arithmetic for SRML. #3456

kianenigma commented Aug 21, 2019 •

edited

Loading

This comment has been minimized.

kianenigma commented Aug 29, 2019

marcio-diaz commented Sep 1, 2019 •

edited

Loading

kianenigma commented Sep 1, 2019

thiolliere left a comment

thiolliere Sep 24, 2019

HCastano Sep 24, 2019

kianenigma Sep 24, 2019

HCastano Sep 25, 2019

HCastano Sep 25, 2019

kianenigma Sep 25, 2019 •

edited

Loading

Refactor: fixed point arithmetic for SRML. #3456

Refactor: fixed point arithmetic for SRML. #3456

Conversation

kianenigma commented Aug 21, 2019 • edited Loading

This comment has been minimized.

kianenigma commented Aug 29, 2019

marcio-diaz commented Sep 1, 2019 • edited Loading

kianenigma commented Sep 1, 2019

thiolliere left a comment

Choose a reason for hiding this comment

thiolliere Sep 24, 2019

Choose a reason for hiding this comment

HCastano Sep 24, 2019

Choose a reason for hiding this comment

kianenigma Sep 24, 2019

Choose a reason for hiding this comment

HCastano Sep 25, 2019

Choose a reason for hiding this comment

HCastano Sep 25, 2019

Choose a reason for hiding this comment

kianenigma Sep 25, 2019 • edited Loading

Choose a reason for hiding this comment

kianenigma commented Aug 21, 2019 •

edited

Loading

marcio-diaz commented Sep 1, 2019 •

edited

Loading

kianenigma Sep 25, 2019 •

edited

Loading