Skip to content
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.

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

Delete ineffectual RPC limits SIGNED_BEACON_BLOCK_BELLATRIX_MAX #6790

Closed
michaelsproul opened this issue Jan 10, 2025 · 2 comments
Closed

Delete ineffectual RPC limits SIGNED_BEACON_BLOCK_BELLATRIX_MAX #6790

michaelsproul opened this issue Jan 10, 2025 · 2 comments

Comments

@michaelsproul
Copy link
Member

Description

We have a problem where every time we add a new fork we copy-pasta some stuff in this section of the code:

/// The `BeaconBlockBellatrix` block has an `ExecutionPayload` field which has a max size ~16 GiB for future proofing.
/// We calculate the value from its fields instead of constructing the block and checking the length.
/// Note: This is only the theoretical upper bound. We further bound the max size we receive over the network
/// with `max_chunk_size`.
pub static SIGNED_BEACON_BLOCK_BELLATRIX_MAX: LazyLock<usize> =
LazyLock::new(|| // Size of a full altair block
*SIGNED_BEACON_BLOCK_ALTAIR_MAX
+ types::ExecutionPayload::<MainnetEthSpec>::max_execution_payload_bellatrix_size() // adding max size of execution payload (~16gb)
+ ssz::BYTES_PER_LENGTH_OFFSET); // Adding the additional ssz offset for the `ExecutionPayload` field
pub static SIGNED_BEACON_BLOCK_CAPELLA_MAX: LazyLock<usize> = LazyLock::new(|| {
*SIGNED_BEACON_BLOCK_CAPELLA_MAX_WITHOUT_PAYLOAD
+ types::ExecutionPayload::<MainnetEthSpec>::max_execution_payload_capella_size() // adding max size of execution payload (~16gb)
+ ssz::BYTES_PER_LENGTH_OFFSET
}); // Adding the additional ssz offset for the `ExecutionPayload` field
pub static SIGNED_BEACON_BLOCK_DENEB_MAX: LazyLock<usize> = LazyLock::new(|| {
*SIGNED_BEACON_BLOCK_CAPELLA_MAX_WITHOUT_PAYLOAD
+ types::ExecutionPayload::<MainnetEthSpec>::max_execution_payload_deneb_size() // adding max size of execution payload (~16gb)
+ ssz::BYTES_PER_LENGTH_OFFSET // Adding the additional offsets for the `ExecutionPayload`
+ (<types::KzgCommitment as Encode>::ssz_fixed_len() * <MainnetEthSpec>::max_blobs_per_block())
+ ssz::BYTES_PER_LENGTH_OFFSET
}); // Length offset for the blob commitments field.
//
pub static SIGNED_BEACON_BLOCK_ELECTRA_MAX: LazyLock<usize> = LazyLock::new(|| {
*SIGNED_BEACON_BLOCK_ELECTRA_MAX_WITHOUT_PAYLOAD
+ types::ExecutionPayload::<MainnetEthSpec>::max_execution_payload_electra_size() // adding max size of execution payload (~16gb)
+ ssz::BYTES_PER_LENGTH_OFFSET // Adding the additional ssz offset for the `ExecutionPayload` field
+ (<types::KzgCommitment as Encode>::ssz_fixed_len() * <MainnetEthSpec>::max_blobs_per_block())
+ ssz::BYTES_PER_LENGTH_OFFSET
}); // Length offset for the blob commitments field.

I claim that this is completely pointless because these limits are so high anyway. We should do away with the fiction that the fork changes are relevant here and just use some large constant for all forks after Bellatrix.

@pawanjay176
Copy link
Member

I'm in favour. Should have gotten rid of this way back during bellatrix when it started becoming pointless.

@realbigsean realbigsean added das Data Availability Sampling peerdas-devnet-4 labels Jan 12, 2025 — with Linear
@dapplion dapplion removed the das Data Availability Sampling label Jan 13, 2025
@jimmygchen
Copy link
Member

Completed in #6798

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants