perf(l1): improve rlp encoding #5350
Open
+445
−153
Conversation
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
Lines of code reportTotal lines added: Detailed view |
Benchmark Block Execution Results Comparison Against Main
|
Benchmark for ad3c222Click to view benchmark
|
Benchmark for 4ad8835Click to view benchmark
|
Benchmark for c43b4ffClick to view benchmark
|
MegaRedHand
reviewed
Nov 14, 2025
Benchmark for 28969a4Click to view benchmark
|
MegaRedHand
reviewed
Nov 19, 2025
Benchmark for 2383de2Click to view benchmark
|
Copilot finished reviewing on behalf of
mpaulucci
November 19, 2025 13:54
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR improves RLP encoding performance by eliminating extra allocations and loops during encoding. The key changes include:
- Refactored integer encoding to use a single-pass loop instead of multiple passes
- Added specialized
length()methods to all RLP-encodable types to avoid re-encoding when calculating lengths - Removed unnecessary
ArrayVecusage in favor of direct byte array manipulation - Introduced helper functions
list_length()andbytes_length()for calculating encoded lengths without allocation
Reviewed Changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/common/rlp/encode.rs | Core RLP encoding improvements: refactored impl_encode, added list_length/bytes_length helpers, implemented specialized length() methods for all primitive types, removed ArrayVec usage |
| crates/common/types/transaction.rs | Added length() implementations for all transaction types and helper tx_type_byte() methods; contains critical bugs with incorrect transaction type bytes |
| crates/common/types/tx_fields.rs | Added length() and redundant encode_to_vec() methods to AuthorizationTuple |
| crates/common/types/receipt.rs | Added length() implementations for Receipt, ReceiptWithBloom, and Log |
| crates/common/types/block.rs | Added length() implementations for Block, BlockHeader, BlockBody, and Withdrawal |
| crates/common/types/account.rs | Added length() implementations for AccountInfo and AccountState |
| crates/common/types/blobs_bundle.rs | Added length() implementation for BlobsBundle |
| crates/common/types/fork_id.rs | Added length() implementation for ForkId |
| crates/common/types/requests.rs | Added length() implementation for EncodedRequests |
| crates/common/trie/rlp.rs | Added length() implementations for trie nodes (BranchNode, ExtensionNode, LeafNode, Node) |
| crates/common/trie/nibbles.rs | Added compact_length() and compact_first_byte() helper methods, and length() implementation for Nibbles |
| tooling/Cargo.lock | Added lz4-sys dependency (transitive from librocksdb-sys) and removed unused hex dependency from ethrex-vm |
| CHANGELOG.md | Documented the RLP encoding performance improvements |
Comments suppressed due to low confidence (1)
crates/common/types/tx_fields.rs:82
- This
encode_to_vecoverride is redundant. The default implementation in theRLPEncodetrait (lines 65-69 inencode.rs) already does exactly this. Consider removing this method unless there's a specific reason for the override (such as pre-allocating buffer capacity as done inBranchNode).
impl RLPDecode for AuthorizationTuple {
fn decode_unfinished(rlp: &[u8]) -> Result<(Self, &[u8]), RLPDecodeError> {
let decoder = Decoder::new(rlp)?;
let (chain_id, decoder) = decoder.decode_field("chain_id")?;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Benchmark for 6d8f5b0Click to view benchmark
|
Benchmark for 38d4e9dClick to view benchmark
|
Oppen
reviewed
Nov 19, 2025
Oppen
reviewed
Nov 19, 2025
Oppen
reviewed
Nov 19, 2025
Oppen
reviewed
Nov 19, 2025
mpaulucci
reviewed
Nov 19, 2025
MegaRedHand
reviewed
Nov 19, 2025
Oppen
reviewed
Nov 19, 2025
Co-authored-by: Tomás Grüner <47506558+MegaRedHand@users.noreply.github.com>
Co-authored-by: Tomás Grüner <47506558+MegaRedHand@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Motivation
Each missing length impl in the trait impl meant an extra allocation, this should remove them entirely when encoding
Helps with #4956