refactor: Update GetSigners to return []string (#9418)

<!-- < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < ☺
v                               ✰  Thanks for creating a PR! ✰
v    Before smashing the submit button please review the checkboxes.
v    If a checkbox is n/a - please still include it but + a little note why
☺ > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >  -->

## Description

<!-- Add a description of the changes that this PR introduces and the files that
are the most critical to review.
-->

closes: #9239 

---

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

- [ ] Targeted PR against correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
- [ ] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules/structure.md).
- [ ] Wrote unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [ ] Updated relevant documentation (`docs/`) or specification (`x/<module>/spec/`)
- [ ] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code).
- [ ] Added a relevant changelog entry to the `Unreleased` section in `CHANGELOG.md`
- [ ] Re-reviewed `Files changed` in the Github PR explorer
- [ ] Review `Codecov Report` in the comment section below once CI passes

CHANGELOG.md

@@ -53,6 +53,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * [\#9246](https://github.com/cosmos/cosmos-sdk/pull/9246) The `New` method for the network package now returns an error.
 * (codec) [\#9521](https://github.com/cosmos/cosmos-sdk/pull/9521) Removed deprecated `clientCtx.JSONCodec` from `client.Context`.
 * (codec) [\#9521](https://github.com/cosmos/cosmos-sdk/pull/9521) Rename `EncodingConfig.Marshaler` to `Codec`.
+* [\#9418](https://github.com/cosmos/cosmos-sdk/pull/9418) `sdk.Msg`'s  `GetSigners()` method updated to return `[]string`.
 * [\#9594](https://github.com/cosmos/cosmos-sdk/pull/9594) `RESTHandlerFn` argument is removed from the `gov/NewProposalHandler`.
 * [\#9594](https://github.com/cosmos/cosmos-sdk/pull/9594) `types/rest` package moved to `testutil/rest`.
 

baseapp/baseapp.go

@@ -19,6 +19,7 @@ import (
 	"github.com/cosmos/cosmos-sdk/store/rootmulti"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+	sdktx "github.com/cosmos/cosmos-sdk/types/tx"
 	"github.com/cosmos/cosmos-sdk/x/auth/migrations/legacytx"
 )
 
@@ -508,7 +509,7 @@ func validateBasicTxMsgs(msgs []sdk.Msg) error {
 	}
 
 	for _, msg := range msgs {
-		err := msg.ValidateBasic()
+		err := sdktx.ValidateMsg(msg)
 		if err != nil {
 			return err
 		}

baseapp/baseapp_test.go

@@ -717,10 +717,10 @@ func (msg msgCounter) String() string { return "TODO" }
 func (msg msgCounter) ProtoMessage()  {}
 
 // Implements Msg
-func (msg msgCounter) Route() string                { return routeMsgCounter }
-func (msg msgCounter) Type() string                 { return "counter1" }
-func (msg msgCounter) GetSignBytes() []byte         { return nil }
-func (msg msgCounter) GetSigners() []sdk.AccAddress { return nil }
+func (msg msgCounter) Route() string        { return routeMsgCounter }
+func (msg msgCounter) Type() string         { return "counter1" }
+func (msg msgCounter) GetSignBytes() []byte { return nil }
+func (msg msgCounter) GetSigners() []string { return nil }
 func (msg msgCounter) ValidateBasic() error {
 	if msg.Counter >= 0 {
 		return nil
@@ -762,10 +762,10 @@ func (msg msgCounter2) String() string { return "TODO" }
 func (msg msgCounter2) ProtoMessage()  {}
 
 // Implements Msg
-func (msg msgCounter2) Route() string                { return routeMsgCounter2 }
-func (msg msgCounter2) Type() string                 { return "counter2" }
-func (msg msgCounter2) GetSignBytes() []byte         { return nil }
-func (msg msgCounter2) GetSigners() []sdk.AccAddress { return nil }
+func (msg msgCounter2) Route() string        { return routeMsgCounter2 }
+func (msg msgCounter2) Type() string         { return "counter2" }
+func (msg msgCounter2) GetSignBytes() []byte { return nil }
+func (msg msgCounter2) GetSigners() []string { return nil }
 func (msg msgCounter2) ValidateBasic() error {
 	if msg.Counter >= 0 {
 		return nil
@@ -779,13 +779,13 @@ type msgKeyValue struct {
 	Value []byte
 }
 
-func (msg msgKeyValue) Reset()                       {}
-func (msg msgKeyValue) String() string               { return "TODO" }
-func (msg msgKeyValue) ProtoMessage()                {}
-func (msg msgKeyValue) Route() string                { return routeMsgKeyValue }
-func (msg msgKeyValue) Type() string                 { return "keyValue" }
-func (msg msgKeyValue) GetSignBytes() []byte         { return nil }
-func (msg msgKeyValue) GetSigners() []sdk.AccAddress { return nil }
+func (msg msgKeyValue) Reset()               {}
+func (msg msgKeyValue) String() string       { return "TODO" }
+func (msg msgKeyValue) ProtoMessage()        {}
+func (msg msgKeyValue) Route() string        { return routeMsgKeyValue }
+func (msg msgKeyValue) Type() string         { return "keyValue" }
+func (msg msgKeyValue) GetSignBytes() []byte { return nil }
+func (msg msgKeyValue) GetSigners() []string { return nil }
 func (msg msgKeyValue) ValidateBasic() error {
 	if msg.Key == nil {
 		return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "key cannot be nil")

client/tx/tx.go

@@ -35,7 +35,7 @@ func GenerateOrBroadcastTxWithFactory(clientCtx client.Context, txf Factory, msg
 	// Right now, we're factorizing that call inside this function.
 	// ref: https://github.com/cosmos/cosmos-sdk/pull/9236#discussion_r623803504
 	for _, msg := range msgs {
-		if err := msg.ValidateBasic(); err != nil {
+		if err := tx.ValidateMsg(msg); err != nil {
 			return err
 		}
 	}

server/mock/tx.go

@@ -58,7 +58,7 @@ func (tx kvstoreTx) ValidateBasic() error {
 	return nil
 }
 
-func (tx kvstoreTx) GetSigners() []sdk.AccAddress {
+func (tx kvstoreTx) GetSigners() []string {
 	return nil
 }
 

server/rosetta/converter.go

@@ -254,7 +254,7 @@ func (c converter) Ops(status string, msg sdk.Msg) ([]*rosettatypes.Operation, e
 		op := &rosettatypes.Operation{
 			Type:     opName,
 			Status:   &status,
-			Account:  &rosettatypes.AccountIdentifier{Address: signer.String()},
+			Account:  &rosettatypes.AccountIdentifier{Address: signer},
 			Metadata: meta,
 		}
 

testutil/testdata/tx.go

@@ -62,22 +62,12 @@ func (msg *TestMsg) GetSignBytes() []byte {
 	}
 	return sdk.MustSortJSON(bz)
 }
-func (msg *TestMsg) GetSigners() []sdk.AccAddress {
-	addrs := make([]sdk.AccAddress, len(msg.Signers))
-	for i, in := range msg.Signers {
-		addr, err := sdk.AccAddressFromBech32(in)
-		if err != nil {
-			panic(err)
-		}
-
-		addrs[i] = addr
-	}
-
-	return addrs
+func (msg *TestMsg) GetSigners() []string {
+	return msg.Signers
 }
 func (msg *TestMsg) ValidateBasic() error { return nil }
 
 var _ sdk.Msg = &MsgCreateDog{}
 
-func (msg *MsgCreateDog) GetSigners() []sdk.AccAddress { return []sdk.AccAddress{} }
-func (msg *MsgCreateDog) ValidateBasic() error         { return nil }
+func (msg *MsgCreateDog) GetSigners() []string { return []string{} }
+func (msg *MsgCreateDog) ValidateBasic() error { return nil }

types/tx/types.go

@@ -101,9 +101,14 @@ func (t *Tx) GetSigners() []sdk.AccAddress {
 
 	for _, msg := range t.GetMsgs() {
 		for _, addr := range msg.GetSigners() {
-			if !seen[addr.String()] {
-				signers = append(signers, addr)
-				seen[addr.String()] = true
+			if !seen[addr] {
+				signer, err := sdk.AccAddressFromBech32(addr)
+				if err != nil {
+					panic(err)
+				}
+
+				signers = append(signers, signer)
+				seen[addr] = true
 			}
 		}
 	}
@@ -202,3 +207,22 @@ func RegisterInterfaces(registry codectypes.InterfaceRegistry) {
 	registry.RegisterInterface("cosmos.tx.v1beta1.Tx", (*sdk.Tx)(nil))
 	registry.RegisterImplementations((*sdk.Tx)(nil), &Tx{})
 }
+
+// ValidateMsg calls the `sdk.Msg.ValidateBasic()`
+// also validates all the signers are valid bech32 addresses.
+func ValidateMsg(msg sdk.Msg) error {
+	err := msg.ValidateBasic()
+	if err != nil {
+		return err
+	}
+
+	signers := msg.GetSigners()
+	for _, signer := range signers {
+		_, err = sdk.AccAddressFromBech32(signer)
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}

types/tx/types_test.go

@@ -0,0 +1,43 @@
+package tx_test
+
+import (
+	"testing"
+
+	"github.com/cosmos/cosmos-sdk/testutil/testdata"
+	"github.com/cosmos/cosmos-sdk/types/tx"
+	"github.com/stretchr/testify/suite"
+)
+
+type testMsgSuite struct {
+	suite.Suite
+}
+
+func TestValidateMsg(t *testing.T) {
+	suite.Run(t, new(testMsgSuite))
+}
+
+func (s *testMsgSuite) TestMsg() {
+	cases := []struct {
+		signer []byte
+		expErr bool
+	}{
+		{
+			[]byte(""),
+			true,
+		},
+		{
+			[]byte("validAddress"),
+			false,
+		},
+	}
+
+	for _, c := range cases {
+		msg := testdata.NewTestMsg(c.signer)
+		err := tx.ValidateMsg(msg)
+		if c.expErr {
+			s.Require().Error(err)
+		} else {
+			s.Require().NoError(err)
+		}
+	}
+}

types/tx_msg.go

@@ -15,10 +15,10 @@ type (
 		// doesn't require access to any other information.
 		ValidateBasic() error
 
-		// Signers returns the addrs of signers that must sign.
+		// Signers returns the bech32-encoded addrs of signers that must sign.
 		// CONTRACT: All signatures must be present to be valid.
 		// CONTRACT: Returns addrs in some deterministic order.
-		GetSigners() []AccAddress
+		GetSigners() []string
 	}
 
 	// Fee defines an interface for an application application-defined concrete

types/tx_msg_test.go

@@ -23,7 +23,7 @@ func (s *testMsgSuite) TestMsg() {
 
 	msg := testdata.NewTestMsg(accAddr)
 	s.Require().NotNil(msg)
-	s.Require().Equal([]sdk.AccAddress{accAddr}, msg.GetSigners())
+	s.Require().Equal([]string{accAddr.String()}, msg.GetSigners())
 	s.Require().Equal("TestMsg", msg.Route())
 	s.Require().Equal("Test message", msg.Type())
 	s.Require().Nil(msg.ValidateBasic())

x/auth/ante/basic.go

@@ -11,7 +11,8 @@ import (
 	authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing"
 )
 
-// ValidateBasicDecorator will call tx.ValidateBasic and return any non-nil error.
+// ValidateBasicDecorator will call tx.ValidateBasic, ValidateMsg(for each msg inside tx)
+// and return any non-nil error.
 // If ValidateBasic passes, decorator calls next AnteHandler in chain. Note,
 // ValidateBasicDecorator decorator will not get executed on ReCheckTx since it
 // is not dependent on application state.

x/auth/migrations/legacytx/stdtx.go

@@ -143,9 +143,13 @@ func (tx StdTx) GetSigners() []sdk.AccAddress {
 
 	for _, msg := range tx.GetMsgs() {
 		for _, addr := range msg.GetSigners() {
-			if !seen[addr.String()] {
-				signers = append(signers, addr)
-				seen[addr.String()] = true
+			if !seen[addr] {
+				signer, err := sdk.AccAddressFromBech32(addr)
+				if err != nil {
+					panic(err)
+				}
+				signers = append(signers, signer)
+				seen[addr] = true
 			}
 		}
 	}

x/auth/vesting/types/msgs.go

@@ -68,10 +68,6 @@ func (msg MsgCreateVestingAccount) GetSignBytes() []byte {
 }
 
 // GetSigners returns the expected signers for a MsgCreateVestingAccount.
-func (msg MsgCreateVestingAccount) GetSigners() []sdk.AccAddress {
-	from, err := sdk.AccAddressFromBech32(msg.FromAddress)
-	if err != nil {
-		panic(err)
-	}
-	return []sdk.AccAddress{from}
+func (msg MsgCreateVestingAccount) GetSigners() []string {
+	return []string{msg.FromAddress}
 }

x/authz/keeper/keeper.go

@@ -79,7 +79,10 @@ func (k Keeper) DispatchActions(ctx sdk.Context, grantee sdk.AccAddress, msgs []
 		if len(signers) != 1 {
 			return nil, sdkerrors.ErrInvalidRequest.Wrap("authorization can be given to msg with only one signer")
 		}
-		granter := signers[0]
+		granter, err := sdk.AccAddressFromBech32(signers[0])
+		if err != nil {
+			return nil, err
+		}
 		// if granter != grantee then check authorization.Accept, otherwise we implicitly accept.
 		if !granter.Equals(grantee) {
 			authorization, _ := k.GetCleanAuthorization(ctx, grantee, granter, sdk.MsgTypeURL(msg))

x/authz/msgs.go

@@ -42,12 +42,8 @@ func NewMsgGrant(granter sdk.AccAddress, grantee sdk.AccAddress, a Authorization
 }
 
 // GetSigners implements Msg
-func (msg MsgGrant) GetSigners() []sdk.AccAddress {
-	granter, err := sdk.AccAddressFromBech32(msg.Granter)
-	if err != nil {
-		panic(err)
-	}
-	return []sdk.AccAddress{granter}
+func (msg MsgGrant) GetSigners() []string {
+	return []string{msg.Granter}
 }
 
 // ValidateBasic implements Msg
@@ -130,12 +126,8 @@ func NewMsgRevoke(granter sdk.AccAddress, grantee sdk.AccAddress, msgTypeURL str
 }
 
 // GetSigners implements Msg
-func (msg MsgRevoke) GetSigners() []sdk.AccAddress {
-	granter, err := sdk.AccAddressFromBech32(msg.Granter)
-	if err != nil {
-		panic(err)
-	}
-	return []sdk.AccAddress{granter}
+func (msg MsgRevoke) GetSigners() []string {
+	return []string{msg.Granter}
 }
 
 // ValidateBasic implements MsgRequest.ValidateBasic
@@ -209,12 +201,8 @@ func (msg MsgExec) GetMessages() ([]sdk.Msg, error) {
 }
 
 // GetSigners implements Msg
-func (msg MsgExec) GetSigners() []sdk.AccAddress {
-	grantee, err := sdk.AccAddressFromBech32(msg.Grantee)
-	if err != nil {
-		panic(err)
-	}
-	return []sdk.AccAddress{grantee}
+func (msg MsgExec) GetSigners() []string {
+	return []string{msg.Grantee}
 }
 
 // ValidateBasic implements Msg

x/bank/types/msgs.go

@@ -54,12 +54,8 @@ func (msg MsgSend) GetSignBytes() []byte {
 }
 
 // GetSigners Implements Msg.
-func (msg MsgSend) GetSigners() []sdk.AccAddress {
-	from, err := sdk.AccAddressFromBech32(msg.FromAddress)
-	if err != nil {
-		panic(err)
-	}
-	return []sdk.AccAddress{from}
+func (msg MsgSend) GetSigners() []string {
+	return []string{msg.FromAddress}
 }
 
 var _ sdk.Msg = &MsgMultiSend{}
@@ -96,11 +92,10 @@ func (msg MsgMultiSend) GetSignBytes() []byte {
 }
 
 // GetSigners Implements Msg.
-func (msg MsgMultiSend) GetSigners() []sdk.AccAddress {
-	addrs := make([]sdk.AccAddress, len(msg.Inputs))
+func (msg MsgMultiSend) GetSigners() []string {
+	addrs := make([]string, len(msg.Inputs))
 	for i, in := range msg.Inputs {
-		addr, _ := sdk.AccAddressFromBech32(in.Address)
-		addrs[i] = addr
+		addrs[i] = in.Address
 	}
 
 	return addrs