Make Assets Pallet's Privileged Roles `Option`

paritytech / polkadot-sdk

The Parity Polkadot Blockchain SDK

https://polkadot.com/

1.89k stars 695 forks source link

Make Assets Pallet's Privileged Roles `Option` #91

Open joepetrowski opened 2 years ago

joepetrowski commented 2 years ago

Change the privileged asset roles from AccountId to Option<AccountId>. When the owner is set to None, only governance would be able to restore the role.

For discussion, if owner is Some, and they want to set a lower rank role from None to Some, should that require governance?

This would look similar to the PoolRoles struct in Nomination Pools.

_Originally posted by @joepetrowski in https://github.com/paritytech/substrate/pull/12310#discussion_r982367220_

dharjeezy commented 2 years ago

I see this issue is not solved yet. Can i go ahead to open a PR for it? @joepetrowski @vieira-giulia

vieira-giulia commented 2 years ago

@dharjeezy Hey, yea I do have a branch for it, figuring out what this implies in the rest of the pallet and tests, just having some problems with my computer to run things. Anyway, I remember hearing you talk you already had this change implemented for another reason in another branch so if you just want to open a PR that's fine for me

Polkadot-Forum commented 2 years ago

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

https://forum.polkadot.network/t/preventing-attacks-on-xcm-reserves/902/5

dharjeezy commented 2 years ago

@dharjeezy Hey, yea I do have a branch for it, figuring out what this implies in the rest of the pallet and tests, just having some problems with my computer to run things. Anyway, I remember hearing you talk you already had this change implemented for another reason in another branch so if you just want to open a PR that's fine for me

Alright @vieira-giulia i have gone ahead to open the PR

muharem commented 1 year ago

could be also enum, {Root, Account(who)}, which can be extended later if needed

gui1117 commented 7 months ago

Removing ownership priviledge is a common practice in some coin like meme coin for instance.

PINK token recently revoke ownership by using a trick: move ownership to pure proxy and kill the proxy. This is a bit like inscription, prooving an ownership has been revoked is not light-client friendly
solana TOKEN 2022 contract has a function revoke freeze and mint. (and metadata?)

Actually I am only interested in a feature to revoke every priviledge, I feel like revoking only mint, or only freezer, or only admin is not really useful.

2 implementation:

fine grained: make Owner, Freezer, Issuer, Admin an option and add some call to revoke them individually it is a breaking change, needs migration, frontend will have to update a bit.
Easy implementation: make a new asset status: LiveAndNotOwned. doesn't break the storage, frontend only needs to handle this new status.

joepetrowski commented 7 months ago

Good point about the easy implementation, None roles for anyone but the owner is kind of meaningless. I would maybe go for something more like LiveAndLocked as a status.