Merge/Approval rights for Fellows about runtimes

[shawntabrizi]

Just realized that the identity is stored on the relay chain and the fellowship collectives on the collectives parachain. So, it requires connecting to two chains. However, smoldot should do this internally already to find out what is the best block etc, but I'm not sure if both connected chains can be "used".

I think a map from github name to on-chain address is better managed in this repo.

Updating your identity on-chain unfortunately is not super easy, and will remove your "verified" status unless you pay an identity registrar to re-verify your account.

Some things can be done to make this better, for example giving the fellowship itself an identity registrar spot, which makes sense to me, but I still feel that maintaining a simple lookup table makes more sense here than on-chain.

[arrudagates]

i mean would you for example require only people since dan #3 to be able to approve?

Another point to be raised with this is that disallowing lower ranks to review proposals would hinder even more their ability to rise to upper ranks. In my opinion, all members should be able to participate in GitHub proposal reviews with approval rights, even if that approval has a lower weight compared to higher ranks.

[xlc]

People can always comment/review any PR without any permissions. It is just that those won't be considered for merge. I have been reviewing and commenting Substrate PR for years, and that's how I participate and rank up myself. I did not need to ask anyone for permissions to start.

[arrudagates]

I just want to keep it simple. I hear your request and sounds reasonable but at the same time do you really think it is a must have feature for the initial release? Let's get something out first and then consider improving it. Not the other way around.

I agree, but it's important to have these discussions even if not implemented immediately, which is why I commented on it now.

[bkchr]

For merging we should then have some kind of github action bot that monitors comments on prs. When someone writes bot merge and all CI checks (also the approval check outlined above) are green, the bot should merge the pr. The bot should maybe do some check that the person writing bot merge is a fellowship member, just to prevent spam or stuff getting merged randomly. (It will still require an approval, but yeah)

[bkchr]

Just realized that the identity is stored on the relay chain and the fellowship collectives on the collectives parachain. So, it requires connecting to two chains. However, smoldot should do this internally already to find out what is the best block etc, but I'm not sure if both connected chains can be "used".

[mutantcornholio]

The list of fellowship members is maintained on chain, so it would introduce extra overhead to maintain the list on github as well. Like having teams for each rank and then having some CI check that succeeds when "the voting" has passed.

Feels like we could instead do exactly that, and either reuse PRCR, or even use CODEOWNERS.

Synchronising dans from chain to GitHub teams would also bring an ability to grant other types of GH-permissions (e.g. admin, force-push) to fellows of specific dan.
Isn't that also a problem that needs solving?

[mordamax]

map from a github name to the fellowship rank to do "the voting".

@bkchr what would be the logic of "voting"? i mean would you for example require only people since dan #3 to be able to approve? is it described anywhere how dan allows/restricts ability to manipulate with github or it's TBD?

[bkchr]

Synchronising dans from chain to GitHub teams would also bring an ability to grant other types of GH-permissions (e.g. admin, force-push) to fellows of specific dan.

Isn't as "permissionless" as I proposed, as you would first need to invite someone, but if that is easier, I personally don't care that much. As long as the synchronization would be automatic.

Or only people above some DAN are getting invited as there is more trust in these people.

is it described anywhere how dan allows/restricts ability to manipulate with github or it's TBD?

No, but I also don't see this as that complicated when we have the prerequisites.

[bkchr]

Some things can be done to make this better, for example giving the fellowship itself an identity registrar spot

We are currently working on this.

Not sure why we should maintain multiple lists which will then require someone to update the list in here as well etc.

[mutantcornholio]

@arrudagates do you mean something like 10 approves from people of dan 3 should be equal to 1 approve of dan 6?

[arrudagates]

@arrudagates do you mean something like 10 approves from people of dan 3 should be equal to 1 approve of dan 6?

Yes, these ratios would have to be fine tuned, of course, but I think this would be a fair compromise.
Perhaps the ratios could even be dynamic, 75% of total Dan 1 members equals 1 actual approval, then to merge it needs a specific amount of actual approvals

[arrudagates]

People can always comment/review any PR without any permissions. It is just that those won't be considered for merge. I have been reviewing and commenting Substrate PR for years, and that's how I participate and rank up myself. I did not need to ask anyone for permissions to start.

Yes, that's fair and it works out for those outside the fellowship, but at the same time those who are already members of the fellowship should have some power regardless of rank (even if weighted lower than upper ranks), otherwise it hurts the fellowship diversity.

[xlc]

I just want to keep it simple. I hear your request and sounds reasonable but at the same time do you really think it is a must have feature for the initial release? Let's get something out first and then consider improving it. Not the other way around.

[bkchr]

@mordamax what is the status? When do you think we can make use of this bot?

[mordamax]

@mordamax what is the status? When do you think we can make use of this bot?

@Bullrich could you please respond? ☝️

[Bullrich]

Hi @bkchr!

Currently review-bot is being built with this in mind. It will have the ability to hot swap the source from where we get the "team" members (currently only GitHub, but it is a replaceable module).

Version 1 is almost finished.

I had been in contact with Harry and also updated @josepot on our needs so we can use CAPI.

Harry built a small function for me where I was able to obtain all the users and metadata from the chain.

We have a blocker which requires a different team to resolve:

While we can obtain the chain data, the data needs to be updated so the fellows add their GitHub handles. I don't know how this is could be done, or who to ask, but until this is resolved, we won't be able to obtain their handles.

Could you please provide me some guidance with this?

[bkchr]

Version 1 is almost finished.

How much time do you think you will need finish 1.0?

Could you please provide me some guidance with this?

Until this issue is done, we will not have "native" support for the github handle. However, we already support additional data in the identity. We will require to store there ("github", github_handle). If the look at the declaration of Data, we can see that we can at most store up to 32 bytes. github will fit easily in there as a key, however the github handle could maybe be longer than 32 bytes. Thus, I would propose that we use BlakeTwo256(GITHUB_HANDLE) per user there. Then the bot would need to hash the approving accounts as well and check them against this list. Does this sounds reasonable?

[Bullrich]

How much time do you think you will need finish 1.0?

Technically speaking I only need to finish 2 more tasks to achieve the same functionality that is being used in the polkadot-sdk repo.

• paritytech/review-bot#54
• paritytech/review-bot#53

But, I plan to have all my requirements for version 1 ready by the end of next week (which also includes thorough testing).

Until this issue is done, we will not have "native" support for the github handle.

Thanks for passing me that issue, I'll link it to my tasks.

However, we already support additional data in the identity. We will require to store there ("github", github_handle).

This would work if the users update such additional data.

If the look at the declaration of Data, {..} Does this sounds reasonable?

Should be possible, I will need to request the help from the CAPI team, but I don't see any reason for which it wouldn't be achievable.

[bkchr]

This would work if the users update such additional data.

Yes for sure, but this is some I will take care of.

• Assign reviewers paritytech/review-bot#53

But this is optional? And not really required for the issue here.

[Bullrich]

But this is optional? And not really required for the issue here.

Yes, that would only be required to achieve the same functionality that is being used in the polkadot-sdk repo.

But it is completely optional.

[bkchr]

@Bullrich what is the status?

[Bullrich]

Hi @bkchr!

Review-Bot version 1 has been completed yesterday. I'm currently testing complex cases in paritytech-stg and fixing edge cases.

It currently replicates most of PRCR behaviour (it doesn't assign reviewers yet, that will be in version 1.1.0).

Once I have this version's bugs fixed, I'll be merging it into polkadot-sdk.

That's when I'm going to ask @josepot for help with paritytech/review-bot#61

Any updates with the identity field or with paritytech/polkadot-sdk#179?

[bkchr]

We will require to store there ("github", github_handle)

We will start with this, as I said. So, we don't need paritytech/polkadot-sdk#179 for now.

Once I have this version's bugs fixed, I'll be merging it into polkadot-sdk.

I'm interested in this repo here and nothing else.

(it doesn't assign reviewers yet, that will be in version 1.1.0)

We don't need this, we need paritytech/review-bot#61

[bkchr]

@Bullrich it's me again ;) Status?

[Bullrich]

Hi @bkchr!

Yesterday @josepot showed us how he is able to fetch the github handle with his library.

I'm currently close to achieve the same functionality using polkadot-js (which will be replaced by the other library in the future).

We have been able to extract the github data in both cases, so we are almost there! I'll comment here once we have the final PR

[Bullrich]

@bkchr I have a draft Pull Request that is fetching the fellow's handles here: paritytech/review-bot#79

[Bullrich]

I have two more sub tasks:

• paritytech/review-bot#81
• paritytech/review-bot#80

I will finish those today and have the release ready.

In the meanwhile, do you know what kind of rules will you be requiring?

the config is extremely similar to PRCR, so I can adapt it to review-bot.

[bkchr]

For the pr adding the bot, I propose we let everyone from rank 2 voting. Rank - 1 should be your "voting power". All approvals together should be above 20 voting power. We can then discuss the proper configuration in your pr.

[bkchr]

@Bullrich I see all your tasks are finished? Could you please open a pr to add the bot to the repo?

[Bullrich]

@Bullrich I see all your tasks are finished? Could you please open a pr to add the bot to the repo?

@bkchr find the Pull Request here:

• #31

I added example cases, but feel free to request the changes that you find suitable.