feat: add download_spec_tests executable#45
Merged
wemeetagain merged 3 commits intomainfrom Sep 2, 2025
Merged
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @wemeetagain, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces a new executable that automates the process of downloading and extracting Ethereum consensus spec tests. This enhancement streamlines the setup for testing environments by providing a dedicated tool to fetch necessary test data, configurable through build options.
Highlights
- New Executable: A download_spec_tests executable has been added to facilitate the automated download and extraction of Ethereum consensus spec tests.
- Configurable Options: The executable's behavior, including the spec test URL, version, and output directory, can be customized via an options module, allowing overrides through zbuild.zon or command-line flags.
- Build System Integration: The new executable is fully integrated into the build system, allowing it to be run using zbuild run download_spec_tests or zig build run:download_spec_tests.
- Test Data Management: The .gitignore file has been updated to properly ignore the downloaded spec test directory, ensuring clean repository state.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces a new executable script for downloading consensus spec tests, which is a great addition for managing test data. The changes to the build configuration in build.zig and zbuild.zon are well-structured. However, the implementation of the download script in test/spec/download_spec_tests.zig contains a critical bug in the file download logic, a resource leak, and some areas where error handling could be improved. I've provided specific comments and code suggestions to address these points.
Performance Report✔️ no performance regression detected Full benchmark results
|
ensi321
pushed a commit
that referenced
this pull request
Sep 6, 2025
* feat: add download_spec_tests executable * chore: resource cleanup * chore: resource cleanup
wemeetagain
added a commit
that referenced
this pull request
Oct 17, 2025
* feat: convert ExecutionPayload to ExecutionPayloadHeader * feat: processExecutionPayload() * feat: fastAggregateVerify() * feat: randao SignatureSet * refactor: move beacon_state and beacon_block and execution_payload to types folder * feat: model IndexedAttestation * feat: indexed_attestation signature set * feat: AttesterSlashings * fix: implement fork in fork.zig * feat: bls_to_execution_change signature set * feat: proposer signature set * refactor: move Fork definition to types folder * feat: proposer_slashings signature set * feat: voluntary_exits signature set * feat: process_randao * feat: process_eth1_data * feat: slash_validator * feat: process_proposer_slashing * feat: attestation utils * feat: is_valid_indexed_attestation * feat: process_attester_slashing * feat: getIndexedAttestation() and getAttestingIndices() in EpochCache * feat: process_attestation_phase0 * feat: process_attestation_altair * feat: process_attestations * fix: generic validateAttestation * chore: improve processs_attester_slashing * fix: remove fork on process_attestation flow * fix: remove fork in all state_transition functions * feat: process_deposit * feat: process_voluntary_exit * feat: process_bls_to_execution_change * feat: process_deposit_request * feat: process_withdrawal_request * feat: process_consolidation_request * feat: process_operations * feat: getAttestationsSignatureSets() * feat: process_sync_committee * feat: process_blob_kzg_commitments * feat: process_block * feat: process_slot * fix: change ChainConfig property inside BeaconConfig * fix: ForkSeq import path * feat: params and config modules * fix: add test folder * fix: state_transition module in its own folder * feat: add network chain configs * fix: model EpochCache as reference count in CachedBeaconstateAllForks * fix: create EpochCache from a beacon state * feat: separate unit tests and int tests * fix: SyncCommitteeCache * fix: int test createCachedBeaconState * fix: generalize committee_indices * fix: support u64 proposer index * refactor: ReferenceCount * fix: generalize innerShuffleList() * fix: processPendingDeposits * feat: implement TestCachedBeaconStateAllForks * fix: unit test beforeProcessEpoch (#27) * fix: stack allocation EpochTransitionCache * fix: pass by reference in process_epoch() functions * fix: rewards and penalties as output params * fix: unit test process_justification_and_finalization * fix: reuse reards and penalties from EpochTransisionCache * fix: process_epoch_fn compilation errors * fix: PubkeyIndexMap to support both u32 and u64 * fix: processPendingDeposits compilation errors * chore(types): dedup panic statements (#29) * fix: processPendingConsolidations compilation errors * fix: processEffectiveBalanceUpdates compilation errors * fix: processSlashingsReset compilation errors * fix: processHistoricalSummariesUpdate compilation errors * fix: processHistoricalRootsUpdate compilation errors * fix: processParticipationRecordUpdates compilation errors * fix: processParticipationFlagUpdates compiltation errors * fix: processSyncCommitteeUpdates compilation errors * feat: implement and consume getters in EpochCache * fix: wrong type for `ExecutionPayloadHeader` methods (#31) * fix: wrong type for `ExecutionPayloadHeader` methods * add usage to confirm * feat(types): add support for blinded block variants Adds support for `BlindedBeaconBlock` and `BlindedBeaconBlockBody` with a sanity test. * fix(types): correctly type methods in `ExecutionPayloadHeader` * fix(types): remove `isExecutionType` from `BlindedBeaconBlockBody` * feat: testProcessRewardsAndPenalties() * chore: test processInactivityUpdates() * chore: test processRegistryUpdates() * chore: test processSlashings() * fix: missing try in processEpoch() * fix: testProcessEpochFn to handle no-err functions * fix: unit test processPendingDeposits() * fix: unit test processPendingConsolidations() * chore: unit test processEffectiveBalanceUpdates() * chore: unit test processSlashingsReset() * chore: unit test processRandaoMixesReset() * chore: unit test processHistoricalSummariesUpdate() * fix: unit test processParticipationFlagUpdates() * chore: processSyncCommitteeUpdates unit tests * fix: append* instead of add* * fix: unit test the main processEpoch() function * fix: correct method name rotateEpochParticipations() * fix: processSyncCommitteeUpdates next_sync_committee_pubkeys_slices[i] * fix: wrong type for `ExecutionPayloadHeader` methods (#31) * fix: wrong type for `ExecutionPayloadHeader` methods * add usage to confirm * feat(types): add support for blinded block variants Adds support for `BlindedBeaconBlock` and `BlindedBeaconBlockBody` with a sanity test. * fix(types): correctly type methods in `ExecutionPayloadHeader` * fix(types): remove `isExecutionType` from `BlindedBeaconBlockBody` * main stf fix import undo panic dedup deletion * main stf skeleton * fix: inline else in beacon_state.zig * fix: use imported type in beacon_state.zig * chore: add comment for ReferenceCount.get() usage * chore: avoid optional where possible * chore: avoid double negative in process_rewards_and_penalties * chore: avoid optional in process_slashings * fix: expicit params for getTestProcessFn() * fix: avoid negative terms in getTestProcessFn() util * fix: TestRunner and TestOpt * main stf skeleton * more compilation fixes * process attestation fixes * processAttestation * processDeposit * more QoL tweaks * processBlsToExecutionChanges * fix: verifyMerkleBranch * processVoluntaryExits * processDepositRequest * processWithdrawalRequest * processConsolidationRequest * processSyncCommittee * processSyncCommittee pt 2 * processBlobKzgCommitments * unnecessary compileLog; test runs * rename unblinded -> regular for clearer names * more simplification with regular vs blinded * move SignedBlock out * test: processBlockHeader * test: processWithdrawals * test: processExecutionPayload * test:processRandao * test: processEth1Data * test: process operations * test: process sync committee and blob kzg * clean imports * remove print * ergonomic pending withdrawals * zbuild sync * fix utils * fix build * test: skip stf test for now * uncomment beforeProcessEpoch and processEpoch * slot, fork, latestBlockHeader api changes * feat(beacon_state): more un-OOPing of API * feat(beacon_state): more unOOP * feat(beacon_state): finish simplifying beacon_state * feat(beacon_block): unoops * feat(beacon_block): fin * feat(signed_block): clean up * feat(signed_block): move to types/ * fix imports * feat: revamp types this still feels a little wrong, there are only that many reusable types from `consensus_types` that I think the cleaner way might just be to use them directly instead of reimports. * remove ValidatorIndices * feat(process_block): ProcessBlockOpts * remove unnecessary TODO for blockToHeader * missed out removing appendPendingDeposit * chore: remove TODOs Some TODOs are either already completed, or have low-hanging fruit changes that are implemented in this commit. * fix(bls): blst test * fix: compilation issues * chore: nuke primitives.zig we should just directly import and use ssz types where possible instead of needlessly importing and reexporting them. This provides clarity of what the underlying type is, because we also have some types native only to state transition (such as the beacon blocks) * chore: remove script * feat(stf): implement isMergeTransitionComplete * feat: add download_spec_tests executable (#45) * feat: add download_spec_tests executable * chore: resource cleanup * chore: resource cleanup * Add minimal config * use clone * comments * feat(process_epoch): use toBoolArray for justification bits * fix(epoch_transition_cache): use initCapacity * style changes * fix(validator): snake_case instead of camelCase * chore: style * slight stylistic change for readability * fix: typo: acrive -> active * fix(epoch_participation): prefer `clearRetainingCapacity` over `deinit` For 2 reasons: 1. we're about to reuse the same `ArrayList` again, so there's no reason to `deinit` 2. we can assume (at the very least) the capacity of the 2 arraylists are the same because `resize` was called in `beforeProcessEpoch`. * feat(processBlock): handle expected_withdrawals_result deinit * remove unused allocator params * fix(proposer_slashing): verify_signature as bool instead of ?bool * feat(slash_validator): use switch/case instead of if/else to set slashing penalty quotient * feat(slash_validator): switch/case for all fork dependent values * fix(processBlsToExecutionChange): verify_signature as bool instead of ?bool * style * fix(processSyncAggregate): verify_signature as bool instead of ?bool * update zbuild * Add write_spec_tests * build: update zbuild * fix: deinit expected withdrawal result * fix(beacon_state): properly implement rotateEpochParticipation * feat(beacon_state): fully impl clone * feat(stf): upgrade state * chore: move options into params * fix upgrade state * chore(test_utils): move test_utils into state_transition module `test_utils` rely on `state_transition`, so importing both of them in unit tests result in a circular dependency. It makes more sense to put `test_utils` in `state_transition` and reuse it that way. * test: test process attestations * chore: remove test/utils * Update dep * Add types * Fix merge * Add test_case.zig * Add operations test_case * Fix path * git ignore * Improve test * Fix type * Improve test * test * Update ssz-z dep * Address gemini's comment * use pointer for SchemaOut * refactor * Add attester slashing test * Add test logger * Silence stack trace * Polish * Finish process_attestations * Add process_proposer_slashing * Finish all operations tests for phase0 and altair * Temporary workaround * Clean up * Fix merge * Add execution_payload * FInish withdrawals * Support all forks for operations * Temporary workaround 2 * Fix merge * fix merge * Address @twoeths's comment * Initiate tc properly * Copy state pointer * Fix memory leak * Fix typo * chore: refactor spec test runner (#66) * chore: refactor spec test runner * chore: pr review * chore: update operations writer * Update test/spec/test_type/operations.zig Co-authored-by: bing <spiralladder@fastmail.com> * chore: pr review * chore: pr review * chore: pr review * chore: more polish and framework * chore: add readme for more documentation * chore: tweaks * feat: add sanity tests * chore: move bls setting -> bool * chore: pr review * chore: further simplification * feat: add execution_payload test functionality * chore: update zbuild --------- Co-authored-by: NC <17676176+ensi321@users.noreply.github.com> Co-authored-by: bing <spiralladder@fastmail.com> * chore: fix formating --------- Co-authored-by: Tuyen Nguyen <twoeths@users.noreply.github.com> Co-authored-by: twoeths <10568965+twoeths@users.noreply.github.com> Co-authored-by: bing <spiralladder@fastmail.com> Co-authored-by: Cayman <caymannava@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
zbuild run download_spec_testsorzig build run:download_spec_teststo execute