From a4a7f20ac7cc3b994dea2f26498371f153e97839 Mon Sep 17 00:00:00 2001 From: Shaun Wang Date: Sun, 16 May 2021 08:28:00 +0000 Subject: [PATCH 1/2] Migrate auctions pallet to pallet attribute macro. --- runtime/common/src/auctions.rs | 272 +++++++++++++----------- runtime/common/src/integration_tests.rs | 2 +- 2 files changed, 149 insertions(+), 125 deletions(-) diff --git a/runtime/common/src/auctions.rs b/runtime/common/src/auctions.rs index 62c9ec75f612..ab1c183ad582 100644 --- a/runtime/common/src/auctions.rs +++ b/runtime/common/src/auctions.rs @@ -21,15 +21,15 @@ use sp_std::{prelude::*, mem::swap}; use sp_runtime::traits::{CheckedSub, Zero, One, Saturating}; use frame_support::{ - decl_module, decl_storage, decl_event, decl_error, ensure, dispatch::DispatchResult, - traits::{Randomness, Currency, ReservableCurrency, Get, EnsureOrigin}, - weights::{DispatchClass, Weight}, + ensure, dispatch::DispatchResult, + traits::{Randomness, Currency, ReservableCurrency, Get}, + weights::{Weight}, }; use primitives::v1::Id as ParaId; -use frame_system::{ensure_signed, ensure_root}; use crate::slot_range::SlotRange; use crate::traits::{Leaser, LeaseError, Auctioneer, Registrar}; use parity_scale_codec::Decode; +pub use pallet::*; type CurrencyOf = <::Leaser as Leaser>::Currency; type BalanceOf = @@ -50,35 +50,6 @@ impl WeightInfo for TestWeightInfo { fn on_initialize() -> Weight { 0 } } -/// The module's configuration trait. -pub trait Config: frame_system::Config { - /// The overarching event type. - type Event: From> + Into<::Event>; - - /// The type representing the leasing system. - type Leaser: Leaser; - - /// The parachain registrar type. - type Registrar: Registrar; - - /// The number of blocks over which an auction may be retroactively ended. - type EndingPeriod: Get; - - /// The length of each sample to take during the ending period. - /// - /// EndingPeriod / SampleLength = Total # of Samples - type SampleLength: Get; - - /// Something that provides randomness in the runtime. - type Randomness: Randomness; - - /// The origin which may initiate auctions. - type InitiateOrigin: EnsureOrigin; - - /// Weight Information for the Extrinsics in the Pallet - type WeightInfo: WeightInfo; -} - /// An auction index. We count auctions in this type. pub type AuctionIndex = u32; @@ -90,72 +61,89 @@ type WinningData = // index assigned to them, their winning bid and the range that they won. type WinnersData = Vec<(::AccountId, ParaId, BalanceOf, SlotRange)>; -// This module's storage items. -decl_storage! { - trait Store for Module as Auctions { - /// Number of auctions started so far. - pub AuctionCounter get(fn auction_counter): AuctionIndex; +#[frame_support::pallet] +pub mod pallet { + use frame_support::{pallet_prelude::*, traits::EnsureOrigin, weights::DispatchClass}; + use frame_system::{pallet_prelude::*, ensure_signed, ensure_root}; + use super::*; + + #[pallet::pallet] + #[pallet::generate_store(pub(super) trait Store)] + pub struct Pallet(_); + + /// The module's configuration trait. + #[pallet::config] + pub trait Config: frame_system::Config { + /// The overarching event type. + type Event: From> + IsType<::Event>; - /// Information relating to the current auction, if there is one. + /// The type representing the leasing system. + type Leaser: Leaser; + + /// The parachain registrar type. + type Registrar: Registrar; + + /// The number of blocks over which an auction may be retroactively ended. + #[pallet::constant] + type EndingPeriod: Get; + + /// The length of each sample to take during the ending period. /// - /// The first item in the tuple is the lease period index that the first of the four - /// contiguous lease periods on auction is for. The second is the block number when the - /// auction will "begin to end", i.e. the first block of the Ending Period of the auction. - pub AuctionInfo get(fn auction_info): Option<(LeasePeriodOf, T::BlockNumber)>; - - /// Amounts currently reserved in the accounts of the bidders currently winning - /// (sub-)ranges. - pub ReservedAmounts get(fn reserved_amounts): - map hasher(twox_64_concat) (T::AccountId, ParaId) => Option>; - - /// The winning bids for each of the 10 ranges at each sample in the final Ending Period of - /// the current auction. The map's key is the 0-based index into the Sample Size. The - /// first sample of the ending period is 0; the last is `Sample Size - 1`. - pub Winning get(fn winning): map hasher(twox_64_concat) T::BlockNumber => Option>; + /// EndingPeriod / SampleLength = Total # of Samples + #[pallet::constant] + type SampleLength: Get; + + /// Something that provides randomness in the runtime. + type Randomness: Randomness; + + /// The origin which may initiate auctions. + type InitiateOrigin: EnsureOrigin; + + /// Weight Information for the Extrinsics in the Pallet + type WeightInfo: WeightInfo; } -} -decl_event!( - pub enum Event where - AccountId = ::AccountId, - BlockNumber = ::BlockNumber, - LeasePeriod = LeasePeriodOf, - ParaId = ParaId, - Balance = BalanceOf, - { + #[pallet::event] + #[pallet::generate_deposit(pub(super) fn deposit_event)] + #[pallet::metadata( + T::AccountId = "AccountId", + T::BlockNumber = "BlockNumber", + LeasePeriodOf = "LeasePeriod", + BalanceOf = "Balance", + )] + pub enum Event { /// An auction started. Provides its index and the block number where it will begin to /// close and the first lease period of the quadruplet that is auctioned. /// [auction_index, lease_period, ending] - AuctionStarted(AuctionIndex, LeasePeriod, BlockNumber), + AuctionStarted(AuctionIndex, LeasePeriodOf, T::BlockNumber), /// An auction ended. All funds become unreserved. [auction_index] AuctionClosed(AuctionIndex), /// Someone won the right to deploy a parachain. Balance amount is deducted for deposit. /// [bidder, range, parachain_id, amount] - WonDeploy(AccountId, SlotRange, ParaId, Balance), + WonDeploy(T::AccountId, SlotRange, ParaId, BalanceOf), /// An existing parachain won the right to continue. /// First balance is the extra amount reseved. Second is the total amount reserved. /// [parachain_id, begin, count, total_amount] - WonRenewal(ParaId, LeasePeriod, LeasePeriod, Balance), + WonRenewal(ParaId, LeasePeriodOf, LeasePeriodOf, BalanceOf), /// Funds were reserved for a winning bid. First balance is the extra amount reserved. /// Second is the total. [bidder, extra_reserved, total_amount] - Reserved(AccountId, Balance, Balance), + Reserved(T::AccountId, BalanceOf, BalanceOf), /// Funds were unreserved since bidder is no longer active. [bidder, amount] - Unreserved(AccountId, Balance), + Unreserved(T::AccountId, BalanceOf), /// Someone attempted to lease the same slot twice for a parachain. The amount is held in reserve /// but no parachain slot has been leased. /// \[parachain_id, leaser, amount\] - ReserveConfiscated(ParaId, AccountId, Balance), + ReserveConfiscated(ParaId, T::AccountId, BalanceOf), /// A new bid has been accepted as the current winner. /// \[who, para_id, amount, first_slot, last_slot\] - BidAccepted(AccountId, ParaId, Balance, LeasePeriod, LeasePeriod), + BidAccepted(T::AccountId, ParaId, BalanceOf, LeasePeriodOf, LeasePeriodOf), /// The winning offset was chosen for an auction. This will map into the `Winning` storage map. /// \[auction_index, block_number\] - WinningOffset(AuctionIndex, BlockNumber), + WinningOffset(AuctionIndex, T::BlockNumber), } -); -decl_error! { - pub enum Error for Module { + #[pallet::error] + pub enum Error { /// This auction is already in progress. AuctionInProgress, /// The lease period is in the past. @@ -185,19 +173,47 @@ decl_error! { /// Auction has already ended. AuctionEnded, } -} -decl_module! { - pub struct Module for enum Call where origin: T::Origin { - type Error = Error; + /// Number of auctions started so far. + #[pallet::storage] + #[pallet::getter(fn auction_counter)] + pub type AuctionCounter = StorageValue<_, AuctionIndex, ValueQuery>; - const EndingPeriod: T::BlockNumber = T::EndingPeriod::get(); - const SampleLength: T::BlockNumber = T::SampleLength::get(); - const SlotRangeCount: u32 = SlotRange::SLOT_RANGE_COUNT as u32; - const LeasePeriodsPerSlot: u32 = SlotRange::LEASE_PERIODS_PER_SLOT as u32; + /// Information relating to the current auction, if there is one. + /// + /// The first item in the tuple is the lease period index that the first of the four + /// contiguous lease periods on auction is for. The second is the block number when the + /// auction will "begin to end", i.e. the first block of the Ending Period of the auction. + #[pallet::storage] + #[pallet::getter(fn auction_info)] + pub type AuctionInfo = StorageValue<_, (LeasePeriodOf, T::BlockNumber)>; + + /// Amounts currently reserved in the accounts of the bidders currently winning + /// (sub-)ranges. + #[pallet::storage] + #[pallet::getter(fn reserved_amounts)] + pub type ReservedAmounts = StorageMap<_, Twox64Concat, (T::AccountId, ParaId), BalanceOf>; + + /// The winning bids for each of the 10 ranges at each sample in the final Ending Period of + /// the current auction. The map's key is the 0-based index into the Sample Size. The + /// first sample of the ending period is 0; the last is `Sample Size - 1`. + #[pallet::storage] + #[pallet::getter(fn winning)] + pub type Winning = StorageMap<_, Twox64Concat, T::BlockNumber, WinningData>; + + #[pallet::extra_constants] + impl Pallet { + fn slot_range_count() -> u32 { + SlotRange::SLOT_RANGE_COUNT as u32 + } - fn deposit_event() = default; + fn lease_periods_per_slot() -> u32 { + SlotRange::LEASE_PERIODS_PER_SLOT as u32 + } + } + #[pallet::hooks] + impl Hooks> for Pallet { fn on_initialize(n: T::BlockNumber) -> Weight { let mut weight = T::DbWeight::get().reads(1); @@ -228,30 +244,35 @@ decl_module! { weight } + } + #[pallet::call] + impl Pallet { /// Create a new auction. /// /// This can only happen when there isn't already an auction in progress and may only be /// called by the root origin. Accepts the `duration` of this auction and the /// `lease_period_index` of the initial lease period of the four that are to be auctioned. - #[weight = (T::WeightInfo::new_auction(), DispatchClass::Operational)] - pub fn new_auction(origin, - #[compact] duration: T::BlockNumber, - #[compact] lease_period_index: LeasePeriodOf, - ) { + #[pallet::weight((T::WeightInfo::new_auction(), DispatchClass::Operational))] + pub fn new_auction( + origin: OriginFor, + #[pallet::compact] duration: T::BlockNumber, + #[pallet::compact] lease_period_index: LeasePeriodOf, + ) -> DispatchResult { T::InitiateOrigin::ensure_origin(origin)?; ensure!(!Self::is_in_progress(), Error::::AuctionInProgress); ensure!(lease_period_index >= T::Leaser::lease_period_index(), Error::::LeasePeriodInPast); // Bump the counter. - let n = AuctionCounter::mutate(|n| { *n += 1; *n }); + let n = AuctionCounter::::mutate(|n| { *n += 1; *n }); // Set the information. let ending = frame_system::Pallet::::block_number().saturating_add(duration); AuctionInfo::::put((lease_period_index, ending)); - Self::deposit_event(RawEvent::AuctionStarted(n, lease_period_index, ending)) + Self::deposit_event(Event::::AuctionStarted(n, lease_period_index, ending)); + Ok(()) } /// Make a new bid from an account (including a parachain account) for deploying a new @@ -270,23 +291,25 @@ decl_module! { /// absolute lease period index value, not an auction-specific offset. /// - `amount` is the amount to bid to be held as deposit for the parachain should the /// bid win. This amount is held throughout the range. - #[weight = T::WeightInfo::bid()] - pub fn bid(origin, - #[compact] para: ParaId, - #[compact] auction_index: AuctionIndex, - #[compact] first_slot: LeasePeriodOf, - #[compact] last_slot: LeasePeriodOf, - #[compact] amount: BalanceOf - ) { + #[pallet::weight(T::WeightInfo::bid())] + pub fn bid( + origin: OriginFor, + #[pallet::compact] para: ParaId, + #[pallet::compact] auction_index: AuctionIndex, + #[pallet::compact] first_slot: LeasePeriodOf, + #[pallet::compact] last_slot: LeasePeriodOf, + #[pallet::compact] amount: BalanceOf + ) -> DispatchResult { let who = ensure_signed(origin)?; Self::handle_bid(who, para, auction_index, first_slot, last_slot, amount)?; + Ok(()) } /// Cancel an in-progress auction. /// /// Can only be called by Root origin. - #[weight = T::WeightInfo::cancel_auction()] - pub fn cancel_auction(origin) { + #[pallet::weight(T::WeightInfo::cancel_auction())] + pub fn cancel_auction(origin: OriginFor) -> DispatchResult { ensure_root(origin)?; // Unreserve all bids. for ((bidder, _), amount) in ReservedAmounts::::drain() { @@ -294,11 +317,12 @@ decl_module! { } Winning::::remove_all(); AuctionInfo::::kill(); + Ok(()) } } } -impl Auctioneer for Module { +impl Auctioneer for Pallet { type AccountId = T::AccountId; type BlockNumber = T::BlockNumber; type LeasePeriod = T::BlockNumber; @@ -330,7 +354,7 @@ impl Auctioneer for Module { last_slot: LeasePeriodOf, amount: BalanceOf, ) -> DispatchResult { - Self::handle_bid(bidder, para, AuctionCounter::get(), first_slot, last_slot, amount) + Self::handle_bid(bidder, para, AuctionCounter::::get(), first_slot, last_slot, amount) } fn lease_period_index() -> Self::LeasePeriod { @@ -346,7 +370,7 @@ impl Auctioneer for Module { } } -impl Module { +impl Pallet { // A trick to allow me to initialize large arrays with nothing in them. const EMPTY: Option<(::AccountId, ParaId, BalanceOf)> = None; @@ -374,13 +398,13 @@ impl Module { ensure!(lease_period_index >= T::Leaser::lease_period_index(), Error::::LeasePeriodInPast); // Bump the counter. - let n = AuctionCounter::mutate(|n| { *n += 1; *n }); + let n = AuctionCounter::::mutate(|n| { *n += 1; *n }); // Set the information. let ending = frame_system::Pallet::::block_number().saturating_add(duration); AuctionInfo::::put((lease_period_index, ending)); - Self::deposit_event(RawEvent::AuctionStarted(n, lease_period_index, ending)); + Self::deposit_event(Event::::AuctionStarted(n, lease_period_index, ending)); Ok(()) } @@ -403,7 +427,7 @@ impl Module { // Ensure para is registered before placing a bid on it. ensure!(T::Registrar::is_registered(para), Error::::ParaNotRegistered); // Bidding on latest auction. - ensure!(auction_index == AuctionCounter::get(), Error::::NotCurrentAuction); + ensure!(auction_index == AuctionCounter::::get(), Error::::NotCurrentAuction); // Assume it's actually an auction (this should never fail because of above). let (first_lease_period, early_end) = AuctionInfo::::get().ok_or(Error::::NotAuction)?; let late_end = early_end.saturating_add(T::EndingPeriod::get()); @@ -445,7 +469,7 @@ impl Module { // ...and record the amount reserved. ReservedAmounts::::insert(&bidder_para, reserve_required); - Self::deposit_event(RawEvent::Reserved( + Self::deposit_event(Event::::Reserved( bidder.clone(), additional, reserve_required, @@ -467,14 +491,14 @@ impl Module { let err_amt = CurrencyOf::::unreserve(&who, amount); debug_assert!(err_amt.is_zero()); - Self::deposit_event(RawEvent::Unreserved(who, amount)); + Self::deposit_event(Event::::Unreserved(who, amount)); } } } // Update the range winner. Winning::::insert(offset, ¤t_winning); - Self::deposit_event(RawEvent::BidAccepted(bidder, para, amount, first_slot, last_slot)); + Self::deposit_event(Event::::BidAccepted(bidder, para, amount, first_slot, last_slot)); } Ok(()) } @@ -501,8 +525,8 @@ impl Module { .expect("secure hashes should always be bigger than the block number; qed"); let offset = (raw_offset_block_number % ending_period) / T::SampleLength::get(); - let auction_counter = AuctionCounter::get(); - Self::deposit_event(RawEvent::WinningOffset(auction_counter, offset)); + let auction_counter = AuctionCounter::::get(); + Self::deposit_event(Event::::WinningOffset(auction_counter, offset)); let res = Winning::::get(offset).unwrap_or([Self::EMPTY; SlotRange::SLOT_RANGE_COUNT]); // This `remove_all` statement should remove at most `EndingPeriod` / `SampleLength` items, // which should be bounded and sensibly configured in the runtime. @@ -549,14 +573,14 @@ impl Module { // The leaser attempted to get a second lease on the same para ID, possibly griefing us. Let's // keep the amount reserved and let governance sort it out. if CurrencyOf::::reserve(&leaser, amount).is_ok() { - Self::deposit_event(RawEvent::ReserveConfiscated(para, leaser, amount)); + Self::deposit_event(Event::::ReserveConfiscated(para, leaser, amount)); } } Ok(()) => {}, // Nothing to report. } } - Self::deposit_event(RawEvent::AuctionClosed(AuctionCounter::get())); + Self::deposit_event(Event::::AuctionClosed(AuctionCounter::::get())); } /// Calculate the final winners from the winning slots. @@ -825,14 +849,14 @@ mod tests { #[test] fn basic_setup_works() { new_test_ext().execute_with(|| { - assert_eq!(AuctionCounter::get(), 0); + assert_eq!(AuctionCounter::::get(), 0); assert_eq!(TestLeaser::deposit_held(0u32.into(), &1), 0); assert_eq!(Auctions::is_in_progress(), false); assert_eq!(Auctions::is_ending(System::block_number()), None); run_to_block(10); - assert_eq!(AuctionCounter::get(), 0); + assert_eq!(AuctionCounter::::get(), 0); assert_eq!(TestLeaser::deposit_held(0u32.into(), &1), 0); assert_eq!(Auctions::is_in_progress(), false); assert_eq!(Auctions::is_ending(System::block_number()), None); @@ -847,7 +871,7 @@ mod tests { assert_noop!(Auctions::new_auction(Origin::signed(1), 5, 1), BadOrigin); assert_ok!(Auctions::new_auction(Origin::signed(6), 5, 1)); - assert_eq!(AuctionCounter::get(), 1); + assert_eq!(AuctionCounter::::get(), 1); assert_eq!(Auctions::is_in_progress(), true); assert_eq!(Auctions::is_ending(System::block_number()), None); }); @@ -909,7 +933,7 @@ mod tests { assert_ok!(Auctions::new_auction(Origin::signed(6), 5, 1)); - assert_eq!(AuctionCounter::get(), 1); + assert_eq!(AuctionCounter::::get(), 1); assert_eq!(Auctions::is_in_progress(), true); assert_eq!(Auctions::is_ending(System::block_number()), None); @@ -1400,9 +1424,9 @@ mod tests { #[cfg(feature = "runtime-benchmarks")] mod benchmarking { - use super::{*, Module as Auctions}; + use super::{*, Pallet as Auctions}; use frame_system::RawOrigin; - use frame_support::traits::OnInitialize; + use frame_support::traits::{EnsureOrigin, OnInitialize}; use sp_runtime::{traits::Bounded, SaturatedConversion}; use frame_benchmarking::{benchmarks, whitelisted_caller, account, impl_benchmark_test_suite}; @@ -1416,7 +1440,7 @@ mod benchmarking { } fn fill_winners(lease_period_index: LeasePeriodOf) { - let auction_index = AuctionCounter::get(); + let auction_index = AuctionCounter::::get(); let minimum_balance = CurrencyOf::::minimum_balance(); for n in 1 ..= SlotRange::SLOT_RANGE_COUNT as u32 { @@ -1462,8 +1486,8 @@ mod benchmarking { let origin = T::InitiateOrigin::successful_origin(); }: _(RawOrigin::Root, duration, lease_period_index) verify { - assert_last_event::(RawEvent::AuctionStarted( - AuctionCounter::get(), + assert_last_event::(Event::::AuctionStarted( + AuctionCounter::::get(), LeasePeriodOf::::max_value(), T::BlockNumber::max_value(), ).into()); @@ -1489,7 +1513,7 @@ mod benchmarking { T::Registrar::execute_pending_transitions(); // Make an existing bid - let auction_index = AuctionCounter::get(); + let auction_index = AuctionCounter::::get(); let first_slot = AuctionInfo::::get().unwrap().0; let last_slot = first_slot + 3u32.into(); let first_amount = CurrencyOf::::minimum_balance(); @@ -1549,8 +1573,8 @@ mod benchmarking { }: { Auctions::::on_initialize(duration + now + T::EndingPeriod::get()); } verify { - let auction_index = AuctionCounter::get(); - assert_last_event::(RawEvent::AuctionClosed(auction_index).into()); + let auction_index = AuctionCounter::::get(); + assert_last_event::(Event::::AuctionClosed(auction_index).into()); assert!(Winning::::iter().count().is_zero()); } diff --git a/runtime/common/src/integration_tests.rs b/runtime/common/src/integration_tests.rs index 6a6002a164e2..a42c75bd1bb1 100644 --- a/runtime/common/src/integration_tests.rs +++ b/runtime/common/src/integration_tests.rs @@ -371,7 +371,7 @@ fn basic_end_to_end_works() { run_to_block(110); assert_eq!( last_event(), - auctions::RawEvent::AuctionClosed(1).into(), + auctions::Event::::AuctionClosed(1).into(), ); // Paras should have won slots From 3a28f83ce2d1768f8481d9ac374254611c8cf122 Mon Sep 17 00:00:00 2001 From: Shaun Wang Date: Sun, 16 May 2021 08:46:13 +0000 Subject: [PATCH 2/2] Rename extra constants. --- runtime/common/src/auctions.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/runtime/common/src/auctions.rs b/runtime/common/src/auctions.rs index ab1c183ad582..f9854519a1e8 100644 --- a/runtime/common/src/auctions.rs +++ b/runtime/common/src/auctions.rs @@ -203,11 +203,15 @@ pub mod pallet { #[pallet::extra_constants] impl Pallet { - fn slot_range_count() -> u32 { + //TODO: rename to snake case after https://github.com/paritytech/substrate/issues/8826 fixed. + #[allow(non_snake_case)] + fn SlotRangeCount() -> u32 { SlotRange::SLOT_RANGE_COUNT as u32 } - fn lease_periods_per_slot() -> u32 { + //TODO: rename to snake case after https://github.com/paritytech/substrate/issues/8826 fixed. + #[allow(non_snake_case)] + fn LeasePeriodsPerSlot() -> u32 { SlotRange::LEASE_PERIODS_PER_SLOT as u32 } }