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

Pseudo slashing for invalid signers #4269

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
8da8b53
added new field for consensus message - InvalidSigners
ssd04 Jul 4, 2022
765a83b
first implementation for pseudo slashing
ssd04 Jul 6, 2022
514622f
temp version for p2p pubsub
ssd04 Jul 6, 2022
6dcfb39
add custom type for invalid signers
ssd04 Jul 7, 2022
7b78941
first implementation for pseudo slashing: fix unit tests
ssd04 Jul 7, 2022
947f8b8
validity checks for invalisSigners consensus message type + use json …
ssd04 Jul 7, 2022
044b165
p2p: added additional field with marshalled pubsub message to p2p mes…
ssd04 Jul 8, 2022
c2dece2
use original marshalled p2p message instead of json marshalling
ssd04 Jul 8, 2022
c70ac05
added new field for consensus message - number of InvalidSigners
ssd04 Jul 11, 2022
d5e629f
reverted new field for consensus message - number of InvalidSigners
ssd04 Jul 12, 2022
46968f9
added interface for p2p message handling
ssd04 Jul 12, 2022
fbdc098
use p2p message signing component in subround endround
ssd04 Jul 12, 2022
5e2d918
subround endround: unit test for invalid signers verification
ssd04 Jul 12, 2022
53e3bec
p2p: remove marshalled data field
ssd04 Jul 13, 2022
d01da9f
consensus: use p2p message instead of marshalled data
ssd04 Jul 13, 2022
73c46eb
unit tests for consensus core and message validator
ssd04 Jul 13, 2022
9919dc3
cleanup unused core:
ssd04 Jul 13, 2022
a0b5f11
consensus: more unit tests for end round + comment updates
ssd04 Jul 13, 2022
fccb94d
flag check for invalid signers + peer honesty change
ssd04 Jul 13, 2022
6cd0194
fix should send flag on error
ssd04 Jul 13, 2022
d185fe3
bls worker: more checks for invalid signers consensus message
ssd04 Jul 19, 2022
4736186
bls worker: more checks for invalid signers consensus message - fix u…
ssd04 Jul 19, 2022
887d6ed
use single signer to verify sig share; add keyGenerator to consensus …
ssd04 Jul 19, 2022
c818111
use single signer to verify sig share; add keyGenerator to consensus …
ssd04 Jul 19, 2022
9250e77
fix mocks: remove reference to peer blacklist
ssd04 Jul 19, 2022
86e1b51
add message signer mock
ssd04 Jul 19, 2022
4f3b53f
consensus state: rlock for get message with signature
ssd04 Aug 2, 2022
984ac90
Merge branch 'feat/optimise-consensus-sigcheck' into pseudo-slashing-…
ssd04 Sep 15, 2022
67f385a
spos endround: comments and log updates
ssd04 Sep 15, 2022
e6609d0
consensus worker: remove TODO comment
ssd04 Sep 15, 2022
d759b60
Merge branch 'feat/optimise-consensus-sigcheck' into merge-optimize-c…
ssd04 Sep 26, 2022
33334b0
Merge pull request #4509 from ElrondNetwork/merge-optimize-consensus-…
ssd04 Sep 26, 2022
f850519
Merge branch 'feat/optimise-consensus-sigcheck' into merge-optimize-c…
ssd04 Oct 5, 2022
f60633f
Merge pull request #4557 from ElrondNetwork/merge-optimize-consensus-…
ssd04 Oct 5, 2022
4272475
fixes after review: more check, small refactoring, comments update
ssd04 Oct 19, 2022
d92dc33
fix linter issue
ssd04 Oct 19, 2022
387e6f9
added more unit tests for received invalid signers info
ssd04 Oct 19, 2022
63d7e98
fix comment typo
ssd04 Oct 20, 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
2 changes: 1 addition & 1 deletion consensus/mock/consensusDataContainerMock.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ func (ccm *ConsensusCoreMock) PrivateKey() crypto.PrivateKey {
return ccm.blsPrivateKey
}

