From 395289bb05012c9ef441ea7a3ea5ba47e1ea6cfa Mon Sep 17 00:00:00 2001 From: Koby Date: Fri, 13 Oct 2023 18:31:30 +0200 Subject: [PATCH 1/7] chore: warn if backend version mismatch --- tooling/backend_interface/src/cli/mod.rs | 2 ++ tooling/backend_interface/src/cli/version.rs | 28 +++++++++++++++++++ tooling/backend_interface/src/lib.rs | 11 ++++++++ tooling/backend_interface/src/proof_system.rs | 3 ++ tooling/bb_abstraction_leaks/build.rs | 1 + tooling/bb_abstraction_leaks/src/lib.rs | 1 + 6 files changed, 46 insertions(+) create mode 100644 tooling/backend_interface/src/cli/version.rs diff --git a/tooling/backend_interface/src/cli/mod.rs b/tooling/backend_interface/src/cli/mod.rs index 848c5e11e81..4bed573c9ec 100644 --- a/tooling/backend_interface/src/cli/mod.rs +++ b/tooling/backend_interface/src/cli/mod.rs @@ -6,6 +6,7 @@ mod info; mod prove; mod verify; mod write_vk; +mod version; pub(crate) use contract::ContractCommand; pub(crate) use gates::GatesCommand; @@ -13,6 +14,7 @@ pub(crate) use info::InfoCommand; pub(crate) use prove::ProveCommand; pub(crate) use verify::VerifyCommand; pub(crate) use write_vk::WriteVkCommand; +pub(crate) use version::VersionCommand; #[test] fn no_command_provided_works() -> Result<(), crate::BackendError> { diff --git a/tooling/backend_interface/src/cli/version.rs b/tooling/backend_interface/src/cli/version.rs new file mode 100644 index 00000000000..3de8062c2e0 --- /dev/null +++ b/tooling/backend_interface/src/cli/version.rs @@ -0,0 +1,28 @@ +use std::path::Path; + +use crate::BackendError; + +use super::string_from_stderr; + +/// VersionCommand will call the backend binary +/// to query installed version. +pub(crate) struct VersionCommand {} + +impl VersionCommand { + pub(crate) fn run(self, binary_path: &Path) -> Result { + let mut command = std::process::Command::new(binary_path); + + command + .arg("--version"); + + let output = command.output()?; + if output.status.success() { + match String::from_utf8(output.stdout) { + Ok(result) => Ok(result), + Err(_) => Err(BackendError::CommandFailed("Unexpected output from --version check.".to_owned())), + } + } else { + Err(BackendError::CommandFailed(string_from_stderr(&output.stderr))) + } + } +} diff --git a/tooling/backend_interface/src/lib.rs b/tooling/backend_interface/src/lib.rs index d6a9cd0d293..afb42230d8d 100644 --- a/tooling/backend_interface/src/lib.rs +++ b/tooling/backend_interface/src/lib.rs @@ -10,6 +10,8 @@ mod smart_contract; use acvm::acir::circuit::Opcode; use bb_abstraction_leaks::ACVM_BACKEND_BARRETENBERG; +use bb_abstraction_leaks::BB_VERSION; +use cli::VersionCommand; pub use download::download_backend; const BACKENDS_DIR: &str = ".nargo/backends"; @@ -104,6 +106,15 @@ impl Backend { fn crs_directory(&self) -> PathBuf { self.backend_directory().join("crs") } + + fn assert_correct_version(&self) -> Result { + let binary_path = self.binary_path(); + let version_string = VersionCommand{}.run(binary_path)?; + if version_string.as_str() != BB_VERSION { + println!("WARNING!: Configured backend version `{:}` is different from expected `{:}`", version_string, BB_VERSION); + } + Ok(version_string) + } } pub struct BackendOpcodeSupport { diff --git a/tooling/backend_interface/src/proof_system.rs b/tooling/backend_interface/src/proof_system.rs index ffdb7531ed1..fde3fd20a28 100644 --- a/tooling/backend_interface/src/proof_system.rs +++ b/tooling/backend_interface/src/proof_system.rs @@ -39,6 +39,8 @@ impl Backend { ) -> Result, BackendError> { let binary_path = self.assert_binary_exists()?; + self.assert_correct_version()?; + let temp_directory = tempdir().expect("could not create a temporary directory"); let temp_directory = temp_directory.path().to_path_buf(); @@ -78,6 +80,7 @@ impl Backend { is_recursive: bool, ) -> Result { let binary_path = self.assert_binary_exists()?; + self.assert_correct_version()?; let temp_directory = tempdir().expect("could not create a temporary directory"); let temp_directory = temp_directory.path().to_path_buf(); diff --git a/tooling/bb_abstraction_leaks/build.rs b/tooling/bb_abstraction_leaks/build.rs index baf3cfdf6a6..1ae5a28f5a8 100644 --- a/tooling/bb_abstraction_leaks/build.rs +++ b/tooling/bb_abstraction_leaks/build.rs @@ -35,6 +35,7 @@ fn main() -> Result<(), String> { }; println!("cargo:rustc-env=BB_BINARY_URL={}", get_bb_download_url(arch, os)); + println!("cargo:rustc-env=BB_VERSION={}", VERSION); Ok(()) } diff --git a/tooling/bb_abstraction_leaks/src/lib.rs b/tooling/bb_abstraction_leaks/src/lib.rs index 799e14c36f0..e0fdc467c53 100644 --- a/tooling/bb_abstraction_leaks/src/lib.rs +++ b/tooling/bb_abstraction_leaks/src/lib.rs @@ -5,6 +5,7 @@ use acvm::FieldElement; pub const ACVM_BACKEND_BARRETENBERG: &str = "acvm-backend-barretenberg"; pub const BB_DOWNLOAD_URL: &str = env!("BB_BINARY_URL"); +pub const BB_VERSION: &str = env!("BB_VERSION"); /// Embed the Solidity verifier file const ULTRA_VERIFIER_CONTRACT: &str = include_str!("contract.sol"); From d8749cc8ca1ae366d344fd55c69b1d2272ae15c4 Mon Sep 17 00:00:00 2001 From: Koby Date: Fri, 13 Oct 2023 18:42:47 +0200 Subject: [PATCH 2/7] chore: add smart contract assert --- tooling/backend_interface/src/smart_contract.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tooling/backend_interface/src/smart_contract.rs b/tooling/backend_interface/src/smart_contract.rs index 5f56557cad4..c7dbe3713d0 100644 --- a/tooling/backend_interface/src/smart_contract.rs +++ b/tooling/backend_interface/src/smart_contract.rs @@ -10,6 +10,8 @@ impl Backend { pub fn eth_contract(&self, circuit: &Circuit) -> Result { let binary_path = self.assert_binary_exists()?; + self.assert_correct_version()?; + let temp_directory = tempdir().expect("could not create a temporary directory"); let temp_directory_path = temp_directory.path().to_path_buf(); From 9ca358d90019c7dd555112cc19ba726c113808d7 Mon Sep 17 00:00:00 2001 From: Koby Date: Fri, 13 Oct 2023 19:46:35 +0200 Subject: [PATCH 3/7] chore: assert version on ly on barretenberg backends --- tooling/backend_interface/src/cli/mod.rs | 4 ++-- tooling/backend_interface/src/cli/version.rs | 7 ++++--- tooling/backend_interface/src/lib.rs | 14 +++++++++----- tooling/backend_interface/src/proof_system.rs | 4 ++-- tooling/backend_interface/src/smart_contract.rs | 2 +- tooling/bb_abstraction_leaks/src/lib.rs | 1 + 6 files changed, 19 insertions(+), 13 deletions(-) diff --git a/tooling/backend_interface/src/cli/mod.rs b/tooling/backend_interface/src/cli/mod.rs index 4bed573c9ec..d1eebb1ba8e 100644 --- a/tooling/backend_interface/src/cli/mod.rs +++ b/tooling/backend_interface/src/cli/mod.rs @@ -5,16 +5,16 @@ mod gates; mod info; mod prove; mod verify; -mod write_vk; mod version; +mod write_vk; pub(crate) use contract::ContractCommand; pub(crate) use gates::GatesCommand; pub(crate) use info::InfoCommand; pub(crate) use prove::ProveCommand; pub(crate) use verify::VerifyCommand; -pub(crate) use write_vk::WriteVkCommand; pub(crate) use version::VersionCommand; +pub(crate) use write_vk::WriteVkCommand; #[test] fn no_command_provided_works() -> Result<(), crate::BackendError> { diff --git a/tooling/backend_interface/src/cli/version.rs b/tooling/backend_interface/src/cli/version.rs index 3de8062c2e0..29c434ac5d8 100644 --- a/tooling/backend_interface/src/cli/version.rs +++ b/tooling/backend_interface/src/cli/version.rs @@ -12,14 +12,15 @@ impl VersionCommand { pub(crate) fn run(self, binary_path: &Path) -> Result { let mut command = std::process::Command::new(binary_path); - command - .arg("--version"); + command.arg("--version"); let output = command.output()?; if output.status.success() { match String::from_utf8(output.stdout) { Ok(result) => Ok(result), - Err(_) => Err(BackendError::CommandFailed("Unexpected output from --version check.".to_owned())), + Err(_) => Err(BackendError::CommandFailed( + "Unexpected output from --version check.".to_owned(), + )), } } else { Err(BackendError::CommandFailed(string_from_stderr(&output.stderr))) diff --git a/tooling/backend_interface/src/lib.rs b/tooling/backend_interface/src/lib.rs index afb42230d8d..a1132a59fd9 100644 --- a/tooling/backend_interface/src/lib.rs +++ b/tooling/backend_interface/src/lib.rs @@ -10,6 +10,7 @@ mod smart_contract; use acvm::acir::circuit::Opcode; use bb_abstraction_leaks::ACVM_BACKEND_BARRETENBERG; +use bb_abstraction_leaks::BACKEND_BARRETENBERG_SEARCH_STR; use bb_abstraction_leaks::BB_VERSION; use cli::VersionCommand; pub use download::download_backend; @@ -107,13 +108,16 @@ impl Backend { self.backend_directory().join("crs") } - fn assert_correct_version(&self) -> Result { + fn assert_correct_version(&self) { let binary_path = self.binary_path(); - let version_string = VersionCommand{}.run(binary_path)?; - if version_string.as_str() != BB_VERSION { - println!("WARNING!: Configured backend version `{:}` is different from expected `{:}`", version_string, BB_VERSION); + if binary_path.to_string_lossy().contains(BACKEND_BARRETENBERG_SEARCH_STR) { + let version_result = VersionCommand {}.run(binary_path); + if let Ok(version_string) = version_result { + if version_string.as_str() != BB_VERSION { + println!("WARNING!: Configured backend version `{:}` is different from expected `{:}`", version_string, BB_VERSION); + } + } } - Ok(version_string) } } diff --git a/tooling/backend_interface/src/proof_system.rs b/tooling/backend_interface/src/proof_system.rs index fde3fd20a28..7553874a8d7 100644 --- a/tooling/backend_interface/src/proof_system.rs +++ b/tooling/backend_interface/src/proof_system.rs @@ -39,7 +39,7 @@ impl Backend { ) -> Result, BackendError> { let binary_path = self.assert_binary_exists()?; - self.assert_correct_version()?; + self.assert_correct_version(); let temp_directory = tempdir().expect("could not create a temporary directory"); let temp_directory = temp_directory.path().to_path_buf(); @@ -80,7 +80,7 @@ impl Backend { is_recursive: bool, ) -> Result { let binary_path = self.assert_binary_exists()?; - self.assert_correct_version()?; + self.assert_correct_version(); let temp_directory = tempdir().expect("could not create a temporary directory"); let temp_directory = temp_directory.path().to_path_buf(); diff --git a/tooling/backend_interface/src/smart_contract.rs b/tooling/backend_interface/src/smart_contract.rs index c7dbe3713d0..f1e55310f2e 100644 --- a/tooling/backend_interface/src/smart_contract.rs +++ b/tooling/backend_interface/src/smart_contract.rs @@ -10,7 +10,7 @@ impl Backend { pub fn eth_contract(&self, circuit: &Circuit) -> Result { let binary_path = self.assert_binary_exists()?; - self.assert_correct_version()?; + self.assert_correct_version(); let temp_directory = tempdir().expect("could not create a temporary directory"); let temp_directory_path = temp_directory.path().to_path_buf(); diff --git a/tooling/bb_abstraction_leaks/src/lib.rs b/tooling/bb_abstraction_leaks/src/lib.rs index e0fdc467c53..7f13ade18cf 100644 --- a/tooling/bb_abstraction_leaks/src/lib.rs +++ b/tooling/bb_abstraction_leaks/src/lib.rs @@ -3,6 +3,7 @@ use acvm::FieldElement; +pub const BACKEND_BARRETENBERG_SEARCH_STR: &str = "barretenberg"; pub const ACVM_BACKEND_BARRETENBERG: &str = "acvm-backend-barretenberg"; pub const BB_DOWNLOAD_URL: &str = env!("BB_BINARY_URL"); pub const BB_VERSION: &str = env!("BB_VERSION"); From 54616a1ef14aafc68a6513d16ad8b102c801744b Mon Sep 17 00:00:00 2001 From: Tom French Date: Mon, 16 Oct 2023 09:57:39 +0100 Subject: [PATCH 4/7] chore: nits --- tooling/backend_interface/src/cli/version.rs | 2 +- tooling/backend_interface/src/lib.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tooling/backend_interface/src/cli/version.rs b/tooling/backend_interface/src/cli/version.rs index 29c434ac5d8..83ab72a870e 100644 --- a/tooling/backend_interface/src/cli/version.rs +++ b/tooling/backend_interface/src/cli/version.rs @@ -6,7 +6,7 @@ use super::string_from_stderr; /// VersionCommand will call the backend binary /// to query installed version. -pub(crate) struct VersionCommand {} +pub(crate) struct VersionCommand; impl VersionCommand { pub(crate) fn run(self, binary_path: &Path) -> Result { diff --git a/tooling/backend_interface/src/lib.rs b/tooling/backend_interface/src/lib.rs index a1132a59fd9..0968cf03e7d 100644 --- a/tooling/backend_interface/src/lib.rs +++ b/tooling/backend_interface/src/lib.rs @@ -111,10 +111,10 @@ impl Backend { fn assert_correct_version(&self) { let binary_path = self.binary_path(); if binary_path.to_string_lossy().contains(BACKEND_BARRETENBERG_SEARCH_STR) { - let version_result = VersionCommand {}.run(binary_path); + let version_result = VersionCommand.run(binary_path); if let Ok(version_string) = version_result { if version_string.as_str() != BB_VERSION { - println!("WARNING!: Configured backend version `{:}` is different from expected `{:}`", version_string, BB_VERSION); + println!("WARNING!: Configured backend version `{version_string}` is different from expected `{BB_VERSION}`"); } } } From 4be4041d1b32388c9935aadc3aa2d36daf75b7e9 Mon Sep 17 00:00:00 2001 From: Koby Date: Mon, 16 Oct 2023 12:18:50 +0200 Subject: [PATCH 5/7] chore: replace-download correct backend version --- tooling/backend_interface/src/lib.rs | 12 ++++++++---- tooling/backend_interface/src/proof_system.rs | 4 ++-- tooling/backend_interface/src/smart_contract.rs | 2 +- tooling/bb_abstraction_leaks/src/lib.rs | 1 - 4 files changed, 11 insertions(+), 8 deletions(-) diff --git a/tooling/backend_interface/src/lib.rs b/tooling/backend_interface/src/lib.rs index a1132a59fd9..c46a04d9094 100644 --- a/tooling/backend_interface/src/lib.rs +++ b/tooling/backend_interface/src/lib.rs @@ -10,7 +10,6 @@ mod smart_contract; use acvm::acir::circuit::Opcode; use bb_abstraction_leaks::ACVM_BACKEND_BARRETENBERG; -use bb_abstraction_leaks::BACKEND_BARRETENBERG_SEARCH_STR; use bb_abstraction_leaks::BB_VERSION; use cli::VersionCommand; pub use download::download_backend; @@ -108,16 +107,21 @@ impl Backend { self.backend_directory().join("crs") } - fn assert_correct_version(&self) { + fn assert_correct_version(&self) -> Result<&PathBuf, BackendError> { let binary_path = self.binary_path(); - if binary_path.to_string_lossy().contains(BACKEND_BARRETENBERG_SEARCH_STR) { + if binary_path.to_string_lossy().contains(ACVM_BACKEND_BARRETENBERG) { let version_result = VersionCommand {}.run(binary_path); if let Ok(version_string) = version_result { if version_string.as_str() != BB_VERSION { - println!("WARNING!: Configured backend version `{:}` is different from expected `{:}`", version_string, BB_VERSION); + println!("WARNING!: `{:}` version `{:}` is different from expected `{:}`, downloading expected...", ACVM_BACKEND_BARRETENBERG, version_string, BB_VERSION); + // If we're trying to use barretenberg, automatically go and install the correct version. + let bb_url = std::env::var("BB_BINARY_URL") + .unwrap_or_else(|_| bb_abstraction_leaks::BB_DOWNLOAD_URL.to_owned()); + download_backend(&bb_url, binary_path)?; } } } + Ok(binary_path) } } diff --git a/tooling/backend_interface/src/proof_system.rs b/tooling/backend_interface/src/proof_system.rs index 7553874a8d7..fde3fd20a28 100644 --- a/tooling/backend_interface/src/proof_system.rs +++ b/tooling/backend_interface/src/proof_system.rs @@ -39,7 +39,7 @@ impl Backend { ) -> Result, BackendError> { let binary_path = self.assert_binary_exists()?; - self.assert_correct_version(); + self.assert_correct_version()?; let temp_directory = tempdir().expect("could not create a temporary directory"); let temp_directory = temp_directory.path().to_path_buf(); @@ -80,7 +80,7 @@ impl Backend { is_recursive: bool, ) -> Result { let binary_path = self.assert_binary_exists()?; - self.assert_correct_version(); + self.assert_correct_version()?; let temp_directory = tempdir().expect("could not create a temporary directory"); let temp_directory = temp_directory.path().to_path_buf(); diff --git a/tooling/backend_interface/src/smart_contract.rs b/tooling/backend_interface/src/smart_contract.rs index f1e55310f2e..c7dbe3713d0 100644 --- a/tooling/backend_interface/src/smart_contract.rs +++ b/tooling/backend_interface/src/smart_contract.rs @@ -10,7 +10,7 @@ impl Backend { pub fn eth_contract(&self, circuit: &Circuit) -> Result { let binary_path = self.assert_binary_exists()?; - self.assert_correct_version(); + self.assert_correct_version()?; let temp_directory = tempdir().expect("could not create a temporary directory"); let temp_directory_path = temp_directory.path().to_path_buf(); diff --git a/tooling/bb_abstraction_leaks/src/lib.rs b/tooling/bb_abstraction_leaks/src/lib.rs index 7f13ade18cf..e0fdc467c53 100644 --- a/tooling/bb_abstraction_leaks/src/lib.rs +++ b/tooling/bb_abstraction_leaks/src/lib.rs @@ -3,7 +3,6 @@ use acvm::FieldElement; -pub const BACKEND_BARRETENBERG_SEARCH_STR: &str = "barretenberg"; pub const ACVM_BACKEND_BARRETENBERG: &str = "acvm-backend-barretenberg"; pub const BB_DOWNLOAD_URL: &str = env!("BB_BINARY_URL"); pub const BB_VERSION: &str = env!("BB_VERSION"); From b11aa42e0737256e207f701c5b8ffc53a7c5f267 Mon Sep 17 00:00:00 2001 From: Tom French Date: Mon, 16 Oct 2023 12:41:36 +0100 Subject: [PATCH 6/7] chore: add handling for case where `bb --version` returns an error --- tooling/backend_interface/src/lib.rs | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/tooling/backend_interface/src/lib.rs b/tooling/backend_interface/src/lib.rs index c46a04d9094..6c91c181a92 100644 --- a/tooling/backend_interface/src/lib.rs +++ b/tooling/backend_interface/src/lib.rs @@ -110,11 +110,21 @@ impl Backend { fn assert_correct_version(&self) -> Result<&PathBuf, BackendError> { let binary_path = self.binary_path(); if binary_path.to_string_lossy().contains(ACVM_BACKEND_BARRETENBERG) { - let version_result = VersionCommand {}.run(binary_path); - if let Ok(version_string) = version_result { - if version_string.as_str() != BB_VERSION { - println!("WARNING!: `{:}` version `{:}` is different from expected `{:}`, downloading expected...", ACVM_BACKEND_BARRETENBERG, version_string, BB_VERSION); - // If we're trying to use barretenberg, automatically go and install the correct version. + match VersionCommand.run(binary_path) { + // If version matches then do nothing. + Ok(version_string) if version_string == BB_VERSION => (), + + // If version doesn't match then download the correct version. + Ok(version_string) => { + println!("`{ACVM_BACKEND_BARRETENBERG}` version `{version_string}` is different from expected `{BB_VERSION}`. Downloading expected version..."); + let bb_url = std::env::var("BB_BINARY_URL") + .unwrap_or_else(|_| bb_abstraction_leaks::BB_DOWNLOAD_URL.to_owned()); + download_backend(&bb_url, binary_path)?; + } + + // If `bb` fails to report its version, then attempt to fix it by re-downloading the binary. + Err(_) => { + println!("Could not determine version of `{ACVM_BACKEND_BARRETENBERG}`. Downloading expected version..."); let bb_url = std::env::var("BB_BINARY_URL") .unwrap_or_else(|_| bb_abstraction_leaks::BB_DOWNLOAD_URL.to_owned()); download_backend(&bb_url, binary_path)?; From 08189232e8eeac2c6edb80c8b5d2a628f3ff57d4 Mon Sep 17 00:00:00 2001 From: Tom French Date: Mon, 16 Oct 2023 12:50:30 +0100 Subject: [PATCH 7/7] fix: add `assert_correct_version` to methods where it was missing --- tooling/backend_interface/src/proof_system.rs | 3 ++- tooling/backend_interface/src/smart_contract.rs | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tooling/backend_interface/src/proof_system.rs b/tooling/backend_interface/src/proof_system.rs index fde3fd20a28..cbb4a61be8a 100644 --- a/tooling/backend_interface/src/proof_system.rs +++ b/tooling/backend_interface/src/proof_system.rs @@ -13,6 +13,7 @@ use crate::{Backend, BackendError, BackendOpcodeSupport}; impl Backend { pub fn get_exact_circuit_size(&self, circuit: &Circuit) -> Result { let binary_path = self.assert_binary_exists()?; + self.assert_correct_version()?; let temp_directory = tempdir().expect("could not create a temporary directory"); let temp_directory = temp_directory.path().to_path_buf(); @@ -28,6 +29,7 @@ impl Backend { pub fn get_backend_info(&self) -> Result<(Language, BackendOpcodeSupport), BackendError> { let binary_path = self.assert_binary_exists()?; + self.assert_correct_version()?; InfoCommand { crs_path: self.crs_directory() }.run(binary_path) } @@ -38,7 +40,6 @@ impl Backend { is_recursive: bool, ) -> Result, BackendError> { let binary_path = self.assert_binary_exists()?; - self.assert_correct_version()?; let temp_directory = tempdir().expect("could not create a temporary directory"); diff --git a/tooling/backend_interface/src/smart_contract.rs b/tooling/backend_interface/src/smart_contract.rs index c7dbe3713d0..50afd47287b 100644 --- a/tooling/backend_interface/src/smart_contract.rs +++ b/tooling/backend_interface/src/smart_contract.rs @@ -9,7 +9,6 @@ use tempfile::tempdir; impl Backend { pub fn eth_contract(&self, circuit: &Circuit) -> Result { let binary_path = self.assert_binary_exists()?; - self.assert_correct_version()?; let temp_directory = tempdir().expect("could not create a temporary directory");