[Networking] GossipSub Spam Mitigation- Dynamic Decay Speed #4891
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- remove static decay of .99 instead initialize to random decay that is updated based on gossip score - update tests ensure initial decay is between constant lower and upper bounds
- add new GossipSubScoringRegistryConfig struct to hold configs needed for registry that were previously hard coded - update builders
kc1116
commented
Oct 27, 2023
| @@ -136,7 +136,7 @@ func TestInvalidCtrlMsgScoringIntegration(t *testing.T) { | |||
| }) | |||
|
|
|||
| // now simulates node2 spamming node1 with invalid gossipsub control messages. | |||
| for i := 0; i < 30; i++ { | |||
| for i := 0; i < 750; i++ { | |||
There was a problem hiding this comment.
Now that decay will start lower than .99 and only be incremented by default .01 we need to spam more messages in this test so that the node gets disallow listed.
gomisha
reviewed
Oct 30, 2023
gomisha
reviewed
Oct 30, 2023
config/config_test.go
Outdated
| err := c.Validate() | ||
| require.Error(t, err) | ||
| errs, ok := errors.Unwrap(err).(validator.ValidationErrors) | ||
| require.True(t, ok) | ||
| require.Len(t, errs, 2) | ||
| require.Len(t, errs, 3) |
There was a problem hiding this comment.
This only checks that there were 3 errors returned but it doesn't check what those errors were. Would be better to check the actual errors returned.
gomisha
reviewed
Oct 30, 2023
gomisha
reviewed
Oct 30, 2023
gomisha
reviewed
Oct 30, 2023
config/default-config.yml
Outdated
| # The lower bound on the decay value for a spam record when initialized. | ||
| # A random value in a range of InitDecayLowerBound and InitDecayUpperBound is used when initializing the decay | ||
| # of a spam record. | ||
| gossipsub-scoring-registry-init-decay-lowerbound: .5 |
There was a problem hiding this comment.
Suggested change
| gossipsub-scoring-registry-init-decay-lowerbound: .5 | |
| gossipsub-scoring-registry-init-decay-lower-bound: .5 |
gomisha
reviewed
Oct 30, 2023
config/default-config.yml
Outdated
| # of a spam record. | ||
| gossipsub-scoring-registry-init-decay-lowerbound: .5 | ||
| # The upper bound on the decay value for a spam record when initialized. | ||
| gossipsub-scoring-registry-init-decay-upperbound: .7 |
There was a problem hiding this comment.
Suggested change
| gossipsub-scoring-registry-init-decay-upperbound: .7 | |
| gossipsub-scoring-registry-init-decay-upper-bound: .7 |
gomisha
reviewed
Oct 30, 2023
gomisha
reviewed
Oct 30, 2023
gomisha
reviewed
Oct 30, 2023
gomisha
reviewed
Oct 30, 2023
network/p2p/p2pconf/gossipsub.go
Outdated
| // InitDecayLowerBound is the lower bound on the decay value for a spam record when initialized. | ||
| // A random value in a range of InitDecayLowerBound and InitDecayUpperBound is used when initializing the decay | ||
| // of a spam record. | ||
| InitDecayLowerBound float64 `validate:"gt=0,lt=1" mapstructure:"gossipsub-scoring-registry-init-decay-lowerbound"` |
There was a problem hiding this comment.
Suggested change
| InitDecayLowerBound float64 `validate:"gt=0,lt=1" mapstructure:"gossipsub-scoring-registry-init-decay-lowerbound"` | |
| InitDecayLowerBound float64 `validate:"gt=0,lt=1,ltfield=InitDecayUpperBound" mapstructure:"gossipsub-scoring-registry-init-decay-lowerbound"` |
gomisha
reviewed
Oct 30, 2023
network/p2p/scoring/registry.go
Outdated
Comment on lines
354
to
355
| // - decayLowerbound: the lower bound of the range for the initial random decay value | ||
| // - decayUpperbound: the upper bound of the range for the initial random decay value |
There was a problem hiding this comment.
Suggested change
| // - decayLowerbound: the lower bound of the range for the initial random decay value | |
| // - decayUpperbound: the upper bound of the range for the initial random decay value | |
| // - decayLowerBound: the lower bound of the range for the initial random decay value | |
| // - decayUpperBound: the upper bound of the range for the initial random decay value |
gomisha
reviewed
Oct 30, 2023
network/p2p/scoring/registry.go
Outdated
Comment on lines
359
to
362
| func InitAppScoreRecordStateFunc(decayLowerbound, decayUpperbound float64) func() p2p.GossipSubSpamRecord { | ||
| return func() p2p.GossipSubSpamRecord { | ||
| return p2p.GossipSubSpamRecord{ | ||
| Decay: rand.Float64()*(decayUpperbound-decayLowerbound) + decayLowerbound, |
There was a problem hiding this comment.
Suggested change
| func InitAppScoreRecordStateFunc(decayLowerbound, decayUpperbound float64) func() p2p.GossipSubSpamRecord { | |
| return func() p2p.GossipSubSpamRecord { | |
| return p2p.GossipSubSpamRecord{ | |
| Decay: rand.Float64()*(decayUpperbound-decayLowerbound) + decayLowerbound, | |
| func InitAppScoreRecordStateFunc(decayLowerBound, decayUpperBound float64) func() p2p.GossipSubSpamRecord { | |
| return func() p2p.GossipSubSpamRecord { | |
| return p2p.GossipSubSpamRecord{ | |
| Decay: rand.Float64()*(decayUpperBound-decayLowerBound) + decayLowerBound, |
gomisha
reviewed
Oct 30, 2023
gomisha
approved these changes
Oct 30, 2023
Co-authored-by: Misha <15269764+gomisha@users.noreply.github.com>
Co-authored-by: Misha <15269764+gomisha@users.noreply.github.com>
Co-authored-by: Misha <15269764+gomisha@users.noreply.github.com>
Co-authored-by: Misha <15269764+gomisha@users.noreply.github.com>
Co-authored-by: Misha <15269764+gomisha@users.noreply.github.com>
Co-authored-by: Misha <15269764+gomisha@users.noreply.github.com>
Co-authored-by: Misha <15269764+gomisha@users.noreply.github.com>
Co-authored-by: Yahya Hassanzadeh, Ph.D. <yhassanzadeh@ieee.org>
Co-authored-by: Yahya Hassanzadeh, Ph.D. <yhassanzadeh@ieee.org>
Co-authored-by: Yahya Hassanzadeh, Ph.D. <yhassanzadeh@ieee.org>
Co-authored-by: Yahya Hassanzadeh, Ph.D. <yhassanzadeh@ieee.org>
Co-authored-by: Yahya Hassanzadeh, Ph.D. <yhassanzadeh@ieee.org>
…& MaximumSpamPenaltyDecaySpeed -> MaximumSpamPenaltyDecayFactor
…w/flow-go into khalil/6662-peer-id-specific-decay
…eer-id-specific-decay
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR addresses the issue of spam record decay in the GossipSub scoring registry, which currently decays at a constant rate of 0.99, a hard-coded package-level value. This constant decay rate affects both honest and malicious nodes equally, potentially causing network degradation, especially when honest nodes take longer to recover their gossip scores.
To mitigate this issue, this PR introduces a mechanism for dynamic spam record decay. With this enhancement, each spam record is now initialized with a random decay value, denoted as 'n,' falling within the range specified by
...scoringRegistryConfig.InitDecayLowerBoundand...scoringRegistryConfig.InitDecayUpperBound.Furthermore, when a record's penalty reaches the threshold defined by
...scoringRegistryConfig.IncreaseDecayThreshold, the record's decay rate is increased by...scoringRegistryConfig.DecayThresholdIncrementer. This change ensures that nodes engaging in sustained malicious behavior will have spam records with the highest decay rate, resulting in a slower recovery process.By introducing dynamic decay rates for spam records, this PR enhances the fairness and resilience of the GossipSub scoring mechanism, creating a more robust foundation for the network's health and performance.
ref: https://github.com/dapperlabs/flow-go/issues/6662