Conversation
winder
force-pushed
the
will/accessor-exec-iface-2
branch
3 times, most recently
from
dec42d8 to
4d76eff
Compare
winder
force-pushed
the
will/accessor-exec-iface-2
branch
from
4d76eff to
56b427f
Compare
There was a problem hiding this comment.
Pull request overview
This PR extends pkg/chainaccess configuration overlays and accessor interfaces so the chainaccess.Registry can construct executor-oriented components (destination readers + contract transmitters) from the same TOML shape used by the executor service.
Changes:
- Add
ExecutorConfig+DestinationChainConfigoverlay types tochainaccess.GenericConfigto read[chain_configuration.<selector>]andmax_retry_duration. - Expand
chainaccess.Accessorto optionally exposeDestinationReaderandContractTransmitter(in addition toSourceReader). - Update the EVM accessor factory and the standalone executor to source destination readers / transmitters from the Registry instead of constructing them directly.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/chainaccess/registry.go | Adds executor config overlay types (ExecutorConfig, DestinationChainConfig) into GenericConfig. |
| pkg/chainaccess/interfaces.go | Extends Accessor with optional DestinationReader + ContractTransmitter accessors and clarifies nil semantics. |
| pkg/chainaccess/registry_test.go | Updates test accessor stub to satisfy the expanded Accessor interface. |
| integration/pkg/accessors/evm/factory_constructor.go | Plumbs executor overlay config and env private key into EVM factory creation; collects RPC URLs. |
| integration/pkg/accessors/evm/factory.go | Makes SourceReader optional and adds construction of DestinationReader + ContractTransmitter. |
| executor/config.go | Embeds chainaccess.DestinationChainConfig into executor ChainConfiguration to preserve TOML field paths. |
| executor/config_test.go | Updates config tests for the embedded DestinationChainConfig. |
| evm/executor_config.go | Updates adapter to populate the embedded DestinationChainConfig. |
| deployment/changesets/apply_executor_config.go | Updates job-spec generation to populate embedded DestinationChainConfig. |
| cmd/executor/standalone/main.go | Switches standalone executor startup to build readers/transmitters via chainaccess.Registry. |
| build/devenv/go.mod | Adds an indirect dependency entry. |
| build/devenv/go.sum | Adds checksums for the new indirect dependency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| OfframpAddress: destCfg.OffRampAddress, | ||
| RmnRemoteAddress: destCfg.RmnAddress, | ||
| ExecutionVisabilityWindow: f.executionVisibilityWindow, | ||
| Monitoring: monitoring.NewNoopExecutorMonitoring(), | ||
| }) |
There was a problem hiding this comment.
Destination reader is always constructed with monitoring.NewNoopExecutorMonitoring(), which disables real executor metrics/tracing even when the executor is configured to use Beholder monitoring. Consider plumbing the Monitoring implementation into the EVM accessor factory (e.g., store it on factory and default to noop only when nil) so destination reader metrics are emitted correctly.
|
|
||
| const ( | ||
| configPathEnvVar = "EXECUTOR_CONFIG_PATH" | ||
| privateKeyEnvVar = "EXECUTOR_TRANSMITTER_PRIVATE_KEY" |
There was a problem hiding this comment.
There should be a better way to inject this into the executor.
There was a problem hiding this comment.
In the bootstrap approach this would be provided by the keystore
| registry, err := chainaccess.NewRegistry(lggr, string(content)) | ||
| if err != nil { | ||
| return nil, nil, err | ||
| } |
There was a problem hiding this comment.
The main change. Create the registry, then use it in place of the other constructor calls that used to be in this file.
winder
changed the title
| ChainConfig Infos[any] `toml:"blockchain_infos"` | ||
|
|
||
| CommitteeConfig | ||
| ExecutorConfig |
There was a problem hiding this comment.
New overlay config. This defines the offramp/rmn addresses and other config used by the new services.
|
Code coverage report:
|
|
|
||
| const ( | ||
| configPathEnvVar = "EXECUTOR_CONFIG_PATH" | ||
| privateKeyEnvVar = "EXECUTOR_TRANSMITTER_PRIVATE_KEY" |
There was a problem hiding this comment.
In the bootstrap approach this would be provided by the keystore
This PR supports a family agnostic executor by: