From 914f4783bb271b0b8f292bb8d3f95802900f79da Mon Sep 17 00:00:00 2001 From: Yogesh Shahi Date: Mon, 19 Oct 2026 17:43:53 +0530 Subject: [PATCH 1/8] Rename R1CSArtifact to Artifact --- compiler/src/bin/main.rs | 4 +-- compiler/src/driver.rs | 9 ++---- mavros-artifacts/src/lib.rs | 6 ++-- mavros-artifacts/src/manifest.rs | 47 ++++++++++++++++---------------- 4 files changed, 31 insertions(+), 35 deletions(-) diff --git a/compiler/src/bin/main.rs b/compiler/src/bin/main.rs index 81c6bff1..e80d40b6 100644 --- a/compiler/src/bin/main.rs +++ b/compiler/src/bin/main.rs @@ -1154,7 +1154,7 @@ mod tests { /// Covers LogUp slots, unused private inputs and partially read compound parameters. #[test] fn compile_writes_versioned_witness_manifest() { - use mavros_artifacts::{ParamVisibility, R1CSArtifact}; + use mavros_artifacts::{Artifact, ParamVisibility}; let dir = tempfile::tempdir().unwrap(); let root = dir.path().to_path_buf(); fs::create_dir_all(root.join("src")).unwrap(); @@ -1178,7 +1178,7 @@ mod tests { ExitCode::SUCCESS ); let bytes = fs::read(&r1cs).unwrap(); - let artifact: R1CSArtifact = R1CSArtifact::from_bytes(&bytes).unwrap(); + let artifact: Artifact = Artifact::from_bytes(&bytes).unwrap(); let manifest = artifact.manifest(); assert_eq!(manifest.entry_blob_field_count(), 8); let columns: Vec<_> = manifest diff --git a/compiler/src/driver.rs b/compiler/src/driver.rs index 471fbbf5..f214201c 100644 --- a/compiler/src/driver.rs +++ b/compiler/src/driver.rs @@ -17,7 +17,7 @@ use noirc_frontend::{ use tracing::info; use mavros_artifacts::{ - FieldConfig, FieldId, ManifestParam, MavrosField, ParamVisibility, R1CSArtifact, ReturnSlot, + Artifact, FieldConfig, FieldId, ManifestParam, MavrosField, ParamVisibility, ReturnSlot, WitnessManifest, }; @@ -1040,12 +1040,9 @@ impl Driver { Ok(manifest) } - pub fn r1cs_artifact( - &self, - r1cs: SealedR1CS, - ) -> Result, Error> { + pub fn r1cs_artifact(&self, r1cs: SealedR1CS) -> Result, Error> { let manifest = self.witness_manifest(&r1cs)?; - Ok(R1CSArtifact::new(manifest, r1cs)) + Ok(Artifact::new(manifest, r1cs)) } /// The parameters only; the return is computed, not an input. diff --git a/mavros-artifacts/src/lib.rs b/mavros-artifacts/src/lib.rs index ca63ac6a..c6eec3c8 100644 --- a/mavros-artifacts/src/lib.rs +++ b/mavros-artifacts/src/lib.rs @@ -9,8 +9,8 @@ use tracing::error; pub use field::{FieldConfig, FieldElement, FieldId, Goldilocks, IntoFieldElement, MavrosField}; pub use manifest::{ - ArtifactError, ManifestParam, ParamVisibility, R1CS_ARTIFACT_DIGEST_DOMAIN, - R1CS_ARTIFACT_FORMAT_VERSION, R1CS_ARTIFACT_MAGIC, R1CSArtifact, ReturnSlot, WitnessManifest, + ARTIFACT_DIGEST_DOMAIN, ARTIFACT_FORMAT_VERSION, ARTIFACT_MAGIC, Artifact, ArtifactError, + ManifestParam, ParamVisibility, ReturnSlot, WitnessManifest, }; // FIELD-ASSUMPTION: L1-alias @@ -638,7 +638,7 @@ impl SealedR1CS { F::ID } - /// Use [`R1CSArtifact::digest`] to also bind the witness manifest. + /// Use [`Artifact::digest`] to also bind the witness manifest. pub fn digest(&self) -> [u8; 32] { *self.digest.get_or_init(|| self.system.digest()) } diff --git a/mavros-artifacts/src/manifest.rs b/mavros-artifacts/src/manifest.rs index a10d70f3..f44954e3 100644 --- a/mavros-artifacts/src/manifest.rs +++ b/mavros-artifacts/src/manifest.rs @@ -6,7 +6,7 @@ //! ``` //! //! The digest binds the field, R1CS and witness manifest; reading verifies it and refuses bytes -//! after the body. Bump [`R1CS_ARTIFACT_FORMAT_VERSION`] when changing the header, the serialized +//! after the body. Bump [`ARTIFACT_FORMAT_VERSION`] when changing the header, the serialized //! manifest, the R1CS or the layouts. //! //! The manifest records only what the R1CS cannot say: each parameter's name, visibility, width @@ -146,14 +146,14 @@ impl WitnessManifest { } } -pub const R1CS_ARTIFACT_MAGIC: [u8; 8] = *b"MAVR1CS\0"; +pub const ARTIFACT_MAGIC: [u8; 8] = *b"MAVR1CS\0"; /// Format version this crate writes and the only one it reads. -pub const R1CS_ARTIFACT_FORMAT_VERSION: u32 = 1; +pub const ARTIFACT_FORMAT_VERSION: u32 = 1; -/// Domain separator of [`R1CSArtifact::digest`]. Bump when its preimage encoding changes. -pub const R1CS_ARTIFACT_DIGEST_DOMAIN: &[u8] = b"mavros-r1cs-artifact-v1\0"; +/// Domain separator of [`Artifact::digest`]. Bump when its preimage encoding changes. +pub const ARTIFACT_DIGEST_DOMAIN: &[u8] = b"mavros-r1cs-artifact-v1\0"; -const VERSION_OFFSET: usize = R1CS_ARTIFACT_MAGIC.len(); +const VERSION_OFFSET: usize = ARTIFACT_MAGIC.len(); const FIELD_OFFSET: usize = VERSION_OFFSET + 4; const DIGEST_OFFSET: usize = FIELD_OFFSET + 4; const HEADER_LEN: usize = DIGEST_OFFSET + 32; @@ -162,7 +162,7 @@ const HEADER_LEN: usize = DIGEST_OFFSET + 32; pub enum ArtifactError { #[error("r1cs.bin does not start with the MAVR1CS header, so it is not a Mavros R1CS artifact")] MissingHeader, - #[error("unsupported r1cs.bin format version {0}; expected {R1CS_ARTIFACT_FORMAT_VERSION}")] + #[error("unsupported r1cs.bin format version {0}; expected {ARTIFACT_FORMAT_VERSION}")] UnsupportedVersion(u32), #[error("r1cs.bin names unknown field {0}")] UnknownField(u32), @@ -203,16 +203,16 @@ struct BodyRef<'a, F: MavrosField> { } #[derive(Debug)] -pub struct R1CSArtifact { +pub struct Artifact { manifest: WitnessManifest, r1cs: SealedR1CS, digest: [u8; 32], } -impl R1CSArtifact { +impl Artifact { pub fn new(manifest: WitnessManifest, r1cs: SealedR1CS) -> Self { let mut hasher = Sha256::new(); - hasher.update(R1CS_ARTIFACT_DIGEST_DOMAIN); + hasher.update(ARTIFACT_DIGEST_DOMAIN); hasher.update(r1cs.digest()); bincode::serialize_into(&mut hasher, &manifest) .expect("hashing a witness manifest cannot fail: the hasher accepts every byte"); @@ -227,7 +227,7 @@ impl R1CSArtifact { self.r1cs.field() } - /// SHA-256 over [`R1CS_ARTIFACT_DIGEST_DOMAIN`], the field-bound [`R1CS::digest`] and the + /// SHA-256 over [`ARTIFACT_DIGEST_DOMAIN`], the field-bound [`R1CS::digest`] and the /// bincode-encoded manifest. Pin this digest to identify both the system and its public /// input/output mapping; [`SealedR1CS::digest`] identifies only the constraint system. pub fn digest(&self) -> [u8; 32] { @@ -248,8 +248,8 @@ impl R1CSArtifact { pub fn to_bytes(&self) -> Result, ArtifactError> { let mut bytes = Vec::new(); - bytes.extend_from_slice(&R1CS_ARTIFACT_MAGIC); - bytes.extend_from_slice(&R1CS_ARTIFACT_FORMAT_VERSION.to_le_bytes()); + bytes.extend_from_slice(&ARTIFACT_MAGIC); + bytes.extend_from_slice(&ARTIFACT_FORMAT_VERSION.to_le_bytes()); bytes.extend_from_slice(&field_tag(self.field()).to_le_bytes()); bytes.extend_from_slice(&self.digest()); debug_assert_eq!(bytes.len(), HEADER_LEN); @@ -266,13 +266,13 @@ impl R1CSArtifact { /// Reads an artifact, verifying its digest and the manifest against the R1CS. Trailing bytes are /// an error. pub fn from_bytes(bytes: &[u8]) -> Result { - if bytes.len() < HEADER_LEN || bytes[..R1CS_ARTIFACT_MAGIC.len()] != R1CS_ARTIFACT_MAGIC { + if bytes.len() < HEADER_LEN || bytes[..ARTIFACT_MAGIC.len()] != ARTIFACT_MAGIC { return Err(ArtifactError::MissingHeader); } let u32_at = |start: usize| u32::from_le_bytes(bytes[start..start + 4].try_into().expect("4 bytes")); let version = u32_at(VERSION_OFFSET); - if version != R1CS_ARTIFACT_FORMAT_VERSION { + if version != ARTIFACT_FORMAT_VERSION { return Err(ArtifactError::UnsupportedVersion(version)); } let field_code = u32_at(FIELD_OFFSET); @@ -358,16 +358,16 @@ mod tests { } fn bytes_of(manifest: WitnessManifest, r1cs: R1CS) -> Vec { - R1CSArtifact::new(manifest, SealedR1CS::new(r1cs)) + Artifact::new(manifest, SealedR1CS::new(r1cs)) .to_bytes() .unwrap() } - fn read(bytes: &[u8]) -> Result { - R1CSArtifact::from_bytes(bytes) + fn read(bytes: &[u8]) -> Result { + Artifact::from_bytes(bytes) } - fn round_trip(manifest: WitnessManifest, r1cs: R1CS) -> Result { + fn round_trip(manifest: WitnessManifest, r1cs: R1CS) -> Result { read(&bytes_of(manifest, r1cs)) } @@ -394,7 +394,7 @@ mod tests { let (mut m, mut r) = (manifest(), r1cs()); mutate(&mut m, &mut r); let bytes = bytes_of(m.clone(), r.clone()); - assert!(bytes.starts_with(&R1CS_ARTIFACT_MAGIC)); + assert!(bytes.starts_with(&ARTIFACT_MAGIC)); assert_eq!( bytes, bytes_of(m.clone(), r.clone()), @@ -422,15 +422,14 @@ mod tests { read(&bytes[..HEADER_LEN - 1]), Err(ArtifactError::MissingHeader) )); - for version in [0, R1CS_ARTIFACT_FORMAT_VERSION + 1] { + for version in [0, ARTIFACT_FORMAT_VERSION + 1] { bytes[VERSION_OFFSET..FIELD_OFFSET].copy_from_slice(&version.to_le_bytes()); assert!(matches!( read(&bytes), Err(ArtifactError::UnsupportedVersion(found)) if found == version )); } - bytes[VERSION_OFFSET..FIELD_OFFSET] - .copy_from_slice(&R1CS_ARTIFACT_FORMAT_VERSION.to_le_bytes()); + bytes[VERSION_OFFSET..FIELD_OFFSET].copy_from_slice(&ARTIFACT_FORMAT_VERSION.to_le_bytes()); bytes[FIELD_OFFSET..DIGEST_OFFSET].copy_from_slice(&99u32.to_le_bytes()); assert!(matches!(read(&bytes), Err(ArtifactError::UnknownField(99)))); } @@ -490,7 +489,7 @@ mod tests { hex(&r1cs().digest()), "a1826c2cf819ba7805777209988b33f98071393e14c2671b9ddb3fe9fafe2c52" ); - let artifact = R1CSArtifact::new(manifest(), SealedR1CS::new(r1cs())); + let artifact = Artifact::new(manifest(), SealedR1CS::new(r1cs())); assert_eq!( hex(&artifact.digest()), "91938a47b4fbf034bad07ba83f8289ca1703176a63fae4867f05c0cd4505de18" From e897b1996bfa012365a844fa088ca6da7edfc38e Mon Sep 17 00:00:00 2001 From: Yogesh Shahi Date: Mon, 19 Oct 2026 17:49:37 +0530 Subject: [PATCH 2/8] Write one program.mavros carrying the bytecode, ABI and compiler version --- compiler/src/bin/main.rs | 217 ++++++++++++------------------- compiler/src/driver.rs | 17 ++- compiler/tests/verifier.rs | 2 +- mavros-artifacts/src/lib.rs | 2 +- mavros-artifacts/src/manifest.rs | 141 +++++++++++++++----- 5 files changed, 210 insertions(+), 169 deletions(-) diff --git a/compiler/src/bin/main.rs b/compiler/src/bin/main.rs index e80d40b6..130ba280 100644 --- a/compiler/src/bin/main.rs +++ b/compiler/src/bin/main.rs @@ -92,19 +92,15 @@ pub struct ProgramOptions { #[derive(Clone, Debug, Subcommand)] pub enum Command { - /// Compile a Noir project and output R1CS and binary artifacts. + /// Compile a Noir project into one Mavros artifact. Compile { /// Path to the Noir project root. #[arg(default_value = DEFAULT_NOIR_PROJECT_PATH, value_parser = parse_path)] path: PathBuf, - /// Output path for the versioned R1CS artifact. - #[arg(long, default_value = "target/r1cs.bin")] - r1cs_output: PathBuf, - - /// Output path for binaries and ABI (JSON). - #[arg(long, default_value = "target/basic.json")] - binary_output: PathBuf, + /// Output path for the artifact: header, witness manifest, R1CS, VM bytecode and ABI. + #[arg(long, default_value = "target/program.mavros")] + output: PathBuf, /// Optionally write a separate verifier binary and layouts (JSON). #[arg(long)] @@ -146,14 +142,12 @@ fn main() -> ExitCode { .into()), Some(Command::Compile { path, - r1cs_output, - binary_output, + output, verifier_output, draw_graphs, }) => run_compile( path, - r1cs_output, - binary_output, + output, verifier_output.as_ref(), *draw_graphs, args.include_debug_info, @@ -198,12 +192,10 @@ fn debug_path_root(driver: &Driver, absolute_paths: bool) -> Option<&Path> { } } -/// Compile phase: compile the Noir project and save R1CS and binary artifacts -/// to separate files. +/// Compile phase: compile the Noir project and write its artifact. pub fn run_compile( path: &PathBuf, - r1cs_output: &PathBuf, - binary_output: &PathBuf, + output: &PathBuf, verifier_output: Option<&PathBuf>, draw_graphs: bool, include_debug_info: bool, @@ -214,8 +206,7 @@ pub fn run_compile( let roots = Project::binary_package_roots(&requested_root)?; let multiple = roots.len() > 1; if multiple - && [r1cs_output, binary_output] - .into_iter() + && std::iter::once(output) .chain(verifier_output) .any(|output| { output.components().any(|component| { @@ -232,15 +223,10 @@ pub fn run_compile( } run_packages(&roots, |root| { let rebase = root != &requested_root; - let r1cs_output = if rebase { - root.join(r1cs_output) + let output = if rebase { + root.join(output) } else { - r1cs_output.clone() - }; - let binary_output = if rebase { - root.join(binary_output) - } else { - binary_output.clone() + output.clone() }; let verifier_output = verifier_output.map(|output| { if rebase { @@ -251,8 +237,7 @@ pub fn run_compile( }); run_compile_package( root, - &r1cs_output, - &binary_output, + &output, verifier_output.as_ref(), draw_graphs, include_debug_info, @@ -264,56 +249,37 @@ pub fn run_compile( fn run_compile_package( path: &PathBuf, - r1cs_output: &PathBuf, - binary_output: &PathBuf, + output: &PathBuf, verifier_output: Option<&PathBuf>, draw_graphs: bool, include_debug_info: bool, absolute_paths: bool, logup_soundness: u32, ) -> Result { - check_compile_outputs( - r1cs_output, - binary_output, - verifier_output.map(|path| path.as_path()), - )?; - info!(message = %"Compiling Noir project", root = ?path, r1cs_output = ?r1cs_output, binary_output = ?binary_output); + check_compile_outputs(output, verifier_output.map(|path| path.as_path()))?; + info!(message = %"Compiling Noir project", root = ?path, output = ?output); let (mut driver, r1cs) = compile_to_r1cs(path.clone(), draw_graphs, logup_soundness, false)?; - let artifact = api::compile_bytecode_artifact( + let bytecode = api::compile_bytecode_artifact( &mut driver, CodeGenOptions { include_debug_info, ..CodeGenOptions::default() }, )?; - let binary = artifact.binary; - // Ensure output directories exist - if let Some(parent) = r1cs_output.parent() { - if !parent.as_os_str().is_empty() { - fs::create_dir_all(parent)?; - } - } - if let Some(parent) = binary_output.parent() { + if let Some(parent) = output.parent() { if !parent.as_os_str().is_empty() { fs::create_dir_all(parent)?; } } - let r1cs_artifact = driver.r1cs_artifact(r1cs)?; - let r1cs_constraints = r1cs_artifact.r1cs().constraints.len(); - let r1cs_bytes = r1cs_artifact.to_bytes()?; - fs::write(r1cs_output, &r1cs_bytes)?; + let abi = serde_json::to_vec(driver.abi())?; + let artifact = driver.artifact(r1cs, bytecode.binary, abi)?; + let bytes = artifact.to_bytes()?; + fs::write(output, &bytes)?; - let basic = serde_json::json!({ - "binary": binary, - "abi": serde_json::to_value(driver.abi())?, - }); - let basic_json = serde_json::to_string_pretty(&basic)?; - fs::write(binary_output, basic_json)?; - - write_vm_debug_info(binary_output, artifact.debug_info, &driver, absolute_paths)?; + write_vm_debug_info(output, bytecode.debug_info, &driver, absolute_paths)?; if let Some(output) = verifier_output { let verifier = api::compile_verifier_bytecode_artifact( @@ -330,32 +296,27 @@ fn run_compile_package( output, serde_json::to_string_pretty(&serde_json::json!({ "binary": verifier.binary, - "witness_layout": r1cs_artifact.r1cs().witness_layout, - "constraints_layout": r1cs_artifact.r1cs().constraints_layout, + "witness_layout": artifact.r1cs().witness_layout, + "constraints_layout": artifact.r1cs().constraints_layout, }))?, )?; write_vm_debug_info(output, verifier.debug_info, &driver, absolute_paths)?; } info!( - message = %"Artifacts saved successfully", - r1cs_output = ?r1cs_output, - binary_output = ?binary_output, - r1cs_constraints, - r1cs_bytes = r1cs_bytes.len(), - binary_size = binary.len() * 8, + message = %"Artifact saved successfully", + output = ?output, + artifact_bytes = bytes.len(), + r1cs_constraints = artifact.r1cs().constraints.len(), + binary_size = artifact.bytecode().len() * 8, ); Ok(ExitCode::SUCCESS) } /// Reserve debug sidecars even without debug info: stale sidecars are removed on that path. -fn check_compile_outputs(r1cs: &Path, binary: &Path, verifier: Option<&Path>) -> Result<(), Error> { - let mut outputs = vec![ - r1cs.to_path_buf(), - binary.to_path_buf(), - binary.with_extension("debug.json"), - ]; +fn check_compile_outputs(output: &Path, verifier: Option<&Path>) -> Result<(), Error> { + let mut outputs = vec![output.to_path_buf(), output.with_extension("debug.json")]; if let Some(verifier) = verifier { outputs.extend([ verifier.to_path_buf(), @@ -859,24 +820,20 @@ mod tests { fn compile_output_collisions_are_rejected_before_writing() { let dir = tempfile::tempdir().unwrap(); let root = dir.path(); - let r1cs = root.join("r1cs.bin"); - for (binary, verifier) in [ - ("p.json", "p.bin"), - ("p.json", "p.debug.json"), + for (output, verifier) in [ + ("p.mavros", "p.mavros"), + ("p.mavros", "p.debug.json"), ("p.debug.json", "p.json"), - ("p.json", "r1cs.bin"), - ("p.json", "./p.json"), - ("p.json", "unused/../p.json"), + ("p.mavros", "./p.mavros"), + ("p.mavros", "unused/../p.mavros"), ] { - let binary = root.join(binary); + let output = root.join(output); let verifier = root.join(verifier); for include_debug_info in [false, true] { - fs::write(&binary, "keep prover").unwrap(); - fs::write(&r1cs, "keep r1cs").unwrap(); + fs::write(&output, "keep artifact").unwrap(); let error = run_compile_package( &root.to_path_buf(), - &r1cs, - &binary, + &output, Some(&verifier), false, include_debug_info, @@ -888,20 +845,18 @@ mod tests { error.to_string().contains("output paths overlap"), "{error}" ); - assert_eq!(fs::read_to_string(&binary).unwrap(), "keep prover"); - assert_eq!(fs::read_to_string(&r1cs).unwrap(), "keep r1cs"); + assert_eq!(fs::read_to_string(&output).unwrap(), "keep artifact"); } } - check_compile_outputs(&r1cs, &root.join("p.json"), Some(&root.join("v.json"))).unwrap(); + check_compile_outputs(&root.join("p.mavros"), Some(&root.join("v.json"))).unwrap(); #[cfg(unix)] { fs::create_dir_all(root.join("nested/sub")).unwrap(); std::os::unix::fs::symlink(root.join("nested/sub"), root.join("alias")).unwrap(); assert!( check_compile_outputs( - &r1cs, - &root.join("nested/p.json"), - Some(&root.join("alias/../p.bin")) + &root.join("nested/p.mavros"), + Some(&root.join("alias/../p.json")) ) .is_err() ); @@ -912,28 +867,26 @@ mod tests { fn compile_output_collisions_follow_filesystem_case_sensitivity() { let dir = tempfile::tempdir().unwrap(); let root = dir.path(); - let r1cs = root.join("r1cs.bin"); - fs::write(&r1cs, "keep r1cs").unwrap(); + let sentinel = root.join("sentinel"); + fs::write(&sentinel, "keep").unwrap(); fs::create_dir(root.join("nested")).unwrap(); - let case_insensitive = root.join("R1CS.BIN").exists(); - for (binary, verifier) in [ - ("p.json", "P.json"), - ("p.json", "P.bin"), - ("new/p.json", "NEW/P.bin"), - ("nested/p.json", "NESTED/P.bin"), - ("p.json", "R1CS.BIN"), - ("é.json", "É.bin"), + let case_insensitive = root.join("SENTINEL").exists(); + for (output, verifier) in [ + ("p.mavros", "P.mavros"), + ("p.mavros", "P.json"), + ("new/p.mavros", "NEW/P.json"), + ("nested/p.mavros", "NESTED/P.json"), + ("é.mavros", "É.json"), ] { - let result = - check_compile_outputs(&r1cs, &root.join(binary), Some(&root.join(verifier))); - assert_eq!(result.is_err(), case_insensitive, "{binary}, {verifier}"); + let result = check_compile_outputs(&root.join(output), Some(&root.join(verifier))); + assert_eq!(result.is_err(), case_insensitive, "{output}, {verifier}"); if let Err(error) = result { assert!( error.to_string().contains("output paths overlap"), "{error}" ); } - assert_eq!(fs::read_to_string(&r1cs).unwrap(), "keep r1cs"); + assert_eq!(fs::read_to_string(&sentinel).unwrap(), "keep"); // No output directories or temporary probes survive the check. assert_eq!(fs::read_dir(root).unwrap().count(), 2); assert_eq!(fs::read_dir(root.join("nested")).unwrap().count(), 0); @@ -1073,13 +1026,11 @@ mod tests { fn workspace_compile_keeps_member_artifacts_separate() { let dir = workspace_fixture(); let root = dir.path().to_path_buf(); - let r1cs = PathBuf::from("target/r1cs.bin"); - let binary = PathBuf::from("target/basic.json"); + let output = PathBuf::from("target/program.mavros"); assert!( run_compile( &root, - &root.join("shared.bin"), - &binary, + &root.join("shared.mavros"), None, false, true, @@ -1088,12 +1039,11 @@ mod tests { ) .is_err() ); - assert!(!root.join("shared.bin").exists()); + assert!(!root.join("shared.mavros").exists()); assert_eq!( run_compile( &root, - &r1cs, - &binary, + &output, Some(&PathBuf::from("target/verifier.json")), false, true, @@ -1105,9 +1055,8 @@ mod tests { ); for name in ["a", "b"] { for file in [ - "r1cs.bin", - "basic.json", - "basic.debug.json", + "program.mavros", + "program.debug.json", "verifier.json", "verifier.debug.json", ] { @@ -1115,8 +1064,8 @@ mod tests { } } assert_ne!( - fs::read(root.join("a/target/basic.json")).unwrap(), - fs::read(root.join("b/target/basic.json")).unwrap() + fs::read(root.join("a/target/program.mavros")).unwrap(), + fs::read(root.join("b/target/program.mavros")).unwrap() ); } @@ -1134,8 +1083,7 @@ mod tests { assert_eq!( run_compile( &dir.path().to_path_buf(), - &PathBuf::from("target/r1cs.bin"), - &PathBuf::from("target/basic.json"), + &PathBuf::from("target/program.mavros"), None, false, true, @@ -1145,7 +1093,7 @@ mod tests { .unwrap(), ExitCode::SUCCESS ); - for file in ["r1cs.bin", "basic.json", "basic.debug.json"] { + for file in ["program.mavros", "program.debug.json"] { assert!(dir.path().join(member).join("target").join(file).is_file()); } } @@ -1153,7 +1101,7 @@ mod tests { /// Covers LogUp slots, unused private inputs and partially read compound parameters. #[test] - fn compile_writes_versioned_witness_manifest() { + fn compile_writes_one_artifact() { use mavros_artifacts::{Artifact, ParamVisibility}; let dir = tempfile::tempdir().unwrap(); let root = dir.path().to_path_buf(); @@ -1171,16 +1119,23 @@ mod tests { }", ) .unwrap(); - let r1cs = root.join("target/r1cs.bin"); - let binary = root.join("target/basic.json"); + let output = root.join("target/program.mavros"); assert_eq!( - run_compile(&root, &r1cs, &binary, None, false, false, false, 128).unwrap(), + run_compile(&root, &output, None, false, false, false, 128).unwrap(), ExitCode::SUCCESS ); - let bytes = fs::read(&r1cs).unwrap(); + let bytes = fs::read(&output).unwrap(); let artifact: Artifact = Artifact::from_bytes(&bytes).unwrap(); let manifest = artifact.manifest(); assert_eq!(manifest.entry_blob_field_count(), 8); + assert_eq!( + mavros_vm::bytecode::parse_program_header(artifact.bytecode()).entry_blob_field_count, + 8 + ); + let abi: noirc_abi::Abi = serde_json::from_slice(artifact.abi()).unwrap(); + let names: Vec<_> = abi.parameters.iter().map(|p| p.name.as_str()).collect(); + assert_eq!(names, ["unused", "unused_u8", "x", "y"]); + assert_eq!(artifact.compiler(), env!("CARGO_PKG_VERSION")); let columns: Vec<_> = manifest .parameters .iter() @@ -1211,9 +1166,9 @@ mod tests { ); // Compiling again yields the same bytes: the digest is a stable identity. - let again = root.join("target/again.bin"); + let again = root.join("target/again.mavros"); assert_eq!( - run_compile(&root, &again, &binary, None, false, false, false, 128).unwrap(), + run_compile(&root, &again, None, false, false, false, 128).unwrap(), ExitCode::SUCCESS ); assert_eq!(fs::read(&again).unwrap(), bytes); @@ -1224,15 +1179,12 @@ mod tests { let dir = workspace_fixture(); // Direct package compilation resolves relative output paths from the caller's // working directory. - let output = tempfile::tempdir_in(".").unwrap(); - let output_path = Path::new(output.path().file_name().unwrap()); - let r1cs = output_path.join("r1cs.bin"); - let binary = output_path.join("basic.json"); + let output_dir = tempfile::tempdir_in(".").unwrap(); + let output = Path::new(output_dir.path().file_name().unwrap()).join("program.mavros"); assert_eq!( run_compile( &dir.path().join("a"), - &r1cs, - &binary, + &output, None, false, false, @@ -1242,8 +1194,7 @@ mod tests { .unwrap(), ExitCode::SUCCESS ); - assert!(r1cs.is_file()); - assert!(binary.is_file()); + assert!(output.is_file()); } #[test] diff --git a/compiler/src/driver.rs b/compiler/src/driver.rs index f214201c..c595d7d5 100644 --- a/compiler/src/driver.rs +++ b/compiler/src/driver.rs @@ -227,7 +227,7 @@ impl Driver { /// feature can be prototyped before the frontend supports it. /// /// Such a driver has no [`Self::abi`], so [`Self::entry_blob_field_count`], - /// [`Self::witness_manifest`], [`Self::r1cs_artifact`] and the WASM metadata sidecar are + /// [`Self::witness_manifest`], [`Self::artifact`] and the WASM metadata sidecar are /// unavailable, and no [`Self::package_root`]: all belong to the Noir package, which an SSA /// does not carry. /// @@ -1040,9 +1040,20 @@ impl Driver { Ok(manifest) } - pub fn r1cs_artifact(&self, r1cs: SealedR1CS) -> Result, Error> { + pub fn artifact( + &self, + r1cs: SealedR1CS, + bytecode: Vec, + abi: Vec, + ) -> Result, Error> { let manifest = self.witness_manifest(&r1cs)?; - Ok(Artifact::new(manifest, r1cs)) + Ok(Artifact::new( + manifest, + r1cs, + bytecode, + abi, + env!("CARGO_PKG_VERSION").to_string(), + )) } /// The parameters only; the return is computed, not an input. diff --git a/compiler/tests/verifier.rs b/compiler/tests/verifier.rs index 87e46174..a92cc6c6 100644 --- a/compiler/tests/verifier.rs +++ b/compiler/tests/verifier.rs @@ -604,7 +604,7 @@ fn cli_exports_verifier_separately_without_changing_prover_binary() { ); }; invoke(&[]); - let prover_path = project.path().join("target/basic.json"); + let prover_path = project.path().join("target/program.mavros"); let original_prover = std::fs::read(&prover_path).unwrap(); assert!( !project diff --git a/mavros-artifacts/src/lib.rs b/mavros-artifacts/src/lib.rs index c6eec3c8..c131f40c 100644 --- a/mavros-artifacts/src/lib.rs +++ b/mavros-artifacts/src/lib.rs @@ -638,7 +638,7 @@ impl SealedR1CS { F::ID } - /// Use [`Artifact::digest`] to also bind the witness manifest. + /// Use [`Artifact::digest`] to also bind the witness manifest, bytecode and ABI. pub fn digest(&self) -> [u8; 32] { *self.digest.get_or_init(|| self.system.digest()) } diff --git a/mavros-artifacts/src/manifest.rs b/mavros-artifacts/src/manifest.rs index f44954e3..504aeb1e 100644 --- a/mavros-artifacts/src/manifest.rs +++ b/mavros-artifacts/src/manifest.rs @@ -1,13 +1,14 @@ -//! Witness metadata and the versioned `r1cs.bin` format. +//! Witness metadata and the versioned `program.mavros` artifact. //! //! ```text //! [ MAVR1CS\0 (8 bytes) ][ format version: u32 LE ][ field: u32 LE ][ artifact digest: 32 bytes ] -//! [ bincode { manifest, r1cs } ] +//! [ bincode { compiler, manifest, r1cs, bytecode, abi } ] //! ``` //! -//! The digest binds the field, R1CS and witness manifest; reading verifies it and refuses bytes -//! after the body. Bump [`ARTIFACT_FORMAT_VERSION`] when changing the header, the serialized -//! manifest, the R1CS or the layouts. +//! The digest binds the field, R1CS, witness manifest, VM bytecode and ABI; reading verifies it +//! and refuses bytes after the body. The compiler version is informational and stays outside the +//! digest. Bump [`ARTIFACT_FORMAT_VERSION`] when changing the header, the serialized body or the +//! layouts. //! //! The manifest records only what the R1CS cannot say: each parameter's name, visibility, width //! and witness columns, and the return's. Witness and constraint counts come from the R1CS @@ -151,7 +152,7 @@ pub const ARTIFACT_MAGIC: [u8; 8] = *b"MAVR1CS\0"; pub const ARTIFACT_FORMAT_VERSION: u32 = 1; /// Domain separator of [`Artifact::digest`]. Bump when its preimage encoding changes. -pub const ARTIFACT_DIGEST_DOMAIN: &[u8] = b"mavros-r1cs-artifact-v1\0"; +pub const ARTIFACT_DIGEST_DOMAIN: &[u8] = b"mavros-artifact-v1\0"; const VERSION_OFFSET: usize = ARTIFACT_MAGIC.len(); const FIELD_OFFSET: usize = VERSION_OFFSET + 4; @@ -160,19 +161,17 @@ const HEADER_LEN: usize = DIGEST_OFFSET + 32; #[derive(Debug, thiserror::Error)] pub enum ArtifactError { - #[error("r1cs.bin does not start with the MAVR1CS header, so it is not a Mavros R1CS artifact")] + #[error("the file does not start with the MAVR1CS header, so it is not a Mavros artifact")] MissingHeader, - #[error("unsupported r1cs.bin format version {0}; expected {ARTIFACT_FORMAT_VERSION}")] + #[error("unsupported artifact format version {0}; expected {ARTIFACT_FORMAT_VERSION}")] UnsupportedVersion(u32), - #[error("r1cs.bin names unknown field {0}")] + #[error("the artifact names unknown field {0}")] UnknownField(u32), - #[error("r1cs.bin is over {found:?} but {expected:?} was requested")] + #[error("the artifact is over {found:?} but {expected:?} was requested")] WrongField { expected: FieldId, found: FieldId }, - #[error("r1cs.bin body: {0}")] + #[error("artifact body: {0}")] Body(#[from] bincode::Error), - #[error( - "r1cs.bin digest mismatch: the header says {expected} but the artifact hashes to {actual}" - )] + #[error("artifact digest mismatch: the header says {expected} but the body hashes to {actual}")] DigestMismatch { expected: String, actual: String }, #[error("{0}")] Invalid(String), @@ -191,34 +190,53 @@ fn hex(bytes: &[u8]) -> String { #[derive(Debug, Deserialize)] #[serde(bound = "F: MavrosField")] struct Body { + compiler: String, manifest: WitnessManifest, r1cs: R1CS, + bytecode: Vec, + abi: Vec, } #[derive(Serialize)] #[serde(bound = "F: MavrosField")] struct BodyRef<'a, F: MavrosField> { + compiler: &'a str, manifest: &'a WitnessManifest, r1cs: &'a R1CS, + bytecode: &'a [u64], + abi: &'a [u8], } #[derive(Debug)] pub struct Artifact { + compiler: String, manifest: WitnessManifest, r1cs: SealedR1CS, + bytecode: Vec, + abi: Vec, digest: [u8; 32], } impl Artifact { - pub fn new(manifest: WitnessManifest, r1cs: SealedR1CS) -> Self { + pub fn new( + manifest: WitnessManifest, + r1cs: SealedR1CS, + bytecode: Vec, + abi: Vec, + compiler: String, + ) -> Self { let mut hasher = Sha256::new(); hasher.update(ARTIFACT_DIGEST_DOMAIN); hasher.update(r1cs.digest()); - bincode::serialize_into(&mut hasher, &manifest) - .expect("hashing a witness manifest cannot fail: the hasher accepts every byte"); + bincode::serialize_into(&mut hasher, &(&manifest, &bytecode)) + .expect("hashing cannot fail: the hasher accepts every byte"); + hasher.update(&abi); Self { + compiler, manifest, r1cs, + bytecode, + abi, digest: hasher.finalize().into(), } } @@ -227,9 +245,9 @@ impl Artifact { self.r1cs.field() } - /// SHA-256 over [`ARTIFACT_DIGEST_DOMAIN`], the field-bound [`R1CS::digest`] and the - /// bincode-encoded manifest. Pin this digest to identify both the system and its public - /// input/output mapping; [`SealedR1CS::digest`] identifies only the constraint system. + /// SHA-256 over [`ARTIFACT_DIGEST_DOMAIN`], the field-bound [`R1CS::digest`], the + /// bincode-encoded manifest and bytecode, and the ABI bytes. Pin this digest to identify the + /// whole program; [`SealedR1CS::digest`] identifies only the constraint system. pub fn digest(&self) -> [u8; 32] { self.digest } @@ -242,8 +260,26 @@ impl Artifact { &self.r1cs } - pub fn into_parts(self) -> (FieldId, WitnessManifest, SealedR1CS) { - (self.r1cs.field(), self.manifest, self.r1cs) + pub fn bytecode(&self) -> &[u64] { + &self.bytecode + } + + pub fn abi(&self) -> &[u8] { + &self.abi + } + + pub fn compiler(&self) -> &str { + &self.compiler + } + + pub fn into_parts(self) -> (FieldId, WitnessManifest, SealedR1CS, Vec, Vec) { + ( + self.r1cs.field(), + self.manifest, + self.r1cs, + self.bytecode, + self.abi, + ) } pub fn to_bytes(&self) -> Result, ArtifactError> { @@ -256,8 +292,11 @@ impl Artifact { body_options().serialize_into( &mut bytes, &BodyRef { + compiler: &self.compiler, manifest: &self.manifest, r1cs: self.r1cs.system(), + bytecode: &self.bytecode, + abi: &self.abi, }, )?; Ok(bytes) @@ -288,7 +327,13 @@ impl Artifact { .try_into() .expect("32 bytes"); let body: Body = body_options().deserialize(&bytes[HEADER_LEN..])?; - let artifact = Self::new(body.manifest, SealedR1CS::new(body.r1cs)); + let artifact = Self::new( + body.manifest, + SealedR1CS::new(body.r1cs), + body.bytecode, + body.abi, + body.compiler, + ); let actual = artifact.digest(); if actual != digest { return Err(ArtifactError::DigestMismatch { @@ -357,10 +402,30 @@ mod tests { } } + fn bytecode() -> Vec { + vec![1, 2, 3] + } + + fn abi() -> Vec { + b"{}".to_vec() + } + + fn build(manifest: WitnessManifest, r1cs: R1CS, bytecode: Vec, abi: Vec) -> Artifact { + Artifact::new( + manifest, + SealedR1CS::new(r1cs), + bytecode, + abi, + "test".into(), + ) + } + + fn artifact(manifest: WitnessManifest, r1cs: R1CS) -> Artifact { + build(manifest, r1cs, bytecode(), abi()) + } + fn bytes_of(manifest: WitnessManifest, r1cs: R1CS) -> Vec { - Artifact::new(manifest, SealedR1CS::new(r1cs)) - .to_bytes() - .unwrap() + artifact(manifest, r1cs).to_bytes().unwrap() } fn read(bytes: &[u8]) -> Result { @@ -406,6 +471,9 @@ mod tests { assert_eq!(&bytes[DIGEST_OFFSET..HEADER_LEN], &decoded.digest()); assert_ne!(decoded.digest(), decoded.r1cs().digest()); assert_eq!(decoded.manifest(), &m); + assert_eq!(decoded.bytecode(), bytecode()); + assert_eq!(decoded.abi(), abi()); + assert_eq!(decoded.compiler(), "test"); assert_eq!( decoded.r1cs().witness_layout.size(), r.witness_layout.size() @@ -489,15 +557,14 @@ mod tests { hex(&r1cs().digest()), "a1826c2cf819ba7805777209988b33f98071393e14c2671b9ddb3fe9fafe2c52" ); - let artifact = Artifact::new(manifest(), SealedR1CS::new(r1cs())); assert_eq!( - hex(&artifact.digest()), - "91938a47b4fbf034bad07ba83f8289ca1703176a63fae4867f05c0cd4505de18" + hex(&artifact(manifest(), r1cs()).digest()), + "b99bd3f535b2a9e8536834ae55ffe41732c9ae1b92a44564b89f9f30e9c7fd9c" ); } - /// A flipped header byte, another R1CS under the honest digest, or a manifest edit that - /// still validates: each is a digest mismatch. + /// A flipped header byte, another R1CS, bytecode or ABI under the honest digest, or a + /// manifest edit that still validates: each is a digest mismatch. #[test] fn tampering_is_rejected() { let honest = bytes_of(manifest(), r1cs()); @@ -523,6 +590,18 @@ mod tests { "changed coefficient", ); + let with_parts = |bytecode: Vec, abi: Vec| { + build(manifest(), r1cs(), bytecode, abi).to_bytes().unwrap() + }; + rejects( + &with_honest_digest(with_parts(vec![1, 2, 4], abi())), + "changed bytecode word", + ); + rejects( + &with_honest_digest(with_parts(bytecode(), b"{ }".to_vec())), + "changed ABI byte", + ); + let variants: [fn(&mut WitnessManifest); 6] = [ |m| m.returns.as_mut().unwrap().column_start = Some(3), |m| m.returns.as_mut().unwrap().visibility = ParamVisibility::Private, From b3c0604702b48232376c1156e7604d6a01e3e5d2 Mon Sep 17 00:00:00 2001 From: Yogesh Shahi Date: Mon, 19 Oct 2026 17:49:38 +0530 Subject: [PATCH 3/8] Document the single artifact --- README.md | 27 ++++++++++++--------------- docs/ARCHITECTURE.md | 2 +- 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 417ba7e9..8e1ad52c 100644 --- a/README.md +++ b/README.md @@ -26,17 +26,15 @@ information on how to use Mavros. To use Mavros, navigate to the **root of a Noir project** (the directory containing `Nargo.toml`) in your shell. Then run `mavros compile`. -This will produce the following two files inside the project's `target/` directory: +This writes one file, `target/program.mavros`. It starts with a `MAVR1CS` header (format version, +field tag, digest) followed by the witness manifest, the R1CS, the VM bytecode with the witgen and +AD entry points, the ABI as JSON, and the compiler version. The digest covers everything except the +compiler version and is checked whenever the file is read. -| File | Contents | -| ------------------- | --------------------------------------------------------------------------------------------- | -| `target/basic.json` | ABI and the program binary (`binary`) containing the witgen and AD entry points | -| `target/r1cs.bin` | Versioned R1CS artifact (`MAVR1CS` header): format version, field tag, digest, manifest, R1CS | - -Pass `--include-debug-info` to additionally write `target/basic.debug.json`. This standalone, +Pass `--include-debug-info` to additionally write `target/program.debug.json`. This standalone, versioned source map records VM bytecode word offsets, function names, and Noir source locations; -the executable bytecode in `basic.json` remains byte-for-byte unchanged. Source paths are relative -to the Noir package root by default; pass `--absolute-paths` to retain absolute paths. +the artifact itself remains byte-for-byte unchanged. Source paths are relative to the Noir package +root by default; pass `--absolute-paths` to retain absolute paths. ### Verifier Mode @@ -126,10 +124,10 @@ compiles every binary member in manifest order; library members remain available Each binary uses its own `Prover.toml` and `mavros_debug` directory. A failed member makes the workspace run fail, while the remaining members are still checked. -When `mavros compile` resolves a package root different from the supplied path, relative output -paths use that package root, including for a single default member. The defaults then produce -`/target/basic.json` and `/target/r1cs.bin`. Absolute output paths and paths -containing `..` are rejected for multi-binary compilations. Point Mavros directly at a member +When `mavros compile` resolves a package root different from the supplied path, a relative +`--output` (and `--verifier-output`) is taken relative to that package root, including for a single +default member, so the default produces `/target/program.mavros`. An absolute output path, +or one containing `..`, is rejected for multi-binary compilations. Point Mavros directly at a member directory to compile just that package with the usual output-path behavior. The functional test runner reports each selected binary workspace member as a separate test, with @@ -140,8 +138,7 @@ its own checks, circuit size, and artifact sizes. Libraries are not separate exe ```bash cd my-noir-project mavros compile -# → target/basic.json -# → target/r1cs.bin +# → target/program.mavros ``` ## Noir Feature Support and Completeness diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 1eb956fa..5fb082d9 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -218,7 +218,7 @@ constant one, public parameters, surviving private parameters, internal witnesse returns in that order. The constant, public parameters and all returns are pinned. Private parameters may be elided only as a whole; integer range checks keep otherwise unused parameters live. Synthetic source origins let the R1CS generator recover the surviving columns for the -`r1cs.bin` witness manifest. Return columns are constrained to the computation; an expected `return` +artifact's witness manifest. Return columns are constrained to the computation; an expected `return` in `Prover.toml` is checked by the host after witness generation. ### Stage 6: R1CS Generation and Further Optimization From 3174052c75bae92c7aa57dcad9c3ebd238f05ca1 Mon Sep 17 00:00:00 2001 From: Yogesh Shahi Date: Thu, 22 Oct 2026 19:30:11 +0530 Subject: [PATCH 4/8] Bump the artifact format version and hash the ABI with bincode --- mavros-artifacts/src/manifest.rs | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/mavros-artifacts/src/manifest.rs b/mavros-artifacts/src/manifest.rs index 504aeb1e..64f60520 100644 --- a/mavros-artifacts/src/manifest.rs +++ b/mavros-artifacts/src/manifest.rs @@ -149,7 +149,7 @@ impl WitnessManifest { pub const ARTIFACT_MAGIC: [u8; 8] = *b"MAVR1CS\0"; /// Format version this crate writes and the only one it reads. -pub const ARTIFACT_FORMAT_VERSION: u32 = 1; +pub const ARTIFACT_FORMAT_VERSION: u32 = 2; /// Domain separator of [`Artifact::digest`]. Bump when its preimage encoding changes. pub const ARTIFACT_DIGEST_DOMAIN: &[u8] = b"mavros-artifact-v1\0"; @@ -228,9 +228,8 @@ impl Artifact { let mut hasher = Sha256::new(); hasher.update(ARTIFACT_DIGEST_DOMAIN); hasher.update(r1cs.digest()); - bincode::serialize_into(&mut hasher, &(&manifest, &bytecode)) + bincode::serialize_into(&mut hasher, &(&manifest, &bytecode, &abi)) .expect("hashing cannot fail: the hasher accepts every byte"); - hasher.update(&abi); Self { compiler, manifest, @@ -245,9 +244,9 @@ impl Artifact { self.r1cs.field() } - /// SHA-256 over [`ARTIFACT_DIGEST_DOMAIN`], the field-bound [`R1CS::digest`], the - /// bincode-encoded manifest and bytecode, and the ABI bytes. Pin this digest to identify the - /// whole program; [`SealedR1CS::digest`] identifies only the constraint system. + /// SHA-256 over [`ARTIFACT_DIGEST_DOMAIN`], the field-bound [`R1CS::digest`] and the + /// bincode-encoded manifest, bytecode and ABI. Pin this digest to identify the whole program; + /// [`SealedR1CS::digest`] identifies only the constraint system. pub fn digest(&self) -> [u8; 32] { self.digest } @@ -490,7 +489,7 @@ mod tests { read(&bytes[..HEADER_LEN - 1]), Err(ArtifactError::MissingHeader) )); - for version in [0, ARTIFACT_FORMAT_VERSION + 1] { + for version in [0, ARTIFACT_FORMAT_VERSION - 1, ARTIFACT_FORMAT_VERSION + 1] { bytes[VERSION_OFFSET..FIELD_OFFSET].copy_from_slice(&version.to_le_bytes()); assert!(matches!( read(&bytes), @@ -559,7 +558,21 @@ mod tests { ); assert_eq!( hex(&artifact(manifest(), r1cs()).digest()), - "b99bd3f535b2a9e8536834ae55ffe41732c9ae1b92a44564b89f9f30e9c7fd9c" + "ed60cb0a19c5725509c601e780ffb5869f58d6f5d05db0be110f6bf0f4cb4de6" + ); + + // The compiler version stays outside the digest. + let other = Artifact::new( + manifest(), + SealedR1CS::new(r1cs()), + bytecode(), + abi(), + "other".into(), + ); + assert_eq!(other.digest(), artifact(manifest(), r1cs()).digest()); + assert_eq!( + read(&other.to_bytes().unwrap()).unwrap().compiler(), + "other" ); } From 8c3e686c77d443d0c325522da96cb72dc3af3a44 Mon Sep 17 00:00:00 2001 From: Yogesh Shahi Date: Thu, 22 Oct 2026 19:30:11 +0530 Subject: [PATCH 5/8] Derive the ABI and release version in the driver and tidy compile --- compiler/Cargo.toml | 2 +- compiler/src/bin/main.rs | 46 +++++++++++++++++++------------------- compiler/src/driver.rs | 9 ++++++-- compiler/tests/verifier.rs | 6 ++--- 4 files changed, 34 insertions(+), 29 deletions(-) diff --git a/compiler/Cargo.toml b/compiler/Cargo.toml index 03c98740..1e8f202f 100644 --- a/compiler/Cargo.toml +++ b/compiler/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mavros-compiler" -version = "0.1.0" +version.workspace = true edition = "2024" [dependencies] diff --git a/compiler/src/bin/main.rs b/compiler/src/bin/main.rs index 130ba280..01f69e6b 100644 --- a/compiler/src/bin/main.rs +++ b/compiler/src/bin/main.rs @@ -98,7 +98,7 @@ pub enum Command { #[arg(default_value = DEFAULT_NOIR_PROJECT_PATH, value_parser = parse_path)] path: PathBuf, - /// Output path for the artifact: header, witness manifest, R1CS, VM bytecode and ABI. + /// Output path for the Mavros artifact. #[arg(long, default_value = "target/program.mavros")] output: PathBuf, @@ -222,22 +222,17 @@ pub fn run_compile( ).into()); } run_packages(&roots, |root| { - let rebase = root != &requested_root; - let output = if rebase { - root.join(output) - } else { - output.clone() - }; - let verifier_output = verifier_output.map(|output| { - if rebase { - root.join(output) + let rebase = |path: &PathBuf| { + if root != &requested_root { + root.join(path) } else { - output.clone() + path.clone() } - }); + }; + let verifier_output = verifier_output.map(rebase); run_compile_package( root, - &output, + &rebase(output), verifier_output.as_ref(), draw_graphs, include_debug_info, @@ -269,15 +264,15 @@ fn run_compile_package( )?; if let Some(parent) = output.parent() { - if !parent.as_os_str().is_empty() { - fs::create_dir_all(parent)?; - } + fs::create_dir_all(parent)?; } - let abi = serde_json::to_vec(driver.abi())?; - let artifact = driver.artifact(r1cs, bytecode.binary, abi)?; - let bytes = artifact.to_bytes()?; - fs::write(output, &bytes)?; + let artifact = driver.artifact(r1cs, bytecode.binary)?; + let artifact_bytes = { + let bytes = artifact.to_bytes()?; + fs::write(output, &bytes)?; + bytes.len() + }; write_vm_debug_info(output, bytecode.debug_info, &driver, absolute_paths)?; @@ -289,7 +284,7 @@ fn run_compile_package( ..CodeGenOptions::default() }, )?; - if let Some(parent) = output.parent().filter(|p| !p.as_os_str().is_empty()) { + if let Some(parent) = output.parent() { fs::create_dir_all(parent)?; } fs::write( @@ -306,7 +301,7 @@ fn run_compile_package( info!( message = %"Artifact saved successfully", output = ?output, - artifact_bytes = bytes.len(), + artifact_bytes, r1cs_constraints = artifact.r1cs().constraints.len(), binary_size = artifact.bytecode().len() * 8, ); @@ -822,6 +817,7 @@ mod tests { let root = dir.path(); for (output, verifier) in [ ("p.mavros", "p.mavros"), + ("p.mavros", "p.json"), ("p.mavros", "p.debug.json"), ("p.debug.json", "p.json"), ("p.mavros", "./p.mavros"), @@ -872,6 +868,7 @@ mod tests { fs::create_dir(root.join("nested")).unwrap(); let case_insensitive = root.join("SENTINEL").exists(); for (output, verifier) in [ + ("sentinel", "SENTINEL"), ("p.mavros", "P.mavros"), ("p.mavros", "P.json"), ("new/p.mavros", "NEW/P.json"), @@ -1135,7 +1132,10 @@ mod tests { let abi: noirc_abi::Abi = serde_json::from_slice(artifact.abi()).unwrap(); let names: Vec<_> = abi.parameters.iter().map(|p| p.name.as_str()).collect(); assert_eq!(names, ["unused", "unused_u8", "x", "y"]); - assert_eq!(artifact.compiler(), env!("CARGO_PKG_VERSION")); + assert_eq!( + artifact.compiler(), + mavros_compiler::driver::COMPILER_VERSION + ); let columns: Vec<_> = manifest .parameters .iter() diff --git a/compiler/src/driver.rs b/compiler/src/driver.rs index c595d7d5..f889e042 100644 --- a/compiler/src/driver.rs +++ b/compiler/src/driver.rs @@ -101,6 +101,9 @@ pub struct EntryColumns { /// the forward tripwire once a small field (goldilocks) needs more than one challenge. pub const DEFAULT_LOGUP_SOUNDNESS_BITS: u32 = 128; +/// Recorded in every artifact: the release version of the Mavros workspace. +pub const COMPILER_VERSION: &str = env!("CARGO_PKG_VERSION"); + pub struct Driver { /// The Noir package being compiled, or `None` for a driver seeded with a hand-built /// [`HLSSA`] (see [`Driver::from_ssa`]). Only the frontend stage and the source-path @@ -1040,19 +1043,21 @@ impl Driver { Ok(manifest) } + /// Packs `r1cs` and `bytecode` with this driver's witness manifest and ABI; requires what + /// [`Self::witness_manifest`] does. pub fn artifact( &self, r1cs: SealedR1CS, bytecode: Vec, - abi: Vec, ) -> Result, Error> { let manifest = self.witness_manifest(&r1cs)?; + let abi = serde_json::to_vec(self.abi()).expect("a Noir ABI always serializes to JSON"); Ok(Artifact::new( manifest, r1cs, bytecode, abi, - env!("CARGO_PKG_VERSION").to_string(), + COMPILER_VERSION.to_string(), )) } diff --git a/compiler/tests/verifier.rs b/compiler/tests/verifier.rs index a92cc6c6..e1a90863 100644 --- a/compiler/tests/verifier.rs +++ b/compiler/tests/verifier.rs @@ -604,8 +604,8 @@ fn cli_exports_verifier_separately_without_changing_prover_binary() { ); }; invoke(&[]); - let prover_path = project.path().join("target/program.mavros"); - let original_prover = std::fs::read(&prover_path).unwrap(); + let program_path = project.path().join("target/program.mavros"); + let original_program = std::fs::read(&program_path).unwrap(); assert!( !project .path() @@ -617,7 +617,7 @@ fn cli_exports_verifier_separately_without_changing_prover_binary() { "target/verifier.json", "--include-debug-info", ]); - assert_eq!(std::fs::read(&prover_path).unwrap(), original_prover); + assert_eq!(std::fs::read(&program_path).unwrap(), original_program); let artifact: serde_json::Value = serde_json::from_slice( &std::fs::read(project.path().join("target/verifier.json")).unwrap(), ) From 172824e74d39cb766f655a295409d3c4102d4030 Mon Sep 17 00:00:00 2001 From: Yogesh Shahi Date: Thu, 22 Oct 2026 19:30:11 +0530 Subject: [PATCH 6/8] Update the README for the single artifact --- README.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 8e1ad52c..7575039d 100644 --- a/README.md +++ b/README.md @@ -27,9 +27,10 @@ To use Mavros, navigate to the **root of a Noir project** (the directory contain your shell. Then run `mavros compile`. This writes one file, `target/program.mavros`. It starts with a `MAVR1CS` header (format version, -field tag, digest) followed by the witness manifest, the R1CS, the VM bytecode with the witgen and -AD entry points, the ABI as JSON, and the compiler version. The digest covers everything except the -compiler version and is checked whenever the file is read. +field tag, digest) followed by the compiler's release version (the workspace `version` in +`Cargo.toml`), the witness manifest, the R1CS, the VM bytecode with the witgen and AD entry points, +and the ABI as JSON. The digest covers everything except the compiler version and is checked +whenever the file is read. Pass `--include-debug-info` to additionally write `target/program.debug.json`. This standalone, versioned source map records VM bytecode word offsets, function names, and Noir source locations; @@ -61,7 +62,8 @@ The command compiles the project and prints a JSON object with `r_aw`, `r_bw`, a strings. It needs no `Prover.toml` and does not check R1CS satisfaction. This operation is a component of a verifier, not a complete proof verification protocol. -Prover artifacts contain only witgen and AD. To additionally emit a separate verifier artifact: +The bytecode in `program.mavros` holds only the witgen and AD entry points. To additionally emit a +separate verifier artifact: ```bash mavros compile ./my-noir-project --verifier-output target/verifier.json From 2c68b6a6e4435667bb046ec0c93866973480c86d Mon Sep 17 00:00:00 2001 From: Yogesh Shahi Date: Tue, 27 Oct 2026 21:32:58 +0530 Subject: [PATCH 7/8] PR review fixes --- README.md | 38 ++- compiler/src/bin/main.rs | 38 ++- compiler/src/driver.rs | 2 +- compiler/tests/verifier.rs | 21 ++ docs/field-agnosticism.md | 2 +- mavros-artifacts/src/lib.rs | 7 +- mavros-artifacts/src/manifest.rs | 452 ++++++++++++++++++++----- mavros-artifacts/src/program_header.rs | 138 ++++++++ vm/src/bytecode.rs | 109 +----- vm/src/interpreter.rs | 7 +- 10 files changed, 602 insertions(+), 212 deletions(-) create mode 100644 mavros-artifacts/src/program_header.rs diff --git a/README.md b/README.md index 7575039d..0d378c1e 100644 --- a/README.md +++ b/README.md @@ -26,11 +26,13 @@ information on how to use Mavros. To use Mavros, navigate to the **root of a Noir project** (the directory containing `Nargo.toml`) in your shell. Then run `mavros compile`. -This writes one file, `target/program.mavros`. It starts with a `MAVR1CS` header (format version, -field tag, digest) followed by the compiler's release version (the workspace `version` in -`Cargo.toml`), the witness manifest, the R1CS, the VM bytecode with the witgen and AD entry points, -and the ABI as JSON. The digest covers everything except the compiler version and is checked -whenever the file is read. +This writes one file, `target/program.mavros`. Its `MAVR1CS` header holds the format version, the +compiler's release version (the workspace `version` in `Cargo.toml`), the field tag and the artifact +digest; the body holds the witness manifest, the R1CS, the VM bytecode with the witgen and AD entry +points, and the ABI as JSON. Two digests identify it: the circuit digest hashes the R1CS (which +hashes its field tag) and the manifest; the artifact digest hashes the circuit digest, the bytecode +and the ABI, and is checked whenever the file is read. The header itself is checked, not hashed, and +the compiler version is informational. Pass `--include-debug-info` to additionally write `target/program.debug.json`. This standalone, versioned source map records VM bytecode word offsets, function names, and Noir source locations; @@ -69,23 +71,23 @@ separate verifier artifact: mavros compile ./my-noir-project --verifier-output target/verifier.json ``` -`verifier.json` contains only verifier bytecode plus `witness_layout` and `constraints_layout`; it -contains no matrix rows or prover entry points. `--include-debug-info` also writes -`verifier.debug.json`. Default compilation emits no verifier code. `--verify` compiles and runs only -the verifier artifact. +`verifier.json` contains the verifier bytecode, `witness_layout`, `constraints_layout` and the +`circuit_digest` of the `program.mavros` it was compiled from; it contains no matrix rows or prover +entry points. `--include-debug-info` also writes `verifier.debug.json`. Default compilation emits no +verifier code. `--verify` compiles and runs only the verifier artifact. Rust callers compile it with `mavros_compiler::api::compile_verifier_bytecode(&mut driver, options)` and pass that binary to `run_verifier_from_binary(&binary, &r, &w, witness_layout, constraints_layout, None)`. No R1CS -matrices are required at runtime. Both artifacts must come from the same circuit compilation; -applications must associate the verifier artifact with their trusted circuit configuration. Verifier -execution uses a dedicated forward bytecode entry point: witness writes read the next value from -`w`, linear combinations use field arithmetic, and constraints accumulate three scalars weighted by -`r`. It allocates no AD expression nodes or matrix output vectors. Lookup and table constraints are -evaluated from the same layouts. Bytecode format version 3 binds the field and constraint layouts to -each compiled artifact; recompile older artifacts before using the current VM. AD and verifier -execution reject mismatched layouts. LLVM and WASM artifacts still expose the existing witgen and AD -entry points; the forward verifier is currently available through the VM API and CLI. +matrices are required at runtime. Compare the verifier's `circuit_digest` with the artifact's before +trusting the pair. Verifier execution uses a dedicated forward bytecode entry point: witness writes +read the next value from `w`, linear combinations use field arithmetic, and constraints accumulate +three scalars weighted by `r`. It allocates no AD expression nodes or matrix output vectors. Lookup +and table constraints are evaluated from the same layouts. The bytecode header names the field and +the constraint layouts of the artifact it belongs to; recompile older artifacts before using the +current VM. AD and verifier execution reject mismatched layouts. LLVM and WASM artifacts still +expose the existing witgen and AD entry points; the forward verifier is currently available through +the VM API and CLI. ### Per-Function FlameGraphs diff --git a/compiler/src/bin/main.rs b/compiler/src/bin/main.rs index 11f348a4..13fe4dc4 100644 --- a/compiler/src/bin/main.rs +++ b/compiler/src/bin/main.rs @@ -293,6 +293,7 @@ fn run_compile_package( "binary": verifier.binary, "witness_layout": artifact.r1cs().witness_layout, "constraints_layout": artifact.r1cs().constraints_layout, + "circuit_digest": mavros_artifacts::hex(&artifact.circuit_digest()), }))?, )?; write_vm_debug_info(output, verifier.debug_info, &driver, absolute_paths)?; @@ -1105,7 +1106,7 @@ mod tests { /// Covers LogUp slots, unused private inputs and partially read compound parameters. #[test] fn compile_writes_one_artifact() { - use mavros_artifacts::{Artifact, ParamVisibility}; + use mavros_artifacts::{Artifact, ArtifactPreamble, ParamVisibility}; let dir = tempfile::tempdir().unwrap(); let root = dir.path().to_path_buf(); fs::create_dir_all(root.join("src")).unwrap(); @@ -1131,10 +1132,6 @@ mod tests { let artifact: Artifact = Artifact::from_bytes(&bytes).unwrap(); let manifest = artifact.manifest(); assert_eq!(manifest.entry_blob_field_count(), 8); - assert_eq!( - mavros_vm::bytecode::parse_program_header(artifact.bytecode()).entry_blob_field_count, - 8 - ); let abi: noirc_abi::Abi = serde_json::from_slice(artifact.abi()).unwrap(); let names: Vec<_> = abi.parameters.iter().map(|p| p.name.as_str()).collect(); assert_eq!(names, ["unused", "unused_u8", "x", "y"]); @@ -1142,6 +1139,10 @@ mod tests { artifact.compiler(), mavros_compiler::driver::COMPILER_VERSION ); + assert_eq!( + ArtifactPreamble::peek(&bytes).unwrap().compiler.as_deref(), + Some(mavros_compiler::driver::COMPILER_VERSION) + ); let columns: Vec<_> = manifest .parameters .iter() @@ -1178,6 +1179,33 @@ mod tests { ExitCode::SUCCESS ); assert_eq!(fs::read(&again).unwrap(), bytes); + + // What a consumer does with the file: witgen and AD from its bytecode, R1CS and ABI. + fs::write( + root.join("Prover.toml"), + "unused = \"7\"\nunused_u8 = \"9\"\nx = [\"1\", \"5\", \"2\"]\ny = [\"3\", \"0\", \"0\"]\nreturn = [\"6\", \"5\"]\n", + ) + .unwrap(); + let (params, declared_return) = api::read_prover_inputs(&root, &abi).unwrap(); + let r1cs = artifact.r1cs(); + let mut bytecode = artifact.bytecode().to_vec(); + let witgen = api::run_witgen_from_binary(&mut bytecode, r1cs, ¶ms, None).unwrap(); + assert!(api::check_witgen(r1cs, &witgen)); + mavros_compiler::abi_helpers::check_witness_manifest( + manifest, + ¶ms, + declared_return.as_ref(), + &witgen.out_wit_pre_comm, + ) + .unwrap(); + use ark_ff::UniformRand as _; + use rand::SeedableRng as _; + let mut rng = rand::rngs::StdRng::seed_from_u64(42); + let coeffs: Vec = (0..r1cs.constraints.len()) + .map(|_| RawField::rand(&mut rng)) + .collect(); + let (a, b, c, _) = api::run_ad_from_binary(&mut bytecode, r1cs, &coeffs, None).unwrap(); + assert!(api::check_ad(r1cs, &coeffs, &a, &b, &c)); } #[test] diff --git a/compiler/src/driver.rs b/compiler/src/driver.rs index f889e042..0a512fce 100644 --- a/compiler/src/driver.rs +++ b/compiler/src/driver.rs @@ -1053,11 +1053,11 @@ impl Driver { let manifest = self.witness_manifest(&r1cs)?; let abi = serde_json::to_vec(self.abi()).expect("a Noir ABI always serializes to JSON"); Ok(Artifact::new( + COMPILER_VERSION.to_string(), manifest, r1cs, bytecode, abi, - COMPILER_VERSION.to_string(), )) } diff --git a/compiler/tests/verifier.rs b/compiler/tests/verifier.rs index e1a90863..878bedc5 100644 --- a/compiler/tests/verifier.rs +++ b/compiler/tests/verifier.rs @@ -587,6 +587,21 @@ fn verifier_artifact_cannot_run_as_a_prover() { assert!(error.to_string().contains("no witgen entry point")); } +#[test] +fn a_verifier_binary_is_not_a_prover_artifact() { + use mavros_artifacts::{Artifact, ArtifactError}; + let project = common::noir_project("fn main(x: Field) { assert(x * x == 9); }"); + let (mut driver, r1cs) = api::compile_to_r1cs(project.path().to_path_buf(), false).unwrap(); + let verifier = api::compile_verifier_bytecode(&mut driver, CodeGenOptions::default()).unwrap(); + let bytes = driver.artifact(r1cs, verifier).unwrap().to_bytes().unwrap(); + let error = Artifact::::from_bytes(&bytes).err().unwrap(); + assert!(matches!(error, ArtifactError::Bytecode(_)), "{error}"); + assert!( + error.to_string().contains("not a prover artifact"), + "{error}" + ); +} + #[test] fn cli_exports_verifier_separately_without_changing_prover_binary() { let project = common::noir_project("fn main(x: Field) -> pub Field { x * x + 3 }"); @@ -623,6 +638,12 @@ fn cli_exports_verifier_separately_without_changing_prover_binary() { ) .unwrap(); assert!(artifact.get("constraints").is_none()); + let program: mavros_artifacts::Artifact = + mavros_artifacts::Artifact::from_bytes(&original_program).unwrap(); + assert_eq!( + artifact["circuit_digest"], + mavros_artifacts::hex(&program.circuit_digest()) + ); let binary: Vec = serde_json::from_value(artifact["binary"].clone()).unwrap(); let wl: mavros_vm::WitnessLayout = serde_json::from_value(artifact["witness_layout"].clone()).unwrap(); diff --git a/docs/field-agnosticism.md b/docs/field-agnosticism.md index c983d7bc..24e88c9e 100644 --- a/docs/field-agnosticism.md +++ b/docs/field-agnosticism.md @@ -100,7 +100,7 @@ bytecode and runs on the VM, witgen and AD both checked against the R1CS. Undern constants through `FieldConfig` (`constant` takes `IntoFieldElement`), which refuses an element of another field. - **The VM** carries `goldilocks_*` copies of every field opcode in every build. A program binary - names its field in its header (format version 3), and a run over another field is refused. + names its field in its header, and a run over another field is refused. - **The R1CS** is generic (`R1CS`, with bn254 as the default): `Driver::generate_r1cs::` refuses an `F` other than the program's field, `R1CGen` converts constants with `MavrosField::from_element`, and a serialized coefficient is `F::CANONICAL_LIMBS` limbs. diff --git a/mavros-artifacts/src/lib.rs b/mavros-artifacts/src/lib.rs index c131f40c..b75cd815 100644 --- a/mavros-artifacts/src/lib.rs +++ b/mavros-artifacts/src/lib.rs @@ -1,5 +1,6 @@ pub mod field; pub mod manifest; +pub mod program_header; use std::{cell::OnceCell, collections::BTreeMap, fmt::Display, ops::Deref}; @@ -10,7 +11,8 @@ use tracing::error; pub use field::{FieldConfig, FieldElement, FieldId, Goldilocks, IntoFieldElement, MavrosField}; pub use manifest::{ ARTIFACT_DIGEST_DOMAIN, ARTIFACT_FORMAT_VERSION, ARTIFACT_MAGIC, Artifact, ArtifactError, - ManifestParam, ParamVisibility, ReturnSlot, WitnessManifest, + ArtifactParts, ArtifactPreamble, CIRCUIT_DIGEST_DOMAIN, ManifestParam, ParamVisibility, + ReturnSlot, WitnessManifest, hex, }; // FIELD-ASSUMPTION: L1-alias @@ -638,7 +640,8 @@ impl SealedR1CS { F::ID } - /// Use [`Artifact::digest`] to also bind the witness manifest, bytecode and ABI. + /// Use [`Artifact::circuit_digest`] to also bind the witness manifest, and [`Artifact::digest`] + /// for the bytecode and ABI too. pub fn digest(&self) -> [u8; 32] { *self.digest.get_or_init(|| self.system.digest()) } diff --git a/mavros-artifacts/src/manifest.rs b/mavros-artifacts/src/manifest.rs index 64f60520..3bdb3a37 100644 --- a/mavros-artifacts/src/manifest.rs +++ b/mavros-artifacts/src/manifest.rs @@ -1,14 +1,21 @@ //! Witness metadata and the versioned `program.mavros` artifact. //! //! ```text -//! [ MAVR1CS\0 (8 bytes) ][ format version: u32 LE ][ field: u32 LE ][ artifact digest: 32 bytes ] -//! [ bincode { compiler, manifest, r1cs, bytecode, abi } ] +//! [ MAVR1CS\0 (8 bytes) ][ format version: u32 LE ][ compiler: u32 LE length, UTF-8 bytes ] +//! [ field: u32 LE ][ artifact digest: 32 bytes ][ bincode { manifest, r1cs, bytecode, abi } ] //! ``` //! -//! The digest binds the field, R1CS, witness manifest, VM bytecode and ABI; reading verifies it -//! and refuses bytes after the body. The compiler version is informational and stays outside the -//! digest. Bump [`ARTIFACT_FORMAT_VERSION`] when changing the header, the serialized body or the -//! layouts. +//! The magic, format version and compiler version form the [`ArtifactPreamble`], laid out the same +//! in every format version so that a reader can name the compiler that wrote a file it cannot +//! otherwise open. Everything after it belongs to [`ARTIFACT_FORMAT_VERSION`]; bump that when +//! changing the header, the serialized body or the layouts. +//! +//! Two digests identify an artifact. [`Artifact::circuit_digest`] binds the field, the R1CS and the +//! witness manifest: the constraint system a proof is about. [`Artifact::digest`] binds the circuit +//! digest, the VM bytecode and the ABI: the whole file. The header carries the latter and reading +//! verifies it; the header itself is checked, not hashed, and the compiler version is informational. +//! Reading also checks the manifest and the bytecode header against the R1CS and refuses bytes after +//! the body. //! //! The manifest records only what the R1CS cannot say: each parameter's name, visibility, width //! and witness columns, and the return's. Witness and constraint counts come from the R1CS @@ -26,7 +33,10 @@ use bincode::Options as _; use serde::{Deserialize, Serialize}; use sha2::{Digest as _, Sha256}; -use crate::{Field, FieldId, MavrosField, R1CS, SealedR1CS, field_tag}; +use crate::{ + Field, FieldId, MavrosField, R1CS, SealedR1CS, field_tag, + program_header::{ENTRY_AD, ENTRY_WITGEN, NO_ENTRY_POINT, ProgramHeader}, +}; #[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] @@ -150,21 +160,39 @@ impl WitnessManifest { pub const ARTIFACT_MAGIC: [u8; 8] = *b"MAVR1CS\0"; /// Format version this crate writes and the only one it reads. pub const ARTIFACT_FORMAT_VERSION: u32 = 2; +/// The first format whose header records the compiler version; format 1 recorded none. +const COMPILER_SINCE_VERSION: u32 = 2; + +/// Domain separator of [`Artifact::circuit_digest`]. Its bytes and preimage are those of the +/// `r1cs.bin` digest this format replaced, so a circuit keeps the digest it had there. Bump when +/// the preimage encoding changes. +pub const CIRCUIT_DIGEST_DOMAIN: &[u8] = b"mavros-r1cs-artifact-v1\0"; /// Domain separator of [`Artifact::digest`]. Bump when its preimage encoding changes. pub const ARTIFACT_DIGEST_DOMAIN: &[u8] = b"mavros-artifact-v1\0"; const VERSION_OFFSET: usize = ARTIFACT_MAGIC.len(); -const FIELD_OFFSET: usize = VERSION_OFFSET + 4; -const DIGEST_OFFSET: usize = FIELD_OFFSET + 4; -const HEADER_LEN: usize = DIGEST_OFFSET + 32; +const COMPILER_LEN_OFFSET: usize = VERSION_OFFSET + 4; +const COMPILER_OFFSET: usize = COMPILER_LEN_OFFSET + 4; +const FIELD_LEN: usize = 4; +const DIGEST_LEN: usize = 32; #[derive(Debug, thiserror::Error)] pub enum ArtifactError { - #[error("the file does not start with the MAVR1CS header, so it is not a Mavros artifact")] + #[error( + "the file does not start with a well-formed MAVR1CS header, so it is not a Mavros artifact" + )] MissingHeader, - #[error("unsupported artifact format version {0}; expected {ARTIFACT_FORMAT_VERSION}")] - UnsupportedVersion(u32), + #[error( + "unsupported artifact format version {found}, written by {}; this reader expects \ + {ARTIFACT_FORMAT_VERSION}", + writer(.compiler) + )] + UnsupportedVersion { + found: u32, + /// `None` for a format that recorded no compiler version. + compiler: Option, + }, #[error("the artifact names unknown field {0}")] UnknownField(u32), #[error("the artifact is over {found:?} but {expected:?} was requested")] @@ -175,6 +203,8 @@ pub enum ArtifactError { DigestMismatch { expected: String, actual: String }, #[error("{0}")] Invalid(String), + #[error("artifact bytecode: {0}")] + Bytecode(String), } fn body_options() -> impl bincode::Options { @@ -183,14 +213,71 @@ fn body_options() -> impl bincode::Options { .reject_trailing_bytes() } -fn hex(bytes: &[u8]) -> String { +/// Lowercase hex, the form digests are printed and recorded in. +pub fn hex(bytes: &[u8]) -> String { bytes.iter().map(|byte| format!("{byte:02x}")).collect() } +fn writer(compiler: &Option) -> String { + compiler.as_deref().map_or_else( + || "an unknown compiler".to_string(), + |compiler| format!("mavros {compiler}"), + ) +} + +fn u32_at(bytes: &[u8], start: usize) -> Result { + bytes + .get(start..start + 4) + .map(|bytes| u32::from_le_bytes(bytes.try_into().expect("4 bytes"))) + .ok_or(ArtifactError::MissingHeader) +} + +/// The start of every artifact, laid out the same in every format version: enough to name the +/// compiler that wrote the file before, or instead of, decoding the rest. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct ArtifactPreamble { + pub format_version: u32, + /// The release version of the compiler that wrote the artifact; `None` for a format that + /// recorded none. + pub compiler: Option, +} + +impl ArtifactPreamble { + /// Needs only the magic, so it reads an artifact of any format version. + pub fn peek(bytes: &[u8]) -> Result { + if bytes.get(..ARTIFACT_MAGIC.len()) != Some(&ARTIFACT_MAGIC[..]) { + return Err(ArtifactError::MissingHeader); + } + let format_version = u32_at(bytes, VERSION_OFFSET)?; + let compiler = if format_version >= COMPILER_SINCE_VERSION { + let len = u32_at(bytes, COMPILER_LEN_OFFSET)? as usize; + let compiler = COMPILER_OFFSET + .checked_add(len) + .and_then(|end| bytes.get(COMPILER_OFFSET..end)) + .and_then(|compiler| std::str::from_utf8(compiler).ok()) + .ok_or(ArtifactError::MissingHeader)?; + Some(compiler.to_string()) + } else { + None + }; + Ok(Self { + format_version, + compiler, + }) + } + + /// Where the format-specific part of the header begins. + pub(crate) fn encoded_len(&self) -> usize { + match &self.compiler { + Some(compiler) => COMPILER_OFFSET + compiler.len(), + None => COMPILER_LEN_OFFSET, + } + } +} + #[derive(Debug, Deserialize)] #[serde(bound = "F: MavrosField")] struct Body { - compiler: String, manifest: WitnessManifest, r1cs: R1CS, bytecode: Vec, @@ -200,7 +287,6 @@ struct Body { #[derive(Serialize)] #[serde(bound = "F: MavrosField")] struct BodyRef<'a, F: MavrosField> { - compiler: &'a str, manifest: &'a WitnessManifest, r1cs: &'a R1CS, bytecode: &'a [u64], @@ -214,21 +300,38 @@ pub struct Artifact { r1cs: SealedR1CS, bytecode: Vec, abi: Vec, + circuit_digest: [u8; 32], digest: [u8; 32], } +/// The owned contents of an [`Artifact`]; see [`Artifact::into_parts`]. +#[derive(Debug)] +pub struct ArtifactParts { + pub compiler: String, + pub manifest: WitnessManifest, + pub r1cs: SealedR1CS, + pub bytecode: Vec, + pub abi: Vec, +} + impl Artifact { pub fn new( + compiler: String, manifest: WitnessManifest, r1cs: SealedR1CS, bytecode: Vec, abi: Vec, - compiler: String, ) -> Self { let mut hasher = Sha256::new(); - hasher.update(ARTIFACT_DIGEST_DOMAIN); + hasher.update(CIRCUIT_DIGEST_DOMAIN); hasher.update(r1cs.digest()); - bincode::serialize_into(&mut hasher, &(&manifest, &bytecode, &abi)) + bincode::serialize_into(&mut hasher, &manifest) + .expect("hashing cannot fail: the hasher accepts every byte"); + let circuit_digest: [u8; 32] = hasher.finalize().into(); + let mut hasher = Sha256::new(); + hasher.update(ARTIFACT_DIGEST_DOMAIN); + hasher.update(circuit_digest); + bincode::serialize_into(&mut hasher, &(&bytecode, &abi)) .expect("hashing cannot fail: the hasher accepts every byte"); Self { compiler, @@ -236,6 +339,7 @@ impl Artifact { r1cs, bytecode, abi, + circuit_digest, digest: hasher.finalize().into(), } } @@ -244,13 +348,21 @@ impl Artifact { self.r1cs.field() } - /// SHA-256 over [`ARTIFACT_DIGEST_DOMAIN`], the field-bound [`R1CS::digest`] and the - /// bincode-encoded manifest, bytecode and ABI. Pin this digest to identify the whole program; - /// [`SealedR1CS::digest`] identifies only the constraint system. + /// SHA-256 over [`ARTIFACT_DIGEST_DOMAIN`], [`Self::circuit_digest`] and the bincode-encoded + /// bytecode and ABI: the identity of the whole file, which the header carries. Pin it to mean + /// "these exact bytes". pub fn digest(&self) -> [u8; 32] { self.digest } + /// SHA-256 over [`CIRCUIT_DIGEST_DOMAIN`], the field-bound [`R1CS::digest`] and the + /// bincode-encoded manifest: the identity of the constraint system and its witness layout, + /// which a codegen-only change leaves alone. Pin it to mean "the same circuit", as a verifier + /// or a proof transcript should. + pub fn circuit_digest(&self) -> [u8; 32] { + self.circuit_digest + } + pub fn manifest(&self) -> &WitnessManifest { &self.manifest } @@ -271,27 +383,32 @@ impl Artifact { &self.compiler } - pub fn into_parts(self) -> (FieldId, WitnessManifest, SealedR1CS, Vec, Vec) { - ( - self.r1cs.field(), - self.manifest, - self.r1cs, - self.bytecode, - self.abi, - ) + /// For a consumer that must own the R1CS without cloning it. Read [`Self::digest`] or + /// [`Self::circuit_digest`] first; they are not among the parts. + pub fn into_parts(self) -> ArtifactParts { + ArtifactParts { + compiler: self.compiler, + manifest: self.manifest, + r1cs: self.r1cs, + bytecode: self.bytecode, + abi: self.abi, + } } pub fn to_bytes(&self) -> Result, ArtifactError> { + let compiler_len = u32::try_from(self.compiler.len()).map_err(|_| { + ArtifactError::Invalid("the compiler version does not fit in u32 bytes".to_string()) + })?; let mut bytes = Vec::new(); bytes.extend_from_slice(&ARTIFACT_MAGIC); bytes.extend_from_slice(&ARTIFACT_FORMAT_VERSION.to_le_bytes()); + bytes.extend_from_slice(&compiler_len.to_le_bytes()); + bytes.extend_from_slice(self.compiler.as_bytes()); bytes.extend_from_slice(&field_tag(self.field()).to_le_bytes()); - bytes.extend_from_slice(&self.digest()); - debug_assert_eq!(bytes.len(), HEADER_LEN); + bytes.extend_from_slice(&self.digest); body_options().serialize_into( &mut bytes, &BodyRef { - compiler: &self.compiler, manifest: &self.manifest, r1cs: self.r1cs.system(), bytecode: &self.bytecode, @@ -301,19 +418,20 @@ impl Artifact { Ok(bytes) } - /// Reads an artifact, verifying its digest and the manifest against the R1CS. Trailing bytes are - /// an error. + /// Reads an artifact, verifying its digest, then the manifest and the bytecode header against + /// the R1CS. Trailing bytes are an error. pub fn from_bytes(bytes: &[u8]) -> Result { - if bytes.len() < HEADER_LEN || bytes[..ARTIFACT_MAGIC.len()] != ARTIFACT_MAGIC { - return Err(ArtifactError::MissingHeader); - } - let u32_at = - |start: usize| u32::from_le_bytes(bytes[start..start + 4].try_into().expect("4 bytes")); - let version = u32_at(VERSION_OFFSET); - if version != ARTIFACT_FORMAT_VERSION { - return Err(ArtifactError::UnsupportedVersion(version)); + let preamble = ArtifactPreamble::peek(bytes)?; + if preamble.format_version != ARTIFACT_FORMAT_VERSION { + return Err(ArtifactError::UnsupportedVersion { + found: preamble.format_version, + compiler: preamble.compiler, + }); } - let field_code = u32_at(FIELD_OFFSET); + let field_offset = preamble.encoded_len(); + let digest_offset = field_offset + FIELD_LEN; + let body_offset = digest_offset + DIGEST_LEN; + let field_code = u32_at(bytes, field_offset)?; let field = FieldId::from_code(u64::from(field_code)) .ok_or(ArtifactError::UnknownField(field_code))?; if field != F::ID { @@ -322,36 +440,92 @@ impl Artifact { found: field, }); } - let digest: [u8; 32] = bytes[DIGEST_OFFSET..HEADER_LEN] + let digest: [u8; 32] = bytes + .get(digest_offset..body_offset) + .ok_or(ArtifactError::MissingHeader)? .try_into() .expect("32 bytes"); - let body: Body = body_options().deserialize(&bytes[HEADER_LEN..])?; + let body: Body = body_options().deserialize(&bytes[body_offset..])?; + let compiler = preamble + .compiler + .expect("the current format records the compiler version"); let artifact = Self::new( + compiler, body.manifest, SealedR1CS::new(body.r1cs), body.bytecode, body.abi, - body.compiler, ); - let actual = artifact.digest(); - if actual != digest { + if artifact.digest != digest { return Err(ArtifactError::DigestMismatch { expected: hex(&digest), - actual: hex(&actual), + actual: hex(&artifact.digest), }); } artifact .manifest .validate(&artifact.r1cs) .map_err(ArtifactError::Invalid)?; + check_bytecode::(&artifact.bytecode, &artifact.r1cs, &artifact.manifest) + .map_err(ArtifactError::Bytecode)?; Ok(artifact) } } +/// The bytecode header must describe the program the R1CS and manifest describe. +fn check_bytecode( + bytecode: &[u64], + r1cs: &R1CS, + manifest: &WitnessManifest, +) -> Result<(), String> { + let header = ProgramHeader::parse(bytecode)?; + if header.field != F::ID { + return Err(format!( + "the bytecode is over {:?} but the artifact is over {:?}", + header.field, + F::ID + )); + } + if !header.has_layouts(r1cs.witness_layout, r1cs.constraints_layout) { + return Err("the bytecode's constraint layouts disagree with the R1CS".to_string()); + } + let len = bytecode.len(); + if header.code_start > len + || header + .entry_points + .iter() + .any(|&entry| entry != NO_ENTRY_POINT && entry >= len) + { + return Err("the bytecode's header points past its end".to_string()); + } + for (entry, name) in [(ENTRY_WITGEN, "witgen"), (ENTRY_AD, "AD")] { + if header + .entry_points + .get(entry) + .is_none_or(|offset| *offset == NO_ENTRY_POINT) + { + return Err(format!( + "the bytecode has no {name} entry point; a verifier binary is not a prover artifact" + )); + } + } + let blob = manifest.entry_blob_field_count(); + if header.entry_blob_field_count != blob { + return Err(format!( + "the bytecode reads an entry blob of {} fields but the manifest has {blob}", + header.entry_blob_field_count + )); + } + Ok(()) +} + #[cfg(test)] mod tests { use super::*; - use crate::{ConstraintsLayout, Field, R1C, WitnessLayout}; + use crate::{ + ConstraintsLayout, Field, R1C, WitnessLayout, + program_header::{ConstraintLayouts, PROGRAM_FORMAT_VERSION, PROGRAM_MAGIC}, + }; /// `fn main(x: Field, y: pub Field) -> pub Field { x * y }`: columns are /// `[1, y, x, t3, t4, ret]`, with one constraint `y * x = ret`. @@ -401,8 +575,27 @@ mod tests { } } - fn bytecode() -> Vec { - vec![1, 2, 3] + /// A header agreeing with `manifest` and `r1cs`: no constant pool or globals, witgen and AD + /// entries at the one function marker that follows, no code. + fn bytecode(manifest: &WitnessManifest, r1cs: &R1CS) -> Vec { + let mut words = vec![ + PROGRAM_MAGIC, + PROGRAM_FORMAT_VERSION, + FieldId::Bn254.code(), + 1, + ]; + words.extend(ConstraintLayouts::new(r1cs.witness_layout, r1cs.constraints_layout).words()); + let marker = words.len() as u64 + 6; + words.extend([ + 0, + 0, + manifest.entry_blob_field_count() as u64, + 2, + marker, + marker, + ]); + words.push(u64::MAX); + words } fn abi() -> Vec { @@ -411,16 +604,17 @@ mod tests { fn build(manifest: WitnessManifest, r1cs: R1CS, bytecode: Vec, abi: Vec) -> Artifact { Artifact::new( + "test".into(), manifest, SealedR1CS::new(r1cs), bytecode, abi, - "test".into(), ) } fn artifact(manifest: WitnessManifest, r1cs: R1CS) -> Artifact { - build(manifest, r1cs, bytecode(), abi()) + let bytecode = bytecode(&manifest, &r1cs); + build(manifest, r1cs, bytecode, abi()) } fn bytes_of(manifest: WitnessManifest, r1cs: R1CS) -> Vec { @@ -435,6 +629,11 @@ mod tests { read(&bytes_of(manifest, r1cs)) } + fn digest_range(bytes: &[u8]) -> Range { + let start = ArtifactPreamble::peek(bytes).unwrap().encoded_len() + FIELD_LEN; + start..start + DIGEST_LEN + } + #[test] fn artifact_round_trips() { let cases: [fn(&mut WitnessManifest, &mut R1CS); 5] = [ @@ -467,10 +666,11 @@ mod tests { let decoded = read(&bytes).unwrap(); assert_eq!(decoded.field(), FieldId::Bn254); assert_eq!(decoded.r1cs().digest(), r.digest()); - assert_eq!(&bytes[DIGEST_OFFSET..HEADER_LEN], &decoded.digest()); - assert_ne!(decoded.digest(), decoded.r1cs().digest()); + assert_eq!(&bytes[digest_range(&bytes)], &decoded.digest()); + assert_ne!(decoded.digest(), decoded.circuit_digest()); + assert_ne!(decoded.circuit_digest(), decoded.r1cs().digest()); assert_eq!(decoded.manifest(), &m); - assert_eq!(decoded.bytecode(), bytecode()); + assert_eq!(decoded.bytecode(), bytecode(&m, &r)); assert_eq!(decoded.abi(), abi()); assert_eq!(decoded.compiler(), "test"); assert_eq!( @@ -478,6 +678,9 @@ mod tests { r.witness_layout.size() ); assert_eq!(decoded.r1cs().constraints.len(), r.constraints.len()); + let parts = decoded.into_parts(); + assert_eq!(parts.manifest, m); + assert_eq!(parts.compiler, "test"); } } @@ -485,20 +688,49 @@ mod tests { fn artifact_rejects_invalid_headers() { assert!(matches!(read(&[]), Err(ArtifactError::MissingHeader))); let mut bytes = bytes_of(manifest(), r1cs()); - assert!(matches!( - read(&bytes[..HEADER_LEN - 1]), - Err(ArtifactError::MissingHeader) - )); + let body = digest_range(&bytes).end; + for cut in [COMPILER_OFFSET + 1, body - 1] { + assert!( + matches!(read(&bytes[..cut]), Err(ArtifactError::MissingHeader)), + "cut at {cut}" + ); + } for version in [0, ARTIFACT_FORMAT_VERSION - 1, ARTIFACT_FORMAT_VERSION + 1] { - bytes[VERSION_OFFSET..FIELD_OFFSET].copy_from_slice(&version.to_le_bytes()); + bytes[VERSION_OFFSET..COMPILER_LEN_OFFSET].copy_from_slice(&version.to_le_bytes()); + let compiler = (version >= COMPILER_SINCE_VERSION).then(|| "test".to_string()); assert!(matches!( read(&bytes), - Err(ArtifactError::UnsupportedVersion(found)) if found == version + Err(ArtifactError::UnsupportedVersion { found, compiler: found_compiler }) + if found == version && found_compiler == compiler )); + assert_eq!( + ArtifactPreamble::peek(&bytes).unwrap(), + ArtifactPreamble { + format_version: version, + compiler, + } + ); } - bytes[VERSION_OFFSET..FIELD_OFFSET].copy_from_slice(&ARTIFACT_FORMAT_VERSION.to_le_bytes()); - bytes[FIELD_OFFSET..DIGEST_OFFSET].copy_from_slice(&99u32.to_le_bytes()); + bytes[VERSION_OFFSET..COMPILER_LEN_OFFSET] + .copy_from_slice(&ARTIFACT_FORMAT_VERSION.to_le_bytes()); + let field = digest_range(&bytes).start - FIELD_LEN; + bytes[field..field + FIELD_LEN].copy_from_slice(&99u32.to_le_bytes()); assert!(matches!(read(&bytes), Err(ArtifactError::UnknownField(99)))); + bytes[COMPILER_OFFSET] = 0xff; + assert!(matches!(read(&bytes), Err(ArtifactError::MissingHeader))); + + // A format-1 file: the field tag and digest follow the version, and no compiler. + let mut format_1 = ARTIFACT_MAGIC.to_vec(); + format_1.extend_from_slice(&1u32.to_le_bytes()); + format_1.extend_from_slice(&1u32.to_le_bytes()); + format_1.extend_from_slice(&[0xff; 32]); + assert!(matches!( + read(&format_1), + Err(ArtifactError::UnsupportedVersion { + found: 1, + compiler: None, + }) + )); } #[test] @@ -542,38 +774,99 @@ mod tests { for (case, mutate) in cases.into_iter().enumerate() { let (mut m, mut r) = (manifest(), r1cs()); mutate(&mut m, &mut r); + let bytes = build(m, r, bytecode(&manifest(), &r1cs()), abi()) + .to_bytes() + .unwrap(); assert!( - matches!(round_trip(m, r), Err(ArtifactError::Invalid(_))), + matches!(read(&bytes), Err(ArtifactError::Invalid(_))), "overflow case {case}" ); } } - /// The digest is a compatibility promise to consumers that record it: pin its encoding. + /// Words: 0 magic, 1 version, 2 field, 3 layout tag, 4..12 layouts, 12 pool length, + /// 13 globals, 14 entry blob, 15 entry count, 16 witgen entry, 17 AD entry, 18 marker. + #[test] + fn artifact_rejects_bytecode_that_disagrees_with_the_circuit() { + let cases: [(&str, fn(&mut Vec)); 10] = [ + ("empty", |b| b.clear()), + ("no magic", |b| b[0] = 0), + ("another format version", |b| b[1] += 1), + ("another field", |b| b[2] = FieldId::Goldilocks.code()), + ("another algebraic size", |b| b[4] += 1), + ("another entry blob", |b| b[14] += 1), + ("entry past the end", |b| b[16] = 99), + ("no code after the header", |b| b.truncate(18)), + ("no AD entry", |b| b[17] = NO_ENTRY_POINT as u64), + ("witgen entry only", |b| { + b[15] = 1; + b.remove(17); + }), + ]; + for (case, mutate) in cases { + let mut words = bytecode(&manifest(), &r1cs()); + mutate(&mut words); + let bytes = build(manifest(), r1cs(), words, abi()).to_bytes().unwrap(); + assert!( + matches!(read(&bytes), Err(ArtifactError::Bytecode(_))), + "{case}" + ); + } + } + + /// Both digests are compatibility promises to consumers that record them: pin their encoding. #[test] fn digest_encoding_is_pinned() { assert_eq!( hex(&r1cs().digest()), "a1826c2cf819ba7805777209988b33f98071393e14c2671b9ddb3fe9fafe2c52" ); + let pinned = artifact(manifest(), r1cs()); + // The digest the r1cs.bin artifact carried for this circuit. + assert_eq!( + hex(&pinned.circuit_digest()), + "91938a47b4fbf034bad07ba83f8289ca1703176a63fae4867f05c0cd4505de18" + ); assert_eq!( - hex(&artifact(manifest(), r1cs()).digest()), - "ed60cb0a19c5725509c601e780ffb5869f58d6f5d05db0be110f6bf0f4cb4de6" + hex(&pinned.digest()), + "9c577fef5ebe660bad1ad1974cc92d98c4655bcafaad820615679b897ac90b6c" ); - // The compiler version stays outside the digest. + // The compiler version stays outside both digests. let other = Artifact::new( + "other".into(), manifest(), SealedR1CS::new(r1cs()), - bytecode(), + bytecode(&manifest(), &r1cs()), abi(), - "other".into(), ); - assert_eq!(other.digest(), artifact(manifest(), r1cs()).digest()); + assert_eq!(other.circuit_digest(), pinned.circuit_digest()); + assert_eq!(other.digest(), pinned.digest()); assert_eq!( read(&other.to_bytes().unwrap()).unwrap().compiler(), "other" ); + + // Bytecode and ABI move the artifact digest only; the circuit moves both. + let mut words = bytecode(&manifest(), &r1cs()); + *words.last_mut().unwrap() = 7; + for other in [ + build(manifest(), r1cs(), words, abi()), + build( + manifest(), + r1cs(), + bytecode(&manifest(), &r1cs()), + b"{ }".to_vec(), + ), + ] { + assert_eq!(other.circuit_digest(), pinned.circuit_digest()); + assert_ne!(other.digest(), pinned.digest()); + } + let mut renamed = manifest(); + renamed.parameters[0].name = "another_input".into(); + let other = artifact(renamed, r1cs()); + assert_ne!(other.circuit_digest(), pinned.circuit_digest()); + assert_ne!(other.digest(), pinned.digest()); } /// A flipped header byte, another R1CS, bytecode or ABI under the honest digest, or a @@ -588,12 +881,13 @@ mod tests { ); }; let with_honest_digest = |mut forged: Vec| { - forged[DIGEST_OFFSET..HEADER_LEN].copy_from_slice(&honest[DIGEST_OFFSET..HEADER_LEN]); + let range = digest_range(&honest); + forged[range.clone()].copy_from_slice(&honest[range]); forged }; let mut flipped = honest.clone(); - flipped[DIGEST_OFFSET] ^= 1; + flipped[digest_range(&honest).start] ^= 1; rejects(&flipped, "flipped digest byte"); let mut other = r1cs(); @@ -606,12 +900,14 @@ mod tests { let with_parts = |bytecode: Vec, abi: Vec| { build(manifest(), r1cs(), bytecode, abi).to_bytes().unwrap() }; + let mut words = bytecode(&manifest(), &r1cs()); + *words.last_mut().unwrap() = 7; rejects( - &with_honest_digest(with_parts(vec![1, 2, 4], abi())), + &with_honest_digest(with_parts(words, abi())), "changed bytecode word", ); rejects( - &with_honest_digest(with_parts(bytecode(), b"{ }".to_vec())), + &with_honest_digest(with_parts(bytecode(&manifest(), &r1cs()), b"{ }".to_vec())), "changed ABI byte", ); diff --git a/mavros-artifacts/src/program_header.rs b/mavros-artifacts/src/program_header.rs new file mode 100644 index 00000000..b8d10e64 --- /dev/null +++ b/mavros-artifacts/src/program_header.rs @@ -0,0 +1,138 @@ +//! The header every VM program binary opens with. +//! +//! The compiler writes it, the VM reads it before dispatch, and [`Artifact::from_bytes`] checks +//! the bytecode an artifact carries against its R1CS and manifest through it. It lives here, +//! below both, because it is their shared contract and encodes this crate's [`WitnessLayout`] and +//! [`ConstraintsLayout`]. +//! +//! [`Artifact::from_bytes`]: crate::Artifact::from_bytes + +use crate::{ConstraintsLayout, FieldId, WitnessLayout}; + +/// Index of the witness-generation entry point in a program's entry table. +pub const ENTRY_WITGEN: usize = 0; +/// Index of the AD entry point in a program's entry table. +pub const ENTRY_AD: usize = 1; +/// Index of the direct forward verifier entry point. +pub const ENTRY_VERIFIER: usize = 2; +/// An absent entry in a program or serialized entry table. +pub const NO_ENTRY_POINT: usize = usize::MAX; + +/// The first word of every program binary: `MAVROSVM` as big-endian ASCII. +pub const PROGRAM_MAGIC: u64 = u64::from_be_bytes(*b"MAVROSVM"); + +/// The program binary's format version, its second word. Changing existing production opcode +/// numbers or operands, the `BoxedLayout` encoding, or the header layout must bump this. +/// Appending opcodes without changing existing production encodings does not require a bump. +/// The third word names the program's field ([`FieldId::code`]). +pub const PROGRAM_FORMAT_VERSION: u64 = 4; + +/// Buffer layouts bound to the compiled constraint traversal. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct ConstraintLayouts([u64; 8]); + +impl ConstraintLayouts { + pub fn new(witness: WitnessLayout, constraints: ConstraintsLayout) -> Self { + Self([ + witness.algebraic_size as u64, + witness.multiplicities_size as u64, + witness.challenges_size as u64, + witness.tables_data_size as u64, + witness.lookups_data_size as u64, + constraints.algebraic_size as u64, + constraints.tables_data_size as u64, + constraints.lookups_data_size as u64, + ]) + } + + /// The eight header words, in the order [`Self::new`] reads its fields. + pub fn words(self) -> [u64; 8] { + self.0 + } +} + +/// The decoded header of a program binary. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct ProgramHeader { + /// The field the program computes over. + pub field: FieldId, + /// Layouts emitted by the compiler, checked before constraint execution. + pub constraint_layouts: Option, + /// Interned constant pool (flat words), read at runtime by `mov_const_pool`. + pub constant_pool: Vec, + pub global_frame_size: usize, + /// The exact number of input fields the caller must supply. + pub entry_blob_field_count: usize, + /// Absolute word offsets of each entry point's function marker, in entry-table order + /// ([`ENTRY_WITGEN`], [`ENTRY_AD`], ...). The entry's frame size lives at `offset + 1` and + /// its first opcode at `offset + 2`. [`NO_ENTRY_POINT`] marks an absent entry. + pub entry_points: Vec, + /// Word offset of the first function marker, i.e. where the opcode stream begins. + pub code_start: usize, +} + +impl ProgramHeader { + /// Whether the program was compiled for exactly these layouts. + pub fn has_layouts(&self, witness: WitnessLayout, constraints: ConstraintsLayout) -> bool { + self.constraint_layouts == Some(ConstraintLayouts::new(witness, constraints)) + } + + /// Decodes a program binary's field, layouts, constant pool, global frame size, entry blob + /// field count and entry-point table. + /// + /// # Errors + /// + /// If the binary does not open with [`PROGRAM_MAGIC`] followed by [`PROGRAM_FORMAT_VERSION`]: + /// a binary of any other format numbers its opcodes differently, so they would select the + /// wrong handlers. If its field word names no supported field, or it ends inside the header. + pub fn parse(program: &[u64]) -> Result { + if program.first() != Some(&PROGRAM_MAGIC) { + return Err("not a mavros VM program binary".to_string()); + } + let version = program.get(1).copied(); + if version != Some(PROGRAM_FORMAT_VERSION) { + return Err(format!( + "unsupported VM program format version {version:?}; expected {PROGRAM_FORMAT_VERSION}" + )); + } + let words = |start: usize, len: usize| { + start + .checked_add(len) + .and_then(|end| program.get(start..end)) + .ok_or_else(|| "the VM program ends inside its header".to_string()) + }; + let word = |index: usize| words(index, 1).map(|words| words[0]); + + let field_word = word(2)?; + let field = FieldId::from_code(field_word).ok_or_else(|| { + format!("the VM program names no supported field (word {field_word})") + })?; + let (constraint_layouts, off) = match word(3)? { + 0 => (None, 4), + 1 => { + let layouts = words(4, 8)?.try_into().expect("8 words"); + (Some(ConstraintLayouts(layouts)), 12) + } + tag => return Err(format!("invalid constraint layout tag {tag}")), + }; + let pool_len = word(off)? as usize; + let constant_pool = words(off + 1, pool_len)?.to_vec(); + let off = off + 1 + pool_len; + let global_frame_size = word(off)? as usize; + let entry_blob_field_count = word(off + 1)? as usize; + let num_entries = word(off + 2)? as usize; + let entry_points = words(off + 3, num_entries)? + .iter() + .map(|&offset| offset as usize) + .collect(); + Ok(ProgramHeader { + field, + constraint_layouts, + constant_pool, + global_frame_size, + entry_blob_field_count, + entry_points, + code_start: off + 3 + num_entries, + }) + } +} diff --git a/vm/src/bytecode.rs b/vm/src/bytecode.rs index 59d75afc..35eda232 100644 --- a/vm/src/bytecode.rs +++ b/vm/src/bytecode.rs @@ -3865,33 +3865,10 @@ impl Display for Function { } } -/// Index of the witness-generation entry point in a program's entry table. -pub const ENTRY_WITGEN: usize = 0; -/// Index of the AD entry point in a program's entry table. -pub const ENTRY_AD: usize = 1; -/// Index of the direct forward verifier entry point. -pub const ENTRY_VERIFIER: usize = 2; -/// An absent entry in a program or serialized entry table. -pub const NO_ENTRY_POINT: usize = usize::MAX; - -/// Buffer layouts bound to the compiled constraint traversal. -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub struct ConstraintLayouts([u64; 8]); - -impl ConstraintLayouts { - pub fn new(witness: WitnessLayout, constraints: ConstraintsLayout) -> Self { - Self([ - witness.algebraic_size as u64, - witness.multiplicities_size as u64, - witness.challenges_size as u64, - witness.tables_data_size as u64, - witness.lookups_data_size as u64, - constraints.algebraic_size as u64, - constraints.tables_data_size as u64, - constraints.lookups_data_size as u64, - ]) - } -} +pub use mavros_artifacts::program_header::{ + ConstraintLayouts, ENTRY_AD, ENTRY_VERIFIER, ENTRY_WITGEN, NO_ENTRY_POINT, + PROGRAM_FORMAT_VERSION, PROGRAM_MAGIC, ProgramHeader, +}; pub struct Program { /// Required for AD/verifier execution; absent for programs built only for inspection or witgen. @@ -3934,15 +3911,6 @@ impl Display for Program { } } -/// The first word of every program binary: `MAVROSVM` as big-endian ASCII. -pub const PROGRAM_MAGIC: u64 = u64::from_be_bytes(*b"MAVROSVM"); - -/// The program binary's format version, its second word. Changing existing production opcode -/// numbers or operands, the `BoxedLayout` encoding, or the header layout must bump this. -/// Appending opcodes without changing existing production encodings does not require a bump. -/// The third word names the program's field ([`FieldId::code`]). -pub const PROGRAM_FORMAT_VERSION: u64 = 4; - impl Program { /// Serialize executable VM bytecode. Debug information is never embedded in this output. pub fn to_binary(&self) -> Vec { @@ -3967,7 +3935,7 @@ impl Program { let mut binary = vec![PROGRAM_MAGIC, PROGRAM_FORMAT_VERSION, self.field.code()]; binary.push(u64::from(self.constraint_layouts.is_some())); if let Some(layouts) = self.constraint_layouts { - binary.extend_from_slice(&layouts.0); + binary.extend_from_slice(&layouts.words()); } // Constant pool: [pool_len, ...words...]. @@ -4058,76 +4026,15 @@ impl Program { } } -/// The decoded header of a program binary. -pub struct ProgramHeader { - /// The field the program computes over. - pub field: FieldId, - /// Layouts emitted by the compiler, checked before constraint execution. - pub constraint_layouts: Option, - /// Interned constant pool (flat words), read at runtime by `mov_const_pool`. - pub constant_pool: Vec, - pub global_frame_size: usize, - /// The exact number of input fields the caller must supply. - pub entry_blob_field_count: usize, - /// Absolute word offsets of each entry point's function marker, in entry-table order - /// ([`ENTRY_WITGEN`], [`ENTRY_AD`], ...). The entry's frame size lives at `offset + 1` and - /// its first opcode at `offset + 2`. [`NO_ENTRY_POINT`] marks an absent entry. - pub entry_points: Vec, - /// Word offset of the first function marker, i.e. where the opcode stream begins. - pub code_start: usize, -} - -/// Decode a program binary's field, layouts, constant pool, global frame size, entry blob -/// field count and entry-point table. +/// Decode a program binary's header; see [`ProgramHeader::parse`]. /// /// # Panics /// /// If the binary does not open with [`PROGRAM_MAGIC`] followed by [`PROGRAM_FORMAT_VERSION`]: a /// binary of any other format numbers its opcodes differently, so they would select the wrong -/// handlers. If its field word names no supported field. +/// handlers. If its field word names no supported field, or it ends inside the header. pub fn parse_program_header(program: &[u64]) -> ProgramHeader { - assert!( - program.first() == Some(&PROGRAM_MAGIC), - "not a mavros VM program binary" - ); - let version = program.get(1).copied(); - assert!( - version == Some(PROGRAM_FORMAT_VERSION), - "unsupported VM program format version {version:?}; expected {PROGRAM_FORMAT_VERSION}" - ); - let field = FieldId::from_code(program[2]).unwrap_or_else(|| { - panic!( - "the VM program names no supported field (word {})", - program[2] - ) - }); - let (constraint_layouts, off) = match program[3] { - 0 => (None, 4), - 1 => ( - Some(ConstraintLayouts(program[4..12].try_into().unwrap())), - 12, - ), - _ => panic!("invalid constraint layout tag"), - }; - let pool_len = program[off] as usize; - let constant_pool = program[off + 1..off + 1 + pool_len].to_vec(); - let off = off + 1 + pool_len; - let global_frame_size = program[off] as usize; - let entry_blob_field_count = program[off + 1] as usize; - let num_entries = program[off + 2] as usize; - let entry_points: Vec = (0..num_entries) - .map(|i| program[off + 3 + i] as usize) - .collect(); - let code_start = off + 3 + num_entries; - ProgramHeader { - field, - constraint_layouts, - constant_pool, - global_frame_size, - entry_blob_field_count, - entry_points, - code_start, - } + ProgramHeader::parse(program).unwrap_or_else(|error| panic!("{error}")) } /// Every width the `_int128` lane holds that is worth sweeping, narrowest first. diff --git a/vm/src/interpreter.rs b/vm/src/interpreter.rs index f197075a..c5859442 100644 --- a/vm/src/interpreter.rs +++ b/vm/src/interpreter.rs @@ -1119,12 +1119,7 @@ fn run_constraint_program( }.to_string()), stack_trace: Vec::new(), })?; - if header.constraint_layouts - != Some(bytecode::ConstraintLayouts::new( - witness_layout, - constraints_layout, - )) - { + if !header.has_layouts(witness_layout, constraints_layout) { return Err(TrapError { message: Some("constraint layouts do not match bytecode".to_string()), stack_trace: Vec::new(), From 8d10b2625692bbfc5a4f4ffef3373b6dfdc181d9 Mon Sep 17 00:00:00 2001 From: Yogesh Shahi Date: Thu, 5 Nov 2026 05:06:50 +0530 Subject: [PATCH 8/8] pr review fixes --- README.md | 42 ++-- compiler/src/driver.rs | 4 +- mavros-artifacts/src/lib.rs | 4 +- mavros-artifacts/src/manifest.rs | 271 +++++++++++++++++-------- mavros-artifacts/src/program_header.rs | 38 +++- vm/src/bytecode.rs | 32 ++- vm/src/interpreter.rs | 2 +- 7 files changed, 267 insertions(+), 126 deletions(-) diff --git a/README.md b/README.md index 0d378c1e..e90fe202 100644 --- a/README.md +++ b/README.md @@ -26,13 +26,16 @@ information on how to use Mavros. To use Mavros, navigate to the **root of a Noir project** (the directory containing `Nargo.toml`) in your shell. Then run `mavros compile`. -This writes one file, `target/program.mavros`. Its `MAVR1CS` header holds the format version, the -compiler's release version (the workspace `version` in `Cargo.toml`), the field tag and the artifact -digest; the body holds the witness manifest, the R1CS, the VM bytecode with the witgen and AD entry -points, and the ABI as JSON. Two digests identify it: the circuit digest hashes the R1CS (which -hashes its field tag) and the manifest; the artifact digest hashes the circuit digest, the bytecode -and the ABI, and is checked whenever the file is read. The header itself is checked, not hashed, and -the compiler version is informational. +This writes one file, `target/program.mavros`: + +| Part | Contents | +| ------ | ----------------------------------------------------------------------------- | +| Header | `MAVR1CS` magic, format version, compiler version, field tag, artifact digest | +| Body | Witness manifest, R1CS, VM bytecode (witgen and AD entry points), ABI as JSON | + +The circuit digest covers the R1CS, which hashes its field tag, and the manifest. The artifact +digest covers the circuit digest, the bytecode and the ABI, and is checked whenever the file is +read. The magic, format version and compiler version are not hashed. Pass `--include-debug-info` to additionally write `target/program.debug.json`. This standalone, versioned source map records VM bytecode word offsets, function names, and Noir source locations; @@ -72,22 +75,27 @@ mavros compile ./my-noir-project --verifier-output target/verifier.json ``` `verifier.json` contains the verifier bytecode, `witness_layout`, `constraints_layout` and the -`circuit_digest` of the `program.mavros` it was compiled from; it contains no matrix rows or prover +`circuit_digest` of the `program.mavros` written alongside it; it contains no matrix rows or prover entry points. `--include-debug-info` also writes `verifier.debug.json`. Default compilation emits no verifier code. `--verify` compiles and runs only the verifier artifact. Rust callers compile it with `mavros_compiler::api::compile_verifier_bytecode(&mut driver, options)` and pass that binary to `run_verifier_from_binary(&binary, &r, &w, witness_layout, constraints_layout, None)`. No R1CS -matrices are required at runtime. Compare the verifier's `circuit_digest` with the artifact's before -trusting the pair. Verifier execution uses a dedicated forward bytecode entry point: witness writes -read the next value from `w`, linear combinations use field arithmetic, and constraints accumulate -three scalars weighted by `r`. It allocates no AD expression nodes or matrix output vectors. Lookup -and table constraints are evaluated from the same layouts. The bytecode header names the field and -the constraint layouts of the artifact it belongs to; recompile older artifacts before using the -current VM. AD and verifier execution reject mismatched layouts. LLVM and WASM artifacts still -expose the existing witgen and AD entry points; the forward verifier is currently available through -the VM API and CLI. +matrices are required at runtime. + +Both artifacts must come from the same circuit compilation; applications must associate the verifier +artifact with their trusted circuit configuration. Comparing the two `circuit_digest`s catches a +verifier from another circuit, not a forged one: nothing hashes the verifier bytecode. + +The verifier runs forward through its own bytecode entry point, allocating no AD nodes or matrix +outputs: witness writes read the next value from `w`, and every constraint, lookups and tables +included, adds to three scalars weighted by `r`. + +The bytecode header names its artifact's field and constraint layouts, and AD and verifier execution +reject mismatched layouts. Recompile older artifacts before using the current VM. LLVM and WASM +artifacts expose only the witgen and AD entry points; the forward verifier runs through the VM API +and CLI. ### Per-Function FlameGraphs diff --git a/compiler/src/driver.rs b/compiler/src/driver.rs index a33b35f7..14cd4b55 100644 --- a/compiler/src/driver.rs +++ b/compiler/src/driver.rs @@ -1092,8 +1092,8 @@ impl Driver { Ok(manifest) } - /// Packs `r1cs` and `bytecode` with this driver's witness manifest and ABI; requires what - /// [`Self::witness_manifest`] does. + /// Packs `r1cs` and `bytecode` with this driver's manifest and ABI; needs what + /// [`Self::witness_manifest`] needs. pub fn artifact( &self, r1cs: SealedR1CS, diff --git a/mavros-artifacts/src/lib.rs b/mavros-artifacts/src/lib.rs index b1e9eee9..6b14e636 100644 --- a/mavros-artifacts/src/lib.rs +++ b/mavros-artifacts/src/lib.rs @@ -742,8 +742,8 @@ impl SealedR1CS { F::ID } - /// Use [`Artifact::circuit_digest`] to also bind the witness manifest, and [`Artifact::digest`] - /// for the bytecode and ABI too. + /// Covers the R1CS only: [`Artifact::circuit_digest`] adds the manifest, and + /// [`Artifact::digest`] the bytecode and ABI. pub fn digest(&self) -> [u8; 32] { *self.digest.get_or_init(|| self.system.digest()) } diff --git a/mavros-artifacts/src/manifest.rs b/mavros-artifacts/src/manifest.rs index 6ddb1b88..1dc14c70 100644 --- a/mavros-artifacts/src/manifest.rs +++ b/mavros-artifacts/src/manifest.rs @@ -5,16 +5,18 @@ //! [ field: u32 LE ][ artifact digest: 32 bytes ][ bincode { manifest, r1cs, bytecode, abi } ] //! ``` //! -//! The magic, format version and compiler version form the [`ArtifactPreamble`], laid out the same -//! in every format version so that a reader can name the compiler that wrote a file it cannot -//! otherwise open. Everything after it belongs to [`ARTIFACT_FORMAT_VERSION`]; bump that when -//! changing the header, the serialized body or the layouts. +//! A reader must learn a file's format before it can decode the file, and should still be able to +//! name the compiler of a file it cannot decode. So a file opens with the [`ArtifactPreamble`]: the +//! magic, the format version and the compiler version. In formats older than the compiler field, +//! the preamble ends at the version. //! -//! Two digests identify an artifact. [`Artifact::circuit_digest`] binds the field, the R1CS and the -//! witness manifest: the constraint system a proof is about. [`Artifact::digest`] binds the circuit -//! digest, the VM bytecode and the ABI: the whole file. The header carries the latter and reading -//! verifies it; the header itself is checked, not hashed, and the compiler version is informational. -//! Reading also checks the manifest and the bytecode header against the R1CS and refuses bytes after +//! The format version covers how everything after the preamble is encoded: bump +//! [`ARTIFACT_FORMAT_VERSION`] when that changes. The bytecode's words are the exception, with +//! their own [`PROGRAM_FORMAT_VERSION`]. The format version cannot cover the preamble that holds +//! it, so changing the preamble takes a new magic. +//! +//! Reading checks that a file is intact and consistent: [`Artifact::digest`] must match, the +//! manifest must agree with the R1CS and the bytecode's header with both, and no bytes may follow //! the body. //! //! The manifest records only what the R1CS cannot say: each parameter's name, visibility, width and @@ -28,6 +30,8 @@ //! write order instead: column 0 is the constant one, then the public parameters in ABI order, //! then the private parameters that survived dead-code elimination in ABI order, and the computed //! return after `main`'s internal columns. +//! +//! [`PROGRAM_FORMAT_VERSION`]: crate::program_header::PROGRAM_FORMAT_VERSION use std::ops::Range; @@ -162,13 +166,11 @@ impl WitnessManifest { pub const ARTIFACT_MAGIC: [u8; 8] = *b"MAVR1CS\0"; /// Format version this crate writes and the only one it reads. pub const ARTIFACT_FORMAT_VERSION: u32 = 3; -/// The first format whose header records the compiler version; the two `r1cs.bin` formats -/// recorded none. +/// The first format version with a compiler field. const COMPILER_SINCE_VERSION: u32 = 3; -/// Domain separator of [`Artifact::circuit_digest`]. Its bytes and preimage are those of the -/// `r1cs.bin` digest this format replaced, so a circuit keeps the digest it had there. Bump when -/// the preimage encoding changes. +/// Domain separator of [`Artifact::circuit_digest`], the one `r1cs.bin`'s digest used, so that a +/// circuit keeps its digest. Bump when its preimage encoding changes. pub const CIRCUIT_DIGEST_DOMAIN: &[u8] = b"mavros-r1cs-artifact-v1\0"; /// Domain separator of [`Artifact::digest`]. Bump when its preimage encoding changes. @@ -182,9 +184,7 @@ const DIGEST_LEN: usize = 32; #[derive(Debug, thiserror::Error)] pub enum ArtifactError { - #[error( - "the file does not start with a well-formed MAVR1CS header, so it is not a Mavros artifact" - )] + #[error("the file does not start with a well-formed MAVR1CS header")] MissingHeader, #[error( "unsupported artifact format version {found}, written by {}; this reader expects \ @@ -235,8 +235,7 @@ fn u32_at(bytes: &[u8], start: usize) -> Result { .ok_or(ArtifactError::MissingHeader) } -/// The start of every artifact, laid out the same in every format version: enough to name the -/// compiler that wrote the file before, or instead of, decoding the rest. +/// The start of every artifact. Later format versions must keep this layout. #[derive(Clone, Debug, PartialEq, Eq)] pub struct ArtifactPreamble { pub format_version: u32, @@ -246,7 +245,7 @@ pub struct ArtifactPreamble { } impl ArtifactPreamble { - /// Needs only the magic, so it reads an artifact of any format version. + /// Reads the preamble of any format version, including ones [`Artifact::from_bytes`] refuses. pub fn peek(bytes: &[u8]) -> Result { if bytes.get(..ARTIFACT_MAGIC.len()) != Some(&ARTIFACT_MAGIC[..]) { return Err(ArtifactError::MissingHeader); @@ -268,14 +267,6 @@ impl ArtifactPreamble { compiler, }) } - - /// Where the format-specific part of the header begins. - pub(crate) fn encoded_len(&self) -> usize { - match &self.compiler { - Some(compiler) => COMPILER_OFFSET + compiler.len(), - None => COMPILER_LEN_OFFSET, - } - } } #[derive(Debug, Deserialize)] @@ -351,17 +342,16 @@ impl Artifact { self.r1cs.field() } - /// SHA-256 over [`ARTIFACT_DIGEST_DOMAIN`], [`Self::circuit_digest`] and the bincode-encoded - /// bytecode and ABI: the identity of the whole file, which the header carries. Pin it to mean - /// "these exact bytes". + /// SHA-256 over [`ARTIFACT_DIGEST_DOMAIN`], [`Self::circuit_digest`], the bytecode and the ABI. + /// The magic, format version and compiler version are not hashed, so two files that differ + /// only in those share a digest. Reading checks it to catch corruption; a forger can recompute + /// it. pub fn digest(&self) -> [u8; 32] { self.digest } - /// SHA-256 over [`CIRCUIT_DIGEST_DOMAIN`], the field-bound [`R1CS::digest`] and the - /// bincode-encoded manifest: the identity of the constraint system and its witness layout, - /// which a codegen-only change leaves alone. Pin it to mean "the same circuit", as a verifier - /// or a proof transcript should. + /// SHA-256 over [`CIRCUIT_DIGEST_DOMAIN`], [`R1CS::digest`] (which covers the field) and the + /// manifest. A change to only the bytecode or the ABI does not move it. pub fn circuit_digest(&self) -> [u8; 32] { self.circuit_digest } @@ -386,8 +376,7 @@ impl Artifact { &self.compiler } - /// For a consumer that must own the R1CS without cloning it. Read [`Self::digest`] or - /// [`Self::circuit_digest`] first; they are not among the parts. + /// Moves the contents out. The digests are left behind, so read them first. pub fn into_parts(self) -> ArtifactParts { ArtifactParts { compiler: self.compiler, @@ -421,18 +410,20 @@ impl Artifact { Ok(bytes) } - /// Reads an artifact, verifying its digest, then the manifest and the LogUp layout - /// ([`LogupStrides::try_new`]) against the R1CS, and the bytecode header against both. - /// Trailing bytes are an error. + /// Reads an artifact, checking its digest, the manifest and LogUp layout against the R1CS, and + /// the bytecode's header against both. Trailing bytes are an error. pub fn from_bytes(bytes: &[u8]) -> Result { - let preamble = ArtifactPreamble::peek(bytes)?; - if preamble.format_version != ARTIFACT_FORMAT_VERSION { - return Err(ArtifactError::UnsupportedVersion { - found: preamble.format_version, - compiler: preamble.compiler, - }); - } - let field_offset = preamble.encoded_len(); + let compiler = match ArtifactPreamble::peek(bytes)? { + ArtifactPreamble { + format_version: ARTIFACT_FORMAT_VERSION, + compiler: Some(compiler), + } => compiler, + ArtifactPreamble { + format_version: found, + compiler, + } => return Err(ArtifactError::UnsupportedVersion { found, compiler }), + }; + let field_offset = COMPILER_OFFSET + compiler.len(); let digest_offset = field_offset + FIELD_LEN; let body_offset = digest_offset + DIGEST_LEN; let field_code = u32_at(bytes, field_offset)?; @@ -450,9 +441,6 @@ impl Artifact { .try_into() .expect("32 bytes"); let body: Body = body_options().deserialize(&bytes[body_offset..])?; - let compiler = preamble - .compiler - .expect("the current format records the compiler version"); let artifact = Self::new( compiler, body.manifest, @@ -483,7 +471,7 @@ impl Artifact { } } -/// The bytecode header must describe the program the R1CS and manifest describe. +/// Checks the bytecode's header against the R1CS and manifest. The opcode stream is not validated. fn check_bytecode( bytecode: &[u64], r1cs: &R1CS, @@ -500,15 +488,6 @@ fn check_bytecode( if !header.has_layouts(r1cs.witness_layout, r1cs.constraints_layout) { return Err("the bytecode's constraint layouts disagree with the R1CS".to_string()); } - let len = bytecode.len(); - if header.code_start > len - || header - .entry_points - .iter() - .any(|&entry| entry != NO_ENTRY_POINT && entry >= len) - { - return Err("the bytecode's header points past its end".to_string()); - } for (entry, name) in [(ENTRY_WITGEN, "witgen"), (ENTRY_AD, "AD")] { if header .entry_points @@ -534,8 +513,10 @@ fn check_bytecode( mod tests { use super::*; use crate::{ - ConstraintsLayout, Field, MAX_LOGUP_REPETITIONS, R1C, WitnessLayout, - program_header::{ConstraintLayouts, PROGRAM_FORMAT_VERSION, PROGRAM_MAGIC}, + ConstraintsLayout, Field, Goldilocks, MAX_LOGUP_REPETITIONS, R1C, WitnessLayout, + program_header::{ + ConstraintLayouts, FUNCTION_MARKER, PROGRAM_FORMAT_VERSION, PROGRAM_MAGIC, + }, }; /// `fn main(x: Field, y: pub Field) -> pub Field { x * y }`: columns are @@ -588,7 +569,7 @@ mod tests { } /// A header agreeing with `manifest` and `r1cs`: no constant pool or globals, witgen and AD - /// entries at the one function marker that follows, no code. + /// entries at the one function that follows, which is a marker, a frame size and one opcode. fn bytecode(manifest: &WitnessManifest, r1cs: &R1CS) -> Vec { let mut words = vec![ PROGRAM_MAGIC, @@ -606,7 +587,7 @@ mod tests { marker, marker, ]); - words.push(u64::MAX); + words.extend([FUNCTION_MARKER, 0, 0]); words } @@ -642,7 +623,8 @@ mod tests { } fn digest_range(bytes: &[u8]) -> Range { - let start = ArtifactPreamble::peek(bytes).unwrap().encoded_len() + FIELD_LEN; + let compiler = ArtifactPreamble::peek(bytes).unwrap().compiler.unwrap(); + let start = COMPILER_OFFSET + compiler.len() + FIELD_LEN; start..start + DIGEST_LEN } @@ -700,6 +682,13 @@ mod tests { fn artifact_rejects_invalid_headers() { assert!(matches!(read(&[]), Err(ArtifactError::MissingHeader))); let mut bytes = bytes_of(manifest(), r1cs()); + assert!(matches!( + Artifact::::from_bytes(&bytes), + Err(ArtifactError::WrongField { + expected: FieldId::Goldilocks, + found: FieldId::Bn254, + }) + )); let body = digest_range(&bytes).end; for cut in [COMPILER_OFFSET + 1, body - 1] { assert!( @@ -745,6 +734,29 @@ mod tests { )); } + /// A new format version must not change the preamble. This spells it in literal bytes at the + /// highest version, so a version bump does not need to touch it. + #[test] + fn preamble_layout_is_stable() { + let mut bytes = b"MAVR1CS\0".to_vec(); + bytes.extend_from_slice(&[0xff; 4]); + bytes.extend_from_slice(&[3, 0, 0, 0]); + bytes.extend_from_slice(b"9.9"); + bytes.extend_from_slice(b"whatever a later format puts here"); + assert_eq!( + ArtifactPreamble::peek(&bytes).unwrap(), + ArtifactPreamble { + format_version: u32::MAX, + compiler: Some("9.9".to_string()), + } + ); + assert!(matches!( + read(&bytes), + Err(ArtifactError::UnsupportedVersion { found: u32::MAX, compiler: Some(compiler) }) + if compiler == "9.9" + )); + } + #[test] fn trailing_bytes_are_rejected() { let mut padded = bytes_of(manifest(), r1cs()); @@ -855,33 +867,116 @@ mod tests { } } - /// Words: 0 magic, 1 version, 2 field, 3 layout tag, 4..12 layouts, 12 pool length, - /// 13 globals, 14 entry blob, 15 entry count, 16 witgen entry, 17 AD entry, 18 marker. + /// Words: 0 magic, 1 version, 2 field, 3 layout tag, 4..13 layouts, 13 pool length, + /// 14 globals, 15 entry blob, 16 entry count, 17 witgen entry, 18 AD entry, 19 marker, + /// 20 frame size, 21 opcode. #[test] fn artifact_rejects_bytecode_that_disagrees_with_the_circuit() { - let cases: [(&str, fn(&mut Vec)); 10] = [ - ("empty", |b| b.clear()), - ("no magic", |b| b[0] = 0), - ("another format version", |b| b[1] += 1), - ("another field", |b| b[2] = FieldId::Goldilocks.code()), - ("another algebraic size", |b| b[4] += 1), - ("another entry blob", |b| b[15] += 1), - ("entry past the end", |b| b[17] = 99), - ("no code after the header", |b| b.truncate(19)), - ("no AD entry", |b| b[18] = NO_ENTRY_POINT as u64), - ("witgen entry only", |b| { - b[16] = 1; - b.remove(17); - }), + let cases: [(&str, fn(&mut Vec), &str); 22] = [ + ("empty", |b| b.clear(), "not a mavros VM program binary"), + ("no magic", |b| b[0] = 0, "not a mavros VM program binary"), + ( + "another format version", + |b| b[1] += 1, + "unsupported VM program format version", + ), + ("unknown field", |b| b[2] = 99, "names no supported field"), + ( + "another field", + |b| b[2] = FieldId::Goldilocks.code(), + "the bytecode is over", + ), + ( + "invalid layout tag", + |b| b[3] = 2, + "invalid constraint layout tag", + ), + ("no layouts", |b| b[3] = 0, "constraint layouts disagree"), + ( + "another algebraic size", + |b| b[4] += 1, + "constraint layouts disagree", + ), + ("another entry blob", |b| b[15] += 1, "entry blob of"), + ( + "ends inside the header", + |b| b.truncate(3), + "ends inside its header", + ), + ( + "pool past the end", + |b| b[13] = u64::MAX, + "ends inside its header", + ), + ( + "entry table past the end", + |b| b[16] = u64::MAX, + "ends inside its header", + ), + ( + "entry past the end", + |b| b[17] = 99, + "lies outside its code", + ), + ( + "entry at the top of the address space", + |b| b[17] = u64::MAX - 1, + "lies outside its code", + ), + ( + "entry inside the header", + |b| b[17] = 5, + "lies outside its code", + ), + ( + "entry one word before the code", + |b| b[17] = 18, + "lies outside its code", + ), + ( + "no code after the header", + |b| b.truncate(19), + "lies outside its code", + ), + ( + "entry without an opcode", + |b| b.truncate(21), + "lies outside its code", + ), + ( + "entry not at a marker", + |b| b[19] = 0, + "is not a function marker", + ), + ( + "no witgen entry", + |b| b[17] = NO_ENTRY_POINT as u64, + "no witgen entry point", + ), + ( + "no AD entry", + |b| b[18] = NO_ENTRY_POINT as u64, + "no AD entry point", + ), + ( + "witgen entry only", + |b| { + b[16] = 1; + b.remove(17); + // The marker moved down with the removed slot. + b[17] = 18; + }, + "no AD entry point", + ), ]; - for (case, mutate) in cases { + for (case, mutate, expected) in cases { let mut words = bytecode(&manifest(), &r1cs()); mutate(&mut words); let bytes = build(manifest(), r1cs(), words, abi()).to_bytes().unwrap(); - assert!( - matches!(read(&bytes), Err(ArtifactError::Bytecode(_))), - "{case}" - ); + match read(&bytes) { + Err(ArtifactError::Bytecode(message)) if message.contains(expected) => {} + other => panic!("{case}: expected {expected:?}, got {:?}", other.err()), + } } } @@ -900,7 +995,7 @@ mod tests { ); assert_eq!( hex(&pinned.digest()), - "b9c22073231d955fba155a857fb6f5cbf099f28df986f8c2065b1e2fc63ac0e5" + "102d64fce93fcfd06a24434db0568142ea92ec39e04078c99cc47e6274b08545" ); // The compiler version stays outside both digests. diff --git a/mavros-artifacts/src/program_header.rs b/mavros-artifacts/src/program_header.rs index 804103a2..d91305a8 100644 --- a/mavros-artifacts/src/program_header.rs +++ b/mavros-artifacts/src/program_header.rs @@ -1,9 +1,6 @@ -//! The header every VM program binary opens with. -//! -//! The compiler writes it, the VM reads it before dispatch, and [`Artifact::from_bytes`] checks -//! the bytecode an artifact carries against its R1CS and manifest through it. It lives here, -//! below both, because it is their shared contract and encodes this crate's [`WitnessLayout`] and -//! [`ConstraintsLayout`]. +//! The header every VM program binary opens with. The compiler writes it, the VM reads it, and +//! [`Artifact::from_bytes`] checks it against the artifact's R1CS and manifest. The opcode stream +//! after it is not validated. //! //! [`Artifact::from_bytes`]: crate::Artifact::from_bytes @@ -17,12 +14,15 @@ pub const ENTRY_AD: usize = 1; pub const ENTRY_VERIFIER: usize = 2; /// An absent entry in a program or serialized entry table. pub const NO_ENTRY_POINT: usize = usize::MAX; +/// The word that opens every function, followed by its frame size and then its opcodes. +pub const FUNCTION_MARKER: u64 = u64::MAX; /// The first word of every program binary: `MAVROSVM` as big-endian ASCII. pub const PROGRAM_MAGIC: u64 = u64::from_be_bytes(*b"MAVROSVM"); /// The program binary's format version, its second word. Changing existing production opcode -/// numbers or operands, the `BoxedLayout` encoding, or the header layout must bump this. +/// numbers or operands, the `BoxedLayout` encoding, the function marker, or the header layout must +/// bump this. /// Appending opcodes without changing existing production encodings does not require a bump. /// The third word names the program's field ([`FieldId::code`]). pub const PROGRAM_FORMAT_VERSION: u64 = 5; @@ -85,7 +85,8 @@ impl ProgramHeader { /// /// If the binary does not open with [`PROGRAM_MAGIC`] followed by [`PROGRAM_FORMAT_VERSION`]: /// a binary of any other format numbers its opcodes differently, so they would select the - /// wrong handlers. If its field word names no supported field, or it ends inside the header. + /// wrong handlers. If its field word names no supported field, it ends inside the header, or + /// an entry point is not on a function marker with room for a frame size and an opcode. pub fn parse(program: &[u64]) -> Result { if program.first() != Some(&PROGRAM_MAGIC) { return Err("not a mavros VM program binary".to_string()); @@ -122,10 +123,27 @@ impl ProgramHeader { let global_frame_size = word(off)? as usize; let entry_blob_field_count = word(off + 1)? as usize; let num_entries = word(off + 2)? as usize; - let entry_points = words(off + 3, num_entries)? + let entry_points: Vec = words(off + 3, num_entries)? .iter() .map(|&offset| offset as usize) .collect(); + let code_start = off + 3 + num_entries; + for (index, &entry) in entry_points.iter().enumerate() { + if entry == NO_ENTRY_POINT { + continue; + } + // The VM reads the frame size at `entry + 1` and starts at `entry + 2`, unchecked. + if !(code_start..program.len() - 2).contains(&entry) { + return Err(format!( + "the VM program's entry point {index} at word {entry} lies outside its code" + )); + } + if program[entry] != FUNCTION_MARKER { + return Err(format!( + "the VM program's entry point {index} at word {entry} is not a function marker" + )); + } + } Ok(ProgramHeader { field, constraint_layouts, @@ -133,7 +151,7 @@ impl ProgramHeader { global_frame_size, entry_blob_field_count, entry_points, - code_start: off + 3 + num_entries, + code_start, }) } } diff --git a/vm/src/bytecode.rs b/vm/src/bytecode.rs index c28110cf..5a3d988c 100644 --- a/vm/src/bytecode.rs +++ b/vm/src/bytecode.rs @@ -3750,7 +3750,7 @@ impl Display for Function { } pub use mavros_artifacts::program_header::{ - ConstraintLayouts, ENTRY_AD, ENTRY_VERIFIER, ENTRY_WITGEN, NO_ENTRY_POINT, + ConstraintLayouts, ENTRY_AD, ENTRY_VERIFIER, ENTRY_WITGEN, FUNCTION_MARKER, NO_ENTRY_POINT, PROGRAM_FORMAT_VERSION, PROGRAM_MAGIC, ProgramHeader, }; @@ -3830,7 +3830,7 @@ impl Program { binary.push(self.entry_blob_field_count as u64); // Entry-point table: [num_entries, marker_offset_0, ...]. The offsets are absolute word - // indices of each entry function's `u64::MAX` marker; they are patched in once function + // indices of each entry function's `FUNCTION_MARKER`; they are patched in once function // positions are known. binary.push(self.entry_points.len() as u64); let entry_table_start = binary.len(); @@ -3855,7 +3855,7 @@ impl Program { // Function marker function_markers.push(binary.len()); let function_offset = binary.len(); - binary.push(u64::MAX); + binary.push(FUNCTION_MARKER); binary.push(function.frame_size as u64); let mut locations = debug_info.as_ref().map(|_| Vec::new()); @@ -3916,7 +3916,8 @@ impl Program { /// /// If the binary does not open with [`PROGRAM_MAGIC`] followed by [`PROGRAM_FORMAT_VERSION`]: a /// binary of any other format numbers its opcodes differently, so they would select the wrong -/// handlers. If its field word names no supported field, or it ends inside the header. +/// handlers. If its field word names no supported field, it ends inside the header, or an entry +/// point is not on a function marker with room for a frame size and an opcode. pub fn parse_program_header(program: &[u64]) -> ProgramHeader { ProgramHeader::parse(program).unwrap_or_else(|error| panic!("{error}")) } @@ -6130,7 +6131,7 @@ mod tests { 0, 0, 0, - u64::MAX, + FUNCTION_MARKER, 0, ]; let header = parse_program_header(&binary); @@ -6165,7 +6166,26 @@ mod tests { 0, 0, 0, - u64::MAX, + FUNCTION_MARKER, + 0, + ]); + } + + #[test] + #[should_panic(expected = "entry point 0 at word 5 lies outside its code")] + fn an_entry_point_inside_the_header_is_refused() { + parse_program_header(&[ + PROGRAM_MAGIC, + PROGRAM_FORMAT_VERSION, + FieldId::Goldilocks.code(), + 0, + 0, + 0, + 0, + 1, + 5, + FUNCTION_MARKER, + 0, 0, ]); } diff --git a/vm/src/interpreter.rs b/vm/src/interpreter.rs index 8f0b2ad3..a2900ed8 100644 --- a/vm/src/interpreter.rs +++ b/vm/src/interpreter.rs @@ -263,7 +263,7 @@ pub(crate) fn prepare_dispatch(program: &mut [u64], code_start: usize) { let mut current_offset = code_start; while current_offset < program.len() { let opcode = program[current_offset]; - if opcode == u64::MAX { + if opcode == bytecode::FUNCTION_MARKER { current_offset += 2; continue; }