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

Revert partial definition concept #1591

Closed
corverroos opened this issue Jan 5, 2023 · 0 comments
Closed

Revert partial definition concept #1591

corverroos opened this issue Jan 5, 2023 · 0 comments

Comments

@corverroos
Copy link
Contributor

corverroos commented Jan 5, 2023

Problem to be solved

We introduced the concept of "partial cluster definition", as part of #1490 and #1520. This was however incorrect.

The solo flow usecase is as follows:

  • Solo operator performs the solo flow on the launchpad
  • They only populate the creator field, both address and config_signature
  • They do not populate the operator fields, no addresses or config_signatures nor enrs or enr_signatures.
  • They get a link to the generated "(partial) definition" they just created
  • They use that link in the charon create cluster command to create a p2p keys and validator key shares and the lock file. See cmd: add --definition-file flag to create cluster command #1558 for the first attempt.

The problem with the first attempt was that the lock file creator.config_signature must remain valid.

  • The current Go logic uses N empty operator addresses to calculate the config hash (since the there are N operator structs containing only enr fields).
  • But the partial cluster definition require empty operator array, which isn't the case after the enrs have been added.
  • We do not want to add switching logic to detect some "special" case where it will use an empty operator slice when calculating the config hash (like was originally suggested).

Proposed solution

So the suggestion is to scrap the concept of a "partial definition" and instead:

  • The launchpad create a normal solo flow cluster definition.
  • The requirement is that all operator fields are empty strings, so N operators with empty string address, enr, config_signature, and enr_signature fields (note not an empty operator array, but empty operator json objects, [{},{},{}] vs [].
  • The resulting config_hash should therefore use N empty string operator addresses.
  • The definition_hash can also be populated as normal, it will be overwritten by the create cluster command.

The charon create cluster will then do the following:

  • download the --definition-file from the url.
  • verify the hash and signatures it like any normal definition file.
  • adhoc verify that operators are empty structs.
  • Populate the operator enrs
  • Recalculate the definition_hash
  • Use that definition as is when creating the lock file.
  • The config_hash and operator config_signature therefore remains valid.

Out of Scope

If there is anything to highlight as out of scope for this issue, please outline it here.

@corverroos corverroos added this to the M1 Guarded Launch milestone Jan 5, 2023
obol-bulldozer bot pushed a commit that referenced this issue Jan 9, 2023
Removes partial definition concept. Supports "solo flow" cluster definition with signed creator and empty operators. 

category: refactor
ticket: #1591
xenowits pushed a commit that referenced this issue Jan 9, 2023
Removes partial definition concept. Supports "solo flow" cluster definition with signed creator and empty operators. 

category: refactor
ticket: #1591
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

No branches or pull requests

1 participant