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

Cluster definition hash incorrect for empty addresses or signatures #1689

Closed
corverroos opened this issue Jan 25, 2023 · 0 comments
Closed
Labels
protocol Protocol Team tickets

Comments

@corverroos
Copy link
Contributor

corverroos commented Jan 25, 2023

Problem to be solved

The current (and previous) versions of the charon definition (<=v1.4) calculates the incorrect SSZ hash for empty Bytes20 (eth addresses) and Bytes65 (k1 signatures).

BytesN type fields should ALWAYS be of length N. So an empty address should be 20 bytes of just 0s, similarly an empty signature should be 56 bytes of just 0s. Charon uses empty arrays instead which is incorrect.

This is causing an incompatibility with the launchpad, specifically the solo slow that includes empty operator addresses. The JS implementation is doing it correctly. That means the launchpad generated solo flow definitions are not compatible with charon v1.4.

We cannot however fix this in charon v1.4, since it would break any lock files out there with empty addresses or signatures, specifically lock files created by the create cluster cli.

The proper solution is to fix this in charon, so as part of the upcoming v1.5 that will be released as part of charon v0.14.

Proposed solution

If the launchpad needs to support solo flow with charon v0.13 (so cluster definition v1.4), then we either need to:

  • fix this in charon now v1.4, thereby throwing v1.4 create cluster users under the bus (requiring them to use --no-verify).
  • or better, the launchpad can implement a workaround to achieve the same "buggy" behavior for the solo flow (and fix it in their v1.5 later).

Charon v1.5 should however include the fix. Charon v1.3 must still be compatible with buggy logic.

@github-actions github-actions bot added the protocol Protocol Team tickets label Jan 25, 2023
obol-bulldozer bot pushed a commit that referenced this issue Jan 27, 2023
Fixes the SSZ issue that didn't ensure `BytesN` are always of length N. This is only included in v1.5 onwards. 

Also removes the lock.DistributedValidator.FeeRecipient field completely, also for previous versions as this isn't used at all and we do not need to support it for older versions.

category: bug
ticket: #1689
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
protocol Protocol Team tickets
Projects
None yet
Development

No branches or pull requests

1 participant