Skip to content

Add support for VM gRPC services #3294

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

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Add support for VM gRPC services #3294

wants to merge 10 commits into from

Conversation

joshua-kim
Copy link
Contributor

@joshua-kim joshua-kim commented Aug 14, 2024

Why this should be merged

Adds native support for GRPC services being registered over the avalanchego http server. This adds support for streaming via grpc as an alternative to websockets.

How this works

  1. Uses h2c's http handler to support plaintext http2
  2. Makes a change to rpcchainvm to fix a bug where headers that are updated in the plugin process are not reflected on avalanchego's side by returning a response at the end of the Handle rpc that includes headers
  3. Introduces a new rpc for the http request request.Body so that the plugin process can stream the body instead of the current behavior which uses io.ReadAll which may not be possible in the case of a streaming rpc.

How this was tested

Added a e2e test for unary + streaming rpcs over rpcchainvm

@joshua-kim joshua-kim force-pushed the cmux branch 5 times, most recently from 56ba07f to a67b617 Compare August 22, 2024 18:38
@joshua-kim joshua-kim changed the title wip Add support for VM gRPC services Sep 3, 2024
message HttpResponse {
// header is sent to synchronize any header modifications from the plugin
// process
repeated Element header = 1;
Copy link
Contributor Author

@joshua-kim joshua-kim Sep 3, 2024

Choose a reason for hiding this comment

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

grpc requires a header to indicate the end of an rpc, which is directly set onto the map in the plugin process (ref). We need to propagate this update back into avalanchego (I believe current rpcchainvm is not handling plugin-side header updates correctly).

@joshua-kim joshua-kim force-pushed the cmux branch 2 times, most recently from e59b451 to e0a43d2 Compare September 4, 2024 14:41
@joshua-kim joshua-kim self-assigned this Sep 4, 2024
@joshua-kim joshua-kim added the enhancement New feature or request label Sep 4, 2024
@joshua-kim joshua-kim marked this pull request as ready for review September 4, 2024 15:09
Copy link
Contributor

@marun marun left a comment

Choose a reason for hiding this comment

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

Would it be possible to elaborate as to the motivation for this addition? The current 'why this should be merged' in the description seems more like 'what'.


wg.Wait()

require.NoError(stream.CloseSend())
Copy link
Contributor

Choose a reason for hiding this comment

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

(No action required) Will the stream be closed automatically on going out of scope (i.e. this explicit close is superfluous)? If not, maybe ginkgo.DeferCleanup is suggested?

@joshua-kim
Copy link
Contributor Author

Would it be possible to elaborate as to the motivation for this addition? The current 'why this should be merged' in the description seems more like 'what'.

Added

@marun
Copy link
Contributor

marun commented Sep 4, 2024

The upgrade failure is legit - I'm guessing you might need to rebase?

From the logs:

[09-04|15:26:54.904] FATAL chains/manager.go:392 error creating required chain {"subnetID": "11111111111111111111111111111111LpoYY", "chainID": "2BhsWVwQzJL67dSHQfkJ9hEgv79RPURVGeAzmmyeRkkX4tyn8A", "vmID": "mgj786NP7uDwBCcq6YwThhaN8FLyybkCa4zBWTQbNgmK6k9A6", "error": "error while creating new snowman vm database contains incompatible genesis (have ed752c73b66a9c010be62b2cec0088e9e120e6bdd7680fcec50bf880a0b13a87, new 751020f3cac7bf223d5754fef14810191c6669f758a2eb10be17dd31d43e81f2)"}

@joshua-kim
Copy link
Contributor Author

joshua-kim commented Sep 4, 2024

The upgrade failure is legit - I'm guessing you might need to rebase?

I think my coreth branch fell behind master. Gonna rebase my coreth branch again.

Copy link

github-actions bot commented Oct 6, 2024

This PR has become stale because it has been open for 30 days with no activity. Adding the lifecycle/frozen label will cause this PR to ignore lifecycle events.

@joshua-kim joshua-kim requested a review from maru-ava as a code owner April 16, 2025 17:52
@joshua-kim joshua-kim force-pushed the cmux branch 2 times, most recently from b58e928 to c585b94 Compare April 24, 2025 19:31
@joshua-kim joshua-kim changed the base branch from master to rpcchainvm-bug-fix April 24, 2025 19:34
@joshua-kim joshua-kim force-pushed the cmux branch 2 times, most recently from b81c52b to efc3fda Compare April 24, 2025 21:31
Base automatically changed from rpcchainvm-bug-fix to master April 25, 2025 20:30
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
@joshua-kim joshua-kim changed the base branch from master to rpcchainvm-headers-bug April 29, 2025 19:27
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
joshua-kim and others added 5 commits April 30, 2025 11:48
Co-authored-by: Stephen Buttolph <stephen@avalabs.org>
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Co-authored-by: Stephen Buttolph <stephen@avalabs.org>
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Co-authored-by: Stephen Buttolph <stephen@avalabs.org>
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Base automatically changed from rpcchainvm-headers-bug to master May 1, 2025 15:58
@joshua-kim joshua-kim moved this to In Progress 🏗️ in avalanchego May 1, 2025
@joshua-kim joshua-kim moved this from In Progress 🏗️ to Ready 🚦 in avalanchego May 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request lifecycle/frozen
Projects
Status: Ready 🚦
Development

Successfully merging this pull request may close these issues.

3 participants