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

Society v2 #11324

Merged
merged 55 commits into from
Jun 18, 2023
Merged

Society v2 #11324

merged 55 commits into from
Jun 18, 2023

Conversation

gavofyork
Copy link
Member

@gavofyork gavofyork commented Apr 30, 2022

Closes #10465
Polkadot companion: paritytech/polkadot#7356

Substantial changes to Society:

  • The number of members are no longer limited technically (Members is no longer a Vec item).
  • Proper use of BoundedVec.
  • Max members and intake size can be altered by founder without runtime upgrade.
  • Members are ranked. Voting weight is determined as a rank-plus-one-squared.
  • Right now there's only two ranks, with all members starting as rank zero (vote weight 1) and all members eligible to become rank one (vote weight 4) if they return any funds they received from the society back into its treasury and give up the possibility of getting future funds.
  • The concept of a suspended candidate is removed: Candidates can remain a candidate indefinitely, however unless there is persistent indecision among the membership and inaction by the founder, then all candidates will be either approved as a member or rejected entirely after a full additional intake round.

Candiate voting:

  • The process for moving from candidate to member is no longer automatic and requires a call into one of a number of free dispatchables.
  • The voting/reward/punishment scheme for candidates and challenged members is completely different.
  • The candidate voting round is two phase, initially a voting phase and then a period afterwards where membership may be claimed based on those votes before the next candidate(s) are selected.
  • There is now only one Skeptic and their duty is changed.
  • No members are rewarded for voting.
  • Only the Skeptic is punished for invalid/missing votes.
  • Unlike the original voting scheme, the new scheme involves no random input.
  • The Skeptic is selected at random from the members, but no vote is attributed to them.
  • Members may vote on any candidate at any time.
  • The "clear result" of voting is when there are twice as many votes for either approval or rejection as there are for the alternative.
  • There is no "clear result" of voting if neither the votes for approval nor the votes for rejection are more than twice as great as the other.
  • A candidate may always resign their candidacy at any time, but this incurs the loss of their deposit.
  • If they do not, then their candidacy may not end until the voting period is elapsed.
  • Once the voting period is elapsed, they may elevate themselves to become a member if the clear result of voting is in approval.
  • Once the following intake round is ended, then anyone may cancel the candidate's candidacy if there is a clear rejection of the candidate among voters.
  • Once the voting period is elapsed, the founder may elevate them to member if there is not a clear rejection of the candidate among voters.
  • Similarly, once the voting period is elapsed, the founder may cancel their candidacy if there is not a clear approval of the candidate among voters.
  • At any point once the voting period is over, the Skeptic can be forced by anyone to receive a strike for any candidate on which they either did not vote or did vote but voted against the clear result (if there was one). The Skeptic may only be punished once for any candidate.

Member challenges:

  • At every Challenge Period, a member is selected to become the new Defender and another member is selected at random to become the Challenge Skeptic.
    • The Defender is generally selected at random from the members.
    • However, if the previous Challenge Skeptic received a strike for missing or invalid voting, then they become the Defender.
  • Prior to the commencement of the next Challenge Period, any member may vote on the continued validity of the member's membership.
  • The Challenge Skeptic receives a strike and becomes the next Defender if they did not vote or if their vote is against the clear result of the voters (if any).
  • A random member is selected as the next Defender if the Challenge Skeptic's vote is present and valid.
  • If the clear result of the voters is to reject the Defender's evidence of continued eligibility for membership, then the Defender is removed as a member and placed in the SuspendedMembers list. The founder may restore their membership or remove them as before.

New dispatchables to be integrated in the UI (CC @jacogr):

  • waive_repay: Attempt to clear any payouts and promote the member to rank 1. This transfers the given amount of the sender's balance back into the Society. This will only work if the given amount is at least as great as the member's debt to the society.
  • punish_skeptic: Callable by a candidate after the voting period has ended to ensure that the Skeptic is punished for inaction or invalid voting.
  • claim_membership: Callable by a candidate to claim their membership after the voting period has ended and when there is a clear result of approval.
  • bestow_membership: Callable by the Founder to elevate a candidate to member after the voting period has ended and when there is not a clear result of rejection.
  • kick_candidate: Callable by the Founder to reject a candidate after the voting period has ended and when there is not a clear result of approval. The candidate loses their deposit.
  • resign_candidacy: Callable by a candidate to end their candidacy. The candidate loses their deposit.
  • drop_candidate: Callable by anyone to end a candidate's candidacy after the intake period following the one in which the candidate applied and only when there is a clear result of rejection. The candidate loses their deposit.

TODO:

  • Repay / Waive
  • Rank rules
  • Old tests
  • New test(s)?
  • Migration
  • Prove that the remove_all(None) calls are bounded.
  • Tidy any old rank/group stuff.

@gavofyork gavofyork added A3-in_progress Pull request is in progress. No review needed at this stage. B7-runtimenoteworthy 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 Apr 30, 2022
@gavofyork gavofyork added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels May 5, 2022
@gavofyork gavofyork marked this pull request as ready for review May 5, 2022 15:27
bkchr and others added 7 commits May 10, 2022 14:10
This rewrites the `generate_storage_alias!` declarative macro as proc-macro attribute. While doing
this the name is changed to `storage_alias`. The prefix can now also be the name of a pallet. This
makes storage aliases work in migrations for all kind of chains and not just for the ones that use
predefined prefixes.
@paritytech-processbot
Copy link

