Skip to content

In-protocol consolidation - target liable #10

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

Closed
dapplion wants to merge 5 commits into from
Closed

In-protocol consolidation - target liable #10

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ee0bda4
In-protocol consolidation - target not liable
michaelneuder Aug 1, 2023
a325a97
Target is liable
dapplion Aug 1, 2023
95209f8
Update specs/_features/maxeb_increase/capella.py
dapplion Aug 8, 2023
80bc11c
Update specs/_features/maxeb_increase/capella.py
dapplion Aug 8, 2023
f000312
fixes
dapplion Nov 19, 2023
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
63 changes: 63 additions & 0 deletions specs/_features/maxeb_increase/capella.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,8 @@ def floorlog2(x: int) -> uint64:
DOMAIN_BLS_TO_EXECUTION_CHANGE = DomainType('0x0A000000')
SLASHED_ATTESTER_FLAG_INDEX = 0
SLASHED_PROPOSER_FLAG_INDEX = 1
DOMAIN_CONSOLIDATION = DomainType('0x0B000000')
UNSET_CONSOLIDATED_TO = Epoch(2**64 - 1)

# Preset vars
MAX_COMMITTEES_PER_SLOT = uint64(64)
Expand Down Expand Up @@ -321,6 +323,8 @@ class Validator(Container):
activation_epoch: Epoch
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

@mkalinin mkalinin Nov 27, 2023

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.



class AttestationData(Container):
Expand Down Expand Up @@ -621,6 +625,13 @@ class SignedBLSToExecutionChange(Container):
signature: BLSSignature


class Consolidation(Container):
source_index: ValidatorIndex
target_index: ValidatorIndex
target_signature: BLSSignature
source_address: ExecutionAddress


class BeaconBlockBody(Container):
randao_reveal: BLSSignature
eth1_data: Eth1Data # Eth1 data vote
Expand Down Expand Up @@ -1203,6 +1214,7 @@ def slash_validator(state: BeaconState,
"""
Slash the validator with index ``slashed_index``.
"""
slashed_index = resolve_slashed_index(state, slashed_index)
epoch = get_current_epoch(state)
initiate_validator_exit(state, slashed_index)
validator = state.validators[slashed_index]
Expand Down Expand Up @@ -1776,6 +1788,49 @@ def process_execution_layer_withdraw_request(
))


def is_consolidated(validator: Validator) -> bool:
validator.consolidated_to != UNSET_CONSOLIDATED_TO


def resolve_slashed_index(state: BeaconState, index: ValidatorIndex) -> ValidatorIndex:
if is_consolidated(state.validators[index]):
# Recursively resolve consolidated index
return resolve_slashed_index(state, state.validators[index].consolidated_to)
else:
return index


def process_consolidation(state: BeaconState, consolidation: Consolidation) -> None:
target_validator = state[consolidation.target_index]
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

@mkalinin mkalinin Nov 27, 2023

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


# 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

@mkalinin mkalinin Nov 27, 2023

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

assert source_validator.withdrawal_credentials[12:] == consolidation.source_address

# target validator accepts consolidation operation onto itself
# Fork-agnostic domain since address changes are valid across forks
domain = compute_domain(DOMAIN_CONSOLIDATION, genesis_validators_root=state.genesis_validators_root)
signing_root = compute_signing_root(source_validator.pubkey, domain)
assert bls.Verify(target_validator.pubkey, signing_root, consolidation.target_signature)

active_balance = min(state.balances[consolidation.source_index], get_max_effective_balance(source_validator))
excess_balance = state.balances[consolidation.source_index] - active_balance
state.balances[consolidation.target_index] += active_balance
state.balances[consolidation.source_index] = 0

# Excess balance above current active balance ceil, send to inbound churn
state.pending_balance_deposits.append(PendingBalanceDeposit(consolidation.target_index, excess_balance))

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)
Comment thread
dapplion marked this conversation as resolved.
Copy link
Copy Markdown
Collaborator

@mkalinin mkalinin Nov 27, 2023

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

@mkalinin mkalinin Nov 27, 2023

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.withdrawable_epoch = Epoch(source_validator.exit_epoch + config.MIN_VALIDATOR_WITHDRAWABILITY_DELAY)


def process_proposer_slashing(state: BeaconState, proposer_slashing: ProposerSlashing) -> None:
header_1 = proposer_slashing.signed_header_1.message
header_2 = proposer_slashing.signed_header_2.message
Expand Down Expand Up @@ -3668,6 +3723,14 @@ def is_fully_withdrawable_validator(validator: Validator, balance: Gwei, epoch:
and balance > 0
)


def get_max_effective_balance(validator: Validator) -> Gwei:
if (has_compounding_withdrawal_credential(validator):
return MAX_EFFECTIVE_BALANCE
else:
return MIN_ACTIVATION_BALANCE


def get_validator_excess_balance(validator: Validator, balance: Gwei) -> Gwei:
"""
Get excess balance for partial withdrawals for ``validator``.
Expand Down