test: add tastora tests for force inclusion and enable feature

[julienrbrt]

Closes: #2906

[github-actions]

The latest Buf updates on your PR. Results from workflow CI / buf-check (pull_request).

Build	Format	Lint	Breaking	Updated (UTC)
✅ passed	⏩ skipped	✅ passed	✅ passed	Jan 9, 2026, 4:34 PM

[claude]

Claude finished @julienrbrt's task in 2m 33s —— View job

---

Code Review Complete

I've reviewed this PR which enables the force inclusion feature and adds comprehensive E2E tests. Here's my detailed feedback:

---

Summary

Overall Assessment: The implementation is solid and well-structured. The main changes enable the forced inclusion namespace validation (removing the "not yet live" guard) and add comprehensive E2E tests covering both sequencer-only and full-node synchronization scenarios.

Key Changes:

• pkg/config/config.go: Enables forced inclusion namespace validation
• test/e2e/evm_force_inclusion_e2e_test.go: Adds 270 lines of E2E test coverage

---

Detailed Feedback

1. Code Quality & Architecture ✅

Strengths:

• Clean separation of concerns with helper functions (enableForceInclusionInGenesis, submitForceInclusionTx, setupSequencerWithForceInclusion)
• Follows existing test patterns from evm_test_common.go and evm_full_node_e2e_test.go
• Proper use of t.Helper() in test utilities
• Good use of table-driven approach (two separate tests for different scenarios)

Minor Observations:

• The config change in config.go:281-285 is minimal and correctly uncomments the validation logic
• Test structure follows established conventions in the E2E test suite

---

2. Error Handling & Resilience ⚠️

Issues Found:

2.1 Insufficient Error Context in Force Inclusion Submission

evm_force_inclusion_e2e_test.go:66 - When RPC errors occur, the error context is limited:

require.Nil(t, res["error"], "RPC returned error: %v", res["error"])

Recommendation: Add more diagnostic information:

if res["error"] != nil {
    t.Logf("Full RPC response: %+v", res)
}
require.Nil(t, res["error"], "RPC returned error: %v", res["error"])

2.2 HTTP Response Body Not Always Read

evm_force_inclusion_e2e_test.go:59-64 - The function decodes the response but doesn't check for partial reads or malformed JSON with better error messages.

Recommendation: Add validation:

var res map[string]interface{}
if err := json.NewDecoder(resp.Body).Decode(&res); err != nil {
    t.Fatalf("Failed to decode RPC response: %v", err)
}

2.3 No Retry Logic for Force Inclusion Submission

The submitForceInclusionTx function doesn't handle transient network failures. In E2E tests with many concurrent operations, HTTP requests can occasionally fail.

Recommendation: Consider adding retry logic with exponential backoff, similar to how transaction inclusion is polled with require.Eventually.

---

3. Security ✅

Strengths:

• No hardcoded secrets (uses TestPrivateKey constant defined elsewhere)
• Proper file permissions on created files (via existing helpers)
• JWT authentication properly configured via test helpers

Observations:

• The test uses predictable test keys, which is appropriate for E2E tests
• Force inclusion server runs on localhost only (secure for tests)

---

4. Performance & Resource Efficiency ⚠️

4.1 Polling Intervals Could Be Optimized

evm_force_inclusion_e2e_test.go:149-150 and 265-267:

}, 15*time.Second, 500*time.Millisecond, "Normal transaction not included")
...
}, 30*time.Second, 1*time.Second, "Forced inclusion transaction not included")

Recommendation: Consider making these configurable or using constants from evm_test_common.go:

• Use SlowPollingInterval (250ms) for more responsive tests
• Document why force inclusion needs longer timeout (DA epoch dependency)

4.2 Generous Timeouts

The full node test uses 40*time.Second timeout for force inclusion sync. While this ensures robustness, it could slow down test execution.

Recommendation: Add a comment explaining the timeout calculation based on DA epoch (2 blocks) and DA block time (1s), making it clear why this buffer is needed.

---

5. Testing & Reliability ⚠️

5.1 Missing Test Cases

The PR adds good coverage but could benefit from additional scenarios:

Edge Cases Not Covered:

1. Invalid force inclusion transaction - What happens if the transaction is malformed?
2. Duplicate submission - What if the same transaction is submitted twice?
3. Concurrent normal + forced txs - Mix of both transaction types in quick succession
4. Force inclusion with invalid nonce - How does the system handle nonce mismatches?
5. Genesis epoch value edge cases - What if epoch is 0 or 1?

5.2 Test Isolation Concerns

evm_force_inclusion_e2e_test.go:23-42 - The enableForceInclusionInGenesis function modifies the genesis file directly. If tests run in parallel, this could cause conflicts.

Status: Currently using build evm tag and t.TempDir(), so should be safe. But worth documenting.

5.3 Magic Numbers

• epoch uint64 = 2 is hardcoded in test calls (lines 93, 200)
• fiPort allocation doesn't have error handling context

Recommendation:

const (
    TestForceInclusionEpoch = 2 // Use 2 DA blocks for faster testing
)

---

6. Documentation & Developer Experience ⚠️

6.1 Missing Function Documentation

Several functions lack comprehensive documentation:

enableForceInclusionInGenesis (line 25):

