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

Fix INACTIVITY_PENALTY_QUOTIENT #1712

Merged
merged 1 commit into from
Apr 14, 2020
Merged

Conversation

hwwhww
Copy link
Contributor

@hwwhww hwwhww commented Apr 8, 2020

#1693 alternative

Issue

The amount of inactivity penalty was adjusted to half since we were applying penalty for missing FFG target and source. But now we only apply it for missing target, so INACTIVITY_PENALTY_QUOTIENT should have been 2**24.

How did I solve it

Update INACTIVITY_PENALTY_QUOTIENT from 2**25 to 2**24.

Note that the status-quo 2**25 doesn't break security:
#1693 (comment)

@vbuterin: Though it doesn't break security at all if it's kept at 2**25; it would just mean the leak takes 41% longer, so if this ends up being the only change we need to do then not bothering with it is fine too.

IMO this PR makes the current eth2.0-specs explainers (e.g., Serenity Design Rationale) stay consistent leaking rate. Also, we can avoid making INACTIVITY_PENALTY_QUOTIENT configuration explainer complicated in #1693.

It's not required for testnet, but it would be nice to include this configuration adjustment in the final v1.0.0 ™️ spec.

The amount of inactivity penalty was adjusted to half since we were applying penalty for missing FFG target and source. But now we only apply it for missing target, so `INACTIVITY_PENALTY_QUOTIENT` should be `2**24`.
@djrtwo
Copy link
Contributor

djrtwo commented Apr 8, 2020

Maybe we should maintain a v0.12 branch off of dev that will incorporate the BLS IETF changes and anything minor like this. I'd like to keep dev (or another branch) ready to release v0.11.x releases that have updated network specs other backward compatible changes

@hwwhww hwwhww changed the base branch from dev to v012x April 10, 2020 06:32
@hwwhww
Copy link
Contributor Author

hwwhww commented Apr 10, 2020

@djrtwo sounds good! I added v012x branch for the next minor version upgrade.

Copy link
Contributor

@djrtwo djrtwo left a comment

Choose a reason for hiding this comment

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

great!

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.

None yet

2 participants