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] - Refuse to sign sync committee messages when head is optimistic #3191

Closed
wants to merge 1 commit into from

Conversation

macladson
Copy link
Member

@macladson macladson commented May 19, 2022

Issue Addressed

Resolves #3151

Proposed Changes

When fetching duties for sync committee contributions, check the value of execution_optimistic of the head block from the BN and refuse to sign any sync committee messages if execution_optimistic == true.

Additional Info

  • Is backwards compatible with older BNs
  • Finding a way to add test coverage for this would be prudent. Open to suggestions.

@macladson macladson added bellatrix Required to support the Bellatrix Upgrade work-in-progress PR is a work-in-progress blocked labels May 19, 2022
@macladson macladson force-pushed the sync-com-opt branch 2 times, most recently from 17387af to 749d8ec Compare July 13, 2022 02:18
@paulhauner paulhauner added the v2.5.0 Required for Goerli merge release label Jul 20, 2022
@macladson macladson removed the blocked label Jul 25, 2022
@macladson macladson added ready-for-review The code is ready for review and removed work-in-progress PR is a work-in-progress labels Jul 26, 2022
Copy link
Member

@paulhauner paulhauner left a comment

Choose a reason for hiding this comment

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

Beautiful!

I'm happy to merge as-is!

@paulhauner paulhauner added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Jul 26, 2022
@paulhauner
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Jul 26, 2022
## Issue Addressed

Resolves #3151 

## Proposed Changes

When fetching duties for sync committee contributions, check the value of `execution_optimistic` of the head block from the BN and refuse to sign any sync committee messages `if execution_optimistic == true`.

## Additional Info
- Is backwards compatible with older BNs
- Finding a way to add test coverage for this would be prudent. Open to suggestions.
@bors
Copy link

bors bot commented Jul 26, 2022

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Jul 26, 2022
## Issue Addressed

Resolves #3151 

## Proposed Changes

When fetching duties for sync committee contributions, check the value of `execution_optimistic` of the head block from the BN and refuse to sign any sync committee messages `if execution_optimistic == true`.

## Additional Info
- Is backwards compatible with older BNs
- Finding a way to add test coverage for this would be prudent. Open to suggestions.
@bors
Copy link

bors bot commented Jul 26, 2022

Build failed:

@michaelsproul
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Jul 27, 2022
## Issue Addressed

Resolves #3151 

## Proposed Changes

When fetching duties for sync committee contributions, check the value of `execution_optimistic` of the head block from the BN and refuse to sign any sync committee messages `if execution_optimistic == true`.

## Additional Info
- Is backwards compatible with older BNs
- Finding a way to add test coverage for this would be prudent. Open to suggestions.
@bors bors bot changed the title Refuse to sign sync committee messages when head is optimistic [Merged by Bors] - Refuse to sign sync committee messages when head is optimistic Jul 27, 2022
@bors bors bot closed this Jul 27, 2022
@macladson macladson deleted the sync-com-opt branch July 27, 2022 03:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bellatrix Required to support the Bellatrix Upgrade ready-for-merge This PR is ready to merge. v2.5.0 Required for Goerli merge release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants