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

Add test verifying zero-amount pending withdrawal is correctly processed #4107

Closed
wants to merge 1 commit into from

Conversation

bshastry
Copy link
Contributor

Adds a new test called test_pending_withdrawal_amount_zero_ineffective_sweep to verify:

  • Processing of zero-amount pending withdrawals
  • Interaction with sweep eligibility
  • State cleanup after processing

@jtraglia
Copy link
Member

Hey @bshastry, a zero-amount withdrawal isn't a possible situation.

See here, a zero-amount withdrawal request is considered a full exit:

def process_withdrawal_request(
state: BeaconState,
withdrawal_request: WithdrawalRequest
) -> None:
amount = withdrawal_request.amount
is_full_exit_request = amount == FULL_EXIT_REQUEST_AMOUNT

| `FULL_EXIT_REQUEST_AMOUNT` | `uint64(0)` | *[New in Electra:EIP7002]* Withdrawal amount used to signal a full validator exit |

@bshastry
Copy link
Contributor Author

Hey @bshastry, a zero-amount withdrawal isn't a possible situation.

See here, a zero-amount withdrawal request is considered a full exit:

def process_withdrawal_request(
state: BeaconState,
withdrawal_request: WithdrawalRequest
) -> None:
amount = withdrawal_request.amount
is_full_exit_request = amount == FULL_EXIT_REQUEST_AMOUNT

| `FULL_EXIT_REQUEST_AMOUNT` | `uint64(0)` | *[New in Electra:EIP7002]* Withdrawal amount used to signal a full validator exit |

Thank you so much for pointing out. Well, I suppose that leaves us with three options (or potentially both)

  1. Test for a full exit perhaps add an assertion that the validator exited after the zero-amount withdrawal was requested?
  2. Test for the minimal non-zero amount e.g., 1, and assert that it was processed correctly
  3. Add tests for both (1) and (2)

Wdyt?

@jtraglia
Copy link
Member

jtraglia commented Jan 30, 2025

Hey @bshastry, those are good test ideas but they already exist 😄

Test for a full exit perhaps add an assertion that the validator exited after the zero-amount withdrawal was requested?

def test_basic_withdrawal_request(spec, state):
rng = random.Random(1337)
# move state forward SHARD_COMMITTEE_PERIOD epochs to allow for exit
state.slot += spec.config.SHARD_COMMITTEE_PERIOD * spec.SLOTS_PER_EPOCH
current_epoch = spec.get_current_epoch(state)
validator_index = rng.choice(spec.get_active_validator_indices(state, current_epoch))
validator_pubkey = state.validators[validator_index].pubkey
address = b"\x22" * 20
set_eth1_withdrawal_credential_with_balance(
spec, state, validator_index, address=address
)
withdrawal_request = spec.WithdrawalRequest(
source_address=address,
validator_pubkey=validator_pubkey,
amount=spec.FULL_EXIT_REQUEST_AMOUNT,
)
yield from run_withdrawal_request_processing(
spec, state, withdrawal_request
)

The assertions are in the run_withdrawal_request_processing helper function:

if withdrawal_request.amount == spec.FULL_EXIT_REQUEST_AMOUNT:
assert pre_exit_epoch == spec.FAR_FUTURE_EPOCH
assert state.validators[validator_index].exit_epoch < spec.FAR_FUTURE_EPOCH
assert spec.get_pending_balance_to_withdraw(state, validator_index) == 0
assert state.pending_partial_withdrawals == pre_pending_partial_withdrawals

Test for the minimal non-zero amount e.g., 1, and assert that it was processed correctly

def test_partial_withdrawal_request_with_low_amount(spec, state):
rng = random.Random(1351)
state.slot += spec.config.SHARD_COMMITTEE_PERIOD * spec.SLOTS_PER_EPOCH
current_epoch = spec.get_current_epoch(state)
validator_index = rng.choice(spec.get_active_validator_indices(state, current_epoch))
validator_pubkey = state.validators[validator_index].pubkey
address = b"\x22" * 20
amount = 1
# Give the validator some excess balance to withdraw
state.balances[validator_index] += amount
set_compounding_withdrawal_credential(spec, state, validator_index, address=address)
withdrawal_request = spec.WithdrawalRequest(
source_address=address,
validator_pubkey=validator_pubkey,
amount=amount,
)
yield from run_withdrawal_request_processing(
spec,
state,
withdrawal_request,
)
# Check that the assigned exit epoch is correct
assert state.earliest_exit_epoch == spec.compute_activation_exit_epoch(
current_epoch
)

@bshastry
Copy link
Contributor Author

Sorry for taking your time @jtraglia . I'm going to close this PR since the tests are already there.

@bshastry bshastry closed this Jan 31, 2025
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.

2 participants