Merge pull request #3073 from autonomys/fix/history-seeding-signed-ex…

…tension

Add `CheckHistorySeeder` signed extension to prevent feeless and invalid remarks

crates/pallet-history-seeding/src/lib.rs

@@ -25,12 +25,6 @@ pub mod pallet {
     #[pallet::pallet]
     pub struct Pallet<T>(_);
 
-    #[pallet::error]
-    pub enum Error<T> {
-        /// The sender is not authorized to seed history
-        NotAuthorized,
-    }
-
     #[pallet::storage]
     #[pallet::getter(fn history_seeder)]
     pub(super) type HistorySeeder<T: Config> = StorageValue<_, T::AccountId, OptionQuery>;
@@ -40,14 +34,7 @@ pub mod pallet {
         /// Seed history with a remark
         #[pallet::call_index(0)]
         #[pallet::weight((T::WeightInfo::seed_history(remark.len() as u32), Pays::No))]
-        pub fn seed_history(origin: OriginFor<T>, remark: Vec<u8>) -> DispatchResult {
-            let who = ensure_signed(origin.clone())?;
-
-            ensure!(
-                Some(who.clone()) == Self::history_seeder(),
-                Error::<T>::NotAuthorized
-            );
-
+        pub fn seed_history(_origin: OriginFor<T>, remark: Vec<u8>) -> DispatchResult {
             let _ = remark;
 
             Ok(())

crates/pallet-history-seeding/src/tests.rs

@@ -1,4 +1,4 @@
-use crate::{self as pallet_history_seeding, Error};
+use crate::{self as pallet_history_seeding};
 use frame_support::traits::BuildGenesisConfig;
 use frame_support::{assert_noop, assert_ok, construct_runtime, derive_impl};
 use frame_system as system;
@@ -68,26 +68,5 @@ fn seed_history_works() {
             RuntimeOrigin::signed(1),
             remark.clone()
         ));
-
-        // Ensure unauthorized account cannot seed history
-        assert_noop!(
-            HistorySeeding::seed_history(RuntimeOrigin::signed(2), remark),
-            Error::<Test>::NotAuthorized
-        );
-    });
-}
-
-#[test]
-fn seed_history_fails_when_no_seeder_set() {
-    new_test_ext().execute_with(|| {
-        let remark = vec![1, 2, 3];
-        assert_noop!(
-            HistorySeeding::seed_history(RuntimeOrigin::signed(1), remark.clone()),
-            Error::<Test>::NotAuthorized
-        );
-        assert_noop!(
-            HistorySeeding::seed_history(RuntimeOrigin::root(), remark),
-            sp_runtime::DispatchError::BadOrigin
-        );
     });
 }

crates/subspace-malicious-operator/src/malicious_bundle_producer.rs

@@ -31,7 +31,9 @@ use sp_transaction_pool::runtime_api::TaggedTransactionQueue;
 use std::error::Error;
 use std::sync::Arc;
 use subspace_core_primitives::PotOutput;
-use subspace_runtime::{DisablePallets, Runtime, RuntimeCall, SignedExtra, UncheckedExtrinsic};
+use subspace_runtime::{
+    CheckHistorySeeder, DisablePallets, Runtime, RuntimeCall, SignedExtra, UncheckedExtrinsic,
+};
 use subspace_runtime_primitives::opaque::Block as CBlock;
 use subspace_runtime_primitives::{AccountId, Balance, Nonce};
 
@@ -426,6 +428,7 @@ pub fn construct_signed_extrinsic(
         frame_system::CheckWeight::<Runtime>::new(),
         pallet_transaction_payment::ChargeTransactionPayment::<Runtime>::from(0u128),
         DisablePallets,
+        CheckHistorySeeder::<Runtime>::new(),
     );
     let raw_payload = generic::SignedPayload::<RuntimeCall, SignedExtra>::from_raw(
         call.clone(),
@@ -440,6 +443,7 @@ pub fn construct_signed_extrinsic(
             (),
             (),
             (),
+            (),
         ),
     );
 

crates/subspace-runtime/src/lib.rs

@@ -35,7 +35,7 @@ include!(concat!(env!("OUT_DIR"), "/wasm_binary.rs"));
 
 use crate::fees::{OnChargeTransaction, TransactionByteFee};
 use crate::object_mapping::extract_block_object_mapping;
-pub use crate::signed_extensions::DisablePallets;
+pub use crate::signed_extensions::{CheckHistorySeeder, DisablePallets};
 use codec::{Decode, Encode, MaxEncodedLen};
 use core::mem;
 use core::num::NonZeroU64;
@@ -968,6 +968,7 @@ pub type SignedExtra = (
     frame_system::CheckWeight<Runtime>,
     pallet_transaction_payment::ChargeTransactionPayment<Runtime>,
     DisablePallets,
+    CheckHistorySeeder<Runtime>,
 );
 /// Unchecked extrinsic type as expected by this runtime.
 pub type UncheckedExtrinsic =

crates/subspace-runtime/src/signed_extensions.rs

@@ -1,5 +1,6 @@
 use crate::{Runtime, RuntimeCall, RuntimeConfigs};
 use codec::{Decode, Encode};
+use core::marker::PhantomData;
 use scale_info::TypeInfo;
 use sp_runtime::traits::{DispatchInfoOf, SignedExtension};
 use sp_runtime::transaction_validity::{
@@ -68,3 +69,127 @@ impl SignedExtension for DisablePallets {
         }
     }
 }
+
+/// A custom signed extension to check if the caller is an authorized history seeder for
+/// the `history_seeding` pallet.
+#[derive(Encode, Decode, Clone, Eq, PartialEq, TypeInfo)]
+#[scale_info(skip_type_params(T))]
+pub struct CheckHistorySeeder<T: pallet_history_seeding::Config>(PhantomData<T>);
+
+impl<T: pallet_history_seeding::Config> core::fmt::Debug for CheckHistorySeeder<T> {
+    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+        f.debug_struct("CheckHistorySeeder").finish()
+    }
+}
+
+impl<T: pallet_history_seeding::Config + Send + Sync> CheckHistorySeeder<T> {
+    pub fn new() -> Self {
+        Self(core::marker::PhantomData)
+    }
+}
+
+impl<T: pallet_history_seeding::Config + Send + Sync> Default for CheckHistorySeeder<T> {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
+impl<T: pallet_history_seeding::Config + Send + Sync> SignedExtension for CheckHistorySeeder<T> {
+    const IDENTIFIER: &'static str = "CheckHistorySeeder";
+
+    type AccountId = T::AccountId;
+    type Call = <Runtime as frame_system::Config>::RuntimeCall;
+    type AdditionalSigned = ();
+    type Pre = ();
+
+    fn additional_signed(&self) -> Result<Self::AdditionalSigned, TransactionValidityError> {
+        Ok(())
+    }
+
+    fn validate(
+        &self,
+        who: &Self::AccountId,
+        call: &Self::Call,
+        _info: &DispatchInfoOf<Self::Call>,
+        _len: usize,
+    ) -> TransactionValidity {
+        match call {
+            crate::RuntimeCall::HistorySeeding(pallet_history_seeding::Call::seed_history {
+                ..
+            }) => {
+                if Some(who.clone()) != pallet_history_seeding::Pallet::<T>::history_seeder() {
+                    return Err(TransactionValidityError::Invalid(
+                        InvalidTransaction::BadSigner,
+                    ));
+                }
+
+                Ok(ValidTransaction::default())
+            }
+            _ => Ok(ValidTransaction::default()),
+        }
+    }
+
+    fn pre_dispatch(
+        self,
+        who: &Self::AccountId,
+        _call: &Self::Call,
+        _info: &DispatchInfoOf<Self::Call>,
+        _len: usize,
+    ) -> Result<Self::Pre, TransactionValidityError> {
+        self.validate(who, _call, _info, _len)?;
+        Ok(())
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::{CheckHistorySeeder, Runtime, RuntimeCall, RuntimeOrigin};
+    use codec::Encode;
+    use frame_support::dispatch::DispatchInfo;
+    use frame_support::pallet_prelude::{InvalidTransaction, TransactionValidityError};
+    use frame_support::{assert_err, assert_ok};
+    use sp_runtime::traits::SignedExtension;
+    use sp_runtime::{AccountId32, BuildStorage};
+
+    pub fn new_test_ext() -> sp_io::TestExternalities {
+        let t = frame_system::GenesisConfig::<Runtime>::default()
+            .build_storage()
+            .unwrap();
+        t.into()
+    }
+
+    #[test]
+    fn test_check_history_seeder_works() {
+        new_test_ext().execute_with(|| {
+            let call = RuntimeCall::HistorySeeding(pallet_history_seeding::Call::seed_history {
+                remark: vec![0u8; 256],
+            });
+
+            let who = AccountId32::new([0u8; 32]);
+
+            assert_err!(
+                CheckHistorySeeder::<Runtime>::new().pre_dispatch(
+                    &who,
+                    &call,
+                    &DispatchInfo::default(),
+                    call.encoded_size()
+                ),
+                TransactionValidityError::Invalid(InvalidTransaction::BadSigner),
+            );
+
+            // set seeder
+            pallet_history_seeding::Pallet::<Runtime>::set_history_seeder(
+                RuntimeOrigin::root(),
+                who.clone(),
+            )
+            .unwrap();
+
+            assert_ok!(CheckHistorySeeder::<Runtime>::new().pre_dispatch(
+                &who,
+                &call,
+                &DispatchInfo::default(),
+                call.encoded_size()
+            ));
+        });
+    }
+}