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

Offchain Phragmén BREAKING. #4517

Merged
merged 144 commits into from
Mar 26, 2020
Merged

Offchain Phragmén BREAKING. #4517

merged 144 commits into from
Mar 26, 2020

Conversation

kianenigma
Copy link
Contributor

@kianenigma kianenigma commented Jan 2, 2020

Fix #4410


Currently, Kusama is suffering from severe stalls at the end of each era. Seemingly validators keeep missing their slot, which makes whatever we do in new_era a suspect. Phragmén election is one of them. I have made some attempts (which I hope to finish soon as reusable tools) to benchmark the wasm code but hadn't got any decisive conclusion yet. Nonetheless, using https://github.com/kianenigma/offline-phragmen I could verify that the current Kusama network (~200 candidates, ~600 voters) takes around 0.5 seconds just to compute the election result in native. In wasm, and taking the overlooked overhead of state lookup, hashing and decoding, it is enough to prioritise offchain phragmen as it is obviously hindering the block's limited 2 seconds.

In the offchain model:

  • A certain number of blocks before the era is expected to end, the staking module enables a flag that allows for solutions to be submitted.
  • All validators who opt to run the offchain code start computing phragmen and submit the solution back to the chain.
  • Any other account can also submit a solution.
  • The staking module evaluates and stores only the best result.
  • If for any reason, at the end of the era, no solutions have been submitted, we fallback to the current system: on-chain phragmen.

To be Done

In this PR

In follow ups:
  • Staking should freeze and not allow any changes to sensitive data when election window is open. (already done)
  • Clean the reduce code. Add index cache (+ other optimizations) Add benchmarks and keep track of performance. This is not critical as that code will run offchain.
  • Run equalize() again.
  • rewards? punishment?

Discussions

  • The key that validators sign the transaction with would be the ephemeral session key AFAIK, how would they pay for it? Update: Unsigned support added

  • Maybe disable equalisation and only do seq-phragmen in fallback mode. Update: already done

  • im-online is assuming that each session has a fixed length by assuming slot == block. Maybe we should do the same? @andresilva

  • Encoding and Reduction research https://hackmd.io/JOn9x98iS0e0DPWQ87zGWg?view


polkadot companion: paritytech/polkadot#940
polkadot companion: paritytech/polkadot#940

@kianenigma kianenigma added the A3-in_progress Pull request is in progress. No review needed at this stage. label Jan 2, 2020
@bkchr
Copy link
Member

bkchr commented Jan 2, 2020

Why not an unsigned transaction?

@kianenigma
Copy link
Contributor Author

Why not an unsigned transaction?

Because we want normal accounts (not just a validator) or entities to also be able to submit a solution. I can very well see a tool that does this for example (if we allocate a small reward to a good solution)

@bkchr
Copy link
Member

bkchr commented Jan 2, 2020

Why don't we support unsigned and signed? I still don't see how you want to have funds in the account associated to your session keys.

@kianenigma
Copy link
Contributor Author

Why don't we support unsigned and signed? I still don't see how you want to have funds in the account associated to your session keys.

It is one of the discussion items and I am not sure either. But:

  • Only unsigned is not an option. Supporting both, maybe. Validators can submit unsigned then.
  • We can hack around it by tweaking the TakeFee to make some exceptions in on certain transaction, if they come from certain account (but it might not look good :D). But we allow the signed extension to actually examine the origin, here's a use case.

Copy link
Contributor

@tomusdrw tomusdrw left a comment

Choose a reason for hiding this comment

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

Good start!

The key that validators sign the transaction with would be the ephemeral session key AFAIK, how would they pay for it?

I think we should allow both Unsigned transactions coming from validators and Signed transactions from external nodes. Possibly the external nodes are accepted only as an improvement
over what validators have proposed.

speaking of that, maybe this whole approach of X blocks before the era ends for the window of offchain is simply too complicated. We could just say: in the last session the offchain is allowed.

Why make any requirements like that? Aren't we going to accept the best result anyway? So by definition the results computed ealier would be worse or even invalid.

frame/babe/src/lib.rs Outdated Show resolved Hide resolved
frame/babe/src/lib.rs Outdated Show resolved Hide resolved
frame/babe/src/mock.rs Show resolved Hide resolved
frame/babe/src/lib.rs Outdated Show resolved Hide resolved
frame/staking/src/lib.rs Show resolved Hide resolved
frame/staking/src/lib.rs Outdated Show resolved Hide resolved
frame/staking/src/lib.rs Outdated Show resolved Hide resolved
frame/staking/src/lib.rs Outdated Show resolved Hide resolved
frame/staking/src/lib.rs Outdated Show resolved Hide resolved
frame/staking/src/lib.rs Outdated Show resolved Hide resolved
@kianenigma
Copy link
Contributor Author

Good start!

The key that validators sign the transaction with would be the ephemeral session key AFAIK, how would they pay for it?

I think we should allow both Unsigned transactions coming from validators and Signed transactions from external nodes. Possibly the external nodes are accepted only as an improvement
over what validators have proposed.

speaking of that, maybe this whole approach of X blocks before the era ends for the window of offchain is simply too complicated. We could just say: in the last session the offchain is allowed.

Why make any requirements like that? Aren't we going to accept the best result anyway? So by definition the results computed ealier would be worse or even invalid.

Suggestion of accepting both signed and unsigned is indeed an option. Also recommended by @bkchr. We'll see about it.

Regarding the timespan and always allowing solutions: I initially disagree. The votes and validations might, and most likely will, change during an era. A solution submitted toward the beginning is most likely a waste to even compute and verify since the votes will change.

I am more in favour of doing something moderate in between: the X-block window that I implemented now is also a bit too strict. Making the window wider to something like the last quarter of the era is more sensible. We could do this either at granularity of blocks or sessions.

@burdges
Copy link

burdges commented Jan 4, 2020

Would we reward validators for their solutions too?

We looked for VRF tricks to prove ownership of Phragmén solutions, but if I recall Phragmén does not use randomness in the right way for this. Assuming this, if you want solutions to be owned then you must commit to them in advance and reveal after the commit finalizes. We do not need this now but it might be something we'd want in 2021 or so.

I'm only mentioning this in case it impacts what code goes into the runtime, but presumably the runtime could always be modified to do commits.

@kianenigma kianenigma mentioned this pull request Jan 15, 2020
@gavofyork gavofyork added this to the 3.0 milestone Jan 15, 2020
@kianenigma
Copy link
Contributor Author

@bkchr @thiolliere you could have a look at the phragmen/compact part already as it is isolated and can be reviewed separately.

@kianenigma
Copy link
Contributor Author

Ok recap:

  • No need for Signature, key, signing etc. OCW will submit an unsigned call without any additional information.
  • This transaction will live in the pool until a node is designated to be the author. Then it will get validated. It is for the longevity of this transaction to be only until the end of the era, and
  • ValidateUnsigned will validate it only if Source::Local and will not propagate it.

One thing that I don't get is: Now that we will soon have source in ValidateUnsigned, this is not relevant anymore, right?

The alternative is to have validate_unsigned actually check the solution, and then accept from external sources (though nobody would actually be publishing).

ValidateUnsigned can just validate the call being the correct one and the source being local. The rest can be done in the execution phase.

@tomusdrw
Copy link
Contributor

The alternative is to have validate_unsigned actually check the solution, and then accept from external sources (though nobody would actually be publishing).

ValidateUnsigned can just validate the call being the correct one and the source being local. The rest can be done in the execution phase.

I believe so, yes. Make sure to check the source as the first thing (i.e. reject if source is External accept if it's Local or InBlock). Validating the entire solution for everyone is a potential DoS vector. You could spam unsigned transactions with seemingly valid solutions that just don't bring any improvement when included on chain, yet we have to validate and execute them to learn that.

frame/staking/src/lib.rs Outdated Show resolved Hide resolved
score,
era,
)?
// TODO: instead of returning an error, panic. This makes the entire produced block
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gavofyork this is the only remaining part that I told you about. I propose adding the panic/slash in a second phase. The code change would be simple and we merely panic here if check_and_replace_solution was anything other than Ok(()).

Given

  • Validators wanting to behave correctly,
  • Our offchain worker code working correctly,

submit_election_solution_unsigned will only ever be called if a solution is an improvement to what we already have on chain (because we check the score in ValidateUnsigned), hence it must never panic unless if a validator attempted to put a non-improving solution on chain.

frame/staking/src/lib.rs Outdated Show resolved Hide resolved
@kianenigma
Copy link
Contributor Author

@jacogr from the end user's perspective, maybe an optimisation idea for the UI is to show a warning in the Account Actions page of Staking whenElectionStatus::Open(_), saying that you cannot submit any transactions until the end of the era.

Co-Authored-By: Gavin Wood <gavin@parity.io>
@kianenigma
Copy link
Contributor Author

polkadot fix here: paritytech/polkadot#941

@gavofyork gavofyork merged commit d123842 into master Mar 26, 2020
@kianenigma kianenigma deleted the kiz-offchain-phragmen-4 branch April 1, 2020 06:18
@Lohann Lohann mentioned this pull request Mar 22, 2021
6 tasks
kianenigma added a commit to kianenigma/seeding that referenced this pull request Sep 27, 2022
# 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.
bkchr added a commit to polkadot-fellows/seeding that referenced this pull request Sep 27, 2022
# 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>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. U1-asap No need to stop dead in your tracks, however issue should be addressed as soon as possible.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Off-chain Phragmen