Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
98 commits
Select commit Hold shift + click to select a range
dd4607b
Fix elections-phragmen and proxy issue
kianenigma Sep 7, 2020
6bd6221
remove TODO
kianenigma Sep 7, 2020
57de14e
Update bond to be per-vote
kianenigma Sep 8, 2020
684802e
Update frame/elections-phragmen/src/lib.rs
kianenigma Sep 9, 2020
70852fa
Fix benchmakrs
kianenigma Sep 9, 2020
4b7d0a6
Merge branch 'kiz-fix-proxy-election-drain' of github.com:paritytech/…
kianenigma Sep 9, 2020
77caa5c
Fix weight as well.
kianenigma Sep 9, 2020
06a0aa0
Add license
kianenigma Sep 9, 2020
9943042
Make weight interpreted wasm! 🤦🏻‍♂️
kianenigma Sep 9, 2020
f9bd4a3
Merge branch 'master' into kiz-fix-proxy-election-drain
gavofyork Sep 14, 2020
f9ea73a
Remove a bunch of TODOs
kianenigma Sep 14, 2020
24c416b
Add migration
kianenigma Sep 14, 2020
10e3687
Better storage version.
kianenigma Sep 14, 2020
4495ead
Functionify.
kianenigma Sep 14, 2020
3c1a2ef
Fix deposit scheme.
kianenigma Sep 17, 2020
1b95224
remove legacy bond.
kianenigma Sep 17, 2020
2c677c2
Master.into()
kianenigma Sep 17, 2020
d2ec181
Master.into()
kianenigma Sep 17, 2020
a628850
better logging.
kianenigma Sep 17, 2020
f6189d5
Fix benchmarking test
kianenigma Sep 17, 2020
e812815
Fix confused deposit collection.
kianenigma Sep 18, 2020
498376c
Add fine
kianenigma Sep 21, 2020
d9b2204
Merge branch 'master' of github.com:paritytech/substrate into kiz-fix…
kianenigma Sep 21, 2020
4691703
Merge branch 'master' of github.com:paritytech/substrate into kiz-fix…
kianenigma Sep 21, 2020
88db61d
Master.into()
kianenigma Sep 23, 2020
b4e2a7c
Master.into()
kianenigma Oct 12, 2020
c871c81
Better name for storage item
kianenigma Oct 12, 2020
12c18d5
Fix name again.
kianenigma Oct 12, 2020
f900b68
remove unused
kianenigma Oct 12, 2020
6c942b4
Update frame/elections-phragmen/src/lib.rs
kianenigma Oct 13, 2020
8e3c2eb
Update frame/elections-phragmen/src/lib.rs
kianenigma Oct 13, 2020
e674a60
Merge branch 'master' of github.com:paritytech/substrate into kiz-fix…
kianenigma Oct 13, 2020
a99bebc
Merge remote-tracking branch 'origin/master' into kiz-fix-proxy-elect…
shawntabrizi Oct 13, 2020
f5700cf
cargo run --release --features runtime-benchmarks --manifest-path bin…
shawntabrizi Oct 13, 2020
0564e59
new weight fns
kianenigma Oct 13, 2020
60d7d38
Merge branch 'kiz-fix-proxy-election-drain' of github.com:paritytech/…
kianenigma Oct 13, 2020
1b48d0d
Fix build
kianenigma Oct 13, 2020
960a474
Fix line width
kianenigma Oct 13, 2020
19e9436
fix benchmakrs
kianenigma Oct 13, 2020
76cb36f
fix warning
kianenigma Oct 13, 2020
820d76f
cargo run --release --features runtime-benchmarks --manifest-path bin…
Oct 13, 2020
4caea9f
Tune the stake again
kianenigma Oct 13, 2020
0c1043f
Merge branch 'kiz-fix-proxy-election-drain' of github.com:paritytech/…
kianenigma Oct 13, 2020
df267fa
cargo run --release --features runtime-benchmarks --manifest-path bin…
Oct 13, 2020
1621131
Merge branch 'master' of github.com:paritytech/substrate into kiz-fix…
kianenigma Oct 22, 2020
c79b522
Merge branch 'kiz-fix-proxy-election-drain' of github.com:paritytech/…
kianenigma Oct 22, 2020
50fa257
Fix builds
kianenigma Nov 17, 2020
8bd074f
All tests work again.
kianenigma Nov 18, 2020
29880e4
A large number of fixes.
kianenigma Nov 18, 2020
d78f6bb
Fix event as well.
kianenigma Nov 18, 2020
5d09983
more fixes.
kianenigma Nov 19, 2020
6668ac1
Fix node build
kianenigma Nov 19, 2020
3dd01d8
Some fixes to benchmarks
kianenigma Nov 19, 2020
694e42a
Merge remote-tracking branch 'origin/master' into kiz-fix-proxy-elect…
Nov 19, 2020
cf94f49
Fix some warnings.
kianenigma Nov 19, 2020
ea69a54
cargo run --release --features=runtime-benchmarks --manifest-path=bin…
Nov 20, 2020
b2f36bd
cargo run --release --features=runtime-benchmarks --manifest-path=bin…
Nov 20, 2020
1fd2b08
Update frame/elections-phragmen/src/lib.rs
kianenigma Nov 25, 2020
585e599
a batch of review comments.
kianenigma Nov 25, 2020
4b9b207
Merge branch 'kiz-fix-proxy-election-drain' of github.com:paritytech/…
kianenigma Nov 25, 2020
ea17535
Master.into()
kianenigma Nov 25, 2020
4b34e72
Fix a test.
kianenigma Nov 25, 2020
ecb0e4e
Fix some more tests.
kianenigma Nov 25, 2020
e810cc7
do migration with pallet version???
kianenigma Nov 25, 2020
e650c02
Master.into()
kianenigma Nov 25, 2020
8742865
Final touches.
kianenigma Nov 26, 2020
d6de615
Remove unused storage.
kianenigma Nov 27, 2020
6cae3da
another rounds of changes and fixes.
kianenigma Nov 30, 2020
3c0d341
Update frame/elections-phragmen/src/lib.rs
kianenigma Nov 30, 2020
3ca1590
Update frame/elections-phragmen/src/lib.rs
kianenigma Nov 30, 2020
9c9819b
Review grumbles.
kianenigma Nov 30, 2020
68ff142
Merge branch 'kiz-fix-proxy-election-drain' of github.com:paritytech/…
kianenigma Nov 30, 2020
797d04c
Fix ocnflics
kianenigma Dec 2, 2020
73a3537
Fix a bit more.
kianenigma Dec 2, 2020
98f349e
Fix build
kianenigma Dec 2, 2020
9454a68
Experimental: independent migration.
kianenigma Dec 2, 2020
c750f74
WIP: isolated migration logics
gui1117 Dec 2, 2020
e11a216
clean up.
kianenigma Dec 2, 2020
1e2c8e9
make migration struct private and move migration to own file
gui1117 Dec 3, 2020
583aa82
add doc
gui1117 Dec 3, 2020
dca4bd5
Merge remote-tracking branch 'origin/master' into kiz-fix-proxy-elect…
gui1117 Dec 3, 2020
c90d25a
Merge remote-tracking branch 'origin/master' into kiz-fix-proxy-elect…
gui1117 Dec 3, 2020
586c61c
fix StorageInstance new syntax
gui1117 Dec 3, 2020
0a0ef71
Update frame/elections-phragmen/src/migrations_3_0_0.rs
gui1117 Dec 4, 2020
a205401
Master.into()
kianenigma Dec 17, 2020
63a4c56
another round of self-review.
kianenigma Dec 17, 2020
ec067c9
bit better formatting
kianenigma Dec 17, 2020
5dab8bc
Merge remote-tracking branch 'origin/master' into kiz-fix-proxy-elect…
Dec 17, 2020
32cb6c1
cargo run --release --features=runtime-benchmarks --manifest-path=bin…
Dec 17, 2020
38881e0
Fix tests.
kianenigma Dec 21, 2020
e4d7464
Master.into()
kianenigma Dec 21, 2020
79d1d5b
Master.into()
kianenigma Jan 19, 2021
7d95b15
Round of self-review
kianenigma Jan 20, 2021
ba12aec
Clean migrations
kianenigma Jan 20, 2021
673aed7
Merge remote-tracking branch 'origin/master' into kiz-fix-proxy-elect…
Jan 20, 2021
99f5fd2
cargo run --release --features=runtime-benchmarks --manifest-path=bin…
Jan 20, 2021
92dbabe
Revert unwanted change to construct-runtime
kianenigma Jan 20, 2021
c688a6f
Merge branch 'kiz-fix-proxy-election-drain' of github.com:paritytech/…
kianenigma Jan 20, 2021
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
Prev Previous commit
Next Next commit
Update bond to be per-vote
  • Loading branch information
kianenigma committed Sep 8, 2020
commit 57de14ea00fddbb4b8c454d558f4a23b59249742
5 changes: 4 additions & 1 deletion bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,8 @@ impl pallet_collective::Trait<CouncilCollective> for Runtime {

parameter_types! {
pub const CandidacyBond: Balance = 10 * DOLLARS;
pub const VotingBondBase: Balance = 1 * DOLLARS;
pub const VotingBondFactor: Balance = 50 * CENTS; // per 32 bytes on-chain
pub const VotingBond: Balance = 1 * DOLLARS;
pub const TermDuration: BlockNumber = 7 * DAYS;
pub const DesiredMembers: u32 = 13;
Expand All @@ -552,7 +554,8 @@ impl pallet_elections_phragmen::Trait for Runtime {
type InitializeMembers = Council;
type CurrencyToVote = CurrencyToVoteHandler;
type CandidacyBond = CandidacyBond;
type VotingBond = VotingBond;
type VotingBondBase = VotingBondBase;
type VotingBondFactor = VotingBondFactor;
type LoserCandidate = ();
type BadReport = ();
type KickedMember = ();
Expand Down
116 changes: 103 additions & 13 deletions frame/elections-phragmen/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@
#![cfg_attr(not(feature = "std"), no_std)]

use codec::{Encode, Decode};
use sp_std::prelude::*;
use sp_std::{prelude::*, cmp::Ordering};
use sp_runtime::{
DispatchError, RuntimeDebug, Perbill,
traits::{Zero, StaticLookup, Convert, Saturating},
Expand Down Expand Up @@ -198,7 +198,10 @@ pub trait Trait: frame_system::Trait {
///
/// This should be sensibly high to economically ensure the pallet cannot be attacked by
/// creating a gigantic number of votes.
type VotingBond: Get<BalanceOf<Self>>;
type VotingBondBase: Get<BalanceOf<Self>>;

/// The amount of bond that need to be locked for each vote (32 bytes)
type VotingBondFactor: Get<BalanceOf<Self>>;

/// Handler for the unbalanced reduction when a candidate has lost (and is not a runner-up)
type LoserCandidate: OnUnbalanced<NegativeImbalanceOf<Self>>;
Expand Down Expand Up @@ -328,7 +331,8 @@ decl_module! {
fn deposit_event() = default;

const CandidacyBond: BalanceOf<T> = T::CandidacyBond::get();
const VotingBond: BalanceOf<T> = T::VotingBond::get();
const VotingBondBase: BalanceOf<T> = T::VotingBondBase::get();
const VotingBondFactor: BalanceOf<T> = T::VotingBondFactor::get();
const DesiredMembers: u32 = T::DesiredMembers::get();
const DesiredRunnersUp: u32 = T::DesiredRunnersUp::get();
const TermDuration: T::BlockNumber = T::TermDuration::get();
Expand Down Expand Up @@ -384,10 +388,29 @@ decl_module! {

ensure!(value > T::Currency::minimum_balance(), Error::<T>::LowBalance);

// first time voter. Reserve bond.
if !Self::is_voter(&who) {
T::Currency::reserve(&who, T::VotingBond::get())
// Reserve bond.
let (_, old_votes) = <Voting<T>>::get(&who);
if old_votes.is_empty() {
// First time voter. Get full deposit.
T::Currency::reserve(&who, Self::deposit_of(votes.len()))
.map_err(|_| Error::<T>::UnableToPayBond)?;
} else {
// Old voter.
match votes.len().cmp(&old_votes.len()) {
Copy link
Member

@gavofyork gavofyork Sep 18, 2020

Choose a reason for hiding this comment

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

This is bad.

Instead of messing with lens, just calculate the new amount to be reserved, and find the difference with what has currently been reserved. If it's greater, reserve the difference. If it's less, unreserve the differrence.

This way even if the calculation/constants for deposit_factor_of change between votes, you'll still ensure that the latest prices are the ones that the user pays.

If you don't do that, then suppose the prices go up 2x between votes: A user could make 10 votes before, replace them with 5 votes later, yet be fully unreserved, and still have 5 votes in the system.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right this logic was made with the assumption of not storing the deposit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed, thanks!

Ordering::Greater => {
// Must reserve a bit more.
let to_reserve = Self::deposit_factor_of(votes.len() - old_votes.len());
T::Currency::reserve(&who, to_reserve)
.map_err(|_| Error::<T>::UnableToPayBond)?;
},
Ordering::Equal => {},
Ordering::Less => {
// Must unreserve a bit.
let to_unreserve = Self::deposit_factor_of(old_votes.len() - votes.len());
let _remainder = T::Currency::unreserve(&who, to_unreserve);
debug_assert!(_remainder.is_zero());
},
}
}

// Amount to be locked up.
Expand Down Expand Up @@ -727,6 +750,18 @@ decl_event!(
);

impl<T: Trait> Module<T> {
/// The deposit value of `count` votes.
fn deposit_of(count: usize) -> BalanceOf<T> {
T::VotingBondBase::get().saturating_add(
T::VotingBondFactor::get().saturating_mul((count as u32).into())
)
}

/// Same as [`deposit_of`] but only returns the factor deposit.
fn deposit_factor_of(count: usize) -> BalanceOf<T> {
T::VotingBondFactor::get().saturating_mul((count as u32).into())
}

/// Attempts to remove a member `who`. If a runner-up exists, it is used as the replacement and
/// Ok(true). is returned.
///
Expand Down Expand Up @@ -843,12 +878,14 @@ impl<T: Trait> Module<T> {
///
/// DB access: Voting and Lock are always written to, if unreserve, then 1 read and write added.
fn do_remove_voter(who: &T::AccountId, unreserve: bool) {
let (_, votes) = <Voting<T>>::get(who);
// remove storage and lock.
<Voting<T>>::remove(who);
T::Currency::remove_lock(T::ModuleId::get(), who);

if unreserve {
T::Currency::unreserve(who, T::VotingBond::get());
let _remainder = T::Currency::unreserve(who, Self::deposit_of(votes.len()));
debug_assert!(_remainder.is_zero());
}
}

Expand Down Expand Up @@ -1163,15 +1200,21 @@ mod tests {
}

thread_local! {
static VOTING_BOND: RefCell<u64> = RefCell::new(2);
static VOTING_BOND_BASE: RefCell<u64> = RefCell::new(2);
static VOTING_BOND_FACTOR: RefCell<u64> = RefCell::new(0);
static DESIRED_MEMBERS: RefCell<u32> = RefCell::new(2);
static DESIRED_RUNNERS_UP: RefCell<u32> = RefCell::new(2);
static TERM_DURATION: RefCell<u64> = RefCell::new(5);
}

pub struct VotingBond;
impl Get<u64> for VotingBond {
fn get() -> u64 { VOTING_BOND.with(|v| *v.borrow()) }
pub struct VotingBondBase;
impl Get<u64> for VotingBondBase {
fn get() -> u64 { VOTING_BOND_BASE.with(|v| *v.borrow()) }
}

pub struct VotingBondFactor;
impl Get<u64> for VotingBondFactor {
fn get() -> u64 { VOTING_BOND_FACTOR.with(|v| *v.borrow()) }
}

pub struct DesiredMembers;
Expand Down Expand Up @@ -1257,7 +1300,8 @@ mod tests {
type ChangeMembers = TestChangeMembers;
type InitializeMembers = ();
type CandidacyBond = CandidacyBond;
type VotingBond = VotingBond;
type VotingBondBase = VotingBondBase;
type VotingBondFactor = VotingBondFactor;
type TermDuration = TermDuration;
type DesiredMembers = DesiredMembers;
type DesiredRunnersUp = DesiredRunnersUp;
Expand Down Expand Up @@ -1286,6 +1330,7 @@ mod tests {
genesis_members: Vec<(u64, u64)>,
balance_factor: u64,
voter_bond: u64,
voter_bond_factor: u64,
term_duration: u64,
desired_runners_up: u32,
desired_members: u32,
Expand All @@ -1297,6 +1342,7 @@ mod tests {
genesis_members: vec![],
balance_factor: 1,
voter_bond: 2,
voter_bond_factor: 0,
term_duration: 5,
desired_runners_up: 0,
desired_members: 2,
Expand All @@ -1309,6 +1355,10 @@ mod tests {
self.voter_bond = fee;
self
}
pub fn voter_bond_factor(mut self, fee: u64) -> Self {
self.voter_bond_factor = fee;
self
}
pub fn desired_runners_up(mut self, count: u32) -> Self {
self.desired_runners_up = count;
self
Expand All @@ -1331,7 +1381,8 @@ mod tests {
self
}
fn set_constants(&self) {
VOTING_BOND.with(|v| *v.borrow_mut() = self.voter_bond);
VOTING_BOND_BASE.with(|v| *v.borrow_mut() = self.voter_bond);
VOTING_BOND_FACTOR.with(|v| *v.borrow_mut() = self.voter_bond_factor);
TERM_DURATION.with(|v| *v.borrow_mut() = self.term_duration);
DESIRED_RUNNERS_UP.with(|v| *v.borrow_mut() = self.desired_runners_up);
DESIRED_MEMBERS.with(|m| *m.borrow_mut() = self.desired_members);
Expand Down Expand Up @@ -1718,6 +1769,45 @@ mod tests {
});
}

#[test]
fn voting_reserves_per_bond_per_vote() {
ExtBuilder::default().voter_bond_factor(1).build_and_execute(|| {
assert_eq!(balances(&2), (20, 0));

assert_ok!(submit_candidacy(Origin::signed(5)));
assert_ok!(submit_candidacy(Origin::signed(4)));

// initial vote.
assert_ok!(vote(Origin::signed(2), vec![5], 10));

// 2 + 1
assert_eq!(balances(&2), (17, 3));
assert_eq!(has_lock(&2), 10);
assert_eq!(Elections::locked_stake_of(&2), 10);

// can update; different stake; different lock and reserve.
assert_ok!(vote(Origin::signed(2), vec![5, 4], 15));
// 2 + 2
assert_eq!(balances(&2), (16, 4));
assert_eq!(has_lock(&2), 15);
assert_eq!(Elections::locked_stake_of(&2), 15);

// stay at two votes with different stake.
assert_ok!(vote(Origin::signed(2), vec![5, 3], 18));
// 2 + 2
assert_eq!(balances(&2), (16, 4));
assert_eq!(has_lock(&2), 18);
assert_eq!(Elections::locked_stake_of(&2), 18);

// back to 1 vote.
assert_ok!(vote(Origin::signed(2), vec![4], 12));
// 2 + 1
assert_eq!(balances(&2), (17, 3));
assert_eq!(has_lock(&2), 12);
assert_eq!(Elections::locked_stake_of(&2), 12);
});
}

#[test]
fn cannot_vote_for_no_candidate() {
ExtBuilder::default().build_and_execute(|| {
Expand Down