risc0 / risc0-foundry-template

Template for integrating RISC Zero and Ethereum using Foundry
https://risczero.com
107 stars 60 forks source link

Add some versioning & tags relating to upstream releases #124

Open nuke-web3 opened 5 months ago

nuke-web3 commented 5 months ago

It's not clear what version of this template is compatible with https://github.com/risc0/risc0/ and/or https://github.com/risc0/risc0-ethereum/ releases.

I can guess based on the PRs and commit history & cargo files... but that is really not great UX

nuke-web3 commented 5 months ago

from #110 it seems 27eba00a5237cbefd0c742dee73ced697df3527a is a candidate to tag v1.0.0 ?

also perhaps missed in that PR: https://github.com/risc0/risc0-foundry-template/blob/37d4ce7f927b2c018538378799258373fbbba4c8/.gitmodules#L10

https://stackoverflow.com/questions/1777854/how-can-i-specify-a-branch-tag-when-adding-a-git-submodule suggests perhaps branch should be omitted and manually update the submodules correctly before the tag is set at a commit that locks in those submodule upstream commits used?

Update: diff to the "right" branch might be a blocker: https://github.com/risc0/risc0-ethereum/compare/v1.0.0-rc.6...release-1.0

nuke-web3 commented 5 months ago

Error: failed to fill whole bufferthread main' panicked at risc0/r0vm/src/lib.rs:192:18:calledResult::unwrap()on anErrvalue: incompatible client version: 1.1.0-alpha.1, server version: 1.0.1

This comes from using an incompatible submodule in forge test. This is a really bad error message to inform that is the case IMHO. The error is also not rendered with \n in stdout, so I needed to manually parse it to find that error in a single line of 1000s of chars :see_no_evil:

This .gitsubmodule worked for me, being careful here that I MUST match my Cargo.toml deps on all versions to match as well.

[submodule "remote-network-prover/lib/risc0-ethereum"]
    path = remote-network-prover/lib/risc0-ethereum
    url = https://github.com/risc0/risc0-ethereum
    branch = release-1.0
[submodule "remote-network-prover/lib/forge-std"]
    path = remote-network-prover/lib/forge-std
    url = https://github.com/foundry-rs/forge-std
[submodule "remote-network-prover/lib/openzeppelin-contracts"]
    path = remote-network-prover/lib/openzeppelin-contracts
    url = https://github.com/OpenZeppelin/openzeppelin-contracts

I then get another error after a long time waiting on some proofs to run for a not so easy to spot in a single line of 1000s of chars -> I need docker:

2024-07-02T03:21:59.418511Z ERROR cheatcodes: non-empty stderr input=["cargo", "run", "--manifest-path", "lib/risc0-ethereum/ffi/Cargo.toml", "--bin", "risc0-forge-ffi", "-q", "prove", "/home/nuke/git/r0/workshops/test/remote-network-prover/target/riscv-guest/riscv32im-risc0-zkvm-elf/release/is-even", "0x0000000000000000000000000000000000000000000000000000000000bc614e"] stderr="Error: Please install docker first.

Ideally if we know we need docker, we check that before doing any more work & fail fast with a message.

Finally I still error with

2024-07-02T03:30:57.235374Z ERROR cheatcodes: non-empty stderr input=["cargo", "run", "--manifest-path", "lib/risc0-ethereum/ffi/Cargo.toml", "--bin", "risc0-forge-ffi", "-q", "prove", "/home/nuke/git/r0/workshops/test/remote-network-prover/target/riscv-guest/riscv32im-risc0-zkvm-elf/release/is-even", "0x0000000000000000000000000000000000000000000000000000000000bc614e"] stderr="Error: docker returned failure exit code: Some(125)

Note that I am trying to alias docker to podman with ln -s /usr/bin/podman /usr/bin/docker that might be to blame... I would love to have podman as a first class option or other alts to hard-coded docker as the only container option we support.

UPDATE: tried with docker too, so likely there is some issue on my end. Using foundry template from 37d4ce7f927b2c018538378799258373fbbba4c8 with the edited submodule listed above.

https://stackoverflow.com/a/35410993 suggests that there is something happening in the container or the command itself at fault? This is not great UX :see_no_evil:

Subproject commit
openzeppelin-contracts ccc110360f1028143e137258d12a0891f17df3a4
forge-std 37d4ce7f927b2c018538378799258373fbbba4c8
risc0-ethereum ebec385cc526adb9279c1af55d699c645ca6d694

RISC0_DEV_MODE=true forge test -vvv passes test (with errors warning about dev mode) when using risc0-ethereum ebec385cc526adb9279c1af55d699c645ca6d694 - but same issues as above with with a real proof.

nuke-web3 commented 5 months ago

manually checking out the v1.0.0-rc.6 tag mentioned in the existing (but not working! .gitmodule as it must be a branch!) and committing this

non-empty stderr input=["cargo", "run", "--manifest-path", "lib/risc0-ethereum/ffi/Cargo.toml", "--bin", "risc0-forge-ffi", "-q", "prove", "/home/nuke/git/r0/workshops/test/remote-network-prover/target/riscv-guest/riscv32im-risc0-zkvm-elf/release/is-even", "0x0000000000000000000000000000000000000000000000000000000000000000"] stderr="Error: failed to fill whole bufferthread 'main' panicked at risc0/r0vm/src/lib.rs:192:18:\ncalledResult::unwrap()on anErrvalue: incompatible client version: 1.0.0-rc.6, server version: 1.0.1\n\nStack backtrace:

So again in version hell :face_exhaling: why are patch versions not good enough to work here? Semver not respected?

resolved in #126

nategraf commented 5 months ago

So again in version hell 😮‍💨 why are patch versions not good enough to work here? Semver not respected?

1.0.0-rc.6 is not compatible with 1.0.0 or 1.0.1 by the rules of semver. https://semver.org/#spec-item-9

nuke-web3 commented 5 months ago

TIL more about semver :bow:

Putting here for more motivation on better versions for the many moving parts in risc0 ethereum https://github.com/risc0/risc0-ethereum/issues/137

Also noticing in examples many will want to use that we have cargo.io patches in use - what is the versioning and upgrade story on those? Like:

https://github.com/risc0/risc0-ethereum/blob/604be7888bba8b804236f41c241750f98cac820b/examples/erc20/methods/guest/Cargo.toml#L17-L21