Repository navigation
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The changelog incorrectly presents existing behavior as a newly fixed bug.
1 open finding
What changed in this PR
Adds regression coverage confirming identity sender keys are unsatisfiable in the transaction circuit.
Changes:
- Tests identity values for both sender key components.
- Documents reliance on Schnorr’s non-identity constraint.
- Updates test-helper lint handling and changelog.
| File | Description |
|---|---|
circuits/tests/sender_key.rs |
Adds the identity-key regression test. |
circuits/tests/common/mod.rs |
Allows test-specific unused helper code. |
circuits/src/sender_enc.rs |
Documents sender-key constraints. |
circuits/CHANGELOG.md |
Adds an inaccurate behavioral fix entry. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
HDauven
force-pushed
the
phoenix/identity-sender-key
branch
from
October 10, 2026 23:13
3d38aff to
205c900
Compare
HDauven
changed the title
Member
Author
|
Force-pushed: added a commit that constrains the output note keys to differ from the identity, and moved the sender-key changelog entry to #320. The verifier-data assets and gate counts are updated. |
HDauven
force-pushed
the
phoenix/identity-sender-key
branch
from
October 11, 2026 01:32
205c900 to
4440cce
Compare
Member
Author
|
Force-pushed: rebased on the amended #320, with the verifier data regenerated. |
HDauven
marked this pull request as ready for review
October 11, 2026 02:02
moCello
approved these changes
Oct 11, 2026
moCello
left a comment
moCello
left a comment
Member
There was a problem hiding this comment.
APPROVE — The new constraint is sound: the type requires a subgroup point, in that subgroup only the identity has x = 0, and the inverse gate uses the same idiom as dusk-plonk's own truncation gadget. The regressions isolate the constraint they test. One small naming note below asks for nothing.
A transaction whose sender public key has an identity `A` or `B` is unsatisfiable, since the signature gadgets of jubjub-schnorr 0.7.0, the version both crates require, reject identity keys. The circuit needs no constraint of its own for it. Document that the sender keys rely on it, and pin it with a regression: a transaction with an identity `A`, then `B`, signed with a signature that satisfies the signature equation for that key, must be unsatisfiable, while the same construction proves for the honest key.
The transaction circuit encrypts the sender public key to the note public key of each output, a public input it constrains to the prime-order subgroup. That subgroup contains the identity, which is no ElGamal key, and only the signature gadgets constrained their own keys to differ from it. Constrain each output note key to differ from the identity before the sender encryption, as the signature gadgets of jubjub-schnorr do for their keys: the prover supplies the inverse of the `x` coordinate, which only the identity lacks in the subgroup, one gate per key. The regression gives the first output, then the second, an identity note key, with the sender encrypted to it as the circuit computes it, and checks that both transactions are unsatisfiable while the honest one proves. Without the constraint, both prove. The circuits grow by two gates, which changes the verifier data of every circuit, so the test assets are regenerated.
HDauven
force-pushed
the
phoenix/identity-sender-key
branch
from
October 11, 2026 17:43
4440cce to
7843e47
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #321.
AorBis the identity. This adds a regression test for both keys and documents the dependency where the sender keys enter the circuit.The circuits change, so the verifier-data test assets are updated.
Resolves #314