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.

Sign up for GitHub

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

Jump to bottom

Pseudo slashing fixes #4630

Merged
merged 5 commits into from
Oct 24, 2022
Merged

Pseudo slashing fixes #4630

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f24d5b0
handle error if aggregate sig creation fails
ssd04 Oct 24, 2022
dd0e57d
small refactoring to remove bool var
ssd04 Oct 24, 2022
ddea3c1
update unit test and integration test
ssd04 Oct 24, 2022
10f19dc
Merge branch 'feat/optimise-consensus-sigcheck' into pseudo-slashing-…
ssd04 Oct 24, 2022
d71c6aa
fixes after review: refactorings
ssd04 Oct 24, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion consensus/spos/bls/export_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ func (sr *subroundEndRound) IsOutOfTime() bool {
}

func (sr *subroundEndRound) VerifyNodesOnAggSigVerificationFail() ([]string, error) {
return sr.verifyNodesOnAggSigVerificationFail()
return sr.verifyNodesOnAggSigFail()
}

func (sr *subroundEndRound) ComputeAggSigOnValidNodes() ([]byte, []byte, error) {
Expand All @@ -317,3 +317,7 @@ func (sr *subroundEndRound) VerifyInvalidSigners(invalidSigners []byte) error {
func GetStringValue(messageType consensus.MessageType) string {
return getStringValue(messageType)
}

func (sr *subroundEndRound) CreateAndBroadcastInvalidSigners(invalidSigners []byte) {
sr.createAndBroadcastInvalidSigners(invalidSigners)
}
68 changes: 40 additions & 28 deletions consensus/spos/bls/subroundEndRound.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,39 +302,28 @@ func (sr *subroundEndRound) doEndRoundJobByLeader() bool {
sig, err := sr.SignatureHandler().AggregateSigs(bitmap, sr.Header.GetEpoch())
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can extract L301-L328 in a separate method which will return bitmap, sig and err, actually you can directly return from there sr.handleInvalidSignersOnAggSigFail(), and make the code easier to read there without so much if else levels. Here you should have only something like this:

bitmap, sig, err := sr.aggregateSigsAndHandleInvalidSigners(bitmap)
if err != nil {
	log.Debug("doEndRoundJobByLeader.aggregateSigsAndHandleInvalidSigners", "error", err.Error())
	return false
}

if err != nil {
log.Debug("doEndRoundJobByLeader.AggregateSigs", "error", err.Error())
return false
}

err = sr.SignatureHandler().SetAggregatedSig(sig)
if err != nil {
log.Debug("doEndRoundJobByLeader.SetAggregatedSig", "error", err.Error())
return false
}

err = sr.SignatureHandler().Verify(sr.GetData(), bitmap, sr.Header.GetEpoch())
if err != nil {
log.Debug("doEndRoundJobByLeader.Verify", "error", err.Error())

invalidPubKeys, err := sr.verifyNodesOnAggSigVerificationFail()
bitmap, sig, err = sr.handleInvalidSignersOnAggSigFail()
if err != nil {
log.Debug("doEndRoundJobByLeader.verifyNodesOnAggSigVerificationFail", "error", err.Error())
log.Debug("doEndRoundJobByLeader.handleInvalidSignersOnAggSigFail", "error", err.Error())
return false
}

invalidSigners, err := sr.getFullMessagesForInvalidSigners(invalidPubKeys)
} else {
err = sr.SignatureHandler().SetAggregatedSig(sig)
if err != nil {
log.Debug("doEndRoundJobByLeader.getFullMessagesForInvalidSigners", "error", err.Error())
log.Debug("doEndRoundJobByLeader.SetAggregatedSig", "error", err.Error())
return false
}

if len(invalidSigners) > 0 {
sr.createAndBroadcastInvalidSigners(invalidSigners)
}

bitmap, sig, err = sr.computeAggSigOnValidNodes()
err = sr.SignatureHandler().Verify(sr.GetData(), bitmap, sr.Header.GetEpoch())
if err != nil {
log.Debug("doEndRoundJobByLeader.computeAggSigOnValidNodes", "error", err.Error())
return false
log.Debug("doEndRoundJobByLeader.Verify", "error", err.Error())

bitmap, sig, err = sr.handleInvalidSignersOnAggSigFail()
if err != nil {
log.Debug("doEndRoundJobByLeader.handleInvalidSignersOnAggSigFail", "error", err.Error())
return false
}
}
}

Expand Down Expand Up @@ -422,10 +411,7 @@ func (sr *subroundEndRound) doEndRoundJobByLeader() bool {
return true
}

// TODO:
// - slashing for invalid sig shares
// - handle sig share verifications concurrently
func (sr *subroundEndRound) verifyNodesOnAggSigVerificationFail() ([]string, error) {
func (sr *subroundEndRound) verifyNodesOnAggSigFail() ([]string, error) {
invalidPubKeys := make([]string, 0)
pubKeys := sr.ConsensusGroup()

Expand Down Expand Up @@ -491,6 +477,32 @@ func (sr *subroundEndRound) getFullMessagesForInvalidSigners(invalidPubKeys []st
return invalidSigners, nil
}

func (sr *subroundEndRound) handleInvalidSignersOnAggSigFail() ([]byte, []byte, error) {
invalidPubKeys, err := sr.verifyNodesOnAggSigFail()
if err != nil {
log.Debug("doEndRoundJobByLeader.verifyNodesOnAggSigVerificationFail", "error", err.Error())
Copy link
Contributor

Choose a reason for hiding this comment

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

.verifyNodesOnAggSigFail

return nil, nil, err
}

invalidSigners, err := sr.getFullMessagesForInvalidSigners(invalidPubKeys)
if err != nil {
log.Debug("doEndRoundJobByLeader.getFullMessagesForInvalidSigners", "error", err.Error())
return nil, nil, err
}

if len(invalidSigners) > 0 {
sr.createAndBroadcastInvalidSigners(invalidSigners)
}

bitmap, sig, err := sr.computeAggSigOnValidNodes()
if err != nil {
log.Debug("doEndRoundJobByLeader.computeAggSigOnValidNodes", "error", err.Error())
return nil, nil, err
}

return bitmap, sig, nil
}

func (sr *subroundEndRound) computeAggSigOnValidNodes() ([]byte, []byte, error) {
threshold := sr.Threshold(sr.Current())
numValidSigShares := sr.ComputeSize(SrSignature)
Expand Down
29 changes: 29 additions & 0 deletions consensus/spos/bls/subroundEndRound_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package bls_test

import (
"errors"
"sync"
"testing"
"time"

Expand Down Expand Up @@ -1470,3 +1471,31 @@ func TestVerifyInvalidSigners(t *testing.T) {
require.Nil(t, err)
})
}

func TestSubroundEndRound_CreateAndBroadcastInvalidSigners(t *testing.T) {
t.Parallel()

wg := &sync.WaitGroup{}
wg.Add(1)

expectedInvalidSigners := []byte("invalid signers")

wasCalled := false
container := mock.InitConsensusCore()
messenger := &mock.BroadcastMessengerMock{
BroadcastConsensusMessageCalled: func(message *consensus.Message) error {
wg.Done()
assert.Equal(t, expectedInvalidSigners, message.InvalidSigners)
wasCalled = true
return nil
},
}
container.SetBroadcastMessenger(messenger)
sr := *initSubroundEndRoundWithContainer(container, &statusHandler.AppStatusHandlerStub{})

sr.CreateAndBroadcastInvalidSigners(expectedInvalidSigners)

wg.Wait()

require.True(t, wasCalled)
}
12 changes: 11 additions & 1 deletion integrationTests/consensus/consensusSigning_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package consensus

import (
"bytes"
"encoding/hex"
"fmt"
"sync"
Expand Down Expand Up @@ -44,7 +45,16 @@ func initNodesWithTestSigner(
fmt.Println("invalid sig share from ",
getPkEncoded(nodes[shardID][iCopy].NodeKeys.Pk),
)
invalidSigShare, _ := hex.DecodeString("2ee350b9a821e20df97ba487a80b0d0ffffca7da663185cf6a562edc7c2c71e3ca46ed71b31bccaf53c626b87f2b6e08")

var invalidSigShare []byte
if i%2 == 0 {
// invalid sig share but with valid format
invalidSigShare, _ = hex.DecodeString("2ee350b9a821e20df97ba487a80b0d0ffffca7da663185cf6a562edc7c2c71e3ca46ed71b31bccaf53c626b87f2b6e08")
} else {
// sig share with invalid size
invalidSigShare = bytes.Repeat([]byte("a"), 3)
}

return invalidSigShare, nil
}
}
Expand Down