Bump zk-sdk to v5.0#10000
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #10000 +/- ##
=======================================
Coverage 82.5% 82.5%
=======================================
Files 844 844
Lines 316727 316727
=======================================
+ Hits 261546 261564 +18
+ Misses 55181 55163 -18 🚀 New features to boost your workflow:
|
| "solana-program-option", | ||
| "solana-pubkey 3.0.0", | ||
| "solana-zk-sdk", | ||
| "solana-zk-sdk 4.0.0", |
There was a problem hiding this comment.
Is it problematic that this dep (and a couple others) are still depending on v4.0 ?
There was a problem hiding this comment.
Yeah it is mainly token-2022-interface and the related crates. It would have been cleaner if we bumped zk-sdk in these crates first and then bump in agave.
I think it is still fine since the breaking changes in zk-sdk v5.0 only involve proof components. The agave crates that depend on token-2022-interface and its related crates do not use the proof components at all (they just other components like encryption, public keys, etc.).
| let incorrect_pubkey = elgamal_keypair.pubkey(); | ||
| let incorrect_secret = ElGamalSecretKey::new_rand(); | ||
| let incorrect_keypair = ElGamalKeypair::new_for_tests(*incorrect_pubkey, incorrect_secret); | ||
|
|
||
| let fail_proof_data = | ||
| ZeroCiphertextProofData::new(&incorrect_keypair, &zero_ciphertext).unwrap(); | ||
| let mut fail_proof_context = success_proof_data.context; | ||
| fail_proof_context.pubkey = ElGamalPubkey::default().into(); | ||
| let fail_proof_data = ZeroCiphertextProofData { | ||
| context: fail_proof_context, | ||
| proof: success_proof_data.proof, | ||
| }; |
There was a problem hiding this comment.
Forgive the possibly naive question as I'm not super familiar with this code - It looks like the old code generated more "incorrect" stuff. Namely, the secret and keypair.
The new code looks to just be sticking a bad pubkey in with the otherwise "good" fields form success_proof_data. Does this impact anything or is the change inconsequential ?
There was a problem hiding this comment.
Right, in older versions, when we provided "incorrect" inputs to the proof constructors, it still went through with the proof generation logic to produce some garbage output.
This can be misused, so the auditors recommended that when an "incorrect" input is provided to a constructor, it just terminates early and return an error instead.
The zk-elgamal-proof program just does verification, not proof verification, so this change in proof generation doesn't impact the program at all. The only issue is that it makes it slightly more tedious to make invalid proofs for tests. We have to now manually tweak things to generate invalid proofs to test that the zk-elgamal-proof program indeed fails on invalid proofs.
t-nelson
left a comment
t-nelson
left a comment
There was a problem hiding this comment.
odd that the sdk made a major bump, but there are no api changes here
|
Thanks for the review! Yeah the zk-sdk only uses |
anza-xyz/agave#10000 Fixes all the zksdk failures in the instruction harness
4 participants
Problem
The zk-sdk went through a couple of audits. Nothing major in terms of security has been found, but there have been a number of additional sanity checks that have been added to v5.0.0.
You can refer to the release notes, but most of the changes are non-breaking internal changes or proof generation, which doesn't affect proof verification in agave. The only breaking changes are the following:
ProofDataErrorsolana-program/zk-elgamal-proof#216Currently, the only place in the repo that depends on the
solana-zk-sdkis the zk-elgamal proof program, which is currently DISABLED.Summary of Changes
solana-zk-sdkto v5.0solana-zk-sdk, if it took in invalid inputs, it just produced invalid proofs. We used this to generate wrong proof instances in the zk-elgamal-proof-tests. However, now with v5.0, these invalid proofs are outright rejected by the constructor. So I updated the proof logic to manually generate invalid proofs for tests.reenable-zk-elgamal-proof-programfeature gate.Fixes #