paritytech / trappist

Apache License 2.0
82 stars 17 forks source link

[Trappist on Rococo] Add pallet for maintenance mode #144

Closed stiiifff closed 1 year ago

stiiifff commented 1 year ago

Add a pallet to Trappist's runtime to allow for a maintenance (aka emergency) mode. Moonbeam and Acala, among others, have a similar feature.

hbulgarini commented 1 year ago

I have analyzed both pallets and my vote goes for Moonbeam since seems to be more generic and better structured.

It also contains XCMP handles.

hbulgarini commented 1 year ago

Maintenance mode pallet spec

The pallet logic itself will be very basic and it will just expose two extrinsics that will handle the on/off property for the maintenance mode. But the specific part goes in the impls exposed from the pallet. The traits implementations frame_support::traits::Contains and cumulus_primitives_core::DmpMessageHandler will help to a list of defined runtime calls and XCM messages respectively:

Runtime Call Filtering (Contains<RuntimeCall>)

The Contains<RuntimeCall> trait implementation twill be set to be used by the the BaseCallFilter feature of the frame_system pallet. The logic is not hard to follow:

    impl<T: Config> Contains<T::RuntimeCall> for Pallet<T> {
        fn contains(call: &T::RuntimeCall) -> bool {
            if MaintenanceMode::<T>::get() {
                T::FilteredCalls::contains(call)
            }  else {
                             return false;
                         }
        }
    }

From the runtime the FilteredCalls type will be provided to the pallet so we can filter which calls we want:

pub struct RuntimeFilteredCalls;
impl Contains<RuntimeCall> for RuntimeFilteredCalls {
    fn contains(call: &RuntimeCall) -> bool {
        match call {
            RuntimeCall::Assets(method) => match method {
                pallet_assets::Call::transfer { .. } => true,
                pallet_assets::Call::transfer_keep_alive { .. } => true,
                                  ...
                _ => false,
            },
                    ....
        }
    }
}

XCM messages (cumulus_primitives_core::DmpMessageHandler)

From the XCM messages perspective, the DMP Queue messages will be filtered by relying on the cumulus_primitives_core::DmpMessageHandler::handle_dmp_messages(iter, limit). By providing Weight::zero() to the second argument, basically the messages are queued for next iteration.

From the runtime perspective, the struct MaintenanceDmpHandler will be a wrapper of the DmpQueue::handle_dmp_messages and it will be provided as a type to the pallet:

pub struct MaintenanceDmpHandler;
impl DmpMessageHandler for MaintenanceDmpHandler {
    fn handle_dmp_messages(
        iter: impl Iterator<Item = (RelayBlockNumber, Vec<u8>)>,
        limit: Weight,
    ) -> Weight {
        DmpQueue::handle_dmp_messages(iter,limit)
    }
}

Then from the pallet side, the DmpMessageHandler will use the MaintenanceDmpHandler in the following way:

    impl<T: Config> DmpMessageHandler for Pallet<T> {
        fn handle_dmp_messages(
            iter: impl Iterator<Item = (RelayBlockNumber, Vec<u8>)>,
            limit: Weight,
        ) -> Weight {
            if MaintenanceMode::<T>::get() {
                T::MaintenanceDmpHandler::handle_dmp_messages(iter, Weight::zero())
            } else {
                T::MaintenanceDmpHandler::handle_dmp_messages(iter, limit)
            }
        }
    }

Pallet adjustments over the runtime


construct_runtime! {
    pub enum Runtime where
        Block = Block,
        NodeBlock = opaque::Block,
        UncheckedExtrinsic = UncheckedExtrinsic
    {
                    MaintenanceMode: pallet_maintenance_mode::{Pallet, Call, Config, Storage, Event} = 50,
         }
}

impl frame_system::Config for Runtime {
        ...
    type BaseCallFilter = MaintenanceMode;
        ...
}

impl cumulus_pallet_parachain_system::Config for Runtime {
    ...
    type DmpMessageHandler = MaintenanceMode;
        ...
}

Transaction pool

Finally we should filter as well the calls in the transaction pool so they are not included during the maintenance:

    impl sp_transaction_pool::runtime_api::TaggedTransactionQueue<Block> for Runtime {
        fn validate_transaction(
            source: TransactionSource,
            tx: <Block as BlockT>::Extrinsic,
            block_hash: <Block as BlockT>::Hash,
        ) -> TransactionValidity {
            if !<Runtime as frame_system::Config>::BaseCallFilter::contains(&xt.0.function) {
                return InvalidTransaction::Call.into();
            }
            Executive::validate_transaction(source, tx, block_hash)

        }
    }

CC: @stiiifff @valentinfernandez1 @kalaninja

stiiifff commented 1 year ago

@hbulgarini Looks good, thanks for spec-ing this out before.

kalaninja commented 1 year ago
    impl<T: Config> Contains<T::RuntimeCall> for Pallet<T> {
        fn contains(call: &T::RuntimeCall) -> bool {
            if MaintenanceMode::<T>::get() {
                T::FilteredCalls::contains(call)
            }  else {
                             return false; // <-- should be `true` here if you are whitelisting
                         }
        }
    }
valentinfernandez1 commented 1 year ago

Would it be useful to be able to select which pallets/calls are whitelisted when the Lockdown mode is active? This feature would enable us to gradually enable pallets during testing and isolate any problematic pallets for individual testing purposes.

By selectively choosing which pallets are allowed in the whitelist, we can thoroughly test the node as we progressively enable more pallets.

I was just thinking about it, but I'm not fully sure how much extra value it would provide to the pallet.

hbulgarini commented 1 year ago

Would it be useful to be able to select which pallets/calls are whitelisted when the Lockdown mode is active? This feature would enable us to gradually enable pallets during testing and isolate any problematic pallets for individual testing purposes.

By selectively choosing which pallets are allowed in the whitelist, we can thoroughly test the node as we progressively enable more pallets.

I was just thinking about it, but I'm not fully sure how much extra value it would provide to the pallet.

We already do for this pallet: https://github.com/paritytech/trappist/blob/main/runtime/trappist/src/impls.rs#L76

However i think that what you want to have based in your comments is the functionality of the Proxy pallet. Here you have an example: https://github.com/paritytech/cumulus/blob/master/parachains/runtimes/assets/statemine/src/lib.rs#L386