From 385a9e65e1dc9e60f2e0b1e5675248e716718241 Mon Sep 17 00:00:00 2001 From: nhtyy Date: Thu, 14 Nov 2024 14:38:09 -0800 Subject: [PATCH 01/10] feat: version compat checking in build --- crates/build/Cargo.toml | 3 ++ crates/build/src/build.rs | 78 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 79 insertions(+), 2 deletions(-) diff --git a/crates/build/Cargo.toml b/crates/build/Cargo.toml index b28b75d62..d63a239ad 100644 --- a/crates/build/Cargo.toml +++ b/crates/build/Cargo.toml @@ -15,3 +15,6 @@ anyhow = { version = "1.0.83" } clap = { version = "4.5.9", features = ["derive", "env"] } dirs = "5.0.1" chrono = { version = "0.4.38", default-features = false, features = ["clock"] } +serde = { version = "1.0.215", features = ["derive"] } +toml = "0.8.19" +semver = "1.0.23" diff --git a/crates/build/src/build.rs b/crates/build/src/build.rs index 37ee7392a..c2ad8b8c4 100644 --- a/crates/build/src/build.rs +++ b/crates/build/src/build.rs @@ -1,6 +1,9 @@ -use std::path::PathBuf; +use std::{ + io::{BufRead, BufReader}, + path::{Path, PathBuf}, +}; -use anyhow::Result; +use anyhow::{Context, Result}; use cargo_metadata::camino::Utf8PathBuf; use crate::{ @@ -57,6 +60,8 @@ pub fn execute_build_program( pub(crate) fn build_program_internal(path: &str, args: Option) { // Get the root package name and metadata. let program_dir = std::path::Path::new(path); + verify_locked_version(program_dir).expect("locked version mismatch"); + let metadata_file = program_dir.join("Cargo.toml"); let mut metadata_cmd = cargo_metadata::MetadataCommand::new(); let metadata = metadata_cmd.manifest_path(metadata_file).exec().unwrap(); @@ -180,3 +185,72 @@ fn print_elf_paths_cargo_directives(target_elf_paths: &[(String, Utf8PathBuf)]) println!("cargo:rustc-env=SP1_ELF_{}={}", target_name, elf_path); } } + +fn verify_locked_version(program_dir: impl AsRef) -> Result<()> { + #[derive(serde::Deserialize)] + struct LockFile { + package: Vec, + } + + #[derive(serde::Deserialize)] + struct Package { + name: String, + version: String, + } + + // This might be a workspace, so we need optinally search parent dirs for lock files + let canon = program_dir.as_ref().canonicalize()?; + let mut lock_path = canon.join("Cargo.lock"); + if !lock_path.is_file() { + let mut curr_path: &Path = canon.as_ref(); + + while let Some(parent) = curr_path.parent() { + let maybe_lock_path = parent.join("Cargo.lock"); + + if maybe_lock_path.is_file() { + lock_path = maybe_lock_path; + break; + } else { + curr_path = parent; + } + } + + if !lock_path.is_file() { + return Err(anyhow::anyhow!("Cargo.lock not found")); + } + } + + println!("cargo:warning=Found Cargo.lock at {}", lock_path.display()); + + // strip any comments for serialization and the rust compiler header + let reader = BufReader::new(std::fs::File::open(&lock_path)?).lines(); + let toml_string = reader + .skip(4) + .map(|line| line.context("Failed to readline from cargo lock file")) + .map(|line| line.map(|line| line + "\n")) + .collect::>()?; + + let locked = toml::from_str::(&toml_string)?; + + let vm_package = locked + .package + .iter() + .find(|p| p.name == "sp1-zkvm") + .ok_or_else(|| anyhow::anyhow!("sp1-zkvm not found in lock file!"))?; + + // print these just to be useful + let toolchain_version = env!("CARGO_PKG_VERSION"); + println!("cargo:warning=Locked version of sp1-zkvm is {}", vm_package.version); + println!("cargo:warning=Current toolchain version = {}", toolchain_version); + + let vm_version = semver::Version::parse(&vm_package.version)?; + let toolchain_version = semver::Version::parse(toolchain_version)?; + + if vm_version.major != toolchain_version.major || vm_version.minor != toolchain_version.minor { + return Err(anyhow::anyhow!( + "Locked version of sp1-zkvm is incompatible with the current toolchain version" + )); + } + + Ok(()) +} From 1cdb3b2d35f3211d0639b14cc87f40326efd6182 Mon Sep 17 00:00:00 2001 From: nhtyy Date: Thu, 14 Nov 2024 14:41:54 -0800 Subject: [PATCH 02/10] fix: spelling --- crates/build/src/build.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/build/src/build.rs b/crates/build/src/build.rs index c2ad8b8c4..5146feba0 100644 --- a/crates/build/src/build.rs +++ b/crates/build/src/build.rs @@ -198,7 +198,7 @@ fn verify_locked_version(program_dir: impl AsRef) -> Result<()> { version: String, } - // This might be a workspace, so we need optinally search parent dirs for lock files + // This might be a workspace, so we need optionally search parent dirs for lock files let canon = program_dir.as_ref().canonicalize()?; let mut lock_path = canon.join("Cargo.lock"); if !lock_path.is_file() { @@ -221,7 +221,7 @@ fn verify_locked_version(program_dir: impl AsRef) -> Result<()> { } println!("cargo:warning=Found Cargo.lock at {}", lock_path.display()); - + // strip any comments for serialization and the rust compiler header let reader = BufReader::new(std::fs::File::open(&lock_path)?).lines(); let toml_string = reader @@ -237,13 +237,13 @@ fn verify_locked_version(program_dir: impl AsRef) -> Result<()> { .iter() .find(|p| p.name == "sp1-zkvm") .ok_or_else(|| anyhow::anyhow!("sp1-zkvm not found in lock file!"))?; - + // print these just to be useful let toolchain_version = env!("CARGO_PKG_VERSION"); println!("cargo:warning=Locked version of sp1-zkvm is {}", vm_package.version); println!("cargo:warning=Current toolchain version = {}", toolchain_version); - let vm_version = semver::Version::parse(&vm_package.version)?; + let vm_version = semver::Version::parse(&vm_package.version)?; let toolchain_version = semver::Version::parse(toolchain_version)?; if vm_version.major != toolchain_version.major || vm_version.minor != toolchain_version.minor { From 418678b247a3fe052eed8658197f258056a8d93e Mon Sep 17 00:00:00 2001 From: nhtyy Date: Thu, 14 Nov 2024 17:59:31 -0800 Subject: [PATCH 03/10] fix: check sp1-spk --- crates/build/src/build.rs | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/crates/build/src/build.rs b/crates/build/src/build.rs index 5146feba0..f371e4f00 100644 --- a/crates/build/src/build.rs +++ b/crates/build/src/build.rs @@ -57,6 +57,9 @@ pub fn execute_build_program( } /// Internal helper function to build the program with or without arguments. +/// +/// Note: This function is not intended to be used by the CLI, as it looks for the sp1-sdk, +/// which is probably in the same crate lockfile as this function is only called by build scipr pub(crate) fn build_program_internal(path: &str, args: Option) { // Get the root package name and metadata. let program_dir = std::path::Path::new(path); @@ -186,6 +189,15 @@ fn print_elf_paths_cargo_directives(target_elf_paths: &[(String, Utf8PathBuf)]) } } +/// Verify that the locked version of `sp1-zkvm` in the Cargo.lock file is compatible with the +/// current version of this crate. +/// +/// This also checks to ensure that `sp1-sdk` is also the correct version. +/// +/// ## Note: This function assumes that version compatibility is given by matching major and minor +/// semver. +/// +/// This is also correct if future releases sharing the workspace version, which should be the case. fn verify_locked_version(program_dir: impl AsRef) -> Result<()> { #[derive(serde::Deserialize)] struct LockFile { @@ -238,17 +250,29 @@ fn verify_locked_version(program_dir: impl AsRef) -> Result<()> { .find(|p| p.name == "sp1-zkvm") .ok_or_else(|| anyhow::anyhow!("sp1-zkvm not found in lock file!"))?; + let sp1_sdk = locked + .package + .iter() + .find(|p| p.name == "sp1-sdk") + .ok_or_else(|| anyhow::anyhow!("sp1-sdk not found in lock file!"))?; + // print these just to be useful let toolchain_version = env!("CARGO_PKG_VERSION"); println!("cargo:warning=Locked version of sp1-zkvm is {}", vm_package.version); + println!("cargo:warning=Locked version of sp1-sdk is {}", sp1_sdk.version); println!("cargo:warning=Current toolchain version = {}", toolchain_version); let vm_version = semver::Version::parse(&vm_package.version)?; let toolchain_version = semver::Version::parse(toolchain_version)?; + let sp1_sdk_version = semver::Version::parse(&sp1_sdk.version)?; - if vm_version.major != toolchain_version.major || vm_version.minor != toolchain_version.minor { + if vm_version.major != toolchain_version.major + || vm_version.minor != toolchain_version.minor + || sp1_sdk_version.major != toolchain_version.major + || sp1_sdk_version.minor != toolchain_version.minor + { return Err(anyhow::anyhow!( - "Locked version of sp1-zkvm is incompatible with the current toolchain version" + "Locked version of sp1-zkvm or sp1-sdk is incompatible with the current toolchain version" )); } From 5018a27488727888ce7e3e9e67b9b704e048c60b Mon Sep 17 00:00:00 2001 From: nhtyy Date: Fri, 15 Nov 2024 16:37:44 -0800 Subject: [PATCH 04/10] fix: exception for test fixtures --- Cargo.lock | 29 +++++++++++++++++++++++++++++ crates/build/src/build.rs | 7 ++++++- crates/test-artifacts/Cargo.toml | 2 +- 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 21b1074ae..6b2cc1a56 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5267,6 +5267,15 @@ dependencies = [ "thiserror 1.0.69", ] +[[package]] +name = "serde_spanned" +version = "0.6.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "87607cb1398ed59d48732e575a4c28a7a8ebf2454b964fe3f224f2afc07909e1" +dependencies = [ + "serde", +] + [[package]] name = "serde_urlencoded" version = "0.7.1" @@ -5517,6 +5526,9 @@ dependencies = [ "chrono", "clap", "dirs", + "semver 1.0.23", + "serde", + "toml", ] [[package]] @@ -6624,11 +6636,26 @@ dependencies = [ "tokio", ] +[[package]] +name = "toml" +version = "0.8.19" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a1ed1f98e3fdc28d6d910e6737ae6ab1a93bf1985935a1193e68f93eeb68d24e" +dependencies = [ + "serde", + "serde_spanned", + "toml_datetime", + "toml_edit 0.22.22", +] + [[package]] name = "toml_datetime" version = "0.6.8" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0dd7358ecb8fc2f8d014bf86f6f638ce72ba252a2c3a2572f2a795f1d23efb41" +dependencies = [ + "serde", +] [[package]] name = "toml_edit" @@ -6648,6 +6675,8 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4ae48d6208a266e853d946088ed816055e556cc6028c5e8e2b84d9fa5dd7c7f5" dependencies = [ "indexmap 2.6.0", + "serde", + "serde_spanned", "toml_datetime", "winnow 0.6.20", ] diff --git a/crates/build/src/build.rs b/crates/build/src/build.rs index f371e4f00..289a8f154 100644 --- a/crates/build/src/build.rs +++ b/crates/build/src/build.rs @@ -59,7 +59,7 @@ pub fn execute_build_program( /// Internal helper function to build the program with or without arguments. /// /// Note: This function is not intended to be used by the CLI, as it looks for the sp1-sdk, -/// which is probably in the same crate lockfile as this function is only called by build scipr +/// which is probably in the same crate lockfile as this function is only called by build script. pub(crate) fn build_program_internal(path: &str, args: Option) { // Get the root package name and metadata. let program_dir = std::path::Path::new(path); @@ -210,6 +210,11 @@ fn verify_locked_version(program_dir: impl AsRef) -> Result<()> { version: String, } + // we need an exception for our test fixtures + if env!("CARGO_PKG_NAME") != "test-artifacts" { + return Ok(()); + } + // This might be a workspace, so we need optionally search parent dirs for lock files let canon = program_dir.as_ref().canonicalize()?; let mut lock_path = canon.join("Cargo.lock"); diff --git a/crates/test-artifacts/Cargo.toml b/crates/test-artifacts/Cargo.toml index 6954ca7c0..14b9571aa 100644 --- a/crates/test-artifacts/Cargo.toml +++ b/crates/test-artifacts/Cargo.toml @@ -11,4 +11,4 @@ categories.workspace = true sp1-build = { workspace = true } [build-dependencies] -sp1-build = { workspace = true } \ No newline at end of file +sp1-build = { workspace = true } From 872c431809930135869d8104bacb7d727f8b6ae9 Mon Sep 17 00:00:00 2001 From: nhtyy Date: Wed, 20 Nov 2024 15:44:39 -0800 Subject: [PATCH 05/10] fix: use cargo-lock package, imporve err msg --- Cargo.lock | 14 +++++++++- crates/build/Cargo.toml | 4 +-- crates/build/src/build.rs | 57 +++++++++++++-------------------------- 3 files changed, 34 insertions(+), 41 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6b2cc1a56..6d03ed470 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1288,6 +1288,18 @@ dependencies = [ "serde", ] +[[package]] +name = "cargo-lock" +version = "10.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6469776d007022d505bbcc2be726f5f096174ae76d710ebc609eb3029a45b551" +dependencies = [ + "semver 1.0.23", + "serde", + "toml", + "url", +] + [[package]] name = "cargo-platform" version = "0.1.8" @@ -5522,13 +5534,13 @@ name = "sp1-build" version = "3.0.0" dependencies = [ "anyhow", + "cargo-lock", "cargo_metadata", "chrono", "clap", "dirs", "semver 1.0.23", "serde", - "toml", ] [[package]] diff --git a/crates/build/Cargo.toml b/crates/build/Cargo.toml index d63a239ad..d81f3e36d 100644 --- a/crates/build/Cargo.toml +++ b/crates/build/Cargo.toml @@ -15,6 +15,6 @@ anyhow = { version = "1.0.83" } clap = { version = "4.5.9", features = ["derive", "env"] } dirs = "5.0.1" chrono = { version = "0.4.38", default-features = false, features = ["clock"] } -serde = { version = "1.0.215", features = ["derive"] } -toml = "0.8.19" +serde = { workspace = true, features = ["derive"] } semver = "1.0.23" +cargo-lock = "10.0.1" diff --git a/crates/build/src/build.rs b/crates/build/src/build.rs index 289a8f154..147903643 100644 --- a/crates/build/src/build.rs +++ b/crates/build/src/build.rs @@ -1,9 +1,5 @@ -use std::{ - io::{BufRead, BufReader}, - path::{Path, PathBuf}, -}; - -use anyhow::{Context, Result}; +use std::path::{Path, PathBuf}; +use anyhow::Result; use cargo_metadata::camino::Utf8PathBuf; use crate::{ @@ -199,17 +195,6 @@ fn print_elf_paths_cargo_directives(target_elf_paths: &[(String, Utf8PathBuf)]) /// /// This is also correct if future releases sharing the workspace version, which should be the case. fn verify_locked_version(program_dir: impl AsRef) -> Result<()> { - #[derive(serde::Deserialize)] - struct LockFile { - package: Vec, - } - - #[derive(serde::Deserialize)] - struct Package { - name: String, - version: String, - } - // we need an exception for our test fixtures if env!("CARGO_PKG_NAME") != "test-artifacts" { return Ok(()); @@ -239,26 +224,18 @@ fn verify_locked_version(program_dir: impl AsRef) -> Result<()> { println!("cargo:warning=Found Cargo.lock at {}", lock_path.display()); - // strip any comments for serialization and the rust compiler header - let reader = BufReader::new(std::fs::File::open(&lock_path)?).lines(); - let toml_string = reader - .skip(4) - .map(|line| line.context("Failed to readline from cargo lock file")) - .map(|line| line.map(|line| line + "\n")) - .collect::>()?; + let lock_file = cargo_lock::Lockfile::load(lock_path)?; - let locked = toml::from_str::(&toml_string)?; - - let vm_package = locked - .package + let vm_package = lock_file + .packages .iter() - .find(|p| p.name == "sp1-zkvm") + .find(|p| p.name.as_str() == "sp1-zkvm") .ok_or_else(|| anyhow::anyhow!("sp1-zkvm not found in lock file!"))?; - let sp1_sdk = locked - .package + let sp1_sdk = lock_file + .packages .iter() - .find(|p| p.name == "sp1-sdk") + .find(|p| p.name.as_str() == "sp1-sdk") .ok_or_else(|| anyhow::anyhow!("sp1-sdk not found in lock file!"))?; // print these just to be useful @@ -267,17 +244,21 @@ fn verify_locked_version(program_dir: impl AsRef) -> Result<()> { println!("cargo:warning=Locked version of sp1-sdk is {}", sp1_sdk.version); println!("cargo:warning=Current toolchain version = {}", toolchain_version); - let vm_version = semver::Version::parse(&vm_package.version)?; let toolchain_version = semver::Version::parse(toolchain_version)?; - let sp1_sdk_version = semver::Version::parse(&sp1_sdk.version)?; + let vm_version = &vm_package.version; + let sp1_sdk_version = &sp1_sdk.version; + + if vm_version.major != toolchain_version.major || vm_version.minor != toolchain_version.minor { + return Err(anyhow::anyhow!( + "Locked version of sp1-zkvm is incompatible with the current toolchain version" + )); + } - if vm_version.major != toolchain_version.major - || vm_version.minor != toolchain_version.minor - || sp1_sdk_version.major != toolchain_version.major + if sp1_sdk_version.major != toolchain_version.major || sp1_sdk_version.minor != toolchain_version.minor { return Err(anyhow::anyhow!( - "Locked version of sp1-zkvm or sp1-sdk is incompatible with the current toolchain version" + "Locked version of sp1-sdk is incompatible with the current toolchain version" )); } From c062eef813910620af4abc6c6380fb74db278d44 Mon Sep 17 00:00:00 2001 From: nhtyy Date: Wed, 20 Nov 2024 15:47:21 -0800 Subject: [PATCH 06/10] fix: grammar --- crates/build/src/build.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/build/src/build.rs b/crates/build/src/build.rs index 147903643..c61e0d907 100644 --- a/crates/build/src/build.rs +++ b/crates/build/src/build.rs @@ -195,7 +195,7 @@ fn print_elf_paths_cargo_directives(target_elf_paths: &[(String, Utf8PathBuf)]) /// /// This is also correct if future releases sharing the workspace version, which should be the case. fn verify_locked_version(program_dir: impl AsRef) -> Result<()> { - // we need an exception for our test fixtures + // We need an exception for our test fixtures. if env!("CARGO_PKG_NAME") != "test-artifacts" { return Ok(()); } @@ -238,7 +238,7 @@ fn verify_locked_version(program_dir: impl AsRef) -> Result<()> { .find(|p| p.name.as_str() == "sp1-sdk") .ok_or_else(|| anyhow::anyhow!("sp1-sdk not found in lock file!"))?; - // print these just to be useful + // Print these just to be useful. let toolchain_version = env!("CARGO_PKG_VERSION"); println!("cargo:warning=Locked version of sp1-zkvm is {}", vm_package.version); println!("cargo:warning=Locked version of sp1-sdk is {}", sp1_sdk.version); From b636df5a36a06262dafeee2d6a276172334c635a Mon Sep 17 00:00:00 2001 From: nhtyy Date: Fri, 22 Nov 2024 10:39:58 -0800 Subject: [PATCH 07/10] fix: fmt --- crates/build/src/build.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/build/src/build.rs b/crates/build/src/build.rs index c61e0d907..96ff22ef0 100644 --- a/crates/build/src/build.rs +++ b/crates/build/src/build.rs @@ -1,6 +1,6 @@ -use std::path::{Path, PathBuf}; use anyhow::Result; use cargo_metadata::camino::Utf8PathBuf; +use std::path::{Path, PathBuf}; use crate::{ command::{docker::create_docker_command, local::create_local_command, utils::execute_command}, From eaa6ffa700fcf7f186492f86a0c31108e0d07108 Mon Sep 17 00:00:00 2001 From: N Date: Tue, 26 Nov 2024 00:46:50 -0800 Subject: [PATCH 08/10] fix: check install cargo prove not build crate --- crates/build/src/build.rs | 61 +++++++++++++++++++++++++-------------- crates/cli/src/lib.rs | 12 ++++++-- 2 files changed, 49 insertions(+), 24 deletions(-) diff --git a/crates/build/src/build.rs b/crates/build/src/build.rs index 96ff22ef0..60d024aa2 100644 --- a/crates/build/src/build.rs +++ b/crates/build/src/build.rs @@ -1,6 +1,9 @@ use anyhow::Result; use cargo_metadata::camino::Utf8PathBuf; -use std::path::{Path, PathBuf}; +use std::{ + os::unix::process::CommandExt, + path::{Path, PathBuf}, +}; use crate::{ command::{docker::create_docker_command, local::create_local_command, utils::execute_command}, @@ -200,7 +203,7 @@ fn verify_locked_version(program_dir: impl AsRef) -> Result<()> { return Ok(()); } - // This might be a workspace, so we need optionally search parent dirs for lock files + // This might be a workspace, need optionally search parent dirs for lock files let canon = program_dir.as_ref().canonicalize()?; let mut lock_path = canon.join("Cargo.lock"); if !lock_path.is_file() { @@ -226,27 +229,39 @@ fn verify_locked_version(program_dir: impl AsRef) -> Result<()> { let lock_file = cargo_lock::Lockfile::load(lock_path)?; - let vm_package = lock_file + // You need an entrypoint, therefore you need sp1-zkvm. + let vm_version = &lock_file .packages .iter() .find(|p| p.name.as_str() == "sp1-zkvm") - .ok_or_else(|| anyhow::anyhow!("sp1-zkvm not found in lock file!"))?; + .ok_or_else(|| anyhow::anyhow!("sp1-zkvm not found in lock file!"))? + .version; - let sp1_sdk = lock_file - .packages - .iter() - .find(|p| p.name.as_str() == "sp1-sdk") - .ok_or_else(|| anyhow::anyhow!("sp1-sdk not found in lock file!"))?; + let maybe_sdk = + lock_file.packages.iter().find(|p| p.name.as_str() == "sp1-sdk").map(|p| &p.version); // Print these just to be useful. - let toolchain_version = env!("CARGO_PKG_VERSION"); - println!("cargo:warning=Locked version of sp1-zkvm is {}", vm_package.version); - println!("cargo:warning=Locked version of sp1-sdk is {}", sp1_sdk.version); - println!("cargo:warning=Current toolchain version = {}", toolchain_version); + // Most people will install the actual rust toolchain along with thier cargo-prove, + // so we can just use the cargo-prove version. + let toolchain_version = { + let output = std::process::Command::new("cargo") + .args(["prove", "--version"]).output().expect("Failed to find check toolchain version, see https://docs.succinct.xyz/getting-started/install.html"); + + let version = String::from_utf8(output.stdout).expect("Failed to parse version string"); + + semver::Version::parse( + version.split_once('\n').expect("The version should have whitespace, this is a bug").1, + )? + }; - let toolchain_version = semver::Version::parse(toolchain_version)?; - let vm_version = &vm_package.version; - let sp1_sdk_version = &sp1_sdk.version; + // Print these just to be useful. + if let Some(ref sdk) = maybe_sdk { + println!("cargo:warning=Locked version of sp1-sdk is {}", sdk); + } else { + println!("cargo:warning=sp1-sdk not found in lock file!"); + } + println!("cargo:warning=Locked version of sp1-zkvm is {}", vm_version); + println!("cargo:warning=Current toolchain version = {}", toolchain_version); if vm_version.major != toolchain_version.major || vm_version.minor != toolchain_version.minor { return Err(anyhow::anyhow!( @@ -254,12 +269,14 @@ fn verify_locked_version(program_dir: impl AsRef) -> Result<()> { )); } - if sp1_sdk_version.major != toolchain_version.major - || sp1_sdk_version.minor != toolchain_version.minor - { - return Err(anyhow::anyhow!( - "Locked version of sp1-sdk is incompatible with the current toolchain version" - )); + if let Some(sdk_version) = maybe_sdk { + if sdk_version.major != toolchain_version.major + || sdk_version.minor != toolchain_version.minor + { + return Err(anyhow::anyhow!( + "Locked version of sp1-sdk is incompatible with the current toolchain version" + )); + } } Ok(()) diff --git a/crates/cli/src/lib.rs b/crates/cli/src/lib.rs index 9e717aada..32817a1e9 100644 --- a/crates/cli/src/lib.rs +++ b/crates/cli/src/lib.rs @@ -6,8 +6,16 @@ use std::process::{Command, Stdio}; pub const RUSTUP_TOOLCHAIN_NAME: &str = "succinct"; -pub const SP1_VERSION_MESSAGE: &str = - concat!("sp1", " (", env!("VERGEN_GIT_SHA"), " ", env!("VERGEN_BUILD_TIMESTAMP"), ")"); +pub const SP1_VERSION_MESSAGE: &str = concat!( + "sp1", + " (", + env!("VERGEN_GIT_SHA"), + " ", + env!("VERGEN_BUILD_TIMESTAMP"), + ")", + "\n", + env!("CARGO_PKG_VERSION") +); trait CommandExecutor { fn run(&mut self) -> Result<()>; From 0f29948b47be51873560605df9ecf7e2a2cac1ee Mon Sep 17 00:00:00 2001 From: nhtyy Date: Tue, 26 Nov 2024 10:26:42 -0800 Subject: [PATCH 09/10] fix: dev env --- crates/build/src/build.rs | 67 ++++++++++++++++++++++++++------------- crates/build/src/lib.rs | 3 ++ 2 files changed, 48 insertions(+), 22 deletions(-) diff --git a/crates/build/src/build.rs b/crates/build/src/build.rs index 60d024aa2..1cf0beac4 100644 --- a/crates/build/src/build.rs +++ b/crates/build/src/build.rs @@ -1,9 +1,6 @@ use anyhow::Result; use cargo_metadata::camino::Utf8PathBuf; -use std::{ - os::unix::process::CommandExt, - path::{Path, PathBuf}, -}; +use std::path::{Path, PathBuf}; use crate::{ command::{docker::create_docker_command, local::create_local_command, utils::execute_command}, @@ -34,6 +31,8 @@ pub fn execute_build_program( let program_dir: Utf8PathBuf = program_dir.try_into().expect("Failed to convert PathBuf to Utf8PathBuf"); + verify_locked_version(&program_dir, args.verbose).expect("Version check mismatch"); + // Get the program metadata. let program_metadata_file = program_dir.join("Cargo.toml"); let mut program_metadata_cmd = cargo_metadata::MetadataCommand::new(); @@ -62,7 +61,8 @@ pub fn execute_build_program( pub(crate) fn build_program_internal(path: &str, args: Option) { // Get the root package name and metadata. let program_dir = std::path::Path::new(path); - verify_locked_version(program_dir).expect("locked version mismatch"); + verify_locked_version(program_dir, args.as_ref().map(|args| args.verbose).unwrap_or(false)) + .expect("Locked version mismatch"); let metadata_file = program_dir.join("Cargo.toml"); let mut metadata_cmd = cargo_metadata::MetadataCommand::new(); @@ -197,14 +197,31 @@ fn print_elf_paths_cargo_directives(target_elf_paths: &[(String, Utf8PathBuf)]) /// semver. /// /// This is also correct if future releases sharing the workspace version, which should be the case. -fn verify_locked_version(program_dir: impl AsRef) -> Result<()> { +/// +/// # Errors: +/// - If the locked version of `sp1-zkvm` is incompatible with the current toolchain version. +/// - If the locked version of `sp1-sdk` is incompatible with the current toolchain version. +/// - +/// +/// # Panics +/// - If we cant IO +/// - If we cant find your lockfile +/// - If we cant parse the version from the `cargo-prove` +fn verify_locked_version(program_dir: impl AsRef, verbose: bool) -> Result<()> { + if std::env::var("DEV_SP1_SKIP_VERSION_CHECK").is_ok() { + println!("cargo:warning=Skipping version check"); + return Ok(()); + } + // We need an exception for our test fixtures. - if env!("CARGO_PKG_NAME") != "test-artifacts" { + if std::env::var("CARGO_PKG_NAME").unwrap_or_default() == "test-artifacts" { return Ok(()); } - // This might be a workspace, need optionally search parent dirs for lock files - let canon = program_dir.as_ref().canonicalize()?; + // This might be a workspace, need to optionally search parent dirs for lock files + let canon = + program_dir.as_ref().canonicalize().expect("Failed to canonicalize path to program dir"); + let mut lock_path = canon.join("Cargo.lock"); if !lock_path.is_file() { let mut curr_path: &Path = canon.as_ref(); @@ -221,20 +238,20 @@ fn verify_locked_version(program_dir: impl AsRef) -> Result<()> { } if !lock_path.is_file() { - return Err(anyhow::anyhow!("Cargo.lock not found")); + panic!("Failed to find Cargo.lock in the program directory or its parent directories"); } } println!("cargo:warning=Found Cargo.lock at {}", lock_path.display()); - let lock_file = cargo_lock::Lockfile::load(lock_path)?; + let lock_file = cargo_lock::Lockfile::load(lock_path).expect("Failed to load Cargo.lock"); // You need an entrypoint, therefore you need sp1-zkvm. let vm_version = &lock_file .packages .iter() .find(|p| p.name.as_str() == "sp1-zkvm") - .ok_or_else(|| anyhow::anyhow!("sp1-zkvm not found in lock file!"))? + .expect("The guest program should have a sp1-zkvm dependency") .version; let maybe_sdk = @@ -245,23 +262,29 @@ fn verify_locked_version(program_dir: impl AsRef) -> Result<()> { // so we can just use the cargo-prove version. let toolchain_version = { let output = std::process::Command::new("cargo") - .args(["prove", "--version"]).output().expect("Failed to find check toolchain version, see https://docs.succinct.xyz/getting-started/install.html"); + .args(["prove", "--version"]) + .output() + .expect("Failed to find check toolchain version, see https://docs.succinct.xyz/getting-started/install.html"); let version = String::from_utf8(output.stdout).expect("Failed to parse version string"); semver::Version::parse( - version.split_once('\n').expect("The version should have whitespace, this is a bug").1, - )? + version + .split_once('\n') + .expect("The version should have whitespace, this is a bug") + .1 + .trim(), + ) + .expect("Failed to parse version string, this is a bug") }; - // Print these just to be useful. - if let Some(ref sdk) = maybe_sdk { - println!("cargo:warning=Locked version of sp1-sdk is {}", sdk); - } else { - println!("cargo:warning=sp1-sdk not found in lock file!"); + if verbose { + maybe_sdk.inspect(|v| { + println!("cargo:warning=Locked version of sp1-sdk is {}", v); + }); + println!("cargo:warning=Locked version of sp1-zkvm is {}", vm_version); + println!("cargo:warning=Current toolchain version = {}", toolchain_version); } - println!("cargo:warning=Locked version of sp1-zkvm is {}", vm_version); - println!("cargo:warning=Current toolchain version = {}", toolchain_version); if vm_version.major != toolchain_version.major || vm_version.minor != toolchain_version.minor { return Err(anyhow::anyhow!( diff --git a/crates/build/src/lib.rs b/crates/build/src/lib.rs index 1dbbec134..b9e448aca 100644 --- a/crates/build/src/lib.rs +++ b/crates/build/src/lib.rs @@ -76,6 +76,8 @@ pub struct BuildArgs { default_value = DEFAULT_OUTPUT_DIR )] pub output_directory: String, + #[clap(long, action, help = "Any debug logs from the build script will be printed to stdout")] + pub verbose: bool, } // Implement default args to match clap defaults. @@ -93,6 +95,7 @@ impl Default for BuildArgs { output_directory: DEFAULT_OUTPUT_DIR.to_string(), locked: false, no_default_features: false, + verbose: false, } } } From 8f5a6362d3ddf8a5a4535e4e6caa722f37431de1 Mon Sep 17 00:00:00 2001 From: nhtyy Date: Tue, 26 Nov 2024 10:50:25 -0800 Subject: [PATCH 10/10] fix: workflow should install cargo prove --- .github/workflows/pr.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 391d00322..241f4b0be 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -168,6 +168,8 @@ jobs: - name: Install SP1 toolchain run: | cargo run -p sp1-cli -- prove install-toolchain + cd crates/cli + cargo install --locked --path . - name: Run cargo fmt run: |