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

The first two steps for the key generation protocol #17

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 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
112 changes: 112 additions & 0 deletions gjkr/evidence_log.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
package gjkr

import (
"fmt"
"sync"
)

// For complaint resolution, group members need to have access to messages
// exchanged between the accuser and the accused party. There are two situations
// in the DKG protocol where group members generate values individually for
// every other group member:
//
// - Ephemeral ECDH (phase 2) - after each group member generates an ephemeral
// keypair for each other group member and broadcasts those ephemeral public keys
// in the clear (phase 1), group members must ECDH those public keys with the
// ephemeral private key for that group member to derive a symmetric key.
// In the case of an accusation, members performing compliant resolution need to
// validate the private ephemeral key revealed by the accuser. To perform the
// validation, members need to compare public ephemeral key published by the
// accuser in phase 1 with the private ephemeral key published by the accuser.
//
// - Polynomial generation (phase 3) - each group member generates two sharing
// polynomials, and calculates shares as points on these polynomials individually
// for each other group member. Shares are publicly broadcast, encrypted with a
// symmetric key established between the sender and receiver. In the case of an
// accusation, members performing compliant resolution need to look at the shares
// sent by the accused party. To do this, they read the round 3 message from the
// log, and decrypt it using the symmetric key used between the accuser and
// accused party. The key is publicly revealed by the accuser.
type evidenceLog interface {
Copy link
Member

Choose a reason for hiding this comment

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

What do you think about making evidenceLog a concrete structure and removing dkgEvidenceLog altogether? It does not make much sense to have an interface here. We are using one in-house implementation anyway. I know this was the original design in keep-core but maybe we can take the opportunity and clean it up.

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes sense. I was blindly assuming we'll use some sort of a mock interface in protocol integration tests but I browsed keep-core and I see it is not the case. Updated in 20254f7.

// getEphemeralPublicKeyMessage returns the `ephemeralPublicKeyMessage`
// broadcast in the first protocol round by the given sender.
getEphemeralPublicKeyMessage(sender memberIndex) *ephemeralPublicKeyMessage

// putEphemeralMessage is a function that takes a single
// EphemeralPubKeyMessage, and stores that as evidence for future
// accusation trials for a given (sender, receiver) pair. If a message
// already exists for the given sender, we return an error to the user.
putEphemeralPublicKeyMessage(pubKeyMessage *ephemeralPublicKeyMessage) error
}

// dkgEvidenceLog is the default implementation of an evidenceLog.
type dkgEvidenceLog struct {
// senderIndex -> *ephemeralPublicKeyMessage
pubKeyMessageLog *messageStorage
}

func newDkgEvidenceLog() *dkgEvidenceLog {
return &dkgEvidenceLog{
pubKeyMessageLog: newMessageStorage(),
}
}

func (d *dkgEvidenceLog) putEphemeralPublicKeyMessage(
pubKeyMessage *ephemeralPublicKeyMessage,
) error {
return d.pubKeyMessageLog.putMessage(
pubKeyMessage.senderIndex,
pubKeyMessage,
)
}

func (d *dkgEvidenceLog) getEphemeralPublicKeyMessage(
sender memberIndex,
) *ephemeralPublicKeyMessage {
storedMessage := d.pubKeyMessageLog.getMessage(sender)
switch message := storedMessage.(type) {
case *ephemeralPublicKeyMessage:
return message
}
return nil
}

