Add tests for `process_withdrawals` block processing by terencechain · Pull Request #4468 · ethereum/consensus-specs

terencechain · 2025-07-28T04:01:09Z

No description provided.

jtraglia · 2025-07-31T19:24:33Z

tests/core/pyspec/eth2spec/test/eip7732/block_processing/test_process_withdrawals.py

+    if is_post_eip7732(spec):
+        assert len(expected_withdrawals_result[0]) == 0
+    else:
+        assert len(expected_withdrawals_result) == 0


Why is the else condition necessary if we use @with_eip7732_and_later?

jtraglia · 2025-07-31T19:32:35Z

tests/core/pyspec/eth2spec/test/eip7732/block_processing/test_process_withdrawals.py

+    else:
+        expected_withdrawals = spec.get_expected_withdrawals(state)


Same thing with this else block. We don't need to check is_post_eip7732.

jtraglia · 2025-07-31T19:40:26Z

tests/core/pyspec/eth2spec/test/eip7732/block_processing/test_process_withdrawals.py

+    # Verify priority ordering: builder -> pending -> sweep
+    assert len(expected_withdrawals) == 3
+    assert expected_withdrawals[0].validator_index == builder_index  # Builder first
+    assert expected_withdrawals[1].validator_index == pending_index  # Pending second
+    assert expected_withdrawals[2].validator_index == sweep_index  # Sweep third


The naming here is a bit confusing. Maybe these would be better:

Builder payments

Pending partial withdrawals

Exit/excess withdrawals

jtraglia · 2025-07-31T19:48:20Z

tests/core/pyspec/eth2spec/test/eip7732/block_processing/test_process_withdrawals.py

+
+@with_eip7732_and_later
+@spec_state_test
+def test_maximum_withdrawals_per_payload_limit(spec, state):


We should also test for:

There are more than MAX_WITHDRAWALS_PER_PAYLOAD builder payments and the pending partial withdrawals queue is not empty. Assert that the builder_pending_withdrawals queue is still not empty. Assert that none of the other withdrawals were processed.

No builder payments, MAX_WITHDRAWALS_PER_PAYLOAD pending partial withdrawals, some sweep withdrawals. There would be sweep withdrawals because of MAX_PENDING_PARTIALS_PER_WITHDRAWALS_SWEEP.

No builder payments, no pending partial withdrawals, MAX_WITHDRAWALS_PER_PAYLOAD sweep withdrawals.

jtraglia · 2025-07-31T19:53:55Z

tests/core/pyspec/eth2spec/test/eip7732/block_processing/test_process_withdrawals.py

+    # Add multiple pending withdrawals
+    for i in range(3):
+        prepare_pending_withdrawal(spec, state, i)
+
+    # EIP-7732 limits pending withdrawals to min(MAX_PENDING_PARTIALS_PER_WITHDRAWALS_SWEEP, MAX_WITHDRAWALS_PER_PAYLOAD - 1)
+    # In minimal config: min(2, 4-1) = 2
+    expected_withdrawals = min(3, spec.MAX_PENDING_PARTIALS_PER_WITHDRAWALS_SWEEP)


Maybe add enough pending partial withdrawals for this to test the mainnet value (8) too. So maybe queue MAX_PENDING_PARTIALS_PER_WITHDRAWALS_SWEEP withdrawals and set num_expected to that too?

jtraglia · 2025-07-31T19:58:33Z

tests/core/pyspec/eth2spec/test/eip7732/block_processing/test_process_withdrawals.py

+    Test withdrawal processing with validator not yet active.
+    """
+    set_parent_block_full(spec, state)
+    validator_index = min(len(state.validators) // 2, spec.MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP - 1)


Why is this so complicated?

jtraglia · 2025-07-31T19:59:46Z

tests/core/pyspec/eth2spec/test/eip7732/block_processing/test_process_withdrawals.py

+    validator_index = min(
+        len(state.validators) // 2 + 1, spec.MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP - 1
+    )


Same here? Why not just index 0?

jtraglia · 2025-07-31T20:02:04Z

tests/core/pyspec/eth2spec/test/eip7732/block_processing/test_process_withdrawals.py

We can do this in a follow up PR, but there are some missing tests that I would like to see here. We need to ensure that builders can perform the full range of "regular" withdrawals too. Eg a builder can do partial withdrawals.

jtraglia · 2025-07-31T20:05:16Z

tests/core/pyspec/eth2spec/test/eip7732/block_processing/test_process_withdrawals.py

+    set_parent_block_full(spec, state)
+    current_epoch = spec.get_current_epoch(state)
+    set_validator_fully_withdrawable(spec, state, 3, current_epoch)
+    state.validators[3].effective_balance = 10000000000


I'd prefer we use MIN_ACTIVATION_BALANCE here.

jtraglia · 2025-07-31T20:05:32Z

tests/core/pyspec/eth2spec/test/eip7732/block_processing/test_process_withdrawals.py

+    current_epoch = spec.get_current_epoch(state)
+    set_validator_fully_withdrawable(spec, state, 4, current_epoch)
+    state.validators[4].effective_balance = 0
+    state.balances[4] = 100000000


I'd prefer we use MIN_ACTIVATION_BALANCE here too.

terencechain · 2025-08-04T15:26:16Z

thanks @jtraglia , I think I addressed everything in 63ebc37

jtraglia · 2025-08-04T15:27:55Z

thanks @jtraglia , I think I addressed everything in 63ebc37

Thank you. Will review these changes in a bit. We'll need @potuz to review too.

PS: The CI runners are currently broken. I've asked EF devops to look into it.

jtraglia · 2025-08-04T16:26:03Z

@terencechain currently failing lint checks.

Make lint

leolara · 2025-12-07T18:03:33Z

Most of the tests here don't assert what should be the final result. This is bad, if the specs are wrong that is passed inadvertently to the tests.

Some tests that we should add:

More combinations for withdrawal priorites
Tests with different amounts of validator withdrawing
Tests with different address prefixes (including builders with non-builder prefixes)
Slashed builders in different circustances
More tests with differnt convinations in the boundary of MAX_WITHDRAWALS_PER_PAYLOAD

I can work on these

leolara · 2026-01-05T13:37:41Z

Most of this code has been refactored here: #4810

terencechain force-pushed the process-wd-tests branch 3 times, most recently from 5f30233 to 6836408 Compare July 28, 2025 21:54

This comment was marked as spam.

Sign in to view

This comment was marked as duplicate.

Sign in to view

jtraglia added the eip7732 ePBS label Jul 29, 2025

jtraglia reviewed Jul 31, 2025

View reviewed changes

terencechain force-pushed the process-wd-tests branch 3 times, most recently from 1870941 to f07705d Compare August 4, 2025 18:37

terencechain and others added 3 commits August 18, 2025 15:18

eip7732: add test suite for process_withdrawals block processing

b975386

Justin's feedback

28e0b6c

Make lint

Rename eip7732 to gloas

fa1b5c5

jtraglia force-pushed the process-wd-tests branch from f07705d to fa1b5c5 Compare August 18, 2025 20:39

terencechain marked this pull request as draft August 20, 2025 22:59

will-corcoran mentioned this pull request Sep 11, 2025

EIP-7732 Breakout Room Call #23, September 12, 2025 ethereum/pm#1714

Closed

3 tasks

jtraglia changed the title ~~eip7732: add tests for process_withdrawals block processing~~ eip7732: add tests for process_withdrawals block processing Sep 25, 2025

jtraglia mentioned this pull request Sep 25, 2025

EIP-7732 Breakout Room Call #24, September 26, 2025 ethereum/pm#1735

Closed

3 tasks

jtraglia mentioned this pull request Oct 9, 2025

EIP-7732 Breakout Room Call #25, October 10, 2025 ethereum/pm#1744

Closed

3 tasks

jtraglia mentioned this pull request Oct 23, 2025

EIP-7732 Breakout Room Call #26, October 24, 2025 ethereum/pm#1759

Closed

18 tasks

jtraglia mentioned this pull request Nov 7, 2025

EIP-7732 Breakout Room Call #27, November 7, 2025 ethereum/pm#1783

Closed

16 tasks

This was referenced Dec 1, 2025

Add more get_expected_withdrawals tests #4762

Closed

EIP-7732 Breakout Room Call #28, December 5, 2025 ethereum/pm#1801

Closed

leolara and others added 2 commits December 8, 2025 02:14

Add assertions to tests

75e9483

Merge branch 'master' into process-wd-tests

07dd005

sacha699612-hub approved these changes Dec 8, 2025

View reviewed changes

jtraglia changed the title ~~eip7732: add tests for process_withdrawals block processing~~ Add tests for process_withdrawals block processing Dec 8, 2025

leolara and others added 4 commits December 9, 2025 17:43

Merge branch 'master' into process-wd-tests

293f9d8

Merge branch 'master' into process-wd-tests

cebc04c

Fix error of accessing removed state field in Gloas

b2d858a

Refactor and fix some tests

9bafca8

sacha699612-hub approved these changes Dec 25, 2025

View reviewed changes

leolara closed this Jan 5, 2026

leolara mentioned this pull request Jan 12, 2026

Add process_withdrawals tests for Gloas #4830

Merged

leolara added a commit to leolara/consensus-specs that referenced this pull request Jan 15, 2026

Remove duplicated tests from PR ethereum#4468 that are in capella

e76c190

		else:
		expected_withdrawals = spec.get_expected_withdrawals(state)

Conversation

terencechain commented Jul 28, 2025

Uh oh!

This comment was marked as spam.

This comment was marked as duplicate.

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

terencechain commented Aug 4, 2025

Uh oh!

jtraglia commented Aug 4, 2025

Uh oh!

jtraglia commented Aug 4, 2025

Uh oh!

leolara commented Dec 7, 2025

Uh oh!

leolara commented Jan 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants