feat: add RecursiveChallenger for in-circuit FS challenge sampling
#143
Conversation
Nashtare
force-pushed
the
robin/recursive_challenger
branch
from
bfdde19 to
b3b5eae
Compare
hratoanina
left a comment
hratoanina
left a comment
There was a problem hiding this comment.
Looks good, thanks!
You don't have to address the code comments, if I need it I'll just change it in the Poseidon sponge table PR.
| fn test_hash_absorb_squeeze_sequence() { | ||
| let mut circuit = CircuitBuilder::<BabyBear>::new(); | ||
| circuit.enable_op( | ||
| NonPrimitiveOpType::HashAbsorb { reset: true }, |
There was a problem hiding this comment.
Does it need you would need to potentially separately allow with reset = true and reset = false?
| } | ||
|
|
||
| /// Flush the absorb buffer, performing the actual hash absorb operation. | ||
| fn flush_absorb<F: Field>(&mut self, circuit: &mut CircuitBuilder<F>) { |
There was a problem hiding this comment.
I'm trying to think of a case where you would need to absorb outside of for sampling, or if we can just put the logic directly into sample.
| self.buffer_flushed = false; | ||
| } | ||
|
|
||
| fn sample(&mut self, circuit: &mut CircuitBuilder<F>) -> Target { |
There was a problem hiding this comment.
For easier compatibility with the table, it would be better if add_hash_squeeze were called only with vecs of RATE elements.
It could be done either by maintaining an output vector, or by always returning RATE elements when calling sample. I'd prefer the former.
github-project-automation
bot
moved this from Ready for review
to Ready to be merged
in Plonky3 Recursion
|
@hratoanina should I just merge as is then? |
|
@Nashtare Yeah, I'm fine with merging it right now. |
github-project-automation
bot
moved this from Ready to be merged
to Done
in Plonky3 Recursion
builds on top of #141
Part of some more global refactoring around
PublicInputlogic andChallengesgeneration.Attempt at proposing a
RecursiveChallengertrait along with a concrete impl throughCircuitChallengerto observe values and sample challenges. There remains to actually have hashing constraints (via fixed Poseidon2 or generic).There are a couple tweaks / cleanups / lighter refactoring that I'd still like to do but decoupling the 3 PRs got a bit messy so I may hold it off for now (dropped some TODOs inline)