Skip to content

Port to whir PR #215 + PR #217 with SHA-256#288

Open
Bisht13 wants to merge 7 commits intomainfrom
px/whir-pr215-compat
Open

Port to whir PR #215 + PR #217 with SHA-256#288
Bisht13 wants to merge 7 commits intomainfrom
px/whir-pr215-compat

Conversation

@Bisht13
Copy link
Collaborator

@Bisht13 Bisht13 commented Feb 13, 2026

Ports provekit to whir's PR #215 transcript API and PR #217 Weights trait. Uses SHA-256 for Merkle tree hash, transcript sponge, and proof of work.

Changes

  • Transcript: IOPatternDomainSeparator/ProverState/VerifierState. Spongefish v1.0.0-rc1 (byte-oriented).
  • Weights: Weights enum → Covector<F> + dyn Weights<F> trait objects.
  • Hash: SHA-256 for Merkle tree (whir::hash::SHA2), transcript sponge (spongefish::instantiations::SHA256), and PoW. Skyscraper code retained for future use.
  • Build: Added release-fast profile, jemalloc, reduced tracing noise.
  • Cleanup: Removed SumcheckIOPattern, WitnessIOPattern, witness_io_pattern.rs.

Tested

Prepare → Prove → Verify on complete_age_check.

Replace IOPattern-based transcript with whir's DomainSeparator/ProverState/
VerifierState API (rev 246dae28). Update spongefish to v1.0.0-rc1 with
byte-oriented Permutation<64> interface. Proof now carries narg_string +
hints instead of a single transcript blob.

Key changes:
- Sponge: Permutation<64> with U=u8, WIDTH=64, RATE=32
- PoW: PoWSolution return type, solution() method
- Prover: config.commit()/config.prove() replacing CommitmentWriter/Prover
- Verifier: config.receive_commitment()/config.verify() replacing readers
- Public weights use Weights::geometric (non-deferred) on both sides
- Cross-evaluation hints for dual-commitment path
- NTT registration via global type-map for ark_bn254::Fr
- Clippy clean on prover/verifier crates
Propagate whir's debug_assertions transcript pattern through
WhirR1CSProof so in-process prove→verify (bench tests) passes
in debug builds. Add register_ntt() call in bench tests.

Point whir to 22efa1b2 (px/reduce-tracing-noise branch).
@Bisht13 Bisht13 changed the title Port to whir PR #215 transcript API Port to whir refactor transcript API Feb 14, 2026
Replace ark-crypto-primitives-based SkyscraperCRH/SkyscraperMerkleConfig
with a whir HashEngine implementation that registers Skyscraper for
leaf hashing, internal nodes, and proof-of-work.

Uses block4 NEON SIMD on aarch64 for batched compression, with a
stack-allocated pair buffer to avoid heap allocation in the hot path.
@Bisht13 Bisht13 changed the title Port to whir refactor transcript API Port provekit to whir PR #215 transcript API + PR #217 Weights trait + Skyscraper HashEngine Feb 14, 2026
@Bisht13 Bisht13 changed the title Port provekit to whir PR #215 transcript API + PR #217 Weights trait + Skyscraper HashEngine Port to whir PR #215 + PR #217 with Skyscraper HashEngine Feb 14, 2026
@Bisht13 Bisht13 requested a review from ashpect February 15, 2026 17:11
#[derive(Debug, Serialize, Deserialize)]

pub struct WHIRConfigGnark {
/// number of rounds
Copy link
Collaborator

Choose a reason for hiding this comment

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

There does not seem any logical reason to remove comments. Maybe its better to keep it for more context when fixing gnark.

fn main() -> Result<()> {
let args = argh::from_env::<cmd::Args>();
let subscriber = Registry::default().with(SpanStats);
let subscriber = Registry::default().with(SpanStats.with_filter(LevelFilter::INFO));
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think its better to define level instead of defaulting to INFO.

for eg :
// Debug builds: track ALL spans for detailed profiling
// Release builds: only INFO+ to reduce overhead
#[cfg(debug_assertions)]
let level = LevelFilter::TRACE;
#[cfg(not(debug_assertions))]
let level = LevelFilter::INFO;

pub extern "C" fn pk_init() -> c_int {
// Initialize tracing/logging if needed
// For now, we'll keep it simple and just return success
provekit_common::register_ntt();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Keep the tracing/logging as a TODO comment

challenge: [u64; 4],
bits: f64,
challenge: [u8; 32],
challenge_u64: [u64; 4],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Storing u64 challenge seems redundant. transmuting when required might be a better approach.

For eg : see verify code below

Self {
challenge: transmute!(challenge),
challenge,
challenge_u64: transmute!(challenge),
Copy link
Collaborator

Choose a reason for hiding this comment

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

redundant

.map(|w| covector_dot(w, &c1r_evals))
.collect();

merlin.prover_hint_ark(&(f_sums_1.clone(), g_sums_1.clone()));
Copy link
Collaborator

Choose a reason for hiding this comment

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

unnecessary cloning, these 4 aren't used again hence can be directly passed

Copy link
Collaborator

Choose a reason for hiding this comment

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

*8

// weights_2 on c1's polynomials. Whir's prove() expects evaluations
// for ALL (weight, polynomial) pairs in row-major order.
let c1m_evals = coeffs_to_evals(&c1.masked_polynomial_coeff);
let c1r_evals = coeffs_to_evals(&c1.random_polynomial_coeff);
Copy link
Collaborator

Choose a reason for hiding this comment

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

redundant call , its same as c1m_evals

&c2.random_polynomial_coeff,
];

let poly_evals: Vec<Vec<FieldElement>> =
Copy link
Collaborator

Choose a reason for hiding this comment

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

c1m and c1r evals are already calculated before.

use let poly_evals = vec![c1m_evals, c1r_evals, c2m_evals, c2r_evals];

(sums_2.0.try_into().unwrap(), sums_2.1.try_into().unwrap());

let mut statement_1 = prepare_statement_for_witness_verifier::<3>(
_public_weights_challenge,
Copy link
Collaborator

Choose a reason for hiding this comment

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

This variable starts with _ , but it is used and not unused, fix naming

// Check deferred linear constraints.
// The public weight is Geometric (non-deferred), so it's not in the deferred
// list.
let offset = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Irrelevent comment and remove offset.

Bisht13 added a commit that referenced this pull request Feb 16, 2026
- Remove redundant challenge_u64 field in SkyscraperPoW, inline transmute
- Add SAFETY comment for unsafe block in SkyscraperHashEngine
- Add PoW unit test for Skyscraper
- Move coeffs_to_evals and covector_dot to common/utils/zk_utils
- Derive Debug on WhirR1CSScheme (WhirConfig now supports it)
- Call register_ntt() in create_domain_separator for robustness
- Remove unnecessary clones on prover hint submissions
- Reuse pre-computed evaluations instead of redundant recomputation
- Remove unused _public_weights_challenge from verifier destructuring
- Remove dead offset variable in deferred eval checks
- Use cfg-based LevelFilter (TRACE debug, INFO release) in CLI
- Add TODO for FFI tracing initialization
- Restore doc comments on GnarkConfig and WHIRConfigGnark fields
- Restore comment about arbitrary no-public-inputs hint
- Remove redundant challenge_u64 field in SkyscraperPoW, inline transmute
- Add SAFETY comment for unsafe block in SkyscraperHashEngine
- Add PoW unit test for Skyscraper
- Move coeffs_to_evals and covector_dot to common/utils/zk_utils
- Derive Debug on WhirR1CSScheme (WhirConfig now supports it)
- Call register_ntt() in create_domain_separator for robustness
- Remove unnecessary clones on prover hint submissions
- Reuse pre-computed evaluations instead of redundant recomputation
- Remove unused _public_weights_challenge from verifier destructuring
- Remove dead offset variable in deferred eval checks
- Use cfg-based LevelFilter (TRACE debug, INFO release) in CLI
- Add TODO for FFI tracing initialization
- Restore doc comments on GnarkConfig and WHIRConfigGnark fields
- Restore comment about arbitrary no-public-inputs hint
@Bisht13 Bisht13 force-pushed the px/whir-pr215-compat branch from e17c47c to ac651c1 Compare February 16, 2026 06:11
@Bisht13 Bisht13 changed the title Port to whir PR #215 + PR #217 with Skyscraper HashEngine Port to whir PR #215 + PR #217 with SHA-256 Feb 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants