feat: load extra chain definitions and method specs at startup#193
Open
robbeverhelst wants to merge 2 commits into
Open
feat: load extra chain definitions and method specs at startup#193robbeverhelst wants to merge 2 commits into
robbeverhelst wants to merge 2 commits into
Conversation
Adds two opt-in env vars that let deployers extend the embedded registries without forking, addressing drpcorg#186: - NODECORE_EXTRA_CHAINS_PATH: path to a YAML file using the same schema as pkg/chains/public/chains.yaml. Its entries are merged on top of the embedded registry. Each extra short-name not already registered is given a dynamically allocated Chain id (>= 1<<30) so Chain.String() round-trips correctly and eth_chainId / net_version answer with the configured chain-id instead of falling back to the spoofed chain. - NODECORE_EXTRA_SPECS_PATH: directory of JSON method-spec files. Wires up the already-exported NewMethodSpecLoaderWithFs constructor. Refactor pkg/chains: - configureChains -> configureChainsFromBytes(rawYaml) so the same code path handles the embedded and the extra YAML. - LoadExtraChains(yamlBytes) public entry-point with conflict checks: rejects duplicate short-names or grpcId collisions with chains that are already registered. - Generated chains_data.go gains dynamicChainNames + a Chain.String() fallback so dynamically allocated ids resolve to their canonical short-name (updated cmd/chains/init_chains.go template). Docs: new section in docs/nodecore/05-upstream-config.md covering the two env vars with a minimal Besu-style example. Tests: pkg/chains/chains_extra_test.go covers the happy path, empty input, duplicate short-name rejection, grpcId collision rejection, and malformed YAML.
Guard against accidental misuse: - sync.Mutex around LoadExtraChains so concurrent callers do not race on the package-level chain registry maps. - A boolean flag flipped on success only. Failed validation leaves the flag false so the caller can retry with corrected input. - Second successful load is rejected with a clear error instead of silently mutating the registry while consumers may already be reading from it. Tests reordered so validation cases run before the happy-path case, plus TestLoadExtraChains_SecondCallIsRejected exercises the lock.
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
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.
Closes #186
What
Two opt-in env vars that let deployers extend nodecore's embedded chain
registry and method specs at startup, without forking the image:
NODECORE_EXTRA_CHAINS_PATHpkg/chains/public/chains.yaml. Merged on top of the embedded registry.NODECORE_EXTRA_SPECS_PATHpkg/methods/specs/*.json). Wires up the already-exportedNewMethodSpecLoaderWithFsconstructor.If both vars are unset, behaviour is identical to today.
Why
The motivating use case is documented in #186: running nodecore in
front of EVM-compatible upstreams that aren't in
drpcorg/publicandnever will be — customer-private Besu / consortium networks with their
own chain ids.
The existing per-upstream
disable-chain-validationflag isn't enoughbecause
eth_chainIdandnet_versionare marked"settings": {"local": true}in
eth.jsonand answered locally from the chain registry. With thechain spoofed as
ethereum, those return0x1instead of the realchain id, which breaks EIP-155 signing in every modern client
(ethers / viem / web3.js / hardhat / foundry).
With this PR, a deployer drops a small
extra-chains.yamlcontainingtheir private chain entry, sets
NODECORE_EXTRA_CHAINS_PATH, and theregistry returns the correct chain-id end-to-end.
How
pkg/chains/chains.goconfigureChains→configureChainsFromBytes(raw []byte)so thesame code path handles the embedded YAML and any extra YAML.
LoadExtraChains(yamlBytes []byte) error. Conflict checks:short-nameagainst an already-registered chain → error.grpcIdcollision against an already-registered chain → error.chainsMap, allocate aChainidstarting at
dynamicChainBaseId(1 << 30) so it doesn't collidewith the codegen-emitted constants, and register it in
dynamicChainNamessoChain.String()round-trips correctly.cmd/chains/init_chains.go(code-generator template)const dynamicChainBaseId Chain = 1 << 30andvar dynamicChainNames = map[Chain]string{}in the generated file.Chain.String()falls through todynamicChainNames[c]for ids inthe dynamic range.
The generated
pkg/chains/chains_data.gois gitignored as today; it'sre-generated next time
make chainsruns.cmd/nodecore/main.goNODECORE_EXTRA_CHAINS_PATHafterflag.Parse()and beforeconfig.NewAppConfig(); callschains.LoadExtraChainsif set.NODECORE_EXTRA_SPECS_PATHnext and usesspecs.NewMethodSpecLoaderWithFs(os.DirFS(path))instead of theembedded loader if set.
Docs
New subsection in
docs/nodecore/05-upstream-config.mdcovering bothenv vars, the schema, and a minimal Besu example.
Tests
pkg/chains/chains_extra_test.go:Chain.String()round-trips,ethmethod-spec inherited)All existing tests still pass;
go vetandgo build ./...clean.Open questions (carried over from #186)
internal/config/config.goalready doesConfigPathVar. Happy tomove it under
app-configif you prefer.override existing ones. Felt like the safe default; I can switch to
"extras win" or make it configurable if you'd rather.
net-versionderivation — extras follow the same fall-throughas embedded entries (explicit
net-versionelse big-int decimal ofchain-id). Worth calling out in case private chains have weirdnet versions.
Happy to iterate on any of the above.
Recreated from the SettleMint fork because the original fork backing #187 was deleted, which closed that PR.