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]: Parsing error #19874

Closed
1 task done
hg-cardinalchain opened this issue Mar 26, 2024 · 5 comments · Fixed by #19923
Closed
1 task done

[Bug]: Parsing error #19874

hg-cardinalchain opened this issue Mar 26, 2024 · 5 comments · Fixed by #19923
Labels

Comments

@hg-cardinalchain
Copy link

hg-cardinalchain commented Mar 26, 2024

Is there an existing issue for this?

  • I have searched the existing issues

What happened?

This transaction on the mainnet, https://www.mintscan.io/cosmos/tx/18F9E67C39156EE8C6C39892E22BEC6C4F61121C0C0784C7635F3A53DCB5C888?height=19664155
contains non utf-8 characters in the raw log which causes parsing errors from protobuf-java when querying transaction events (cosmos.tx.v1beta1.Service/GetTxsEvent).

the proto docs state

A string must always contain UTF-8 encoded or 7-bit ASCII text, and cannot be longer than 232.

Cosmos SDK Version

0.47.10

How to reproduce?

No response

@facundomedica
Copy link
Member

This is the result of doing string([]byte(...)). If possible you should cut the last part and avoid decoding it as UTF-8.

Maybe we can do something like this for all the places we have this issue, although I don't know if it can cause breakage:

- return errorsmod.Wrapf(authz.ErrNoAuthorizationFound, "failed to delete grant with key %s", string(skey))
+ return errorsmod.Wrapf(authz.ErrNoAuthorizationFound, "failed to delete grant with key %X", skey)

That way they'd get printed as hex.

@hg-cardinalchain
Copy link
Author

hg-cardinalchain commented Mar 27, 2024

If possible you should cut the last part and avoid decoding it as UTF-8.

Doesn't really seem like a good long term solution, the grpc client code we use is all auto generated from the proto files in this repository.

@EmilGeorgiev
Copy link
Contributor

@hg-cardinalchain Could you clarify your concerns? Are you suggesting that we need a human-readable string representation of the key, and because the protobuf clients are auto-generated, they cannot convert the hex into a human-readable format?

Also, we know that the key is constructed from: 0x01 | granterAddressLen (1 Byte) | granterAddress_Bytes | granteeAddressLen (1 Byte) | granteeAddress_Bytes | msgType_Bytes. We can write a function that converts this key into a valid UTF-8 encoded string. Can this solve the issue?

@EmilGeorgiev
Copy link
Contributor

Can I take this issue?

@hg-cardinalchain
Copy link
Author

hg-cardinalchain commented Apr 2, 2024

Could you clarify your concerns? Are you suggesting that we need a human-readable string representation of the key, and because the protobuf clients are auto-generated, they cannot convert the hex into a human-readable format?

Sure. A hex string for the key should be fine. The string just needs to be valid UTF-8.

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.

3 participants