refactor!: turn MsgsV2 into ReflectMessages to make it less confusing (…

…#19839)

Co-authored-by: marbar3778 <marbar3778@yahoo.com>
Co-authored-by: sontrinh16 <trinhleson2000@gmail.com>
Co-authored-by: Marko <marko@baricevic.me>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

CHANGELOG.md

@@ -181,6 +181,7 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i
     * Every module has the codec already, passing it created an unneeded dependency.
     * Additionally, to reflect this change, the module manager does not take a codec either.
 * (runtime) [#19747](https://github.com/cosmos/cosmos-sdk/pull/19747) `runtime.ValidatorAddressCodec` and `runtime.ConsensusAddressCodec` have been moved to `core`.
+* [#19839](https://github.com/cosmos/cosmos-sdk/pull/19839) `Tx.GetMsgsV2` has been replaced with `Tx.GetReflectMessages`, and `Codec.GetMsgV1Signers` and `Codec.GetMsgV2Signers` have been replaced with `GetMsgSigners` and `GetReflectMsgSigners` respectively. These API changes clear up confusion as to the use and purpose of these methods.
 * (baseapp) [#19993](https://github.com/cosmos/cosmos-sdk/pull/19993) Indicate pruning with error code "not found" rather than "invalid request".
 * (x/consensus) [#20010](https://github.com/cosmos/cosmos-sdk/pull/20010) Move consensus module to be its own go.mod
 * (server) [#20140](https://github.com/cosmos/cosmos-sdk/pull/20140) Remove embedded grpc-web proxy in favor of standalone grpc-web proxy. [Envoy Proxy](https://www.envoyproxy.io/docs/envoy/latest/start/start)

baseapp/baseapp.go

@@ -15,7 +15,7 @@ import (
 	dbm "github.com/cosmos/cosmos-db"
 	"github.com/cosmos/gogoproto/proto"
 	"golang.org/x/exp/maps"
-	protov2 "google.golang.org/protobuf/proto"
+	"google.golang.org/protobuf/reflect/protoreflect"
 
 	"cosmossdk.io/core/header"
 	errorsmod "cosmossdk.io/errors"
@@ -950,9 +950,9 @@ func (app *BaseApp) runTx(mode execMode, txBytes []byte) (gInfo sdk.GasInfo, res
 	// Attempt to execute all messages and only update state if all messages pass
 	// and we're in DeliverTx. Note, runMsgs will never return a reference to a
 	// Result if any single message fails or does not have a registered Handler.
-	msgsV2, err := tx.GetMsgsV2()
+	reflectMsgs, err := tx.GetReflectMessages()
 	if err == nil {
-		result, err = app.runMsgs(runMsgCtx, msgs, msgsV2, mode)
+		result, err = app.runMsgs(runMsgCtx, msgs, reflectMsgs, mode)
 	}
 
 	// Run optional postHandlers (should run regardless of the execution result).
@@ -998,7 +998,7 @@ func (app *BaseApp) runTx(mode execMode, txBytes []byte) (gInfo sdk.GasInfo, res
 // and DeliverTx. An error is returned if any single message fails or if a
 // Handler does not exist for a given message route. Otherwise, a reference to a
 // Result is returned. The caller must not commit state if an error is returned.
-func (app *BaseApp) runMsgs(ctx sdk.Context, msgs []sdk.Msg, msgsV2 []protov2.Message, mode execMode) (*sdk.Result, error) {
+func (app *BaseApp) runMsgs(ctx sdk.Context, msgs []sdk.Msg, reflectMsgs []protoreflect.Message, mode execMode) (*sdk.Result, error) {
 	events := sdk.EmptyEvents()
 	msgResponses := make([]*codectypes.Any, 0, len(msgs))
 
@@ -1020,7 +1020,7 @@ func (app *BaseApp) runMsgs(ctx sdk.Context, msgs []sdk.Msg, msgsV2 []protov2.Me
 		}
 
 		// create message events
-		msgEvents, err := createEvents(app.cdc, msgResult.GetEvents(), msg, msgsV2[i])
+		msgEvents, err := createEvents(app.cdc, msgResult.GetEvents(), msg, reflectMsgs[i])
 		if err != nil {
 			return nil, errorsmod.Wrapf(err, "failed to create message events; message index: %d", i)
 		}
@@ -1066,12 +1066,12 @@ func makeABCIData(msgResponses []*codectypes.Any) ([]byte, error) {
 	return proto.Marshal(&sdk.TxMsgData{MsgResponses: msgResponses})
 }
 
-func createEvents(cdc codec.Codec, events sdk.Events, msg sdk.Msg, msgV2 protov2.Message) (sdk.Events, error) {
+func createEvents(cdc codec.Codec, events sdk.Events, msg sdk.Msg, reflectMsg protoreflect.Message) (sdk.Events, error) {
 	eventMsgName := sdk.MsgTypeURL(msg)
 	msgEvent := sdk.NewEvent(sdk.EventTypeMessage, sdk.NewAttribute(sdk.AttributeKeyAction, eventMsgName))
 
 	// we set the signer attribute as the sender
-	signers, err := cdc.GetMsgV2Signers(msgV2)
+	signers, err := cdc.GetReflectMsgSigners(reflectMsg)
 	if err != nil {
 		return nil, err
 	}

codec/codec.go

@@ -3,7 +3,7 @@ package codec
 import (
 	"github.com/cosmos/gogoproto/proto"
 	"google.golang.org/grpc/encoding"
-	protov2 "google.golang.org/protobuf/proto"
+	"google.golang.org/protobuf/reflect/protoreflect"
 
 	"github.com/cosmos/cosmos-sdk/codec/types"
 )
@@ -24,17 +24,19 @@ type (
 		InterfaceRegistry() types.InterfaceRegistry
 
 		// GetMsgAnySigners returns the signers of the given message encoded in a protobuf Any
-		// as well as the decoded google.golang.org/protobuf/proto.Message that was used to
-		// extract the signers so that this can be used in other contexts.
-		GetMsgAnySigners(msg *types.Any) ([][]byte, protov2.Message, error)
-
-		// GetMsgV2Signers returns the signers of the given message.
-		GetMsgV2Signers(msg protov2.Message) ([][]byte, error)
-
-		// GetMsgV1Signers returns the signers of the given message plus the
-		// decoded google.golang.org/protobuf/proto.Message that was used to extract the
-		// signers so that this can be used in other contexts.
-		GetMsgV1Signers(msg proto.Message) ([][]byte, protov2.Message, error)
+		// as well as the decoded protoreflect.Message that was used to extract the
+		// signers so that this can be used in other context where proto reflection
+		// is needed.
+		GetMsgAnySigners(msg *types.Any) ([][]byte, protoreflect.Message, error)
+
+		// GetMsgSigners returns the signers of the given message plus the
+		// decoded protoreflect.Message that was used to extract the
+		// signers so that this can be used in other context where proto reflection
+		// is needed.
+		GetMsgSigners(msg proto.Message) ([][]byte, protoreflect.Message, error)
+
+		// GetReflectMsgSigners returns the signers of the given reflected proto message.
+		GetReflectMsgSigners(msg protoreflect.Message) ([][]byte, error)
 
 		// mustEmbedCodec requires that all implementations of Codec embed an official implementation from the codec
 		// package. This allows new methods to be added to the Codec interface without breaking backwards compatibility.

codec/proto_codec.go

@@ -299,7 +299,7 @@ func (pc *ProtoCodec) InterfaceRegistry() types.InterfaceRegistry {
 	return pc.interfaceRegistry
 }
 
-func (pc ProtoCodec) GetMsgAnySigners(msg *types.Any) ([][]byte, proto.Message, error) {
+func (pc ProtoCodec) GetMsgAnySigners(msg *types.Any) ([][]byte, protoreflect.Message, error) {
 	msgv2, err := anyutil.Unpack(&anypb.Any{
 		TypeUrl: msg.TypeUrl,
 		Value:   msg.Value,
@@ -309,17 +309,17 @@ func (pc ProtoCodec) GetMsgAnySigners(msg *types.Any) ([][]byte, proto.Message,
 	}
 
 	signers, err := pc.interfaceRegistry.SigningContext().GetSigners(msgv2)
-	return signers, msgv2, err
+	return signers, msgv2.ProtoReflect(), err
 }
 
-func (pc *ProtoCodec) GetMsgV2Signers(msg proto.Message) ([][]byte, error) {
-	return pc.interfaceRegistry.SigningContext().GetSigners(msg)
+func (pc *ProtoCodec) GetReflectMsgSigners(msg protoreflect.Message) ([][]byte, error) {
+	return pc.interfaceRegistry.SigningContext().GetSigners(msg.Interface())
 }
 
-func (pc *ProtoCodec) GetMsgV1Signers(msg gogoproto.Message) ([][]byte, proto.Message, error) {
+func (pc *ProtoCodec) GetMsgSigners(msg gogoproto.Message) ([][]byte, protoreflect.Message, error) {
 	if msgV2, ok := msg.(proto.Message); ok {
 		signers, err := pc.interfaceRegistry.SigningContext().GetSigners(msgV2)
-		return signers, msgV2, err
+		return signers, msgV2.ProtoReflect(), err
 	}
 	a, err := types.NewAnyWithValue(msg)
 	if err != nil {

codec/proto_codec_test.go

@@ -191,12 +191,12 @@ func TestGetSigners(t *testing.T) {
 	msgSendV1 := &countertypes.MsgIncreaseCounter{Signer: testAddrStr, Count: 1}
 	msgSendV2 := &counterv1.MsgIncreaseCounter{Signer: testAddrStr, Count: 1}
 
-	signers, msgSendV2Copy, err := cdc.GetMsgV1Signers(msgSendV1)
+	signers, msgSendV2Copy, err := cdc.GetMsgSigners(msgSendV1)
 	require.NoError(t, err)
 	require.Equal(t, [][]byte{testAddr}, signers)
-	require.True(t, protov2.Equal(msgSendV2, msgSendV2Copy))
+	require.True(t, protov2.Equal(msgSendV2, msgSendV2Copy.Interface()))
 
-	signers, err = cdc.GetMsgV2Signers(msgSendV2)
+	signers, err = cdc.GetReflectMsgSigners(msgSendV2.ProtoReflect())
 	require.NoError(t, err)
 	require.Equal(t, [][]byte{testAddr}, signers)
 
@@ -205,7 +205,7 @@ func TestGetSigners(t *testing.T) {
 	signers, msgSendV2Copy, err = cdc.GetMsgAnySigners(msgSendAny)
 	require.NoError(t, err)
 	require.Equal(t, [][]byte{testAddr}, signers)
-	require.True(t, protov2.Equal(msgSendV2, msgSendV2Copy))
+	require.True(t, protov2.Equal(msgSendV2, msgSendV2Copy.Interface()))
 }
 
 type testAddressCodec struct{}

server/mock/tx.go

@@ -4,7 +4,7 @@ import (
 	"bytes"
 	"fmt"
 
-	protov2 "google.golang.org/protobuf/proto"
+	"google.golang.org/protobuf/reflect/protoreflect"
 
 	bankv1beta1 "cosmossdk.io/api/cosmos/bank/v1beta1"
 	errorsmod "cosmossdk.io/errors"
@@ -103,8 +103,8 @@ func (msg *KVStoreTx) GetMsgs() []sdk.Msg {
 	return []sdk.Msg{msg}
 }
 
-func (msg *KVStoreTx) GetMsgsV2() ([]protov2.Message, error) {
-	return []protov2.Message{&bankv1beta1.MsgSend{FromAddress: msg.address.String()}}, nil // this is a hack for tests
+func (msg *KVStoreTx) GetReflectMessages() ([]protoreflect.Message, error) {
+	return []protoreflect.Message{(&bankv1beta1.MsgSend{FromAddress: msg.address.String()}).ProtoReflect()}, nil // this is a hack for tests
 }
 
 func (msg *KVStoreTx) GetSignBytes() []byte {

types/mempool/mempool_test.go

@@ -7,7 +7,7 @@ import (
 
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
-	protov2 "google.golang.org/protobuf/proto"
+	"google.golang.org/protobuf/reflect/protoreflect"
 
 	_ "cosmossdk.io/api/cosmos/counter/v1"
 	_ "cosmossdk.io/api/cosmos/crypto/secp256k1"
@@ -77,7 +77,7 @@ var (
 
 func (tx testTx) GetMsgs() []sdk.Msg { return nil }
 
-func (tx testTx) GetMsgsV2() ([]protov2.Message, error) { return nil, nil }
+func (tx testTx) GetReflectMessages() ([]protoreflect.Message, error) { return nil, nil }
 
 func (tx testTx) ValidateBasic() error { return nil }
 
@@ -93,7 +93,7 @@ func (sigErrTx) Size() int64 { return 0 }
 
 func (sigErrTx) GetMsgs() []sdk.Msg { return nil }
 
-func (sigErrTx) GetMsgsV2() ([]protov2.Message, error) { return nil, nil }
+func (sigErrTx) GetReflectMessages() ([]protoreflect.Message, error) { return nil, nil }
 
 func (sigErrTx) ValidateBasic() error { return nil }
 

types/mempool/signer_extraction_adapater_test.go

@@ -6,7 +6,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
-	"google.golang.org/protobuf/proto"
+	"google.golang.org/protobuf/reflect/protoreflect"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/types/mempool"
@@ -20,7 +20,7 @@ func (n nonVerifiableTx) GetMsgs() []sdk.Msg {
 	panic("not implemented")
 }
 
-func (n nonVerifiableTx) GetMsgsV2() ([]proto.Message, error) {
+func (n nonVerifiableTx) GetReflectMessages() ([]protoreflect.Message, error) {
 	panic("not implemented")
 }
 

types/tx/types.go

@@ -3,7 +3,7 @@ package tx
 import (
 	"fmt"
 
-	protov2 "google.golang.org/protobuf/proto"
+	"google.golang.org/protobuf/reflect/protoreflect"
 
 	errorsmod "cosmossdk.io/errors"
 
@@ -97,18 +97,18 @@ func (t *Tx) ValidateBasic() error {
 // GetSigners retrieves all the signers of a tx.
 // This includes all unique signers of the messages (in order),
 // as well as the FeePayer (if specified and not already included).
-func (t *Tx) GetSigners(cdc codec.Codec) ([][]byte, []protov2.Message, error) {
+func (t *Tx) GetSigners(cdc codec.Codec) ([][]byte, []protoreflect.Message, error) {
 	var signers [][]byte
 	seen := map[string]bool{}
 
-	var msgsv2 []protov2.Message
+	var reflectMsgs []protoreflect.Message
 	for _, msg := range t.Body.Messages {
-		xs, msgv2, err := cdc.GetMsgAnySigners(msg)
+		xs, reflectMsg, err := cdc.GetMsgAnySigners(msg)
 		if err != nil {
 			return nil, nil, err
 		}
 
-		msgsv2 = append(msgsv2, msgv2)
+		reflectMsgs = append(reflectMsgs, reflectMsg)
 
 		for _, signer := range xs {
 			if !seen[string(signer)] {
@@ -133,7 +133,7 @@ func (t *Tx) GetSigners(cdc codec.Codec) ([][]byte, []protov2.Message, error) {
 		seen[string(feePayerAddr)] = true
 	}
 
-	return signers, msgsv2, nil
+	return signers, reflectMsgs, nil
 }
 
 func (t *Tx) GetGas() uint64 {

types/tx_msg.go

@@ -5,7 +5,7 @@ import (
 	"fmt"
 	"strings"
 
-	protov2 "google.golang.org/protobuf/proto"
+	"google.golang.org/protobuf/reflect/protoreflect"
 
 	coretransaction "cosmossdk.io/core/transaction"
 
@@ -53,8 +53,13 @@ type (
 	Tx interface {
 		HasMsgs
 
-		// GetMsgsV2 gets the transaction's messages as google.golang.org/protobuf/proto.Message's.
-		GetMsgsV2() ([]protov2.Message, error)
+		// GetReflectMessages gets a reflected version of the transaction's messages
+		// that can be used by dynamic APIs. These messages should not be used for actual
+		// processing as they cannot be guaranteed to be what handlers are expecting, but
+		// they can be used for dynamically reading specific fields from the message such
+		// as signers or validation data. Message processors should ALWAYS use the messages
+		// returned by GetMsgs.
+		GetReflectMessages() ([]protoreflect.Message, error)
 	}
 
 	// FeeTx defines the interface to be implemented by Tx to use the FeeDecorators

x/accounts/defaults/multisig/utils_test.go

@@ -36,17 +36,17 @@ type mockStateCodec struct {
 }
 
 // GetMsgAnySigners implements codec.Codec.
-func (mockStateCodec) GetMsgAnySigners(msg *types.Any) ([][]byte, protoreflect.ProtoMessage, error) {
+func (mockStateCodec) GetMsgAnySigners(msg *types.Any) ([][]byte, protoreflect.Message, error) {
 	panic("unimplemented")
 }
 
 // GetMsgV1Signers implements codec.Codec.
-func (mockStateCodec) GetMsgV1Signers(msg gogoproto.Message) ([][]byte, protoreflect.ProtoMessage, error) {
+func (mockStateCodec) GetMsgSigners(msg gogoproto.Message) ([][]byte, protoreflect.Message, error) {
 	panic("unimplemented")
 }
 
 // GetMsgV2Signers implements codec.Codec.
-func (mockStateCodec) GetMsgV2Signers(msg protoreflect.ProtoMessage) ([][]byte, error) {
+func (mockStateCodec) GetReflectMsgSigners(msg protoreflect.Message) ([][]byte, error) {
 	panic("unimplemented")
 }
 

x/accounts/keeper.go

@@ -325,7 +325,7 @@ func (k Keeper) sendAnyMessages(ctx context.Context, sender []byte, anyMessages
 // It should be used when the response type is not known by the caller.
 func (k Keeper) SendModuleMessageUntyped(ctx context.Context, sender []byte, msg implementation.ProtoMsg) (implementation.ProtoMsg, error) {
 	// do sender assertions.
-	wantSenders, _, err := k.codec.GetMsgV1Signers(msg)
+	wantSenders, _, err := k.codec.GetMsgSigners(msg)
 	if err != nil {
 		return nil, fmt.Errorf("cannot get signers: %w", err)
 	}
@@ -348,7 +348,7 @@ func (k Keeper) SendModuleMessageUntyped(ctx context.Context, sender []byte, msg
 // is not trying to impersonate another account.
 func (k Keeper) sendModuleMessage(ctx context.Context, sender []byte, msg, msgResp implementation.ProtoMsg) error {
 	// do sender assertions.
-	wantSenders, _, err := k.codec.GetMsgV1Signers(msg)
+	wantSenders, _, err := k.codec.GetMsgSigners(msg)
 	if err != nil {
 		return fmt.Errorf("cannot get signers: %w", err)
 	}

x/accounts/utils_test.go

@@ -85,17 +85,17 @@ type bankQueryServer struct {
 	bankv1beta1.UnimplementedQueryServer
 }
 
+type bankMsgServer struct {
+	bankv1beta1.UnimplementedMsgServer
+}
+
 func (b bankQueryServer) Balance(context.Context, *bankv1beta1.QueryBalanceRequest) (*bankv1beta1.QueryBalanceResponse, error) {
 	return &bankv1beta1.QueryBalanceResponse{Balance: &basev1beta1.Coin{
 		Denom:  "atom",
 		Amount: "1000",
 	}}, nil
 }
 
-type bankMsgServer struct {
-	bankv1beta1.UnimplementedMsgServer
-}
-
 func (b bankMsgServer) Send(context.Context, *bankv1beta1.MsgSend) (*bankv1beta1.MsgSendResponse, error) {
 	return &bankv1beta1.MsgSendResponse{}, nil
 }

x/auth/tx/builder.go

@@ -56,7 +56,7 @@ func newBuilderFromDecodedTx(addrCodec address.Codec, decoder *decode.Decoder, c
 		addressCodec:                addrCodec,
 		decoder:                     decoder,
 		codec:                       codec,
-		msgs:                        decoded.msgsV1,
+		msgs:                        decoded.msgs,
 		timeoutHeight:               decoded.GetTimeoutHeight(),
 		granter:                     decoded.FeeGranter(),
 		payer:                       payer,