refactor: drop legacy FixedPoint impls for NullifierK and ValueCommitV#20
Open
p0mvn wants to merge 7 commits into
Open
refactor: drop legacy FixedPoint impls for NullifierK and ValueCommitV#20p0mvn wants to merge 7 commits into
p0mvn wants to merge 7 commits into
Conversation
…base feat: add SpendAuthG fixed-base multiplication support
Introduces a public associated constant `orchard::value::NoteValue::ZERO`, replacing the crate-private `NoteValue::zero()` associated function. All internal call sites in the builder, bundle testing helpers, and note dummy construction are rewired to use the new constant. The new constant is usable in `const` contexts and gives downstream consumers a direct name for the zero value without going through `NoteValue::from_raw(0)`. Co-Authored-By: Dev Ojha <dojha@berkeley.edu>
value: Replace `NoteValue::zero()` with `NOTE_VALUE_ZERO` const
Release orchard v0.13.1
After zcash#489's enum refactor, base-field and short-scalar fixed-base dispatch routes through `OrchardBaseFieldBases` and `OrchardShortScalarBases`. The standalone `impl FixedPoint` blocks for `NullifierK` and `ValueCommitV` are no longer on the in-circuit dispatch path. This commit: - Removes `impl FixedPoint<pallas::Affine> for NullifierK` and `impl FixedPoint<pallas::Affine> for ValueCommitV`. - Symmetrizes `value_commit_v_short_routes_correctly` to compare against `value_commit_v::*` constants directly, mirroring the existing `nullifier_k_base_field_routes_correctly` test. - Adds a `### Removed` entry to `CHANGELOG.md` documenting the breaking change and the migration path (`OrchardBaseFieldBases::NullifierK` / `OrchardShortScalarBases::ValueCommitV`). The `pub struct NullifierK` / `pub struct ValueCommitV` definitions and their `From` conversions into the new enums are retained, so `NullifierK.into()` / `ValueCommitV.into()` continue to work in caller code. The pinned Orchard verification key is unchanged (`circuit::tests::round_trip` still passes).
The 3-variant enum and its five `From<X>` impls had no readers — `OrchardFixedBases` is only used as a type parameter to `EccChip` / `MerkleConfig` / `SinsemillaConfig`, never as a value. The `.into()` calls in `circuit::gadget` resolve to the per-slot enums. All affected items live behind `unstable-voting-circuits`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ebfull
force-pushed
the
valar/drop-legacy-fixed-point-impls
branch
from
April 27, 2026 18:06
dbacee3 to
53014be
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
3 participants
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 top of zcash/orchard#489 (
adam/spend-auth-g-fixed-base). Addresses TalDerei'snitonsrc/constants/fixed_bases.rs:218("areimpl FixedPoint for NullifierK / ValueCommitVblocks necessary anymore?").Summary
After zcash#489's enum refactor, base-field and short-scalar fixed-base dispatch routes through
OrchardBaseFieldBases/OrchardShortScalarBases. The standaloneimpl FixedPointblocks forNullifierKandValueCommitVare no longer on the in-circuit dispatch path, so this PR removes them.impl FixedPoint<pallas::Affine> for NullifierKandimpl FixedPoint<pallas::Affine> for ValueCommitV(~34 LOC).value_commit_v_short_routes_correctlyto compare againstvalue_commit_v::generator()/value_commit_v::U_SHORT/value_commit_v::Z_SHORTdirectly, mirroringnullifier_k_base_field_routes_correctly. The test no longer relies on the (now removed)FixedPointimpl as its "ground truth"; both regression tests now anchor against the precomputed constants.### Removedentry toCHANGELOG.mdunder[Unreleased]documenting the breaking change and the migration path.The
pub struct NullifierK/pub struct ValueCommitVdefinitions and theirFromconversions into the new enums are retained, soNullifierK.into()andValueCommitV.into()continue to work in caller code.Why stacked on zcash#489 rather than against
mainThis is strictly cleanup of vestiges left behind by the enum refactor in zcash#489. Removal is a breaking change so I wanted to separate it to keep zcash#489 focused.
Verification
Run locally on top of
adam/spend-auth-g-fixed-base:cargo check --features circuitcargo check --no-default-featurescargo check --all-featurescargo clippy --all-features -- -D warningscargo fmt --all --checkcargo test --features circuit --lib constants::fixed_bases::(28 tests, including the symmetrized regression test)cargo test --release --features circuit --lib circuit::tests::round_trip(pinned Orchard verification key unchanged)