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

tendermint-proto: grpc server definitions for rpc/abci #1338

Merged
merged 11 commits into from
Aug 29, 2023

Conversation

erwanor
Copy link
Collaborator

@erwanor erwanor commented Jul 24, 2023

Partially addresses #1225. This PR adds gRPC server definitions for the rpc and abci services, but not (yet) for the privval service since it is blocked on #9256 for now. I have added @tony-iqlusion and @tomtau as coauthors since this PR is duplicating work that was previously included-then-yanked from 0.35.

  • Referenced an issue explaining the need for the change
  • Updated all relevant documentation in docs
  • Updated all code comments where relevant
  • Wrote tests
  • Added entry in .changelog/

erwanor and others added 6 commits July 24, 2023 18:11
Co-authored-by: Tomas Tauber <2410580+tomtau@users.noreply.github.com>
Co-authored-by: Tony Arcieri <tony@iqlusion.io>
Co-authored-by: Tomas Tauber <2410580+tomtau@users.noreply.github.com>
Co-authored-by: Tony Arcieri <tony@iqlusion.io>
Co-authored-by: Tomas Tauber <2410580+tomtau@users.noreply.github.com>
Co-authored-by: Tony Arcieri <tony@iqlusion.io>
Co-authored-by: Tomas Tauber <2410580+tomtau@users.noreply.github.com>
Co-authored-by: Tony Arcieri <tony@iqlusion.io>
@codecov-commenter
Copy link

codecov-commenter commented Jul 24, 2023

Codecov Report

Merging #1338 (0576585) into main (c2b5c9e) will increase coverage by 0.2%.
The diff coverage is n/a.

❗ Current head 0576585 differs from pull request most recent head 16f5b5c. Consider uploading reports for the commit 16f5b5c to get more accurate results

@@           Coverage Diff           @@
##            main   #1338     +/-   ##
=======================================
+ Coverage   59.3%   59.5%   +0.2%     
=======================================
  Files        273     272      -1     
  Lines      27095   26973    -122     
=======================================
  Hits       16068   16068             
+ Misses     11027   10905    -122     
Files Changed Coverage Δ
proto/src/lib.rs 100.0% <ø> (ø)

... and 4 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

proto/Cargo.toml Outdated Show resolved Hide resolved
proto/src/prost/v0_34/tendermint.rpc.grpc.rs Show resolved Hide resolved
proto/Cargo.toml Outdated Show resolved Hide resolved
proto/Cargo.toml Outdated Show resolved Hide resolved
@erwanor
Copy link
Collaborator Author

erwanor commented Aug 3, 2023

Thanks for the review @mzabaluev, I have bumped to tonic@0.9, removed the grpc-client feature, and modified the proto compiler to not generate bindings for the rpc server definitions.

However, I was not able to run the proto compiler as I previously did, so the PR has the old definitions:

⋊> ~/p/t/t/proto-compiler on tendermint-proto/grpc-server-traits ⨯ cargo run                                                                                                                                                                                                                                                                                                                                18:18:00
   Compiling tendermint-proto-compiler v0.3.0 (/Users/user/prod/tendermint-rs/tools/proto-compiler)
    Finished dev [unoptimized + debuginfo] target(s) in 0.88s
     Running `/Users/user/prod/tendermint-rs/tools/target/debug/tendermint-proto-compiler`
[info] => Fetching https://github.com/cometbft/cometbft at v0.34.28 into "/Users/user/prod/tendermint-rs/tools/proto-compiler/../target/tendermint"
  [info] => Cloning https://github.com/cometbft/cometbft into /Users/user/prod/tendermint-rs/tools/proto-compiler/../target/tendermint folder
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Error { code: -1, klass: 2, message: "could not remove directory '/Users/user/prod/tendermint-rs/tools/target/tendermint/.git/refs/remotes/origin/pierre/fast-prototyping-1059': parent is not directory: Not a directory" }', proto-compiler/src/functions.rs:42:29
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

@mzabaluev
Copy link
Contributor

mzabaluev commented Aug 4, 2023

@erwanor This looks like a weird problem with the local git repository checkout. Try removing target/tendermint and running the compiler again.

There may be changes in the code generated by tonic-build 0.9, so I'd like to be sure the output is checked in.

@mzabaluev mzabaluev self-requested a review August 4, 2023 18:11
@mzabaluev
Copy link
Contributor

modified the proto compiler to not generate bindings for the rpc server definitions.

They are still generated, but I have slapped the grpc-server attribute on them. Let's leave it at that, generating everything is simpler.

@erwanor
Copy link
Collaborator Author

erwanor commented Aug 14, 2023

Thanks for your help with this, I owe you one!

@erwanor erwanor merged commit cc04d6b into main Aug 29, 2023
23 checks passed
@erwanor erwanor deleted the tendermint-proto/grpc-server-traits branch August 29, 2023 23:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants