-
Notifications
You must be signed in to change notification settings - Fork 353
Remove Deneb/Electra from blob schedule and default to Electra limit #9516
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
Remove Deneb/Electra from blob schedule and default to Electra limit #9516
Conversation
| .orElseGet(() -> blobSchedule.getLast().maxBlobsPerBlock()); | ||
| .orElseGet(specConfigFulu::getMaxBlobsPerBlock); |
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.
not sure this is quite right, but then the previous isnt really either given they can go down... we can come back to this if its not causing issues, but i think we'd need to get the list and sort and find max blobs per block from schedule if we do need that value...
| // test deneb max blobs boundary | ||
| assertThat(miscHelpersFulu.getMaxBlobsPerBlock(UInt64.valueOf(269568))).isEqualTo(6); | ||
| assertThat(miscHelpersFulu.getMaxBlobsPerBlock(UInt64.valueOf(269569))).isEqualTo(6); | ||
| assertThat(miscHelpersFulu.getMaxBlobsPerBlock(UInt64.valueOf(269568))) | ||
| .isEqualTo(maxBlobsPerBlockElectra); | ||
| assertThat(miscHelpersFulu.getMaxBlobsPerBlock(UInt64.valueOf(269569))) | ||
| .isEqualTo(maxBlobsPerBlockElectra); | ||
| // last epoch of deneb | ||
| assertThat(miscHelpersFulu.getMaxBlobsPerBlock(UInt64.valueOf(364031))).isEqualTo(6); | ||
| assertThat(miscHelpersFulu.getMaxBlobsPerBlock(UInt64.valueOf(364031))) | ||
| .isEqualTo(maxBlobsPerBlockElectra); | ||
| // electra boundary | ||
| assertThat(miscHelpersFulu.getMaxBlobsPerBlock(UInt64.valueOf(364032))).isEqualTo(9); | ||
| assertThat(miscHelpersFulu.getMaxBlobsPerBlock(UInt64.valueOf(364032))) | ||
| .isEqualTo(maxBlobsPerBlockElectra); |
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.
most of this test is meaningless now, we can just simplify
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.
just removed it, we can add it again when we have an actual bpo schedule
rolfyone
left a 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.
some minor comments...
due to the old release having the list not being empty as a requirement, this does need to be listed as a breaking change again for changelog.
Added to changelog |
PR Description
As per ethereum/consensus-specs#4341 and ethereum/consensus-specs#4342
Fixed Issue(s)
N/A
Documentation
doc-change-requiredlabel to this PR if updates are required.Changelog