Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix vote weights of ranked members in the Society pallet #2758

Merged
merged 8 commits into from
Jan 4, 2024
Merged

Fix vote weights of ranked members in the Society pallet #2758

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9094e2d
Fix tally accrual for ranked members and improve tests
laurogripa Dec 19, 2023
eaa9726
Merge branch 'paritytech:master' into master
laurogripa Dec 19, 2023
eecbf08
Merge branch 'master' into master
bkchr Dec 22, 2023
26aab2d
".git/.scripts/commands/fmt/fmt.sh"
Dec 22, 2023
00c0f00
Add prdoc
laurogripa Dec 22, 2023
3ff0360
Merge branch 'master' into master
laurogripa Dec 22, 2023
006ae58
Update prdoc/pr_2758.prdoc
bkchr Jan 4, 2024
71342fc
Merge branch 'master' into master
bkchr Jan 4, 2024
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
4 changes: 2 additions & 2 deletions substrate/frame/society/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1433,8 +1433,8 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
let weight_root = rank + 1;
let weight = weight_root * weight_root;
match approve {
true => tally.approvals.saturating_accrue(1),
false => tally.rejections.saturating_accrue(1),
true => tally.approvals.saturating_accrue(weight),
false => tally.rejections.saturating_accrue(weight),
}
Vote { approve, weight }
}
Expand Down
71 changes: 44 additions & 27 deletions substrate/frame/society/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -720,7 +720,7 @@ fn unvouch_works() {
// But their pick doesn't resign (yet).
conclude_intake(false, None);
// Voting still happening and voucher cannot unvouch.
assert_eq!(candidacies(), vec![(20, candidacy(1, 100, Vouch(10, 0), 0, 1))]);
assert_eq!(candidacies(), vec![(20, candidacy(1, 100, Vouch(10, 0), 0, 4))]);
assert_eq!(Members::<Test>::get(10).unwrap().vouching, Some(VouchingStatus::Vouching));

// Candidate gives in and resigns.
Expand Down Expand Up @@ -836,72 +836,72 @@ fn founder_and_head_cannot_be_removed() {
fn challenges_work() {
EnvBuilder::new().execute(|| {
// Add some members
place_members([20, 30, 40]);
place_members([20, 30, 40, 50]);
// Votes are empty
assert_eq!(DefenderVotes::<Test>::get(0, 10), None);
assert_eq!(DefenderVotes::<Test>::get(0, 20), None);
assert_eq!(DefenderVotes::<Test>::get(0, 30), None);
assert_eq!(DefenderVotes::<Test>::get(0, 40), None);
assert_eq!(DefenderVotes::<Test>::get(0, 50), None);
// Check starting point
assert_eq!(members(), vec![10, 20, 30, 40]);
assert_eq!(members(), vec![10, 20, 30, 40, 50]);
assert_eq!(Defending::<Test>::get(), None);

// 30 will be challenged during the challenge rotation
// 40 will be challenged during the challenge rotation
next_challenge();
assert_eq!(Defending::<Test>::get().unwrap().0, 30);
assert_eq!(Defending::<Test>::get().unwrap().0, 40);
// They can always free vote for themselves
assert_ok!(Society::defender_vote(Origin::signed(30), true));
assert_ok!(Society::defender_vote(Origin::signed(40), true));

// If no one else votes, nothing happens
next_challenge();
assert_eq!(members(), vec![10, 20, 30, 40]);
assert_eq!(members(), vec![10, 20, 30, 40, 50]);
// Reset votes for last challenge
assert_ok!(Society::cleanup_challenge(Origin::signed(0), 0, 10));
// New challenge period
assert_eq!(Defending::<Test>::get().unwrap().0, 30);
// New challenge period, 20 is challenged
assert_eq!(Defending::<Test>::get().unwrap().0, 20);
// Non-member cannot vote
assert_noop!(Society::defender_vote(Origin::signed(1), true), Error::<Test>::NotMember);
// 3 people say accept, 1 reject
assert_ok!(Society::defender_vote(Origin::signed(10), true));
assert_ok!(Society::defender_vote(Origin::signed(20), true));
assert_ok!(Society::defender_vote(Origin::signed(30), true));
assert_ok!(Society::defender_vote(Origin::signed(40), false));
assert_ok!(Society::defender_vote(Origin::signed(40), true));
assert_ok!(Society::defender_vote(Origin::signed(50), false));

next_challenge();
// 30 survives
assert_eq!(members(), vec![10, 20, 30, 40]);
// 20 survives
assert_eq!(members(), vec![10, 20, 30, 40, 50]);
// Reset votes for last challenge
assert_ok!(Society::cleanup_challenge(Origin::signed(0), 1, 10));
// Votes are reset
assert_eq!(DefenderVotes::<Test>::get(0, 10), None);
assert_eq!(DefenderVotes::<Test>::get(0, 20), None);
assert_eq!(DefenderVotes::<Test>::get(0, 30), None);
assert_eq!(DefenderVotes::<Test>::get(0, 40), None);
assert_eq!(DefenderVotes::<Test>::get(0, 50), None);

// One more time
assert_eq!(Defending::<Test>::get().unwrap().0, 30);
// One more time, 40 is challenged
assert_eq!(Defending::<Test>::get().unwrap().0, 40);
// 2 people say accept, 2 reject
assert_ok!(Society::defender_vote(Origin::signed(10), true));
assert_ok!(Society::defender_vote(Origin::signed(20), true));
assert_ok!(Society::defender_vote(Origin::signed(30), false));
assert_ok!(Society::defender_vote(Origin::signed(30), true));
assert_ok!(Society::defender_vote(Origin::signed(40), false));
assert_ok!(Society::defender_vote(Origin::signed(50), false));

next_challenge();
// 30 is suspended
assert_eq!(members(), vec![10, 20, 40]);
// 40 is suspended
assert_eq!(members(), vec![10, 20, 30, 50]);
assert_eq!(
SuspendedMembers::<Test>::get(30),
Some(MemberRecord { rank: 0, strikes: 0, vouching: None, index: 2 })
SuspendedMembers::<Test>::get(40),
Some(MemberRecord { rank: 0, strikes: 0, vouching: None, index: 3 })
);
// Reset votes for last challenge
assert_ok!(Society::cleanup_challenge(Origin::signed(0), 2, 10));
// New defender is chosen
assert_eq!(Defending::<Test>::get().unwrap().0, 20);
// New defender is chosen, 30 is challenged
assert_eq!(Defending::<Test>::get().unwrap().0, 30);
// Votes are reset
assert_eq!(DefenderVotes::<Test>::get(0, 10), None);
assert_eq!(DefenderVotes::<Test>::get(0, 20), None);
assert_eq!(DefenderVotes::<Test>::get(0, 30), None);
assert_eq!(DefenderVotes::<Test>::get(0, 40), None);
assert_eq!(DefenderVotes::<Test>::get(0, 50), None);
});
}

Expand Down Expand Up @@ -1044,6 +1044,9 @@ fn vouching_handles_removed_member_with_candidate() {
fn votes_are_working() {
EnvBuilder::new().execute(|| {
place_members([20]);
// Member 10 is rank 1 and Member 20 is rank 0
assert_eq!(Members::<Test>::get(10).unwrap().rank, 1);
assert_eq!(Members::<Test>::get(20).unwrap().rank, 0);
// Users make bids of various amounts
assert_ok!(Society::bid(RuntimeOrigin::signed(50), 500));
assert_ok!(Society::bid(RuntimeOrigin::signed(40), 400));
Expand All @@ -1061,6 +1064,20 @@ fn votes_are_working() {
assert_eq!(Votes::<Test>::get(30, 20), Some(Vote { approve: true, weight: 1 }));
assert_eq!(Votes::<Test>::get(40, 10), Some(Vote { approve: true, weight: 4 }));
assert_eq!(Votes::<Test>::get(50, 10), None);

// Votes have correct weights on the tally
assert_eq!(Candidates::<Test>::get(30).unwrap().tally, Tally { approvals: 5, rejections: 0 });
assert_eq!(Candidates::<Test>::get(40).unwrap().tally, Tally { approvals: 4, rejections: 0 });
// Member 10 changes his vote for Candidate 30
assert_ok!(Society::vote(Origin::signed(10), 30, false));
// Assert the tally calculation is correct
assert_eq!(Candidates::<Test>::get(30).unwrap().tally, Tally { approvals: 1, rejections: 4 });
// Member 10 changes his vote again
assert_ok!(Society::vote(Origin::signed(10), 30, true));
// Assert the tally is still correct
assert_eq!(Candidates::<Test>::get(30).unwrap().tally, Tally { approvals: 5, rejections: 0});

// Finish intake
conclude_intake(false, None);
// Cleanup the candidacy
assert_ok!(Society::cleanup_candidacy(Origin::signed(0), 30, 10));
Expand Down Expand Up @@ -1240,7 +1257,7 @@ fn waive_repay_works() {
Payouts::<Test>::get(20),
PayoutRecord { paid: 0, payouts: vec![].try_into().unwrap() }
);
assert_eq!(Members::<Test>::get(10).unwrap().rank, 1);
assert_eq!(Members::<Test>::get(20).unwrap().rank, 1);
assert_eq!(Balances::free_balance(20), 50);
});
}
Expand Down
Loading