Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Cleaning "common good" vs "system" #7488

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Cleaning "common good" vs "system" #7488

wants to merge 3 commits into from

Conversation

bkontur
Copy link
Contributor

@bkontur bkontur commented Jul 11, 2023

Everything with paraId<2000 should be considered as system parachain.
(Here where also introduced system_parachain constants: #7005)

There are 3 possible places that are effected by change para<1000 -> para<2000.
So the main question is where to use "explicit paraIds" or relaxed para<2000?

How to setup:


Polkadot:

AllowExplicitUnpaidExecutionFrom

  • actual: AllowExplicitUnpaidExecutionFrom<CollectivesOrFellows>
  • update:
    • no change
    • AllowExplicitUnpaidExecutionFrom<SystemParachainsOrFellows>
      • SystemParachains -> paraId<2000
      • SystemParachains -> explicitly say which one (using constants)

ChildSystemParachainAsSuperuser

  • actual: does not use for LocalOriginConverter
  • update:
    • no change
    • add ChildSystemParachainAsSuperuser<SystemParachains>
      • SystemParachains -> paraId<2000
      • SystemParachains -> explicitly say which one (using constants)

TrustedTeleporters


Kusama:

AllowExplicitUnpaidExecutionFrom

  • actual: AllowExplicitUnpaidExecutionFrom<IsChildSystemParachain<ParaId> (effected bychange in PR paraId<1000 -> paraId<2000)
  • update:
    • no change (with this PR this will automatically include AssetHub, BridgeHub, Collectives, Encointer)
    • AllowExplicitUnpaidExecutionFrom<SystemParachains> -> explicitly say which one (using constants)

ChildSystemParachainAsSuperuser

  • actual: ChildSystemParachainAsSuperuser<ParaId> (effected by change in PR paraId<1000 -> paraId<2000)
  • update:
    • no change (with this PR this will automatically include AssetHub, BridgeHub, Collectives, Encointer)
    • ChildSystemParachainAsSuperuser<SystemParachains> -> explicitly say which one (using constants)

TrustedTeleporters


Westend/Rococo:

the same as Kusama or relaxed more


  • SiblingSystemParachainAsSuperuser - never used in Cumulus (effected by change in PR paraId<1000 -> paraId<2000)

  • move NativeAssetFromSiblingSystemParachain to xcm-builder from here

@bkontur bkontur added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C3-medium PR touches the given topic and has a medium impact on builders. labels Jul 11, 2023
@paritytech-ci paritytech-ci requested review from a team July 11, 2023 11:25
@bkontur bkontur requested review from joepetrowski and removed request for a team July 11, 2023 11:25
@paritytech-ci paritytech-ci requested review from a team July 11, 2023 11:26
@joepetrowski
Copy link
Contributor

ChildSystemParachainAsSuperuser

Superusers should be explicitly listed IMO. Would not use < 2000 for that.

TrustedTeleporters

I am OK with using < 2000 for this.

@bkontur
Copy link
Contributor Author

bkontur commented Jul 11, 2023

ChildSystemParachainAsSuperuser

Superusers should be explicitly listed IMO. Would not use < 2000 for that.

TrustedTeleporters

I am OK with using < 2000 for this.

cool, thank you, so I am going to change that,
and what about:

  • Kusama AllowExplicitUnpaidExecutionFrom<IsChildSystemParachain - < 2000 or explicit?
  • Polkadot AllowExplicitUnpaidExecutionFrom<CollectivesOrFellows> - should we add AssetHub/BridgeHub?

@bkontur bkontur marked this pull request as draft July 11, 2023 19:38
@joepetrowski
Copy link
Contributor

For AllowExplicitUnpaidExecutionFrom, I am not 100% sure. I think paraId < 2000 is OK but lean toward keeping them explicit. It all comes down to whether the chain lets user construct their own XCM programs, and as long as we require governance to use xcm.send, it's only Collectives and Fellowship that can send a message with UnpaidExecution.

So I think either is OK, but perhaps @KiChjang can confirm, or we just keep it explicit for now.

/// System parachain ids are `< 2000`
const SYSTEM_INDEX_END: u32 = 1999;

const USER_INDEX_START: u32 = 2000;
const PUBLIC_INDEX_START: u32 = 2000;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since both USER_INDEX_START and PUBLIC_INDEX_START are 2000, is there an actual difference between the two?

@KiChjang
Copy link
Contributor

So the only problem that we may have in doing < 2000 is that we may accidentally whitelist a chain where we don't really know how its runtime code looks like, and I think Encointer then comes to mind. Being explicit is definitely the safest choice.

@bkontur
Copy link
Contributor Author

bkontur commented Jul 31, 2023

So the only problem that we may have in doing < 2000 is that we may accidentally whitelist a chain where we don't really know how its runtime code looks like, and I think Encointer then comes to mind. Being explicit is definitely the safest choice.

@KiChjang
thank you, and would you mind if I copy out system_parachains constants from your PR here?

@KiChjang
Copy link
Contributor

KiChjang commented Aug 3, 2023

Yeah, it's just a matter of merging master depending on whether your PR or mine lands first.

@bkontur
Copy link
Contributor Author

bkontur commented Aug 16, 2023

bot rebase

@paritytech-processbot
Copy link

Rebased

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C3-medium PR touches the given topic and has a medium impact on builders.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants