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

EIP6914 - Reuse indexes with full sweep #3307

Merged
merged 10 commits into from
Apr 19, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
65 changes: 65 additions & 0 deletions specs/_features/reuse_indexes/beacon-chain.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
# Reuse indexes -- The Beacon Chain
dapplion marked this conversation as resolved.
Show resolved Hide resolved

## Table of contents

<!-- TOC -->
<!-- START doctoc generated TOC please keep comment here to allow auto update -->
<!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE -->

- [Introduction](#introduction)
- [Preset](#preset)
- [Time parameters](#time-parameters)
- [Helpers](#helpers)
- [Predicates](#predicates)
- [`is_reusable_validator`](#is_reusable_validator)
- [Beacon chain state transition function](#beacon-chain-state-transition-function)
- [Block processing](#block-processing)
- [Modified `get_index_for_new_validator`](#modified-get_index_for_new_validator)

<!-- END doctoc generated TOC please keep comment here to allow auto update -->
<!-- /TOC -->

## Introduction

This is the beacon chain specification to assign new deposits to existing validator records that have withdrawn long ago.

*Note:* This specification is built upon [Capella](../../capella/beacon_chain.md) and is under active development.

## Preset

### Time parameters

| Name | Value | Unit | Duration |
| - | - | - |
dapplion marked this conversation as resolved.
Show resolved Hide resolved
| `REUSE_VALIDATOR_INDEX_DELAY` | `uint64(2**16)` (= 65,536) | epochs | ~1 year |
dapplion marked this conversation as resolved.
Show resolved Hide resolved

## Helper functions

### Predicates

#### `is_reusable_validator`

```python
def is_reusable_validator(validator: Validator, balance: Gwei, epoch: Epoch) -> bool:
"""
Check if ``validator`` index can be re-assigned to a new deposit.
"""
return (
validator.withdrawable_epoch < epoch - REUSE_VALIDATOR_INDEX_DELAY
dapplion marked this conversation as resolved.
Show resolved Hide resolved
and balance == 0
dapplion marked this conversation as resolved.
Show resolved Hide resolved
)
```

## Beacon chain state transition function

### Block processing

#### Modified `get_index_for_new_validator`

```python
def get_index_for_new_validator(state: BeaconState) -> int:
for index, validator in enumerate(state.validators):
if is_reusable_validator(validator, state.balances[index], get_current_epoch(state)):
return index
return len(state.validators)
dapplion marked this conversation as resolved.
Show resolved Hide resolved
```
22 changes: 17 additions & 5 deletions specs/altair/beacon-chain.md
Original file line number Diff line number Diff line change
Expand Up @@ -511,16 +511,28 @@ def apply_deposit(state: BeaconState,
signing_root = compute_signing_root(deposit_message, domain)
# Initialize validator if the deposit signature is valid
if bls.Verify(pubkey, signing_root, signature):
state.validators.append(get_validator_from_deposit(pubkey, withdrawal_credentials, amount))
state.balances.append(amount)
index = get_index_for_new_validator(state)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggesting to define add_validator_to_registry (there could be a better name) and doing the following refactor:

Phase0

def add_validator_to_registry(state: BeaconState, validator: Validator) -> None:
    # Add validator and balance entries
    state.validators.append(get_validator_from_deposit(pubkey, withdrawal_credentials, amount))
    state.balances.append(amount)

Replace the two lines with a call to this method

Altair

Modify add_validator_to_registry to be aligned with Altair updates. Which also makes Altair spec leaner as a side effect (no need to redefine entire apply_deposit, just add_validator_to_registry).

Reuse indexes

Modify add_validator_to_registry respectively, and encapsulate get_index_for_new_validator and update or append logic in this feature only.

Copy link
Collaborator Author

@dapplion dapplion Mar 30, 2023

Choose a reason for hiding this comment

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

The intent of update_or_append_to_list was to prevent having to duplicate initialization statements, which will keep growing with every fork. And to keep this doc agnostic of altair initialization itself. No strong opinion on either way. I'll PR the add_validator_to_registry separately since it's mostly orthogonal to the change

update_or_append_to_list(state.validators, index, get_validator_from_deposit(pubkey, withdrawal_credentials, amount))
update_or_append_to_list(state.balances, index, amount)
# [New in Altair]
state.previous_epoch_participation.append(ParticipationFlags(0b0000_0000))
state.current_epoch_participation.append(ParticipationFlags(0b0000_0000))
state.inactivity_scores.append(uint64(0))
update_or_append_to_list(state.previous_epoch_participation, index, ParticipationFlags(0b0000_0000))
update_or_append_to_list(state.current_epoch_participation, index, ParticipationFlags(0b0000_0000))
update_or_append_to_list(state.inactivity_scores, index, uint64(0))
else:
# Increase balance by deposit amount
index = ValidatorIndex(validator_pubkeys.index(pubkey))
increase_balance(state, index, amount)


def get_index_for_new_validator(state: BeaconState) -> int:
return len(state.validators)
dapplion marked this conversation as resolved.
Show resolved Hide resolved


def update_or_append_to_list(list: List, index: int, value: Any) -> None:
dapplion marked this conversation as resolved.
Show resolved Hide resolved
if index == len(list):
list.append(value)
else:
list[index] = value
dapplion marked this conversation as resolved.
Show resolved Hide resolved
```

#### Sync aggregate processing
Expand Down