Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ members = [
"config_macros", # proc macros used to declare a new config, this has to a separate crate due to rust compilation issues
"gkr",
"gkr_engine", # definitions of GKR engine and associated types
"gpu", # GPU support and circuit serialization
"hasher", # definitions of FiatShamirFieldHasher, FiatShamirBytesHash, and associated types
"poly_commit",
"serdes", # serialization and deserialization of various data structures
Expand Down
19 changes: 16 additions & 3 deletions bin/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use circuit::Circuit;
use clap::Parser;
use gkr::{
BN254ConfigMIMC5KZG, BN254ConfigSha2Hyrax, BN254ConfigSha2Raw, GF2ExtConfigSha2Orion,
GF2ExtConfigSha2Raw, Goldilocksx8ConfigSha2Orion, Goldilocksx8ConfigSha2Raw,
GF2ExtConfigSha2Raw, Goldilocksx1ConfigSha2Raw, Goldilocksx8ConfigSha2Orion, Goldilocksx8ConfigSha2Raw,
M31x1ConfigSha2RawVanilla, M31x16ConfigSha2OrionSquare, M31x16ConfigSha2OrionVanilla,
M31x16ConfigSha2RawSquare, M31x16ConfigSha2RawVanilla, Prover,
utils::{
Expand Down Expand Up @@ -69,7 +69,13 @@ fn main() {

"m31ext3" => match pcs_type {
PolynomialCommitmentType::Raw => match args.circuit.as_str() {
"keccak" => run_benchmark::<M31x16ConfigSha2RawVanilla>(&args, mpi_config.clone()),
"keccak" => {
if std::env::var("EXPANDER_GPU").map_or(false, |v| v == "1") {
run_benchmark::<M31x1ConfigSha2RawVanilla>(&args, mpi_config.clone())
} else {
run_benchmark::<M31x16ConfigSha2RawVanilla>(&args, mpi_config.clone())
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The check for the EXPANDER_GPU environment variable and its value ("1") is duplicated across multiple match arms (e.g., here and on lines 122-126). This introduces redundancy and makes the code less maintainable. Consider extracting this check into a boolean variable at the start of main and using constants for the environment variable name and value. This improves readability and reduces repetition.

Example:

// At the top of main()
const EXPANDER_GPU_ENV: &str = "EXPANDER_GPU";
const EXPANDER_GPU_ENABLED_VALUE: &str = "1";
let is_gpu_enabled = std::env::var(EXPANDER_GPU_ENV).map_or(false, |v| v == EXPANDER_GPU_ENABLED_VALUE);

// ... later in the match statement
if is_gpu_enabled {
    run_benchmark::<M31x1ConfigSha2RawVanilla>(&args, mpi_config.clone())
} else {
    run_benchmark::<M31x16ConfigSha2RawVanilla>(&args, mpi_config.clone())
}

},
"poseidon" => run_benchmark::<M31x16ConfigSha2RawSquare>(&args, mpi_config.clone()),
_ => unreachable!(),
},
Expand Down Expand Up @@ -112,7 +118,13 @@ fn main() {
},
"goldilocks" => match pcs_type {
PolynomialCommitmentType::Raw => match args.circuit.as_str() {
"keccak" => run_benchmark::<Goldilocksx8ConfigSha2Raw>(&args, mpi_config.clone()),
"keccak" => {
if std::env::var("EXPANDER_GPU").map_or(false, |v| v == "1") {
run_benchmark::<Goldilocksx1ConfigSha2Raw>(&args, mpi_config.clone())
} else {
run_benchmark::<Goldilocksx8ConfigSha2Raw>(&args, mpi_config.clone())
}
},
_ => unreachable!(),
},
PolynomialCommitmentType::Orion => match args.circuit.as_str() {
Expand Down Expand Up @@ -206,6 +218,7 @@ where
(FieldType::M31x1, "keccak") => 2,
(FieldType::M31x16, "keccak") => 2,
(FieldType::M31x16, "poseidon") => 120,
(FieldType::Goldilocksx1, "keccak") => 2,
(FieldType::Goldilocksx8, "keccak") => 2,
(FieldType::BabyBearx16, "keccak") => 2,
(FieldType::BN254, "keccak") => 2,
Expand Down
1 change: 1 addition & 0 deletions gkr/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ gf2_128 = { path = "../arith/gf2_128" }
gkr_engine = { path = "../gkr_engine" }
gkr_hashers = { path = "../hasher" }
goldilocks = { path = "../arith/goldilocks" }
gpu = { path = "../gpu" }
mersenne31 = { path = "../arith/mersenne31" }
poly_commit = { path = "../poly_commit" }
polynomials = { path = "../arith/polynomials" }
Expand Down
30 changes: 28 additions & 2 deletions gkr/src/prover/gkr_vanilla.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ pub fn gkr_prove<F: FieldEngine>(
sp: &mut ProverScratchPad<F>,
transcript: &mut impl Transcript,
mpi_config: &MPIConfig,
) -> (F::ChallengeField, ExpanderDualVarChallenge<F>) {
) -> (F::ChallengeField, ExpanderDualVarChallenge<F>)
where F::CircuitField: std::fmt::Debug, F::SimdCircuitField: std::fmt::Debug {
let layer_num = circuit.layers.len();

let mut challenge: ExpanderDualVarChallenge<F> =
Expand All @@ -36,6 +37,16 @@ pub fn gkr_prove<F: FieldEngine>(
mpi_config,
);

// Serialize circuit to file if EXPANDER_GPU environment variable is set to 1
if std::env::var("EXPANDER_GPU").map_or(false, |v| v == "1") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The EXPANDER_GPU environment variable check is duplicated here and at the end of the function (line 89). It's more efficient and maintainable to perform this check once at the beginning of the gkr_prove function and store the result in a local boolean variable.

if let Err(e) = gpu::serdes::serial_circuit_witness_as_plaintext(circuit, transcript, &challenge) {
println!("Failed to serialize circuit: {}", e);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using println! for error messages in library code is generally not recommended. Consider using a structured logging framework (e.g., log::warn!) or propagating the SerializationError to the caller. This provides more flexible error handling and better integration with application-level logging.

}
}

let mut final_vx_claim = None;
let mut final_vy_claim = None;

for i in (0..layer_num).rev() {
let timer = Timer::new(
&format!(
Expand All @@ -47,7 +58,7 @@ pub fn gkr_prove<F: FieldEngine>(
mpi_config.is_root(),
);

(_, _) = sumcheck_prove_gkr_layer(
let (vx_claim, vy_claim) = sumcheck_prove_gkr_layer(
&circuit.layers[i],
&mut challenge,
alpha,
Expand All @@ -57,6 +68,12 @@ pub fn gkr_prove<F: FieldEngine>(
i == layer_num - 1,
);

// Store the final layer claims for later use
if i == 0 {
final_vx_claim = Some(vx_claim);
final_vy_claim = vy_claim;
}

if challenge.rz_1.is_some() {
// TODO: try broadcast beta.unwrap directly
let mut tmp = transcript.generate_field_element::<F::ChallengeField>();
Expand All @@ -68,5 +85,14 @@ pub fn gkr_prove<F: FieldEngine>(
timer.stop();
}

// Print final claims if EXPANDER_GPU environment variable is set to 1
if std::env::var("EXPANDER_GPU").map_or(false, |v| v == "1") {
if let Some(vx) = final_vx_claim {
gpu::serdes::print_final_claims::<F>(&vx, &final_vy_claim);
println!("GKR final proof claims as shown above.");
std::process::exit(0);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Calling std::process::exit(0) here terminates the process abruptly. This prevents the function from returning its values and may bypass proper resource cleanup. While this is likely intended for a debug/development workflow to get the serialized data and stop, it's a very aggressive approach.

A cleaner design would be to have this function return a special value or an error indicating that the process should stop. The caller (e.g., in bin/src/main.rs) can then decide whether to exit gracefully. This makes the gkr_prove function more reusable and its behavior more predictable.


(claimed_v, challenge)
}
11 changes: 11 additions & 0 deletions gpu/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
[package]
name = "gpu"
version = "0.1.0"
edition = "2021"

[dependencies]
circuit = { path = "../circuit" }
gkr_engine = { path = "../gkr_engine" }
gkr_hashers = { path = "../hasher" }
transcript = { path = "../transcript" }
thiserror.workspace = true
1 change: 1 addition & 0 deletions gpu/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pub mod serdes;
Loading
Loading