Add helpers methods for simple config derivations #4814
Conversation
| ssz_objects = spec_object.ssz_objects | ||
| for name, value in spec_object.custom_types.items(): | ||
| if any(k in name for k in all_config_dependencies): | ||
| if any(name in k for k in all_config_dependencies): |
There was a problem hiding this comment.
Note, this was a bug in the spec generator & this change fixes it.
See: #4799 (comment)
jtraglia
left a comment
jtraglia
left a comment
There was a problem hiding this comment.
LGTM, I think this would be a nice simplification.
I'll need to see support from others before merging though.
|
IIUC, this PR aims to replace composite configs with helpers. Do we also want to replace all such configs? For instance, this one remains. Other than that, the change itself looks good to me, but as a minor request, I would like to learn what client devs think about this refactor. |
Ah good catch, I missed this. Yes, we should replace this one too.
Yes, me too. We shouldn't merge this without their approvals. |
nflaig
left a comment
nflaig
left a comment
There was a problem hiding this comment.
seems like a good change, in general it doesn't make much sense to have values in config that are supposed to be derived from other values in config
5 participants
Description
In the spec there are several config values that are simple calculations of other config values. An example is
ATTESTATION_SUBNET_PREFIX_BITSwhich is equal toceillog2(ATTESTATION_SUBNET_COUNT) + ATTESTATION_SUBNET_EXTRA_BITS. HavingATTESTATION_SUBNET_PREFIX_BITSbe its own config causes ambiguity in the behavior of clients if the prefix bits does not equal the formula.For more details: #4799
As a result this PR converts
ATTESTATION_SUBNET_PREFIX_BITS,MAX_REQUEST_BLOB_SIDECARS_ELECTRA,MAX_REQUEST_DATA_COLUMN_SIDECARS, andMAX_REQUEST_BLOB_SIDECARSto helper methods likecompute_max_request_blob_sidecarsetc.Testing
ran
make test fork=fulumake test fork=electra,make test fork=deneb, andmake test fork=phase0