Skip to content

Add support for tracing#489

Open
srinathsetty wants to merge 6 commits intomainfrom
tracing
Open

Add support for tracing#489
srinathsetty wants to merge 6 commits intomainfrom
tracing

Conversation

@srinathsetty
Copy link
Copy Markdown
Collaborator

No description provided.

Fixes cargo doc failure caused by redundant intra-doc link path
on AllocatedBit in AllocatedNum::from_parts doc comment.
Add tracing instrumentation to all public API methods:

- nova: PublicParams::setup, RecursiveSNARK::new/prove_step/verify,
  CompressedSNARK::setup/prove/verify
- neutron: PublicParams::setup, RecursiveSNARK::new/prove_step/verify
- nova::nifs: NIFS::prove/verify, NIFSRelaxed::prove/verify
- neutron::nifs: NIFS::prove/verify

info! for setup/compression milestones, debug! for per-step operations.
No subscriber installed — callers decide output. Near-zero overhead
when no subscriber is registered.
- spartan/snark.rs: #[instrument] on setup/prove/verify
- spartan/ppsnark.rs: #[instrument] on setup/prove/verify, warn on
  invalid commitment key length and sumcheck mismatch
- spartan/direct.rs: #[instrument] on DirectSNARK setup/prove/verify
- r1cs/mod.rs: warn! on is_sat/is_sat_relaxed failures, info! on
  ptau file loading, warn! when no ptau file found
- provider/ptau.rs: warn! on all validation errors (invalid header,
  on read_ptau success
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds structured tracing across Nova/Neutron and Spartan SNARK flows to improve observability (spans for setup/prove/verify and leveled events), and documents how downstream applications can enable log output.

Changes:

  • Add tracing dependency and instrument key public APIs (setup/prove/verify) with #[instrument(skip_all, ...)].
  • Emit info/debug/warn events for key lifecycle points and validation failures (e.g., setup completion, verification passed, ptau/commitment key validation).
  • Document tracing usage in README.md and adjust a rustdoc intra-link in the numeric gadget docs.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
Cargo.toml Adds tracing as a dependency.
README.md Documents how to enable tracing output via a subscriber and RUST_LOG.
src/spartan/snark.rs Adds spans for Spartan SNARK setup/prove/verify and a debug success event.
src/spartan/ppsnark.rs Adds spans/events for ppsnark setup/prove/verify plus warn logs on validation failures.
src/spartan/direct.rs Adds spans around DirectSNARK APIs and a debug success event.
src/r1cs/mod.rs Adds warn/info events for ptau selection/loading and satisfiability failure reasons.
src/provider/ptau.rs Adds warn/debug/info events during ptau parsing and validation.
src/nova/nifs.rs Adds spans for NIFS prove/verify paths.
src/nova/mod.rs Adds spans/events for PublicParams/RecursiveSNARK/CompressedSNARK lifecycle.
src/neutron/nifs.rs Adds spans for Neutron NIFS prove/verify.
src/neutron/mod.rs Adds spans/events for Neutron PublicParams/RecursiveSNARK lifecycle.
src/frontend/gadgets/num.rs Updates a rustdoc intra-link for AllocatedBit.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 469 to 471
/// Updates the provided `RecursiveSNARK` by executing a step of the incremental computation
#[instrument(skip_all, name = "nova::RecursiveSNARK::prove_step", fields(step = self.i))]
pub fn prove_step(&mut self, pp: &PublicParams<E1, E2, C>, c: &C) -> Result<(), NovaError> {
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

#[instrument(..., fields(step = self.i))] records the value of self.i at function entry, but self.i is incremented before the "step complete" debug event. This makes the span field and debug event disagree (and is off-by-one for the first call where self.i is bumped from 0→1). Consider recording a stable value (e.g., steps_executed = self.i), or compute the step number in a local and use that for both span fields and logs (or record the field after increment).

Copilot uses AI. Check for mistakes.
Comment on lines 319 to 321
/// Updates the provided `RecursiveSNARK` by executing a step of the incremental computation
#[instrument(skip_all, name = "neutron::RecursiveSNARK::prove_step", fields(step = self.i))]
pub fn prove_step(&mut self, pp: &PublicParams<E1, E2, C>, c: &C) -> Result<(), NovaError> {
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

#[instrument(..., fields(step = self.i))] captures self.i at entry, but self.i is incremented during the step; the later "step complete" debug event logs the incremented value. This makes the span field misleading/off-by-one. Consider recording a stable field name/value (e.g., steps_executed), using a local let step = ... for both, or recording the span field after increment.

Copilot uses AI. Check for mistakes.
srinathsetty and others added 3 commits April 1, 2026 14:52
The info! macro is only used inside #[cfg(feature = "io")], so importing
it unconditionally causes an unused-import error with --no-default-features.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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