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

tbls: replace verifier with pubshares #604

Merged
merged 1 commit into from
May 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 0 additions & 8 deletions cluster/cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,16 +99,8 @@ func TestEncode(t *testing.T) {
Validators: []cluster.DistValidator{
{
PubKey: testutil.RandomETHAddress(),
Verifiers: [][]byte{
testutil.RandomBytes32(),
testutil.RandomBytes32(),
},
}, {
PubKey: testutil.RandomETHAddress(),
Verifiers: [][]byte{
testutil.RandomBytes32(),
testutil.RandomBytes32(),
},
},
},
}
Expand Down
10 changes: 3 additions & 7 deletions cluster/distvalidator.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,6 @@ type DistValidator struct {
// It can be used to verify a partial signature created by any node in the cluster.
PubShares [][]byte `json:"public_shares,omitempty"`

// Verifiers are the threshold verifier commitments.
// Deprecated: Use PubShares.
Verifiers [][]byte `json:"threshold_verifiers,omitempty"`

// FeeRecipientAddress Ethereum address override for this validator, defaults to definition withdrawal address.
FeeRecipientAddress string `json:"fee_recipient_address,omitempty"`
}
Expand All @@ -52,9 +48,9 @@ func (v DistValidator) HashTreeRootWith(hh *ssz.Hasher) error {
// Field (0) 'PubKey'
hh.PutBytes([]byte(v.PubKey))

for _, verifier := range v.Verifiers {
// Field (1+i) 'Verifier'
hh.PutBytes(verifier)
for _, pubshare := range v.PubShares {
// Field (1+i) 'Pubshare'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can remove this comment

hh.PutBytes(pubshare)
}

// Field (N) 'FeeRecipientAddress'
Expand Down
9 changes: 1 addition & 8 deletions cluster/test_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,9 @@ func NewForT(t *testing.T, dv, k, n, seed int) (Lock, []*ecdsa.PrivateKey, [][]*
pk, err := tss.PublicKey().MarshalBinary()
require.NoError(t, err)

var verifiers [][]byte
for _, commitment := range tss.Verifier().Commitments {
verifiers = append(verifiers, commitment.ToAffineCompressed())
}

var pubshares [][]byte
for i := 0; i < n; i++ {
share, err := tss.PublicShare(i + 1) // Share indexes are 1-indexed.
require.NoError(t, err)
share := tss.PublicShare(i + 1) // Share indexes are 1-indexed.

b, err := share.MarshalBinary()
require.NoError(t, err)
Expand All @@ -84,7 +78,6 @@ func NewForT(t *testing.T, dv, k, n, seed int) (Lock, []*ecdsa.PrivateKey, [][]*
vals = append(vals, DistValidator{
PubKey: fmt.Sprintf("%#x", pk),
PubShares: pubshares,
Verifiers: verifiers,
})
dvShares = append(dvShares, shares)
}
Expand Down
14 changes: 3 additions & 11 deletions cluster/testdata/TestEncode_lock_json.golden
Original file line number Diff line number Diff line change
Expand Up @@ -31,20 +31,12 @@
},
"distributed_validators": [
{
"distributed_public_key": "0x7b182e046410f44bc4b0f3f03a0d06820a30f257",
"threshold_verifiers": [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't threshold_verifiers be replaced with pubshares?

"NCyLgFXEZtiGRB0lmQbWms2JS5aK6fDrnZZc5qRpPE4=",
"vogVAbfZhGtm6wK1flzae2y6aJHWFr1obDe4NGE6yLo="
]
"distributed_public_key": "0x7b182e046410f44bc4b0f3f03a0d06820a30f257"
},
{
"distributed_public_key": "0xa22c008ffe688352734ae4e3f1217acd5f832708",
"threshold_verifiers": [
"eVeydxnOPzGI3+V97r9vgllaEPe7ViygTVw9J5QpWMY=",
"2zJiZwZJ87yX2aIxZzXt5oKl3+bxoBH7yYrQ++eQADw="
]
"distributed_public_key": "0x342c8b8055c466d886441d259906d69acd894b96"
}
],
"signature_aggregate": "bbXBREw6NNMqXEp/++jRgfftO4z+kE+T+PBtKbzZ7YQ=",
"lock_hash": "wcS2qPDhSo0jvYr6tM+7pk4H+nsglA/cf8+baqBnuK0="
"lock_hash": "c1GjLXPvkrYyXHyyATXvQ64yEjCcc/YmXl4PrK5MeDQ="
}
10 changes: 2 additions & 8 deletions cluster/testdata/TestEncode_lock_yaml.golden
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,6 @@ cluster_definition:
- otcjnEC0WsOVDZQfxP4cDLlq0yLWIoIpX7/hHiakMwc=
distributed_validators:
- distributed_public_key: "0x7b182e046410f44bc4b0f3f03a0d06820a30f257"
threshold_verifiers:
- NCyLgFXEZtiGRB0lmQbWms2JS5aK6fDrnZZc5qRpPE4=
- vogVAbfZhGtm6wK1flzae2y6aJHWFr1obDe4NGE6yLo=
- distributed_public_key: "0xa22c008ffe688352734ae4e3f1217acd5f832708"
threshold_verifiers:
- eVeydxnOPzGI3+V97r9vgllaEPe7ViygTVw9J5QpWMY=
- 2zJiZwZJ87yX2aIxZzXt5oKl3+bxoBH7yYrQ++eQADw=
- distributed_public_key: "0x342c8b8055c466d886441d259906d69acd894b96"
signature_aggregate: bbXBREw6NNMqXEp/++jRgfftO4z+kE+T+PBtKbzZ7YQ=
lock_hash: wcS2qPDhSo0jvYr6tM+7pk4H+nsglA/cf8+baqBnuK0=
lock_hash: c1GjLXPvkrYyXHyyATXvQ64yEjCcc/YmXl4PrK5MeDQ=
5 changes: 1 addition & 4 deletions cmd/createcluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -337,10 +337,7 @@ func newLock(conf clusterConfig, dvs []tbls.TSS, peers []p2p.Peer) (cluster.Lock

var pubshares [][]byte
for i := 0; i < dv.NumShares(); i++ {
share, err := dv.PublicShare(i + 1) // Shares are 1-indexed.
if err != nil {
return cluster.Lock{}, err
}
share := dv.PublicShare(i + 1) // Shares are 1-indexed.
b, err := share.MarshalBinary()
if err != nil {
return cluster.Lock{}, errors.Wrap(err, "marshal pubshare")
Expand Down
3 changes: 1 addition & 2 deletions core/validatorapi/validatorapi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -626,8 +626,7 @@ func TestComponent_ProposerDuties(t *testing.T) {

// Create keys (just use normal keys, not split tbls)
pubkey := tss.PublicKey()
pubshare, err := tss.PublicShare(vIdx)
require.NoError(t, err)
pubshare := tss.PublicShare(vIdx)

eth2Share, err := tblsconv.KeyToETH2(pubshare)
require.NoError(t, err)
Expand Down
8 changes: 2 additions & 6 deletions dkg/dkg.go
Original file line number Diff line number Diff line change
Expand Up @@ -328,12 +328,9 @@ func aggLockHashSig(data map[core.PubKey][]core.ParSignedData, shares map[core.P
var pubshare *bls_sig.PublicKey
switch dkgAlgo {
case "keycast":
pubshare, err = tbls.GetPubShare(s.ShareIdx, shares[pk].Verifier)
if err != nil {
return nil, nil, errors.Wrap(err, "get pubshare from verifier")
}
pubshare = shares[pk].PublicShares[s.ShareIdx]
case "frost":
pubshare = shares[pk].PublicShares[uint32(s.ShareIdx)]
pubshare = shares[pk].PublicShares[s.ShareIdx]
default:
return nil, nil, errors.New("invalid dkg algo")
}
Expand Down Expand Up @@ -493,7 +490,6 @@ func dvsFromShares(shares []share) ([]cluster.DistValidator, error) {

dvs = append(dvs, cluster.DistValidator{
PubKey: fmt.Sprintf("%#x", msg.PubKey),
Verifiers: msg.Verifiers,
PubShares: msg.PubShares,
})
}
Expand Down
6 changes: 3 additions & 3 deletions dkg/frost.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ func makeShares(
targetID uint32,
) ([]share, error) {
// Get set of public shares for each validator.
pubShares := make(map[uint32]map[uint32]*bls_sig.PublicKey) // map[ValIdx]map[SourceID]*bls_sig.PublicKey
pubShares := make(map[uint32]map[int]*bls_sig.PublicKey) // map[ValIdx]map[SourceID]*bls_sig.PublicKey
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: why int instead of uint32?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while going through the code i didn't found any specific reason to use uint32 for dkg, that's why i went with fundamental types such as int

can change if there was any specific reason behind the usage of uint32 for dkg purposes

for key, result := range r2Result {
if key.TargetID != targetID {
// Not for us.
Expand All @@ -232,10 +232,10 @@ func makeShares(

m, ok := pubShares[key.ValIdx]
if !ok {
m = make(map[uint32]*bls_sig.PublicKey)
m = make(map[int]*bls_sig.PublicKey)
pubShares[key.ValIdx] = m
}
m[key.SourceID] = pubShare
m[int(key.SourceID)] = pubShare
}

// Sort shares by vIdx
Expand Down
45 changes: 8 additions & 37 deletions dkg/keycast.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ import (
"io"
"sort"

"github.com/coinbase/kryptology/pkg/core/curves"
"github.com/coinbase/kryptology/pkg/sharing"
"github.com/coinbase/kryptology/pkg/signatures/bls/bls_sig"

"github.com/obolnetwork/charon/app/errors"
Expand All @@ -46,16 +44,13 @@ type share struct {
PubKey *bls_sig.PublicKey
SecretShare *bls_sig.SecretKeyShare

// One of the two below will be populated,
Verifier *sharing.FeldmanVerifier
PublicShares map[uint32]*bls_sig.PublicKey // map[shareIdx]*bls_sig.PublicKey
PublicShares map[int]*bls_sig.PublicKey // map[shareIdx]*bls_sig.PublicKey
}

// shareMsg is the share message wire format sent by the dealer.
type shareMsg struct {
PubKey []byte
PubShares [][]byte
Verifiers [][]byte
SecretShare []byte
}

Expand Down Expand Up @@ -169,12 +164,7 @@ func leadKeyCast(ctx context.Context, tp kcTransport, def cluster.Definition, ra
func createShares(numValidators, numNodes, threshold int, random io.Reader) ([][]share, error) {
resp := make([][]share, numNodes)
for i := 0; i < numValidators; i++ {
pubkey, secret, err := tbls.Keygen()
if err != nil {
return nil, err
}

shares, verifier, err := tbls.SplitSecret(secret, threshold, numNodes, random)
tss, shares, err := tbls.GenerateTSS(threshold, numNodes, random)
if err != nil {
return nil, err
}
Expand All @@ -185,9 +175,9 @@ func createShares(numValidators, numNodes, threshold int, random io.Reader) ([][

for ni := 0; ni < numNodes; ni++ {
resp[ni] = append(resp[ni], share{
PubKey: pubkey,
Verifier: verifier,
SecretShare: shares[ni],
PubKey: tss.PublicKey(),
PublicShares: tss.PublicShares(),
SecretShare: shares[ni],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can we rename the loop variable ni?

})
}
}
Expand All @@ -202,13 +192,6 @@ func msgFromShare(s share) (shareMsg, error) {
return shareMsg{}, errors.Wrap(err, "marshal pubkey")
}

var verifiers [][]byte
if s.Verifier != nil {
for _, commitment := range s.Verifier.Commitments {
verifiers = append(verifiers, commitment.ToAffineCompressed())
}
}

// Sort pub shares by id/index.
var pubSharesIDs []int
for id := range s.PublicShares {
Expand All @@ -218,7 +201,7 @@ func msgFromShare(s share) (shareMsg, error) {

var pubShares [][]byte
for _, id := range pubSharesIDs {
b, err := s.PublicShares[uint32(id)].MarshalBinary()
b, err := s.PublicShares[id].MarshalBinary()
if err != nil {
return shareMsg{}, errors.Wrap(err, "marshal public share")
}
Expand All @@ -232,7 +215,6 @@ func msgFromShare(s share) (shareMsg, error) {

return shareMsg{
PubKey: pubkey,
Verifiers: verifiers,
SecretShare: secretShare,
PubShares: pubShares,
}, nil
Expand All @@ -245,24 +227,14 @@ func shareFromMsg(msg shareMsg) (share, error) {
return share{}, errors.Wrap(err, "unmarshal pubkey")
}

var commitments []curves.Point
for _, v := range msg.Verifiers {
c, err := curves.BLS12381G1().Point.FromAffineCompressed(v)
if err != nil {
return share{}, errors.Wrap(err, "verifier hex")
}

commitments = append(commitments, c)
}

pubShares := make(map[uint32]*bls_sig.PublicKey)
pubShares := make(map[int]*bls_sig.PublicKey)
for id, bytes := range msg.PubShares {
pubShare := new(bls_sig.PublicKey)
if err := pubShare.UnmarshalBinary(bytes); err != nil {
return share{}, errors.Wrap(err, "unmarshal public share")
}

pubShares[uint32(id+1)] = pubShare // Public shares IDs are 1-indexed.
pubShares[id+1] = pubShare // Public shares IDs are 1-indexed.
}

secretShare := new(bls_sig.SecretKeyShare)
Expand All @@ -272,7 +244,6 @@ func shareFromMsg(msg shareMsg) (share, error) {

return share{
PubKey: pubKey,
Verifier: &sharing.FeldmanVerifier{Commitments: commitments},
SecretShare: secretShare,
PublicShares: pubShares,
}, nil
Expand Down
38 changes: 22 additions & 16 deletions tbls/tss.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,18 +60,14 @@ func KeygenWithSeed(reader io.Reader) (*bls_sig.PublicKey, *bls_sig.SecretKey, e
// TSS (threshold signing scheme) wraps PubKey (PublicKey), Verifiers (the coefficients of the public polynomial)
// and threshold (number of shares).
type TSS struct {
verifier *share.FeldmanVerifier
pubshares map[int]*bls_sig.PublicKey
numShares int
threshold int

// publicKey inferred from verifier commitments in NewTSS.
publicKey *bls_sig.PublicKey
}

// Verifier returns the feldman verifier containing the public shares of the threshold signature scheme.
func (t TSS) Verifier() *share.FeldmanVerifier {
return t.verifier
}

// NumShares returns the number of shares in the threshold signature scheme.
func (t TSS) NumShares() int {
return t.numShares
Expand All @@ -84,12 +80,16 @@ func (t TSS) PublicKey() *bls_sig.PublicKey {

// Threshold returns the minimum number of partial signatures required to aggregate the threshold signature.
func (t TSS) Threshold() int {
return len(t.verifier.Commitments)
return t.threshold
}

// PublicShare returns a share's public key by share index (identifier).
func (t TSS) PublicShare(shareIdx int) (*bls_sig.PublicKey, error) {
return GetPubShare(shareIdx, t.verifier)
func (t TSS) PublicShare(shareIdx int) *bls_sig.PublicKey {
return t.pubshares[shareIdx]
}

func (t TSS) PublicShares() map[int]*bls_sig.PublicKey {
return t.pubshares
}

func NewTSS(verifier *share.FeldmanVerifier, numShares int) (TSS, error) {
Expand All @@ -99,10 +99,19 @@ func NewTSS(verifier *share.FeldmanVerifier, numShares int) (TSS, error) {
return TSS{}, errors.Wrap(err, "unmarshal pubkey")
}

pubshares := make(map[int]*bls_sig.PublicKey)
for i := 1; i <= numShares; i++ {
pubshares[i], err = getPubShare(i, verifier)
if err != nil {
return TSS{}, err
}
}

return TSS{
verifier: verifier,
pubshares: pubshares,
publicKey: pk,
numShares: numShares,
threshold: len(verifier.Commitments),
}, nil
}

Expand Down Expand Up @@ -153,10 +162,7 @@ func VerifyAndAggregate(tss TSS, partialSigs []*bls_sig.PartialSignature, msg []

for _, psig := range partialSigs {
// TODO(dhruv): add break condition if valid shares >= threshold
pubShare, err := tss.PublicShare(int(psig.Identifier))
if err != nil {
return nil, nil, errors.Wrap(err, "get Public Share")
}
pubShare := tss.PublicShare(int(psig.Identifier))

sig := &bls_sig.Signature{Value: psig.Signature}
ok, err := blsScheme.Verify(pubShare, msg, sig)
Expand Down Expand Up @@ -247,8 +253,8 @@ func SplitSecret(secret *bls_sig.SecretKey, t, n int, reader io.Reader) ([]*bls_
return sks, verifier, nil
}

// GetPubShare returns the public key share for the i'th/identifier/shareIdx share from the verifier commitments.
func GetPubShare(identifier int, verifier *share.FeldmanVerifier) (*bls_sig.PublicKey, error) {
// getPubShare returns the public key share for the i'th/identifier/shareIdx share from the verifier commitments.
func getPubShare(identifier int, verifier *share.FeldmanVerifier) (*bls_sig.PublicKey, error) {
curve := curves.GetCurveByName(verifier.Commitments[0].CurveName())
if curve != curves.BLS12381G1() {
return nil, errors.New("curve mismatch")
Expand Down
3 changes: 1 addition & 2 deletions tbls/tss_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,7 @@ func TestAggregateSignatures(t *testing.T) {

partialSigs[i] = psig

pubshare, err := tss.PublicShare(int(psig.Identifier))
require.NoError(t, err)
pubshare := tss.PublicShare(int(psig.Identifier))

ok, err := tbls.Verify(pubshare, msg, &bls_sig.Signature{Value: psig.Signature})
require.NoError(t, err)
Expand Down