Skip to content
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

EL clients require TTD to be known in advance #2724

Closed
mkalinin opened this issue Nov 16, 2021 · 6 comments
Closed

EL clients require TTD to be known in advance #2724

mkalinin opened this issue Nov 16, 2021 · 6 comments
Labels
Bellatrix CL+EL Merge

Comments

@mkalinin
Copy link
Collaborator

Original discussion happened in Discord: https://discord.com/channels/595666850260713488/692062809701482577/910086648346329099
Related to #2643

The essence of the discussion -- TTD should be known in advance and should be the only trigger of consensus upgrade on EL side, details and implications:

  • EL client must know TTD in advance to have a clear boundary at which the consensus upgrade happens. Handling the case when sister blocks have different consensus rules adds a lot of complexity to the fork machinery of EL clients.
  • TTD must be the only trigger of the consensus upgrade because adding another brings the same complexity issue as the previous case. It means that TERMINAL_BLOCK_HASH can't be used as a trigger for consensus upgrade on EL side. It means that TBH functionality may only be implemented by block whitelisting via its hash value + TTD override. EL clients currently support the whitelisting feature.

Option 1

Remove TTD and TBH overrides and rely on "release/update" procedure in case if there is a hashpower migration that significantly delays the Merge (the TTD override case) and in case of 51% attacks (the TBH case). Cons: delays the emergency reaction by X hours -- hours required for the network to upgrade their clients.

Option 2

Have override settings in CL and EL clients. Cons: bricks the client in case when user restarts only CL or EL client with a new setting, i.e. overrides go inconsistent. Potentially, EL may respond with specific errors to executePayload and forkchoiceUpdated if they reference a block that isn't a valid terminal block. It would aid figuring out the issue and restarting a node with correct settings.

Option 3

Have EL responsible for handling terminal block functionality and have override settings on EL only. TBH functionality can't be handled entirely by EL as TBH also comes with an epoch number which coordinates the transition on CL side. So, this option seems not viable as it lefts an issue with consistency of overrides between the layers.

@mkalinin
Copy link
Collaborator Author

With the assumption that upgrading binary distributions takes near the same time as restarting clients with additional CLI arguments, I am inclined to have the following approach:

  • Remove overrides via CLI args entirely and rely on overrides via releasing/upgrading binary distributions
  • Add the corresponding errors to Engine API, i.e. respond with error if executePayload or forkchoiceUpdated deviates from TTD hardcoded on EL side

Depending on the case, with this approach the Merge acceleration process will look as follows:

  • Hashpower reduction causing significant Merge delay
    • Release CL and EL binaries with the new TTD value that is lower than the previous one
  • Attacks on the network
    • Release CL binaries with the new TTD + TBH_ACTIVATION_EPOCH
    • Release EL binaries with the new TTD + TBH (whitelisted block hash)

The aforementioned assumption may not held and worth checking.

@hwwhww hwwhww added the Bellatrix CL+EL Merge label Nov 17, 2021
@karalabe
Copy link
Member

With the assumption that upgrading binary distributions takes near the same time as restarting clients with additional CLI arguments

I don think this is true. Pushing the code is one thing, but that needs to go through the pipeline. Go is super fast at building and even for us it takes 1-2h until all the binaries are published everywhere.

But I don't understand why you're so against that CLI flag. Let the option remain and users will do whatever is best for them. I don't see any advantage in trying to force CL clients to handle this in one specific way.

@karalabe
Copy link
Member

We've had hard forks before, we had to delay hard forks before. The current mechanisms work. No need to reinvent it.

@mkalinin
Copy link
Collaborator Author

But I don't understand why you're so against that CLI flag. Let the option remain and users will do whatever is best for them.

It neglects the original idea behind CLI flags because then we will have to take in account users that prefer to download binaries and have the Merge delayed with enough time for them doing so. In this case CLI flags will just contribute to UX. Though, I am not opposed to this it probably doesn't worth the complexity. I'm fine with emergency Merge strategy relying on binary releases and distribution but wondering can and should we do better or not. Better means less chaostime in the network in case of attacks.

@karalabe
Copy link
Member

I completely agree with cutting new releases, just saying that there's no particular reason to disable the other protective measures.

@djrtwo
Copy link
Contributor

djrtwo commented Nov 19, 2021

I'm not certain that having the flag overrides buys us much time in practice given that (1) clients will cut releases almost certainly anyway (thus we prbably assume the worst case that people rely on these) and (2) I don't suspect we could coordinate faster than 24 hours no matter what and this is more like a 48 coordination time at best, at which point the 12 hour boost (at best) from flags isn't making a tangible impact.

Flag overrides are okay but given the above, just introduce complexity. I suppose the main positive for flag overrides is that it will ensure that clients know how to handle TBH elegantly before needing to use it in an emergency. So specifying it here would force their hand in a sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bellatrix CL+EL Merge
Projects
None yet
Development

No branches or pull requests

4 participants