Add encoded length methods to transactions#2
Add encoded length methods to transactions#2librelois merged 7 commits intomoonbeam-polkadot-stable2506from
Conversation
librelois
left a comment
There was a problem hiding this comment.
Please add some unit tests, suggested scenarios:
-
encoded_lenmatches actual encoded bytes: forTransactionV0..V3, asserttx.encoded_len() == tx.encode().len(). -
payload_lenmatches payload encoding: asserttx.payload_len() == tx.encode_payload().len()for each tx variant. -
Typed vs legacy size rule: assert typed tx (
2930/1559/7702) hasencoded_len() == payload_len() + 1, and legacy hasencoded_len() == payload_len(). -
Unsigned message length methods: for
Legacy/EIP2930/EIP1559/EIP7702message structs, assertmessage.encoded_len() == rlp::encode(&message).len(). -
Size growth sanity: mutate fields with variable length (large
input/data, longeraccess_list, largerauthorization_list) and assertencoded_lenincreases. -
Boundary-ish DoS shape case: construct a near-limit large tx payload (e.g., ~128KB data field) and verify length methods still exactly match full encoding length.
Addressed in 0d9c2e4 |
There was a problem hiding this comment.
encoded_len()/payload_len() still perform full payload encoding and allocation, so they do not deliver the intended "size check without full encoding" DoS-hardening behavior.
Evidence:
src/enveloped.rs:48 uses self.encode_payload().len() for payload_len().
src/transaction/legacy.rs:288, src/transaction/eip2930.rs:250, src/transaction/eip1559.rs:125, src/transaction/eip7702.rs:289 each compute length via rlp::encode(self).len().
Impact: size validation remains allocation-heavy on untrusted inputs, which weakens the performance/DoS motivation.
Please implement true non-allocating length paths using RlpStream length accounting (begin_unbounded_list + append + finalize_unbounded_list) or explicit per-field RLP length math, and override payload_len() per transaction type.
Addressed in ee70e83 |
librelois
left a comment
There was a problem hiding this comment.
length_of_length is capped at 4, so it undercounts RLP headers when payload length exceeds 0xFFFF_FFFF bytes (4 GiB). That propagates into list/string length math and can make encoded_len/rlp_len incorrect for very large payloads.
File: src/transaction/rlp_len.rs (line 28), src/transaction/rlp_len.rs (line 131)
Suggestions:
- Fix length_of_length to compute bytes needed for arbitrary usize (not capped at 4).
- Add unit tests around boundary values (0xFF, 0xFFFF, 0xFFFFFF, 0xFFFFFFFF, 0x1_0000_0000 on 64-bit) for
rlp_list_header_len/byte-string length helpers.
Replace the hard-coded if/else chain (capped at 4) with a bit-width formula matching alloy-rlp's approach. This ensures rlp_list_header_len, byte-string rlp_len, and all downstream encoded_len calculations are correct for arbitrary usize values. Also mark the function const fn and add boundary tests for length_of_length, rlp_list_header_len, and byte-string rlp_len at 0xFF, 0xFFFF, 0xFFFFFF, 0xFFFFFFFF, and above on 64-bit targets.
Addressed in d813d5d |
Summary
Add
encoded_len()andpayload_len()methods to compute transaction sizes without full encoding. This enables transaction size validation for DoS protection, matching reth/alloy's approach.Motivation
Transaction pools need to enforce size limits for performance reasons and to prevent DoS attacks. Reth enforces a default maximum of 128 KB per transaction and validates transactions.
Alloy provides similar methods on transaction envelopes for this purpose.