feat(node/test): add tx inclusion test

[theochap]

Description

Adds a tx inclusion test to the E2e tests.

[claude]

Claude finished @theochap's task —— View job

---

Code Review: Transaction Inclusion Test

I've completed my review of the new transaction inclusion test. Here's my comprehensive feedback:

Review Checklist:

• Read and analyze the new test file
• Evaluate code quality and best practices
• Check for potential bugs or issues
• Assess performance considerations
• Review security concerns
• Evaluate test coverage and effectiveness
• Provide comprehensive feedback

✅ Strengths

Good Test Structure: The test follows established patterns in the codebase, using devtest.ParallelT(gt) and node_utils.NewMixedOpKona(t) consistently with other tests in the suite.

Comprehensive Validation: The test properly validates both:

• Block hash consistency across all nodes (tests/node/common/tx_inclusion_test.go:41)
• Balance updates reflecting the transaction (tests/node/common/tx_inclusion_test.go:46)

Error Handling: Appropriate error handling for transaction receipt lookup with descriptive error messages (tests/node/common/tx_inclusion_test.go:26).

Multi-Node Testing: Correctly tests transaction propagation across all L2 execution layer nodes, ensuring network-wide consistency.

🟡 Potential Improvements

Edge Case Handling: The test handles the case where WaitForBlockNumber returns a later block (tests/node/common/tx_inclusion_test.go:36-38), but this logic could be more robust:

// Consider adding a timeout or max retry logic here
if block.Number > inclusionBlock.Number {
    blockID = node.BlockRefByNumber(inclusionBlock.Number).ID()
}

Test Documentation: Consider adding a brief comment explaining the test's purpose, similar to other tests in the suite (see sync_test.go:14).

Magic Numbers: The test uses eth.OneEther and eth.HalfEther constants, which is good practice. Consider if these amounts are appropriate for the test scenario.

🟢 Code Quality Assessment

Best Practices: ✅

• Follows Go naming conventions
• Uses appropriate error handling
• Leverages existing test utilities effectively
• Consistent with codebase patterns

Performance: ✅

• Uses devtest.ParallelT() for parallel execution
• Efficient balance checking with VerifyBalanceExact()
• No obvious performance bottlenecks

Security: ✅

• Uses proper funded EOA creation through the test framework
• No hardcoded private keys or sensitive data
• Follows secure testing patterns

Test Coverage: ✅

• Tests the critical path of transaction inclusion
• Validates both block propagation and state changes
• Covers multi-node scenario which is important for L2 networks

🎯 Verdict

This is a well-implemented test that adds valuable coverage for transaction inclusion across the network. The code follows established patterns, handles edge cases appropriately, and provides comprehensive validation. The test would benefit from minor documentation improvements but is otherwise ready for merge.

Recommendation: ✅ Approve - This test enhances the test suite with important transaction inclusion validation.

---

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.0%. Comparing base (c42e2bb) to head (d503832).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.