refactor: make simulator chain-agnostic with pluggable chain families#373
Draft
refactor: make simulator chain-agnostic with pluggable chain families#373
Conversation
|
| Version | Value |
|---|---|
| Current Fork | v1.17.0 |
| Latest Upstream | null |
Action Required
- Review abigen changes in upstream (only the
accounts/abi/binddirectory matters) - Compare with our fork in
cmd/generate-bindings/bindings/abigen/ - If relevant changes exist, sync them and update
FORK_METADATA.md - If no abigen changes, just update the version in
FORK_METADATA.mdtonull
Files to Review
cmd/generate-bindings/bindings/abigen/bind.gocmd/generate-bindings/bindings/abigen/bindv2.gocmd/generate-bindings/bindings/abigen/template.go
cc @smartcontractkit/bix-framework
…rchitecture Introduce ChainFamily interface and package-level registry enabling future Aptos/Solana chain support. Extract all EVM-specific simulation code into cmd/workflow/simulate/chain/evm/ package with self-registration via init(). - Add ChainFamily interface with 7 methods (Name, ResolveClients, RegisterCapabilities, ExecuteTrigger, ParseTriggerChainSelector, RunHealthCheck, SupportedChains) - Add thread-safe chain family registry with Register/Get/All/Names - Move EVM health checks, trigger parsing, supported chains list, and limited capabilities into chain/evm package - Refactor simulate.go to iterate registered families instead of hardcoding EVM logic - Fix EVM chains not being Start()ed after registration - Remove redundant blank import (named import triggers init()) - Rename test stubs for clarity
Fletch153
force-pushed
the
plex-2751-modularise-chain-families
branch
from
b13b64c to
1d3000b
Compare
- Restore missing debug/error log statements in EVM ResolveClients - Restore ExecuteTrigger nil guards (f.evmChains, EVMChains[selector]) - Restore ui.Success for EVM log discovery in fetchAndConvertLog - Restore comments: Use Opaque, runRPCHealthCheck semantics, regex example - Track experimentalSelectors on EVMFamily, pass to RunHealthCheck - Factory-pattern family registration with logger injection (chain.Build) - Extract ResolveKey + ResolveTriggerData to EVM family - Extract simulator_utils.go contents; WorkflowExecutionTimeout to simulate.go - RegisterCapabilities returns []services.Service for lifecycle parity - getTriggerDataForFamily doc comment chain-agnostic - PROMPT.md + SMOKE_TESTS.md audit artifacts (65 tests)
Covers ResolveKey private-key matrix (valid/invalid/sentinel/empty across broadcast and non-broadcast), LimitedEVMChain delegation for every method, trigger hash validation and log-fetch mock scenarios, supported-chains invariants (unique selectors, valid 20-byte forwarders), and extended health-check labelling for experimental, known, and unknown selectors. 100+ new test cases, zero production code changes.
Fletch153
force-pushed
the
plex-2751-modularise-chain-families
branch
from
dab3e25 to
cbf498d
Compare
Replace EVMTxHash/EVMEventIndex on TriggerParams with a generic FamilyInputs string map so each chain family owns its input keys. Unify EVMChainLimits as a type alias for chain.Limits, removing the redundant interface and the type assertion in RegisterCapabilities. Guard against the typed-nil interface trap when boxing SimulationLimits.
…gistry Chain families now own their CLI flag definitions and input collection, removing all EVM-specific knowledge from simulate.go. Fixes a potential deadlock in registry.Get() by extracting namesLocked() helper, and guards ResolveKey to skip families without configured clients.
Fletch153
force-pushed
the
plex-2751-modularise-chain-families
branch
from
88045cc to
b9a4f58
Compare
Drop EVM prefix from the chain.Limits interface method to better reflect the generic abstraction layer.
fetchAndConvertLog now takes a verbose flag. Interactive mode emits ui.Success messages as before; non-interactive mode stays silent, matching the original behavior.
Rename the chain abstraction from "family" to "type" for clearer domain language. EVM is a chain type, not a chain family. - ChainFamily interface → ChainType - EVMFamily struct → EVMChainType - family.go → chaintype.go (file rename) - FamilyClients/Forwarders/Keys/Inputs → ChainTypeClients/etc - getTriggerDataForFamily → getTriggerDataForChainType - Import alias chaintype → corekeys (conflict avoidance) - All comments, error strings, test names updated
- Remove redundant copyloopvar 'tt := tt' (Go 1.22+)
- Remove unused evmCapabilityBaseStub embed in fullStubCapability
- Convert 'switch { case x == a }' to 'switch x { case a }' (staticcheck QF1002)
- Fix goimports formatting
|
| Version | Value |
|---|---|
| Current Fork | v1.17.0 |
| Latest Upstream | null |
Action Required
- Review abigen changes in upstream (only the
accounts/abi/binddirectory matters) - Compare with our fork in
cmd/generate-bindings/bindings/abigen/ - If relevant changes exist, sync them and update
FORK_METADATA.md - If no abigen changes, just update the version in
FORK_METADATA.mdtonull
Files to Review
cmd/generate-bindings/bindings/abigen/bind.gocmd/generate-bindings/bindings/abigen/bindv2.gocmd/generate-bindings/bindings/abigen/template.go
cc @smartcontractkit/bix-framework
Consolidated review-feedback fixes on top of the chain-family refactor.
Interface cleanup (makes the abstraction live up to its chain-agnostic
name):
- chain.Limits: narrow to ChainWriteReportSizeLimit only; move
ChainWriteGasLimit onto a new evm.EVMChainLimits sub-interface so
EVM-specific accessors do not leak into the generic contract.
- chain.ResolvedChains: new struct returned from ResolveClients, carrying
clients, forwarders, and experimental-selector flags. Replaces the
hidden cross-method state the EVM implementation was keeping on its
struct for RunHealthCheck to read.
- ChainType.ResolveClients and ChainType.RunHealthCheck adopt the new
return/argument shape.
User-visible parity with main:
- Per-family error prefix restored when trigger-data resolution fails,
using the family name (e.g. 'Failed to get evm trigger data: ...').
- 'No {name} chain initialized for selector %d' uses the registry-key
name rather than a manually-uppercased variant.
Small cleanups touched on the way:
- Hoist the 'RPC health check failed:' wrap from evm.RunRPCHealthCheck
to the simulate.go caller so a future second chain family cannot
produce doubled headers.
- Remove dead ManualTriggers.Close (no callers before or after this PR).
- Rename the EVMChainLimits test stub to stubEVMLimits to stop it
colliding by one letter with the production EVMChainLimits alias.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
chain.ChainTypeinterface and thread-safe registry enabling pluggable chain support (Aptos, Solana future)cmd/workflow/simulate/chain/evm/withinit()self-registrationsimulate.goto iterate registered chain types instead of hardcoding EVM logicevm.EVMLimitssub-interface so EVM-specific gas limits do not pollute the chain-agnosticchain.Limitschain.StartAll/chain.CloseAlllifecycle helpers with rewind-on-partial-failure semanticschain.ResolvedChainsso chain-type-private state (e.g. EVM experimental-selector set) flows through return values instead of hidden struct fieldsos.Exit(1)path now closes services that had already startedArchitecture
Non-refactor behavior changes in this PR
supported_chains.goexpands the EVM chain-selector list (adds multiple new selectors, un-commentsPHAROS_ATLANTIC_TESTNET). Zero selectors were removed — this is additive. Called out here so the "pure internal refactor" summary is honest.Known follow-ups (NOT blockers)
chain.All()(a map) and breaks on the first match. Deterministic today since EVM is the only registered chain type; must be scoped per-family and iteration-ordered before a second chain type lands.LimitedEVMChaindelegates 12 methods explicitly. Could be reduced to an embeddedevmserver.ClientCapabilitywith overrides onWriteReportonly.CLIFlagTypecovers onlyString/Int. Extend when a family needsUint64/Bool/StringSlice.Test plan
go test ./cmd/workflow/simulate/...)Allreturns copy,Namessorted