Skip to content

Conversation

jwasinger
Copy link
Contributor

@jwasinger jwasinger commented Jun 9, 2025

This PR adds a check against the maximum block size, as required by EIP-7934, and also applies a slightly lower
size limit during block building.

miner/worker.go Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a comment.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, the block body size is also influenced by other things, such as withdrawals. So we need to add a bit of buffer into the size limit here. For the withdrawals, we may be able to add their true size into env.size before adding transactions.

miner/worker.go Outdated
Copy link
Member

Choose a reason for hiding this comment

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

From the CL team:

potuz — 2025/6/10 21:11
This makes the payload invalid
Technically it will make the consensus block that contains that payload invalid.

Probably we need to put a note here, at least adding a warning log that a part of withdrawals specified by the CL are discarded due to the size restriction. In practice, it's impossible to occur.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This happens before we fill the block with txs. Also, max block withdrawal count is capped by the protocol.

spencer-tb pushed a commit to spencer-tb/go-ethereum that referenced this pull request Jun 13, 2025
@fjl fjl changed the title core,miner,params: implement EIP-7934 - RLP Execution Block Size Limit core,miner: implement EIP-7934 - RLP Execution Block Size Limit Jul 8, 2025
@fjl
Copy link
Contributor

fjl commented Jul 8, 2025

I have made two changes:

  • the miner now returns an error when with withdrawals list exceeds the max block size
  • we apply the check unconditionally, i.e. the miner will always adhere to the configured limit

rjl493456442
rjl493456442 previously approved these changes Jul 8, 2025
Copy link
Member

@rjl493456442 rjl493456442 left a comment

Choose a reason for hiding this comment

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

LGTM

rjl493456442
rjl493456442 previously approved these changes Jul 8, 2025
Copy link
Member

@rjl493456442 rjl493456442 left a comment

Choose a reason for hiding this comment

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

LGTM!

// The tx had a sidecar before, so we need to subtract it from the size.
scSize := rlp.ListSize(blobtx.Sidecar.encodedSize())
cpy.size.Store(size - scSize)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I've been verifying this, and this part is correct, but the Size method has some issues related with the new sidecar v2. I was thinking about fixing it in this PR before merging, but will separate this out.

@fjl fjl merged commit e6b9d0c into ethereum:master Jul 9, 2025
4 of 5 checks passed
@fjl fjl added this to the 1.16.2 milestone Jul 9, 2025
howjmay pushed a commit to iotaledger/go-ethereum that referenced this pull request Aug 27, 2025
…reum#31990)

This PR adds a block validation check for the maximum block size, as required by
EIP-7934, and also applies a slightly lower size limit during block building.

---------

Co-authored-by: spencer-tb <[email protected]>
Co-authored-by: Felix Lange <[email protected]>
Co-authored-by: Gary Rong <[email protected]>
gballet pushed a commit to gballet/go-ethereum that referenced this pull request Sep 11, 2025
…reum#31990)

This PR adds a block validation check for the maximum block size, as required by
EIP-7934, and also applies a slightly lower size limit during block building.

---------

Co-authored-by: spencer-tb <[email protected]>
Co-authored-by: Felix Lange <[email protected]>
Co-authored-by: Gary Rong <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants