Store cv and rk in their compressed encodings - #194
Draft
elicbarbieri wants to merge 1 commit into
Draft
Conversation
Only verification needs the points. - Parse: 2 sqrt -> 0 per Spend, 1 -> 0 per Output; serialize: 1 inversion -> 0 per `cv` - `SpendDescription` 288 B smaller, `OutputDescription` 128 B - `check_spend`/`check_output` take every field as wire bytes and own every rule (`epk` and `zkproof` encodings were the caller's) - New `primitives`: `InvalidPoint` + `decompress_not_small_order`, one per cv/rk/epk - Removed: `ValueCommitment::from_bytes_not_small_order` Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
cvcvSame shape as the companion orchard PR. Sapling bundle costs 2 sqrt per Spend and 1 per Output to read off disk & parse, plus one inversion per
cvto write back to the wire. The peer receiving them does not use the decompressed pointsThis PR brings parse/serialize down to 0 elliptic curve arithmetic, and moves to verify path.
Curve operations, per description
cvdecompress (sqrt)check_spend/check_outputrkdecompress (sqrt)check_spend->check_spendrkdecompress in the reddsa batchbatch::Verifier::verify, unchangedepkdecompress (sqrt)check_outputcv-> affine, for the circuitcv.as_inner().to_affine()epk-> affine, for the circuitepk.to_affine(), Output onlyThis also removes 288 bytes per Spend (160 for
rk, 128 forcv) and 128 bytes per Output. A JubjubExtendedPointcarries five field elements (U, V, Z, T1, T2) so that group arithmetic is fast during verification. Not worth it for types sent over the wire then storedSpendDescriptionV5,OutputDescriptionandOutputDescriptionV5change the same way.ValueCommitmentitself is untouchedAPI
Validation Rules
cvcanonical encodingcheck_spend/check_outputcvnot small ordercheck_spend/check_outputrkcanonical encodingcheck_spendrknot small ordercheck_spendepknot small ordercheck_outputepkcanonical encodingcheck_outputzkproofcanonical lengthcheck_spend/check_outputVerification context
Building the circuit public inputs
The batch validator
Peer-triggerable Batch verification Errors exist via small-order
rkon the second Spend after the first Spend's items are queued. This PR does widen the trigger set to include a non-canonicalcv/rk, which previously could not get past the parser.The companion orchard PR closes its equivalent by hoisting all fallible work above every Bundle::push, which it could do cheaply because it had to build its instances up front anyway via Action::to_instance. Here there is no Action -> Instance operation. This uses the check_spend for those same operations, and consumers should think about the batch poisioning attacks. Beyond my depth of knowledge...
Downstream changes
Zebra
zebra-consensusshould require no changes — it reaches Sapling verification only throughcheck_bundle, and that signature is unchanged:zebra-chainandzebra-rpcwill need to update because they wrap the Sapling value-commitment in their own typeI dont see any arithmetic anywhere w/ the Zebra
ValueCommitmenttype, so this should just be a simple change that avoids de-compressing the point when reading from disk, and re-compressing when sending over wire. Zaino and some light-clients will likely all need to update as well to the ValueCommitmentBytes, but that is a small fix for a big improvementBehaviour change
cv, and a non-canonicalrk, are rejected at verification rather than at parse. Parse-then-verify is unaffected; parsing alone now accepts encodings it used to reject. A consumer relying on parsing alone must now verify.check_bundle's documented partial-contribution behaviour is now reachable via a malformedcv/rkinstead of just the existing small-orderrkruleSaplingVerificationContext::{check_spend, check_output}take four fields in different types. Callers pass the description's own accessors directly and delete their decompression andProof::readpreamble.