Skip to content

[program] Re-order confidential mint burn ciphertexts to dest/src, supply, then auditor #173

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

Merged
merged 3 commits into from
Feb 8, 2025

Conversation

samkim-crypto
Copy link
Contributor

@samkim-crypto samkim-crypto commented Feb 7, 2025

Problem

Auditor ciphertexts for the confidential mint and burn amounts were added to instruction data in solana-labs/solana-program-library#7480. In the processor logic for confidential mint and burn, the auditor ciphertext that is included in the instruction data should be checked whether it corresponds to the auditor ciphertext in the proof data.

Currently, the wrong component of the grouped ciphertext is checked. A grouped ElGamal ciphertext for the confidential mint and confidential burn amounts have the second component correspond to the "auditor" component. However, the third component that corresponds to the confidential "supply" is checked for consistency instead.

Summary of Changes

A simple fix is to just update the processor logic to check the consistency of the second component of the ciphertexts. However, as suggested in #128, I swapped the order of the auditor and supply ciphertext components in the proof data as suggested in #128. This way, the auditor ciphertext component is always third in grouped ciphertexts in confidential transfer, confidential transfer with fee, and confidential mint burn extensions, which could prevent confusion.

Fixes #128.

@samkim-crypto samkim-crypto marked this pull request as ready for review February 7, 2025 14:01
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.

Sorry if this is wrong, but shouldn't you then be using the ciphertexts at position 1 in

mint_burn_extension.confidential_supply = ciphertext_arithmetic::subtract_with_lo_hi(
&current_supply,
&proof_context
.burn_amount_ciphertext_lo
.try_extract_ciphertext(2)
.map_err(|_| ProgramError::InvalidAccountData)?,
&proof_context
.burn_amount_ciphertext_hi
.try_extract_ciphertext(2)
.map_err(|_| ProgramError::InvalidAccountData)?,
)
.ok_or(TokenError::CiphertextArithmeticFailed)?;
and
mint_burn_extension.confidential_supply = ciphertext_arithmetic::add_with_lo_hi(
&current_supply,
&proof_context
.mint_amount_ciphertext_lo
.try_extract_ciphertext(2)
.map_err(|_| ProgramError::InvalidAccountData)?,
&proof_context
.mint_amount_ciphertext_hi
.try_extract_ciphertext(2)
.map_err(|_| ProgramError::InvalidAccountData)?,
)
.ok_or(TokenError::CiphertextArithmeticFailed)?;
?

@samkim-crypto
Copy link
Contributor Author

Oh no! I missed this part. Thanks for the catch 🙏 !

I think the problem is that the current tests do not check whether the current supply is updated correctly. I will update the tests to account for this...

@samkim-crypto
Copy link
Contributor Author

I fixed the supply update and I added the relevant checks in the tests!

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 great!

@samkim-crypto samkim-crypto merged commit ca08984 into solana-program:main Feb 8, 2025
19 of 20 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.

[confidential-mint-burn] Wrong auditor ciphertexts (Neodyme L02)
2 participants