Add tests to test sync aggregate's order of balance operation#3157
Add tests to test sync aggregate's order of balance operation#3157
Conversation
djrtwo
left a comment
djrtwo
left a comment
There was a problem hiding this comment.
Looks good. a few nits on code duplication but otherwise good to go
...e/pyspec/eth2spec/test/altair/block_processing/sync_aggregate/test_process_sync_aggregate.py
Outdated
Show resolved
Hide resolved
| assert active_validator_count < spec.SYNC_COMMITTEE_SIZE | ||
| assert committee_size > len(set(committee_indices)) |
There was a problem hiding this comment.
Maybe move this up higher so it's more of a "blocking condition" at the top of the test rather than midway
There was a problem hiding this comment.
@ralexstokes iiuc the preconditions were about duplicate or nonduplicate?
I changed it to be checked with is_duplicate_sync_committee helper in c7e102a. IMO it's more explicit and more precise than checking with committee size.
| spec, state, committee_indices, committee_bits, | ||
| skip_reward_validation=skip_reward_validation) | ||
|
|
||
| return validator_index |
There was a problem hiding this comment.
@djrtwo I remember you hated that I returned variables from a yielding function 😅 but it is so tempting here...
otherwise, I have to break _run_sync_committee_selected_twice into two functions.
3 participants
Thank the Prysm team for proposing this test case.
sync_committee_rewards_duplicate_committee_only_participate_first_oneandsync_committee_rewards_duplicate_committee_only_participate_second_oneboth:validator_indexvalidator gets selected twice in the sync committee (duplicate).validator_indexvalidator's balance is0.sync_committee_rewards_duplicate_committee_only_participate_first_one:sync_committee_rewards_duplicate_committee_only_participate_second_one: