-
Notifications
You must be signed in to change notification settings - Fork 84
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.
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
sync master <> dev #552
sync master <> dev #552
Conversation
Bumps [cachix/cachix-action](https://github.com/cachix/cachix-action) from 13 to 14. - [Release notes](https://github.com/cachix/cachix-action/releases) - [Commits](cachix/cachix-action@v13...v14) --- updated-dependencies: - dependency-name: cachix/cachix-action dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]>
Bumps [cachix/install-nix-action](https://github.com/cachix/install-nix-action) from 24 to 25. - [Release notes](https://github.com/cachix/install-nix-action/releases) - [Commits](cachix/install-nix-action@v24...v25) --- updated-dependencies: - dependency-name: cachix/install-nix-action dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]>
…hix/install-nix-action-25 Bump cachix/install-nix-action from 24 to 25
…hix/cachix-action-14 Bump cachix/cachix-action from 13 to 14
Bumps [pypa/gh-action-pypi-publish](https://github.com/pypa/gh-action-pypi-publish) from 1.8.11 to 1.8.12. - [Release notes](https://github.com/pypa/gh-action-pypi-publish/releases) - [Commits](pypa/gh-action-pypi-publish@v1.8.11...v1.8.12) --- updated-dependencies: - dependency-name: pypa/gh-action-pypi-publish dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]>
…a/gh-action-pypi-publish-1.8.12 Bump pypa/gh-action-pypi-publish from 1.8.11 to 1.8.12
Bumps [pypa/gh-action-pypi-publish](https://github.com/pypa/gh-action-pypi-publish) from 1.8.12 to 1.8.14. - [Release notes](https://github.com/pypa/gh-action-pypi-publish/releases) - [Commits](pypa/gh-action-pypi-publish@v1.8.12...v1.8.14) --- updated-dependencies: - dependency-name: pypa/gh-action-pypi-publish dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]>
…a/gh-action-pypi-publish-1.8.14 Bump pypa/gh-action-pypi-publish from 1.8.12 to 1.8.14
Allow library users to use a custom current working directory
feat: automatically handle solc configuration for Etherscan Platform
WalkthroughThe recent updates streamline development workflows and enhance compilation capabilities. The GitHub actions for Continuous Integration (CI) and publishing to PyPI have been upgraded for better efficiency. Additionally, enhancements in the Changes
Poem
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (5)
- .github/workflows/ci.yml (1 hunks)
- .github/workflows/publish.yml (1 hunks)
- crytic_compile/crytic_compile.py (2 hunks)
- crytic_compile/platform/etherscan.py (1 hunks)
- scripts/ci_test_etherscan.sh (1 hunks)
Additional comments: 4
.github/workflows/publish.yml (1)
- 48-48: The update to
pypa/[email protected]
looks good. It's always a good practice to check the changelog for the action to ensure there are no breaking changes that could affect your workflow..github/workflows/ci.yml (2)
- 66-66: The update to
cachix/install-nix-action@v25
looks good. It's always beneficial to check the changelog for the action to ensure there are no breaking changes that could affect your workflow.- 69-69: The update to
cachix/cachix-action@v14
looks good. As with any action update, reviewing the changelog for potential breaking changes or new features is recommended.crytic_compile/crytic_compile.py (1)
- 148-152: The implementation for specifying a custom working directory (
cwd
) in the__init__
method is correctly handled. It allows for greater flexibility in specifying the compilation context.
|
||
# 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=$(find crytic-export/etherscan-contracts/ -type d -name "*0x9AB6b21cDF116f611110b048987E58894786C244*" -print -quit) | ||
cd "$dir_name" || { echo "Failed to change directory"; exit 255; } | ||
|
||
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::" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The addition of the Etherscan #8 test case is a good enhancement for testing coverage. However, the style of checking exit codes can be improved for better readability. Instead of checking $?
after each command, you can directly check the command's success, like so:
- if [ $? -ne 0 ]
+ if ! crytic-compile 0x9AB6b21cDF116f611110b048987E58894786C244 --etherscan-apikey "$GITHUB_ETHERSCAN"; then
This change can be applied to all instances where exit codes are checked in this script.
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.
# 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=$(find crytic-export/etherscan-contracts/ -type d -name "*0x9AB6b21cDF116f611110b048987E58894786C244*" -print -quit) | |
cd "$dir_name" || { echo "Failed to change directory"; exit 255; } | |
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" | |
if ! crytic-compile 0x9AB6b21cDF116f611110b048987E58894786C244 --etherscan-apikey "$GITHUB_ETHERSCAN"; then | |
echo "Etherscan #8 test failed" | |
exit 255 | |
fi | |
dir_name=$(find crytic-export/etherscan-contracts/ -type d -name "*0x9AB6b21cDF116f611110b048987E58894786C244*" -print -quit) | |
cd "$dir_name" || { echo "Failed to change directory"; exit 255; } | |
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 '.' | |
if ! crytic-compile 'contracts/InterestRates/InterestRatePositionManager.f.sol' --config-file crytic_compile.config.json; then | |
echo "crytic-compile command failed" | |
exit 255 | |
fi | |
cd ../../../ | |
echo "::endgroup::" |
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) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The addition of the metadata_config
dictionary and its export to a JSON file is a valuable enhancement for customizable compilation processes. However, for clarity and consistency, consider renaming the dictionary keys to match common naming conventions in Solc configurations:
{
- "solc_remaps": remappings if remappings else {},
+ "solcRemappings": remappings if remappings else {},
- "solc_solcs_select": compiler_version,
+ "solcVersion": compiler_version,
- "solc_args": " ".join(
+ "solcArgs": " ".join(
This change would make the keys more intuitive and consistent with Solc's naming conventions.
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.
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) | |
metadata_config = { | |
"solcRemappings": remappings if remappings else {}, | |
"solcVersion": compiler_version, | |
"solcArgs": " ".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) | |
Summary by CodeRabbit
Chores
New Features
Refactor