chainlink
chainlink copied to clipboard
Adding gRPC dial capability for mercury
Summary
This adds a grpc (over HTTP/2
) mode to the mercury client, in addition to the currently in production wsrpc mode.
We will need to support both modes for ~4-8 weeks while we learn to operate the service with grpc.
Discussion After lots of iteration, I chose to implement this change by manually changing the autogenerated proto client so that I could add grpc handlers. I found this to be by far the most efficient way to accomplish this - both generics and introducing a new higher level grpc client result in loads of duplicated and unnecessarily complicated code.
What do you need to review?
- Proper test coverage to ensure no regressions to wsrpc behavior
- Review the related WSRPC changes: https://github.com/smartcontractkit/wsrpc/pull/59. Includes the update to the autogeneration script
- Metadata key naming. The key naming of the KV pairs end up in HTTP headers.
Key material in this PR is for testing only and is fine to be leaked.
I see you updated files related to core
. Please run pnpm changeset
in the root directory to add a changeset as well as in the text include at least one of the following tags:
-
#added
For any new functionality added. -
#breaking_change
For any functionality that requires manual action for the node to boot. -
#bugfix
For bug fixes. -
#changed
For any change to the existing functionality. -
#db_update
For any feature that introduces updates to database schema. -
#deprecation_notice
For any upcoming deprecation functionality. -
#internal
For changesets that need to be excluded from the final changelog. -
#nops
For any feature that is NOP facing and needs to be in the official Release Notes for the release. -
#removed
For any functionality/config that is removed. -
#updated
For any functionality that is updated. -
#wip
For any change that is not ready yet and external communication about it should be held off till it is feature complete.
Quality Gate failed
Failed conditions
C Reliability Rating on New Code (required ≥ A)
See analysis details on SonarQube
Catch issues before they fail your Quality Gate with our IDE extension
SonarLint
Quality Gate failed
Failed conditions
8 New Major Issues (required ≤ 5)
C Reliability Rating on New Code (required ≥ A)
See analysis details on SonarQube
Catch issues before they fail your Quality Gate with our IDE extension
SonarLint
We should separate the logic for gRPC and WS more clearly, especially when authentication is involved. Also, we shouldn't use the presence of a config tlsCertFile
to decide on the control flow between gRPC and WS. We need something more explicit.
We should separate the logic for gRPC and WS more clearly, especially when authentication is involved.
Do you have ideas?
If its by separating the higher level abstractions, like Pool
, I'm very hesitant to revisit that - I've already tried that and the code is an absolute mess. Most maintainers should not even be aware of this low level detail, so the code is isolated about as cleanly as I could make it.
Also, we shouldn't use the presence of a config
tlsCertFile
to decide on the control flow between gRPC and WS. We need something more explicit.
It also requires the env var CHAINLINK_MERCURY_USE_GRPC