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

Test z_getsubtreesbyindex using a lightwalletd gRPC request #7521

Merged
merged 6 commits into from
Sep 12, 2023

Conversation

upbqdn
Copy link
Member

@upbqdn upbqdn commented Sep 11, 2023

Motivation

Close #7451.

Specifications

I got the docs for the protobuf message types from https://github.com/zcash/lightwalletd/blob/8269810eeefac56ce326b0f6878c1ea3821b8ad5/docs/rtd/index.html.

Solution

  • Add protobuf types for the lightwalletd's GetSubtreeRoots RPC.
  • Call the RPC, and check the results.

I got bitten by this bug: tokio-rs/prost#332 when I added the enum ShieldedProtocol to the file service.proto. The problem is that prost implicitly derives Eq for enums, so deriving it explicitly via type_attribute causes a conflict. Lukily, there is another method message_attribute that operates only on messages and not enums.

Review

Reviewer Checklist

  • Will the PR name make sense to users?
    • Does it need extra CHANGELOG info? (new features, breaking changes, large changes)
  • Are the PR labels correct?
  • Does the code do what the ticket and PR says?
    • Does it change concurrent code, unsafe code, or consensus rules?
  • How do you know it works? Does it have tests?

I got bitten by this bug: tokio-rs/prost#332
when I added the enum `ShieldedProtocol` to the file `service.proto`.
The problem is that `prost` implicitly derives `Eq` for enums, so
deriving it explicitly via `type_attribute` causes a conflict. Lukily,
there is another method `message_attribute` that operates only on
messages and not enums.
@upbqdn upbqdn added P-Medium ⚡ C-testing Category: These are tests A-rpc Area: Remote Procedure Call interfaces lightwalletd any work associated with lightwalletd A-compatibility Area: Compatibility with other nodes or wallets, or standard rules labels Sep 11, 2023
@upbqdn upbqdn requested a review from a team as a code owner September 11, 2023 13:23
@upbqdn upbqdn self-assigned this Sep 11, 2023
@github-actions github-actions bot added the C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG label Sep 11, 2023
@upbqdn upbqdn removed the C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG label Sep 11, 2023
@github-actions github-actions bot added the C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG label Sep 11, 2023
@upbqdn upbqdn removed the C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG label Sep 12, 2023
Copy link
Collaborator

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

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

Thanks, looks good!

@mergify mergify bot merged commit dc6aa70 into main Sep 12, 2023
340 checks passed
@mergify mergify bot deleted the grpc-test-z-getsubtreesbyindex branch September 12, 2023 23:59
arya2 pushed a commit that referenced this pull request Sep 29, 2023
* Add lightwalletd's protobuf types

* Don't explicitly derive `Eq` for enums

I got bitten by this bug: tokio-rs/prost#332
when I added the enum `ShieldedProtocol` to the file `service.proto`.
The problem is that `prost` implicitly derives `Eq` for enums, so
deriving it explicitly via `type_attribute` causes a conflict. Lukily,
there is another method `message_attribute` that operates only on
messages and not enums.

* Test the `z_getsubtreesbyindex` RPC

* Fix a typo

* Add test vectors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-compatibility Area: Compatibility with other nodes or wallets, or standard rules A-rpc Area: Remote Procedure Call interfaces C-testing Category: These are tests lightwalletd any work associated with lightwalletd
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test z_getsubtreesbyindex using a lightwalletd gRPC request
2 participants