Merge PR #4663: Refactor bank keeper by removing private funcs

This commit is contained in:
Federico Kunze 2019-07-02 17:19:21 +02:00 committed by Alexander Bezobchuk
parent aba1f649ad
commit 55e6b25035
2 changed files with 148 additions and 210 deletions

View File

@ -0,0 +1,2 @@
#4663 Refactor bank keeper by removing private functions
- `InputOutputCoins`, `SetCoins`, `SubtractCoins` and `AddCoins` are now part of the `SendKeeper` instead of the `Keeper` interface

View File

@ -19,11 +19,6 @@ var _ Keeper = (*BaseKeeper)(nil)
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.Error)
AddCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) (sdk.Coins, sdk.Error)
InputOutputCoins(ctx sdk.Context, inputs []types.Input, outputs []types.Output) sdk.Error
DelegateCoins(ctx sdk.Context, delegatorAddr, moduleAccAddr sdk.AccAddress, amt sdk.Coins) sdk.Error
UndelegateCoins(ctx sdk.Context, moduleAccAddr, delegatorAddr sdk.AccAddress, amt sdk.Coins) sdk.Error
}
@ -49,47 +44,19 @@ func NewBaseKeeper(ak types.AccountKeeper,
}
}
// SetCoins sets the coins at the addr.
func (keeper BaseKeeper) SetCoins(
ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins,
) sdk.Error {
return setCoins(ctx, keeper.ak, addr, amt)
}
// SubtractCoins subtracts amt from the coins at the addr.
func (keeper BaseKeeper) SubtractCoins(
ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins,
) (sdk.Coins, sdk.Error) {
return subtractCoins(ctx, keeper.ak, addr, amt)
}
// AddCoins adds amt to the coins at the addr.
func (keeper BaseKeeper) AddCoins(
ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins,
) (sdk.Coins, sdk.Error) {
return addCoins(ctx, keeper.ak, addr, amt)
}
// InputOutputCoins handles a list of inputs and outputs
func (keeper BaseKeeper) InputOutputCoins(ctx sdk.Context, inputs []types.Input, outputs []types.Output) sdk.Error {
return inputOutputCoins(ctx, keeper.ak, inputs, outputs)
}
// DelegateCoins performs delegation by deducting amt coins from an account with
// address addr. For vesting accounts, delegations amounts are tracked for both
// vesting and vested coins.
// The coins are then transferred from the delegator address to a ModuleAccount address.
// If any of the delegation amounts are negative, an error is returned.
func (keeper BaseKeeper) DelegateCoins(
ctx sdk.Context, delegatorAddr, moduleAccAddr sdk.AccAddress, amt sdk.Coins,
) sdk.Error {
func (keeper BaseKeeper) DelegateCoins(ctx sdk.Context, delegatorAddr, moduleAccAddr sdk.AccAddress, amt sdk.Coins) sdk.Error {
delegatorAcc := getAccount(ctx, keeper.ak, delegatorAddr)
delegatorAcc := keeper.ak.GetAccount(ctx, delegatorAddr)
if delegatorAcc == nil {
return sdk.ErrUnknownAddress(fmt.Sprintf("account %s does not exist", delegatorAddr))
}
moduleAcc := getAccount(ctx, keeper.ak, moduleAccAddr)
moduleAcc := keeper.ak.GetAccount(ctx, moduleAccAddr)
if moduleAcc == nil {
return sdk.ErrUnknownAddress(fmt.Sprintf("module account %s does not exist", moduleAccAddr))
}
@ -107,14 +74,13 @@ func (keeper BaseKeeper) DelegateCoins(
)
}
// TODO: return error on account.TrackDelegation
if err := trackDelegation(delegatorAcc, ctx.BlockHeader().Time, amt); err != nil {
return sdk.ErrInternal(fmt.Sprintf("failed to track delegation: %v", err))
}
setAccount(ctx, keeper.ak, delegatorAcc)
keeper.ak.SetAccount(ctx, delegatorAcc)
_, err := addCoins(ctx, keeper.ak, moduleAccAddr, amt)
_, err := keeper.AddCoins(ctx, moduleAccAddr, amt)
if err != nil {
return err
}
@ -127,16 +93,14 @@ func (keeper BaseKeeper) DelegateCoins(
// vesting and vested coins.
// The coins are then transferred from a ModuleAccount address to the delegator address.
// If any of the undelegation amounts are negative, an error is returned.
func (keeper BaseKeeper) UndelegateCoins(
ctx sdk.Context, moduleAccAddr, delegatorAddr sdk.AccAddress, amt sdk.Coins,
) sdk.Error {
func (keeper BaseKeeper) UndelegateCoins(ctx sdk.Context, moduleAccAddr, delegatorAddr sdk.AccAddress, amt sdk.Coins) sdk.Error {
delegatorAcc := getAccount(ctx, keeper.ak, delegatorAddr)
delegatorAcc := keeper.ak.GetAccount(ctx, delegatorAddr)
if delegatorAcc == nil {
return sdk.ErrUnknownAddress(fmt.Sprintf("account %s does not exist", delegatorAddr))
}
moduleAcc := getAccount(ctx, keeper.ak, moduleAccAddr)
moduleAcc := keeper.ak.GetAccount(ctx, moduleAccAddr)
if moduleAcc == nil {
return sdk.ErrUnknownAddress(fmt.Sprintf("module account %s does not exist", moduleAccAddr))
}
@ -154,18 +118,16 @@ func (keeper BaseKeeper) UndelegateCoins(
)
}
err := setCoins(ctx, keeper.ak, moduleAccAddr, newCoins)
err := keeper.SetCoins(ctx, moduleAccAddr, newCoins)
if err != nil {
return err
}
// TODO: return error on account.TrackUndelegation
if err := trackUndelegation(delegatorAcc, amt); err != nil {
return sdk.ErrInternal(fmt.Sprintf("failed to track undelegation: %v", err))
}
setAccount(ctx, keeper.ak, delegatorAcc)
keeper.ak.SetAccount(ctx, delegatorAcc)
return nil
}
@ -174,8 +136,13 @@ func (keeper BaseKeeper) UndelegateCoins(
type SendKeeper interface {
ViewKeeper
InputOutputCoins(ctx sdk.Context, inputs []types.Input, outputs []types.Output) sdk.Error
SendCoins(ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) sdk.Error
SubtractCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) (sdk.Coins, sdk.Error)
AddCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) (sdk.Coins, sdk.Error)
SetCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) sdk.Error
GetSendEnabled(ctx sdk.Context) bool
SetSendEnabled(ctx sdk.Context, enabled bool)
}
@ -202,14 +169,56 @@ func NewBaseSendKeeper(ak types.AccountKeeper,
}
}
// TODO combine with sendCoins
// SendCoins moves coins from one account to another
func (keeper BaseSendKeeper) SendCoins(
ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins,
) sdk.Error {
// InputOutputCoins handles a list of inputs and outputs
func (keeper BaseSendKeeper) InputOutputCoins(ctx sdk.Context, inputs []types.Input, outputs []types.Output) sdk.Error {
// Safety check ensuring that when sending coins the keeper must maintain the
// Check supply invariant and validity of Coins.
if err := types.ValidateInputsOutputs(inputs, outputs); err != nil {
return err
}
if !amt.IsValid() {
return sdk.ErrInvalidCoins(amt.String())
for _, in := range inputs {
_, err := keeper.SubtractCoins(ctx, in.Address, in.Coins)
if err != nil {
return err
}
ctx.EventManager().EmitEvent(
sdk.NewEvent(
sdk.EventTypeMessage,
sdk.NewAttribute(types.AttributeKeySender, in.Address.String()),
),
)
}
for _, out := range outputs {
_, err := keeper.AddCoins(ctx, out.Address, out.Coins)
if err != nil {
return err
}
ctx.EventManager().EmitEvent(
sdk.NewEvent(
types.EventTypeTransfer,
sdk.NewAttribute(types.AttributeKeyRecipient, out.Address.String()),
),
)
}
return nil
}
// SendCoins moves coins from one account to another
func (keeper BaseSendKeeper) SendCoins(ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) sdk.Error {
_, err := keeper.SubtractCoins(ctx, fromAddr, amt)
if err != nil {
return err
}
_, err = keeper.AddCoins(ctx, toAddr, amt)
if err != nil {
return err
}
ctx.EventManager().EmitEvents(sdk.Events{
@ -223,7 +232,80 @@ func (keeper BaseSendKeeper) SendCoins(
),
})
return sendCoins(ctx, keeper.ak, fromAddr, toAddr, amt)
return nil
}
// SubtractCoins subtracts amt from the coins at the addr.
//
// CONTRACT: If the account is a vesting account, the amount has to be spendable.
func (keeper BaseSendKeeper) SubtractCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) (sdk.Coins, sdk.Error) {
if !amt.IsValid() {
return nil, sdk.ErrInvalidCoins(amt.String())
}
oldCoins, spendableCoins := sdk.NewCoins(), sdk.NewCoins()
acc := keeper.ak.GetAccount(ctx, addr)
if acc != nil {
oldCoins = acc.GetCoins()
spendableCoins = acc.SpendableCoins(ctx.BlockHeader().Time)
}
// For non-vesting accounts, spendable coins will simply be the original coins.
// So the check here is sufficient instead of subtracting from oldCoins.
_, hasNeg := spendableCoins.SafeSub(amt)
if hasNeg {
return amt, sdk.ErrInsufficientCoins(
fmt.Sprintf("insufficient account funds; %s < %s", spendableCoins, amt),
)
}
newCoins := oldCoins.Sub(amt) // should not panic as spendable coins was already checked
err := keeper.SetCoins(ctx, addr, newCoins)
return newCoins, err
}
// AddCoins adds amt to the coins at the addr.
func (keeper BaseSendKeeper) AddCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) (sdk.Coins, sdk.Error) {
if !amt.IsValid() {
return nil, sdk.ErrInvalidCoins(amt.String())
}
oldCoins := keeper.GetCoins(ctx, addr)
newCoins := oldCoins.Add(amt)
if newCoins.IsAnyNegative() {
return amt, sdk.ErrInsufficientCoins(
fmt.Sprintf("insufficient account funds; %s < %s", oldCoins, amt),
)
}
err := keeper.SetCoins(ctx, addr, newCoins)
return newCoins, err
}
// SetCoins sets the coins at the addr.
func (keeper BaseSendKeeper) SetCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) sdk.Error {
if !amt.IsValid() {
return sdk.ErrInvalidCoins(amt.String())
}
acc := keeper.ak.GetAccount(ctx, addr)
if acc == nil {
acc = keeper.ak.NewAccountWithAddress(ctx, addr)
}
err := acc.SetCoins(amt)
if err != nil {
panic(err)
}
keeper.ak.SetAccount(ctx, acc)
return nil
}
// GetSendEnabled returns the current SendEnabled
@ -268,12 +350,16 @@ func (keeper BaseViewKeeper) Logger(ctx sdk.Context) log.Logger {
// GetCoins returns the coins at the addr.
func (keeper BaseViewKeeper) GetCoins(ctx sdk.Context, addr sdk.AccAddress) sdk.Coins {
return getCoins(ctx, keeper.ak, addr)
acc := keeper.ak.GetAccount(ctx, addr)
if acc == nil {
return sdk.NewCoins()
}
return acc.GetCoins()
}
// HasCoins returns whether or not an account has at least amt coins.
func (keeper BaseViewKeeper) HasCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) bool {
return hasCoins(ctx, keeper.ak, addr, amt)
return keeper.GetCoins(ctx, addr).IsAllGTE(amt)
}
// Codespace returns the keeper's codespace.
@ -281,162 +367,11 @@ func (keeper BaseViewKeeper) Codespace() sdk.CodespaceType {
return keeper.codespace
}
// getCoins returns the account coins
func getCoins(ctx sdk.Context, am types.AccountKeeper, addr sdk.AccAddress) sdk.Coins {
acc := am.GetAccount(ctx, addr)
if acc == nil {
return sdk.NewCoins()
}
return acc.GetCoins()
}
func setCoins(ctx sdk.Context, ak types.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins) sdk.Error {
if !amt.IsValid() {
return sdk.ErrInvalidCoins(amt.String())
}
acc := ak.GetAccount(ctx, addr)
if acc == nil {
acc = ak.NewAccountWithAddress(ctx, addr)
}
err := acc.SetCoins(amt)
if err != nil {
// Handle w/ #870
panic(err)
}
ak.SetAccount(ctx, acc)
return nil
}
// HasCoins returns whether or not an account has at least amt coins.
func hasCoins(ctx sdk.Context, ak types.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins) bool {
return getCoins(ctx, ak, addr).IsAllGTE(amt)
}
// getAccount implements AccountKeeper
func getAccount(ctx sdk.Context, ak types.AccountKeeper, addr sdk.AccAddress) exported.Account {
return ak.GetAccount(ctx, addr)
}
// setAccount implements AccountKeeper
func setAccount(ctx sdk.Context, ak types.AccountKeeper, acc exported.Account) {
ak.SetAccount(ctx, acc)
}
// subtractCoins subtracts amt coins from an account with the given address addr.
//
// CONTRACT: If the account is a vesting account, the amount has to be spendable.
func subtractCoins(ctx sdk.Context, ak types.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins) (sdk.Coins, sdk.Error) {
if !amt.IsValid() {
return nil, sdk.ErrInvalidCoins(amt.String())
}
oldCoins, spendableCoins := sdk.NewCoins(), sdk.NewCoins()
acc := getAccount(ctx, ak, addr)
if acc != nil {
oldCoins = acc.GetCoins()
spendableCoins = acc.SpendableCoins(ctx.BlockHeader().Time)
}
// For non-vesting accounts, spendable coins will simply be the original coins.
// So the check here is sufficient instead of subtracting from oldCoins.
_, hasNeg := spendableCoins.SafeSub(amt)
if hasNeg {
return amt, sdk.ErrInsufficientCoins(
fmt.Sprintf("insufficient account funds; %s < %s", spendableCoins, amt),
)
}
newCoins := oldCoins.Sub(amt) // should not panic as spendable coins was already checked
err := setCoins(ctx, ak, addr, newCoins)
return newCoins, err
}
// AddCoins adds amt to the coins at the addr.
func addCoins(ctx sdk.Context, ak types.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins) (sdk.Coins, sdk.Error) {
if !amt.IsValid() {
return nil, sdk.ErrInvalidCoins(amt.String())
}
oldCoins := getCoins(ctx, ak, addr)
newCoins := oldCoins.Add(amt)
if newCoins.IsAnyNegative() {
return amt, sdk.ErrInsufficientCoins(
fmt.Sprintf("insufficient account funds; %s < %s", oldCoins, amt),
)
}
err := setCoins(ctx, ak, addr, newCoins)
return newCoins, err
}
// SendCoins moves coins from one account to another
// Returns ErrInvalidCoins if amt is invalid.
func sendCoins(ctx sdk.Context, ak types.AccountKeeper, fromAddr, toAddr sdk.AccAddress, amt sdk.Coins) sdk.Error {
_, err := subtractCoins(ctx, ak, fromAddr, amt)
if err != nil {
return err
}
_, err = addCoins(ctx, ak, toAddr, amt)
if err != nil {
return err
}
return nil
}
// InputOutputCoins handles a list of inputs and outputs
// NOTE: Make sure to revert state changes from tx on error
func inputOutputCoins(ctx sdk.Context, ak types.AccountKeeper, inputs []types.Input, outputs []types.Output) sdk.Error {
// Safety check ensuring that when sending coins the keeper must maintain the
// Check supply invariant and validity of Coins.
if err := types.ValidateInputsOutputs(inputs, outputs); err != nil {
return err
}
for _, in := range inputs {
_, err := subtractCoins(ctx, ak, in.Address, in.Coins)
if err != nil {
return err
}
ctx.EventManager().EmitEvent(
sdk.NewEvent(
sdk.EventTypeMessage,
sdk.NewAttribute(types.AttributeKeySender, in.Address.String()),
),
)
}
for _, out := range outputs {
_, err := addCoins(ctx, ak, out.Address, out.Coins)
if err != nil {
return err
}
ctx.EventManager().EmitEvent(
sdk.NewEvent(
types.EventTypeTransfer,
sdk.NewAttribute(types.AttributeKeyRecipient, out.Address.String()),
),
)
}
return nil
}
// CONTRACT: assumes that amt is valid.
func trackDelegation(acc exported.Account, blockTime time.Time, amt sdk.Coins) error {
vacc, ok := acc.(exported.VestingAccount)
if ok {
// TODO: return error on account.TrackDelegation
vacc.TrackDelegation(blockTime, amt)
return nil
}
@ -448,6 +383,7 @@ func trackDelegation(acc exported.Account, blockTime time.Time, amt sdk.Coins) e
func trackUndelegation(acc exported.Account, amt sdk.Coins) error {
vacc, ok := acc.(exported.VestingAccount)
if ok {
// TODO: return error on account.TrackUndelegation
vacc.TrackUndelegation(amt)
return nil
}