feat(evm-forge): foundry support and template for Nibiru EVM develoment #2084
Conversation
WalkthroughThe pull request introduces several changes across multiple files, primarily focusing on modifications to GitHub Actions workflows for EVM and Wasm end-to-end tests, updates to documentation, and the introduction of new configuration files for the Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (38)
evm-forge/.github/FUNDING.yml (2)
1-1: Consider adding a comment to explain the custom URL.The custom funding URL is correctly formatted, but it contains a long encoded parameter that may be difficult to understand or maintain.
Consider adding a comment above this line to explain what the URL represents and how to update it if needed. For example:
# Custom donation link for the project (update the encoded parameter as needed) custom: "https://3cities.xyz/#/pay?c=CAESFAKY9DMuOFdjE4Wzl2YyUFipPiSfIgICATICCAJaFURvbmF0aW9uIHRvIFBhdWwgQmVyZw"
2-2: LGTM. Consider adding context about PaulRBerg's role.The GitHub funding entry is correctly formatted and will enable the built-in sponsorship feature for the specified user.
To provide more context, consider adding a comment explaining PaulRBerg's role in the project. For example:
# Project maintainer and primary contributor github: "PaulRBerg"evm-forge/src/Foo.sol (4)
1-2: Consider license choice and Solidity version compatibility.
The
UNLICENSEDSPDX identifier means no license is granted. Consider if this aligns with the project's goals or if a more permissive license (e.g., MIT, Apache 2.0) would be more appropriate.The Solidity version is set to a very recent version (>=0.8.25). While using the latest version is good for security and features, it might limit compatibility with some tools or platforms. Consider if a slightly more permissive range (e.g., ^0.8.0) would be more appropriate for your use case.
4-4: Use a more descriptive contract name.The contract name
Foois typically used for examples or placeholders. Consider renaming it to something more descriptive that reflects its purpose or functionality in the context of the Nibiru EVM development.
5-7: Function implementation looks good. Consider adding documentation.The
idfunction is correctly implemented as an identity function. The use ofexternalvisibility andpuremodifier is appropriate.To improve code readability and maintainability, consider adding a NatSpec comment to document the function's purpose and parameters. For example:
/// @notice Returns the input value unchanged /// @param value The input value /// @return The same value that was passed in function id(uint256 value) external pure returns (uint256) { return value; }
1-8: Overall, the contract provides a good starting point for Foundry integration.This simple contract serves as a basic template or example for the new Foundry support in Nibiru EVM development. While the implementation is correct, consider the suggestions above to improve naming, documentation, and configuration choices.
As you develop this further:
- Expand on this contract or add more contracts to demonstrate Foundry's capabilities in the Nibiru ecosystem.
- Consider adding test files to showcase Foundry's testing features.
- Update the project documentation to guide developers on how to use this new Foundry setup for Nibiru EVM development.
These steps will help achieve the PR's objective of streamlining the development process and providing a comprehensive template for Foundry integration.
evm-forge/.gitpod.yml (1)
11-14: Consider adding more EVM development-specific extensions.The current extensions (Prettier and Hardhat Solidity) are excellent choices for code formatting and Solidity development. However, for a more comprehensive EVM development environment, you might want to consider adding a few more extensions.
Here are some additional extensions you might find useful:
vscode: extensions: - "esbenp.prettier-vscode" - "NomicFoundation.hardhat-solidity" - "JuanBlanco.solidity" # Solidity language support - "tintinweb.solidity-visual-auditor" # Solidity security-centric syntax and semantic highlighting - "ms-vscode.vscode-typescript-tsdoc" # TypeScript JSDoc support - "dbaeumer.vscode-eslint" # ESLint supportThese additional extensions provide enhanced Solidity support, security-focused highlighting, and linting capabilities which could be beneficial for EVM development.
evm-forge/script/Deploy.s.sol (2)
8-9: LGTM: Contract declaration and inheritance are well-defined.The
Deploycontract is appropriately named and inherits fromBaseScript. The comment providing a link to the Solidity Scripting tutorial is helpful.Consider adding a brief comment explaining the purpose of this specific deployment script, e.g.:
/// @dev Deployment script for the Foo contract contract Deploy is BaseScript {
10-12: LGTM: Therun()function correctly deploys the Foo contract.The function is appropriately marked as
publicwith thebroadcastmodifier, and it correctly creates and returns a newFoocontract instance.Consider adding a simple log statement to improve the script's feedback:
function run() public broadcast returns (Foo foo) { foo = new Foo(); console.log("Foo contract deployed at: %s", address(foo)); }This addition would require importing the
consolelibrary from Forge-std:import "forge-std/console.sol";evm-forge/package.json (3)
1-8: Consider revising the package name for a project template.The current package name "@prb/foundry-template" suggests a scoped package, which is unusual for a project template. For a template that others might fork or use as a starting point, consider using a more generic name without a scope, such as "nibiru-foundry-template" or "evm-forge-template".
9-17: Dependencies look good. Consider adding a package lock file.The dependencies are appropriate for a Foundry-based Solidity project. Using OpenZeppelin contracts is a good practice for security and standardization. The dev dependencies cover essential tools for development, testing, and code quality.
Consider adding a package lock file (e.g.,
package-lock.jsonorbun.lockb) to ensure consistent installations across different environments.
28-38: Scripts are comprehensive, but consider improving compatibility.The provided scripts cover essential tasks for development, testing, and code quality, which is excellent. However, there are a few points to consider:
- The use of
bunas a package manager might limit compatibility, as it's relatively new and not as widely adopted asnpmoryarn.- The coverage report generation relies on
genhtml, which might not be available in all environments.Consider the following improvements:
- Provide alternative scripts using more common package managers (npm or yarn) alongside the bun scripts.
- For the coverage report, consider using a more widely available tool or documenting the requirement for
genhtml.Example:
"scripts": { ... "lint": "npm run lint:sol && npm run prettier:check", "lint:bun": "bun run lint:sol && bun run prettier:check", ... }evm-forge/justfile (8)
6-8: Consider adding a safeguard to the clean commandWhile the
cleanrecipe effectively removes build artifacts, usingrm -rfcan be risky. Consider adding a safeguard to ensure you're in the correct directory before running the command.You could modify the recipe as follows:
clean: - rm -rf cache out + [ -f "foundry.toml" ] && rm -rf cache out || echo "Error: Not in the project root directory"This change ensures the command only runs if
foundry.tomlis present in the current directory.
14-15: LGTM: Comprehensive linting recipeThe
lintrecipe effectively combines Solidity linting and Prettier checks, which is a good practice for maintaining code quality.Consider adding a comment explaining what each sub-recipe does:
# Run linters -lint: lint-sol prettier-check +lint: lint-sol prettier-check # Run Solidity linter and check formatting with PrettierThis addition would improve clarity for developers new to the project.
17-20: LGTM: Comprehensive Solidity lintingThe
lint-solrecipe effectively combines Foundry's formatter check with solhint for thorough Solidity linting.Consider adding error handling to ensure the linting process stops if
forge fmtfails:lint-sol: - forge fmt --check - bun solhint "{script,src,test}/**/*.sol" + forge fmt --check && bun solhint "{script,src,test}/**/*.sol"This change ensures that both linting steps are only successful if both commands succeed.
26-28: LGTM: Consistent Prettier formatting recipeThe
prettier-writerecipe correctly applies Prettier formatting to JSON, Markdown, and YAML files, maintaining consistency with theprettier-checkrecipe.Consider combining the
prettier-checkandprettier-writerecipes into a single parameterized recipe to reduce duplication:-# Check formatting with Prettier -prettier-check: - prettier --check "**/*.{json,md,yml}" --ignore-path ".prettierignore" - -# Format files with Prettier -prettier-write: - prettier --write "**/*.{json,md,yml}" --ignore-path ".prettierignore" +# Run Prettier (check or write) +prettier action="check": + prettier --{{action}} "**/*.{json,md,yml}" --ignore-path ".prettierignore"This change would allow you to use
just prettier checkandjust prettier writewhile reducing code duplication.
30-32: LGTM: Standard test recipeThe
testrecipe correctly usesforge testto run the project's tests, which is the standard approach in Foundry projects.Consider adding some commonly used options as variables to make the test command more flexible:
# Run tests -test: - forge test +test ARGS="": + forge test {{ARGS}}This change would allow developers to easily add options when running tests, like
just test "-vv"for more verbose output.
38-40: LGTM: Basic coverage recipeThe
coveragerecipe correctly usesforge coverageto generate test coverage data, which is the standard approach in Foundry projects.Consider adding an option to specify the output format:
# Run test coverage -coverage: - forge coverage +coverage FORMAT="summary": + forge coverage --format {{FORMAT}}This change would allow developers to easily switch between different coverage report formats, like
summary,lcov, orhtml.
42-45: LGTM: Detailed coverage report recipeThe
coverage-reportrecipe generates a comprehensive HTML coverage report, which is very useful for visualizing test coverage.Consider adding a check for the
genhtmlcommand and providing instructions if it's not found:# Generate test coverage report coverage-report: forge coverage --report lcov - genhtml lcov.info --branch-coverage --output-dir coverage + if command -v genhtml >/dev/null 2>&1; then \ + genhtml lcov.info --branch-coverage --output-dir coverage; \ + else \ + echo "Error: genhtml not found. Please install lcov."; \ + exit 1; \ + fiThis change ensures that users are informed if
genhtmlis not available on their system.
57-59: LGTM: Useful gas report recipeThe
gas-reportrecipe provides a convenient way to generate gas usage reports during testing, which is valuable for optimizing contract efficiency.Consider adding an option to specify additional test arguments:
# Get gas report -gas-report: - forge test --gas-report +gas-report ARGS="": + forge test --gas-report {{ARGS}}This change would allow developers to add other test options when running the gas report, like
just gas-report "-vv"for more verbose output.evm-forge/.github/scripts/rename.sh (2)
15-26: LGTM: Efficient package.json modification using jq.The script effectively uses
jqto modify thepackage.jsonfile, updating the necessary fields with the provided input. The JSON manipulation and file overwriting are done correctly.Consider improving readability by breaking the long
jqcommand into multiple lines:JQ_OUTPUT=$(jq \ --arg NAME "@$GITHUB_REPOSITORY" \ --arg AUTHOR_NAME "$GITHUB_REPOSITORY_OWNER" \ --arg URL "https://github.com/$GITHUB_REPOSITORY_OWNER" \ --arg DESCRIPTION "$GITHUB_REPOSITORY_DESCRIPTION" \ '.name = $NAME | .description = $DESCRIPTION | .author |= ( .name = $AUTHOR_NAME | .url = $URL )' \ package.json )This format makes it easier to read and maintain the
jqcommand.
1-38: Overall: Well-structured and effective repository renaming script.This script effectively automates the process of renaming a GitHub repository and updating associated files. It demonstrates good practices such as:
- Strict error handling
- Input validation
- Cross-platform compatibility
- Efficient use of tools like
jqandsedTo further improve the script, consider adding a brief comment at the beginning explaining its purpose and usage. For example:
#!/usr/bin/env bash # This script renames a GitHub repository and updates associated files. # Usage: ./rename.sh <new_repo_name> <owner_username> [repo_description] # # Example: ./rename.sh "myuser/new-repo" "myuser" "A fantastic new project" set -euo pipefail # ... rest of the scriptThis addition would make the script more self-documenting and easier for other developers to use and maintain.
🧰 Tools
🪛 Shellcheck
[warning] 35-35: The surrounding quotes actually unquote this. Remove or escape them.
(SC2027)
[warning] 36-36: The surrounding quotes actually unquote this. Remove or escape them.
(SC2027)
[warning] 37-37: The surrounding quotes actually unquote this. Remove or escape them.
(SC2027)
[warning] 38-38: The surrounding quotes actually unquote this. Remove or escape them.
(SC2027)
evm-forge/.github/workflows/use-template.yml (2)
16-39: Steps for repository setup look good, with a minor suggestion.The steps for checking out the repository, updating package.json, and removing unnecessary files are well-structured. The use of step summaries provides good feedback in the GitHub UI.
However, consider adding error handling to the script execution and file removal steps. This can be achieved by using the
set -ebash option to ensure the workflow fails if any command fails. For example:- name: "Update package.json" run: | set -e ./.github/scripts/rename.sh "$GITHUB_REPOSITORY" "$GITHUB_REPOSITORY_OWNER" "$GITHUB_REPOSITORY_DESCRIPTION" - name: "Remove files not needed in the user's copy of the template" run: | set -e rm -f "./.github/FUNDING.yml" rm -f "./.github/scripts/rename.sh" rm -f "./.github/workflows/create.yml"This will ensure that any errors in these critical steps are caught and reported.
49-52: Final summary step is good, with room for improvement.Adding a final summary to the GitHub step summary is a good practice as it provides clear feedback on the workflow execution. However, the current summary is quite basic and could be more informative.
Consider expanding the summary to include more details about what was done. For example:
- name: "Add commit summary" run: | echo "## Workflow Summary" >> $GITHUB_STEP_SUMMARY echo "✅ Repository initialized from template" >> $GITHUB_STEP_SUMMARY echo "✅ package.json updated" >> $GITHUB_STEP_SUMMARY echo "✅ Unnecessary files removed" >> $GITHUB_STEP_SUMMARY echo "✅ Changes committed and pushed" >> $GITHUB_STEP_SUMMARYThis provides a more comprehensive overview of the actions performed by the workflow.
evm-forge/test/Foo.t.sol (3)
15-22: LGTM: Contract setup follows best practices.The
FooTestcontract is well-structured, inheriting fromTestand using asetUpfunction to initialize the contract under test. MakingsetUpvirtual is a good practice for extensibility.Consider adding a comment explaining why
setUpis marked asvirtual, as it might not be immediately obvious to all developers./// @dev A function invoked before each test case is run. +/// It's marked as virtual to allow potential overriding in derived test contracts. function setUp() public virtual { // Instantiate the contract-under-test. foo = new Foo(); }
24-29: LGTM: Basic test function is well-implemented.The
test_Examplefunction demonstrates proper use of console logging and assertions. It effectively tests theidfunction of theFoocontract.Consider adding more test cases to cover different scenarios or edge cases for the
idfunction, if applicable.For example, you could add tests for extreme values:
function test_ExampleWithMaxUint() external view { uint256 x = type(uint256).max; assertEq(foo.id(x), x, "max uint256 value mismatch"); }
31-37: LGTM: Fuzz test is well-implemented and documented.The
testFuzz_Examplefunction demonstrates proper use of fuzz testing withvm.assumeto handle input constraints. The comment about usingboundas an alternative is informative.Consider adding a complementary test using the
boundfunction to showcase both approaches:function testFuzz_ExampleWithBound(uint256 x) external view { x = bound(x, 1, type(uint256).max); assertEq(foo.id(x), x, "value mismatch"); }This addition would provide a practical example of using
boundalongside the existingvm.assumeapproach..github/workflows/e2e-evm.yml (1)
Line range hint
1-83: Consider workflow optimizationsThe changes made to this workflow are minimal and focused on naming and directory structure, which is good. However, there might be opportunities to further optimize this workflow:
Consider using composite actions for repeated steps like setting up Go and Node.js. This can help reduce duplication across workflows.
The
justcommand is used multiple times. You might want to create a custom action forjustcommands to encapsulate this functionality and make it reusable across workflows.The sleep command after launching the localnet is a bit arbitrary. Consider implementing a more robust way to wait for the localnet to be ready, such as polling an endpoint.
Think about adding a step to cache npm dependencies to speed up future runs.
Would you like assistance in implementing any of these optimizations?
evm-forge/.github/workflows/ci.yml (4)
3-6: LGTM: Environment variables are properly set.Good job on using a GitHub secret for the API key, which is a secure practice. The FOUNDRY_PROFILE is set to "ci", which is appropriate for CI runs.
Consider adding a comment explaining the purpose of the FOUNDRY_PROFILE variable for better clarity:
env: API_KEY_ALCHEMY: ${{ secrets.API_KEY_ALCHEMY }} # Set Foundry profile to 'ci' for CI-specific configurations FOUNDRY_PROFILE: "ci"
15-36: LGTM: Lint job is well-structured.The lint job is set up correctly with appropriate tool installations (Foundry, Bun, Node.js). Running the lint process and adding a summary to the GitHub Actions output is a good practice.
Consider adding error handling to the lint step:
- name: "Lint the code" run: "bun run lint" continue-on-error: true - name: "Add lint summary" run: | echo "## Lint result" >> $GITHUB_STEP_SUMMARY if [ $? -eq 0 ]; then echo "✅ Passed" >> $GITHUB_STEP_SUMMARY else echo "❌ Failed" >> $GITHUB_STEP_SUMMARY exit 1 fiThis will provide more accurate feedback in the summary if the lint process fails.
38-59: LGTM: Build job is well-structured.The build job is set up correctly with appropriate tool installations. Building contracts and printing their sizes is a useful step for monitoring contract size.
Consider adding error handling to the build step and including the contract sizes in the summary:
- name: "Build the contracts and print their sizes" run: | forge build --sizes > build_output.txt cat build_output.txt - name: "Add build summary" run: | echo "## Build result" >> $GITHUB_STEP_SUMMARY if [ $? -eq 0 ]; then echo "✅ Passed" >> $GITHUB_STEP_SUMMARY echo "### Contract Sizes" >> $GITHUB_STEP_SUMMARY cat build_output.txt >> $GITHUB_STEP_SUMMARY else echo "❌ Failed" >> $GITHUB_STEP_SUMMARY exit 1 fiThis will provide more detailed information in the summary, including the contract sizes.
61-92: LGTM: Test job is well-structured with good practices.The test job is set up correctly with appropriate dependencies, tool installations, and execution steps. The weekly changing fuzz seed is a good practice for managing RPC allowance.
Consider the following improvements:
- Add error handling to the test step:
- name: "Run the tests" run: "forge test" continue-on-error: true - name: "Add test summary" run: | echo "## Tests result" >> $GITHUB_STEP_SUMMARY if [ $? -eq 0 ]; then echo "✅ Passed" >> $GITHUB_STEP_SUMMARY else echo "❌ Failed" >> $GITHUB_STEP_SUMMARY exit 1 fi
- Consider adding a step to capture and display test output for better visibility:
- name: "Run the tests" run: | forge test -vv > test_output.txt cat test_output.txt - name: "Add test summary" run: | echo "## Tests result" >> $GITHUB_STEP_SUMMARY if [ $? -eq 0 ]; then echo "✅ Passed" >> $GITHUB_STEP_SUMMARY else echo "❌ Failed" >> $GITHUB_STEP_SUMMARY fi echo "### Test Output" >> $GITHUB_STEP_SUMMARY cat test_output.txt >> $GITHUB_STEP_SUMMARYThese changes will provide more detailed information in case of test failures and improve the visibility of test results in the GitHub Actions summary.
justfile (1)
51-53: Great addition of thelocalnet-fastrecipe!This new recipe provides a quick way to start a local network without rebuilding, which is excellent for improving development efficiency. It aligns perfectly with the PR's goal of streamlining tasks.
Consider updating the comment to be more descriptive:
-# Runs a Nibiru local network without building and installing. "just localnet --no-build" +# Runs a Nibiru local network without rebuilding. Equivalent to "just localnet --no-build"This change clarifies that the command skips rebuilding rather than both building and installing.
evm-forge/README.md (5)
13-14: Enhance clarity of Unix-based system note.Consider specifying the exact operating systems supported or providing alternative instructions for non-Unix systems. This will help users across different platforms.
Suggested improvement:
-Note that the shell instructions assume you're using Unix (e.g. MacOS or Ubuntu). +Note: These shell instructions are for Unix-based systems (e.g. macOS, Ubuntu). For Windows users, consider using WSL or Git Bash.🧰 Tools
🪛 LanguageTool
[grammar] ~13-~13: The operating system from Apple is written “macOS”.
Context: ...ructions assume you're using Unix (e.g. MacOS or Ubuntu). Foundryup is the official ...(MAC_OS)
25-25: Maintain consistent header styling.For better readability and consistency with the rest of the document, consider using a markdown header for "Option 2".
Suggested improvement:
-Option 2: Build from source with `cargo` +### Option 2: Build from source with `cargo`
74-81: Enhance the "Features" section with more specific information.While referring users to external documentation is helpful, consider adding a brief list of key features specific to this template. This would give users a quick overview of what makes this template unique or particularly useful.
For example, you could add:
Some key features of this template include: - Pre-configured GitHub Actions for CI - Integration with popular development tools (Prettier, Solhint) - Sensible default configurations - Easy dependency management using Node.js packages
98-104: Enhance VSCode integration guidance.While the current guidance is helpful, consider providing more specific steps or recommendations for VSCode users. This could include mentioning recommended extensions or specific settings that work well with this template.
You could add:
Recommended VSCode extensions for this template: - Solidity by Nomic Foundation - Prettier - Code formatter - Solidity Visual Developer Consider adding the following to your VSCode settings: ```json { "solidity.formatter": "prettier", "editor.formatOnSave": true }<details> <summary>🧰 Tools</summary> <details> <summary>🪛 LanguageTool</summary><blockquote> [uncategorized] ~100-~100: This expression is ususally spelled with a hyphen Context: ...## VSCode Integration This template is IDE agnostic, but for the best user experience, you ... (SPECIFIC_HYPHEN) </blockquote></details> </details> --- `221-222`: **Clarify the use of `bun` in commands.** The usage of `bun` in the commands might not be familiar to all users. Consider adding a brief explanation of what `bun` is and why it's used in this project, or provide alternative commands using more common tools like `npm` or `yarn`. You could add: ```markdown Note: This project uses [Bun](https://bun.sh/) as a fast all-in-one JavaScript runtime. If you prefer using `npm` or `yarn`, you can replace `bun` with your preferred package manager in these commands.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (3)
evm-e2e/bun.lockbis excluded by!**/bun.lockbevm-e2e/package-lock.jsonis excluded by!**/package-lock.jsonevm-forge/bun.lockbis excluded by!**/bun.lockb
📒 Files selected for processing (29)
- .github/workflows/e2e-evm.yml (3 hunks)
- .github/workflows/e2e-wasm.yml (1 hunks)
- docs/adr/00-adr-template.md (0 hunks)
- docs/adr/01-adr-msg-server-keeper.md (0 hunks)
- docs/adr/README.md (0 hunks)
- evm-e2e/test/contract_send_nibi.test.ts (1 hunks)
- evm-forge/.editorconfig (1 hunks)
- evm-forge/.env.example (1 hunks)
- evm-forge/.github/FUNDING.yml (1 hunks)
- evm-forge/.github/scripts/rename.sh (1 hunks)
- evm-forge/.github/workflows/ci.yml (1 hunks)
- evm-forge/.github/workflows/use-template.yml (1 hunks)
- evm-forge/.gitignore (1 hunks)
- evm-forge/.gitpod.yml (1 hunks)
- evm-forge/.prettierignore (1 hunks)
- evm-forge/.prettierrc.yml (1 hunks)
- evm-forge/.solhint.json (1 hunks)
- evm-forge/LICENSE.md (1 hunks)
- evm-forge/README.md (1 hunks)
- evm-forge/foundry.toml (1 hunks)
- evm-forge/justfile (1 hunks)
- evm-forge/package.json (1 hunks)
- evm-forge/remappings.txt (1 hunks)
- evm-forge/script/Base.s.sol (1 hunks)
- evm-forge/script/Deploy.s.sol (1 hunks)
- evm-forge/src/Foo.sol (1 hunks)
- evm-forge/test/Foo.t.sol (1 hunks)
- justfile (2 hunks)
- x/evm/msg.go (1 hunks)
💤 Files with no reviewable changes (3)
- docs/adr/00-adr-template.md
- docs/adr/01-adr-msg-server-keeper.md
- docs/adr/README.md
✅ Files skipped from review due to trivial changes (9)
- .github/workflows/e2e-wasm.yml
- evm-e2e/test/contract_send_nibi.test.ts
- evm-forge/.editorconfig
- evm-forge/.env.example
- evm-forge/.gitignore
- evm-forge/.prettierignore
- evm-forge/.prettierrc.yml
- evm-forge/LICENSE.md
- evm-forge/remappings.txt
🧰 Additional context used
🪛 Shellcheck
evm-forge/.github/scripts/rename.sh
[warning] 35-35: The surrounding quotes actually unquote this. Remove or escape them.
(SC2027)
[warning] 36-36: The surrounding quotes actually unquote this. Remove or escape them.
(SC2027)
[warning] 37-37: The surrounding quotes actually unquote this. Remove or escape them.
(SC2027)
[warning] 38-38: The surrounding quotes actually unquote this. Remove or escape them.
(SC2027)
🪛 LanguageTool
evm-forge/README.md
[grammar] ~13-~13: The operating system from Apple is written “macOS”.
Context: ...ructions assume you're using Unix (e.g. MacOS or Ubuntu). Foundryup is the official ...(MAC_OS)
[uncategorized] ~100-~100: This expression is ususally spelled with a hyphen
Context: ...## VSCode Integration This template is IDE agnostic, but for the best user experience, you ...(SPECIFIC_HYPHEN)
[grammar] ~225-~225: Did you mean “copy and paste”?
Context: ...` file in your browser, to do so simply copy paste the path): ```bash bun run test:covera...(COPY_PASTE)
🪛 Markdownlint
evm-forge/README.md
42-42: Expected: atx; Actual: atx_closed
Heading style(MD003, heading-style)
🔇 Additional comments (41)
evm-forge/.github/FUNDING.yml (1)
1-2: Overall, the FUNDING.yml file is well-structured and functional.The file correctly sets up both a custom funding URL and a GitHub sponsorship option. These additions will help support the project financially. The suggestions provided are aimed at improving clarity and maintainability, but the current implementation is already functional and adheres to GitHub's funding configuration standards.
evm-forge/.gitpod.yml (1)
1-1: Verify if the chosen image meets all project requirements.The
gitpod/workspace-bunimage is a good starting point as it includes Bun. However, for EVM development, you might need additional tools pre-installed.Consider if the following tools are necessary for your workflow and if they're included in this image:
- Node.js and npm (for running JavaScript/TypeScript tools)
- Python (often used in blockchain development scripts)
- Go (used in some Ethereum tools)
If these are needed, you might want to create a custom Docker image that extends
gitpod/workspace-bunwith these additional tools.evm-forge/script/Deploy.s.sol (2)
1-2: LGTM: License and Solidity version are appropriately set.The SPDX license identifier and Solidity version pragma are correctly specified. The narrow version range (>=0.8.25 <0.9.0) ensures compatibility while allowing for minor updates.
4-6: LGTM: Imports are relevant and correctly specified.The imports for
FooandBaseScriptare appropriate for this deployment script.Please ensure that the imported files exist and are correctly implemented:
✅ Verification successful
Verified: Imported files exist and are correctly implemented.
All imported files are present and contain the expected code.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the existence and basic structure of imported files # Check Foo.sol echo "Checking Foo.sol:" fd -p "src/Foo.sol" -x cat # Check Base.s.sol echo "Checking Base.s.sol:" fd -p "script/Base.s.sol" -x catLength of output: 1861
evm-forge/package.json (2)
18-26: Keywords are well-chosen and relevant.The selected keywords accurately represent the project's nature and technologies used. They will aid in discoverability if the package is published to a registry.
27-27: Good practice: Package marked as private.Setting
"private": trueprevents accidental publication of the package to a public registry, which is appropriate for a template or internal project.evm-forge/justfile (8)
1-4: LGTM: Helpful setup recipeThe
setuprecipe provides a convenient way for developers to view available commands. This is a good practice for improving the project's usability.
10-12: LGTM: Standard build recipeThe
buildrecipe correctly usesforge buildto compile the contracts, which is the standard approach in Foundry projects.
22-24: LGTM: Proper use of Prettier for non-Solidity filesThe
prettier-checkrecipe correctly uses Prettier to check formatting for JSON, Markdown, and YAML files, while respecting the.prettierignorefile. This is a good practice for maintaining consistent formatting across non-Solidity files in the project.
34-36: LGTM: Useful verbose test recipeThe
test-verboserecipe provides a convenient way to run tests with maximum verbosity. This is helpful for debugging and understanding test output in detail.
47-50: LGTM: Comprehensive dependency installationThe
installrecipe correctly handles both npm (via Bun) and Forge dependencies. The order of installation (npm first, then Forge) is appropriate, ensuring all necessary tools are available for the Forge installation.
52-55: LGTM: Consistent dependency update recipeThe
updaterecipe correctly handles updating both npm (via Bun) and Forge dependencies. It maintains consistency with theinstallrecipe in terms of order and structure.
61-64: LGTM: Comprehensive formatting recipeThe
fmtrecipe effectively combines Solidity formatting usingforge fmtwith Prettier formatting for other files. This ensures consistent formatting across all file types in the project.
1-64: Overall: Well-structured and comprehensive justfileThis justfile provides a robust set of recipes for managing a Foundry-based Ethereum development workflow. It covers essential tasks such as building, testing, linting, formatting, and dependency management. The recipes are logically organized and follow good practices for task automation.
Some minor improvements were suggested for individual recipes, mainly focusing on adding flexibility and error handling. These suggestions, if implemented, would further enhance the usability and robustness of the workflow.
Great job on setting up this development environment!
evm-forge/.github/scripts/rename.sh (2)
1-13: LGTM: Robust script setup and input validation.The script starts with good practices:
- Using
set -euo pipefailfor strict error handling.- Properly validating and setting default values for input parameters.
- Echoing input parameters for confirmation, which aids in debugging.
These practices contribute to a more reliable and maintainable script.
28-32: LGTM: Cross-platform compatible sed function.The
sedifunction is a well-implemented solution for ensuringsedcommand compatibility across different operating systems. It correctly checks for thesedversion and adjusts the syntax accordingly.This approach enhances the script's portability and reliability across different environments.
evm-forge/.github/workflows/use-template.yml (2)
8-14: Job configuration looks good.The job configuration is well-structured and appropriate for its purpose:
- The condition
if: ${{ !github.event.repository.is_template }}ensures the workflow only runs on repositories created from the template, not on the template itself.- The
write-allpermission is necessary for the file modifications and commit operations.- Using
ubuntu-latestas the runner is a good choice for general-purpose workflows.
41-47: Commit step is efficient, but be cautious with force pushing.The use of
stefanzweifel/git-auto-commit-action@v4for committing changes is a good choice, as it simplifies the commit process. Amending the existing commit and force pushing keeps the repository history clean, which is appropriate for an initialization workflow.However, it's important to note that force pushing can be risky:
- If multiple people are working on the repository simultaneously (unlikely in this scenario, but possible), force pushing could cause conflicts or lost work.
- If the workflow runs multiple times, each force push will overwrite the previous commit, potentially losing information.
Consider adding a comment in the workflow file explaining why force pushing is necessary in this case, to inform future maintainers of the rationale behind this decision.
To ensure that force pushing is only happening in the intended scenarios, you could add a conditional check before the commit step. Here's a script to verify the repository state:
You can add this as a step before the commit step and use its exit code to conditionally execute the force push.
evm-forge/foundry.toml (5)
3-16: LGTM: Default profile settings are well-configured.The default profile settings are appropriate for Ethereum development:
- Using the latest stable Solidity version (0.8.25)
- EVM version set to "shanghai"
- Optimizer enabled with 10,000 runs, balancing deployment cost and runtime efficiency
- Reasonable fuzz test runs (1,000) for default testing
These settings provide a good foundation for development and testing.
18-20: LGTM: CI profile enhances testing rigor.The CI profile appropriately increases testing thoroughness:
- Fuzz test runs increased to 10,000 for more comprehensive testing
- Verbosity set to maximum (4) for detailed output in CI logs
These settings will help catch more potential issues and facilitate debugging in the CI environment.
34-42: LGTM: Well-defined formatting preferences.The formatting preferences are well-configured:
- Consistent with common Solidity style guides
- 120-character line length is reasonable for modern displays
- Comment wrapping enabled for improved readability
- Use of double quotes for consistency
These settings will help maintain a consistent and readable codebase.
44-55: LGTM: Comprehensive RPC endpoint configuration with security considerations.The RPC endpoint configuration is well-structured:
- Covers a wide range of popular networks
- Uses environment variables for API keys, which is a good security practice
- Utilizes multiple providers (Infura, Alchemy) for redundancy
Ensure that these environment variables are securely managed in your CI/CD pipeline and development environments. To verify the security of these variables, you can run the following script:
#!/bin/bash # Description: Check for hardcoded API keys or URLs in the repository # Test: Search for potential hardcoded API keys or URLs rg -i '(api_key|apikey|secret|http|https).*=.*[A-Za-z0-9]{32,}'This script will help identify any accidentally committed API keys or URLs in the repository.
Consider adding fallback RPC endpoints for critical networks to improve reliability. For example:
mainnet = "https://eth-mainnet.g.alchemy.com/v2/${API_KEY_ALCHEMY},https://mainnet.infura.io/v3/${API_KEY_INFURA}"This way, if one provider is down, Foundry will automatically try the next one.
22-32: LGTM: Comprehensive Etherscan API configuration with a security reminder.The Etherscan API configuration is well-structured:
- Covers a wide range of popular networks
- Uses environment variables for API keys, which is a good security practice
Ensure that these environment variables are securely managed in your CI/CD pipeline and development environments. To verify the security of these variables, you can run the following script:
This script will help identify any accidentally committed API keys in the repository.
✅ Verification successful
Further Verification: Ensure no hardcoded secrets are present in the repository.
Verification Successful: No hardcoded secrets found.
All potential hardcoded secrets have been reviewed and confirmed to be non-sensitive placeholder values.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for hardcoded API keys in the repository # Test: Search for potential hardcoded API keys rg -i '(api_key|apikey|secret).*=.*[A-Za-z0-9]{32,}'Length of output: 65
Script:
#!/bin/bash # Description: Verify absence of hardcoded secrets in the repository # Search for potential hardcoded secrets with extended patterns rg -i '(api_key|apikey|secret|token|password|key).*=.*[A-Za-z0-9]{32,}' .Length of output: 406
evm-forge/test/Foo.t.sol (3)
1-11: LGTM: File header and imports are well-structured.The file header, license, Solidity version, and imports are appropriately set up for a Forge test file. The IERC20 interface definition is minimal but sufficient for the test requirements.
1-56: Overall, excellent test implementation with room for minor enhancements.This test file for the
Foocontract demonstrates a comprehensive approach to Solidity testing using Forge. It covers basic unit tests, fuzz testing, and fork-based tests, showcasing a range of testing techniques.Final recommendations:
- Consider adding more test cases to cover edge cases and potential failure scenarios.
- Implement the suggested improvements for each test function to enhance readability and extensibility.
- Add comments explaining the purpose of each test and any non-obvious implementation details.
- If applicable, consider adding tests for any uncovered functions or behaviors of the
Foocontract.Great job on implementing a robust test suite! These minor enhancements will further improve its effectiveness and maintainability.
39-55: LGTM: Fork test demonstrates advanced testing techniques.The
testFork_Examplefunction showcases proper use of forking for testing against Mainnet state. It handles the API key gracefully and uses a specific block number for consistency.Suggestions for improvement:
- Consider using a constant for the block number to make it easier to update.
- Add a comment explaining the significance of the chosen block number and address.
- Consider parameterizing the test to allow checking different tokens or addresses.
Example implementation of suggestions:
// At the top of the contract uint256 constant FORK_BLOCK_NUMBER = 16_428_000; address constant USDC_ADDRESS = 0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48; address constant HOLDER_ADDRESS = 0x7713974908Be4BEd47172370115e8b1219F4A5f0; // In the test function vm.createSelectFork({ urlOrAlias: "mainnet", blockNumber: FORK_BLOCK_NUMBER }); // Comment explaining the significance of the block number and address uint256 actualBalance = IERC20(USDC_ADDRESS).balanceOf(HOLDER_ADDRESS);To verify the correctness of the USDC address and holder address, run the following script:
This script will help ensure the consistency of USDC and holder addresses across the project and identify any other relevant stablecoin references.
✅ Verification successful
To address the issue with the previous script, please run the following updated script to verify the USDC address and holder address:
Verified: USDC and holder addresses are correctly referenced.
The
testFork_Examplefunction accurately references the USDC address and the holder address within the.solfiles. Additionally, all other USDC-related references align with the intended test functionality.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify USDC address and a known holder echo "Verifying USDC address and holder..." rg --type solidity "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48" . rg --type solidity "0x7713974908Be4BEd47172370115e8b1219F4A5f0" . # Check for other potential USDC or stablecoin references echo "Checking for other USDC or stablecoin references..." rg --type solidity -i "usdc|stablecoin|tether|dai" .Length of output: 481
Script:
#!/bin/bash # Verify USDC address and a known holder by targeting .sol files echo "Verifying USDC address and holder..." rg "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48" --glob "*.sol" rg "0x7713974908Be4BEd47172370115e8b1219F4A5f0" --glob "*.sol" # Check for other potential USDC or stablecoin references in .sol files echo "Checking for other USDC or stablecoin references..." rg -i "usdc|stablecoin|tether|dai" --glob "*.sol"Length of output: 740
.github/workflows/e2e-evm.yml (2)
1-1: LGTM: Improved workflow name consistencyThe change from "EVM e2e tests" to "EVM E2E tests" improves the consistency of capitalization in the workflow name. This is a good practice for maintaining a professional and uniform appearance across your CI/CD configuration.
72-72: Verify directory structure changeThe working directory for the "just install" and "Run tests (just test)" steps has been updated from "e2e/evm" to "evm-e2e". This change appears to reflect a project restructuring, which is fine. The new directory name is more concise and follows a consistent naming convention.
To ensure this change is consistent with the actual project structure, please run the following command:
This script will confirm the existence of the new directory and check for any remaining references to the old directory structure that might need updating.
Also applies to: 81-81
✅ Verification successful
Directory Structure Change Verified
The
evm-e2edirectory exists with the appropriate contents, and no references to the olde2e/evmdirectory were found. The restructuring is correctly implemented.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the existence of the new directory and its contents if [ -d "evm-e2e" ]; then echo "Directory evm-e2e exists. Contents:" ls -la evm-e2e else echo "Error: Directory evm-e2e does not exist." exit 1 fi # Check for any remaining references to the old directory if grep -r "e2e/evm" .; then echo "Warning: Found references to old directory 'e2e/evm'. Please update these." else echo "No references to old directory 'e2e/evm' found." fiLength of output: 1193
evm-forge/.github/workflows/ci.yml (2)
1-13: LGTM: Workflow name and trigger events are well-defined.The workflow name "CI" is clear and concise. The trigger events cover manual runs, pull requests, and pushes to the main branch, which is a good practice for continuous integration.
1-92: Great job on the CI workflow implementation!This CI workflow is well-structured and comprehensive, covering essential aspects of the development process including linting, building, and testing. It aligns well with the PR objectives of introducing Foundry support for Nibiru EVM development.
Key strengths:
- Proper use of Foundry and Bun, which are appropriate tools for EVM projects.
- Well-defined job dependencies ensuring a logical flow of CI processes.
- Good security practices, such as using secrets for API keys.
- Implementation of a weekly changing fuzz seed to manage RPC allowance.
The minor suggestions provided in the previous comments can further enhance the workflow's robustness and information output. Overall, this CI setup provides a solid foundation for streamlining the development process and ensuring code quality for the Nibiru EVM project.
justfile (3)
46-46: Excellent enhancement to thelocalnetrecipe!The addition of the
*PASS_FLAGSparameter allows for greater flexibility when running the local network. Users can now pass custom flags to thelocalnetcommand, which aligns well with the PR's objective of improving the development workflow for Nibiru EVM projects.
Line range hint
1-83: Overall, the changes to the justfile are well-implemented and beneficial.The modifications to the
localnetrecipe, the addition of thelocalnet-fastrecipe, and the update to thetest-e2erecipe all contribute to a more flexible and efficient development workflow for Nibiru EVM projects. These changes align perfectly with the PR objectives of streamlining tasks and improving the development experience.
83-83: Directory change for E2E tests looks good.The update from
e2e/evmtoevm-e2ealigns with the mentioned restructuring of the end-to-end test directory. This change improves the organization of the project structure.To ensure this change doesn't break any existing workflows or CI/CD pipelines, please run the following script to check for any references to the old path:
If the script returns any results, those files may need to be updated to use the new
evm-e2epath.✅ Verification successful
The previous script encountered an error due to an unrecognized file type. I'll provide a corrected script to search for references to
e2e/evmusing valid file types.
To ensure that the change from
e2e/evmtoevm-e2edoesn't break any existing workflows or configurations, let's perform a more comprehensive search across additional file types.
Directory change for E2E tests verified.
No references to the old
e2e/evmpath were found in the codebase, ensuring that the update toevm-e2edoes not break existing workflows or configurations.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for references to the old e2e/evm path echo "Searching for references to 'e2e/evm':" rg --type yaml --type bash --type go "e2e/evm"Length of output: 168
Script:
#!/bin/bash # Description: Check for references to the old e2e/evm path echo "Searching for references to 'e2e/evm':" rg --type yaml --type sh --type go "e2e/evm"Length of output: 137
Script:
#!/bin/bash # Description: Comprehensive check for references to the old e2e/evm path across various file types echo "Searching for references to 'e2e/evm' in yaml, sh, go, js, ts, json, and md files:" rg --type yaml --type sh --type go --type js --type ts --type json --type md "e2e/evm"Length of output: 267
evm-forge/README.md (3)
1-10: LGTM: Clear introduction and comprehensive component list.The introduction and "What's Inside" section provide a concise overview of the project and its key components. The inclusion of links to relevant resources is helpful for users.
37-53: LGTM: Proper attribution and informative badges.The "Credits and Related Efforts" section appropriately acknowledges the source of the template. The inclusion of badges provides useful information about the project's status, tools, and license.
🧰 Tools
🪛 Markdownlint
42-42: Expected: atx; Actual: atx_closed
Heading style(MD003, heading-style)
1-240: Overall, excellent README with minor suggestions for improvement.This README provides a comprehensive and well-structured guide for the
nibiru/evm-forgeproject. It covers all essential aspects including setup, usage, testing, and deployment. The suggested improvements are minor and mostly relate to enhancing clarity and consistency. Great job on creating a user-friendly and informative document!🧰 Tools
🪛 LanguageTool
[grammar] ~13-~13: The operating system from Apple is written “macOS”.
Context: ...ructions assume you're using Unix (e.g. MacOS or Ubuntu). Foundryup is the official ...(MAC_OS)
[uncategorized] ~100-~100: This expression is ususally spelled with a hyphen
Context: ...## VSCode Integration This template is IDE agnostic, but for the best user experience, you ...(SPECIFIC_HYPHEN)
[grammar] ~225-~225: Did you mean “copy and paste”?
Context: ...` file in your browser, to do so simply copy paste the path): ```bash bun run test:covera...(COPY_PASTE)
🪛 Markdownlint
42-42: Expected: atx; Actual: atx_closed
Heading style(MD003, heading-style)
x/evm/msg.go (1)
318-319: Improved parameter naming for clarityThe change from
baseFeetobaseFeeWeiin the method signature enhances code readability by explicitly indicating that the fee is in Wei (the smallest denomination of Ether). This naming convention is more consistent with Ethereum-related code practices.evm-forge/script/Base.s.sol (4)
1-5: Code Initialization Looks GoodThe SPDX license identifier and Solidity version pragma are correctly specified, and the import statement is properly formatted.
7-8: EnsureTEST_MNEMONICis Used Only in DevelopmentThe
TEST_MNEMONICconstant is intended for testing purposes. Please verify that this test mnemonic is not accidentally used in production environments to prevent security risks.
36-40:broadcastModifier Is Correctly ImplementedThe
broadcastmodifier appropriately starts and stops the broadcast, ensuring that any function it wraps executes within the desired transaction context.
26-34:⚠️ Potential issueConfirm Secure Handling of Mnemonic and Broadcaster Initialization
The constructor defaults to
TEST_MNEMONICif neither$ETH_FROMnor$MNEMONICare provided. In a production setting, relying on a test mnemonic could lead to security vulnerabilities. Consider enforcing the requirement of setting either$ETH_FROMor$MNEMONICin non-development environments, possibly by adding a check that reverts if none are provided.You might apply the following change to enforce this:
constructor() { address from = vm.envOr({ name: "ETH_FROM", defaultValue: address(0) }); if (from != address(0)) { broadcaster = from; } else { mnemonic = vm.envOr({ name: "MNEMONIC", defaultValue: "" }); + require(bytes(mnemonic).length != 0, "MNEMONIC environment variable must be set in production."); (broadcaster,) = deriveRememberKey({ mnemonic: mnemonic, index: 0 }); } }
| tasks: | ||
| - name: "Install dependencies" | ||
| before: | | ||
| curl -L https://foundry.paradigm.xyz | bash | ||
| source ~/.bashrc | ||
| foundryup | ||
| init: "bun install" |
There was a problem hiding this comment.
Enhance security and reliability of the installation process.
While the current setup works, there are a few areas for improvement:
-
Security: Downloading and executing a script directly from the internet (line 6) can be a security risk. Consider downloading the script first, verifying its integrity, and then executing it.
-
Version Control: There's no explicit version pinning for Foundry. This could lead to inconsistencies across different environments.
-
Error Handling: The current setup doesn't have explicit error handling. If any step fails, it might not be immediately apparent.
Here's a suggested improvement:
tasks:
- name: "Install dependencies"
before: |
FOUNDRY_VERSION="nightly"
curl -L https://foundry.paradigm.xyz > foundry_installer.sh
# Add a step to verify the script's integrity here
chmod +x foundry_installer.sh
./foundry_installer.sh -v $FOUNDRY_VERSION || { echo "Foundry installation failed"; exit 1; }
source ~/.bashrc
foundryup
init: bun install || { echo "Bun install failed"; exit 1; }This version:
- Downloads the script separately.
- Allows for version pinning of Foundry.
- Adds basic error handling.
Remember to replace "nightly" with a specific version if you want to pin to a particular release.
| { | ||
| "extends": "solhint:recommended", | ||
| "rules": { | ||
| "code-complexity": ["error", 8], | ||
| "compiler-version": ["error", ">=0.8.25"], | ||
| "func-name-mixedcase": "off", | ||
| "func-visibility": ["error", { "ignoreConstructors": true }], | ||
| "max-line-length": ["error", 120], | ||
| "named-parameters-mapping": "warn", | ||
| "no-console": "off", | ||
| "not-rely-on-time": "off", | ||
| "one-contract-per-file": "off" | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Review of Solhint configuration
The Solhint configuration extends the recommended settings, which is a good practice. However, there are a few points that require attention:
-
The compiler version is set to a very recent version (>=0.8.25). While using the latest version is generally good, it might limit compatibility with some tools or platforms. Consider if this strict requirement is necessary for your project.
-
The
func-name-mixedcaserule is turned off. This goes against common Solidity naming conventions where functions typically use camelCase. Unless there's a specific reason for this, consider enabling this rule to maintain consistency with broader Solidity practices. -
The
no-consoleandnot-rely-on-timerules are turned off. While this can be useful for development and testing, it could lead to potential security vulnerabilities if left in production code. Consider enabling these rules with exceptions for specific files or contracts where they are necessary. -
The
one-contract-per-filerule is turned off. While this allows for more flexible code organization, it can make the codebase harder to navigate and maintain. Consider enabling this rule to promote better code organization, unless there's a specific need for multiple contracts per file in your project structure.
Consider the following adjustments:
{
"extends": "solhint:recommended",
"rules": {
- "compiler-version": ["error", ">=0.8.25"],
+ "compiler-version": ["error", "^0.8.0"],
- "func-name-mixedcase": "off",
+ "func-name-mixedcase": "error",
"func-visibility": ["error", { "ignoreConstructors": true }],
"max-line-length": ["error", 120],
"named-parameters-mapping": "warn",
- "no-console": "off",
+ "no-console": "warn",
- "not-rely-on-time": "off",
+ "not-rely-on-time": "warn",
- "one-contract-per-file": "off"
+ "one-contract-per-file": "error"
}
}These changes will align the configuration more closely with common Solidity practices while still allowing for warnings in cases where exceptions might be necessary.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| { | |
| "extends": "solhint:recommended", | |
| "rules": { | |
| "code-complexity": ["error", 8], | |
| "compiler-version": ["error", ">=0.8.25"], | |
| "func-name-mixedcase": "off", | |
| "func-visibility": ["error", { "ignoreConstructors": true }], | |
| "max-line-length": ["error", 120], | |
| "named-parameters-mapping": "warn", | |
| "no-console": "off", | |
| "not-rely-on-time": "off", | |
| "one-contract-per-file": "off" | |
| } | |
| } | |
| { | |
| "extends": "solhint:recommended", | |
| "rules": { | |
| "code-complexity": ["error", 8], | |
| "compiler-version": ["error", "^0.8.0"], | |
| "func-name-mixedcase": "error", | |
| "func-visibility": ["error", { "ignoreConstructors": true }], | |
| "max-line-length": ["error", 120], | |
| "named-parameters-mapping": "warn", | |
| "no-console": "warn", | |
| "not-rely-on-time": "warn", | |
| "one-contract-per-file": "error" | |
| } | |
| } |
| # Rename instances of "PaulRBerg/foundry-template" to the new repo name in README.md for badges only | ||
| sedi "/gitpod/ s|PaulRBerg/foundry-template|"${GITHUB_REPOSITORY}"|;" "README.md" | ||
| sedi "/gitpod-badge/ s|PaulRBerg/foundry-template|"${GITHUB_REPOSITORY}"|;" "README.md" | ||
| sedi "/gha/ s|PaulRBerg/foundry-template|"${GITHUB_REPOSITORY}"|;" "README.md" | ||
| sedi "/gha-badge/ s|PaulRBerg/foundry-template|"${GITHUB_REPOSITORY}"|;" "README.md" |
There was a problem hiding this comment.
Fix quoting in sed commands to prevent unintended word splitting.
The sedi commands are correctly targeting the specific lines in the README.md file. However, there's a potential issue with the quoting of variables.
To address the Shellcheck warnings and prevent potential word splitting or globbing, modify the sedi commands as follows:
sedi "/gitpod/ s|PaulRBerg/foundry-template|${GITHUB_REPOSITORY}|;" "README.md"
sedi "/gitpod-badge/ s|PaulRBerg/foundry-template|${GITHUB_REPOSITORY}|;" "README.md"
sedi "/gha/ s|PaulRBerg/foundry-template|${GITHUB_REPOSITORY}|;" "README.md"
sedi "/gha-badge/ s|PaulRBerg/foundry-template|${GITHUB_REPOSITORY}|;" "README.md"This change ensures that the ${GITHUB_REPOSITORY} variable is properly expanded within the sed command.
🧰 Tools
🪛 Shellcheck
[warning] 35-35: The surrounding quotes actually unquote this. Remove or escape them.
(SC2027)
[warning] 36-36: The surrounding quotes actually unquote this. Remove or escape them.
(SC2027)
[warning] 37-37: The surrounding quotes actually unquote this. Remove or escape them.
(SC2027)
[warning] 38-38: The surrounding quotes actually unquote this. Remove or escape them.
(SC2027)
| name: "Create" | ||
|
|
||
| # The workflow will run when the "Use this template" button is used | ||
| on: | ||
| push: |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider improving workflow name and trigger specificity.
-
The workflow name "Create" is not very descriptive. Consider renaming it to something more specific, like "Initialize Repository from Template".
-
The current trigger (
on: push) might be too broad, potentially causing unnecessary workflow runs. Since this workflow is intended to run when the "Use this template" button is used, consider using a more specific trigger, such as:
on:
create:
branches:
- mainThis will ensure the workflow only runs when the main branch is created, which typically happens when a repository is created from a template.
|
|
||
| ## License | ||
|
|
||
| The "NibiruChain/Nibiru/evm-forge" project is licensed under MIT. |
There was a problem hiding this comment.
Correct the project name in the license section.
The project name mentioned in the license section doesn't match the repository name used earlier in the document. This could lead to confusion.
Suggested correction:
-The "NibiruChain/Nibiru/evm-forge" project is licensed under MIT.
+The "nibiru/evm-forge" project is licensed under MIT.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| The "NibiruChain/Nibiru/evm-forge" project is licensed under MIT. | |
| The "nibiru/evm-forge" project is licensed under MIT. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2084 +/- ##
==========================================
- Coverage 63.99% 63.89% -0.10%
==========================================
Files 270 270
Lines 21114 21114
==========================================
- Hits 13511 13490 -21
- Misses 6664 6688 +24
+ Partials 939 936 -3
|
Purpose / Abstract
Summary by CodeRabbit
New Features
.editorconfig,.env.example, and.gitpod.yml, to streamline the setup process.Fooand its corresponding test suite to validate functionality.Documentation
Chores