This repository has been archived by the owner on Jan 22, 2025. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
filters crds values obtained through gossip by their shred version #18072
Merged
behzadnouri
merged 1 commit into
solana-labs:master
from
behzadnouri:enforce-shred-version
Jun 23, 2021
Merged
filters crds values obtained through gossip by their shred version #18072
behzadnouri
merged 1 commit into
solana-labs:master
from
behzadnouri:enforce-shred-version
Jun 23, 2021
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
774d3e9
to
3365bd5
Compare
Codecov Report
@@ Coverage Diff @@
## master #18072 +/- ##
=======================================
Coverage 82.3% 82.3%
=======================================
Files 433 433
Lines 120912 120901 -11
=======================================
+ Hits 99586 99610 +24
+ Misses 21326 21291 -35 |
carllin
reviewed
Jun 23, 2021
gossip/src/cluster_info.rs
Outdated
@@ -3102,6 +3047,70 @@ pub fn stake_weight_peers( | |||
ClusterInfo::sorted_stakes_with_index(peers, stakes) | |||
} | |||
|
|||
// Filters out values from nodes with different shred-version. | |||
fn check_shred_version( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: check_shred_version -> filter_on_shred_version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
carllin
previously approved these changes
Jun 23, 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks!
filter_by_shred_version does not check the shred-version of the owner of the crds-value. It only checks the shred-version of the node which is relaying the value: https://github.com/solana-labs/solana/blob/5cc073420/gossip/src/cluster_info.rs#L2274-L2289 So crds-values with different shred versions can still pass through this function as long as they are relayed by a node with matching shred version; and so, a single node can bridge different shred values through-out the cluster.
3365bd5
to
aece480
Compare
mergify bot
pushed a commit
that referenced
this pull request
Jun 23, 2021
…18072) filter_by_shred_version does not check the shred-version of the owner of the crds-value. It only checks the shred-version of the node which is relaying the value: https://github.com/solana-labs/solana/blob/5cc073420/gossip/src/cluster_info.rs#L2274-L2289 So crds-values with different shred versions can still pass through this function as long as they are relayed by a node with matching shred version; and so, a single node can bridge different shred values through-out the cluster. (cherry picked from commit 69a5f0e) # Conflicts: # gossip/src/cluster_info.rs
mergify bot
added a commit
that referenced
this pull request
Jun 23, 2021
…ackport #18072) (#18175) * filters crds values obtained through gossip by their shred version (#18072) filter_by_shred_version does not check the shred-version of the owner of the crds-value. It only checks the shred-version of the node which is relaying the value: https://github.com/solana-labs/solana/blob/5cc073420/gossip/src/cluster_info.rs#L2274-L2289 So crds-values with different shred versions can still pass through this function as long as they are relayed by a node with matching shred version; and so, a single node can bridge different shred values through-out the cluster. (cherry picked from commit 69a5f0e) # Conflicts: # gossip/src/cluster_info.rs * removes backport merge conflicts Co-authored-by: behzad nouri <[email protected]>
CriesofCarrots
pushed a commit
to CriesofCarrots/solana
that referenced
this pull request
Jun 28, 2021
…rsion (solana-labs#18072)" This reverts commit 69a5f0e.
Closed
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Problem
filter_by_shred_version does not check the shred-version of the owner of
the crds-value. It only checks the shred-version of the node which is
relaying the value:
https://github.com/solana-labs/solana/blob/5cc073420/gossip/src/cluster_info.rs#L2274-L2289
So crds-values with different shred versions can still pass through this
function as long as they are relayed by a node with matching shred
version; and so, a single node can bridge different shred values
through-out the cluster.
see comments in #17940
Summary of Changes