Offline members after signalling key shouldn't be in group (#752)

* failing test that reproduces the issue

* failing test before updating kyber

* comment on callback

* updated with latest versions of kyber & bls12-381

* include compatibility test

* relaxed parameters for CI

* simplified test for broadcast

core/broadcast_test.go

@@ -69,14 +69,14 @@ func TestBroadcast(t *testing.T) {
 	require.NoError(t, err)
 	time.Sleep(500 * time.Millisecond)
 	broads[1].Lock()
-	// so here it shouldnt have the entry since we deleted it
-	// make sure first that the channel is empty
-	require.Len(t, broads[1].dealCh, 0)
 	select {
 	case <-broads[1].dealCh:
 		require.False(t, true, "deal shouldn't be passed down to application")
 	case <-time.After(500 * time.Millisecond):
-		// all good
+		// all good - the message is not supposed to be passed down since it was
+		// already sent in the first round, so this broadcast instance should
+		// never have received it, because broads[0] should never have sent it
+		// in the first place
 	}
 	// put it again
 	broads[1].hashes.put(deal.Hash())

core/drand.go

@@ -56,6 +56,11 @@ type Drand struct {
 	// but not participating. Drand calls the cancel func when the node
 	// participates to a resharing.
 	syncerCancel context.CancelFunc
+
+	// only used for testing currently
+	// XXX need boundaries between gRPC and control plane such that we can give
+	// a list of paramteres at each DKG (inluding this callback)
+	setupCB func(*key.Group)
 }
 
 // NewDrand returns an drand struct. It assumes the private key pair

core/drand_control.go

@@ -519,6 +519,11 @@ func (d *Drand) InitReshare(c context.Context, in *drand.InitResharePacket) (*dr
 		d.log.Error("init_reshare", "leader setup", "err", err)
 		return nil, fmt.Errorf("drand: invalid setup configuration: %s", err)
 	}
+	if d.setupCB != nil {
+		// XXX Currently a bit hacky - we should split the control plane and the
+		// gRPC interface and give that callback as argument
+		d.setupCB(newGroup)
+	}
 	// some assertions that should always be true but never too safe
 	if oldGroup.GenesisTime != newGroup.GenesisTime {
 		return nil, errors.New("control: old and new group have different genesis time")

core/drand_test.go

@@ -62,8 +62,8 @@ func TestDrandDKGFresh(t *testing.T) {
 }
 
 func TestDrandDKGBroadcastDeny(t *testing.T) {
-	n := 10
-	thr := 6
+	n := 5
+	thr := 4
 	beaconPeriod := 1 * time.Second
 
 	dt := NewDrandTest2(t, n, thr, beaconPeriod)
@@ -110,6 +110,50 @@ func TestDrandReshareForce(t *testing.T) {
 	fmt.Println(group3)
 }
 
+// This tests when a node first signal his intention to participate into a
+// resharing but is down right after  - he shouldn't be in the final group
+func TestDrandDKGReshareAbsent(t *testing.T) {
+	oldN := 3
+	newN := 4
+	oldThr := 2
+	newThr := 3
+	timeout := 1 * time.Second
+	beaconPeriod := 2 * time.Second
+
+	dt := NewDrandTest2(t, oldN, oldThr, beaconPeriod)
+	defer dt.Cleanup()
+	group1 := dt.RunDKG()
+	// make sure all nodes had enough time to run their go routines to start the
+	// beacon handler - related to CI problems
+	time.Sleep(getSleepDuration())
+	dt.MoveToTime(group1.GenesisTime)
+	// move to genesis time - so nodes start to make a round
+	// dt.MoveTime(offsetGenesis)
+	// two = genesis + 1st round (happens at genesis)
+	dt.TestBeaconLength(2, false, dt.Ids(oldN, false)...)
+	// so nodes think they are going forward with round 2
+	dt.MoveTime(1 * time.Second)
+
+	toAdd := newN - oldN
+	dt.SetupNewNodes(toAdd)
+	// we want to stop one node right after the group is created
+	nodeToStop := 1
+	leader := 0
+	dt.nodes[leader].drand.setupCB = func(g *key.Group) {
+		fmt.Printf("\n\nSTOPPING NODE 1\n\n")
+		dt.nodes[nodeToStop].drand.Stop(context.Background())
+		fmt.Printf("\n\nSTOPPING NODE 1 DONE \n\n")
+	}
+	fmt.Println("SETUP RESHARE DONE")
+	newGroup, err := dt.RunReshare(oldN, toAdd, newThr, timeout, false, false, true)
+	require.NoError(t, err)
+	require.NotNil(t, newGroup)
+	// the node that had stopped must not be in the group
+	missingPublic := dt.nodes[nodeToStop].drand.priv.Public
+	require.Nil(t, newGroup.Find(missingPublic), "missing public is found", missingPublic)
+
+}
+
 func TestDrandDKGReshareTimeout(t *testing.T) {
 	oldN := 3
 	newN := 4

core/util_test.go

@@ -111,9 +111,11 @@ func (d *DrandTest2) RunDKG() *key.Group {
 	wg.Add(d.n)
 	// first run the leader and then run the other nodes
 	go func() {
-		_, err := controlClient.InitDKGLeader(d.n, d.thr, d.period, d.catchupPeriod, testDkgTimeout, nil, secret, testBeaconOffset)
+		gp, err := controlClient.InitDKGLeader(d.n, d.thr, d.period, d.catchupPeriod, testDkgTimeout, nil, secret, testBeaconOffset)
 		require.NoError(d.t, err)
-		fmt.Printf("\n\nTEST LEADER FINISHED\n\n")
+		g, err := key.GroupFromProto(gp)
+		require.NoError(d.t, err)
+		fmt.Printf("\n\nTEST LEADER FINISHED %x\n\n", g.Hash())
 		wg.Done()
 	}()
 
@@ -125,9 +127,11 @@ func (d *DrandTest2) RunDKG() *key.Group {
 		go func(n *Node) {
 			client, err := net.NewControlClient(n.drand.opts.controlPort)
 			require.NoError(d.t, err)
-			_, err = client.InitDKG(root.drand.priv.Public, nil, secret)
+			gp, err := client.InitDKG(root.drand.priv.Public, nil, secret)
+			require.NoError(d.t, err)
+			g, err := key.GroupFromProto(gp)
 			require.NoError(d.t, err)
-			fmt.Printf("\n\nTEST NONLEADER FINISHED\n\n")
+			fmt.Printf("\n\nTEST NONLEADER FINISHED %x\n\n", g.Hash())
 			wg.Done()
 		}(node)
 	}
@@ -318,7 +322,11 @@ func (d *DrandTest2) SetupNewNodes(newNodes int) []*Node {
 // running, and "newRun" new nodes running (the ones created via SetupNewNodes).
 // It sets the given threshold to the group.
 // It stops the nodes excluded first.
-func (d *DrandTest2) RunReshare(oldRun, newRun, newThr int, timeout time.Duration, force, onlyLeader bool) (*key.Group, error) {
+func (d *DrandTest2) RunReshare(oldRun, newRun, newThr int, timeout time.Duration, force, onlyLeader bool, ignoreErrs ...bool) (*key.Group, error) {
+	var ignoreErr bool
+	if len(ignoreErrs) > 0 {
+		ignoreErr = true
+	}
 	d.Lock()
 	fmt.Printf(" -- Running RESHARE with %d/%d old, %d/%d new nodes\n", oldRun, len(d.nodes), newRun, len(d.newNodes))
 	var secret = "thisistheresharing"
@@ -347,6 +355,7 @@ func (d *DrandTest2) RunReshare(oldRun, newRun, newThr int, timeout time.Duratio
 		require.NoError(d.t, err)
 		_, err = client.InitReshare(leader.drand.priv.Public, secret, d.groupPath, force)
 		if err != nil {
+			fmt.Println("error in NON LEADER: ", err)
 			errCh <- err
 			return
 		}
@@ -365,6 +374,7 @@ func (d *DrandTest2) RunReshare(oldRun, newRun, newThr int, timeout time.Duratio
 		finalGroup, err := client.InitReshareLeader(d.newN, d.newThr, timeout, 0, secret, "", testBeaconOffset)
 		// Done resharing
 		if err != nil {
+			fmt.Println("error in LEADER: ", err)
 			errCh <- err
 		}
 		fg, err := key.GroupFromProto(finalGroup)
@@ -393,14 +403,20 @@ func (d *DrandTest2) RunReshare(oldRun, newRun, newThr int, timeout time.Duratio
 	}
 	d.Unlock()
 	// wait for the return of the clients
-	select {
-	case finalGroup := <-groupCh:
-		d.newGroup = finalGroup
-		require.NoError(d.t, key.Save(d.groupPath, d.newGroup, false))
-		return finalGroup, nil
-	case err := <-errCh:
-		fmt.Println("ERRROR: ", err)
-		return nil, err
+	for {
+		select {
+		case finalGroup := <-groupCh:
+			d.newGroup = finalGroup
+			require.NoError(d.t, key.Save(d.groupPath, d.newGroup, false))
+			return finalGroup, nil
+		case err := <-errCh:
+			fmt.Println("ERRROR: ", err)
+			if !ignoreErr {
+				return nil, err
+			}
+		case <-time.After(500 * time.Millisecond):
+			d.MoveTime(d.period)
+		}
 	}
 }
 

go.mod

@@ -7,8 +7,8 @@ require (
 	github.com/aws/aws-sdk-go v1.32.11
 	github.com/briandowns/spinner v1.11.1
 	github.com/cpuguy83/go-md2man/v2 v2.0.0 // indirect
-	github.com/drand/kyber v1.1.2
-	github.com/drand/kyber-bls12381 v0.1.0
+	github.com/drand/kyber v1.1.4
+	github.com/drand/kyber-bls12381 v0.2.1
 	github.com/go-kit/kit v0.10.0
 	github.com/gogo/googleapis v1.4.0 // indirect
 	github.com/gogo/status v1.1.0 // indirect
@@ -49,7 +49,7 @@ require (
 	github.com/urfave/cli/v2 v2.2.0
 	github.com/weaveworks/common v0.0.0-20200512154658-384f10054ec5
 	go.etcd.io/bbolt v1.3.4
-	golang.org/x/crypto v0.0.0-20200604202706-70a84ac30bf9
+	golang.org/x/crypto v0.0.0-20200820211705-5c72a883971a
 	golang.org/x/net v0.0.0-20200602114024-627f9648deb9
 	golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543
 	google.golang.org/genproto v0.0.0-20200608115520-7c474a2e3482 // indirect

go.sum

@@ -110,10 +110,12 @@ github.com/drand/bls12-381 v0.3.2 h1:RImU8Wckmx8XQx1tp1q04OV73J9Tj6mmpQLYDP7V1XE
 github.com/drand/bls12-381 v0.3.2/go.mod h1:dtcLgPtYT38L3NO6mPDYH0nbpc5tjPassDqiniuAt4Y=
 github.com/drand/kyber v1.0.1-0.20200110225416-8de27ed8c0e2/go.mod h1:UpXoA0Upd1N9l4TvRPHr1qAUBBERj6JQ/mnKI3BPEmw=
 github.com/drand/kyber v1.0.2/go.mod h1:x6KOpK7avKj0GJ4emhXFP5n7M7W7ChAPmnQh/OL6vRw=
-github.com/drand/kyber v1.1.2 h1:faemqlaFyLrbBSjZGRzzu5SG/do+uTYpHlnrJIHbAhQ=
-github.com/drand/kyber v1.1.2/go.mod h1:x6KOpK7avKj0GJ4emhXFP5n7M7W7ChAPmnQh/OL6vRw=
-github.com/drand/kyber-bls12381 v0.1.0 h1:/P4C65VnyEwxzR5ZYYVMNzY1If+aYBrdUU5ukwh7LQw=
-github.com/drand/kyber-bls12381 v0.1.0/go.mod h1:N1emiHpm+jj7kMlxEbu3MUyOiooTgNySln564cgD9mk=
+github.com/drand/kyber v1.1.4 h1:YvKM03QWGvLrdTnYmxxP5iURAX+Gdb6qRDUOgg8i60Q=
+github.com/drand/kyber v1.1.4/go.mod h1:9+IgTq7kadePhZg7eRwSD7+bA+bmvqRK+8DtmoV5a3U=
+github.com/drand/kyber-bls12381 v0.2.0 h1:3GJfiHaMggQS2l2n7yrfX0PjY9BYikLM2f0zKP1eZTs=
+github.com/drand/kyber-bls12381 v0.2.0/go.mod h1:zQip/bHdeEB6HFZSU3v+d3cQE0GaBVQw9aR2E7AdoeI=
+github.com/drand/kyber-bls12381 v0.2.1 h1:/d5/YAdaCmHpYjF1NZevOEcKGaq6LBbyvkCTIdGqDjs=
+github.com/drand/kyber-bls12381 v0.2.1/go.mod h1:JwWn4nHO9Mp4F5qCie5sVIPQZ0X6cw8XAeMRvc/GXBE=
 github.com/dustin/go-humanize v0.0.0-20171111073723-bb3d318650d4/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk=
 github.com/dustin/go-humanize v1.0.0 h1:VSnTsYCnlFHaM2/igO1h6X3HA71jcobQuxemgkq4zYo=
 github.com/dustin/go-humanize v1.0.0/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk=
@@ -332,6 +334,10 @@ github.com/kabukky/httpscerts v0.0.0-20150320125433-617593d7dcb3/go.mod h1:BYpt4
 github.com/kami-zh/go-capturer v0.0.0-20171211120116-e492ea43421d/go.mod h1:P2viExyCEfeWGU259JnaQ34Inuec4R38JCyBx2edgD0=
 github.com/kilic/bls12-381 v0.0.0-20200607163746-32e1441c8a9f h1:qET3Wx0v8tMtoTOQnsJXVvqvCopSf48qobR6tcJuDHo=
 github.com/kilic/bls12-381 v0.0.0-20200607163746-32e1441c8a9f/go.mod h1:XXfR6YFCRSrkEXbNlIyDsgXVNJWVUV30m/ebkVy9n6s=
+github.com/kilic/bls12-381 v0.0.0-20200731194930-64c428e1bff5 h1:RAGCvOaqSiey3BGHopL/JI6+baO7D7AYQVDb6I8pRTs=
+github.com/kilic/bls12-381 v0.0.0-20200731194930-64c428e1bff5/go.mod h1:XXfR6YFCRSrkEXbNlIyDsgXVNJWVUV30m/ebkVy9n6s=
+github.com/kilic/bls12-381 v0.0.0-20200820230200-6b2c19996391 h1:51kHw7l/dUDdOdW06AlUGT5jnpj6nqQSILebcsikSjA=
+github.com/kilic/bls12-381 v0.0.0-20200820230200-6b2c19996391/go.mod h1:XXfR6YFCRSrkEXbNlIyDsgXVNJWVUV30m/ebkVy9n6s=
 github.com/kisielk/errcheck v1.1.0/go.mod h1:EZBBE59ingxPouuu3KfxchcWSUPOHkagtvWXihfKN4Q=
 github.com/kisielk/errcheck v1.2.0/go.mod h1:/BMXB+zMLi60iA8Vv6Ksmxu/1UDYcXs4uQLJ+jE2L00=
 github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck=
@@ -828,6 +834,10 @@ golang.org/x/crypto v0.0.0-20200423211502-4bdfaf469ed5/go.mod h1:LzIPMQfyMNhhGPh
 golang.org/x/crypto v0.0.0-20200510223506-06a226fb4e37/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
 golang.org/x/crypto v0.0.0-20200604202706-70a84ac30bf9 h1:vEg9joUBmeBcK9iSJftGNf3coIG4HqZElCPehJsfAYM=
 golang.org/x/crypto v0.0.0-20200604202706-70a84ac30bf9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
+golang.org/x/crypto v0.0.0-20200728195943-123391ffb6de h1:ikNHVSjEfnvz6sxdSPCaPt572qowuyMDMJLLm3Db3ig=
+golang.org/x/crypto v0.0.0-20200728195943-123391ffb6de/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
+golang.org/x/crypto v0.0.0-20200820211705-5c72a883971a h1:vclmkQCjlDX5OydZ9wv8rBCcS0QyQY66Mpf/7BZbInM=
+golang.org/x/crypto v0.0.0-20200820211705-5c72a883971a/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
 golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
 golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE=
 golang.org/x/lint v0.0.0-20190227174305-5b3e6a55c961/go.mod h1:wehouNa3lNwaWXcvxsM5YxQ5yQlVC4a0KAMCusXpPoU=
@@ -918,6 +928,10 @@ golang.org/x/sys v0.0.0-20200509044756-6aff5f38e54f/go.mod h1:h1NjWce9XRLGQEsW7w
 golang.org/x/sys v0.0.0-20200519105757-fe76b779f299/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20200602225109-6fdc65e7d980 h1:OjiUf46hAmXblsZdnoSXsEUSKU8r1UEzcL5RVZ4gO9Y=
 golang.org/x/sys v0.0.0-20200602225109-6fdc65e7d980/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/sys v0.0.0-20200812155832-6a926be9bd1d h1:QQrM/CCYEzTs91GZylDCQjGHudbPTxF/1fvXdVh5lMo=
+golang.org/x/sys v0.0.0-20200812155832-6a926be9bd1d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/sys v0.0.0-20200926100807-9d91bd62050c h1:38q6VNPWR010vN82/SB121GujZNIfAUb4YttE2rhGuc=
+golang.org/x/sys v0.0.0-20200926100807-9d91bd62050c/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.3.2 h1:tW2bmiBqwgJj/UpqtC8EpXEZVYOwU0yG4iWbprSVAcs=
 golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk=

key/curve_test.go

@@ -0,0 +1,30 @@
+package key
+
+import (
+	"encoding/hex"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestBLS12381Compatv112(t *testing.T) {
+	privHex := "643d6c704505385387a20d98aba19664e3ee81c600d21a0da910cc87f5dc4ab3"
+	privBuff, err := hex.DecodeString(privHex)
+	require.NoError(t, err)
+	msgHex := "7061737320746865207369676e6174757265"
+	msg, err := hex.DecodeString(msgHex)
+	require.NoError(t, err)
+	sigExp, err := hex.DecodeString("9940ca447bab3bab393c3a07866349343630437167eae" +
+		"ab063ef1e47acedc51e85c513121cf319a8832c3d136d7f36490fa7241194b403a3bbbba9e7d5" +
+		"e73c9a86f67a9585c6fe077cd6576b2f76560efbab3550d9d5124242c728e3a7ef6989")
+
+	require.NoError(t, err)
+
+	priv := KeyGroup.Scalar()
+	require.NoError(t, priv.UnmarshalBinary(privBuff))
+	pub := KeyGroup.Point().Mul(priv, nil)
+	sig, err := AuthScheme.Sign(priv, msg)
+	require.NoError(t, err)
+	require.NoError(t, AuthScheme.Verify(pub, msg, sig))
+	require.Equal(t, sig, sigExp)
+}

net/client.go

@@ -16,6 +16,12 @@ type Client interface {
 	HTTPClient
 }
 
+// Stopppable is an interface that some clients can implement to close their
+// operations
+type Stoppable interface {
+	Stop()
+}
+
 // CallOption is simply a wrapper around the grpc options
 type CallOption = grpc.CallOption
 

net/client_grpc.go

@@ -338,3 +338,12 @@ func (g *grpcClient) HandleHTTP(p Peer) (http.Handler, error) {
 
 	return &httpHandler{client}, nil
 }
+
+func (g *grpcClient) Stop() {
+	g.Lock()
+	defer g.Unlock()
+	for _, c := range g.conns {
+		c.Close()
+	}
+	g.conns = make(map[string]*grpc.ClientConn)
+}

net/gateway.go

@@ -28,6 +28,9 @@ func (g *PrivateGateway) StartAll() {
 // StopAll stops the control and public functionalities of the node
 func (g *PrivateGateway) StopAll(ctx context.Context) {
 	g.Listener.Stop(ctx)
+	if s, ok := g.ProtocolClient.(Stoppable); ok {
+		s.Stop()
+	}
 }
 
 // Listener is the active listener for incoming requests.