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

[Merged by Bors] - Don't downscore peers on duplicate blocks #4791

Closed
wants to merge 1 commit into from

Conversation

pawanjay176
Copy link
Member

Issue Addressed

N/A

Proposed Changes

We were currently downscoring a peer for sending us a block that we already have in fork choice. This is unnecessary as we get duplicates in lighthouse only when

  1. We published the block, so the block is already in fork choice
  2. We imported the same block over rpc

In both scenarios, the peer who sent us the block over gossip is not at fault.

This isn't exploitable as valid duplicates will get dropped by the gossipsub duplicate filter

@pawanjay176 pawanjay176 added the ready-for-review The code is ready for review label Sep 28, 2023
Copy link
Member

@jimmygchen jimmygchen left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@michaelsproul michaelsproul left a comment

Choose a reason for hiding this comment

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

Good catch, I think this is definitely the right decision!

@jimmygchen jimmygchen added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Sep 28, 2023
@paulhauner
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Sep 29, 2023
## Issue Addressed

N/A

## Proposed Changes

We were currently downscoring a peer for sending us a block that we already have in fork choice. This is unnecessary as we get duplicates in lighthouse only when
1. We published the block, so the block is already in fork choice
2. We imported the same block over rpc

In both scenarios, the peer who sent us the block over gossip is not at fault.

This isn't exploitable as valid duplicates will get dropped by the gossipsub duplicate filter
@bors
Copy link

bors bot commented Sep 29, 2023

This PR was included in a batch that was canceled, it will be automatically retried

@paulhauner
Copy link
Member

bors r-

@bors
Copy link

bors bot commented Sep 29, 2023

Canceled.

@paulhauner
Copy link
Member

bors r+

@paulhauner
Copy link
Member

bors r-

@bors
Copy link

bors bot commented Sep 29, 2023

Canceled.

@paulhauner
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Sep 29, 2023
## Issue Addressed

N/A

## Proposed Changes

We were currently downscoring a peer for sending us a block that we already have in fork choice. This is unnecessary as we get duplicates in lighthouse only when
1. We published the block, so the block is already in fork choice
2. We imported the same block over rpc

In both scenarios, the peer who sent us the block over gossip is not at fault.

This isn't exploitable as valid duplicates will get dropped by the gossipsub duplicate filter
@bors
Copy link

bors bot commented Sep 29, 2023

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Sep 29, 2023
## Issue Addressed

N/A

## Proposed Changes

We were currently downscoring a peer for sending us a block that we already have in fork choice. This is unnecessary as we get duplicates in lighthouse only when
1. We published the block, so the block is already in fork choice
2. We imported the same block over rpc

In both scenarios, the peer who sent us the block over gossip is not at fault.

This isn't exploitable as valid duplicates will get dropped by the gossipsub duplicate filter
@bors
Copy link

bors bot commented Sep 29, 2023

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Sep 29, 2023
## Issue Addressed

N/A

## Proposed Changes

We were currently downscoring a peer for sending us a block that we already have in fork choice. This is unnecessary as we get duplicates in lighthouse only when
1. We published the block, so the block is already in fork choice
2. We imported the same block over rpc

In both scenarios, the peer who sent us the block over gossip is not at fault.

This isn't exploitable as valid duplicates will get dropped by the gossipsub duplicate filter
@bors
Copy link

bors bot commented Sep 29, 2023

Build failed:

@paulhauner
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Oct 3, 2023
## Issue Addressed

N/A

## Proposed Changes

We were currently downscoring a peer for sending us a block that we already have in fork choice. This is unnecessary as we get duplicates in lighthouse only when
1. We published the block, so the block is already in fork choice
2. We imported the same block over rpc

In both scenarios, the peer who sent us the block over gossip is not at fault.

This isn't exploitable as valid duplicates will get dropped by the gossipsub duplicate filter
@bors
Copy link

bors bot commented Oct 4, 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 Don't downscore peers on duplicate blocks [Merged by Bors] - Don't downscore peers on duplicate blocks Oct 4, 2023
@bors bors bot closed this Oct 4, 2023
Woodpile37 pushed a commit to Woodpile37/lighthouse that referenced this pull request Jan 6, 2024
## Issue Addressed

N/A

## Proposed Changes

We were currently downscoring a peer for sending us a block that we already have in fork choice. This is unnecessary as we get duplicates in lighthouse only when
1. We published the block, so the block is already in fork choice
2. We imported the same block over rpc

In both scenarios, the peer who sent us the block over gossip is not at fault.

This isn't exploitable as valid duplicates will get dropped by the gossipsub duplicate filter
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.

4 participants