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 randao core signed data implementation #919

Closed
corverroos opened this issue Aug 5, 2022 · 3 comments
Closed

Refactor randao core signed data implementation #919

corverroos opened this issue Aug 5, 2022 · 3 comments
Assignees
Labels
enhancement New feature or request

Comments

@corverroos
Copy link
Contributor

corverroos commented Aug 5, 2022

Problem to be solved

While refactoring signature verification into its own standalone function, we discovered that randao signatures cannot be verified on their own, since they do not contain the raw data that we signed, which is the epoch. We previously relied on the associated duty/slot. This creates additional coupling and means we cannot extract and decouple signature verification properly.

Before #217 can be completed, we need to address this issue.

Proposed solution

  • Introduce our own custom ETH2 type called eth2util.SignedEpoch:
type SignedEpoch struct {
  Epoch eth2p0.Epoch
  Signature eth2p0.BLSSignature
}
  • Refactor eth2util.EpochHashRoot as a method of this type.

  • Introduce a new core.SignedEpoch wrapper of eth2util.SignedEpoch implementing core.SignedData, exactly the same as the other siigned data implemenations.

  • Rename DutyRando to DutySignature, use that duty in DKG.

  • Add a new duty type, DutyRandao, use this duty in charon run. (this is a breaking change)

  • Use core.SignedEpoch for DutyRandao instead of core.Signature.

  • Note we are not staying backwards compatible, DutyRandao (and therefore DutyPropose) will not work in cluster with a mix of previous and later versions since the wire value of DutyRandao is changing.

  • Decouple Randao signature verification from core.Duty.Slot

@corverroos corverroos added the enhancement New feature or request label Aug 5, 2022
@thomasheremans
Copy link
Contributor

Paused until v1.0

@corverroos
Copy link
Contributor Author

corverroos commented Aug 17, 2022

Yeah, this will preferably require breaking changes to the API.

To keep backwards wire compatibility, we need to refactor randao to a new duty type enum, and send both enums from the new servers (which will result in errors in older servers) and convert the old duty type to the new duty in parsigex (for charon run only not dkg) which introduces more coupling, which is what we are trying to fix.

If this can go in with breaking change, then it would be best.

obol-bulldozer bot pushed a commit that referenced this issue Aug 23, 2022
Refactors DutyRandao to use SignedRandao and move away from Signature type. Also introduces DutySignature which will be used by DKG.

category: refactor
ticket: #919
@dB2510
Copy link
Contributor

dB2510 commented Aug 23, 2022

Closed by #1019

@dB2510 dB2510 closed this as completed Aug 23, 2022
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

3 participants