Repository navigation
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The validation is correctly implemented, documented, and covered across relevant decoding paths.
0 open findings
What changed in this PR
Strengthens Phoenix public-key decoding by rejecting identity points and documents/tests strict versus legacy-compatible decoding.
Changes:
- Rejects identity components in
PublicKey::from_bytes. - Adds identity and legacy-decoder regression tests.
- Updates decoder documentation and changelog.
| File | Description |
|---|---|
core/src/keys/public.rs |
Validates decoded public-key points. |
core/src/stealth_address.rs |
Clarifies checked-decoder guarantees. |
core/tests/keys.rs |
Tests identity-point rejection. |
core/tests/legacy_decode.rs |
Tests strict and legacy-compatible decoding. |
core/CHANGELOG.md |
Records the decoding fix. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
7357001 to
493b42e
Compare
|
Force-pushed: rebased on the amended #320. No changes. |
493b42e to
f3280db
Compare
|
Force-pushed: rebased on the amended #320. No changes. |
moCello
left a comment
moCello
left a comment
There was a problem hiding this comment.
APPROVE — the identity check on PublicKey::from_bytes is correct, the serde path inherits it, and the new tests pin both decoder paths; two small notes below ask for nothing.
- The sources disagree on the legacy decoders: #287 asks the tests to assert that strict and legacy-compatible decoding are equivalent and that the legacy path is dormant, and the PR description and tests state that the two paths diverge since dusk-jubjub 0.15.2.
Add the coverage #287 asks for: `Sender::from_bytes_legacy_compat`, which no test exercised, and strict against legacy-compatible decoding of the bytes of honestly created notes and transaction skeletons, next to the existing `StealthAddress` cases. #287 expects the two decoders to be equivalent, with the legacy path dormant until a dependency diverges. That has not held since dusk-jubjub 0.15.2, whose point decoder, behind the strict decoders, rejects a point with a small-order component. The tests pin both halves: the decoders agree on honest bytes, whose points are torsion-free, and the legacy-compatible ones keep a small-order component in any point of a note byte for byte, where the strict ones reject it. Both reject an unknown sender tag and an invalid point encoding.
f3280db to
b2f546f
Compare
|
#287 now states the divergence the tests pin: since dusk-jubjub 0.15.2 the two paths agree on honest bytes, and the legacy-compatible decoders keep points the strict ones reject. |
Stacked on #320.
PublicKey::from_bytesrejects anAorBthat is the identity, as the stealth-address decoder already does for its points.StealthAddress::from_bytes_checkeddocs now name only the checks it performs.Sender::from_bytes_legacy_compat. Since dusk-jubjub 0.15.2 the two paths are no longer equivalent. They agree on honest bytes, and the legacy-compatible decoders keep points that the strict ones reject; the tests pin both.Resolves #312
Resolves #287