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.

Sign up for GitHub

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

Jump to bottom

Validator penalty issues #196

Closed
djrtwo opened this issue Nov 30, 2018 · 3 comments
Closed

Validator penalty issues #196

djrtwo opened this issue Nov 30, 2018 · 3 comments
Labels
general:bug Something isn't working
Milestone
January Release

Comments

@djrtwo
Copy link
Contributor

djrtwo commented Nov 30, 2018

Issue

Within get_changed_validators there is a bug such that validators_to_penalize get penalized at every validator registry change. Penalties should only be applied at withdrawal.

Proposed solution

  • Either add withdrawal logic back in and keep validators in a holding pattern
  • Just remove this penalty logic for now, but note the withdrawal process and associated penalties in an issue/doc
@vbuterin
Copy link
Contributor

vbuterin commented Dec 1, 2018

I'd be ok with adding withdrawal logic back in, and just setting the deletion time to infinity.

We should also add a receipt tree that includes withdrawal receipts (potentially doable as easily as just storing in the state a list of withdrawals in the last block, completely overwriting it each block, though this would make processing harder; the alternative is to have a receipt_root in the block, which would allow more unified receipt processing code on the beacon chain and the shards).

@hwwhww hwwhww added the general:bug Something isn't working label Dec 1, 2018
@djrtwo djrtwo changed the title get_changed_validators repeatedly penalizing Validator penalty issues Dec 11, 2018
@djrtwo
Copy link
Contributor Author

djrtwo commented Dec 11, 2018

Related issue:

COLLECTIVE_PENALTY_CALCULATION_PERIOD and "withdrawal period": In the codes, we use COLLECTIVE_PENALTY_CALCULATION_PERIOD while it is described as "withdrawal period" everywhere.

@djrtwo djrtwo mentioned this issue Dec 11, 2018
Closed
4 tasks
@hwwhww hwwhww added this to the January Release milestone Dec 22, 2018
@hwwhww
Copy link
Contributor

hwwhww commented Dec 23, 2018

link #354

@djrtwo djrtwo closed this as completed Jan 8, 2019
JustinDrake added a commit that referenced this issue May 1, 2019
@JustinDrake
Cleaner bitfields
8e62927 
This draft PR try to fix a longstanding wart (see #196, #371, #667) regarding bitfields. We define bitfields as lists or vectors of `bool`s, as opposed to `bytes`. Benefits:

* Remove ugly-as-hell helper function `verify_bitfield`
* No more bit manipulation with `justification_bitfield`
* Merkleisation-friendly `justification_bitfield` (i.e. `justification_bitfield` does not change unless a new epoch is justified).
* Easy parametrisation of the number of bits in `justification_bitfield` (can be more or fewer than 64—more may be useful for dApps and light clients)
* Semantically cleaner typing

This requires a minor SSZ change (Merkleisation not affected) where lists and vectors of `bool`s should be packed, similar to packing of `uintN`. We could alias `bool` to `uint1` so that the packing logic only needs to be defined once for `uintN`.
@JustinDrake JustinDrake mentioned this issue May 1, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
general:bug Something isn't working
Projects
None yet
Milestone
January Release
Development

No branches or pull requests
3 participants
@djrtwo @vbuterin @hwwhww