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

feat: add docker #40

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

distributedstatemachine

Motivation

Provide an additional method of running cryo

Solution

Add a dockerfile to the repo

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

Copy link
Contributor

@kskalski kskalski left a comment

Choose a reason for hiding this comment

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

FYI
you may compare with my version https://github.com/gradcap/cryo/blob/main/Dockerfile

Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
@distributedstatemachine
Copy link
Author

FYI you may compare with my version https://github.com/gradcap/cryo/blob/main/Dockerfile

@kskalski Thanks alot for this ! The Docker image is based off foundry's , but we could not use alpine as the python bindings require cylib which rust doesnt support in alpine, so book work is a good shout !

Copy link
Contributor

@kskalski kskalski left a comment

Choose a reason for hiding this comment

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

fine from my side, you will need approval from repo owners though

# RUN apt-get update && apt-get install -y clang lld curl build-essential linux-generic git \
# && curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs > rustup.sh \
# && chmod +x ./rustup.sh \
# && ./rustup.sh -y
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess you can clean up this comment

@@ -31,7 +31,6 @@ cryo can extract the following datasets from EVM nodes:
- `blocks`
- `transactions` (alias = `txs`)
- `logs` (alias = `events`)
- `contracts`
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this is unrelated, rebase?

RUN [[ "$TARGETARCH" = "x86_64-unknown-linux-gnu" ]] && echo "export CFLAGS=-mno-outline-atomics" >> $HOME/.profile || true

WORKDIR /opt/cryo
COPY . .
Copy link
Contributor

Choose a reason for hiding this comment

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

to avoid pulling unnecessary parts of repo I would explicitly add crates and Cargo.toml Cargo.lock are src copy

@sslivkoff
Copy link
Member

this looks really nice. @samtvlabs can you resolve the issues highlighted by @kskalski then it should be ready for merge

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.

3 participants