From 124aed4b28e866bd2d42d5dad57d4d5692b4fd41 Mon Sep 17 00:00:00 2001 From: Amaury <1293565+amaurym@users.noreply.github.com> Date: Mon, 20 Mar 2023 09:13:27 +0100 Subject: [PATCH] feat(x/tx): Support gogo registry in Textual (#15302) --- go.mod | 2 +- go.sum | 4 +- simapp/go.mod | 2 +- simapp/go.sum | 4 +- simapp/simd/cmd/root.go | 6 +- tests/go.mod | 2 +- tests/go.sum | 4 +- x/auth/ante/sigverify_test.go | 8 +- x/auth/tx/config/config.go | 5 +- x/auth/tx/config/textual.go | 89 +++++++++++-------- x/tx/CHANGELOG.md | 6 ++ x/tx/signing/std/handler_map.go | 13 ++- x/tx/signing/textual/any.go | 39 +++++--- x/tx/signing/textual/any_test.go | 61 ++++++++++++- x/tx/signing/textual/bytes_test.go | 3 +- x/tx/signing/textual/coin_test.go | 17 ++-- x/tx/signing/textual/coins.go | 12 ++- x/tx/signing/textual/coins_test.go | 3 +- x/tx/signing/textual/dec_test.go | 3 +- x/tx/signing/textual/duration.go | 7 +- x/tx/signing/textual/e2e_test.go | 3 +- x/tx/signing/textual/enum_test.go | 3 +- .../textual/{valuerenderer.go => handler.go} | 89 +++++++++++++++++-- ...{valuerenderer_test.go => handler_test.go} | 3 +- x/tx/signing/textual/int_test.go | 3 +- x/tx/signing/textual/message_test.go | 2 +- x/tx/signing/textual/repeated_test.go | 2 +- x/tx/signing/textual/timestamp.go | 13 ++- x/tx/signing/textual/tx_test.go | 2 +- 29 files changed, 301 insertions(+), 109 deletions(-) rename x/tx/signing/textual/{valuerenderer.go => handler.go} (68%) rename x/tx/signing/textual/{valuerenderer_test.go => handler_test.go} (92%) diff --git a/go.mod b/go.mod index ce1772a315..88baa36aa2 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( cosmossdk.io/log v0.1.0 cosmossdk.io/math v1.0.0-rc.0 cosmossdk.io/store v0.1.0-alpha.1 - cosmossdk.io/x/tx v0.3.0 + cosmossdk.io/x/tx v0.3.1-0.20230320072322-5fceb7c0495f 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 diff --git a/go.sum b/go.sum index bfec3fca3b..db3eaeccf0 100644 --- a/go.sum +++ b/go.sum @@ -51,8 +51,8 @@ cosmossdk.io/math v1.0.0-rc.0 h1:ml46ukocrAAoBpYKMidF0R2tQJ1Uxfns0yH8wqgMAFc= cosmossdk.io/math v1.0.0-rc.0/go.mod h1:Ygz4wBHrgc7g0N+8+MrnTfS9LLn9aaTGa9hKopuym5k= cosmossdk.io/store v0.1.0-alpha.1 h1:NGomhLUXzAxvK4OF8+yP6eNUG5i4LwzOzx+S494pTCg= cosmossdk.io/store v0.1.0-alpha.1/go.mod h1:kmCMbhrleCZ6rDZPY/EGNldNvPebFNyVPFYp+pv05/k= -cosmossdk.io/x/tx v0.3.0 h1:AgVYy6bxL3XqEV7RLyxFh1uT+wywsrbgVMmYnL3FgWM= -cosmossdk.io/x/tx v0.3.0/go.mod h1:ELY0bP2SmOqyffJFp00g979xsI1zBdmc55A6JCi1Qe8= +cosmossdk.io/x/tx v0.3.1-0.20230320072322-5fceb7c0495f h1:yXEE3D6L0Ykwlp4FuS1SoHgT9vZ8brBJ/dkHezXBU9o= +cosmossdk.io/x/tx v0.3.1-0.20230320072322-5fceb7c0495f/go.mod h1:V/7DjCSReJ7LBBYrNtVFUec7t63hVNyFh0vBXOBK2Yg= dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU= filippo.io/edwards25519 v1.0.0 h1:0wAIcmJUqRdI8IJ/3eGi5/HwXZWPujYXXlkrQogz0Ek= filippo.io/edwards25519 v1.0.0/go.mod h1:N1IkdkCkiLB6tki+MYJoSx2JTY9NUlxZE7eHn5EwJns= diff --git a/simapp/go.mod b/simapp/go.mod index 40550ead94..260d64d69a 100644 --- a/simapp/go.mod +++ b/simapp/go.mod @@ -38,7 +38,7 @@ require ( cloud.google.com/go/storage v1.29.0 // indirect cosmossdk.io/collections v0.0.0-20230309163709-87da587416ba // indirect cosmossdk.io/errors v1.0.0-beta.7 // indirect - cosmossdk.io/x/tx v0.3.0 // indirect + cosmossdk.io/x/tx v0.3.1-0.20230320072322-5fceb7c0495f // 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 diff --git a/simapp/go.sum b/simapp/go.sum index 0fbb348305..e9f583e52f 100644 --- a/simapp/go.sum +++ b/simapp/go.sum @@ -206,8 +206,8 @@ cosmossdk.io/math v1.0.0-rc.0 h1:ml46ukocrAAoBpYKMidF0R2tQJ1Uxfns0yH8wqgMAFc= cosmossdk.io/math v1.0.0-rc.0/go.mod h1:Ygz4wBHrgc7g0N+8+MrnTfS9LLn9aaTGa9hKopuym5k= cosmossdk.io/store v0.1.0-alpha.1 h1:NGomhLUXzAxvK4OF8+yP6eNUG5i4LwzOzx+S494pTCg= cosmossdk.io/store v0.1.0-alpha.1/go.mod h1:kmCMbhrleCZ6rDZPY/EGNldNvPebFNyVPFYp+pv05/k= -cosmossdk.io/x/tx v0.3.0 h1:AgVYy6bxL3XqEV7RLyxFh1uT+wywsrbgVMmYnL3FgWM= -cosmossdk.io/x/tx v0.3.0/go.mod h1:ELY0bP2SmOqyffJFp00g979xsI1zBdmc55A6JCi1Qe8= +cosmossdk.io/x/tx v0.3.1-0.20230320072322-5fceb7c0495f h1:yXEE3D6L0Ykwlp4FuS1SoHgT9vZ8brBJ/dkHezXBU9o= +cosmossdk.io/x/tx v0.3.1-0.20230320072322-5fceb7c0495f/go.mod h1:V/7DjCSReJ7LBBYrNtVFUec7t63hVNyFh0vBXOBK2Yg= dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU= filippo.io/edwards25519 v1.0.0 h1:0wAIcmJUqRdI8IJ/3eGi5/HwXZWPujYXXlkrQogz0Ek= filippo.io/edwards25519 v1.0.0/go.mod h1:N1IkdkCkiLB6tki+MYJoSx2JTY9NUlxZE7eHn5EwJns= diff --git a/simapp/simd/cmd/root.go b/simapp/simd/cmd/root.go index f572b81545..f98002d6ad 100644 --- a/simapp/simd/cmd/root.go +++ b/simapp/simd/cmd/root.go @@ -85,10 +85,14 @@ func NewRootCmd() *cobra.Command { // TODO Currently, the TxConfig below doesn't include Textual, so // an error will arise when using the --textual flag. // ref: https://github.com/cosmos/cosmos-sdk/issues/11970 + txt, err := txmodule.NewTextualWithGRPCConn(initClientCtx) + if err != nil { + return err + } txConfigWithTextual := tx.NewTxConfigWithTextual( codec.NewProtoCodec(encodingConfig.InterfaceRegistry), encodingConfig.TxConfig.SignModeHandler().Modes(), - txmodule.NewTextualWithGRPCConn(initClientCtx), + txt, ) initClientCtx = initClientCtx.WithTxConfig(txConfigWithTextual) diff --git a/tests/go.mod b/tests/go.mod index 3bb7cfd335..f94ea97cea 100644 --- a/tests/go.mod +++ b/tests/go.mod @@ -13,7 +13,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.3.0 + cosmossdk.io/x/tx v0.3.1-0.20230320072322-5fceb7c0495f cosmossdk.io/x/upgrade v0.0.0-20230127052425-54c8e1568335 github.com/cometbft/cometbft v0.37.0 github.com/cosmos/cosmos-db v1.0.0-rc.1 diff --git a/tests/go.sum b/tests/go.sum index ec773768cb..66b38c50ba 100644 --- a/tests/go.sum +++ b/tests/go.sum @@ -206,8 +206,8 @@ cosmossdk.io/math v1.0.0-rc.0 h1:ml46ukocrAAoBpYKMidF0R2tQJ1Uxfns0yH8wqgMAFc= cosmossdk.io/math v1.0.0-rc.0/go.mod h1:Ygz4wBHrgc7g0N+8+MrnTfS9LLn9aaTGa9hKopuym5k= cosmossdk.io/store v0.1.0-alpha.1 h1:NGomhLUXzAxvK4OF8+yP6eNUG5i4LwzOzx+S494pTCg= cosmossdk.io/store v0.1.0-alpha.1/go.mod h1:kmCMbhrleCZ6rDZPY/EGNldNvPebFNyVPFYp+pv05/k= -cosmossdk.io/x/tx v0.3.0 h1:AgVYy6bxL3XqEV7RLyxFh1uT+wywsrbgVMmYnL3FgWM= -cosmossdk.io/x/tx v0.3.0/go.mod h1:ELY0bP2SmOqyffJFp00g979xsI1zBdmc55A6JCi1Qe8= +cosmossdk.io/x/tx v0.3.1-0.20230320072322-5fceb7c0495f h1:yXEE3D6L0Ykwlp4FuS1SoHgT9vZ8brBJ/dkHezXBU9o= +cosmossdk.io/x/tx v0.3.1-0.20230320072322-5fceb7c0495f/go.mod h1:V/7DjCSReJ7LBBYrNtVFUec7t63hVNyFh0vBXOBK2Yg= dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU= filippo.io/edwards25519 v1.0.0 h1:0wAIcmJUqRdI8IJ/3eGi5/HwXZWPujYXXlkrQogz0Ek= filippo.io/edwards25519 v1.0.0/go.mod h1:N1IkdkCkiLB6tki+MYJoSx2JTY9NUlxZE7eHn5EwJns= diff --git a/x/auth/ante/sigverify_test.go b/x/auth/ante/sigverify_test.go index 68e38c9265..2af60c81a8 100644 --- a/x/auth/ante/sigverify_test.go +++ b/x/auth/ante/sigverify_test.go @@ -131,10 +131,12 @@ func TestSigVerification(t *testing.T) { enabledSignModes := []signing.SignMode{signing.SignMode_SIGN_MODE_DIRECT, signing.SignMode_SIGN_MODE_TEXTUAL, signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON} // Since TEXTUAL is not enabled by default, we create a custom TxConfig // here which includes it. + txt, err := txmodule.NewTextualWithGRPCConn(suite.clientCtx) + require.NoError(t, err) suite.clientCtx.TxConfig = authtx.NewTxConfigWithTextual( codec.NewProtoCodec(suite.encCfg.InterfaceRegistry), enabledSignModes, - txmodule.NewTextualWithGRPCConn(suite.clientCtx), + txt, ) suite.txBuilder = suite.clientCtx.TxConfig.NewTxBuilder() @@ -161,10 +163,12 @@ func TestSigVerification(t *testing.T) { gasLimit := testdata.NewTestGasLimit() spkd := ante.NewSetPubKeyDecorator(suite.accountKeeper) + txt, err = txmodule.NewTextualWithBankKeeper(suite.txBankKeeper) + require.NoError(t, err) anteTxConfig := authtx.NewTxConfigWithTextual( codec.NewProtoCodec(suite.encCfg.InterfaceRegistry), enabledSignModes, - txmodule.NewTextualWithBankKeeper(suite.txBankKeeper), + txt, ) svd := ante.NewSigVerificationDecorator(suite.accountKeeper, anteTxConfig.SignModeHandler()) antehandler := sdk.ChainAnteDecorators(spkd, svd) diff --git a/x/auth/tx/config/config.go b/x/auth/tx/config/config.go index cbb55b92c9..346c9d0eeb 100644 --- a/x/auth/tx/config/config.go +++ b/x/auth/tx/config/config.go @@ -50,7 +50,10 @@ type TxOutputs struct { } func ProvideModule(in TxInputs) TxOutputs { - textual := NewTextualWithBankKeeper(in.TxBankKeeper) + textual, err := NewTextualWithBankKeeper(in.TxBankKeeper) + if err != nil { + panic(err) + } var txConfig client.TxConfig if in.CustomSignModeHandlers == nil { txConfig = tx.NewTxConfigWithTextual(in.ProtoCodecMarshaler, tx.DefaultSignModes, textual) diff --git a/x/auth/tx/config/textual.go b/x/auth/tx/config/textual.go index 28d1f39e6f..3a93cb689d 100644 --- a/x/auth/tx/config/textual.go +++ b/x/auth/tx/config/textual.go @@ -3,6 +3,7 @@ package tx import ( "context" + gogoproto "github.com/cosmos/gogoproto/proto" "google.golang.org/grpc" "google.golang.org/grpc/codes" grpcstatus "google.golang.org/grpc/status" @@ -21,17 +22,25 @@ import ( // // clientCtx := client.GetClientContextFromCmd(cmd) // txt := tx.NewTextualWithGRPCConn(clientCtxx) -func NewTextualWithGRPCConn(grpcConn grpc.ClientConnInterface) *textual.SignModeHandler { - return textual.NewSignModeHandler(func(ctx context.Context, denom string) (*bankv1beta1.Metadata, error) { - bankQueryClient := bankv1beta1.NewQueryClient(grpcConn) - res, err := bankQueryClient.DenomMetadata(ctx, &bankv1beta1.QueryDenomMetadataRequest{ - Denom: denom, - }) - if err != nil { - return nil, metadataExists(err) - } +func NewTextualWithGRPCConn(grpcConn grpc.ClientConnInterface) (*textual.SignModeHandler, error) { + protoFiles, err := gogoproto.MergedRegistry() + if err != nil { + return nil, err + } - return res.Metadata, nil + return textual.NewSignModeHandler(textual.SignModeOptions{ + CoinMetadataQuerier: func(ctx context.Context, denom string) (*bankv1beta1.Metadata, error) { + bankQueryClient := bankv1beta1.NewQueryClient(grpcConn) + res, err := bankQueryClient.DenomMetadata(ctx, &bankv1beta1.QueryDenomMetadataRequest{ + Denom: denom, + }) + if err != nil { + return nil, metadataExists(err) + } + + return res.Metadata, nil + }, + FileResolver: protoFiles, }) } @@ -41,37 +50,43 @@ func NewTextualWithGRPCConn(grpcConn grpc.ClientConnInterface) *textual.SignMode // Note: Once we switch to ADR-033, and keepers become ADR-033 clients to each // other, this function could probably be deprecated in favor of // `NewTextualWithGRPCConn`. -func NewTextualWithBankKeeper(bk BankKeeper) *textual.SignModeHandler { - textual := textual.NewSignModeHandler(func(ctx context.Context, denom string) (*bankv1beta1.Metadata, error) { - res, err := bk.DenomMetadata(ctx, &types.QueryDenomMetadataRequest{Denom: denom}) - if err != nil { - return nil, metadataExists(err) - } +func NewTextualWithBankKeeper(bk BankKeeper) (*textual.SignModeHandler, error) { + protoFiles, err := gogoproto.MergedRegistry() + if err != nil { + return nil, err + } - m := &bankv1beta1.Metadata{ - Base: res.Metadata.Base, - Display: res.Metadata.Display, - // fields below are not strictly needed by Textual - // but added here for completeness. - Description: res.Metadata.Description, - Name: res.Metadata.Name, - Symbol: res.Metadata.Symbol, - Uri: res.Metadata.URI, - UriHash: res.Metadata.URIHash, - } - m.DenomUnits = make([]*bankv1beta1.DenomUnit, len(res.Metadata.DenomUnits)) - for i, d := range res.Metadata.DenomUnits { - m.DenomUnits[i] = &bankv1beta1.DenomUnit{ - Denom: d.Denom, - Exponent: d.Exponent, - Aliases: d.Aliases, + return textual.NewSignModeHandler(textual.SignModeOptions{ + CoinMetadataQuerier: func(ctx context.Context, denom string) (*bankv1beta1.Metadata, error) { + res, err := bk.DenomMetadata(ctx, &types.QueryDenomMetadataRequest{Denom: denom}) + if err != nil { + return nil, metadataExists(err) } - } - return m, nil + m := &bankv1beta1.Metadata{ + Base: res.Metadata.Base, + Display: res.Metadata.Display, + // fields below are not strictly needed by Textual + // but added here for completeness. + Description: res.Metadata.Description, + Name: res.Metadata.Name, + Symbol: res.Metadata.Symbol, + Uri: res.Metadata.URI, + UriHash: res.Metadata.URIHash, + } + m.DenomUnits = make([]*bankv1beta1.DenomUnit, len(res.Metadata.DenomUnits)) + for i, d := range res.Metadata.DenomUnits { + m.DenomUnits[i] = &bankv1beta1.DenomUnit{ + Denom: d.Denom, + Exponent: d.Exponent, + Aliases: d.Aliases, + } + } + + return m, nil + }, + FileResolver: protoFiles, }) - - return textual } // metadataExists parses the error, and only propagates the error if it's diff --git a/x/tx/CHANGELOG.md b/x/tx/CHANGELOG.md index 5ef5614054..fbf674c28b 100644 --- a/x/tx/CHANGELOG.md +++ b/x/tx/CHANGELOG.md @@ -34,3 +34,9 @@ Ref: https://keepachangelog.com/en/1.0.0/ ## API Breaking * [#15278](https://github.com/cosmos/cosmos-sdk/pull/15278) Move `x/tx/{textual,aminojson}` into `x/tx/signing`. +* [#15302](https://github.com/cosmos/cosmos-sdk/pull/15302) `textual.NewSignModeHandler` now takes an options struct instead of a simple coin querier argument. It also returns an error. + +## Improvements + +* [#15302](https://github.com/cosmos/cosmos-sdk/pull/15302) Add support for a custom registry (e.g. gogo's MergedRegistry) to be plugged into SIGN_MODE_TEXTUAL. + diff --git a/x/tx/signing/std/handler_map.go b/x/tx/signing/std/handler_map.go index c94ca03687..db889be2de 100644 --- a/x/tx/signing/std/handler_map.go +++ b/x/tx/signing/std/handler_map.go @@ -1,8 +1,6 @@ package std import ( - "fmt" - "cosmossdk.io/x/tx/signing" "cosmossdk.io/x/tx/signing/direct" "cosmossdk.io/x/tx/signing/textual" @@ -10,19 +8,20 @@ import ( // SignModeOptions are options for configuring the standard sign mode handler map. type SignModeOptions struct { - // CoinMetadataQueryFn is the CoinMetadataQueryFn required for SIGN_MODE_TEXTUAL. - CoinMetadataQueryFn textual.CoinMetadataQueryFn + // Textual are options for SIGN_MODE_TEXTUAL + Textual textual.SignModeOptions } // HandlerMap returns a sign mode handler map that Cosmos SDK apps can use out // of the box to support all "standard" sign modes. func (s SignModeOptions) HandlerMap() (*signing.HandlerMap, error) { - if s.CoinMetadataQueryFn == nil { - return nil, fmt.Errorf("missing %T needed for SIGN_MODE_TEXTUAL", s.CoinMetadataQueryFn) + txt, err := textual.NewSignModeHandler(s.Textual) + if err != nil { + return nil, err } return signing.NewHandlerMap( direct.SignModeHandler{}, - textual.NewSignModeHandler(s.CoinMetadataQueryFn), + txt, ), nil } diff --git a/x/tx/signing/textual/any.go b/x/tx/signing/textual/any.go index 93677dc02b..516496f9fc 100644 --- a/x/tx/signing/textual/any.go +++ b/x/tx/signing/textual/any.go @@ -3,10 +3,12 @@ package textual import ( "context" "fmt" + "strings" "github.com/cosmos/cosmos-proto/anyutil" "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" ) @@ -26,17 +28,17 @@ func NewAnyValueRenderer(t *SignModeHandler) ValueRenderer { // Format implements the ValueRenderer interface. func (ar anyValueRenderer) Format(ctx context.Context, v protoreflect.Value) ([]Screen, error) { msg := v.Message().Interface() - omitHeader := 0 - - anymsg, ok := msg.(*anypb.Any) - if !ok { - return nil, fmt.Errorf("expected Any, got %T", msg) - } - - internalMsg, err := anymsg.UnmarshalNew() + anymsg := &anypb.Any{} + err := coerceToMessage(msg, anymsg) if err != nil { - return nil, fmt.Errorf("error unmarshalling any %s: %w", anymsg.TypeUrl, err) + return nil, err } + + internalMsg, err := anyutil.Unpack(anymsg, ar.tr.fileResolver, ar.tr.typeResolver) + if err != nil { + return nil, err + } + vr, err := ar.tr.GetMessageValueRenderer(internalMsg.ProtoReflect().Descriptor()) if err != nil { return nil, err @@ -48,6 +50,7 @@ func (ar anyValueRenderer) Format(ctx context.Context, v protoreflect.Value) ([] } // The Any value renderer suppresses emission of the object header + omitHeader := 0 _, isMsgRenderer := vr.(*messageValueRenderer) if isMsgRenderer && subscreens[0].Content == fmt.Sprintf("%s object", internalMsg.ProtoReflect().Descriptor().Name()) { omitHeader = 1 @@ -72,8 +75,22 @@ func (ar anyValueRenderer) Parse(ctx context.Context, screens []Screen) (protore return nilValue, fmt.Errorf("bad indentation: want 0, got %d", screens[0].Indent) } - msgType, err := protoregistry.GlobalTypes.FindMessageByURL(screens[0].Content) - if err != nil { + typeURL := screens[0].Content + msgType, err := ar.tr.typeResolver.FindMessageByURL(typeURL) + if err == protoregistry.NotFound { + // If the proto v2 registry doesn't have this message, then we use + // protoFiles (which can e.g. be initialized to gogo's MergedRegistry) + // to retrieve the message descriptor, and then use dynamicpb on that + // message descriptor to create a proto.Message + typeURL := strings.TrimPrefix(typeURL, "/") + + msgDesc, err := ar.tr.fileResolver.FindDescriptorByName(protoreflect.FullName(typeURL)) + if err != nil { + return nilValue, fmt.Errorf("textual protoFiles does not have descriptor %s: %w", typeURL, err) + } + + msgType = dynamicpb.NewMessageType(msgDesc.(protoreflect.MessageDescriptor)) + } else if err != nil { return nilValue, err } vr, err := ar.tr.GetMessageValueRenderer(msgType.Descriptor()) diff --git a/x/tx/signing/textual/any_test.go b/x/tx/signing/textual/any_test.go index 6a8f289ea1..b15d63355e 100644 --- a/x/tx/signing/textual/any_test.go +++ b/x/tx/signing/textual/any_test.go @@ -6,16 +6,24 @@ import ( "fmt" "os" "testing" + "time" + "github.com/cosmos/cosmos-proto/anyutil" "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/require" - - "cosmossdk.io/x/tx/signing/textual" - "google.golang.org/protobuf/encoding/protojson" + "google.golang.org/protobuf/proto" "google.golang.org/protobuf/reflect/protoreflect" + "google.golang.org/protobuf/reflect/protoregistry" "google.golang.org/protobuf/testing/protocmp" "google.golang.org/protobuf/types/known/anypb" + "google.golang.org/protobuf/types/known/durationpb" + "google.golang.org/protobuf/types/known/timestamppb" + + bankv1beta1 "cosmossdk.io/api/cosmos/bank/v1beta1" + basev1beta1 "cosmossdk.io/api/cosmos/base/v1beta1" + "cosmossdk.io/x/tx/internal/testpb" + "cosmossdk.io/x/tx/signing/textual" ) type anyJsonTest struct { @@ -31,7 +39,8 @@ func TestAny(t *testing.T) { err = json.Unmarshal(raw, &testcases) require.NoError(t, err) - tr := textual.NewSignModeHandler(EmptyCoinMetadataQuerier) + tr, err := textual.NewSignModeHandler(textual.SignModeOptions{CoinMetadataQuerier: EmptyCoinMetadataQuerier}) + require.NoError(t, err) for i, tc := range testcases { t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { anyMsg := anypb.Any{} @@ -55,3 +64,47 @@ func TestAny(t *testing.T) { }) } } + +func TestDynamicpb(t *testing.T) { + tr, err := textual.NewSignModeHandler(textual.SignModeOptions{ + CoinMetadataQuerier: EmptyCoinMetadataQuerier, + TypeResolver: &protoregistry.Types{}, // Set to empty to force using dynamicpb + }) + require.NoError(t, err) + + testAny, err := anyutil.New(&testpb.Foo{FullName: "foobar"}) + require.NoError(t, err) + + testcases := []struct { + name string + msg proto.Message + }{ + {"coin", &basev1beta1.Coin{Denom: "stake", Amount: "1"}}, + {"nested coins", &bankv1beta1.MsgSend{Amount: []*basev1beta1.Coin{{Denom: "stake", Amount: "1"}}}}, + {"any", testAny}, + {"nested any", &testpb.A{ANY: testAny}}, + {"duration", durationpb.New(time.Hour)}, + {"timestamp", timestamppb.New(time.Now())}, + } + + for _, tc := range testcases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + any, err := anyutil.New(tc.msg) + require.NoError(t, err) + val := &testpb.A{ + ANY: any, + } + vr, err := tr.GetMessageValueRenderer(val.ProtoReflect().Descriptor()) + require.NoError(t, err) + + // Round trip. + screens, err := vr.Format(context.Background(), protoreflect.ValueOf(val.ProtoReflect())) + require.NoError(t, err) + parsedVal, err := vr.Parse(context.Background(), screens) + require.NoError(t, err) + diff := cmp.Diff(val, parsedVal.Message().Interface(), protocmp.Transform()) + require.Empty(t, diff) + }) + } +} diff --git a/x/tx/signing/textual/bytes_test.go b/x/tx/signing/textual/bytes_test.go index 5ca1dcacdd..1f9fae13a8 100644 --- a/x/tx/signing/textual/bytes_test.go +++ b/x/tx/signing/textual/bytes_test.go @@ -21,7 +21,8 @@ func TestBytesJsonTestCases(t *testing.T) { err = json.Unmarshal(raw, &testcases) require.NoError(t, err) - textual := textual.NewSignModeHandler(nil) + textual, err := textual.NewSignModeHandler(textual.SignModeOptions{CoinMetadataQuerier: EmptyCoinMetadataQuerier}) + require.NoError(t, err) for _, tc := range testcases { t.Run(tc.hex, func(t *testing.T) { diff --git a/x/tx/signing/textual/coin_test.go b/x/tx/signing/textual/coin_test.go index 85160cacbf..492e5d2705 100644 --- a/x/tx/signing/textual/coin_test.go +++ b/x/tx/signing/textual/coin_test.go @@ -52,18 +52,18 @@ func addMetadataToContext(ctx context.Context, metadata *bankv1beta1.Metadata) c func TestMetadataQuerier(t *testing.T) { // Errors on nil metadata querier - txt := textual.NewSignModeHandler(nil) - vr, err := txt.GetFieldValueRenderer(fieldDescriptorFromName("COIN")) - require.NoError(t, err) - _, err = vr.Format(context.Background(), protoreflect.ValueOf((&basev1beta1.Coin{}).ProtoReflect())) + _, err := textual.NewSignModeHandler(textual.SignModeOptions{}) require.Error(t, err) // Errors if metadata querier returns an error expErr := fmt.Errorf("mock error") - txt = textual.NewSignModeHandler(func(_ context.Context, _ string) (*bankv1beta1.Metadata, error) { - return nil, expErr + txt, err := textual.NewSignModeHandler(textual.SignModeOptions{ + CoinMetadataQuerier: func(_ context.Context, _ string) (*bankv1beta1.Metadata, error) { + return nil, expErr + }, }) - vr, err = txt.GetFieldValueRenderer(fieldDescriptorFromName("COIN")) + require.NoError(t, err) + vr, err := txt.GetFieldValueRenderer(fieldDescriptorFromName("COIN")) require.NoError(t, err) _, err = vr.Format(context.Background(), protoreflect.ValueOf((&basev1beta1.Coin{}).ProtoReflect())) require.ErrorIs(t, err, expErr) @@ -78,7 +78,8 @@ func TestCoinJsonTestcases(t *testing.T) { err = json.Unmarshal(raw, &testcases) require.NoError(t, err) - textual := textual.NewSignModeHandler(mockCoinMetadataQuerier) + textual, err := textual.NewSignModeHandler(textual.SignModeOptions{CoinMetadataQuerier: mockCoinMetadataQuerier}) + require.NoError(t, err) vr, err := textual.GetFieldValueRenderer(fieldDescriptorFromName("COIN")) require.NoError(t, err) diff --git a/x/tx/signing/textual/coins.go b/x/tx/signing/textual/coins.go index 305b701387..addd95ca8b 100644 --- a/x/tx/signing/textual/coins.go +++ b/x/tx/signing/textual/coins.go @@ -37,7 +37,11 @@ func (vr coinsValueRenderer) Format(ctx context.Context, v protoreflect.Value) ( // Since this value renderer has a FormatRepeated method, the Format one // here only handles single coin. - coin := v.Interface().(protoreflect.Message).Interface().(*basev1beta1.Coin) + coin := &basev1beta1.Coin{} + err := coerceToMessage(v.Interface().(protoreflect.Message).Interface(), coin) + if err != nil { + return nil, err + } metadata, err := vr.coinMetadataQuerier(ctx, coin.Denom) if err != nil { @@ -61,7 +65,11 @@ func (vr coinsValueRenderer) FormatRepeated(ctx context.Context, v protoreflect. coins, metadatas := make([]*basev1beta1.Coin, protoCoins.Len()), make([]*bankv1beta1.Metadata, protoCoins.Len()) var err error for i := 0; i < protoCoins.Len(); i++ { - coin := protoCoins.Get(i).Interface().(protoreflect.Message).Interface().(*basev1beta1.Coin) + coin := &basev1beta1.Coin{} + err := coerceToMessage(protoCoins.Get(i).Interface().(protoreflect.Message).Interface(), coin) + if err != nil { + return nil, err + } coins[i] = coin metadatas[i], err = vr.coinMetadataQuerier(ctx, coin.Denom) if err != nil { diff --git a/x/tx/signing/textual/coins_test.go b/x/tx/signing/textual/coins_test.go index e7fdf12d0d..55ff78482c 100644 --- a/x/tx/signing/textual/coins_test.go +++ b/x/tx/signing/textual/coins_test.go @@ -22,7 +22,8 @@ func TestCoinsJsonTestcases(t *testing.T) { err = json.Unmarshal(raw, &testcases) require.NoError(t, err) - txt := textual.NewSignModeHandler(mockCoinMetadataQuerier) + txt, err := textual.NewSignModeHandler(textual.SignModeOptions{CoinMetadataQuerier: mockCoinMetadataQuerier}) + require.NoError(t, err) vr, err := txt.GetFieldValueRenderer(fieldDescriptorFromName("COINS")) vrr := vr.(textual.RepeatedValueRenderer) require.NoError(t, err) diff --git a/x/tx/signing/textual/dec_test.go b/x/tx/signing/textual/dec_test.go index a3204afdf0..1ee1615a7b 100644 --- a/x/tx/signing/textual/dec_test.go +++ b/x/tx/signing/textual/dec_test.go @@ -23,7 +23,8 @@ func TestDecJsonTestcases(t *testing.T) { err = json.Unmarshal(raw, &testcases) require.NoError(t, err) - textual := textual.NewSignModeHandler(nil) + textual, err := textual.NewSignModeHandler(textual.SignModeOptions{CoinMetadataQuerier: EmptyCoinMetadataQuerier}) + require.NoError(t, err) for _, tc := range testcases { tc := tc diff --git a/x/tx/signing/textual/duration.go b/x/tx/signing/textual/duration.go index c32241b957..7afe077571 100644 --- a/x/tx/signing/textual/duration.go +++ b/x/tx/signing/textual/duration.go @@ -67,9 +67,10 @@ func formatSeconds(seconds int64, nanos int32) string { func (dr durationValueRenderer) Format(_ context.Context, v protoreflect.Value) ([]Screen, error) { // Reify the reflected message as a proto Duration msg := v.Message().Interface() - duration, ok := msg.(*dpb.Duration) - if !ok { - return nil, fmt.Errorf("expected Duration, got %T", msg) + duration := &dpb.Duration{} + err := coerceToMessage(msg, duration) + if err != nil { + return nil, err } // Bypass use of time.Duration, as the range is more limited than that of dpb.Duration. diff --git a/x/tx/signing/textual/e2e_test.go b/x/tx/signing/textual/e2e_test.go index 168e157e8e..e2260b21db 100644 --- a/x/tx/signing/textual/e2e_test.go +++ b/x/tx/signing/textual/e2e_test.go @@ -39,7 +39,8 @@ func TestE2EJsonTestcases(t *testing.T) { t.Run(tc.Name, func(t *testing.T) { _, bodyBz, _, authInfoBz, signerData := createTextualData(t, tc.Proto, tc.SignerData) - tr := textual.NewSignModeHandler(mockCoinMetadataQuerier) + tr, err := textual.NewSignModeHandler(textual.SignModeOptions{CoinMetadataQuerier: mockCoinMetadataQuerier}) + require.NoError(t, err) rend := textual.NewTxValueRenderer(tr) ctx := addMetadataToContext(context.Background(), tc.Metadata) diff --git a/x/tx/signing/textual/enum_test.go b/x/tx/signing/textual/enum_test.go index ce109c607e..21daf7541a 100644 --- a/x/tx/signing/textual/enum_test.go +++ b/x/tx/signing/textual/enum_test.go @@ -29,7 +29,8 @@ func TestEnumJsonTestcases(t *testing.T) { err = json.Unmarshal(raw, &testcases) require.NoError(t, err) - textual := textual.NewSignModeHandler(nil) + textual, err := textual.NewSignModeHandler(textual.SignModeOptions{CoinMetadataQuerier: EmptyCoinMetadataQuerier}) + require.NoError(t, err) for _, tc := range testcases { t.Run(tc.Text, func(t *testing.T) { diff --git a/x/tx/signing/textual/valuerenderer.go b/x/tx/signing/textual/handler.go similarity index 68% rename from x/tx/signing/textual/valuerenderer.go rename to x/tx/signing/textual/handler.go index 11a3774158..3aeaf7423a 100644 --- a/x/tx/signing/textual/valuerenderer.go +++ b/x/tx/signing/textual/handler.go @@ -4,10 +4,12 @@ import ( "bytes" "context" "fmt" + "reflect" signingv1beta1 "cosmossdk.io/api/cosmos/tx/signing/v1beta1" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/reflect/protoreflect" + "google.golang.org/protobuf/reflect/protoregistry" "google.golang.org/protobuf/types/known/anypb" "google.golang.org/protobuf/types/known/durationpb" "google.golang.org/protobuf/types/known/timestamppb" @@ -27,13 +29,28 @@ type CoinMetadataQueryFn func(ctx context.Context, denom string) (*bankv1beta1.M // ValueRendererCreator is a function returning a textual. type ValueRendererCreator func(protoreflect.FieldDescriptor) ValueRenderer -// SignModeHandler holds the configuration for dispatching -// to specific value renderers for SIGN_MODE_TEXTUAL. -type SignModeHandler struct { +// SignModeOptions are options to be passed to Textual's sign mode handler. +type SignModeOptions struct { // coinMetadataQuerier defines a function to query the coin metadata from // state. It should use bank module's `DenomsMetadata` gRPC query to fetch // each denom's associated metadata, either using the bank keeper (for // server-side code) or a gRPC query client (for client-side code). + CoinMetadataQuerier CoinMetadataQueryFn + + // FileResolver are the protobuf files to use for resolving message + // descriptors. If it is nil, the global protobuf registry will be used. + FileResolver *protoregistry.Files + + // TypeResolver are the protobuf type resolvers to use for resolving message + // types. If it is nil, then a dynamicpb will be used on top of FileResolver. + TypeResolver protoregistry.MessageTypeResolver +} + +// SignModeHandler holds the configuration for dispatching +// to specific value renderers for SIGN_MODE_TEXTUAL. +type SignModeHandler struct { + fileResolver *protoregistry.Files + typeResolver protoregistry.MessageTypeResolver coinMetadataQuerier CoinMetadataQueryFn // scalars defines a registry for Cosmos scalars. scalars map[string]ValueRendererCreator @@ -47,10 +64,25 @@ type SignModeHandler struct { } // NewSignModeHandler returns a new SignModeHandler which generates sign bytes and provides value renderers. -func NewSignModeHandler(q CoinMetadataQueryFn) *SignModeHandler { - t := &SignModeHandler{coinMetadataQuerier: q} +func NewSignModeHandler(o SignModeOptions) (*SignModeHandler, error) { + if o.CoinMetadataQuerier == nil { + return nil, fmt.Errorf("coinMetadataQuerier must be non-empty") + } + if o.FileResolver == nil { + o.FileResolver = protoregistry.GlobalFiles + } + if o.TypeResolver == nil { + o.TypeResolver = protoregistry.GlobalTypes + } + + t := &SignModeHandler{ + coinMetadataQuerier: o.CoinMetadataQuerier, + fileResolver: o.FileResolver, + typeResolver: o.TypeResolver, + } t.init() - return t + + return t, nil } // GetFieldValueRenderer returns the value renderer for the given FieldDescriptor. @@ -186,3 +218,48 @@ func (r *SignModeHandler) Mode() signingv1beta1.SignMode { } var _ signing.SignModeHandler = &SignModeHandler{} + +// getValueFromFieldName is an utility function to get the protoreflect.Value of a +// proto Message from its field name. +func getValueFromFieldName(m proto.Message, fieldName string) protoreflect.Value { + fd := m.ProtoReflect().Descriptor().Fields().ByName(protoreflect.Name(fieldName)) + + return m.ProtoReflect().Get(fd) +} + +// coerceToMessage initializes the given desiredMsg (presented as a protov2 +// concrete message) with the values of givenMsg. +// +// If givenMsg is a protov2 concrete message of the same type, then it will +// fast-path to be initialized to the same pointer value. +// For a dynamicpb message it checks that the names match then uses proto +// reflection to initialize the fields of desiredMsg. +// Otherwise throws an error. +// +// Example: +// +// // Assume `givenCoin` is a dynamicpb.Message representing a Coin +// coin := &basev1beta1.Coin{} +// err := coerceToMessage(givenCoin, coin) +// if err != nil { /* handler error */ } +// fmt.Println(coin) // Will have the same field values as `givenCoin` +func coerceToMessage(givenMsg, desiredMsg proto.Message) error { + if reflect.TypeOf(givenMsg) == reflect.TypeOf(desiredMsg) { + // Below is a way of saying "*desiredMsg = *givenMsg" using go reflect + reflect.Indirect(reflect.ValueOf(desiredMsg)).Set(reflect.Indirect(reflect.ValueOf(givenMsg))) + return nil + } + + givenName, desiredName := givenMsg.ProtoReflect().Descriptor().FullName(), desiredMsg.ProtoReflect().Descriptor().FullName() + if givenName != desiredName { + return fmt.Errorf("expected dynamicpb.Message with FullName %s, got %s", desiredName, givenName) + } + + desiredFields := desiredMsg.ProtoReflect().Descriptor().Fields() + for i := 0; i < desiredFields.Len(); i++ { + fd := desiredFields.Get(i) + desiredMsg.ProtoReflect().Set(fd, getValueFromFieldName(givenMsg, string(fd.Name()))) + } + + return nil +} diff --git a/x/tx/signing/textual/valuerenderer_test.go b/x/tx/signing/textual/handler_test.go similarity index 92% rename from x/tx/signing/textual/valuerenderer_test.go rename to x/tx/signing/textual/handler_test.go index a9e22e6770..70c2ea124a 100644 --- a/x/tx/signing/textual/valuerenderer_test.go +++ b/x/tx/signing/textual/handler_test.go @@ -34,7 +34,8 @@ func TestDispatcher(t *testing.T) { for _, tc := range testcases { tc := tc t.Run(tc.name, func(t *testing.T) { - textual := textual.NewSignModeHandler(nil) + textual, err := textual.NewSignModeHandler(textual.SignModeOptions{CoinMetadataQuerier: EmptyCoinMetadataQuerier}) + require.NoError(t, err) rend, err := textual.GetFieldValueRenderer(fieldDescriptorFromName(tc.name)) if tc.expErr { diff --git a/x/tx/signing/textual/int_test.go b/x/tx/signing/textual/int_test.go index e67eb3b3bf..33dbdb4fec 100644 --- a/x/tx/signing/textual/int_test.go +++ b/x/tx/signing/textual/int_test.go @@ -23,7 +23,8 @@ func TestIntJsonTestcases(t *testing.T) { err = json.Unmarshal(raw, &testcases) require.NoError(t, err) - textual := textual.NewSignModeHandler(nil) + textual, err := textual.NewSignModeHandler(textual.SignModeOptions{CoinMetadataQuerier: EmptyCoinMetadataQuerier}) + require.NoError(t, err) for _, tc := range testcases { t.Run(tc[0], func(t *testing.T) { diff --git a/x/tx/signing/textual/message_test.go b/x/tx/signing/textual/message_test.go index 446a80b94b..b4d04b27f9 100644 --- a/x/tx/signing/textual/message_test.go +++ b/x/tx/signing/textual/message_test.go @@ -36,7 +36,7 @@ func TestMessageJsonTestcases(t *testing.T) { err = json.Unmarshal(raw, &testcases) require.NoError(t, err) - tr := textual.NewSignModeHandler(EmptyCoinMetadataQuerier) + tr, err := textual.NewSignModeHandler(textual.SignModeOptions{CoinMetadataQuerier: EmptyCoinMetadataQuerier}) for i, tc := range testcases { t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { rend := textual.NewMessageValueRenderer(tr, (&testpb.Foo{}).ProtoReflect().Descriptor()) diff --git a/x/tx/signing/textual/repeated_test.go b/x/tx/signing/textual/repeated_test.go index 50a66968c5..b2a6a98318 100644 --- a/x/tx/signing/textual/repeated_test.go +++ b/x/tx/signing/textual/repeated_test.go @@ -29,7 +29,7 @@ func TestRepeatedJsonTestcases(t *testing.T) { err = json.Unmarshal(raw, &testcases) require.NoError(t, err) - tr := textual.NewSignModeHandler(mockCoinMetadataQuerier) + tr, err := textual.NewSignModeHandler(textual.SignModeOptions{CoinMetadataQuerier: mockCoinMetadataQuerier}) for i, tc := range testcases { t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { // Create a context.Context containing all coins metadata, to simulate diff --git a/x/tx/signing/textual/timestamp.go b/x/tx/signing/textual/timestamp.go index 9e02c0d219..b4114fd9fd 100644 --- a/x/tx/signing/textual/timestamp.go +++ b/x/tx/signing/textual/timestamp.go @@ -20,15 +20,12 @@ func NewTimestampValueRenderer() ValueRenderer { // Format implements the ValueRenderer interface. func (vr timestampValueRenderer) Format(_ context.Context, v protoreflect.Value) ([]Screen, error) { - // Reify the reflected message as a proto Timestamp - msg := v.Message().Interface() - timestamp, ok := msg.(*tspb.Timestamp) - if !ok { - return nil, fmt.Errorf("expected Timestamp, got %T", msg) + ts := &tspb.Timestamp{} + err := coerceToMessage(v.Message().Interface(), ts) + if err != nil { + return nil, err } - - // Convert proto timestamp to a Go Time. - t := timestamp.AsTime() + t := ts.AsTime() // Format the Go Time as RFC 3339. s := t.Format(time.RFC3339Nano) diff --git a/x/tx/signing/textual/tx_test.go b/x/tx/signing/textual/tx_test.go index 60c1cad64a..d990366087 100644 --- a/x/tx/signing/textual/tx_test.go +++ b/x/tx/signing/textual/tx_test.go @@ -56,7 +56,7 @@ func TestTxJsonTestcases(t *testing.T) { t.Run(tc.Name, func(t *testing.T) { txBody, bodyBz, txAuthInfo, authInfoBz, signerData := createTextualData(t, tc.Proto, tc.SignerData) - tr := textual.NewSignModeHandler(mockCoinMetadataQuerier) + tr, err := textual.NewSignModeHandler(textual.SignModeOptions{CoinMetadataQuerier: mockCoinMetadataQuerier}) rend := textual.NewTxValueRenderer(tr) ctx := addMetadataToContext(context.Background(), tc.Metadata)