[token-2022] Add support for record program for Transfer and TransferWithFee instructions
#7127
Conversation
token/program-2022/src/extension/confidential_transfer/instruction.rs
Outdated
Show resolved
Hide resolved
| ciphertext_validity_proof_data_location | ||
| { | ||
| let proof_instruction_offset: i8 = proof_instruction_offset.into(); | ||
| if proof_instruction_offset != 2 { |
There was a problem hiding this comment.
Since all of the proof instructions are optional, should this start at 1 and then get added for each proof that isn't a context state account? For example, if I use a context state account for the first proof, and then a record account for this one, then the proof instruction would actually be 1, and not 2.
Feel free to do this in follow-up work btw, since it's fine to say that this works all-or-nothing right now.
| equality_proof_data_location | ||
| { | ||
| let proof_instruction_offset: i8 = proof_instruction_offset.into(); | ||
| if proof_instruction_offset != 1 { |
There was a problem hiding this comment.
Same comment over here, but again, it can be done in follow-up work
| let equality_proof_location = Self::confidential_transfer_create_proof_location( | ||
| equality_proof_data.as_ref(), | ||
| equality_proof_account, | ||
| 1, | ||
| ) | ||
| .unwrap(); | ||
| let ciphertext_validity_proof_location = Self::confidential_transfer_create_proof_location( | ||
| ciphertext_validity_proof_data.as_ref(), | ||
| ciphertext_validity_proof_account, | ||
| 2, | ||
| ) | ||
| .unwrap(); | ||
| let range_proof_location = Self::confidential_transfer_create_proof_location( | ||
| range_proof_data.as_ref(), | ||
| range_proof_account, | ||
| 3, | ||
| ) |
There was a problem hiding this comment.
Same here, should the offset only get incremented if it's using a context state account?
| let equality_proof_location = Self::confidential_transfer_create_proof_location( | ||
| equality_proof_data.as_ref(), | ||
| equality_proof_account, | ||
| 1, | ||
| ) | ||
| .unwrap(); | ||
| let transfer_amount_ciphertext_validity_proof_location = | ||
| Self::confidential_transfer_create_proof_location( | ||
| transfer_amount_ciphertext_validity_proof_data.as_ref(), | ||
| transfer_amount_ciphertext_validity_proof_account, | ||
| 2, | ||
| ) | ||
| .unwrap(); | ||
| let fee_sigma_proof_location = Self::confidential_transfer_create_proof_location( | ||
| fee_sigma_proof_data.as_ref(), | ||
| fee_sigma_proof_account, | ||
| 3, | ||
| ) | ||
| .unwrap(); | ||
| let fee_ciphertext_validity_proof_location = | ||
| Self::confidential_transfer_create_proof_location( | ||
| fee_ciphertext_validity_proof_data.as_ref(), | ||
| fee_ciphertext_validity_proof_account, | ||
| 4, | ||
| ) | ||
| .unwrap(); | ||
| let range_proof_location = Self::confidential_transfer_create_proof_location( | ||
| range_proof_data.as_ref(), | ||
| range_proof_account, | ||
| 5, | ||
| ) | ||
| .unwrap(); |
There was a problem hiding this comment.
Same with these -- only increment the offset if needed?
…tion.rs Co-authored-by: Jon C <me@jonc.dev>
mergify
bot
dismissed
joncinque’s stale review
Pull request has been modified.
|
Thanks for the review! I'll address the increment on a follow-up PR as you suggested 🙏 |
Problem
zk-sdkdoes not have support for theVerifyTransferandVerifyTransferWithFeeproofs any moreSummary of Changes
3dc64b0: I first realized that there are a lot of repetitive logic in the
verify_*_prooffunctions, so I refactored the logic into averify_and_extract_contextfunction. In the end, this turned out quite nicely with the use of generics and traits.c4cbfa9: I made each of the individual proofs in the
TransferWithSplitProofsinstruction to be read either from instruction data, record account, or context state account. Previously, the proofs could only be read from context accounts. In the process (after a bit of wrestling), I ended up removing the custom logic for parallel execution and auto-close. These custom logic does not really make much sense any more with the added flexibility of instruction data and record account and were over-complicating the logic. In this commit, I made some changes in the verification function lifetimes, but these changes are undone in the next commit 🙏 .e7a08d7: Since
zk-sdkdoes not supportVerifyTransferandVerifyTransferWithFeeany more, the original confidentialTransferinstruction is to be removed. Instead, now I made the originalTransferinstruction theTransferWithSplitProofwithout fee (but still with the nameTransfer) and theTransferWithSplitProofinstruction to beTransferWithFeeinstruction now that these two instructions require different instruction data. In the process, I updated the transfer amount ciphertext validity proof in transfer instructions to beBatchedGroupedCiphertext3HandlesValidityProofinstead of for 2 handles.ca8b1ad: This is a smaller commit. I ended up just removing
verify_*_prooffunctions and usingverify_and_extract_contextfunction directly.46a180b: I updated the client according to the new updates in the program interface. Since the parallel execution logics were removed, I ended up removing the relevant functions in the client.
4c8c53b: Updated the tests to test for all three possible ways to include zk proofs in the instruction (instruction data, record account, context state account).
b9343d9: Updated token-cli for the newly changes interface in the token-client. Right now, the confidential transfer logic uses context state accounts to include proofs. I'll update these to use record program instead in a follow-up PR.