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

Use the mesh_n value from NetworkLoad for PeerScoreSettings #5013

Merged
merged 3 commits into from
Apr 11, 2024

Conversation

jimmygchen
Copy link
Member

@jimmygchen jimmygchen commented Dec 18, 2023

Issue Addressed

We're using the libp2p default gs_config here for PeerScoreSettings:

let score_settings = PeerScoreSettings::new(ctx.chain_spec, &config.gs_config);

and then we build a Lighthouse specific one here and use it for Gossipsub

let snappy_transform = SnappyTransform::new(config.gs_config.max_transmit_size());
let mut gossipsub = Gossipsub::new_with_subscription_filter_and_transform(
MessageAuthenticity::Anonymous,
config.gs_config.clone(),
gossipsub_metrics,
filter,
snappy_transform,
)

The only value that is used for initialising PeerScoreSettings from the gs_config appears to be the mesh degree mesh_n, which is taken from NetworkLoad.

Proposed Changes

Build the correct gs_config and use the mesh_n value for PeerScoreSetttings.
Note that the score settings needs a bit of rework, so this would just be a temporary fix.

The impact of this fix is that scoring will now use the mesh_n from NetworkLoad (default to 4) rather than the libp2p2 hard coded value (6).

@jimmygchen jimmygchen added Networking work-in-progress PR is a work-in-progress labels Dec 18, 2023
@jimmygchen
Copy link
Member Author

Marking this as work-in-progress as this PR is mainly for tracking and need some review from @AgeManning

@AgeManning AgeManning self-assigned this Jan 10, 2024
# Conflicts:
#	beacon_node/lighthouse_network/src/config.rs
#	beacon_node/lighthouse_network/src/service/gossipsub_scoring_parameters.rs
#	beacon_node/lighthouse_network/src/service/mod.rs
@jimmygchen jimmygchen added ready-for-review The code is ready for review and removed work-in-progress PR is a work-in-progress labels Apr 11, 2024
Copy link
Member

@pawanjay176 pawanjay176 left a comment

Choose a reason for hiding this comment

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

So much cleaner!

@pawanjay176 pawanjay176 added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Apr 11, 2024
@realbigsean
Copy link
Member

@Mergifyio queue

Copy link

mergify bot commented Apr 11, 2024

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at d30ba97

mergify bot added a commit that referenced this pull request Apr 11, 2024
@mergify mergify bot merged commit d30ba97 into sigp:unstable Apr 11, 2024
29 checks passed
@jimmygchen jimmygchen deleted the peer-score-settings branch April 12, 2024 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Networking ready-for-merge This PR is ready to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants