-
Notifications
You must be signed in to change notification settings - Fork 973
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
EIP4844: Enable withdrawal #3181
Conversation
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!
I'm cool leaving this as a placeholder but there is quite a bit of testing code to re-enable before this gets merged. And for the record, I too would like to get this back into it's final form as soon as is feasible |
I can update the test suite quickly. Just please ensure it is what EIP-4844 interop wants and when. :) |
8d48b0f
to
da79f3f
Compare
da79f3f
to
dba75ee
Compare
Update: per the EIP-4844 call decision (ethereum/pm#701), let's aim to include it in the next release! I pushed commits to update the tests. Now running the testgen. I will review it again tomorrow. |
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.
Test vectors look good to me. Really glad that we can finally remove the no-op 🥳
p.s. we are testing and fixing the new GitHub Actions CI integration. Please ignore the weird GitHub Actions lint error atm.
CircleCI still covers all tests.
Thanks so much @hwwhww ! |
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.
looks good!
Given EIP4844 is decoupled from Shanghai and targeted for Cancun, and with the withdrawal spec almost finalized, it'd make sense to enable withdrawal for the subsequent 4844 devnets. This is more realistic to what the final product would look like, and it's cleaner for the client implementations, the less "custom" code, the better.
No rush to merge this. I think we can target this for devnet4.