Skip to content

In-protocol consolidation - target liable#10

Closed
dapplion wants to merge 5 commits into
michaelneuder:maxeb-pyspec-min-viablefrom
dapplion:maxeb-consolidation-liable
Closed

In-protocol consolidation - target liable#10
dapplion wants to merge 5 commits into
michaelneuder:maxeb-pyspec-min-viablefrom
dapplion:maxeb-consolidation-liable

Conversation

@dapplion
Copy link
Copy Markdown
Collaborator

@dapplion dapplion commented Aug 1, 2023

A Consolidation operation moves all balance from validator source to validator target without leaving the active set. This operation is authorized by the source withdrawal credentials, and the target validating key.

Slashing liability

In this implementation

  • source is liable for target's slashable offenses before the consolidation event
  • target is liable for source's slashable offenses before the consolidation event

Rationale

Preventing liability between source and target validator's actions adds significant protocol complexity. During a consolidating event, operators only need to ensure their keys do not produce slashing offenses during MIN_VALIDATOR_WITHDRAWABILITY_DELAY. Outside of that time window the risk profile is identical to status quo.

Comment thread specs/_features/maxeb_increase/capella.py
Comment thread specs/_features/maxeb_increase/capella.py Outdated
Comment thread specs/_features/maxeb_increase/capella.py Outdated
dapplion and others added 2 commits August 8, 2023 16:20
Co-authored-by: Mikhail Kalinin <noblesse.knight@gmail.com>
Co-authored-by: Mikhail Kalinin <noblesse.knight@gmail.com>
@dapplion dapplion force-pushed the maxeb-consolidation-liable branch 2 times, most recently from 2f489c0 to 80bc11c Compare November 14, 2023 13:47
assert source_validator.exit_epoch == FAR_FUTURE_EPOCH

# verify source withdrawal credentials, which have authority over validating keys
assert source_validator.withdrawal_credentials[:1] == ETH1_ADDRESS_WITHDRAWAL_PREFIX
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
assert source_validator.withdrawal_credentials[:1] == ETH1_ADDRESS_WITHDRAWAL_PREFIX
assert has_eth1_withdrawal_credential(source_validator) or has_compounding_withdrawal_credential(source_validator)

I think consolidation should be allowed for both of these cred types


source_validator.consolidated_to = consolidation.target_index
# Balance is not exiting the active set, do not apply churn
source_validator.exit_epoch = get_current_epoch(state)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Note: instant exit breaks a MAX_SEED_LOOKAHEAD invariant. I don’t see anything is immediately broken by that but might worth keeping in mind. Target validator can also be made liable to the source’s duties if that need be (if the protocol had any duties assigned to a validator for the MAX_SEED_LOOKAHEAD epochs into the future).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Btw, setting exit epoch this way makes changes of duty_dependent_root possible to happen within an epoch which I guess would entail an update in VCs logic.

source_validator = state[consolidation.source_index]

assert target_validator.exit_epoch == FAR_FUTURE_EPOCH
assert source_validator.exit_epoch == FAR_FUTURE_EPOCH
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think we should add the following checks to the source (exiting) validator (from process_voluntary_exits):

# Verify the validator is active
assert is_active_validator(source_validator, get_current_epoch(state))
# Verify the validator has been active long enough
assert get_current_epoch(state) >= source_validator.activation_epoch + config.SHARD_COMMITTEE_PERIOD

exit_epoch: Epoch
withdrawable_epoch: Epoch # When validator can withdraw funds
# TODO: may compress into some other validator field
consolidated_to: ValidatorIndex
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

From our Devconnect discussion:

If adding an extra field will become a concern, one of the alternatives is to rename slashed to e.g. flags and introduce CONSOLIDATED_FLAG_INDEX to use activation_eligibility_epoch (the field that is not used anywhere else other than assigning activation_epoch) as a consolidated_to if the flag is set.

@dapplion
Copy link
Copy Markdown
Collaborator Author

Closing in favour of

@dapplion dapplion closed this Jan 21, 2024
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.

3 participants