Skip to content
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

architecture: infra should not depend on interfaces #2448

Open
battlmonstr opened this issue Oct 24, 2024 · 0 comments
Open

architecture: infra should not depend on interfaces #2448

battlmonstr opened this issue Oct 24, 2024 · 0 comments

Comments

@battlmonstr
Copy link
Contributor

infra is a collection of generic low level components.
interfaces contains code generated from .proto files for each of the GRPC services we implement

currently infra depends on interfaces for the following reasons:

  1. client_context_pool_test and call_test depend on kv.proto as a service example to do the calls against
  2. mock fix 24351: extra mocks added on top of existing generated mocks to fix an bug
  3. version.hpp: defines versions of each service and helpers to call rpc Version method (it exists on all services)
  4. conversion.hpp: defines convertions between numeric types from types.proto (e.g. ::types::H256 and intx::uint256, types::H160 and evmc::address etc.)

Logically each generated interface is a requirement only for each particular service client/server implementation. interfaces should be decoupled from infra, otherwise since everything depends on infra everything ingests low level details about all existing services.

Short term:

  1. move tests to a higher level module, e.g. a module that implements KV server/client.
  2. generate mock fix 24351 mocks within interfaces (or update grpc to fix 24351)
  3. version.hpp: the methods are only used in daemon.cpp Daemon::run_checklist(), so wait_for_xxx_protocol_check helper methods could be moved to rpc. The actual version number constants should be moved to the implementation of each service. ProtocolVersion struct can and wait_for_protocol_check template be kept in infra.
  4. conversion.hpp: move to a separate library that depends on types.proto (types.pb.h) and implements conversions. This could be a sub-library of infra.

Long term:
Instead of all services being generated in one place (interfaces). We could refactor cmake code gen functions to be able to work with any service .proto with details about a particular service provided as inputs. Using these functions the actual code gen can be used in other modules. Each implementation module defines targets like silkworm_interfaces_xxx for internal use. For example, sentry defines silkworm_interfaces_sentry, which is then used for sentry client/server impls.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants
@battlmonstr and others