test: add E2E test infrastructure and helpers#63
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds nextest profiles and workflow concurrency; introduces large E2E test utilities and genesis fixture; exposes Changes
Sequence Diagram(s)sequenceDiagram
participant TestHarness as Test Harness
participant Node as Morph Node
participant Engine as Execution Engine
participant Forkchoice as Forkchoice
TestHarness->>Node: build payload (MorphTxBuilder / MorphPayloadBuilder)
TestHarness->>Node: send_new_payload(payload)
Node->>Engine: submit payload for execution
Engine->>Engine: validate & execute
Engine-->>Node: resolve_kind / execution outcome
Node->>Forkchoice: forkchoice_updated(head, status)
Forkchoice-->>Node: confirm / sync
Node-->>TestHarness: return payload id / block result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
2 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
crates/node/src/test_utils.rs (3)
434-457: Clarify that EIP-4844 tx uses placeholder blob commitment.The
blob_versioned_hashesat line 450 uses a placeholder (B256::with_last_byte(0x01)) that won't pass KZG proof validation. This transaction will be rejected by nodes that validate blob commitments.Consider adding a doc comment noting this is for rejection tests or pool admission tests, not for executing valid blob transactions.
Suggested doc improvement
/// Creates a signed EIP-4844 (type 0x03) transaction. +/// +/// **Note**: Uses a placeholder blob versioned hash that will fail KZG validation. +/// This helper is suitable for testing pool rejection or transaction type recognition, +/// not for tests that require valid blob execution. pub fn make_eip4844_tx(chain_id: u64, signer: PrivateKeySigner, nonce: u64) -> eyre::Result<Bytes> {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/node/src/test_utils.rs` around lines 434 - 457, The EIP-4844 tx created by make_eip4844_tx uses a placeholder blob commitment (blob_versioned_hashes set via B256::with_last_byte(0x01)) which will fail KZG validation; update the function's doc comment to explicitly state this is a synthetic/placeholder commitment intended only for tests that expect pool admission or validation rejection (not for executing valid blob transactions), and mention that nodes validating KZG proofs will reject it so callers/tests should not assume full consensus acceptance.
325-333: Consider returning an error instead of defaulting to zero hash.If
sealed_header_by_number_or_tag(Latest)returnsNone, the code defaults toparent = B256::ZERO. This would cause the new block to reference a non-existent parent, resulting in either a rejected payload or a broken chain state.While this should never happen in properly initialized E2E tests, an explicit error would surface misconfigurations faster.
Suggested improvement
- let (head_hash, head_ts) = head - .map(|h| (h.hash(), h.timestamp())) - .unwrap_or((B256::ZERO, 0)); + let (head_hash, head_ts) = head + .map(|h| (h.hash(), h.timestamp())) + .ok_or_else(|| eyre::eyre!("no chain head found; ensure genesis is initialized"))?;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/node/src/test_utils.rs` around lines 325 - 333, The code currently unwraps the Optional header from sealed_header_by_number_or_tag and substitutes (B256::ZERO, 0) when None; instead, in the function that computes head_hash/head_ts (using sealed_header_by_number_or_tag, head.map(...).unwrap_or(...)), return an explicit error when the header is None so callers don't proceed with a zero parent. Replace the unwrap_or fallback with an early Err (e.g., using eyre::eyre! or anyhow::anyhow!) that includes context like "no sealed header found for Latest" and adjust the function's signature to return a Result so callers of head_hash/head_ts can handle the failure.
657-670: Moveusestatement to function start for clarity.The
use alloy_rlp::{BufMut, Header};at line 661 appears afterbuf.put_u8()at line 660. While Rust allows this (block-scopeduseapplies to the entire block), it's unconventional and may confuse readers.Suggested reordering
pub fn build_encoded(self) -> Bytes { + use alloy_rlp::{BufMut, Header}; + let tx = TxL1Msg { queue_index: self.queue_index, gas_limit: self.gas_limit, to: self.target, value: self.value, sender: self.sender, input: self.data, }; // EIP-2718 encoding: type byte (0x7E) + RLP body let mut buf = Vec::with_capacity(1 + tx.fields_len() + 4); - // TxL1Msg::encode_2718 writes the type byte followed by RLP list buf.put_u8(L1_TX_TYPE_ID); - use alloy_rlp::{BufMut, Header}; let header = Header { list: true, payload_length: tx.fields_len(), };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/node/src/test_utils.rs` around lines 657 - 670, Move the local import into the top of the function scope for clarity: the statement `use alloy_rlp::{BufMut, Header};` should be placed before the first use of `buf`/`buf.put_u8()` (i.e., at the start of the function containing this snippet) so imports appear before code that uses `L1_TX_TYPE_ID`, `buf.put_u8()`, `Header`, and `tx.encode_fields()`; ensure no other scoping change is introduced and keep the existing logic that builds the buffer and calls `header.encode(&mut buf)` and `tx.encode_fields(&mut buf)`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/lint.yml:
- Around line 57-59: The CI step invoking "cargo clippy -p morph-node --test it
--features test-utils -- -D warnings" fails because the test target named "it"
(declared in crates/node/Cargo.toml) requires a file at tests/it/main.rs that is
missing; either add a minimal stub test file tests/it/main.rs (e.g., a basic
#[test] or main test harness) to satisfy the --test it target, or remove/adjust
the workflow step to omit the --test it flag until the actual integration tests
are added.
In @.github/workflows/test.yml:
- Around line 64-94: The E2E job named "e2e" invokes cargo nextest with the flag
"--test it" which requires a test binary named "it" (e.g.,
crates/node/tests/it/main.rs) that doesn't exist; either add the missing test
file (create the test binary under crates/node/tests/it/main.rs or equivalent
test target) or remove/replace the "--test it" token in the Run step (cargo
nextest run --profile ci -p morph-node [--test <existing-name>] --features
test-utils) so the job targets an existing test; update the Run step accordingly
to reference the actual test target or add the "it" test source.
In `@crates/node/Cargo.toml`:
- Around line 87-90: The CI is failing because the test target declared in
Cargo.toml ([[test]] name = "it", path = "tests/it/main.rs") points to a missing
file; create the missing test stub at crates/node/tests/it/main.rs with a
minimal Rust test binary (e.g., module comment header and an empty fn main() so
the --test it target can compile) so the test target resolves and CI can
proceed.
---
Nitpick comments:
In `@crates/node/src/test_utils.rs`:
- Around line 434-457: The EIP-4844 tx created by make_eip4844_tx uses a
placeholder blob commitment (blob_versioned_hashes set via
B256::with_last_byte(0x01)) which will fail KZG validation; update the
function's doc comment to explicitly state this is a synthetic/placeholder
commitment intended only for tests that expect pool admission or validation
rejection (not for executing valid blob transactions), and mention that nodes
validating KZG proofs will reject it so callers/tests should not assume full
consensus acceptance.
- Around line 325-333: The code currently unwraps the Optional header from
sealed_header_by_number_or_tag and substitutes (B256::ZERO, 0) when None;
instead, in the function that computes head_hash/head_ts (using
sealed_header_by_number_or_tag, head.map(...).unwrap_or(...)), return an
explicit error when the header is None so callers don't proceed with a zero
parent. Replace the unwrap_or fallback with an early Err (e.g., using
eyre::eyre! or anyhow::anyhow!) that includes context like "no sealed header
found for Latest" and adjust the function's signature to return a Result so
callers of head_hash/head_ts can handle the failure.
- Around line 657-670: Move the local import into the top of the function scope
for clarity: the statement `use alloy_rlp::{BufMut, Header};` should be placed
before the first use of `buf`/`buf.put_u8()` (i.e., at the start of the function
containing this snippet) so imports appear before code that uses
`L1_TX_TYPE_ID`, `buf.put_u8()`, `Header`, and `tx.encode_fields()`; ensure no
other scoping change is introduced and keep the existing logic that builds the
buffer and calls `header.encode(&mut buf)` and `tx.encode_fields(&mut buf)`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 40971610-741b-4cce-bfbc-e81dc0acd1f8
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
.config/nextest.toml.github/workflows/lint.yml.github/workflows/test.yml.gitignoreCargo.tomlcrates/node/Cargo.tomlcrates/node/src/lib.rscrates/node/src/node.rscrates/node/src/test_utils.rscrates/node/tests/assets/test-genesis.jsoncrates/payload/types/src/attributes.rs
panos-xyz
added a commit
that referenced
this pull request
Mar 26, 2026
Move E2E-specific configuration from infrastructure PR (#63) to this stacked PR where the test files actually live: - Add [[test]] name="it" to Cargo.toml - Add E2E job to test.yml (nextest with ci profile) - Add clippy --test it step to lint.yml - Fix rustfmt in rpc.rs (long assert_eq line)
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.config/nextest.toml:
- Line 2: The file .config/nextest.toml contains leftover Git merge conflict
markers (e.g., lines with <<<<<<<, =======, >>>>>>>) making the TOML invalid and
blocking the nextest step; open .config/nextest.toml, locate and remove all
conflict markers (references: the conflict markers themselves and the file
.config/nextest.toml), resolve the conflicting blocks by keeping the intended
configuration content (choose the correct block between the
<<<<<<</=======/>>>>>>> sections or merge them manually), ensure the final file
is valid TOML with no conflict markers, and save the cleaned file so the
workflow can run.
- Around line 25-31: The override under profile.ci.overrides (filter =
"package(morph-node) & binary(it)") no longer guarantees one-at-a-time MDBX
tests because test-threads is now "num-cpus" (4); update the CI config so MDBX
tests consume all slots: either revert test-threads back to 2 to keep the
current threads-required = 2 behavior, or increase threads-required to 4 for the
profile.ci.overrides entry (threads-required = 4) so these tests run
exclusively; modify the appropriate keys (test-threads or threads-required)
accordingly to restore strict MDBX isolation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
panos-xyz
force-pushed
the
panos/e2e-infra
branch
from
4a2073a to
7697218
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/test.yml (1)
62-84: Well-structured E2E job configuration.The job correctly mirrors the test job setup with appropriate
timeout-minutes: 15. The nextest CI profile overrides (threads-required=2, 120s slow-timeout) align well with MDBX isolation requirements per the nextest config.One optional consideration: adding
--lockedto the cargo commands would ensure CI uses exact Cargo.lock versions for reproducible builds.,
💡 Optional: Add --locked for reproducibility
- name: Run E2E tests - run: cargo nextest run --profile ci -p morph-node --test it --features test-utils + run: cargo nextest run --locked --profile ci -p morph-node --test it --features test-utilsThis also applies to the other
cargocommands in the workflow if reproducibility is desired.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/test.yml around lines 62 - 84, Add the --locked flag to the cargo invocation in the E2E job to ensure reproducible builds: update the "Run E2E tests" step that currently runs `cargo nextest run --profile ci -p morph-node --test it --features test-utils` to include `--locked`; also scan other steps that invoke cargo in this workflow and add --locked to those commands as well so all Cargo executions use the exact Cargo.lock versions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/test.yml:
- Around line 62-84: Add the --locked flag to the cargo invocation in the E2E
job to ensure reproducible builds: update the "Run E2E tests" step that
currently runs `cargo nextest run --profile ci -p morph-node --test it
--features test-utils` to include `--locked`; also scan other steps that invoke
cargo in this workflow and add --locked to those commands as well so all Cargo
executions use the exact Cargo.lock versions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: aaec0e39-f17c-420e-81c6-679eb2cd243a
📒 Files selected for processing (4)
.config/nextest.toml.github/workflows/build.yml.github/workflows/lint.yml.github/workflows/test.yml
✅ Files skipped from review due to trivial changes (2)
- .github/workflows/build.yml
- .config/nextest.toml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/lint.yml
panos-xyz
added a commit
that referenced
this pull request
Mar 27, 2026
Move E2E-specific configuration from infrastructure PR (#63) to this stacked PR where the test files actually live: - Add [[test]] name="it" to Cargo.toml - Add E2E job to test.yml (nextest with ci profile) - Add clippy --test it step to lint.yml - Fix rustfmt in rpc.rs (long assert_eq line)
chengwenxi
approved these changes
Mar 31, 2026
Contributor
chengwenxi
left a comment
chengwenxi
left a comment
There was a problem hiding this comment.
LGTM. Solid E2E test infrastructure — TestNodeBuilder with HardforkSchedule, transaction builders, chain advancement helpers. Minor notes:
serde_jsonandalloy-genesisshould be feature-gated undertest-utilsinstead of unconditional depsnextest.tomlcomment about serialization is slightly misleading (threads-required = 2only serializes on 2-CPU runners)
panos-xyz
added a commit
that referenced
this pull request
Mar 31, 2026
* test: add E2E integration tests for morph-node Add comprehensive E2E integration tests covering all morph-reth features: - sync: basic can_sync test (10 blocks via Engine API) - block_building: empty/single-tx/multi-tx/L1-msg/mixed blocks - consensus: header validation, L1 message sequencing, timestamp rules - engine: assembleL2Block, newL2Block, validateL2Block, newSafeL2Block - evm: contract deploy/state, BLOCKHASH, SELFDESTRUCT, L1 fee, revert - hardfork: Curie→Jade EVM transitions, disabled precompile stubs - l1_messages: deposit flow, sequential queue indices, gas prepaid - morph_tx: MorphTx v0 lifecycle, ERC20 token fee, revert behavior, fee log ordering, receipt fields - rpc: eth_getTransactionReceipt/ByHash Morph fields, debug_trace*, trace_* namespace - txpool: accept/reject by tx type, blob rejection, EIP-7702 gating, MorphTx version/hardfork constraints * chore: add E2E CI config, [[test]] declaration, and fmt fix Move E2E-specific configuration from infrastructure PR (#63) to this stacked PR where the test files actually live: - Add [[test]] name="it" to Cargo.toml - Add E2E job to test.yml (nextest with ci profile) - Add clippy --test it step to lint.yml - Fix rustfmt in rpc.rs (long assert_eq line)
Add the foundation for morph-node E2E integration tests: - test_utils module with setup(), advance_chain(), advance_empty_block() helpers, morph_payload_attributes() builder, and make_*_tx() helpers for constructing signed EIP-1559/EIP-7702/L1/MorphTx/Deploy/EIP-4844 transactions - test-genesis.json with pre-funded test accounts, L1 Gas Price Oracle Curie storage, L2TokenRegistry, and system contract state - From<EthPayloadBuilderAttributes> for MorphPayloadBuilderAttributes to enable reth E2E test framework compatibility - nextest config with CI profile (threads-required=2 for E2E isolation) - CI workflow: E2E test job using nextest, clippy with test-utils feature - test-utils feature gate for optional E2E test dependencies
The [[test]] entry, E2E CI job, and clippy --test it step reference tests/it/main.rs which lives in the stacked PR (#64). Move these declarations there so this infrastructure PR can pass CI independently.
- Replace `cargo test` with `cargo nextest run --profile ci` - Add E2E test job with 15min timeout - Remove hardcoded test-threads, use num-cpus for both profiles - Add slow-timeout = 30s with exponential backoff retries (ref: scroll-reth) - Add concurrency groups to cancel duplicate CI runs (ref: scroll/tempo) - Replace manual cache with Swatinem/rust-cache - Set RUST_MIN_STACK=8388608 for large test binaries
The nextest overrides reference binary(it) which only exists after the E2E test files are added (stacked PR #64). Without the binary, nextest fails with "operator didn't match any binary names". Also remove the E2E job from test.yml for the same reason.
* test: add E2E integration tests for morph-node Add comprehensive E2E integration tests covering all morph-reth features: - sync: basic can_sync test (10 blocks via Engine API) - block_building: empty/single-tx/multi-tx/L1-msg/mixed blocks - consensus: header validation, L1 message sequencing, timestamp rules - engine: assembleL2Block, newL2Block, validateL2Block, newSafeL2Block - evm: contract deploy/state, BLOCKHASH, SELFDESTRUCT, L1 fee, revert - hardfork: Curie→Jade EVM transitions, disabled precompile stubs - l1_messages: deposit flow, sequential queue indices, gas prepaid - morph_tx: MorphTx v0 lifecycle, ERC20 token fee, revert behavior, fee log ordering, receipt fields - rpc: eth_getTransactionReceipt/ByHash Morph fields, debug_trace*, trace_* namespace - txpool: accept/reject by tx type, blob rejection, EIP-7702 gating, MorphTx version/hardfork constraints * chore: add E2E CI config, [[test]] declaration, and fmt fix Move E2E-specific configuration from infrastructure PR (#63) to this stacked PR where the test files actually live: - Add [[test]] name="it" to Cargo.toml - Add E2E job to test.yml (nextest with ci profile) - Add clippy --test it step to lint.yml - Fix rustfmt in rpc.rs (long assert_eq line)
panos-xyz
force-pushed
the
panos/e2e-infra
branch
from
0d2c2b5 to
29e9972
Compare
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
2 participants
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
test_utilsmodule withsetup(),advance_chain(),advance_empty_block()helpers andmake_*_tx()transaction builderstest-genesis.jsonwith pre-funded accounts, L1 Gas Price Oracle Curie storage, L2TokenRegistry, and system contract stateFrom<EthPayloadBuilderAttributes>forMorphPayloadBuilderAttributes(reth E2E framework compatibility)test-utilsfeature gate with optional E2E test dependenciesScope
This PR provides the foundation for E2E integration tests. The actual test cases follow in a separate stacked PR.
Test plan
cargo check -p morph-node --features test-utils— compiles cleanSummary by CodeRabbit
New Features
Tests
Chores
Style