Skip to content
This repository has been archived by the owner on Oct 31, 2024. It is now read-only.

feat(p2p/grpc): creating gRPC behaviour #331

Merged
merged 3 commits into from
Oct 17, 2023

Conversation

Freyskeyd
Copy link
Member

@Freyskeyd Freyskeyd commented Oct 9, 2023

https://img.shields.io/badge/1%20PR-grpc%20over%20p2p-blue

Description

Next PR: #332

This PR adds the gRPC behaviour in order to use gRPC client/server with p2p TCP connections.

The goal is to be able to rely on protobuf schema to dictate our network behaviours, allowing any other services that use gRPC to communicate with us transparently.
It also prevents us to generate and deal with complex custom-made Client/Server.
It also eases the request/response between two nodes and encapsulate the logic into dedicated structs

PR Checklist:

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added or updated tests that comprehensively prove my change is effective or that my feature works

@Freyskeyd Freyskeyd requested a review from a team as a code owner October 9, 2023 18:03
@Freyskeyd Freyskeyd force-pushed the feature/create-grpc-behaviour branch from 496fcbd to 5534046 Compare October 9, 2023 18:12
@codecov
Copy link

codecov bot commented Oct 9, 2023

Codecov Report

Attention: 326 lines in your changes are missing coverage. Please review.

Comparison is base (da2e946) 60.90% compared to head (be119eb) 61.05%.
Report is 8 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #331      +/-   ##
==========================================
+ Coverage   60.90%   61.05%   +0.15%     
==========================================
  Files         226      237      +11     
  Lines       12310    13248     +938     
==========================================
+ Hits         7497     8089     +592     
- Misses       4813     5159     +346     
Files Coverage Δ
crates/topos-p2p/src/behaviour.rs 100.00% <ø> (ø)
...s/topos-p2p/src/behaviour/grpc/handler/protocol.rs 100.00% <100.00%> (ø)
crates/topos-p2p/src/behaviour/grpc/proxy.rs 100.00% <100.00%> (ø)
crates/topos-p2p/src/constant.rs 66.66% <ø> (ø)
crates/topos-test-sdk/build.rs 100.00% <100.00%> (ø)
crates/topos-test-sdk/src/lib.rs 0.00% <ø> (ø)
crates/topos/src/config/tce.rs 65.57% <100.00%> (+2.41%) ⬆️
crates/topos-p2p/src/behaviour/grpc/event.rs 0.00% <0.00%> (ø)
...ates/topos-p2p/src/behaviour/grpc/handler/event.rs 0.00% <0.00%> (ø)
crates/topos-p2p/src/behaviour/grpc/error.rs 0.00% <0.00%> (ø)
... and 7 more

... and 9 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Freyskeyd Freyskeyd force-pushed the feature/create-grpc-behaviour branch 8 times, most recently from db66cf4 to f6f1936 Compare October 10, 2023 13:43
@Freyskeyd Freyskeyd force-pushed the feature/create-grpc-behaviour branch from f6f1936 to 2311ede Compare October 10, 2023 15:32
@Freyskeyd Freyskeyd changed the title feat(p2p/grpc): creating gRPC behaviour feat(p2p/grpc): creating gRPC behaviour https://img.shields.io/badge/1%20PR-grpc%20over%20p2p-blue Oct 11, 2023
@Freyskeyd Freyskeyd changed the title feat(p2p/grpc): creating gRPC behaviour https://img.shields.io/badge/1%20PR-grpc%20over%20p2p-blue feat(p2p/grpc): creating gRPC behaviour 1[https://img.shields.io/badge/1%20PR-grpc%20over%20p2p-blue]() Oct 11, 2023
@Freyskeyd Freyskeyd changed the title feat(p2p/grpc): creating gRPC behaviour 1[https://img.shields.io/badge/1%20PR-grpc%20over%20p2p-blue]() feat(p2p/grpc): creating gRPC behaviour 1[https://img.shields.io/badge/1%20PR-grpc%20over%20p2p-blue](https://img.shields.io/badge/1%20PR-grpc%20over%20p2p-blue) Oct 11, 2023
@Freyskeyd Freyskeyd changed the title feat(p2p/grpc): creating gRPC behaviour 1[https://img.shields.io/badge/1%20PR-grpc%20over%20p2p-blue](https://img.shields.io/badge/1%20PR-grpc%20over%20p2p-blue) feat(p2p/grpc): creating gRPC behaviour ![https://img.shields.io/badge/1%20PR-grpc%20over%20p2p-blue](https://img.shields.io/badge/1%20PR-grpc%20over%20p2p-blue) Oct 11, 2023
@Freyskeyd Freyskeyd changed the title feat(p2p/grpc): creating gRPC behaviour ![https://img.shields.io/badge/1%20PR-grpc%20over%20p2p-blue](https://img.shields.io/badge/1%20PR-grpc%20over%20p2p-blue) feat(p2p/grpc): creating gRPC behaviour Oct 11, 2023
Copy link
Contributor

@dvdplm dvdplm left a comment

Choose a reason for hiding this comment

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

This is a big PR, good job! Can't say I understood the whole thing fully, but overall I think it's looking good. I'm approving this, but it would be good if you addressed the various nitpicks. The one true concern I have is the possibility of race conditions on RequestId.

crates/topos-test-sdk/Cargo.toml Outdated Show resolved Hide resolved
crates/topos-p2p/Cargo.toml Show resolved Hide resolved
crates/topos-p2p/src/constant.rs Outdated Show resolved Hide resolved
crates/topos-p2p/src/constant.rs Outdated Show resolved Hide resolved
crates/topos-p2p/src/tests/behaviour/grpc.rs Show resolved Hide resolved
crates/topos-p2p/src/behaviour/grpc/stream.rs Outdated Show resolved Hide resolved
crates/topos-p2p/src/behaviour/grpc/event.rs Outdated Show resolved Hide resolved
crates/topos-p2p/src/behaviour/grpc/event.rs Outdated Show resolved Hide resolved
crates/topos-p2p/src/behaviour/grpc/connection.rs Outdated Show resolved Hide resolved
Signed-off-by: Simon Paitrault <[email protected]>
@Freyskeyd Freyskeyd merged commit 64dfdfd into main Oct 17, 2023
20 checks passed
@Freyskeyd Freyskeyd deleted the feature/create-grpc-behaviour branch October 17, 2023 10:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants