Skip to content

fix: decode payload transactions with EIP-2718 exact format#219

Merged
randygrok merged 1 commit intomainfrom
fix/payload-decoder-eip2718
Apr 13, 2026
Merged

fix: decode payload transactions with EIP-2718 exact format#219
randygrok merged 1 commit intomainfrom
fix/payload-decoder-eip2718

Conversation

@randygrok
Copy link
Copy Markdown
Contributor

@randygrok randygrok commented Apr 13, 2026

Summary

  • Replace TransactionSigned::network_decode with TransactionSigned::decode_2718_exact in the Engine API payload builder path
  • Align builder-time decoding with executor-time validation, which already uses decode_2718_exact when processing ExecutionData

Background

Engine API (engine_newPayload*) specifies transactions as opaque EIP-2718 encoded bytes. The previous network_decode expects the devp2p wire format, which wraps typed transactions in an RLP string header (length prefix + opaque payload). On EIP-2718 bytes, this wrapping is absent — so typed transactions (EIP-1559, EIP-2930, and the EvNode types 0x76 / 0x78) were silently dropped by the filter_map, logging a warning and continuing to build without them.

The executor uses TxTy::<EvPrimitives>::decode_2718_exact when processing the same transactions via ExecutionData (crates/node/src/executor.rs:387). Build-time acceptance must match validation-time acceptance to avoid consensus divergence.

Addresses the CodeRabbit review comment on #207.

Summary by CodeRabbit

  • Refactor
    • Updated transaction decoding mechanism for payload processing. No change to user-facing behavior; transaction validation and error handling remain consistent.

Engine API specifies transactions as opaque EIP-2718 encoded bytes, not
devp2p wire format. Using network_decode here could silently drop typed
transactions (EIP-1559, EIP-2930, EvNode 0x76/0x78) whose bytes are
valid EIP-2718 but lack the wire-format RLP string wrapping.

The executor already uses decode_2718_exact when validating the same
transactions via ExecutionData. This aligns the payload builder with
executor semantics, closing a potential consensus mismatch where
build-time acceptance diverges from validation-time rules.
@randygrok randygrok requested a review from a team as a code owner April 13, 2026 15:03
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 13, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 97a4b522-2328-4027-a615-95f312d8bccb

📥 Commits

Reviewing files that changed from the base of the PR and between fb3fbd0 and 1ef8d42.

📒 Files selected for processing (1)
  • crates/node/src/payload_service.rs

📝 Walkthrough

Walkthrough

A method call in the payload service was updated from TransactionSigned::network_decode to TransactionSigned::decode_2718_exact for decoding raw payload transactions. The error handling and control flow remain unchanged.

Changes

Cohort / File(s) Summary
Transaction Decoding Method Update
crates/node/src/payload_service.rs
Replaced network_decode with decode_2718_exact for transaction decoding in the payload processing pipeline. Success and error handling paths remain identical.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 From network_decode's path so worn,
To 2718_exact, freshly born!
One line changed, control flows stay,
Transactions decode a better way! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description provides a clear summary of changes and background context explaining why the change is necessary, but does not follow the required template structure with Type of Change checkboxes, Related Issues section, Checklist, and Testing section. Add the missing template sections including Type of Change (Bug fix checkbox), Related Issues (reference to issue #207), Checklist items (self-review, testing verification), and Testing section describing validation performed.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely summarizes the main change: replacing network_decode with decode_2718_exact for EIP-2718 format compatibility in payload transaction decoding.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/payload-decoder-eip2718

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@claude
Copy link
Copy Markdown

claude bot commented Apr 13, 2026

Claude finished @randygrok's task —— View job


Pull Request Review

Todo List:

  • Read the modified payload_service.rs file
  • Analyze the executor validation logic mentioned in PR description
  • Check for related test files and coverage
  • Provide comprehensive review feedback

Review Summary

This is a well-executed fix that addresses a critical consensus vulnerability in the Engine API payload builder. The change is minimal, targeted, and aligns with existing validation logic.

Code Quality & Best Practices

Excellent alignment with existing patterns:

  • The fix uses the same decode_2718_exact method already used in the executor (crates/node/src/executor.rs:387)
  • Maintains consistent error handling with proper warning logs
  • Method signature change removes the need for mutable reference (&mut tx_bytes.as_ref()tx_bytes.as_ref())

Critical Bug Fix

Resolves consensus divergence:

  • Before: network_decode expected devp2p wire format (RLP string wrapper + payload), causing typed transactions (EIP-1559, EIP-2930, EvNode 0x76/0x78) to be silently dropped
  • After: decode_2718_exact correctly handles raw EIP-2718 bytes as specified by Engine API
  • Impact: Prevents scenarios where payload builder accepts different transactions than executor validation

Performance Considerations

Positive performance impact:

  • Eliminates unnecessary buffer mutation (&mut no longer needed)
  • decode_2718_exact is likely more efficient as it doesn't expect/parse RLP string wrapping
  • Reduces memory allocations by working directly with byte slices

Security Considerations

Strengthens consensus security:

  • Eliminates silent transaction dropping that could lead to consensus splits
  • Ensures build-time transaction acceptance matches validation-time acceptance
  • No new attack vectors introduced - same error handling and logging maintained

Test Coverage

Existing test provides good coverage:

  • try_build_drops_invalid_raw_transactions() test (lines 546-628) validates that invalid transactions are properly dropped with warning logs
  • This test will now exercise the decode_2718_exact code path instead of network_decode
  • The test uses malformed bytes ([0xde, 0xad, 0xbe, 0xef]) which should fail with both methods, confirming graceful error handling

Recommendation: Consider adding a specific test case with valid EIP-1559/EIP-2930 transactions to verify they are no longer dropped.

Minor Observations

  1. Import consistency: The existing use alloy_eips::Decodable2718; import (line 26) suggests this change was planned
  2. Method parameter cleanup: Removing &mut parameter simplifies the call site
  3. Error path unchanged: Same warning message and graceful handling preserved

Conclusion

This is a high-quality fix that resolves a significant consensus issue with minimal code change. The alignment with executor logic and existing test coverage provide confidence in the change.

Recommendation: ✅ Approve and merge


@randygrok randygrok enabled auto-merge (squash) April 13, 2026 15:06
@randygrok randygrok disabled auto-merge April 13, 2026 15:15
@randygrok randygrok merged commit d0bc639 into main Apr 13, 2026
18 checks passed
@randygrok randygrok deleted the fix/payload-decoder-eip2718 branch April 13, 2026 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants