Skip to content

Commit

Permalink
release-pio/v0.46.xLlama incorporate pr feedback (#356)
Browse files Browse the repository at this point in the history
* PR feedback for #270"

* fix assertion

* fix broken unit test
  • Loading branch information
llama-del-rey authored Nov 15, 2022
1 parent 7462a84 commit f0cfe13
Show file tree
Hide file tree
Showing 10 changed files with 117 additions and 43 deletions.
1 change: 1 addition & 0 deletions proto/cosmos/bank/v1beta1/tx.proto
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ message MsgMultiSend {
// MsgMultiSendResponse defines the Msg/MultiSend response type.
message MsgMultiSendResponse {}

// MsgUpdateDenomMetadata represents a message to update denom metadata
message MsgUpdateDenomMetadata {
option (cosmos.msg.v1.signer) = "from_address";

Expand Down
1 change: 0 additions & 1 deletion simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,6 @@ func NewSimApp(
appCodec, keys[authtypes.StoreKey], app.GetSubspace(authtypes.ModuleName), authtypes.ProtoBaseAccount, maccPerms, sdk.Bech32MainPrefix,
)

// set the governance module account as the authority
app.BankKeeper = bankkeeper.NewBaseKeeper(
appCodec, keys[banktypes.StoreKey], app.AccountKeeper, app.GetSubspace(banktypes.ModuleName), app.ModuleAccountAddrs(),
)
Expand Down
9 changes: 5 additions & 4 deletions x/bank/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package keeper
import (
"fmt"

govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
"cosmossdk.io/math"
"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/internal/conv"
Expand All @@ -19,6 +18,8 @@ import (

var _ Keeper = (*BaseKeeper)(nil)

const govModuleName = "gov"

// Keeper defines a module interface that facilitates the transfer of coins
// between accounts.
type Keeper interface {
Expand Down Expand Up @@ -62,7 +63,7 @@ type BaseKeeper struct {
storeKey storetypes.StoreKey
paramSpace paramtypes.Subspace
mintCoinsRestrictionFn MintingRestrictionFn
authority string
authority string
}

type MintingRestrictionFn func(ctx sdk.Context, coins sdk.Coins) error
Expand Down Expand Up @@ -117,7 +118,7 @@ func NewBaseKeeper(
storeKey: storeKey,
paramSpace: paramSpace,
mintCoinsRestrictionFn: func(ctx sdk.Context, coins sdk.Coins) error { return nil },
authority: authtypes.NewModuleAddress(govtypes.ModuleName).String(), // hardcoded till all transitive dependencies can be updated to pass in the constructor instead
authority: authtypes.NewModuleAddress(govModuleName).String(), // hardcoded till all transitive dependencies can be updated to pass in the constructor instead
}
}

Expand Down Expand Up @@ -558,4 +559,4 @@ func (k BaseViewKeeper) IterateTotalSupply(ctx sdk.Context, cb func(sdk.Coin) bo

func (k BaseKeeper) GetAuthority() string {
return k.authority
}
}
2 changes: 2 additions & 0 deletions x/bank/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ package keeper

import (
"context"

"github.com/armon/go-metrics"

"cosmossdk.io/errors"
"github.com/cosmos/cosmos-sdk/telemetry"
sdk "github.com/cosmos/cosmos-sdk/types"
Expand Down
74 changes: 74 additions & 0 deletions x/bank/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
package keeper_test

import (
"github.com/cosmos/cosmos-sdk/simapp"
"github.com/cosmos/cosmos-sdk/x/bank/keeper"
"github.com/cosmos/cosmos-sdk/x/bank/types"
"github.com/stretchr/testify/require"
tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
"testing"
)

func TestUpdateDenomMetadata(t *testing.T) {
app := simapp.Setup(t, false)
ctx := app.BaseApp.NewContext(false, tmproto.Header{})
msgServer := keeper.NewMsgServerImpl(app.BankKeeper)
testCases := []struct {
Name string
ExpectErr bool
ExpectedErr string
req types.MsgUpdateDenomMetadata
}{
{
Name: "fail - wrong authority",
ExpectErr: true,
ExpectedErr: "expected cosmos10d07y265gmmuvt4z0w9aw880jnsr700j6zn9kn got nongovaccount: expected gov account as only signer for proposal message",
req: types.MsgUpdateDenomMetadata{
FromAddress: "nongovaccount",
Title: "title",
Description: "description",
Metadata: &types.Metadata{
Name: "diamondback",
Symbol: "DB",
Description: "The native staking token",
DenomUnits: []*types.DenomUnit{
{"udiamondback", uint32(0), []string{"microdiamondback"}},
},
},
},
},
{
Name: "success - correct authority",
ExpectErr: false,
ExpectedErr: "",
req: types.MsgUpdateDenomMetadata{
FromAddress: "cosmos10d07y265gmmuvt4z0w9aw880jnsr700j6zn9kn",
Title: "title",
Description: "description",
Metadata: &types.Metadata{
Base: "diamondback",
Name: "diamondback",
Symbol: "DB",
Description: "The native staking token",
DenomUnits: []*types.DenomUnit{
{"udiamondback", uint32(0), []string{"microdiamondback"}},
},
},
},
},
}

for _, testCase := range testCases {
t.Run(testCase.Name, func(t *testing.T) {
_, err := msgServer.UpdateDenomMetadata(ctx, &testCase.req)
if testCase.ExpectErr {
require.Error(t, err)
require.Equal(t, testCase.ExpectedErr, err.Error())
} else {
require.NoError(t, err)
metadata, _ := app.BankKeeper.GetDenomMetaData(ctx, "diamondback")
require.Equal(t, testCase.req.Metadata.String(), metadata.String())
}
})
}
}
13 changes: 7 additions & 6 deletions x/bank/keeper/send.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ func (k BaseSendKeeper) subUnlockedCoins(ctx sdk.Context, addr sdk.AccAddress, a

_, hasNeg := sdk.Coins{spendable}.SafeSub(coin)
if hasNeg {
return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, "%s is smaller than %s", spendable, balance)
return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, "%s is smaller than %s", spendable, coin)
}

newBalance := balance.Sub(coin)
Expand Down Expand Up @@ -425,11 +425,12 @@ func (k BaseSendKeeper) GetAllSendEnabledEntries(ctx sdk.Context) []types.SendEn
// getSendEnabled returns whether send is enabled and whether that flag was set for a denom.
//
// Example usage:
// store := ctx.KVStore(k.storeKey)
// sendEnabled, found := getSendEnabled(store, "atom")
// if !found {
// sendEnabled = DefaultSendEnabled
// }
//
// store := ctx.KVStore(k.storeKey)
// sendEnabled, found := getSendEnabled(store, "atom")
// if !found {
// sendEnabled = DefaultSendEnabled
// }
func (k BaseSendKeeper) getSendEnabled(store sdk.KVStore, denom string) (bool, bool) {
key := types.CreateSendEnabledKey(denom)
if !store.Has(key) {
Expand Down
6 changes: 1 addition & 5 deletions x/bank/types/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,11 +210,7 @@ func (msg MsgUpdateDenomMetadata) Type() string {

// ValidateBasic Implements Msg.
func (msg MsgUpdateDenomMetadata) ValidateBasic() error {
err := msg.Metadata.Validate()
if err != nil {
return err
}
return nil
return msg.Metadata.Validate()
}

// GetSignBytes Implements Msg.
Expand Down
12 changes: 6 additions & 6 deletions x/bank/types/msgs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -283,22 +283,22 @@ func TestUpdateDenomMetadataGetSignBytes(t *testing.T) {
Title: "title",
Description: "description",
Metadata: &Metadata {
Name: "Cosmos Hub Atom",
Symbol: "ATOM",
Description: "The native staking token of the Cosmos Hub.",
Name: "diamondback",
Symbol: "DB",
Description: "The native staking token",
DenomUnits: []*DenomUnit{
{"uatom", uint32(0), []string{"microatom"}},
{"udiamondback", uint32(0), []string{"microdiamondback"}},
},
},
}
res := msg.GetSignBytes()

expected := `{"type":"cosmos-sdk/MsgUpdateDenomMetadata","value":{"description":"description","metadata":{"denom_units":[{"aliases":["microatom"],"denom":"uatom"}],"description":"The native staking token of the Cosmos Hub.","name":"Cosmos Hub Atom","symbol":"ATOM"},"title":"title"}}`
expected := `{"type":"cosmos-sdk/MsgUpdateDenomMetadata","value":{"description":"description","metadata":{"denom_units":[{"aliases":["microdiamondback"],"denom":"udiamondback"}],"description":"The native staking token","name":"diamondback","symbol":"DB"},"title":"title"}}`
require.Equal(t, expected, string(res))
}

func TestUpdateDenomMetadataGetSigners(t *testing.T) {
from := sdk.AccAddress("input111111111111111")
from := sdk.AccAddress("cosmos1d9h8qat57ljhcm")
title := "Proposal Title"
description := "Proposal description"
metadata := Metadata{}
Expand Down
6 changes: 3 additions & 3 deletions x/gov/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ func (suite *KeeperTestSuite) SetupTest() {
err = app.BankKeeper.SendCoinsFromModuleToModule(ctx, minttypes.ModuleName, types.ModuleName, coins)
suite.NoError(err)

// Create a new denom 'atom' so the proposal to update fee denom can be tested
atomCoins := sdk.NewCoins(sdk.NewCoin("atom", sdk.NewInt(100000)))
err = app.BankKeeper.MintCoins(ctx, minttypes.ModuleName, atomCoins)
// Create a new denom 'diamondback' so the proposal to update fee denom can be tested
diamondbackCoins := sdk.NewCoins(sdk.NewCoin("diamondback", sdk.NewInt(100000)))
err = app.BankKeeper.MintCoins(ctx, minttypes.ModuleName, diamondbackCoins)
suite.NoError(err)

queryHelper := baseapp.NewQueryServerTestHelper(ctx, app.InterfaceRegistry())
Expand Down
36 changes: 18 additions & 18 deletions x/gov/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,13 @@ func (suite *KeeperTestSuite) TestSubmitProposalReq() {
Title: "title",
Description: "description",
Metadata: &banktypes.Metadata{
Base: "atom",
Name: "Cosmos Hub Atom",
Symbol: "ATOM",
Display: "atom",
Description: "The native staking token of the Cosmos Hub.",
Base: "diamondback",
Name: "diamondback",
Symbol: "DB",
Display: "diamondback",
Description: "The native token.",
DenomUnits: []*banktypes.DenomUnit{
{"atom", uint32(0), []string{"atom"}},
{"diamondback", uint32(0), []string{"diamondback"}},
},
},
}
Expand Down Expand Up @@ -186,13 +186,13 @@ func (suite *KeeperTestSuite) TestVoteReq() {
Title: "title",
Description: "description",
Metadata: &banktypes.Metadata{
Base: "atom",
Name: "Cosmos Hub Atom",
Symbol: "ATOM",
Display: "atom",
Description: "The native staking token of the Cosmos Hub.",
Base: "diamondback",
Name: "diamondback",
Symbol: "DB",
Display: "diamondback",
Description: "Native token",
DenomUnits: []*banktypes.DenomUnit{
{"atom", uint32(0), []string{"atom"}},
{"diamondback", uint32(0), []string{"diamondback"}},
},
},
}
Expand Down Expand Up @@ -335,13 +335,13 @@ func (suite *KeeperTestSuite) TestVoteWeightedReq() {
Title: "title",
Description: "description",
Metadata: &banktypes.Metadata{
Base: "atom",
Name: "Cosmos Hub Atom",
Symbol: "ATOM",
Display: "atom",
Description: "The native staking token of the Cosmos Hub.",
Base: "diamondback",
Name: "diamondback",
Symbol: "DB",
Display: "diamondback",
Description: "The native token",
DenomUnits: []*banktypes.DenomUnit{
{"atom", uint32(0), []string{"atom"}},
{"diamondback", uint32(0), []string{"diamondback"}},
},
},
}
Expand Down

0 comments on commit f0cfe13

Please sign in to comment.