git.delta.rocks / unique-network / refs/commits / 8fee3684a030

difftreelog

feat(Scheduler [WIP]) critical improvements

Fahrrader2022-02-07parent: #8443538.patch.diff
in: master

5 files changed

modifiedCargo.lockdiffbeforeafterboth
before · Cargo.lock
931 packageslockfile v3
after · Cargo.lock
933 packageslockfile v3
modifiedpallets/scheduler/src/lib.rsdiffbeforeafterboth
--- a/pallets/scheduler/src/lib.rs
+++ b/pallets/scheduler/src/lib.rs
@@ -59,7 +59,7 @@
 use codec::{Encode, Decode, Codec};
 use sp_runtime::{
 	RuntimeDebug,
-	traits::{Zero, One, BadOrigin, Saturating},
+	traits::{Zero, One, BadOrigin, Saturating, SignedExtension},
 };
 use frame_support::{
 	decl_module, decl_storage, decl_event, decl_error,
@@ -75,8 +75,6 @@
 pub use weights::WeightInfo;
 use up_sponsorship::SponsorshipHandler;
 use scale_info::TypeInfo;
-
-pub const MAX_TASK_ID_LENGTH_IN_BYTES: u32 = 16;
 
 /// Our pallet's configuration trait. All our types and constants go in here. If the
 /// pallet is dependent on specific other pallets, then their configuration traits
@@ -118,8 +116,15 @@
 
 	/// Weight information for extrinsics in this pallet.
 	type WeightInfo: WeightInfo;
+
+	/// Unchecked extrinsic type as expected by the runtime that uses this pallet.
+	type PaymentHandler: SignedExtension;
 }
 
+pub const MAX_TASK_ID_LENGTH_IN_BYTES: u8 = 16;
+
+pub const PERIODIC_CALLS_LIMIT: u32 = 100;
+
 // pub type SelfWeightInfo<T> = <T as system::Config>::WeightInfo;
 
 /// Just a simple index for naming period tasks.
@@ -250,7 +255,7 @@
 		)
 		{
 			let origin = <T as Config>::Origin::from(origin);
-			Self::do_schedule(DispatchTime::At(when), maybe_periodic, priority, origin.caller().clone(), *call)?;
+			Self::do_schedule_nameless(DispatchTime::At(when), maybe_periodic, priority, origin.caller().clone(), *call)?;
 		}
 
 		/// Cancel an anonymously scheduled task.
@@ -326,7 +331,7 @@
 		) {
 			T::ScheduleOrigin::ensure_origin(origin.clone())?;
 			let origin = <T as Config>::Origin::from(origin);
-			Self::do_schedule(
+			Self::do_schedule_nameless(
 				DispatchTime::After(after), maybe_periodic, priority, origin.caller().clone(), *call
 			)?;
 		}
@@ -364,7 +369,8 @@
 		/// # </weight>
 		fn on_initialize(now: T::BlockNumber) -> Weight {
 			let limit = T::MaximumWeight::get();
-			let mut queued = Agenda::<T>::take(now).into_iter()
+			let mut queued = Agenda::<T>::take(now)
+				.into_iter()
 				.enumerate()
 				.filter_map(|(index, s)| s.map(|inner| (index as u32, inner)))
 				.collect::<Vec<_>>();
@@ -377,7 +383,7 @@
 			}
 			queued.sort_by_key(|(_, s)| s.priority);
 			let base_weight: Weight = T::DbWeight::get().reads_writes(1, 2); // Agenda + Agenda(next)
-			let mut total_weight: Weight = 0;
+			let mut total_weight: Weight = 0; // T::WeightInfo::on_initialize(0)
 			queued.into_iter()
 				.enumerate()
 				.scan(base_weight, |cumulative_weight, (order, (index, s))| {
@@ -412,13 +418,13 @@
 					// - It is the first item in the schedule
 					if s.priority <= schedule::HARD_DEADLINE || cumulative_weight <= limit || order == 0 {
 
-						let origin = <<T as Config>::Origin as From<T::PalletsOrigin>>::from(
-							s.origin.clone()
-						).into();
-						let sender = ensure_signed(origin).unwrap_or_default();
-						let who_will_pay = T::SponsorshipHandler::get_sponsor(&sender, &s.call).unwrap_or(sender);
-						let sponsor = T::PalletsOrigin::from(system::RawOrigin::Signed(who_will_pay));
-						let r = s.call.clone().dispatch(sponsor.into());
+						// let origin = <<T as Config>::Origin as From<T::PalletsOrigin>>::from(
+						// 	s.origin.clone()
+						// ).into();
+						// let sender = ensure_signed(origin).unwrap_or_default();
+						// let who_will_pay = T::SponsorshipHandler::get_sponsor(&sender, &s.call).unwrap_or(sender);
+						// let sponsor = T::PalletsOrigin::from(system::RawOrigin::Signed(who_will_pay));
+						// let r = s.call.clone().dispatch(sponsor.into());
 						let maybe_id = s.maybe_id.clone();
 						if let Some((period, count)) = s.maybe_periodic {
 							if count > 1 {
@@ -439,7 +445,7 @@
 						Self::deposit_event(RawEvent::Dispatched(
 							(now, index),
 							maybe_id,
-							r.map(|_| ()).map_err(|e| e.error)
+							Ok(())// r.map(|_| ()).map_err(|e| e.error)
 						));
 						total_weight = cumulative_weight;
 						None
@@ -476,21 +482,32 @@
 	}
 
 	fn do_schedule(
+		maybe_id: Option<Vec<u8>>,
 		when: DispatchTime<T::BlockNumber>,
 		maybe_periodic: Option<schedule::Period<T::BlockNumber>>,
 		priority: schedule::Priority,
 		origin: T::PalletsOrigin,
 		call: <T as Config>::Call,
-	) -> Result<TaskAddress<T::BlockNumber>, DispatchError> {
+	) -> Result<(T::BlockNumber, u32), DispatchError> {
 		let when = Self::resolve_time(when)?;
 
+		let sender = ensure_signed(
+			<<T as Config>::Origin as From<T::PalletsOrigin>>::from(origin.clone()).into()
+		).unwrap_or_default();
+		let who_will_pay = T::SponsorshipHandler::get_sponsor(&sender, &call).unwrap_or(sender);
+		let sponsor = T::PalletsOrigin::from(system::RawOrigin::Signed(who_will_pay));
+		let r = call.clone().dispatch(sponsor.into());
+
+		//T::PaymentHandler::validate(sponsor.into(), call.clone(), ); todo dispatch call
+
 		// sanitize maybe_periodic
 		let maybe_periodic = maybe_periodic
 			.filter(|p| p.1 > 1 && !p.0.is_zero())
-			// Remove one from the number of repetitions since we will schedule one now.
-			.map(|(p, c)| (p, c - 1));
+			// Limit the repetitions to 100 calls and remove one from the number of repetitions since we will schedule one now.
+			.map(|(p, c)| (p, std::cmp::min(c, PERIODIC_CALLS_LIMIT) - 1));
+			
 		let s = Some(Scheduled {
-			maybe_id: None,
+			maybe_id,
 			priority,
 			call,
 			maybe_periodic,
@@ -506,9 +523,23 @@
 				expected from the runtime configuration. An update might be needed.",
 			);
 		}
+
+		Ok((when, index))
+	}
+
+	fn do_schedule_nameless(
+		when: DispatchTime<T::BlockNumber>,
+		maybe_periodic: Option<schedule::Period<T::BlockNumber>>,
+		priority: schedule::Priority,
+		origin: T::PalletsOrigin,
+		call: <T as Config>::Call,
+	) -> Result<TaskAddress<T::BlockNumber>, DispatchError> {
+		let address = Self::do_schedule(None, when, maybe_periodic, priority, origin, call)?;
+		let (when, index) = address;
+
 		Self::deposit_event(RawEvent::Scheduled(when, index));
 
-		Ok((when, index))
+		Ok(address)
 	}
 
 	fn do_cancel(
@@ -578,35 +609,11 @@
 			return Err(Error::<T>::FailedToSchedule.into());
 		}
 
-		let when = Self::resolve_time(when)?;
-
-		// sanitize maybe_periodic
-		let maybe_periodic = maybe_periodic
-			.filter(|p| p.1 > 1 && !p.0.is_zero())
-			// Remove one from the number of repetitions since we will schedule one now.
-			.map(|(p, c)| (p, c - 1));
+		let address = Self::do_schedule(Some(id.clone()), when, maybe_periodic, priority, origin, call)?;
+		let (when, index) = address;
 
-		let s = Scheduled {
-			maybe_id: Some(id.clone()),
-			priority,
-			call,
-			maybe_periodic,
-			origin,
-			_phantom: Default::default(),
-		};
-		Agenda::<T>::append(when, Some(s));
-		let index = Agenda::<T>::decode_len(when).unwrap_or(1) as u32 - 1;
-		if index > T::MaxScheduledPerBlock::get() {
-			log::warn!(
-				target: "runtime::scheduler",
-				"Warning: There are more items queued in the Scheduler than \
-				expected from the runtime configuration. An update might be needed.",
-			);
-		}
-		let address = (when, index);
 		Lookup::<T>::insert(&id, &address);
 		Self::deposit_event(RawEvent::Scheduled(when, index));
-
 		Ok(address)
 	}
 
@@ -680,7 +687,7 @@
 		origin: T::PalletsOrigin,
 		call: <T as Config>::Call,
 	) -> Result<Self::Address, DispatchError> {
-		Self::do_schedule(when, maybe_periodic, priority, origin, call)
+		Self::do_schedule_nameless(when, maybe_periodic, priority, origin, call)
 	}
 
 	fn cancel((when, index): Self::Address) -> Result<(), ()> {
@@ -796,8 +803,6 @@
 	}
 
 	type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic<Test>;
-	// todo check current cumulus implementation, add changes accordingly, do something with signedExtra and (un)checked extrinsic (integral to Scheduler?)
-	// todo to include pallet in runtime, not only uncomment but also (implement?) trait
 	type Block = frame_system::mocking::MockBlock<Test>;
 
 	frame_support::construct_runtime!(
@@ -871,5 +876,6 @@
 		type MaxScheduledPerBlock = MaxScheduledPerBlock;
 		type WeightInfo = ();
 		type SponsorshipHandler = ();
+		type PaymentHandler = ();
 	}
 }
modifiedruntime/src/lib.rsdiffbeforeafterboth
--- a/runtime/src/lib.rs
+++ b/runtime/src/lib.rs
@@ -798,11 +798,11 @@
 	type BlockNumberProvider = RelayChainBlockNumberProvider<Runtime>;
 }
 
-// parameter_types! {
-// 	pub MaximumSchedulerWeight: Weight = Perbill::from_percent(50) *
-// 		RuntimeBlockWeights::get().max_block;
-// 	pub const MaxScheduledPerBlock: u32 = 50;
-// }
+parameter_types! {
+	pub MaximumSchedulerWeight: Weight = Perbill::from_percent(50) *
+		RuntimeBlockWeights::get().max_block;
+	pub const MaxScheduledPerBlock: u32 = 50;
+}
 
 type EvmSponsorshipHandler = (
 	pallet_unique::UniqueEthSponsorshipHandler<Runtime>,
@@ -814,17 +814,18 @@
 	pallet_evm_transaction_payment::BridgeSponsorshipHandler<Runtime>,
 );
 
-// impl pallet_unq_scheduler::Config for Runtime {
-// 	type Event = Event;
-// 	type Origin = Origin;
-// 	type PalletsOrigin = OriginCaller;
-// 	type Call = Call;
-// 	type MaximumWeight = MaximumSchedulerWeight;
-// 	type ScheduleOrigin = EnsureSigned<AccountId>;
-// 	type MaxScheduledPerBlock = MaxScheduledPerBlock;
-// 	type SponsorshipHandler = SponsorshipHandler;
-// 	type WeightInfo = ();
-// }
+impl pallet_unq_scheduler::Config for Runtime {
+	type Event = Event;
+	type Origin = Origin;
+	type PalletsOrigin = OriginCaller;
+	type Call = Call;
+	type MaximumWeight = MaximumSchedulerWeight;
+	type ScheduleOrigin = EnsureSigned<AccountId>;
+	type MaxScheduledPerBlock = MaxScheduledPerBlock;
+	type SponsorshipHandler = SponsorshipHandler;
+	type WeightInfo = ();
+	type PaymentHandler = pallet_charge_transaction::ChargeTransactionPayment<Runtime>;
+}
 
 impl pallet_evm_transaction_payment::Config for Runtime {
 	type EvmSponsorshipHandler = EvmSponsorshipHandler;
@@ -885,7 +886,7 @@
 		// Unique Pallets
 		Inflation: pallet_inflation::{Pallet, Call, Storage} = 60,
 		Unique: pallet_unique::{Pallet, Call, Storage, Event<T>} = 61,
-		// Scheduler: pallet_unq_scheduler::{Pallet, Call, Storage, Event<T>} = 62,
+		Scheduler: pallet_unq_scheduler::{Pallet, Call, Storage, Event<T>} = 62,
 		// free = 63
 		Charging: pallet_charge_transaction::{Pallet, Call, Storage } = 64,
 		// ContractHelpers: pallet_contract_helpers::{Pallet, Call, Storage} = 65,
modifiedtests/package.jsondiffbeforeafterboth
--- a/tests/package.json
+++ b/tests/package.json
@@ -64,6 +64,7 @@
     "testOverflow": "mocha --timeout 9999999 -r ts-node/register ./**/overflow.test.ts",
     "testSetVariableMetadataSponsoringRateLimit": "mocha --timeout 9999999 -r ts-node/register ./**/setVariableMetadataSponsoringRateLimit.test.ts",
     "testInflation": "mocha --timeout 9999999 -r ts-node/register ./**/inflation.test.ts",
+    "testScheduler": "mocha --timeout 9999999 -r ts-node/register ./**/scheduler.test.ts",
     "testPalletPresence": "mocha --timeout 9999999 -r ts-node/register ./**/pallet-presence.test.ts",
     "testBlockProduction": "mocha --timeout 9999999 -r ts-node/register ./**/block-production.test.ts",
     "testEnableDisableTransfers": "mocha --timeout 9999999 -r ts-node/register ./**/enableDisableTransfer.test.ts",
modifiedtests/src/scheduler.test.tsdiffbeforeafterboth
--- a/tests/src/scheduler.test.ts
+++ b/tests/src/scheduler.test.ts
@@ -14,14 +14,23 @@
   setCollectionSponsorExpectSuccess,
   confirmSponsorshipExpectSuccess,
 } from './util/helpers';
+import {IKeyringPair} from '@polkadot/types/types';
 
 chai.use(chaiAsPromised);
 
-describe.skip('Integration Test scheduler base transaction', () => {
+describe('Integration Test scheduler base transaction', () => {
+  let alice: IKeyringPair;
+  let bob: IKeyringPair;
+
+  before(async() => {
+    await usingApi(async () => {
+      alice = privateKey('//Alice');
+      bob = privateKey('//Bob');
+    });
+  });
+
   it('User can transfer owned token with delay (scheduler)', async () => {
     await usingApi(async () => {
-      const alice = privateKey('//Alice');
-      const bob = privateKey('//Bob');
       // nft
       const nftCollectionId = await createCollectionExpectSuccess();
       const newNftTokenId = await createItemExpectSuccess(alice, nftCollectionId, 'NFT');