From 6972e006be0e8cc4496f88ab13b5c93ba8398668 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juli=C3=A1n=20Toledano?= Date: Wed, 27 Mar 2024 22:12:52 +0100 Subject: [PATCH] refactor(x/feegrant): remove Address.String() (#19886) Co-authored-by: son trinh --- x/feegrant/client/cli/tx_test.go | 100 ++++++++++++++++++------------- x/feegrant/simulation/genesis.go | 23 +++++-- x/gov/keeper/deposit.go | 7 ++- x/gov/keeper/proposal.go | 2 +- 4 files changed, 82 insertions(+), 50 deletions(-) diff --git a/x/feegrant/client/cli/tx_test.go b/x/feegrant/client/cli/tx_test.go index ffdbcbb584..16f3bc8ae6 100644 --- a/x/feegrant/client/cli/tx_test.go +++ b/x/feegrant/client/cli/tx_test.go @@ -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" @@ -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) @@ -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), @@ -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)), @@ -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) @@ -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), @@ -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), @@ -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), @@ -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), }, @@ -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), @@ -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), }, @@ -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), }, @@ -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"), @@ -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), @@ -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), @@ -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), @@ -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"), @@ -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), @@ -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"), @@ -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"), @@ -435,6 +443,8 @@ 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) @@ -442,6 +452,8 @@ func (s *CLITestSuite) TestTxWithFeeGrant() { 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), @@ -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)), @@ -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), }, }, } @@ -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{ @@ -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()), @@ -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), @@ -636,9 +652,9 @@ 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()), ) }, @@ -646,8 +662,8 @@ func (s *CLITestSuite) TestFilteredFeeAllowance() { { "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()), ) }, @@ -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), diff --git a/x/feegrant/simulation/genesis.go b/x/feegrant/simulation/genesis.go index 14066e8033..289126f1fc 100644 --- a/x/feegrant/simulation/genesis.go +++ b/x/feegrant/simulation/genesis.go @@ -4,6 +4,7 @@ import ( "math/rand" "time" + "cosmossdk.io/core/address" "cosmossdk.io/math" "cosmossdk.io/x/feegrant" @@ -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 { @@ -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) + } feegrantGenesis := feegrant.NewGenesisState(feegrants) bz, err := simState.Cdc.MarshalJSON(feegrantGenesis) diff --git a/x/gov/keeper/deposit.go b/x/gov/keeper/deposit.go index 9a8eb44043..51b2e728ff 100644 --- a/x/gov/keeper/deposit.go +++ b/x/gov/keeper/deposit.go @@ -181,9 +181,14 @@ func (k Keeper) AddDeposit(ctx context.Context, proposalID uint64, depositorAddr return false, err } + depositorStrAddr, err := k.authKeeper.AddressCodec().BytesToString(depositorAddr) + if err != nil { + return false, err + } + if err := k.environment.EventService.EventManager(ctx).EmitKV( types.EventTypeProposalDeposit, - event.NewAttribute(types.AttributeKeyDepositor, depositorAddr.String()), + event.NewAttribute(types.AttributeKeyDepositor, depositorStrAddr), event.NewAttribute(sdk.AttributeKeyAmount, depositAmount.String()), event.NewAttribute(types.AttributeKeyProposalID, fmt.Sprintf("%d", proposalID)), ); err != nil { diff --git a/x/gov/keeper/proposal.go b/x/gov/keeper/proposal.go index 108a4c3602..b3373f12ac 100644 --- a/x/gov/keeper/proposal.go +++ b/x/gov/keeper/proposal.go @@ -156,7 +156,7 @@ func (k Keeper) SubmitProposal(ctx context.Context, messages []sdk.Msg, metadata types.EventTypeSubmitProposal, event.NewAttribute(types.AttributeKeyProposalType, proposalType.String()), event.NewAttribute(types.AttributeKeyProposalID, fmt.Sprintf("%d", proposalID)), - event.NewAttribute(types.AttributeKeyProposalProposer, proposer.String()), + event.NewAttribute(types.AttributeKeyProposalProposer, proposerAddr), event.NewAttribute(types.AttributeKeyProposalMessages, strings.Join(msgs, ",")), ); err != nil { return v1.Proposal{}, fmt.Errorf("failed to emit event: %w", err)