Skip to content

Conversation

djrtwo
Copy link
Contributor

@djrtwo djrtwo commented Oct 18, 2021

Avoid degenerate race conditions via the following:

  • Require caller to utilize strictly increasing JSON RPC message IDs
  • Require callee to respect absolute ordering in execution of calls

Note, there might be optimizations depending on message type (e.g. if multiple forkhcoice calls, only execute the latest) but these are left to implementation details

@djrtwo djrtwo changed the base branch from main to latest-correct-ancestor October 18, 2021 18:39
@djrtwo djrtwo changed the title Forkchoice under asynchrony Engine API: Forkchoice under asynchrony Oct 19, 2021
@mkalinin
Copy link
Contributor

What do you think about enforcing CL to define a logical order of all messages in Engine API. I don't think it increases the complexity comparing to the original proposal but it could be used by EL in syncing process when it received a batch of executePayload and uses only the last one skipping the old ones. Also, EL respecting the order is good in general as it would reduce the surface of potential inconsistency, if there are two executePayload messages received at the same with a parent and a child then they could be processed in correct order if such order can be derived

@lightclient lightclient force-pushed the latest-correct-ancestor branch from 5e5495f to fa01a68 Compare October 28, 2021 13:59
@djrtwo djrtwo closed this Oct 28, 2021
@djrtwo djrtwo reopened this Oct 28, 2021
@djrtwo
Copy link
Contributor Author

djrtwo commented Oct 28, 2021

I'm not 100% sure here. Respecting order is different depending on the operation and the context.

executePayload order should be respected and each executed (!) because otherwise when in lock-step mode you could miss a dependency.

Whereas forkchoice order only requires the latest message to be applied and earlier RPC IDs can be ignored

@mkalinin
Copy link
Contributor

I'm not 100% sure here. Respecting order is different depending on the operation and the context.

Absolutely. What I am thinking about is CL signalling the order of all (not only some) messages to EL, and EL then decides what to do with the order. For getPayload the order might not matter at all while for executePayload and forkchoiceUpdated it matters but has different semantics -- and deciding on that should be up to EL.

@mkalinin
Copy link
Contributor

Another thing that worries me a bit is hijacking on the JSON-RPC request id. It looks fine but having double semantics for this ID might be inconvenient in some cases. But we should see the need of separate ID in practice before introducing it.

Base automatically changed from latest-correct-ancestor to main October 29, 2021 15:33
@djrtwo djrtwo changed the title Engine API: Forkchoice under asynchrony Engine API: Strict message ordering Oct 29, 2021
mkalinin and others added 2 commits October 29, 2021 22:03
Co-authored-by: Mikhail Kalinin <noblesse.knight@gmail.com>
@djrtwo
Copy link
Contributor Author

djrtwo commented Oct 29, 2021

done

Use direct CL and EL terms for clarity
@djrtwo djrtwo merged commit 7bcb2f0 into main Oct 29, 2021
@djrtwo djrtwo deleted the forkchoice-id branch October 29, 2021 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants