Skip to content

Conversation

@tcoratger
Copy link
Collaborator

Should be useful is general to simplify complex calculations and especially in #160 I think.

Copy link
Contributor

@LindaGuiga LindaGuiga left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@github-project-automation github-project-automation bot moved this from Todo to Ready to be merged in Plonky3 Recursion Oct 30, 2025
@tcoratger
Copy link
Collaborator Author

@sai-deng As discussed here #160 (comment) I've tried to improve a bit the quotient helpers.

  • I've added a rich documentation to be sure everyone reading the code understands the logic here as the quotient polynomial in the circuit world is quite tricky.
  • Let me know what you think about this.

@sai-deng
Copy link
Contributor

sai-deng commented Nov 4, 2025

This may not be related to this PR, but when I change a random line
let one = circuit.add_const(SC::Challenge::ONE);
to
let one = circuit.add_const(SC::Challenge::ZERO);
all tests still pass.
Shouldn’t test_fibonacci_verifier catch this?

@LindaGuiga
Copy link
Contributor

LindaGuiga commented Nov 4, 2025

This may not be related to this PR, but when I change a random line let one = circuit.add_const(SC::Challenge::ONE); to let one = circuit.add_const(SC::Challenge::ZERO); all tests still pass. Shouldn’t test_fibonacci_verifier catch this?

test_fibonacci_verifier doesn't get there because there is only 1 quotient chunk (log_quotient_degree = 1). Before the reorganization of the crates (commit b20ee94), there was a second test test_mul_verifier_circuit which catches the issue. So I think we should put that one back.

@Nashtare Nashtare merged commit fe5c5ee into Plonky3:main Nov 6, 2025
14 checks passed
@github-project-automation github-project-automation bot moved this from Ready to be merged to Done in Plonky3 Recursion Nov 6, 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.

4 participants