Migrate AccountRetriever to proto

[aaronc]

AccountRetriever still uses the amino JSON legacy queriers.

We can migrate it to use the cosmos.auth.Query service.

Or even, better how about a generic AccountRetriever service that doesn't depend on a specific Account type:

service AccountRetriever {
  GetAccountInfo(GetAccountInfoRequest) returns (GetAccountInfoResponse);
}

message GetAccountInfoRequest {
  bytes address = 1;
}

message GetAccountInfoResponse {
  uint64 account_number = 1;
  uint64 account_sequence = 2;
  bool exists = 3;
}

[alexanderbez]

Why wouldn't it return an account (interface)? I can't recall exactly why, but the notion of a AccountRetriever seems like a codesmell. Do we have a gRPC service for x/auth?

[aaronc]

Why wouldn't it return an account (interface)? I can't recall exactly why, but the notion of a AccountRetriever seems like a codesmell. Do we have a gRPC service for x/auth?

We do but it returns an Any which includes vesting accounts. IMHO that's too complex for most tx signing use cases. We need an API that just returns acc num & seq without bothering users about looking through vesting accounts. Removing vesting accounts from x/auth is something I'm really hoping we get to soon, but in the meantime something like a simple AccountRetriever would help.

[anilcse]

Yes, I agree. Let's have a seperate service for retrieving account info

[alexanderbez]

Started to take a look at this and I'm not quite sure the rationale for updating the interface.

In the PR body, you state use the gRPC query service, which is defined on the Keeper (i.e. AccountKeeper#Account and AccountKeeper#Params). If this is what you mean, then we're already returned an Account (Any) which can easily be converted to an AccountI to give you what you need. So what am I missing?

[aaronc]

In the PR body, you state use the gRPC query service, which is defined on the Keeper (i.e. AccountKeeper#Account and AccountKeeper#Params). If this is what you mean, then we're already returned an Account (Any) which can easily be converted to an AccountI to give you what you need. So what am I missing?

But it can't be converted to AccountI easily by clients. Clients have to type switch through all the vesting type URLs. What I'm proposing is a simple way to get the account number and sequence without needing to worry about vesting, etc. Longer term, I think there's a bigger refactor of x/auth along the lines of #4287.

[alexanderbez]

See my PR @aaronc -- I don't see how you get this info w/o actually querying for an account, which means you might as well keep the current interface.

[aaronc]

Your PR makes sense. Maybe we're talking about different things. I'm not suggesting the AccountRetriever go interface change. I'm just talking about adding a gRPC service with basically the same functionality as AccountRetriever.

[alexanderbez]

I see. So extending the existing gRPC service? We can certainly do that, but under the hood, they'll virtually do the same thing (i.e. get an account and return seq or acc num).

[aaronc]

Right more or less, but I think it should be a different service. One that isn't tied to the auth module. Maybe it goes in the cosmos.tx just like it's in client not auth in the SDK. Just a service for the basic data needed to sign transactions, nothing more.