Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: audit x/bank package changes in 0.47 release #14076

Merged
merged 24 commits into from
Dec 9, 2022
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
02323aa
updates
alexanderbez Nov 29, 2022
e34cd18
updates
alexanderbez Nov 29, 2022
4b558dd
updates
alexanderbez Nov 29, 2022
80d7184
updates
alexanderbez Nov 29, 2022
908407f
updates
alexanderbez Nov 29, 2022
da189b7
updates
alexanderbez Nov 29, 2022
6ad381e
updates
alexanderbez Nov 29, 2022
10c1d4a
Merge branch 'main' into bez/13905-x-bank-audit
alexanderbez Nov 29, 2022
bab60ac
Merge branch 'main' into bez/13905-x-bank-audit
alexanderbez Nov 30, 2022
d85f610
Merge branch 'main' into bez/13905-x-bank-audit
alexanderbez Dec 1, 2022
b8b1e2e
Merge branch 'main' into bez/13905-x-bank-audit
alexanderbez Dec 2, 2022
d14df3a
Merge branch 'main' into bez/13905-x-bank-audit
alexanderbez Dec 5, 2022
f80bfd1
Merge branch 'main' into bez/13905-x-bank-audit
alexanderbez Dec 6, 2022
f66a315
Merge branch 'main' into bez/13905-x-bank-audit
alexanderbez Dec 6, 2022
a5e317d
Merge branch 'main' into bez/13905-x-bank-audit
alexanderbez Dec 6, 2022
933850f
Merge branch 'main' into bez/13905-x-bank-audit
alexanderbez Dec 6, 2022
5a33d70
Merge branch 'main' into bez/13905-x-bank-audit
alexanderbez Dec 7, 2022
4d89488
Merge branch 'main' into bez/13905-x-bank-audit
alexanderbez Dec 8, 2022
d9d03bd
updates
alexanderbez Dec 8, 2022
904d821
updates
alexanderbez Dec 8, 2022
b22ef97
Merge branch 'main' into bez/13905-x-bank-audit
alexanderbez Dec 8, 2022
f987789
Merge branch 'main' into bez/13905-x-bank-audit
alexanderbez Dec 8, 2022
69f96c2
Merge branch 'main' into bez/13905-x-bank-audit
alexanderbez Dec 8, 2022
608cfc6
Merge branch 'main' into bez/13905-x-bank-audit
alexanderbez Dec 9, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions x/bank/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"context"

"cosmossdk.io/math"

gogotypes "github.com/cosmos/gogoproto/types"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"

Expand Down Expand Up @@ -262,12 +262,16 @@ func (k BaseKeeper) SendEnabled(goCtx context.Context, req *types.QuerySendEnabl
} else {
store := k.getSendEnabledPrefixStore(ctx)
var err error

resp.Pagination, err = query.FilteredPaginate(
store,
req.Pagination,
func(key []byte, value []byte, accumulate bool) (bool, error) {
if accumulate {
resp.SendEnabled = append(resp.SendEnabled, types.NewSendEnabled(string(key), types.IsTrueB(value)))
var enabled gogotypes.BoolValue
k.cdc.MustUnmarshal(value, &enabled)

resp.SendEnabled = append(resp.SendEnabled, types.NewSendEnabled(string(key), enabled.Value))
}
return true, nil
},
Expand All @@ -276,5 +280,6 @@ func (k BaseKeeper) SendEnabled(goCtx context.Context, req *types.QuerySendEnabl
return nil, status.Error(codes.Internal, err.Error())
}
}

return resp, nil
}
100 changes: 50 additions & 50 deletions x/bank/keeper/send.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package keeper
import (
"fmt"

gogotypes "github.com/cosmos/gogoproto/types"

"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/store/prefix"
storetypes "github.com/cosmos/cosmos-sdk/store/types"
Expand Down Expand Up @@ -100,12 +102,16 @@ func (k BaseSendKeeper) GetParams(ctx sdk.Context) (params types.Params) {

// SetParams sets the total set of bank parameters.
//
//nolint:staticcheck // params.SendEnabled is deprecated but it should be here regardless.
// Note: params.SendEnabled is deprecated but it should be here regardless.
//
//nolint:staticcheck
func (k BaseSendKeeper) SetParams(ctx sdk.Context, params types.Params) error {
// normally SendEnabled is deprecated but we still support it for backwards compatibility
// using params.Validate() would fail due to the SendEnabled deprecation
// Normally SendEnabled is deprecated but we still support it for backwards
// compatibility. Using params.Validate() would fail due to the SendEnabled
// deprecation.
if len(params.SendEnabled) > 0 {
k.SetAllSendEnabled(ctx, params.SendEnabled)

// override params without SendEnabled
params = types.NewParams(params.DefaultSendEnabled)
}
Expand All @@ -115,6 +121,7 @@ func (k BaseSendKeeper) SetParams(ctx sdk.Context, params types.Params) error {
if err != nil {
return err
}

store.Set(types.ParamsKey, bz)
return nil
}
Expand All @@ -139,21 +146,15 @@ func (k BaseSendKeeper) InputOutputCoins(ctx sdk.Context, inputs []types.Input,
if err != nil {
return err
}
ctx.EventManager().EmitEvent(
alexanderbez marked this conversation as resolved.
Show resolved Hide resolved
sdk.NewEvent(
sdk.EventTypeMessage,
sdk.NewAttribute(types.AttributeKeySender, in.Address),
),
)
}

for _, out := range outputs {
outAddress, err := sdk.AccAddressFromBech32(out.Address)
if err != nil {
return err
}
err = k.addCoins(ctx, outAddress, out.Coins)
if err != nil {

if err := k.addCoins(ctx, outAddress, out.Coins); err != nil {
return err
}

Expand Down Expand Up @@ -211,10 +212,6 @@ func (k BaseSendKeeper) SendCoins(ctx sdk.Context, fromAddr sdk.AccAddress, toAd
sdk.NewAttribute(types.AttributeKeySender, fromAddrString),
sdk.NewAttribute(sdk.AttributeKeyAmount, amt.String()),
),
sdk.NewEvent(
sdk.EventTypeMessage,
sdk.NewAttribute(types.AttributeKeySender, fromAddrString),
),
})

return nil
Expand Down Expand Up @@ -255,15 +252,15 @@ func (k BaseSendKeeper) subUnlockedCoins(ctx sdk.Context, addr sdk.AccAddress, a
}
}

// emit coin spent event
ctx.EventManager().EmitEvent(
types.NewCoinSpentEvent(addr, amt),
)

return nil
}

// addCoins increase the addr balance by the given amt. Fails if the provided amt is invalid.
// It emits a coin received event.
// addCoins increase the addr balance by the given amt. Fails if the provided
// amt is invalid. It emits a coin received event.
func (k BaseSendKeeper) addCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) error {
if !amt.IsValid() {
return sdkerrors.Wrap(sdkerrors.ErrInvalidCoins, amt.String())
Expand Down Expand Up @@ -343,6 +340,7 @@ func (k BaseSendKeeper) setBalance(ctx sdk.Context, addr sdk.AccAddress, balance
if err != nil {
return err
}

accountStore.Set([]byte(balance.Denom), amount)

// Store a reverse index from denomination to account address with a
Expand All @@ -356,28 +354,23 @@ func (k BaseSendKeeper) setBalance(ctx sdk.Context, addr sdk.AccAddress, balance
return nil
}

// IsSendEnabledCoins checks the coins provide and returns an ErrSendDisabled if
// any of the coins are not configured for sending. Returns nil if sending is enabled
// for all provided coin
// IsSendEnabledCoins checks the coins provided and returns an ErrSendDisabled
// if any of the coins are not configured for sending. Returns nil if sending is
// enabled for all provided coins.
func (k BaseSendKeeper) IsSendEnabledCoins(ctx sdk.Context, coins ...sdk.Coin) error {
if len(coins) == 0 {
return nil
}

store := ctx.KVStore(k.storeKey)
haveDefault := false
var defaultVal bool
getDefault := func() bool {
if !haveDefault {
defaultVal = k.GetParams(ctx).DefaultSendEnabled
haveDefault = true
}
return defaultVal
}
defaultVal := k.GetParams(ctx).DefaultSendEnabled

for _, coin := range coins {
if !k.getSendEnabledOrDefault(store, coin.Denom, getDefault) {
if !k.getSendEnabledOrDefault(store, coin.Denom, defaultVal) {
return types.ErrSendDisabled.Wrapf("%s transfers are currently disabled", coin.Denom)
}
}

return nil
}

Expand All @@ -399,7 +392,7 @@ func (k BaseSendKeeper) GetBlockedAddresses() map[string]bool {

// IsSendEnabledDenom returns the current SendEnabled status of the provided denom.
func (k BaseSendKeeper) IsSendEnabledDenom(ctx sdk.Context, denom string) bool {
return k.getSendEnabledOrDefault(ctx.KVStore(k.storeKey), denom, func() bool { return k.GetParams(ctx).DefaultSendEnabled })
return k.getSendEnabledOrDefault(ctx.KVStore(k.storeKey), denom, k.GetParams(ctx).DefaultSendEnabled)
}

// GetSendEnabledEntry gets a SendEnabled entry for the given denom.
Expand All @@ -409,6 +402,7 @@ func (k BaseSendKeeper) GetSendEnabledEntry(ctx sdk.Context, denom string) (type
if !found {
return types.SendEnabled{}, false
}

return types.SendEnabled{Denom: denom, Enabled: sendEnabled}, true
}

Expand All @@ -429,8 +423,9 @@ func (k BaseSendKeeper) SetAllSendEnabled(ctx sdk.Context, sendEnableds []*types
// setSendEnabledEntry sets SendEnabled for the given denom to the give value in the provided store.
func (k BaseSendKeeper) setSendEnabledEntry(store sdk.KVStore, denom string, value bool) {
key := types.CreateSendEnabledKey(denom)
val := types.ToBoolB(value)
store.Set(key, []byte{val})

bz := k.cdc.MustMarshal(&gogotypes.BoolValue{Value: value})
store.Set(key, bz)
}

// DeleteSendEnabled deletes the SendEnabled flags for one or more denoms.
Expand All @@ -456,8 +451,11 @@ func (k BaseSendKeeper) IterateSendEnabledEntries(ctx sdk.Context, cb func(denom

for ; iterator.Valid(); iterator.Next() {
denom := string(iterator.Key())
val := types.IsTrueB(iterator.Value())
if cb(denom, val) {

var enabled gogotypes.BoolValue
k.cdc.MustUnmarshal(iterator.Value(), &enabled)

if cb(denom, enabled.Value) {
break
}
}
Expand All @@ -471,10 +469,12 @@ func (k BaseSendKeeper) GetAllSendEnabledEntries(ctx sdk.Context) []types.SendEn
rv = append(rv, types.SendEnabled{Denom: denom, Enabled: sendEnabled})
return false
})

return rv
}

// getSendEnabled returns whether send is enabled and whether that flag was set for a denom.
// getSendEnabled returns whether send is enabled and whether that flag was set
// for a denom.
//
// Example usage:
//
Expand All @@ -488,25 +488,25 @@ func (k BaseSendKeeper) getSendEnabled(store sdk.KVStore, denom string) (bool, b
if !store.Has(key) {
return false, false
}
v := store.Get(key)
if len(v) != 1 {
return false, false
}
switch v[0] {
case types.TrueB:
return true, true
case types.FalseB:
return false, true
default:

bz := store.Get(key)
if len(bz) == 0 {
return false, false
}

var enabled gogotypes.BoolValue
k.cdc.MustUnmarshal(bz, &enabled)

return enabled.Value, true
}

// getSendEnabledOrDefault gets the send_enabled value for a denom. If it's not in the store, this will return the result of the getDefault function.
func (k BaseSendKeeper) getSendEnabledOrDefault(store sdk.KVStore, denom string, getDefault func() bool) bool {
// getSendEnabledOrDefault gets the SendEnabled value for a denom. If it's not
// in the store, this will return defaultVal.
func (k BaseSendKeeper) getSendEnabledOrDefault(store sdk.KVStore, denom string, defaultVal bool) bool {
sendEnabled, found := k.getSendEnabled(store, denom)
if found {
return sendEnabled
}
return getDefault()

return defaultVal
}
7 changes: 3 additions & 4 deletions x/bank/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,14 @@ import (
"fmt"
"time"

modulev1 "cosmossdk.io/api/cosmos/bank/module/v1"
"cosmossdk.io/core/appmodule"
"cosmossdk.io/depinject"
gwruntime "github.com/grpc-ecosystem/grpc-gateway/runtime"
"github.com/spf13/cobra"
abci "github.com/tendermint/tendermint/abci/types"
"golang.org/x/exp/maps"

modulev1 "cosmossdk.io/api/cosmos/bank/module/v1"
"cosmossdk.io/core/appmodule"
"cosmossdk.io/depinject"

"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/codec"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
Expand Down
19 changes: 13 additions & 6 deletions x/bank/types/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,31 +107,38 @@ func GetGenesisStateFromAppState(cdc codec.JSONCodec, appState map[string]json.R
return &genesisState
}

// MigrateSendEnabled moves the SendEnabled info from Params into the GenesisState.SendEnabled field and removes them from Params.
// If the Params.SendEnabled slice is empty, this is a noop.
// If the main SendEnabled slice already has entries, the Params.SendEnabled entries are added.
// In case of the same demon in both, preference is given to the existing (main GenesisState field) entry.
// MigrateSendEnabled moves the SendEnabled info from Params into the
// GenesisState.SendEnabled field and removes them from Params. If the
// Params.SendEnabled slice is empty, this is a noop.
//
// If the main SendEnabled slice already has entries, the Params.SendEnabled
// entries are added. In case of the same demon in both, preference is given to
// the existing (main GenesisState field) entry.
func (g *GenesisState) MigrateSendEnabled() {
g.SendEnabled = g.GetAllSendEnabled()
g.Params.SendEnabled = nil
}

// GetAllSendEnabled returns all the SendEnabled entries from both the SendEnabled field and the Params.
// If a denom has an entry in both, the entry in the SendEnabled field takes precedence over one in Params.
// GetAllSendEnabled returns all the SendEnabled entries from both the SendEnabled
// field and the Params. If a denom has an entry in both, the entry in the
// SendEnabled field takes precedence over one in Params.
func (g GenesisState) GetAllSendEnabled() []SendEnabled {
if len(g.Params.SendEnabled) == 0 {
return g.SendEnabled
}

rv := make([]SendEnabled, len(g.SendEnabled))
knownSendEnabled := map[string]bool{}
for i, se := range g.SendEnabled {
rv[i] = se
knownSendEnabled[se.Denom] = true
}

for _, se := range g.Params.SendEnabled {
if _, known := knownSendEnabled[se.Denom]; !known {
rv = append(rv, *se)
}
}

return rv
}
20 changes: 0 additions & 20 deletions x/bank/types/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,6 @@ var (
ParamsKey = []byte{0x05}
)

const (
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved
// TrueB is a byte with value 1 that represents true.
TrueB = byte(0x01)
// FalseB is a byte with value 0 that represents false.
FalseB = byte(0x00)
)

// AddressAndDenomFromBalancesStore returns an account address and denom from a balances prefix
// store. The key must not contain the prefix BalancesPrefix as the prefix store
// iterator discards the actual prefix.
Expand Down Expand Up @@ -94,16 +87,3 @@ func CreateSendEnabledKey(denom string) []byte {
copy(key[len(SendEnabledPrefix):], denom)
return key
}

// IsTrueB returns true if the provided byte slice has exactly one byte, and it is equal to TrueB.
alexanderbez marked this conversation as resolved.
Show resolved Hide resolved
func IsTrueB(bz []byte) bool {
return len(bz) == 1 && bz[0] == TrueB
}

// ToBoolB returns TrueB if v is true, and FalseB if it's false.
func ToBoolB(v bool) byte {
if v {
return TrueB
}
return FalseB
}
Loading