Remove MAX_REQUEST_* config variables#4322
Conversation
|
These kinds of things are a bit of a compatibility pain, if it's just a clean up I'd probably prefer leaving them given they're on active forks... |
|
If we merge this, I'd set this as a target for fusaka-devnet-1, and not fusaka-devnet-0. |
maybe since those are networking and not consensus critical constants we can get away with it? I just checked lodestar vc and it does not check those on startup as they are considered non-critical |
dapplion
left a comment
dapplion
left a comment
There was a problem hiding this comment.
+1 to remove completely. Issues from this removal should be start-up only so easy to guard against in clients
im less concerned about clients we're yet to release, and more concerned about nodes that are already out in the wild and downloading config from new nodes that are missing required fields. Compatibility issues like this drive issues in discord and we're already trying to do a ton of things at the same time. |
Clients can choose to leave them frozen with latest speced values. But we remove from the specs for clarity. Do some clients use the consensus-specs file as is without modification? |
yes, where ever possible we attempt to use the actual files. |
haxxpop
left a comment
haxxpop
left a comment
There was a problem hiding this comment.
It seems to me that the config vars won't be anything else other than MAX_REQUEST_BLOCKS_DENEB * MAX_BLOBS_PER_BLOCK and
MAX_REQUEST_BLOCKS_DENEB * NUMBER_OF_COLUMNS, so it makes sense to remove them.
|
This doesn't have enough support right now. Going to close this PR. Maybe we can do this in the future. |
7 participants
This PR deletes the following config variables:
MAX_REQUEST_BLOB_SIDECARSMAX_REQUEST_BLOB_SIDECARS_ELECTRAMAX_REQUEST_DATA_COLUMN_SIDECARSIt doesn't make sense to have this, as they can be computed using other config variables.