Skip to content

Commit

Permalink
refactor(x/tx): use dynamicpb instead of type resolver in any marshal (
Browse files Browse the repository at this point in the history
…cosmos#16048)

Co-authored-by: Aaron Craelius <aaron@regen.network>
  • Loading branch information
kocubinski and aaronc committed May 8, 2023
1 parent 12923c3 commit 2f21cb5
Show file tree
Hide file tree
Showing 17 changed files with 180 additions and 48 deletions.
39 changes: 39 additions & 0 deletions codec/proto_codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions simapp/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
7 changes: 3 additions & 4 deletions tests/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 => ../.
Expand Down
15 changes: 12 additions & 3 deletions tests/integration/aminojson/aminojson_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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"}},
},
Expand All @@ -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{
Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/aminojson/repeated_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion tools/rosetta/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 9 additions & 5 deletions types/simulation/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions x/auth/tx/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
2 changes: 1 addition & 1 deletion x/evidence/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions x/tx/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 7 additions & 4 deletions x/tx/signing/aminojson/aminojson.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion x/tx/signing/aminojson/aminojson_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading

0 comments on commit 2f21cb5

Please sign in to comment.