Increase network limits #2796
Increase network limits #2796
Conversation
pawanjay176
force-pushed
the
network-limits
branch
from
891896f
to
cea839d
Compare
pawanjay176
added
ready-for-review
| // length of List * size_of(uint8) | ||
| + (T::max_extra_data_bytes() * <u8 as Encode>::ssz_fixed_len()) | ||
| // length of List * offset size * max size of transaction | ||
| + (T::max_transactions_per_payload() *ssz::BYTES_PER_LENGTH_OFFSET * T::max_bytes_per_transaction()) |
There was a problem hiding this comment.
Are you sure this is correct.
Perhaps:
(T::max_transactions_per_payload() * (ssz::BYTES_PER_LENGTH_OFFSET + T::max_bytes_per_transaction()))There was a problem hiding this comment.
Sorry had already fixed this up in b16ca82 but forgot to push the changes.
| // Fixed part | ||
| Self::empty().as_ssz_bytes().len() | ||
| // length of List * size_of(uint8) | ||
| + (T::max_extra_data_bytes() * <u8 as Encode>::ssz_fixed_len()) |
There was a problem hiding this comment.
What is this calculating?
The comment says length of list * size_of(uint8).
It seems the units we are calculating here are bytes. So the multiplication should go like, elements_in_list (dimensionless) * size_in_bytes_of_elements (bytes/element).
The size of a u8 is always going to be 8, so I think its fine to use that constant.
So, for the dimensions to work, the left side has to be dimensionless. max_extra_data_bytes() seems like it is giving a result in bytes already? If that's the case multiplying by 8 will give size in bits right?
There was a problem hiding this comment.
<u8 as Encode>::ssz_fixed_len() gives the size in bytes = 1. So we are doing T::max_extra_data_bytes() * 1 here.
I thought it'd be consistent to not use constants in the calculation here in case we get another variable length value to calculate here later on.
Sorry the comments were bad here. Cleared it up a bit in 7d3aad2 . Let me know if it's okay :)
pawanjay176
force-pushed
the
network-limits
branch
from
29991b7
to
b16ca82
Compare
|
The Perhaps it would be better to extract the bigger merge payload into a separate test? |
pawanjay176
force-pushed
the
network-limits
branch
2 times, most recently
from
c0ead84
to
2a1e383
Compare
|
Tests seem to be failing |
pawanjay176
force-pushed
the
network-limits
branch
from
2a1e383
to
a9739e7
Compare
|
Fixed! |
Fix max packet sizes Fix max_payload_size function Add merge block test Fix max size calculation; fix up test Clear comments Add a payload_size_function Use safe arith for payload calculation Return an error if block too big in block production Separate test to check if block is over limit
Fix max packet sizes Fix max_payload_size function Add merge block test Fix max size calculation; fix up test Clear comments Add a payload_size_function Use safe arith for payload calculation Return an error if block too big in block production Separate test to check if block is over limit
Fix max packet sizes Fix max_payload_size function Add merge block test Fix max size calculation; fix up test Clear comments Add a payload_size_function Use safe arith for payload calculation Return an error if block too big in block production Separate test to check if block is over limit
Issue Addressed
Addresses spec PR ethereum/consensus-specs#2686
Proposed Changes
Increase the max rpc and gossip sizes to 10Mb from 1Mb. Refactor some ssz bound checking code and add tests for rejecting rpc blocks > MAX_RPC_SIZE.