Skip to content

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
@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