Skip to content

Commit

Permalink
Set RISC0_FEATURE_bigint2 in risc0-build to signal zkVM features and …
Browse files Browse the repository at this point in the history
…fix usage of `stability::unstable` with `no_mangle` (risc0#2567)

This is a proposal for how we might improve build debugging when someone
tries to use bigint2, but it is not available. In this PR, an env var is
added to invocations of `cargo` in `risc0-build` that is a positive
signal to downstream crates that that a new "hardware" feature is
available.

In the case of upcoming cryptography patches that only work with
`bigint2`, we'd add the following snippet to the `lib.rs` file. The
advantage being that we give an error and link to our docs rather than a
linker error at the end of the build process. From there, we can provide
better guidance on either upgrading the `risc0-zkvm` version or
downgrading the cryptography patch version.

```rust
#[cfg(all(target_os = "zkvm", target_arch = "riscv32"))]
const _: () = {
    assert!(
        core::option_env!("RISC0_FEATURE_bigint2").is_some(),
        r#"
RISC Zero zkVM feature bigint2 is not available, and is required by this crate.

If you'd like to use bigint2, please upgrade to risc0-zkvm and risc0-build and ensure the required
feature flags are enabled. See the RISC Zero dev docs for more information.
https://dev.risczero.com/api/zkvm/acceleration
"#
    );
};
```

This is written with bigint2 in mind. It could also be used when
transitioning to the new ABI with zkVM revision 2.

When I went to test this I found that my ECDSA program linked and built
without setting `unstable` on `risc0-zkvm`, which was unexpected.
Digging into the `stability` crate I found that the way the implement
their macro [1], it doesn't play nice with `no_mangle` and the symbol
will still be included.

https://docs.rs/stability/latest/src/stability/unstable.rs.html#85
  • Loading branch information
nategraf authored Dec 2, 2024
1 parent 0034d08 commit 0dea13c
Show file tree
Hide file tree
Showing 7 changed files with 28 additions and 9 deletions.
2 changes: 1 addition & 1 deletion examples/jwt-validator/methods/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ version = "0.1.0"
edition = "2021"

[build-dependencies]
risc0-build = { path = "../../../risc0/build" }
risc0-build = { path = "../../../risc0/build", features = ["unstable"] }

[package.metadata.risc0]
methods = ["guest"]
1 change: 1 addition & 0 deletions examples/jwt-validator/methods/guest/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ edition = "2021"
jwt-core = { path = "../../core/" }
risc0-zkvm = { path = "../../../../risc0/zkvm", default-features = false, features = [
"std",
"unstable",
] }

[patch.crates-io]
Expand Down
2 changes: 1 addition & 1 deletion risc0/bigint2/methods/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ version = { workspace = true }
edition = { workspace = true }

[build-dependencies]
risc0-build = { workspace = true }
risc0-build = { workspace = true, features = ["unstable"] }

[package.metadata.release]
release = false
Expand Down
14 changes: 14 additions & 0 deletions risc0/bigint2/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,20 @@ pub trait ToBigInt2Buffer<const WIDTH: usize> {
fn from_u32_array(array: [u32; WIDTH]) -> Self;
}

#[cfg(all(target_os = "zkvm", target_arch = "riscv32"))]
const _: () = {
assert!(
core::option_env!("RISC0_FEATURE_bigint2").is_some(),
r#"
RISC Zero zkVM feature bigint2 is not available, and is required by this crate.
If you'd like to use bigint2, please upgrade to risc0-zkvm and risc0-build and ensure the required
feature flags are enabled. See the RISC Zero dev docs for more information.
https://dev.risczero.com/api/zkvm/acceleration
"#
);
};

#[cfg(feature = "num-bigint")]
impl<const WIDTH: usize> ToBigInt2Buffer<WIDTH> for num_bigint::BigUint {
fn to_u32_array(&self) -> [u32; WIDTH] {
Expand Down
1 change: 1 addition & 0 deletions risc0/build/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,4 @@ all-features = true
[features]
docker = []
guest-list = []
unstable = []
6 changes: 6 additions & 0 deletions risc0/build/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,12 @@ pub fn cargo_command(subcmd: &str, rust_flags: &[&str]) -> Command {
.join("cpp/bin/riscv32-unknown-elf-gcc");
cmd.env("CC", cc_path)
.env("CFLAGS_riscv32im_risc0_zkvm_elf", "-march=rv32im -nostdlib");
// Signal to dependencies, cryptography patches in particular, that the bigint2 zkVM
// feature is available. Gated behind unstable to match risc0-zkvm-platform. Note that this
// would be seamless if there was a reliable way to tell whether it is enabled in
// risc0-zkvm-platform, however, this problem is also temporary.
#[cfg(feature = "unstable")]
cmd.env("RISC0_FEATURE_bigint2", "");
}

cmd.env("RUSTC", rustc)
Expand Down
11 changes: 4 additions & 7 deletions risc0/zkvm/platform/src/syscall.rs
Original file line number Diff line number Diff line change
Expand Up @@ -895,8 +895,7 @@ pub extern "C" fn sys_exit(status: i32) -> ! {
/// `control_id` must be aligned and dereferenceable.
///
/// `input` must be aligned and have `input_len` u32s dereferenceable
#[cfg(feature = "export-syscalls")]
#[no_mangle]
#[cfg_attr(all(feature = "export-syscalls", feature = "unstable"), no_mangle)]
#[stability::unstable]
pub unsafe extern "C" fn sys_prove_zkr(
claim_digest: *const [u32; DIGEST_WORDS],
Expand Down Expand Up @@ -931,8 +930,7 @@ pub unsafe extern "C" fn sys_prove_zkr(
/// invoked during `hasher.finalize(...)`
///
/// # Safety
#[cfg(feature = "export-syscalls")]
#[no_mangle]
#[cfg_attr(all(feature = "export-syscalls", feature = "unstable"), no_mangle)]
#[stability::unstable]
pub unsafe extern "C" fn sys_keccak(
input_ptr: *const u8,
Expand Down Expand Up @@ -961,8 +959,7 @@ pub unsafe extern "C" fn sys_keccak(
/// `control_root` must be aligned and dereferenceable.
///
/// `input` must be aligned and have `input_len` u32s dereferenceable
#[cfg(feature = "export-syscalls")]
#[no_mangle]
#[cfg_attr(all(feature = "export-syscalls", feature = "unstable"), no_mangle)]
#[stability::unstable]
pub unsafe extern "C" fn sys_prove_keccak(
po2: usize,
Expand Down Expand Up @@ -1017,7 +1014,7 @@ macro_rules! impl_sys_bigint2 {
/// # Safety
///
/// `blob_ptr` and all arguments must be aligned and dereferenceable.
#[cfg_attr(feature = "export-syscalls", no_mangle)]
#[cfg_attr(all(feature = "export-syscalls", feature = "unstable"), no_mangle)]
#[stability::unstable]
pub unsafe extern "C" fn $func_name(blob_ptr: *const u8, a1: *const u32
$(, $a2: *const u32
Expand Down

0 comments on commit 0dea13c

Please sign in to comment.