-
Notifications
You must be signed in to change notification settings - Fork 4
feat: restart repo with PureEngineClient #30
Conversation
WalkthroughThis pull request removes several CLI build and command targets from the Makefile and the evm-middleware command source, updates the genesis configuration and Docker Compose settings, and overhauls the execution client. The EngineAPIExecutionClient is renamed to PureEngineClient with updated method signatures and enhanced payload handling. Additionally, the README has been revised with new sections and a deployment diagram, and the test suite and module dependencies have been thoroughly updated. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant P as PureEngineClient
participant B as Blockchain Node
U->>P: InitChain (initialize chain)
P->>B: Request state root
B-->>P: Return state root
U->>P: GetTxs (retrieve transactions)
P->>B: ExecuteTxs (process transactions)
B-->>P: Return updated state
U->>P: SetFinal (finalize block)
P->>B: Finalize block processing
B-->>P: Finalization confirmation
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
e1b2679 to
57990ab
Compare
Upgraded `github.com/rollkit/go-execution` from `fccb99951758` to `dc52ad420afc` in both `go.mod` and `go.sum`. This ensures compatibility with the latest changes and improvements in the dependency. No other dependencies were affected.
For some reason first payload cannot be used
Cleanup in next commits
- Remove debug print statements and unused imports - Enhance block validation checks in test cases with more precise assertions - Simplify block height and transaction tracking logic - Remove commented-out code and unnecessary debug logging
- Add Ethereum client to PureEngineClient for block information retrieval - Implement `getBlockInfo` method to fetch block details by height - Update `InitChain` and `SetFinal` methods to use dynamic block information - Modify test cases to include `SetFinal` method calls - Update command-line interface to pass Ethereum URL when creating client
- Remove debug print statements and unused imports - Enhance block validation checks in test cases with more precise assertions - Simplify block height and transaction tracking logic - Remove commented-out code and unnecessary debug logging
- Merge `engine_test.go` and `execution_test.go` into a single test file - Remove redundant test code and consolidate test logic - Update function and method names for consistency - Simplify transaction and block processing in test cases - Remove unnecessary comments and debug logging
- Consolidate and refactor execution logic in `execution.go` and `execution_test.go` - Remove deprecated integration tests and related files - Update transaction handling to use byte arrays instead of custom types - Improve error handling and logging in the execution client methods - Adjust Go module dependencies and update to Go 1.24.0 - Remove unnecessary build commands from the Makefile - Enhance test setup for better isolation and cleanup
- Refactor `execution_test.go` to improve test setup and cleanup processes - Change JWT filename from `testsecret.hex` to `jwt.hex` for clarity - Update permissions for JWT directory and file to be more permissive - Replace container setup with Docker Compose for better management - Update Docker image version in `docker-compose.yml` from `v1.1.1` to `v1.2.1` - Remove unnecessary volume bindings in Docker configuration
- Revise architecture section to introduce the `PureEngineClient` and its compatibility with the Engine API - Detail how the Eth API is utilized for block information retrieval and transaction execution - Explain the payload ID management and its significance in transaction processing - Add new sections on development, testing, and reading genesis information with updated commands
6988b9a to
c6fb37e
Compare
…neClient - Add a new section on genesis requirements for the `PureEngineClient`, outlining necessary hardfork settings - Include an example of the required genesis configuration in JSON format - Reorganize the section on how the Eth API is used, ensuring clarity on its role in transaction execution and block information retrieval
43b9eb1 to
ab9ef42
Compare
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.
Actionable comments posted: 2
🧹 Nitpick comments (6)
go.mod (1)
12-217: Reassess large dependency footprint.A wide range of AWS, Docker, containerd, Kubernetes, and other libraries have been added as indirect dependencies. Confirm if all of these are genuinely required. Unnecessary dependencies can enlarge the module size and increase attack surface. Consider pruning, splitting into multiple modules or leveraging build tags to reduce the final binary size.
execution.go (1)
112-112: Resolve or clarify placeholder for random.
Random: common.Hash{}is marked with a TODO comment. Before production use, ensure an appropriate RANDAO or a valid random approach is provided.docker/docker-compose.yml (1)
50-50: Revisit reduced verbosity level.Decreasing the logging level from
-vvvvvto-vvvvmight reduce essential debug data when troubleshooting. Ensure logs remain sufficient for diagnosing issues under typical workloads.README.md (2)
58-69: Validate the 'Payload as First Transaction' approach performance.While the approach of encoding the full payload as the first transaction is innovative, it could cause performance overhead for large payloads. Consider verifying the performance impact under stress or high transaction loads to ensure your design scales well.
🧰 Tools
🪛 LanguageTool
[style] ~67-~67: This phrase is redundant. Consider using “outside”.
Context: ...- It's not possible to create a payload outside of the EVM - Transactions cannot be select...(OUTSIDE_OF)
[style] ~68-~68: This phrase is redundant. Consider using “outside”.
Context: ...nsactions cannot be selected or ordered outside of the EVM ### How Eth API is Used The `...(OUTSIDE_OF)
67-68: Avoid redundant phrasing "outside of the EVM".According to a style hint, “outside of the EVM” can be simplified to “outside the EVM”. This helps ensure concise, clear documentation.
🧰 Tools
🪛 LanguageTool
[style] ~67-~67: This phrase is redundant. Consider using “outside”.
Context: ...- It's not possible to create a payload outside of the EVM - Transactions cannot be select...(OUTSIDE_OF)
[style] ~68-~68: This phrase is redundant. Consider using “outside”.
Context: ...nsactions cannot be selected or ordered outside of the EVM ### How Eth API is Used The `...(OUTSIDE_OF)
execution_test.go (1)
104-104: Replace time.Sleep with a poll-based approach.Using a fixed sleep for block production might be flaky under variable environments. Consider polling for transaction inclusion or a more deterministic synchronization signal to improve test reliability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (11)
Makefile(0 hunks)README.md(1 hunks)cmd/evm-middleware/commands/root.go(0 hunks)cmd/evm-middleware/commands/run.go(0 hunks)cmd/evm-middleware/main.go(0 hunks)docker/chain/genesis.json(2 hunks)docker/docker-compose.yml(3 hunks)execution.go(4 hunks)execution_test.go(1 hunks)go.mod(1 hunks)integration_test.go(0 hunks)
💤 Files with no reviewable changes (5)
- cmd/evm-middleware/main.go
- cmd/evm-middleware/commands/root.go
- integration_test.go
- Makefile
- cmd/evm-middleware/commands/run.go
🧰 Additional context used
🪛 LanguageTool
README.md
[style] ~67-~67: This phrase is redundant. Consider using “outside”.
Context: ...- It's not possible to create a payload outside of the EVM - Transactions cannot be select...
(OUTSIDE_OF)
[style] ~68-~68: This phrase is redundant. Consider using “outside”.
Context: ...nsactions cannot be selected or ordered outside of the EVM ### How Eth API is Used The `...
(OUTSIDE_OF)
🔇 Additional comments (8)
go.mod (2)
3-5: Validate Go version and toolchain upgrade.Bumping to Go 1.24.0 and toolchain go1.24.1 is a welcome update. Confirm that the continuous integration environment supports these versions and no issues arise from the upgrade in downstream pipelines.
7-7: Confirm Ethereum and Rollkit core versions.The requirement on
github.com/ethereum/go-ethereum v1.15.0andgithub.com/rollkit/rollkit/core v0.0.0-20250312114929-104787ba1a4cboth drive critical functionalities. Ensure that no known breaking changes or compatibility issues exist between these versions.Also applies to: 222-225
execution.go (1)
31-33: Implementation interface compatibility confirmed.Ensuring
PureEngineClientimplementsexecution.Executoris a good practice and clarifies the interface compliance.docker/docker-compose.yml (2)
24-24: Verify new Reth image version.Upgrading to
ghcr.io/paradigmxyz/reth:v1.2.1can bring performance and security fixes. Check release notes to ensure no new configuration changes are required.
35-35: Safer volume usage for JWT tokens.Mounting JWT tokens in read-only mode increases security. This is a good improvement.
docker/chain/genesis.json (1)
14-18: Verify post-merge configuration.Setting
mergeNetsplitBlockto 0,terminalTotalDifficultyto 0, andterminalTotalDifficultyPassedto true forces the chain into post-merge mode from genesis. Ensure this is fully intended and that all dependent services align with a zero TTD approach. Also confirm that the chain ID (1234) won't conflict with other networks.Also applies to: 20-26, 35-38
execution_test.go (2)
38-60: Great integrated test implementation.The two-phase build and sync flow is comprehensive, validating the entire life cycle of block production and replay.
378-406: Check concurrency safety of global nonce.
lastNonceis a global variable incremented ingetRandomTransaction. If these tests are ever run in parallel, it could lead to race conditions. Consider encapsulating nonce management within your test or adopting concurrency-safe patterns.
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.
Actionable comments posted: 0
♻️ Duplicate comments (1)
execution.go (1)
253-255: 🛠️ Refactor suggestionImprove RANDAO derivation logic.
The current implementation uses block height as a source of randomness, which isn't truly random. While adequate for testing, this wouldn't be suitable for production environments where proper randomness is crucial for security.
I see this issue has already been addressed in a follow-up issue (#34). Please ensure this is improved before any production usage.
🧹 Nitpick comments (8)
execution_test.go (5)
30-32: Consider using environment variables for test private key.While this is a test file, hardcoding a private key directly in the source code could be a security risk if the key has any value, even in test environments. Consider using environment variables or a dedicated test configuration file loaded at runtime.
- TEST_PRIVATE_KEY = "cece4f25ac74deb1468965160c7185e07dff413f23fcadb611b05ca37ab0a52e" + TEST_PRIVATE_KEY = os.Getenv("TEST_PRIVATE_KEY")If you prefer to keep a default value for local development:
- TEST_PRIVATE_KEY = "cece4f25ac74deb1468965160c7185e07dff413f23fcadb611b05ca37ab0a52e" + TEST_PRIVATE_KEY = getTestPrivateKey() + + func getTestPrivateKey() string { + if key := os.Getenv("TEST_PRIVATE_KEY"); key != "" { + return key + } + return "cece4f25ac74deb1468965160c7185e07dff413f23fcadb611b05ca37ab0a52e" + }🧰 Tools
🪛 Gitleaks (8.21.2)
31-31: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
90-96: Ensure enough time is allowed for block generation.The current test relies on fixed wait times (e.g., 1000ms at line 104), which could potentially lead to test flakiness. While this works for now, consider implementing a more robust polling mechanism for production-ready tests.
- time.Sleep(1000 * time.Millisecond) + // Poll with timeout to wait for transaction confirmation + deadline := time.Now().Add(5 * time.Second) + for time.Now().Before(deadline) { + // Check if transactions are included in pending pool + pendingCount, err := checkPendingTransactionCount(t, ctx) + require.NoError(t, err) + if pendingCount >= nTxs { + break + } + time.Sleep(100 * time.Millisecond) + }You would need to implement a
checkPendingTransactionCountfunction to check the transaction pool.
215-235: Consider enriching error messages in checkLatestBlock.The warning logs at lines 217 and 227 could be more informative by including more context about what specific operation failed. Additionally, consider returning a custom error instead of just logging warnings.
- t.Logf("Warning: Failed to get latest block header: %v", err) + t.Logf("Warning: Failed to get latest block header from %s: %v", TEST_ETH_URL, err)- t.Logf("Warning: Failed to get full block: %v", err) + t.Logf("Warning: Failed to get full block %d from %s: %v", blockNumber, TEST_ETH_URL, err)
267-276: Consider releasing resources in error case.In the cleanup handler, resources might not be fully released if the Docker Compose environment fails to tear down. Consider adding additional cleanup steps for such scenarios.
t.Cleanup(func() { ctx := context.Background() err := compose.Down(ctx, tc.RemoveOrphans(true), tc.RemoveVolumes(true)) if err != nil { t.Logf("Warning: Failed to tear down docker-compose environment: %v", err) + // Try force removal of containers as fallback + t.Logf("Attempting force removal of containers...") + forceRemoveCtx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + _ = compose.Down(forceRemoveCtx, tc.RemoveOrphans(true), tc.RemoveVolumes(true), tc.RemoveImagesLocal) } })
379-406: Enhance transaction generation for better test coverage.The current implementation always uses a monotonically increasing nonce, which is good for basic testing. However, consider adding more variations to test different transaction scenarios.
You could extend
getRandomTransactionto support different types of transactions (EIP-1559, legacy, etc.) and handle different edge cases:+var lastNonce uint64 + +// TransactionType defines the type of transaction to generate +type TransactionType int + +const ( + LegacyTx TransactionType = iota + EIP1559Tx + InvalidTx // For testing error handling +) -func getRandomTransaction(t *testing.T, gasLimit uint64) *ethTypes.Transaction { +func getRandomTransaction(t *testing.T, gasLimit uint64, txType ...TransactionType) *ethTypes.Transaction { + txKind := LegacyTx + if len(txType) > 0 { + txKind = txType[0] + } + privateKey, err := crypto.HexToECDSA(TEST_PRIVATE_KEY) require.NoError(t, err) chainId, _ := new(big.Int).SetString(CHAIN_ID, 10) txValue := big.NewInt(1000000000000000000) gasPrice := big.NewInt(30000000000) toAddress := common.HexToAddress(TEST_TO_ADDRESS) data := make([]byte, 16) _, err = rand.Read(data) require.NoError(t, err) + var tx *ethTypes.Transaction + + switch txKind { + case LegacyTx: + tx = ethTypes.NewTx(ðTypes.LegacyTx{ + Nonce: lastNonce, + To: &toAddress, + Value: txValue, + Gas: gasLimit, + GasPrice: gasPrice, + Data: data, + }) + case EIP1559Tx: + tx = ethTypes.NewTx(ðTypes.DynamicFeeTx{ + ChainID: chainId, + Nonce: lastNonce, + To: &toAddress, + Value: txValue, + Gas: gasLimit, + GasFeeCap: gasPrice, + GasTipCap: new(big.Int).Div(gasPrice, big.NewInt(2)), + Data: data, + }) + case InvalidTx: + // Create transaction with invalid gas limit for testing error handling + tx = ethTypes.NewTx(ðTypes.LegacyTx{ + Nonce: lastNonce, + To: &toAddress, + Value: txValue, + Gas: 0, // Invalid gas limit + GasPrice: gasPrice, + Data: data, + }) + } - tx := ethTypes.NewTx(ðTypes.LegacyTx{ - Nonce: lastNonce, - To: &toAddress, - Value: txValue, - Gas: gasLimit, - GasPrice: gasPrice, - Data: data, - }) lastNonce++ signedTx, err := ethTypes.SignTx(tx, ethTypes.NewEIP155Signer(chainId), privateKey) require.NoError(t, err) return signedTx }execution.go (3)
112-113: Address the TODO comment about random value.The comment indicates this value shouldn't be zero, but there's no clear action planned. This should be addressed, especially if it affects consensus.
- Random: common.Hash{}, // TODO(tzdybal): this probably shouldn't be 0 + Random: common.BigToHash(big.NewInt(0)), // Will be improved in a separate PRWould you like me to help create an enhancement issue for this? I could outline the requirements for implementing proper random value generation for the initialization phase.
192-194: Improve error handling with more context.The error message when a payload submission fails could be enhanced with additional context about the payload.
- return nil, 0, fmt.Errorf("new payload submission failed with: %s", *newPayloadResult.ValidationError) + return nil, 0, fmt.Errorf("new payload submission failed for block %d with: %s", + payloadResult.ExecutionPayload.BlockNumber, + *newPayloadResult.ValidationError)
203-204: Consider including comment on finalization strategy.The commented out
FinalizedBlockHash: blockHashline suggests a deliberate design decision about when blocks are finalized. Consider adding a comment explaining this strategy to aid future maintenance.- // FinalizedBlockHash: blockHash, + // FinalizedBlockHash is not set here because finalization happens explicitly through SetFinal
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
execution.go(4 hunks)execution_test.go(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
execution.go (1)
Learnt from: tzdybal
PR: rollkit/go-execution-evm#30
File: execution.go:253-255
Timestamp: 2025-03-15T22:20:39.828Z
Learning: For the PureEngineClient in the rollkit/go-execution-evm repository, the derivePrevRandao function currently returns block height as a hash, which should be improved with a proper random generation mechanism for production use.
🪛 Gitleaks (8.21.2)
execution_test.go
31-31: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test / Run Unit Tests
🔇 Additional comments (4)
execution_test.go (1)
38-60: Excellent test documentation.The detailed comments explaining the test phases and expected behaviors make the test much more maintainable. This comprehensive documentation clearly outlines the "Build Chain" and "Sync Chain" phases and their specific validations.
execution.go (3)
32-43: Excellent interface implementation verification.Good practice to use a compile-time check to verify interface implementation:
var _ execution.Executor = (*PureEngineClient)(nil)The documentation for the PureEngineClient struct is also very helpful for understanding its purpose and functionality.
136-163: Clever transaction serialization approach.Using the first transaction to store the serialized payload is an elegant solution to maintain context between API calls. This ensures that all transaction execution has the necessary context without relying on additional state tracking.
267-287: JWT token generation is well implemented.The JWT token generation includes proper expiration time and follows security best practices. Good work on this implementation.
MSevey
left a comment
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.
Nice 👏🏻
When GetTxs is called, but payloadID is unknown, new payload is generated. This happens if GetTxs was called without previous calls to InitChain/ExecuteTxs - this happens when rollkit is restarted.
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.
Actionable comments posted: 5
♻️ Duplicate comments (1)
execution.go (1)
219-220:⚠️ Potential issueValidate transaction length before assuming first TX is payload.
There's no validation that the transactions slice has at least one element before accessing it, which could lead to a panic.
// First tx is the serialized payload +if len(txs) == 0 { + return nil, 0, fmt.Errorf("no transactions provided, expected at least one containing the payload") +} firstTx := txs[0]This validation is particularly important since MSevey previously commented on line 219-220 about validating the length of txs to avoid panics.
🧹 Nitpick comments (8)
execution.go (2)
147-182: Consider extracting payloadID recovery logic to a helper method.The handling of nil payloadID is complex and could benefit from being extracted to a separate helper method for better readability and maintainability.
func (c *PureEngineClient) GetTxs(ctx context.Context) ([][]byte, error) { - if c.payloadID == nil { // this happens when rollkit is restarted - latestHeight, err := c.ethClient.BlockNumber(ctx) - if err != nil { - return nil, fmt.Errorf("failed to get latest block height: %w", err) - } - block, err := c.ethClient.BlockByNumber(ctx, new(big.Int).SetUint64(latestHeight)) - if err != nil { - return nil, fmt.Errorf("failed to get latest block: %w", err) - } - blockHash := block.Hash() - timestamp := block.Time() + 1 - var forkchoiceResult engine.ForkChoiceResponse - err = c.engineClient.CallContext(ctx, &forkchoiceResult, "engine_forkchoiceUpdatedV3", - engine.ForkchoiceStateV1{ - HeadBlockHash: blockHash, - SafeBlockHash: blockHash, - // FinalizedBlockHash: blockHash, - }, - &engine.PayloadAttributes{ - Timestamp: timestamp, - Random: c.derivePrevRandao(latestHeight + 1), - SuggestedFeeRecipient: c.feeRecipient, - BeaconRoot: &c.genesisHash, - Withdrawals: []*types.Withdrawal{}, - }, - ) - if err != nil { - return nil, fmt.Errorf("forkchoice update failed with error: %w", err) - } - - if forkchoiceResult.PayloadStatus.Status != engine.VALID { - return nil, ErrInvalidPayloadStatus - } - - c.payloadID = forkchoiceResult.PayloadID - } + if c.payloadID == nil { // this happens when rollkit is restarted + if err := c.recoverPayloadID(ctx); err != nil { + return nil, err + } + } var payloadResult engine.ExecutionPayloadEnvelope err := c.engineClient.CallContext(ctx, &payloadResult, "engine_getPayloadV3", c.payloadID)And add this helper method:
// recoverPayloadID handles the case when payloadID is nil (e.g., after restart) // by finding the latest block and initiating a new payload building process func (c *PureEngineClient) recoverPayloadID(ctx context.Context) error { latestHeight, err := c.ethClient.BlockNumber(ctx) if err != nil { return fmt.Errorf("failed to get latest block height: %w", err) } block, err := c.ethClient.BlockByNumber(ctx, new(big.Int).SetUint64(latestHeight)) if err != nil { return fmt.Errorf("failed to get latest block: %w", err) } blockHash := block.Hash() timestamp := block.Time() + 1 var forkchoiceResult engine.ForkChoiceResponse err = c.engineClient.CallContext(ctx, &forkchoiceResult, "engine_forkchoiceUpdatedV3", engine.ForkchoiceStateV1{ HeadBlockHash: blockHash, SafeBlockHash: blockHash, // FinalizedBlockHash: blockHash, }, &engine.PayloadAttributes{ Timestamp: timestamp, Random: c.derivePrevRandao(latestHeight + 1), SuggestedFeeRecipient: c.feeRecipient, BeaconRoot: &c.genesisHash, Withdrawals: []*types.Withdrawal{}, }, ) if err != nil { return fmt.Errorf("forkchoice update failed with error: %w", err) } if forkchoiceResult.PayloadStatus.Status != engine.VALID { return ErrInvalidPayloadStatus } c.payloadID = forkchoiceResult.PayloadID return nil }
163-163: Add comment explaining why FinalizedBlockHash is commented out.The
FinalizedBlockHashfield is commented out in multiple places without explanation. This makes the code intent unclear.- // FinalizedBlockHash: blockHash, + // FinalizedBlockHash deliberately not set here as this is an intermediate state + // and will be properly finalized in SetFinal methodAlso applies to: 254-254
README.md (2)
12-12: Add missing comma and article.The sentence would be more grammatically correct with a comma after
InitChainand the article "an" before "empty block".-During `InitChain` EVM genesis block is acknowledged (at height 0), and empty block is created (at height 1). +During `InitChain`, EVM genesis block is acknowledged (at height 0), and an empty block is created (at height 1).🧰 Tools
🪛 LanguageTool
[uncategorized] ~12-~12: Possible missing comma found.
Context: ...itial height is1. DuringInitChainEVM genesis block is acknowledged (at heigh...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~12-~12: You might be missing the article “an” here.
Context: ...lock is acknowledged (at height 0), and empty block is created (at height 1). This ap...(AI_EN_LECTOR_MISSING_DETERMINER_AN)
73-74: Simplify redundant phrases.The phrase "outside of" is redundant and can be simplified to just "outside" in these two instances.
-- It's not possible to create a payload outside of the EVM -- Transactions cannot be selected or ordered outside of the EVM +- It's not possible to create a payload outside the EVM +- Transactions cannot be selected or ordered outside the EVM🧰 Tools
🪛 LanguageTool
[style] ~73-~73: This phrase is redundant. Consider using “outside”.
Context: ...- It's not possible to create a payload outside of the EVM - Transactions cannot be select...(OUTSIDE_OF)
[style] ~74-~74: This phrase is redundant. Consider using “outside”.
Context: ...nsactions cannot be selected or ordered outside of the EVM ### How Eth API is Used The `...(OUTSIDE_OF)
execution_test.go (4)
94-96: Document why block height 4 is a special case.Block height 4 is set to have 0 transactions without explanation. This should be documented to explain the test case.
-// randomly use no transactions -if blockHeight == 4 { +// Test edge case: Block with zero transactions to ensure the client handles empty blocks correctly +if blockHeight == 4 {
104-104: Replace hard-coded sleep with a more robust synchronization mechanism.Using a fixed sleep time can make tests flaky and slow. Consider implementing a polling mechanism or other synchronization approach.
-time.Sleep(1000 * time.Millisecond) +// Poll until transactions are visible or timeout occurs +startTime := time.Now() +for { + pendingCount, err := ethClient.PendingTransactionCount(ctx) + require.NoError(tt, err) + if pendingCount >= uint(nTxs) || time.Since(startTime) > 5*time.Second { + break + } + time.Sleep(100 * time.Millisecond) +}
219-219: Remove or uncomment commented-out log statement.This commented-out log statement should either be removed if not needed or uncommented if it provides useful debugging information.
-//t.Logf("Latest block: height=%d, hash=%s, txs=%d", blockNumber, blockHash.Hex(), txCount) +// Uncomment for debugging: +// t.Logf("Latest block: height=%d, hash=%s, txs=%d", blockNumber, blockHash.Hex(), txCount)
378-378: Make lastNonce test-instance specific to avoid parallel test issues.Using a global variable for lastNonce could cause issues if tests are run in parallel. Consider making it instance-specific.
-var lastNonce uint64 +// getRandomTransaction generates a randomized valid ETH transaction +func getRandomTransaction(t *testing.T, gasLimit uint64) *ethTypes.Transaction { + // Use static variable within the function to track nonce per test + static var lastNonce uint64Or use a mutex-protected global if multiple test functions need to access it:
var ( lastNonceMu sync.Mutex lastNonce uint64 ) func getRandomTransaction(t *testing.T, gasLimit uint64) *ethTypes.Transaction { lastNonceMu.Lock() nonce := lastNonce lastNonce++ lastNonceMu.Unlock() // Use nonce in transaction // ... }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
README.md(1 hunks)execution.go(4 hunks)execution_test.go(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
execution.go (1)
Learnt from: tzdybal
PR: rollkit/go-execution-evm#30
File: execution.go:253-255
Timestamp: 2025-03-19T22:35:44.047Z
Learning: For the PureEngineClient in the rollkit/go-execution-evm repository, the derivePrevRandao function currently returns block height as a hash, which should be improved with a proper random generation mechanism for production use.
🪛 Gitleaks (8.21.2)
execution_test.go
31-31: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 GitHub Actions: CI and Release
execution_test.go
[error] 164-164: TestEngineExecution/Sync_chain failed due to panic: runtime error: index out of range [9] with length 9.
README.md
[error] 9-9: markdownlint: Headings should be surrounded by blank lines [Expected: 1; Actual: 0; Below] [Context: '### Genesis and initial height']
🪛 LanguageTool
README.md
[uncategorized] ~7-~7: You might be missing the article “the” here.
Context: ... Reth) and uses both the Engine API and standard Ethereum JSON-RPC API to execute transa...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~12-~12: Possible missing comma found.
Context: ...itial height is 1. During InitChain EVM genesis block is acknowledged (at heigh...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~12-~12: You might be missing the article “an” here.
Context: ...lock is acknowledged (at height 0), and empty block is created (at height 1). This ap...
(AI_EN_LECTOR_MISSING_DETERMINER_AN)
[style] ~73-~73: This phrase is redundant. Consider using “outside”.
Context: ...- It's not possible to create a payload outside of the EVM - Transactions cannot be select...
(OUTSIDE_OF)
[style] ~74-~74: This phrase is redundant. Consider using “outside”.
Context: ...nsactions cannot be selected or ordered outside of the EVM ### How Eth API is Used The `...
(OUTSIDE_OF)
🪛 markdownlint-cli2 (0.17.2)
README.md
9-9: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🔇 Additional comments (1)
execution.go (1)
304-306: RANDAO derivation function is not suitable for production.The
derivePrevRandaoimplementation is simplistic, returning only the block height as a hash, which doesn't provide proper randomness.I see this issue has already been identified in a previous review and an issue (#34) has been created to track this improvement. This implementation is sufficient for testing but should be replaced with a more robust solution for production use.
This test allows for manual submission of transactions to an Ethereum client, primarily for testing purposes. It includes nonce calculation and transaction broadcasting to assist with debugging and validation workflows. The test is skipped by default to avoid interference with automated test runs.
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
execution_test.go (1)
402-428: Transaction generation could be more flexible.The
getRandomTransactionfunction effectively creates transactions for testing, but it would be more robust to handle different test scenarios if it:
- Allowed customizing more parameters (like value and gas price)
- Generated transactions with contract deployments (no recipient address)
- Created transactions with more complex data payloads
-func getRandomTransaction(t *testing.T, gasLimit uint64) *ethTypes.Transaction { +func getRandomTransaction(t *testing.T, gasLimit uint64, opts ...TxOption) *ethTypes.Transaction { privateKey, err := crypto.HexToECDSA(TEST_PRIVATE_KEY) require.NoError(t, err) chainId, _ := new(big.Int).SetString(CHAIN_ID, 10) txValue := big.NewInt(1000000000000000000) gasPrice := big.NewInt(30000000000) toAddress := common.HexToAddress(TEST_TO_ADDRESS) data := make([]byte, 16) _, err = rand.Read(data) require.NoError(t, err) + + // Apply any custom options + txOpts := defaultTxOptions() + for _, opt := range opts { + opt(&txOpts) + } + + // Use custom values if provided + if txOpts.value != nil { + txValue = txOpts.value + } + if txOpts.gasPrice != nil { + gasPrice = txOpts.gasPrice + } + if txOpts.data != nil { + data = txOpts.data + } + var to *common.Address + if !txOpts.isContractCreation { + to = &toAddress + if txOpts.to != nil { + to = txOpts.to + } + }You would also need to add option types at the top of the file:
// TxOption is a function that configures a transaction type TxOption func(*txOptions) type txOptions struct { value *big.Int gasPrice *big.Int data []byte to *common.Address isContractCreation bool } func defaultTxOptions() txOptions { return txOptions{ isContractCreation: false, } } // WithValue sets the transaction value func WithValue(value *big.Int) TxOption { return func(opts *txOptions) { opts.value = value } } // AsContractCreation creates a contract deployment transaction func AsContractCreation() TxOption { return func(opts *txOptions) { opts.isContractCreation = true } } // Other option functions as needed...
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
execution_test.go(1 hunks)
🧰 Additional context used
🪛 Gitleaks (8.21.2)
execution_test.go
31-31: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 GitHub Actions: CI and Release
execution_test.go
[error] 164-164: TestEngineExecution/Sync_chain failed due to panic: runtime error: index out of range [9] with length 9.
🔇 Additional comments (6)
execution_test.go (6)
31-31: Replace hardcoded private key with environment variable or test-only generation.The hardcoded private key is flagged as a potential security issue by static analysis. While this is a test file, it's a good practice to avoid hardcoded credentials.
- TEST_PRIVATE_KEY = "cece4f25ac74deb1468965160c7185e07dff413f23fcadb611b05ca37ab0a52e" + // For tests only, generate a new key or use an environment variable + TEST_PRIVATE_KEY = os.Getenv("TEST_PRIVATE_KEY_FOR_TESTS")Add a fallback in the
getRandomTransactionfunction:func getRandomTransaction(t *testing.T, gasLimit uint64) *ethTypes.Transaction { + privateKeyHex := TEST_PRIVATE_KEY + if privateKeyHex == "" { + // Generate a test-only key if not provided + key, err := crypto.GenerateKey() + require.NoError(t, err) + privateKeyBytes := crypto.FromECDSA(key) + privateKeyHex = hex.EncodeToString(privateKeyBytes) + } - privateKey, err := crypto.HexToECDSA(TEST_PRIVATE_KEY) + privateKey, err := crypto.HexToECDSA(privateKeyHex) require.NoError(t, err)🧰 Tools
🪛 Gitleaks (8.21.2)
31-31: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
164-164: Fix index out of range error in Sync chain test.The CI pipeline is failing with an index out of range error at this line. The issue is in calculating the index into allPayloads.
-payload := allPayloads[blockHeight-initialHeight-1] +// Use a zero-based index to avoid out of range errors +payloadIndex := blockHeight - initialHeight - 1 +if payloadIndex < 0 || int(payloadIndex) >= len(allPayloads) { + tt.Fatalf("Invalid payload index %d for allPayloads length %d", payloadIndex, len(allPayloads)) +} +payload := allPayloads[payloadIndex]🧰 Tools
🪛 GitHub Actions: CI and Release
[error] 164-164: TestEngineExecution/Sync_chain failed due to panic: runtime error: index out of range [9] with length 9.
60-193: Test case covers both build and sync scenarios effectively.The test is well-structured with two distinct phases that verify:
- Building a chain with transactions and empty blocks
- Syncing a new engine instance with the same payloads
This approach thoroughly validates the core functionality of the execution engine.
🧰 Tools
🪛 GitHub Actions: CI and Release
[error] 164-164: TestEngineExecution/Sync_chain failed due to panic: runtime error: index out of range [9] with length 9.
195-236: Helper functions provide good separation of concerns.The
createEthClientandcheckLatestBlockfunctions are well-designed to handle error cases gracefully and provide useful diagnostic information during test failures.
249-309: Robust test environment setup with proper cleanup.The
setupTestRethEnginefunction:
- Generates a secure JWT token
- Sets up Docker Compose environments with unique test identifiers
- Includes proper cleanup handlers to ensure resources are released
- Waits for services to be ready before proceeding
This makes the tests more reliable and prevents resource leaks.
311-368: Effective container readiness checking.The
waitForRethContainerfunction properly:
- Implements a timeout to prevent tests from hanging indefinitely
- Checks both the RPC and Engine endpoints
- Handles authentication requirements
- Uses a reasonable polling interval
This ensures tests only proceed when the environment is fully ready.
b1b90a6 to
6987498
Compare
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
execution.go (1)
147-182: Ensure transaction handling during restarts is robustThe restart handling looks well-implemented, but consider adding a check to verify that the execution client has been properly initialized before proceeding with operations.
func (c *PureEngineClient) GetTxs(ctx context.Context) ([][]byte, error) { + if c.engineClient == nil || c.ethClient == nil { + return nil, errors.New("execution client not properly initialized") + } if c.payloadID == nil { // this happens when rollkit is restarted
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md(1 hunks)execution.go(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
execution.go (1)
Learnt from: tzdybal
PR: rollkit/go-execution-evm#30
File: execution.go:253-255
Timestamp: 2025-03-20T02:29:01.182Z
Learning: For the PureEngineClient in the rollkit/go-execution-evm repository, the derivePrevRandao function currently returns block height as a hash, which should be improved with a proper random generation mechanism for production use.
🪛 LanguageTool
README.md
[uncategorized] ~7-~7: You might be missing the article “the” here.
Context: ... Reth) and uses both the Engine API and standard Ethereum JSON-RPC API to execute transa...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~13-~13: You might be missing the article “an” here.
Context: ...lock is acknowledged (at height 0), and empty block is created (at height 1). This ap...
(AI_EN_LECTOR_MISSING_DETERMINER_AN)
[style] ~74-~74: This phrase is redundant. Consider using “outside”.
Context: ...- It's not possible to create a payload outside of the EVM - Transactions cannot be select...
(OUTSIDE_OF)
[style] ~75-~75: This phrase is redundant. Consider using “outside”.
Context: ...nsactions cannot be selected or ordered outside of the EVM ### How Eth API is Used The `...
(OUTSIDE_OF)
🔇 Additional comments (20)
execution.go (13)
3-22: Review import statements for updated dependenciesThe import section has been updated with new dependencies:
- Added
encoding/jsonfor serialization- Updated JWT library to v5
- Updated rollkit import path to direct repository reference
Ensure these dependencies are consistently used throughout the code.
31-43: LGTM on the PureEngineClient structure definitionThe client structure has been properly renamed and documented with clear comments. The addition of the
payloadIDfield is a good improvement for maintaining state between API calls.
45-85: Client constructor looks goodThe renamed constructor follows the new naming convention and properly initializes all required fields.
87-143: InitChain implementation follows correct patternThe method properly handles initial block creation and payload management. It enforces that initialHeight must be 1, which aligns with the documentation in the README.
116-116: Address the TODO comment about Random being zeroThe Random field is set to empty hash with a TODO comment indicating it shouldn't be zero. This could affect the randomness of block production.
183-188: Move payloadID clearing after error checkingSetting
c.payloadIDto nil before error checking could lead to an inconsistent state if the call fails.
190-208: Approve the serialization approach for payload transactionsThe approach of serializing the payload as the first transaction and keeping original transactions is elegant and maintains compatibility.
219-220: Validate the length of txs to avoid panicsMissing validation for empty transaction arrays could lead to index out of bounds errors when accessing
txs[0].
232-245: Good error handling for payload submissionThe error checking for payload submission is thorough, with proper validation of the payload status and detailed error messages.
248-274: Proper forkchoice update with improved error handlingThe forkchoice update properly sets the block hash and includes error handling. The method returns the correct state root and gas limit.
303-305: Improve RANDAO derivation logic
derivePrevRandaoreturns the block height as a hash. While feasible for test networks, real consensus protocols often require actual random values. Consider substituting a valid VRF or random approach if used in production.
307-315: Utility method for block info retrieval is well structuredThe
getBlockInfomethod properly encapsulates the logic for retrieving block information, making the code more maintainable.
214-217:Details
❓ Verification inconclusive
Consider validating block height more robustly
For the special handling of block 1, consider adding additional validation to ensure the block is properly retrieved and processed.
🏁 Script executed:
#!/bin/bash # Check if block 1 is correctly initialized in various scenarios # First, check if block 1 is available through the Eth API echo "Checking if block 1 is available through Eth API..." curl -X POST -H "Content-Type: application/json" --data '{"jsonrpc":"2.0","method":"eth_getBlockByNumber","params":["0x1", false],"id":1}' http://localhost:8545Length of output: 682
Action Required: Enhance Block 1 Validation and Verify Connectivity
The current handling of block 1 in
execution.go(lines 214–217) only checks ifblockHeight == 1with an empty transaction list and then callsc.getBlockInfo. However, our verification script noted that the Eth API connection failed on localhost:8545, which raises concerns about potential issues in block retrieval under certain conditions.
- API Connectivity Check: Ensure that the Eth API endpoint is reachable before attempting to retrieve block 1. Consider adding connection checks or retries.
- Enhanced Validation: In addition to checking the block height and transaction count, validate that the block data (e.g.,
stateRootandgasLimit) returned byc.getBlockInfois correct and complete.- Error Handling: Provide clear error messages or fallback logic when the API call fails, so that the function does not silently propagate incomplete data.
Please manually verify these changes in an environment with a running Eth API to ensure robust block 1 processing.
README.md (7)
1-15: Clear introduction to the PureEngineClient implementationThe introduction and implementation sections provide a good overview of the client's purpose and how it interacts with the Ethereum execution client.
Consider these minor grammar improvements:
- The `PureEngineClient` is a 100% Engine API compatible implementation of the `execution.Executor` interface. It connects to an Ethereum execution client (like Reth) and uses both the Engine API and standard Ethereum JSON-RPC API to execute transactions. + The `PureEngineClient` is a 100% Engine API compatible implementation of the `execution.Executor` interface. It connects to an Ethereum execution client (like Reth) and uses both the Engine API and the standard Ethereum JSON-RPC API to execute transactions. - During `InitChain` EVM genesis block is acknowledged (at height 0), and empty block is created (at height 1). + During `InitChain` the EVM genesis block is acknowledged (at height 0), and an empty block is created (at height 1).🧰 Tools
🪛 LanguageTool
[uncategorized] ~7-~7: You might be missing the article “the” here.
Context: ... Reth) and uses both the Engine API and standard Ethereum JSON-RPC API to execute transa...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~13-~13: You might be missing the article “an” here.
Context: ...lock is acknowledged (at height 0), and empty block is created (at height 1). This ap...(AI_EN_LECTOR_MISSING_DETERMINER_AN)
16-51: Comprehensive genesis requirements documentationThe genesis requirements section is well-documented with clear examples and explanations.
52-60: Clear explanation of PayloadID storageThe PayloadID storage section clearly explains how the client maintains payload ID consistency between calls, which is crucial for reliable operation.
61-76: Well-explained transaction execution approachThe section on using the payload as the first transaction clearly explains the unique approach to transaction execution and its benefits.
Consider this minor improvement:
- It's not possible to create a payload outside of the EVM - Transactions cannot be selected or ordered outside of the EVM + It's not possible to create a payload outside the EVM + Transactions cannot be selected or ordered outside the EVM🧰 Tools
🪛 LanguageTool
[style] ~74-~74: This phrase is redundant. Consider using “outside”.
Context: ...- It's not possible to create a payload outside of the EVM - Transactions cannot be select...(OUTSIDE_OF)
[style] ~75-~75: This phrase is redundant. Consider using “outside”.
Context: ...nsactions cannot be selected or ordered outside of the EVM ### How Eth API is Used The `...(OUTSIDE_OF)
77-86: Good explanation of Eth API usageThis section clearly describes how the standard Ethereum JSON-RPC API is used alongside the Engine API.
87-101: Improved deployment architecture diagramThe mermaid diagram effectively illustrates the connections between components.
103-122: Helpful development and testing instructionsThe instructions for running Reth in Docker and reading genesis information are clear and practical.
The mock implementations in `mocks_test.go` were no longer in use and have been fully removed. This cleanup simplifies the codebase and eliminates unneeded maintenance of outdated test structures.
Corrects the loop condition to ensure the iteration ends at blockHeight 10 instead of exceeding it. This resolves potential out-of-bounds issues when accessing payloads during testing.
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.
Actionable comments posted: 2
🧹 Nitpick comments (4)
execution_test.go (4)
315-318: Reuse HTTP client for better performance.Creating a new HTTP client for each request in the polling loop is inefficient. The client can be reused throughout the function's lifecycle.
+// Create client outside the loop for better performance func waitForRethContainer(t *testing.T, jwtSecret string) error { t.Helper() client := &http.Client{ Timeout: 100 * time.Millisecond, }
81-82: Consider adding context timeout as a constant or parameter.The 300-second timeout is hardcoded in multiple places. Extract it to a constant for better maintainability.
+const TEST_CONTEXT_TIMEOUT = 300 * time.Second func TestEngineExecution(t *testing.T) { // ... - ctx, cancel := context.WithTimeout(context.Background(), 300*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), TEST_CONTEXT_TIMEOUT)
370-390: Skipped test contains useful functionality.The
TestSubmitTransactionis skipped but contains useful transaction submission functionality. Consider refactoring this into a utility function that can be used by both tests and manual testing.+// Helper function that can be used by tests or invoked manually with t.Skip() removed +func submitTestTransactions(t *testing.T, count int) { + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + defer cancel() + rpcClient, err := ethclient.Dial(TEST_ETH_URL) + require.NoError(t, err) + height, err := rpcClient.BlockNumber(ctx) + require.NoError(t, err) + + privateKey, err := crypto.HexToECDSA(TEST_PRIVATE_KEY) + require.NoError(t, err) + + address := crypto.PubkeyToAddress(privateKey.PublicKey) + nonce, err := rpcClient.NonceAt(ctx, address, new(big.Int).SetUint64(height)) + require.NoError(t, err) + + for i := 0; i < count; i++ { + tx := getRandomTransaction(t, 22000) + submitTransaction(t, tx) + } +} + func TestSubmitTransaction(t *testing.T) { t.Skip("Use this test to submit a transaction manually to the Ethereum client") - // Existing implementation + submitTestTransactions(t, 10) }
60-193: Comprehensive test structure looks good, with a few suggestions.The test is well-structured with clear "Build chain" and "Sync chain" phases, good error handling, and appropriate assertions. Consider these minor improvements:
- Add more descriptive comments for edge cases like "Block 4 has 0 transactions"
- Consider parameterizing the number of blocks (10) instead of hardcoding it
- Add validation that both phases produce the same final state (same state root)
func TestEngineExecution(t *testing.T) { + const ( + initialHeight = uint64(1) + finalHeight = uint64(10) + ) - allPayloads := make([][][]byte, 0, 10) // Slice to store payloads from build to sync phase + allPayloads := make([][][]byte, 0, finalHeight) // Slice to store payloads from build to sync phase - initialHeight := uint64(1)And at the end of the test, add validation:
}) + // Verify that both phases produced the same final state + t.Run("Verify final states match", func(tt *testing.T) { + // Query both execution clients and compare final state roots + }) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
execution_test.go(1 hunks)mocks_test.go(0 hunks)
💤 Files with no reviewable changes (1)
- mocks_test.go
🧰 Additional context used
🪛 Gitleaks (8.21.2)
execution_test.go
31-31: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (2)
execution_test.go (2)
31-31: Replace hardcoded private key with environment variable or test-only generation.The hardcoded private key is flagged as a potential security issue by static analysis. While this is a test file, it's a good practice to avoid hardcoded credentials.
- TEST_PRIVATE_KEY = "cece4f25ac74deb1468965160c7185e07dff413f23fcadb611b05ca37ab0a52e" + // For tests only, generate a new key or use an environment variable + TEST_PRIVATE_KEY = os.Getenv("TEST_PRIVATE_KEY_FOR_TESTS")Add a fallback in the
getRandomTransactionfunction:func getRandomTransaction(t *testing.T, gasLimit uint64) *ethTypes.Transaction { + privateKeyHex := TEST_PRIVATE_KEY + if privateKeyHex == "" { + // Generate a test-only key if not provided + key, err := crypto.GenerateKey() + require.NoError(t, err) + privateKeyBytes := crypto.FromECDSA(key) + privateKeyHex = hex.EncodeToString(privateKeyBytes) + } - privateKey, err := crypto.HexToECDSA(TEST_PRIVATE_KEY) + privateKey, err := crypto.HexToECDSA(privateKeyHex) require.NoError(t, err)🧰 Tools
🪛 Gitleaks (8.21.2)
31-31: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
164-164: Fix index out of range error in Sync chain test.The index calculation may cause an out-of-bounds error. Add bounds checking to prevent potential panics.
-payload := allPayloads[blockHeight-initialHeight-1] +// Use a zero-based index to avoid out of range errors +payloadIndex := blockHeight - initialHeight - 1 +if payloadIndex < 0 || int(payloadIndex) >= len(allPayloads) { + tt.Fatalf("Invalid payload index %d for allPayloads length %d", payloadIndex, len(allPayloads)) +} +payload := allPayloads[payloadIndex]
Pure Engine API-based Execution Client for Rollkit
Overview
This PR provides a complete reimplementation of the go-execution-evm package, with both code and documentation recreated from scratch. It implements the
execution.Executorinterface from the Rollkitfeature/exec_apibranch.Resolves #7.
Resolves #9.
Fixes previously closed, but still broken: #3, #4, #5, #6.
Key Changes
feature/exec_apibranch in RollkitTechnical Highlights
PureEngineClientthat uses both Engine API and standard Ethereum JSON-RPC APIThis implementation provides a clean, standards-compliant way to connect Rollkit to Ethereum execution clients.
Summary by CodeRabbit
Documentation
New Features
Chores