• ✅ Has basic doc comment
• ⚠️ Should explain what "epoch" means (DA blocks before forced inclusion)
• ⚠️ Should mention the tradeoff (lower = faster tests, but less realistic)

submitForceInclusionTx (line 44):

• ✅ Has basic doc comment
• ⚠️ Should document the expected RPC response format
• ⚠️ Should mention this uses eth_sendRawTransaction method

setupSequencerWithForceInclusion (line 71):

• ⚠️ Doc comment should explain the fiPort parameter more clearly
• ⚠️ Should document return values explicitly

6.2 Test Function Documentation

Both test functions (lines 121, 172) could benefit from more detailed comments explaining:

• What "force inclusion" means in the context of ev-node
• The DA epoch mechanism and why it affects timing
• The difference between the two test scenarios

Recommendation:

// TestEvmSequencerForceInclusionE2E tests force inclusion transactions on a single sequencer.
// 
// Force inclusion allows transactions to be submitted directly to the DA layer
// (via a separate HTTP server) and included in blocks based on DA epochs.
// With epoch=2, transactions become eligible after 2 DA blocks (~2 seconds with 1s DA block time).
//
// Test flow:
// 1. Setup sequencer with force inclusion server enabled
// 2. Send normal transaction to verify chain is producing blocks
// 3. Submit transaction via force inclusion server
// 4. Verify the forced transaction is included within expected timeframe
func TestEvmSequencerForceInclusionE2E(t *testing.T) {

6.3 Coverage Report Issue

The Codecov report shows 0% patch coverage with 2 lines missing in config.go. This is because:

• Lines 281-285 are only executed when ForcedInclusionNamespace is set
• The E2E tests set this flag, but coverage isn't being collected from E2E test runs

Not a code issue, but worth noting that unit tests for the config validation would improve coverage metrics.

---

7. Specific Line-by-Line Issues

Issue #1: File Permission Constant

evm_force_inclusion_e2e_test.go:40 - Uses 0644 for genesis file:

err = os.WriteFile(genesisPath, newData, 0644)

Severity: Low
Recommendation: Use 0600 for consistency with other sensitive files (JWT secret uses 0600 in line 223), or add a comment explaining why genesis can be world-readable.

Issue #2: Incomplete Error Recovery

evm_force_inclusion_e2e_test.go:59 - The defer resp.Body.Close() happens after status check:

resp, err := http.Post(fiUrl, "application/json", bytes.NewBuffer(jsonData))
require.NoError(t, err)
defer resp.Body.Close()

require.Equal(t, http.StatusOK, resp.StatusCode)

If resp.StatusCode != 200, the test fails but we lose the response body content which could help debug the failure.

Recommendation:

resp, err := http.Post(fiUrl, "application/json", bytes.NewBuffer(jsonData))
require.NoError(t, err)
defer resp.Body.Close()

if resp.StatusCode != http.StatusOK {
    body, _ := io.ReadAll(resp.Body)
    t.Fatalf("Expected 200, got %d. Body: %s", resp.StatusCode, string(body))
}

Issue #3: Nonce Management

Both tests initialize nonce to 0:

var nonce uint64 = 0

This works because each test uses t.TempDir() and fresh EVM instances. However, it's worth documenting that the test account starts with nonce 0.

Issue #4: Namespace Validation Missing Test

config.go:281-285 - The validation now runs for forced inclusion namespace, but there's no unit test verifying:

• Valid namespace passes validation
• Invalid namespace (wrong format) fails validation
• Empty namespace is handled correctly

Fix this →

---

Recommendations Summary

High Priority

1. ✅ Enable the feature - The code change is correct and minimal
2. ⚠️ Add error context - Improve error messages in submitForceInclusionTx
3. ⚠️ Add unit tests - Test config validation for forced inclusion namespace

Medium Priority

4. 📝 Improve documentation - Add comprehensive doc comments explaining force inclusion mechanism
5. 🧪 Add edge case tests - Cover invalid transactions, duplicates, nonce mismatches
6. ⏱️ Document timeouts - Explain why specific timeout values are chosen

Low Priority

7. 🔧 Use consistent file permissions - Consider 0600 for genesis file
8. 🔄 Add retry logic - Handle transient HTTP failures in force inclusion submission
9. 📊 Improve coverage - Add unit tests to improve coverage metrics

---

Conclusion

This is a well-implemented PR that successfully enables the forced inclusion feature with solid E2E test coverage. The code follows existing patterns and maintains good code quality.

The main areas for improvement are around error handling robustness, test coverage for edge cases, and documentation to help future developers understand the force inclusion mechanism.

Recommendation: ✅ Approve with minor suggestions - The code is production-ready. The suggestions above are enhancements that can be addressed in follow-up PRs if desired.

---

[codecov]

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.75%. Comparing base (aaae087) to head (e7b0284).

Files with missing lines	Patch %	Lines
pkg/config/config.go	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2964      +/-   ##
==========================================
- Coverage   57.88%   57.75%   -0.14%     
==========================================
  Files          97       97              
  Lines        9306     9307       +1     
==========================================
- Hits         5387     5375      -12     
- Misses       3315     3329      +14     
+ Partials      604      603       -1     

Flag	Coverage Δ
combined	57.75% <0.00%> (-0.14%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.