Skip to content

Conversation

@hratoanina
Copy link
Contributor

@hratoanina hratoanina commented Sep 19, 2025

Opening as draft for visibility and for the API. I'm having trouble making it generic for the hash function. It technically compiles and runs, but I'm not satisfied with some of the design.

Copy link
Contributor

@Nashtare Nashtare left a comment

Choose a reason for hiding this comment

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

added my two cents, but not too deep yet given this is still WIP

@hratoanina hratoanina marked this pull request as ready for review September 24, 2025 15:04
@hratoanina
Copy link
Contributor Author

Opening it for review.
Part of #57, but linking everything together (especially handling the transition Extension -> Field from the recursive verifier circuit to the sponge table) is still TODO.

This design restricts generics to the run function of the circuit runner, as a trade-off between modularity, performance and ease of implementation. It can still be changed.

@hratoanina hratoanina changed the title WIP: Add sponge ops Add sponge ops Sep 24, 2025
@Nashtare Nashtare moved this from Todo to Ready for review in Plonky3 Recursion Sep 24, 2025
Copy link
Contributor

@Nashtare Nashtare left a comment

Choose a reason for hiding this comment

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

@hratoanina can you fix conflicts and resolve all addressed comments so we can do another round of review? thanks

@hratoanina
Copy link
Contributor Author

hratoanina commented Oct 1, 2025

I merged with the current design, which is very different from Alonso's approach with a dedicated config per non primitive op. If Alonso's approach is favoured it should be merged before this PR, and I'll merge on top of it.
The discussion about SoA/AoS and about generics was taken outside.

self.inputs.clear();
self.reset_flag = true;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Add some comments explaining what these functions are doing?

pub fn run(mut self) -> Result<Traces<F>, CircuitError> {
pub fn run<
P: CryptographicPermutation<[F; N]>,
const N: usize,
Copy link
Contributor

Choose a reason for hiding this comment

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

What are these values?

let mut current_hash =
if let Some(val) = self.witness.get(leaf.0 as usize).and_then(|x| x.as_ref()) {
*val
// Only handle FakeMerkleVerify ops here
Copy link
Contributor

Choose a reason for hiding this comment

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

Why only FakeMerkle here?


fn main() -> Result<(), ProverError> {
let mut rng = SmallRng::seed_from_u64(1);
let perm = Poseidon2BabyBear::<HASH_STATE_SIZE>::new_from_rng_128(&mut rng);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you could also do default_babybear_poseidon2_16()

rate_values.push(val);
}

// for i in 0..R {
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove commented code?

@hratoanina
Copy link
Contributor Author

On hold, we might go with a circuit approach for the challenger instead, using hashing tables directly (see #131).

@Nashtare
Copy link
Contributor

@hratoanina should we just close this?

@hratoanina hratoanina closed this Oct 13, 2025
@github-project-automation github-project-automation bot moved this from Ready for review to Done in Plonky3 Recursion Oct 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants