Update EIP-7892: harden the spec with p2p details. by raulk · Pull Request #9840 · ethereum/EIPs

raulk · 2025-05-30T11:00:22Z

Supersedes #9818.

Based on conclusions captured in this comment, confirmed in ACDC #158.

To reconcile BPO forks at the p2p level, we make changes to:

compute_fork_digest, to apply an XOR mask mixing in the currently-active MAX_BLOBS_PER_BLOCK.
The ENR record to preserve its ability to accurately convey the next fork.

Furthermore, we require that all future blob parameter changes (even those happening at regular forks) be applied through the blob schedule. This is necessary to guarantee consistency and correct calculation of the updated ForkDigest in all circumstances.

eth-bot · 2025-05-30T11:00:47Z

✅ All reviewers have approved.

barnabasbusa · 2025-05-30T12:50:52Z

EIPS/eip-7892.md

    "baseFeeUpdateFraction": 5007716
  },
-  "1740693335": {
+  "12000000": {


This formatting is not what we have agreed on.

It would be something more like this:

"blobSchedule": { "cancun": { "target": 3, "max": 6, "baseFeeUpdateFraction": 3338477 }, "prague": { "target": 6, "max": 9, "baseFeeUpdateFraction": 5007716 }, "osaka": { "target": 6, "max": 9, "baseFeeUpdateFraction": 5007716 }, "bpo1": { "target": 9, "max": 12, "baseFeeUpdateFraction": 5007716 }, "bpo2": { "target": 4, "max": 6, "baseFeeUpdateFraction": 5007716 } ....

as well as

"pragueTime": 0, "osakaTime": 1748609665, "bpo1Time": 1748609665, "bpo2Time": 1748610049, ...

Hm, I was going by @jtraglia's comment that these weren't timestamps.

I do recall seeing this format discussed somewhere, but I can't find it. Could you link to it, @barnabasbusa?

https://discord.com/channels/595666850260713488/745077610685661265/1372894564087758949

Thanks, @barnabasbusa. I'll assume there's good consensus on that format, so we can take the opportunity to integrate it in the EIP here.

@barnabasbusa can you pls check I got the spec right?

I still don’t see bpo1 and bpo1Time fields

GitHub UI glitch? They were introduced in this commit: 95d580a (#9840).

Edit: up to debate. Follow here: https://discord.com/channels/595666850260713488/1372894564087758949

Regarding this section:

"prague": { "target": 6, "max": 9, "baseFeeUpdateFraction": 5007716 }, "osaka": { "target": 6, "max": 9, "baseFeeUpdateFraction": 5007716 },

We do not need to define osaka here if it's the same. Only when there's a change.

EIPS/eip-7892.md

jtraglia · 2025-05-30T13:48:51Z

EIPS/eip-7892.md

+
+class BlobScheduleEntry(NamedTuple):
+   epoch: Epoch
+   max_blobs_per_block: int       # uint32


Currently, max_blobs_per_block is a uint64 in the specs though it could be a uint16 if we wanted, since MAX_BLOB_COMMITMENTS_PER_BLOCK is 4,096 and unlikely to ever be greater than 65,535.

I'd prefer to align the type here with the existing specs (uint64). However, that makes applying the bitmask unsafe, so to bring safety back we'd need to assert it can be coerced into an uint32. Are we good with that?

Alternatively we could hash the value and bitmask a slice of it, but IMO not worth it.

Yeah this is tricky. I suppose an extra assert would be fine.

jtraglia · 2025-05-30T13:54:49Z

EIPS/eip-7892.md

+    blob_params = next(
+      (entry for entry in sorted_schedule if current_epoch >= entry.epoch),
+      None
+    )


Using next is pythonic, which I like, but in the specs we try not to use language specific features like this. It makes implementing in other languages a little more difficult/confusing. I would recommend using a more traditional method for this, eg a for-loop.

Makes sense. Do we have these conventions / standards written up somewhere, by the way?

Nah, to my knowledge this isn't written down anywhere.

EIPS/eip-7892.md

jtraglia · 2025-05-30T16:10:13Z

EIPS/eip-7892.md

+```python
+class BlobScheduleEntry(NamedTuple):
+   epoch: Epoch
+   max_blobs_per_block: int       # uint64, aligning with the type of MAX_BLOBS_PER_BLOCK.


Let's just use uint64 in the type hint.

Suggested change

max_blobs_per_block: int # uint64, aligning with the type of MAX_BLOBS_PER_BLOCK.

max_blobs_per_block: uint64

Done. Clarifying that we can use this type because we assume https://pypi.org/project/remerkleable/ as a prelude.

Warioadem5@gmail.com

jtraglia

LGTM 👍 thanks @raulk!

terencechain · 2025-05-30T20:56:04Z

EIPS/eip-7892.md

+    # This check enables us to roll out the BPO mechanism without a concurrent parameter change.
+    if blob_params is None:
+      return ForkDigest(base_digest)
+
+    # Safely bitmask blob parameters into the digest.
+    assert 0 <= blob_params.max_blobs_per_block <= 0xFFFFFFFF
+    mask = blob_params.max_blobs_per_block.to_bytes(4, 'big')
+    masked_digest = bytes(a ^ b for a, b in zip(base_digest, mask))


To clarify, this wouldn’t have worked if we had tried to backport Electra and Deneb into the blob schedule?
The network could easily split if a node rolled this out while others didn’t. That’s why we did this pr

That PR probably removed the backport requirement to help alleviate implementation effort for some clients (maybe for the ones whose fork handling logic made the backport painful). But to your point: backporting past forks here would be problematic while those forks were still active. Once they become historical, it doesn't matter any longer. If that's a concern (eg if we want to have backport optionality), we can gate the masking logic behind an epoch gate.

(And yeah, bugs in fork_digest can cause network partitions.)

thanks for the reply, I thought I had found a bug here then realized the electra was removed from bpo schedule:)
I'll remember to remove them for prysm

ethDreamer · 2025-06-02T15:43:07Z

Looks great! I had only a couple of thoughts on the current construction:

If we only mix in the max blobs (because that's all the consensus layer can access), then a BPO fork that only changes the target would not change the fork digest. This is probably fine as such a fork could be implemented entirely on the execution layer.
If we should end up reverting to a lower max blob count after raising it previously, this construction would cause the fork digest to match an earlier fork. I would think this is also probably fine as only a minority of nodes don't update for new forks, and only a minority of those would not update and hang around long enough to also not update on the next fork. And this scenario is a rare case to begin with so I'm not that worried about it.

barnabasbusa · 2025-06-02T17:27:33Z

BPOs will change the fork digest on the EL side.

On the EL side each BPO will be treated as a separate fork, so even if BPO3 == BPO5 they would have different fork ids

raulk · 2025-06-02T18:03:06Z

@ethDreamer Re (2): Good flag. I don't see any major downsides in that edge case. Those nodes are technically running software that's again compatible with the current consensus rules. So the payloads routed via the "recycled" topics would be valid according to the topic validation rules. They just wouldn't be able to sync the interim portion of the chain (assuming blocks with higher blob counts were effectively included).

Re (1): Looks like @barnabasbusa already covered it.

ethDreamer

Amazing work @raulk! Thank you so much for your hard work on this!

eth-bot

All Reviewers Have Approved; Performing Automatic Merge...

Closes #7467. This PR primarily addresses [the P2P changes](ethereum/EIPs#9840) in [fusaka-devnet-2](https://fusaka-devnet-2.ethpandaops.io/). Specifically: * [the new `nfd` parameter added to the `ENR`](ethereum/EIPs#9840) * [the modified `compute_fork_digest()` changes for every BPO fork](ethereum/EIPs#9840) 90% of this PR was absolutely hacked together as fast as possible during the Berlinterop as fast as I could while running between Glamsterdam debates. Luckily, it seems to work. But I was unable to be as careful in avoiding bugs as I usually am. I've cleaned up the things *I remember* wanting to come back and have a closer look at. But still working on this. Progress: * [x] get it working on `fusaka-devnet-2` * [ ] [*optional* disconnect from peers with incorrect `nfd` at the fork boundary](ethereum/consensus-specs#4407) - Can be addressed in a future PR if necessary * [x] first pass clean-up * [x] fix up all the broken tests * [x] final self-review * [x] more thorough review from people more familiar with affected code

Closes sigp#7467. This PR primarily addresses [the P2P changes](ethereum/EIPs#9840) in [fusaka-devnet-2](https://fusaka-devnet-2.ethpandaops.io/). Specifically: * [the new `nfd` parameter added to the `ENR`](ethereum/EIPs#9840) * [the modified `compute_fork_digest()` changes for every BPO fork](ethereum/EIPs#9840) 90% of this PR was absolutely hacked together as fast as possible during the Berlinterop as fast as I could while running between Glamsterdam debates. Luckily, it seems to work. But I was unable to be as careful in avoiding bugs as I usually am. I've cleaned up the things *I remember* wanting to come back and have a closer look at. But still working on this. Progress: * [x] get it working on `fusaka-devnet-2` * [ ] [*optional* disconnect from peers with incorrect `nfd` at the fork boundary](ethereum/consensus-specs#4407) - Can be addressed in a future PR if necessary * [x] first pass clean-up * [x] fix up all the broken tests * [x] final self-review * [x] more thorough review from people more familiar with affected code

update EIP-7892: harden the spec with p2p details.

b3e97b7

raulk requested a review from eth-bot as a code owner May 30, 2025 11:00

github-actions bot added c-update Modifies an existing proposal s-draft This EIP is a Draft t-informational labels May 30, 2025

raulk mentioned this pull request May 30, 2025

Update EIP-7892: harden BPO forks spec. #9818

Closed

eth-bot added the a-review Waiting on author to review label May 30, 2025

eth-bot changed the title ~~update EIP-7892: harden the spec with p2p details.~~ Update EIP-7892: harden the spec with p2p details. May 30, 2025

barnabasbusa reviewed May 30, 2025

View reviewed changes

jtraglia reviewed May 30, 2025

View reviewed changes

EIPS/eip-7892.md Outdated Show resolved Hide resolved

jtraglia reviewed May 30, 2025

View reviewed changes

EIPS/eip-7892.md Show resolved Hide resolved

jtraglia reviewed May 30, 2025

View reviewed changes

EIPS/eip-7892.md Show resolved Hide resolved

address review comments.

f969a47

jtraglia reviewed May 30, 2025

View reviewed changes

more editorial changes + Fulu blob schedule entry.

95d580a

jtraglia approved these changes May 30, 2025

View reviewed changes

terencechain reviewed May 30, 2025

View reviewed changes

ethDreamer approved these changes Jun 3, 2025

View reviewed changes

eth-bot enabled auto-merge (squash) June 3, 2025 12:23

eth-bot approved these changes Jun 3, 2025

View reviewed changes

eth-bot merged commit f37240a into ethereum:master Jun 3, 2025
22 checks passed

Gabriel-Trintinalia mentioned this pull request Jun 7, 2025

EIP-7892: add max blobs per block to fork id hyperledger/besu#8764

Closed

ethDreamer mentioned this pull request Jun 26, 2025

Final changes for fusaka-devnet-2 sigp/lighthouse#7655

Merged

6 tasks

	max_blobs_per_block: int # uint64, aligning with the type of MAX_BLOBS_PER_BLOCK.
	max_blobs_per_block: uint64

Comments

Conversation

raulk commented May 30, 2025

Uh oh!

eth-bot commented May 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

barnabasbusa May 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jtraglia Jun 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jtraglia left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

raulk May 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ethDreamer commented Jun 2, 2025

Uh oh!

barnabasbusa commented Jun 2, 2025

Uh oh!

raulk commented Jun 2, 2025

Uh oh!

ethDreamer left a comment

Choose a reason for hiding this comment

Uh oh!

eth-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

eth-bot commented May 30, 2025 •

edited

Loading

barnabasbusa May 30, 2025 •

edited

Loading

jtraglia Jun 2, 2025 •

edited

Loading

raulk May 30, 2025 •

edited

Loading