Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[program-2022] Add auditor ciphertexts to instruction data #7480

Merged
merged 12 commits into from
Nov 15, 2024

Conversation

samkim-crypto
Copy link
Contributor

@samkim-crypto samkim-crypto commented Nov 11, 2024

Problem

Currently, confidential transfer, mint and burn instructions do not include the transfer amount ciphertext as part of the token instruction data. This ciphertext is included as part of the proof that is either included in the Zk ElGamal proof program instruction, record account, or context state account.

From the perspective of a real world auditor who must constantly be monitoring activities in a particular mint to decrypt transactions, it must access the ciphertexts indirectly by first looking up the token instruction data and then if proof accounts are used, look up the corresponding record account or context state account, parse these accounts, and then get the transfer amount ciphetext. It would make life much simpler for the auditor if the token instruction data simply includes the auditor part of the ciphertext.

Summary of Changes

Added the auditor ciphertext to the confidential transfer, mint, and burn instruction data. I think the program changes should be straightforward. The only tricky part is coming up with the most logical interface on the client side. I updated the transfer, mint, and burn token-client functions to take in the auditor ciphertext as input, but I am open to suggestions on making the client functions a bit simpler.

@samkim-crypto samkim-crypto marked this pull request as ready for review November 12, 2024 20:26
Copy link
Contributor

@joncinque joncinque left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the guts look great! Just a nit on the API for token-client and a nit on converting into TokenError

token/client/src/token.rs Outdated Show resolved Hide resolved
token/client/src/token.rs Outdated Show resolved Hide resolved
token/client/src/token.rs Outdated Show resolved Hide resolved
token/client/src/token.rs Outdated Show resolved Hide resolved
joncinque
joncinque previously approved these changes Nov 14, 2024
Copy link
Contributor

@joncinque joncinque left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks really good! Just one last nit, up to you

/// type is a convenience type that contains the proof data and the extracted
/// ciphertexts.
#[derive(Clone, Copy)]
pub struct CiphertextValidityProofWithCiphertext {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: maybe make this totally clear by specifying the auditor bit?

Suggested change
pub struct CiphertextValidityProofWithCiphertext {
pub struct CiphertextValidityProofWithAuditorCiphertext {

@mergify mergify bot dismissed joncinque’s stale review November 15, 2024 00:19

Pull request has been modified.

@samkim-crypto samkim-crypto merged commit 45b214f into solana-labs:master Nov 15, 2024
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants