From a1262a23c565979782e1c35dc182c00847aacf5a Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Sun, 27 Sep 2020 00:17:37 +0200 Subject: [PATCH 01/29] Basic design --- Cargo.lock | 17 +++ Cargo.toml | 1 + frame/lottery/Cargo.toml | 47 ++++++++ frame/lottery/src/lib.rs | 239 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 304 insertions(+) create mode 100644 frame/lottery/Cargo.toml create mode 100644 frame/lottery/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index b34eee2154a8e..b33d0a42c3999 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4611,6 +4611,23 @@ dependencies = [ "sp-std", ] +[[package]] +name = "pallet-lottery" +version = "2.0.0" +dependencies = [ + "frame-benchmarking", + "frame-support", + "frame-system", + "pallet-balances", + "parity-scale-codec", + "serde", + "sp-core", + "sp-io", + "sp-keyring", + "sp-runtime", + "sp-std", +] + [[package]] name = "pallet-membership" version = "2.0.0" diff --git a/Cargo.toml b/Cargo.toml index 99b1d418a5153..05acbdb1c8201 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -82,6 +82,7 @@ members = [ "frame/identity", "frame/im-online", "frame/indices", + "frame/lottery", "frame/membership", "frame/metadata", "frame/multisig", diff --git a/frame/lottery/Cargo.toml b/frame/lottery/Cargo.toml new file mode 100644 index 0000000000000..311b0bcb0a8a2 --- /dev/null +++ b/frame/lottery/Cargo.toml @@ -0,0 +1,47 @@ +[package] +name = "pallet-lottery" +version = "2.0.0" +authors = ["Parity Technologies "] +edition = "2018" +license = "Apache-2.0" +homepage = "https://substrate.dev" +repository = "https://github.com/paritytech/substrate/" +description = "FRAME Participation Lottery Pallet" +readme = "README.md" + +[package.metadata.docs.rs] +targets = ["x86_64-unknown-linux-gnu"] + +[dependencies] +serde = { version = "1.0.101", optional = true } +codec = { package = "parity-scale-codec", version = "1.3.4", default-features = false, features = ["derive"] } +sp-keyring = { version = "2.0.0", optional = true, path = "../../primitives/keyring" } +sp-std = { version = "2.0.0", default-features = false, path = "../../primitives/std" } +sp-io = { version = "2.0.0", default-features = false, path = "../../primitives/io" } +sp-runtime = { version = "2.0.0", default-features = false, path = "../../primitives/runtime" } +sp-core = { version = "2.0.0", default-features = false, path = "../../primitives/core" } +frame-support = { version = "2.0.0", default-features = false, path = "../support" } +frame-system = { version = "2.0.0", default-features = false, path = "../system" } + +frame-benchmarking = { version = "2.0.0", default-features = false, path = "../benchmarking", optional = true } + +[dev-dependencies] +pallet-balances = { version = "2.0.0", path = "../balances" } + +[features] +default = ["std"] +std = [ + "serde", + "sp-keyring", + "codec/std", + "sp-core/std", + "sp-std/std", + "sp-io/std", + "frame-support/std", + "sp-runtime/std", + "frame-system/std", +] +runtime-benchmarks = [ + "frame-benchmarking", + "frame-support/runtime-benchmarks", +] diff --git a/frame/lottery/src/lib.rs b/frame/lottery/src/lib.rs new file mode 100644 index 0000000000000..8039a6c8274be --- /dev/null +++ b/frame/lottery/src/lib.rs @@ -0,0 +1,239 @@ +// This file is part of Substrate. + +// Copyright (C) 2017-2020 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! A lottery pallet that uses participation in the network to purchase tickets. + +#![cfg_attr(not(feature = "std"), no_std)] + +use sp_std::prelude::*; +use sp_runtime::ModuleId; +use sp_runtime::traits::{AccountIdConversion, Saturating, Zero}; +use frame_support::{Parameter, decl_module, decl_error, decl_event, decl_storage, ensure, RuntimeDebug}; +use frame_support::dispatch::{Dispatchable, PostDispatchInfo, DispatchResult, GetDispatchInfo}; +use frame_support::traits::{ + Currency, ReservableCurrency, Get, EnsureOrigin, ExistenceRequirement::KeepAlive, Randomness, +}; +use frame_support::weights::Weight; +use frame_system::ensure_signed; +use codec::{Encode, Decode}; + +type BalanceOf = <::Currency as Currency<::AccountId>>::Balance; + +pub trait WeightInfo { } + +/// The module's config trait. +pub trait Trait: frame_system::Trait { + /// The Lottery's module id + type ModuleId: Get; + + /// A dispatchable call. + type Call: Parameter + Dispatchable + GetDispatchInfo; + + /// The currency trait. + type Currency: ReservableCurrency; + + /// Something that provides randomness in the runtime. + type Randomness: Randomness; + + /// The overarching event type. + type Event: From> + Into<::Event>; + + /// The manager origin. + type ManagerOrigin: EnsureOrigin; + + /// The max number of calls available in a single lottery. + type MaxCalls: Get; + + /// Weight information for extrinsics in this pallet. + type WeightInfo: WeightInfo; +} + +type Index = u32; + +decl_storage! { + trait Store for Module as Lottery { + /// The configuration for the current lottery. + Lottery: Option, ::Call>>; + /// Users who have purchased a ticket. + Participants: map hasher(twox_64_concat) T::AccountId => Vec<::Call>; + /// Total number of tickets sold. + TicketsCount: Index; + /// Each ticket's owner. + Tickets: map hasher(twox_64_concat) Index => Option; + } +} + +decl_event!( + pub enum Event where + ::AccountId, + Balance = BalanceOf, + Call = ::Call, + { + /// A winner has been chosen! + Winner(AccountId, Balance), + /// A ticket has been bought! + TicketBought(AccountId, Call), + } +); + +decl_error! { + pub enum Error for Module { + /// An overflow has occurred. + Overflow, + /// A lottery has not been configured. + NotConfigured, + /// A lottery has not started. + NotStarted, + /// A lottery is already in progress. + InProgress, + /// A lottery has already ended. + AlreadyEnded, + /// The call is not valid for an open lottery. + InvalidCall, + /// You are already participating in the lottery with this call. + AlreadyParticipating, + /// Too many calls for a single lottery. + TooManyCalls, + } +} + +#[derive(Encode, Decode, Default, RuntimeDebug)] +pub struct LotteryConfig { + /// Price per entry. + price: Balance, + /// Starting block of the lottery. + start: BlockNumber, + /// Ending block of the lottery. + end: BlockNumber, + /// Payout block of the lottery. + payout: BlockNumber, + /// Calls available this lottery. + calls: Vec, +} + +decl_module! { + pub struct Module for enum Call where origin: T::Origin, system = frame_system { + /// TODO: Expose Constants + + fn deposit_event() = default; + + #[weight = 0] + fn setup_lottery( + origin, + price: BalanceOf, + start: T::BlockNumber, + end: T::BlockNumber, + payout: T::BlockNumber, + calls: Vec<::Call>, + ) { + T::ManagerOrigin::ensure_origin(origin)?; + ensure!(calls.len() < T::MaxCalls::get(), Error::::TooManyCalls); + Lottery::::try_mutate(|lottery| -> DispatchResult { + ensure!(lottery.is_none(), Error::::InProgress); + *lottery = Some(LotteryConfig { price, start, end, payout, calls }); + Ok(()) + })?; + } + + #[weight = 0] + fn buy_ticket(origin, call: ::Call) { + let caller = ensure_signed(origin.clone())?; + call.clone().dispatch(origin).map_err(|e| e.error)?; + + // Only try to buy a ticket if the underlying call is successful. + // Not much we can do if this fails. + let _ = Self::do_buy_ticket(&caller, &call); + } + + fn on_initialize(n: T::BlockNumber) -> Weight { + if Lottery::::get().map_or(false, |config| config.payout < n) { + let lottery_balance = Self::pot(); + let lottery_account = Self::account_id(); + let ticket_count = TicketsCount::get(); + + let winning_number = Self::choose_winner(ticket_count); + let winner = Tickets::::get(winning_number).unwrap_or(lottery_account); + // Not much we can do if this fails... + let _ = T::Currency::transfer(&Self::account_id(), &winner, lottery_balance, KeepAlive); + + Self::deposit_event(RawEvent::Winner(winner, lottery_balance)); + + Lottery::::kill(); + TicketsCount::kill(); + Participants::::remove_all(); + Tickets::::remove_all(); + + return T::DbWeight::get().reads_writes(2, ticket_count.saturating_mul(2).into()) + } else { + return Weight::zero() + } + } + + } +} + +impl Module { + + /// The account ID of the treasury pot. + /// + /// This actually does computation. If you need to keep using it, then make sure you cache the + /// value and only call this once. + pub fn account_id() -> T::AccountId { + T::ModuleId::get().into_account() + } + + /// Return the amount of money in the pot. + // The existential deposit is not part of the pot so lottery account never gets deleted. + fn pot() -> BalanceOf { + T::Currency::free_balance(&Self::account_id()) + .saturating_sub(T::Currency::minimum_balance()) + } + + fn do_buy_ticket(caller: &T::AccountId, call: &::Call) -> DispatchResult { + // Check the call is valid lottery + let config = Lottery::::get().ok_or(Error::::NotConfigured)?; + let block_number = frame_system::Module::::block_number(); + ensure!(block_number > config.start, Error::::NotStarted); + ensure!(block_number < config.end, Error::::AlreadyEnded); + ensure!(config.calls.contains(&call), Error::::InvalidCall); + let ticket_count = TicketsCount::get(); + let new_ticket_count = ticket_count.checked_add(1).ok_or(Error::::Overflow)?; + // Try to update the participant status + Participants::::try_mutate(&caller, |participating_calls| -> DispatchResult { + // Check that user is not already participating under this call. + ensure!(!participating_calls.contains(call), Error::::AlreadyParticipating); + // Check user has enough funds and send it to the Lottery account. + T::Currency::transfer(caller, &Self::account_id(), config.price, KeepAlive)?; + // Create a new ticket. + TicketsCount::put(new_ticket_count); + Tickets::::insert(ticket_count, caller.clone()); + participating_calls.push(call.clone()); + Ok(()) + })?; + + Self::deposit_event(RawEvent::TicketBought(caller.clone(), call.clone())); + + Ok(()) + } + + fn choose_winner(total: u32) -> u32 { + let random_seed = T::Randomness::random(b"lottery"); + let random_number = ::decode(&mut random_seed.as_ref()) + .expect("secure hashes should always be bigger than u32; qed"); + random_number % total + } +} From 8ce5540b1797e113f2ff152ec7d843f2e6ee2142 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Tue, 24 Nov 2020 19:48:57 -0800 Subject: [PATCH 02/29] start adding tests --- frame/lottery/src/lib.rs | 39 ++++++---- frame/lottery/src/mock.rs | 139 +++++++++++++++++++++++++++++++++++ frame/lottery/src/tests.rs | 132 +++++++++++++++++++++++++++++++++ frame/lottery/src/weights.rs | 3 + 4 files changed, 299 insertions(+), 14 deletions(-) create mode 100644 frame/lottery/src/mock.rs create mode 100644 frame/lottery/src/tests.rs create mode 100644 frame/lottery/src/weights.rs diff --git a/frame/lottery/src/lib.rs b/frame/lottery/src/lib.rs index 8039a6c8274be..4d6859c7a4e0c 100644 --- a/frame/lottery/src/lib.rs +++ b/frame/lottery/src/lib.rs @@ -19,29 +19,34 @@ #![cfg_attr(not(feature = "std"), no_std)] +#[cfg(test)] +mod mock; +#[cfg(test)] +mod tests; +mod weights; + use sp_std::prelude::*; use sp_runtime::ModuleId; use sp_runtime::traits::{AccountIdConversion, Saturating, Zero}; use frame_support::{Parameter, decl_module, decl_error, decl_event, decl_storage, ensure, RuntimeDebug}; -use frame_support::dispatch::{Dispatchable, PostDispatchInfo, DispatchResult, GetDispatchInfo}; +use frame_support::dispatch::{Dispatchable, DispatchResult, GetDispatchInfo}; use frame_support::traits::{ Currency, ReservableCurrency, Get, EnsureOrigin, ExistenceRequirement::KeepAlive, Randomness, }; use frame_support::weights::Weight; use frame_system::ensure_signed; use codec::{Encode, Decode}; +pub use weights::WeightInfo; type BalanceOf = <::Currency as Currency<::AccountId>>::Balance; -pub trait WeightInfo { } - /// The module's config trait. pub trait Trait: frame_system::Trait { /// The Lottery's module id type ModuleId: Get; /// A dispatchable call. - type Call: Parameter + Dispatchable + GetDispatchInfo; + type Call: Parameter + Dispatchable + GetDispatchInfo; /// The currency trait. type Currency: ReservableCurrency; @@ -160,9 +165,8 @@ decl_module! { } fn on_initialize(n: T::BlockNumber) -> Weight { - if Lottery::::get().map_or(false, |config| config.payout < n) { - let lottery_balance = Self::pot(); - let lottery_account = Self::account_id(); + if Lottery::::get().map_or(false, |config| config.payout <= n) { + let (lottery_account, lottery_balance) = Self::pot(); let ticket_count = TicketsCount::get(); let winning_number = Self::choose_winner(ticket_count); @@ -196,26 +200,29 @@ impl Module { T::ModuleId::get().into_account() } - /// Return the amount of money in the pot. + /// 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. - fn pot() -> BalanceOf { - T::Currency::free_balance(&Self::account_id()) - .saturating_sub(T::Currency::minimum_balance()) + fn pot() -> (T::AccountId, BalanceOf) { + let account_id = Self::account_id(); + let balance = T::Currency::free_balance(&account_id) + .saturating_sub(T::Currency::minimum_balance()); + + (account_id, balance) } fn do_buy_ticket(caller: &T::AccountId, call: &::Call) -> DispatchResult { // Check the call is valid lottery let config = Lottery::::get().ok_or(Error::::NotConfigured)?; let block_number = frame_system::Module::::block_number(); - ensure!(block_number > config.start, Error::::NotStarted); + ensure!(block_number >= config.start, Error::::NotStarted); ensure!(block_number < config.end, Error::::AlreadyEnded); - ensure!(config.calls.contains(&call), Error::::InvalidCall); + ensure!(config.calls.iter().any(|c| variant_eq(call, c)), Error::::InvalidCall); let ticket_count = TicketsCount::get(); let new_ticket_count = ticket_count.checked_add(1).ok_or(Error::::Overflow)?; // Try to update the participant status Participants::::try_mutate(&caller, |participating_calls| -> DispatchResult { // Check that user is not already participating under this call. - ensure!(!participating_calls.contains(call), Error::::AlreadyParticipating); + ensure!(!participating_calls.iter().any(|c| variant_eq(c, call)), Error::::AlreadyParticipating); // Check user has enough funds and send it to the Lottery account. T::Currency::transfer(caller, &Self::account_id(), config.price, KeepAlive)?; // Create a new ticket. @@ -237,3 +244,7 @@ impl Module { random_number % total } } + +fn variant_eq(a: &T, b: &T) -> bool { + sp_std::mem::discriminant(a) == sp_std::mem::discriminant(b) +} diff --git a/frame/lottery/src/mock.rs b/frame/lottery/src/mock.rs new file mode 100644 index 0000000000000..2b03c88e210cb --- /dev/null +++ b/frame/lottery/src/mock.rs @@ -0,0 +1,139 @@ +// This file is part of Substrate. + +// Copyright (C) 2020 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Test utilities + +use super::*; + +use frame_support::{ + impl_outer_origin, impl_outer_dispatch, parameter_types, + traits::{OnInitialize, OnFinalize, TestRandomness}, +}; +use sp_core::H256; +use sp_runtime::{ + Perbill, + testing::Header, + traits::{BlakeTwo256, IdentityLookup}, +}; +use frame_system::EnsureRoot; + +impl_outer_origin! { + pub enum Origin for Test {} +} + +impl_outer_dispatch! { + pub enum Call for Test where origin: Origin { + frame_system::System, + pallet_balances::Balances, + } +} + +#[derive(Clone, Eq, PartialEq)] +pub struct Test; +parameter_types! { + pub const BlockHashCount: u64 = 250; + pub const MaximumBlockWeight: u32 = 1024; + pub const MaximumBlockLength: u32 = 2 * 1024; + pub const AvailableBlockRatio: Perbill = Perbill::one(); +} + +impl frame_system::Trait for Test { + type BaseCallFilter = (); + type Origin = Origin; + type Index = u64; + type BlockNumber = u64; + type Hash = H256; + type Call = Call; + type Hashing = BlakeTwo256; + type AccountId = u64; + type Lookup = IdentityLookup; + type Header = Header; + type Event = (); + type BlockHashCount = BlockHashCount; + type MaximumBlockWeight = MaximumBlockWeight; + type DbWeight = (); + type BlockExecutionWeight = (); + type ExtrinsicBaseWeight = (); + type MaximumExtrinsicWeight = MaximumBlockWeight; + type MaximumBlockLength = MaximumBlockLength; + type AvailableBlockRatio = AvailableBlockRatio; + type Version = (); + type PalletInfo = (); + type OnNewAccount = (); + type OnKilledAccount = (); + type AccountData = pallet_balances::AccountData; + type SystemWeightInfo = (); +} + +parameter_types! { + pub const ExistentialDeposit: u64 = 1; +} + +impl pallet_balances::Trait for Test { + type MaxLocks = (); + type Balance = u64; + type Event = (); + type DustRemoval = (); + type ExistentialDeposit = ExistentialDeposit; + type AccountStore = System; + type WeightInfo = (); +} + +parameter_types! { + pub const LotteryModuleId: ModuleId = ModuleId(*b"py/lotto"); + pub const MaxCalls: usize = 2; + pub const ManagerOrigin: u32 = 1024; +} + +impl Trait for Test { + type ModuleId = LotteryModuleId; + type Call = Call; + type Currency = Balances; + type Randomness = TestRandomness; + type Event = (); + type ManagerOrigin = EnsureRoot; + type MaxCalls = MaxCalls; + type WeightInfo = (); +} + +pub type Lottery = Module; +pub type System = frame_system::Module; +pub type Balances = pallet_balances::Module; + +pub type SystemCall = frame_system::Call; +pub type BalancesCall = pallet_balances::Call; + +pub fn new_test_ext() -> sp_io::TestExternalities { + let mut t = frame_system::GenesisConfig::default().build_storage::().unwrap(); + pallet_balances::GenesisConfig:: { + balances: vec![(1, 100), (2, 100), (3, 100), (4, 100), (5, 100), (Lottery::account_id(), 1)], + }.assimilate_storage(&mut t).unwrap(); + t.into() +} + +/// Run until a particular block. +pub fn run_to_block(n: u64) { + while System::block_number() < n { + if System::block_number() > 1 { + Lottery::on_finalize(System::block_number()); + System::on_finalize(System::block_number()); + } + System::set_block_number(System::block_number() + 1); + System::on_initialize(System::block_number()); + Lottery::on_initialize(System::block_number()); + } +} diff --git a/frame/lottery/src/tests.rs b/frame/lottery/src/tests.rs new file mode 100644 index 0000000000000..a5d4574f7a75b --- /dev/null +++ b/frame/lottery/src/tests.rs @@ -0,0 +1,132 @@ +// This file is part of Substrate. + +// Copyright (C) 2020 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Tests for the module. + +use super::*; +use mock::{ + Lottery, Balances, Test, Origin, Call, BalancesCall, + new_test_ext, run_to_block +}; +use sp_runtime::traits::{BadOrigin}; +use frame_support::{ + assert_noop, assert_ok, + traits::{Currency}, +}; + +#[test] +fn initial_state() { + new_test_ext().execute_with(|| { + assert_eq!(Balances::free_balance(Lottery::account_id()), 1); + assert!(crate::Lottery::::get().is_none()); + assert_eq!(Participants::::get(&1), vec![]); + assert_eq!(TicketsCount::get(), 0); + assert!(Tickets::::get(0).is_none()); + }); +} + +#[test] +fn basic_end_to_end_works() { + new_test_ext().execute_with(|| { + let price = 10; + let start = 5; + let end = 20; + let payout = 25; + let calls = vec![Call::Balances(BalancesCall::transfer(0, 0))]; + + // Setup Lottery + assert_ok!(Lottery::setup_lottery(Origin::root(), price, start, end, payout, calls.clone())); + assert!(crate::Lottery::::get().is_some()); + + // Go to start + run_to_block(5); + + assert_eq!(Balances::free_balance(&1), 100); + let call = Call::Balances(BalancesCall::transfer(2, 20)); + assert_ok!(Lottery::buy_ticket(Origin::signed(1), call.clone())); + // 20 from the transfer, 10 from buying a ticket + assert_eq!(Balances::free_balance(&1), 100 - 20 - 10); + assert_eq!(Participants::::get(&1).len(), 1); + assert_eq!(TicketsCount::get(), 1); + // 1 owns the 0 ticket + assert_eq!(Tickets::::get(0), Some(1)); + + // More ticket purchases + assert_ok!(Lottery::buy_ticket(Origin::signed(2), call.clone())); + assert_ok!(Lottery::buy_ticket(Origin::signed(3), call.clone())); + assert_ok!(Lottery::buy_ticket(Origin::signed(4), call.clone())); + assert_eq!(TicketsCount::get(), 4); + + // Go to end + run_to_block(20); + assert_ok!(Lottery::buy_ticket(Origin::signed(5), call.clone())); + // Ticket isn't bought + assert_eq!(TicketsCount::get(), 4); + + // Go to payout + run_to_block(25); + // Lottery is reset + assert!(crate::Lottery::::get().is_none()); + assert_eq!(Participants::::get(&1), vec![]); + assert_eq!(TicketsCount::get(), 0); + assert!(Tickets::::get(0).is_none()); + // User 1 wins + assert_eq!(Balances::free_balance(&1), 70 + 40); + }); +} + +#[test] +fn setup_lottery_works() { + new_test_ext().execute_with(|| { + let price = 10; + let start = 5; + let end = 20; + let payout = 25; + let calls = vec![Call::Balances(BalancesCall::transfer(0, 0))]; + let too_many_calls = vec![Call::Balances(BalancesCall::transfer(0, 0)); 10]; + + // Setup ignores bad origin + assert_noop!( + Lottery::setup_lottery(Origin::signed(1), price, start, end, payout, calls.clone()), + BadOrigin, + ); + // Too many calls + assert_noop!( + Lottery::setup_lottery(Origin::root(), price, start, end, payout, too_many_calls), + Error::::TooManyCalls, + ); + + // All good + assert_ok!(Lottery::setup_lottery(Origin::root(), price, start, end, payout, calls.clone())); + + // Can't open another one if lottery is already present + assert_noop!( + Lottery::setup_lottery(Origin::root(), price, start, end, payout, calls), + Error::::InProgress, + ); + }); +} + +#[test] +fn buy_ticket_works_as_simple_passthrough() { + // This test checks that even if the user could not buy a ticket, that `buy_ticket` acts + // as a simple passthrough to the real call. + new_test_ext().execute_with(|| { + // No lottery set up + let call = Call::Balances(BalancesCall::transfer(2, 20)); + }); +} diff --git a/frame/lottery/src/weights.rs b/frame/lottery/src/weights.rs new file mode 100644 index 0000000000000..6eff06ca78e31 --- /dev/null +++ b/frame/lottery/src/weights.rs @@ -0,0 +1,3 @@ +pub trait WeightInfo { } + +impl WeightInfo for () { } From fc12be9cba006fff03549f6064f47f27a05f31f6 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Tue, 24 Nov 2020 21:26:34 -0800 Subject: [PATCH 03/29] finish tests --- frame/lottery/src/lib.rs | 3 +- frame/lottery/src/tests.rs | 99 +++++++++++++++++++++++++++++++++++--- 2 files changed, 93 insertions(+), 9 deletions(-) diff --git a/frame/lottery/src/lib.rs b/frame/lottery/src/lib.rs index 4d6859c7a4e0c..c2fc421b376c9 100644 --- a/frame/lottery/src/lib.rs +++ b/frame/lottery/src/lib.rs @@ -146,7 +146,7 @@ decl_module! { calls: Vec<::Call>, ) { T::ManagerOrigin::ensure_origin(origin)?; - ensure!(calls.len() < T::MaxCalls::get(), Error::::TooManyCalls); + ensure!(calls.len() <= T::MaxCalls::get(), Error::::TooManyCalls); Lottery::::try_mutate(|lottery| -> DispatchResult { ensure!(lottery.is_none(), Error::::InProgress); *lottery = Some(LotteryConfig { price, start, end, payout, calls }); @@ -245,6 +245,7 @@ impl Module { } } +// Compare that two enum variants are the same, ignoring the values. fn variant_eq(a: &T, b: &T) -> bool { sp_std::mem::discriminant(a) == sp_std::mem::discriminant(b) } diff --git a/frame/lottery/src/tests.rs b/frame/lottery/src/tests.rs index a5d4574f7a75b..6281585ae40dc 100644 --- a/frame/lottery/src/tests.rs +++ b/frame/lottery/src/tests.rs @@ -19,14 +19,12 @@ use super::*; use mock::{ - Lottery, Balances, Test, Origin, Call, BalancesCall, + Lottery, Balances, Test, Origin, Call, SystemCall, BalancesCall, new_test_ext, run_to_block }; use sp_runtime::traits::{BadOrigin}; -use frame_support::{ - assert_noop, assert_ok, - traits::{Currency}, -}; +use frame_support::{assert_noop, assert_ok}; +use pallet_balances::Error as BalancesError; #[test] fn initial_state() { @@ -46,7 +44,10 @@ fn basic_end_to_end_works() { let start = 5; let end = 20; let payout = 25; - let calls = vec![Call::Balances(BalancesCall::transfer(0, 0))]; + let calls = vec![ + Call::Balances(BalancesCall::force_transfer(0, 0, 0)), + Call::Balances(BalancesCall::transfer(0, 0)), + ]; // Setup Lottery assert_ok!(Lottery::setup_lottery(Origin::root(), price, start, end, payout, calls.clone())); @@ -96,8 +97,15 @@ fn setup_lottery_works() { let start = 5; let end = 20; let payout = 25; - let calls = vec![Call::Balances(BalancesCall::transfer(0, 0))]; - let too_many_calls = vec![Call::Balances(BalancesCall::transfer(0, 0)); 10]; + let calls = vec![ + Call::Balances(BalancesCall::force_transfer(0, 0, 0)), + Call::Balances(BalancesCall::transfer(0, 0)), + ]; + let too_many_calls = vec![ + Call::Balances(BalancesCall::force_transfer(0, 0, 0)), + Call::Balances(BalancesCall::transfer(0, 0)), + Call::System(SystemCall::remark(vec![])), + ]; // Setup ignores bad origin assert_noop!( @@ -128,5 +136,80 @@ fn buy_ticket_works_as_simple_passthrough() { new_test_ext().execute_with(|| { // No lottery set up let call = Call::Balances(BalancesCall::transfer(2, 20)); + // This is just a basic transfer then + assert_ok!(Lottery::buy_ticket(Origin::signed(1), call.clone())); + assert_eq!(Balances::free_balance(&1), 100 - 20); + assert_eq!(TicketsCount::get(), 0); + + // Lottery is set up, but too expensive to enter, so `do_buy_ticket` fails. + let calls = vec![ + Call::Balances(BalancesCall::force_transfer(0, 0, 0)), + Call::Balances(BalancesCall::transfer(0, 0)), + ]; + // Ticket price of 60 would kill the user's account + assert_ok!(Lottery::setup_lottery(Origin::root(), 60, 0, 10, 15, calls.clone())); + assert_ok!(Lottery::buy_ticket(Origin::signed(1), call.clone())); + assert_eq!(Balances::free_balance(&1), 100 - 20 - 20); + assert_eq!(TicketsCount::get(), 0); + + // If call would fail, the whole thing still fails the same + let fail_call = Call::Balances(BalancesCall::transfer(2, 1000)); + assert_noop!( + Lottery::buy_ticket(Origin::signed(1), fail_call), + BalancesError::::InsufficientBalance, + ); + + let bad_origin_call = Call::Balances(BalancesCall::force_transfer(0, 0, 0)); + assert_noop!( + Lottery::buy_ticket(Origin::signed(1), bad_origin_call), + BadOrigin, + ); + + // User can call other txs, but doesn't get a ticket + let remark_call = Call::System(SystemCall::remark(b"hello, world!".to_vec())); + assert_ok!(Lottery::buy_ticket(Origin::signed(1), remark_call)); + assert_eq!(TicketsCount::get(), 0); + }); +} + +#[test] +fn buy_ticket_works() { + new_test_ext().execute_with(|| { + let calls = vec![ + Call::System(SystemCall::remark(vec![])), + Call::Balances(BalancesCall::transfer(0, 0)), + ]; + // Setup lottery + assert_ok!(Lottery::setup_lottery(Origin::root(), 1, 5, 20, 25, calls.clone())); + + // Can't buy ticket before start + let call = Call::Balances(BalancesCall::transfer(2, 1)); + assert_ok!(Lottery::buy_ticket(Origin::signed(1), call.clone())); + assert_eq!(TicketsCount::get(), 0); + + // Go to start, buy ticket for transfer + run_to_block(5); + assert_ok!(Lottery::buy_ticket(Origin::signed(1), call)); + assert_eq!(TicketsCount::get(), 1); + + // Can't buy another of the same ticket (even if call is slightly changed) + let call = Call::Balances(BalancesCall::transfer(3, 30)); + assert_ok!(Lottery::buy_ticket(Origin::signed(1), call)); + assert_eq!(TicketsCount::get(), 1); + + // Buy ticket for remark + let call = Call::System(SystemCall::remark(b"hello, world!".to_vec())); + assert_ok!(Lottery::buy_ticket(Origin::signed(1), call.clone())); + assert_eq!(TicketsCount::get(), 2); + + // Go to end, can't buy tickets anymore + run_to_block(20); + assert_ok!(Lottery::buy_ticket(Origin::signed(2), call.clone())); + assert_eq!(TicketsCount::get(), 2); + + // Go to payout, can't buy tickets when there is no lottery open + run_to_block(25); + assert_ok!(Lottery::buy_ticket(Origin::signed(2), call.clone())); + assert_eq!(TicketsCount::get(), 0); }); } From d8f0598369c308eb5c7bed7e5602c1381317edd2 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Tue, 24 Nov 2020 21:31:09 -0800 Subject: [PATCH 04/29] clean up crates --- Cargo.lock | 2 -- frame/lottery/Cargo.toml | 11 +++-------- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 46e6d112e19c8..9a6b5de294d96 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4727,10 +4727,8 @@ dependencies = [ "frame-system", "pallet-balances", "parity-scale-codec", - "serde", "sp-core", "sp-io", - "sp-keyring", "sp-runtime", "sp-std", ] diff --git a/frame/lottery/Cargo.toml b/frame/lottery/Cargo.toml index 311b0bcb0a8a2..db76316c42966 100644 --- a/frame/lottery/Cargo.toml +++ b/frame/lottery/Cargo.toml @@ -13,13 +13,9 @@ readme = "README.md" targets = ["x86_64-unknown-linux-gnu"] [dependencies] -serde = { version = "1.0.101", optional = true } codec = { package = "parity-scale-codec", version = "1.3.4", default-features = false, features = ["derive"] } -sp-keyring = { version = "2.0.0", optional = true, path = "../../primitives/keyring" } sp-std = { version = "2.0.0", default-features = false, path = "../../primitives/std" } -sp-io = { version = "2.0.0", default-features = false, path = "../../primitives/io" } sp-runtime = { version = "2.0.0", default-features = false, path = "../../primitives/runtime" } -sp-core = { version = "2.0.0", default-features = false, path = "../../primitives/core" } frame-support = { version = "2.0.0", default-features = false, path = "../support" } frame-system = { version = "2.0.0", default-features = false, path = "../system" } @@ -27,21 +23,20 @@ frame-benchmarking = { version = "2.0.0", default-features = false, path = "../b [dev-dependencies] pallet-balances = { version = "2.0.0", path = "../balances" } +sp-core = { version = "2.0.0", path = "../../primitives/core" } +sp-io = { version = "2.0.0", path = "../../primitives/io" } [features] default = ["std"] std = [ - "serde", - "sp-keyring", "codec/std", - "sp-core/std", "sp-std/std", - "sp-io/std", "frame-support/std", "sp-runtime/std", "frame-system/std", ] runtime-benchmarks = [ "frame-benchmarking", + "frame-system/runtime-benchmarks", "frame-support/runtime-benchmarks", ] From 1289a8e0667be5f06724325d9c79da18c0ba7cdb Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Tue, 24 Nov 2020 23:50:40 -0800 Subject: [PATCH 05/29] use call index for match --- frame/lottery/src/benchmarking.rs | 110 ++++++++++++++++++++++++++++++ frame/lottery/src/lib.rs | 52 +++++++++----- 2 files changed, 146 insertions(+), 16 deletions(-) create mode 100644 frame/lottery/src/benchmarking.rs diff --git a/frame/lottery/src/benchmarking.rs b/frame/lottery/src/benchmarking.rs new file mode 100644 index 0000000000000..f8953e06a6404 --- /dev/null +++ b/frame/lottery/src/benchmarking.rs @@ -0,0 +1,110 @@ +// This file is part of Substrate. + +// Copyright (C) 2020 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Lottery pallet benchmarking. + +#![cfg(feature = "runtime-benchmarks")] + +use super::*; + +use frame_system::RawOrigin; +use frame_benchmarking::{benchmarks, whitelisted_caller}; +use sp_runtime::traits::Bounded; + +use crate::Module as Lottery; + +// Set up and start a lottery +fn start_lottery() -> Result<(), &'static str> { + let price = T::Currency::minimum_balance(); + let start = 0.into(); + let end = 10.into(); + let payout = 15.into(); + // Calls will be maximum length... + let mut calls = vec![ + frame_system::Call::::set_code(vec![]).into(); + T::MaxCalls::get().saturating_sub(1) + ]; + // Last call will be the match for worst case scenario. + calls.push(frame_system::Call::::remark(vec![]).into()); + Lottery::::setup_lottery(RawOrigin::Root.into(), price, start, end, payout, calls)?; + Ok(()) +} + +benchmarks! { + _ { } + + setup_lottery { + let price = BalanceOf::::max_value(); + let start = 0.into(); + let end = 10.into(); + let payout = 15.into(); + let calls = vec![ + frame_system::Call::::remark(vec![]).into(), + ]; + + // let call = Call::::setup_lottery(price, start, end, payout, calls); + // let origin = T::ManagerOrigin::successful_origin(); + + // // use success origin + // }: { call.dispatch_bypass_filter(origin)? } + }: _(RawOrigin::Root, price, start, end, payout, calls) + verify { + assert!(crate::Lottery::::get().is_some()); + } + + buy_ticket { + let caller = whitelisted_caller(); + T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); + start_lottery::()?; + // force user to have a long vec of calls participating + let set_code_index: CallIndex = Lottery::::call_to_index( + &frame_system::Call::::set_code(vec![]).into() + )?; + let already_called: Vec = vec![ + set_code_index; + T::MaxCalls::get().saturating_sub(1) + ]; + Participants::::insert(&caller, already_called); + + let call = frame_system::Call::::remark(vec![]); + }: _(RawOrigin::Signed(caller), call.into()) + verify { + assert_eq!(TicketsCount::get(), 1); + } + + on_initialize { + + }: { + + } + +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::mock::{new_test_ext, Test}; + use frame_support::assert_ok; + + #[test] + fn test_benchmarks() { + new_test_ext().execute_with(|| { + assert_ok!(test_benchmark_setup_lottery::()); + assert_ok!(test_benchmark_buy_ticket::()); + }); + } +} diff --git a/frame/lottery/src/lib.rs b/frame/lottery/src/lib.rs index c2fc421b376c9..f925272c0db4f 100644 --- a/frame/lottery/src/lib.rs +++ b/frame/lottery/src/lib.rs @@ -23,10 +23,11 @@ mod mock; #[cfg(test)] mod tests; +mod benchmarking; mod weights; use sp_std::prelude::*; -use sp_runtime::ModuleId; +use sp_runtime::{DispatchError, ModuleId}; use sp_runtime::traits::{AccountIdConversion, Saturating, Zero}; use frame_support::{Parameter, decl_module, decl_error, decl_event, decl_storage, ensure, RuntimeDebug}; use frame_support::dispatch::{Dispatchable, DispatchResult, GetDispatchInfo}; @@ -46,7 +47,7 @@ pub trait Trait: frame_system::Trait { type ModuleId: Get; /// A dispatchable call. - type Call: Parameter + Dispatchable + GetDispatchInfo; + type Call: Parameter + Dispatchable + GetDispatchInfo + From>; /// The currency trait. type Currency: ReservableCurrency; @@ -68,13 +69,14 @@ pub trait Trait: frame_system::Trait { } type Index = u32; +type CallIndex = (u8, u8); decl_storage! { trait Store for Module as Lottery { /// The configuration for the current lottery. - Lottery: Option, ::Call>>; + Lottery: Option>>; /// Users who have purchased a ticket. - Participants: map hasher(twox_64_concat) T::AccountId => Vec<::Call>; + Participants: map hasher(twox_64_concat) T::AccountId => Vec; /// Total number of tickets sold. TicketsCount: Index; /// Each ticket's owner. @@ -88,6 +90,8 @@ decl_event!( Balance = BalanceOf, Call = ::Call, { + /// A lottery has been started! + LotteryStarted, /// A winner has been chosen! Winner(AccountId, Balance), /// A ticket has been bought! @@ -113,11 +117,13 @@ decl_error! { AlreadyParticipating, /// Too many calls for a single lottery. TooManyCalls, + /// Failed to encode calls + EncodingFailed, } } #[derive(Encode, Decode, Default, RuntimeDebug)] -pub struct LotteryConfig { +pub struct LotteryConfig { /// Price per entry. price: Balance, /// Starting block of the lottery. @@ -127,7 +133,7 @@ pub struct LotteryConfig { /// Payout block of the lottery. payout: BlockNumber, /// Calls available this lottery. - calls: Vec, + calls: Vec, } decl_module! { @@ -147,11 +153,13 @@ decl_module! { ) { T::ManagerOrigin::ensure_origin(origin)?; ensure!(calls.len() <= T::MaxCalls::get(), Error::::TooManyCalls); + let indices = Self::calls_to_indices(&calls)?; Lottery::::try_mutate(|lottery| -> DispatchResult { ensure!(lottery.is_none(), Error::::InProgress); - *lottery = Some(LotteryConfig { price, start, end, payout, calls }); + *lottery = Some(LotteryConfig { price, start, end, payout, calls: indices }); Ok(()) })?; + Self::deposit_event(RawEvent::LotteryStarted); } #[weight = 0] @@ -161,7 +169,8 @@ decl_module! { // Only try to buy a ticket if the underlying call is successful. // Not much we can do if this fails. - let _ = Self::do_buy_ticket(&caller, &call); + let res = Self::do_buy_ticket(&caller, &call); + println!("{:?}", res); } fn on_initialize(n: T::BlockNumber) -> Weight { @@ -210,25 +219,41 @@ impl Module { (account_id, balance) } + fn calls_to_indices(calls: &[::Call]) -> Result, DispatchError> { + let mut indices = Vec::with_capacity(calls.len()); + for c in calls.iter() { + let index = Self::call_to_index(c)?; + indices.push(index) + } + Ok(indices) + } + + fn call_to_index(call: &::Call) -> Result { + let encoded_call = call.encode(); + if encoded_call.len() < 2 { Err(Error::::EncodingFailed)? } + return Ok((encoded_call[0], encoded_call[1])) + } + fn do_buy_ticket(caller: &T::AccountId, call: &::Call) -> DispatchResult { // Check the call is valid lottery let config = Lottery::::get().ok_or(Error::::NotConfigured)?; let block_number = frame_system::Module::::block_number(); ensure!(block_number >= config.start, Error::::NotStarted); ensure!(block_number < config.end, Error::::AlreadyEnded); - ensure!(config.calls.iter().any(|c| variant_eq(call, c)), Error::::InvalidCall); + let call_index = Self::call_to_index(call)?; + ensure!(config.calls.iter().any(|c| call_index == *c), Error::::InvalidCall); let ticket_count = TicketsCount::get(); let new_ticket_count = ticket_count.checked_add(1).ok_or(Error::::Overflow)?; // Try to update the participant status Participants::::try_mutate(&caller, |participating_calls| -> DispatchResult { // Check that user is not already participating under this call. - ensure!(!participating_calls.iter().any(|c| variant_eq(c, call)), Error::::AlreadyParticipating); + ensure!(!participating_calls.iter().any(|c| call_index == *c), Error::::AlreadyParticipating); // Check user has enough funds and send it to the Lottery account. T::Currency::transfer(caller, &Self::account_id(), config.price, KeepAlive)?; // Create a new ticket. TicketsCount::put(new_ticket_count); Tickets::::insert(ticket_count, caller.clone()); - participating_calls.push(call.clone()); + participating_calls.push(call_index); Ok(()) })?; @@ -244,8 +269,3 @@ impl Module { random_number % total } } - -// Compare that two enum variants are the same, ignoring the values. -fn variant_eq(a: &T, b: &T) -> bool { - sp_std::mem::discriminant(a) == sp_std::mem::discriminant(b) -} From 82da7df8bba402af9cb84da51f67932b42dd17c4 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Wed, 25 Nov 2020 00:30:27 -0800 Subject: [PATCH 06/29] finish benchmarks --- frame/lottery/src/benchmarking.rs | 63 ++++++++++++++++++++++++++----- frame/lottery/src/lib.rs | 13 +++++-- 2 files changed, 63 insertions(+), 13 deletions(-) diff --git a/frame/lottery/src/benchmarking.rs b/frame/lottery/src/benchmarking.rs index f8953e06a6404..0297eb1a462fd 100644 --- a/frame/lottery/src/benchmarking.rs +++ b/frame/lottery/src/benchmarking.rs @@ -22,7 +22,8 @@ use super::*; use frame_system::RawOrigin; -use frame_benchmarking::{benchmarks, whitelisted_caller}; +use frame_support::traits::{OnInitialize, UnfilteredDispatchable}; +use frame_benchmarking::{benchmarks, account, whitelisted_caller}; use sp_runtime::traits::Bounded; use crate::Module as Lottery; @@ -40,7 +41,8 @@ fn start_lottery() -> Result<(), &'static str> { ]; // Last call will be the match for worst case scenario. calls.push(frame_system::Call::::remark(vec![]).into()); - Lottery::::setup_lottery(RawOrigin::Root.into(), price, start, end, payout, calls)?; + let origin = T::ManagerOrigin::successful_origin(); + Lottery::::setup_lottery(origin, price, start, end, payout, calls)?; Ok(()) } @@ -56,12 +58,11 @@ benchmarks! { frame_system::Call::::remark(vec![]).into(), ]; - // let call = Call::::setup_lottery(price, start, end, payout, calls); - // let origin = T::ManagerOrigin::successful_origin(); + let call = Call::::setup_lottery(price, start, end, payout, calls); + let origin = T::ManagerOrigin::successful_origin(); - // // use success origin - // }: { call.dispatch_bypass_filter(origin)? } - }: _(RawOrigin::Root, price, start, end, payout, calls) + // use success origin + }: { call.dispatch_bypass_filter(origin)? } verify { assert!(crate::Lottery::::get().is_some()); } @@ -86,12 +87,54 @@ benchmarks! { assert_eq!(TicketsCount::get(), 1); } - on_initialize { + do_buy_ticket { + let caller = whitelisted_caller(); + T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); + start_lottery::()?; + // force user to have a long vec of calls participating + let set_code_index: CallIndex = Lottery::::call_to_index( + &frame_system::Call::::set_code(vec![]).into() + )?; + let already_called: Vec = vec![ + set_code_index; + T::MaxCalls::get().saturating_sub(1) + ]; + Participants::::insert(&caller, already_called); + let call = frame_system::Call::::remark(vec![]); }: { - + Lottery::::do_buy_ticket(&caller, &call.into())? + } + verify { + assert_eq!(TicketsCount::get(), 1); } + on_initialize { + start_lottery::()?; + let winner = account("winner", 0, 0); + // User needs more than min balance to get ticket + T::Currency::make_free_balance_be(&winner, T::Currency::minimum_balance() * 10.into()); + // Make sure lottery account has at least min balance too + let lottery_account = Lottery::::account_id(); + T::Currency::make_free_balance_be(&lottery_account, T::Currency::minimum_balance() * 10.into()); + // Buy a ticket + let call = frame_system::Call::::remark(vec![]); + Lottery::::buy_ticket(RawOrigin::Signed(winner.clone()).into(), call.into())?; + // Kill user account for worst case + T::Currency::make_free_balance_be(&winner, 0.into()); + // Assert that lotto is set up for winner + assert_eq!(TicketsCount::get(), 1); + assert!(!Lottery::::pot().1.is_zero()); + }: { + // Start lottery has block 15 configured for payout + Lottery::::on_initialize(15.into()); + } + verify { + assert!(crate::Lottery::::get().is_none()); + assert_eq!(TicketsCount::get(), 0); + assert_eq!(Lottery::::pot().1, 0.into()); + assert!(!T::Currency::free_balance(&winner).is_zero()) + } } #[cfg(test)] @@ -105,6 +148,8 @@ mod tests { new_test_ext().execute_with(|| { assert_ok!(test_benchmark_setup_lottery::()); assert_ok!(test_benchmark_buy_ticket::()); + assert_ok!(test_benchmark_do_buy_ticket::()); + assert_ok!(test_benchmark_on_initialize::()); }); } } diff --git a/frame/lottery/src/lib.rs b/frame/lottery/src/lib.rs index f925272c0db4f..c80a08de5a4e0 100644 --- a/frame/lottery/src/lib.rs +++ b/frame/lottery/src/lib.rs @@ -69,6 +69,9 @@ pub trait Trait: frame_system::Trait { } type Index = u32; + +// Any runtime call can be encoded into two bytes which represent the pallet and call index. +// We use this to uniquely match someone's incoming call with the calls configured for the lottery. type CallIndex = (u8, u8); decl_storage! { @@ -169,8 +172,7 @@ decl_module! { // Only try to buy a ticket if the underlying call is successful. // Not much we can do if this fails. - let res = Self::do_buy_ticket(&caller, &call); - println!("{:?}", res); + let _ = Self::do_buy_ticket(&caller, &call); } fn on_initialize(n: T::BlockNumber) -> Weight { @@ -200,8 +202,7 @@ decl_module! { } impl Module { - - /// The account ID of the treasury pot. + /// The account ID of the lottery pot. /// /// This actually does computation. If you need to keep using it, then make sure you cache the /// value and only call this once. @@ -219,6 +220,7 @@ impl Module { (account_id, balance) } + // Converts a vector of calls into a vector of call indices. fn calls_to_indices(calls: &[::Call]) -> Result, DispatchError> { let mut indices = Vec::with_capacity(calls.len()); for c in calls.iter() { @@ -228,12 +230,14 @@ impl Module { Ok(indices) } + // Convert a call to it's call index by encoding the call and taking the first two bytes. fn call_to_index(call: &::Call) -> Result { let encoded_call = call.encode(); if encoded_call.len() < 2 { Err(Error::::EncodingFailed)? } return Ok((encoded_call[0], encoded_call[1])) } + // Logic for buying a ticket. fn do_buy_ticket(caller: &T::AccountId, call: &::Call) -> DispatchResult { // Check the call is valid lottery let config = Lottery::::get().ok_or(Error::::NotConfigured)?; @@ -262,6 +266,7 @@ impl Module { Ok(()) } + // Randomly choose a winner from among the total number of participants. fn choose_winner(total: u32) -> u32 { let random_seed = T::Randomness::random(b"lottery"); let random_number = ::decode(&mut random_seed.as_ref()) From 14a5c44918ac63335125fe5dc7f9c74f984c1293 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Wed, 25 Nov 2020 00:46:53 -0800 Subject: [PATCH 07/29] add to runtime --- Cargo.lock | 1 + bin/node/runtime/Cargo.toml | 1 + bin/node/runtime/src/lib.rs | 17 +++++++++++++++++ frame/lottery/src/benchmarking.rs | 4 ++-- frame/lottery/src/lib.rs | 2 +- frame/lottery/src/mock.rs | 1 - frame/lottery/src/tests.rs | 16 ++++++++-------- 7 files changed, 30 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9a6b5de294d96..0b2bf3233cb70 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3994,6 +3994,7 @@ dependencies = [ "pallet-identity", "pallet-im-online", "pallet-indices", + "pallet-lottery", "pallet-membership", "pallet-multisig", "pallet-offences", diff --git a/bin/node/runtime/Cargo.toml b/bin/node/runtime/Cargo.toml index eabc9f61c62e5..418ec8863a10b 100644 --- a/bin/node/runtime/Cargo.toml +++ b/bin/node/runtime/Cargo.toml @@ -57,6 +57,7 @@ pallet-grandpa = { version = "2.0.0", default-features = false, path = "../../.. pallet-im-online = { version = "2.0.0", default-features = false, path = "../../../frame/im-online" } pallet-indices = { version = "2.0.0", default-features = false, path = "../../../frame/indices" } pallet-identity = { version = "2.0.0", default-features = false, path = "../../../frame/identity" } +pallet-lottery = { version = "2.0.0", default-features = false, path = "../../../frame/lottery" } pallet-membership = { version = "2.0.0", default-features = false, path = "../../../frame/membership" } pallet-multisig = { version = "2.0.0", default-features = false, path = "../../../frame/multisig" } pallet-offences = { version = "2.0.0", default-features = false, path = "../../../frame/offences" } diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 4feff5d051abe..00bafede51573 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -892,6 +892,22 @@ impl pallet_vesting::Trait for Runtime { type WeightInfo = pallet_vesting::weights::SubstrateWeight; } +parameter_types! { + pub const LotteryModuleId: ModuleId = ModuleId(*b"py/lotto"); + pub const MaxCalls: usize = 10; +} + +impl pallet_lottery::Trait for Runtime { + type ModuleId = LotteryModuleId; + type Call = Call; + type Event = Event; + type Currency = Balances; + type Randomness = RandomnessCollectiveFlip; + type ManagerOrigin = EnsureRoot; + type MaxCalls = MaxCalls; + type WeightInfo = (); +} + construct_runtime!( pub enum Runtime where Block = Block, @@ -929,6 +945,7 @@ construct_runtime!( Scheduler: pallet_scheduler::{Module, Call, Storage, Event}, Proxy: pallet_proxy::{Module, Call, Storage, Event}, Multisig: pallet_multisig::{Module, Call, Storage, Event}, + Lottery: pallet_lottery::{Module, Call, Storage, Event}, } ); diff --git a/frame/lottery/src/benchmarking.rs b/frame/lottery/src/benchmarking.rs index 0297eb1a462fd..b28b8a761e930 100644 --- a/frame/lottery/src/benchmarking.rs +++ b/frame/lottery/src/benchmarking.rs @@ -82,7 +82,7 @@ benchmarks! { Participants::::insert(&caller, already_called); let call = frame_system::Call::::remark(vec![]); - }: _(RawOrigin::Signed(caller), call.into()) + }: _(RawOrigin::Signed(caller), Box::new(call.into())) verify { assert_eq!(TicketsCount::get(), 1); } @@ -119,7 +119,7 @@ benchmarks! { T::Currency::make_free_balance_be(&lottery_account, T::Currency::minimum_balance() * 10.into()); // Buy a ticket let call = frame_system::Call::::remark(vec![]); - Lottery::::buy_ticket(RawOrigin::Signed(winner.clone()).into(), call.into())?; + Lottery::::buy_ticket(RawOrigin::Signed(winner.clone()).into(), Box::new(call.into()))?; // Kill user account for worst case T::Currency::make_free_balance_be(&winner, 0.into()); // Assert that lotto is set up for winner diff --git a/frame/lottery/src/lib.rs b/frame/lottery/src/lib.rs index c80a08de5a4e0..201b9ab44927e 100644 --- a/frame/lottery/src/lib.rs +++ b/frame/lottery/src/lib.rs @@ -166,7 +166,7 @@ decl_module! { } #[weight = 0] - fn buy_ticket(origin, call: ::Call) { + fn buy_ticket(origin, call: Box<::Call>) { let caller = ensure_signed(origin.clone())?; call.clone().dispatch(origin).map_err(|e| e.error)?; diff --git a/frame/lottery/src/mock.rs b/frame/lottery/src/mock.rs index 2b03c88e210cb..09db673b26187 100644 --- a/frame/lottery/src/mock.rs +++ b/frame/lottery/src/mock.rs @@ -96,7 +96,6 @@ impl pallet_balances::Trait for Test { parameter_types! { pub const LotteryModuleId: ModuleId = ModuleId(*b"py/lotto"); pub const MaxCalls: usize = 2; - pub const ManagerOrigin: u32 = 1024; } impl Trait for Test { diff --git a/frame/lottery/src/tests.rs b/frame/lottery/src/tests.rs index 6281585ae40dc..3ebb7d90c6391 100644 --- a/frame/lottery/src/tests.rs +++ b/frame/lottery/src/tests.rs @@ -57,7 +57,7 @@ fn basic_end_to_end_works() { run_to_block(5); assert_eq!(Balances::free_balance(&1), 100); - let call = Call::Balances(BalancesCall::transfer(2, 20)); + let call = Box::new(Call::Balances(BalancesCall::transfer(2, 20))); assert_ok!(Lottery::buy_ticket(Origin::signed(1), call.clone())); // 20 from the transfer, 10 from buying a ticket assert_eq!(Balances::free_balance(&1), 100 - 20 - 10); @@ -135,7 +135,7 @@ fn buy_ticket_works_as_simple_passthrough() { // as a simple passthrough to the real call. new_test_ext().execute_with(|| { // No lottery set up - let call = Call::Balances(BalancesCall::transfer(2, 20)); + let call = Box::new(Call::Balances(BalancesCall::transfer(2, 20))); // This is just a basic transfer then assert_ok!(Lottery::buy_ticket(Origin::signed(1), call.clone())); assert_eq!(Balances::free_balance(&1), 100 - 20); @@ -153,20 +153,20 @@ fn buy_ticket_works_as_simple_passthrough() { assert_eq!(TicketsCount::get(), 0); // If call would fail, the whole thing still fails the same - let fail_call = Call::Balances(BalancesCall::transfer(2, 1000)); + let fail_call = Box::new(Call::Balances(BalancesCall::transfer(2, 1000))); assert_noop!( Lottery::buy_ticket(Origin::signed(1), fail_call), BalancesError::::InsufficientBalance, ); - let bad_origin_call = Call::Balances(BalancesCall::force_transfer(0, 0, 0)); + let bad_origin_call = Box::new(Call::Balances(BalancesCall::force_transfer(0, 0, 0))); assert_noop!( Lottery::buy_ticket(Origin::signed(1), bad_origin_call), BadOrigin, ); // User can call other txs, but doesn't get a ticket - let remark_call = Call::System(SystemCall::remark(b"hello, world!".to_vec())); + let remark_call = Box::new(Call::System(SystemCall::remark(b"hello, world!".to_vec()))); assert_ok!(Lottery::buy_ticket(Origin::signed(1), remark_call)); assert_eq!(TicketsCount::get(), 0); }); @@ -183,7 +183,7 @@ fn buy_ticket_works() { assert_ok!(Lottery::setup_lottery(Origin::root(), 1, 5, 20, 25, calls.clone())); // Can't buy ticket before start - let call = Call::Balances(BalancesCall::transfer(2, 1)); + let call = Box::new(Call::Balances(BalancesCall::transfer(2, 1))); assert_ok!(Lottery::buy_ticket(Origin::signed(1), call.clone())); assert_eq!(TicketsCount::get(), 0); @@ -193,12 +193,12 @@ fn buy_ticket_works() { assert_eq!(TicketsCount::get(), 1); // Can't buy another of the same ticket (even if call is slightly changed) - let call = Call::Balances(BalancesCall::transfer(3, 30)); + let call = Box::new(Call::Balances(BalancesCall::transfer(3, 30))); assert_ok!(Lottery::buy_ticket(Origin::signed(1), call)); assert_eq!(TicketsCount::get(), 1); // Buy ticket for remark - let call = Call::System(SystemCall::remark(b"hello, world!".to_vec())); + let call = Box::new(Call::System(SystemCall::remark(b"hello, world!".to_vec()))); assert_ok!(Lottery::buy_ticket(Origin::signed(1), call.clone())); assert_eq!(TicketsCount::get(), 2); From f67b11994d27d09a52aca8361a2a01196dfd14cb Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Wed, 25 Nov 2020 01:06:11 -0800 Subject: [PATCH 08/29] fix --- bin/node/runtime/Cargo.toml | 2 ++ bin/node/runtime/src/lib.rs | 1 + frame/lottery/src/benchmarking.rs | 22 +++++++++++----------- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/bin/node/runtime/Cargo.toml b/bin/node/runtime/Cargo.toml index 418ec8863a10b..8f77ade0a5c60 100644 --- a/bin/node/runtime/Cargo.toml +++ b/bin/node/runtime/Cargo.toml @@ -108,6 +108,7 @@ std = [ "pallet-im-online/std", "pallet-indices/std", "sp-inherents/std", + "pallet-lottery/std", "pallet-membership/std", "pallet-multisig/std", "pallet-identity/std", @@ -158,6 +159,7 @@ runtime-benchmarks = [ "pallet-identity/runtime-benchmarks", "pallet-im-online/runtime-benchmarks", "pallet-indices/runtime-benchmarks", + "pallet-lottery/runtime-benchmarks", "pallet-multisig/runtime-benchmarks", "pallet-proxy/runtime-benchmarks", "pallet-scheduler/runtime-benchmarks", diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 00bafede51573..57e025d9e75db 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -1222,6 +1222,7 @@ impl_runtime_apis! { add_benchmark!(params, batches, pallet_identity, Identity); add_benchmark!(params, batches, pallet_im_online, ImOnline); add_benchmark!(params, batches, pallet_indices, Indices); + add_benchmark!(params, batches, pallet_lottery, Lottery); add_benchmark!(params, batches, pallet_multisig, Multisig); add_benchmark!(params, batches, pallet_offences, OffencesBench::); add_benchmark!(params, batches, pallet_proxy, Proxy); diff --git a/frame/lottery/src/benchmarking.rs b/frame/lottery/src/benchmarking.rs index b28b8a761e930..2f9dd1c1059d4 100644 --- a/frame/lottery/src/benchmarking.rs +++ b/frame/lottery/src/benchmarking.rs @@ -31,9 +31,9 @@ use crate::Module as Lottery; // Set up and start a lottery fn start_lottery() -> Result<(), &'static str> { let price = T::Currency::minimum_balance(); - let start = 0.into(); - let end = 10.into(); - let payout = 15.into(); + let start = 0u32.into(); + let end = 10u32.into(); + let payout = 15u32.into(); // Calls will be maximum length... let mut calls = vec![ frame_system::Call::::set_code(vec![]).into(); @@ -51,9 +51,9 @@ benchmarks! { setup_lottery { let price = BalanceOf::::max_value(); - let start = 0.into(); - let end = 10.into(); - let payout = 15.into(); + let start = 0u32.into(); + let end = 10u32.into(); + let payout = 15u32.into(); let calls = vec![ frame_system::Call::::remark(vec![]).into(), ]; @@ -113,26 +113,26 @@ benchmarks! { start_lottery::()?; let winner = account("winner", 0, 0); // User needs more than min balance to get ticket - T::Currency::make_free_balance_be(&winner, T::Currency::minimum_balance() * 10.into()); + T::Currency::make_free_balance_be(&winner, T::Currency::minimum_balance() * 10u32.into()); // Make sure lottery account has at least min balance too let lottery_account = Lottery::::account_id(); - T::Currency::make_free_balance_be(&lottery_account, T::Currency::minimum_balance() * 10.into()); + T::Currency::make_free_balance_be(&lottery_account, T::Currency::minimum_balance() * 10u32.into()); // Buy a ticket let call = frame_system::Call::::remark(vec![]); Lottery::::buy_ticket(RawOrigin::Signed(winner.clone()).into(), Box::new(call.into()))?; // Kill user account for worst case - T::Currency::make_free_balance_be(&winner, 0.into()); + T::Currency::make_free_balance_be(&winner, 0u32.into()); // Assert that lotto is set up for winner assert_eq!(TicketsCount::get(), 1); assert!(!Lottery::::pot().1.is_zero()); }: { // Start lottery has block 15 configured for payout - Lottery::::on_initialize(15.into()); + Lottery::::on_initialize(15u32.into()); } verify { assert!(crate::Lottery::::get().is_none()); assert_eq!(TicketsCount::get(), 0); - assert_eq!(Lottery::::pot().1, 0.into()); + assert_eq!(Lottery::::pot().1, 0u32.into()); assert!(!T::Currency::free_balance(&winner).is_zero()) } } From 90a33353f082e4685c6dbb6dfb98ec930151be3e Mon Sep 17 00:00:00 2001 From: Parity Benchmarking Bot Date: Wed, 25 Nov 2020 09:09:33 +0000 Subject: [PATCH 09/29] cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_lottery --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/lottery/src/weights.rs --template=./.maintain/frame-weight-template.hbs --- frame/lottery/src/weights.rs | 101 ++++++++++++++++++++++++++++++++++- 1 file changed, 99 insertions(+), 2 deletions(-) diff --git a/frame/lottery/src/weights.rs b/frame/lottery/src/weights.rs index 6eff06ca78e31..9c4d07e35f6d8 100644 --- a/frame/lottery/src/weights.rs +++ b/frame/lottery/src/weights.rs @@ -1,3 +1,100 @@ -pub trait WeightInfo { } +// This file is part of Substrate. -impl WeightInfo for () { } +// Copyright (C) 2020 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Autogenerated weights for pallet_lottery +//! +//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 2.0.0 +//! DATE: 2020-11-25, STEPS: [50, ], REPEAT: 20, LOW RANGE: [], HIGH RANGE: [] +//! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 128 + +// Executed Command: +// target/release/substrate +// benchmark +// --chain=dev +// --steps=50 +// --repeat=20 +// --pallet=pallet_lottery +// --extrinsic=* +// --execution=wasm +// --wasm-execution=compiled +// --heap-pages=4096 +// --output=./frame/lottery/src/weights.rs +// --template=./.maintain/frame-weight-template.hbs + + +#![allow(unused_parens)] +#![allow(unused_imports)] + +use frame_support::{traits::Get, weights::{Weight, constants::RocksDbWeight}}; +use sp_std::marker::PhantomData; + +/// Weight functions needed for pallet_lottery. +pub trait WeightInfo { + fn setup_lottery() -> Weight; + fn buy_ticket() -> Weight; + fn do_buy_ticket() -> Weight; + fn on_initialize() -> Weight; +} + +/// Weights for pallet_lottery using the Substrate node and recommended hardware. +pub struct SubstrateWeight(PhantomData); +impl WeightInfo for SubstrateWeight { + fn setup_lottery() -> Weight { + (26_604_000 as Weight) + .saturating_add(T::DbWeight::get().reads(1 as Weight)) + .saturating_add(T::DbWeight::get().writes(1 as Weight)) + } + fn buy_ticket() -> Weight { + (110_562_000 as Weight) + .saturating_add(T::DbWeight::get().reads(4 as Weight)) + .saturating_add(T::DbWeight::get().writes(4 as Weight)) + } + fn do_buy_ticket() -> Weight { + (106_527_000 as Weight) + .saturating_add(T::DbWeight::get().reads(4 as Weight)) + .saturating_add(T::DbWeight::get().writes(4 as Weight)) + } + fn on_initialize() -> Weight { + (118_212_000 as Weight) + .saturating_add(T::DbWeight::get().reads(6 as Weight)) + .saturating_add(T::DbWeight::get().writes(6 as Weight)) + } +} + +// For backwards compatibility and tests +impl WeightInfo for () { + fn setup_lottery() -> Weight { + (26_604_000 as Weight) + .saturating_add(RocksDbWeight::get().reads(1 as Weight)) + .saturating_add(RocksDbWeight::get().writes(1 as Weight)) + } + fn buy_ticket() -> Weight { + (110_562_000 as Weight) + .saturating_add(RocksDbWeight::get().reads(4 as Weight)) + .saturating_add(RocksDbWeight::get().writes(4 as Weight)) + } + fn do_buy_ticket() -> Weight { + (106_527_000 as Weight) + .saturating_add(RocksDbWeight::get().reads(4 as Weight)) + .saturating_add(RocksDbWeight::get().writes(4 as Weight)) + } + fn on_initialize() -> Weight { + (118_212_000 as Weight) + .saturating_add(RocksDbWeight::get().reads(6 as Weight)) + .saturating_add(RocksDbWeight::get().writes(6 as Weight)) + } +} From 844646b5711b2a9093e94c300b564c1475e0234f Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Wed, 25 Nov 2020 01:41:18 -0800 Subject: [PATCH 10/29] more efficient storage --- frame/lottery/src/benchmarking.rs | 41 +++++++----------------------- frame/lottery/src/lib.rs | 42 ++++++++++++++++++++++--------- frame/lottery/src/tests.rs | 6 ++--- frame/lottery/src/weights.rs | 17 +++---------- 4 files changed, 44 insertions(+), 62 deletions(-) diff --git a/frame/lottery/src/benchmarking.rs b/frame/lottery/src/benchmarking.rs index 2f9dd1c1059d4..eb3f36b42724a 100644 --- a/frame/lottery/src/benchmarking.rs +++ b/frame/lottery/src/benchmarking.rs @@ -50,18 +50,15 @@ benchmarks! { _ { } setup_lottery { + let n in 0 .. T::MaxCalls::get() as u32; let price = BalanceOf::::max_value(); let start = 0u32.into(); let end = 10u32.into(); let payout = 15u32.into(); - let calls = vec![ - frame_system::Call::::remark(vec![]).into(), - ]; + let calls = vec![frame_system::Call::::remark(vec![]).into(); n as usize]; let call = Call::::setup_lottery(price, start, end, payout, calls); let origin = T::ManagerOrigin::successful_origin(); - - // use success origin }: { call.dispatch_bypass_filter(origin)? } verify { assert!(crate::Lottery::::get().is_some()); @@ -75,10 +72,13 @@ benchmarks! { let set_code_index: CallIndex = Lottery::::call_to_index( &frame_system::Call::::set_code(vec![]).into() )?; - let already_called: Vec = vec![ - set_code_index; - T::MaxCalls::get().saturating_sub(1) - ]; + let already_called: (Index, Vec) = ( + LotteryIndex::get(), + vec![ + set_code_index; + T::MaxCalls::get().saturating_sub(1) + ], + ); Participants::::insert(&caller, already_called); let call = frame_system::Call::::remark(vec![]); @@ -87,28 +87,6 @@ benchmarks! { assert_eq!(TicketsCount::get(), 1); } - do_buy_ticket { - let caller = whitelisted_caller(); - T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); - start_lottery::()?; - // force user to have a long vec of calls participating - let set_code_index: CallIndex = Lottery::::call_to_index( - &frame_system::Call::::set_code(vec![]).into() - )?; - let already_called: Vec = vec![ - set_code_index; - T::MaxCalls::get().saturating_sub(1) - ]; - Participants::::insert(&caller, already_called); - - let call = frame_system::Call::::remark(vec![]); - }: { - Lottery::::do_buy_ticket(&caller, &call.into())? - } - verify { - assert_eq!(TicketsCount::get(), 1); - } - on_initialize { start_lottery::()?; let winner = account("winner", 0, 0); @@ -148,7 +126,6 @@ mod tests { new_test_ext().execute_with(|| { assert_ok!(test_benchmark_setup_lottery::()); assert_ok!(test_benchmark_buy_ticket::()); - assert_ok!(test_benchmark_do_buy_ticket::()); assert_ok!(test_benchmark_on_initialize::()); }); } diff --git a/frame/lottery/src/lib.rs b/frame/lottery/src/lib.rs index 201b9ab44927e..8e3a0371bea6d 100644 --- a/frame/lottery/src/lib.rs +++ b/frame/lottery/src/lib.rs @@ -24,7 +24,7 @@ mod mock; #[cfg(test)] mod tests; mod benchmarking; -mod weights; +pub mod weights; use sp_std::prelude::*; use sp_runtime::{DispatchError, ModuleId}; @@ -76,13 +76,17 @@ type CallIndex = (u8, u8); decl_storage! { trait Store for Module as Lottery { + LotteryIndex: Index; /// The configuration for the current lottery. Lottery: Option>>; - /// Users who have purchased a ticket. - Participants: map hasher(twox_64_concat) T::AccountId => Vec; + /// Users who have purchased a ticket. (Lottery Index, Tickets Purchased) + Participants: map hasher(twox_64_concat) T::AccountId => (Index, Vec); /// Total number of tickets sold. TicketsCount: Index; /// Each ticket's owner. + /// + /// May have residual storage from previous lotteries. Use `TicketsCount` to see which ones + /// are actually valid ticket mappings. Tickets: map hasher(twox_64_concat) Index => Option; } } @@ -127,6 +131,8 @@ decl_error! { #[derive(Encode, Decode, Default, RuntimeDebug)] pub struct LotteryConfig { + /// Index of this lottery + index: Index, /// Price per entry. price: Balance, /// Starting block of the lottery. @@ -145,7 +151,7 @@ decl_module! { fn deposit_event() = default; - #[weight = 0] + #[weight = T::WeightInfo::setup_lottery(calls.len() as u32)] fn setup_lottery( origin, price: BalanceOf, @@ -159,13 +165,20 @@ decl_module! { let indices = Self::calls_to_indices(&calls)?; Lottery::::try_mutate(|lottery| -> DispatchResult { ensure!(lottery.is_none(), Error::::InProgress); - *lottery = Some(LotteryConfig { price, start, end, payout, calls: indices }); + let index = LotteryIndex::get(); + let new_index = index.checked_add(1).ok_or(Error::::Overflow)?; + // Use new_index to more easily track everything with the current state. + *lottery = Some(LotteryConfig { index: new_index, price, start, end, payout, calls: indices }); + LotteryIndex::put(new_index); Ok(()) })?; Self::deposit_event(RawEvent::LotteryStarted); } - #[weight = 0] + #[weight = + T::WeightInfo::buy_ticket() + .saturating_add(call.get_dispatch_info().weight) + ] fn buy_ticket(origin, call: Box<::Call>) { let caller = ensure_signed(origin.clone())?; call.clone().dispatch(origin).map_err(|e| e.error)?; @@ -189,10 +202,8 @@ decl_module! { Lottery::::kill(); TicketsCount::kill(); - Participants::::remove_all(); - Tickets::::remove_all(); - return T::DbWeight::get().reads_writes(2, ticket_count.saturating_mul(2).into()) + return T::WeightInfo::on_initialize() } else { return Weight::zero() } @@ -249,9 +260,16 @@ impl Module { let ticket_count = TicketsCount::get(); let new_ticket_count = ticket_count.checked_add(1).ok_or(Error::::Overflow)?; // Try to update the participant status - Participants::::try_mutate(&caller, |participating_calls| -> DispatchResult { - // Check that user is not already participating under this call. - ensure!(!participating_calls.iter().any(|c| call_index == *c), Error::::AlreadyParticipating); + Participants::::try_mutate(&caller, |(lottery_index, participating_calls)| -> DispatchResult { + let index = LotteryIndex::get(); + // If lottery index doesn't match, then reset participating calls and index. + if *lottery_index != index { + *participating_calls = Vec::new(); + *lottery_index = index; + } else { + // Check that user is not already participating under this call. + ensure!(!participating_calls.iter().any(|c| call_index == *c), Error::::AlreadyParticipating); + } // Check user has enough funds and send it to the Lottery account. T::Currency::transfer(caller, &Self::account_id(), config.price, KeepAlive)?; // Create a new ticket. diff --git a/frame/lottery/src/tests.rs b/frame/lottery/src/tests.rs index 3ebb7d90c6391..d7eb93dffb01f 100644 --- a/frame/lottery/src/tests.rs +++ b/frame/lottery/src/tests.rs @@ -31,7 +31,7 @@ fn initial_state() { new_test_ext().execute_with(|| { assert_eq!(Balances::free_balance(Lottery::account_id()), 1); assert!(crate::Lottery::::get().is_none()); - assert_eq!(Participants::::get(&1), vec![]); + assert_eq!(Participants::::get(&1), (0, vec![])); assert_eq!(TicketsCount::get(), 0); assert!(Tickets::::get(0).is_none()); }); @@ -61,7 +61,7 @@ fn basic_end_to_end_works() { assert_ok!(Lottery::buy_ticket(Origin::signed(1), call.clone())); // 20 from the transfer, 10 from buying a ticket assert_eq!(Balances::free_balance(&1), 100 - 20 - 10); - assert_eq!(Participants::::get(&1).len(), 1); + assert_eq!(Participants::::get(&1).1.len(), 1); assert_eq!(TicketsCount::get(), 1); // 1 owns the 0 ticket assert_eq!(Tickets::::get(0), Some(1)); @@ -82,9 +82,7 @@ fn basic_end_to_end_works() { run_to_block(25); // Lottery is reset assert!(crate::Lottery::::get().is_none()); - assert_eq!(Participants::::get(&1), vec![]); assert_eq!(TicketsCount::get(), 0); - assert!(Tickets::::get(0).is_none()); // User 1 wins assert_eq!(Balances::free_balance(&1), 70 + 40); }); diff --git a/frame/lottery/src/weights.rs b/frame/lottery/src/weights.rs index 9c4d07e35f6d8..845776fef9796 100644 --- a/frame/lottery/src/weights.rs +++ b/frame/lottery/src/weights.rs @@ -44,16 +44,15 @@ use sp_std::marker::PhantomData; /// Weight functions needed for pallet_lottery. pub trait WeightInfo { - fn setup_lottery() -> Weight; + fn setup_lottery(n: u32) -> Weight; fn buy_ticket() -> Weight; - fn do_buy_ticket() -> Weight; fn on_initialize() -> Weight; } /// Weights for pallet_lottery using the Substrate node and recommended hardware. pub struct SubstrateWeight(PhantomData); impl WeightInfo for SubstrateWeight { - fn setup_lottery() -> Weight { + fn setup_lottery(_n: u32) -> Weight { (26_604_000 as Weight) .saturating_add(T::DbWeight::get().reads(1 as Weight)) .saturating_add(T::DbWeight::get().writes(1 as Weight)) @@ -63,11 +62,6 @@ impl WeightInfo for SubstrateWeight { .saturating_add(T::DbWeight::get().reads(4 as Weight)) .saturating_add(T::DbWeight::get().writes(4 as Weight)) } - fn do_buy_ticket() -> Weight { - (106_527_000 as Weight) - .saturating_add(T::DbWeight::get().reads(4 as Weight)) - .saturating_add(T::DbWeight::get().writes(4 as Weight)) - } fn on_initialize() -> Weight { (118_212_000 as Weight) .saturating_add(T::DbWeight::get().reads(6 as Weight)) @@ -77,7 +71,7 @@ impl WeightInfo for SubstrateWeight { // For backwards compatibility and tests impl WeightInfo for () { - fn setup_lottery() -> Weight { + fn setup_lottery(_n: u32) -> Weight { (26_604_000 as Weight) .saturating_add(RocksDbWeight::get().reads(1 as Weight)) .saturating_add(RocksDbWeight::get().writes(1 as Weight)) @@ -87,11 +81,6 @@ impl WeightInfo for () { .saturating_add(RocksDbWeight::get().reads(4 as Weight)) .saturating_add(RocksDbWeight::get().writes(4 as Weight)) } - fn do_buy_ticket() -> Weight { - (106_527_000 as Weight) - .saturating_add(RocksDbWeight::get().reads(4 as Weight)) - .saturating_add(RocksDbWeight::get().writes(4 as Weight)) - } fn on_initialize() -> Weight { (118_212_000 as Weight) .saturating_add(RocksDbWeight::get().reads(6 as Weight)) From a121177d96a442ac7a0e7432b9b933e7c8e63268 Mon Sep 17 00:00:00 2001 From: Parity Benchmarking Bot Date: Wed, 25 Nov 2020 09:45:21 +0000 Subject: [PATCH 11/29] cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_lottery --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/lottery/src/weights.rs --template=./.maintain/frame-weight-template.hbs --- frame/lottery/src/weights.rs | 36 +++++++++++++++++++----------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/frame/lottery/src/weights.rs b/frame/lottery/src/weights.rs index 845776fef9796..7d2f486c810a2 100644 --- a/frame/lottery/src/weights.rs +++ b/frame/lottery/src/weights.rs @@ -44,7 +44,7 @@ use sp_std::marker::PhantomData; /// Weight functions needed for pallet_lottery. pub trait WeightInfo { - fn setup_lottery(n: u32) -> Weight; + fn setup_lottery(n: u32, ) -> Weight; fn buy_ticket() -> Weight; fn on_initialize() -> Weight; } @@ -52,38 +52,40 @@ pub trait WeightInfo { /// Weights for pallet_lottery using the Substrate node and recommended hardware. pub struct SubstrateWeight(PhantomData); impl WeightInfo for SubstrateWeight { - fn setup_lottery(_n: u32) -> Weight { - (26_604_000 as Weight) - .saturating_add(T::DbWeight::get().reads(1 as Weight)) - .saturating_add(T::DbWeight::get().writes(1 as Weight)) + fn setup_lottery(n: u32, ) -> Weight { + (31_350_000 as Weight) + .saturating_add((496_000 as Weight).saturating_mul(n as Weight)) + .saturating_add(T::DbWeight::get().reads(2 as Weight)) + .saturating_add(T::DbWeight::get().writes(2 as Weight)) } fn buy_ticket() -> Weight { - (110_562_000 as Weight) - .saturating_add(T::DbWeight::get().reads(4 as Weight)) + (116_213_000 as Weight) + .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().writes(4 as Weight)) } fn on_initialize() -> Weight { - (118_212_000 as Weight) + (111_483_000 as Weight) .saturating_add(T::DbWeight::get().reads(6 as Weight)) - .saturating_add(T::DbWeight::get().writes(6 as Weight)) + .saturating_add(T::DbWeight::get().writes(4 as Weight)) } } // For backwards compatibility and tests impl WeightInfo for () { - fn setup_lottery(_n: u32) -> Weight { - (26_604_000 as Weight) - .saturating_add(RocksDbWeight::get().reads(1 as Weight)) - .saturating_add(RocksDbWeight::get().writes(1 as Weight)) + fn setup_lottery(n: u32, ) -> Weight { + (31_350_000 as Weight) + .saturating_add((496_000 as Weight).saturating_mul(n as Weight)) + .saturating_add(RocksDbWeight::get().reads(2 as Weight)) + .saturating_add(RocksDbWeight::get().writes(2 as Weight)) } fn buy_ticket() -> Weight { - (110_562_000 as Weight) - .saturating_add(RocksDbWeight::get().reads(4 as Weight)) + (116_213_000 as Weight) + .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(4 as Weight)) } fn on_initialize() -> Weight { - (118_212_000 as Weight) + (111_483_000 as Weight) .saturating_add(RocksDbWeight::get().reads(6 as Weight)) - .saturating_add(RocksDbWeight::get().writes(6 as Weight)) + .saturating_add(RocksDbWeight::get().writes(4 as Weight)) } } From 6b148240f7666121e149afc1e4f9125bb79acc3f Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Wed, 25 Nov 2020 01:49:30 -0800 Subject: [PATCH 12/29] Update lib.rs --- frame/lottery/src/lib.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/frame/lottery/src/lib.rs b/frame/lottery/src/lib.rs index 8e3a0371bea6d..8a91db0b4d84d 100644 --- a/frame/lottery/src/lib.rs +++ b/frame/lottery/src/lib.rs @@ -203,6 +203,10 @@ decl_module! { Lottery::::kill(); TicketsCount::kill(); + // We choose not need to kill Participants and Tickets to avoid a large number + // of writes at one time. Instead, data persists between lotteries, but is not used + // if it is not relevant. + return T::WeightInfo::on_initialize() } else { return Weight::zero() From e9e0b5aedb3a0caf8018be9ac39be17a8846ea36 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Wed, 25 Nov 2020 01:49:49 -0800 Subject: [PATCH 13/29] Update bin/node/runtime/src/lib.rs --- bin/node/runtime/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 57e025d9e75db..55b09a30236e8 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -905,7 +905,7 @@ impl pallet_lottery::Trait for Runtime { type Randomness = RandomnessCollectiveFlip; type ManagerOrigin = EnsureRoot; type MaxCalls = MaxCalls; - type WeightInfo = (); + type WeightInfo = pallet_lottery::weights::SubstrateWeight; } construct_runtime!( From eee7412111a4bf4bb376ecfbbbefd1c9715c19d5 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Mon, 30 Nov 2020 18:53:40 -0800 Subject: [PATCH 14/29] trait -> config --- frame/lottery/src/benchmarking.rs | 2 +- frame/lottery/src/lib.rs | 28 ++++++++++++++-------------- frame/lottery/src/mock.rs | 6 +++--- frame/lottery/src/weights.rs | 2 +- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/frame/lottery/src/benchmarking.rs b/frame/lottery/src/benchmarking.rs index eb3f36b42724a..a52882b68527d 100644 --- a/frame/lottery/src/benchmarking.rs +++ b/frame/lottery/src/benchmarking.rs @@ -29,7 +29,7 @@ use sp_runtime::traits::Bounded; use crate::Module as Lottery; // Set up and start a lottery -fn start_lottery() -> Result<(), &'static str> { +fn start_lottery() -> Result<(), &'static str> { let price = T::Currency::minimum_balance(); let start = 0u32.into(); let end = 10u32.into(); diff --git a/frame/lottery/src/lib.rs b/frame/lottery/src/lib.rs index 8a91db0b4d84d..f6956db76e4d2 100644 --- a/frame/lottery/src/lib.rs +++ b/frame/lottery/src/lib.rs @@ -39,10 +39,10 @@ use frame_system::ensure_signed; use codec::{Encode, Decode}; pub use weights::WeightInfo; -type BalanceOf = <::Currency as Currency<::AccountId>>::Balance; +type BalanceOf = <::Currency as Currency<::AccountId>>::Balance; /// The module's config trait. -pub trait Trait: frame_system::Trait { +pub trait Config: frame_system::Config { /// The Lottery's module id type ModuleId: Get; @@ -56,7 +56,7 @@ pub trait Trait: frame_system::Trait { type Randomness: Randomness; /// The overarching event type. - type Event: From> + Into<::Event>; + type Event: From> + Into<::Event>; /// The manager origin. type ManagerOrigin: EnsureOrigin; @@ -75,7 +75,7 @@ type Index = u32; type CallIndex = (u8, u8); decl_storage! { - trait Store for Module as Lottery { + trait Store for Module as Lottery { LotteryIndex: Index; /// The configuration for the current lottery. Lottery: Option>>; @@ -93,9 +93,9 @@ decl_storage! { decl_event!( pub enum Event where - ::AccountId, + ::AccountId, Balance = BalanceOf, - Call = ::Call, + Call = ::Call, { /// A lottery has been started! LotteryStarted, @@ -107,7 +107,7 @@ decl_event!( ); decl_error! { - pub enum Error for Module { + pub enum Error for Module { /// An overflow has occurred. Overflow, /// A lottery has not been configured. @@ -146,7 +146,7 @@ pub struct LotteryConfig { } decl_module! { - pub struct Module for enum Call where origin: T::Origin, system = frame_system { + pub struct Module for enum Call where origin: T::Origin, system = frame_system { /// TODO: Expose Constants fn deposit_event() = default; @@ -158,7 +158,7 @@ decl_module! { start: T::BlockNumber, end: T::BlockNumber, payout: T::BlockNumber, - calls: Vec<::Call>, + calls: Vec<::Call>, ) { T::ManagerOrigin::ensure_origin(origin)?; ensure!(calls.len() <= T::MaxCalls::get(), Error::::TooManyCalls); @@ -179,7 +179,7 @@ decl_module! { T::WeightInfo::buy_ticket() .saturating_add(call.get_dispatch_info().weight) ] - fn buy_ticket(origin, call: Box<::Call>) { + fn buy_ticket(origin, call: Box<::Call>) { let caller = ensure_signed(origin.clone())?; call.clone().dispatch(origin).map_err(|e| e.error)?; @@ -216,7 +216,7 @@ decl_module! { } } -impl Module { +impl Module { /// The account ID of the lottery pot. /// /// This actually does computation. If you need to keep using it, then make sure you cache the @@ -236,7 +236,7 @@ impl Module { } // Converts a vector of calls into a vector of call indices. - fn calls_to_indices(calls: &[::Call]) -> Result, DispatchError> { + fn calls_to_indices(calls: &[::Call]) -> Result, DispatchError> { let mut indices = Vec::with_capacity(calls.len()); for c in calls.iter() { let index = Self::call_to_index(c)?; @@ -246,14 +246,14 @@ impl Module { } // Convert a call to it's call index by encoding the call and taking the first two bytes. - fn call_to_index(call: &::Call) -> Result { + fn call_to_index(call: &::Call) -> Result { let encoded_call = call.encode(); if encoded_call.len() < 2 { Err(Error::::EncodingFailed)? } return Ok((encoded_call[0], encoded_call[1])) } // Logic for buying a ticket. - fn do_buy_ticket(caller: &T::AccountId, call: &::Call) -> DispatchResult { + fn do_buy_ticket(caller: &T::AccountId, call: &::Call) -> DispatchResult { // Check the call is valid lottery let config = Lottery::::get().ok_or(Error::::NotConfigured)?; let block_number = frame_system::Module::::block_number(); diff --git a/frame/lottery/src/mock.rs b/frame/lottery/src/mock.rs index 09db673b26187..eafa8f5239558 100644 --- a/frame/lottery/src/mock.rs +++ b/frame/lottery/src/mock.rs @@ -51,7 +51,7 @@ parameter_types! { pub const AvailableBlockRatio: Perbill = Perbill::one(); } -impl frame_system::Trait for Test { +impl frame_system::Config for Test { type BaseCallFilter = (); type Origin = Origin; type Index = u64; @@ -83,7 +83,7 @@ parameter_types! { pub const ExistentialDeposit: u64 = 1; } -impl pallet_balances::Trait for Test { +impl pallet_balances::Config for Test { type MaxLocks = (); type Balance = u64; type Event = (); @@ -98,7 +98,7 @@ parameter_types! { pub const MaxCalls: usize = 2; } -impl Trait for Test { +impl Config for Test { type ModuleId = LotteryModuleId; type Call = Call; type Currency = Balances; diff --git a/frame/lottery/src/weights.rs b/frame/lottery/src/weights.rs index 7d2f486c810a2..0bf9f5a2138a1 100644 --- a/frame/lottery/src/weights.rs +++ b/frame/lottery/src/weights.rs @@ -51,7 +51,7 @@ pub trait WeightInfo { /// Weights for pallet_lottery using the Substrate node and recommended hardware. pub struct SubstrateWeight(PhantomData); -impl WeightInfo for SubstrateWeight { +impl WeightInfo for SubstrateWeight { fn setup_lottery(n: u32, ) -> Weight { (31_350_000 as Weight) .saturating_add((496_000 as Weight).saturating_mul(n as Weight)) From 47b18601c09a7673be31175f7a88f08eeaa1ecef Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Mon, 30 Nov 2020 22:14:35 -0800 Subject: [PATCH 15/29] add repeating lottery --- frame/lottery/src/benchmarking.rs | 22 +++--- frame/lottery/src/lib.rs | 121 +++++++++++++++++------------- frame/lottery/src/tests.rs | 51 +++++++------ frame/lottery/src/weights.rs | 23 ++++-- 4 files changed, 126 insertions(+), 91 deletions(-) diff --git a/frame/lottery/src/benchmarking.rs b/frame/lottery/src/benchmarking.rs index a52882b68527d..7010be8805cf5 100644 --- a/frame/lottery/src/benchmarking.rs +++ b/frame/lottery/src/benchmarking.rs @@ -29,11 +29,10 @@ use sp_runtime::traits::Bounded; use crate::Module as Lottery; // Set up and start a lottery -fn start_lottery() -> Result<(), &'static str> { +fn setup_lottery() -> Result<(), &'static str> { let price = T::Currency::minimum_balance(); - let start = 0u32.into(); - let end = 10u32.into(); - let payout = 15u32.into(); + let length = 10u32.into(); + let delay = 5u32.into(); // Calls will be maximum length... let mut calls = vec![ frame_system::Call::::set_code(vec![]).into(); @@ -42,22 +41,21 @@ fn start_lottery() -> Result<(), &'static str> { // Last call will be the match for worst case scenario. calls.push(frame_system::Call::::remark(vec![]).into()); let origin = T::ManagerOrigin::successful_origin(); - Lottery::::setup_lottery(origin, price, start, end, payout, calls)?; + Lottery::::start_lottery(origin, price, length, delay, calls, false)?; Ok(()) } benchmarks! { _ { } - setup_lottery { + start_lottery { let n in 0 .. T::MaxCalls::get() as u32; let price = BalanceOf::::max_value(); - let start = 0u32.into(); let end = 10u32.into(); - let payout = 15u32.into(); + let payout = 5u32.into(); let calls = vec![frame_system::Call::::remark(vec![]).into(); n as usize]; - let call = Call::::setup_lottery(price, start, end, payout, calls); + let call = Call::::start_lottery(price, end, payout, calls, true); let origin = T::ManagerOrigin::successful_origin(); }: { call.dispatch_bypass_filter(origin)? } verify { @@ -67,7 +65,7 @@ benchmarks! { buy_ticket { let caller = whitelisted_caller(); T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); - start_lottery::()?; + setup_lottery::()?; // force user to have a long vec of calls participating let set_code_index: CallIndex = Lottery::::call_to_index( &frame_system::Call::::set_code(vec![]).into() @@ -88,7 +86,7 @@ benchmarks! { } on_initialize { - start_lottery::()?; + setup_lottery::()?; let winner = account("winner", 0, 0); // User needs more than min balance to get ticket T::Currency::make_free_balance_be(&winner, T::Currency::minimum_balance() * 10u32.into()); @@ -124,7 +122,7 @@ mod tests { #[test] fn test_benchmarks() { new_test_ext().execute_with(|| { - assert_ok!(test_benchmark_setup_lottery::()); + assert_ok!(test_benchmark_start_lottery::()); assert_ok!(test_benchmark_buy_ticket::()); assert_ok!(test_benchmark_on_initialize::()); }); diff --git a/frame/lottery/src/lib.rs b/frame/lottery/src/lib.rs index f6956db76e4d2..753bd41253622 100644 --- a/frame/lottery/src/lib.rs +++ b/frame/lottery/src/lib.rs @@ -74,6 +74,23 @@ type Index = u32; // 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)] +pub struct LotteryConfig { + /// Price per entry. + price: Balance, + /// Starting block of the lottery. + start: BlockNumber, + /// Length of the lottery (start + length = end). + length: BlockNumber, + /// Delay for choosing the winner of the lottery. (start + length + delay = payout). + /// Randomness in the "payout" block will be used to determine the winner. + delay: BlockNumber, + /// Calls available this lottery. + calls: Vec, + /// Whether this lottery will repeat after it completes. + repeat: bool, +} + decl_storage! { trait Store for Module as Lottery { LotteryIndex: Index; @@ -112,8 +129,6 @@ decl_error! { Overflow, /// A lottery has not been configured. NotConfigured, - /// A lottery has not started. - NotStarted, /// A lottery is already in progress. InProgress, /// A lottery has already ended. @@ -129,36 +144,20 @@ decl_error! { } } -#[derive(Encode, Decode, Default, RuntimeDebug)] -pub struct LotteryConfig { - /// Index of this lottery - index: Index, - /// Price per entry. - price: Balance, - /// Starting block of the lottery. - start: BlockNumber, - /// Ending block of the lottery. - end: BlockNumber, - /// Payout block of the lottery. - payout: BlockNumber, - /// Calls available this lottery. - calls: Vec, -} - decl_module! { pub struct Module for enum Call where origin: T::Origin, system = frame_system { - /// TODO: Expose Constants + const ModuleId: ModuleId = T::ModuleId::get(); + const MaxCalls: u32 = T::MaxCalls::get() as u32; fn deposit_event() = default; - #[weight = T::WeightInfo::setup_lottery(calls.len() as u32)] - fn setup_lottery( - origin, + #[weight = T::WeightInfo::start_lottery(calls.len() as u32)] + fn start_lottery(origin, price: BalanceOf, - start: T::BlockNumber, - end: T::BlockNumber, - payout: T::BlockNumber, + length: T::BlockNumber, + delay: T::BlockNumber, calls: Vec<::Call>, + repeat: bool, ) { T::ManagerOrigin::ensure_origin(origin)?; ensure!(calls.len() <= T::MaxCalls::get(), Error::::TooManyCalls); @@ -167,8 +166,16 @@ decl_module! { ensure!(lottery.is_none(), Error::::InProgress); let index = LotteryIndex::get(); let new_index = index.checked_add(1).ok_or(Error::::Overflow)?; + let start = frame_system::Module::::block_number(); // Use new_index to more easily track everything with the current state. - *lottery = Some(LotteryConfig { index: new_index, price, start, end, payout, calls: indices }); + *lottery = Some(LotteryConfig { + price, + start, + length, + delay, + calls: indices, + repeat, + }); LotteryIndex::put(new_index); Ok(()) })?; @@ -189,30 +196,43 @@ decl_module! { } fn on_initialize(n: T::BlockNumber) -> Weight { - if Lottery::::get().map_or(false, |config| config.payout <= n) { - let (lottery_account, lottery_balance) = Self::pot(); - let ticket_count = TicketsCount::get(); - - let winning_number = Self::choose_winner(ticket_count); - let winner = Tickets::::get(winning_number).unwrap_or(lottery_account); - // Not much we can do if this fails... - let _ = T::Currency::transfer(&Self::account_id(), &winner, lottery_balance, KeepAlive); - - Self::deposit_event(RawEvent::Winner(winner, lottery_balance)); - - Lottery::::kill(); - TicketsCount::kill(); - - // We choose not need to kill Participants and Tickets to avoid a large number - // of writes at one time. Instead, data persists between lotteries, but is not used - // if it is not relevant. - - return T::WeightInfo::on_initialize() - } else { - return Weight::zero() - } + Lottery::::mutate(|mut lottery| -> Weight { + if let Some(config) = &mut lottery { + let payout_block = 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::get(); + + let winning_number = Self::choose_winner(ticket_count); + let winner = Tickets::::get(winning_number).unwrap_or(lottery_account); + // Not much we can do if this fails... + let _ = T::Currency::transfer(&Self::account_id(), &winner, lottery_balance, KeepAlive); + + Self::deposit_event(RawEvent::Winner(winner, lottery_balance)); + + TicketsCount::kill(); + + if config.repeat { + // If lottery should repeat, increment index by 1. + LotteryIndex::mutate(|index| *index = index.saturating_add(1)); + // Set a new start with the current block. + config.start = n; + return T::WeightInfo::on_initialize_repeat() + } else { + // Else, kill the lottery storage. + *lottery = None; + return T::WeightInfo::on_initialize_end() + } + // We choose not need to kill Participants and Tickets to avoid a large number + // of writes at one time. Instead, data persists between lotteries, but is not used + // if it is not relevant. + } + } + return T::DbWeight::get().reads(1) + }) } - } } @@ -257,8 +277,7 @@ impl Module { // Check the call is valid lottery let config = Lottery::::get().ok_or(Error::::NotConfigured)?; let block_number = frame_system::Module::::block_number(); - ensure!(block_number >= config.start, Error::::NotStarted); - ensure!(block_number < config.end, Error::::AlreadyEnded); + ensure!(block_number < config.start.saturating_add(config.length), Error::::AlreadyEnded); let call_index = Self::call_to_index(call)?; ensure!(config.calls.iter().any(|c| call_index == *c), Error::::InvalidCall); let ticket_count = TicketsCount::get(); diff --git a/frame/lottery/src/tests.rs b/frame/lottery/src/tests.rs index d7eb93dffb01f..4dc3f8a9b32a1 100644 --- a/frame/lottery/src/tests.rs +++ b/frame/lottery/src/tests.rs @@ -41,21 +41,17 @@ fn initial_state() { fn basic_end_to_end_works() { new_test_ext().execute_with(|| { let price = 10; - let start = 5; - let end = 20; - let payout = 25; + let length = 20; + let delay = 5; let calls = vec![ Call::Balances(BalancesCall::force_transfer(0, 0, 0)), Call::Balances(BalancesCall::transfer(0, 0)), ]; - // Setup Lottery - assert_ok!(Lottery::setup_lottery(Origin::root(), price, start, end, payout, calls.clone())); + // Start lottery + assert_ok!(Lottery::start_lottery(Origin::root(), price, length, delay, calls.clone(), true)); assert!(crate::Lottery::::get().is_some()); - // Go to start - run_to_block(5); - assert_eq!(Balances::free_balance(&1), 100); let call = Box::new(Call::Balances(BalancesCall::transfer(2, 20))); assert_ok!(Lottery::buy_ticket(Origin::signed(1), call.clone())); @@ -80,21 +76,31 @@ fn basic_end_to_end_works() { // Go to payout run_to_block(25); - // Lottery is reset - assert!(crate::Lottery::::get().is_none()); - assert_eq!(TicketsCount::get(), 0); // User 1 wins assert_eq!(Balances::free_balance(&1), 70 + 40); + // Lottery is reset and restarted + assert_eq!(TicketsCount::get(), 0); + assert_eq!(LotteryIndex::get(), 2); + assert_eq!( + crate::Lottery::::get().unwrap(), + LotteryConfig { + price, + start: 25, + length, + delay, + calls: vec![(1,2), (1,0)], + repeat: true, + } + ); }); } #[test] -fn setup_lottery_works() { +fn start_lottery_works() { new_test_ext().execute_with(|| { let price = 10; - let start = 5; - let end = 20; - let payout = 25; + let length = 20; + let delay = 5; let calls = vec![ Call::Balances(BalancesCall::force_transfer(0, 0, 0)), Call::Balances(BalancesCall::transfer(0, 0)), @@ -107,21 +113,21 @@ fn setup_lottery_works() { // Setup ignores bad origin assert_noop!( - Lottery::setup_lottery(Origin::signed(1), price, start, end, payout, calls.clone()), + Lottery::start_lottery(Origin::signed(1), price, length, delay, calls.clone(), false), BadOrigin, ); // Too many calls assert_noop!( - Lottery::setup_lottery(Origin::root(), price, start, end, payout, too_many_calls), + Lottery::start_lottery(Origin::root(), price, length, delay, too_many_calls, false), Error::::TooManyCalls, ); // All good - assert_ok!(Lottery::setup_lottery(Origin::root(), price, start, end, payout, calls.clone())); + assert_ok!(Lottery::start_lottery(Origin::root(), price, length, delay, calls.clone(), false)); // Can't open another one if lottery is already present assert_noop!( - Lottery::setup_lottery(Origin::root(), price, start, end, payout, calls), + Lottery::start_lottery(Origin::root(), price, length, delay, calls, false), Error::::InProgress, ); }); @@ -145,7 +151,7 @@ fn buy_ticket_works_as_simple_passthrough() { Call::Balances(BalancesCall::transfer(0, 0)), ]; // Ticket price of 60 would kill the user's account - assert_ok!(Lottery::setup_lottery(Origin::root(), 60, 0, 10, 15, calls.clone())); + assert_ok!(Lottery::start_lottery(Origin::root(), 60, 10, 5, calls.clone(), false)); assert_ok!(Lottery::buy_ticket(Origin::signed(1), call.clone())); assert_eq!(Balances::free_balance(&1), 100 - 20 - 20); assert_eq!(TicketsCount::get(), 0); @@ -177,14 +183,15 @@ fn buy_ticket_works() { Call::System(SystemCall::remark(vec![])), Call::Balances(BalancesCall::transfer(0, 0)), ]; - // Setup lottery - assert_ok!(Lottery::setup_lottery(Origin::root(), 1, 5, 20, 25, calls.clone())); // Can't buy ticket before start let call = Box::new(Call::Balances(BalancesCall::transfer(2, 1))); assert_ok!(Lottery::buy_ticket(Origin::signed(1), call.clone())); assert_eq!(TicketsCount::get(), 0); + // Start lottery + assert_ok!(Lottery::start_lottery(Origin::root(), 1, 20, 5, calls.clone(), false)); + // Go to start, buy ticket for transfer run_to_block(5); assert_ok!(Lottery::buy_ticket(Origin::signed(1), call)); diff --git a/frame/lottery/src/weights.rs b/frame/lottery/src/weights.rs index 0bf9f5a2138a1..50b3a41769cf4 100644 --- a/frame/lottery/src/weights.rs +++ b/frame/lottery/src/weights.rs @@ -44,15 +44,16 @@ use sp_std::marker::PhantomData; /// Weight functions needed for pallet_lottery. pub trait WeightInfo { - fn setup_lottery(n: u32, ) -> Weight; + fn start_lottery(n: u32, ) -> Weight; fn buy_ticket() -> Weight; - fn on_initialize() -> Weight; + fn on_initialize_repeat() -> Weight; + fn on_initialize_end() -> Weight; } /// Weights for pallet_lottery using the Substrate node and recommended hardware. pub struct SubstrateWeight(PhantomData); impl WeightInfo for SubstrateWeight { - fn setup_lottery(n: u32, ) -> Weight { + fn start_lottery(n: u32, ) -> Weight { (31_350_000 as Weight) .saturating_add((496_000 as Weight).saturating_mul(n as Weight)) .saturating_add(T::DbWeight::get().reads(2 as Weight)) @@ -63,7 +64,12 @@ impl WeightInfo for SubstrateWeight { .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().writes(4 as Weight)) } - fn on_initialize() -> Weight { + fn on_initialize_repeat() -> Weight { + (111_483_000 as Weight) + .saturating_add(T::DbWeight::get().reads(6 as Weight)) + .saturating_add(T::DbWeight::get().writes(4 as Weight)) + } + fn on_initialize_end() -> Weight { (111_483_000 as Weight) .saturating_add(T::DbWeight::get().reads(6 as Weight)) .saturating_add(T::DbWeight::get().writes(4 as Weight)) @@ -72,7 +78,7 @@ impl WeightInfo for SubstrateWeight { // For backwards compatibility and tests impl WeightInfo for () { - fn setup_lottery(n: u32, ) -> Weight { + fn start_lottery(n: u32, ) -> Weight { (31_350_000 as Weight) .saturating_add((496_000 as Weight).saturating_mul(n as Weight)) .saturating_add(RocksDbWeight::get().reads(2 as Weight)) @@ -83,7 +89,12 @@ impl WeightInfo for () { .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(4 as Weight)) } - fn on_initialize() -> Weight { + fn on_initialize_repeat() -> Weight { + (111_483_000 as Weight) + .saturating_add(RocksDbWeight::get().reads(6 as Weight)) + .saturating_add(RocksDbWeight::get().writes(4 as Weight)) + } + fn on_initialize_end() -> Weight { (111_483_000 as Weight) .saturating_add(RocksDbWeight::get().reads(6 as Weight)) .saturating_add(RocksDbWeight::get().writes(4 as Weight)) From 05c97ef60c1172079b3a25765000961811ae05d4 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Mon, 30 Nov 2020 22:30:06 -0800 Subject: [PATCH 16/29] new benchmarks --- frame/lottery/src/benchmarking.rs | 41 ++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/frame/lottery/src/benchmarking.rs b/frame/lottery/src/benchmarking.rs index 7010be8805cf5..4ac916f8a2486 100644 --- a/frame/lottery/src/benchmarking.rs +++ b/frame/lottery/src/benchmarking.rs @@ -29,7 +29,7 @@ use sp_runtime::traits::Bounded; use crate::Module as Lottery; // Set up and start a lottery -fn setup_lottery() -> Result<(), &'static str> { +fn setup_lottery(repeat: bool) -> Result<(), &'static str> { let price = T::Currency::minimum_balance(); let length = 10u32.into(); let delay = 5u32.into(); @@ -41,7 +41,7 @@ fn setup_lottery() -> Result<(), &'static str> { // Last call will be the match for worst case scenario. calls.push(frame_system::Call::::remark(vec![]).into()); let origin = T::ManagerOrigin::successful_origin(); - Lottery::::start_lottery(origin, price, length, delay, calls, false)?; + Lottery::::start_lottery(origin, price, length, delay, calls, repeat)?; Ok(()) } @@ -65,7 +65,7 @@ benchmarks! { buy_ticket { let caller = whitelisted_caller(); T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); - setup_lottery::()?; + setup_lottery::(false)?; // force user to have a long vec of calls participating let set_code_index: CallIndex = Lottery::::call_to_index( &frame_system::Call::::set_code(vec![]).into() @@ -85,8 +85,8 @@ benchmarks! { assert_eq!(TicketsCount::get(), 1); } - on_initialize { - setup_lottery::()?; + on_initialize_end { + setup_lottery::(false)?; let winner = account("winner", 0, 0); // User needs more than min balance to get ticket T::Currency::make_free_balance_be(&winner, T::Currency::minimum_balance() * 10u32.into()); @@ -111,6 +111,34 @@ benchmarks! { assert_eq!(Lottery::::pot().1, 0u32.into()); assert!(!T::Currency::free_balance(&winner).is_zero()) } + + on_initialize_repeat { + setup_lottery::(true)?; + let winner = account("winner", 0, 0); + // User needs more than min balance to get ticket + T::Currency::make_free_balance_be(&winner, T::Currency::minimum_balance() * 10u32.into()); + // Make sure lottery account has at least min balance too + let lottery_account = Lottery::::account_id(); + T::Currency::make_free_balance_be(&lottery_account, T::Currency::minimum_balance() * 10u32.into()); + // Buy a ticket + let call = frame_system::Call::::remark(vec![]); + Lottery::::buy_ticket(RawOrigin::Signed(winner.clone()).into(), Box::new(call.into()))?; + // Kill user account for worst case + T::Currency::make_free_balance_be(&winner, 0u32.into()); + // Assert that lotto is set up for winner + assert_eq!(TicketsCount::get(), 1); + assert!(!Lottery::::pot().1.is_zero()); + }: { + // Start lottery has block 15 configured for payout + Lottery::::on_initialize(15u32.into()); + } + verify { + assert!(crate::Lottery::::get().is_some()); + assert_eq!(LotteryIndex::get(), 2); + assert_eq!(TicketsCount::get(), 0); + assert_eq!(Lottery::::pot().1, 0u32.into()); + assert!(!T::Currency::free_balance(&winner).is_zero()) + } } #[cfg(test)] @@ -124,7 +152,8 @@ mod tests { new_test_ext().execute_with(|| { assert_ok!(test_benchmark_start_lottery::()); assert_ok!(test_benchmark_buy_ticket::()); - assert_ok!(test_benchmark_on_initialize::()); + assert_ok!(test_benchmark_on_initialize_end::()); + assert_ok!(test_benchmark_on_initialize_repeat::()); }); } } From c78726fd7104c0ccbfbdd4897ff3bec7a9a89461 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Mon, 30 Nov 2020 22:31:30 -0800 Subject: [PATCH 17/29] fix build --- bin/node/runtime/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index d6e758c56b085..cdd11c7a4877a 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -897,7 +897,7 @@ parameter_types! { pub const MaxCalls: usize = 10; } -impl pallet_lottery::Trait for Runtime { +impl pallet_lottery::Config for Runtime { type ModuleId = LotteryModuleId; type Call = Call; type Event = Event; From 5585006ab6c7c8f015f6d4f8cdd1d52c8e1f76bd Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Mon, 30 Nov 2020 22:34:16 -0800 Subject: [PATCH 18/29] move trait for warning --- frame/lottery/src/benchmarking.rs | 2 +- frame/lottery/src/lib.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/lottery/src/benchmarking.rs b/frame/lottery/src/benchmarking.rs index 4ac916f8a2486..55b0e2d8a1a58 100644 --- a/frame/lottery/src/benchmarking.rs +++ b/frame/lottery/src/benchmarking.rs @@ -24,7 +24,7 @@ use super::*; use frame_system::RawOrigin; use frame_support::traits::{OnInitialize, UnfilteredDispatchable}; use frame_benchmarking::{benchmarks, account, whitelisted_caller}; -use sp_runtime::traits::Bounded; +use sp_runtime::traits::{Bounded, Zero}; use crate::Module as Lottery; diff --git a/frame/lottery/src/lib.rs b/frame/lottery/src/lib.rs index 753bd41253622..d7204e42057e3 100644 --- a/frame/lottery/src/lib.rs +++ b/frame/lottery/src/lib.rs @@ -28,7 +28,7 @@ pub mod weights; use sp_std::prelude::*; use sp_runtime::{DispatchError, ModuleId}; -use sp_runtime::traits::{AccountIdConversion, Saturating, Zero}; +use sp_runtime::traits::{AccountIdConversion, Saturating}; use frame_support::{Parameter, decl_module, decl_error, decl_event, decl_storage, ensure, RuntimeDebug}; use frame_support::dispatch::{Dispatchable, DispatchResult, GetDispatchInfo}; use frame_support::traits::{ From 5ded45d19dfedf30c0cbfb14f669c2d6d21c543d Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Mon, 30 Nov 2020 23:03:59 -0800 Subject: [PATCH 19/29] feedback from @xlc --- frame/lottery/src/lib.rs | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/frame/lottery/src/lib.rs b/frame/lottery/src/lib.rs index d7204e42057e3..7355469670d59 100644 --- a/frame/lottery/src/lib.rs +++ b/frame/lottery/src/lib.rs @@ -68,8 +68,6 @@ pub trait Config: frame_system::Config { type WeightInfo: WeightInfo; } -type Index = u32; - // Any runtime call can be encoded into two bytes which represent the pallet and call index. // We use this to uniquely match someone's incoming call with the calls configured for the lottery. type CallIndex = (u8, u8); @@ -93,18 +91,18 @@ pub struct LotteryConfig { decl_storage! { trait Store for Module as Lottery { - LotteryIndex: Index; + LotteryIndex: u32; /// The configuration for the current lottery. Lottery: Option>>; /// Users who have purchased a ticket. (Lottery Index, Tickets Purchased) - Participants: map hasher(twox_64_concat) T::AccountId => (Index, Vec); + Participants: map hasher(twox_64_concat) T::AccountId => (u32, Vec); /// Total number of tickets sold. - TicketsCount: Index; + TicketsCount: u32; /// Each ticket's owner. /// /// May have residual storage from previous lotteries. Use `TicketsCount` to see which ones /// are actually valid ticket mappings. - Tickets: map hasher(twox_64_concat) Index => Option; + Tickets: map hasher(twox_64_concat) u32 => Option; } } @@ -112,14 +110,13 @@ decl_event!( pub enum Event where ::AccountId, Balance = BalanceOf, - Call = ::Call, { /// A lottery has been started! LotteryStarted, /// A winner has been chosen! Winner(AccountId, Balance), /// A ticket has been bought! - TicketBought(AccountId, Call), + TicketBought(AccountId, CallIndex), } ); @@ -188,11 +185,15 @@ decl_module! { ] fn buy_ticket(origin, call: Box<::Call>) { let caller = ensure_signed(origin.clone())?; - call.clone().dispatch(origin).map_err(|e| e.error)?; + // This conversion may fail, but we want this function to always act + // as a passthrough... so we will parse the result below. + let maybe_call_index = Self::call_to_index(&call); + call.dispatch(origin).map_err(|e| e.error)?; // Only try to buy a ticket if the underlying call is successful. // Not much we can do if this fails. - let _ = Self::do_buy_ticket(&caller, &call); + let call_index = maybe_call_index?; + let _ = Self::do_buy_ticket(&caller, &call_index); } fn on_initialize(n: T::BlockNumber) -> Weight { @@ -273,13 +274,12 @@ impl Module { } // Logic for buying a ticket. - fn do_buy_ticket(caller: &T::AccountId, call: &::Call) -> DispatchResult { + fn do_buy_ticket(caller: &T::AccountId, call_index: &(u8, u8)) -> DispatchResult { // Check the call is valid lottery let config = Lottery::::get().ok_or(Error::::NotConfigured)?; let block_number = frame_system::Module::::block_number(); ensure!(block_number < config.start.saturating_add(config.length), Error::::AlreadyEnded); - let call_index = Self::call_to_index(call)?; - ensure!(config.calls.iter().any(|c| call_index == *c), Error::::InvalidCall); + ensure!(config.calls.iter().any(|c| call_index == c), Error::::InvalidCall); let ticket_count = TicketsCount::get(); let new_ticket_count = ticket_count.checked_add(1).ok_or(Error::::Overflow)?; // Try to update the participant status @@ -291,25 +291,25 @@ impl Module { *lottery_index = index; } else { // Check that user is not already participating under this call. - ensure!(!participating_calls.iter().any(|c| call_index == *c), Error::::AlreadyParticipating); + ensure!(!participating_calls.iter().any(|c| call_index == c), Error::::AlreadyParticipating); } // Check user has enough funds and send it to the Lottery account. T::Currency::transfer(caller, &Self::account_id(), config.price, KeepAlive)?; // Create a new ticket. TicketsCount::put(new_ticket_count); Tickets::::insert(ticket_count, caller.clone()); - participating_calls.push(call_index); + participating_calls.push(*call_index); Ok(()) })?; - Self::deposit_event(RawEvent::TicketBought(caller.clone(), call.clone())); + Self::deposit_event(RawEvent::TicketBought(caller.clone(), *call_index)); Ok(()) } // Randomly choose a winner from among the total number of participants. fn choose_winner(total: u32) -> u32 { - let random_seed = T::Randomness::random(b"lottery"); + let random_seed = T::Randomness::random(&T::ModuleId::get().encode()); let random_number = ::decode(&mut random_seed.as_ref()) .expect("secure hashes should always be bigger than u32; qed"); random_number % total From 9739b404398e46dd5c2250bc7a1948e40694f000 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Tue, 1 Dec 2020 00:01:24 -0800 Subject: [PATCH 20/29] add stop_repeat --- frame/lottery/src/benchmarking.rs | 40 +++++++++++++++++++------------ frame/lottery/src/lib.rs | 37 ++++++++++++++++++++++++++++ frame/lottery/src/tests.rs | 3 ++- frame/lottery/src/weights.rs | 11 +++++++++ 4 files changed, 75 insertions(+), 16 deletions(-) diff --git a/frame/lottery/src/benchmarking.rs b/frame/lottery/src/benchmarking.rs index 55b0e2d8a1a58..556c101712e5b 100644 --- a/frame/lottery/src/benchmarking.rs +++ b/frame/lottery/src/benchmarking.rs @@ -48,20 +48,6 @@ fn setup_lottery(repeat: bool) -> Result<(), &'static str> { benchmarks! { _ { } - start_lottery { - let n in 0 .. T::MaxCalls::get() as u32; - let price = BalanceOf::::max_value(); - let end = 10u32.into(); - let payout = 5u32.into(); - let calls = vec![frame_system::Call::::remark(vec![]).into(); n as usize]; - - let call = Call::::start_lottery(price, end, payout, calls, true); - let origin = T::ManagerOrigin::successful_origin(); - }: { call.dispatch_bypass_filter(origin)? } - verify { - assert!(crate::Lottery::::get().is_some()); - } - buy_ticket { let caller = whitelisted_caller(); T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); @@ -70,7 +56,7 @@ benchmarks! { let set_code_index: CallIndex = Lottery::::call_to_index( &frame_system::Call::::set_code(vec![]).into() )?; - let already_called: (Index, Vec) = ( + let already_called: (u32, Vec) = ( LotteryIndex::get(), vec![ set_code_index; @@ -85,6 +71,30 @@ benchmarks! { assert_eq!(TicketsCount::get(), 1); } + start_lottery { + let n in 0 .. T::MaxCalls::get() as u32; + let price = BalanceOf::::max_value(); + let end = 10u32.into(); + let payout = 5u32.into(); + let calls = vec![frame_system::Call::::remark(vec![]).into(); n as usize]; + + let call = Call::::start_lottery(price, end, payout, calls, true); + let origin = T::ManagerOrigin::successful_origin(); + }: { call.dispatch_bypass_filter(origin)? } + verify { + assert!(crate::Lottery::::get().is_some()); + } + + stop_repeat { + setup_lottery::(true)?; + assert_eq!(crate::Lottery::::get().unwrap().repeat, true); + let call = Call::::stop_repeat(); + let origin = T::ManagerOrigin::successful_origin(); + }: { call.dispatch_bypass_filter(origin)? } + verify { + assert!(crate::Lottery::::get().unwrap().repeat, false); + } + on_initialize_end { setup_lottery::(false)?; let winner = account("winner", 0, 0); diff --git a/frame/lottery/src/lib.rs b/frame/lottery/src/lib.rs index 7355469670d59..ce1d27d8fe19e 100644 --- a/frame/lottery/src/lib.rs +++ b/frame/lottery/src/lib.rs @@ -148,6 +148,18 @@ decl_module! { fn deposit_event() = default; + /// Start a lottery using the provided configuration. + /// + /// This extrinsic must be called by the `ManagerOrigin`. + /// + /// Parameters: + /// + /// * `price`: The cost of a single ticket. + /// * `length`: How long the lottery should run for starting at the current block. + /// * `delay`: How long after the lottery end we should wait before picking a winner. + /// * `calls`: The calls allowed for purchasing a lottery ticket. **Important**: Only + /// the call index is used to determine if the ticket purchase is valid. + /// * `repeat`: If the lottery should repeat when completed. #[weight = T::WeightInfo::start_lottery(calls.len() as u32)] fn start_lottery(origin, price: BalanceOf, @@ -179,6 +191,17 @@ decl_module! { Self::deposit_event(RawEvent::LotteryStarted); } + /// Buy a ticket to enter the lottery. + /// + /// This extrinsic acts as a passthrough function for `call`. In all + /// situations where `call` alone would succeed, this extrinsic should + /// succeed. + /// + /// If `call` is successful, then we will attempt to purchase a ticket, + /// which may fail silently. To detect success of a ticket purchase, you + /// should listen for the `TicketBought` event. + /// + /// This extrinsic must be called by a signed origin. #[weight = T::WeightInfo::buy_ticket() .saturating_add(call.get_dispatch_info().weight) @@ -196,6 +219,20 @@ decl_module! { let _ = Self::do_buy_ticket(&caller, &call_index); } + /// If a lottery is repeating, you can use this to stop the repeat. + /// The lottery will continue to run to completion. + /// + /// This extrinsic must be called by the `ManagerOrigin`. + #[weight = T::WeightInfo::stop_repeat()] + fn stop_repeat(origin) { + T::ManagerOrigin::ensure_origin(origin)?; + Lottery::::mutate(|mut lottery| { + if let Some(config) = &mut lottery { + config.repeat = false + } + }); + } + fn on_initialize(n: T::BlockNumber) -> Weight { Lottery::::mutate(|mut lottery| -> Weight { if let Some(config) = &mut lottery { diff --git a/frame/lottery/src/tests.rs b/frame/lottery/src/tests.rs index 4dc3f8a9b32a1..9a3660daaa3fd 100644 --- a/frame/lottery/src/tests.rs +++ b/frame/lottery/src/tests.rs @@ -48,7 +48,7 @@ fn basic_end_to_end_works() { Call::Balances(BalancesCall::transfer(0, 0)), ]; - // Start lottery + // Start lottery, it repeats assert_ok!(Lottery::start_lottery(Origin::root(), price, length, delay, calls.clone(), true)); assert!(crate::Lottery::::get().is_some()); @@ -216,5 +216,6 @@ fn buy_ticket_works() { run_to_block(25); assert_ok!(Lottery::buy_ticket(Origin::signed(2), call.clone())); assert_eq!(TicketsCount::get(), 0); + assert_eq!(LotteryIndex::get(), 1); }); } diff --git a/frame/lottery/src/weights.rs b/frame/lottery/src/weights.rs index 50b3a41769cf4..17b7dd2462c01 100644 --- a/frame/lottery/src/weights.rs +++ b/frame/lottery/src/weights.rs @@ -46,6 +46,7 @@ use sp_std::marker::PhantomData; pub trait WeightInfo { fn start_lottery(n: u32, ) -> Weight; fn buy_ticket() -> Weight; + fn stop_repeat() -> Weight; fn on_initialize_repeat() -> Weight; fn on_initialize_end() -> Weight; } @@ -64,6 +65,11 @@ impl WeightInfo for SubstrateWeight { .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().writes(4 as Weight)) } + fn stop_repeat() -> Weight { + (111_483_000 as Weight) + .saturating_add(T::DbWeight::get().reads(6 as Weight)) + .saturating_add(T::DbWeight::get().writes(4 as Weight)) + } fn on_initialize_repeat() -> Weight { (111_483_000 as Weight) .saturating_add(T::DbWeight::get().reads(6 as Weight)) @@ -89,6 +95,11 @@ impl WeightInfo for () { .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(4 as Weight)) } + fn stop_repeat() -> Weight { + (116_213_000 as Weight) + .saturating_add(RocksDbWeight::get().reads(5 as Weight)) + .saturating_add(RocksDbWeight::get().writes(4 as Weight)) + } fn on_initialize_repeat() -> Weight { (111_483_000 as Weight) .saturating_add(RocksDbWeight::get().reads(6 as Weight)) From ef67c0e4a0a2ffae3dfca93fdebfd061fb79a04c Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Tue, 1 Dec 2020 00:07:37 -0800 Subject: [PATCH 21/29] fix --- frame/lottery/src/benchmarking.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frame/lottery/src/benchmarking.rs b/frame/lottery/src/benchmarking.rs index 556c101712e5b..7c3960f53d385 100644 --- a/frame/lottery/src/benchmarking.rs +++ b/frame/lottery/src/benchmarking.rs @@ -92,7 +92,7 @@ benchmarks! { let origin = T::ManagerOrigin::successful_origin(); }: { call.dispatch_bypass_filter(origin)? } verify { - assert!(crate::Lottery::::get().unwrap().repeat, false); + assert_eq!(crate::Lottery::::get().unwrap().repeat, false); } on_initialize_end { @@ -160,8 +160,9 @@ mod tests { #[test] fn test_benchmarks() { new_test_ext().execute_with(|| { - assert_ok!(test_benchmark_start_lottery::()); assert_ok!(test_benchmark_buy_ticket::()); + assert_ok!(test_benchmark_start_lottery::()); + assert_ok!(test_benchmark_stop_repeat::()); assert_ok!(test_benchmark_on_initialize_end::()); assert_ok!(test_benchmark_on_initialize_repeat::()); }); From 649cfa7a0420dc0a56e3affc844c23873e3a8a95 Mon Sep 17 00:00:00 2001 From: Parity Benchmarking Bot Date: Tue, 1 Dec 2020 08:14:17 +0000 Subject: [PATCH 22/29] cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_lottery --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/lottery/src/weights.rs --template=./.maintain/frame-weight-template.hbs --- frame/lottery/src/weights.rs | 70 ++++++++++++++++++------------------ 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/frame/lottery/src/weights.rs b/frame/lottery/src/weights.rs index 17b7dd2462c01..c8aec2120b3a8 100644 --- a/frame/lottery/src/weights.rs +++ b/frame/lottery/src/weights.rs @@ -18,7 +18,7 @@ //! Autogenerated weights for pallet_lottery //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 2.0.0 -//! DATE: 2020-11-25, STEPS: [50, ], REPEAT: 20, LOW RANGE: [], HIGH RANGE: [] +//! DATE: 2020-12-01, STEPS: [50, ], REPEAT: 20, LOW RANGE: [], HIGH RANGE: [] //! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 128 // Executed Command: @@ -44,70 +44,70 @@ use sp_std::marker::PhantomData; /// Weight functions needed for pallet_lottery. pub trait WeightInfo { - fn start_lottery(n: u32, ) -> Weight; fn buy_ticket() -> Weight; + fn start_lottery(n: u32, ) -> Weight; fn stop_repeat() -> Weight; - fn on_initialize_repeat() -> Weight; fn on_initialize_end() -> Weight; + fn on_initialize_repeat() -> Weight; } /// Weights for pallet_lottery using the Substrate node and recommended hardware. pub struct SubstrateWeight(PhantomData); impl WeightInfo for SubstrateWeight { - fn start_lottery(n: u32, ) -> Weight { - (31_350_000 as Weight) - .saturating_add((496_000 as Weight).saturating_mul(n as Weight)) - .saturating_add(T::DbWeight::get().reads(2 as Weight)) - .saturating_add(T::DbWeight::get().writes(2 as Weight)) - } fn buy_ticket() -> Weight { - (116_213_000 as Weight) + (111_373_000 as Weight) .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().writes(4 as Weight)) } - fn stop_repeat() -> Weight { - (111_483_000 as Weight) - .saturating_add(T::DbWeight::get().reads(6 as Weight)) - .saturating_add(T::DbWeight::get().writes(4 as Weight)) + fn start_lottery(n: u32, ) -> Weight { + (32_014_000 as Weight) + .saturating_add((509_000 as Weight).saturating_mul(n as Weight)) + .saturating_add(T::DbWeight::get().reads(2 as Weight)) + .saturating_add(T::DbWeight::get().writes(2 as Weight)) } - fn on_initialize_repeat() -> Weight { - (111_483_000 as Weight) - .saturating_add(T::DbWeight::get().reads(6 as Weight)) - .saturating_add(T::DbWeight::get().writes(4 as Weight)) + fn stop_repeat() -> Weight { + (10_620_000 as Weight) + .saturating_add(T::DbWeight::get().reads(1 as Weight)) + .saturating_add(T::DbWeight::get().writes(1 as Weight)) } fn on_initialize_end() -> Weight { - (111_483_000 as Weight) + (108_441_000 as Weight) .saturating_add(T::DbWeight::get().reads(6 as Weight)) .saturating_add(T::DbWeight::get().writes(4 as Weight)) } + fn on_initialize_repeat() -> Weight { + (115_804_000 as Weight) + .saturating_add(T::DbWeight::get().reads(7 as Weight)) + .saturating_add(T::DbWeight::get().writes(5 as Weight)) + } } // For backwards compatibility and tests impl WeightInfo for () { - fn start_lottery(n: u32, ) -> Weight { - (31_350_000 as Weight) - .saturating_add((496_000 as Weight).saturating_mul(n as Weight)) - .saturating_add(RocksDbWeight::get().reads(2 as Weight)) - .saturating_add(RocksDbWeight::get().writes(2 as Weight)) - } fn buy_ticket() -> Weight { - (116_213_000 as Weight) + (111_373_000 as Weight) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(4 as Weight)) } - fn stop_repeat() -> Weight { - (116_213_000 as Weight) - .saturating_add(RocksDbWeight::get().reads(5 as Weight)) - .saturating_add(RocksDbWeight::get().writes(4 as Weight)) + fn start_lottery(n: u32, ) -> Weight { + (32_014_000 as Weight) + .saturating_add((509_000 as Weight).saturating_mul(n as Weight)) + .saturating_add(RocksDbWeight::get().reads(2 as Weight)) + .saturating_add(RocksDbWeight::get().writes(2 as Weight)) } - fn on_initialize_repeat() -> Weight { - (111_483_000 as Weight) - .saturating_add(RocksDbWeight::get().reads(6 as Weight)) - .saturating_add(RocksDbWeight::get().writes(4 as Weight)) + fn stop_repeat() -> Weight { + (10_620_000 as Weight) + .saturating_add(RocksDbWeight::get().reads(1 as Weight)) + .saturating_add(RocksDbWeight::get().writes(1 as Weight)) } fn on_initialize_end() -> Weight { - (111_483_000 as Weight) + (108_441_000 as Weight) .saturating_add(RocksDbWeight::get().reads(6 as Weight)) .saturating_add(RocksDbWeight::get().writes(4 as Weight)) } + fn on_initialize_repeat() -> Weight { + (115_804_000 as Weight) + .saturating_add(RocksDbWeight::get().reads(7 as Weight)) + .saturating_add(RocksDbWeight::get().writes(5 as Weight)) + } } From d50bf8473c4dd41298f3da6a6b4928ece703d1c5 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Tue, 1 Dec 2020 13:07:42 -0800 Subject: [PATCH 23/29] Support static calls --- bin/node/runtime/src/lib.rs | 1 + frame/lottery/src/benchmarking.rs | 22 ++++- frame/lottery/src/lib.rs | 131 ++++++++++++++++++++---------- frame/lottery/src/mock.rs | 1 + frame/lottery/src/tests.rs | 63 ++++++++++---- frame/lottery/src/weights.rs | 17 +++- 6 files changed, 169 insertions(+), 66 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index cdd11c7a4877a..b2aa538c07292 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -905,6 +905,7 @@ impl pallet_lottery::Config for Runtime { type Randomness = RandomnessCollectiveFlip; type ManagerOrigin = EnsureRoot; type MaxCalls = MaxCalls; + type ValidateCall = Lottery; type WeightInfo = pallet_lottery::weights::SubstrateWeight; } diff --git a/frame/lottery/src/benchmarking.rs b/frame/lottery/src/benchmarking.rs index 7c3960f53d385..8550c5334abca 100644 --- a/frame/lottery/src/benchmarking.rs +++ b/frame/lottery/src/benchmarking.rs @@ -41,7 +41,8 @@ fn setup_lottery(repeat: bool) -> Result<(), &'static str> { // Last call will be the match for worst case scenario. calls.push(frame_system::Call::::remark(vec![]).into()); let origin = T::ManagerOrigin::successful_origin(); - Lottery::::start_lottery(origin, price, length, delay, calls, repeat)?; + Lottery::::set_calls(origin.clone(), calls)?; + Lottery::::start_lottery(origin, price, length, delay, repeat)?; Ok(()) } @@ -71,14 +72,26 @@ benchmarks! { assert_eq!(TicketsCount::get(), 1); } - start_lottery { + set_calls { let n in 0 .. T::MaxCalls::get() as u32; + let calls = vec![frame_system::Call::::remark(vec![]).into(); n as usize]; + + let call = Call::::set_calls(calls); + let origin = T::ManagerOrigin::successful_origin(); + assert!(CallIndices::get().is_empty()); + }: { call.dispatch_bypass_filter(origin)? } + verify { + if !n.is_zero() { + assert!(!CallIndices::get().is_empty()); + } + } + + start_lottery { let price = BalanceOf::::max_value(); let end = 10u32.into(); let payout = 5u32.into(); - let calls = vec![frame_system::Call::::remark(vec![]).into(); n as usize]; - let call = Call::::start_lottery(price, end, payout, calls, true); + let call = Call::::start_lottery(price, end, payout, true); let origin = T::ManagerOrigin::successful_origin(); }: { call.dispatch_bypass_filter(origin)? } verify { @@ -161,6 +174,7 @@ mod tests { fn test_benchmarks() { new_test_ext().execute_with(|| { assert_ok!(test_benchmark_buy_ticket::()); + assert_ok!(test_benchmark_set_calls::()); assert_ok!(test_benchmark_start_lottery::()); assert_ok!(test_benchmark_stop_repeat::()); assert_ok!(test_benchmark_on_initialize_end::()); diff --git a/frame/lottery/src/lib.rs b/frame/lottery/src/lib.rs index ce1d27d8fe19e..aee1dc1becf70 100644 --- a/frame/lottery/src/lib.rs +++ b/frame/lottery/src/lib.rs @@ -16,6 +16,26 @@ // limitations under the License. //! A lottery pallet that uses participation in the network to purchase tickets. +//! +//! With this pallet, you can configure a lottery, which is a pot of money that +//! users contribute to, and that is reallocated to a single user at the end of +//! the lottery period. Just like a normal lottery system, to participate, you +//! need to "buy a ticket", which is used to fund the pot. +//! +//! The unique feature of this lottery system is that tickets can only be +//! purchased by making a "valid call" dispatched through this pallet. +//! By configuring certain calls to be valid for the lottery, you can encourage +//! 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. +//! +//! This pallet can be configured to use dynamically set calls or statically set +//! calls. Call validation happens through the `ValidateCall` implementation. +//! This pallet provides one implementation of this using the `CallIndices` +//! storage item. You can also make your own implementation at the runtime level +//! which can contain much more complex logic, such as validation of the +//! parameters, which this pallet alone cannot do. #![cfg_attr(not(feature = "std"), no_std)] @@ -64,6 +84,9 @@ pub trait Config: frame_system::Config { /// The max number of calls available in a single lottery. type MaxCalls: Get; + /// Used to determine if a call would be valid for purchasing a ticket. + type ValidateCall: ValidateCall; + /// Weight information for extrinsics in this pallet. type WeightInfo: WeightInfo; } @@ -83,12 +106,29 @@ pub struct LotteryConfig { /// Delay for choosing the winner of the lottery. (start + length + delay = payout). /// Randomness in the "payout" block will be used to determine the winner. delay: BlockNumber, - /// Calls available this lottery. - calls: Vec, /// Whether this lottery will repeat after it completes. repeat: bool, } +pub trait ValidateCall { + fn validate_call(call: &::Call) -> bool; +} + +impl ValidateCall for () { + fn validate_call(_: &::Call) -> bool { false } +} + +impl ValidateCall for Module { + fn validate_call(call: &::Call) -> bool { + let valid_calls = CallIndices::get(); + let call_index = match Self::call_to_index(&call) { + Ok(call_index) => call_index, + Err(_) => return false, + }; + valid_calls.iter().any(|c| call_index == *c) + } +} + decl_storage! { trait Store for Module as Lottery { LotteryIndex: u32; @@ -103,6 +143,9 @@ decl_storage! { /// May have residual storage from previous lotteries. Use `TicketsCount` to see which ones /// are actually valid ticket mappings. Tickets: map hasher(twox_64_concat) u32 => Option; + /// The calls stored in this pallet to be used in an active lottery if configured + /// by `Config::ValidateCall`. + CallIndices: Vec; } } @@ -113,6 +156,8 @@ decl_event!( { /// A lottery has been started! LotteryStarted, + /// A new set of calls have been set! + CallsUpdated, /// A winner has been chosen! Winner(AccountId, Balance), /// A ticket has been bought! @@ -148,6 +193,41 @@ decl_module! { fn deposit_event() = default; + /// Buy a ticket to enter the lottery. + /// + /// This extrinsic acts as a passthrough function for `call`. In all + /// situations where `call` alone would succeed, this extrinsic should + /// succeed. + /// + /// If `call` is successful, then we will attempt to purchase a ticket, + /// which may fail silently. To detect success of a ticket purchase, you + /// should listen for the `TicketBought` event. + /// + /// This extrinsic must be called by a signed origin. + #[weight = + T::WeightInfo::buy_ticket() + .saturating_add(call.get_dispatch_info().weight) + ] + fn buy_ticket(origin, call: Box<::Call>) { + let caller = ensure_signed(origin.clone())?; + call.clone().dispatch(origin).map_err(|e| e.error)?; + + let _ = Self::do_buy_ticket(&caller, &call); + } + + #[weight = T::WeightInfo::set_calls(calls.len() as u32)] + fn set_calls(origin, calls: Vec<::Call>) { + T::ManagerOrigin::ensure_origin(origin)?; + ensure!(calls.len() <= T::MaxCalls::get(), Error::::TooManyCalls); + if calls.is_empty() { + CallIndices::kill(); + } else { + let indices = Self::calls_to_indices(&calls)?; + CallIndices::put(indices); + } + Self::deposit_event(RawEvent::CallsUpdated); + } + /// Start a lottery using the provided configuration. /// /// This extrinsic must be called by the `ManagerOrigin`. @@ -157,20 +237,15 @@ decl_module! { /// * `price`: The cost of a single ticket. /// * `length`: How long the lottery should run for starting at the current block. /// * `delay`: How long after the lottery end we should wait before picking a winner. - /// * `calls`: The calls allowed for purchasing a lottery ticket. **Important**: Only - /// the call index is used to determine if the ticket purchase is valid. /// * `repeat`: If the lottery should repeat when completed. - #[weight = T::WeightInfo::start_lottery(calls.len() as u32)] + #[weight = T::WeightInfo::start_lottery()] fn start_lottery(origin, price: BalanceOf, length: T::BlockNumber, delay: T::BlockNumber, - calls: Vec<::Call>, repeat: bool, ) { T::ManagerOrigin::ensure_origin(origin)?; - ensure!(calls.len() <= T::MaxCalls::get(), Error::::TooManyCalls); - let indices = Self::calls_to_indices(&calls)?; Lottery::::try_mutate(|lottery| -> DispatchResult { ensure!(lottery.is_none(), Error::::InProgress); let index = LotteryIndex::get(); @@ -182,7 +257,6 @@ decl_module! { start, length, delay, - calls: indices, repeat, }); LotteryIndex::put(new_index); @@ -191,34 +265,6 @@ decl_module! { Self::deposit_event(RawEvent::LotteryStarted); } - /// Buy a ticket to enter the lottery. - /// - /// This extrinsic acts as a passthrough function for `call`. In all - /// situations where `call` alone would succeed, this extrinsic should - /// succeed. - /// - /// If `call` is successful, then we will attempt to purchase a ticket, - /// which may fail silently. To detect success of a ticket purchase, you - /// should listen for the `TicketBought` event. - /// - /// This extrinsic must be called by a signed origin. - #[weight = - T::WeightInfo::buy_ticket() - .saturating_add(call.get_dispatch_info().weight) - ] - fn buy_ticket(origin, call: Box<::Call>) { - let caller = ensure_signed(origin.clone())?; - // This conversion may fail, but we want this function to always act - // as a passthrough... so we will parse the result below. - let maybe_call_index = Self::call_to_index(&call); - call.dispatch(origin).map_err(|e| e.error)?; - - // Only try to buy a ticket if the underlying call is successful. - // Not much we can do if this fails. - let call_index = maybe_call_index?; - let _ = Self::do_buy_ticket(&caller, &call_index); - } - /// If a lottery is repeating, you can use this to stop the repeat. /// The lottery will continue to run to completion. /// @@ -311,12 +357,13 @@ impl Module { } // Logic for buying a ticket. - fn do_buy_ticket(caller: &T::AccountId, call_index: &(u8, u8)) -> DispatchResult { + fn do_buy_ticket(caller: &T::AccountId, call: &::Call) -> DispatchResult { // Check the call is valid lottery let config = Lottery::::get().ok_or(Error::::NotConfigured)?; let block_number = frame_system::Module::::block_number(); ensure!(block_number < config.start.saturating_add(config.length), Error::::AlreadyEnded); - ensure!(config.calls.iter().any(|c| call_index == c), Error::::InvalidCall); + ensure!(T::ValidateCall::validate_call(call), Error::::InvalidCall); + let call_index = Self::call_to_index(call)?; let ticket_count = TicketsCount::get(); let new_ticket_count = ticket_count.checked_add(1).ok_or(Error::::Overflow)?; // Try to update the participant status @@ -328,18 +375,18 @@ impl Module { *lottery_index = index; } else { // Check that user is not already participating under this call. - ensure!(!participating_calls.iter().any(|c| call_index == c), Error::::AlreadyParticipating); + ensure!(!participating_calls.iter().any(|c| call_index == *c), Error::::AlreadyParticipating); } // Check user has enough funds and send it to the Lottery account. T::Currency::transfer(caller, &Self::account_id(), config.price, KeepAlive)?; // Create a new ticket. TicketsCount::put(new_ticket_count); Tickets::::insert(ticket_count, caller.clone()); - participating_calls.push(*call_index); + participating_calls.push(call_index); Ok(()) })?; - Self::deposit_event(RawEvent::TicketBought(caller.clone(), *call_index)); + Self::deposit_event(RawEvent::TicketBought(caller.clone(), call_index)); Ok(()) } diff --git a/frame/lottery/src/mock.rs b/frame/lottery/src/mock.rs index eafa8f5239558..8833fc25e7449 100644 --- a/frame/lottery/src/mock.rs +++ b/frame/lottery/src/mock.rs @@ -106,6 +106,7 @@ impl Config for Test { type Event = (); type ManagerOrigin = EnsureRoot; type MaxCalls = MaxCalls; + type ValidateCall = Lottery; type WeightInfo = (); } diff --git a/frame/lottery/src/tests.rs b/frame/lottery/src/tests.rs index 9a3660daaa3fd..4d807a0684184 100644 --- a/frame/lottery/src/tests.rs +++ b/frame/lottery/src/tests.rs @@ -48,8 +48,11 @@ fn basic_end_to_end_works() { Call::Balances(BalancesCall::transfer(0, 0)), ]; + // Set calls for the lottery + assert_ok!(Lottery::set_calls(Origin::root(), calls)); + // Start lottery, it repeats - assert_ok!(Lottery::start_lottery(Origin::root(), price, length, delay, calls.clone(), true)); + assert_ok!(Lottery::start_lottery(Origin::root(), price, length, delay, true)); assert!(crate::Lottery::::get().is_some()); assert_eq!(Balances::free_balance(&1), 100); @@ -88,7 +91,6 @@ fn basic_end_to_end_works() { start: 25, length, delay, - calls: vec![(1,2), (1,0)], repeat: true, } ); @@ -96,38 +98,56 @@ fn basic_end_to_end_works() { } #[test] -fn start_lottery_works() { +fn set_calls_works() { new_test_ext().execute_with(|| { - let price = 10; - let length = 20; - let delay = 5; + assert!(!CallIndices::exists()); + let calls = vec![ Call::Balances(BalancesCall::force_transfer(0, 0, 0)), Call::Balances(BalancesCall::transfer(0, 0)), ]; + + assert_ok!(Lottery::set_calls(Origin::root(), calls)); + assert!(CallIndices::exists()); + let too_many_calls = vec![ Call::Balances(BalancesCall::force_transfer(0, 0, 0)), Call::Balances(BalancesCall::transfer(0, 0)), Call::System(SystemCall::remark(vec![])), ]; - // Setup ignores bad origin assert_noop!( - Lottery::start_lottery(Origin::signed(1), price, length, delay, calls.clone(), false), - BadOrigin, + Lottery::set_calls(Origin::root(), too_many_calls), + Error::::TooManyCalls, ); - // Too many calls + + // Clear calls + assert_ok!(Lottery::set_calls(Origin::root(), vec![])); + assert!(CallIndices::get().is_empty()); + }); +} + +#[test] +fn start_lottery_works() { + new_test_ext().execute_with(|| { + let price = 10; + let length = 20; + let delay = 5; + + + // Setup ignores bad origin assert_noop!( - Lottery::start_lottery(Origin::root(), price, length, delay, too_many_calls, false), - Error::::TooManyCalls, + Lottery::start_lottery(Origin::signed(1), price, length, delay, false), + BadOrigin, ); + // All good - assert_ok!(Lottery::start_lottery(Origin::root(), price, length, delay, calls.clone(), false)); + assert_ok!(Lottery::start_lottery(Origin::root(), price, length, delay, false)); // Can't open another one if lottery is already present assert_noop!( - Lottery::start_lottery(Origin::root(), price, length, delay, calls, false), + Lottery::start_lottery(Origin::root(), price, length, delay, false), Error::::InProgress, ); }); @@ -150,8 +170,10 @@ fn buy_ticket_works_as_simple_passthrough() { Call::Balances(BalancesCall::force_transfer(0, 0, 0)), Call::Balances(BalancesCall::transfer(0, 0)), ]; + assert_ok!(Lottery::set_calls(Origin::root(), calls)); + // Ticket price of 60 would kill the user's account - assert_ok!(Lottery::start_lottery(Origin::root(), 60, 10, 5, calls.clone(), false)); + assert_ok!(Lottery::start_lottery(Origin::root(), 60, 10, 5, false)); assert_ok!(Lottery::buy_ticket(Origin::signed(1), call.clone())); assert_eq!(Balances::free_balance(&1), 100 - 20 - 20); assert_eq!(TicketsCount::get(), 0); @@ -171,18 +193,25 @@ fn buy_ticket_works_as_simple_passthrough() { // User can call other txs, but doesn't get a ticket let remark_call = Box::new(Call::System(SystemCall::remark(b"hello, world!".to_vec()))); - assert_ok!(Lottery::buy_ticket(Origin::signed(1), remark_call)); + assert_ok!(Lottery::buy_ticket(Origin::signed(2), remark_call)); assert_eq!(TicketsCount::get(), 0); + + let successful_call = Box::new(Call::Balances(BalancesCall::transfer(2, 1))); + assert_ok!(Lottery::buy_ticket(Origin::signed(2), successful_call)); + assert_eq!(TicketsCount::get(), 1); }); } #[test] fn buy_ticket_works() { new_test_ext().execute_with(|| { + // Set calls for the lottery. let calls = vec![ Call::System(SystemCall::remark(vec![])), Call::Balances(BalancesCall::transfer(0, 0)), ]; + assert_ok!(Lottery::set_calls(Origin::root(), calls)); + // Can't buy ticket before start let call = Box::new(Call::Balances(BalancesCall::transfer(2, 1))); @@ -190,7 +219,7 @@ fn buy_ticket_works() { assert_eq!(TicketsCount::get(), 0); // Start lottery - assert_ok!(Lottery::start_lottery(Origin::root(), 1, 20, 5, calls.clone(), false)); + assert_ok!(Lottery::start_lottery(Origin::root(), 1, 20, 5, false)); // Go to start, buy ticket for transfer run_to_block(5); diff --git a/frame/lottery/src/weights.rs b/frame/lottery/src/weights.rs index c8aec2120b3a8..29e712885093d 100644 --- a/frame/lottery/src/weights.rs +++ b/frame/lottery/src/weights.rs @@ -45,7 +45,8 @@ use sp_std::marker::PhantomData; /// Weight functions needed for pallet_lottery. pub trait WeightInfo { fn buy_ticket() -> Weight; - fn start_lottery(n: u32, ) -> Weight; + fn set_calls(n: u32) -> Weight; + fn start_lottery() -> Weight; fn stop_repeat() -> Weight; fn on_initialize_end() -> Weight; fn on_initialize_repeat() -> Weight; @@ -59,12 +60,17 @@ impl WeightInfo for SubstrateWeight { .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().writes(4 as Weight)) } - fn start_lottery(n: u32, ) -> Weight { + fn set_calls(n: u32, ) -> Weight { (32_014_000 as Weight) .saturating_add((509_000 as Weight).saturating_mul(n as Weight)) .saturating_add(T::DbWeight::get().reads(2 as Weight)) .saturating_add(T::DbWeight::get().writes(2 as Weight)) } + fn start_lottery() -> Weight { + (32_014_000 as Weight) + .saturating_add(T::DbWeight::get().reads(2 as Weight)) + .saturating_add(T::DbWeight::get().writes(2 as Weight)) + } fn stop_repeat() -> Weight { (10_620_000 as Weight) .saturating_add(T::DbWeight::get().reads(1 as Weight)) @@ -89,12 +95,17 @@ impl WeightInfo for () { .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(4 as Weight)) } - fn start_lottery(n: u32, ) -> Weight { + fn set_calls(n: u32, ) -> Weight { (32_014_000 as Weight) .saturating_add((509_000 as Weight).saturating_mul(n as Weight)) .saturating_add(RocksDbWeight::get().reads(2 as Weight)) .saturating_add(RocksDbWeight::get().writes(2 as Weight)) } + fn start_lottery() -> Weight { + (32_014_000 as Weight) + .saturating_add(RocksDbWeight::get().reads(2 as Weight)) + .saturating_add(RocksDbWeight::get().writes(2 as Weight)) + } fn stop_repeat() -> Weight { (10_620_000 as Weight) .saturating_add(RocksDbWeight::get().reads(1 as Weight)) From 5460912804535320c5dd3aeaf54f824addfb261b Mon Sep 17 00:00:00 2001 From: Parity Benchmarking Bot Date: Tue, 1 Dec 2020 21:12:05 +0000 Subject: [PATCH 24/29] cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_lottery --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/lottery/src/weights.rs --template=./.maintain/frame-weight-template.hbs --- frame/lottery/src/weights.rs | 40 +++++++++++++++++------------------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/frame/lottery/src/weights.rs b/frame/lottery/src/weights.rs index 29e712885093d..7c43342a0abbc 100644 --- a/frame/lottery/src/weights.rs +++ b/frame/lottery/src/weights.rs @@ -45,7 +45,7 @@ use sp_std::marker::PhantomData; /// Weight functions needed for pallet_lottery. pub trait WeightInfo { fn buy_ticket() -> Weight; - fn set_calls(n: u32) -> Weight; + fn set_calls(n: u32, ) -> Weight; fn start_lottery() -> Weight; fn stop_repeat() -> Weight; fn on_initialize_end() -> Weight; @@ -56,33 +56,32 @@ pub trait WeightInfo { pub struct SubstrateWeight(PhantomData); impl WeightInfo for SubstrateWeight { fn buy_ticket() -> Weight { - (111_373_000 as Weight) - .saturating_add(T::DbWeight::get().reads(5 as Weight)) + (116_300_000 as Weight) + .saturating_add(T::DbWeight::get().reads(6 as Weight)) .saturating_add(T::DbWeight::get().writes(4 as Weight)) } fn set_calls(n: u32, ) -> Weight { - (32_014_000 as Weight) - .saturating_add((509_000 as Weight).saturating_mul(n as Weight)) - .saturating_add(T::DbWeight::get().reads(2 as Weight)) - .saturating_add(T::DbWeight::get().writes(2 as Weight)) + (21_137_000 as Weight) + .saturating_add((470_000 as Weight).saturating_mul(n as Weight)) + .saturating_add(T::DbWeight::get().writes(1 as Weight)) } fn start_lottery() -> Weight { - (32_014_000 as Weight) + (31_851_000 as Weight) .saturating_add(T::DbWeight::get().reads(2 as Weight)) .saturating_add(T::DbWeight::get().writes(2 as Weight)) } fn stop_repeat() -> Weight { - (10_620_000 as Weight) + (9_633_000 as Weight) .saturating_add(T::DbWeight::get().reads(1 as Weight)) .saturating_add(T::DbWeight::get().writes(1 as Weight)) } fn on_initialize_end() -> Weight { - (108_441_000 as Weight) + (108_203_000 as Weight) .saturating_add(T::DbWeight::get().reads(6 as Weight)) .saturating_add(T::DbWeight::get().writes(4 as Weight)) } fn on_initialize_repeat() -> Weight { - (115_804_000 as Weight) + (114_658_000 as Weight) .saturating_add(T::DbWeight::get().reads(7 as Weight)) .saturating_add(T::DbWeight::get().writes(5 as Weight)) } @@ -91,33 +90,32 @@ impl WeightInfo for SubstrateWeight { // For backwards compatibility and tests impl WeightInfo for () { fn buy_ticket() -> Weight { - (111_373_000 as Weight) - .saturating_add(RocksDbWeight::get().reads(5 as Weight)) + (116_300_000 as Weight) + .saturating_add(RocksDbWeight::get().reads(6 as Weight)) .saturating_add(RocksDbWeight::get().writes(4 as Weight)) } fn set_calls(n: u32, ) -> Weight { - (32_014_000 as Weight) - .saturating_add((509_000 as Weight).saturating_mul(n as Weight)) - .saturating_add(RocksDbWeight::get().reads(2 as Weight)) - .saturating_add(RocksDbWeight::get().writes(2 as Weight)) + (21_137_000 as Weight) + .saturating_add((470_000 as Weight).saturating_mul(n as Weight)) + .saturating_add(RocksDbWeight::get().writes(1 as Weight)) } fn start_lottery() -> Weight { - (32_014_000 as Weight) + (31_851_000 as Weight) .saturating_add(RocksDbWeight::get().reads(2 as Weight)) .saturating_add(RocksDbWeight::get().writes(2 as Weight)) } fn stop_repeat() -> Weight { - (10_620_000 as Weight) + (9_633_000 as Weight) .saturating_add(RocksDbWeight::get().reads(1 as Weight)) .saturating_add(RocksDbWeight::get().writes(1 as Weight)) } fn on_initialize_end() -> Weight { - (108_441_000 as Weight) + (108_203_000 as Weight) .saturating_add(RocksDbWeight::get().reads(6 as Weight)) .saturating_add(RocksDbWeight::get().writes(4 as Weight)) } fn on_initialize_repeat() -> Weight { - (115_804_000 as Weight) + (114_658_000 as Weight) .saturating_add(RocksDbWeight::get().reads(7 as Weight)) .saturating_add(RocksDbWeight::get().writes(5 as Weight)) } From c343e03ea2ccbf608f03855161a389a38e03f161 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Fri, 1 Jan 2021 16:59:26 -0400 Subject: [PATCH 25/29] fix test --- frame/lottery/src/mock.rs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/frame/lottery/src/mock.rs b/frame/lottery/src/mock.rs index 8833fc25e7449..3deb98d9fcba4 100644 --- a/frame/lottery/src/mock.rs +++ b/frame/lottery/src/mock.rs @@ -53,30 +53,27 @@ parameter_types! { impl frame_system::Config for Test { type BaseCallFilter = (); + type BlockWeights = (); + type BlockLength = (); + type DbWeight = (); type Origin = Origin; type Index = u64; + type Call = Call; type BlockNumber = u64; type Hash = H256; - type Call = Call; type Hashing = BlakeTwo256; type AccountId = u64; type Lookup = IdentityLookup; type Header = Header; type Event = (); type BlockHashCount = BlockHashCount; - type MaximumBlockWeight = MaximumBlockWeight; - type DbWeight = (); - type BlockExecutionWeight = (); - type ExtrinsicBaseWeight = (); - type MaximumExtrinsicWeight = MaximumBlockWeight; - type MaximumBlockLength = MaximumBlockLength; - type AvailableBlockRatio = AvailableBlockRatio; type Version = (); type PalletInfo = (); + type AccountData = pallet_balances::AccountData; type OnNewAccount = (); type OnKilledAccount = (); - type AccountData = pallet_balances::AccountData; type SystemWeightInfo = (); + type SS58Prefix = (); } parameter_types! { From 8fc534c40b5bd7f508d29526639ffc0228758770 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Fri, 1 Jan 2021 17:52:28 -0400 Subject: [PATCH 26/29] add loop to mitigate modulo bias --- bin/node/runtime/src/lib.rs | 2 ++ frame/lottery/src/lib.rs | 35 +++++++++++++++++++++++++++++++++-- frame/lottery/src/mock.rs | 2 ++ 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index ba7ae63477801..18f1318735d00 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -942,6 +942,7 @@ impl pallet_mmr::Config for Runtime { parameter_types! { pub const LotteryModuleId: ModuleId = ModuleId(*b"py/lotto"); pub const MaxCalls: usize = 10; + pub const MaxGenerateRandom: u32 = 10; } impl pallet_lottery::Config for Runtime { @@ -953,6 +954,7 @@ impl pallet_lottery::Config for Runtime { type ManagerOrigin = EnsureRoot; type MaxCalls = MaxCalls; type ValidateCall = Lottery; + type MaxGenerateRandom = MaxGenerateRandom; type WeightInfo = pallet_lottery::weights::SubstrateWeight; } diff --git a/frame/lottery/src/lib.rs b/frame/lottery/src/lib.rs index aee1dc1becf70..0244077f578e7 100644 --- a/frame/lottery/src/lib.rs +++ b/frame/lottery/src/lib.rs @@ -36,6 +36,13 @@ //! storage item. You can also make your own implementation at the runtime level //! which can contain much more complex logic, such as validation of the //! parameters, which this pallet alone cannot do. +//! +//! This pallet uses the modulus operator to pick a random winner. It is known +//! that this might introduce a bias if the random number chosen in a range that +//! is not perfectly divisible by the total number of participants. The +//! `MaxGenerateRandom` configuration can help mitigate this by generating new +//! numbers until we hit the limit or we find a "fair" number. This is best +//! effort only. #![cfg_attr(not(feature = "std"), no_std)] @@ -87,6 +94,11 @@ pub trait Config: frame_system::Config { /// Used to determine if a call would be valid for purchasing a ticket. type ValidateCall: ValidateCall; + /// Number of time we should try to generate a random number that has no modulo bias. + /// The larger this number, the more potential computation is used for picking the winner, + /// but also the more likely that the chosen winner is done fairly. + type MaxGenerateRandom: Get; + /// Weight information for extrinsics in this pallet. type WeightInfo: WeightInfo; } @@ -393,9 +405,28 @@ impl Module { // Randomly choose a winner from among the total number of participants. fn choose_winner(total: u32) -> u32 { - let random_seed = T::Randomness::random(&T::ModuleId::get().encode()); + let mut random_number = Self::generate_random_number(0); + + // Best effort attempt to remove bias from modulus operator. + for i in 1 .. T::MaxGenerateRandom::get() { + if random_number < u32::MAX - u32::MAX % total { + break; + } + + random_number = Self::generate_random_number(i); + } + + 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`. + fn generate_random_number(seed: u32) -> u32 { + let random_seed = T::Randomness::random(&(T::ModuleId::get(), seed).encode()); let random_number = ::decode(&mut random_seed.as_ref()) .expect("secure hashes should always be bigger than u32; qed"); - random_number % total + random_number } } diff --git a/frame/lottery/src/mock.rs b/frame/lottery/src/mock.rs index 3deb98d9fcba4..1a03f5721fa67 100644 --- a/frame/lottery/src/mock.rs +++ b/frame/lottery/src/mock.rs @@ -93,6 +93,7 @@ impl pallet_balances::Config for Test { parameter_types! { pub const LotteryModuleId: ModuleId = ModuleId(*b"py/lotto"); pub const MaxCalls: usize = 2; + pub const MaxGenerateRandom: u32 = 10; } impl Config for Test { @@ -104,6 +105,7 @@ impl Config for Test { type ManagerOrigin = EnsureRoot; type MaxCalls = MaxCalls; type ValidateCall = Lottery; + type MaxGenerateRandom = MaxGenerateRandom; type WeightInfo = (); } From 34e9c433ba792e1be32448d87f9ce16d969ed5b3 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Fri, 1 Jan 2021 17:57:20 -0400 Subject: [PATCH 27/29] Update weights for worst case scenario loop --- frame/lottery/src/benchmarking.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/frame/lottery/src/benchmarking.rs b/frame/lottery/src/benchmarking.rs index 8550c5334abca..34a7f236c1812 100644 --- a/frame/lottery/src/benchmarking.rs +++ b/frame/lottery/src/benchmarking.rs @@ -125,6 +125,10 @@ benchmarks! { assert_eq!(TicketsCount::get(), 1); assert!(!Lottery::::pot().1.is_zero()); }: { + // Generate `MaxGenerateRandom` numbers for worst case scenario + for i in 0 .. T::MaxGenerateRandom::get() { + Lottery::::generate_random_number(i); + } // Start lottery has block 15 configured for payout Lottery::::on_initialize(15u32.into()); } @@ -152,6 +156,10 @@ benchmarks! { assert_eq!(TicketsCount::get(), 1); assert!(!Lottery::::pot().1.is_zero()); }: { + // Generate `MaxGenerateRandom` numbers for worst case scenario + for i in 0 .. T::MaxGenerateRandom::get() { + Lottery::::generate_random_number(i); + } // Start lottery has block 15 configured for payout Lottery::::on_initialize(15u32.into()); } From e3c6a16bd4a4baea319d87c94499f9a2b4ce6ba4 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Mon, 4 Jan 2021 21:07:58 -0400 Subject: [PATCH 28/29] Initialize pot with ED --- frame/lottery/src/lib.rs | 34 +++++++++++++++++++++++++++------- frame/lottery/src/mock.rs | 2 +- frame/lottery/src/tests.rs | 17 ++++++++++++++--- 3 files changed, 42 insertions(+), 11 deletions(-) diff --git a/frame/lottery/src/lib.rs b/frame/lottery/src/lib.rs index 0244077f578e7..b8568ad269f5c 100644 --- a/frame/lottery/src/lib.rs +++ b/frame/lottery/src/lib.rs @@ -28,7 +28,8 @@ //! 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. +//! validators every month. A user can ony purchase one ticket per valid call +//! per lottery. //! //! This pallet can be configured to use dynamically set calls or statically set //! calls. Call validation happens through the `ValidateCall` implementation. @@ -54,12 +55,16 @@ mod benchmarking; pub mod weights; use sp_std::prelude::*; -use sp_runtime::{DispatchError, ModuleId}; -use sp_runtime::traits::{AccountIdConversion, Saturating}; -use frame_support::{Parameter, decl_module, decl_error, decl_event, decl_storage, ensure, RuntimeDebug}; -use frame_support::dispatch::{Dispatchable, DispatchResult, GetDispatchInfo}; -use frame_support::traits::{ - Currency, ReservableCurrency, Get, EnsureOrigin, ExistenceRequirement::KeepAlive, Randomness, +use sp_runtime::{ + DispatchError, ModuleId, + traits::{AccountIdConversion, Saturating, Zero}, +}; +use frame_support::{ + Parameter, decl_module, decl_error, decl_event, decl_storage, ensure, RuntimeDebug, + dispatch::{Dispatchable, DispatchResult, GetDispatchInfo}, + traits::{ + Currency, ReservableCurrency, Get, EnsureOrigin, ExistenceRequirement::KeepAlive, Randomness, + }, }; use frame_support::weights::Weight; use frame_system::ensure_signed; @@ -92,6 +97,10 @@ pub trait Config: frame_system::Config { type MaxCalls: Get; /// Used to determine if a call would be valid for purchasing a ticket. + /// + /// Be conscious of the implementation used here. We assume at worst that + /// a vector of `MaxCalls` indices are queried for any call validation. + /// You may need to provide a custom benchmark if this assumption is broken. type ValidateCall: ValidateCall; /// Number of time we should try to generate a random number that has no modulo bias. @@ -227,6 +236,12 @@ decl_module! { let _ = Self::do_buy_ticket(&caller, &call); } + /// Set calls in storage which can be used to purchase a lottery ticket. + /// + /// This function only matters if you use the `ValidateCall` implementation + /// provided by this pallet, which uses storage to determine the valid calls. + /// + /// This extrinsic must be called by the Manager origin. #[weight = T::WeightInfo::set_calls(calls.len() as u32)] fn set_calls(origin, calls: Vec<::Call>) { T::ManagerOrigin::ensure_origin(origin)?; @@ -274,6 +289,11 @@ decl_module! { LotteryIndex::put(new_index); Ok(()) })?; + // Make sure pot exists. + let lottery_account = Self::account_id(); + if T::Currency::total_balance(&lottery_account).is_zero() { + T::Currency::deposit_creating(&lottery_account, T::Currency::minimum_balance()); + } Self::deposit_event(RawEvent::LotteryStarted); } diff --git a/frame/lottery/src/mock.rs b/frame/lottery/src/mock.rs index 1a03f5721fa67..67ecb6cbb63a2 100644 --- a/frame/lottery/src/mock.rs +++ b/frame/lottery/src/mock.rs @@ -119,7 +119,7 @@ pub type BalancesCall = pallet_balances::Call; pub fn new_test_ext() -> sp_io::TestExternalities { let mut t = frame_system::GenesisConfig::default().build_storage::().unwrap(); pallet_balances::GenesisConfig:: { - balances: vec![(1, 100), (2, 100), (3, 100), (4, 100), (5, 100), (Lottery::account_id(), 1)], + balances: vec![(1, 100), (2, 100), (3, 100), (4, 100), (5, 100)], }.assimilate_storage(&mut t).unwrap(); t.into() } diff --git a/frame/lottery/src/tests.rs b/frame/lottery/src/tests.rs index 4d807a0684184..69a8a1267dd48 100644 --- a/frame/lottery/src/tests.rs +++ b/frame/lottery/src/tests.rs @@ -29,7 +29,7 @@ use pallet_balances::Error as BalancesError; #[test] fn initial_state() { new_test_ext().execute_with(|| { - assert_eq!(Balances::free_balance(Lottery::account_id()), 1); + assert_eq!(Balances::free_balance(Lottery::account_id()), 0); assert!(crate::Lottery::::get().is_none()); assert_eq!(Participants::::get(&1), (0, vec![])); assert_eq!(TicketsCount::get(), 0); @@ -134,14 +134,12 @@ fn start_lottery_works() { let length = 20; let delay = 5; - // Setup ignores bad origin assert_noop!( Lottery::start_lottery(Origin::signed(1), price, length, delay, false), BadOrigin, ); - // All good assert_ok!(Lottery::start_lottery(Origin::root(), price, length, delay, false)); @@ -248,3 +246,16 @@ fn buy_ticket_works() { assert_eq!(LotteryIndex::get(), 1); }); } + +#[test] +fn start_lottery_will_create_account() { + new_test_ext().execute_with(|| { + let price = 10; + let length = 20; + let delay = 5; + + assert_eq!(Balances::total_balance(&Lottery::account_id()), 0); + assert_ok!(Lottery::start_lottery(Origin::root(), price, length, delay, false)); + assert_eq!(Balances::total_balance(&Lottery::account_id()), 1); + }); +} From 3de5a8ba47ce72c0c83f9426240f74f7fe60b610 Mon Sep 17 00:00:00 2001 From: Parity Benchmarking Bot Date: Tue, 5 Jan 2021 01:13:30 +0000 Subject: [PATCH 29/29] cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_lottery --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/lottery/src/weights.rs --template=./.maintain/frame-weight-template.hbs --- frame/lottery/src/weights.rs | 42 +++++++++++++++++++----------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/frame/lottery/src/weights.rs b/frame/lottery/src/weights.rs index 7c43342a0abbc..28d5ac0945b1d 100644 --- a/frame/lottery/src/weights.rs +++ b/frame/lottery/src/weights.rs @@ -1,6 +1,6 @@ // This file is part of Substrate. -// Copyright (C) 2020 Parity Technologies (UK) Ltd. +// Copyright (C) 2021 Parity Technologies (UK) Ltd. // SPDX-License-Identifier: Apache-2.0 // Licensed under the Apache License, Version 2.0 (the "License"); @@ -18,7 +18,7 @@ //! Autogenerated weights for pallet_lottery //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 2.0.0 -//! DATE: 2020-12-01, STEPS: [50, ], REPEAT: 20, LOW RANGE: [], HIGH RANGE: [] +//! DATE: 2021-01-05, STEPS: [50, ], REPEAT: 20, LOW RANGE: [], HIGH RANGE: [] //! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 128 // Executed Command: @@ -56,32 +56,33 @@ pub trait WeightInfo { pub struct SubstrateWeight(PhantomData); impl WeightInfo for SubstrateWeight { fn buy_ticket() -> Weight { - (116_300_000 as Weight) + (97_799_000 as Weight) .saturating_add(T::DbWeight::get().reads(6 as Weight)) .saturating_add(T::DbWeight::get().writes(4 as Weight)) } fn set_calls(n: u32, ) -> Weight { - (21_137_000 as Weight) - .saturating_add((470_000 as Weight).saturating_mul(n as Weight)) + (20_932_000 as Weight) + // Standard Error: 9_000 + .saturating_add((513_000 as Weight).saturating_mul(n as Weight)) .saturating_add(T::DbWeight::get().writes(1 as Weight)) } fn start_lottery() -> Weight { - (31_851_000 as Weight) - .saturating_add(T::DbWeight::get().reads(2 as Weight)) - .saturating_add(T::DbWeight::get().writes(2 as Weight)) + (77_600_000 as Weight) + .saturating_add(T::DbWeight::get().reads(3 as Weight)) + .saturating_add(T::DbWeight::get().writes(3 as Weight)) } fn stop_repeat() -> Weight { - (9_633_000 as Weight) + (10_707_000 as Weight) .saturating_add(T::DbWeight::get().reads(1 as Weight)) .saturating_add(T::DbWeight::get().writes(1 as Weight)) } fn on_initialize_end() -> Weight { - (108_203_000 as Weight) + (162_126_000 as Weight) .saturating_add(T::DbWeight::get().reads(6 as Weight)) .saturating_add(T::DbWeight::get().writes(4 as Weight)) } fn on_initialize_repeat() -> Weight { - (114_658_000 as Weight) + (169_310_000 as Weight) .saturating_add(T::DbWeight::get().reads(7 as Weight)) .saturating_add(T::DbWeight::get().writes(5 as Weight)) } @@ -90,32 +91,33 @@ impl WeightInfo for SubstrateWeight { // For backwards compatibility and tests impl WeightInfo for () { fn buy_ticket() -> Weight { - (116_300_000 as Weight) + (97_799_000 as Weight) .saturating_add(RocksDbWeight::get().reads(6 as Weight)) .saturating_add(RocksDbWeight::get().writes(4 as Weight)) } fn set_calls(n: u32, ) -> Weight { - (21_137_000 as Weight) - .saturating_add((470_000 as Weight).saturating_mul(n as Weight)) + (20_932_000 as Weight) + // Standard Error: 9_000 + .saturating_add((513_000 as Weight).saturating_mul(n as Weight)) .saturating_add(RocksDbWeight::get().writes(1 as Weight)) } fn start_lottery() -> Weight { - (31_851_000 as Weight) - .saturating_add(RocksDbWeight::get().reads(2 as Weight)) - .saturating_add(RocksDbWeight::get().writes(2 as Weight)) + (77_600_000 as Weight) + .saturating_add(RocksDbWeight::get().reads(3 as Weight)) + .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } fn stop_repeat() -> Weight { - (9_633_000 as Weight) + (10_707_000 as Weight) .saturating_add(RocksDbWeight::get().reads(1 as Weight)) .saturating_add(RocksDbWeight::get().writes(1 as Weight)) } fn on_initialize_end() -> Weight { - (108_203_000 as Weight) + (162_126_000 as Weight) .saturating_add(RocksDbWeight::get().reads(6 as Weight)) .saturating_add(RocksDbWeight::get().writes(4 as Weight)) } fn on_initialize_repeat() -> Weight { - (114_658_000 as Weight) + (169_310_000 as Weight) .saturating_add(RocksDbWeight::get().reads(7 as Weight)) .saturating_add(RocksDbWeight::get().writes(5 as Weight)) }