starts engine_preparePayload and engine_getPayload API call implementation

[tbenr]

changelog:

• introduces Duty preparation
• uses Duty preparation in BlockProductionDuty to handle the call to the execution engine to prepare the block
• starts extending other required classes to be able to call the new method
• currently we are using slot as payloadId

[Nashatyrev]

I would move this field initialization to the constructor and make it final
Does it make sense using random() for generating requestId here? Wouldn't a simple counter be enough?

[tbenr]

the idea was to avoid any potential concurrency issues (can they happen here?) and generate different id in case of duty prepared twice (late consensus block arrival). In the latter I assumed we need to refresh the requestId (going to change it to payloadId as per latest HackMD update).

[Nashatyrev]

Oh, I follow the logic now... prepareDuty() is designed to be called more than once

[ajsutton]

I would suggest using AtomicInteger here even if it's a static field to generate rather than random. There's no reason we need to be using a secure random at this point.

I do wonder if we should be using the same requestId when we resend prepareDuty though - otherwise we're leaving the previous "prepare" call hanging and it will never be cancelled or followed by "get". Might be a question for the API spec as much as anything.

[tbenr]

I moved to AtomicLong and made executionPayloadId constant at every preparation

[Nashatyrev]

BTW, should the preparePayload be called on reorg?
Or just a new Duty recreated on reorg?

[tbenr]

BTW, should the preparePayload be called on reorg?
Or just a new Duty recreated on reorg?

oh, I see onChainReorg event in DutyScheduler. But does it trigger an onAttestationCreationDue for the same slot in any case? Here I need @ajsutton :-)

[ajsutton]

BTW, should the preparePayload be called on reorg?
Or just a new Duty recreated on reorg?

oh, I see onChainReorg event in DutyScheduler. But does it trigger an onAttestationCreationDue for the same slot in any case? Here I need @ajsutton :-)

It's not necessarily a chain reorg that triggers calling prepare again but any head update. The most common case will be that we're proposing at slot X and the block at X-1 is received late. That's a normal head update to fill the empty slot but it still changes the parent root of the block we'll create so need to send a new prepare call.

You should get another call to onAttestationCreationDue in that case - the duplicates are filtered by AbstractDutyScheduler.onProductionDue where it checks lastProductionSlot and we just don't want to do that same kind of check for preparations.

[tbenr]

@ajsutton a unit test is still failing i guess not related to current PR (GetPeersScoreTest)

btw I was trying to focus on preparePayload only but I ended up starting getPayload as well (they are quite related)

[Nashatyrev]

LGTM