Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix foundry version check #534

Merged
merged 1 commit into from
Feb 24, 2025
Merged

Fix foundry version check #534

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion dev/up
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
set -e

if ! which forge &>/dev/null; then echo "ERROR: Missing foundry binaries. Run 'curl -L https://foundry.paradigm.xyz | bash' and follow the instructions to install foundry 1.0.0" && exit 1; fi
if ! forge --version | grep -q "v1.0.0"; then echo "ERROR: Foundry version is not 1.0.0. Please install the correct version." && exit 1; fi
if ! forge --version | grep -q "1.0.0"; then echo "ERROR: Foundry version is not 1.0.0. Please install the correct version." && exit 1; fi
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Verify Foundry version output format and improve version check precision.

While removing the "v" prefix aligns with Foundry's version output, the current grep check could be too permissive. It might match "1.0.0" within other version strings (e.g., "11.0.0" or "1.0.01").

Let's verify the Foundry version output format and check for potential version mismatches:

Consider using a more precise version check:

-if ! forge --version | grep -q "1.0.0"; then
+if ! forge --version | grep -q "^forge 1\.0\.0\b"; then

This ensures:

  1. The version string starts with "forge "
  2. Exactly matches "1.0.0"
  3. Prevents partial matches within other version numbers

🏁 Script executed:

#!/bin/bash
# Description: Check Foundry version output format and test version string matching

# Get Foundry version output format
forge --version

# Test if the current grep would incorrectly match these versions
echo "forge 11.0.0" | grep -q "1.0.0" && echo "WARNING: Would incorrectly match 11.0.0"
echo "forge 1.0.01" | grep -q "1.0.0" && echo "WARNING: Would incorrectly match 1.0.01"

Length of output: 367

Action Required: Refine the Foundry version check for precise matching.

  • The current grep check with "1.0.0" is too permissive and incorrectly matches version strings such as 11.0.0 and 1.0.01.
  • Verification via test commands confirms that these false positives occur.
  • It is recommended to use a regex that anchors to the start of the version string and enforces exact matching.

Consider updating the version check in dev/up as follows:

-if ! forge --version | grep -q "1.0.0"; then echo "ERROR: Foundry version is not 1.0.0. Please install the correct version." && exit 1; fi
+if ! forge --version | grep -q "^forge 1\.0\.0\b"; then echo "ERROR: Foundry version is not 1.0.0. Please install the correct version." && exit 1; fi
📝 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.

Suggested change
if ! forge --version | grep -q "1.0.0"; then echo "ERROR: Foundry version is not 1.0.0. Please install the correct version." && exit 1; fi
if ! forge --version | grep -q "^forge 1\.0\.0\b"; then echo "ERROR: Foundry version is not 1.0.0. Please install the correct version." && exit 1; fi

if ! which shellcheck &>/dev/null; then brew install shellcheck; fi
if ! which jq &>/dev/null; then brew install jq; fi

Expand Down
Loading