refactor: inject purpose keys structurally through SeismicNode#360
Open
ameya-deshmukh wants to merge 2 commits intoseismicfrom
Open
refactor: inject purpose keys structurally through SeismicNode#360ameya-deshmukh wants to merge 2 commits intoseismicfrom
ameya-deshmukh wants to merge 2 commits intoseismicfrom
Conversation
SeismicNode::new(keys) stores purpose keys and threads them through the executor builder, avoiding reliance on the global OnceLock for the main node path. The global fallback is retained for tests and CLI stage command.
ameya-deshmukh
force-pushed
the
ameya/inject-purpose-keys
branch
from
acbe45a to
d363d4d
Compare
Contributor
|
Updates purpose key injection to use structural dependency injection instead of relying solely on global state. Phase 1
Both structs need custom Debug implementations that exclude the purpose_keys field or show only LGTM otherwise — the structural injection approach is good architecture and the Box::leak pattern for 'static lifetime is appropriate here. |
- Derive Default instead of manual impl for SeismicNode - Make purpose_keys() a const fn - Update CLAUDE.md CI Requirements to match actual Seismic CI clippy command
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
SeismicNodenow accepts purpose keys viaSeismicNode::new(keys), threading them through the node builder lifecycle (executor builder → EVM config) without relying on the globalOnceLockSeismicExecutorBuilderuses injected keys when available, falling back toget_purpose_keys()when constructed viaDefault(tests, CLI stage command)main.rsusesSeismicNode::new(purpose_keys)instead ofSeismicNode::default()init_purpose_keys()/get_purpose_keys()retained as deprecated fallback for code paths that can't yet receive keys structurallyAddresses #352 (comment)
Test plan
cargo check -p reth-seismic-node -p seismic-reth— compiles cleancargo nextest run -p reth-seismic-node -E 'kind(test)' --no-fail-fast— all integration tests passcargo +nightly fmt --all🤖 Generated with Claude Code