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(x/feegrant): remove Address.String() #19886

Merged
merged 3 commits into from
Mar 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
100 changes: 58 additions & 42 deletions x/feegrant/client/cli/tx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
_ "cosmossdk.io/api/cosmos/feegrant/v1beta1"
v1 "cosmossdk.io/api/cosmos/gov/v1"
v1beta1 "cosmossdk.io/api/cosmos/gov/v1beta1"
"cosmossdk.io/core/address"
sdkmath "cosmossdk.io/math"
"cosmossdk.io/x/feegrant"
"cosmossdk.io/x/feegrant/client/cli"
Expand Down Expand Up @@ -90,7 +91,7 @@ func (s *CLITestSuite) SetupSuite() {

granter := accounts[0].Address
grantee := accounts[1].Address
s.createGrant(granter, grantee)
s.createGrant(granter, grantee, s.baseCtx.AddressCodec)

granteeStr, err := s.baseCtx.AddressCodec.BytesToString(grantee)
s.Require().NoError(err)
Expand All @@ -112,7 +113,7 @@ func (s *CLITestSuite) SetupSuite() {
}

// createGrant creates a new basic allowance fee grant from granter to grantee.
func (s *CLITestSuite) createGrant(granter, grantee sdk.Address) {
func (s *CLITestSuite) createGrant(granter, grantee sdk.AccAddress, addressCodec address.Codec) {
commonFlags := []string{
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
Expand All @@ -121,10 +122,15 @@ func (s *CLITestSuite) createGrant(granter, grantee sdk.Address) {

fee := sdk.NewCoin("stake", sdkmath.NewInt(100))

granterAddr, err := addressCodec.BytesToString(granter)
s.Require().NoError(err)
granteeAddr, err := addressCodec.BytesToString(grantee)
s.Require().NoError(err)

args := append(
[]string{
granter.String(),
grantee.String(),
granterAddr,
granteeAddr,
fmt.Sprintf("--%s=%s", cli.FlagSpendLimit, fee.String()),
fmt.Sprintf("--%s=%s", flags.FlagFrom, granter),
fmt.Sprintf("--%s=%s", cli.FlagExpiration, getFormattedExpiration(oneYear)),
Expand All @@ -143,10 +149,12 @@ func (s *CLITestSuite) createGrant(granter, grantee sdk.Address) {

func (s *CLITestSuite) TestNewCmdFeeGrant() {
granter := s.accounts[0]
alreadyExistedGrantee := s.addedGrantee
clientCtx := s.clientCtx

fromAddr, fromName, _, err := client.GetFromFields(s.baseCtx, s.kr, granter.String())
granterAddr, err := s.baseCtx.AddressCodec.BytesToString(granter)
s.Require().NoError(err)
alreadyExistedGranteeAddr, err := s.baseCtx.AddressCodec.BytesToString(s.addedGrantee)
s.Require().NoError(err)
fromAddr, fromName, _, err := client.GetFromFields(s.baseCtx, s.kr, granterAddr)
s.Require().Equal(fromAddr, granter)
s.Require().NoError(err)

Expand Down Expand Up @@ -180,7 +188,7 @@ func (s *CLITestSuite) TestNewCmdFeeGrant() {
"wrong grantee address",
append(
[]string{
granter.String(),
granterAddr,
"wrong_grantee",
fmt.Sprintf("--%s=%s", cli.FlagSpendLimit, "100stake"),
fmt.Sprintf("--%s=%s", flags.FlagFrom, granter),
Expand All @@ -206,7 +214,7 @@ func (s *CLITestSuite) TestNewCmdFeeGrant() {
"valid basic fee grant",
append(
[]string{
granter.String(),
granterAddr,
"cosmos1nph3cfzk6trsmfxkeu943nvach5qw4vwstnvkl",
fmt.Sprintf("--%s=%s", cli.FlagSpendLimit, "100stake"),
fmt.Sprintf("--%s=%s", flags.FlagFrom, granter),
Expand All @@ -232,7 +240,7 @@ func (s *CLITestSuite) TestNewCmdFeeGrant() {
"valid basic fee grant with amino",
append(
[]string{
granter.String(),
granterAddr,
"cosmos1v57fx2l2rt6ehujuu99u2fw05779m5e2ux4z2h",
fmt.Sprintf("--%s=%s", cli.FlagSpendLimit, "100stake"),
fmt.Sprintf("--%s=%s", flags.FlagFrom, granter),
Expand All @@ -246,7 +254,7 @@ func (s *CLITestSuite) TestNewCmdFeeGrant() {
"valid basic fee grant without spend limit",
append(
[]string{
granter.String(),
granterAddr,
"cosmos17h5lzptx3ghvsuhk7wx4c4hnl7rsswxjer97em",
fmt.Sprintf("--%s=%s", flags.FlagFrom, granter),
},
Expand All @@ -258,7 +266,7 @@ func (s *CLITestSuite) TestNewCmdFeeGrant() {
"valid basic fee grant without expiration",
append(
[]string{
granter.String(),
granterAddr,
"cosmos16dlc38dcqt0uralyd8hksxyrny6kaeqfjvjwp5",
fmt.Sprintf("--%s=%s", cli.FlagSpendLimit, "100stake"),
fmt.Sprintf("--%s=%s", flags.FlagFrom, granter),
Expand All @@ -271,7 +279,7 @@ func (s *CLITestSuite) TestNewCmdFeeGrant() {
"valid basic fee grant without spend-limit and expiration",
append(
[]string{
granter.String(),
granterAddr,
"cosmos1ku40qup9vwag4wtf8cls9mkszxfthaklxkp3c8",
fmt.Sprintf("--%s=%s", flags.FlagFrom, granter),
},
Expand All @@ -283,8 +291,8 @@ func (s *CLITestSuite) TestNewCmdFeeGrant() {
"try to add existed grant",
append(
[]string{
granter.String(),
alreadyExistedGrantee.String(),
granterAddr,
alreadyExistedGranteeAddr,
fmt.Sprintf("--%s=%s", cli.FlagSpendLimit, "100stake"),
fmt.Sprintf("--%s=%s", flags.FlagFrom, granter),
},
Expand All @@ -296,7 +304,7 @@ func (s *CLITestSuite) TestNewCmdFeeGrant() {
"invalid number of args(periodic fee grant)",
append(
[]string{
granter.String(),
granterAddr,
"cosmos1nph3cfzk6trsmfxkeu943nvach5qw4vwstnvkl",
fmt.Sprintf("--%s=%s", cli.FlagSpendLimit, "100stake"),
fmt.Sprintf("--%s=%s", cli.FlagPeriodLimit, "10stake"),
Expand All @@ -311,7 +319,7 @@ func (s *CLITestSuite) TestNewCmdFeeGrant() {
"period mentioned and period limit omitted, invalid periodic grant",
append(
[]string{
granter.String(),
granterAddr,
"cosmos1nph3cfzk6trsmfxkeu943nvach5qw4vwstnvkl",
fmt.Sprintf("--%s=%s", cli.FlagSpendLimit, "100stake"),
fmt.Sprintf("--%s=%d", cli.FlagPeriod, tenHours),
Expand All @@ -326,7 +334,7 @@ func (s *CLITestSuite) TestNewCmdFeeGrant() {
"period cannot be greater than the actual expiration(periodic fee grant)",
append(
[]string{
granter.String(),
granterAddr,
"cosmos1nph3cfzk6trsmfxkeu943nvach5qw4vwstnvkl",
fmt.Sprintf("--%s=%s", cli.FlagSpendLimit, "100stake"),
fmt.Sprintf("--%s=%d", cli.FlagPeriod, tenHours),
Expand All @@ -342,7 +350,7 @@ func (s *CLITestSuite) TestNewCmdFeeGrant() {
"valid periodic fee grant",
append(
[]string{
granter.String(),
granterAddr,
"cosmos1w55kgcf3ltaqdy4ww49nge3klxmrdavrr6frmp",
fmt.Sprintf("--%s=%s", cli.FlagSpendLimit, "100stake"),
fmt.Sprintf("--%s=%d", cli.FlagPeriod, oneHour),
Expand All @@ -358,7 +366,7 @@ func (s *CLITestSuite) TestNewCmdFeeGrant() {
"valid periodic fee grant without spend-limit",
append(
[]string{
granter.String(),
granterAddr,
"cosmos1vevyks8pthkscvgazc97qyfjt40m6g9xe85ry8",
fmt.Sprintf("--%s=%d", cli.FlagPeriod, oneHour),
fmt.Sprintf("--%s=%s", cli.FlagPeriodLimit, "10stake"),
Expand All @@ -373,7 +381,7 @@ func (s *CLITestSuite) TestNewCmdFeeGrant() {
"valid periodic fee grant without expiration",
append(
[]string{
granter.String(),
granterAddr,
"cosmos14cm33pvnrv2497tyt8sp9yavhmw83nwej3m0e8",
fmt.Sprintf("--%s=%s", cli.FlagSpendLimit, "100stake"),
fmt.Sprintf("--%s=%d", cli.FlagPeriod, oneHour),
Expand All @@ -388,7 +396,7 @@ func (s *CLITestSuite) TestNewCmdFeeGrant() {
"valid periodic fee grant without spend-limit and expiration",
append(
[]string{
granter.String(),
granterAddr,
"cosmos12nyk4pcf4arshznkpz882e4l4ts0lt0ap8ce54",
fmt.Sprintf("--%s=%d", cli.FlagPeriod, oneHour),
fmt.Sprintf("--%s=%s", cli.FlagPeriodLimit, "10stake"),
Expand All @@ -402,7 +410,7 @@ func (s *CLITestSuite) TestNewCmdFeeGrant() {
"invalid expiration",
append(
[]string{
granter.String(),
granterAddr,
"cosmos1vevyks8pthkscvgazc97qyfjt40m6g9xe85ry8",
fmt.Sprintf("--%s=%d", cli.FlagPeriod, oneHour),
fmt.Sprintf("--%s=%s", cli.FlagPeriodLimit, "10stake"),
Expand Down Expand Up @@ -435,13 +443,17 @@ func (s *CLITestSuite) TestNewCmdFeeGrant() {
func (s *CLITestSuite) TestTxWithFeeGrant() {
clientCtx := s.clientCtx
granter := s.addedGranter
granterAddr, err := s.baseCtx.AddressCodec.BytesToString(granter)
s.Require().NoError(err)

// creating an account manually (This account won't be exist in state)
k, _, err := s.baseCtx.Keyring.NewMnemonic("grantee", keyring.English, sdk.FullFundraiserPath, keyring.DefaultBIP39Passphrase, hd.Secp256k1)
s.Require().NoError(err)
pub, err := k.GetPubKey()
s.Require().NoError(err)
grantee := sdk.AccAddress(pub.Address())
granteeAddr, err := s.baseCtx.AddressCodec.BytesToString(grantee)
s.Require().NoError(err)

commonFlags := []string{
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
Expand All @@ -453,8 +465,8 @@ func (s *CLITestSuite) TestTxWithFeeGrant() {

args := append(
[]string{
granter.String(),
grantee.String(),
granterAddr,
granteeAddr,
fmt.Sprintf("--%s=%s", cli.FlagSpendLimit, fee.String()),
fmt.Sprintf("--%s=%s", flags.FlagFrom, granter),
fmt.Sprintf("--%s=%s", cli.FlagExpiration, getFormattedExpiration(oneYear)),
Expand All @@ -477,30 +489,30 @@ func (s *CLITestSuite) TestTxWithFeeGrant() {
}{
{
name: "granted fee allowance for an account which is not in state and creating any tx with it by using --fee-granter shouldn't fail",
from: grantee.String(),
flags: []string{fmt.Sprintf("--%s=%s", flags.FlagFeeGranter, granter.String())},
from: granteeAddr,
flags: []string{fmt.Sprintf("--%s=%s", flags.FlagFeeGranter, granterAddr)},
},
{
name: "--fee-payer should also sign the tx (direct)",
from: grantee.String(),
flags: []string{fmt.Sprintf("--%s=%s", flags.FlagFeePayer, granter.String())},
from: granteeAddr,
flags: []string{fmt.Sprintf("--%s=%s", flags.FlagFeePayer, granterAddr)},
expErrCode: 4,
},
{
name: "--fee-payer should also sign the tx (amino-json)",
from: grantee.String(),
from: granteeAddr,
flags: []string{
fmt.Sprintf("--%s=%s", flags.FlagFeePayer, granter.String()),
fmt.Sprintf("--%s=%s", flags.FlagFeePayer, granterAddr),
fmt.Sprintf("--%s=%s", flags.FlagSignMode, flags.SignModeLegacyAminoJSON),
},
expErrCode: 4,
},
{
name: "use --fee-payer and --fee-granter together works",
from: grantee.String(),
from: granteeAddr,
flags: []string{
fmt.Sprintf("--%s=%s", flags.FlagFeePayer, grantee.String()),
fmt.Sprintf("--%s=%s", flags.FlagFeeGranter, granter.String()),
fmt.Sprintf("--%s=%s", flags.FlagFeePayer, granteeAddr),
fmt.Sprintf("--%s=%s", flags.FlagFeeGranter, granterAddr),
},
},
}
Expand Down Expand Up @@ -547,11 +559,15 @@ func (s *CLITestSuite) msgSubmitLegacyProposal(clientCtx client.Context, from, t

func (s *CLITestSuite) TestFilteredFeeAllowance() {
granter := s.addedGranter
granterAddr, err := s.baseCtx.AddressCodec.BytesToString(granter)
s.Require().NoError(err)
k, _, err := s.baseCtx.Keyring.NewMnemonic("grantee1", keyring.English, sdk.FullFundraiserPath, keyring.DefaultBIP39Passphrase, hd.Secp256k1)
s.Require().NoError(err)
pub, err := k.GetPubKey()
s.Require().NoError(err)
grantee := sdk.AccAddress(pub.Address())
granteeAddr, err := s.baseCtx.AddressCodec.BytesToString(grantee)
s.Require().NoError(err)
clientCtx := s.clientCtx

commonFlags := []string{
Expand Down Expand Up @@ -585,7 +601,7 @@ func (s *CLITestSuite) TestFilteredFeeAllowance() {
"invalid grantee address",
append(
[]string{
granter.String(),
granterAddr,
"not an address",
fmt.Sprintf("--%s=%s", cli.FlagAllowedMsgs, allowMsgs),
fmt.Sprintf("--%s=%s", cli.FlagSpendLimit, spendLimit.String()),
Expand All @@ -599,8 +615,8 @@ func (s *CLITestSuite) TestFilteredFeeAllowance() {
"valid filter fee grant",
append(
[]string{
granter.String(),
grantee.String(),
granterAddr,
granteeAddr,
fmt.Sprintf("--%s=%s", cli.FlagAllowedMsgs, allowMsgs),
fmt.Sprintf("--%s=%s", cli.FlagSpendLimit, spendLimit.String()),
fmt.Sprintf("--%s=%s", flags.FlagFrom, granter),
Expand Down Expand Up @@ -636,18 +652,18 @@ func (s *CLITestSuite) TestFilteredFeeAllowance() {
{
"valid proposal tx",
func() error {
return s.msgSubmitLegacyProposal(s.baseCtx, grantee.String(),
return s.msgSubmitLegacyProposal(s.baseCtx, granteeAddr,
"Text Proposal", "No desc", "Text",
fmt.Sprintf("--%s=%s", flags.FlagFeeGranter, granter.String()),
fmt.Sprintf("--%s=%s", flags.FlagFeeGranter, granterAddr),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(100))).String()),
)
},
},
{
"valid weighted_vote tx",
func() error {
return s.msgVote(s.baseCtx, grantee.String(), "0", "yes",
fmt.Sprintf("--%s=%s", flags.FlagFeeGranter, granter.String()),
return s.msgVote(s.baseCtx, granteeAddr, "0", "yes",
fmt.Sprintf("--%s=%s", flags.FlagFeeGranter, granterAddr),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(100))).String()),
)
},
Expand All @@ -657,7 +673,7 @@ func (s *CLITestSuite) TestFilteredFeeAllowance() {
func() error {
args := append(
[]string{
grantee.String(),
granteeAddr,
"cosmos14cm33pvnrv2497tyt8sp9yavhmw83nwej3m0e8",
fmt.Sprintf("--%s=%s", cli.FlagSpendLimit, "100stake"),
fmt.Sprintf("--%s=%s", flags.FlagFeeGranter, granter),
Expand Down
23 changes: 17 additions & 6 deletions x/feegrant/simulation/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"math/rand"
"time"

"cosmossdk.io/core/address"
"cosmossdk.io/math"
"cosmossdk.io/x/feegrant"

Expand All @@ -13,14 +14,20 @@ import (
)

// genFeeGrants returns a slice of randomly generated allowances.
func genFeeGrants(r *rand.Rand, accounts []simtypes.Account) []feegrant.Grant {
func genFeeGrants(r *rand.Rand, accounts []simtypes.Account, addressCodec address.Codec) ([]feegrant.Grant, error) {
allowances := make([]feegrant.Grant, len(accounts)-1)
for i := 0; i < len(accounts)-1; i++ {
granter := accounts[i].Address
grantee := accounts[i+1].Address
allowances[i] = generateRandomAllowances(granter.String(), grantee.String(), r) // TODO decouple this from call .String()
granter, err := addressCodec.BytesToString(accounts[i].Address)
if err != nil {
return allowances, err
}
grantee, err := addressCodec.BytesToString(accounts[i+1].Address)
if err != nil {
return allowances, err
}
allowances[i] = generateRandomAllowances(granter, grantee, r)
}
return allowances
return allowances, nil
}

func generateRandomAllowances(granter, grantee string, r *rand.Rand) feegrant.Grant {
Expand Down Expand Up @@ -63,11 +70,15 @@ func generateRandomAllowances(granter, grantee string, r *rand.Rand) feegrant.Gr
// RandomizedGenState generates a random GenesisState for feegrant
func RandomizedGenState(simState *module.SimulationState) {
var feegrants []feegrant.Grant
var err error

simState.AppParams.GetOrGenerate(
"feegrant", &feegrants, simState.Rand,
func(r *rand.Rand) { feegrants = genFeeGrants(r, simState.Accounts) },
func(r *rand.Rand) { feegrants, err = genFeeGrants(r, simState.Accounts, simState.AddressCodec) },
)
if err != nil {
panic(err)
}
JulianToledano marked this conversation as resolved.
Show resolved Hide resolved

feegrantGenesis := feegrant.NewGenesisState(feegrants)
bz, err := simState.Cdc.MarshalJSON(feegrantGenesis)
Expand Down
Loading
Loading