copy/pasta more of the Withdrawals PR #65
Conversation
roberto-bayardo
force-pushed
the
roberto-dev
branch
5 times, most recently
from
c0af963
to
2c645c6
Compare
roberto-bayardo
force-pushed
the
roberto-dev
branch
from
2c645c6
to
b75e1a6
Compare
|
@Inphi had some issues with tests failing but looks all good now @realbigsean after this is merged I'll add in the updates from ethereum/execution-apis#322 |
|
Are these changes solely to get the execution API v2 working? Is there anything else, like withdrawals functionality, that I should watch out for in my review? |
|
Yes getting the API v2 implemented is the main goal but I also want to minimize merge pain with upstream once Withdrawals PR is merged. There's nothing significant in here that isn't already being reviewed in the Withdrawals PR above so perhaps no need to review those changes so thoroughly, I'll keep up with any updates over there. |
| func (api *ConsensusAPI) ForkchoiceUpdatedV1(update beacon.ForkchoiceStateV1, payloadAttributes *beacon.PayloadAttributesV1) (beacon.ForkChoiceResponse, error) { | ||
| func (api *ConsensusAPI) ForkchoiceUpdatedV1(update beacon.ForkchoiceStateV1, payloadAttributes *beacon.PayloadAttributes) (beacon.ForkChoiceResponse, error) { | ||
| if payloadAttributes != nil && payloadAttributes.Withdrawals != nil { | ||
| return beacon.STATUS_INVALID, fmt.Errorf("withdrawals not supported in V1") |
There was a problem hiding this comment.
nit: blob transactions aren't supported in V1 either. Let's also add a check for excessDataGas here to be defensive.
There was a problem hiding this comment.
Hmm, doesn't seem like we have (easy) access to that field here?
There was a problem hiding this comment.
ah you're right. I misread that as the execution payload. This is fine as-is.
pulling in more of ethereum#25838 related to the execution api