Skip to content

feat: feat: add CI that ream-consensus and ream-bls compiles to SP1 #154

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

Jump to bottom
Merged
merged 12 commits into from
Mar 3, 2025
Merged

feat: feat: add CI that ream-consensus and ream-bls compiles to SP1 #154

merged 12 commits into from
Mar 3, 2025

Conversation

Kayden-ML
Copy link
Contributor

Issue: #152

@Kayden-ML Kayden-ML self-assigned this Mar 1, 2025
@Kayden-ML Kayden-ML added this to the v0.1.0 milestone Mar 2, 2025
@Kayden-ML Kayden-ML moved this to In Progress in Ream Client Mar 2, 2025
@Kayden-ML Kayden-ML marked this pull request as ready for review March 2, 2025 00:52
@Kayden-ML
feat: Add CI that compiles to SP1
5a4e5f4 
fix: testing

fix: testing

fix: testing

fix: testing

fix: testing

fix: testing

fix: testing

fix: testing

fix: testing

fix: testing

fix: testing

fix: testing

fix: testing

fix: testing

fix: testing

test: testing

fix: testing

fix: testing

fix: testing

fix: testing

fix: testing

fix: testing

fix: feating

fix: testing

fix: testing

fix: testing

fix: testing
@Kayden-ML Kayden-ML force-pushed the Add-CI-that-compiles-to-SP1 branch from ae70851 to 5a4e5f4 Compare March 2, 2025 00:56
KolbyML
KolbyML reviewed Mar 2, 2025
View reviewed changes
.github/workflows/rust.yml Outdated
Comment on lines 21 to 24
- name: Install Dependencies
run: |
sudo apt-get update
sudo apt-get install -y curl git
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- name: Install Dependencies
run: |
sudo apt-get update
sudo apt-get install -y curl git

we don't need this

KolbyML
KolbyML reviewed Mar 2, 2025
View reviewed changes
.github/workflows/rust.yml Outdated
Comment on lines 29 to 31
source ~/.bashrc
export PATH=$PATH:~/.sp1/bin
sp1up
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
source ~/.bashrc
export PATH=$PATH:~/.sp1/bin
sp1up
/home/runner/.sp1/bin/sp1up

You can simplify this like you did for risc0

@syjn99
Copy link
Member

syjn99 commented Mar 2, 2025

All looks good. Thanks!
One suggestion after this PR is merged: how about separating workflow files for checking RISC-V compatability? @KolbyML What do you think of this idea?

@syjn99 syjn99 linked an issue Mar 2, 2025 that may be closed by this pull request
Add CI that compiles to SP1 #152
Closed
@KolbyML
Copy link
Contributor

KolbyML commented Mar 2, 2025 •
edited
Loading

All looks good. Thanks! One suggestion after this PR is merged: how about separating workflow files for checking RISC-V compatability? @KolbyML What do you think of this idea?

I think the riscv CI's should be in the same file, but a different file then this one is fine.

@Kayden-ML could you move these 2 compiling riscv code to a different workflow file

KolbyML
KolbyML reviewed Mar 2, 2025
View reviewed changes
.github/workflows/riscV.yml
Copy link
Contributor

Choose a reason for hiding this comment

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

can you make the V lowercase in the file and in the file name

KolbyML
KolbyML reviewed Mar 2, 2025
View reviewed changes
.github/workflows/riscV.yml Outdated
- name: Build Risc0
run: cargo build --target=riscv32im-risc0-zkvm-elf -p ream-consensus -p ream-bls

No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

missing a new line here
image

KolbyML
KolbyML reviewed Mar 3, 2025
View reviewed changes
.github/workflows/riscV.yml Outdated
@@ -0,0 +1,46 @@
name: Riscv
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
name: Riscv
name: RISC-V

KolbyML
KolbyML previously approved these changes Mar 3, 2025
View reviewed changes
Copy link
Contributor

@KolbyML KolbyML 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 figuring this all out!

@KolbyML KolbyML dismissed their stale review March 3, 2025 00:05

one second

KolbyML
KolbyML reviewed Mar 3, 2025
View reviewed changes
.github/workflows/riscV.yml
Copy link
Contributor

Choose a reason for hiding this comment

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

this files name still has a uppercase V

KolbyML
KolbyML reviewed Mar 3, 2025
View reviewed changes
.github/workflows/riscV.yml Outdated
jobs:
build-sp1:
runs-on: ubuntu-latest
needs: [cargo-fmt, cargo-clippy]
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like the risc CI's are now not running?

trying removing the needs lines for both of them. I am guessing it is due to these jobs not being in the same file node

KolbyML
KolbyML approved these changes Mar 3, 2025
View reviewed changes
Copy link
Contributor

@KolbyML KolbyML left a comment

Choose a reason for hiding this comment

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

:shipit: looks good, good shit man!

@Kayden-ML Kayden-ML added this pull request to the merge queue Mar 3, 2025
Merged via the queue into ReamLabs:master with commit 50e0ccc Mar 3, 2025
7 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in Ream Client Mar 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@unnawut unnawut unnawut approved these changes

@KolbyML KolbyML KolbyML approved these changes

@jihoonsong jihoonsong Awaiting requested review from jihoonsong

@syjn99 syjn99 Awaiting requested review from syjn99

Assignees

@Kayden-ML Kayden-ML

Labels

Consensus ZK Proving

Projects

Ream Client
Status: Done

Milestone

v0.1.0

Development

Successfully merging this pull request may close these issues.

Add CI that compiles to SP1

4 participants

@Kayden-ML @syjn99 @KolbyML @unnawut