Rebased

frame/society/src/benchmarking.rs Outdated Show resolved Hide resolved
frame/society/src/migrations.rs Outdated Show resolved Hide resolved
frame/society/src/weights.rs Show resolved Hide resolved
Copy link
Contributor

@sam0x17 sam0x17 left a comment

Choose a reason for hiding this comment

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

LGTM, though wondering why the later call indexes don't have complexity notation like the earlier ones do? I find that sort of thing really helpful when reviewing an unfamiliar function/algorithm/procedure so would be cool if every call had complexity notation IMO

/// pending payments, and elevate them from rank 0 to rank 1.
#[pallet::call_index(7)]
#[pallet::weight(T::WeightInfo::waive_repay())]
pub fn waive_repay(origin: OriginFor<T>, amount: BalanceOf<T, I>) -> DispatchResult {
Copy link
Contributor

@sam0x17 sam0x17 Jun 15, 2023

Choose a reason for hiding this comment

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

Could we get complexity notation for this one and later ones as well since we have it on the earlier calls?

Copy link
Contributor

Choose a reason for hiding this comment

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

Or the other way, I find most of these complexity/weight comments pretty useless. Even if they should exist, it should be in the benchmarking code, not in the call documentation (where end users will see it through metadata).

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed - they're actually mostly old and incorrect. As can be seen from the weight calls, they're all O(1) now. The only caveat to that is that BoundedVecs in storage items must generally be full to ensure the maximum possible weight is determined. But I don't think that's especially useful to have in the function docs since it's a) fairly obvious; and b) as closely related to the storage item itself as it is the function.

frame/society/src/migrations.rs Outdated Show resolved Hide resolved
frame/society/src/migrations.rs Show resolved Hide resolved
frame/society/src/lib.rs Show resolved Hide resolved
@gavofyork
Copy link
Member Author

bot merge

@paritytech-processbot
Copy link

Error: "Check reviews" status is not passing for paritytech/polkadot#7356

@gavofyork gavofyork merged commit ae1a608 into master Jun 18, 2023
@gavofyork gavofyork deleted the gav-society-v2 branch June 18, 2023 16:22
rossbulat pushed a commit that referenced this pull request Jun 19, 2023
* New Society

* More logic drafting

* More work

* Building

* Some tests

* Fixes

* Improvements to the voting process

* More tests

* Test number 20

* Tests

* 30 tests

* Another test]

* All tests enabled

* Minor stuff

* generate_storage_alias: Rewrite as proc macro attribute

This rewrites the `generate_storage_alias!` declarative macro as proc-macro attribute. While doing
this the name is changed to `storage_alias`. The prefix can now also be the name of a pallet. This
makes storage aliases work in migrations for all kind of chains and not just for the ones that use
predefined prefixes.

* Maintenance operations don't pay fee

* Fix compilation and FMT

* Moare fixes

* Migrations

* Fix tests and add migration testing

* Introduce lazy-cleanup and avoid unbounded prefix removal

* Fixes

* Fixes

* [WIP][Society] Adding benchmarking to the v2. (#11776)

* [Society] Adding benchmarking to the v2.

* [Society] Code review.

* [Society] Better code.

* Using clear() + clear_prefix() and adding more tests.

* Benchmarking again...

* Fix Cargo

* Fixes

* Fixes

* Spelling

* Fix benchmarks

* Another fix

* Remove println

---------

Co-authored-by: Bastian Köcher <[email protected]>
Co-authored-by: Artur Gontijo <[email protected]>
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
* New Society

* More logic drafting

* More work

* Building

* Some tests

* Fixes

* Improvements to the voting process

* More tests

* Test number 20

* Tests

* 30 tests

* Another test]

* All tests enabled

* Minor stuff

* generate_storage_alias: Rewrite as proc macro attribute

This rewrites the `generate_storage_alias!` declarative macro as proc-macro attribute. While doing
this the name is changed to `storage_alias`. The prefix can now also be the name of a pallet. This
makes storage aliases work in migrations for all kind of chains and not just for the ones that use
predefined prefixes.

* Maintenance operations don't pay fee

* Fix compilation and FMT

* Moare fixes

* Migrations

* Fix tests and add migration testing

* Introduce lazy-cleanup and avoid unbounded prefix removal

* Fixes

* Fixes

* [WIP][Society] Adding benchmarking to the v2. (paritytech#11776)

* [Society] Adding benchmarking to the v2.

* [Society] Code review.

* [Society] Better code.

* Using clear() + clear_prefix() and adding more tests.

* Benchmarking again...

* Fix Cargo

* Fixes

* Fixes

* Spelling

* Fix benchmarks

* Another fix

* Remove println

---------

Co-authored-by: Bastian Köcher <[email protected]>
Co-authored-by: Artur Gontijo <[email protected]>
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. B1-note_worthy Changes should be noted in the 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. T1-runtime This PR/Issue is related to the topic “runtime”.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Introduce FromEntropy trait to replace dummy values
8 participants