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

pallet-lottery: add generate_storage_info #10594

Merged
merged 14 commits into from
Jan 11, 2022
Merged

pallet-lottery: add generate_storage_info #10594

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
8106370
pallet-lottery: add generate_storage_info
ggwpez Jan 5, 2022
ebad8a9
pallet-lottery: test call_to_indices with TooManyCalls
ggwpez Jan 5, 2022
b6758bb
review: move try_push above transfer
ggwpez Jan 6, 2022
df623a9
pallet-lottery: test stop_repeat
ggwpez Jan 6, 2022
f0e32fe
pallet-lottery: test do_buy_ticket as white-box
ggwpez Jan 6, 2022
0fd76a9
pallet-lottery: use BoundedVec in bechmarks
ggwpez Jan 6, 2022
91478f1
pallet-lottery: fix zero div panic
ggwpez Jan 6, 2022
9a2c732
review: extend buy_ticket tests
ggwpez Jan 7, 2022
60133a3
review: test buy_ticket AlreadyParticipating
ggwpez Jan 7, 2022
39a11be
fmt
ggwpez Jan 7, 2022
318da78
Merge remote-tracking branch 'parity/master' into lottery-bounded-len
ggwpez Jan 7, 2022
1152f5a
review: use /// comments on private functions
ggwpez Jan 10, 2022
c100eec
Merge remote-tracking branch 'parity/master' into lottery-bounded-len
ggwpez Jan 10, 2022
64e1b01
review: use with_bounded_capacity
ggwpez Jan 10, 2022
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
11 changes: 7 additions & 4 deletions frame/lottery/src/benchmarking.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@
use super::*;

use frame_benchmarking::{account, benchmarks, whitelisted_caller};
use frame_support::traits::{EnsureOrigin, OnInitialize};
use frame_support::{
storage::bounded_vec::BoundedVec,
traits::{EnsureOrigin, OnInitialize},
};
use frame_system::RawOrigin;
use sp_runtime::traits::{Bounded, Zero};

Expand Down Expand Up @@ -55,12 +58,12 @@ benchmarks! {
let set_code_index: CallIndex = Lottery::<T>::call_to_index(
&frame_system::Call::<T>::set_code{ code: vec![] }.into()
)?;
let already_called: (u32, Vec<CallIndex>) = (
let already_called: (u32, BoundedVec<CallIndex, T::MaxCalls>) = (
LotteryIndex::<T>::get(),
vec![
BoundedVec::<CallIndex, T::MaxCalls>::try_from(vec![
set_code_index;
T::MaxCalls::get().saturating_sub(1) as usize
],
]).unwrap(),
Comment on lines +63 to +66
Copy link
Contributor

Choose a reason for hiding this comment

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

I recently added a bounded_vec! macro for very basic usages. I think you can easily make it understand vec![_; _] syntax as well and then use it here.

Copy link
Member Author

Choose a reason for hiding this comment

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

I can't find the macro in the code-base, can you point me to it please?

Copy link
Contributor

Choose a reason for hiding this comment

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

ahh it is in #10601, maybe you can review it ? :p

Copy link
Contributor

Choose a reason for hiding this comment

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

not a big deal, we can proceed without it.

Would be good if either of us remembers to come clean it up after the aforementioned PR is merged.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok thanks.

);
Participants::<T>::insert(&caller, already_called);

Expand Down
77 changes: 51 additions & 26 deletions frame/lottery/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
//! users to make those calls on your network. An example of how this could be
//! used is to set validator nominations as a valid lottery call. If the lottery
//! is set to repeat every month, then users would be encouraged to re-nominate
//! validators every month. A user can ony purchase one ticket per valid call
//! validators every month. A user can only purchase one ticket per valid call
//! per lottery.
//!
//! This pallet can be configured to use dynamically set calls or statically set
Expand Down Expand Up @@ -58,6 +58,8 @@ use codec::{Decode, Encode};
use frame_support::{
dispatch::{DispatchResult, Dispatchable, GetDispatchInfo},
ensure,
pallet_prelude::MaxEncodedLen,
storage::bounded_vec::BoundedVec,
traits::{Currency, ExistenceRequirement::KeepAlive, Get, Randomness, ReservableCurrency},
PalletId, RuntimeDebug,
};
Expand All @@ -76,7 +78,9 @@ type BalanceOf<T> =
// We use this to uniquely match someone's incoming call with the calls configured for the lottery.
type CallIndex = (u8, u8);

#[derive(Encode, Decode, Default, Eq, PartialEq, RuntimeDebug, scale_info::TypeInfo)]
#[derive(
Encode, Decode, Default, Eq, PartialEq, RuntimeDebug, scale_info::TypeInfo, MaxEncodedLen,
)]
pub struct LotteryConfig<BlockNumber, Balance> {
/// Price per entry.
price: Balance,
Expand Down Expand Up @@ -120,6 +124,7 @@ pub mod pallet {

#[pallet::pallet]
#[pallet::generate_store(pub(super) trait Store)]
#[pallet::generate_storage_info]
pub struct Pallet<T>(_);

/// The pallet's config trait.
Expand Down Expand Up @@ -209,8 +214,13 @@ pub mod pallet {

/// Users who have purchased a ticket. (Lottery Index, Tickets Purchased)
#[pallet::storage]
pub(crate) type Participants<T: Config> =
StorageMap<_, Twox64Concat, T::AccountId, (u32, Vec<CallIndex>), ValueQuery>;
pub(crate) type Participants<T: Config> = StorageMap<
_,
Twox64Concat,
T::AccountId,
(u32, BoundedVec<CallIndex, T::MaxCalls>),
ValueQuery,
>;

/// Total number of tickets sold.
#[pallet::storage]
Expand All @@ -226,7 +236,8 @@ pub mod pallet {
/// The calls stored in this pallet to be used in an active lottery if configured
/// by `Config::ValidateCall`.
#[pallet::storage]
pub(crate) type CallIndices<T> = StorageValue<_, Vec<CallIndex>, ValueQuery>;
pub(crate) type CallIndices<T: Config> =
StorageValue<_, BoundedVec<CallIndex, T::MaxCalls>, ValueQuery>;

#[pallet::hooks]
impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
Expand All @@ -237,10 +248,8 @@ pub mod pallet {
config.start.saturating_add(config.length).saturating_add(config.delay);
if payout_block <= n {
let (lottery_account, lottery_balance) = Self::pot();
let ticket_count = TicketsCount::<T>::get();

let winning_number = Self::choose_winner(ticket_count);
let winner = Tickets::<T>::get(winning_number).unwrap_or(lottery_account);
let winner = Self::choose_account().unwrap_or(lottery_account);
// Not much we can do if this fails...
let res = T::Currency::transfer(
&Self::account_id(),
Expand Down Expand Up @@ -385,7 +394,7 @@ impl<T: Config> Pallet<T> {
}

/// Return the pot account and amount of money in the pot.
// The existential deposit is not part of the pot so lottery account never gets deleted.
/// The existential deposit is not part of the pot so lottery account never gets deleted.
fn pot() -> (T::AccountId, BalanceOf<T>) {
let account_id = Self::account_id();
let balance =
Expand All @@ -394,17 +403,19 @@ impl<T: Config> Pallet<T> {
(account_id, balance)
}

// Converts a vector of calls into a vector of call indices.
fn calls_to_indices(calls: &[<T as Config>::Call]) -> Result<Vec<CallIndex>, DispatchError> {
let mut indices = Vec::with_capacity(calls.len());
/// Converts a vector of calls into a vector of call indices.
fn calls_to_indices(
calls: &[<T as Config>::Call],
) -> Result<BoundedVec<CallIndex, T::MaxCalls>, DispatchError> {
let mut indices = BoundedVec::<CallIndex, T::MaxCalls>::with_bounded_capacity(calls.len());
for c in calls.iter() {
let index = Self::call_to_index(c)?;
indices.push(index)
indices.try_push(index).map_err(|_| Error::<T>::TooManyCalls)?;
}
Ok(indices)
}

// Convert a call to it's call index by encoding the call and taking the first two bytes.
/// Convert a call to it's call index by encoding the call and taking the first two bytes.
fn call_to_index(call: &<T as Config>::Call) -> Result<CallIndex, DispatchError> {
let encoded_call = call.encode();
if encoded_call.len() < 2 {
Expand All @@ -413,7 +424,7 @@ impl<T: Config> Pallet<T> {
return Ok((encoded_call[0], encoded_call[1]))
}

// Logic for buying a ticket.
/// Logic for buying a ticket.
fn do_buy_ticket(caller: &T::AccountId, call: &<T as Config>::Call) -> DispatchResult {
// Check the call is valid lottery
let config = Lottery::<T>::get().ok_or(Error::<T>::NotConfigured)?;
Expand All @@ -433,7 +444,7 @@ impl<T: Config> Pallet<T> {
let index = LotteryIndex::<T>::get();
// If lottery index doesn't match, then reset participating calls and index.
if *lottery_index != index {
*participating_calls = Vec::new();
*participating_calls = Default::default();
*lottery_index = index;
} else {
// Check that user is not already participating under this call.
Expand All @@ -442,12 +453,12 @@ impl<T: Config> Pallet<T> {
Error::<T>::AlreadyParticipating
Copy link
Member Author

Choose a reason for hiding this comment

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

Using a BoundedSet here would make matters easier, but I'm not sure if it is worth the refactor rn.

);
}
participating_calls.try_push(call_index).map_err(|_| Error::<T>::TooManyCalls)?;
ggwpez marked this conversation as resolved.
Show resolved Hide resolved
// Check user has enough funds and send it to the Lottery account.
T::Currency::transfer(caller, &Self::account_id(), config.price, KeepAlive)?;
shawntabrizi marked this conversation as resolved.
Show resolved Hide resolved
// Create a new ticket.
TicketsCount::<T>::put(new_ticket_count);
Tickets::<T>::insert(ticket_count, caller.clone());
participating_calls.push(call_index);
shawntabrizi marked this conversation as resolved.
Show resolved Hide resolved
Ok(())
},
)?;
Expand All @@ -457,8 +468,22 @@ impl<T: Config> Pallet<T> {
Ok(())
}

// Randomly choose a winner from among the total number of participants.
fn choose_winner(total: u32) -> u32 {
/// Randomly choose a winning ticket and return the account that purchased it.
/// The more tickets an account bought, the higher are its chances of winning.
/// Returns `None` if there is no winner.
fn choose_account() -> Option<T::AccountId> {
match Self::choose_ticket(TicketsCount::<T>::get()) {
None => None,
Some(ticket) => Tickets::<T>::get(ticket),
}
}

/// Randomly choose a winning ticket from among the total number of tickets.
/// Returns `None` if there are no tickets.
fn choose_ticket(total: u32) -> Option<u32> {
if total == 0 {
return None
}
let mut random_number = Self::generate_random_number(0);

// Best effort attempt to remove bias from modulus operator.
Expand All @@ -470,15 +495,15 @@ impl<T: Config> Pallet<T> {
random_number = Self::generate_random_number(i);
}

random_number % total
Some(random_number % total)
}

// Generate a random number from a given seed.
// Note that there is potential bias introduced by using modulus operator.
// You should call this function with different seed values until the random
// number lies within `u32::MAX - u32::MAX % n`.
// TODO: deal with randomness freshness
// https://github.com/paritytech/substrate/issues/8311
/// Generate a random number from a given seed.
/// Note that there is potential bias introduced by using modulus operator.
/// You should call this function with different seed values until the random
/// number lies within `u32::MAX - u32::MAX % n`.
/// TODO: deal with randomness freshness
/// https://github.com/paritytech/substrate/issues/8311
fn generate_random_number(seed: u32) -> u32 {
let (random_seed, _) = T::Randomness::random(&(T::PalletId::get(), seed).encode());
let random_number = <u32>::decode(&mut random_seed.as_ref())
Expand Down
Loading