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

Refactor cluster definition and lock fields to hex #848

Closed
corverroos opened this issue Jul 25, 2022 · 1 comment
Closed

Refactor cluster definition and lock fields to hex #848

corverroos opened this issue Jul 25, 2022 · 1 comment
Assignees
Labels
enhancement New feature or request

Comments

@corverroos
Copy link
Contributor

corverroos commented Jul 25, 2022

Problem to be solved

We currently use golang/json package's default formatting of bytes in our cluster definition and cluster lock json objects which is base64 standard.

This is causing problems for DV-launchpad as it uses the definition config hash for routing to its backend and base64 standard isn't URL safe.

Proposed solution

The following formats are available to choose from:

  • hex: We leave hex for "eth2" things like group pubkey and ENRs. We want to use a different format for charon/obol specific things so users are less easily confused. Like thining the DV pubkey is one of the pubshares since those would also look like addresses.
  • base64 URL: base64 URL addresses the problem of using base64 in URLs be replacing "/" with "_" and "+" with "-". This could work but the resulting encoding still contain "special characters" which can be hard to work and look at for humans.
  • base58: base58 is both URL safe and human friendly. It only contains alpha numeric characters exluding look-alikes like "I" and "l". Base58 is also a common format in other crypto currencies, but not ETH (yet).

Final proposal:

  • We therefore suggest refactoring all byte fields from base64 standard to )x prefixed hex since that is the Ethereum standard which simplifies the config and aligns it with our industry.
  • These fields include:
    • cluster-definition.config_hash
    • cluster-definition.definition_hash
    • cluster-definition.operators.config_signature
    • cluster-definition.operators.enr_signature
    • cluster-lock.lock_hash
    • cluster-lock.signature_aggregate
    • cluster-lock.distributed_validators.public_shares
  • Increase the version from 1.1.0 to 1.2.0.
  • Since this is only cosmetic (the data is unchanged), the new logic should be backwards compatible with v1.0.0 and v1.1.0 files. Note the new file format v1.2.0 will not be compatible with old logic.
  • This can be achieved by created 2 sets of "json formatters", e.g. lockJSON_v1_1 (for old) and lockJSON_v1_2 (for new). All child formatters (definition, operators, distributed validators) would require the same.
  • funcs func unmarshalLock_v1_1(*Lock, []byte) error should then implemented to contain the separate logic.
  • Add backwards compatibility unit and smoke tests

Out of Scope

No other config upgrades included.

@corverroos corverroos added the enhancement New feature or request label Jul 25, 2022
@corverroos
Copy link
Contributor Author

corverroos commented Jul 25, 2022

@OisinKyne I think we should actually upgrade cluster-definition.operators.config_signature and
cluster-definition.operators.enr_signature to hex (not base58), as they are standard EIP712 signatures which are commonly hex with 0x prefix. Leave lock.signature_aggregate as base58 since that is a BLS sig.

obol-bulldozer bot pushed a commit that referenced this issue Jul 26, 2022
Introduces **draft** cluster config version `1.2.0`. EIP712 signatures are refactored to industry standard hex. Other base64 fields are refactored to base58 to be URL safe and easy for humans to work with.

`v1.2.0` will contain more upgrades, so not "released" yet.

category: feature
ticket: #848
@corverroos corverroos changed the title Refactor cluster definition and lock hashes to base58 Refactor cluster definition and lock fields to hex Jul 26, 2022
@corverroos corverroos self-assigned this Jul 26, 2022
obol-bulldozer bot pushed a commit that referenced this issue Jul 27, 2022
Refactors v1.2.0 to hex instead of base58 since that is standard ETH format for bytes.

category: refactor
ticket: #848
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant