Merge pull request #374 from ElrondNetwork/bugs/fix-hb-shard-id

Bugs/fix hb shard
multiversx · Aug 16, 2019 · 0bf0b40 · 0bf0b40
2 parents 53f4cf7 + d8674e9
commit 0bf0b40
Show file tree

Hide file tree

Showing 7 changed files with 42 additions and 38 deletions.
diff --git a/integrationTests/node/heartbeat_test.go b/integrationTests/node/heartbeat_test.go
@@ -168,7 +168,7 @@ func createMonitor(maxDurationPeerUnresponsive time.Duration) *heartbeat.Monitor
 		keyGen,
 		integrationTests.TestMarshalizer,
 		maxDurationPeerUnresponsive,
-		[]string{""},
+		map[uint32][]string{0: {""}},
 	)
 
 	return monitor

diff --git a/node/heartbeat/errors.go b/node/heartbeat/errors.go
@@ -2,8 +2,8 @@ package heartbeat
 
 import "errors"
 
-// ErrEmptyPublicKeyList signals that a nil or empty public key list has been provided
-var ErrEmptyPublicKeyList = errors.New("nil or empty public key list")
+// ErrEmptyPublicKeysMap signals that a nil or empty public keys map has been provided
+var ErrEmptyPublicKeysMap = errors.New("nil or empty public keys map")
 
 // ErrNilMessenger signals that a nil p2p messenger has been provided
 var ErrNilMessenger = errors.New("nil P2P Messenger")
@@ -31,3 +31,6 @@ var ErrInvalidMaxDurationPeerUnresponsive = errors.New("invalid max duration to
 
 // ErrNilAppStatusHandler defines the error for setting a nil AppStatusHandler
 var ErrNilAppStatusHandler = errors.New("nil AppStatusHandler")
+
+// ErrNilShardCoordinator signals that an operation has been attempted to or with a nil shard coordinator
+var ErrNilShardCoordinator = errors.New("nil shard coordinator")
diff --git a/node/heartbeat/heartbeat.go b/node/heartbeat/heartbeat.go
@@ -20,10 +20,11 @@ type PubKeyHeartbeat struct {
 	TimeStamp       time.Time `json:"timeStamp"`
 	MaxInactiveTime Duration  `json:"maxInactiveTime"`
 	IsActive        bool      `json:"isActive"`
-	ShardID         uint32    `json:"shardID"`
-	TotalUpTime     Duration  `json:"totalUpTime"`
-	TotalDownTime   Duration  `json:"totalDownTime"`
-	VersionNumber   string    `json:"versionNumber"`
-	IsValidator     bool      `json:"isValidator"`
-	NodeDisplayName string    `json:"nodeDisplayName"`
+	//TODO should have 2 fields for this: receivedShardID and computedShardID
+	ShardID         uint32   `json:"shardID"`
+	TotalUpTime     Duration `json:"totalUpTime"`
+	TotalDownTime   Duration `json:"totalDownTime"`
+	VersionNumber   string   `json:"versionNumber"`
+	IsValidator     bool     `json:"isValidator"`
+	NodeDisplayName string   `json:"nodeDisplayName"`
 }
diff --git a/node/heartbeat/monitor.go b/node/heartbeat/monitor.go
@@ -34,7 +34,7 @@ func NewMonitor(
 	keygen crypto.KeyGenerator,
 	marshalizer marshal.Marshalizer,
 	maxDurationPeerUnresponsive time.Duration,
-	pubKeyList []string,
+	pubKeysMap map[uint32][]string,
 ) (*Monitor, error) {
 
 	if singleSigner == nil {
@@ -46,8 +46,8 @@ func NewMonitor(
 	if marshalizer == nil {
 		return nil, ErrNilMarshalizer
 	}
-	if len(pubKeyList) == 0 {
-		return nil, ErrEmptyPublicKeyList
+	if len(pubKeysMap) == 0 {
+		return nil, ErrEmptyPublicKeysMap
 	}
 
 	mon := &Monitor{
@@ -59,11 +59,15 @@ func NewMonitor(
 		appStatusHandler:            &statusHandler.NilStatusHandler{},
 	}
 
-	var err error
-	for _, pubkey := range pubKeyList {
-		mon.heartbeatMessages[pubkey], err = newHeartbeatMessageInfo(maxDurationPeerUnresponsive, true)
-		if err != nil {
-			return nil, err
+	for shardId, pubKeys := range pubKeysMap {
+		for _, pubkey := range pubKeys {
+			mhbi, err := newHeartbeatMessageInfo(maxDurationPeerUnresponsive, true)
+			if err != nil {
+				return nil, err
+			}
+
+			mhbi.shardID = shardId
+			mon.heartbeatMessages[pubkey] = mhbi
 		}
 	}
 

diff --git a/node/heartbeat/monitor_test.go b/node/heartbeat/monitor_test.go
@@ -23,7 +23,7 @@ func TestNewMonitor_NilSingleSignerShouldErr(t *testing.T) {
 		&mock.KeyGenMock{},
 		&mock.MarshalizerMock{},
 		0,
-		[]string{""},
+		map[uint32][]string{0: {""}},
 	)
 
 	assert.Nil(t, mon)
@@ -38,7 +38,7 @@ func TestNewMonitor_NilKeygenShouldErr(t *testing.T) {
 		nil,
 		&mock.MarshalizerMock{},
 		0,
-		[]string{""},
+		map[uint32][]string{0: {""}},
 	)
 
 	assert.Nil(t, mon)
@@ -53,7 +53,7 @@ func TestNewMonitor_NilMarshalizerShouldErr(t *testing.T) {
 		&mock.KeyGenMock{},
 		nil,
 		0,
-		[]string{""},
+		map[uint32][]string{0: {""}},
 	)
 
 	assert.Nil(t, mon)
@@ -68,11 +68,11 @@ func TestNewMonitor_EmptyPublicKeyListShouldErr(t *testing.T) {
 		&mock.KeyGenMock{},
 		&mock.MarshalizerMock{},
 		0,
-		make([]string, 0),
+		make(map[uint32][]string),
 	)
 
 	assert.Nil(t, mon)
-	assert.Equal(t, heartbeat.ErrEmptyPublicKeyList, err)
+	assert.Equal(t, heartbeat.ErrEmptyPublicKeysMap, err)
 }
 
 func TestNewMonitor_OkValsShouldCreatePubkeyMap(t *testing.T) {
@@ -83,7 +83,7 @@ func TestNewMonitor_OkValsShouldCreatePubkeyMap(t *testing.T) {
 		&mock.KeyGenMock{},
 		&mock.MarshalizerMock{},
 		1,
-		[]string{"pk1", "pk2"},
+		map[uint32][]string{0: {"pk1", "pk2"}},
 	)
 
 	assert.NotNil(t, mon)
@@ -102,7 +102,7 @@ func TestMonitor_ProcessReceivedMessageNilMessageShouldErr(t *testing.T) {
 		&mock.KeyGenMock{},
 		&mock.MarshalizerMock{},
 		1,
-		[]string{"pk1"},
+		map[uint32][]string{0: {"pk1"}},
 	)
 
 	err := mon.ProcessReceivedMessage(nil)
@@ -118,7 +118,7 @@ func TestMonitor_ProcessReceivedMessageNilDataShouldErr(t *testing.T) {
 		&mock.KeyGenMock{},
 		&mock.MarshalizerMock{},
 		0,
-		[]string{"pk1"},
+		map[uint32][]string{0: {"pk1"}},
 	)
 
 	err := mon.ProcessReceivedMessage(&mock.P2PMessageStub{})
@@ -140,7 +140,7 @@ func TestMonitor_ProcessReceivedMessageMarshalFailsShouldErr(t *testing.T) {
 			},
 		},
 		1,
-		[]string{"pk1"},
+		map[uint32][]string{0: {"pk1"}},
 	)
 
 	err := mon.ProcessReceivedMessage(&mock.P2PMessageStub{DataField: []byte("")})
@@ -166,7 +166,7 @@ func TestMonitor_ProcessReceivedMessageWrongPubkeyShouldErr(t *testing.T) {
 			},
 		},
 		1,
-		[]string{"pk1"},
+		map[uint32][]string{0: {"pk1"}},
 	)
 
 	err := mon.ProcessReceivedMessage(&mock.P2PMessageStub{DataField: []byte("")})
@@ -196,7 +196,7 @@ func TestMonitor_ProcessReceivedMessageVerifyFailsShouldErr(t *testing.T) {
 			},
 		},
 		1,
-		[]string{"pk1"},
+		map[uint32][]string{0: {"pk1"}},
 	)
 
 	err := mon.ProcessReceivedMessage(&mock.P2PMessageStub{DataField: []byte("")})
@@ -227,7 +227,7 @@ func TestMonitor_ProcessReceivedMessageShouldWork(t *testing.T) {
 			},
 		},
 		time.Second*1000,
-		[]string{pubKey},
+		map[uint32][]string{0: {pubKey}},
 	)
 
 	err := mon.ProcessReceivedMessage(&mock.P2PMessageStub{DataField: []byte("")})
@@ -264,7 +264,7 @@ func TestMonitor_ProcessReceivedMessageWithNewPublicKey(t *testing.T) {
 			},
 		},
 		time.Second*1000,
-		[]string{"pk2"},
+		map[uint32][]string{0: {"pk2"}},
 	)
 
 	err := mon.ProcessReceivedMessage(&mock.P2PMessageStub{DataField: []byte("")})
@@ -305,7 +305,7 @@ func TestMonitor_ProcessReceivedMessageWithNewShardID(t *testing.T) {
 			},
 		},
 		time.Second*1000,
-		[]string{"pk1"},
+		map[uint32][]string{0: {"pk1"}},
 	)
 
 	// First send from pk1 from shard 0
@@ -375,7 +375,7 @@ func TestMonitor_ProcessReceivedMessageShouldSetPeerInactive(t *testing.T) {
 			},
 		},
 		time.Millisecond*5,
-		[]string{pubKey1, pubKey2},
+		map[uint32][]string{0: {pubKey1, pubKey2}},
 	)
 
 	// First send from pk1

diff --git a/node/heartbeat/sender.go b/node/heartbeat/sender.go
@@ -45,6 +45,7 @@ func NewSender(
 	if marshalizer == nil {
 		return nil, ErrNilMarshalizer
 	}
+	//TODO add test for shardCoordinator
 
 	sender := &Sender{
 		peerMessenger:    peerMessenger,

diff --git a/node/node.go b/node/node.go
@@ -714,17 +714,12 @@ func (n *Node) StartHeartbeat(config config.HeartbeatConfig, versionNumber strin
 		return err
 	}
 
-	allPubKeys := make([]string, 0)
-	for _, shardPubKeys := range n.initialNodesPubkeys {
-		allPubKeys = append(allPubKeys, shardPubKeys...)
-	}
-
 	n.heartbeatMonitor, err = heartbeat.NewMonitor(
 		n.singleSigner,
 		n.keyGen,
 		n.marshalizer,
 		time.Second*time.Duration(config.DurationInSecToConsiderUnresponsive),
-		allPubKeys,
+		n.initialNodesPubkeys,
 	)
 	if err != nil {
 		return err