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

Ambiguity around partial withdrawal #3911

Open
ensi321 opened this issue Sep 2, 2024 · 4 comments
Open

Ambiguity around partial withdrawal #3911

ensi321 opened this issue Sep 2, 2024 · 4 comments

Comments

@ensi321
Copy link
Contributor

ensi321 commented Sep 2, 2024

Description

The term partial withdrawal is ambiguous in Electra.

In Capella and Deneb, partial withdrawal refers to withdrawals on the portion of the validator balance with exceeds 32 ETH created during withdrawal sweep.

Although Capella spec does not directly use the term partial withdrawal, but its spec tests use it extensively. Wordings include:

  • Spec
    • is_partially_withdrawable_validator
  • Spec test
    • partial_withdrawals_indices
    • num_partial_withdrawals
    • run_random_partial_withdrawals_test
    • test_partial_withdrawal_in_epoch_transition
    • _run_activate_and_partial_withdrawal

In Electra, EIP-7002 and EIP-7251 adds partial withdrawals from execution payload. Wordings include:

  • Spec
    • PENDING_PARTIAL_WITHDRAWALS_LIMIT
    • state.pending_partial_withdrawals
    • partial_withdrawals_count
  • Spec test
    • partial_withdrawal
    • test_partial_withdrawal_request_with_low_amount
    • expected_partial_withdrawal

Therefore partial withdrawals can be interpreted as partial withdrawal from withdrawal sweep, or EL-triggered partial withdrawal in Electra.

Suggestion

Number of ways we can mitigate this. But any suggestion is welcome:

  • Avoid using "partial withdrawal" in Capella spec test. Try to use "partially withdrawables" instead.
  • Be explicit with the source of partial withdrawals. For example, partial withdrawal from withdrawal sweep -> partial_withdrawal_from_sweep, EL triggered partial withdrawal -> execution_partial_withdrawal.

Thanks @twoeths for bringing this up.

@ensi321 ensi321 changed the title Ambiguity of partial withdrawal Ambiguity around partial withdrawal Sep 2, 2024
@potuz
Copy link
Contributor

potuz commented Sep 3, 2024

I think the term "partial withdrawal" is self-explanatory: it is a withdrawal of a part of the total balance. As opposed to "total withdrawal" which the spec refers to as "full withdrawal" or just "withdrawal" meaning to withdraw all of the balance in the validator. I do not oppose adding wording to clarify this, but also seems to me that it's clear enough.

@ralexstokes
Copy link
Member

+1 to potuz

partial withdrawal just means withdrawal of stake above some security-relevant amount (e.g. MAX_EFFECTIVE_BALANCE pre-7251)

we are accumulating multiple ways to "generate" partial withdrawals which does complicate the validator life cycle graph but this seems unavoidable given the features we want to support

also support better/more explicit wording but we should keep in mind where we want complexity of various kinds (operational, semantic) to live

@mkalinin
Copy link
Collaborator

mkalinin commented Sep 4, 2024

I see the contradiction with is_partially_withdrawable_validator because this function would return False for compounding validator when MIN_ACTIVATION_BALANCE <= validator.effective_balance < MAX_EFFECTIVE_BALANCE_ELECTRA, while such validator is partially withdrawable via EL withdrawal requests. Ideally, the name of this function could be more specific

@mkalinin
Copy link
Collaborator

We could probably rename pending_partial_withdrawals to pending_withdrawal_requests seems to be more precise name and can also get rid of the ambiguity with partial withdrawals induced by the sweep.

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

No branches or pull requests

4 participants