erigontech / erigontech/erigon

Refactor execution module gRPC interface to plain Go interface

Open
#19,527 1 comment 0 reactions 1 assignee Claimed by @mh0lt View on GitHub
tech debt reduction
Dominant language
Go
Stars
3.6k
Forks
1.5k
Avg merge
1d 15h
Merged PRs (30d)
463

Description

### Problem

The execution module implements `executionproto.ExecutionServer` (19 gRPC methods) defined in `node/gointerfaces/executionproto/execution_grpc.pb.go`, but the gRPC server is never registered — `RegisterExecutionServer()` is never called anywhere in the codebase. All communication is in-process through a pass-through adapter (`node/direct/execution_client.go`) that simply delegates to the server methods, discarding the `grpc.CallOption` args.

This means:
- Proto messages are serialized and immediately deserialized at every call boundary (encoding overhead for no benefit)
- Conversion functions in `execution/execmodule/moduleutil/grpc.go` convert native Go types → proto types → and back to native Go types at the other end
- `emptypb.Empty` parameters on methods like `CurrentHeader`, `Ready`, `GetForkChoice` are pure gRPC ceremony
- The `node/direct/execution_client.go` adapter exists solely to bridge the `ExecutionServer` and `ExecutionClient` interfaces that differ only by the unused `grpc.CallOption` variadic

### Proposal

Replace the gRPC-generated `ExecutionClient`/`ExecutionServer` interfaces with a plain Go interface using native types (`types.Header`, `types.Block`, `types.RawBody`, `common.Hash`, etc.), eliminating the conversion round-trips and the pass-through adapter.

### Scope

#### Phase 1 — Define a plain Go interface

A new `ExecutionModule` interface using native Go types instead of proto types. Roughly 19 methods mirroring the current gRPC surface but with idiomatic Go signatures.

#### Phase 2 — Adapt the ExecModule implementation (6 files)

The server implementation in `execution/execmodule/` currently accepts proto types, converts them to native types, does work, then converts results back to proto types. The refactoring would have methods accept/return native types directly.

| File | Changes | Complexity |
|------|---------|-----------|
| `execution/execmodule/exec_module.go` | Change interface embedding, method signatures | Medium |
| `execution/execmodule/getters.go` | Remove proto→native→proto round-trips (36 refs) | Medium |
| `execution/execmodule/forkchoice.go` | Simplify ForkChoice/Receipt types (26 refs) | Medium |
| `execution/execmodule/inserters.go` | Accept native Block types directly (7 refs) | Low |
| `execution/execmodule/block_building.go` | Simplify assembly request/response (10 refs) | Low |
| `execution/execmodule/moduleutil/grpc.go` | Remove most conversion functions | High (but deletion) |

#### Phase 3 — Update consumers (10 files across 5 packages)

| Package | Files | Impact |
|---------|-------|--------|
| `node/direct/` | `execution_client.go` | **Eliminated entirely** — adapter no longer needed |
| `execution/engineapi/` | `engine_server.go`, `block_downloader.go` | Medium — use native types |
| `execution/execmodule/chainreader/` | `chain_reader.go` (29 refs) | Medium — simplifies significantly |
| `polygon/sync/` | `execution_client.go`, `service.go` (15 refs) | Low-Medium |
| `cl/phase1/execution_client/` | `execution_client_direct.go` (6 refs) | Low |
| `cmd/utils/app/` | `import_cmd.go` (2 refs) | Low |
| `execution/types/` | `block_access_list.go` (11 refs) | Low |

#### Phase 4 — Test updates (2 files)

| File | Changes |
|------|---------|
| `execution/execmodule/exec_module_test.go` | Update test assertions (22 refs) |
| `execution/execmodule/execmoduletester/exec_module_tester.go` | Update test helper (4 refs) |

### Risk assessment

#### Low risk

- The gRPC transport is never used — this is dead infrastructure, not a live wire
- All callers are in-process; no external consumers to break
- Conversion functions already exist proving the type mapping is well-understood
- The higher-level `ExecutionEngine` interface in `cl/phase1/execution_client/interface.go` already abstracts away proto types for Caplin, so the consensus layer is partially insulated
- Changes are largely mechanical (type substitutions), not behavioral
- Existing tests cover the execution module behavior

#### Medium risk

- 21 files across 8 packages need modification — the blast radius is not small
- The proto types have complex nested structures (`Header` has 40+ fields, `BlockAccessList` is 4 levels deep) — new Go struct definitions must be exact equivalents
- `typesproto.H256` / `typesproto.H160` are used for hashes/addresses throughout — these map to `common.Hash` / `common.Address` but the mapping needs care
- The `ExecutionStatus` enum is used in ~60 places — needs a native Go equivalent
- The `node/interfaces` proto is an external submodule shared across projects — changes must not break the proto contract if other consumers exist

#### External proto submodule

The proto definition lives in `github.com/erigontech/interfaces` (a git submodule). Other Erigon ecosystem tools may depend on this proto definition. The refactoring should keep the proto/gRPC generated code in place and add a parallel Go interface, rather than deleting the proto definitions.

### Suggested implementation approach

An incremental migration rather than a big-bang refactoring:

1. Define a new Go interface alongside the existing proto interface (not replacing it)
2. Have `ExecModule` implement both interfaces (the Go one natively, the proto one via thin wrappers)
3. Migrate consumers one package at a time from `executionproto.ExecutionClient` to the new Go interface
4. Delete the `direct.ExecutionClientDirect` adapter and proto conversion code once all consumers are migrated
5. Eventually remove the `ExecutionServer` implementation from `ExecModule` if no external proto consumers exist

Each step can be independently tested and merged.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.