Skip to content

[Merged by Bors] - Do not attempt to resubscribe to core topics#4271

Closed
jmcph4 wants to merge 2 commits intosigp:unstablefrom
jmcph4:4258-resubscribe-core-topics
Closed

[Merged by Bors] - Do not attempt to resubscribe to core topics#4271
jmcph4 wants to merge 2 commits intosigp:unstablefrom
jmcph4:4258-resubscribe-core-topics

Conversation

@jmcph4
Copy link
Contributor

@jmcph4 jmcph4 commented May 8, 2023

This commit adds a check to the networking service when handling core gossipsub topic subscription requests. If the BN is already subscribed to the core topics, we won't attempt to resubscribe.

Issue Addressed

#4258

Proposed Changes

  • In the networking service, check if we're already subscribed to all of the core gossipsub topics and, if so, do nothing

Additional Info

N/A

This commit adds a check to the networking service when handling core
gossipsub topic subscription requests. If the BN is already subscribed
to the core topics, this call becomes a no-op.
@jmcph4 jmcph4 changed the base branch from stable to unstable May 8, 2023 00:38
@divagant-martian divagant-martian changed the title Add topic subscription check to networking service Do not attempt to resubscribe to core topics May 8, 2023
Copy link
Member

@AgeManning AgeManning left a comment

Choose a reason for hiding this comment

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

This looks good to me. Nice work

@AgeManning AgeManning added the ready-for-merge This PR is ready to merge. label May 8, 2023
@AgeManning
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request May 8, 2023
This commit adds a check to the networking service when handling core gossipsub topic subscription requests. If the BN is already subscribed to the core topics, we won't attempt to resubscribe.

## Issue Addressed

#4258 

## Proposed Changes

 - In the networking service, check if we're already subscribed to all of the core gossipsub topics and, if so, do nothing

## Additional Info

N/A
@bors
Copy link

bors bot commented May 8, 2023

Pull request successfully merged into unstable.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot changed the title Do not attempt to resubscribe to core topics [Merged by Bors] - Do not attempt to resubscribe to core topics May 8, 2023
@bors bors bot closed this May 8, 2023
ghost pushed a commit to oone-world/lighthouse that referenced this pull request Jul 13, 2023
This commit adds a check to the networking service when handling core gossipsub topic subscription requests. If the BN is already subscribed to the core topics, we won't attempt to resubscribe.

## Issue Addressed

sigp#4258 

## Proposed Changes

 - In the networking service, check if we're already subscribed to all of the core gossipsub topics and, if so, do nothing

## Additional Info

N/A
Woodpile37 pushed a commit to Woodpile37/lighthouse that referenced this pull request Jan 6, 2024
This commit adds a check to the networking service when handling core gossipsub topic subscription requests. If the BN is already subscribed to the core topics, we won't attempt to resubscribe.

## Issue Addressed

sigp#4258 

## Proposed Changes

 - In the networking service, check if we're already subscribed to all of the core gossipsub topics and, if so, do nothing

## Additional Info

N/A
Woodpile37 pushed a commit to Woodpile37/lighthouse that referenced this pull request Jan 6, 2024
This commit adds a check to the networking service when handling core gossipsub topic subscription requests. If the BN is already subscribed to the core topics, we won't attempt to resubscribe.

## Issue Addressed

sigp#4258 

## Proposed Changes

 - In the networking service, check if we're already subscribed to all of the core gossipsub topics and, if so, do nothing

## Additional Info

N/A
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-merge This PR is ready to merge.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants