From f633ad2b0d618326d5e8af63ba35b05baa27e1a5 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Thu, 11 Apr 2024 12:45:27 +0530 Subject: [PATCH 01/10] Adds ability to trigger tasks via unsigned txs --- substrate/frame/examples/tasks/src/lib.rs | 30 +++++++++++++++++++-- substrate/frame/examples/tasks/src/mock.rs | 22 ++++++++++++++- substrate/frame/examples/tasks/src/tests.rs | 28 +++++++++++++++++++ substrate/frame/system/src/lib.rs | 6 +++++ 4 files changed, 83 insertions(+), 3 deletions(-) diff --git a/substrate/frame/examples/tasks/src/lib.rs b/substrate/frame/examples/tasks/src/lib.rs index c65d8095bcf6..c6436b69b45d 100644 --- a/substrate/frame/examples/tasks/src/lib.rs +++ b/substrate/frame/examples/tasks/src/lib.rs @@ -19,6 +19,7 @@ #![cfg_attr(not(feature = "std"), no_std)] use frame_support::dispatch::DispatchResult; +use frame_system::offchain::{SendTransactionTypes, SubmitTransaction}; // Re-export pallet items so that they can be accessed from the crate namespace. pub use pallet::*; @@ -35,6 +36,7 @@ pub use weights::*; pub mod pallet { use super::*; use frame_support::pallet_prelude::*; + use frame_system::pallet_prelude::*; #[pallet::error] pub enum Error { @@ -59,9 +61,33 @@ pub mod pallet { } } + #[pallet::hooks] + impl Hooks> for Pallet { + #[cfg(feature = "experimental")] + fn offchain_worker(_block_number: BlockNumberFor) { + if let Some(key) = Numbers::::iter_keys().next() { + // Create a valid task + let task = Task::::AddNumberIntoTotal { i: key }; + let runtime_task = ::RuntimeTask::from(task); + let call = frame_system::Call::::do_task { task: runtime_task.into() }; + + // Submit the task as an unsigned transaction + SubmitTransaction::>::submit_unsigned_transaction( + call.into(), + ) + .map_err(|()| "Unable to submit unsigned transaction.") + .unwrap(); + } + } + } + #[pallet::config] - pub trait Config: frame_system::Config { - type RuntimeTask: frame_support::traits::Task; + pub trait Config: + SendTransactionTypes> + frame_system::Config + { + type RuntimeTask: frame_support::traits::Task + + IsType<::RuntimeTask> + + From>; type WeightInfo: WeightInfo; } diff --git a/substrate/frame/examples/tasks/src/mock.rs b/substrate/frame/examples/tasks/src/mock.rs index 76ac9e76bff8..58ea2f6f5f2a 100644 --- a/substrate/frame/examples/tasks/src/mock.rs +++ b/substrate/frame/examples/tasks/src/mock.rs @@ -20,8 +20,9 @@ use crate::{self as tasks_example}; use frame_support::derive_impl; +use sp_runtime::testing::TestXt; -pub type AccountId = u32; +pub type AccountId = u64; pub type Balance = u32; type Block = frame_system::mocking::MockBlock; @@ -32,12 +33,31 @@ frame_support::construct_runtime!( } ); +pub type Extrinsic = TestXt; + #[derive_impl(frame_system::config_preludes::TestDefaultConfig)] impl frame_system::Config for Runtime { type Block = Block; } +impl frame_system::offchain::SendTransactionTypes for Runtime +where + RuntimeCall: From, +{ + type OverarchingCall = RuntimeCall; + type Extrinsic = Extrinsic; +} + impl tasks_example::Config for Runtime { type RuntimeTask = RuntimeTask; type WeightInfo = (); } + +pub fn advance_to(b: u64) { + use frame_support::traits::Hooks; + while System::block_number() < b { + System::set_block_number(System::block_number() + 1); + #[cfg(feature = "experimental")] + TasksExample::offchain_worker(System::block_number()); + } +} diff --git a/substrate/frame/examples/tasks/src/tests.rs b/substrate/frame/examples/tasks/src/tests.rs index fc3c69f4aef9..8675a4529219 100644 --- a/substrate/frame/examples/tasks/src/tests.rs +++ b/substrate/frame/examples/tasks/src/tests.rs @@ -19,7 +19,9 @@ #![cfg(test)] use crate::{mock::*, Numbers}; +use codec::Decode; use frame_support::traits::Task; +use sp_core::offchain::{testing, OffchainWorkerExt, TransactionPoolExt}; use sp_runtime::BuildStorage; #[cfg(feature = "experimental")] @@ -130,3 +132,29 @@ fn task_execution_fails_for_invalid_task() { ); }); } + +#[cfg(feature = "experimental")] +#[test] +fn task_with_offchain_worker() { + let (offchain, _offchain_state) = testing::TestOffchainExt::new(); + let (pool, pool_state) = testing::TestTransactionPoolExt::new(); + + let mut t = sp_io::TestExternalities::default(); + t.register_extension(OffchainWorkerExt::new(offchain)); + t.register_extension(TransactionPoolExt::new(pool)); + + t.execute_with(|| { + advance_to(1); + assert!(pool_state.read().transactions.is_empty()); + + Numbers::::insert(0, 10); + assert_eq!(crate::Total::::get(), (0, 0)); + + advance_to(2); + + let tx = pool_state.write().transactions.pop().unwrap(); + assert!(pool_state.read().transactions.is_empty()); + let tx = Extrinsic::decode(&mut &*tx).unwrap(); + assert_eq!(tx.signature, None); + }); +} diff --git a/substrate/frame/system/src/lib.rs b/substrate/frame/system/src/lib.rs index 184f27b61ed2..8d39fa2e228e 100644 --- a/substrate/frame/system/src/lib.rs +++ b/substrate/frame/system/src/lib.rs @@ -1032,6 +1032,12 @@ pub mod pallet { }) } } + #[cfg(feature = "experimental")] + if let Call::do_task { ref task } = call { + if task.is_valid() { + return Ok(ValidTransaction::default()) + } + } Err(InvalidTransaction::Call.into()) } } From 00b64be6d28912fdb817cdd147d04e16753d1706 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Thu, 11 Apr 2024 12:48:03 +0530 Subject: [PATCH 02/10] Minor --- substrate/frame/examples/tasks/src/mock.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/examples/tasks/src/mock.rs b/substrate/frame/examples/tasks/src/mock.rs index 58ea2f6f5f2a..b32f547a3b38 100644 --- a/substrate/frame/examples/tasks/src/mock.rs +++ b/substrate/frame/examples/tasks/src/mock.rs @@ -22,7 +22,7 @@ use crate::{self as tasks_example}; use frame_support::derive_impl; use sp_runtime::testing::TestXt; -pub type AccountId = u64; +pub type AccountId = u32; pub type Balance = u32; type Block = frame_system::mocking::MockBlock; From 9dacc4d9addf734132fb4cf9d1c789ef336e7d14 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Thu, 11 Apr 2024 13:00:57 +0530 Subject: [PATCH 03/10] Fixes unused imports --- substrate/frame/examples/tasks/src/lib.rs | 4 +++- substrate/frame/examples/tasks/src/mock.rs | 1 + substrate/frame/examples/tasks/src/tests.rs | 2 ++ 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/substrate/frame/examples/tasks/src/lib.rs b/substrate/frame/examples/tasks/src/lib.rs index c6436b69b45d..07f846701a75 100644 --- a/substrate/frame/examples/tasks/src/lib.rs +++ b/substrate/frame/examples/tasks/src/lib.rs @@ -19,7 +19,9 @@ #![cfg_attr(not(feature = "std"), no_std)] use frame_support::dispatch::DispatchResult; -use frame_system::offchain::{SendTransactionTypes, SubmitTransaction}; +use frame_system::offchain::SendTransactionTypes; +#[cfg(feature = "experimental")] +use frame_system::offchain::SubmitTransaction; // Re-export pallet items so that they can be accessed from the crate namespace. pub use pallet::*; diff --git a/substrate/frame/examples/tasks/src/mock.rs b/substrate/frame/examples/tasks/src/mock.rs index b32f547a3b38..33912bb5269c 100644 --- a/substrate/frame/examples/tasks/src/mock.rs +++ b/substrate/frame/examples/tasks/src/mock.rs @@ -54,6 +54,7 @@ impl tasks_example::Config for Runtime { } pub fn advance_to(b: u64) { + #[cfg(feature = "experimental")] use frame_support::traits::Hooks; while System::block_number() < b { System::set_block_number(System::block_number() + 1); diff --git a/substrate/frame/examples/tasks/src/tests.rs b/substrate/frame/examples/tasks/src/tests.rs index 8675a4529219..6c8acb0194bd 100644 --- a/substrate/frame/examples/tasks/src/tests.rs +++ b/substrate/frame/examples/tasks/src/tests.rs @@ -19,8 +19,10 @@ #![cfg(test)] use crate::{mock::*, Numbers}; +#[cfg(feature = "experimental")] use codec::Decode; use frame_support::traits::Task; +#[cfg(feature = "experimental")] use sp_core::offchain::{testing, OffchainWorkerExt, TransactionPoolExt}; use sp_runtime::BuildStorage; From 793850dc24ec4106a30f098d1772800ee6d205cc Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Thu, 11 Apr 2024 15:50:20 +0530 Subject: [PATCH 04/10] Adds PrDoc and removes signing requirement for do_task --- prdoc/pr_4075.prdoc | 14 ++++++++++++++ substrate/frame/system/src/lib.rs | 2 -- 2 files changed, 14 insertions(+), 2 deletions(-) create mode 100644 prdoc/pr_4075.prdoc diff --git a/prdoc/pr_4075.prdoc b/prdoc/pr_4075.prdoc new file mode 100644 index 000000000000..ba616b1fd7e4 --- /dev/null +++ b/prdoc/pr_4075.prdoc @@ -0,0 +1,14 @@ +title: Adds ability to trigger tasks via unsigned transactions + +doc: + - audience: Runtime Dev + description: | + This PR updates the `validate_unsigned` hook for `frame_system` to allow valid tasks + to be submitted as unsigned transactions. It also updates the task example to be able to + submit such transactions via an off-chain worker. + +crates: + - name: frame-system + bump: patch + - name: pallet-example-tasks + bump: minor diff --git a/substrate/frame/system/src/lib.rs b/substrate/frame/system/src/lib.rs index 8d39fa2e228e..22b9b41ca4ba 100644 --- a/substrate/frame/system/src/lib.rs +++ b/substrate/frame/system/src/lib.rs @@ -742,8 +742,6 @@ pub mod pallet { #[pallet::call_index(8)] #[pallet::weight(task.weight())] pub fn do_task(origin: OriginFor, task: T::RuntimeTask) -> DispatchResultWithPostInfo { - ensure_signed(origin)?; - if !task.is_valid() { return Err(Error::::InvalidTask.into()) } From 3a2a8c52d685171a72c6f5fceceb3db7810b650b Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Thu, 11 Apr 2024 16:28:57 +0530 Subject: [PATCH 05/10] Minor fix --- substrate/frame/system/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/system/src/lib.rs b/substrate/frame/system/src/lib.rs index 22b9b41ca4ba..24dba6a95039 100644 --- a/substrate/frame/system/src/lib.rs +++ b/substrate/frame/system/src/lib.rs @@ -741,7 +741,7 @@ pub mod pallet { #[cfg(feature = "experimental")] #[pallet::call_index(8)] #[pallet::weight(task.weight())] - pub fn do_task(origin: OriginFor, task: T::RuntimeTask) -> DispatchResultWithPostInfo { + pub fn do_task(_origin: OriginFor, task: T::RuntimeTask) -> DispatchResultWithPostInfo { if !task.is_valid() { return Err(Error::::InvalidTask.into()) } From 7c7ec1eb4032c745b9336e45e9a57bd2dd862347 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Fri, 19 Apr 2024 14:44:44 +0530 Subject: [PATCH 06/10] Adds docs --- substrate/frame/support/src/traits/tasks.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/substrate/frame/support/src/traits/tasks.rs b/substrate/frame/support/src/traits/tasks.rs index 24f3430cf50b..d635613df31c 100644 --- a/substrate/frame/support/src/traits/tasks.rs +++ b/substrate/frame/support/src/traits/tasks.rs @@ -46,6 +46,9 @@ pub trait Task: Sized + FullCodec + TypeInfo + Clone + Debug + PartialEq + Eq { fn iter() -> Self::Enumeration; /// Checks if a particular instance of this `Task` variant is a valid piece of work. + /// This is used to validate tasks for unsigned execution. Hence, it MUST be cheap + /// with minimal to no storage reads. Else, it can make the blockchain vulnerable + /// to DoS attacks. fn is_valid(&self) -> bool; /// Performs the work for this particular `Task` variant. From 01790910a21f7ef61d2f2f3eced4e19b3ec700b0 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Fri, 19 Apr 2024 14:48:30 +0530 Subject: [PATCH 07/10] Updates PrDoc: --- prdoc/pr_4075.prdoc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/prdoc/pr_4075.prdoc b/prdoc/pr_4075.prdoc index ba616b1fd7e4..e131ceb21171 100644 --- a/prdoc/pr_4075.prdoc +++ b/prdoc/pr_4075.prdoc @@ -7,6 +7,9 @@ doc: to be submitted as unsigned transactions. It also updates the task example to be able to submit such transactions via an off-chain worker. + Note that `is_valid` call on a task MUST be cheap with minimal to no storage reads. + Else, it can make the blockchain vulnerable to DoS attacks. + crates: - name: frame-system bump: patch From 0b803c771b1721202da61118d0594a9b5df2d9f2 Mon Sep 17 00:00:00 2001 From: gupnik Date: Tue, 23 Apr 2024 12:05:52 +0200 Subject: [PATCH 08/10] Update substrate/frame/support/src/traits/tasks.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- substrate/frame/support/src/traits/tasks.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/substrate/frame/support/src/traits/tasks.rs b/substrate/frame/support/src/traits/tasks.rs index d635613df31c..42b837e55970 100644 --- a/substrate/frame/support/src/traits/tasks.rs +++ b/substrate/frame/support/src/traits/tasks.rs @@ -46,6 +46,7 @@ pub trait Task: Sized + FullCodec + TypeInfo + Clone + Debug + PartialEq + Eq { fn iter() -> Self::Enumeration; /// Checks if a particular instance of this `Task` variant is a valid piece of work. + /// /// This is used to validate tasks for unsigned execution. Hence, it MUST be cheap /// with minimal to no storage reads. Else, it can make the blockchain vulnerable /// to DoS attacks. From a967ade2811a5b8ccf9e811829377298557b62c0 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Tue, 23 Apr 2024 18:43:05 +0530 Subject: [PATCH 09/10] Addresses review comments --- prdoc/pr_4075.prdoc | 2 ++ substrate/frame/examples/tasks/src/lib.rs | 15 ++++++++++----- substrate/frame/support/src/lib.rs | 3 +++ substrate/frame/system/src/lib.rs | 8 +++++++- 4 files changed, 22 insertions(+), 6 deletions(-) diff --git a/prdoc/pr_4075.prdoc b/prdoc/pr_4075.prdoc index e131ceb21171..05e54073b6c7 100644 --- a/prdoc/pr_4075.prdoc +++ b/prdoc/pr_4075.prdoc @@ -10,6 +10,8 @@ doc: Note that `is_valid` call on a task MUST be cheap with minimal to no storage reads. Else, it can make the blockchain vulnerable to DoS attacks. + Further, these tasks will be executed in a random order. + crates: - name: frame-system bump: patch diff --git a/substrate/frame/examples/tasks/src/lib.rs b/substrate/frame/examples/tasks/src/lib.rs index 07f846701a75..e46e56879818 100644 --- a/substrate/frame/examples/tasks/src/lib.rs +++ b/substrate/frame/examples/tasks/src/lib.rs @@ -34,6 +34,8 @@ mod benchmarking; pub mod weights; pub use weights::*; +const LOG_TARGET: &str = "pallet-example-tasks"; + #[frame_support::pallet(dev_mode)] pub mod pallet { use super::*; @@ -74,11 +76,14 @@ pub mod pallet { let call = frame_system::Call::::do_task { task: runtime_task.into() }; // Submit the task as an unsigned transaction - SubmitTransaction::>::submit_unsigned_transaction( - call.into(), - ) - .map_err(|()| "Unable to submit unsigned transaction.") - .unwrap(); + let res = + SubmitTransaction::>::submit_unsigned_transaction( + call.into(), + ); + match res { + Ok(_) => log::info!(target: LOG_TARGET, "Submitted the task."), + Err(e) => log::error!(target: LOG_TARGET, "Error submitting task: {:?}", e), + } } } } diff --git a/substrate/frame/support/src/lib.rs b/substrate/frame/support/src/lib.rs index 984a7f7537fe..7eddea1259d7 100644 --- a/substrate/frame/support/src/lib.rs +++ b/substrate/frame/support/src/lib.rs @@ -2465,6 +2465,9 @@ pub mod pallet_macros { /// Finally, the `RuntimeTask` can then used by a script or off-chain worker to create and /// submit such tasks via an extrinsic defined in `frame_system` called `do_task`. /// + /// When submitted as unsigned transactions (for example via an off-chain workder), note + /// that the tasks will be executed in a random order. + /// /// ## Example #[doc = docify::embed!("src/tests/tasks.rs", tasks_example)] /// Now, this can be executed as follows: diff --git a/substrate/frame/system/src/lib.rs b/substrate/frame/system/src/lib.rs index 24dba6a95039..30df4dcfd43e 100644 --- a/substrate/frame/system/src/lib.rs +++ b/substrate/frame/system/src/lib.rs @@ -1033,7 +1033,13 @@ pub mod pallet { #[cfg(feature = "experimental")] if let Call::do_task { ref task } = call { if task.is_valid() { - return Ok(ValidTransaction::default()) + return Ok(ValidTransaction { + priority: u64::max_value(), + requires: Vec::new(), + provides: vec![T::Hashing::hash_of(&task.encode()).as_ref().to_vec()], + longevity: TransactionLongevity::max_value(), + propagate: true, + }) } } Err(InvalidTransaction::Call.into()) From af10ae53c8495bfb99d557dd0a9915e41f8e111b Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Wed, 24 Apr 2024 10:56:03 +0530 Subject: [PATCH 10/10] Minor --- substrate/frame/examples/tasks/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/substrate/frame/examples/tasks/src/lib.rs b/substrate/frame/examples/tasks/src/lib.rs index e46e56879818..1908a235ba15 100644 --- a/substrate/frame/examples/tasks/src/lib.rs +++ b/substrate/frame/examples/tasks/src/lib.rs @@ -34,6 +34,7 @@ mod benchmarking; pub mod weights; pub use weights::*; +#[cfg(feature = "experimental")] const LOG_TARGET: &str = "pallet-example-tasks"; #[frame_support::pallet(dev_mode)]