-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
op-service: withdrawals typing fixes #7767
Conversation
…k verification, add test vectors and rpc-block tests
Generally looks good to me, needs linting
|
54f8d74
to
2b57c30
Compare
fixed linting issue 👍 |
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.
I really like the script to generate good test data
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.
Co-authored-by: Adrian Sutton <adrian@oplabs.co>
Semgrep found 1
Please create a Linear ticket for this TODO. Ignore this finding from todos_require_linear. |
There is no need for a new
eth.Withdrawal
andeth.Withdrawals
type, since the withdrawal is a small and lightweight struct already; matching what we have in the execution-payload.In the EthClient and its RPC block decoding, we need the withdrawal attributes to be decoded and verified correctly, or completely ignored and non-exposed.
For shanghai support the withdrawals block data was added, but the parsing was broken, causing the node to get stuck on withdrawal data.
Tests
Added test-vectors for pre and post shanghai blocks, and turned the old header tests into the same test vector format.
Additional context
Things got broken in: