Skip to content

fix(l1): use 0x80 sentinel for missing eth/71 BAL per EIP-8159 #6744

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
edg-l wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(l1): use 0x80 sentinel for missing eth/71 BAL per EIP-8159 #6744

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 36 additions & 4 deletions crates/networking/p2p/rlpx/eth/block_access_lists.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,30 +16,34 @@ use ethrex_rlp::{
pub const BLOCK_ACCESS_LIST_LIMIT: usize = 1024;

/// Wrapper for optional BAL in eth/71 protocol messages.
/// `None` (BAL unavailable) is encoded as an empty RLP list (0xc0).
///
/// Per EIP-8159 §"BlockAccessLists (0x13)": "The RLP empty string (`0x80`)
/// is returned for blocks where the BAL is unavailable." An empty list
/// (`0xc0`) is a valid BAL encoding (block with no state changes), so the
/// empty string is the only sentinel that can never alias a real BAL.
/// `Some(bal)` is encoded as the BAL's normal RLP list encoding.
#[derive(Debug, Clone)]
struct OptionalBal(Option<BlockAccessList>);

impl RLPEncode for OptionalBal {
fn encode(&self, buf: &mut dyn BufMut) {
match &self.0 {
None => buf.put_u8(0xc0),
None => buf.put_u8(0x80),
Some(bal) => bal.encode(buf),
}
}

fn length(&self) -> usize {
match &self.0 {
None => 1, // empty list = 0xc0
None => 1, // empty string = 0x80 per EIP-8159
Some(bal) => bal.length(),
}
}
}

impl RLPDecode for OptionalBal {
fn decode_unfinished(rlp: &[u8]) -> Result<(Self, &[u8]), RLPDecodeError> {
if rlp.first() == Some(&0xc0) {
if rlp.first() == Some(&0x80) {
Copy link
Copy Markdown
Contributor

@ElFantasma ElFantasma May 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just confirming this is now consistent with the snap/2 codec in #6544 — that PR's Snap2OptionalBal::decode_unfinished does the same rlp.first() == Some(&0x80) check (codec.rs:376 there). I left an inline on the snap/2 side asking for an invariant comment about why 0x80 is unambiguous (BAL is always RLP-encoded as a list, so >= 0xc0). The same comment is worth landing here so a future refactor of BlockAccessList's encoding doesn't silently break this decoder.

One-line suffix on the existing comment block at lines 19–24 is enough — something like "INVARIANT: BlockAccessList always encodes as an RLP list (first byte ≥ 0xc0), so 0x80 is unambiguously the None sentinel." Non-blocking.

return Ok((OptionalBal(None), &rlp[1..]));
}
let (bal, rest) = BlockAccessList::decode_unfinished(rlp)?;
Expand Down Expand Up @@ -139,6 +143,7 @@ impl RLPxMessage for BlockAccessLists {
#[cfg(test)]
mod tests {
use super::*;
use crate::rlpx::utils::snappy_decompress;
use ethereum_types::Address;
use ethrex_common::types::block_access_list::{AccountChanges, BalanceChange};

Expand Down Expand Up @@ -244,4 +249,31 @@ mod tests {
let decoded = BlockAccessLists::decode(&buf).unwrap();
assert_eq!(decoded.block_access_lists.len(), BLOCK_ACCESS_LIST_LIMIT);
}

/// Locks EIP-8159 §"BlockAccessLists (0x13)" sentinel: missing BALs encode
/// as RLP empty string (`0x80`), NOT empty list (`0xc0`). Empty list is a
/// valid BAL encoding (block with no state changes), so the empty string
/// is the only byte that can't alias a real BAL. geth uses the same
/// sentinel (`rlp.EmptyString` in `eth/protocols/eth/handlers.go`); any
/// drift here is silent interop breakage.
#[test]
Copy link
Copy Markdown
Contributor

@iovoid iovoid May 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be on the test/ folder

fn block_access_lists_none_uses_0x80_sentinel() {
let msg = BlockAccessLists::new(0, vec![None]);
let mut buf = Vec::new();
msg.encode(&mut buf).unwrap();
let decompressed = snappy_decompress(&buf).unwrap();
assert!(
decompressed.contains(&0x80),
"decompressed payload must contain the 0x80 None sentinel"
);
assert!(
!decompressed.contains(&0xc0),
Copy link
Copy Markdown
Contributor

@ElFantasma ElFantasma May 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!decompressed.contains(&0xc0) is correct today but happens to hold via RLP-encoding luck. The outer [request_id, [bals]] wrapper is encoded with a 0xc0 + length prefix; for (request_id=0, bals=[None]) the lengths come out to 0xc4 / 0xc1 (not 0xc0). The moment someone changes the test fixture in a way that makes the outer list have exactly zero payload bytes, the prefix becomes 0xc0 and the test fails spuriously even though the sentinel is right.

More robust assertion: target OptionalBal directly rather than the whole message.

let mut bytes = Vec::new();
OptionalBal(None).encode(&mut bytes);
assert_eq!(bytes, vec![0x80]);

Keeps the intent ("None encodes as exactly 0x80") and survives unrelated encoding changes elsewhere.

Non-blocking — the current test is correct, just brittle.

"decompressed payload must not contain the 0xc0 empty-list sentinel"
);

// Roundtrip must preserve the None.
let decoded = BlockAccessLists::decode(&buf).unwrap();
assert_eq!(decoded.block_access_lists.len(), 1);
assert!(decoded.block_access_lists[0].is_none());
}
}
Loading