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.

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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Init simple installer #1082

wants to merge 7 commits into from

Conversation

bohendo
Copy link

@bohendo bohendo commented Jun 21, 2023

resolves #811

This script will:

  • exit if echidna is already installed
  • exit if python3 is not installed
  • install pip3 if not already present
  • install slither via pip3 if not already present
  • download the mac or linux tarball per current system
  • verify checksum
  • unpack & place echidna executable at $HOME/.echidna/bin/echidna
  • add PATH update to .bashrc or .zshenv or other shell-specific config

This has been tested manually on MacOS.

TODO:

  • upgrade old echidna to latest version (basic layout of $HOME/.echidna is well suited to supporting this feature)
  • add automated tests for MacOS + Linux
  • add Windows support + tests
  • optional: host this script somewhere memorable & easy to type out on the command line

Copy link
Member

@elopez elopez left a 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
Copy link
Member

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"
Copy link
Member

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.

Copy link
Author

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

Copy link
Member

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"
Copy link
Member

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)

Copy link
Author

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 👍

@ggrieco-tob
Copy link
Member

I think we should address @elopez concerns and then merge.

set -e

version="2.2.0"
slither_version="0.9.3"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
slither_version="0.9.3"
slither_version="0.9.6"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create a 1-line installer and upgrader
4 participants