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

Reverse broadcast-slashing flag to disable-broadcast-slashings #5952

Merged
merged 7 commits into from May 22, 2020

Conversation

0xKiwi
Copy link
Contributor

@0xKiwi 0xKiwi commented May 22, 2020

What type of PR is this?
Flag reversal

What does this PR do? Why is it needed?
This PR deprecates the --broadcast-slashing flag, and introduces a --disable-broadcast-slashings that blocks slashings that are submitted into the beacon pool from being propagated. Reversing allows every node with a slasher to by default, propagate the slashings submitted.

@0xKiwi 0xKiwi requested a review from a team as a code owner May 22, 2020 02:35
@0xKiwi 0xKiwi added the Ready For Review A pull request ready for code review label May 22, 2020
@0xKiwi 0xKiwi changed the title Reverse broadcast-slashings to disable-broadcast-slashings Reverse broadcast-slashing flag to disable-broadcast-slashings May 22, 2020
@terencechain
Copy link
Member

Test failed @0xKiwi

@codecov
Copy link

codecov bot commented May 22, 2020

Codecov Report

Merging #5952 into master will increase coverage by 0.09%.
The diff coverage is 51.06%.

@@            Coverage Diff             @@
##           master    #5952      +/-   ##
==========================================
+ Coverage   59.47%   59.56%   +0.09%     
==========================================
  Files         319      318       -1     
  Lines       26820    26777      -43     
==========================================
- Hits        15950    15949       -1     
+ Misses       8706     8662      -44     
- Partials     2164     2166       +2     

@@ -4,16 +4,19 @@ import (
"context"
"testing"

"github.com/prysmaticlabs/prysm/shared/featureconfig"

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

@@ -197,9 +197,9 @@ func ConfigureBeaconChain(ctx *cli.Context) {
log.Warn("Enabling state reference copy")
cfg.EnableStateRefCopy = true
}
if ctx.Bool(broadcastSlashingFlag.Name) {
if ctx.Bool(disableBroadcastSlashingFlag.Name) {
log.Warn("Enabling broadcast slashing to p2p network")
Copy link
Member

Choose a reason for hiding this comment

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

This log message doesn't seem right

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, thank you

@0xKiwi 0xKiwi requested a review from terencechain May 22, 2020 15:10
@rauljordan
Copy link
Contributor

PTAL @terencechain

@prylabs-bulldozer prylabs-bulldozer bot merged commit 6aabea2 into master May 22, 2020
@delete-merged-branch delete-merged-branch bot deleted the toggle-broadcast-flag branch May 22, 2020 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready For Review A pull request ready for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants