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

fix(proto): update DomainType::Proto for ClientState/ConsensusState #3529

Merged
merged 2 commits into from
Dec 21, 2023

Conversation

noot
Copy link
Collaborator

@noot noot commented Dec 21, 2023

Found this bug while testing the IBC implementation with Hermes. Previously it was writing an Any type to state at the client_state/consensus_state storage locations, but the type Hermes was expecting was the ibc_proto ClientState/ConsensusState. This resulted in a proto decoding error in these locations:
https://github.com/informalsystems/hermes/blob/master/crates/relayer/src/client_state.rs#L127
https://github.com/informalsystems/hermes/blob/master/crates/relayer/src/consensus_state.rs#L59

These are called in the Endpoint query_client_state/query_consensus_state methods (for example here https://github.com/informalsystems/hermes/blob/b98ca86991fc1cc6d3f6c595918251a34eb6f1db/crates/relayer/src/chain/cosmos.rs#L1238)

specifically the error I got from Hermes was Error::decode_raw_client_state:

ERROR failed while querying for client 07-tendermint-0 on chain id astria: other error: error decoding raw client state: error decoding buffer into message: failed to decode Protobuf message: Fraction.numerator: ClientState.trust_level: invalid wire type: LengthDelimited (expected Varint)

I can give more details on how I was running everything if needed!

also linted the protos cause CI complained 😭

Copy link
Member

@hdevalence hdevalence left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for the find.

@hdevalence hdevalence merged commit 1eb74b7 into penumbra-zone:main Dec 21, 2023
5 checks passed
@cratelyn cratelyn added the protobuf-changes Makes changes to the protobuf definitions. label Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
protobuf-changes Makes changes to the protobuf definitions.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants