From c85f58a91b33bb1ba8a759cae8f2b6c28b1b4925 Mon Sep 17 00:00:00 2001 From: debjit Date: Sat, 18 Jan 2025 19:13:31 +0530 Subject: [PATCH 1/6] Add test for pending deposits with same pubkey and different withdrawal credentials --- .../electra/fork/test_electra_fork_basic.py | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/core/pyspec/eth2spec/test/electra/fork/test_electra_fork_basic.py b/tests/core/pyspec/eth2spec/test/electra/fork/test_electra_fork_basic.py index 4416063b39..24ae4ae1f9 100644 --- a/tests/core/pyspec/eth2spec/test/electra/fork/test_electra_fork_basic.py +++ b/tests/core/pyspec/eth2spec/test/electra/fork/test_electra_fork_basic.py @@ -1,3 +1,4 @@ +import random from eth2spec.test.context import ( with_phases, with_custom_state, @@ -151,6 +152,38 @@ def test_fork_has_compounding_withdrawal_credential(spec, phases, state): )] +@with_phases(phases=[DENEB], other_phases=[ELECTRA]) +@spec_test +@with_state +@with_meta_tags(ELECTRA_FORK_TEST_META_TAGS) +def test_fork_pending_deposits_with_same_pubkey_different_withdrawal_credentials(spec, phases, state): + post_spec = phases[ELECTRA] + + num_validators = len(state.validators) + indexes_with_same_pubkey = random.sample(range(num_validators), min(10, num_validators)) + constant_pubkey = b'\x11' * 48 + + for index in range(num_validators): + state.validators[index].activation_epoch = spec.FAR_FUTURE_EPOCH + + withdrawal_credentials = [] + for index in indexes_with_same_pubkey: + state.validators[index].pubkey = constant_pubkey + withdrawal_credentials.append(state.validators[index].withdrawal_credentials) + + # ensure that the withdrawal credentials are unique + assert len(set(withdrawal_credentials)) == len(withdrawal_credentials) + + post_state = yield from run_fork_test(post_spec, state) + + assert len(post_state.pending_deposits) == num_validators + + for index in indexes_with_same_pubkey: + assert post_state.pending_deposits[index].pubkey == constant_pubkey + expected_withdrawal_credentials = state.validators[index].withdrawal_credentials + assert post_state.pending_deposits[index].withdrawal_credentials == expected_withdrawal_credentials + + @with_phases(phases=[DENEB], other_phases=[ELECTRA]) @spec_test @with_state From 6db823856de272a29406cdd1825a7d8bd9637be0 Mon Sep 17 00:00:00 2001 From: debjit Date: Sat, 18 Jan 2025 22:37:01 +0530 Subject: [PATCH 2/6] Add tests for deposit transitions with same pubkey and different withdrawal credentials --- .../electra/fork/test_electra_fork_basic.py | 9 ++---- .../sanity/blocks/test_deposit_transition.py | 29 +++++++++++++++++++ 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/electra/fork/test_electra_fork_basic.py b/tests/core/pyspec/eth2spec/test/electra/fork/test_electra_fork_basic.py index 24ae4ae1f9..5da86b1c3f 100644 --- a/tests/core/pyspec/eth2spec/test/electra/fork/test_electra_fork_basic.py +++ b/tests/core/pyspec/eth2spec/test/electra/fork/test_electra_fork_basic.py @@ -166,13 +166,8 @@ def test_fork_pending_deposits_with_same_pubkey_different_withdrawal_credentials for index in range(num_validators): state.validators[index].activation_epoch = spec.FAR_FUTURE_EPOCH - withdrawal_credentials = [] for index in indexes_with_same_pubkey: state.validators[index].pubkey = constant_pubkey - withdrawal_credentials.append(state.validators[index].withdrawal_credentials) - - # ensure that the withdrawal credentials are unique - assert len(set(withdrawal_credentials)) == len(withdrawal_credentials) post_state = yield from run_fork_test(post_spec, state) @@ -180,8 +175,8 @@ def test_fork_pending_deposits_with_same_pubkey_different_withdrawal_credentials for index in indexes_with_same_pubkey: assert post_state.pending_deposits[index].pubkey == constant_pubkey - expected_withdrawal_credentials = state.validators[index].withdrawal_credentials - assert post_state.pending_deposits[index].withdrawal_credentials == expected_withdrawal_credentials + assert (post_state.pending_deposits[index].withdrawal_credentials + == state.validators[index].withdrawal_credentials) @with_phases(phases=[DENEB], other_phases=[ELECTRA]) diff --git a/tests/core/pyspec/eth2spec/test/electra/sanity/blocks/test_deposit_transition.py b/tests/core/pyspec/eth2spec/test/electra/sanity/blocks/test_deposit_transition.py index a9c2c62814..c350d201e7 100644 --- a/tests/core/pyspec/eth2spec/test/electra/sanity/blocks/test_deposit_transition.py +++ b/tests/core/pyspec/eth2spec/test/electra/sanity/blocks/test_deposit_transition.py @@ -1,3 +1,4 @@ +import random from eth2spec.test.helpers.block import ( build_empty_block_for_next_slot, ) @@ -262,3 +263,31 @@ def test_deposit_transition__deposit_and_top_up_same_block(spec, state): assert state.pending_deposits[pre_pending_deposits].amount == block.body.deposits[0].data.amount amount_from_deposit = block.body.execution_requests.deposits[0].amount assert state.pending_deposits[pre_pending_deposits + 1].amount == amount_from_deposit + + +@with_phases([ELECTRA]) +@spec_state_test +def test_deposit_transition__deposit_with_same_pubkey_different_withdrawal_credentials(spec, state): + deposit_count = 1 + deposit_request_count = 4 + + state, block = prepare_state_and_block(spec, state, + deposit_cnt=deposit_count, + deposit_request_cnt=deposit_request_count) + + # pick 2 indices among deposit requests to have the same pubkey as the deposit + indexes_with_same_pubkey = random.sample(range(deposit_count, deposit_request_count), 2) + for index in indexes_with_same_pubkey: + block.body.execution_requests.deposits[index].pubkey = block.body.deposits[0].data.pubkey + + block.body.execution_payload.block_hash = compute_el_block_hash_for_block(spec, block) + + deposit_requests = block.body.execution_requests.deposits.copy() + + yield from run_deposit_transition_block(spec, state, block) + + assert len(state.pending_deposits) == deposit_request_count + deposit_count + for index in indexes_with_same_pubkey: + assert state.pending_deposits[deposit_count + index].pubkey == deposit_requests[index].pubkey + assert (state.pending_deposits[deposit_count + index].withdrawal_credentials + == deposit_requests[index].withdrawal_credentials) \ No newline at end of file From 5c2d5511b8b28ad2832d1597e9908e31ded77e2a Mon Sep 17 00:00:00 2001 From: debjit Date: Sat, 18 Jan 2025 22:41:32 +0530 Subject: [PATCH 3/6] fmt and revert test for pending deposits with same pubkey and different withdrawal credentials --- .../electra/fork/test_electra_fork_basic.py | 28 ------------------- .../sanity/blocks/test_deposit_transition.py | 4 +-- 2 files changed, 2 insertions(+), 30 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/electra/fork/test_electra_fork_basic.py b/tests/core/pyspec/eth2spec/test/electra/fork/test_electra_fork_basic.py index 5da86b1c3f..4416063b39 100644 --- a/tests/core/pyspec/eth2spec/test/electra/fork/test_electra_fork_basic.py +++ b/tests/core/pyspec/eth2spec/test/electra/fork/test_electra_fork_basic.py @@ -1,4 +1,3 @@ -import random from eth2spec.test.context import ( with_phases, with_custom_state, @@ -152,33 +151,6 @@ def test_fork_has_compounding_withdrawal_credential(spec, phases, state): )] -@with_phases(phases=[DENEB], other_phases=[ELECTRA]) -@spec_test -@with_state -@with_meta_tags(ELECTRA_FORK_TEST_META_TAGS) -def test_fork_pending_deposits_with_same_pubkey_different_withdrawal_credentials(spec, phases, state): - post_spec = phases[ELECTRA] - - num_validators = len(state.validators) - indexes_with_same_pubkey = random.sample(range(num_validators), min(10, num_validators)) - constant_pubkey = b'\x11' * 48 - - for index in range(num_validators): - state.validators[index].activation_epoch = spec.FAR_FUTURE_EPOCH - - for index in indexes_with_same_pubkey: - state.validators[index].pubkey = constant_pubkey - - post_state = yield from run_fork_test(post_spec, state) - - assert len(post_state.pending_deposits) == num_validators - - for index in indexes_with_same_pubkey: - assert post_state.pending_deposits[index].pubkey == constant_pubkey - assert (post_state.pending_deposits[index].withdrawal_credentials - == state.validators[index].withdrawal_credentials) - - @with_phases(phases=[DENEB], other_phases=[ELECTRA]) @spec_test @with_state diff --git a/tests/core/pyspec/eth2spec/test/electra/sanity/blocks/test_deposit_transition.py b/tests/core/pyspec/eth2spec/test/electra/sanity/blocks/test_deposit_transition.py index c350d201e7..40058dbe20 100644 --- a/tests/core/pyspec/eth2spec/test/electra/sanity/blocks/test_deposit_transition.py +++ b/tests/core/pyspec/eth2spec/test/electra/sanity/blocks/test_deposit_transition.py @@ -274,7 +274,7 @@ def test_deposit_transition__deposit_with_same_pubkey_different_withdrawal_crede state, block = prepare_state_and_block(spec, state, deposit_cnt=deposit_count, deposit_request_cnt=deposit_request_count) - + # pick 2 indices among deposit requests to have the same pubkey as the deposit indexes_with_same_pubkey = random.sample(range(deposit_count, deposit_request_count), 2) for index in indexes_with_same_pubkey: @@ -290,4 +290,4 @@ def test_deposit_transition__deposit_with_same_pubkey_different_withdrawal_crede for index in indexes_with_same_pubkey: assert state.pending_deposits[deposit_count + index].pubkey == deposit_requests[index].pubkey assert (state.pending_deposits[deposit_count + index].withdrawal_credentials - == deposit_requests[index].withdrawal_credentials) \ No newline at end of file + == deposit_requests[index].withdrawal_credentials) From 70be586422ddada26c69e0fa01a69d7d7ed5e6e1 Mon Sep 17 00:00:00 2001 From: debjit Date: Mon, 20 Jan 2025 09:57:03 +0530 Subject: [PATCH 4/6] predefined indices which have the same pubkey --- .../test/electra/sanity/blocks/test_deposit_transition.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/electra/sanity/blocks/test_deposit_transition.py b/tests/core/pyspec/eth2spec/test/electra/sanity/blocks/test_deposit_transition.py index 40058dbe20..375871685a 100644 --- a/tests/core/pyspec/eth2spec/test/electra/sanity/blocks/test_deposit_transition.py +++ b/tests/core/pyspec/eth2spec/test/electra/sanity/blocks/test_deposit_transition.py @@ -1,4 +1,3 @@ -import random from eth2spec.test.helpers.block import ( build_empty_block_for_next_slot, ) @@ -276,7 +275,7 @@ def test_deposit_transition__deposit_with_same_pubkey_different_withdrawal_crede deposit_request_cnt=deposit_request_count) # pick 2 indices among deposit requests to have the same pubkey as the deposit - indexes_with_same_pubkey = random.sample(range(deposit_count, deposit_request_count), 2) + indexes_with_same_pubkey = [1, 3] for index in indexes_with_same_pubkey: block.body.execution_requests.deposits[index].pubkey = block.body.deposits[0].data.pubkey From a079dbfd4c4d257be2a34a1a7920884ffc0145ce Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Mon, 20 Jan 2025 09:28:10 -0600 Subject: [PATCH 5/6] Rename indexes to indices --- .../test/electra/sanity/blocks/test_deposit_transition.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/electra/sanity/blocks/test_deposit_transition.py b/tests/core/pyspec/eth2spec/test/electra/sanity/blocks/test_deposit_transition.py index 375871685a..a71c9e6c4f 100644 --- a/tests/core/pyspec/eth2spec/test/electra/sanity/blocks/test_deposit_transition.py +++ b/tests/core/pyspec/eth2spec/test/electra/sanity/blocks/test_deposit_transition.py @@ -275,8 +275,8 @@ def test_deposit_transition__deposit_with_same_pubkey_different_withdrawal_crede deposit_request_cnt=deposit_request_count) # pick 2 indices among deposit requests to have the same pubkey as the deposit - indexes_with_same_pubkey = [1, 3] - for index in indexes_with_same_pubkey: + indices_with_same_pubkey = [1, 3] + for index in indices_with_same_pubkey: block.body.execution_requests.deposits[index].pubkey = block.body.deposits[0].data.pubkey block.body.execution_payload.block_hash = compute_el_block_hash_for_block(spec, block) @@ -286,7 +286,7 @@ def test_deposit_transition__deposit_with_same_pubkey_different_withdrawal_crede yield from run_deposit_transition_block(spec, state, block) assert len(state.pending_deposits) == deposit_request_count + deposit_count - for index in indexes_with_same_pubkey: + for index in indices_with_same_pubkey: assert state.pending_deposits[deposit_count + index].pubkey == deposit_requests[index].pubkey assert (state.pending_deposits[deposit_count + index].withdrawal_credentials == deposit_requests[index].withdrawal_credentials) From 13d9aa1374ed4e34e4377064e1bda8ebe6ce7d1c Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Mon, 20 Jan 2025 09:48:23 -0600 Subject: [PATCH 6/6] Add extra assert to ensure withdrawal creds are different --- .../test/electra/sanity/blocks/test_deposit_transition.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/core/pyspec/eth2spec/test/electra/sanity/blocks/test_deposit_transition.py b/tests/core/pyspec/eth2spec/test/electra/sanity/blocks/test_deposit_transition.py index a71c9e6c4f..f2d9a6f11a 100644 --- a/tests/core/pyspec/eth2spec/test/electra/sanity/blocks/test_deposit_transition.py +++ b/tests/core/pyspec/eth2spec/test/electra/sanity/blocks/test_deposit_transition.py @@ -278,6 +278,9 @@ def test_deposit_transition__deposit_with_same_pubkey_different_withdrawal_crede indices_with_same_pubkey = [1, 3] for index in indices_with_same_pubkey: block.body.execution_requests.deposits[index].pubkey = block.body.deposits[0].data.pubkey + # ensure top-up deposit request withdrawal credentials are different than the deposit + assert (block.body.execution_requests.deposits[index].withdrawal_credentials + != block.body.deposits[0].data.withdrawal_credentials) block.body.execution_payload.block_hash = compute_el_block_hash_for_block(spec, block) @@ -288,5 +291,6 @@ def test_deposit_transition__deposit_with_same_pubkey_different_withdrawal_crede assert len(state.pending_deposits) == deposit_request_count + deposit_count for index in indices_with_same_pubkey: assert state.pending_deposits[deposit_count + index].pubkey == deposit_requests[index].pubkey + # ensure withdrawal credentials are retained, rather than being made the same assert (state.pending_deposits[deposit_count + index].withdrawal_credentials == deposit_requests[index].withdrawal_credentials)