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

[Networking] Handling iHave broken promises (and part-2 of overpromising) spams #4566

Merged
merged 50 commits into from
Jul 19, 2023
Merged
Changes from 1 commit
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
6aadc4d
adds scoring parameters for mesh message delivery
yhassanzadeh13 Jul 5, 2023
b6a4630
refactors config-based approach to override-based approach
yhassanzadeh13 Jul 5, 2023
60d39d4
generates mocks
yhassanzadeh13 Jul 5, 2023
7742875
adds a documentation
yhassanzadeh13 Jul 5, 2023
c8b3a96
extends godoc
yhassanzadeh13 Jul 5, 2023
e3ce3cc
adds test skeleton
yhassanzadeh13 Jul 5, 2023
76d05a2
fixes test
yhassanzadeh13 Jul 5, 2023
ea4309b
implements under-performing test
yhassanzadeh13 Jul 6, 2023
1cbe332
adds test for under-delivery in two topics
yhassanzadeh13 Jul 6, 2023
a31489d
refactors test fixture
yhassanzadeh13 Jul 6, 2023
c763b97
wip
yhassanzadeh13 Jul 10, 2023
572f1d0
err fix
yhassanzadeh13 Jul 11, 2023
d589916
applies refactoring
yhassanzadeh13 Jul 11, 2023
7c21e79
fixes test
yhassanzadeh13 Jul 12, 2023
93e31f8
parallelize the fixture
yhassanzadeh13 Jul 12, 2023
9ed306d
adds logs for overriding parameters
yhassanzadeh13 Jul 12, 2023
2fef6e8
adds logging for network type to builder
yhassanzadeh13 Jul 12, 2023
889ade0
fmt
yhassanzadeh13 Jul 12, 2023
6da4f4c
renames a fixture function
yhassanzadeh13 Jul 12, 2023
6ae3d82
adds warn logging for overriding score parameters
yhassanzadeh13 Jul 12, 2023
f31d2da
revises godocs
yhassanzadeh13 Jul 12, 2023
2509a7a
adds readme
yhassanzadeh13 Jul 12, 2023
0788b8b
fixes scoring test
yhassanzadeh13 Jul 12, 2023
85eb327
fixing lint
yhassanzadeh13 Jul 12, 2023
6f7eb69
lint fix
yhassanzadeh13 Jul 12, 2023
c52a3eb
Merge remote-tracking branch 'origin/master' into yahya/6459-ihave-br…
yhassanzadeh13 Jul 12, 2023
0310507
improves test quality
yhassanzadeh13 Jul 12, 2023
5962dfb
enables behavior penalty
yhassanzadeh13 Jul 14, 2023
990b15b
adds size method to protected map
yhassanzadeh13 Jul 17, 2023
977f826
refactors gossipsub parameters
yhassanzadeh13 Jul 17, 2023
2110da7
implements spammer constructor with inspector
yhassanzadeh13 Jul 17, 2023
e614680
implements broken promises test (below threshold)
yhassanzadeh13 Jul 17, 2023
3023693
adds ihave spam tests
yhassanzadeh13 Jul 17, 2023
36ba284
adds godoc
yhassanzadeh13 Jul 18, 2023
6f9f8f7
extends godocs
yhassanzadeh13 Jul 18, 2023
26b441a
extends comments
yhassanzadeh13 Jul 18, 2023
94be8d8
extends readme
yhassanzadeh13 Jul 18, 2023
1879b01
revises parameters
yhassanzadeh13 Jul 18, 2023
50866b3
updates tests
yhassanzadeh13 Jul 18, 2023
dce1e56
Merge remote-tracking branch 'origin/master' into yahya/6459-ihave-br…
yhassanzadeh13 Jul 18, 2023
9c43ee2
updates a godoc
yhassanzadeh13 Jul 18, 2023
8a7f29c
updates godocs
yhassanzadeh13 Jul 18, 2023
2501502
fixes a godoc
yhassanzadeh13 Jul 18, 2023
908bd5e
Merge branch 'master' into yahya/6459-ihave-broken-promises-part-2
yhassanzadeh13 Jul 19, 2023
8fbf9a4
adds t.Parallel
yhassanzadeh13 Jul 19, 2023
ea8d7d9
Merge remote-tracking branch 'origin/yahya/6459-ihave-broken-promises…
yhassanzadeh13 Jul 19, 2023
9f23780
adds t.Parallel to all other tests
yhassanzadeh13 Jul 19, 2023
1551546
Merge branch 'master' into yahya/6459-ihave-broken-promises-part-2
yhassanzadeh13 Jul 19, 2023
3d29c56
fmt
yhassanzadeh13 Jul 19, 2023
80d470e
Merge remote-tracking branch 'origin/yahya/6459-ihave-broken-promises…
yhassanzadeh13 Jul 19, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
adds test for under-delivery in two topics
  • Loading branch information
yhassanzadeh13 committed Jul 6, 2023
commit 1cbe33250e4e51b8bfeb43fe883a5783cf4fe84c
Original file line number Diff line number Diff line change
@@ -18,6 +18,7 @@ import (
"github.com/onflow/flow-go/network/p2p"
"github.com/onflow/flow-go/network/p2p/scoring"
p2ptest "github.com/onflow/flow-go/network/p2p/test"
validator "github.com/onflow/flow-go/network/validator/pubsub"
"github.com/onflow/flow-go/utils/unittest"
)

@@ -187,7 +188,9 @@ func TestGossipSubMeshDeliveryScoring_UnderDelivery_SingleTopic(t *testing.T) {
// we override some of the default scoring parameters in order to speed up the test in a time-efficient manner.
blockTopicOverrideParams := scoring.DefaultTopicScoreParams()
blockTopicOverrideParams.MeshMessageDeliveriesActivation = 1 * time.Second // we start observing the mesh message deliveries after 1 second of the node startup.
thisNode, thisId := p2ptest.NodeFixture( // this node is the one that will be penalizing the under-performer node.
dkgTopicOverrideParams := scoring.DefaultTopicScoreParams()
dkgTopicOverrideParams.MeshMessageDeliveriesActivation = 1 * time.Second // we start observing the mesh message deliveries after 1 second of the node startup.
thisNode, thisId := p2ptest.NodeFixture( // this node is the one that will be penalizing the under-performer node.
t,
sporkId,
t.Name(),
@@ -270,3 +273,113 @@ func TestGossipSubMeshDeliveryScoring_UnderDelivery_SingleTopic(t *testing.T) {
return unittest.ProposalFixture(), blockTopic
})
}

// TestGossipSubMeshDeliveryScoring_UnderDelivery_TwoTopics tests that when a peer is under-performing in two topics, it is penalized in both topics.
func TestGossipSubMeshDeliveryScoring_UnderDelivery_TwoTopics(t *testing.T) {
role := flow.RoleConsensus
sporkId := unittest.IdentifierFixture()

idProvider := mock.NewIdentityProvider(t)
ctx, cancel := context.WithCancel(context.Background())
signalerCtx := irrecoverable.NewMockSignalerContext(t, ctx)

blockTopic := channels.TopicFromChannel(channels.PushBlocks, sporkId)
dkgTopic := channels.TopicFromChannel(channels.DKGCommittee, sporkId)

// we override some of the default scoring parameters in order to speed up the test in a time-efficient manner.
blockTopicOverrideParams := scoring.DefaultTopicScoreParams()
blockTopicOverrideParams.MeshMessageDeliveriesActivation = 1 * time.Second // we start observing the mesh message deliveries after 1 second of the node startup.
dkgTopicOverrideParams := scoring.DefaultTopicScoreParams()
dkgTopicOverrideParams.MeshMessageDeliveriesActivation = 1 * time.Second // we start observing the mesh message deliveries after 1 second of the node startup.
thisNode, thisId := p2ptest.NodeFixture( // this node is the one that will be penalizing the under-performer node.
t,
sporkId,
t.Name(),
idProvider,
p2ptest.WithRole(role),
p2ptest.WithPeerScoreTracerInterval(1*time.Second),
p2ptest.WithPeerScoringEnabled(
&p2p.PeerScoringConfigOverride{
TopicScoreParams: map[channels.Topic]*pubsub.TopicScoreParams{
blockTopic: blockTopicOverrideParams,
dkgTopic: dkgTopicOverrideParams,
},
DecayInterval: 1 * time.Second, // we override the decay interval to 1 second so that the score is updated within 1 second intervals.
}),
)

underPerformerNode, underPerformerId := p2ptest.NodeFixture(
t,
sporkId,
t.Name(),
idProvider,
p2ptest.WithRole(role),
)

idProvider.On("ByPeerID", thisNode.Host().ID()).Return(&thisId, true).Maybe()
idProvider.On("ByPeerID", underPerformerNode.Host().ID()).Return(&underPerformerId, true).Maybe()
ids := flow.IdentityList{&underPerformerId, &thisId}
nodes := []p2p.LibP2PNode{underPerformerNode, thisNode}

p2ptest.StartNodes(t, signalerCtx, nodes, 100*time.Millisecond)
defer p2ptest.StopNodes(t, nodes, cancel, 2*time.Second)

p2ptest.LetNodesDiscoverEachOther(t, ctx, nodes, ids)
p2ptest.TryConnectionAndEnsureConnected(t, ctx, nodes)

// subscribe to the topics.
for _, node := range nodes {
for _, topic := range []channels.Topic{blockTopic, dkgTopic} {
_, err := node.Subscribe(topic, validator.TopicValidator(unittest.Logger(), unittest.AllowAllPeerFilter()))
require.NoError(t, err)
}
}

// Initially the under-performing node should have a score that is at least equal to the MaxAppSpecificReward.
// The reason is in our scoring system, we reward the staked nodes by MaxAppSpecificReward, and the under-performing node is considered staked
// as it is in the id provider of thisNode.
require.Eventually(t, func() bool {
underPerformingNodeScore, ok := thisNode.PeerScoreExposer().GetScore(underPerformerNode.Host().ID())
if !ok {
return false
}
if underPerformingNodeScore < scoring.MaxAppSpecificReward {
// ensure the score is high enough so that gossip is routed by victim node to spammer node.
return false
}

return true
}, 2*time.Second, 100*time.Millisecond)

// No message delivery happens intentionally, so that the under-performing node is penalized.

// however, after one decay interval, we expect the score of the under-performing node to be penalized by ~ 2 * -0.05 * MaxAppSpecificReward.
require.Eventually(t, func() bool {
underPerformingNodeScore, ok := thisNode.PeerScoreExposer().GetScore(underPerformerNode.Host().ID())
if !ok {
return false
}
if underPerformingNodeScore > 0.91*scoring.MaxAppSpecificReward { // score must be penalized by ~ 2 * -0.05 * MaxAppSpecificReward.
return false
}
if underPerformingNodeScore < scoring.DefaultGossipThreshold { // even the node is slightly penalized, it should still be able to gossip with this node.
return false
}
if underPerformingNodeScore < scoring.DefaultPublishThreshold { // even the node is slightly penalized, it should still be able to publish to this node.
return false
}
if underPerformingNodeScore < scoring.DefaultGraylistThreshold { // even the node is slightly penalized, it should still be able to establish rpc connection with this node.
return false
}

return true
}, 3*time.Second, 100*time.Millisecond)

// even though the under-performing node is penalized, it should still be able to publish and receive messages from this node in both topic meshes.
p2ptest.EnsurePubsubMessageExchange(t, ctx, nodes, func() (interface{}, channels.Topic) {
return unittest.ProposalFixture(), blockTopic
})
p2ptest.EnsurePubsubMessageExchange(t, ctx, nodes, func() (interface{}, channels.Topic) {
return unittest.DKGMessageFixture(), dkgTopic
})
}
1 change: 1 addition & 0 deletions network/p2p/p2pnode/gossipSubAdapter.go
Original file line number Diff line number Diff line change
@@ -158,6 +158,7 @@ func (g *GossipSubAdapter) Join(topic string) (p2p.Topic, error) {
topicParamsLogger.Info().Msg("joined topic with score params set")
} else {
g.logger.Warn().
Bool(logging.KeySecurity, true).
Str("topic", topic).
Msg("joining topic without score params, this is not recommended from a security perspective")
}
12 changes: 9 additions & 3 deletions utils/logging/consts.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
package logging

// KeySuspicious is a logging label that is used to flag the log event as suspicious behavior
// This is used to add an easily searchable label to the log event
const KeySuspicious = "suspicious"
const (
// KeySuspicious is a logging label that is used to flag the log event as suspicious behavior
// This is used to add an easily searchable label to the log event
KeySuspicious = "suspicious"

// KeySecurity is a logging label that is used to flag the log event as a security issue
// This is used to add an easily searchable label to the log event
KeySecurity = "security"
)