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

feat: automatically handle solc configuration for Etherscan Platform #544

Merged
merged 16 commits into from
Mar 20, 2024
Merged

feat: automatically handle solc configuration for Etherscan Platform #544

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
0ec001c
Merge pull request #503 from crytic/dev
0xalpharush Oct 17, 2023
35bbe54
prepare 0.3.5 release
0xalpharush Oct 17, 2023
a741e4a
Merge pull request #504 from crytic/bump-version
0xalpharush Oct 17, 2023
3a4b0de
Merge pull request #506 from crytic/dev
0xalpharush Oct 18, 2023
62b16c3
Merge pull request #541 from crytic/dev
0xalpharush Jan 16, 2024
bf781b1
0.3.6 release
0xalpharush Jan 16, 2024
1b2998e
Merge pull request #542 from crytic/0xalpharush-patch-1
0xalpharush Jan 16, 2024
1e5deb2
feat: PoC of auto-generating solc-remaps file
shortdoom Jan 25, 2024
b5f0d62
feat: auto-generate crytic_compile.config.json
shortdoom Jan 28, 2024
d47b73a
test: ci_test_etherscan.sh, chore: lint/refactor
shortdoom Feb 1, 2024
6bdc8e3
chore: satisfy super-linter
shortdoom Feb 1, 2024
cf981c7
Improve handling of "version" string from Etherscan
ggrieco-tob Feb 7, 2024
1bc8da8
Update etherscan.py
ggrieco-tob Feb 7, 2024
ad9bf30
fixed lint
ggrieco-tob Feb 7, 2024
c0ad9f5
Merge pull request #545 from crytic/improve-_convert_version
0xalpharush Feb 7, 2024
9c397a7
Merge branch 'crytic:master' into dev-remaps
shortdoom Feb 7, 2024
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
22 changes: 22 additions & 0 deletions crytic_compile/platform/etherscan.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,28 @@ def compile(self, crytic_compile: "CryticCompile", **kwargs: str) -> None:
via_ir=via_ir_enabled,
)

metadata_config = {
"solc_remaps": remappings if remappings else {},
"solc_solcs_select": compiler_version,
"solc_args": " ".join(
filter(
None,
[
"--via-ir" if via_ir_enabled else "",
"--optimize --optimize-runs " + str(optimize_runs) if optimize_runs else "",
"--evm-version " + evm_version if evm_version else "",
],
)
),
}

with open(
os.path.join(working_dir if working_dir else export_dir, "crytic_compile.config.json"),
"w",
encoding="utf-8",
) as f:
json.dump(metadata_config, f)

def clean(self, **_kwargs: str) -> None:
pass

Expand Down
31 changes: 31 additions & 0 deletions scripts/ci_test_etherscan.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,3 +99,34 @@ then
exit 255
fi
echo "::endgroup::"

# From crytic/crytic-compile#544
echo "::group::Etherscan #8"
crytic-compile 0x9AB6b21cDF116f611110b048987E58894786C244 --etherscan-apikey "$GITHUB_ETHERSCAN"

if [ $? -ne 0 ]
then
echo "Etherscan #8 test failed"
exit 255
fi

dir_name=$(ls crytic-export/etherscan-contracts/ | grep 0x9AB6b21cDF116f611110b048987E58894786C244)
cd crytic-export/etherscan-contracts/$dir_name

if [ ! -f crytic_compile.config.json ]; then
echo "crytic_compile.config.json does not exist"
exit 255
fi

# TODO: Globbing at crytic_compile.py:720 to run with '.'
crytic-compile 'contracts/InterestRates/InterestRatePositionManager.f.sol' --config-file crytic_compile.config.json

if [ $? -ne 0 ]
then
echo "crytic-compile command failed"
exit 255
fi

cd ../../../

echo "::endgroup::"
Copy link

Choose a reason for hiding this comment

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

The addition of Etherscan test #8 introduces a new test case for compiling a specific contract and checking for the existence of crytic_compile.config.json before running the compilation process. This is a positive step towards automating the generation of solc-remaps files for projects downloaded from Etherscan. However, there are a few areas that could be improved for clarity, maintainability, and robustness:

  1. Use of $? for Exit Status: Directly after running a command, the script checks $? for the exit status. This is a common practice but can lead to errors if additional commands are inadvertently inserted between the command and its status check. Consider capturing the exit status immediately in a variable to avoid such issues.

  2. Hardcoded Contract Address: The contract address 0x9AB6b21cDF116f611110b048987E58894786C244 is used twice, suggesting it has a specific significance for this test. It would be beneficial to define this as a variable at the start of the test section for clarity and ease of maintenance.

  3. Checking for Configuration File Existence: The script checks for the existence of crytic_compile.config.json but does not verify its content or structure. While the presence of the file is a good first step, ensuring it contains the expected configuration (even in a basic form) could prevent future errors.

  4. TODO Comment on Globbing: The TODO comment regarding globbing at crytic_compile.py:720 to run with '.' suggests an improvement or a fix that is pending. It's important to track these TODOs outside of the codebase, such as in an issue tracker, to ensure they are not forgotten.

  5. Directory Navigation: The script navigates into and out of directories using cd. While this works, it can be error-prone if the script fails or exits unexpectedly before reaching the cd ../../../. Consider using pushd and popd for more robust directory navigation, or running the commands in a subshell to avoid changing the working directory of the script.

  6. Error Handling and Messages: The error messages are clear but could include more context to aid in debugging. For example, specifying which part of the test failed or including the contract address in the error message could provide immediate clues without needing to dig through the script.

Improvements in these areas could enhance the script's readability, maintainability, and error resilience.

+ CONTRACT_ADDRESS="0x9AB6b21cDF116f611110b048987E58894786C244"
+ echo "Testing Etherscan compilation for contract $CONTRACT_ADDRESS"
- 105~
- 106~ if [ $? -ne 0 ]
+ COMPILE_STATUS=$?
+ if [ $COMPILE_STATUS -ne 0 ]
- 117~ # TODO: Globbing at crytic_compile.py:720 to run with '.'
+ # Consider tracking this TODO in an issue tracker
- 119~ if [ $? -ne 0 ]
+ COMPILE_STATUS=$?
+ if [ $COMPILE_STATUS -ne 0 ]
- 124~ cd ../../../
+ popd
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.

Suggested change
# From crytic/crytic-compile#544
echo "::group::Etherscan #8"
crytic-compile 0x9AB6b21cDF116f611110b048987E58894786C244 --etherscan-apikey "$GITHUB_ETHERSCAN"
if [ $? -ne 0 ]
then
echo "Etherscan #8 test failed"
exit 255
fi
dir_name=$(ls crytic-export/etherscan-contracts/ | grep 0x9AB6b21cDF116f611110b048987E58894786C244)
cd crytic-export/etherscan-contracts/$dir_name
if [ ! -f crytic_compile.config.json ]; then
echo "crytic_compile.config.json does not exist"
exit 255
fi
# TODO: Globbing at crytic_compile.py:720 to run with '.'
crytic-compile 'contracts/InterestRates/InterestRatePositionManager.f.sol' --config-file crytic_compile.config.json
if [ $? -ne 0 ]
then
echo "crytic-compile command failed"
exit 255
fi
cd ../../../
echo "::endgroup::"
# From crytic/crytic-compile#544
echo "::group::Etherscan #8"
CONTRACT_ADDRESS="0x9AB6b21cDF116f611110b048987E58894786C244"
echo "Testing Etherscan compilation for contract $CONTRACT_ADDRESS"
crytic-compile $CONTRACT_ADDRESS --etherscan-apikey "$GITHUB_ETHERSCAN"
COMPILE_STATUS=$?
if [ $COMPILE_STATUS -ne 0 ]
then
echo "Etherscan #8 test failed"
exit 255
fi
dir_name=$(ls crytic-export/etherscan-contracts/ | grep $CONTRACT_ADDRESS)
cd crytic-export/etherscan-contracts/$dir_name
if [ ! -f crytic_compile.config.json ]; then
echo "crytic_compile.config.json does not exist"
exit 255
fi
# Consider tracking this TODO in an issue tracker
crytic-compile 'contracts/InterestRates/InterestRatePositionManager.f.sol' --config-file crytic_compile.config.json
COMPILE_STATUS=$?
if [ $COMPILE_STATUS -ne 0 ]
then
echo "crytic-compile command failed"
exit 255
fi
popd
echo "::endgroup::"

2 changes: 1 addition & 1 deletion setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
description="Util to facilitate smart contracts compilation.",
url="https://github.com/crytic/crytic-compile",
author="Trail of Bits",
version="0.3.4",
version="0.3.6",
packages=find_packages(),
# Python 3.12.0 on Windows suffers from https://github.com/python/cpython/issues/109590
# breaking some of our integrations. The issue is fixed in 3.12.1
Expand Down