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

Remove all non-seed Head Ambassador members from the Program #422

Merged
merged 9 commits into from
Aug 16, 2024

Conversation

ggwpez
Copy link
Member

@ggwpez ggwpez commented Aug 5, 2024

The original OpenGov for the Ambassador Program stated 21 maximal Head Ambassadors. The limit of 21 has now been put in place, but there area already more than 21 in there. This Merge Requests removes everyone that was not among the first 21.

quote:

The program should only allow up to 21 Head Ambassadors. Once the number is reached, adding a new HA shall require replacing an existing HA or making a proposal to all token holders on the Ambassador Admin track to alter the program.

Change:
Add a migration that executes with the next runtime upgrade and removes everyone from the Ambassador program that is not part of the first 21 elected Head Ambassadors:

- 14Gn7SEmCgMX7Ukuppnw5TRjA7pao2HFpuJo39frB42tYLEh
- 1LboBQLsa1iTpGzZvXcSd5VF7jfUYf6MzPNoRy2HT9D9FNk
- 12xRcHjvStkUYgzTh9vyqinN3tUpddgcPnSUcLXZ3ty44Mq1
- 16SQKanFTrN18k9UE8EFbtyeSGNFPjRBg8caVhGoUNL8cdh6
- 1HPKZzzd9nyr2DdvtPxytNMZm3Ld5nh3BBY4Ecgg9JxgL7G
- 13Ec62Cvw9jmPxA23EidSwASPs9X2Vohqv9RCogCfDvXC4c8
- 14N5cTFuzJf6irrQkKNAjiADKsCxgk48LUKx2fA77YRruzMW
- 1hFmn2CuqXqxHgKDqqs2xRBpsPkiRXzJfcLbfDgsW7qgmpA
- 15rYBV5YwGmhzee5PWqrnQtb2HhwWP2rK2f4cLMhFfcNdPZL
- 14x5RbyJxD6KvNyncbJQuJJJ3zHinXg57YKwhJ7q9T9aJq4n
- 1ZSPR3zNg5Po3obkhXTPR95DepNBzBZ3CyomHXGHK9Uvx6w
- 1yCg8NSCgjS4K5KDK5DZGhxUCxmgVyhyG6vBPn5wqUmLuYo
- 14VDkd5mWY9SajUUnEg2LgMgVrbY412H7xn7Y7EXjhnGkiBF
- 1CRmVRcQymMpot855oKDq76kF19jJezMMkGcrvHh1ozEqXa
- 15cZn8K1DaE7qiBWK6mGFJMKYKjFrALTVwe5urpD9PzKSsPY
- 146ZZqm2cMHLf3ju7oc8M9JnPaAktuADAKThagKnXqzjPJbZ
- 14DJADQdE3bUQMFsjsejwCLiG1tiMDsFhCCiXavyBTKHu6kr
- 1HGnvAkk9nbfZ58CzUJhjcrLdEDMkr5qNkqqYkyD5BF5v6Y
- 155G4q3yS7gW933PrdxrY4ersg2YhqWUnGC8GUd7NWiZwuKj
- 16XYgDGN6MxvdmjhRsHLT1oqQVDwGdEPVQqC42pRXiZrE8su
- 15fHj7Q7SYxqMgZ38UpjXS8cxdq77rczTP3JgY9JVi5piMPN

Implications

  • Also removes all non-HA members
  • Could end up with less than 21 HAs if seed HAs are being removed in the meantime

@ggwpez ggwpez changed the title Ambassador Program: Truncate HAs to 21 Remove all non-seed Head Ambassador members from the Ambassador Program Aug 6, 2024
@ggwpez ggwpez changed the title Remove all non-seed Head Ambassador members from the Ambassador Program Remove all non-seed Head Ambassador members from the Program Aug 6, 2024
ggwpez added 5 commits August 6, 2024 14:27
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]>
@ggwpez ggwpez marked this pull request as ready for review August 6, 2024 12:53
CHANGELOG.md Outdated Show resolved Hide resolved
ggwpez and others added 2 commits August 6, 2024 17:07
@Vickysnipe
Copy link

It seems this address 16SQKanFTrN18k9UE8EFbtyeSGNFPjRBg8caVhGoUNL8cdh6 (on line 4 on this list) is a fake persona and there's a ref => https://polkadot.polkassembly.io/referenda/1041 to remove this address from the list of collectives.

@joepetrowski
Copy link
Contributor

It seems this address 16SQKanFTrN18k9UE8EFbtyeSGNFPjRBg8caVhGoUNL8cdh6 (on line 4 on this list) is a fake persona and there's a ref => https://polkadot.polkassembly.io/referenda/1041 to remove this address from the list of collectives.

That's fine.

// The pallet has no nice trait that we could call, so need to use the extrinsic...
let origin: RuntimeOrigin = RawOrigin::Root.into();

let call = pallet_ranked_collective::Call::<Runtime, AmbassadorCollectiveInstance>::remove_member { who: acc.clone().into(), min_rank: 3 };
Copy link
Contributor

Choose a reason for hiding this comment

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

they also stored within the core fellowship pallet, Member storage map.
to remove from ranked collective you can also use do_remove_member_from_rank, it's pub function.

Copy link
Contributor

Choose a reason for hiding this comment

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

@ggwpez can you still do this?

Copy link
Member Author

Choose a reason for hiding this comment

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

they also stored within the core fellowship pallet, Member storage map.

We would have to call pallet-core-fellowship::offboard then? I think it requires signed origin. There should only be a handful of HAs be removed by this migration, so we can probably offboard them manually?
Or i can fudge a signed origin, but its a bit hacky.

to remove from ranked collective you can also use do_remove_member_from_rank, it's pub function.

Yes but it does not emit an event.

@@ -45,6 +45,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
### Added

- Fast promotion tracks for the Fellowship ranks I-III ([polkadot-fellows/runtimes#356](https://github.com/polkadot-fellows/runtimes/pull/356)).
- Migration to remove all but the 21 first elected Head Ambassador members from the Program ([polkadot-fellows/runtimes#422](https://github.com/polkadot-fellows/runtimes/pull/422)).
Copy link
Contributor

Choose a reason for hiding this comment

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

Why? Because we have now an upper maximum? Would be nice to have somewhere documented the reason.

Copy link
Member Author

@ggwpez ggwpez Aug 12, 2024

Choose a reason for hiding this comment

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

Yea the original OpenGov referendum stated 21 as upper limit: #264

Just enacting the limit now will result in more than 21 people being in there, hence why i added this migration. I will extend the MR description.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not do this as public referendum?

Copy link
Contributor

Choose a reason for hiding this comment

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

We discussed with a few HAs and thought this would be better. Doing public referenda would put us back into the same situation, where a number of simultaneous referenda pass and remove more than necessary (which, TBH, is also fine with me). This solution just reduces the number of referenda and makes the selection criteria clear: first 21.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we not remove them in a batch? But if you discussed this, fine.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah but it's the same thing, people can put up competing batches and we don't have a system to say "just pick one of these referenda". I am also fine with either solution -- the migration to keep the first 21 or allow free-for-all removal referenda. Either way new people won't be able to join until it's below 21.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea Free-for-all-removals could also work, but then they would probably remove too many.
Without the migration we are in a constant race-condition with other proposals.

@ggwpez
Copy link
Member Author

ggwpez commented Aug 16, 2024

/merge

@fellowship-merge-bot fellowship-merge-bot bot enabled auto-merge (squash) August 16, 2024 15:57
@fellowship-merge-bot
Copy link
Contributor

Enabled auto-merge in Pull Request

Available commands
  • /merge: Enables auto-merge for Pull Request
  • /merge cancel: Cancels auto-merge for Pull Request
  • /merge help: Shows this menu

For more information see the documentation

@fellowship-merge-bot fellowship-merge-bot bot merged commit f4c5d27 into main Aug 16, 2024
47 checks passed
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 this pull request may close these issues.

5 participants