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

Bring forward changes to withdrawability from phase 1 #615

Merged
merged 6 commits into from
Feb 14, 2019
Merged

Bring forward changes to withdrawability from phase 1 #615

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4d92547
Bring forward changes to withdrawability from phase 1
vbuterin Feb 13, 2019
7198d5b
Fixed a docstring
vbuterin Feb 13, 2019
d1434ac
Update 0_beacon-chain.md
JustinDrake Feb 13, 2019
833831c
Update 0_beacon-chain.md
JustinDrake Feb 13, 2019
2b9f631
Update 0_beacon-chain.md
JustinDrake Feb 13, 2019
a7c44e9
Update 0_beacon-chain.md
JustinDrake Feb 14, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 19 additions & 14 deletions specs/core/0_beacon-chain.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@
- [Attestations](#attestations-1)
- [Deposits](#deposits-1)
- [Exits](#exits-1)
- [Transfers](#transfers-1)
- [Per-epoch processing](#per-epoch-processing)
- [Helper variables](#helper-variables)
- [Eth1 data](#eth1-data-1)
Expand Down Expand Up @@ -232,8 +233,7 @@ Code snippets appearing in `this style` are to be interpreted as Python code.
| `SEED_LOOKAHEAD` | `2**0` (= 1) | epochs | 6.4 minutes |
| `ENTRY_EXIT_DELAY` | `2**2` (= 4) | epochs | 25.6 minutes |
| `ETH1_DATA_VOTING_PERIOD` | `2**4` (= 16) | epochs | ~1.7 hours |
| `MIN_VALIDATOR_WITHDRAWAL_EPOCHS` | `2**8` (= 256) | epochs | ~27 hours |
| `MIN_EXIT_EPOCHS_BEFORE_TRANSFER` | `2**13` (= 8,192) | epochs | ~36 days |
| `MIN_VALIDATOR_WITHDRAWABILITY_DELAY` | `2**8` (= 256) | epochs | ~27 hours |

### State list lengths

Expand Down Expand Up @@ -261,7 +261,6 @@ Code snippets appearing in `this style` are to be interpreted as Python code.
| Name | Value |
| - | - |
| `INITIATED_EXIT` | `2**0` (= 1) |
| `WITHDRAWABLE` | `2**1` (= 2) |

### Max operations per block

Expand Down Expand Up @@ -445,6 +444,8 @@ The following data structures are defined as [SimpleSerialize (SSZ)](https://git
}
```

#### Transfers

##### `Transfer`

```python
Expand Down Expand Up @@ -569,8 +570,8 @@ The following data structures are defined as [SimpleSerialize (SSZ)](https://git
'activation_epoch': 'uint64',
# Epoch when validator exited
'exit_epoch': 'uint64',
# Epoch when validator withdrew
'withdrawal_epoch': 'uint64',
# Epoch when validator is eligible to withdraw
'withdrawable_epoch': 'uint64',
# Epoch when validator was penalized
'penalized_epoch': 'uint64',
# Status flags
Expand Down Expand Up @@ -1298,7 +1299,7 @@ def process_deposit(state: BeaconState,
withdrawal_credentials=withdrawal_credentials,
activation_epoch=FAR_FUTURE_EPOCH,
exit_epoch=FAR_FUTURE_EPOCH,
withdrawal_epoch=FAR_FUTURE_EPOCH,
withdrawable_epoch=FAR_FUTURE_EPOCH,
penalized_epoch=FAR_FUTURE_EPOCH,
status_flags=0,
)
Expand Down Expand Up @@ -1368,6 +1369,7 @@ def penalize_validator(state: BeaconState, index: ValidatorIndex) -> None:
Penalize the validator of the given ``index``.
Note that this function mutates ``state``.
"""
assert state.slot < validator.withdrawable_epoch # [TO BE REMOVED IN PHASE 2]
exit_validator(state, index)
validator = state.validator_registry[index]
state.latest_penalized_balances[get_current_epoch(state) % LATEST_PENALIZED_EXIT_LENGTH] += get_effective_balance(state, index)
Expand All @@ -1377,18 +1379,20 @@ def penalize_validator(state: BeaconState, index: ValidatorIndex) -> None:
state.validator_balances[whistleblower_index] += whistleblower_reward
state.validator_balances[index] -= whistleblower_reward
validator.penalized_epoch = get_current_epoch(state)
validator.withdrawable_epoch = get_current_epoch(state) + LATEST_PENALIZED_EXIT_LENGTH
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need a condition that validators can only be penalized if they have not already entered the withdrawable state? I'm imagining someone exits, enters withdrawable, transfers some amount of funds, and then is found to be slashable. Although it would be nice to penalize them a bit, I think we should not allow this and other unexpected flows because it opens up unexpected state transitions (hard to test, potential attack vectors, etc)

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need a condition that validators can only be penalized if they have not already entered the withdrawable state?

Automatic withdrawals in phase 2 effectively provide that. I'd support adding extra conditions (to be removed in phase 2) for consistency with phase 2.

Copy link
Contributor

Choose a reason for hiding this comment

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

Added assert validator.withdrawable_epoch == FAR_FUTURE_EPOCH # [TO BE REMOVED IN PHASE 2] in penalize_validator.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It needs to be possible to penalize validators that are in the "waiting for PoC challenges" stage. So I'd recommend the simple assert validator.withdrawable_epoch >= state.slot or assert validator.withdrawable_epoch > state.slot + ENTRY_EXIT_DELAY.

```

#### `prepare_validator_for_withdrawal`

```python
def prepare_validator_for_withdrawal(state: BeaconState, index: ValidatorIndex) -> None:
"""
Set the validator with the given ``index`` with ``WITHDRAWABLE`` flag.
Set the validator with the given ``index`` as withdrawable
``MIN_VALIDATOR_WITHDRAWABILITY_DELAY`` after the current epoch.
Note that this function mutates ``state``.
"""
validator = state.validator_registry[index]
validator.status_flags |= WITHDRAWABLE
validator.withdrawable_epoch = get_current_epoch(state) + MIN_VALIDATOR_WITHDRAWABILITY_DELAY
```

## Ethereum 1.0 deposit contract
Expand Down Expand Up @@ -1804,8 +1808,8 @@ For each `transfer` in `block.body.transfers`:
* Verify that `state.validator_balances[transfer.from] >= transfer.amount`.
* Verify that `state.validator_balances[transfer.from] >= transfer.fee`.
* Verify that `state.validator_balances[transfer.from] == transfer.amount + transfer.fee` or `state.validator_balances[transfer.from] >= transfer.amount + transfer.fee + MIN_DEPOSIT_AMOUNT`.
* Verify that `transfer.slot == state.slot`.
* Verify that `get_current_epoch(state) >= state.validator_registry[transfer.from].exit_epoch + MIN_EXIT_EPOCHS_BEFORE_TRANSFER`.
* Verify that `state.slot == transfer.slot`.
* Verify that `get_current_epoch(state) >= state.validator_registry[transfer.from].withdrawable_epoch`.
* Verify that `state.validator_registry[transfer.from].withdrawal_credentials == BLS_WITHDRAWAL_PREFIX_BYTE + hash(transfer.pubkey)[1:]`.
* Let `transfer_message = hash_tree_root(Transfer(from=transfer.from, to=transfer.to, amount=transfer.amount, fee=transfer.fee, slot=transfer.slot, signature=EMPTY_SIGNATURE))`.
* Verify that `bls_verify(pubkey=transfer.pubkey, message_hash=transfer_message, signature=transfer.signature, domain=get_domain(state.fork, slot_to_epoch(transfer.slot), DOMAIN_TRANSFER))`.
Expand Down Expand Up @@ -2065,11 +2069,12 @@ def process_penalties_and_exits(state: BeaconState) -> None:

def eligible(index):
validator = state.validator_registry[index]
if validator.penalized_epoch <= current_epoch:
penalized_withdrawal_epochs = LATEST_PENALIZED_EXIT_LENGTH // 2
return current_epoch >= validator.penalized_epoch + penalized_withdrawal_epochs
# Cannot exit if you already have
if validator.withdrawable_epoch < FAR_FUTURE_EPOCH:
return False
# Can exit if at least the minimum amount of time has passed
else:
return current_epoch >= validator.exit_epoch + MIN_VALIDATOR_WITHDRAWAL_EPOCHS
return current_epoch >= validator.exit_epoch + MIN_VALIDATOR_WITHDRAWABILITY_DELAY
JustinDrake marked this conversation as resolved.
Show resolved Hide resolved

all_indices = list(range(len(state.validator_registry)))
eligible_indices = filter(eligible, all_indices)
Expand Down