Conversation
There was a problem hiding this comment.
Pull request overview
Adds Emerald gRPC support to Nodecore by importing/providing generated protobuf bindings, wiring up a gRPC server, and introducing optional JWT-based gRPC authentication.
Changes:
- Added generated Emerald protobuf + gRPC stubs under
pkg/dshackleand a Makefile target to regenerate them from theemerald-grpcsubmodule. - Implemented gRPC server bootstrap plus Blockchain/Auth services (including session-based auth).
- Extended chain configuration to include
GrpcIdresolution and added tests around gRPC chain lookup and gRPC service logic.
Reviewed changes
Copilot reviewed 20 out of 22 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/dshackle/common.pb.go | Adds generated protobuf types/enums used by Emerald API (ChainRef, addresses, etc.). |
| pkg/dshackle/blockchain_grpc.pb.go | Adds generated gRPC client/server interfaces for Blockchain service. |
| pkg/dshackle/auth_grpc.pb.go | Adds generated gRPC client/server interfaces for Auth service. |
| pkg/dshackle/auth.pb.go | Adds generated protobuf messages for Auth request/response. |
| pkg/chains/chains.go | Adds GrpcId to chain config and lookup map for resolving chains from gRPC enum IDs. |
| pkg/chains/chains_test.go | Adds tests for GetChainByGrpcId behavior. |
| internal/server/grpc_server.go | Wires up and runs a gRPC server (TLS optional), registers Auth/Blockchain services. |
| internal/server/grpc_blockchain.go | Implements Blockchain streaming RPCs and subscription mapping/heartbeats. |
| internal/server/grpc_blockchain_test.go | Adds tests for native call chunking, unauthenticated behavior, and subscribe mapping. |
| internal/server/grpc_auth.go | Implements JWT-based auth exchange and session store/validation. |
| internal/server/grpc_auth_test.go | Adds tests for auth success/disabled/invalid token cases and session TTL/auth. |
| internal/config/server_config.go | Adds grpc-port and grpc-auth config + validation. |
| internal/config/defaults.go | Adds defaults for GrpcAuthConfig. |
| internal/config/server_config_test.go | Updates server config defaults test to include GrpcAuthConfig. |
| internal/config/config_test.go | Updates full-config test to include GrpcAuthConfig. |
| internal/app/app.go | Starts the gRPC server alongside the HTTP server. |
| Makefile | Adds dshackle-proto-gen target to generate protobuf bindings from emerald-grpc. |
| .gitmodules | Adds emerald-grpc submodule reference. |
| emerald-grpc | Adds the emerald-grpc submodule pointer (commit ref). |
| go.mod | Adds direct deps for grpc and protobuf, plus genproto rpc indirect. |
| go.sum | Adds checksums for newly pulled deps. |
Comments suppressed due to low confidence (1)
pkg/chains/chains.go:1
- UnknownChain
doesn’t set a sentinel value forChain. With the new gRPC resolution flow, leavingChainat its zero value risks mapping unknown gRPC IDs to a real chain (commonly index 0). SetUnknownChain.Chainto a negative sentinel (e.g., -1) and keep downstream checks consistent, or adjustGetChainByGrpcId`/resolution logic to make unknown unambiguously invalid.
package chains
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
internal/server/grpc_blockchain.go
Outdated
| specMethod = chainSupervisor.GetMethod(item.GetMethod()) | ||
| } | ||
| requestID := strconv.FormatUint(uint64(item.GetId()), 10) | ||
| requests = append(requests, protocol.NewUpstreamJsonRpcRequest( |
There was a problem hiding this comment.
It creates a non-streaming request, but it's a good idea to control it through the API params
There was a problem hiding this comment.
Addressed in f568dc0.
What was added:
- Streaming request mode is now selected for stream-capable methods in
buildNativeCallRequests(protocol.NewStreamUpstreamJsonRpcRequestforeth_getLogs/getProgramAccounts):internal/server/grpc_blockchain.go. - Chunk size is controlled through API input (
NativeCallRequest.ChunkSize) and passed into reply emission inNativeCall:sendNativeCallReplyItems(stream, wrapper, request.GetChunkSize())ininternal/server/grpc_blockchain.go.
internal/server/grpc_blockchain.go
Outdated
| return append([]byte(nil), response.ResponseResult()...), nil | ||
| } | ||
|
|
||
| rawStreamResponse, err := io.ReadAll(response.EncodeResponse([]byte("0"))) |
There was a problem hiding this comment.
However, even if we create a streaming request, we read all the data at once. It suppresses the streaming feature
There was a problem hiding this comment.
Addressed in f568dc0.
It no longer reads the whole JSON-RPC body into memory for streaming path:
streamNativeCallPayloadnow processes anio.Readerand emits gRPC chunks incrementally.streamJsonRPCResultextracts only theresultfield from the streaming JSON-RPC envelope token-by-token, then writes to chunk emitter.- Parser was moved into a dedicated file with dedicated tests:
internal/server/json_rpc_result_stream.gointernal/server/json_rpc_result_stream_test.go.
internal/server/grpc_blockchain.go
Outdated
| return nil, false, nil | ||
| } | ||
|
|
||
| var envelope struct { |
There was a problem hiding this comment.
In SubscriptionRequestProcessor.ProcessRequest we already have two fields - the whole event and result, so I think it's possible to pass a special flag here to return in that case the result field only instead of the whole event
There was a problem hiding this comment.
Addressed in f568dc0.
Implemented result-only subscription mode:
- NativeSubscribe now sets
WithSubscriptionResultOnly(true)when building upstream request (internal/server/grpc_blockchain.go). SubscriptionRequestProcessorchecks this flag and sendsNewSubscriptionResultEventResponse(result-only payload) instead of full event frame (internal/upstreams/flow/sub_processor.go).- Flag plumbing is in
UpstreamJsonRpcRequest(WithSubscriptionResultOnly/IsSubscriptionResultOnly) ininternal/protocol/json_rpc_request.go.
No description provided.