x/auth Improvements

[aaronc]

I sort of got into this a bit in #7091, but didn't really go into too many details... anyway I want to quickly document my thoughts on places where I think x/auth could be improved.

Remove vesting from x/auth

To begin with, I really think vesting should be pulled out of x/auth. It just feels like it's the wrong place for this and is a holdover from when all balances were also stored in x/auth. Ideally we should have a separate x/vesting module that just interacts with x/bank and in the long run maybe allows for different vesting locks per denom per account.

Simplify Account

Currently we have an AccountI interface which is implemented by all of the vesting accounts, BaseAccount and ModuleAccount. This doesn't really work for generalized vesting though because you can't have a module account or anything else which isn't a BaseAccount(ex: a group account or cosmwasm contract) use vesting. Removing vesting from x/auth will help but this really begs the question of why an AccountI interface at all.

I think we could simplify things and just have a single non-interface Account struct:

message Account {
  string address = 1;
  uint64 account_number = 2;
  uint64 sequence = 3;
  google.protobuf.Any addressable = 4;
}

The Addressable interface was introduced in ADR 028 (see #8398):

type Addressable interface {
  Address() sdk.AccAddress
}

All PubKeys would implement it, but we could also implement it for ModuleID from ADR 033 (#7459).

This would allow us to convert a pubkey account to a group account or a cosmwasm contract via ADR 034 (#7491) which allows us to change the pubkey of an account.

I think this model would be much simpler and more flexible.

Simplify AccountKeeper interface

I need to think about this a bit more, but we really shouldn't be exposing SetAccount directly like this but it should be a much more encapsulated design...

[fedekunze]

also note #6407, although idk if it will apply with the account simplification

[fdymylja]

I generally approve the idea to get rid of vesting accounts from auth. Auth should deal with identity and authentication, whilst bank should take care of balance and supply accounting.

A vested account in the end has some conditions on how he/she can spend coins, and what happens after, so maybe an idea could be creating smart balances.

They're enabled by ADR-033.

I've created a small example:

// SpendCondition defines a set of authorization rules which bind a balance
// and they are smart enough to be interact with the state
type SpendCondition interface {
	Spendable(ctx sdk.Context, amt sdk.Coins) error
	AfterSpend(ctx sdk.Context) error
}

type UnlocksAtVestingCondition struct {
	UnlocksAt time.Time // timestamp in which unlocks
}

func (u UnlocksAtVestingCondition) Spendable(ctx sdk.Context, amt sdk.Coins) error {
	if ctx.BlockTime().After(u.UnlocksAt) {
		return nil
	}
	return fmt.Errorf("cannot spend")
}

func (u UnlocksAtVestingCondition) AfterSpend(ctx sdk.Context) error {
	// using ADR-033 it's possible to query and eventually
	// perform actions such as track delegation etc
	
}

type BalanceV2 struct {
	// address is the address of the balance holder.
	Address string `protobuf:"bytes,1,opt,name=address,proto3" json:"address,omitempty"`
	// coins defines the different coins this balance holds.
	Coins sdk.Coins `protobuf:"bytes,2,rep,name=coins,proto3,castrepeated=github.com/cosmos/cosmos-sdk/types.Coins" json:"coins"`
	
	Condition SpendCondition //
}

func SendCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) {
	var balance BalanceV2
	var err error
	balance, err = bankKeeper.GetBalance(ctx, addr)
	if err := balance.Condition.Spendable(ctx, amt); err != nil {
		panic(err)
	}
	// move coins
	// ---
	// perform after condition
	balance.Condition.AfterSpend(ctx)
	// ecc
}

It might need some thinking, but if it worked it could remove the need of creating a new vesting module, whilst at the same time allowing enough extensibility to build different type of balances which can perform actions on different conditions.

[robert-zaremba]

This is definitely something we need to create. The current AccountI interface is too limited. When introducing Adddressable in ADR-28, the main motivation was to have something that can store assets. We should think how to expand in a beautiful way to DIDs - #6168 .

[robert-zaremba]

Here is a vision to separate things which are Addressable from Credentials. It's oriented towards DID scheme. If things will get right we can have IBC oriented credentials which work cross - chain. In the image below, BaseAccount is just one of the way to store credentials.

[robert-zaremba]

In the account message: why do we have both address and Addressable?

Also, there are accounts that don't use sequence: hash scripts, logic / smart contracts. Those will be Addressable. So, for clarity we should defied an Account as an addressable, managed object with signature and sequence based authorization.

[aaronc]

In the account message: why do we have both address and Addressable?

In the case where the Addressable is a PubKey, the Addressable object isn't known until a Tx is signed.