Merge pull request #2264 from opentensor/feat/reject-incorrect-sudo-exts

Filter for incorrect sudo calls

Author: sam0x17

node/src/benchmarking.rs

@@ -5,8 +5,8 @@
 use crate::client::FullClient;
 
 use node_subtensor_runtime as runtime;
-use node_subtensor_runtime::pallet_subtensor;
 use node_subtensor_runtime::{check_nonce, transaction_payment_wrapper};
+use node_subtensor_runtime::{pallet_subtensor, sudo_wrapper};
 use runtime::{BalancesCall, SystemCall};
 use sc_cli::Result;
 use sc_client_api::BlockBackend;
@@ -139,6 +139,7 @@ pub fn create_benchmark_extrinsic(
             transaction_payment_wrapper::ChargeTransactionPaymentWrapper::new(
                 pallet_transaction_payment::ChargeTransactionPayment::<runtime::Runtime>::from(0),
             ),
+            sudo_wrapper::SudoTransactionExtension::<runtime::Runtime>::new(),
             pallet_subtensor::transaction_extension::SubtensorTransactionExtension::<
                 runtime::Runtime,
             >::new(),
@@ -160,6 +161,7 @@ pub fn create_benchmark_extrinsic(
             (),
             (),
             (),
+            (),
             None,
         ),
     );

node/src/mev_shield/author.rs

@@ -391,6 +391,8 @@ where
             >::new(pallet_transaction_payment::ChargeTransactionPayment::<
                 runtime::Runtime,
             >::from(0u64)),
+            node_subtensor_runtime::sudo_wrapper::SudoTransactionExtension::<runtime::Runtime>::new(
+            ),
             pallet_subtensor::transaction_extension::SubtensorTransactionExtension::<
                 runtime::Runtime,
             >::new(),
@@ -427,6 +429,7 @@ where
         (),           // CheckNonce::Implicit = ()
         (),           // CheckWeight::Implicit = ()
         (),           // ChargeTransactionPaymentWrapper::Implicit = ()
+        (),           // SudoTransactionExtension::Implicit = ()
         (),           // SubtensorTransactionExtension::Implicit = ()
         (),           // DrandPriority::Implicit = ()
         None,         // CheckMetadataHash::Implicit = Option<[u8; 32]>

runtime/src/lib.rs

@@ -12,6 +12,7 @@ use core::num::NonZeroU64;
 
 pub mod check_nonce;
 mod migrations;
+pub mod sudo_wrapper;
 pub mod transaction_payment_wrapper;
 
 extern crate alloc;
@@ -173,6 +174,7 @@ impl frame_system::offchain::CreateSignedTransaction<pallet_drand::Call<Runtime>
             ChargeTransactionPaymentWrapper::new(
                 pallet_transaction_payment::ChargeTransactionPayment::<Runtime>::from(0),
             ),
+            SudoTransactionExtension::<Runtime>::new(),
             pallet_subtensor::transaction_extension::SubtensorTransactionExtension::<Runtime>::new(
             ),
             pallet_drand::drand_priority::DrandPriority::<Runtime>::new(),
@@ -1158,6 +1160,7 @@ impl pallet_subtensor_swap::Config for Runtime {
     type WeightInfo = pallet_subtensor_swap::weights::DefaultWeight<Runtime>;
 }
 
+use crate::sudo_wrapper::SudoTransactionExtension;
 use crate::transaction_payment_wrapper::ChargeTransactionPaymentWrapper;
 use sp_runtime::BoundedVec;
 
