From a4ed8aaab250a0351860b5d84826019447995b32 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Fri, 26 Mar 2021 00:01:36 +0100 Subject: [PATCH] Check that Para is Registered before Accepting a Bid (#2656) * Check that para is registered before accepting a bid * Update lib.rs * Update lib.rs * remove println * fix benchmarks * Update runtime/common/src/auctions.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> --- polkadot/runtime/common/src/auctions.rs | 63 +++++++++++++++-- polkadot/runtime/common/src/crowdloan.rs | 1 + .../runtime/common/src/integration_tests.rs | 70 +++++++++++++++++-- polkadot/runtime/common/src/traits.rs | 7 +- polkadot/runtime/rococo/src/lib.rs | 1 + 5 files changed, 130 insertions(+), 12 deletions(-) diff --git a/polkadot/runtime/common/src/auctions.rs b/polkadot/runtime/common/src/auctions.rs index 826ddc8e81..2e40d5fa68 100644 --- a/polkadot/runtime/common/src/auctions.rs +++ b/polkadot/runtime/common/src/auctions.rs @@ -28,7 +28,7 @@ use frame_support::{ use primitives::v1::Id as ParaId; use frame_system::{ensure_signed, ensure_root}; use crate::slot_range::{SlotRange, SLOT_RANGE_COUNT}; -use crate::traits::{Leaser, LeaseError, Auctioneer}; +use crate::traits::{Leaser, LeaseError, Auctioneer, Registrar}; use parity_scale_codec::Decode; type CurrencyOf = <::Leaser as Leaser>::Currency; @@ -57,6 +57,9 @@ pub trait Config: frame_system::Config { /// The number of blocks over which a single period lasts. 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; @@ -155,6 +158,8 @@ decl_error! { LeasePeriodInPast, /// The origin for this call must be a parachain. NotParaOrigin, + /// Para is not registered + ParaNotRegistered, /// The parachain ID is not on-boarding. ParaNotOnboarding, /// The origin for this call must be the origin who registered the parachain. @@ -379,6 +384,8 @@ impl Module { last_slot: LeasePeriodOf, amount: BalanceOf, ) -> DispatchResult { + // 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); // Assume it's actually an auction (this should never fail because of above). @@ -611,7 +618,7 @@ mod tests { }; use frame_system::{EnsureSignedBy, EnsureOneOf, EnsureRoot}; use pallet_balances; - use crate::auctions; + use crate::{auctions, mock::TestRegistrar}; use primitives::v1::{BlockNumber, Header, Id as ParaId}; type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic; @@ -769,6 +776,7 @@ mod tests { impl Config for Test { type Event = Event; type Leaser = TestLeaser; + type Registrar = TestRegistrar; type EndingPeriod = EndingPeriod; type SampleLength = SampleLength; type Randomness = TestPastRandomness; @@ -783,7 +791,15 @@ mod tests { pallet_balances::GenesisConfig::{ balances: vec![(1, 10), (2, 20), (3, 30), (4, 40), (5, 50), (6, 60)], }.assimilate_storage(&mut t).unwrap(); - t.into() + let mut ext: sp_io::TestExternalities = t.into(); + ext.execute_with(|| { + // Register para 0, 1, 2, and 3 for tests + assert_ok!(TestRegistrar::::register(1, 0.into(), Default::default(), Default::default())); + assert_ok!(TestRegistrar::::register(1, 1.into(), Default::default(), Default::default())); + assert_ok!(TestRegistrar::::register(1, 2.into(), Default::default(), Default::default())); + assert_ok!(TestRegistrar::::register(1, 3.into(), Default::default(), Default::default())); + }); + ext } fn run_to_block(n: BlockNumber) { @@ -1336,6 +1352,17 @@ mod tests { }); } + #[test] + fn handle_bid_requires_registered_para() { + new_test_ext().execute_with(|| { + run_to_block(1); + assert_ok!(Auctions::new_auction(Origin::signed(6), 5, 1)); + assert_noop!(Auctions::bid(Origin::signed(1), 1337.into(), 1, 1, 4, 1), Error::::ParaNotRegistered); + assert_ok!(TestRegistrar::::register(1, 1337.into(), Default::default(), Default::default())); + assert_ok!(Auctions::bid(Origin::signed(1), 1337.into(), 1, 1, 4, 1)); + }); + } + // Here we will test that taking only 10 samples during the ending period works as expected. #[test] fn less_winning_samples_work() { @@ -1497,6 +1524,22 @@ mod benchmarking { let auction_index = AuctionCounter::get(); let minimum_balance = CurrencyOf::::minimum_balance(); + for n in 1 ..= SLOT_RANGE_COUNT as u32 { + let owner = account("owner", n, 0); + let worst_validation_code = T::Registrar::worst_validation_code(); + let worst_head_data = T::Registrar::worst_head_data(); + CurrencyOf::::make_free_balance_be(&owner, BalanceOf::::max_value()); + + assert!(T::Registrar::register( + owner, + ParaId::from(n), + worst_head_data, + worst_validation_code + ).is_ok()); + } + + T::Registrar::execute_pending_transitions(); + for n in 1 ..= SLOT_RANGE_COUNT as u32 { let bidder = account("bidder", n, 0); CurrencyOf::::make_free_balance_be(&bidder, BalanceOf::::max_value()); @@ -1549,8 +1592,19 @@ mod benchmarking { let lease_period_index = LeasePeriodOf::::zero(); Auctions::::new_auction(RawOrigin::Root.into(), duration, lease_period_index)?; - // Make an existing bid let para = ParaId::from(0); + let new_para = ParaId::from(1); + + // Register the paras + let owner = account("owner", 0, 0); + CurrencyOf::::make_free_balance_be(&owner, BalanceOf::::max_value()); + let worst_head_data = T::Registrar::worst_head_data(); + let worst_validation_code = T::Registrar::worst_validation_code(); + T::Registrar::register(owner.clone(), para, worst_head_data.clone(), worst_validation_code.clone())?; + T::Registrar::register(owner, new_para, worst_head_data, worst_validation_code)?; + T::Registrar::execute_pending_transitions(); + + // Make an existing bid let auction_index = AuctionCounter::get(); let first_slot = AuctionInfo::::get().unwrap().0; let last_slot = first_slot + 3u32.into(); @@ -1568,7 +1622,6 @@ mod benchmarking { let caller: T::AccountId = whitelisted_caller(); CurrencyOf::::make_free_balance_be(&caller, BalanceOf::::max_value()); - let new_para = ParaId::from(1); let bigger_amount = CurrencyOf::::minimum_balance().saturating_mul(10u32.into()); assert_eq!(CurrencyOf::::reserved_balance(&first_bidder), first_amount); }: _(RawOrigin::Signed(caller.clone()), new_para, auction_index, first_slot, last_slot, bigger_amount) diff --git a/polkadot/runtime/common/src/crowdloan.rs b/polkadot/runtime/common/src/crowdloan.rs index a5e2cff46e..c92d5fc5a0 100644 --- a/polkadot/runtime/common/src/crowdloan.rs +++ b/polkadot/runtime/common/src/crowdloan.rs @@ -1346,6 +1346,7 @@ mod benchmarking { let head_data = T::Registrar::worst_head_data(); let validation_code = T::Registrar::worst_validation_code(); assert_ok!(T::Registrar::register(caller.clone(), para_id, head_data, validation_code)); + T::Registrar::execute_pending_transitions(); assert_ok!(Crowdloan::::create( RawOrigin::Signed(caller).into(), diff --git a/polkadot/runtime/common/src/integration_tests.rs b/polkadot/runtime/common/src/integration_tests.rs index 4cc5b5154a..162716bbed 100644 --- a/polkadot/runtime/common/src/integration_tests.rs +++ b/polkadot/runtime/common/src/integration_tests.rs @@ -76,6 +76,7 @@ frame_support::construct_runtime!( ); use crate::crowdloan::Error as CrowdloanError; +use crate::auctions::Error as AuctionsError; parameter_types! { pub const BlockHashCount: u32 = 250; @@ -189,6 +190,7 @@ parameter_types! { impl auctions::Config for Test { type Event = Event; type Leaser = Slots; + type Registrar = Registrar; type EndingPeriod = EndingPeriod; type SampleLength = SampleLength; type Randomness = TestRandomness; @@ -492,13 +494,16 @@ fn competing_slots() { } // Start a new auction in the future - let duration = 99u32; + let duration = 149u32; let lease_period_index_start = 4u32; assert_ok!(Auctions::new_auction(Origin::root(), duration, lease_period_index_start)); + // Paras should be onboarded + run_to_block(20); // session 2 + for n in 1 ..= max_bids { // Increment block number - run_to_block(n * 10); + run_to_block(System::block_number() + 10); Balances::make_free_balance_be(&(n * 10), n * 1_000); @@ -516,7 +521,7 @@ fn competing_slots() { _ => panic!("test not meant for this"), }; - // User 10 will bid directly for parachain 1 + // Users will bid directly for parachain assert_ok!(Auctions::bid( Origin::signed(n * 10), ParaId::from(n), @@ -532,8 +537,8 @@ fn competing_slots() { assert!(winner.is_some()); } - // Auction should be done - run_to_block(110); + // Auction should be done after ending period + run_to_block(160); // Appropriate Paras should have won slots // 900 + 4500 + 2x 8100 = 21,600 @@ -883,3 +888,58 @@ fn crowdloan_ending_period_bid() { )); }) } + +#[test] +fn auction_bid_requires_registered_para() { + new_test_ext().execute_with(|| { + assert!(System::block_number().is_one()); // So events are emitted + + // Start a new auction in the future + let duration = 99u32; + let lease_period_index_start = 4u32; + assert_ok!(Auctions::new_auction(Origin::root(), duration, lease_period_index_start)); + + // Can't bid with non-registered paras + Balances::make_free_balance_be(&1, 1_000); + assert_noop!(Auctions::bid( + Origin::signed(1), + ParaId::from(1), + 1, // Auction Index + lease_period_index_start + 0, // First Slot + lease_period_index_start + 1, // Last slot + 900, // Amount + ), AuctionsError::::ParaNotRegistered); + + // Now we register the para + assert_ok!(Registrar::register( + Origin::signed(1), + ParaId::from(1), + test_genesis_head(10), + test_validation_code(10), + )); + + // Still can't bid until it is fully onboarded + assert_noop!(Auctions::bid( + Origin::signed(1), + ParaId::from(1), + 1, // Auction Index + lease_period_index_start + 0, // First Slot + lease_period_index_start + 1, // Last slot + 900, // Amount + ), AuctionsError::::ParaNotRegistered); + + // Onboarded on Session 2 + run_to_session(2); + + // Success + Balances::make_free_balance_be(&1, 1_000); + assert_ok!(Auctions::bid( + Origin::signed(1), + ParaId::from(1), + 1, // Auction Index + lease_period_index_start + 0, // First Slot + lease_period_index_start + 1, // Last slot + 900, // Amount + )); + }); +} diff --git a/polkadot/runtime/common/src/traits.rs b/polkadot/runtime/common/src/traits.rs index 97edf65adb..a866156a52 100644 --- a/polkadot/runtime/common/src/traits.rs +++ b/polkadot/runtime/common/src/traits.rs @@ -47,7 +47,8 @@ pub trait Registrar { Self::is_parathread(id) || Self::is_parachain(id) } - // Register a Para ID under control of `who`. + /// Register a Para ID under control of `who`. Registration may be be + /// delayed by session rotation. fn register( who: Self::AccountId, id: ParaId, @@ -55,7 +56,7 @@ pub trait Registrar { validation_code: ValidationCode, ) -> DispatchResult; - // Deregister a Para ID, free any data, and return any deposits. + /// Deregister a Para ID, free any data, and return any deposits. fn deregister(id: ParaId) -> DispatchResult; /// Elevate a para to parachain status. @@ -70,6 +71,8 @@ pub trait Registrar { #[cfg(any(feature = "runtime-benchmarks", test))] fn worst_validation_code() -> ValidationCode; + /// Execute any pending state transitions for paras. + /// For example onboarding to parathread, or parathread to parachain. #[cfg(any(feature = "runtime-benchmarks", test))] fn execute_pending_transitions(); } diff --git a/polkadot/runtime/rococo/src/lib.rs b/polkadot/runtime/rococo/src/lib.rs index 3107ca1d9f..6b805ac584 100644 --- a/polkadot/runtime/rococo/src/lib.rs +++ b/polkadot/runtime/rococo/src/lib.rs @@ -634,6 +634,7 @@ parameter_types! { impl auctions::Config for Runtime { type Event = Event; type Leaser = Slots; + type Registrar = Registrar; type EndingPeriod = EndingPeriod; type SampleLength = SampleLength; type Randomness = ParentHashRandomness;