relayer: config simplification - #1328
Conversation
…list Replaces relayer.clients[]+routesToRelay[] (joined by alias, with duplicated counterparty fields) with relayer.connections[], each holding two client ends (clientA/clientB) whose counterparty is simply the other end. Replaces attestor.attestations[] + embedded per-client AttestorEntry objects with one top-level attestors[] list (local and remote together), referenced by alias from attestorSet.attestors[]. Updates every downstream consumer of the old shape: pipeline opts, dispatch routing, proofgen attestor resolution, relayer packet handling, bootstrap's dual-mode gate, and the attestor service/local attestor.
Rewrites testdata/sample.yml and the config_test.go/relayer_test.go table-driven cases to match relayer.connections[]/top-level attestors[]. Updates every other test file constructing the old ClientConfig/ RouteConfig/AttestationConfig literals inline.
ibc config validate --live now queries each connection's two chains' routers (ICS02Client.getCounterparty) and confirms the on-chain registered counterparty actually matches clientA/clientB, in both directions -- catching a config that names two clients as counterparties when the chains themselves disagree. Adds chains.Client.GetCounterparty, regenerates its mock.
e2e/internal/harness/ibclink hand-mirrors link's config YAML shape to generate configs for containerized relayer/attestor processes in e2e tests. Since it's a separate Go module, the internal/config refactor didn't surface this as a compile error -- it would have silently kept writing the old clients[]+routesToRelay[]+attestor.attestations[] shape. Updates the file-shape types and buildRelayerFileConfig to emit relayer.connections[] and a unified top-level attestors[] list. Also fixes a real gap the old code never needed: attestors referenced by an explicit RelayerAttestorSet with type: remote were never declared in the unified list (only locals were), which the real schema's cross-validation now requires.
…solidate live validation and attestor resolution
Greptile SummaryThe PR replaces static per-client attestor and finality configuration with startup-time on-chain quorum resolution, connection-first bidirectional configuration, and proof-generator-backed finality checks.
Confidence Score: 5/5The PR appears safe to merge after the acknowledged branch rebase, with no concrete blocking or independently actionable non-blocking defects identified in the reviewed changes. The connection model rejects ambiguous client ends, live validation checks both counterparties, startup quorum matching fails safely when configured attestors cannot satisfy on-chain requirements, and finality processors consistently use route-specific provable heights. Important Files Changed
|
…into livevalidate
…n-first-config # Conflicts: # e2e/internal/e2etest/traffic_setup.go # e2e/internal/harness/ibclink/attestor.go # link/docs/configuration.md # link/internal/chains/evm/client.go # link/internal/chains/evm/client_test.go # link/internal/relay/dispatch/dispatcher_test.go # link/internal/relay/pipeline/pipeline_test.go # link/internal/service/attestor/service_test.go
…tion-first schema
| // Dual mode: if .attestor config is provided, then we can run both relayer and attestor in the same process. | ||
| // This might be useful for PoC/testing environments or when an operator wants to run the relayer | ||
| // Attestors | ||
| local, remote, err := attestor.ResolveFromConfig(ctx, cfg.Attestors, clientSet, signers) |
There was a problem hiding this comment.
We resolve both local and remote attestors up front now as opposed to the previous flow which was confusing. We previously initialized the attestor grpc service handlers, which internally resolved the local attestors. We'd pipe that attestor service into the ProofGenerators which retrieved local attestors from it and did their own remote attestor resolution as needed.
| output = pipeline.ProcessConcurrently(ctx, stageConcurrency, | ||
| NewProcessorMW(deps.Storage, processors.NewCheckPacketCommitment(deps.Chains, deps.Storage)), output) | ||
|
|
||
| // wait for the send tx to finalize on the source chain |
There was a problem hiding this comment.
We pipe the ProofGenerators directly into the processors that handle waiting for tx finality so that they can query the LatestProvableHeight method to determine when a packet commitment can be relayed. This replaces static finality offsets configs.
| // MatchAttestors resolves self's on-chain attestation set and returns the | ||
| // attestors that watch counterparty's chain and whose self-reported address | ||
| // appears in it, erroring if too few match to meet the on-chain threshold. | ||
| func MatchAttestors( |
There was a problem hiding this comment.
We no longer specify in the configs which attestors belong to which light client in the relayer configs. It is replaced with this logic that associates the attestors with their light clients on startup. This lets us get rid of the attestorSet config block entirely. The attestorSet config block was confusing to dog fooders because you configure it under the client for one chain but it actually should be attesting to the counterparty client's chain.
| rpc LatestHeight(LatestHeightRequest) returns (LatestHeightResponse); | ||
|
|
||
| // Returns identity information about a configured attestor. | ||
| rpc Info(InfoRequest) returns (InfoResponse); |
There was a problem hiding this comment.
Added an info endpoint to the attestor so that the relayer can discover the attestor's chain id and address on startup.
| continue | ||
| } | ||
|
|
||
| matched = append(matched, a) |
There was a problem hiding this comment.
might want to dedup here. with two configured attestors with the same attestor key this could "reach quorum" according to this code (misconfiguration probably, but it would create some confusing errors later).
| if attestation, ok := cfg.AttestationByName(token); ok { | ||
| alias = attestation.Signer | ||
| if attestor, ok := cfg.AttestorByName(token); ok { | ||
| alias = attestor.Signer |
There was a problem hiding this comment.
Given:
attestors:
- name: attestor-1
type: remote
grpc: attestor.example.com:3000
And:
ibc deploy client
--chain 1
--counterparty-chain 8453
--attestors attestor-1
The failure is:
- The CLI finds attestor-1 in attestors[].
- link/cmd/ibc/attestors.go:13 tries to resolve its signer.
- The resolver fails to find a signer and returns the original string "attestor-1".
- The EVM driver receives that string and rejects it as invalid attestor address "attestor-1".
| // wait for the send tx to finalize on the source chain | ||
| // wait for the send tx's packet event to be at or before what the | ||
| // destination client can currently prove | ||
| checkSendFinality, err := processors.NewCheckSendFinality(deps.Chains, deps.ProofGenerators, route) |
There was a problem hiding this comment.
🤖: These per-transfer finality checks duplicate the proof-height/timestamp gates already enforced by BatchRecvPacket, BatchTimeoutPacket, and BatchAckPacket. This adds redundant RPCs and leaves TxHeight plus the finalized-time fields with no independent runtime purpose.
| AutoRelay AutoRelayConfig `yaml:"autoRelay,omitempty"` | ||
| // AutoRelay configures auto-relay for packets flowing FROM this end's | ||
| // chain TOWARD the counterparty end. | ||
| AutoRelay AutoRelayConfig `yaml:"autoRelay,omitempty"` |
There was a problem hiding this comment.
🤖: autoRelay.enabled and lookback have no runtime readers. In particular, enabled: false does not disable anything, so the config accepts and documents values that silently have no effect.
| // relays, in both directions. ClientA's counterparty is simply ClientB (and | ||
| // vice versa). | ||
| type ConnectionConfig struct { | ||
| Alias string `yaml:"alias"` |
There was a problem hiding this comment.
the connection alias is required and uniqueness-validated, but ConnectionByAlias has no production callers and is otherwise only used in diagnostics atm. should we remove it?
| return New(attestorsSpecs) | ||
| } | ||
|
|
||
| // New Service constructor. Attestors should have unique aliases |
There was a problem hiding this comment.
code smell? Alias() is identical to Name() for both implementations and only used to key Service. can we drop it?
| clients := make(map[string]string, 2*len(cfg.Connections)) | ||
| type localAttestor struct{ chainID, keyFile string } | ||
| locals := make(map[string]localAttestor) | ||
| sourceClients := make(map[string]struct{}, 2*len(cfg.Connections)) |
There was a problem hiding this comment.
🤖: sourceClients exists only to validate cfg.Routes; those routes never reach the generated YAML, while Connections already configure both directions. The harness shape therefore suggests route-level control that the relayer config no longer has.
| } | ||
|
|
||
| remote = append(remote, a) | ||
| default: |
There was a problem hiding this comment.
🤖: The unsupported-type fallback here and the missing-gRPC check in resolveRemote duplicate authoritative config validation. Production callers cannot reach either state, so maintaining the same schema policy in this resolver risks drift.
| } | ||
|
|
||
| // Locals returns the subset of attestors this process runs itself. | ||
| func (a Attestors) Locals() []AttestorConfig { |
There was a problem hiding this comment.
looking at how this is used, it's unclear if we need this?
| attestorIDs := env.Attestors() | ||
| attestorSets := make(map[string]*ibclink.RelayerAttestorSet, len(attestorIDs)) | ||
| for _, id := range attestorIDs { | ||
| declaredAttestors := map[string]bool{} |
There was a problem hiding this comment.
🤖: Environment.Attestors() enumerates a map whose IDs were already uniqueness-validated, so duplicates cannot reach this loop. declaredAttestors only rechecks an invariant already guaranteed structurally.
Summary
attestorSetconfig and local attestors were defined in the top levelattestorconfiguration block. Now remote and local attestor definition is consolidated in a top levelattestorsconfig.attestorSetconfig(threshold + attestor list + finality offset, per client end) and instead associates the attestors configured in the top levelattestorsblock with their corresponding light client by associating their self-reported addresses with the addresses queried from the on-chain attestation light client. A misconfigured or stale quorum now fails at startup instead of silently drifting from what the chain actually enforces.attestorSetto determine when a packet commitment is finalized. This PR removes that as a concept and instead just waits till all the attestors report they can attest to the height of a commitment. This happens in the Relayer's CheckSendFinality/CheckTimeoutFinality/CheckWriteAckFinality pipeline stages where they now determine finality by calling LatestProvableHeight() on the attestor light client implementation of the ProofGenerator abstraction.The refactors required to support the new configuration format are quite large - I added some comments in the diff to help reviewers parse the changes, but let me know if anyone prefers me to split into smaller PRs.
Closes FOU-1219, FOU-1221