Reproducible Docker Workflow#964
Conversation
Summary of ChangesHello @lukeiannucci, 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 focuses on establishing a robust and reproducible Docker build process for smart contracts, ensuring consistent compilation across environments. Simultaneously, it refines the Espresso TEE verifier's service registration logic by introducing a retry mechanism, thereby improving the system's resilience and error handling for critical registration operations. 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 introduces a reproducible Docker build for contracts and refactors the Espresso signer registration logic. The new Dockerfile is well-constructed, using pinned versions and checksums for dependencies to ensure reproducibility. The refactoring of the signer registration retry logic, moving it from batch_poster to espresso_verifier, is a good architectural improvement that centralizes responsibility. My review includes suggestions to enhance the Dockerfile's maintainability by reducing repetition and to improve logging in the batch_poster for better clarity and severity accuracy following the refactor. Overall, these changes positively impact the project's build process and code structure.
I am having trouble creating individual review comments. Click here to see my feedback.
Dockerfile.contracts-builder-stagex (96-116)
The section for pre-downloading Solidity compilers is very repetitive. Each version adds 3 lines (curl, echo | sha256sum, chmod). This makes the Dockerfile harder to read and maintain. Consider refactoring this into a shell loop to make it more concise and easier to add new compiler versions in the future.
RUN mkdir -p /home/user/.cache/hardhat-nodejs/compilers/linux-amd64 && \
for spec in \
"v0.7.6+commit.7338295f bd69ea85427bf2f4da74cb426ad951dd78db9dfdd01d791208eccc2d4958a6bb" \
"v0.8.17+commit.8df45f5f 99f2070b776e9714f1f76c43c229cf99b8978a92938ee8d2364c6de11c1a03d4" \
"v0.8.19+commit.7dd6d404 7a5c1d3dc9a8eba62bb2ec37192c9178ae5fe8a54a56e5573fd3c9c17cd9eb48" \
"v0.8.20+commit.a1b79de6 0479d44fdf9c501c25337fdc540419f1593b884a87b47f023da4f1c700fda782" \
"v0.8.24+commit.e11b9ed9 fb03a29a517452b9f12bcf459ef37d0a543765bb3bbc911e70a87d6a37c30d5f" \
; do \
set -- $spec; \
version=$1; \
sha256=$2; \
compiler_file="solc-linux-amd64-${version}"; \
compiler_path="/home/user/.cache/hardhat-nodejs/compilers/linux-amd64/${compiler_file}"; \
url="https://binaries.soliditylang.org/linux-amd64/${compiler_file}"; \
echo "Downloading ${url}"; \
curl -L -o "${compiler_path}" "${url}"; \
echo "${sha256} ${compiler_path}" | sha256sum -c; \
chmod +x "${compiler_path}"; \
done
arbnode/batch_poster.go (2587-2593)
The logging here could be improved. Since the retry logic has been moved to espresso_verifier.go, this error is now fatal for the batch poster. Using log.Warn might understate the severity. Also, the message "Espresso signer registration failed consecutively" is a bit misleading, as the consecutive failures happened in the underlying service, not here. Consider increasing the log level to log.Error and clarifying the message to indicate that all retries have been exhausted.
log.Error(
"Espresso signer registration failed after all retries. Stopping batch poster.",
"maxRetries", espressotee.EspressoMaxRetries,
"err", err,
)
b.fatalErrChan <- err
|
✅ All tests successful. No failed tests were found. 📣 Thoughts on this report? Let Codecov know! | Powered by Codecov |
Closes #<ISSUE_NUMBER>
This PR:
To run this:
This PR does not:
Key places to review: