fix: aggregating signature for hello message (pactus-project#640)

sync/bundle/message/hello.go

@@ -18,15 +18,15 @@ const (
 )
 
 type HelloMessage struct {
-	PeerID      peer.ID        `cbor:"1,keyasint"`
-	Agent       string         `cbor:"2,keyasint"`
-	Moniker     string         `cbor:"3,keyasint"`
-	PublicKey   *bls.PublicKey `cbor:"4,keyasint"`
-	Signature   *bls.Signature `cbor:"5,keyasint"`
-	Height      uint32         `cbor:"6,keyasint"`
-	Flags       int            `cbor:"7,keyasint"`
-	GenesisHash hash.Hash      `cbor:"8,keyasint"`
-	BlockHash   hash.Hash      `cbor:"10,keyasint"`
+	PeerID      peer.ID          `cbor:"1,keyasint"`
+	Agent       string           `cbor:"2,keyasint"`
+	Moniker     string           `cbor:"3,keyasint"`
+	PublicKeys  []*bls.PublicKey `cbor:"4,keyasint,"`
+	Signature   *bls.Signature   `cbor:"5,keyasint"`
+	Height      uint32           `cbor:"6,keyasint"`
+	Flags       int              `cbor:"7,keyasint"`
+	GenesisHash hash.Hash        `cbor:"8,keyasint"`
+	BlockHash   hash.Hash        `cbor:"10,keyasint"`
 }
 
 func NewHelloMessage(pid peer.ID, moniker string,
@@ -47,22 +47,15 @@ func (m *HelloMessage) SanityCheck() error {
 	if m.Signature == nil {
 		return errors.Error(errors.ErrInvalidSignature)
 	}
-	if m.PublicKey == nil {
+	if len(m.PublicKeys) == 0 {
 		return errors.Error(errors.ErrInvalidPublicKey)
 	}
-	return m.PublicKey.Verify(m.SignBytes(), m.Signature)
+	aggPublicKey := bls.PublicKeyAggregate(m.PublicKeys)
+	return aggPublicKey.Verify(m.SignBytes(), m.Signature)
 }
 
 func (m *HelloMessage) SignBytes() []byte {
-	return []byte(fmt.Sprintf("%s:%s:%s", m.Type(), m.Agent, m.PeerID))
-}
-
-func (m *HelloMessage) SetSignature(sig crypto.Signature) {
-	m.Signature = sig.(*bls.Signature)
-}
-
-func (m *HelloMessage) SetPublicKey(pub crypto.PublicKey) {
-	m.PublicKey = pub.(*bls.PublicKey)
+	return []byte(fmt.Sprintf("%s:%s:%s:%s", m.Type(), m.Agent, m.PeerID, m.GenesisHash.String()))
 }
 
 func (m *HelloMessage) Type() Type {
@@ -76,3 +69,16 @@ func (m *HelloMessage) String() string {
 	}
 	return fmt.Sprintf("{%s %v%s}", m.Moniker, m.Height, ack)
 }
+
+func (m *HelloMessage) Sign(signers ...crypto.Signer) {
+	signatures := make([]*bls.Signature, len(signers))
+	publicKeys := make([]*bls.PublicKey, len(signers))
+	signBytes := m.SignBytes()
+	for i, signer := range signers {
+		signatures[i] = signer.SignData(signBytes).(*bls.Signature)
+		publicKeys[i] = signer.PublicKey().(*bls.PublicKey)
+	}
+	aggSignature := bls.SignatureAggregate(signatures)
+	m.Signature = aggSignature
+	m.PublicKeys = publicKeys
+}

sync/bundle/message/hello_test.go

@@ -3,6 +3,8 @@ package message
 import (
 	"testing"
 
+	"github.com/pactus-project/pactus/crypto/bls"
+
 	"github.com/pactus-project/pactus/util/errors"
 	"github.com/pactus-project/pactus/util/testsuite"
 	"github.com/stretchr/testify/assert"
@@ -17,37 +19,36 @@ func TestHelloMessage(t *testing.T) {
 	ts := testsuite.NewTestSuite(t)
 
 	t.Run("Invalid signature", func(t *testing.T) {
-		signer1 := ts.RandomSigner()
-		signer2 := ts.RandomSigner()
+		signer := ts.RandomSigner()
 		m := NewHelloMessage(ts.RandomPeerID(), "Oscar", 100, 0, ts.RandomHash(), ts.RandomHash())
-		signer1.SignMsg(m)
-		m.SetPublicKey(signer2.PublicKey())
+		m.Sign(signer)
+		m.Signature = ts.RandomBLSSignature()
 
 		assert.Equal(t, errors.Code(m.SanityCheck()), errors.ErrInvalidSignature)
 	})
 
 	t.Run("Signature is nil", func(t *testing.T) {
 		signer := ts.RandomSigner()
 		m := NewHelloMessage(ts.RandomPeerID(), "Oscar", 100, 0, ts.RandomHash(), ts.RandomHash())
-		signer.SignMsg(m)
+		m.Sign(signer)
 		m.Signature = nil
 
 		assert.Equal(t, errors.Code(m.SanityCheck()), errors.ErrInvalidSignature)
 	})
 
-	t.Run("PublicKey is nil", func(t *testing.T) {
+	t.Run("PublicKeys are empty", func(t *testing.T) {
 		signer := ts.RandomSigner()
 		m := NewHelloMessage(ts.RandomPeerID(), "Oscar", 100, 0, ts.RandomHash(), ts.RandomHash())
-		signer.SignMsg(m)
-		m.PublicKey = nil
+		m.Sign(signer)
+		m.PublicKeys = make([]*bls.PublicKey, 0)
 
 		assert.Equal(t, errors.Code(m.SanityCheck()), errors.ErrInvalidPublicKey)
 	})
 
 	t.Run("Ok", func(t *testing.T) {
 		signer := ts.RandomSigner()
 		m := NewHelloMessage(ts.RandomPeerID(), "Alice", 100, 0, ts.RandomHash(), ts.RandomHash())
-		signer.SignMsg(m)
+		m.Sign(signer)
 
 		assert.NoError(t, m.SanityCheck())
 		assert.Contains(t, m.String(), "Alice")

sync/handler_blocks_request_test.go

@@ -2,47 +2,12 @@ package sync
 
 import (
 	"testing"
-	"time"
 
 	"github.com/pactus-project/pactus/sync/bundle/message"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
-func TestSessionTimeout(t *testing.T) {
-	config := testConfig()
-	config.SessionTimeout = 200 * time.Millisecond
-	td := setup(t, config)
-
-	t.Run("An unknown peers claims to have more blocks. Session should be closed after timeout", func(t *testing.T) {
-		signer := td.RandomSigner()
-		pid := td.RandomPeerID()
-		claimedHeight := uint32(6666)
-		msg := message.NewHelloMessage(pid, "Oscar", claimedHeight, message.FlagNodeNetwork,
-			td.state.LastBlockHash(), td.state.Genesis().Hash())
-		signer.SignMsg(msg)
-
-		assert.NoError(t, td.receivingNewMessage(td.sync, msg, pid))
-
-		td.shouldPublishMessageWithThisType(t, td.network, message.TypeBlocksRequest)
-
-		assert.True(t, td.sync.peerSet.HasAnyOpenSession())
-		time.Sleep(2 * config.SessionTimeout)
-		assert.False(t, td.sync.peerSet.HasAnyOpenSession())
-
-		peer := td.sync.peerSet.GetPeer(pid)
-		assert.Equal(t, peer.Height, claimedHeight)
-		assert.Equal(t, td.sync.peerSet.MaxClaimedHeight(), claimedHeight)
-		// TODO: This is not really good that a bad peer can manipulate the MaxCalim height
-		// Here is a possible solution:
-		// 1- A peer claims that he has more blocks
-		// 2- We ask him a block_request message
-		// 3- He doesn't respond
-		// 4- We close the session, marking the peer as a bad peer.
-		// 5- If MaxClaimedHeight is same as peer height, we set it to zero
-	})
-}
-
 func TestLatestBlocksRequestMessages(t *testing.T) {
 	config := testConfig()
 	config.NodeNetwork = false

sync/handler_hello.go

@@ -46,7 +46,7 @@ func (handler *helloHandler) ParseMessage(m message.Message, initiator peer.ID)
 		peerset.StatusCodeKnown,
 		msg.Moniker,
 		msg.Agent,
-		msg.PublicKey,
+		msg.PublicKeys,
 		util.IsFlagSet(msg.Flags, message.FlagNodeNetwork))
 	handler.peerSet.UpdateHeight(initiator, msg.Height, msg.BlockHash)
 

sync/handler_hello_test.go

@@ -22,8 +22,7 @@ func TestParsingHelloMessages(t *testing.T) {
 			initiator := td.RandomPeerID()
 			msg := message.NewHelloMessage(pid, "bad-genesis", 0, 0,
 				td.state.LastBlockHash(), td.state.Genesis().Hash())
-			signer.SignMsg(msg)
-			assert.True(t, msg.PublicKey.EqualsTo(signer.PublicKey()))
+			msg.Sign(signer)
 
 			assert.Error(t, td.receivingNewMessage(td.sync, msg, initiator))
 			assert.Equal(t, td.sync.peerSet.GetPeer(initiator).Status, peerset.StatusCodeBanned)
@@ -36,8 +35,7 @@ func TestParsingHelloMessages(t *testing.T) {
 			pid := td.RandomPeerID()
 			msg := message.NewHelloMessage(pid, "bad-genesis", 0, 0,
 				td.state.LastBlockHash(), invGenHash)
-			signer.SignMsg(msg)
-			assert.True(t, msg.PublicKey.EqualsTo(signer.PublicKey()))
+			msg.Sign(signer)
 
 			assert.Error(t, td.receivingNewMessage(td.sync, msg, pid))
 			td.shouldNotPublishMessageWithThisType(t, td.network, message.TypeHello)
@@ -51,12 +49,11 @@ func TestParsingHelloMessages(t *testing.T) {
 			pid := td.RandomPeerID()
 			msg := message.NewHelloMessage(pid, "kitty", height, message.FlagNodeNetwork,
 				td.state.LastBlockHash(), td.state.Genesis().Hash())
-			signer.SignMsg(msg)
+			msg.Sign(signer)
 
 			assert.NoError(t, td.receivingNewMessage(td.sync, msg, pid))
 
-			td.shouldPublishMessageWithThisType(t, td.network, message.TypeHello) // Alice key 1
-			td.shouldPublishMessageWithThisType(t, td.network, message.TypeHello) // Alice key 2
+			td.shouldPublishMessageWithThisType(t, td.network, message.TypeHello)
 
 			// Check if the peer info is updated
 			p := td.sync.peerSet.GetPeer(pid)
@@ -65,7 +62,7 @@ func TestParsingHelloMessages(t *testing.T) {
 			assert.Equal(t, p.Status, peerset.StatusCodeKnown)
 			assert.Equal(t, p.Agent, version.Agent())
 			assert.Equal(t, p.Moniker, "kitty")
-			assert.Contains(t, p.ConsensusKeys, *pub)
+			assert.Contains(t, p.ConsensusKeys, pub)
 			assert.Equal(t, p.PeerID, pid)
 			assert.Equal(t, p.Height, height)
 			assert.True(t, util.IsFlagSet(p.Flags, peerset.PeerFlagNodeNetwork))
@@ -78,7 +75,7 @@ func TestParsingHelloMessages(t *testing.T) {
 			pid := td.RandomPeerID()
 			msg := message.NewHelloMessage(pid, "kitty", height, message.FlagHelloAck,
 				td.state.LastBlockHash(), td.state.Genesis().Hash())
-			signer.SignMsg(msg)
+			msg.Sign(signer)
 
 			assert.NoError(t, td.receivingNewMessage(td.sync, msg, pid))
 			td.shouldNotPublishMessageWithThisType(t, td.network, message.TypeHello)
@@ -97,7 +94,7 @@ func TestParsingHelloMessages(t *testing.T) {
 			pid := td.RandomPeerID()
 			msg := message.NewHelloMessage(pid, "kitty", claimedHeight, message.FlagHelloAck,
 				td.state.LastBlockHash(), td.state.Genesis().Hash())
-			signer.SignMsg(msg)
+			msg.Sign(signer)
 
 			assert.NoError(t, td.receivingNewMessage(td.sync, msg, pid))
 			td.shouldPublishMessageWithThisType(t, td.network, message.TypeBlocksRequest)

sync/mock.go

@@ -3,6 +3,8 @@ package sync
 import (
 	"time"
 
+	"github.com/pactus-project/pactus/crypto/bls"
+
 	"github.com/libp2p/go-libp2p/core/peer"
 	"github.com/pactus-project/pactus/sync/peerset"
 	"github.com/pactus-project/pactus/util/testsuite"
@@ -27,7 +29,7 @@ func MockingSync(ts *testsuite.TestSuite) *MockSync {
 		peerset.StatusCodeKnown,
 		"test-peer-1",
 		version.Agent(),
-		pub1,
+		[]*bls.PublicKey{pub1},
 		true)
 	ps.UpdateHeight(pid1, ts.RandUint32(100000), ts.RandomHash())
 
@@ -36,7 +38,7 @@ func MockingSync(ts *testsuite.TestSuite) *MockSync {
 		peerset.StatusCodeBanned,
 		"test-peer-2",
 		version.Agent(),
-		pub2,
+		[]*bls.PublicKey{pub2},
 		false)
 	ps.UpdateHeight(pid1, ts.RandUint32(100000), ts.RandomHash())
 

sync/peerset/peer.go

@@ -19,7 +19,7 @@ type Peer struct {
 	Moniker         string
 	Agent           string
 	PeerID          peer.ID
-	ConsensusKeys   map[bls.PublicKey]bool
+	ConsensusKeys   []*bls.PublicKey
 	Flags           int
 	LastSent        time.Time
 	LastReceived    time.Time
@@ -35,7 +35,7 @@ type Peer struct {
 
 func NewPeer(peerID peer.ID) *Peer {
 	return &Peer{
-		ConsensusKeys: make(map[bls.PublicKey]bool),
+		ConsensusKeys: make([]*bls.PublicKey, 0),
 		Status:        StatusCodeUnknown,
 		PeerID:        peerID,
 		ReceivedBytes: make(map[message.Type]int64),

sync/peerset/peer_set.go

@@ -243,7 +243,7 @@ func (ps *PeerSet) UpdatePeerInfo(
 	status StatusCode,
 	moniker string,
 	agent string,
-	consKey *bls.PublicKey,
+	consKeys []*bls.PublicKey,
 	nodeNetwork bool,
 ) {
 	ps.lk.Lock()
@@ -253,7 +253,7 @@ func (ps *PeerSet) UpdatePeerInfo(
 	p.Status = status
 	p.Moniker = moniker
 	p.Agent = agent
-	p.ConsensusKeys[*consKey] = true
+	p.ConsensusKeys = consKeys
 
 	if nodeNetwork {
 		p.Flags = util.SetFlag(p.Flags, PeerFlagNodeNetwork)

sync/peerset/peer_set_test.go

@@ -5,6 +5,8 @@ import (
 	"testing"
 	"time"
 
+	"github.com/pactus-project/pactus/crypto/bls"
+
 	"github.com/libp2p/go-libp2p/core/peer"
 	"github.com/pactus-project/pactus/sync/bundle/message"
 	"github.com/pactus-project/pactus/util/testsuite"
@@ -20,12 +22,14 @@ func TestPeerSet(t *testing.T) {
 	pk1, _ := ts.RandomBLSKeyPair()
 	pk2, _ := ts.RandomBLSKeyPair()
 	pk3, _ := ts.RandomBLSKeyPair()
+	pk4, _ := ts.RandomBLSKeyPair()
+	pk5, _ := ts.RandomBLSKeyPair()
 	pid1 := peer.ID("peer1")
 	pid2 := peer.ID("peer2")
 	pid3 := peer.ID("peer3")
-	peerSet.UpdatePeerInfo(pid1, StatusCodeBanned, "Moniker1", "Agent1", pk1, true)
-	peerSet.UpdatePeerInfo(pid2, StatusCodeKnown, "Moniker2", "Agent2", pk2, false)
-	peerSet.UpdatePeerInfo(pid3, StatusCodeTrusty, "Moniker3", "Agent3", pk3, true)
+	peerSet.UpdatePeerInfo(pid1, StatusCodeBanned, "Moniker1", "Agent1", []*bls.PublicKey{pk1, pk2}, true)
+	peerSet.UpdatePeerInfo(pid2, StatusCodeKnown, "Moniker2", "Agent2", []*bls.PublicKey{pk3}, false)
+	peerSet.UpdatePeerInfo(pid3, StatusCodeTrusty, "Moniker3", "Agent3", []*bls.PublicKey{pk4, pk5}, true)
 
 	t.Run("Testing Len", func(t *testing.T) {
 		assert.Equal(t, 3, peerSet.Len())
@@ -69,6 +73,13 @@ func TestPeerSet(t *testing.T) {
 		assert.Equal(t, StatusCodeUnknown, p.Status)
 	})
 
+	t.Run("Testing PublicKeys", func(t *testing.T) {
+		p := peerSet.GetPeer(pid3)
+
+		assert.Contains(t, p.ConsensusKeys, pk4)
+		assert.Contains(t, p.ConsensusKeys, pk5)
+	})
+
 	t.Run("Testing counters", func(t *testing.T) {
 		peerSet.IncreaseInvalidBundlesCounter(pid1)
 		peerSet.IncreaseReceivedBundlesCounter(pid1)
@@ -231,7 +242,7 @@ func TestGetRandomWeightedPeer(t *testing.T) {
 		pid := peer.ID(fmt.Sprintf("peer_%v", i+1))
 		peerSet.UpdatePeerInfo(
 			pid, StatusCodeKnown,
-			fmt.Sprintf("Moniker_%v", i+1), "Agent1", pk, true)
+			fmt.Sprintf("Moniker_%v", i+1), "Agent1", []*bls.PublicKey{pk}, true)
 
 		for s := 0; s < 4; s++ {
 			peerSet.IncreaseSendSuccessCounter(pid)
@@ -262,11 +273,11 @@ func TestGetRandomPeerUnknown(t *testing.T) {
 
 	pk, _ := ts.RandomBLSKeyPair()
 	pidUnknown := peer.ID("peer_unknown")
-	peerSet.UpdatePeerInfo(pidUnknown, StatusCodeUnknown, "Moniker_unknown", "Agent1", pk, true)
+	peerSet.UpdatePeerInfo(pidUnknown, StatusCodeUnknown, "Moniker_unknown", "Agent1", []*bls.PublicKey{pk}, true)
 
 	pk, _ = ts.RandomBLSKeyPair()
 	pidBanned := peer.ID("peer_banned")
-	peerSet.UpdatePeerInfo(pidBanned, StatusCodeBanned, "Moniker_banned", "Agent1", pk, true)
+	peerSet.UpdatePeerInfo(pidBanned, StatusCodeBanned, "Moniker_banned", "Agent1", []*bls.PublicKey{pk}, true)
 
 	p := peerSet.GetRandomPeer()
 
@@ -281,7 +292,7 @@ func TestGetRandomPeerOnePeer(t *testing.T) {
 
 	pk, _ := ts.RandomBLSKeyPair()
 	pid := peer.ID("peer_known")
-	peerSet.UpdatePeerInfo(pid, StatusCodeKnown, "Moniker_known", "Agent1", pk, true)
+	peerSet.UpdatePeerInfo(pid, StatusCodeKnown, "Moniker_known", "Agent1", []*bls.PublicKey{pk}, true)
 	peerSet.IncreaseSendSuccessCounter(pid)
 
 	p := peerSet.GetRandomPeer()