-
Notifications
You must be signed in to change notification settings - Fork 370
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
Init simple installer #1082
base: master
Are you sure you want to change the base?
Init simple installer #1082
Conversation
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.
Great work @bohendo! 🎉 I left some thoughts inline
if ! slither --version > /dev/null 2>&1 | ||
then | ||
echo "slither is not available, installing it now.." | ||
pip3 install slither-analyzer --user |
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.
It would be cool to have slither updated if it's out of date. PATH may also need to be adjusted to account for slither installed with --user
.
Maybe we should use a (fresh?) venv instead and just link slither{,-*}
and crytic-compile
to the directory we then add to the PATH for echidna?
install.sh
Outdated
BIN_PATH="$ECHIDNA_BIN_DIR/echidna" | ||
|
||
if [[ "$(uname)" == "Darwin" ]] | ||
then arch="MacOS" |
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.
we should differentiate ARM vs x86 builds, maybe by inspecting uname -m
. Otherwise M1 users will get slow fuzzing performance.
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.
Is there a separate release for ARM vs x86 macs? Under release assets I only see MacOS vs Ubuntu vs Windows
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.
@bohendo yeah, there's MacOS-aarch64 files, with a different structure inside the tarball, e.g see https://github.com/crytic/echidna/releases/tag/v2.1.0
These are manually built (with the nix bundle target in the readme), compressed and uploaded; it seems the latest release is missing them. We used to have a more comprehensive build process that generated a ready to upload archive and fixed TERMINFO on the resulting binaries (the current process has this bug again), maybe we can bring that back to aid releases going forward?
install.sh
Outdated
fi | ||
|
||
TARBALL_URL="https://github.com/crytic/echidna/releases/download/v$version/echidna-$version-$arch.tar.gz" | ||
DIGEST_URL="https://github.com/crytic/echidna/releases/download/v$version/echidna-$version-$arch.tar.gz.sha256" |
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.
Our digests are not signed, so someone tampering with the releases may also modify the digest. Is this meant only as an integrity check? Otherwise hardcoding the hashes next to the version in the script may provide a bit more of certainty (unless someone tampers with the script as well)
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.
Just integrity, hardcoding the hashes seems like a better path forward 👍
I think we should address @elopez concerns and then merge. |
set -e | ||
|
||
version="2.2.0" | ||
slither_version="0.9.3" |
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.
slither_version="0.9.3" | |
slither_version="0.9.6" |
bohendo seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
resolves #811
This script will:
$HOME/.echidna/bin/echidna
This has been tested manually on MacOS.
TODO:
$HOME/.echidna
is well suited to supporting this feature)