// SingleSigner returns the bls single signer stored in the ConsensusStore
// SingleSigner returns the bls single signer stored in the ConsensusCore
func (ccm *ConsensusCoreMock) SingleSigner() crypto.SingleSigner {
return ccm.blsSingleSigner
}
Expand Down
42 changes: 27 additions & 15 deletions consensus/spos/bls/subroundEndRound.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,35 +158,48 @@ func (sr *subroundEndRound) isBlockHeaderFinalInfoValid(cnsDta *consensus.Messag

// receivedInvalidSignersInfo method is called when a message with invalid signers has been received
func (sr *subroundEndRound) receivedInvalidSignersInfo(_ context.Context, cnsDta *consensus.Message) bool {
AdoAdoAdo marked this conversation as resolved.
Show resolved Hide resolved
node := string(cnsDta.PubKey)
messageSender := string(cnsDta.PubKey)

if !sr.IsConsensusDataSet() {
return false
}

if !sr.IsNodeLeaderInCurrentRound(node) { // is NOT this node leader in current round?
if !sr.IsNodeLeaderInCurrentRound(messageSender) { // is NOT this node leader in current round?
sr.PeerHonestyHandler().ChangeScore(
node,
messageSender,
spos.GetConsensusTopicID(sr.ShardCoordinator()),
spos.LeaderPeerHonestyDecreaseFactor,
)

return false
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I will also add here the following checks, just to avoid some edge case situations, exactly as they are used in the receivedFinalInfo method:

	if sr.IsSelfLeaderInCurrentRound() {
		return false
	}

	if !sr.IsConsensusDataEqual(cnsDta.BlockHeaderHash) {
		return false
	}

	if !sr.CanProcessReceivedMessage(cnsDta, sr.RoundHandler().Index(), sr.Current()) {
		return false
	}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

if sr.IsSelfLeaderInCurrentRound() {
return false
}

if !sr.IsConsensusDataEqual(cnsDta.BlockHeaderHash) {
return false
}

if !sr.CanProcessReceivedMessage(cnsDta, sr.RoundHandler().Index(), sr.Current()) {
return false
}

if len(cnsDta.InvalidSigners) == 0 {
return false
}

err := sr.verifyInvalidSigners(cnsDta.InvalidSigners)
if err != nil {
log.Trace("receivedInvalidSignersInfo.verifyInvalidSigners", "error", err.Error())
return false
}

log.Debug("step 3: invalid signers info has been evaluated")

sr.PeerHonestyHandler().ChangeScore(
node,
messageSender,
spos.GetConsensusTopicID(sr.ShardCoordinator()),
spos.LeaderPeerHonestyIncreaseFactor,
)
Expand Down Expand Up @@ -298,7 +311,6 @@ func (sr *subroundEndRound) doEndRoundJobByLeader() bool {
}

invalidSigners := make([]byte, 0)
shouldSendInvalidSigners := false

err = sr.SignatureHandler().Verify(sr.GetData(), bitmap, sr.Header.GetEpoch())
if err != nil {
Expand All @@ -310,12 +322,16 @@ func (sr *subroundEndRound) doEndRoundJobByLeader() bool {
return false
}

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

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

bitmap, sig, err = sr.computeAggSigOnValidNodes()
if err != nil {
log.Debug("doEndRoundJobByLeader.computeAggSigOnValidNodes", "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.

Also in the case of insufficient threshold of valid signatures I think we should call sr.createAndBroadcastInvalidSigners(invalidSigners). I think this method should be called mandatory if we have invalidSigners set, no matter if we will exit from the current method earlier.

One suggestion is to replace L313-L317 with the code below and remove L365-L367 and L300-L301 as method getFullMessagesForInvalidSigners could return only two parameters, not needed the middle one.

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Expand Down Expand Up @@ -362,9 +378,7 @@ func (sr *subroundEndRound) doEndRoundJobByLeader() bool {
return false
}

if shouldSendInvalidSigners {
sr.createAndBroadcastInvalidSigners(invalidSigners)
}
// broadcast header and final info section

// create and broadcast header final info
sr.createAndBroadcastHeaderFinalInfo()
Expand Down Expand Up @@ -442,7 +456,7 @@ func (sr *subroundEndRound) verifyNodesOnAggSigVerificationFail() ([]string, err
}

// use increase factor since it was added optimistically, and it proved to be wrong
decreaseFactor := -spos.ValidatorPeerHonestyIncreaseFactor
decreaseFactor := -spos.ValidatorPeerHonestyIncreaseFactor + spos.ValidatorPeerHonestyDecreaseFactor
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

sr.PeerHonestyHandler().ChangeScore(
pk,
spos.GetConsensusTopicID(sr.ShardCoordinator()),
Expand All @@ -458,8 +472,7 @@ func (sr *subroundEndRound) verifyNodesOnAggSigVerificationFail() ([]string, err
return invalidPubKeys, nil
}

func (sr *subroundEndRound) getFullMessagesForInvalidSigners(invalidPubKeys []string) ([]byte, bool, error) {
shouldSend := false
func (sr *subroundEndRound) getFullMessagesForInvalidSigners(invalidPubKeys []string) ([]byte, error) {
p2pMessages := make([]p2p.MessageP2P, 0)

for _, pk := range invalidPubKeys {
Expand All @@ -469,15 +482,14 @@ func (sr *subroundEndRound) getFullMessagesForInvalidSigners(invalidPubKeys []st
}

p2pMessages = append(p2pMessages, p2pMsg)
shouldSend = true
}

invalidSigners, err := sr.MessageSigningHandler().Serialize(p2pMessages)
if err != nil {
return nil, false, err
return nil, err
}

return invalidSigners, shouldSend, nil
return invalidSigners, nil
}

func (sr *subroundEndRound) computeAggSigOnValidNodes() ([]byte, []byte, error) {
Expand Down
17 changes: 0 additions & 17 deletions consensus/spos/bls/subroundEndRound_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1230,23 +1230,6 @@ func TestSubroundEndRound_ReceivedInvalidSignersInfo(t *testing.T) {
assert.False(t, res)
})

t.Run("cannot process message", func(t *testing.T) {
t.Parallel()

container := mock.InitConsensusCore()

sr := *initSubroundEndRoundWithContainer(container, &statusHandler.AppStatusHandlerStub{})
sr.SetSelfPubKey("A")

cnsData := consensus.Message{
BlockHeaderHash: []byte("X"),
PubKey: []byte("A"),
}

res := sr.ReceivedInvalidSignersInfo(&cnsData)
assert.False(t, res)
})

t.Run("empty invalid signers", func(t *testing.T) {
t.Parallel()

Expand Down
6 changes: 3 additions & 3 deletions consensus/spos/consensusCore.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,17 +180,17 @@ func (cc *ConsensusCore) EpochStartRegistrationHandler() epochStart.Registration
return cc.epochStartRegistrationHandler
}

// PrivateKey returns the BLS private key stored in the ConsensusStore
// PrivateKey returns the BLS private key stored in the ConsensusCore
func (cc *ConsensusCore) PrivateKey() crypto.PrivateKey {
return cc.blsPrivateKey
}

// SingleSigner returns the bls single signer stored in the ConsensusStore
// SingleSigner returns the bls single signer stored in the ConsensusCore
func (cc *ConsensusCore) SingleSigner() crypto.SingleSigner {
return cc.blsSingleSigner
}

// KeyGenerator returns the bls key generator stored in the ConsensusStore
// KeyGenerator returns the bls key generator stored in the ConsensusCore
func (cc *ConsensusCore) KeyGenerator() crypto.KeyGenerator {
return cc.keyGenerator
}
Expand Down
8 changes: 4 additions & 4 deletions consensus/spos/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,11 @@ type ConsensusCoreHandler interface {
NodesCoordinator() nodesCoordinator.NodesCoordinator
// EpochStartRegistrationHandler gets the RegistrationHandler stored in the ConsensusCore
EpochStartRegistrationHandler() epochStart.RegistrationHandler
// PrivateKey returns the private key stored in the ConsensusStore used for randomness and leader's signature generation
// PrivateKey returns the private key stored in the ConsensusCore used for randomness and leader's signature generation
PrivateKey() crypto.PrivateKey
// SingleSigner returns the single signer stored in the ConsensusStore used for randomness and leader's signature generation
// SingleSigner returns the single signer stored in the ConsensusCore used for randomness and leader's signature generation
SingleSigner() crypto.SingleSigner
// KeyGenerator returns the key generator stored in the ConsensusStore
// KeyGenerator returns the key generator stored in the ConsensusCore
KeyGenerator() crypto.KeyGenerator
// PeerHonestyHandler returns the peer honesty handler which will be used in subrounds
PeerHonestyHandler() consensus.PeerHonestyHandler
Expand All @@ -65,7 +65,7 @@ type ConsensusCoreHandler interface {
NodeRedundancyHandler() consensus.NodeRedundancyHandler
// ScheduledProcessor returns the scheduled txs processor
ScheduledProcessor() consensus.ScheduledProcessor
// MessageSignerHandler
// MessageSignerHandler return the p2p signing handler
Copy link
Contributor

Choose a reason for hiding this comment

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

returns

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

MessageSigningHandler() consensus.P2PSigningHandler
// SignatureHandler returns the signature handler component
SignatureHandler() consensus.SignatureHandler
Expand Down