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

Commit

Permalink
Additional zero bid logic for Society pallet (#4604)
Browse files Browse the repository at this point in the history
* Select only one zero bid, better ordering of new bids

* Select zero bid as head

* Update comment

* Update frame/society/src/tests.rs

* Add new test, logic not updated yet

* Implement `put_bid` to order same value bids

Note that this extra logic currently does not do anything per the implementation of `binary_search` in Rust.
  • Loading branch information
shawntabrizi authored and gavofyork committed Jan 13, 2020
1 parent 3ed7c8b commit 70f1d8d
Show file tree
Hide file tree
Showing 2 changed files with 149 additions and 22 deletions.
106 changes: 84 additions & 22 deletions frame/society/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1108,7 +1108,36 @@ impl<T: Trait<I>, I: Instance> Module<T, I> {
const MAX_BID_COUNT: usize = 1000;

match bids.binary_search_by(|bid| bid.value.cmp(&value)) {
Ok(pos) | Err(pos) => bids.insert(pos, Bid {
// Insert new elements after the existing ones. This ensures new bids
// with the same bid value are further down the list than existing ones.
Ok(pos) => {
let different_bid = bids.iter()
// Easily extract the index we are on
.enumerate()
// Skip ahead to the suggested position
.skip(pos)
// Keep skipping ahead until the position changes
.skip_while(|(_, x)| x.value <= bids[pos].value)
// Get the element when things changed
.next();
// If the element is not at the end of the list, insert the new element
// in the spot.
if let Some((p, _)) = different_bid {
bids.insert(p, Bid {
value,
who: who.clone(),
kind: bid_kind,
});
// If the element is at the end of the list, push the element on the end.
} else {
bids.push(Bid {
value,
who: who.clone(),
kind: bid_kind,
});
}
},
Err(pos) => bids.insert(pos, Bid {
value,
who: who.clone(),
kind: bid_kind,
Expand Down Expand Up @@ -1264,7 +1293,7 @@ impl<T: Trait<I>, I: Instance> Module<T, I> {
// We track here the total_approvals so that every candidate has a unique range
// of numbers from 0 to `total_approvals` with length `approval_count` so each
// candidate is proportionally represented when selecting a "primary" below.
Some((candidate, total_approvals))
Some((candidate, total_approvals, value))
} else {
// Suspend Candidate
<SuspendedCandidates<T, I>>::insert(&candidate, (value, kind));
Expand Down Expand Up @@ -1299,8 +1328,8 @@ impl<T: Trait<I>, I: Instance> Module<T, I> {
// select one as primary, randomly chosen from the accepted, weighted by approvals.
// Choose a random number between 0 and `total_approvals`
let primary_point = pick_usize(&mut rng, total_approvals - 1);
// Find the user who falls on that point
let primary = accepted.iter().find(|e| e.1 > primary_point)
// Find the zero bid or the user who falls on that point
let primary = accepted.iter().find(|e| e.2.is_zero() || e.1 > primary_point)
.expect("e.1 of final item == total_approvals; \
worst case find will always return that item; qed")
.0.clone();
Expand Down Expand Up @@ -1484,24 +1513,57 @@ impl<T: Trait<I>, I: Instance> Module<T, I> {
{
let max_members = MaxMembers::<I>::get() as usize;
// No more than 10 will be returned.
let max_selections: usize = 10.min(max_members.saturating_sub(members_len));

// Get the number of left-most bidders whose bids add up to less than `pot`.
let mut bids = <Bids<T, I>>::get();
let taken = bids.iter()
.scan(<BalanceOf<T, I>>::zero(), |total, bid| {
*total = total.saturating_add(bid.value);
Some((*total, bid.who.clone(), bid.kind.clone()))
})
.take(max_selections)
.take_while(|x| pot >= x.0)
.count();

// No need to reset Bids if we're not taking anything.
if taken > 0 {
<Bids<T, I>>::put(&bids[taken..]);
let mut max_selections: usize = 10.min(max_members.saturating_sub(members_len));

if max_selections > 0 {
// Get the number of left-most bidders whose bids add up to less than `pot`.
let mut bids = <Bids<T, I>>::get();

// The list of selected candidates
let mut selected = Vec::new();

if bids.len() > 0 {
// Can only select at most the length of bids
max_selections = max_selections.min(bids.len());
// Number of selected bids so far
let mut count = 0;
// Check if we have already selected a candidate with zero bid
let mut zero_selected = false;
// A running total of the cost to onboard these bids
let mut total_cost: BalanceOf<T, I> = Zero::zero();

bids.retain(|bid| {
if count < max_selections {
// Handle zero bids. We only want one of them.
if bid.value.is_zero() {
// Select only the first zero bid
if !zero_selected {
selected.push(bid.clone());
zero_selected = true;
count += 1;
return false
}
} else {
total_cost += bid.value;
// Select only as many users as the pot can support.
if total_cost <= pot {
selected.push(bid.clone());
count += 1;
return false
}
}
}
true
});

// No need to reset Bids if we're not taking anything.
if count > 0 {
<Bids<T, I>>::put(bids);
}
}
selected
} else {
vec![]
}
bids.truncate(taken);
bids
}
}
65 changes: 65 additions & 0 deletions frame/society/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -742,3 +742,68 @@ fn max_limits_work() {
assert_eq!(Society::candidates().len(), 10);
});
}

#[test]
fn zero_bid_works() {
// This tests:
// * Only one zero bid is selected.
// * That zero bid is placed as head when accepted.
EnvBuilder::new().execute(|| {
// Users make bids of various amounts
assert_ok!(Society::bid(Origin::signed(60), 400));
assert_ok!(Society::bid(Origin::signed(50), 300));
assert_ok!(Society::bid(Origin::signed(30), 0));
assert_ok!(Society::bid(Origin::signed(20), 0));
assert_ok!(Society::bid(Origin::signed(40), 0));

// Rotate period
run_to_block(4);
// Pot is 1000 after "PeriodSpend"
assert_eq!(Society::pot(), 1000);
assert_eq!(Balances::free_balance(Society::account_id()), 10_000);
// Choose smallest bidding users whose total is less than pot, with only one zero bid.
assert_eq!(Society::candidates(), vec![
create_bid(0, 30, BidKind::Deposit(25)),
create_bid(300, 50, BidKind::Deposit(25)),
create_bid(400, 60, BidKind::Deposit(25)),
]);
assert_eq!(<Bids<Test>>::get(), vec![
create_bid(0, 20, BidKind::Deposit(25)),
create_bid(0, 40, BidKind::Deposit(25)),
]);
// A member votes for these candidates to join the society
assert_ok!(Society::vote(Origin::signed(10), 30, true));
assert_ok!(Society::vote(Origin::signed(10), 50, true));
assert_ok!(Society::vote(Origin::signed(10), 60, true));
run_to_block(8);
// Candidates become members after a period rotation
assert_eq!(Society::members(), vec![10, 30, 50, 60]);
// The zero bid is selected as head
assert_eq!(Society::head(), Some(30));
});
}

#[test]
fn bids_ordered_correctly() {
// This tests that bids with the same value are placed in the list ordered
// with bidders who bid first earlier on the list.
EnvBuilder::new().execute(|| {
for i in 0..5 {
for j in 0..5 {
// Give them some funds
let _ = Balances::make_free_balance_be(&(100 + (i * 5 + j) as u128), 1000);
assert_ok!(Society::bid(Origin::signed(100 + (i * 5 + j) as u128), j));
}
}

let mut final_list = Vec::new();

for j in 0..5 {
for i in 0..5 {
final_list.push(create_bid(j, 100 + (i * 5 + j) as u128, BidKind::Deposit(25)));
}
}

assert_eq!(<Bids<Test>>::get(), final_list);
});
}

0 comments on commit 70f1d8d

Please sign in to comment.