succinctlabs / sp1-project-template

Template for creating an SP1 project that can generate a proof of any RISC-V program and verify the proof on-chain.
MIT License
51 stars 9 forks source link

Move program and script to a shared workspace #11

Open srdtrk opened 1 month ago

srdtrk commented 1 month ago

Currently, the program and script are maintained in separate workspaces. This separation has led to code duplication, such as the Solidity definition of PublicValuesTuple which is defined both in the script (here) and in the program (here).

While this is a minor example of code duplication, the impact could be more significant in larger projects. To address this, I propose extending the workspace to include both program,script, and maybe even a shared types package. This would facilitate better project structure and eliminate redundant code.

While there are other solutions, such as locally importing a shared crate, I don't think this approach is considered best practice. Combining the workspaces could provide a cleaner, more maintainable solution.

I am willing to try open a PR to implement this change if there is agreement on this approach.

alxiong commented 2 weeks ago

@srdtrk did you get it to work?

When i removed the [workspace] in script/Cargo.toml and program/Cargo.toml, building inside the script/ would fail.

[fibonacci-script 0.1.0] path: "../program"
[fibonacci-script 0.1.0] cargo::rerun-if-changed=../program/src
[fibonacci-script 0.1.0] cargo::rerun-if-changed=../program/Cargo.toml
[fibonacci-script 0.1.0] cargo::rerun-if-changed=../program/Cargo.lock
[fibonacci-script 0.1.0] cargo:warning=fibonacci-program built at 2024-07-04 00:25:02
[fibonacci-script 0.1.0] [sp1]     Blocking waiting for file lock on build directory
    Building [=======================> ] 698/700: fibonacci-script(build)  

this is a deadlock. (will get stuck indefinitely)

I suspect that: when build_program() inside the script/build.rs would first run cargo build on entire workspace of ../program , and if program/ and script are inside the same workspace, then it will then try to build script/ as well.

^^ this is probably why they have to be in their own separate workspaces. but it's a bit annoying.

cc @ratankaliani

srdtrk commented 2 weeks ago

I have a repo where I did more complex stuff, all in the same workspace. I actually don't like using build_program() so I removed it from my setup, instead I build it manually