-
Notifications
You must be signed in to change notification settings - Fork 208
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
Dockernet Slash Test Configuration #330
Conversation
473b75d
to
cd717bf
Compare
scripts/vars.sh
Outdated
@@ -65,6 +65,7 @@ HOST_WEEK_EPOCH_DURATION="60s" | |||
UNBONDING_TIME="120s" | |||
MAX_DEPOSIT_PERIOD="30s" | |||
VOTING_PERIOD="30s" | |||
HARSHEN_SLASHING=false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add a slightly longer comment describing what this does?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can - although, do you think we should just enable these by default? I think you have to manually kill containers for any of these configurations to matter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Although I think we can slightly lessen the harshness, and agreed with Aidan that a comment would be helpful
scripts/init_chain.sh
Outdated
# Optionally harshen slashing parameters | ||
if [[ "$HARSHEN_SLASHING" == "true" ]]; then | ||
sed -i -E 's|"signed_blocks_window": "100"|"signed_blocks_window": "10"|g' $genesis_config | ||
sed -i -E 's|"min_signed_per_window": ""0.500000000000000000"|"min_signed_per_window": ""1.000000000000000000"|g' $genesis_config |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 seems a bit too harsh (will we get any accidental slashes + jailing if a container gets too slow?), should we do 0.85 instead? Requires them to be down for 2 blocks which is still quite fast!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I can make it a little more conservative! I also just realized our blocks are 1s.
However, I do think we should either keep it strict and keep it optional, or relax it a tad and enable these by default. I'm not really sure how often slow containers cause missed blocks.
Maybe we leave the min_signed_per_window at 0.5, do signed_blocks_window as either 10 or 20, and then keep these by default (i.e. remove the HASHEN_SLASHING param)?
Then I think they have to miss 5 or 10 blocks which is probably both unlikely to occur by accident and fast enough when it occurs intentionally?
cd717bf
to
4f27b3f
Compare
Closes: #XXX
Context and purpose of the change
We often need to test out functionality related to slashing (e.g. updating validator weights after a slash). This PR adds configuration settings to dockernet to harshen parameters related to slashing, enabling a quicker dev cycle.
Brief Changelog
vars.sh
calledHARSHEN_SLASHING
, which, if enabled to true:Author's Checklist
I have...
If skipped any of the tests above, explain.
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
Documentation and Release Note
Unreleased
section inCHANGELOG.md
?How is the feature or change documented?
XXX
x/<module>/spec/
)