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

proto: 🔧 conversions for tendermint-* compatibility #4447

Merged
merged 18 commits into from
May 29, 2024

Conversation

cratelyn
Copy link
Contributor

💭 describe your changes

in order to integrate mock consensus tests with the view server, we will need
the ability to respond to requests made by the view server to the tendermint
proxy service.

currently conversions from tendermint, tendermint-rpc, and
tendermint-proto types into penumbra-proto types are defined inline inside
of the TendermintProxyService implementation.

to aid that journey, this branch takes that code and decomposes it into a
family of From<T> and TryFrom<T, Error = E> implementations. this is not
a comprehensive pass at a compatibility layer between penumbra-proto and the
tendermint crates, it is tightly focused on laying the groundwork to
mock this service in tests.

to prevent the additions of new dependencies from interfering with builds in
environments such as wasm32-wasi or wasm32-unknown-unknown, these
facilities are gated behind a #[cfg(feature = "tendermint")] feature flag.

🔖 issue ticket number and link

see #3913.

✅ checklist before requesting a review

  • if this code contains consensus-breaking changes, i have added the "consensus-breaking" label. otherwise, i declare my belief that there are not consensus-breaking changes, for the following reason:

    this does not make behavior changes to any of the code written, it is a
    noöp, like-for-like refactoring. this generalizes logic for future reuse.

cratelyn added 18 commits May 23, 2024 10:08
inherit settings in the package manifest from the workspace.

remove `default-features = true` directives while we are here. they are
*default* featues, this only needs to be explicitly turned *off* via
`default-features = false`.

we also tidy up the dependencies while we are here.
add a link to the RPC this implements, and what it does. add a link to
the proxy type.
there are only two files in this library. rather than reëxporting the
type in the `lib.rs`, just define it there and import it where we use
it.

this puts the important, core type provided by the library right at the
top of the reader's entrypoint.
…nversions

first, we break down the `get_tx(..)` endpoint, outlining the
conversions therein.

- GetTxResponse
- TxResult
- Tag

NB: there is a `TODO(kate)` added here noting what looks like an
errantly disabled boolean in `tendermint::abci::EventAttribute`.
…sync(..)` conversions

- BroadcastTxAsyncResponse
…ync(..)` conversions

- BroadcastTxSyncResponse
…eight(..)` conversions

this is by far the most involved of the endpoints, as it involves
converting blocks between the two libraries.
@cratelyn cratelyn added the A-testing Area: Relates to testing of Penumbra label May 23, 2024
@cratelyn cratelyn added this to the Sprint 7 milestone May 23, 2024
@cratelyn cratelyn self-assigned this May 23, 2024
@cratelyn cratelyn marked this pull request as ready for review May 23, 2024 15:25
@cratelyn cratelyn requested a review from zbuc May 23, 2024 16:45
@cratelyn cratelyn added the A-mock-consensus Area: Relates to the mock consensus engine label May 24, 2024
cratelyn added a commit that referenced this pull request May 24, 2024
see #3913.

this is the exciting culmination of work in pr's #3996, #4349, #4435,
 #4447, #4460, and #4468.

this adds a mock consensus test that spawns a gRPC server, creates a
view server, and then plans a transaction using the `Planner`. the
effects of this are then observed using the `ViewClient`.
Self {
key: key.into_bytes(),
value: value.into_bytes(),
// TODO(kate): this was set to false previously, but it should probably use the
Copy link
Member

@zbuc zbuc May 28, 2024

Choose a reason for hiding this comment

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

good catch

Copy link
Member

@zbuc zbuc left a comment

Choose a reason for hiding this comment

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

LGTM, much cleaner 🥇

@conorsch conorsch merged commit 6ee1ee8 into main May 29, 2024
13 checks passed
@conorsch conorsch deleted the kate/tendermint-proxy-proto-compat branch May 29, 2024 00:25
cratelyn added a commit that referenced this pull request May 29, 2024
see #3913.

this is the exciting culmination of work in pr's #3996, #4349, #4435,
 #4447, #4460, and #4468.

this adds a mock consensus test that spawns a gRPC server, creates a
view server, and then plans a transaction using the `Planner`. the
effects of this are then observed using the `ViewClient`.
cratelyn added a commit that referenced this pull request May 30, 2024
see #3913.

this is the exciting culmination of work in pr's #3996, #4349, #4435,
 #4447, #4460, and #4468.

this adds a mock consensus test that spawns a gRPC server, creates a
view server, and then plans a transaction using the `Planner`. the
effects of this are then observed using the `ViewClient`.
cratelyn added a commit that referenced this pull request May 31, 2024
see #3913.

this is the exciting culmination of work in pr's #3996, #4349, #4435,
 #4447, #4460, and #4468.

this adds a mock consensus test that spawns a gRPC server, creates a
view server, and then plans a transaction using the `Planner`. the
effects of this are then observed using the `ViewClient`.
cratelyn added a commit that referenced this pull request May 31, 2024
see #3913.

this is the exciting culmination of work in pr's #3996, #4349, #4435,
 #4447, #4460, and #4468.

this adds a mock consensus test that spawns a gRPC server, creates a
view server, and then plans a transaction using the `Planner`. the
effects of this are then observed using the `ViewClient`.
cratelyn added a commit that referenced this pull request Jun 3, 2024
see #3913.

this is the exciting culmination of work in pr's #3996, #4349, #4435,
 #4447, #4460, and #4468.

this adds a mock consensus test that spawns a gRPC server, creates a
view server, and then plans a transaction using the `Planner`. the
effects of this are then observed using the `ViewClient`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-mock-consensus Area: Relates to the mock consensus engine A-testing Area: Relates to testing of Penumbra
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants