Return `ValidatorId` instead of `AuthorityId` in `grandpa_roundState` RPC

[liuchengxu]

Currently, the grandpa_roundState RPC returns the missing parts in a list of AuthorityId which is not quite useful as we have to manually look up the associate ValidatorId using the AuthorityId as a key. What we actually care about is a list of missing validator accounts, then we know which validators are missing from the grandpa consensus, and we can try notifying the missing validators to take some actions, preventing from making a worse situation. This allows us quickly find the missing grandpa participants when bad things happened and possibly fix the network by doing anything earlier. @andresilva what do you think?

[andresilva]

The problem is that from GRANDPA side it is not very easy to get this information, GRANDPA doesn't know about any ValidatorId or any of the staking identities that are behind a given AuthorityId. And the code is currently generic, you could launch a substrate chain without session and staking in which case ValidatorId doesn't even exist.

I do agree with you though, and I understand that figuring out what's the ValidatorId that's offline is most useful as a lot of users will only be aware of their validator keys and not so much about the address of a particular session key. Darwinia has built a tool to track grandpa_roundState and then map AuthorityId to ValidatorId https://github.com/darwinia-network/grandma, I have not used it but maybe you could also have a look if it helps your use case.

[liuchengxu]

Yeah, we can definitely build a tool for converting the AuthorityId to ValidatorId for a specific substrate chain as we can find the ValidatorId in the session module and most chains include session in their runtime. Just want to raise a discussion to see if we could make it native to substrate so that we don't have to maintain such a tool on our own.

Here are my quick thoughts:

Provide a way to convert AuthorityId to ValidatorId in GrandpaApi.

pub trait GrandpaApi {
    type Metadata;

    type ValidatorId;

    type ValidatorIdOf: Convert<AuthorityId, Option<Self::ValidatorId>>;
}

Provide an implementation of this conversion in the session module which supports passing a pair of KeyTypeId and encoded session key and finding the related ValidatorId.

Invoke such a conversion inside the RPC, then any runtime including the session module can have the ValidatorId for free.

Does this make any sense?

[liuchengxu]

@andresilva If you feel the direction might be right, I can find some time to actually work on it.

[andresilva]

@liuchengxu Yes, that seems sensible, and would be an improvement over the current situation. In situations where grandpa is lagging behind (and therefore it has not progressed to latest sets since it's not finalizing) it might be possible that the session information isn't available anymore. I think the API should probably provide both AuthorityId and ValidatorId (where the latter one might not be available in some edge cases).

[MOZGIII]

Please don't, this is causing a lot of issues at places where it's already implemented (i.e. babe), and we're planning to implement some cleaner abstractions to get rid of ValidatorIdOf in consensuses and implement it via an external abstraction. The thing is, we don't use ValidatorId as a concept (by design) and only want to have respective consensuses' AuthorityIds. Additional integration of ValidatorId and ValidatorIdOf everywhere seriously hurts composability for us.

[liuchengxu]

@MOZGIII Fair enough. But do we have another way to integrate the ValidatorId info if there is one in the substrate RPC instead of having to rely on an external tool? Making this feature built into the substrate will directly benefit all the substrate-based projects.

[MOZGIII]

Yep, it can be in the substrate - just not in the grandpa or any consensus implementations directly. Currently, aura and grandpa are pretty independent from the AccountId, which is a good thing. The pallet_session and babe consensus already suffer from too tight coupling, making it impossible to use them as building blocks for anything that's not in line with the standard "tie everything through AccountId" idea.

It is quite easy to land an implementation that would be external to the grandpa crate, yet still provide the runtime API and RPCs.