Safe Mode and Transaction Pause Pallets

As requested in Parachain Technical Summit - Next Steps - #10 by OliverTY the pallets are effectively feature complete IMHO, but there are a few nice-to-have features that I wanted to get feedback on and implement:

  1. Here is a PR to change the tx-pause interface from “stringy” to concretely typed against RuntimeCall variants that needs review & feedback :pray: : WIP safe mode and tx pause {continued} by NukeManDan · Pull Request #12371 · paritytech/substrate · GitHub

    • Basically:
      pub fn pause_call(
      -			pallet: PalletNameOf<T>,
      -			call: CallNameOf<T>,
      +			call: Box<<T as Config>::RuntimeCall>,
      )
      
    • To ensure that the RuntimeCall type used as a field needed to be set by end users, I wanted to make sure it wasn’t too difficult and that it would prevent “footguns” using that PR’s commit 917961b I have made an MVP demo of using the tx-pause with subxt: GitHub - NukeManDan/pause-subxt: Demo of subxt using the tx-pause pallet. . IMHO it is worth the engineering overhead to reason about this to enable more user friendly UX when submitting tx-pause extrinsics.
  2. A request on the primary PR suggests an in-between set of funcionality: not quite a filter per call specificity of tx-pause and one “big red HALT EVERYTHING button” that safe mode is implied to be. This is for sure something to add… but to what pallet?

    I would suggest making a more variable selection for safe-mode that specified more parameters, perhaps even per-origin details (deposit required, what filter selections are callable, etc.) and leave tx-pause the precision tool to (batch as needed) pause and unpause very specific behavior.

WDYT? @xlc @crystalin I know you were the ones to chime in on github so far, would be happy to continue convo on a PR or here with you and the Polakdot community :grin:

I also wanted to open the floor to discuss these pallets in the context of #xcm , as the “plumbing” that parachains use to handle XCM is directly through call dispatch as it stands now in cumulus/pallets at master · paritytech/cumulus · GitHub that relate to XCM, DMP, and XCMP.

With the ability for a runtime call to filter XCM communication via these pallets, what considerations must chain’s take into account?
(I will use this thread to refine the rust docs on these pallets for would-be users to evaluate their use)

With that context and the request 2 above, at least one variant that makes sense to me in a option to isolate the chain: XCM related dispatches should all be filtered (inbound and outbound interchain), but innerchain traffic would remain unfiltered. This filter seems to make most sense as the “default” option to me as well, presuming that if absolutely required innerchain mishaps that would have been filtered could be cleared up with on-chain governance and runtime upgrades/migrations.

I am not deeply in the weeds here but looking at the attached diff, I think this cannot work. The reason being that a Box<<T as Config>::RuntimeCall> is actually a enum that encodes the call and has to contain the arguments of the dispatchable as well.

If you match against this Box<<T as Config>::RuntimeCall>, then it is wrong because you will only filter out a call with specific arguments.

If you take this call, then get the pallet name and call name from it, it would work, but then you are encoding the rest of the call (dispatch arguments for no reason).

All in all, while this can work too, I think just passing in the pallet and call name is an easier approach. Also makes the traceability of this extrinsic in events orders of magnitude easier, not needing to match indices to pallets and calls anymore.

this is how we go about applying the filer to calls, inspired by the proxy pallet methods and functionality. and here we can parse out names from runtime calls that are valid members of the enum, checking statically for build time of the node and subxt-like type checked ways to build extrinsics. Plus at runtime we check the extrinsic data as well.

Not sure what you mean here :sweat_smile: this event is the same using that helper fn from before to extract names… am I missing something?

I see – so if the event is still showing the name, then it is as traceable. What I meant was that instead of pause_transaction("system", "remark"), we would send pause_transaction(vec![0x07, 0x21, 0xafafafaf), which is harder to read.

In this example 0x07 is the system pallet’s index, 0x21 is the remark call index, and 0xafafafa is basically wasted data, which is another minor downside of what you are suggesting (although acceptable).

In other words, we always have to pass in the Default::default() impl pf the call arguments in, because we are passing in the full enum, in the transaction payload.

1 Like

Sorry for the dumb q: In what context would you need to use this raw encoding? I made a demo with subxt that I discuss here where the extrinsics are statically typed.

Yes, Default::default() of args would work, if you impled them, as they are not for example in this subxt example accessible.

Dumb question: is it possible to specify a type that is inclusive of only the call without values? Perhaps its somewhere in the macro logic to generate the RuntimeCall enum or maybe a way to cheaply impl a type that does this from the fully generated RuntimeCall enum…?

Perhaps yes: rust - Compare enums only by variant, not value - Stack Overflow
Will look into this

If this is possible and cheap, it’s another option. Perhaps even something to make accessible within FRAME, if we think other pallets would want a type safe way to check what calls exist (but not their values) within pallets. :thinking: