client: Implement withdrawals via engine api #2401
Conversation
g11tech
added
PR state: WIP
package: client
target: master (v7)
Codecov Report
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more. |
g11tech
changed the title
|
Uh, this spawns a lot of libraries. Interesting. 🤔 🙂 |
g11tech
force-pushed
the
g11tech/withdrawals-api
branch
from
4685445
to
199c5c5
Compare
g11tech
added
package: block
package: blockchain
package: vm
package: common
package: util
PR state: needs review
and removed
PR state: WIP
labels
g11tech
force-pushed
the
g11tech/withdrawals-api
branch
from
e49ac66
to
2b9cb54
Compare
|
seems like need to add some more coverage, but should be good for some review feedback! |
|
|
||
| async getPayloadV2(params: [string]) { | ||
| return this.getPayload(params) | ||
| } | ||
| /** | ||
| * Compare transition configuration parameters. | ||
| * |
There was a problem hiding this comment.
Comment for the whole file:
can't completely oversee for now so just to make sure: are these error cases for newPayloadV2 and fcuV2 from here https://github.com/ethereum/execution-apis/pull/195/files#diff-571851156237b81c20006ce9d7a70b3421f19540cead91f45955316c52fe94c0R367 (e.g.) integrated?
So the checks on the withdrawal functionality being activated or not together with the INVALID responses?
There was a problem hiding this comment.
would like to do a followup PR on the same (and merge this first) if thats ok? 🙂
g11tech
force-pushed
the
g11tech/withdrawals-api
branch
3 times, most recently
from
88fe0e5
to
6f8dd71
Compare
|
@g11tech you'll let us know when this will be ready for a final review, right? |
yes, just need to add the vm test 🙂 |
g11tech
force-pushed
the
g11tech/withdrawals-api
branch
from
d8e1d00
to
81d9187
Compare
|
@holgerd77 this should be merge ready |
There was a problem hiding this comment.
Ok, I am taking a bit of a risk here and not doing an as-in-depth-review-as-might-desired, since I am still so side tracked with all this roadmap (and Hack Exchange) stuff, but Gajinder needs this for Shandong, so it's good if we get this over the finish line.
Had some final look, I guess there are no security sensitive parts (no tx changes e.g., but also nothing apparent in Block) touched here, and the lower-level API changes and additions - in particular in Block and Util and Common - seems solid.
So I will approve here, thanks a lot Gajinder, great great work, really wonderful to see this getting into the main code base. 🥳
Implement :ethereum/execution-apis#195
i.e produce/process/respond withdrawals enabled blocks
Followup of