Joystream / joystream

Joystream Monorepo
http://www.joystream.org
GNU General Public License v3.0
1.42k stars 115 forks source link

`FreezePallet` Proposal and pallet #4897

Closed bedeho closed 1 year ago

bedeho commented 1 year ago

Background

We want to be able to allow the council to block the functioning of entire pallets on very short notice to deal with 0-day problems. The idea is then that this freezing will be in place until an upgrade can pass. This is an imperfect solution, but its still useful.

Proposal

  1. Introduce a new pallet pallet_freezer which simply stores an individual indicator of whether each of our pallets are enabled or not. This means that this pallet is coupled with whatever the set of pallets will be in the full runtime, which is fine. The storage representation can be something simple as BtreeSet<FreezablePallet> where FreezablePallet is an enum representing all the pallets that are freezable. There is no need for any extrinsics, but there does need to be a public getter and setter which can be used by other pallets to check if they are frozen or the proposal to update what pallets are frozen, respectively. Getter can have signature like is_fronze(FreezablePallet) and set_freezer(BTreeMap<FreezablePallet, Bool>).
  2. Introduce a new proposal in the codex, it takes BTreeMap<FreezablePallet, Bool> as a proposal parameter.
  3. Proposal parameters are needed!
  4. Make sure to add a distinct getter trait that other pallets can use for their runtime configuration to dial out and check if they are frozen.

We can incorporate all pallets that are written as part of Joystream code base, but we don't need to actually implement sensitivity to the freezing off the bat, only really CRT freezing is needed.

┆Issue is synchronized with this Asana task by Unito

bedeho commented 1 year ago

NOTE:

bedeho commented 1 year ago

We decided to use a different approach which is to locally store whether any given pallet is frozen or not as a configurable boolean that is consulted on all extrinsics and public methods in the pallet. So no new pallet is needed, however a cross plallet proposal is still needed.

freakstatic commented 1 year ago

@bedeho should root still be allowed to perform an extrinsics if a pallet was frozen?

bedeho commented 1 year ago

@bedeho should root still be allowed to perform an extrinsics if a pallet was frozen?

We agreed on Nara call that it does not matter for Nara, because we are only covering project_token pallet, which does not have root calls. But for the future, probably root should not be frozen.

bedeho commented 1 year ago

We agreed on Nara call that a calls into a frozen pallet - such as project_token, should be defensive, which in the case of the project_token pallet means that all issuer calls must check up front that the pallet is not frozen. If it is, it has to return a suitable error.

freakstatic commented 1 year ago

@bedeho should initialize_channel_transfer be allowed when CRT pallet is frozen?

I'm asking because of this ensures calls

            if let Ok(token_id) = channel.ensure_creator_token_issued::<T>() {
                ensure!(
                    T::ProjectToken::is_revenue_split_inactive(token_id),
                    Error::<T>::ChannelTransfersBlockedDuringRevenueSplits
                );

                ensure!(
                    T::ProjectToken::is_sale_unscheduled(token_id),
                    Error::<T>::ChannelTransfersBlockedDuringTokenSales,
                );
            }
bedeho commented 1 year ago

@bedeho should initialize_channel_transfer be allowed when CRT pallet is frozen?

Yes it should be allowed.

freakstatic commented 1 year ago

is this parameters for the freeze proposal ok?

voting_period: days!(3),
grace_period: hours!(2),
approval_quorum_percentage: TWO_OUT_OF_THREE,
approval_threshold_percentage: TWO_OUT_OF_THREE,
slashing_quorum_percentage: ALL,
slashing_threshold_percentage: ALL,
required_stake: Some(dollars!(50)),
constitutionality: 1,
bedeho commented 1 year ago

@mochet recommends 1 block grace period to match the purpose of being an emergency proposal.