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

Multi-Block Election part 0: preparation and some cleanup. #9442

Merged
22 commits merged into from
Aug 11, 2021
Merged

Multi-Block Election part 0: preparation and some cleanup. #9442

22 commits merged into from
Aug 11, 2021

Conversation

kianenigma
Copy link
Contributor

@kianenigma kianenigma commented Jul 27, 2021 •
edited
Loading

This is the first step towards multi-block election.

As first step, I did a lot of renaming that I have been meaning to do for a while. This is to prevent calling the output of create_solution_type compact, since it is confused with codec::Compact. Other than that, I repotted some items in sp-npos-elections. There are no logical changes in this PR.

The only other change is that I factored out votes2 and treat it like all other fields, instead of being vec![(voter , (t1, p1), t2), .. ] it is now vec![(voter , [(t1, p1)], t2), .. ].

polkadot companion: paritytech/polkadot#3594

@kianenigma kianenigma added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. labels Jul 27, 2021
@kianenigma kianenigma changed the title Partially applied Multi-Block Election -- part0: paged npos solution type Jul 27, 2021
@kianenigma kianenigma changed the title Multi-Block Election -- part0: paged npos solution type Multi-Block Election part 0: *paged npos solution type* Jul 27, 2021
@emostov
Copy link
Contributor

emostov commented Jul 31, 2021

I did a lot of renaming to make sure this struct is simply called Solution, no longer CompactSolution. This was confusing because you actually could use the #[compact] codec annotation within the struct as well. Because of this, be aware that ALL CHANGES TO /frame ARE JUST RENAMING THINGS.

@kianenigma sounds like something we can extract to its own PR and get in now?

@kianenigma
Copy link
Contributor Author

I did a lot of renaming to make sure this struct is simply called Solution, no longer CompactSolution. This was confusing because you actually could use the #[compact] codec annotation within the struct as well. Because of this, be aware that ALL CHANGES TO /frame ARE JUST RENAMING THINGS.

@kianenigma sounds like something we can extract to its own PR and get in now?

Yeah, sounds good.

@kianenigma kianenigma added A0-please_review Pull request needs code review. and removed A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). labels Aug 7, 2021
shawntabrizi
shawntabrizi approved these changes Aug 7, 2021
View reviewed changes
Copy link
Member

@shawntabrizi shawntabrizi left a comment

Choose a reason for hiding this comment

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

without digging to deep into the macro, other changes and renaming seems sensible.

Should not use my review if there are still open question in the macro from others

@kianenigma kianenigma mentioned this pull request Aug 7, 2021
Merged
gui1117
gui1117 reviewed Aug 9, 2021
View reviewed changes
primitives/npos-elections/solution-type/src/lib.rs Show resolved Hide resolved
primitives/npos-elections/solution-type/src/lib.rs
mod codec;
mod from_assignment_helpers;
mod index_assignment;
mod single_page;
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like the module name doesn't make much sense anymore.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

soon I will introduce paged into the jargon of this and other crates, I think it is okay.

@kianenigma kianenigma mentioned this pull request Aug 10, 2021
Closed
@kianenigma
Copy link
Contributor Author

bot merge

@ghost
Copy link

ghost commented Aug 11, 2021

Waiting for commit status.

@kianenigma
Copy link
Contributor Author

bot merge

@ghost
Copy link

ghost commented Aug 11, 2021

Waiting for commit status.

@ghost
Copy link

ghost commented Aug 11, 2021

Merge aborted: Head SHA changed from 538d36c to 419e7a3

@ghost ghost merged commit 09c016b into master Aug 11, 2021
@ghost ghost deleted the kiz-multi-block-snapshot-pt1-paged-solution-macro branch August 11, 2021 15:45
chevdor pushed a commit that referenced this pull request Sep 13, 2021
@kianenigma @gui1117 @chevdor
Multi-Block Election part 0: preparation and some cleanup. (#9442)
6e74231 
* Partially applied

* Everything builds, need to implement compact encoding as well.

* Fix some tests, add a ui test as well.

* Fix everything and everything.

* small nits

* a bunch more rename

* more reorg

* more reorg

* last nit of self-review

* Seemingly fixed the build now

* Fix build

* make it work again

* Update primitives/npos-elections/solution-type/src/lib.rs

Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>

* Update primitives/npos-elections/solution-type/src/lib.rs

Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>

* nits

* factor out double type

* fix try-build

Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>
This was referenced Sep 13, 2021
Merged
Merged
gilescope pushed a commit that referenced this pull request Sep 13, 2021
@chevdor @gui1117 @kianenigma
Multi-Block Election part 0: preparation and some cleanup. (#9442) (#…
58f0223 
…9767)

* Partially applied

* Everything builds, need to implement compact encoding as well.

* Fix some tests, add a ui test as well.

* Fix everything and everything.

* Update primitives/npos-elections/solution-type/src/lib.rs

* factor out double type

* fix try-build

Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@emostov emostov emostov left review comments

@gui1117 gui1117 gui1117 approved these changes

@shawntabrizi shawntabrizi shawntabrizi approved these changes

@KiChjang KiChjang Awaiting requested review from KiChjang

Assignees
No one assigned
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed.
Projects
(Nominated) Proof of Stake
Status: ✅ Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kianenigma @emostov @gui1117 @shawntabrizi