feat(evm): add force inclusion command

[julienrbrt]

Add force inclusion command for evm app.
Similar to evstack/ev-abci#295

Blocked on #2797

[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	Nov 28, 2025, 4:00 PM

[claude]

Claude finished @julienrbrt's task —— View job

---

Code Review Complete

I've conducted a comprehensive code review of the force inclusion command implementation. Here are my findings:

---

Summary

This PR adds force inclusion functionality for the EVM app, allowing users to submit transactions directly to the DA layer for guaranteed inclusion. The implementation includes:

• CLI command (post-tx) for submitting transactions
• HTTP server with Ethereum JSON-RPC compatible API
• Integration with the existing EVM executor
• Comprehensive test coverage

Code Quality & Architecture

✅ Strengths

1. Clean separation of concerns: The implementation properly separates CLI logic (post_tx_cmd.go), server logic (force_inclusion.go), and integration (run.go)

2. Modular design: Force inclusion server is optional and can be enabled via flag, maintaining backward compatibility

3. Proper use of Go idioms: Good use of context, defer patterns, and error wrapping

4. Graceful invalid transaction handling: Both execution/evm/execution.go and apps/testapp/kv/kvexecutor.go properly filter out invalid/gibberish transactions instead of failing (lines 254-287 in execution.go, lines 242-284 in kvexecutor.go)

⚠️ Issues & Recommendations

1. Typo in Documentation (apps/evm/cmd/post_tx_cmd.go:36)

Long: `Post a signed Ethereum transaction tzo the configured DA layer for forced inclusion.

Should be "to" not "tzo"

2. Hardcoded Transaction Hash (apps/evm/server/force_inclusion.go:299)

txHash := fmt.Sprintf("0x%064x", daHeight)

Issue: Returns DA height as a fake transaction hash. This could be misleading to users/tools expecting a real Ethereum transaction hash.

Recommendation: Document this behavior clearly in the README and consider:

• Returning the DA blob ID instead
• Or computing a deterministic hash from the transaction + DA height
• Adding a comment explaining why this approach was chosen

3. Missing Context Cancellation in CLI (apps/evm/cmd/post_tx_cmd.go:144)

The DAClient.Submit uses cmd.Context() but there's no timeout, which could hang indefinitely if DA is unavailable. Consider adding:

ctx, cancel := context.WithTimeout(cmd.Context(), 60*time.Second)
defer cancel()

4. Unchecked Gas Price Flag (apps/evm/cmd/post_tx_cmd.go:110-113)

gasPrice, err := cmd.Flags().GetFloat64(flagGasPrice)
if err != nil {
    return fmt.Errorf("failed to get gas-price flag: %w", err)
}

The error is checked but the value is never validated. If user provides -1 or invalid values, they're passed through. The server hardcodes -1 (line 260) which is documented as "auto", but this isn't validated in the CLI.

Recommendation: Add validation:

if gasPrice != -1 && gasPrice < 0 {
    return fmt.Errorf("gas-price must be positive or -1 for auto")
}

Error Handling & Resilience

✅ Strengths

1. Comprehensive error handling: All DA submission failure cases are handled with appropriate status codes (apps/evm/cmd/post_tx_cmd.go:148-190)

2. Graceful degradation: Invalid transactions are filtered with logging rather than failing entire batches

3. Context cancellation: Proper context handling in most places with timeout on DA submission (force_inclusion.go:255)

⚠️ Issues & Recommendations

5. Silent HTTP Write Failures (apps/evm/server/force_inclusion.go:226-228, 347-349)

if _, err := w.Write(respBody); err != nil {
    s.logger.Error().Err(err).Msg("failed to write proxy response")
}

Errors are logged but not returned to the caller. While this is acceptable for HTTP handlers, consider whether this could lead to partial responses.

6. No Retry Logic in CLI (apps/evm/cmd/post_tx_cmd.go:144)

The CLI command doesn't retry on transient failures. The underlying DAClient.Submit may have retry logic, but exposing it to CLI users would improve UX.

Security

✅ Strengths

1. No credential exposure: Auth tokens are passed securely through config
2. Input validation: Hex decoding validates transaction format
3. Reserved key protection: Test app properly protects reserved keys from transaction modification (kvexecutor.go:268-272)

⚠️ Issues & Recommendations

7. No Rate Limiting (apps/evm/server/force_inclusion.go)

The force inclusion HTTP server has no rate limiting. Since the node operator pays for DA submission costs (as documented in README.md:111), this could be exploited for DoS or cost amplification attacks.

Recommendation: Add rate limiting middleware:

• Per-IP rate limits
• Per-key rate limits based on transaction sender
• Maximum concurrent requests
• Cost-based throttling

Severity: HIGH - Could lead to significant financial impact for node operators

8. No Authentication/Authorization (apps/evm/server/force_inclusion.go)

The server accepts transactions from anyone. Combined with lack of rate limiting, this is a serious concern.

Recommendation: Consider adding:

• API key authentication
• Allowlist of authorized addresses
• At minimum, document this security consideration prominently

Severity: HIGH - Financial and operational security risk

9. CORS Not Configured (apps/evm/server/force_inclusion.go:74-82)

HTTP server doesn't configure CORS. This could be intentional but should be documented.

Performance & Resource Efficiency

✅ Strengths

1. Efficient channel draining: GetTxs uses non-blocking select (kvexecutor.go:203-219)
2. Buffered channels: Transaction channel has 10000 buffer (kvexecutor.go:22)
3. Proper connection pooling: HTTP client reuse (force_inclusion.go:69-71)
4. Timeouts configured: Read/write timeouts on HTTP server (force_inclusion.go:26-27, 80-81)

⚠️ Issues & Recommendations

10. Unbounded Request Body (apps/evm/server/force_inclusion.go:152-156)

body, err := io.ReadAll(r.Body)

No size limit on request body. Could lead to memory exhaustion.

Recommendation: Add size limit:

r.Body = http.MaxBytesReader(w, r.Body, 1024*1024) // 1MB limit

11. No Connection Limits (apps/evm/server/force_inclusion.go:77-82)

HTTP server doesn't configure MaxHeaderBytes or connection limits.

Recommendation:

s.server = &http.Server{
    Addr:           addr,
    Handler:        mux,
    ReadTimeout:    defaultReadTimeout,
    WriteTimeout:   defaultWriteTimeout,
    MaxHeaderBytes: 1 << 20, // 1MB
}

12. Synchronous DA Submission (apps/evm/server/force_inclusion.go:262)

DA submission blocks the HTTP handler. For high throughput, consider async submission with ticket/receipt system.

Testing & Reliability

✅ Strengths

1. Comprehensive test coverage: 480 lines of tests covering happy paths, error cases, proxying, etc.
2. Mock implementations: Clean mock DA interface
3. Test isolation: Uses httptest for server testing
4. Edge case coverage: Tests invalid params, DA errors, JSON parsing errors
5. Idempotency tests: Genesis initialization tested for idempotency (kvexecutor_test.go:12-42)

⚠️ Issues & Recommendations

13. Missing Integration Tests

No end-to-end tests that verify:

• CLI → DA → Sequencer → Inclusion flow
• Force inclusion timing/epoch boundaries
• Multiple concurrent submissions

Recommendation: Add integration test in test/integration/ directory

14. Codecov Warning

Codecov shows 0% coverage for pkg/config/config.go changes (2 lines missing). Review if these lines need test coverage.

Documentation & Developer Experience

✅ Strengths

1. Excellent README: Comprehensive documentation with examples (apps/evm/README.md:107-172)
2. Clear CLI help text: Good examples in command description
3. Helpful logging: Informative log messages with structured fields
4. Code comments: Key functions are well-documented

⚠️ Issues & Recommendations

15. Missing Security Warnings in README

The README mentions that "the node operator will be paying for the gas costs" (line 111) but doesn't warn about:

• Lack of rate limiting
• Lack of authentication
• Potential DoS/cost amplification attacks

Recommendation: Add a Security Considerations section:

### Security Considerations

⚠️ **Important**: The Force Inclusion API has no built-in authentication or rate limiting. 
Anyone with access to this endpoint can submit transactions at the node operator's expense.

Recommended mitigations:
- Use firewall rules to restrict access
- Deploy behind a reverse proxy with rate limiting
- Monitor DA submission costs
- Consider implementing API key authentication

16. Unclear Fake Transaction Hash Behavior

README doesn't explain that the returned "transaction hash" is actually the DA height zero-padded (force_inclusion.go:299).

---

Priority Issues Summary

🔴 High Priority (Security & Correctness)

1. No rate limiting - Could lead to financial loss for node operators
2. No authentication - Allows unauthorized DA submissions
3. Unbounded request body - Memory exhaustion risk
4. Missing security documentation - Operators unaware of risks

🟡 Medium Priority (Robustness)

5. Missing context timeout in CLI
6. No validation of gas price flag
7. Missing integration tests
8. Connection limits not configured

🟢 Low Priority (Polish)

9. Typo in documentation
10. Fake transaction hash not documented
11. Consider async DA submission for performance

---

Verdict

The implementation is well-structured and follows Go best practices, with excellent test coverage for unit-level functionality. However, there are serious security concerns that must be addressed before production use:

Required Before Merge

1. ✅ Add rate limiting to the force inclusion server
2. ✅ Add authentication/authorization OR document security risks prominently
3. ✅ Add request body size limits
4. ✅ Update README with security warnings

Recommended

• Add integration tests
• Add CLI timeout
• Document fake transaction hash behavior
• Fix typo

---

[codecov]

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.63%. Comparing base (230263d) to head (fe51201).

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

Additional details and impacted files

@@              Coverage Diff              @@
##           julien/fi    #2888      +/-   ##
=============================================
- Coverage      65.64%   65.63%   -0.01%     
=============================================
  Files             85       85              
  Lines           7822     7823       +1     
=============================================
  Hits            5135     5135              
- Misses          2121     2122       +1     
  Partials         566      566              

Flag	Coverage Δ
combined	65.63% <0.00%> (-0.01%)	⬇️

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.