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

Prevent Reinsertion of Orphaned Attestations Into Pool #9442

Merged
merged 13 commits into from
Aug 24, 2021

Conversation

terencechain
Copy link
Member

Fixes #9441

This ensures we can re-insert orphaned attestations back to the attestation pool for later inclusion into beacon block. Attestation that is one epoch old will get filtered out

@terencechain terencechain self-assigned this Aug 21, 2021
@terencechain terencechain requested a review from a team as a code owner August 21, 2021 17:01
@terencechain terencechain requested review from rauljordan, prestonvanloon and nisdas and removed request for a team August 21, 2021 17:01
@codecov
Copy link

codecov bot commented Aug 21, 2021

Codecov Report

Merging #9442 (1f033da) into develop (cac1e94) will increase coverage by 0.50%.
The diff coverage is 51.13%.

@@             Coverage Diff             @@
##           develop    #9442      +/-   ##
===========================================
+ Coverage    60.41%   60.92%   +0.50%     
===========================================
  Files          581      581              
  Lines        42830    42915      +85     
===========================================
+ Hits         25877    26146     +269     
+ Misses       13054    12804     -250     
- Partials      3899     3965      +66     

Copy link
Member

@prestonvanloon prestonvanloon left a comment

Choose a reason for hiding this comment

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

Please add feature flag for testing

prestonvanloon
prestonvanloon previously approved these changes Aug 23, 2021
@rauljordan rauljordan changed the title Can re-insert orphaned attestation Prevent Reinsertion of Orphaned Attestations Into Pool Aug 23, 2021
@@ -143,6 +144,12 @@ func (s *Service) saveHead(ctx context.Context, headRoot [32]byte) error {
},
})

if featureconfig.Get().CorrectlyInsertOrphanedAtts {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's move this inside of the saveOrphanedAtts function so that callers dont need to keep track of feature flags

rauljordan
rauljordan previously approved these changes Aug 23, 2021
@prylabs-bulldozer prylabs-bulldozer bot merged commit 5c96a27 into develop Aug 24, 2021
@delete-merged-branch delete-merged-branch bot deleted the save-orphaned-att branch August 24, 2021 03:05
prestonvanloon pushed a commit that referenced this pull request Aug 26, 2021
* Can re-save orphaned attestation

* Go fmt

* Fix tests

* Go fmt

* Bug fix flag

* Bug fix flag

* Go fmt

Co-authored-by: Raul Jordan <raul@prysmaticlabs.com>
Co-authored-by: prylabs-bulldozer[bot] <58059840+prylabs-bulldozer[bot]@users.noreply.github.com>
(cherry picked from commit 5c96a27)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Proper re-insertion of orphaned attestation
3 participants