@@ -1609,6 +1612,7 @@ pub type TransactionExtensions = (
     check_nonce::CheckNonce<Runtime>,
     frame_system::CheckWeight<Runtime>,
     ChargeTransactionPaymentWrapper<Runtime>,
+    SudoTransactionExtension<Runtime>,
     pallet_subtensor::transaction_extension::SubtensorTransactionExtension<Runtime>,
     pallet_drand::drand_priority::DrandPriority<Runtime>,
     frame_metadata_hash_extension::CheckMetadataHash<Runtime>,

runtime/src/sudo_wrapper.rs

@@ -0,0 +1,84 @@
+use codec::{Decode, DecodeWithMemTracking, Encode};
+use frame_support::dispatch::{DispatchInfo, PostDispatchInfo};
+use frame_support::traits::IsSubType;
+use frame_system::Config;
+use pallet_sudo::Call as SudoCall;
+use scale_info::TypeInfo;
+use sp_runtime::impl_tx_ext_default;
+use sp_runtime::traits::{
+    AsSystemOriginSigner, DispatchInfoOf, Dispatchable, Implication, TransactionExtension,
+    ValidateResult,
+};
+use sp_runtime::transaction_validity::{InvalidTransaction, TransactionSource};
+use sp_std::marker::PhantomData;
+use subtensor_macros::freeze_struct;
+
+#[freeze_struct("99dce71278b36b44")]
+#[derive(Default, Encode, Decode, DecodeWithMemTracking, Clone, Eq, PartialEq, TypeInfo)]
+pub struct SudoTransactionExtension<T: Config + Send + Sync + TypeInfo>(pub PhantomData<T>);
+
+impl<T: Config + Send + Sync + TypeInfo> sp_std::fmt::Debug for SudoTransactionExtension<T> {
+    #[cfg(feature = "std")]
+    fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
+        write!(f, "SudoTransactionExtension",)
+    }
+    #[cfg(not(feature = "std"))]
+    fn fmt(&self, _: &mut core::fmt::Formatter) -> core::fmt::Result {
+        Ok(())
+    }
+}
+
+impl<T: Config + Send + Sync + TypeInfo> SudoTransactionExtension<T> {
+    pub fn new() -> Self {
+        Self(Default::default())
+    }
+}
+
+impl<T: Config + Send + Sync + TypeInfo + pallet_sudo::Config>
+    TransactionExtension<<T as Config>::RuntimeCall> for SudoTransactionExtension<T>
+where
+    <T as Config>::RuntimeCall: Dispatchable<Info = DispatchInfo, PostInfo = PostDispatchInfo>,
+    <T as Config>::RuntimeOrigin: AsSystemOriginSigner<T::AccountId> + Clone,
+    <T as Config>::RuntimeCall: IsSubType<SudoCall<T>>,
+{
+    const IDENTIFIER: &'static str = "SudoTransactionExtension";
+
+    type Implicit = ();
+    type Val = ();
+    type Pre = ();
+
+    impl_tx_ext_default!(<T as Config>::RuntimeCall; weight prepare);
+
+    fn validate(
+        &self,
+        origin: <T as Config>::RuntimeOrigin,
+        call: &<T as Config>::RuntimeCall,
+        _info: &DispatchInfoOf<<T as Config>::RuntimeCall>,
+        _len: usize,
+        _self_implicit: Self::Implicit,
+        _inherited_implication: &impl Implication,
+        _source: TransactionSource,
+    ) -> ValidateResult<Self::Val, <T as Config>::RuntimeCall> {
+        // Ensure the transaction is signed, else we just skip the extension.
+        let Some(who) = origin.as_system_origin_signer() else {
+            return Ok((Default::default(), (), origin));
+        };
+
+        // Check validity of the signer for sudo call
+        if let Some(_sudo_call) = IsSubType::<pallet_sudo::Call<T>>::is_sub_type(call) {
+            let sudo_key = pallet_sudo::pallet::Key::<T>::get();
+
+            // No sudo key configured → reject
+            let Some(expected_who) = sudo_key else {
+                return Err(InvalidTransaction::BadSigner.into());
+            };
+
+            // Signer does not match the sudo key → reject
+            if *who != expected_who {
+                return Err(InvalidTransaction::BadSigner.into());
+            }
+        }
+
+        Ok((Default::default(), (), origin))
+    }
+}

runtime/tests/sudo_wrapper.rs

@@ -0,0 +1,112 @@
+#![allow(clippy::unwrap_used)]
+
+use frame_support::assert_ok;
+use frame_support::dispatch::GetDispatchInfo;
+use node_subtensor_runtime::{
+    BuildStorage, Runtime, RuntimeCall, RuntimeGenesisConfig, RuntimeOrigin, System, SystemCall,
+    sudo_wrapper,
+};
+use sp_runtime::traits::{TransactionExtension, TxBaseImplication, ValidateResult};
+use sp_runtime::transaction_validity::{
+    InvalidTransaction, TransactionSource, TransactionValidityError,
+};
+use subtensor_runtime_common::AccountId;
+
+const SUDO_ACCOUNT: [u8; 32] = [1_u8; 32];
+const OTHER_ACCOUNT: [u8; 32] = [3_u8; 32];
+
+fn new_test_ext() -> sp_io::TestExternalities {
+    let mut ext: sp_io::TestExternalities = RuntimeGenesisConfig {
+        sudo: pallet_sudo::GenesisConfig { key: None },
+        ..Default::default()
+    }
+    .build_storage()
+    .unwrap()
+    .into();
+    ext.execute_with(|| System::set_block_number(1));
+    ext
+}
+
+fn call_remark() -> RuntimeCall {
+    let remark = vec![1, 2, 3];
+    RuntimeCall::System(SystemCall::remark { remark })
+}
+
+fn sudo_extrinsic(inner: RuntimeCall) -> RuntimeCall {
+    RuntimeCall::Sudo(pallet_sudo::Call::sudo {
+        call: Box::new(inner),
+    })
+}
+
+fn validate_ext(origin: RuntimeOrigin, call: &RuntimeCall) -> ValidateResult<(), RuntimeCall> {
+    let ext = sudo_wrapper::SudoTransactionExtension::<Runtime>::new();
+
+    ext.validate(
+        origin,
+        call,
+        &call.get_dispatch_info(),
+        0,
+        (),
+        &TxBaseImplication(()),
+        TransactionSource::External,
+    )
+}
+#[test]
+fn sudo_signed_by_correct_key_is_valid() {
+    new_test_ext().execute_with(|| {
+        let sudo_key = AccountId::from(SUDO_ACCOUNT);
+        pallet_sudo::Key::<Runtime>::put(sudo_key.clone());
+        let sudo_call = sudo_extrinsic(call_remark());
+
+        // Signed origin with correct sudo key
+        let origin = RuntimeOrigin::signed(sudo_key);
+        let res = validate_ext(origin, &sudo_call);
+        assert_ok!(res);
+    });
+}
+
+#[test]
+fn sudo_signed_by_wrong_account_is_rejected() {
+    new_test_ext().execute_with(|| {
+        let sudo_key = AccountId::from(SUDO_ACCOUNT);
+        // Set sudo key in storage
+        pallet_sudo::Key::<Runtime>::put(sudo_key.clone());
+        let sudo_call = sudo_extrinsic(call_remark());
+        // Wrong signer
+        let origin = RuntimeOrigin::signed(AccountId::from(OTHER_ACCOUNT));
+        let res = validate_ext(origin, &sudo_call);
+        assert!(matches!(
+            res,
+            Err(TransactionValidityError::Invalid(
+                InvalidTransaction::BadSigner
+            ))
+        ));
+    });
+}
+
+#[test]
+fn sudo_when_no_sudo_key_configured_is_rejected() {
+    new_test_ext().execute_with(|| {
+        // Remove sudo key
+        pallet_sudo::Key::<Runtime>::kill();
+        let sudo_call = sudo_extrinsic(call_remark());
+        let origin = RuntimeOrigin::signed(AccountId::from(SUDO_ACCOUNT));
+        let res = validate_ext(origin, &sudo_call);
+        assert!(matches!(
+            res,
+            Err(TransactionValidityError::Invalid(
+                InvalidTransaction::BadSigner
+            ))
+        ));
+    });
+}
+
+#[test]
+fn non_sudo_extrinsic_does_not_trigger_filter() {
+    new_test_ext().execute_with(|| {
+        let origin = RuntimeOrigin::signed(AccountId::from(OTHER_ACCOUNT));
+        let call = call_remark();
+        let res = validate_ext(origin, &call);
+        assert!(res.is_ok());
+    });
+}