-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add EIP-7916 ProgressiveList specs and tests #4445
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
base: master
Are you sure you want to change the base?
Conversation
EIP-7916 defines a new SSZ type for lists of unknown length. Historically, we used List[type, N] with very large N values for this: - This wastes hashes, as log(N) hashes are still required even if only a single item is present. This is especially bad when nesting lists. - Generalized indicse break, as is what happened when the Attestation count got reduced in Electra, introducing maintenance burden to apps that possibly involves security countil for upgrading smart contracts. This PR adds specs and tests for EIP-7916 which fixes these issues, making the new list type available to EIP implementations in _features. - https://eips.ethereum.org/EIPS/eip-7916
Implement test runner for new consensus-spec test format for EIP-7916 ProgressiveList, so that we will immediately pick up the new tests when they become available. - ethereum/consensus-specs#4445
|
Hey @etan-status, I like the idea, but I believe there must be sufficient support from the community (core devs) before this can be merged. If there isn't sufficient support, we could try to make a |
Implement test runner for new consensus-spec test format for EIP-7916 ProgressiveList, so that we will immediately pick up the new tests when they become available. - ethereum/consensus-specs#4445
Please, let me know if this change to the specs is accepted for inclusion. Then, I can review the tests and check if I can add more tests to it :-) |
Address feedback from consensus-specs: - ethereum/consensus-specs#4445 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @etan-status!
We will wait until July 31st to merge this, to give others time to review.
cc @leolara
Address feedback from consensus-specs: - ethereum/consensus-specs#4445 (comment)
EIP-7916 defines a new SSZ type for lists of unknown length.
Historically, we used List[type, N] with very large N values for this:
This wastes hashes, as log(N) hashes are still required even if only a single item is present. This is especially bad when nesting lists.
Generalized indices break, as is what happened when the Attestation count got reduced in Electra, introducing maintenance burden to apps that possibly involves security countil for upgrading smart contracts.
This PR adds specs and tests for EIP-7916 which fixes these issues, making the new list type available to EIP implementations in _features.