refactor(x/authz)!: remove Accounts.String() (#19783)

cosmos · Mar 20, 2024 · fa19df1 · fa19df1
1 parent 0e1d620
commit fa19df1
Show file tree

Hide file tree

Showing 15 changed files with 314 additions and 177 deletions.
diff --git a/x/authz/CHANGELOG.md b/x/authz/CHANGELOG.md
@@ -31,6 +31,10 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### API Breaking Changes
 
+* [#19783](https://github.com/cosmos/cosmos-sdk/pull/19783) Removes the use of Accouts String() method
+    * `NewMsgExec`, `NewMsgGrant` and `NewMsgRevoke` now takes strings as arguments instead of `sdk.AccAddress`.
+    * `ExportGenesis` also returns an error.
+    * `IterateGrants` returns an error, its handler function also returns an error.
 * [#19637](https://github.com/cosmos/cosmos-sdk/pull/19637) `NewKeeper` doesn't take a message router anymore. Set the message router in the `appmodule.Environment` instead.
 * [#19490](https://github.com/cosmos/cosmos-sdk/pull/19490) `appmodule.Environment` is received on the Keeper to get access to different application services.
 * [#18737](https://github.com/cosmos/cosmos-sdk/pull/18737) Update the keeper method `DequeueAndDeleteExpiredGrants` to take a limit argument for the number of grants to prune.

diff --git a/x/authz/client/cli/tx.go b/x/authz/client/cli/tx.go
@@ -67,7 +67,10 @@ func NewCmdExecAuthorization() *cobra.Command {
 			if err != nil {
 				return err
 			}
-			grantee := clientCtx.GetFromAddress()
+			grantee, err := clientCtx.AddressCodec.BytesToString(clientCtx.GetFromAddress())
+			if err != nil {
+				return err
+			}
 
 			if offline, _ := cmd.Flags().GetBool(flags.FlagOffline); offline {
 				return errors.New("cannot broadcast tx during offline mode")
@@ -106,15 +109,20 @@ Examples:
 				return err
 			}
 
-			if strings.EqualFold(args[0], clientCtx.GetFromAddress().String()) {
-				return errors.New("grantee and granter should be different")
+			grantee := args[0]
+			if _, err := clientCtx.AddressCodec.StringToBytes(grantee); err != nil {
+				return err
 			}
 
-			grantee, err := clientCtx.AddressCodec.StringToBytes(args[0])
+			granter, err := clientCtx.AddressCodec.BytesToString(clientCtx.GetFromAddress())
 			if err != nil {
 				return err
 			}
 
+			if strings.EqualFold(grantee, granter) {
+				return errors.New("grantee and granter should be different")
+			}
+
 			var authorization authz.Authorization
 			switch args[1] {
 			case "send":
@@ -230,7 +238,7 @@ Examples:
 				return err
 			}
 
-			msg, err := authz.NewMsgGrant(clientCtx.GetFromAddress(), grantee, authorization, expire)
+			msg, err := authz.NewMsgGrant(granter, grantee, authorization, expire)
 			if err != nil {
 				return err
 			}

diff --git a/x/authz/client/cli/tx_test.go b/x/authz/client/cli/tx_test.go
@@ -80,6 +80,8 @@ func (s *CLITestSuite) SetupSuite() {
 
 	val := testutil.CreateKeyringAccounts(s.T(), s.kr, 1)
 	s.grantee = make([]sdk.AccAddress, 6)
+	valAddr, err := s.baseCtx.AddressCodec.BytesToString(val[0].Address)
+	s.Require().NoError(err)
 
 	s.addrs = make([]sdk.AccAddress, 1)
 	s.addrs[0] = s.createAccount("validator address")
@@ -93,13 +95,15 @@ func (s *CLITestSuite) SetupSuite() {
 	s.grantee[1] = s.createAccount("grantee2")
 	// Send some funds to the new account.
 	s.msgSendExec(s.grantee[1])
+	grantee1Addr, err := s.baseCtx.AddressCodec.BytesToString(s.grantee[1])
+	s.Require().NoError(err)
 
 	// grant send authorization to grantee2
 	out, err := authzclitestutil.CreateGrant(s.clientCtx, []string{
-		s.grantee[1].String(),
+		grantee1Addr,
 		"send",
 		fmt.Sprintf("--%s=100stake", cli.FlagSpendLimit),
-		fmt.Sprintf("--%s=%s", flags.FlagFrom, val[0].Address.String()),
+		fmt.Sprintf("--%s=%s", flags.FlagFrom, valAddr),
 		fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
 		fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
 		fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(10))).String()),
@@ -112,13 +116,15 @@ func (s *CLITestSuite) SetupSuite() {
 
 	// Create new account in the keyring.
 	s.grantee[2] = s.createAccount("grantee3")
+	grantee2Addr, err := s.baseCtx.AddressCodec.BytesToString(s.grantee[2])
+	s.Require().NoError(err)
 
 	// grant send authorization to grantee3
 	_, err = authzclitestutil.CreateGrant(s.clientCtx, []string{
-		s.grantee[2].String(),
+		grantee2Addr,
 		"send",
 		fmt.Sprintf("--%s=100stake", cli.FlagSpendLimit),
-		fmt.Sprintf("--%s=%s", flags.FlagFrom, val[0].Address.String()),
+		fmt.Sprintf("--%s=%s", flags.FlagFrom, valAddr),
 		fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
 		fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
 		fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(10))).String()),
@@ -129,18 +135,20 @@ func (s *CLITestSuite) SetupSuite() {
 	// Create new accounts in the keyring.
 	s.grantee[3] = s.createAccount("grantee4")
 	s.msgSendExec(s.grantee[3])
+	grantee3Addr, err := s.baseCtx.AddressCodec.BytesToString(s.grantee[3])
+	s.Require().NoError(err)
 
 	s.grantee[4] = s.createAccount("grantee5")
 	s.grantee[5] = s.createAccount("grantee6")
 
 	// grant send authorization with allow list to grantee4
 	out, err = authzclitestutil.CreateGrant(s.clientCtx,
 		[]string{
-			s.grantee[3].String(),
+			grantee3Addr,
 			"send",
 			fmt.Sprintf("--%s=100stake", cli.FlagSpendLimit),
 			fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
-			fmt.Sprintf("--%s=%s", flags.FlagFrom, val[0].Address.String()),
+			fmt.Sprintf("--%s=%s", flags.FlagFrom, valAddr),
 			fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
 			fmt.Sprintf("--%s=%d", cli.FlagExpiration, time.Now().Add(time.Minute*time.Duration(120)).Unix()),
 			fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(10))).String()),
@@ -170,9 +178,13 @@ func (s *CLITestSuite) msgSendExec(grantee sdk.AccAddress) {
 
 	coins := sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(200)))
 	from := val[0].Address
+	fromAddr, err := s.clientCtx.AddressCodec.BytesToString(from)
+	s.Require().NoError(err)
+	granteeAddr, err := s.clientCtx.AddressCodec.BytesToString(grantee)
+	s.Require().NoError(err)
 	msgSend := &banktypes.MsgSend{
-		FromAddress: from.String(),
-		ToAddress:   grantee.String(),
+		FromAddress: fromAddr,
+		ToAddress:   granteeAddr,
 		Amount:      coins,
 	}
 
@@ -182,8 +194,14 @@ func (s *CLITestSuite) msgSendExec(grantee sdk.AccAddress) {
 
 func (s *CLITestSuite) TestCLITxGrantAuthorization() {
 	val := testutil.CreateKeyringAccounts(s.T(), s.kr, 1)
+	valAddress, err := s.clientCtx.ValidatorAddressCodec.BytesToString(s.addrs[0])
+	s.Require().NoError(err)
+	fromAddr, err := s.baseCtx.AddressCodec.BytesToString(val[0].Address)
+	s.Require().NoError(err)
 
 	grantee := s.grantee[0]
+	granteeAddr, err := s.baseCtx.AddressCodec.BytesToString(grantee)
+	s.Require().NoError(err)
 
 	twoHours := time.Now().Add(time.Minute * 120).Unix()
 	pastHour := time.Now().Add(-time.Minute * 60).Unix()
@@ -213,7 +231,7 @@ func (s *CLITestSuite) TestCLITxGrantAuthorization() {
 				"grantee_addr",
 				"send",
 				fmt.Sprintf("--%s=100stake", cli.FlagSpendLimit),
-				fmt.Sprintf("--%s=%s", flags.FlagFrom, val[0].Address.String()),
+				fmt.Sprintf("--%s=%s", flags.FlagFrom, fromAddr),
 				fmt.Sprintf("--%s=true", flags.FlagGenerateOnly),
 				fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
 			},
@@ -223,10 +241,10 @@ func (s *CLITestSuite) TestCLITxGrantAuthorization() {
 		{
 			"Invalid spend limit",
 			[]string{
-				grantee.String(),
+				granteeAddr,
 				"send",
 				fmt.Sprintf("--%s=0stake", cli.FlagSpendLimit),
-				fmt.Sprintf("--%s=%s", flags.FlagFrom, val[0].Address.String()),
+				fmt.Sprintf("--%s=%s", flags.FlagFrom, fromAddr),
 				fmt.Sprintf("--%s=true", flags.FlagGenerateOnly),
 				fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
 			},
@@ -236,10 +254,10 @@ func (s *CLITestSuite) TestCLITxGrantAuthorization() {
 		{
 			"Invalid expiration time",
 			[]string{
-				grantee.String(),
+				granteeAddr,
 				"send",
 				fmt.Sprintf("--%s=100stake", cli.FlagSpendLimit),
-				fmt.Sprintf("--%s=%s", flags.FlagFrom, val[0].Address.String()),
+				fmt.Sprintf("--%s=%s", flags.FlagFrom, fromAddr),
 				fmt.Sprintf("--%s=true", flags.FlagBroadcastMode),
 				fmt.Sprintf("--%s=%d", cli.FlagExpiration, pastHour),
 			},
@@ -249,10 +267,10 @@ func (s *CLITestSuite) TestCLITxGrantAuthorization() {
 		{
 			"fail with error invalid msg-type",
 			[]string{
-				grantee.String(),
+				granteeAddr,
 				"generic",
 				fmt.Sprintf("--%s=invalid-msg-type", cli.FlagMsgType),
-				fmt.Sprintf("--%s=%s", flags.FlagFrom, val[0].Address.String()),
+				fmt.Sprintf("--%s=%s", flags.FlagFrom, fromAddr),
 				fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
 				fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
 				fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(10))).String()),
@@ -264,14 +282,14 @@ func (s *CLITestSuite) TestCLITxGrantAuthorization() {
 		{
 			"invalid bond denom for tx delegate authorization allowed validators",
 			[]string{
-				grantee.String(),
+				granteeAddr,
 				"delegate",
 				fmt.Sprintf("--%s=100xyz", cli.FlagSpendLimit),
 				fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
-				fmt.Sprintf("--%s=%s", flags.FlagFrom, val[0].Address.String()),
+				fmt.Sprintf("--%s=%s", flags.FlagFrom, fromAddr),
 				fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
 				fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
-				fmt.Sprintf("--%s=%s", cli.FlagAllowedValidators, sdk.ValAddress(s.addrs[0]).String()),
+				fmt.Sprintf("--%s=%s", cli.FlagAllowedValidators, valAddress),
 				fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(10))).String()),
 			},
 			true,
@@ -280,14 +298,14 @@ func (s *CLITestSuite) TestCLITxGrantAuthorization() {
 		{
 			"invalid bond denom for tx delegate authorization deny validators",
 			[]string{
-				grantee.String(),
+				granteeAddr,
 				"delegate",
 				fmt.Sprintf("--%s=100xyz", cli.FlagSpendLimit),
 				fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
-				fmt.Sprintf("--%s=%s", flags.FlagFrom, val[0].Address.String()),
+				fmt.Sprintf("--%s=%s", flags.FlagFrom, fromAddr),
 				fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
 				fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
-				fmt.Sprintf("--%s=%s", cli.FlagDenyValidators, sdk.ValAddress(s.addrs[0]).String()),
+				fmt.Sprintf("--%s=%s", cli.FlagDenyValidators, valAddress),
 				fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(10))).String()),
 			},
 			true,
@@ -296,14 +314,14 @@ func (s *CLITestSuite) TestCLITxGrantAuthorization() {
 		{
 			"invalid bond denom for tx undelegate authorization",
 			[]string{
-				grantee.String(),
+				granteeAddr,
 				"unbond",
 				fmt.Sprintf("--%s=100xyz", cli.FlagSpendLimit),
 				fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
-				fmt.Sprintf("--%s=%s", flags.FlagFrom, val[0].Address.String()),
+				fmt.Sprintf("--%s=%s", flags.FlagFrom, fromAddr),
 				fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
 				fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
-				fmt.Sprintf("--%s=%s", cli.FlagAllowedValidators, sdk.ValAddress(s.addrs[0]).String()),
+				fmt.Sprintf("--%s=%s", cli.FlagAllowedValidators, valAddress),
 				fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(10))).String()),
 			},
 			true,
@@ -312,14 +330,14 @@ func (s *CLITestSuite) TestCLITxGrantAuthorization() {
 		{
 			"invalid bond denom for tx redelegate authorization",
 			[]string{
-				grantee.String(),
+				granteeAddr,
 				"redelegate",
 				fmt.Sprintf("--%s=100xyz", cli.FlagSpendLimit),
 				fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
-				fmt.Sprintf("--%s=%s", flags.FlagFrom, val[0].Address.String()),
+				fmt.Sprintf("--%s=%s", flags.FlagFrom, fromAddr),
 				fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
 				fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
-				fmt.Sprintf("--%s=%s", cli.FlagAllowedValidators, sdk.ValAddress(s.addrs[0]).String()),
+				fmt.Sprintf("--%s=%s", cli.FlagAllowedValidators, valAddress),
 				fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(10))).String()),
 			},
 			true,
@@ -328,14 +346,14 @@ func (s *CLITestSuite) TestCLITxGrantAuthorization() {
 		{
 			"invalid decimal coin expression with more than single coin",
 			[]string{
-				grantee.String(),
+				granteeAddr,
 				"delegate",
 				fmt.Sprintf("--%s=100stake,20xyz", cli.FlagSpendLimit),
 				fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
-				fmt.Sprintf("--%s=%s", flags.FlagFrom, val[0].Address.String()),
+				fmt.Sprintf("--%s=%s", flags.FlagFrom, fromAddr),
 				fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
 				fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
-				fmt.Sprintf("--%s=%s", cli.FlagAllowedValidators, sdk.ValAddress(s.addrs[0]).String()),
+				fmt.Sprintf("--%s=%s", cli.FlagAllowedValidators, valAddress),
 				fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(10))).String()),
 			},
 			true,
@@ -344,7 +362,7 @@ func (s *CLITestSuite) TestCLITxGrantAuthorization() {
 		{
 			"invalid authorization type",
 			[]string{
-				grantee.String(),
+				granteeAddr,
 				"invalid authz type",
 			},
 			true,
@@ -353,10 +371,10 @@ func (s *CLITestSuite) TestCLITxGrantAuthorization() {
 		{
 			"Valid tx send authorization",
 			[]string{
-				grantee.String(),
+				granteeAddr,
 				"send",
 				fmt.Sprintf("--%s=100stake", cli.FlagSpendLimit),
-				fmt.Sprintf("--%s=%s", flags.FlagFrom, val[0].Address.String()),
+				fmt.Sprintf("--%s=%s", flags.FlagFrom, fromAddr),
 				fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
 				fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
 				fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
@@ -368,10 +386,10 @@ func (s *CLITestSuite) TestCLITxGrantAuthorization() {
 		{
 			"Valid tx send authorization with allow list",
 			[]string{
-				grantee.String(),
+				granteeAddr,
 				"send",
 				fmt.Sprintf("--%s=100stake", cli.FlagSpendLimit),
-				fmt.Sprintf("--%s=%s", flags.FlagFrom, val[0].Address.String()),
+				fmt.Sprintf("--%s=%s", flags.FlagFrom, fromAddr),
 				fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
 				fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
 				fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
@@ -384,10 +402,10 @@ func (s *CLITestSuite) TestCLITxGrantAuthorization() {
 		{
 			"Invalid tx send authorization with duplicate allow list",
 			[]string{
-				grantee.String(),
+				granteeAddr,
 				"send",
 				fmt.Sprintf("--%s=100stake", cli.FlagSpendLimit),
-				fmt.Sprintf("--%s=%s", flags.FlagFrom, val[0].Address.String()),
+				fmt.Sprintf("--%s=%s", flags.FlagFrom, fromAddr),
 				fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
 				fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
 				fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
@@ -400,10 +418,10 @@ func (s *CLITestSuite) TestCLITxGrantAuthorization() {
 		{
 			"Valid tx generic authorization",
 			[]string{
-				grantee.String(),
+				granteeAddr,
 				"generic",
 				fmt.Sprintf("--%s=%s", cli.FlagMsgType, typeMsgVote),
-				fmt.Sprintf("--%s=%s", flags.FlagFrom, val[0].Address.String()),
+				fmt.Sprintf("--%s=%s", flags.FlagFrom, fromAddr),
 				fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
 				fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
 				fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
@@ -415,10 +433,10 @@ func (s *CLITestSuite) TestCLITxGrantAuthorization() {
 		{
 			"fail when granter = grantee",
 			[]string{
-				grantee.String(),
+				granteeAddr,
 				"generic",
 				fmt.Sprintf("--%s=%s", cli.FlagMsgType, typeMsgVote),
-				fmt.Sprintf("--%s=%s", flags.FlagFrom, grantee.String()),
+				fmt.Sprintf("--%s=%s", flags.FlagFrom, granteeAddr),
 				fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
 				fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
 				fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
@@ -430,10 +448,10 @@ func (s *CLITestSuite) TestCLITxGrantAuthorization() {
 		{
 			"Valid tx with amino",
 			[]string{
-				grantee.String(),
+				granteeAddr,
 				"generic",
 				fmt.Sprintf("--%s=%s", cli.FlagMsgType, typeMsgVote),
-				fmt.Sprintf("--%s=%s", flags.FlagFrom, val[0].Address.String()),
+				fmt.Sprintf("--%s=%s", flags.FlagFrom, fromAddr),
 				fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
 				fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
 				fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),