-
Notifications
You must be signed in to change notification settings - Fork 90
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
dkg: add signing to dkg #564
Conversation
Codecov Report
@@ Coverage Diff @@
## main #564 +/- ##
==========================================
+ Coverage 54.39% 54.70% +0.31%
==========================================
Files 92 92
Lines 8496 8636 +140
==========================================
+ Hits 4621 4724 +103
+ Misses 3243 3239 -4
- Partials 632 673 +41
Continue to review full report at Codecov.
|
dkg/dkg.go
Outdated
sig := &bls_sig.Signature{} | ||
if err = sig.UnmarshalBinary(s.Signature); err != nil { | ||
return nil, nil, errors.Wrap(err, "Unmarshal signature") | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can do tblsconv.SigFromCore(core.Signature(s))
dkg/dkg.go
Outdated
if err != nil { | ||
return nil, errors.Wrap(err, "aggregate signature") | ||
return nil, nil, errors.Wrap(err, "BLS Aggregate Signatures") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
errors should not be capitalised
dkg/dkg.go
Outdated
@@ -136,24 +148,128 @@ func Run(ctx context.Context, conf Config) error { | |||
return err | |||
} | |||
|
|||
pubkeyToVerifiers := make(map[core.PubKey]*sharing.FeldmanVerifier) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not pass in shares directly into aggSignLock hash, no need for these verbose types I think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i did this because we are getting response from exchange according to pubkeys and in aggSignLockhash also we are iterating according to pubkeys
dkg/dkg.go
Outdated
lock := cluster.Lock{ | ||
Definition: def, | ||
Validators: dvs, | ||
} | ||
|
||
if conf.TestSigning { | ||
sig, err := aggSignLockHash(ctx, tcpNode, nodeIdx, nil, lock, shares) | ||
sigLockHash, err := signLockHash(lock, nodeIdx.ShareIdx, shares) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
split lockhash signing/exchage/aggregation and depositdata signing/exchage/aggregation into separate functions, there is too much going on this root function
dkg/dkg.go
Outdated
return errors.Wrap(err, "verify multisignature") | ||
} | ||
|
||
if !verified { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
else if
maybe?
Adds signing and aggregation of lock hash and deposit data to dkg.
category: feature
ticket: #522