Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: math.LegacyDec type is not deserialized in GRPC queries #18430

Open
1 task done
wojtek-coreum opened this issue Nov 10, 2023 · 14 comments · Fixed by #20912
Open
1 task done

[Bug]: math.LegacyDec type is not deserialized in GRPC queries #18430

wojtek-coreum opened this issue Nov 10, 2023 · 14 comments · Fixed by #20912
Assignees
Labels

Comments

@wojtek-coreum
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

What happened?

When querying for inflation using GRPC client:

$ cored query mint inflation --grpc-addr 127.0.0.1:9090 --grpc-insecure

it panics:

panic: invalid Go type math.LegacyDec for field cosmos.mint.v1beta1.QueryInflationResponse.inflation

Call stack:

goroutine 1 [running]:
google.golang.org/protobuf/internal/impl.newSingularConverter({0x33c00d0?, 0x2714860}, {0x33c1508, 0xc000cf0870})
	google.golang.org/protobuf@v1.31.0/internal/impl/convert.go:142 +0xaa8
google.golang.org/protobuf/internal/impl.NewConverter({0x33c00d0, 0x2714860}, {0x33c1508?, 0xc000cf0870?})
	google.golang.org/protobuf@v1.31.0/internal/impl/convert.go:60 +0x9d
google.golang.org/protobuf/internal/impl.fieldInfoForScalar({0x33c1508, 0xc000cf0870}, {{0x242e3c2, 0x9}, {0x0, 0x0}, {0x33c00d0, 0x2714860}, {0x242e3cc, 0x6f}, ...}, ...)
	google.golang.org/protobuf@v1.31.0/internal/impl/message_reflect_field.go:270 +0x17c
google.golang.org/protobuf/internal/impl.(*MessageInfo).makeKnownFieldsFunc(0xc0001c62c0, {0xffffffffffffffff, {0x0, 0x0}, 0xffffffffffffffff, {0x0, 0x0}, 0xffffffffffffffff, {0x0, 0x0}, ...})
	google.golang.org/protobuf@v1.31.0/internal/impl/message_reflect.go:80 +0x78a
google.golang.org/protobuf/internal/impl.(*MessageInfo).makeReflectFuncs(0xc0001c62c0, {0x33c00d0, 0x2400020}, {0xffffffffffffffff, {0x0, 0x0}, 0xffffffffffffffff, {0x0, 0x0}, 0xffffffffffffffff, ...})
	google.golang.org/protobuf@v1.31.0/internal/impl/message_reflect.go:42 +0x58
google.golang.org/protobuf/internal/impl.(*MessageInfo).initOnce(0xc0001c62c0)
	google.golang.org/protobuf@v1.31.0/internal/impl/message.go:90 +0x1b0
google.golang.org/protobuf/internal/impl.(*MessageInfo).init(...)
	google.golang.org/protobuf@v1.31.0/internal/impl/message.go:72
google.golang.org/protobuf/internal/impl.(*messageReflectWrapper).ProtoMethods(0xc0014963d0)
	google.golang.org/protobuf@v1.31.0/internal/impl/message_reflect_gen.go:150 +0x28
google.golang.org/protobuf/proto.protoMethods(...)
	google.golang.org/protobuf@v1.31.0/proto/proto_methods.go:19
google.golang.org/protobuf/proto.UnmarshalOptions.unmarshal({{}, 0x1, 0x1, 0x0, {0x3378ec0, 0xc0000ebd40}, 0x2710}, {0xc000058af8, 0x14, 0x14}, ...)
	google.golang.org/protobuf@v1.31.0/proto/decode.go:93 +0xe8
google.golang.org/protobuf/proto.UnmarshalOptions.UnmarshalState({{}, 0x1, 0x1, 0x0, {0x0, 0x0}, 0x2710}, {{}, {0x33b6e78, 0xc0014963d0}, ...})
	google.golang.org/protobuf@v1.31.0/proto/decode.go:77 +0xa8
github.com/golang/protobuf/proto.UnmarshalMerge({0xc000058af8, 0x14, 0x14}, {0x3385070?, 0xc000206888?})
	github.com/golang/protobuf@v1.5.3/proto/wire.go:67 +0x12b
github.com/golang/protobuf/proto.Unmarshal({0xc000058af8, 0x14, 0x14}, {0x3385070, 0xc000206888?})
	github.com/golang/protobuf@v1.5.3/proto/wire.go:58 +0x53
google.golang.org/grpc/encoding/proto.codec.Unmarshal({}, {0xc000058af8, 0x14, 0x14}, {0x25ff2a0, 0xc000206888})
	google.golang.org/grpc@v1.59.0/encoding/proto/proto.go:53 +0x5f
google.golang.org/grpc.recv(0xc00148c1b0, {0x7ff1bc6c8168, 0x4a03dc0}, 0xc0000ac048?, {0x0, 0x0}, {0x25ff2a0, 0xc000206888}, 0x25ff520?, 0x0, ...)
	google.golang.org/grpc@v1.59.0/rpc_util.go:800 +0x10a
google.golang.org/grpc.(*csAttempt).recvMsg(0xc0015908f0, {0x25ff2a0?, 0xc000206888}, 0x4a03dc0?)
	google.golang.org/grpc@v1.59.0/stream.go:1084 +0x27d
google.golang.org/grpc.(*clientStream).RecvMsg.func1(0x0?)
	google.golang.org/grpc@v1.59.0/stream.go:927 +0x1f
google.golang.org/grpc.(*clientStream).withRetry(0xc00162b320, 0xc00119e6d8, 0xc00119e6c8)
	google.golang.org/grpc@v1.59.0/stream.go:776 +0x13a
google.golang.org/grpc.(*clientStream).RecvMsg(0xc00162b320, {0x25ff2a0?, 0xc000206888?})
	google.golang.org/grpc@v1.59.0/stream.go:926 +0x113
google.golang.org/grpc.invoke({0x33972e0?, 0xc001434090?}, {0x2788fcb?, 0x3396da0?}, {0x25ff520, 0x4a03dc0}, {0x25ff2a0, 0xc000206888}, 0x0?, {0x0, ...})
	google.golang.org/grpc@v1.59.0/call.go:73 +0xcb
google.golang.org/grpc.(*ClientConn).Invoke(0xc001626c00, {0x33972e0?, 0xc001434090?}, {0x2788fcb?, 0x33acfa0?}, {0x25ff520?, 0x4a03dc0?}, {0x25ff2a0?, 0xc000206888?}, {0x0, ...})
	google.golang.org/grpc@v1.59.0/call.go:37 +0x23f
github.com/cosmos/cosmos-sdk/client.Context.Invoke({{0x0, 0x0, 0x0}, {0x33acfa0, 0xc0015886c0}, 0xc001626c00, {0x274255b, 0x10}, {0x33b73d0, 0xc001087bd0}, ...}, ...)
	github.com/cosmos/cosmos-sdk@v0.47.5/client/grpc_query.go:63 +0x7e5
github.com/cosmos/cosmos-sdk/x/mint/types.(*queryClient).Inflation(0xc00119f610, {0x33972e0, 0xc001434090}, 0x33acfa0?, {0x0, 0x0, 0x0})
	github.com/cosmos/cosmos-sdk@v0.47.5/x/mint/types/query.pb.go:356 +0xcb
github.com/cosmos/cosmos-sdk/x/mint/client/cli.GetCmdQueryInflation.func1(0xc000fbcc00, {0xc001436300?, 0x0?, 0x4?})
	github.com/cosmos/cosmos-sdk@v0.47.5/x/mint/client/cli/query.go:76 +0x10f
github.com/spf13/cobra.(*Command).execute(0xc000fbcc00, {0xc0014362c0, 0x4, 0x4})
	github.com/spf13/cobra@v1.7.0/command.go:940 +0x87c