type messageStorage struct {
cache map[memberIndex]interface{}
Copy link
Member

Choose a reason for hiding this comment

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

Loose idea: We can rejuvenate this code and use generics:

type messageStorage[[T interface{}] struct {
    cache map[memberIndex]T
    (...)
}

This way, we can create typed storages within dkgEvidenceLog:

type dkgEvidenceLog struct {
	pubKeyMessageLog *messageStorage[*ephemeralPublicKeyMessage]
}

and get rid of the type assertion in getEphemeralPublicKeyMessage.

With generics, this code will be more elegant once we add more messages.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good call. Done in baff3ad.

cacheLock sync.Mutex
Copy link
Member

Choose a reason for hiding this comment

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

Using sync.RWMutex will be a small improvement here.

Copy link
Member Author

Choose a reason for hiding this comment

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

}

func newMessageStorage() *messageStorage {
return &messageStorage{
cache: make(map[memberIndex]interface{}),
}
}

func (ms *messageStorage) getMessage(sender memberIndex) interface{} {
ms.cacheLock.Lock()
defer ms.cacheLock.Unlock()

message, ok := ms.cache[sender]
if !ok {
return nil
}

return message
}

func (ms *messageStorage) putMessage(
sender memberIndex, message interface{},
) error {
ms.cacheLock.Lock()
defer ms.cacheLock.Unlock()

if _, ok := ms.cache[sender]; ok {
return fmt.Errorf(
"message exists for sender %v",
sender,
)
}

ms.cache[sender] = message
return nil
}
61 changes: 61 additions & 0 deletions gjkr/evidence_log_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package gjkr

import (
"reflect"
"testing"

"threshold.network/roast/internal/testutils"
)

func TestPutEphemeralPublicKeyMessageTwice(t *testing.T) {
dkgEvidenceLog := newDkgEvidenceLog()
err := dkgEvidenceLog.putEphemeralPublicKeyMessage(
&ephemeralPublicKeyMessage{
senderIndex: memberIndex(1),
})
if err != nil {
t.Fatalf("unexpected error: [%v]", err)
}

err = dkgEvidenceLog.putEphemeralPublicKeyMessage(
&ephemeralPublicKeyMessage{
senderIndex: memberIndex(1),
})
if err == nil {
t.Fatal("expected an error")
}

testutils.AssertStringsEqual(
t,
"error",
"message exists for sender 1",
err.Error(),
)
}

func TestPutGetEphemeralPublicKeyMessage(t *testing.T) {
dkgEvidenceLog := newDkgEvidenceLog()

message := &ephemeralPublicKeyMessage{
senderIndex: memberIndex(1),
}

m := dkgEvidenceLog.getEphemeralPublicKeyMessage(memberIndex(1))
if m != nil {
t.Fatalf("expected message not to be found but has [%v]", m)
}

err := dkgEvidenceLog.putEphemeralPublicKeyMessage(message)
if err != nil {
t.Fatalf("unexpected error: [%v]", err)
}

m = dkgEvidenceLog.getEphemeralPublicKeyMessage(memberIndex(1))
if !reflect.DeepEqual(message, m) {
t.Fatalf(
"unexpected message\nexpected: %v\nactual: %v",
message,
m,
)
}
}
78 changes: 78 additions & 0 deletions gjkr/group.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
package gjkr

// group represents the current state of information about the GJKR key
// generation group. Each GJKR protocol participant should have the same group
// state at the end of each protocol step.
type group struct {
dishonestThreshold uint16
Copy link
Member

Choose a reason for hiding this comment

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

Nit: dishonestThreshold caused some confusion in keep-core in the past. What do you think about leaning on threshold and exposing a function dishonestThreshold that will compute it as groupSize - threshold? This approach feels much more intuitive.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was not sure about it either but wanted to approach it in the next PR when we will be adding the remaining steps of the protocol. I noticed we usually do +1 in keep-core unit tests which is a symptom of the problem you described but I did not find yet in the protocol where the dishonest threshold would be problematic and I assumed we could have some reason for leaving it. What about not worrying about it now but leaving this thread open and revisiting in the next PR?

groupSize uint16

allMemberIndexes []memberIndex
inactiveMemberIndexes []memberIndex
disqualifiedMemberIndexes []memberIndex
}

func newGroup(dishonestThreshold uint16, groupSize uint16) *group {
allMemberIndexes := make([]memberIndex, groupSize)
for i := uint16(0); i < groupSize; i++ {
allMemberIndexes[i] = memberIndex(i + 1)
}

return &group{
dishonestThreshold: dishonestThreshold,
groupSize: groupSize,
allMemberIndexes: allMemberIndexes,
inactiveMemberIndexes: []memberIndex{},
disqualifiedMemberIndexes: []memberIndex{},
}
}

// markMemberAsDisqualified adds the member with the given index to the list of
// disqualified members. If the member is not a part of the group, is already
// disqualified or marked as inactive, the function does nothing.
func (g *group) markMemberAsDisqualified(memberIndex memberIndex) {
if g.isOperating(memberIndex) {
g.disqualifiedMemberIndexes = append(g.disqualifiedMemberIndexes, memberIndex)
}
}

// markMemberAsInactive adds the member with the given index to the list of
// inactive members. If the member is not a part of the group, is already
// disqualified or marked as inactive, the function does nothing.
func (g *group) markMemberAsInactive(memberIndex memberIndex) {
if g.isOperating(memberIndex) {
g.inactiveMemberIndexes = append(g.inactiveMemberIndexes, memberIndex)
}
}

// isOperating returns true if member with the given index belongs to the group
// and has not been marked as inactive or disqualified.
func (g *group) isOperating(memberIndex memberIndex) bool {
return g.isInGroup(memberIndex) &&
!g.isInactive(memberIndex) &&
!g.isDisqualified(memberIndex)
}

func (g *group) isInGroup(memberIndex memberIndex) bool {
return memberIndex > 0 && uint16(memberIndex) <= g.groupSize
}

func (g *group) isInactive(memberIndex memberIndex) bool {
for _, inactiveMemberIndex := range g.inactiveMemberIndexes {
if memberIndex == inactiveMemberIndex {
return true
}
}

return false
}

func (g *group) isDisqualified(memberIndex memberIndex) bool {
for _, disqualifiedMemberIndex := range g.disqualifiedMemberIndexes {
if memberIndex == disqualifiedMemberIndex {
return true
}
}

return false
Copy link
Member

Choose a reason for hiding this comment

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

Same as above:

Suggested change
for _, disqualifiedMemberIndex := range g.disqualifiedMemberIndexes {
if memberIndex == disqualifiedMemberIndex {
return true
}
}
return false
return slices.Contains(g.disqualifiedMemberIndexes, memberIndex)

Copy link
Member Author

Choose a reason for hiding this comment

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

Totally! Please see 2ea14cd.

}
Loading