Skip to content

Support header-based routing#4036

Merged
StephenButtolph merged 8 commits intomasterfrom
xsvm-connectrpc
Jul 11, 2025
Merged

Support header-based routing#4036
StephenButtolph merged 8 commits intomasterfrom
xsvm-connectrpc

Conversation

@joshua-kim
Copy link
Copy Markdown
Contributor

Why this should be merged

We currently decouple our routers across protocols, but this doesn't support connectrpc which is agnostic to protocols, and wants to handle h1 + h2.

How this works

  1. Supports routing based on a routing header
  2. Refactors our handlers on our vm interface to be against a single http handler, agnostic of protocol
  3. handlers must implement their own routing

How this was tested

CI passes

Need to be documented in RELEASES.md?

Yes

Comment thread api/server/router.go
Comment thread api/server/server.go Outdated
func (s *server) RegisterChain(chainName string, ctx *snow.ConsensusContext, vm common.VM) {
ctx.Lock.Lock()
handlers, err := vm.CreateHandlers(context.TODO())
handler, err := vm.NewHTTPHandler(context.TODO())
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We also did a rename, which I can also undo. But since we're already messing with this interface signature it seemed forgivable to try to sneak in.

Comment thread connectproto/buf.gen.yaml
- name: go-grpc
out: pb
opt: paths=source_relative
- plugin: connect-go
Copy link
Copy Markdown
Contributor Author

@joshua-kim joshua-kim Jun 25, 2025

Choose a reason for hiding this comment

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

We need to compile our connectrpc stuff with this plugin - so for now I think separate buf modules are the cleanest way to do this. I think ideally it's all under proto, so we'd have a dir structure like proto/connect and proto/everythingelse, but if we do a move in this PR it's going to make the diff huge so I made them both top-level dirs for this PR and I think we should do the move under a single proto dir in a follow-up.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Follow-up issue: #4066

@joshua-kim joshua-kim added the DO NOT MERGE This PR must not be merged in its current state label Jun 25, 2025
@joshua-kim joshua-kim moved this to In Progress 🏗️ in avalanchego Jun 25, 2025
@joshua-kim joshua-kim self-assigned this Jun 25, 2025
Comment thread vms/example/xsvm/api/ping.go
Copy link
Copy Markdown
Contributor

@maru-ava maru-ava left a comment

Choose a reason for hiding this comment

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

Flake and e2e changes lgtm

Base automatically changed from rpcchainvm-eof to master July 1, 2025 17:11
Comment thread api/server/router.go Outdated
Comment thread api/server/router.go Outdated
Comment thread api/server/router.go Outdated
Comment thread api/server/router.go
Comment thread proto/vm/vm.proto Outdated
Comment thread vms/avm/vm.go Outdated
Comment thread vms/example/xsvm/api/ping.go
Comment thread vms/rpcchainvm/ghttp/greader/greader_test.go
Comment thread vms/rpcchainvm/vm_client.go Outdated
Comment thread vms/rpcchainvm/vm_server.go
@joshua-kim joshua-kim linked an issue Jul 8, 2025 that may be closed by this pull request
@joshua-kim joshua-kim requested a review from rrazvan1 as a code owner July 9, 2025 19:23
@joshua-kim joshua-kim force-pushed the xsvm-connectrpc branch 9 times, most recently from d7153f7 to 5d62147 Compare July 9, 2025 21:01
@joshua-kim joshua-kim removed the request for review from rrazvan1 July 9, 2025 21:12
Comment thread api/connectclient/client.go Outdated
Comment thread api/server/router.go
Comment thread go.mod Outdated
Comment thread utils/resource/usage.go Outdated
Comment thread snow/engine/common/vm.go Outdated
joshua-kim and others added 5 commits July 10, 2025 15:34
Co-authored-by: Stephen Buttolph <[email protected]>
Signed-off-by: Joshua Kim <[email protected]>
Co-authored-by: Stephen Buttolph <[email protected]>
Signed-off-by: Joshua Kim <[email protected]>
Signed-off-by: Joshua Kim <[email protected]>
Signed-off-by: Joshua Kim <[email protected]>
Signed-off-by: Joshua Kim <[email protected]>
Comment thread api/server/router.go Outdated
Comment thread utils/resource/usage.go
joshua-kim and others added 2 commits July 11, 2025 11:00
Co-authored-by: Stephen Buttolph <[email protected]>
Signed-off-by: Joshua Kim <[email protected]>
Co-authored-by: Stephen Buttolph <[email protected]>
Signed-off-by: Joshua Kim <[email protected]>
@StephenButtolph StephenButtolph added this pull request to the merge queue Jul 11, 2025
@joshua-kim joshua-kim removed the DO NOT MERGE This PR must not be merged in its current state label Jul 11, 2025
Merged via the queue into master with commit 66ce7a7 Jul 11, 2025
29 checks passed
@StephenButtolph StephenButtolph deleted the xsvm-connectrpc branch July 11, 2025 15:56
@github-project-automation github-project-automation bot moved this from In Progress 🏗️ to Done 🎉 in avalanchego Jul 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Consecutive HTTP/2 API Calls Failing Since avalanchego Version v1.13.2

3 participants