github.com/spf13/cobra.(*Command).ExecuteC(0xc000bc5200)
	github.com/spf13/cobra@v1.7.0/command.go:1068 +0x3a5
github.com/spf13/cobra.(*Command).Execute(...)
	github.com/spf13/cobra@v1.7.0/command.go:992
github.com/spf13/cobra.(*Command).ExecuteContext(...)
	github.com/spf13/cobra@v1.7.0/command.go:985
github.com/cosmos/cosmos-sdk/server/cmd.Execute(0x339a880?, {0x272f3da, 0x5}, {0xc001045dd0, 0x14})
	github.com/cosmos/cosmos-sdk@v0.47.5/server/cmd/execute.go:32 +0x11b
main.main()
	github.com/CoreumFoundation/coreum/v3/cmd/cored/main.go:30 +0x1d0

I guess all the types using math.LegacyDec are affected everywhere.

Cosmos SDK Version

0.47.5

How to reproduce?

No response

@robert-zaremba
Copy link
Collaborator

We have bunch of queries that contain sdk.LegacyDec and it works fine both in web-grpc and RPC (CLI).

@wojtek-coreum
Copy link
Author

@robert-zaremba Did you try the one related to inflation? Same thing is reported here: forbole/callisto#655

@robert-zaremba
Copy link
Collaborator

@robert-zaremba Did you try the one related to inflation? Same thing is reported here: forbole/callisto#655

No, I will try.

@adamewozniak
Copy link

adamewozniak commented Nov 30, 2023

The same thing happens for osmosis's ArithmeticTwap query on the latest version - we're using it to measure differences between Ojo's prices & osmosis's. I'll comment here if we find anything

Code example:

	ctx, cancel := context.WithTimeout(ctx, c.grpcTimeout)
	ctx = metadata.AppendToOutgoingContext(ctx, c.headers...)
	defer cancel()

	queryClient := twaptypes.NewQueryClient(c.grpcConnection)

	res, err := queryClient.ArithmeticTwap(ctx, &req)
	if err != nil {
		return nil, err
	}

	return res, nil

Results in:

panic: invalid Go type math.LegacyDec for field osmosis.twap.v1beta1.ArithmeticTwapResponse.arithmetic_twap

@adamewozniak
Copy link

Update: I found an old SDK issue with the same problem. It was solved by adding this replace:

replace google.golang.org/grpc => google.golang.org/grpc v1.33.2

Relevant issue: #8426

I tested it for my problem above and the patch works for now @robert-zaremba @wojtek-coreum, but it should really be fixed

@alexanderbez
Copy link
Contributor

@adamewozniak by any chance do you know the underlying issue? If a replace of grpc seems to "fix" it, then is it an issue with gRPC or something else?

@adamewozniak
Copy link

@adamewozniak by any chance do you know the underlying issue? If a replace of grpc seems to "fix" it, then is it an issue with gRPC or something else?

Didn't find any details as to the actual fix, though to me it makes the most sense that this bug is somewhere in the SDK if it reappeared only recently with the dec package moving. I'll post here if I have some time to look into it.

Also, good to see you @alexanderbez !

@robert-zaremba
Copy link
Collaborator

When I run locally our latest v0.47 based version (main branch) it works well. We don't do google.golang.org/grpc replace.

image

Same with RPC:

❯ ./build/umeed query mint inflation --grpc-insecure
0.130001914115398131

❯ ./build/umeed query mint inflation
0.130001944988227148

@doggystylez
Copy link

@robert-zaremba when you use --grpc-insecure without a grpc address it does nothing. can you try with a grpc address? I think you will encounter the issue

@doggystylez
Copy link

doggystylez commented Mar 1, 2024

@tac0turtle @julienrbrt can someone pls check this out. the problem occurs for any use of cosmossdk.io/math.LegacyDec and Int in gprc queries as far as i can tell

@dadamu
Copy link
Contributor

dadamu commented Apr 8, 2024

I met the same issue when using this gRPC query, any updates?

@maxrobot
Copy link

maxrobot commented Jul 1, 2024

Just to add another instance of this bug, when trying to call some liquidity queries in Osmosis:

panic: invalid Go type math.LegacyDec for field osmosis.concentratedliquidity.v1beta1.LiquidityNetInDirectionResponse.current_liquidity

@akhilkumarpilli akhilkumarpilli self-assigned this Jul 4, 2024
@akhilkumarpilli akhilkumarpilli moved this from 📋 Backlog to 🤸‍♂️ In Progress in Cosmos-SDK Jul 4, 2024
@akhilkumarpilli akhilkumarpilli moved this from 🤸‍♂️ In Progress to 👀 Waiting / In review in Cosmos-SDK Jul 11, 2024
@akhilkumarpilli akhilkumarpilli linked a pull request Jul 16, 2024 that will close this issue
12 tasks
@github-project-automation github-project-automation bot moved this from 👀 Waiting / In review to 🥳 Done in Cosmos-SDK Jul 16, 2024
@tac0turtle tac0turtle removed this from Cosmos-SDK Sep 4, 2024
@adamewozniak
Copy link

adamewozniak commented Sep 18, 2024

We're getting an instance of this with the newest osmosis version - the replace directive I mentioned doesn't work with github.com/osmosis-labs/osmosis/v26 / github.com/osmosis-labs/cosmos-sdk v0.50.6-v26-osmo-2:

panic: invalid Go type math.LegacyDec for field osmosis.twap.v1beta1.ArithmeticTwapResponse.arithmetic_twap

Update - if anyone else has this problem, we fixed it by overriding the codec GRPC uses to serialize:

import proto "github.com/cosmos/gogoproto/proto"
...
type customCodec struct {
	parentCodec encoding.Codec
}

func (c customCodec) Marshal(v interface{}) ([]byte, error) {
	protoMsg, ok := v.(proto.Message)
	if !ok {
		return nil, fmt.Errorf("failed to assert proto.Message")
	}
	return proto.Marshal(protoMsg)
}

func (c customCodec) Unmarshal(data []byte, v interface{}) error {
	protoMsg, ok := v.(proto.Message)
	if !ok {
		return fmt.Errorf("failed to assert proto.Message")
	}
	return proto.Unmarshal(data, protoMsg)
}

func (c customCodec) Name() string {
	return "gogoproto"
}

// connectGRPC dials up our grpc connection endpoint.
func (c *Client) connectGRPC() error {
	config := &tls.Config{
		InsecureSkipVerify: false,
	}

	customCodec := &customCodec{parentCodec: encoding.GetCodec("proto")}

	grpcOpts := []grpc.DialOption{
		grpc.WithContextDialer(dial),
		grpc.WithInsecure(),
		grpc.WithDefaultCallOptions(grpc.ForceCodec(customCodec)),
	}
	if c.tls {
		grpcOpts = []grpc.DialOption{
			grpc.WithTransportCredentials(credentials.NewTLS(config)),
			grpc.WithContextDialer(dial),
			grpc.WithKeepaliveParams(keepalive.ClientParameters{}),
			grpc.WithDefaultCallOptions(grpc.ForceCodec(customCodec)),
		}
	}

	grpcConn, err := grpc.Dial(
		c.grpcEndpoint,
		grpcOpts...,
	)
	if err != nil {
		return fmt.Errorf("failed to dial Cosmos gRPC service: %w", err)
	}

	c.grpcConnection = grpcConn
	return nil
}

@janfabian
Copy link

@adamewozniak I had the same experience with github.com/InjectiveLabs/cosmos-sdk v0.50.8-inj-0

invalid Go type *math.LegacyDec for field injective.exchange.v1beta1.QuerySpotOrderbookRequest.limit_cumulative_notional

Thanks for your suggested fix, it also works in my case

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants