[AHM] Add Polkadot call filtering #559
Conversation
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
| type RcCallEnabledDuringMigration: Contains<<Self as frame_system::Config>::RuntimeCall>; | ||
| /// Contains all calls that are allowed after the migration finished. | ||
| type RcCallEnabledAfterMigration: Contains<<Self as frame_system::Config>::RuntimeCall>; |
There was a problem hiding this comment.
| type RcCallEnabledDuringMigration: Contains<<Self as frame_system::Config>::RuntimeCall>; | |
| /// Contains all calls that are allowed after the migration finished. | |
| type RcCallEnabledAfterMigration: Contains<<Self as frame_system::Config>::RuntimeCall>; | |
| type RcCallsEnabledDuringMigration: Contains<<Self as frame_system::Config>::RuntimeCall>; | |
| /// Contains all calls that are allowed after the migration finished. | |
| type RcCallsEnabledAfterMigration: Contains<<Self as frame_system::Config>::RuntimeCall>; |
Or even something like RcIntraMigrationCalls and RcPostMigrationCalls for shorter naming.
| const OFF: bool = true; | ||
|
|
||
| match call { | ||
| System(..) => (OFF, ON), |
There was a problem hiding this comment.
Should we consider an escape hatch for System? If we bork the migration for some unknown reason and get stuck and don't get to the MigrationDone stage for more than an era (or some reasonable time period) we should allow some subset of root-only calls to rescue ourselves from doomsday gov. CC @bkontur
| // ParaSessionInfo has no calls | ||
| ParasDisputes(..) => (OFF, ON), // TODO check with security | ||
| ParasSlashing(..) => (OFF, ON), // TODO check with security | ||
| OnDemand(..) => (OFF, ON), |
There was a problem hiding this comment.
If we get paritytech/polkadot-sdk#5990 merged in time we should try to avoid filtering on-demand, as there will no longer be a balances dependency and we should avoid a situation where parachains can't make blocks through the migration.
| OnDemand(..) => (OFF, ON), | ||
| // CoretimeAssignmentProvider has no calls | ||
| Registrar(..) => (OFF, ON), | ||
| Slots(..) => (OFF, ON), // TODO not sure |
There was a problem hiding this comment.
| Slots(..) => (OFF, ON), // TODO not sure | |
| Slots(..) => (OFF, OFF), |
We don't need these extrinsics with coretime.
| Registrar(..) => (OFF, ON), | ||
| Slots(..) => (OFF, ON), // TODO not sure | ||
| Auctions(..) => (OFF, OFF), | ||
| Crowdloan(..) => (OFF, ON), // TODO maybe only payouts |
There was a problem hiding this comment.
Yea probably just withdraw, refund and dissolve.
Ideally we'd transfer this state to AH and allow people to withdraw or refund there directly, but it would be basically an entirely new pallet and would break all the crowdloan apps maybe unnecessarily. That's likely going to be the next phase of AHM though when the EDs go up. Still like 20 months until the last crowdloans expire so we need something long term
| FastUnstake(..) => (OFF, OFF), | ||
| // DelegatedStaking has on calls | ||
| // ParachainsOrigin has no calls | ||
| Configuration(..) => (OFF, ON), |
There was a problem hiding this comment.
| Configuration(..) => (OFF, ON), | |
| Configuration(..) => (ON, ON), |
All root calls
| ParaInherent(..) => (ON, ON), // only inherents | ||
| // ParaScheduler has no calls | ||
| Paras(..) => (OFF, ON), // TODO only root so could think about keeping it on | ||
| Initializer(..) => (OFF, ON), // only root so could think about keeping it on |
There was a problem hiding this comment.
| Initializer(..) => (OFF, ON), // only root so could think about keeping it on | |
| Initializer(..) => (ON, ON), |
| ParaInclusion(..) => (OFF, OFF), /* Has no calls but a call enum https://github.com/paritytech/polkadot-sdk/blob/74ec1ee226ace087748f38dfeffc869cd5534ac8/polkadot/runtime/parachains/src/inclusion/mod.rs#L352-L353 */ | ||
| ParaInherent(..) => (ON, ON), // only inherents | ||
| // ParaScheduler has no calls | ||
| Paras(..) => (OFF, ON), // TODO only root so could think about keeping it on |
There was a problem hiding this comment.
| Paras(..) => (OFF, ON), // TODO only root so could think about keeping it on | |
| Paras(..) => (ON, ON), |
I agree, and also this (and the other parachains pallet extrinsics) shouldn't have any bearing on the balances or state that we're migrating, but CC @x3c41a would be good to test that the migrations work even if these extrinsics run mid migration. Maybe more like fuzzing since there are so many calls and stage combinations and a stretch, but would be nice to have if we have time.
| let mut t: sp_io::TestExternalities = | ||
| frame_system::GenesisConfig::<T>::default().build_storage().unwrap().into(); | ||
|
|
||
| // MQ calls are never filtered: |
There was a problem hiding this comment.
Did we get to the bottom of the prioritisation of system messages? I lost track of the status of your streaking work
| } | ||
|
|
||
| fn is_forbidden(call: &polkadot_runtime::RuntimeCall) -> bool { | ||
| let Err(err) = call.clone().dispatch(RuntimeOrigin::signed(AccountId32::from([0; 32]))) else { |
There was a problem hiding this comment.
This is actually a nice test to run on basically every pallet and extrinsic during the migration except for a few exceptions that are explicitly allowed from signed origins - the ones that are root only will fail anyway from this origin.
We could then have a separate check for the root calls that should be filtered, with the baseline being that all root calls are allowed except the ones we know will cause problems.
To be merged into the AHM working branch.
Changes:
Configure the RC Migrator pallet as call filter for the Polkadot Relay chain
Test
Does not require a CHANGELOG entry