feat(bank/auth): Do not allow bank to initialize a base account when coins are sent to an account does not exist.

[testinginprod]

If coins are sent to an account which does not exist, then bank will initialize a BaseAccount for that address, this is undesirable and has caused issues as highlighted in #14453.

What I propose is that we remove the part of the code in bank that initializes the base account:

https://github.com/cosmos/cosmos-sdk/blob/4e8d05b516c2c872577d283c45d6cf95cbc31f71/x/bank/keeper/send.go#L190:L193

And we adjust the logic in the SetPubKeyDecorator to create the account, this is currently used to initialize the pubkeys for an address which has an account but not a pubkey associated to it.

Consequences:

1. A bank balance can exist without an account backing it.
2. Bank will not initialize accounts anymore.
3. The account can initialize itself, as long as someone pays fees for the TX (note: the account can pay fees also, because of 1), by simply sending a tx with its own pubkey in it, which is what already happens when it's sending a tx for the first time.

[alexanderbez]

@testinginprod so this issue, as it stands, seems to simply propose moving the initialization logic. But I imagine we also want to avoid creating a BaseAccount, and rather create an account differently. Can you expand upon that?

[testinginprod]

@alexanderbez I think we will eventually want to remove x/auth in favour of x/accounts, thing is that x/accounts expects explicit creation of an account, which is not what the sdk currently does, as people expect to be able to send txs as long as a balance is deposited to an address.

(NOTE: this works well assuming pubkey.hash()->account.address, which will not be always the case once x/accounts is deployed to chains)

[zmanian]

I think this design is reasonable and solves a significant design flaw in the SDK

[webmaster128]

Strong support for this direction. This would work very well for wasmd too.

While we are here, can we also remove CreateVestingAccount the way it is working today (a non-owner of the account can define the account type and do address squatting, leading to all sort of DoS as described here)? Those two things combined would hopefully create a future in which only the owner of an address can set the account type.

[tac0turtle]

we are working on new vesting accounts, when released we would aim to remove creation of legacy accounts

[AdityaSripal]

Supported as well. We ran into this issue with ICA account creation and have to be vigilant agains this type of bug in our audits

[tac0turtle]

we should audit all modules for usage of for use of setaccount, just found it is used in x/feegrant

cosmos-sdk/x/feegrant/keeper/keeper.go

Lines 74 to 80 in 1f0c68d

	// create the account if it is not in account state
	granteeAcc := k.authKeeper.GetAccount(ctx, grantee)
	if granteeAcc == nil {
	granteeAcc = k.authKeeper.NewAccountWithAddress(ctx, grantee)
	k.authKeeper.SetAccount(ctx, granteeAcc)
	}