chore: use era files for ssz benchmarks#180
Conversation
Summary of ChangesHello @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 streamlines the SSZ benchmarking process by migrating the data source for attestation, block, and state benchmarks to project-managed era files. This transition enhances the consistency and reproducibility of benchmark results, as all tests will now operate on a unified and controlled dataset, ultimately simplifying benchmark execution and maintenance. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe 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 by creating a comment using either
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 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
|
There was a problem hiding this comment.
Code Review
This pull request refactors the SSZ benchmarks for attestations, blocks, and states to use project-managed era files instead of local .ssz files. This is a great improvement for ensuring benchmark consistency and making them easier to run. The code changes look good overall. I've added a few comments:
- A suggestion to add an assertion in
attestation.zigto prevent a potential panic if the test data doesn't contain any attestations. This improves the robustness of the benchmark. - A few suggestions to sort module imports alphabetically in the benchmark files for better readability and consistency with other parts of the codebase, in line with the repository's style guide.
| var block = SignedBeaconBlock.default_value; | ||
| try SignedBeaconBlock.deserializeFromBytes(allocator, block_bytes, &block); | ||
|
|
||
| const attestation = &block.message.body.attestations.items[0]; |
There was a problem hiding this comment.
Accessing items[0] will cause a panic if the attestations list is empty. It's safer to assert that the list is not empty before accessing an element, especially since the test data is now being loaded from an external era file which might not guarantee the presence of attestations in the selected block.
This aligns with the style guide's emphasis on safety and assertions: "A function must not operate blindly on data it has not checked." (line 52) and "Assert all function arguments and return values, pre/postconditions and invariants." (line 51).
std.debug.assert(block.message.body.attestations.items.len > 0);
const attestation = &block.message.body.attestations.items[0];
References
- A function must not operate blindly on data it has not checked. Assertions should be used to validate pre/postconditions and invariants. (link)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 39736def68
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Co-authored-by: Chen Kai <281165273grape@gmail.com>
Use project-managed era files for ssz state, block, and attestation benchmarks. This makes it easier to run benchmarks and ensure that benchmarks are consistently run on the same data. --------- Co-authored-by: Chen Kai <281165273grape@gmail.com>
Use project-managed era files for ssz state, block, and attestation benchmarks.
This makes it easier to run benchmarks and ensure that benchmarks are consistently run on the same data.