Deprecate x/auth AccountI abstraction

[aaronc]

Summary

We should replace the AccountI abstraction with a simpler and more minimalistic x/auth design.

Problem Definition

The current AccountI abstraction exposes four things: address, pub key, account number, and sequence. These are all concrete fields on BaseAccount.

In protobuf, in order for clients to get any of this info, they need to get the account Any and switch over all the different types of accounts (BaseAccount, PeriodicVestingAccount, ModuleAccount, etc. and even custom defined ones outside the SDK).

There is no point to this. None of the vesting stuff should be in account to begin with and the additional abstractions that making AccountI an interface could be dealt with in different ways. I think this is tech debt from the time when balances were stored with accounts and the account abstraction was sort of a “document” to capture all things related to accounts. I think we’ve learned that this is not the right way to do things but just haven’t refactored things. Interfaces make sense when there is different behavior in all the implementations. But there is no different behavior for any AccountI methods, they all just embed BaseAccount actually obfuscating it from being just a simple struct.

The other stuff in AccountI has nothing to do with the actual interface but is just stuffed into the same storage location. Vesting should live elsewhere. The module account stuff isn’t really done correctly and is more related to bank.

Also the overloading of all this stuff into x/auth creates circular dependencies (in particular with x/bank) making #11899 harder.

Proposal

I propose two phases:

Phase 1: Simple account info query service

Create a query service for clients to get the account info in a simpler way without all the Any overhead as proposed in #13210, which will service as the starting point for x/auth v1.

Phase 2: Refactor and simplify x/auth

• move vesting and tx stuff to a different modules altogether
• deal with module account stuff better in bank (for example Add bank module escrows instead of account balances/blocked addresses #13212)
• refactor x/auth to only store:
  • address
  • account number
  • sequence
  • credential - which abstracts over both pubkeys, ADR 028 pub keys, etc.

The current cosmos.auth.v1beta1 query service can be still supported in legacy mode with this design returning all accounts just as BaseAccount.

[amaury1093]

I agree to remove AccountI, and to migrate all accounts (vesting, module, group...) to a simple BaseAccount struct.

Do we need sequence to be in BaseAccount? Or can it go to a real pubkey's credential type?

[aaronc]

I agree to remove AccountI, and to migrate all accounts (vesting, module, group...) to a simple BaseAccount struct.

Do we need sequence to be in BaseAccount? Or can it go to a real pubkey's credential type?

I think it should be separate from the credential. Sequence should even be a separate store key from the rest of state as it's the most frequently updated

[amaury1093]

Sequence in an separate store key makes sense to me too for performance reasons. However, from a data model perspective, sequence is only needed if your credential has a VerifySignature function. Group accounts, module accounts don't need sequences.

Edit: it's not a strong opinion though, maybe it's simpler to keep sequence on BaseAccount, and keep it 0 for privkey-less accounts.

[aaronc]

Sequence in an separate store key makes sense to me too for performance reasons. However, from a data model perspective, sequence is only needed if your credential has a VerifySignature function. Group accounts, module accounts don't need sequences.

Edit: it's not a strong opinion though, maybe it's simpler to keep sequence on BaseAccount, and keep it 0 for privkey-less accounts.

That's what I'm thinking. It could be essentially unset by a credential which doesn't verify signatures and take up zero storage space. It would get set only on the first transaction.

[tac0turtle]

done with accounts