move Un/DelegateCoins to x/staking

[fedekunze]

Summary

Discuss whether DelegateCoins and UndelegateCoins should live within the bank or staking module.

Problem Definition

Currently the bank keeper handles the delegation and delegation of coins. The thing with this is that as an SDK dev I don't want to use the staking module (say by implementing an own PoA or PoW module). Then this 2 functions are useless:

cosmos-sdk/x/bank/keeper.go

Lines 14 to 26 in 055d219

	// Keeper defines a module interface that facilitates the transfer of coins
	// between accounts.
	type Keeper interface {
	SendKeeper
	SetCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) sdk.Error
	SubtractCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) (sdk.Coins, sdk.Tags, sdk.Error)
	AddCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) (sdk.Coins, sdk.Tags, sdk.Error)
	InputOutputCoins(ctx sdk.Context, inputs []Input, outputs []Output) (sdk.Tags, sdk.Error)
	DelegateCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) (sdk.Tags, sdk.Error)
	UndelegateCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) (sdk.Tags, sdk.Error)
	}

cc: @rigelrozanski @cwgoes @alessio @jackzampolin

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[alexanderbez]

The staking keeper when then need access to the auth's account keeper...just for this sole purpose. The staking keeper already needed the bank keeper, hence we decided to implement on such. Otherwise, the staking keeper would need to get the account, do the arithmetic, and set the coins. Sounds like a banking operation to me.

[alessio]

@alexanderbez I don't think @fedekunze meant to call into question the design rationale, which is very clear indeed. However in his use case of develping a PoA dapp, as an SDK end-user he wants to use bank features and at the same time avoids exposing staking delegation features.

(I'm CC'ing @sunnya97 @mossid @sabau too)

[alexanderbez]

Sure, so just don't simply use those features 😉. That or update the staking module to do accounting.

[rigelrozanski]

Yeah I think it could make sense to include these accounting features within the staking module - the ultimate home of delegation logic is in fact staking. Ideally we could somehow abstract out accounting patterns within bank so that any duplicate accounting patterns required for use within staking for the delegation logic can simply come directly from bank.

This said, I think that the fact that the initial build of the delegation logic has been added to bank is totally acceptable as the MVP. This issue simply describes the next evolution

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.