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.

Sign up for GitHub

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

Jump to bottom

GetBlockByHeight request does not work in GRPCUI #2673

Closed
hdevalence opened this issue Jun 4, 2023 · 4 comments · Fixed by #2826
Closed

GetBlockByHeight request does not work in GRPCUI #2673

hdevalence opened this issue Jun 4, 2023 · 4 comments · Fixed by #2826
Assignees
@conorsch

Comments

@hdevalence
Copy link
Member

Describe the bug

The TendermintProxyService.GetBlockByHeight request does not work or return an error. The network console reports something about a wrong timestamp.

Screen Shot 2023-06-04 at 1 48 10 PM
@erwanor erwanor moved this to Testnet 54: Europa in Testnets Jun 4, 2023
@redshiftzero redshiftzero moved this from Testnet 54: Europa to Testnet 55: Io in Testnets Jun 8, 2023
@conorsch conorsch self-assigned this Jun 22, 2023
@conorsch
Copy link
Contributor

I'll take a look at this one. Not sure if it's a proto/RPC problem, or a grpcui problem. FYI, we're using an old version of grpcui. We'd have to build our own container to get a newer version. Will test locally.

@aubrika aubrika moved this from Testnet 55: Io to Testnet 56: Callisto in Testnets Jun 23, 2023
@aubrika aubrika moved this from Testnet 56: Callisto to Next in Testnets Jun 23, 2023
@conorsch
Copy link
Contributor

Took a look at this today. Even with a locally-built extremely recent version of grpcui, the problem persists. The browser console emits an error about an undefined JS error that is the result of a legitimate bug in the grpcui JS. That's beside the point for our needs, though. Will try to make sense of the actual method call and its parameters next.

@conorsch conorsch moved this from Next to Testnet 56: Callisto in Testnets Jul 6, 2023
@conorsch conorsch moved this from Testnet 56: Callisto to In Progress in Testnets Jul 6, 2023
@conorsch
Copy link
Contributor

conorsch commented Jul 6, 2023

Progress: looks like it's the nanoseconds field in the constructed Block that's causing the error. What's puzzling is that we cast the nanos as an i32, which the type constraints like, but there's an artificial limit imposed on the nano seconds field that's lower than i32:

>>> 1033892547 / 1000000000 
1.033892547
>>> 1033892547 / 2**32
0.24072186718694866
>>>

The 1033892547 value above was taken from ad-hoc debug logs during the Block construction in the get_block_by_height method. Next I'll try to isolate where that max value 1000000000 is coming from.

@conorsch
Copy link
Contributor

Next I'll try to isolate where that max value 1000000000 is coming from.

That value limit is 1 billion, or the maximum number of nanoseconds within one second. The error message is coming from golang/protobuf: https://github.com/golang/protobuf/blob/5d5e8c018a13017f9d5b8bf4fad64aaa42a87308/jsonpb/encode.go#L177-L179. In that part of the rust code, we're calling DateTime::timestamp_nanos, but I think we actually want DateTime::timestamp_subsec_nanos.

conorsch added a commit that referenced this issue Jul 12, 2023
@conorsch
fix(pd): use proper nanoseconds in tm proxy
7bf5dc5 
The gRPCUI service was throwing an error on `TendermintProxyService.GetBlockByHeight`,
with log messages pointing to `ns out of range [0, 1000000000]`.
The relevant pbjson type [0] is not well documented,
but consulting the associated protobuf type [1] makes it clear that we
want subsecond nanos here.

Closes #2673.

[0] https://docs.rs/pbjson-types/0.5.1/pbjson_types/struct.Timestamp.html
[1] https://protobuf.dev/reference/java/api-docs/com/google/protobuf/Timestamp
conorsch added a commit that referenced this issue Jul 12, 2023
@conorsch
fix(pd): use proper nanoseconds in tm proxy
1009427 
The gRPCUI service was throwing an error on `TendermintProxyService.GetBlockByHeight`,
with log messages pointing to `ns out of range [0, 1000000000]`.
The relevant pbjson type [0] is not well documented,
but consulting the associated protobuf type [1] makes it clear that we
want subsecond nanos here.

Closes #2673.

[0] https://docs.rs/pbjson-types/0.5.1/pbjson_types/struct.Timestamp.html
[1] https://protobuf.dev/reference/java/api-docs/com/google/protobuf/Timestamp
@conorsch conorsch mentioned this issue Jul 12, 2023
Merged
conorsch added a commit that referenced this issue Jul 13, 2023
@conorsch
fix(pd): use proper nanoseconds in tm proxy
67c2752 
The gRPCUI service was throwing an error on `TendermintProxyService.GetBlockByHeight`,
with log messages pointing to `ns out of range [0, 1000000000]`.
The relevant pbjson type [0] is not well documented,
but consulting the associated protobuf type [1] makes it clear that we
want subsecond nanos here.

Closes #2673.

[0] https://docs.rs/pbjson-types/0.5.1/pbjson_types/struct.Timestamp.html
[1] https://protobuf.dev/reference/java/api-docs/com/google/protobuf/Timestamp
@github-project-automation github-project-automation bot moved this from In Progress to Testnet 57: Ganymede in Testnets Jul 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@conorsch conorsch

Labels
None yet
Projects
No open projects
Testnets
Status: Testnet 57: Ganymede
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(pd): use proper nanoseconds in tm proxy penumbra-zone/penumbra
2 participants
@hdevalence @conorsch