-
-
Notifications
You must be signed in to change notification settings - Fork 405
chore(testing): initial Kurtosis integration for exploratory migration #8296
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: feature/kurtosis-sim-tests
Are you sure you want to change the base?
chore(testing): initial Kurtosis integration for exploratory migration #8296
Conversation
**Motivation** Exploratory work for replacing Docker-based simulation infra with Kurtosis. Goal is to validate feasibility and document integration points with the current test framework **Description** - Add KurtosisSDKRunner to wrap enclave lifecycle and expose services (working) - Add runner-test.ts to run the KurtosisSDKRunner and print service info from Kurtosis (working) - Add simulation-kurtosis.ts and interfaces-kurtosis.ts as mock scaffolding → contain errors / unused params, not usable, shared only for feedback - Add kurtosisTypes.ts and helper loadKurtosisConfig.ts for typing and YAML parsing **Notes** Only KurtosisSDKRunner and runner-test.ts are functional Other files are placeholders for structure review and not production-ready
@@ -0,0 +1,497 @@ | |||
import {ChildProcess} from "node:child_process"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems this file contains a lot of duplicate interfaces from other interface.ts
file. Keep it consistent and add new interfaces in the interface.ts
.
@@ -0,0 +1,15 @@ | |||
participants: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We also need to update relevant multiFork.ts
file so instead of inline declerations of nodes, it loads up the Yaml file.
* | ||
* enclaveName: Default name for the Kurtosis enclave - can be overridden | ||
*/ | ||
constructor(enclaveName = "crucible-enclave") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make the name required, so the enclave name already be unique with the simulation running. And don't mixup the services among different simulations.
id: serviceName, | ||
serviceContext, | ||
beaconApiUrl: ports.get("http") ? `http://localhost:${ports.get("http")?.number}` : undefined, | ||
roles: this.inferRoles(serviceName), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can there be a service with multiple roles? I think not.
} | ||
|
||
const raw = await fs.readFile(fullPath, "utf8"); | ||
return parse(raw) as KurtosisNetworkConfig; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to validate the kurtosis config at this point? may be some helper function in their SDK.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The create()
function calls runStarlarkRemotePackageBlocking()
, which in turn invokes sanity_check()
and input_parser()
from the ethereum-package.
These already provide an initial validation of the Kurtosis config. Is this sufficient?
If sufficient but should occur earlier, one possible adjustment could be to move the validation (happening in simulation-kurtosis.ts) to run right after await env.runner.start(
sim-${id});
and before const services = await env.runner.create(kurtosisConfig);
, so that it happens before create()
rather than inside it
Nethermind = "execution-nethermind", | ||
} | ||
|
||
export enum ExecutionStartMode { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may delete this one now. Not needed ny more.
@@ -0,0 +1,258 @@ | |||
/** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this file from this PR and keep it locally for your testing.
- Update NodeService to use single role instead of multiple boolean flags - Change beaconApiUrl to generic apiUrl for better abstraction - Enclave name changed to be unique with the simulation running - Addressed kurtosis config validation - Added new interfaces in the interface.ts - Add TODO comment indicating future merge of simulation-kurtosis into simulation - Updated multiFork.test.ts wiith the new kurtosis configuration - Removed runner-test.ts from the PR - Improved multi-fork.yml parameters to be similar to the multifork.test.ts config
# Kurtosis test files (keep locally, don't commit) | ||
packages/cli/test/utils/crucible/kurtosis/test/runner-test.ts | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We would remove this line when this PR get ready.
|
||
//❌ REMOVE - Docker-specific | ||
async createNodePair<B extends BeaconClient, V extends ValidatorClient, E extends ExecutionClient>({ | ||
/*async createNodePair<B extends BeaconClient, V extends ValidatorClient, E extends ExecutionClient>({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can remove the code which is not needed from this file anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As previously suggested for interfaces-kurtosis.ts
to keep it consistent and append new code, should simulation-kurtosis.ts
be unified into simulation.ts
and just rely on interfaces.ts
+ simulation.ts
, removing simulation-kurtosis.ts
and interface.ts.
entirely?
- Update simulation-kurtosis.ts - Removed runner-test.ts from the PR - Removed Docker Runner elements - Aligned multi-fork.yml parameters to the multifork.test.ts config, as in unstable branch
Motivation
This PR explores an initial approach to migrating Lodestar's testing framework from Docker-based execution to Kurtosis-managed Ethereum network simulations. It proposes an abstraction layer that could preserve compatibility with existing test assertions while shifting the underlying infrastructure.
The core logic is maintaining the exact same test assertions while swapping the implementation from Docker containers to Kurtosis services.
This PR is opened for feedback and team discussion about the architectural approach before proceeding with the complete implementation.
This work is part of the Ethereum Protocol Fellowship - Cohort 6.
More details, including weekly updates on implementation and proposals, can be found in this documentation.
Description
Proposed Solution
Implement a Kurtosis SDK-based runner that provides:
Technical Implementation
1. Core Architecture (runner/)
2. Interface Layer (simulation/interfaces-kurtosis.ts)
3. Simulation Integration (simulation/simulation-kurtosis.ts)
4. Test Infrastructure (test/)
Data Flow Architecture
YAML Config → KurtosisNetworkConfig → KurtosisSDKRunner → KurtosisServicesMap → NodeService[] → NodePair[] → Test Assertions
Example of a possible workflow:
Future Work
If team feedback is positive on the architectural approach:
Note: The current approach only explains the architectural strategy. Actual NodePair replication from Kurtosis services is not yet implemented and requires further investigation to determine feasibility and approach
Feedback Request
This PR is opened specifically for team feedback and discussion on: