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

[People] Add new registrar #382

Open
ghost opened this issue Jul 14, 2024 · 11 comments · Fixed by #383
Open

[People] Add new registrar #382

ghost opened this issue Jul 14, 2024 · 11 comments · Fixed by #383

Comments

@ghost
Copy link

ghost commented Jul 14, 2024

Hello guys,

I've been testing with Chopsticks a preimage for a referenda to add a new registrar at People Parachain.
So far I've tried multiple combinations and none of them worked:

  1. Preimage:
    xcmPallet.send => v4, parachain 1004, Unpaid Execution + Transact SuperUser with the encoded call to 'identity.addRegistrar'
  2. Referenda
    I add the preImage above at the General Admin track, do all steps, vote, confirming, etc...
    When it dispatches the call I get: scheduler.Dispatched Err BadOrigin

So I decided to use Native instead of SuperUser again by the General Admin track, and I got the same error BadOrigin.

Do I need to use another track instead of General Admin?

Though I can see this at the runtime code:

pub type IdentityAdminOrigin = EitherOfDiverse<
	EnsureRoot<AccountId>,
	EnsureXcm<IsVoiceOfBody<GovernanceLocation, GeneralAdminBodyId>>,
>;

Thanks,

@bkchr
Copy link
Contributor

bkchr commented Jul 15, 2024

CC @joepetrowski @muharem

@muharem
Copy link
Contributor

muharem commented Jul 15, 2024

@winterstamp Kusama I assume? There is no mapping on Kusama Relay for General Admin Origin to it's location that would be recognized on People Chain right now. Root should work now. With the next Kusama Relay release we can fix it.

fix #383

later I can add a test case for it.

@ghost
Copy link
Author

ghost commented Jul 15, 2024

I'm glad to hear we have a fix. Since we are here, I would like to report another concern...

Our team is making a registrar project, and after some research and reading, we thought this would be the best approach:

  • Create a multiSig for the "admins" of the projects
  • Create a pure proxy for the multiSig since in theory the admins can change and we may need to change the multiSig
  • Use the pure proxy address as the registrar address and add it at identity.addRegistrar
  • Delegate this pure proxy address to a normal wallet (a normal proxy) with a seed phrase where it can be used to call the identity extrinsic automatically from the system

The problem we faced was that it was not possible to have the same pure proxy address in both the relaychain and the parachain. Since they are derived from the block + extrinsic index.

In this case, what do you guys think would be the best approach to handle this?

@muharem
Copy link
Contributor

muharem commented Jul 15, 2024

@winterstamp I am looking into the problem with pure proxy cross chain transfer. here the issue - paritytech/polkadot-sdk#3288. I have a solution proposal working locally, I will post it this week, and will will wait for feedback. I cannot say if the solution I will propose the one we will release, but we definitely willing to solve this problem soon, it's a high priority.

@ghost
Copy link
Author

ghost commented Jul 15, 2024

Ah, nice, it is already being worked on, thanks.

I tried to dispatch it from the Root track as suggested, but though this time the xcm was sent successfully in the parachain, I got this. Do you know why?

I saw in the Polkassembly referenda this message:

Add a registrar to the system. The dispatch origin for this call must be `T::RegistrarOrigin`. - `account`: the account of the registrar. Emits `RegistrarAdded` if successful. ## Complexity - `O(R)` where `R` registrar-count (governance-bounded and code-bounded).

Do we need to use the "registrar" account as origin instead of SuperUser ?

image
image

@muharem
Copy link
Contributor

muharem commented Jul 15, 2024

@winterstamp success: no, so it did fail. you need to provide requireWeightAtMost. if rust, you can use the weights from the runtime, if not, you can use the runtime API.

@joepetrowski
Copy link
Contributor

joepetrowski commented Jul 15, 2024

Also note that if you are pulling on-chain versions with Chopsticks, you will need Root, not General Admin.

The origin was just changed in a recent commit to allow General Admin, but this is unreleased. The current privilege to add a new registrar can only be done by Root.

Of course we still need @muharem 's change to allow General Admin to send messages.

@muharem
Copy link
Contributor

muharem commented Jul 16, 2024

I have added the tests to the PR with the fix

@ghost
Copy link
Author

ghost commented Jul 16, 2024

Do you guys think I should create the proposal at GeneralAdmin or Root ?
Considering the fact that right now it only works through root, do you guys think a new version will be released and the runtime will be upgraded before the referenda end if I create it through GeneralAdmin ?

EDIT: Well, I've created here, please let me know if you think I should switch the track and I will gladly do it
https://kusama.polkassembly.io/referenda/418

fellowship-merge-bot bot pushed a commit that referenced this issue Jul 17, 2024
Fixes: #382

General Admin Origin to xcm `Location` mapping for outgoing messages
@ghost
Copy link
Author

ghost commented Jul 17, 2024

Ahh ok, I guess I see now the "problem" of the root track. The 3333.33 KSM deciding deposit 😅

@leonardocustodio
Copy link

leonardocustodio commented Aug 6, 2024

Anyone knows if it is already possible to use the GeneralAdmin track to add a registrar at Polkadot?

Ok, I found it on v1.2.8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants