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.

Sign up for GitHub

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

Jump to bottom

Move partial signature verification to parsigdb #217

Closed
corverroos opened this issue Mar 15, 2022 · 3 comments · Fixed by #956
Closed

Move partial signature verification to parsigdb #217

corverroos opened this issue Mar 15, 2022 · 3 comments · Fixed by #956
Assignees
@dB2510
Labels
enhancement New feature or request

Comments

@corverroos
Copy link
Contributor

corverroos commented Mar 15, 2022 •
edited
Loading

Problem to solve

We currently verify partial signatures submitted by the VC in the validatorapi. This doesn't prevent invalid signatures being added by our peers.

Proposed solution

  • Decouple verification by providing a VerifyFunc to parsigDB.
  • type VerifyFunc func(ctx context.Context, duty core.Duty, pubkey core.PubKey, parSig core.ParSignedData) error
  • Pass in verifyFunc into NewParSigDB, call this function before storing.
  • Extract verification logic from validatorapi and move to eth2util/signing, providing a Veirfy(...) error funciton.
  • For DKG, we implement a custom Varify function that can do lockhash and depositdata verifucation based on custom duty types.
@corverroos corverroos added the enhancement New feature or request label Mar 15, 2022
@corverroos
Copy link
Contributor Author

Note this is a security feature, assuming Byzantium/unexpected behaviour from peers. This could be moved out to V1 milestone.

@corverroos corverroos mentioned this issue Mar 15, 2022
Closed
@dB2510 dB2510 self-assigned this Apr 7, 2022
@corverroos
Copy link
Contributor Author

Note we should probably do verification in parsigex and validagtorAPI, as close to the source as possible (fail fast). Only doing this in parsigdb results in invalid "events" being generated and complicated Tracker.

@dB2510 dB2510 mentioned this issue Jul 26, 2022
Closed
@corverroos
Copy link
Contributor Author

Note verification in DKG and Charon Run is different. Will need to pass in the verification function as param to parsigex.New()

@dB2510 dB2510 mentioned this issue Aug 4, 2022
Merged
obol-bulldozer bot pushed a commit that referenced this issue Aug 4, 2022
@dB2510
eth2util/signing: add NewVerifyFunc (#912)
b7d5237 
Adds NewVerifyFunc in eth2util/signing.

category: refactor
ticket: #217
@dB2510 dB2510 mentioned this issue Aug 5, 2022
Merged
@corverroos corverroos mentioned this issue Aug 5, 2022
Closed
obol-bulldozer bot pushed a commit that referenced this issue Aug 5, 2022
@dB2510
core/validatorapi: add verifyFunc to validatorapi (#916)
811b303 
Adds verifyFunc to Validatorapi component and removes old methods.

category: refactor
ticket: #217
This was referenced Aug 9, 2022
Closed
Merged
obol-bulldozer bot pushed a commit that referenced this issue Aug 10, 2022
@dB2510
core/validatorapi: add verification functions to validatorapi (#953)
e55f167 
Adds verification functions for each duty in eth2util/signing and use them in validatorapi.

category: refactor
ticket: #217
@dB2510 dB2510 mentioned this issue Aug 10, 2022
Merged
@dB2510 dB2510 linked a pull request Aug 11, 2022 that will close this issue
core/parsigex: add verification function to parsigex #956
Merged
obol-bulldozer bot pushed a commit that referenced this issue Aug 11, 2022
@dB2510
core/parsigex: add verification function to parsigex (#956)
df106a6 
Adds verification function to parsigex and related constructors for verifyFunc: NewEth2Verifier and newDKGVerifier.

category: feature
ticket: #217
@dB2510 dB2510 mentioned this issue Aug 11, 2022
Merged
obol-bulldozer bot pushed a commit that referenced this issue Aug 12, 2022
@dB2510
dkg: add verification of partial signature (#967)
c7bbf51 
Adds partial signature verification before aggregation.

category: refactor
ticket: #217
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dB2510 dB2510

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

core/parsigex: add verification function to parsigex ObolNetwork/charon
3 participants
@corverroos @dB2510 @thomasheremans