refactor(x/tx): use dynamicpb instead of type resolver in any marshal (#16048)

Co-authored-by: Aaron Craelius <aaron@regen.network>
This commit is contained in:
Matt Kocubinski 2023-05-08 14:20:23 -05:00 committed by GitHub
parent 12923c33ed
commit 2f21cb5050
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
17 changed files with 181 additions and 49 deletions

View File

@ -11,8 +11,12 @@ import (
gogoproto "github.com/cosmos/gogoproto/proto"
"google.golang.org/grpc/encoding"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/reflect/protoreflect"
"google.golang.org/protobuf/reflect/protoregistry"
"google.golang.org/protobuf/types/dynamicpb"
"google.golang.org/protobuf/types/known/anypb"
"cosmossdk.io/x/tx/signing/aminojson"
"github.com/cosmos/cosmos-sdk/codec/types"
)
@ -160,6 +164,41 @@ func (pc *ProtoCodec) MustMarshalJSON(o gogoproto.Message) []byte {
return bz
}
// MarshalAminoJSON provides aminojson.Encoder compatibility for gogoproto messages.
// x/tx/signing/aminojson cannot marshal gogoproto messages directly since this type does not implement
// the standard library google.golang.org/protobuf/proto.Message.
// We convert gogo types to dynamicpb messages and then marshal that directly to amino JSON.
func (pc *ProtoCodec) MarshalAminoJSON(msg gogoproto.Message) ([]byte, error) {
encoder := aminojson.NewEncoder(aminojson.EncoderOptions{FileResolver: pc.interfaceRegistry})
gogoBytes, err := gogoproto.Marshal(msg)
if err != nil {
return nil, err
}
var protoMsg protoreflect.ProtoMessage
typ, err := protoregistry.GlobalTypes.FindMessageByURL(fmt.Sprintf("/%s", gogoproto.MessageName(msg)))
if typ != nil && err != nil {
protoMsg = typ.New().Interface()
} else {
desc, err := pc.interfaceRegistry.FindDescriptorByName(protoreflect.FullName(gogoproto.MessageName(msg)))
if err != nil {
return nil, err
}
dynamicMsgType := dynamicpb.NewMessageType(desc.(protoreflect.MessageDescriptor))
protoMsg = dynamicMsgType.New().Interface()
}
err = proto.Unmarshal(gogoBytes, protoMsg)
if err != nil {
return nil, err
}
jsonBytes, err := encoder.Marshal(protoMsg)
if err != nil {
return nil, err
}
return jsonBytes, nil
}
// UnmarshalJSON implements JSONCodec.UnmarshalJSON method,
// it unmarshals from JSON using proto codec.
// NOTE: this function must be used with a concrete type which

4
go.mod
View File

@ -11,7 +11,7 @@ require (
cosmossdk.io/log v1.1.0
cosmossdk.io/math v1.0.0
cosmossdk.io/store v0.1.0-alpha.1.0.20230328185921-37ba88872dbc
cosmossdk.io/x/tx v0.6.1
cosmossdk.io/x/tx v0.6.3
github.com/99designs/keyring v1.2.1
github.com/armon/go-metrics v0.4.1
github.com/bgentry/speakeasy v0.1.1-0.20220910012023-760eaf8b6816
@ -164,7 +164,7 @@ require (
replace (
cosmossdk.io/core => ./core
cosmossdk.io/store => ./store
// TODO: remove after release 0.6.2
// TODO: remove after 0.7.0 release
cosmossdk.io/x/tx => ./x/tx
// use cosmos fork of keyring
github.com/99designs/keyring => github.com/cosmos/keyring v1.2.0

View File

@ -38,7 +38,7 @@ require (
cloud.google.com/go/storage v1.30.0 // indirect
cosmossdk.io/collections v0.1.0 // indirect
cosmossdk.io/errors v1.0.0-beta.7.0.20230429155654-3ee8242364e4 // indirect
cosmossdk.io/x/tx v0.6.1 // indirect
cosmossdk.io/x/tx v0.6.3 // indirect
filippo.io/edwards25519 v1.0.0 // indirect
github.com/99designs/go-keychain v0.0.0-20191008050251-8e49817e8af4 // indirect
github.com/99designs/keyring v1.2.1 // indirect
@ -204,14 +204,14 @@ replace (
cosmossdk.io/x/evidence => ../x/evidence
cosmossdk.io/x/feegrant => ../x/feegrant
cosmossdk.io/x/nft => ../x/nft
// TODO: remove after release 0.6.2
cosmossdk.io/x/tx => ../x/tx
cosmossdk.io/x/upgrade => ../x/upgrade
)
// Below are the long-lived replace of the SimApp
replace (
cosmossdk.io/core => ../core
// TODO: remove after 0.7.0 release
cosmossdk.io/x/tx => ../x/tx
// use cosmos fork of keyring
github.com/99designs/keyring => github.com/cosmos/keyring v1.2.0
// Simapp always use the latest version of the cosmos-sdk

View File

@ -14,7 +14,7 @@ require (
cosmossdk.io/x/evidence v0.1.0
cosmossdk.io/x/feegrant v0.0.0-20230117113717-50e7c4a4ceff
cosmossdk.io/x/nft v0.0.0-20230113085233-fae3332d62fc
cosmossdk.io/x/tx v0.6.1
cosmossdk.io/x/tx v0.6.3
cosmossdk.io/x/upgrade v0.0.0-20230127052425-54c8e1568335
github.com/cometbft/cometbft v0.37.1
github.com/cosmos/cosmos-db v1.0.0-rc.1
@ -200,14 +200,13 @@ replace (
cosmossdk.io/x/upgrade => ../x/upgrade
)
// temporary replace
replace cosmossdk.io/x/tx => ../x/tx
// Below are the long-lived replace for tests.
replace (
cosmossdk.io/core => ../core
// We always want to test against the latest version of the simapp.
cosmossdk.io/simapp => ../simapp
// TODO: remove after 0.7.0 release
cosmossdk.io/x/tx => ../x/tx
github.com/99designs/keyring => github.com/cosmos/keyring v1.2.0
// We always want to test against the latest version of the SDK.
github.com/cosmos/cosmos-sdk => ../.

View File

@ -47,6 +47,7 @@ import (
"github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/bech32"
"github.com/cosmos/cosmos-sdk/types/module/testutil"
txtypes "github.com/cosmos/cosmos-sdk/types/tx"
signingtypes "github.com/cosmos/cosmos-sdk/types/tx/signing"
"github.com/cosmos/cosmos-sdk/x/auth"
"github.com/cosmos/cosmos-sdk/x/auth/signing"
@ -92,7 +93,7 @@ func TestAminoJSON_Equivalence(t *testing.T) {
distribution.AppModuleBasic{}, evidence.AppModuleBasic{}, feegrantmodule.AppModuleBasic{},
gov.AppModuleBasic{}, groupmodule.AppModuleBasic{}, mint.AppModuleBasic{}, params.AppModuleBasic{},
slashing.AppModuleBasic{}, staking.AppModuleBasic{}, upgrade.AppModuleBasic{}, vesting.AppModuleBasic{})
aj := aminojson.NewEncoder()
aj := aminojson.NewEncoder(aminojson.EncoderOptions{})
for _, tt := range rapidgen.DefaultGeneratedTypes {
name := string(tt.Pulsar.ProtoReflect().Descriptor().FullName())
@ -143,6 +144,10 @@ func TestAminoJSON_Equivalence(t *testing.T) {
AccNum: 1,
AccSeq: 2,
SignerAddress: "signerAddress",
Tip: &txv1beta1.Tip{
Tipper: "tipper",
Amount: []*v1beta1.Coin{{Denom: "uatom", Amount: "1000"}},
},
Fee: &txv1beta1.Fee{
Amount: []*v1beta1.Coin{{Denom: "uatom", Amount: "1000"}},
},
@ -160,6 +165,10 @@ func TestAminoJSON_Equivalence(t *testing.T) {
require.NoError(t, txBuilder.SetMsgs([]types.Msg{gogoMsg}...))
txBuilder.SetMemo(handlerOptions.Memo)
txBuilder.SetFeeAmount(types.Coins{types.NewInt64Coin("uatom", 1000)})
txBuilder.SetTip(&txtypes.Tip{
Amount: types.Coins{types.NewInt64Coin("uatom", 1000)},
Tipper: "tipper",
})
theTx := txBuilder.GetTx()
legacySigningData := signing.SignerData{
@ -193,7 +202,7 @@ func TestAminoJSON_LegacyParity(t *testing.T) {
bank.AppModuleBasic{}, distribution.AppModuleBasic{}, slashing.AppModuleBasic{}, staking.AppModuleBasic{},
vesting.AppModuleBasic{})
aj := aminojson.NewEncoder()
aj := aminojson.NewEncoder(aminojson.EncoderOptions{})
addr1 := types.AccAddress("addr1")
now := time.Now()
@ -481,7 +490,7 @@ func TestSendAuthorization(t *testing.T) {
encCfg := testutil.MakeTestEncodingConfig(auth.AppModuleBasic{}, authzmodule.AppModuleBasic{},
distribution.AppModuleBasic{}, bank.AppModuleBasic{})
aj := aminojson.NewEncoder()
aj := aminojson.NewEncoder(aminojson.EncoderOptions{})
// beware, Coins has as custom MarshalJSON method which changes how nil is handled
// nil -> [] (empty list)

View File

@ -17,7 +17,7 @@ import (
func TestRepeatedFields(t *testing.T) {
cdc := codec.NewLegacyAmino()
aj := aminojson.NewEncoder()
aj := aminojson.NewEncoder(aminojson.EncoderOptions{})
cases := map[string]struct {
gogo gogoproto.Message

View File

@ -23,7 +23,7 @@ require (
cosmossdk.io/depinject v1.0.0-alpha.3 // indirect
cosmossdk.io/errors v1.0.0-beta.7.0.20230429155654-3ee8242364e4 // indirect
cosmossdk.io/store v0.1.0-alpha.1.0.20230328185921-37ba88872dbc // indirect
cosmossdk.io/x/tx v0.6.1 // indirect
cosmossdk.io/x/tx v0.6.3 // indirect
filippo.io/edwards25519 v1.0.0 // indirect
github.com/99designs/go-keychain v0.0.0-20191008050251-8e49817e8af4 // indirect
github.com/99designs/keyring v1.2.1 // indirect

View File

@ -2,14 +2,15 @@ package simulation
import (
"encoding/json"
"fmt"
"math/rand"
"time"
"github.com/cosmos/cosmos-sdk/baseapp"
"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/codec/types"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/kv"
"github.com/cosmos/cosmos-sdk/x/auth/migrations/legacytx"
)
// Deprecated: Use WeightedProposalMsg instead.
@ -94,12 +95,15 @@ func NewOperationMsg(msg sdk.Msg, ok bool, comment string, cdc *codec.ProtoCodec
if moduleName == "" {
moduleName = msgType
}
if legacyMsg, okType := msg.(legacytx.LegacyMsg); okType {
return NewOperationMsgBasic(moduleName, msgType, comment, ok, legacyMsg.GetSignBytes())
if cdc == nil {
cdc = codec.NewProtoCodec(types.NewInterfaceRegistry())
}
jsonBytes, err := cdc.MarshalAminoJSON(msg)
if err != nil {
panic(fmt.Errorf("failed to marshal aminon JSON: %w", err))
}
return NewOperationMsgBasic(moduleName, msgType, comment, ok, cdc.MustMarshalJSON(msg))
return NewOperationMsgBasic(moduleName, msgType, comment, ok, jsonBytes)
}
// NoOpMsg - create a no-operation message

View File

@ -142,11 +142,9 @@ func NewTxConfigWithOptions(protoCodec codec.ProtoCodecMarshaler, configOptions
panic(err)
}
case signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON:
aminoJSONEncoder := aminojson.NewEncoder()
handlers[i] = aminojson.NewSignModeHandler(aminojson.SignModeHandlerOptions{
FileResolver: signingOpts.FileResolver,
TypeResolver: signingOpts.TypeResolver,
Encoder: &aminoJSONEncoder,
})
case signingtypes.SignMode_SIGN_MODE_TEXTUAL:
handlers[i], err = textual.NewSignModeHandler(textual.SignModeOptions{

View File

@ -26,7 +26,7 @@ require (
require (
cosmossdk.io/collections v0.1.0 // indirect
cosmossdk.io/x/tx v0.6.1 // indirect
cosmossdk.io/x/tx v0.6.3 // indirect
filippo.io/edwards25519 v1.0.0 // indirect
github.com/99designs/go-keychain v0.0.0-20191008050251-8e49817e8af4 // indirect
github.com/99designs/keyring v1.2.1 // indirect

View File

@ -31,6 +31,13 @@ Ref: https://keepachangelog.com/en/1.0.0/
## Unreleased
## v0.7.0
### API Breaking
* [#16044](https://github.com/cosmos/cosmos-sdk/pull/16044): rename aminojson.NewAminoJSON -> aminojson.NewEncoder
* [#16047](https://github.com/cosmos/cosmos-sdk/pull/16047): aminojson.NewEncoder now takes EncoderOptions as an argument.
## v0.6.2
### Improvements

View File

@ -5,7 +5,6 @@ import (
"encoding/json"
"fmt"
"google.golang.org/protobuf/reflect/protodesc"
"google.golang.org/protobuf/reflect/protoregistry"
signingv1beta1 "cosmossdk.io/api/cosmos/tx/signing/v1beta1"
@ -16,14 +15,14 @@ import (
// SignModeHandler implements the SIGN_MODE_LEGACY_AMINO_JSON signing mode.
type SignModeHandler struct {
fileResolver protodesc.Resolver
fileResolver signing.ProtoFileResolver
typeResolver protoregistry.MessageTypeResolver
encoder Encoder
}
// SignModeHandlerOptions are the options for the SignModeHandler.
type SignModeHandlerOptions struct {
FileResolver protodesc.Resolver
FileResolver signing.ProtoFileResolver
TypeResolver protoregistry.MessageTypeResolver
Encoder *Encoder
}
@ -42,7 +41,10 @@ func NewSignModeHandler(options SignModeHandlerOptions) *SignModeHandler {
h.typeResolver = options.TypeResolver
}
if options.Encoder == nil {
h.encoder = NewEncoder()
h.encoder = NewEncoder(EncoderOptions{
FileResolver: options.FileResolver,
TypeResolver: options.TypeResolver,
})
} else {
h.encoder = *options.Encoder
}
@ -106,6 +108,7 @@ func (h SignModeHandler) GetSignBytes(_ context.Context, signerData signing.Sign
Memo: body.Memo,
Msgs: txData.Body.Messages,
Fee: fee,
Tip: tip,
}
bz, err := h.encoder.Marshal(signDoc)

View File

@ -106,7 +106,7 @@ func TestAminoJsonSignMode(t *testing.T) {
func TestNewSignModeHandler(t *testing.T) {
handler := aminojson.NewSignModeHandler(aminojson.SignModeHandlerOptions{})
require.NotNil(t, handler)
aj := aminojson.NewEncoder()
aj := aminojson.NewEncoder(aminojson.EncoderOptions{})
handler = aminojson.NewSignModeHandler(aminojson.SignModeHandlerOptions{
FileResolver: protoregistry.GlobalFiles,
TypeResolver: protoregistry.GlobalTypes,

View File

@ -4,34 +4,87 @@ import (
"fmt"
"io"
"google.golang.org/protobuf/types/dynamicpb"
"google.golang.org/protobuf/types/known/anypb"
"github.com/pkg/errors"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/reflect/protoreflect"
"google.golang.org/protobuf/reflect/protoregistry"
)
func (enc Encoder) marshalAny(message protoreflect.Message, writer io.Writer) error {
anyMsg := message.Interface().(*anypb.Any)
resolver := protoregistry.GlobalTypes
typ, err := resolver.FindMessageByURL(anyMsg.TypeUrl)
if err != nil {
return errors.Wrapf(err, "can't resolve type URL %s", anyMsg.TypeUrl)
// when a message contains a nested any field, and the top-level message has been unmarshalled into a dyanmicpb,
// the nested any field will also be a dynamicpb. In this case, we must use the dynamicpb API.
_, ok := message.Interface().(*dynamicpb.Message)
if ok {
return enc.marshalDynamic(message, writer)
}
valueMsg := typ.New()
err = proto.Unmarshal(anyMsg.Value, valueMsg.Interface())
if err != nil {
return err
anyMsg, ok := message.Interface().(*anypb.Any)
if !ok {
return fmt.Errorf("expected *anypb.Any, got %T", message.Interface())
}
_, named := getMessageAminoName(valueMsg)
// build a message of the correct type
var protoMessage protoreflect.Message
typ, err := enc.typeResolver.FindMessageByURL(anyMsg.TypeUrl)
if err == nil {
// If the type is registered, we can use the proto API to unmarshal into a concrete type.
valueMsg := typ.New()
err = proto.Unmarshal(anyMsg.Value, valueMsg.Interface())
if err != nil {
return err
}
protoMessage = valueMsg
} else {
// otherwise we use the dynamicpb API to unmarshal into a dynamic message.
desc, err := enc.fileResolver.FindDescriptorByName(protoreflect.FullName(anyMsg.TypeUrl[1:]))
if err != nil {
return errors.Wrapf(err, "can't resolve type URL %s", anyMsg.TypeUrl)
}
valueMsg := dynamicpb.NewMessageType(desc.(protoreflect.MessageDescriptor)).New().Interface()
err = proto.Unmarshal(anyMsg.Value, valueMsg)
if err != nil {
return err
}
protoMessage = valueMsg.ProtoReflect()
}
_, named := getMessageAminoName(protoMessage.Descriptor().Options())
if !named {
return fmt.Errorf("message %s is packed into an any field, so requires an amino.name annotation",
anyMsg.TypeUrl)
}
return enc.beginMarshal(valueMsg, writer)
return enc.beginMarshal(protoMessage, writer)
}
const (
anyTypeURLFieldName = "type_url"
anyValueFieldName = "value"
)
func (enc Encoder) marshalDynamic(message protoreflect.Message, writer io.Writer) error {
msgName := message.Get(message.Descriptor().Fields().ByName(anyTypeURLFieldName)).String()[1:]
msgBytes := message.Get(message.Descriptor().Fields().ByName(anyValueFieldName)).Bytes()
desc, err := enc.fileResolver.FindDescriptorByName(protoreflect.FullName(msgName))
if err != nil {
return errors.Wrapf(err, "can't resolve type URL %s", msgName)
}
_, named := getMessageAminoName(desc.Options())
if !named {
return fmt.Errorf("message %s is packed into an any field, so requires an amino.name annotation",
msgName)
}
valueMsg := dynamicpb.NewMessageType(desc.(protoreflect.MessageDescriptor)).New().Interface()
err = proto.Unmarshal(msgBytes, valueMsg)
if err != nil {
return err
}
return enc.beginMarshal(valueMsg.ProtoReflect(), writer)
}

View File

@ -9,6 +9,9 @@ import (
"github.com/pkg/errors"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/reflect/protoreflect"
"google.golang.org/protobuf/reflect/protoregistry"
"cosmossdk.io/x/tx/signing"
)
// MessageEncoder is a function that can encode a protobuf protoreflect.Message to JSON.
@ -17,17 +20,33 @@ type MessageEncoder func(*Encoder, protoreflect.Message, io.Writer) error
// FieldEncoder is a function that can encode a protobuf protoreflect.Value to JSON.
type FieldEncoder func(*Encoder, protoreflect.Value, io.Writer) error
// EncoderOptions are options for creating a new Encoder.
type EncoderOptions struct {
// TypeResolver is used to resolve protobuf message types by TypeURL when marshaling any packed messages.
TypeResolver protoregistry.MessageTypeResolver
// FileResolver is used to resolve protobuf file descriptors TypeURL when TypeResolver fails.
FileResolver signing.ProtoFileResolver
}
// Encoder is a JSON encoder that uses the Amino JSON encoding rules for protobuf messages.
type Encoder struct {
// maps cosmos_proto.scalar -> field encoder
scalarEncoders map[string]FieldEncoder
messageEncoders map[string]MessageEncoder
fieldEncoders map[string]FieldEncoder
fileResolver signing.ProtoFileResolver
typeResolver protoregistry.MessageTypeResolver
}
// NewEncoder returns a new Encoder capable of serializing protobuf messages to JSON using the Amino JSON encoding
// rules.
func NewEncoder() Encoder {
func NewEncoder(options EncoderOptions) Encoder {
if options.FileResolver == nil {
options.FileResolver = protoregistry.GlobalFiles
}
if options.TypeResolver == nil {
options.TypeResolver = protoregistry.GlobalTypes
}
enc := Encoder{
scalarEncoders: map[string]FieldEncoder{
"cosmos.Dec": cosmosDecEncoder,
@ -42,6 +61,8 @@ func NewEncoder() Encoder {
"legacy_coins": nullSliceAsEmptyEncoder,
"cosmos_dec_bytes": cosmosDecEncoder,
},
fileResolver: options.FileResolver,
typeResolver: options.TypeResolver,
}
return enc
}
@ -92,7 +113,7 @@ func (enc Encoder) Marshal(message proto.Message) ([]byte, error) {
}
func (enc Encoder) beginMarshal(msg protoreflect.Message, writer io.Writer) error {
name, named := getMessageAminoName(msg)
name, named := getMessageAminoName(msg.Descriptor().Options())
if named {
_, err := writer.Write([]byte(fmt.Sprintf(`{"type":"%s","value":`, name)))
if err != nil {

View File

@ -29,7 +29,7 @@ func TestAminoJSON_EdgeCases(t *testing.T) {
cdc := amino.NewCodec()
cdc.RegisterConcrete(&testpb.ABitOfEverything{}, "ABitOfEverything", nil)
cdc.RegisterConcrete(&testpb.NestedMessage{}, "NestedMessage", nil)
aj := aminojson.NewEncoder()
aj := aminojson.NewEncoder(aminojson.EncoderOptions{})
cases := map[string]struct {
msg proto.Message
@ -91,7 +91,7 @@ func TestAminoJSON(t *testing.T) {
Si64: -59268425823934,
Sf64: -659101379604211154,
}
bz, err := aminojson.NewEncoder().Marshal(msg)
bz, err := aminojson.NewEncoder(aminojson.EncoderOptions{}).Marshal(msg)
assert.NilError(t, err)
legacyBz, err := cdc.MarshalJSON(msg)
assert.NilError(t, err)
@ -102,7 +102,7 @@ func TestRapid(t *testing.T) {
gen := rapidproto.MessageGenerator(&testpb.ABitOfEverything{}, rapidproto.GeneratorOptions{})
rapid.Check(t, func(t *rapid.T) {
msg := gen.Draw(t, "msg")
bz, err := aminojson.NewEncoder().Marshal(msg)
bz, err := aminojson.NewEncoder(aminojson.EncoderOptions{}).Marshal(msg)
assert.NilError(t, err)
checkInvariants(t, msg, bz)
})

View File

@ -12,10 +12,9 @@ import (
// getMessageAminoName returns the amino name of a message if it has been set by the `amino.name` option.
// If the message does not have an amino name, then the function returns false.
func getMessageAminoName(msg protoreflect.Message) (string, bool) {
opts := msg.Descriptor().Options()
if proto.HasExtension(opts, amino.E_Name) {
name := proto.GetExtension(opts, amino.E_Name)
func getMessageAminoName(messageOptions proto.Message) (string, bool) {
if proto.HasExtension(messageOptions, amino.E_Name) {
name := proto.GetExtension(messageOptions, amino.E_Name)
return name.(string), true
}
return "", false