Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable Calls to Identity Pallet #1476

Merged
merged 7 commits into from
Sep 20, 2023
Merged

Disable Calls to Identity Pallet #1476

merged 7 commits into from
Sep 20, 2023

Conversation

joepetrowski
Copy link
Contributor

This PR filters calls from the Identity pallet from all Relay Chain runtimes as preparation to move the identity state and logic to a system parachain within each network.

After this change is deployed to a runtime, no more changes such as adding new sub-identities will be possible. The frozen state will be part of the genesis state of the system chain. After the system chain launches, the pallet and all state will be removed from each Relay Chain.

Applications and UIs that render display information from this pallet will need to read from the system chain when it launches.

@joepetrowski joepetrowski added the T8-polkadot This PR/Issue is related to/affects the Polkadot network. label Sep 8, 2023
Copy link
Member

@ggwpez ggwpez left a comment

Choose a reason for hiding this comment

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

We need to make sure that this is in the changelog. And maybe also ping Jaco.

There is a similar MR over here that is looking for a second review 🙏 #1303

@gilescope
Copy link
Contributor

Will this prevent identity calls within a utility.batch or a utility.asDerivative call ? (Ideally we should have a generic base call filter that can walk containers)

@joepetrowski
Copy link
Contributor Author

Will this prevent identity calls within a utility.batch or a utility.asDerivative call ? (Ideally we should have a generic base call filter that can walk containers)

Yes, it does

@bkchr
Copy link
Member

bkchr commented Sep 9, 2023

This pr should be opened against the fellowship runtimes repo.

@joepetrowski
Copy link
Contributor Author

Copy link
Contributor

@muharem muharem left a comment

Choose a reason for hiding this comment

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

was curious what an error a user will see, its going to be CallFiltered - link

@joepetrowski joepetrowski requested a review from a team as a code owner September 18, 2023 09:51
Copy link
Contributor

@KiChjang KiChjang left a comment

Choose a reason for hiding this comment

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

Best get this merged ASAP before we have to redo this again in the fellowship repo.

@joepetrowski
Copy link
Contributor Author

Best get this merged ASAP before we have to redo this again in the fellowship repo.

We will still need it in both repos AFAIK because Rococo/Westend will stay here but Polkadot/Kusama will only be synced with the 1.1.0 commit.

@xlc
Copy link
Contributor

xlc commented Sep 19, 2023

@joepetrowski could you write something at https://forum.polkadot.network about this? People need to know this feature will be paused for some time. How long do you expect for the down time? And combined with polkadot-fellows/runtimes#31, I want to see a written down plan.

@xlc
Copy link
Contributor

xlc commented Sep 19, 2023

And this is going to conflict with #1291 a bit. We need to make sure the fellowship can be a registrar on the new chain as well.

@joepetrowski
Copy link
Contributor Author

joepetrowski commented Sep 19, 2023

And this is going to conflict with #1291 a bit. We need to make sure the fellowship can be a registrar on the new chain as well.

That is not a conflict, it's just a feature request for the new chain. There is nothing about having Identity pallet on a parachain that makes it impossible for Fellowship to do something. The pallet's location in the system is purely architectural.

@xlc
Copy link
Contributor

xlc commented Sep 19, 2023

I am not saying this is going to cause problem, but more work will be required. At least my old testing script have to be updated.
In order words, the fellowship as registrar work have to be paused until the new chain is up.

@joepetrowski
Copy link
Contributor Author

@joepetrowski could you write something at https://forum.polkadot.network about this? People need to know this feature will be paused for some time. How long do you expect for the down time? And combined with polkadot-fellows/runtimes#31, I want to see a written down plan.

The PR description is a written plan. Yes I am working on something to share on the forum and in a Fellowship RFC. It should not be too shocking, the goal of system parachains has always been to migrate functionality off the Relay Chain (mentioned way back in 2019).

@joepetrowski
Copy link
Contributor Author

I am not saying this is going to cause problem, but more work will be required. At least my old testing script have to be updated. In order words, the fellowship as registrar work have to be paused until the new chain is up.

Yes, Polkadot is upgradeable and sometimes testing scripts need to be updated. Do you know that the Fellowship wants to act as a registrar? And if so, having Identity on a parachain seems like a perfect way to eat our own dog food and show a cross-parachain application.

@joepetrowski joepetrowski merged commit 771c3fb into master Sep 20, 2023
6 checks passed
@joepetrowski joepetrowski deleted the joe-disable-identity branch September 20, 2023 05:28
joepetrowski added a commit that referenced this pull request Oct 23, 2023
Reverts #1476

The `lock_pallet` / `unlock_pallet` additions in
#1814 will result in less
downtime for users than using runtime upgrades.
@Polkadot-Forum
Copy link

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/polkadot-release-analysis-v1-2-0/4451/1

ggwpez added a commit to polkadot-fellows/runtimes that referenced this pull request Jan 30, 2024
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
ggwpez added a commit to polkadot-fellows/runtimes that referenced this pull request Jan 31, 2024
This reverts commit fc9da09.

Signed-off-by: Oliver Tale-Yazdi <[email protected]>
fellowship-merge-bot bot referenced this pull request in polkadot-fellows/runtimes Feb 2, 2024
We imported the runtimes into this repo at tag
[`v1.1.0-rc2`](https://github.com/paritytech/polkadot-sdk/commits/v1.1.0-rc2/)
but still merged stuff until they got removed in
[#1731](paritytech/polkadot-sdk#1731).
This re-applies all changes in that commit range
[`v1.1.0-rc2..bf90cb0b`](paritytech/polkadot-sdk@v1.1.0-rc2...bf90cb0)
checked in the SDK with
`git log v1.1.0-rc2..bf90cb0b -- polkadot/runtime/`. Closes
#112

Most changes got applied as part of
#56, but two were
missing:
- paritytech/polkadot-sdk#1303
- <s>https://github.com/paritytech/polkadot-sdk/pull/1476</s> (reverted)

<s>One [open
Q](#67 (comment))
since there was a difference between
[#1291](paritytech/polkadot-sdk#1291) and
#67.</s>

---------

Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Co-authored-by: fellowship-merge-bot[bot] <151052383+fellowship-merge-bot[bot]@users.noreply.github.com>
Co-authored-by: Bastian Köcher <[email protected]>
Co-authored-by: joe petrowski <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T8-polkadot This PR/Issue is related to/affects the Polkadot network.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants